feat(share): custom /l/:alias pretty links (share_aliases table) (#205) #214
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: vvzvlad/gitmost#214
Reference in New Issue
Block a user
Delete Branch "feat/205-share-aliases"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Closes #205
Custom human-readable vanity links
/l/<alias>for public shares, parallel to the untouched/share/...routes.Server
share_aliasestable (migration20260626T130000-share-aliases): workspace-scoped,UNIQUE(workspace_id, alias),page_idnullableON DELETE SET NULL(address outlives its target), index onpage_id. Entity types +db.d.tsupdated.ShareAliasRepo(findByAliasAndWorkspace / findByPageId / findById / insert / updatePageId / delete, all workspace-scoped).ShareAliasService: set (insert / no-op / 409 reassign guard with current target / unique-race -> 409), remove, availability probe, andresolveReadableTargetthat re-runs the single existing share boundary (resolveReadableSharePage+isSharingAllowed).ShareAliasRedirectController@Controller(l)GET /l/:alias-> 302 (never 301) to/share/:key/p/:slug; unknown/dangling/no-longer-readable aliases serve the SPA index with no existence leak.l/:aliasadded to the global/apiprefix exclude in main.ts.ShareAliasController@Controller(share-aliases): set/remove/availability/for-page (JWT-guarded, page edit/view checks).normalizeShareAlias/isValidShareAliasutil./share/...left untouched.Client
Tests (unit only)
tsc --noEmitgreen, clienttsc -bgreen, new jest (32) + vitest (4) green.🤖 Generated with Claude Code
Code review — PR #214: кастомные pretty-ссылки
/l/:aliasчерез таблицуshare_aliases(#205)Вердикт: Request changes. Логика корректна и стабильна (security/stability/regressions без блокеров), но две новейшие и самые чувствительные поверхности — публичный редирект-контроллер
/l/:aliasи authz-гейты alias-контроллера — уходят в merge без единого теста, при том что у структурно идентичногоShareSeoControllerтакой spec есть. Плюс мёртвый код (неиспользуемыеLoggerиPagePermissionRepo), отсутствие i18n-регистрации новых строк и записи в CHANGELOG. Must-fix: тесты на оба контроллера, удаление dead-code, регистрация строк в каталогах локалей.Объём: дифф
develop…feat/205-share-aliases(merge-base3ddc329b), 23 файлов, +1660/−4. Прогнаны параллельные аспектные ревьюеры (security, stability, conventions, documentation, regressions, test-coverage, simplification, architecture) + judge-проход.Must fix before merge
[test-coverage] Добавить routing/leak-spec для публичного редиректа
/l/:alias—apps/server/src/core/share/share-alias-redirect.controller.tsЭто самая security-чувствительная поверхность PR: неаутентифицированный публичный роут, который обязан отдавать обычный SPA-index на неизвестный ИЛИ нечитаемый алиас, чтобы существование имени не утекало. Ни одна ветка не покрыта (на ветке добавлены только service/util/migration/repo-specs). Параллельный
ShareSeoControllerровно для этого имеетshare-seo.controller.routing.spec.ts. Fix: добавитьshare-alias-redirect.controller.spec.tsпо образцу seo-spec и проверить: (a) резолвимый алиас →res.redirect('/share/<key>/p/<title-slug>-<slugId>', 302); (b) неизвестный/висячий/нечитаемый алиас иworkspace === null→ стрим index без 302; (c) битый percent-encoding декодируется защитно и трактуется как неизвестный; (d) self-hosted идёт черезfindFirst, subdomain — черезfindByHostname.[test-coverage] Добавить spec на authz-гейты alias-контроллера —
apps/server/src/core/share/share-alias.controller.ts:53-134Решения о доступе при создании/ретаргете/удалении алиаса живут именно в контроллере (service-spec явно делегирует авторизацию вызывающему), но ни один гейт не протестирован. Непокрыты: cross-workspace/несуществующая страница →
NotFoundException;validateCanEdit;resolveReadableSharePagenull →BadRequestException('Page is not publicly shared');isSharingAllowedfalse →ForbiddenException;remove()висячего алиаса (pageId null) пропускаетvalidateCanEditи всё равно удаляет;for-pageзовётvalidateCanView. Fix: добавитьshare-alias.controller.spec.tsс мокнутымиPageRepo/ShareService/ShareAliasService/PageAccessService, проверяющий каждый гейт и happy-path делегацию вsetAlias.[conventions] Удалить неиспользуемые
Loggerimport и поле вShareAliasRedirectController—apps/server/src/core/share/share-alias-redirect.controller.ts:1,27Подтверждено:
this.loggerне вызывается ни разу (0 usages) — скопировано из SEO-контроллера, где логгер реально нужен. Change-introduced dead code. Fix: убратьLoggerиз импорта@nestjs/commonи удалить строкуprivate readonly logger = new Logger(...).[conventions] Удалить неиспользуемые
PagePermissionRepoimport и зависимость вShareAliasController—apps/server/src/core/share/share-alias.controller.ts:17,40Подтверждено:
this.pagePermissionRepoне используется (0 usages), авторизация идёт целиком черезpageAccessService. Change-introduced dead code. Fix: удалить строкуimport { PagePermissionRepo } ...и параметр конструктораprivate readonly pagePermissionRepo: PagePermissionRepo,.[documentation] Зарегистрировать новые share-alias строки в каталогах локалей en-US и ru-RU —
apps/client/src/features/share/components/share-alias-section.tsx:134-238,apps/client/src/features/share/queries/share-query.ts:332,357Политика i18n в
apps/client/src/i18n.tsгласит, что en-US — source of truth, а en-US и ru-RU полностью поддерживаются, чтобы UI не рендерил mixed-language. PR вводит строки черезt()(«Custom address», «A short, memorable link…», «Use 2-60 lowercase letters, digits and hyphens», «This address is already in use», «Move custom address?», «Move here», два интерполируемых reassign-предложения, тосты «Failed to set/remove custom address»), но не трогает ни одинtranslation.json(подтверждено по--stat). Русский пользователь получит этот блок на английском. Fix: добавить новые ключи вen-US/translation.json(текст = ключ = значение) и их переводы вru-RU/translation.json.[documentation] Добавить запись
[Unreleased]/Addedв CHANGELOG для/l/:alias—CHANGELOG.mdCHANGELOG ведётся по Keep-a-Changelog, у каждой свежей фичи есть запись с номером issue, «changelog» — трекаемый review-item в истории develop. PR поставляет заметную публичную возможность (ретаргетируемые vanity-ссылки, новая таблица, публичный роут), но CHANGELOG.md не тронут (подтверждено). Fix: добавить bullet под
## [Unreleased]с описанием/l/:alias(workspace-scoped retargetable alias, 302 на каноническую share-страницу, ON DELETE SET NULL) и ссылкой (#205).[security][suggestion] Закрыть раскрытие
currentPageTitle/currentPageIdв 409-ответе reassign проверкой текущей читаемости старой цели —apps/server/src/core/share/share-alias.service.ts:88-99В ветке
!confirmReassign(code: 'ALIAS_REASSIGN_REQUIRED') возвращаютсяexisting.pageIdиcurrentPage?.titleСУЩЕСТВУЮЩЕЙ цели алиаса без проверки, что вызывающий вправе её видеть; контроллер авторизует только НОВУЮ целевую страницу. Утечка ограничена ранее-публичными страницами и только title+id (не контент), но если такую страницу позже расшарить отменили, любой участник workspace, угадав имя алиаса, узнаёт её (уже непубличный) заголовок. Fix: перед возвратом перепроверять, что старая цель всё ещё публично читаема (resolveReadableSharePage+isSharingAllowedпоexisting.pageId), и опускать title/id иначе — либо вовсе убратьcurrentPageTitleи подтверждать по имени.[simplification][suggestion] Убрать тавтологичные runtime-проверки в тесте «entity types expose the alias columns» —
apps/server/src/database/migrations/share-aliases.migration.spec.ts:1574-1594Тест присваивает литералы типизированным объектам и тут же
expect(row.alias).toBe('foo')и т.п. — эти проверки не могут упасть, реальную ценность даёт только compile-time проверкаtsc. Fix: удалить блокit(...)или оставить только типизированные объявления без трёхexpect.[simplification][suggestion] Убрать boilerplate-тест «exports up and down functions» —
apps/server/src/database/migrations/share-aliases.migration.spec.ts:1512-1515expect(typeof migration.up).toBe('function')дублируется следующим тестом, который реально вызываетmigration.up(...)/down(...). Соседние createTable/createIndex/dropTable-проверки несут сигнал (пинят имена таблиц/индексов) — их оставить. Fix: удалить толькоit('exports up and down functions', ...).Test coverage
Новая бизнес-логика: сервис, util, миграция и repo покрыты (
share-alias.service.spec.ts,share-alias.util.spec.ts,share-aliases.migration.spec.ts,share-alias.repo.spec.ts). Без тестов остаются два контроллера, и это самые чувствительные поверхности PR — обе вынесены в Must fix выше:/l/:alias(share-alias-redirect.controller.ts) — no-leak гарантия и решение 302-vs-index не верифицированы;share-alias.controller.ts— регрессия, убирающая любую из проверок доступа, не будет поймана.Architecture & design (forward-looking, non-blocking)
1. Дублирование публичных excluded-prefix контроллеров (
ShareSeoController+ новыйShareAliasRedirectController). Новый контроллер построчно повторяет три вещи из SEO-контроллера: workaround резолва workspace (isSelfHosted()vs subdomain — нужен, т.к. NestJS не применяет middleware на excluded-путях), построениеclientDistPathиsendIndex()-фолбэк. Копии уже разошлись: новый укрепил subdomain-ветку черезheader?.split('.')[0]и добавил 404 при отсутствии index.html, а SEO-контроллер всё ещё делаетheader.split('.')без guard. Не дефект здесь, но риск forward-looking: расхождение именно на этих публичных роутах — это enumeration/leak-риск.вынести только резолв workspace в маленький чистый helper (
resolvePublicWorkspace(req, env, workspaceRepo)),sendIndexоставить.** Effort: s. Pros: убирает самый рисковый дубль (tenant-resolution, гейтящий leak-гарантию) минимальным изменением, без base-class связывания, попутно переводит небезопасныйheader.splitSEO-контроллера на общий путь. Cons:sendIndex/clientDistPathостаются продублированы (ниже риск).2. Дублирование построения канонического page-slug (
buildPageSlug+ существующие call-sites).buildPageSlug(share-alias-redirect.controller.ts:94-95,slugify(title?.substring(0,70) || 'untitled')-${slugId}) — четвёртая копия правила<title-slug>-<slugId>(есть вimport-formatter.ts:385,export.service.ts:505, клиентскийpage.utils.ts). Копии не идентичны: клиентский вариант передаётslugifycustomReplacements (♥,🦄), три серверных — нет; комментарий «mirrors the client» для таких заголовков неверен. Не баг: канонический роут резолвит страницу по trailingslugId, так что расхождение префикса косметическое (URL-эстетика/canonicalization).оставить новую копию локальной, но выровнять её slugify-опции с клиентом (добавить те же customReplacements).** Effort: s. Pros: наименьшее изменение, комментарий становится буквально верным, без cross-module churn. Cons: всё ещё четыре копии, чинит только drift новой.
Code review (re-review) — PR #214: custom /l/:alias pretty-links для шаринга (share_aliases)
Вердикт: Approve with comments. Все 6 прошлых блокеров/замечаний закрыты по существу; дельта добавляет только тесты и косметику, новых blocking-находок нет. Остаётся одна непокрытая ветка (70-символьный клэмп слага) — non-blocking.
Ре-ревью дельты
fdeede00..1043fe3b(8 файлов, +527/−16). Аспекты: security, stability, conventions, documentation, regressions, test-coverage (параллельные ревьюеры + judge).Must fix before merge
buildPageSlug—share-alias-redirect.controller.spec.ts:95-119Докстринг спеки (стр. 4-7) явно заявляет проверку «70-char clamp», но ни один тест её не драйвит: единственный titled-кейс —Quarterly Report(16 симв.), плюсuntitled-fallback. Веткаtitle?.substring(0, 70)не исполняется, регрессия клэмпа не будет поймана, а докстринг переоценивает покрытие. Fix: добавить кейс сtitleдлиннее 70 символов и проверить, что slug в302-таргете отражает только первые 70 символов (усечённый title-slug + slugId).