arch/public-share: единый ShareService.resolveReadableSharePage вместо трёх копий резолва (shareId,pageId)->страница #92

New Issue

Ghost · 2026-06-21T02:33:15+03:00

Ghost commented

2026-06-21 02:33:15 +03:00

Найдено в multi-aspect code review (грань: architecture, forward-looking, не блокирует мерж).

Область: apps/server/src/core/share (ShareService) + apps/server/src/core/ai-chat (public-share-chat.controller.ts, tools/public-share-chat-tools.service.ts)

Наблюдение
Граница безопасности фичи — «резолвится ли пара (shareId, pageId) в пригодную, не-ограниченную страницу внутри ЭТОЙ шары» — выписана как async-последовательность в 3 местах, которые должны быть байт-в-байт идентичны (getShareForPage → share.id===shareId → findById → deletedAt → hasRestrictedAncestor).

Значимость
Forward-looking, не текущий баг: три пути сейчас согласованы, инструменты перепроверяют scope сервер-сайд. Но расхождение контроллер/инструмент может тихо открыть доступ.

Опции

Option 1 (medium): один async-шов ShareService.resolveReadableSharePage(shareId, pageId, workspaceId): {share, page} | null (null при любом сбое); getSharePage и funnel контроллера зовут его; deriveShareAccess схлопывается до «не-null ⇒ pageInShare». Pros: ядровой инвариант в одном месте, эквивалент уже есть в getSharedPage. Cons: правка нескольких call-site.
Option 2 (small): оставить 3 инлайна, но покрыть спекой restricted-descendant + unresolvable + cross-share-id для ОБОИХ путей, чтобы расхождение валило тест. Pros: дёшево. Cons: дубликат остаётся.

Рекомендация
Склоняюсь к Option 1 — ядровой security-инвариант, эквивалентный резолв уже существует в getSharedPage.

Найдено в multi-aspect code review (грань: architecture, forward-looking, не блокирует мерж). **Область:** apps/server/src/core/share (ShareService) + apps/server/src/core/ai-chat (public-share-chat.controller.ts, tools/public-share-chat-tools.service.ts) **Наблюдение** Граница безопасности фичи — «резолвится ли пара (shareId, pageId) в пригодную, не-ограниченную страницу внутри ЭТОЙ шары» — выписана как async-последовательность в 3 местах, которые должны быть байт-в-байт идентичны (getShareForPage → share.id===shareId → findById → deletedAt → hasRestrictedAncestor). **Значимость** Forward-looking, не текущий баг: три пути сейчас согласованы, инструменты перепроверяют scope сервер-сайд. Но расхождение контроллер/инструмент может тихо открыть доступ. **Опции** - *Option 1 (medium)*: один async-шов `ShareService.resolveReadableSharePage(shareId, pageId, workspaceId): {share, page} | null` (null при любом сбое); getSharePage и funnel контроллера зовут его; deriveShareAccess схлопывается до «не-null ⇒ pageInShare». Pros: ядровой инвариант в одном месте, эквивалент уже есть в getSharedPage. Cons: правка нескольких call-site. - *Option 2 (small)*: оставить 3 инлайна, но покрыть спекой restricted-descendant + unresolvable + cross-share-id для ОБОИХ путей, чтобы расхождение валило тест. Pros: дёшево. Cons: дубликат остаётся. **Рекомендация** Склоняюсь к Option 1 — ядровой security-инвариант, эквивалентный резолв уже существует в getSharedPage.

Ghost referenced this issue from a commit

2026-06-21 04:04:47 +03:00

refactor(share): single resolveReadableSharePage for the share access boundary (#92)

Ghost referenced this issue

2026-06-21 04:19:31 +03:00

fix: review/red-team batch 2 — 30 issues (security, ws, page-templates, html-embed, mcp, tests, docs) #101

Ghost closed this issue

2026-06-21 14:10:34 +03:00

Sign in to join this conversation.