diff --git a/apps/client/src/features/share/components/share-alias-section.test.tsx b/apps/client/src/features/share/components/share-alias-section.test.tsx index f83e00c7..deef2426 100644 --- a/apps/client/src/features/share/components/share-alias-section.test.tsx +++ b/apps/client/src/features/share/components/share-alias-section.test.tsx @@ -13,8 +13,7 @@ let currentAlias: IShareAlias | null = null; let availabilityResult: { valid: boolean; available: boolean; - currentPageId: string | null; -} = { valid: true, available: true, currentPageId: null }; +} = { valid: true, available: true }; vi.mock("@/features/share/queries/share-query.ts", () => ({ useShareAliasForPageQuery: () => ({ data: currentAlias }), @@ -56,7 +55,7 @@ describe("ShareAliasSection — taken-name handling is never a dead end", () => beforeEach(() => { setMutateAsync.mockReset(); currentAlias = null; - availabilityResult = { valid: true, available: true, currentPageId: null }; + availabilityResult = { valid: true, available: true }; }); it("shows a 'will move it here' HINT (not a terminal error) when the name belongs to another page, and keeps Save enabled", async () => { @@ -65,7 +64,6 @@ describe("ShareAliasSection — taken-name handling is never a dead end", () => availabilityResult = { valid: true, available: false, - currentPageId: "page-X", }; renderSection("page-Y"); @@ -97,7 +95,6 @@ describe("ShareAliasSection — taken-name handling is never a dead end", () => availabilityResult = { valid: true, available: false, - currentPageId: "page-X", }; // The server rejects the un-confirmed save asking the client to confirm. setMutateAsync.mockRejectedValueOnce({ @@ -106,7 +103,6 @@ describe("ShareAliasSection — taken-name handling is never a dead end", () => status: 409, data: { code: "ALIAS_REASSIGN_REQUIRED", - currentPageId: "page-X", currentPageTitle: "Alias Test Page X", }, }, diff --git a/apps/client/src/features/share/components/share-alias-section.tsx b/apps/client/src/features/share/components/share-alias-section.tsx index a3938548..4e5cc261 100644 --- a/apps/client/src/features/share/components/share-alias-section.tsx +++ b/apps/client/src/features/share/components/share-alias-section.tsx @@ -48,7 +48,6 @@ export default function ShareAliasSection({ const [availability, setAvailability] = useState<{ valid: boolean; available: boolean; - currentPageId: string | null; } | null>(null); const [reassign, setReassign] = useState<{ alias: string; @@ -76,7 +75,6 @@ export default function ShareAliasSection({ setAvailability({ valid: res.valid, available: res.available, - currentPageId: res.currentPageId, }); } catch { setAvailability(null); diff --git a/apps/client/src/features/share/types/share.types.ts b/apps/client/src/features/share/types/share.types.ts index caba0b1b..0752d12e 100644 --- a/apps/client/src/features/share/types/share.types.ts +++ b/apps/client/src/features/share/types/share.types.ts @@ -108,7 +108,6 @@ export interface IShareAliasAvailability { alias: string; valid: boolean; available: boolean; - currentPageId: string | null; } export interface ISharedPageTree { diff --git a/apps/server/src/core/share/share-alias.service.spec.ts b/apps/server/src/core/share/share-alias.service.spec.ts index 471ccf8e..63f16a1c 100644 --- a/apps/server/src/core/share/share-alias.service.spec.ts +++ b/apps/server/src/core/share/share-alias.service.spec.ts @@ -450,18 +450,22 @@ describe('ShareAliasService', () => { alias: 'free-name', valid: true, available: true, - currentPageId: null, }); + // SECURITY (#495): the availability probe must NOT leak any page id. + expect(res).not.toHaveProperty('currentPageId'); }); - it('reports taken with the current target page', async () => { + it('reports taken WITHOUT leaking the current target page id (#495)', async () => { const { service, shareAliasRepo } = makeService(); shareAliasRepo.findByAliasAndWorkspace.mockResolvedValue({ id: 'a-1', pageId: 'p-9', }); const res = await service.checkAvailability('taken', 'ws-1'); - expect(res).toMatchObject({ available: false, currentPageId: 'p-9' }); + expect(res).toMatchObject({ available: false }); + // The row exists (available:false) but its pageId is never returned — an + // authenticated member cannot map an alias name to a page id it can't view. + expect(res).not.toHaveProperty('currentPageId'); }); }); diff --git a/apps/server/src/core/share/share-alias.service.ts b/apps/server/src/core/share/share-alias.service.ts index 8b05eabb..6af94692 100644 --- a/apps/server/src/core/share/share-alias.service.ts +++ b/apps/server/src/core/share/share-alias.service.ts @@ -223,21 +223,28 @@ export class ShareAliasService { alias: string; valid: boolean; available: boolean; - currentPageId: string | null; }> { const alias = normalizeShareAlias(rawAlias); if (!isValidShareAlias(alias)) { - return { alias, valid: false, available: false, currentPageId: null }; + return { alias, valid: false, available: false }; } const existing = await this.shareAliasRepo.findByAliasAndWorkspace( alias, workspaceId, ); + // SECURITY (#495): return ONLY the boolean availability. The previous shape + // leaked `currentPageId` — the id of whatever page the alias already targets — + // to ANY authenticated workspace member, with no view-permission check on that + // page. An attacker could enumerate alias names and map them to page ids they + // have no access to. The taken/free bit is all the "is this address free" + // probe needs; the reassign flow surfaces the target's title only AFTER a real + // setAlias attempt (the 409 ALIAS_REASSIGN_REQUIRED path), which is access- + // gated. If a caller ever needs the target page id, it must be returned only + // behind an explicit `validateCanView` on that page. return { alias, valid: true, available: !existing, - currentPageId: existing?.pageId ?? null, }; }