From cd71d64523c7788d56ac967fce210b5eadc20dd9 Mon Sep 17 00:00:00 2001 From: root Date: Sun, 31 May 2026 11:06:00 +1000 Subject: [PATCH] =?UTF-8?q?feat(auth):=20capability=20check=20=E2=80=94=20?= =?UTF-8?q?user/cron/worker=20allow;=20agents=20tiered=20allow/suggest/den?= =?UTF-8?q?y?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/security-followups.md | 38 ++++++++++++++++++++++++ lib/auth/capability.js | 19 ++++++++++++ tests/auth/capability.test.js | 55 +++++++++++++++++++++++++++++++++++ 3 files changed, 112 insertions(+) create mode 100644 docs/security-followups.md create mode 100644 lib/auth/capability.js create mode 100644 tests/auth/capability.test.js diff --git a/docs/security-followups.md b/docs/security-followups.md new file mode 100644 index 0000000..d8c31f8 --- /dev/null +++ b/docs/security-followups.md @@ -0,0 +1,38 @@ +# Security Follow-ups for Plan 1 + +Items flagged by the security review plugin that were deferred at the user's request +("ease up on the security plugin") and are to be addressed in a consolidated pass +before Plan 1 is declared complete. + +## Migration 005 (cross-cutting / polymorphic tables) + +The polymorphic shape (`entity_type` text + `entity_id` uuid, no composite FK to a +parent) was an approved design decision in `docs/superpowers/specs/2026-05-31-void-v2-design.md`. +That decision trades referential integrity at the DB layer for one shared join table +across entity types. The plugin's findings are accurate but require revisiting the +design choice — not a one-line fix. + +- **[HIGH] entity_tags / entity_links / attachments lack `space_id`.** + Cross-tenant linkage is possible at the DB layer. Mitigation options: + 1. Add `space_id` NOT NULL to each polymorphic row; require callers to filter. + 2. Per-type junction tables (e.g. `page_tags`, `task_links`) with real FKs. + +- **[MEDIUM] `tags.name UNIQUE` is global, not per-space.** Cross-tenant collision + creates a side-channel for tag-name discovery. Fix: `UNIQUE(space_id, name)`. + +- **[MEDIUM] No cascade on parent delete for `attachments` / `entity_tags`.** + Polymorphic FKs cannot be expressed in standard SQL. Fix options as above, or + a deletion trigger per parent type. + +## Decision pending + +Before closing Plan 1, decide whether to: +- (a) **Tighten now**: add `space_id` columns + per-type composite FKs to `entity_tags`, + `entity_links`, `attachments`, and a per-space UNIQUE on `tags`. Cost: ~1-2 hrs and + invalidates the polymorphic claim in the spec. +- (b) **Document and defer**: accept the tradeoff for Plan 1, gate access at the + REST/MCP layer (where `space_id` IS available from the actor's session), and + revisit when multi-tenant becomes load-bearing. + +Owner-only authn (Plan 1) means there's only one tenant in practice today, so (b) is +defensible for the alpha. (a) is the right long-term answer. diff --git a/lib/auth/capability.js b/lib/auth/capability.js new file mode 100644 index 0000000..91f2d87 --- /dev/null +++ b/lib/auth/capability.js @@ -0,0 +1,19 @@ +export function canAct(actor, action, entity_type) { + if (!actor) return 'deny'; + if (actor.kind === 'user') return 'allow'; + if (actor.kind === 'cron' || actor.kind === 'worker' || actor.kind === 'system') return 'allow'; + + if (actor.kind !== 'agent') return 'deny'; + + const caps = actor.capabilities || {}; + const scopes = actor.scopes || {}; + + if (action === 'read') return caps.read ? 'allow' : 'deny'; + + const isMutation = ['create','update','delete'].includes(action); + if (!isMutation) return 'deny'; + + if (caps.write && scopes[entity_type]) return 'allow'; + if (caps.suggest) return 'suggest'; + return 'deny'; +} diff --git a/tests/auth/capability.test.js b/tests/auth/capability.test.js new file mode 100644 index 0000000..5876cd6 --- /dev/null +++ b/tests/auth/capability.test.js @@ -0,0 +1,55 @@ +import { describe, it, expect } from 'vitest'; +import { canAct } from '../../lib/auth/capability.js'; + +const ownerActor = { kind: 'user', id: null }; +const readAgent = { + kind: 'agent', id: 'a1', + capabilities: { read: true, suggest: true, write: false }, + scopes: {} +}; +const writeAgent = { + kind: 'agent', id: 'a2', + capabilities: { read: true, suggest: true, write: true }, + scopes: { page: true } +}; + +describe('canAct', () => { + it('owner can do anything', () => { + expect(canAct(ownerActor, 'create', 'page')).toBe('allow'); + expect(canAct(ownerActor, 'delete', 'resource')).toBe('allow'); + }); + + it('read-only agent can read', () => { + expect(canAct(readAgent, 'read', 'page')).toBe('allow'); + }); + + it('default agent on create returns "suggest"', () => { + expect(canAct(readAgent, 'create', 'page')).toBe('suggest'); + }); + + it('write-scoped agent can write to its scope', () => { + expect(canAct(writeAgent, 'create', 'page')).toBe('allow'); + }); + + it('write-capable agent without scope still suggests outside it', () => { + expect(canAct(writeAgent, 'create', 'resource')).toBe('suggest'); + }); + + it('agent with no capabilities is deny on mutations', () => { + expect(canAct({ kind: 'agent', id: 'x', capabilities: {} }, 'create', 'page')).toBe('deny'); + }); + + it('agent with no read capability is deny on read', () => { + expect(canAct({ kind: 'agent', id: 'x', capabilities: {} }, 'read', 'page')).toBe('deny'); + }); + + it('null actor is deny', () => { + expect(canAct(null, 'read', 'page')).toBe('deny'); + }); + + it('cron/worker/system get allow', () => { + expect(canAct({ kind: 'cron', id: null }, 'create', 'page')).toBe('allow'); + expect(canAct({ kind: 'worker', id: null }, 'update', 'page')).toBe('allow'); + expect(canAct({ kind: 'system', id: null }, 'delete', 'page')).toBe('allow'); + }); +});