Hynes/Void-Homelab
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(schema): tighten tenant boundaries on pages/refs

Browse Source 
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.
This commit is contained in:
root
2026-05-31 02:21:47 +10:00
parent c8649d753f
commit 9dd944226d
3 changed files with 74 additions and 11 deletions
Download Patch File Download Diff File Expand all files Collapse all files

20
lib/db/migrations/002_knowledge.sql
View File

@@ -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,''))

10
lib/db/repos/refs.js
View File

@@ -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);

55
tests/db/migration_002.test.js
View File

@@ -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);
});
});
});