feat(auth): capability check — user/cron/worker allow; agents tiered allow/suggest/deny
This commit is contained in:
38
docs/security-followups.md
Normal file
38
docs/security-followups.md
Normal file
@@ -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.
|
||||
19
lib/auth/capability.js
Normal file
19
lib/auth/capability.js
Normal file
@@ -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';
|
||||
}
|
||||
55
tests/auth/capability.test.js
Normal file
55
tests/auth/capability.test.js
Normal file
@@ -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');
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user