## Why
An AI agent autonomously deleted a live task (FN-8609) via
`fn_task_delete`. Investigating that led to two audits, which found the
gate machinery is well-built and its *enforcement* well-tested — but the
*decision boundary* was unguarded and the tools that matter never
reached the gate at all.
The headline defect: all ~76 tools in `packages/cli/src/extension.ts`
are delivered through `hostExtensionPaths` → `DefaultResourceLoader`,
which never passes through `wrapToolsWithActionGate`. `fn_task_delete`
*was* classified as gated in `gating-classifications.ts` — that
classification was dead code. Roughly 15 destructive tools were ungated
even for an operator who had locked their policy all the way down.
Second: `POST /api/approvals/:id/decision` took the approver identity
from the request body, validated for shape only, with no check that the
decider was human or differed from the requester. Combined with a bash
tool whose command string was never inspected (only `params.cwd`), an
agent could read the daemon token from `~/.fusion/settings.json`, find
its own pending request, and approve it.
## Scope
This does **not** tighten defaults. The `unrestricted` preset is
byte-for-byte unchanged — `git diff` on `agent-permission-policy.ts` is
empty — and regression tests assert that an out-of-the-box install
behaves exactly as before. The bug was never "the default is too
permissive"; it was "strict policy doesn't enforce." This makes turning
security up actually work.
The one deliberate exception: the containment that stops an agent
escalating its *own* privileges (reading the daemon token / credentials,
calling the approvals API to self-approve) applies at every preset
including `unrestricted`. That is a privilege-escalation boundary rather
than a permission preference — if it only engaged under strict policy it
would not have prevented the incident that prompted this.
## What changed
8 bisectable commits:
- **Approval lifecycle** — self-approval blocked via server-derived
deciders; same-verdict replay 409s; decide re-reads and re-validates
inside the transaction; expiry TTLs; `markCompleted` ownership check;
session identity registry in core.
- **Engine gates enforce for real** — unclassified tools resolve to a
policy-governed category instead of hardcoded `allow`; missing-policy
fail-open closed; bash containment floor + exact-command approval
binding.
- **Dashboard decision routes** — stop trusting client-supplied actors
(decision, bypass-review, worktrunk → 403 on forged actors).
- **`fn serve` authenticated by default** — auto-mints a token following
the existing `fn dashboard` precedent; `--no-auth` opts out.
- **Sibling entry points closed** — user-sourced hard-cancel moves, ACP
execute-once approvals, plugin task-store gating.
- **pi-extension principal resolution** — the extension resolves the
acting principal and can withhold or policy-gate the previously ungated
destructive tools.
- **Root-cause bonus fix** — `findLatestByDedupeKey` was broken in
PostgreSQL backend mode (already-parsed jsonb fed through a string-only
parser), so approved-grant redemption **never matched in production**,
minting duplicate requests. This explains the live DB state of 17
approved / 0 completed. *(Also cherry-picked to `main` as `a9b30013bb`,
since it is an active production defect on its own.)*
- **Review follow-ups** (`627f1b1fa8`) — operator-configured
provisioning privilege and a configurable grant TTL; see below.
## Review follow-ups
**Provisioning privilege is operator-configured, not role-derived.**
`isCallerPrivileged` had gone from `caller.reportsTo == null` (every
top-level agent privileged — permanent escalation by creating a
manager-less agent) to `caller.role === "ceo"`, which swapped an
implicit rule for a magic string: any agent config can claim that role,
while an operator who genuinely wants a privileged agent had no
supported way to say so. Privilege now derives solely from
`agentProvisioning.trustedAgentIds` / `trustedRoles` and fails closed
when settings are unresolvable.
It is also no longer forwarded to `resolveAgentProvisioningPolicy` as
`isPrivileged`, because that flag short-circuits ahead of
`alwaysApproveDelete` — a trusted caller was bypassing delete approval
entirely. The policy applies the same trusted rules itself, in the right
order. The function now governs only the org-chart escape hatch (acting
outside your own direct reports).
**Grant TTL defaults to 1 hour and is configurable.** Approval →
redemption is not instantaneous: an operator approving from their phone,
an engine restart, a queued lane, or a task waiting on a worktree all
routinely exceeded 15 minutes, after which the grant expired and the
agent silently re-requested. One hour remains far short of the
"redeemable forever" hazard the TTL exists to bound. Override via
`FUSION_APPROVAL_GRANT_TTL_MS` or `configureApprovalRequestTtls()`;
invalid overrides are ignored rather than widening the window to
infinity or collapsing it to zero.
## Behavior changes requiring operator review before rollout
1. `fn serve` requires a bearer token by default (`--no-auth` opts out);
unauthenticated clients get 401.
2. Agents can no longer run withheld destructive tools
(`fn_task_delete`, `fn_task_bypass_review`,
mission/milestone/slice/feature/workflow deletes, `experiment_finalize`,
`skills_install`). Operators keep them via CLI/dashboard. **This is the
incident fix.**
3. Agents get provisioning privilege only when the operator lists them
in `agentProvisioning.trustedAgentIds` / `trustedRoles`; the
provisioning gate is now live in production. Previously-implicit
privilege (top-level position, or a `ceo` role) no longer grants
anything on its own.
4. Decision replay 409s (was 200); pending approvals expire after 24h,
approved grants after 1h (configurable); bash approvals bind per exact
command.
5. Forged/body actors on decision, bypass-review, worktrunk routes →
403; `archive-all-done` requires `{confirm:true}` (external scripts
affected).
6. `fn_secret_get` approvals grant exactly one reveal (previously
granted nothing and looped forever); ACP approvals are execute-once
(previously infinite reuse).
7. Bash containment denies token/credential/approvals-API commands in
all agent sessions at every preset.
## Verification
Independently re-run against the branch, not just self-reported:
- 5 typechecks (core, engine, cli, dashboard `tsconfig.json` +
`tsconfig.app.json`) — clean
- `pnpm lint` — clean
- `pnpm test:gate` — 379 passed
- `pnpm build --force` — green (a plain `pnpm build` skips packages as
unchanged and does **not** compile the branch)
- `pnpm check:changesets` — clean
- ~650 file-scoped tests including new negative-path suites for the
decision boundary, which previously had **zero** test coverage
`packages/engine/src/__tests__/plugin-runner.test.ts` fails 56/80 —
**verified pre-existing**, reproducing identically at base commit
`93a403af67` on `main`. Not in the merge gate.
### A mutation check that failed to fail
Worth recording, because it nearly shipped an untested security fix. The
first mutation check on the provisioning change reintroduced the `ceo`
hardcode and **all 17 tests still passed** — the tests asserted through
the policy path, which can no longer observe `isCallerPrivileged` at
all, precisely because `isPrivileged` is no longer forwarded there.
Org-chart cases that do exercise the function were added; the hardcode
now fails exactly 1 of 19, and restoring is green. A green mutation run
is only meaningful if the test can actually see the code under test.
## Known limitations (stated, not papered over)
- The bash containment floor is string-matching: a cost-raiser, not a
sandbox. Quoting, encoding, `$HOME`, symlinks, or an interpreter
one-liner can evade it. The durable protection is the decision route
refusing agent-originated deciders — the filter is the belt, not the
braces.
- Approval expiry is lazy (evaluated at decide/complete/redeem), not
swept, so an expired pending row stays visible in lists until touched.
- The extension's require-approval path returns a pending message but
cannot suspend a pi session mid-turn; engine-side pause hooks cover
engine lanes only.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **Security**
* Hardened approval and permission gating with server-side decider
attribution, self-approval blocking, ownership checks, replay/race
protection, and status/TTL enforcement.
* Added fail-closed behavior for sensitive/unclassified tools and
sandbox provisioning approvals.
* Blocked credential/approval access via bash containment; plugin
destructive task operations now require explicit permission.
* **New Features**
* `fn serve` now defaults to bearer-token auth, with `--no-auth` as the
explicit opt-out.
* **Bug Fixes**
* Improved task move-source attribution (`moveSource: "user"`) and
tightened dashboard archive/bypass confirmation and operator attribution
behavior.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
280 lines
9.8 KiB
TypeScript
280 lines
9.8 KiB
TypeScript
import { randomUUID } from "node:crypto";
|
|
import { count, eq, desc, and } from "drizzle-orm";
|
|
import type { Database } from "./db.js";
|
|
import { fromJson } from "./db.js";
|
|
import type { AsyncDataLayer } from "./postgres/data-layer.js";
|
|
import * as asyncApprovalRequestStore from "./async-approval-request-store.js";
|
|
import * as schema from "./postgres/schema/index.js";
|
|
/*
|
|
FNXC:ApprovalLifecycleSecurity 2026-07-30-14:30 (migration rebase):
|
|
The expiry check moved with the rest of the lifecycle hardening into async-approval-request-store.ts.
|
|
This class no longer has a sync branch to guard — the PostgreSQL cutover deleted it — so the import
|
|
that fed it is gone rather than left dangling.
|
|
*/
|
|
import {
|
|
normalizeApprovalRequestActionCategory,
|
|
type ApprovalRequest,
|
|
type ApprovalRequestActorSnapshot,
|
|
type ApprovalRequestAuditEvent,
|
|
type ApprovalRequestAuditEventType,
|
|
type ApprovalRequestCompletionInput,
|
|
type ApprovalRequestCreateInput,
|
|
type ApprovalRequestDecisionInput,
|
|
type ApprovalRequestListInput,
|
|
type ApprovalRequestStatus,
|
|
} from "./types.js";
|
|
|
|
interface ApprovalRequestRow {
|
|
id: string;
|
|
status: ApprovalRequestStatus;
|
|
requesterActorId: string;
|
|
requesterActorType: ApprovalRequestActorSnapshot["actorType"];
|
|
requesterActorName: string;
|
|
targetActionCategory: string;
|
|
targetActionOperation: string;
|
|
targetActionSummary: string;
|
|
targetResourceType: string;
|
|
targetResourceId: string;
|
|
targetContext: string | null;
|
|
taskId: string | null;
|
|
runId: string | null;
|
|
requestedAt: string;
|
|
decidedAt: string | null;
|
|
completedAt: string | null;
|
|
createdAt: string;
|
|
updatedAt: string;
|
|
}
|
|
|
|
interface ApprovalRequestAuditEventRow {
|
|
id: string;
|
|
requestId: string;
|
|
eventType: ApprovalRequestAuditEventType;
|
|
actorId: string;
|
|
actorType: ApprovalRequestActorSnapshot["actorType"];
|
|
actorName: string;
|
|
note: string | null;
|
|
createdAt: string;
|
|
}
|
|
|
|
export class ApprovalRequestStore {
|
|
/**
|
|
* FNXC:ApprovalRequestStore 2026-06-24-21:15:
|
|
* When non-null, the store is in backend (PostgreSQL) mode and all data
|
|
* access delegates to the async helpers. The sync db is unused in this mode.
|
|
*/
|
|
private readonly asyncLayer: AsyncDataLayer | null;
|
|
|
|
constructor(
|
|
private db: Database | null,
|
|
options?: { asyncLayer?: AsyncDataLayer | null },
|
|
) {
|
|
this.asyncLayer = options?.asyncLayer ?? null;
|
|
}
|
|
|
|
/** True when the store is backed by PostgreSQL (AsyncDataLayer present). */
|
|
private get backendMode(): boolean {
|
|
return this.asyncLayer !== null;
|
|
}
|
|
|
|
/**
|
|
* FNXC:ApprovalRequestStore 2026-06-24-21:20:
|
|
* Asserts the sync SQLite database is available. In backend mode this is
|
|
* never called (the async branch returns first); in SQLite mode the db is
|
|
* always provided at construction.
|
|
*/
|
|
private syncDb(): Database {
|
|
if (!this.db) {
|
|
throw new Error("ApprovalRequestStore: sync Database is null (backend mode requires asyncLayer)");
|
|
}
|
|
return this.db;
|
|
}
|
|
|
|
/*
|
|
FNXC:ApprovalRedemption 2026-07-26-16:40:
|
|
In backend (PostgreSQL) mode `targetContext` is a jsonb column that Drizzle
|
|
returns ALREADY PARSED, while legacy rows may still store a JSON string.
|
|
Feeding the parsed object through the string-only `fromJson` made
|
|
`findLatestByDedupeKey` never match in PG mode, so every gate retry minted a
|
|
duplicate approval request and approved-grant reuse silently never worked in
|
|
production. Normalize both shapes here.
|
|
|
|
FNXC:SqliteDualPathCleanup 2026-07-26-15:05:
|
|
Same helper used on the PG-only runtime path after dual-path collapse.
|
|
*/
|
|
private static normalizeTargetContext(value: unknown): Record<string, unknown> | undefined {
|
|
if (value === null || value === undefined) return undefined;
|
|
if (typeof value === "string") return fromJson<Record<string, unknown>>(value);
|
|
if (typeof value === "object") return value as Record<string, unknown>;
|
|
return undefined;
|
|
}
|
|
|
|
private rowToRequest(row: ApprovalRequestRow): ApprovalRequest {
|
|
return {
|
|
id: row.id,
|
|
status: row.status,
|
|
requester: {
|
|
actorId: row.requesterActorId,
|
|
actorType: row.requesterActorType,
|
|
actorName: row.requesterActorName,
|
|
},
|
|
targetAction: {
|
|
category: normalizeApprovalRequestActionCategory(
|
|
row.targetActionCategory as Parameters<typeof normalizeApprovalRequestActionCategory>[0],
|
|
),
|
|
action: row.targetActionOperation,
|
|
summary: row.targetActionSummary,
|
|
resourceType: row.targetResourceType,
|
|
resourceId: row.targetResourceId,
|
|
context: ApprovalRequestStore.normalizeTargetContext(row.targetContext),
|
|
},
|
|
taskId: row.taskId ?? undefined,
|
|
runId: row.runId ?? undefined,
|
|
requestedAt: row.requestedAt,
|
|
decidedAt: row.decidedAt ?? undefined,
|
|
completedAt: row.completedAt ?? undefined,
|
|
createdAt: row.createdAt,
|
|
updatedAt: row.updatedAt,
|
|
};
|
|
}
|
|
|
|
private rowToAuditEvent(row: ApprovalRequestAuditEventRow): ApprovalRequestAuditEvent {
|
|
return {
|
|
id: row.id,
|
|
requestId: row.requestId,
|
|
eventType: row.eventType,
|
|
actor: {
|
|
actorId: row.actorId,
|
|
actorType: row.actorType,
|
|
actorName: row.actorName,
|
|
},
|
|
note: row.note ?? undefined,
|
|
createdAt: row.createdAt,
|
|
};
|
|
}
|
|
|
|
private appendAuditEvent(
|
|
requestId: string,
|
|
eventType: ApprovalRequestAuditEventType,
|
|
actor: ApprovalRequestActorSnapshot,
|
|
createdAt: string,
|
|
note?: string,
|
|
): ApprovalRequestAuditEvent {
|
|
const event: ApprovalRequestAuditEvent = {
|
|
id: `aprevt-${randomUUID().slice(0, 8)}`,
|
|
requestId,
|
|
eventType,
|
|
actor,
|
|
...(note !== undefined ? { note } : {}),
|
|
createdAt,
|
|
};
|
|
|
|
this.syncDb().prepare(`
|
|
INSERT INTO approval_request_audit_events (id, requestId, eventType, actorId, actorType, actorName, note, createdAt)
|
|
VALUES (?, ?, ?, ?, ?, ?, ?, ?)
|
|
`).run(
|
|
event.id,
|
|
event.requestId,
|
|
event.eventType,
|
|
event.actor.actorId,
|
|
event.actor.actorType,
|
|
event.actor.actorName,
|
|
event.note ?? null,
|
|
event.createdAt,
|
|
);
|
|
|
|
return event;
|
|
}
|
|
|
|
async create(input: ApprovalRequestCreateInput): Promise<ApprovalRequest> {
|
|
/*
|
|
FNXC:SqliteDualPathCleanup 2026-07-27-06:15:
|
|
Dual-path collapse left a local ApprovalRequest construction that was discarded
|
|
while a second random id was generated for the PG insert. Use one id and let
|
|
createApprovalRequest materialize the full row.
|
|
*/
|
|
const id = `apr-${randomUUID().slice(0, 8)}`;
|
|
return asyncApprovalRequestStore.createApprovalRequest(this.asyncLayer!, { ...input, id });
|
|
}
|
|
|
|
async get(id: string): Promise<ApprovalRequest | null> {
|
|
return asyncApprovalRequestStore.getApprovalRequest(this.asyncLayer!.db, id);
|
|
}
|
|
|
|
async list(input: ApprovalRequestListInput = {}): Promise<ApprovalRequest[]> {
|
|
return asyncApprovalRequestStore.listApprovalRequests(this.asyncLayer!.db, input);
|
|
}
|
|
|
|
async getPendingCountsByActor(): Promise<Map<string, number>> {
|
|
const table = schema.project.approvalRequests;
|
|
const rows = await this.asyncLayer!.db
|
|
.select({
|
|
actorId: table.requesterActorId,
|
|
requestCount: count(),
|
|
})
|
|
.from(table)
|
|
.where(eq(table.status, "pending"))
|
|
.groupBy(table.requesterActorId);
|
|
return new Map(rows.map((row) => [row.actorId, Number(row.requestCount)]));
|
|
}
|
|
|
|
async findLatestByDedupeKey(input: { requesterActorId: string; taskId?: string; dedupeKey: string }): Promise<ApprovalRequest | null> {
|
|
/*
|
|
FNXC:SqliteDualPathCleanup 2026-07-26-15:05:
|
|
Production path is PostgreSQL-only. When asyncLayer is absent (unit tests that inject a fake prepare/all Database), fall through to the sync scan so shape-independence tests still drive the real public method.
|
|
*/
|
|
if (this.backendMode) {
|
|
const table = schema.project.approvalRequests;
|
|
const conditions = [eq(table.requesterActorId, input.requesterActorId)];
|
|
if (input.taskId !== undefined) {
|
|
conditions.push(eq(table.taskId, input.taskId));
|
|
}
|
|
const rows = await this.asyncLayer!.db
|
|
.select()
|
|
.from(table)
|
|
.where(and(...conditions))
|
|
.orderBy(desc(table.createdAt), desc(table.id));
|
|
for (const row of rows as ApprovalRequestRow[]) {
|
|
// FNXC:ApprovalRedemption 2026-07-26-16:40: jsonb rows arrive parsed; see normalizeTargetContext.
|
|
const context = ApprovalRequestStore.normalizeTargetContext(row.targetContext);
|
|
if (context?.approvalDedupeKey === input.dedupeKey) {
|
|
return this.rowToRequest(row);
|
|
}
|
|
}
|
|
return null;
|
|
}
|
|
|
|
const where = ["requesterActorId = ?"];
|
|
const params: Array<string> = [input.requesterActorId];
|
|
if (input.taskId !== undefined) {
|
|
where.push("taskId = ?");
|
|
params.push(input.taskId);
|
|
}
|
|
|
|
const rows = this.syncDb().prepare(`
|
|
SELECT * FROM approval_requests
|
|
WHERE ${where.join(" AND ")}
|
|
ORDER BY createdAt DESC, id DESC
|
|
`).all(...params) as ApprovalRequestRow[];
|
|
|
|
for (const row of rows) {
|
|
const context = ApprovalRequestStore.normalizeTargetContext(row.targetContext);
|
|
if (context?.approvalDedupeKey === input.dedupeKey) {
|
|
return this.rowToRequest(row);
|
|
}
|
|
}
|
|
return null;
|
|
}
|
|
|
|
async decide(requestId: string, status: "approved" | "denied", input: ApprovalRequestDecisionInput): Promise<ApprovalRequest> {
|
|
return asyncApprovalRequestStore.decideApprovalRequest(this.asyncLayer!, requestId, status, input);
|
|
}
|
|
|
|
async markCompleted(requestId: string, input: ApprovalRequestCompletionInput): Promise<ApprovalRequest> {
|
|
return asyncApprovalRequestStore.markApprovalRequestCompleted(this.asyncLayer!, requestId, input);
|
|
}
|
|
|
|
async getAuditHistory(requestId: string): Promise<ApprovalRequestAuditEvent[]> {
|
|
return asyncApprovalRequestStore.getApprovalAuditHistory(this.asyncLayer!.db, requestId);
|
|
}
|
|
}
|