vvzvlad/gitmost
1
0
Fork 0
Code Issues 14 Pull Requests 12 Packages Projects Releases Activity

fix(share): SEO route must not leak a restricted page's title (#159)

Browse Source 
`ShareSeoController.getShare` resolved the inherited share with the RAW
`getShareForPage`, which does NOT run the restricted-ancestor gate. So for a
page shared with includeSubPages whose descendant is permission-restricted, the
SEO route served that descendant's real title in <title>/og:title/twitter:title
to anonymous visitors and crawlers — even though the content API returns 404 for
it (red-team finding #3).

Funnel the SEO path through the canonical `resolveReadableSharePage` boundary
(the single place that checks `hasRestrictedAncestor`): a non-readable page now
serves the plain SPA index with no meta. Also honour `isSharingAllowed` — a
share whose workspace/space sharing toggle was flipped off after creation no
longer leaks its title via SEO. Title comes from the server-resolved page;
`buildShareMetaHtml` already emits robots=noindex when the share opted out of
indexing.

Tests (controller routing, fs spied at call time so bcrypt's native loader is
untouched): non-readable page => plain index, no title; sharing-disabled =>
plain index; readable+indexing => title + og:title, no noindex; readable+no-
indexing => noindex. Asserts getShareForPage is never called by the SEO path.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
claude code agent 227
2026-06-25 05:08:02 +03:00
parent 66f9079996
commit 96fb737c9d
2 changed files with 156 additions and 4 deletions
Download Patch File Download Diff File Expand all files Collapse all files

133
apps/server/src/core/share/share-seo.controller.routing.spec.ts Normal file
View File

@@ -0,0 +1,133 @@
import * as fs from 'node:fs';
import { ShareSeoController } from './share-seo.controller';
/**
* Routing guard for ShareSeoController.getShare (red-team finding #3).
*
* The SEO route must NOT leak a shared page's <title>/og:title to anonymous
* visitors / crawlers when the page is not publicly readable. It previously
* called the raw `getShareForPage`, which skips the restricted-ancestor gate, so
* a permission-restricted descendant of an includeSubPages share leaked its
* title. The fix funnels through `resolveReadableSharePage` (the canonical gate)
* AND honours `isSharingAllowed`. These tests pin that routing: a non-readable
* page or sharing-disabled space serves the plain SPA index (no title); only a
* readable, still-shared page gets meta tags.
*/
const SECRET_TITLE = 'Restricted Quarterly Numbers';
const INDEX_HTML = `<!doctype html><html><head><title>App</title><!--meta-tags--></head><body></body></html>`;
const STREAM_SENTINEL = { __isStream: true } as unknown as fs.ReadStream;
// Stub fs at CALL time (jest.spyOn), NOT module load (jest.mock): the controller
// transitively pulls bcrypt, whose native module is located by node-gyp-build
// reading the filesystem at import time — a module-level fs mock breaks that.
beforeEach(() => {
jest.spyOn(fs, 'existsSync').mockReturnValue(true);
jest.spyOn(fs, 'readFileSync').mockReturnValue(INDEX_HTML);
jest.spyOn(fs, 'createReadStream').mockReturnValue(STREAM_SENTINEL);
});
afterEach(() => jest.restoreAllMocks());
function makeRes() {
const res: any = {
sent: undefined as unknown,
type: jest.fn(() => res),
send: jest.fn((v: unknown) => {
res.sent = v;
}),
};
return res;
}
function makeController(opts: {
resolved: { share: any; page: any } | null;
sharingAllowed?: boolean;
}) {
const shareService = {
resolveReadableSharePage: jest.fn(async () => opts.resolved),
isSharingAllowed: jest.fn(async () => opts.sharingAllowed ?? true),
// Must NEVER be used by the SEO path anymore (the bypass is the bug).
getShareForPage: jest.fn(async () => {
throw new Error('getShareForPage must not be called by the SEO path');
}),
};
const workspaceRepo = {
findFirst: async () => ({ id: 'ws-1', settings: {} }),
};
const environmentService = { isSelfHosted: () => true };
const controller = new ShareSeoController(
shareService as any,
workspaceRepo as any,
environmentService as any,
);
return { controller, shareService };
}
const req: any = { raw: { headers: { host: 'self' } } };
describe('ShareSeoController.getShare routing (#3 title-leak gate)', () => {
it('serves the plain index (NO title) when the page is not publicly readable', async () => {
const { controller, shareService } = makeController({ resolved: null });
const res = makeRes();
await controller.getShare(res, req, 'share-key', `slug-pageB`);
// The restricted-ancestor gate ran; the raw bypass did not.
expect(shareService.resolveReadableSharePage).toHaveBeenCalled();
expect(shareService.getShareForPage).not.toHaveBeenCalled();
// The plain index stream was sent — NOT the title-bearing meta HTML.
expect(res.sent).toBe(STREAM_SENTINEL);
});
it('serves the plain index when sharing was disabled at the workspace/space level', async () => {
const { controller } = makeController({
resolved: {
share: { spaceId: 'sp-1', searchIndexing: true },
page: { title: SECRET_TITLE },
},
sharingAllowed: false,
});
const res = makeRes();
await controller.getShare(res, req, 'share-key', 'slug-pageB');
// The plain index stream was sent, so the restricted title never reached
// the response (it is only ever interpolated into the meta HTML string).
expect(res.sent).toBe(STREAM_SENTINEL);
expect(res.sent).not.toBe(SECRET_TITLE);
});
it('injects the title + meta for a readable, still-shared page', async () => {
const { controller } = makeController({
resolved: {
share: { spaceId: 'sp-1', searchIndexing: true },
page: { title: 'Public Handbook' },
},
sharingAllowed: true,
});
const res = makeRes();
await controller.getShare(res, req, 'share-key', 'slug-pageA');
expect(typeof res.sent).toBe('string');
expect(res.sent as string).toContain('<title>Public Handbook</title>');
expect(res.sent as string).toContain('og:title');
// searchIndexing on => crawlable (no noindex).
expect(res.sent as string).not.toContain('content="noindex"');
});
it('adds robots=noindex when the share opted out of search indexing', async () => {
const { controller } = makeController({
resolved: {
share: { spaceId: 'sp-1', searchIndexing: false },
page: { title: 'Internal Notes' },
},
sharingAllowed: true,
});
const res = makeRes();
await controller.getShare(res, req, 'share-key', 'slug-pageA');
expect(res.sent as string).toContain('content="noindex"');
});
});

27
apps/server/src/core/share/share-seo.controller.ts
View File

@@ -63,19 +63,38 @@ export class ShareSeoController {
const pageId = this.extractPageSlugId(pageSlug); const pageId = this.extractPageSlugId(pageSlug);
const share = await this.shareService.getShareForPage( // Funnel through the canonical readable-share boundary (NOT the raw
// getShareForPage) so the restricted-ancestor gate runs: a permission-
// restricted descendant of an includeSubPages share must NOT leak its
// title to anonymous visitors / crawlers (red-team finding #3). null =>
// not publicly readable => serve the plain SPA index with no meta.
const resolved = await this.shareService.resolveReadableSharePage(
undefined,
pageId, pageId,
workspace.id, workspace.id,
); );
if (!share) { if (!resolved) {
return this.sendIndex(indexFilePath, res);
}
// Honour a workspace/space-level sharing toggle flipped off AFTER this
// share was created: the content API gates on isSharingAllowed, so the SEO
// path must too or it keeps serving the title for a no-longer-shared page.
const sharingAllowed = await this.shareService.isSharingAllowed(
workspace.id,
resolved.share.spaceId,
);
if (!sharingAllowed) {
return this.sendIndex(indexFilePath, res); return this.sendIndex(indexFilePath, res);
} }
const html = fs.readFileSync(indexFilePath, 'utf8'); const html = fs.readFileSync(indexFilePath, 'utf8');
// Title of the PAGE being viewed (server-resolved), and noindex unless the
// share opted into search indexing (buildShareMetaHtml injects it).
let transformedHtml = buildShareMetaHtml(html, { let transformedHtml = buildShareMetaHtml(html, {
title: share?.sharedPage.title, title: resolved.page.title,
searchIndexing: share.searchIndexing, searchIndexing: resolved.share.searchIndexing,
}); });
// Deliberate same-origin tracker surface: this is the ONE place where an // Deliberate same-origin tracker surface: this is the ONE place where an