From 9dd944226de5b5c08e8dc5b741b69fc72b6e0761 Mon Sep 17 00:00:00 2001 From: root Date: Sun, 31 May 2026 02:21:47 +1000 Subject: [PATCH] fix(schema): tighten tenant boundaries on pages/refs Three security-review findings on migration 002: - pages.space_id and refs.space_id changed to NOT NULL + ON DELETE CASCADE (was nullable + SET NULL, which allowed orphan rows surviving space deletion). - pages.parent_id wrapped in composite FK (parent_id, space_id) -> pages(id, space_id) to prevent cross-space parent linkage (same pattern as tasks.project_id in 001). - idx_refs_external promoted to UNIQUE on (space_id, source_kind, external_id); upsertByExternal now requires space_id and dedups per-space, not globally. Added 3 regression tests covering composite FK rejection, CASCADE-on-space-delete, and per-space dedup independence. --- lib/db/migrations/002_knowledge.sql | 20 +++++++---- lib/db/repos/refs.js | 10 +++--- tests/db/migration_002.test.js | 55 +++++++++++++++++++++++++++++ 3 files changed, 74 insertions(+), 11 deletions(-) diff --git a/lib/db/migrations/002_knowledge.sql b/lib/db/migrations/002_knowledge.sql index da24ca8..dd37340 100644 --- a/lib/db/migrations/002_knowledge.sql +++ b/lib/db/migrations/002_knowledge.sql @@ -1,15 +1,20 @@ CREATE TABLE pages ( id uuid PRIMARY KEY DEFAULT gen_random_uuid(), - space_id uuid REFERENCES spaces(id) ON DELETE SET NULL, + space_id uuid NOT NULL REFERENCES spaces(id) ON DELETE CASCADE, slug text NOT NULL, title text NOT NULL, body_md text NOT NULL DEFAULT '', body_html text, - parent_id uuid REFERENCES pages(id) ON DELETE SET NULL, + parent_id uuid, + -- Same-space parent enforcement (mirrors tasks.project_id pattern) + FOREIGN KEY (parent_id, space_id) REFERENCES pages(id, space_id) + ON DELETE SET NULL (parent_id), embedding vector(1024), created_at timestamptz NOT NULL DEFAULT now(), updated_at timestamptz NOT NULL DEFAULT now(), - UNIQUE (space_id, slug) + UNIQUE (space_id, slug), + -- Composite key target for parent_id self-reference above + any future FK + UNIQUE (id, space_id) ); CREATE TABLE page_revisions ( @@ -22,7 +27,7 @@ CREATE TABLE page_revisions ( CREATE TABLE refs ( id uuid PRIMARY KEY DEFAULT gen_random_uuid(), - space_id uuid REFERENCES spaces(id) ON DELETE SET NULL, + space_id uuid NOT NULL REFERENCES spaces(id) ON DELETE CASCADE, kind text NOT NULL CHECK (kind IN ('url','video','pdf','image','file')), source_url text, @@ -52,8 +57,11 @@ CREATE INDEX idx_pages_embed ON pages CREATE INDEX idx_refs_space ON refs(space_id); CREATE INDEX idx_refs_kind ON refs(kind); -CREATE INDEX idx_refs_external ON refs(source_kind, external_id) - WHERE source_kind IS NOT NULL; +-- Per-space uniqueness on external dedup (mirrors tenancy-first pattern of 001). +-- upsertByExternal MUST pass space_id; cross-space same-external_id creates distinct rows. +CREATE UNIQUE INDEX idx_refs_external_unique + ON refs(space_id, source_kind, external_id) + WHERE source_kind IS NOT NULL AND external_id IS NOT NULL; CREATE INDEX idx_refs_fts ON refs USING GIN ( to_tsvector('english', coalesce(title,'') || ' ' || coalesce(summary,'') || ' ' || coalesce(body_text,'')) diff --git a/lib/db/repos/refs.js b/lib/db/repos/refs.js index 1ae13d5..348189e 100644 --- a/lib/db/repos/refs.js +++ b/lib/db/repos/refs.js @@ -30,13 +30,13 @@ export async function getById(id) { } export async function upsertByExternal(input, actor) { - const { source_kind, external_id } = input; - if (!source_kind || !external_id) { - throw new Error('upsertByExternal requires source_kind + external_id'); + const { space_id, source_kind, external_id } = input; + if (!space_id || !source_kind || !external_id) { + throw new Error('upsertByExternal requires space_id + source_kind + external_id'); } const { rows: [existing] } = await pool.query( - `SELECT * FROM refs WHERE source_kind=$1 AND external_id=$2`, - [source_kind, external_id] + `SELECT * FROM refs WHERE space_id=$1 AND source_kind=$2 AND external_id=$3`, + [space_id, source_kind, external_id] ); if (existing) { return update(existing.id, input, actor); diff --git a/tests/db/migration_002.test.js b/tests/db/migration_002.test.js index 3f489bc..94c0376 100644 --- a/tests/db/migration_002.test.js +++ b/tests/db/migration_002.test.js @@ -26,4 +26,59 @@ describe('migration 002 — knowledge', () => { )).rejects.toThrow(/check/i); }); }); + + it('pages.parent_id cannot cross spaces (composite FK)', async () => { + await withClient(async (c) => { + const { rows: [s1] } = await c.query( + `INSERT INTO spaces(slug,name) VALUES('a','A') RETURNING id;`); + const { rows: [s2] } = await c.query( + `INSERT INTO spaces(slug,name) VALUES('b','B') RETURNING id;`); + const { rows: [parent] } = await c.query( + `INSERT INTO pages(space_id, slug, title) VALUES($1,'p','P') RETURNING id;`, + [s1.id] + ); + await expect(c.query( + `INSERT INTO pages(space_id, parent_id, slug, title) VALUES($1,$2,'c','C');`, + [s2.id, parent.id] + )).rejects.toThrow(/foreign key/i); + }); + }); + + it('deleting a space cascades pages and refs', async () => { + await withClient(async (c) => { + const { rows: [s] } = await c.query( + `INSERT INTO spaces(slug,name) VALUES('s','S') RETURNING id;`); + await c.query( + `INSERT INTO pages(space_id, slug, title) VALUES($1,'p','P');`, [s.id]); + await c.query( + `INSERT INTO refs(space_id, kind) VALUES($1, 'url');`, [s.id]); + await c.query(`DELETE FROM spaces WHERE id=$1;`, [s.id]); + const { rows: [{ n: pn }] } = await c.query(`SELECT count(*)::int n FROM pages`); + const { rows: [{ n: rn }] } = await c.query(`SELECT count(*)::int n FROM refs`); + expect(pn).toBe(0); + expect(rn).toBe(0); + }); + }); + + it('refs external dedup is per-space (cross-space same external_id is distinct)', async () => { + await withClient(async (c) => { + const { rows: [s1] } = await c.query( + `INSERT INTO spaces(slug,name) VALUES('a','A') RETURNING id;`); + const { rows: [s2] } = await c.query( + `INSERT INTO spaces(slug,name) VALUES('b','B') RETURNING id;`); + const { rows: [r1] } = await c.query( + `INSERT INTO refs(space_id, kind, source_kind, external_id) VALUES($1,'url','karakeep','kk-1') RETURNING id;`, + [s1.id] + ); + const { rows: [r2] } = await c.query( + `INSERT INTO refs(space_id, kind, source_kind, external_id) VALUES($1,'url','karakeep','kk-1') RETURNING id;`, + [s2.id] + ); + expect(r1.id).not.toBe(r2.id); + await expect(c.query( + `INSERT INTO refs(space_id, kind, source_kind, external_id) VALUES($1,'url','karakeep','kk-1');`, + [s1.id] + )).rejects.toThrow(/unique|duplicate/i); + }); + }); });