fix(schema): enforce cross-space FK on tasks.project_id via composite key

Security review flagged that tasks.project_id could reference a project in
a different space. Added composite FK (project_id, space_id) -> projects(id, space_id)
with ON DELETE SET NULL (project_id) so a deleted project leaves the task in
its space with project_id NULL rather than orphaning into a NULL space.

Added two regression tests: cross-space FK rejection + cascade behavior.
This commit is contained in:
root
2026-05-31 02:14:20 +10:00
parent 951016385a
commit 3ca1509935
2 changed files with 52 additions and 4 deletions

View File

@@ -20,13 +20,18 @@ CREATE TABLE projects (
completed_at timestamptz, completed_at timestamptz,
created_at timestamptz NOT NULL DEFAULT now(), created_at timestamptz NOT NULL DEFAULT now(),
updated_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 tasks.(project_id, space_id) FK below
UNIQUE (id, space_id)
); );
CREATE TABLE tasks ( CREATE TABLE tasks (
id uuid PRIMARY KEY DEFAULT gen_random_uuid(), id uuid PRIMARY KEY DEFAULT gen_random_uuid(),
space_id uuid NOT NULL REFERENCES spaces(id) ON DELETE CASCADE, space_id uuid NOT NULL REFERENCES spaces(id) ON DELETE CASCADE,
project_id uuid REFERENCES projects(id) ON DELETE SET NULL, project_id uuid,
-- Cross-space FK enforcement: a task's project (if set) must live in the same space.
FOREIGN KEY (project_id, space_id) REFERENCES projects(id, space_id)
ON DELETE SET NULL (project_id),
title text NOT NULL, title text NOT NULL,
body text, body text,
status text NOT NULL DEFAULT 'todo' status text NOT NULL DEFAULT 'todo'

View File

@@ -1,9 +1,9 @@
import { describe, it, expect, beforeAll } from 'vitest'; import { describe, it, expect, beforeEach } from 'vitest';
import { resetDb, withClient } from '../helpers/db.js'; import { resetDb, withClient } from '../helpers/db.js';
import { migrateUp } from '../../lib/db/migrate.js'; import { migrateUp } from '../../lib/db/migrate.js';
describe('migration 001 — core', () => { describe('migration 001 — core', () => {
beforeAll(async () => { await resetDb(); await migrateUp(); }); beforeEach(async () => { await resetDb(); await migrateUp(); });
it('creates spaces, projects, tasks tables', async () => { it('creates spaces, projects, tasks tables', async () => {
await withClient(async (c) => { await withClient(async (c) => {
@@ -33,4 +33,47 @@ describe('migration 001 — core', () => {
).rejects.toThrow(/duplicate key/i); ).rejects.toThrow(/duplicate key/i);
}); });
}); });
it('rejects a task whose project is in a different space (composite FK)', async () => {
await withClient(async (c) => {
const { rows: [s1] } = await c.query(
`INSERT INTO spaces(slug, name) VALUES('one', 'One') RETURNING id;`
);
const { rows: [s2] } = await c.query(
`INSERT INTO spaces(slug, name) VALUES('two', 'Two') RETURNING id;`
);
const { rows: [p] } = await c.query(
`INSERT INTO projects(space_id, slug, name) VALUES($1, 'p', 'P') RETURNING id;`,
[s1.id]
);
await expect(
c.query(
`INSERT INTO tasks(space_id, project_id, title) VALUES($1, $2, 'x');`,
[s2.id, p.id]
)
).rejects.toThrow(/foreign key/i);
});
});
it('on project delete, task project_id becomes NULL but space_id stays', async () => {
await withClient(async (c) => {
const { rows: [s] } = await c.query(
`INSERT INTO spaces(slug, name) VALUES('s', 'S') RETURNING id;`
);
const { rows: [p] } = await c.query(
`INSERT INTO projects(space_id, slug, name) VALUES($1, 'p', 'P') RETURNING id;`,
[s.id]
);
const { rows: [t] } = await c.query(
`INSERT INTO tasks(space_id, project_id, title) VALUES($1, $2, 'x') RETURNING id;`,
[s.id, p.id]
);
await c.query(`DELETE FROM projects WHERE id=$1;`, [p.id]);
const { rows: [after] } = await c.query(
`SELECT space_id, project_id FROM tasks WHERE id=$1;`, [t.id]
);
expect(after.project_id).toBeNull();
expect(after.space_id).toBe(s.id);
});
});
}); });