diff --git a/.changeset/tool-permission-gates-hardening.md b/.changeset/tool-permission-gates-hardening.md new file mode 100644 index 0000000000..806126b64d --- /dev/null +++ b/.changeset/tool-permission-gates-hardening.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": minor +--- + +summary: Approval and permission gates now enforce: self-approval blocked, bash containment, fn serve authenticated by default. +category: security +dev: "Full approval/permission hardening pass. Decision boundary: the dashboard approvals decision route derives the decider server-side (forged/non-user actors and self-approval 403), same-verdict replay and races 409 via transaction-guarded store updates, pending requests expire after 24h and approved grants after a configurable TTL (FUSION_APPROVAL_GRANT_TTL_MS, default 1h; lazy, no schema change), markCompleted enforces requester ownership. Gates: unclassified tools fail closed to policy-governed command_execution (default `unrestricted` preset behavior unchanged), an unconditional bash containment floor denies daemon-token/credential-store reads and shell calls to the approvals API at every preset, bash approvals bind to the exact command hash, the permanent-agent gate pauses on pending approvals, and agent provisioning approval is live in production (isCallerPrivileged is ceo-only). Extension tools resolve the acting principal via a session identity registry: destructive fn_* tools are withheld from agent principals and policy-gated otherwise; fn_secret_get approvals now actually redeem (execute-once) under category secrets_access. `fn serve` mints/reuses the daemon token by default (`--no-auth` opts out). Sibling entry points: `fn task move`/glasses gestures hard-cancel via moveSource \"user\", ACP approvals are execute-once, and plugin task stores block destructive methods unless the manifest declares `permissions: { destructiveTaskOps: true }`." diff --git a/packages/cli/src/__tests__/extension-experiment-finalize.test.ts b/packages/cli/src/__tests__/extension-experiment-finalize.test.ts index aa50319caa..c6fb2b8fb9 100644 --- a/packages/cli/src/__tests__/extension-experiment-finalize.test.ts +++ b/packages/cli/src/__tests__/extension-experiment-finalize.test.ts @@ -62,6 +62,11 @@ vi.mock("@fusion/core", () => ({ resolveAgentProvisioningPolicy: vi.fn(() => ({ approvalMode: "auto" })), TASK_PRIORITIES: ["low", "normal", "high", "urgent"], getProjectRootFromWorktree: vi.fn(() => null), + // FNXC:ToolPermissionGates 2026-07-26-14:55: fn_experiment_finalize is now withheld from agent + // principals; the guard resolves the caller principal via the session-identity registry. + // These tests call the tool as an operator, so the mock reports an operator principal. + resolveFusionSessionPrincipal: vi.fn(() => ({ kind: "operator" })), + resolveEffectiveAgentPermissionPolicy: vi.fn(() => ({ presetId: "unrestricted", rules: {} })), })); vi.mock("@fusion/dashboard", () => ({ @@ -77,6 +82,9 @@ vi.mock("@fusion/dashboard", () => ({ vi.mock("@fusion/engine", () => ({ installBaselineArchiveWorktreeDisposer: vi.fn(), ...workflowAuthoringEngineMock, + // FNXC:ToolPermissionGates 2026-07-26-14:55: extension.ts now imports the agent action gate; mock completeness gate requires these names. + evaluateAgentActionGate: vi.fn(() => ({ disposition: "allow", category: "exempt", toolName: "", operation: "", summary: "", resourceType: "other", approvalDedupeKey: "", metadata: {} })), + resolveGateOutcome: vi.fn(() => ({ outcome: "allow" })), createFnAgent: vi.fn(), createAgentTask: vi.fn(), fetchWebContent: vi.fn(), diff --git a/packages/cli/src/__tests__/extension-gitlab-tracking.test.ts b/packages/cli/src/__tests__/extension-gitlab-tracking.test.ts index 86f253c874..45f2da3dfc 100644 --- a/packages/cli/src/__tests__/extension-gitlab-tracking.test.ts +++ b/packages/cli/src/__tests__/extension-gitlab-tracking.test.ts @@ -37,6 +37,9 @@ vi.mock("@fusion/dashboard", () => { vi.mock("@fusion/engine", () => ({ installBaselineArchiveWorktreeDisposer: vi.fn(), + // FNXC:ToolPermissionGates 2026-07-26-14:55: extension.ts now imports the agent action gate; mock completeness gate requires these names. + evaluateAgentActionGate: vi.fn(() => ({ disposition: "allow", category: "exempt", toolName: "", operation: "", summary: "", resourceType: "other", approvalDedupeKey: "", metadata: {} })), + resolveGateOutcome: vi.fn(() => ({ outcome: "allow" })), createFnAgent: vi.fn(), createAgentTask: vi.fn(), fetchWebContent: vi.fn(), diff --git a/packages/cli/src/__tests__/extension-permission-gates.test.ts b/packages/cli/src/__tests__/extension-permission-gates.test.ts new file mode 100644 index 0000000000..84f1ecd7cc --- /dev/null +++ b/packages/cli/src/__tests__/extension-permission-gates.test.ts @@ -0,0 +1,463 @@ +/** + * FNXC:ToolPermissionGates 2026-07-26-14:20: + * Security-fix coverage for the host-extension tool permission gates. Root cause: all fn_* + * extension tools are delivered into engine agent sessions via pi's extension loader and + * never pass through the engine's gate wrappers, so destructive tools ran ungated for + * agents (an agent deleted a live task). These tests prove BOTH directions: + * - Agent principals (explicit ctx.agentId or session-identity-registry cwd match) are + * hard-denied on the withheld list and policy-gated on the sensitive list. + * - Operator principals (no ctx.agentId, no registry entry) keep their exact prior + * behavior, and agents under the shipped default `unrestricted` preset stay + * friction-free on policy-gated tools (no approval row minted). + * Expectations are HARDCODED — never derived from the constants under test. + */ +import { afterAll, afterEach, beforeAll, beforeEach, expect, it } from "vitest"; +import { join } from "node:path"; +import { + AgentStore, + ApprovalRequestStore, + SecretsStore, + registerFusionSessionIdentity, + __clearFusionSessionIdentityRegistryForTests, + type AgentPermissionPolicy, +} from "@fusion/core"; +import { + createPgExtensionHarness, + createMockApi, + registerExtension, + requireTool, + pgDescribe, + type MockApi, +} from "./pg-extension-harness.js"; + +const h = createPgExtensionHarness("fn-ext-perm-gates"); + +function buildApprovalStore(): ApprovalRequestStore { + const layer = h.store().getAsyncLayer(); + if (!layer) throw new Error("harness store has no async layer"); + return new ApprovalRequestStore(null, { asyncLayer: layer }); +} + +async function buildAgentStore(): Promise { + const layer = h.store().getAsyncLayer(); + if (!layer) throw new Error("harness store has no async layer"); + const agentStore = new AgentStore({ rootDir: join(h.rootDir(), ".fusion"), asyncLayer: layer }); + await agentStore.init(); + return agentStore; +} + +/** + * FNXC:ToolPermissionGates 2026-07-26-14:20: + * TaskStore.getSecretsStore constructs a MasterKeyManager against the real global dir, + * which resolveGlobalDir hard-refuses under vitest. Pre-seed the store's public + * `secretsStore` cache with a backend-mode SecretsStore using a fixed in-memory test key + * so fn_secret_get exercises the real encrypt/reveal + approval paths without touching + * ~/.fusion. + */ +function injectSecretsStore(): SecretsStore { + const layer = h.store().getAsyncLayer(); + if (!layer) throw new Error("harness store has no async layer"); + const noopDb = { + prepare: () => { + throw new Error("sync DB not available in backend-mode test"); + }, + bumpLastModified: () => {}, + }; + const secretsStore = new SecretsStore( + noopDb as never, + noopDb as never, + async () => Buffer.alloc(32, 7), + { asyncLayer: layer }, + ); + h.store().secretsStore = secretsStore; + return secretsStore; +} + +/** Hardcoded full-rules policy literals (never derived from core preset constants). */ +const LOCKED_DOWN_POLICY: AgentPermissionPolicy = { + presetId: "locked-down", + rules: { + git_write: "block", + file_write_delete: "block", + command_execution: "block", + network_api: "block", + task_agent_mutation: "block", + review_gate_bypass: "block", + file_scope: "block", + }, +}; + +const APPROVAL_REQUIRED_POLICY: AgentPermissionPolicy = { + presetId: "approval-required", + rules: { + git_write: "require-approval", + file_write_delete: "require-approval", + command_execution: "require-approval", + network_api: "require-approval", + task_agent_mutation: "require-approval", + review_gate_bypass: "require-approval", + file_scope: "require-approval", + }, +}; + +function freshApi(): MockApi { + const api = createMockApi(); + registerExtension(api); + return api; +} + +pgDescribe("extension tool permission gates", () => { + beforeAll(h.beforeAll); + beforeEach(async () => { + await h.beforeEach(); + __clearFusionSessionIdentityRegistryForTests(); + }); + afterEach(async () => { + __clearFusionSessionIdentityRegistryForTests(); + await h.afterEach(); + }); + afterAll(h.afterAll); + + // ── Withheld list ──────────────────────────────────────────────── + + it("fn_task_delete: denied for agent principal (task untouched), allowed for operator", async () => { + const cwd = h.rootDir(); + const api = freshApi(); + const tool = requireTool(api, "fn_task_delete"); + const task = await h.store().createTask({ description: "withheld delete target" }); + + const denied = await tool.execute("c1", { id: task.id }, undefined, undefined, { cwd, agentId: "agent-rogue" }); + expect(denied.isError).toBe(true); + expect(denied.details?.deniedFor).toBe("agent-principal"); + expect(denied.details?.tool).toBe("fn_task_delete"); + expect(denied.details?.agentId).toBe("agent-rogue"); + expect(denied.content[0]?.text).toContain("withheld from agent sessions"); + + // The store delete was never invoked: the task row is still live. + const stillAlive = await h.store().getTask(task.id, { includeDeleted: true }); + expect(stillAlive.deletedAt ?? null).toBeNull(); + + // Operator (no agentId, no registry entry) proceeds unchanged. + const ok = await tool.execute("c2", { id: task.id }, undefined, undefined, { cwd }); + expect(ok.isError).toBeUndefined(); + expect(ok.content[0]?.text).toBe(`Deleted ${task.id}`); + const deleted = await h.store().getTask(task.id, { includeDeleted: true }); + expect(deleted.deletedAt).toBeTruthy(); + }); + + it("fn_task_delete: registry-registered session is denied without ctx.agentId; ambiguous fails closed", async () => { + const cwd = h.rootDir(); + const api = freshApi(); + const tool = requireTool(api, "fn_task_delete"); + const task = await h.store().createTask({ description: "registry-denied delete target" }); + + const dispose = registerFusionSessionIdentity(cwd, { agentId: "agent-registered" }); + const denied = await tool.execute("c1", { id: task.id }, undefined, undefined, { cwd }); + expect(denied.isError).toBe(true); + expect(denied.details?.deniedFor).toBe("agent-principal"); + expect(denied.details?.agentId).toBe("agent-registered"); + + // Two live registrations for one cwd = ambiguous = still denied (fail closed), no agentId attributed. + const dispose2 = registerFusionSessionIdentity(cwd, { agentId: "agent-second" }); + const ambiguous = await tool.execute("c2", { id: task.id }, undefined, undefined, { cwd }); + expect(ambiguous.isError).toBe(true); + expect(ambiguous.details?.deniedFor).toBe("agent-principal"); + expect(ambiguous.details?.agentId).toBeUndefined(); + + dispose(); + dispose2(); + + // After both sessions dispose, the same cwd is an operator again. + const ok = await tool.execute("c3", { id: task.id }, undefined, undefined, { cwd }); + expect(ok.isError).toBeUndefined(); + }); + + it("every withheld tool hard-denies an agent principal before doing any work", async () => { + const cwd = h.rootDir(); + const api = freshApi(); + // Hardcoded tool/param pairs — params are irrelevant because the guard runs first. + const calls: Array<[string, Record]> = [ + ["fn_task_bypass_review", { id: "FN-1", reason: "nope" }], + ["fn_mission_delete", { id: "M-1" }], + ["fn_milestone_delete", { milestoneId: "MS-1" }], + ["fn_slice_delete", { sliceId: "SL-1" }], + ["fn_feature_delete", { featureId: "F-1" }], + ["fn_workflow_delete", { workflow_id: "WF-1" }], + ["fn_experiment_finalize", { sessionId: "EXP-1" }], + ["fn_skills_install", { source: "owner/repo" }], + ]; + for (const [name, params] of calls) { + const tool = requireTool(api, name); + const result = await tool.execute("c", params, undefined, undefined, { cwd, agentId: "agent-rogue" }); + expect(result.isError, `${name} should be withheld`).toBe(true); + expect(result.details?.deniedFor, name).toBe("agent-principal"); + expect(result.details?.tool, name).toBe(name); + expect(result.details?.agentId, name).toBe("agent-rogue"); + } + }); + + // ── Policy-gated list ──────────────────────────────────────────── + + it("default (unrestricted) preset: agent fn_task_pause proceeds with NO approval row", async () => { + const cwd = h.rootDir(); + const api = freshApi(); + const tool = requireTool(api, "fn_task_pause"); + const agentStore = await buildAgentStore(); + const worker = await agentStore.createAgent({ name: "Default Worker", role: "executor" }); + const task = await h.store().createTask({ description: "default-policy pause target" }); + + const result = await tool.execute("c1", { id: task.id }, undefined, undefined, { cwd, agentId: worker.id }); + expect(result.isError).toBeUndefined(); + expect(result.content[0]?.text).toBe(`Paused ${task.id}`); + const paused = await h.store().getTask(task.id); + expect(paused.paused).toBe(true); + + // DEFAULT PRESET PATH must stay friction-free: no approval request was minted. + const requests = await buildApprovalStore().list(); + expect(requests).toHaveLength(0); + }); + + it("locked-down agent policy blocks fn_task_pause", async () => { + const cwd = h.rootDir(); + const api = freshApi(); + const tool = requireTool(api, "fn_task_pause"); + const agentStore = await buildAgentStore(); + const locked = await agentStore.createAgent({ + name: "Locked Worker", + role: "executor", + permissionPolicy: LOCKED_DOWN_POLICY, + }); + const task = await h.store().createTask({ description: "locked-down pause target" }); + + const result = await tool.execute("c1", { id: task.id }, undefined, undefined, { cwd, agentId: locked.id }); + expect(result.isError).toBe(true); + expect(result.details?.deniedFor).toBe("agent-permission-policy"); + expect(result.details?.disposition).toBe("block"); + expect(result.details?.agentId).toBe(locked.id); + + const untouched = await h.store().getTask(task.id); + expect(untouched.paused ?? false).toBe(false); + + // Operator remains unaffected by the agent-row policy. + const operatorResult = await tool.execute("c2", { id: task.id }, undefined, undefined, { cwd }); + expect(operatorResult.isError).toBeUndefined(); + expect(operatorResult.content[0]?.text).toBe(`Paused ${task.id}`); + }); + + it("approval-required policy: mints agent-attributed request, reuses pending, redeems approval once", async () => { + const cwd = h.rootDir(); + const api = freshApi(); + const tool = requireTool(api, "fn_task_pause"); + const agentStore = await buildAgentStore(); + const gatedAgent = await agentStore.createAgent({ + name: "Gated Worker", + role: "executor", + permissionPolicy: APPROVAL_REQUIRED_POLICY, + }); + const task = await h.store().createTask({ description: "approval-required pause target" }); + const approvals = buildApprovalStore(); + + const first = await tool.execute("c1", { id: task.id }, undefined, undefined, { cwd, agentId: gatedAgent.id }); + expect(first.isError).toBeUndefined(); + expect(first.details?.outcome).toBe("pending_approval"); + const requestId = first.details?.approvalRequestId as string; + expect(requestId).toBeTruthy(); + + const request = await approvals.get(requestId); + expect(request?.status).toBe("pending"); + expect(request?.requester.actorType).toBe("agent"); + expect(request?.requester.actorId).toBe(gatedAgent.id); + expect(request?.requester.actorName).toBe("Gated Worker"); + expect(request?.targetAction.category).toBe("task_agent_mutation"); + + // Second call while pending reuses the same request — no duplicate row. + const second = await tool.execute("c2", { id: task.id }, undefined, undefined, { cwd, agentId: gatedAgent.id }); + expect(second.details?.outcome).toBe("pending_approval"); + expect(second.details?.approvalRequestId).toBe(requestId); + expect(await approvals.list()).toHaveLength(1); + + // Task was never paused while the request is pending. + expect((await h.store().getTask(task.id)).paused ?? false).toBe(false); + + // Operator approves → the next call consumes the grant exactly once and proceeds. + await approvals.decide(requestId, "approved", { + actor: { actorId: "user", actorType: "user", actorName: "Operator" }, + }); + const third = await tool.execute("c3", { id: task.id }, undefined, undefined, { cwd, agentId: gatedAgent.id }); + expect(third.isError).toBeUndefined(); + expect(third.content[0]?.text).toBe(`Paused ${task.id}`); + expect((await h.store().getTask(task.id)).paused).toBe(true); + expect((await approvals.get(requestId))?.status).toBe("completed"); + + // Grant is consumed: a fourth call mints a NEW pending request instead of re-running. + const fourth = await tool.execute("c4", { id: task.id }, undefined, undefined, { cwd, agentId: gatedAgent.id }); + expect(fourth.details?.outcome).toBe("pending_approval"); + expect(fourth.details?.approvalRequestId).not.toBe(requestId); + }); + + // ── Provisioning caller honesty ────────────────────────────────── + + it("fn_agent_create: operator stays privileged and unchanged; agent caller takes the approval path with a real requester snapshot", async () => { + const cwd = h.rootDir(); + const api = freshApi(); + const createTool = requireTool(api, "fn_agent_create"); + const agentStore = await buildAgentStore(); + const boss = await agentStore.createAgent({ name: "Boss Agent", role: "executor" }); + + // Operator behavior is hardcoded-unchanged: privileged caller, immediate create. + const operatorResult = await createTool.execute( + "c1", + { name: "Operator Made", role: "executor" }, + undefined, + undefined, + { cwd }, + ); + expect(operatorResult.details?.outcome).toBe("created"); + expect(operatorResult.details?.matchedRule).toBe("privileged-caller"); + + // Agent caller is NOT privileged: default trusted-only mode requires approval, + // and the request is attributed to the real agent, not "CLI User". + const agentResult = await createTool.execute( + "c2", + { name: "Agent Made", role: "executor" }, + undefined, + undefined, + { cwd, agentId: boss.id }, + ); + expect(agentResult.details?.outcome).toBe("pending_approval"); + expect(agentResult.details?.matchedRule).toBe("approval-mode-trusted-only"); + const request = await buildApprovalStore().get(agentResult.details?.approvalRequestId as string); + expect(request?.requester.actorType).toBe("agent"); + expect(request?.requester.actorId).toBe(boss.id); + expect(request?.requester.actorName).toBe("Boss Agent"); + expect(request?.targetAction.category).toBe("agent_provisioning"); + }); + + it("fn_agent_delete: agent caller approval request carries the agent requester snapshot; operator delete-approval keeps CLI User", async () => { + const cwd = h.rootDir(); + const api = freshApi(); + const deleteTool = requireTool(api, "fn_agent_delete"); + const agentStore = await buildAgentStore(); + const boss = await agentStore.createAgent({ name: "Boss Agent", role: "executor" }); + const victim = await agentStore.createAgent({ name: "Victim Agent", role: "executor" }); + + // Agent caller → non-privileged → alwaysApproveDelete default → approval with real snapshot. + const agentResult = await deleteTool.execute( + "c1", + { agent_id: victim.id }, + undefined, + undefined, + { cwd, agentId: boss.id }, + ); + expect(agentResult.details?.outcome).toBe("pending_approval"); + const agentRequest = await buildApprovalStore().get(agentResult.details?.approvalRequestId as string); + expect(agentRequest?.requester.actorType).toBe("agent"); + expect(agentRequest?.requester.actorId).toBe(boss.id); + expect(agentRequest?.requester.actorName).toBe("Boss Agent"); + + // Operator remains privileged and deletes immediately (hardcoded prior behavior). + const operatorResult = await deleteTool.execute("c2", { agent_id: victim.id }, undefined, undefined, { cwd }); + expect(operatorResult.details?.outcome).toBe("deleted"); + }); + + // ── fn_secret_get approval lifecycle ───────────────────────────── + + it("fn_secret_get: approved row is redeemed once (reveal + completed), then a fresh request is minted", async () => { + const cwd = h.rootDir(); + const api = freshApi(); + const tool = requireTool(api, "fn_secret_get"); + const secretsStore = injectSecretsStore(); + await secretsStore.createSecret({ + scope: "project", + key: "API_TOKEN", + plaintextValue: "s3cret-value", + accessPolicy: "prompt", + }); + const approvals = buildApprovalStore(); + const agentCtx = { cwd, agentId: "agent-secrets", agentName: "Secrets Agent" }; + + // First call mints a pending request with the secrets_access category (dashboard audit hook contract). + const first = await tool.execute("c1", { key: "API_TOKEN" }, undefined, undefined, agentCtx); + expect(first.details?.outcome).toBe("pending_approval"); + const requestId = first.details?.approvalRequestId as string; + const request = await approvals.get(requestId); + expect(request?.targetAction.category).toBe("secrets_access"); + expect(request?.requester.actorId).toBe("agent-secrets"); + + // While pending: no re-mint. + const stillPending = await tool.execute("c2", { key: "API_TOKEN" }, undefined, undefined, agentCtx); + expect(stillPending.details?.outcome).toBe("pending_approval"); + expect(stillPending.details?.approvalRequestId).toBe(requestId); + expect(await approvals.list()).toHaveLength(1); + + // Approve → redemption: the secret is revealed and the grant is consumed (completed). + await approvals.decide(requestId, "approved", { + actor: { actorId: "user", actorType: "user", actorName: "Operator" }, + }); + const redeemed = await tool.execute("c3", { key: "API_TOKEN" }, undefined, undefined, agentCtx); + expect(redeemed.isError).toBeUndefined(); + expect(redeemed.details?.value).toBe("s3cret-value"); + expect(redeemed.details?.approvalRequestId).toBe(requestId); + expect((await approvals.get(requestId))?.status).toBe("completed"); + + // Grant already redeemed → the next call mints a brand-new request. + const afterRedeem = await tool.execute("c4", { key: "API_TOKEN" }, undefined, undefined, agentCtx); + expect(afterRedeem.details?.outcome).toBe("pending_approval"); + expect(afterRedeem.details?.approvalRequestId).not.toBe(requestId); + }); + + it("fn_secret_get: denied row stays denied without minting a new request", async () => { + const cwd = h.rootDir(); + const api = freshApi(); + const tool = requireTool(api, "fn_secret_get"); + const secretsStore = injectSecretsStore(); + await secretsStore.createSecret({ + scope: "project", + key: "DENIED_TOKEN", + plaintextValue: "never-shown", + accessPolicy: "prompt", + }); + const approvals = buildApprovalStore(); + const agentCtx = { cwd, agentId: "agent-denied", agentName: "Denied Agent" }; + + const first = await tool.execute("c1", { key: "DENIED_TOKEN" }, undefined, undefined, agentCtx); + const requestId = first.details?.approvalRequestId as string; + await approvals.decide(requestId, "denied", { + actor: { actorId: "user", actorType: "user", actorName: "Operator" }, + }); + + const second = await tool.execute("c2", { key: "DENIED_TOKEN" }, undefined, undefined, agentCtx); + expect(second.details?.outcome).toBe("denied"); + expect(second.details?.approvalRequestId).toBe(requestId); + expect(second.details?.value).toBeUndefined(); + // No new request was minted for the denied grant. + expect(await approvals.list()).toHaveLength(1); + }); + + // ── fn_task_retry move source ──────────────────────────────────── + + it("fn_task_retry moves with the user/hard-cancel move source", async () => { + const cwd = h.rootDir(); + const api = freshApi(); + const tool = requireTool(api, "fn_task_retry"); + const task = await h.store().createTask({ description: "retry source target", column: "triage" }); + await h.store().updateTask(task.id, { status: "failed", error: "boom" }); + + const moves: Array<{ to: string; source: string }> = []; + const onMoved = (data: { to: string; source: string }) => { + moves.push({ to: data.to, source: data.source }); + }; + h.store().on("task:moved", onMoved as never); + try { + const result = await tool.execute("c1", { id: task.id }, undefined, undefined, { cwd }); + expect(result.isError).toBeUndefined(); + } finally { + h.store().off("task:moved", onMoved as never); + } + + const todoMove = moves.find((m) => m.to === "todo"); + expect(todoMove).toBeTruthy(); + expect(todoMove?.source).toBe("user"); + expect((await h.store().getTask(task.id)).column).toBe("todo"); + }); +}); diff --git a/packages/cli/src/__tests__/extension-web-fetch.test.ts b/packages/cli/src/__tests__/extension-web-fetch.test.ts index b6382dbc48..f645629de8 100644 --- a/packages/cli/src/__tests__/extension-web-fetch.test.ts +++ b/packages/cli/src/__tests__/extension-web-fetch.test.ts @@ -16,6 +16,9 @@ vi.mock("@fusion/dashboard", () => ({ vi.mock("@fusion/engine", () => ({ installBaselineArchiveWorktreeDisposer: vi.fn(), ...workflowAuthoringEngineMock, + // FNXC:ToolPermissionGates 2026-07-26-14:55: extension.ts now imports the agent action gate; mock completeness gate requires these names. + evaluateAgentActionGate: vi.fn(() => ({ disposition: "allow", category: "exempt", toolName: "", operation: "", summary: "", resourceType: "other", approvalDedupeKey: "", metadata: {} })), + resolveGateOutcome: vi.fn(() => ({ outcome: "allow" })), createFnAgent: vi.fn(), createAgentTask: vi.fn(), fetchWebContent: fetchWebContentMock, diff --git a/packages/cli/src/__tests__/serve-daemon-token.test.ts b/packages/cli/src/__tests__/serve-daemon-token.test.ts new file mode 100644 index 0000000000..47e86521e1 --- /dev/null +++ b/packages/cli/src/__tests__/serve-daemon-token.test.ts @@ -0,0 +1,79 @@ +// @vitest-environment node + +/* +FNXC:ServeSecureByDefault 2026-07-26-17:35: +`fn serve` must be authenticated BY DEFAULT: resolveServeDaemonToken returns a token in +every configuration except the explicit `--no-auth` opt-out. A plain `fn serve` used to +resolve no token (only `--daemon` did), leaving the entire API — including the +approval-decision route — unauthenticated. Resolution priority (matching fn daemon / +fn dashboard): env FUSION_DAEMON_TOKEN > stored token (getOrCreateToken) > getToken() +> generateToken(). Pure DI unit tests — no disk, no settings store, no server. +*/ + +import { describe, it, expect, vi } from "vitest"; +import { resolveServeDaemonToken, type ServeTokenManagerLike } from "../commands/serve-daemon-token.js"; + +function makeManager(overrides: Partial = {}): ServeTokenManagerLike { + return { + getToken: vi.fn(async () => undefined), + generateToken: vi.fn(async () => "generated-token"), + ...overrides, + }; +} + +describe("resolveServeDaemonToken", () => { + it("returns undefined ONLY for the explicit --no-auth opt-out, even when an env token exists", async () => { + const manager = makeManager(); + const token = await resolveServeDaemonToken( + { noAuth: true }, + { env: { FUSION_DAEMON_TOKEN: "env-token" }, createTokenManager: () => manager }, + ); + + expect(token).toBeUndefined(); + expect(manager.getToken).not.toHaveBeenCalled(); + expect(manager.generateToken).not.toHaveBeenCalled(); + }); + + it("prefers the FUSION_DAEMON_TOKEN env var over the stored token", async () => { + const manager = makeManager({ getOrCreateToken: vi.fn(async () => "stored-token") }); + const token = await resolveServeDaemonToken( + {}, + { env: { FUSION_DAEMON_TOKEN: "env-token" }, createTokenManager: () => manager }, + ); + + expect(token).toBe("env-token"); + expect(manager.getOrCreateToken).not.toHaveBeenCalled(); + }); + + it("uses getOrCreateToken when the manager provides it (dashboard parity)", async () => { + const manager = makeManager({ getOrCreateToken: vi.fn(async () => "stored-or-minted-token") }); + const token = await resolveServeDaemonToken({}, { env: {}, createTokenManager: () => manager }); + + expect(token).toBe("stored-or-minted-token"); + expect(manager.getToken).not.toHaveBeenCalled(); + expect(manager.generateToken).not.toHaveBeenCalled(); + }); + + it("falls back to an existing stored token when getOrCreateToken is unavailable", async () => { + const manager = makeManager({ getToken: vi.fn(async () => "legacy-stored-token") }); + const token = await resolveServeDaemonToken({}, { env: {}, createTokenManager: () => manager }); + + expect(token).toBe("legacy-stored-token"); + expect(manager.generateToken).not.toHaveBeenCalled(); + }); + + it("mints and persists a new token when nothing is configured — plain `fn serve` is authenticated by default", async () => { + const manager = makeManager(); + const token = await resolveServeDaemonToken({}, { env: {}, createTokenManager: () => manager }); + + expect(token).toBe("generated-token"); + expect(manager.generateToken).toHaveBeenCalledTimes(1); + }); + + it("resolves a token when noAuth is absent/false (never silently unauthenticated)", async () => { + const manager = makeManager(); + const token = await resolveServeDaemonToken({ noAuth: false }, { env: {}, createTokenManager: () => manager }); + + expect(token).toBe("generated-token"); + }); +}); diff --git a/packages/cli/src/__tests__/task-delete-allow-resurrection.test.ts b/packages/cli/src/__tests__/task-delete-allow-resurrection.test.ts index 0af1bd2efa..ee288a2034 100644 --- a/packages/cli/src/__tests__/task-delete-allow-resurrection.test.ts +++ b/packages/cli/src/__tests__/task-delete-allow-resurrection.test.ts @@ -58,6 +58,15 @@ pgTest("task delete allowResurrection plumbing", () => { expect(deleted.allowResurrection).toBeUndefined(); }); + /* + FNXC:ToolPermissionGates 2026-07-26-14:55: + INTENDED BEHAVIOR CHANGE: fn_task_delete is now hard-withheld from agent principals + (a ctx.agentId caller is denied before the store is touched — covered in + extension-permission-gates.test.ts). The two task-bound-caller tests below therefore + exercise the store-level self-delete guard and cross-task delete as OPERATOR contexts + (taskId/runId without agentId), which is the only principal that can still reach + store.deleteTask through this tool. + */ it("fn_task_delete rejects deleting the caller task and leaves it live", async () => { const store = h.store(); const task = await store.createTask({ title: "self", description: "current task", column: "in-progress" }); @@ -69,7 +78,6 @@ pgTest("task delete allowResurrection plumbing", () => { const result = await tool.execute("call-self", { id: task.id }, undefined, undefined, { cwd: h.rootDir(), taskId: task.id, - agentId: "agent-test", runId: "run-test", }); expect(result.isError).toBe(true); @@ -90,7 +98,6 @@ pgTest("task delete allowResurrection plumbing", () => { const result = await tool.execute("call-other", { id: target.id }, undefined, undefined, { cwd: h.rootDir(), taskId: caller.id, - agentId: "agent-test", runId: "run-test", }); diff --git a/packages/cli/src/bin.ts b/packages/cli/src/bin.ts index dfdd7ff884..f5391d2caa 100644 --- a/packages/cli/src/bin.ts +++ b/packages/cli/src/bin.ts @@ -310,7 +310,7 @@ Usage: fn dashboard --dev Start dashboard in development mode fn dashboard --no-engine Start web UI only (no AI engine) fn dashboard --interactive Start with interactive port selection - fn serve [--port ] [--host ] [--paused] [--daemon] [--project ] [--no-auto-register] + fn serve [--port ] [--host ] [--paused] [--daemon] [--no-auth] [--project ] [--no-auto-register] Start Fusion as a headless node (API + engine, no UI) Auto-registers cwd project on first run (use --no-auto-register to disable) fn daemon [--port ] [--host ] [--token ] [--paused] [--token-only] [--project ] [--no-auto-register] @@ -495,7 +495,7 @@ Options: --port, -p Dashboard/serve port (default: 4040) --host Serve host (default: 127.0.0.1 — localhost only; pass 0.0.0.0 to expose) --token Dashboard/daemon bearer token. Default: $FUSION_DASHBOARD_TOKEN, $FUSION_DAEMON_TOKEN, or auto-generated. - --no-auth Disable dashboard bearer-token auth for dashboard/desktop (local-only; not recommended on 0.0.0.0) + --no-auth Disable bearer-token auth for dashboard/desktop/serve (local-only; not recommended on 0.0.0.0) --interactive Interactive mode (port selection for dashboard, issue selection for import) --paused Start with engine paused (automation disabled) --dev Start dashboard in development mode @@ -928,9 +928,12 @@ async function main() { const hostIdx = args.indexOf("--host"); const host = hostIdx !== -1 && hostIdx + 1 < args.length ? args[hostIdx + 1] : undefined; const daemon = args.includes("--daemon"); + // FNXC:ServeSecureByDefault 2026-07-26-17:00: `fn serve` is authenticated by + // default; `--no-auth` is the explicit local-trust opt-out (mirrors dashboard). + const noAuth = args.includes("--no-auth"); const project = getFlagValue(args, "--project"); const noAutoRegister = args.includes("--no-auto-register"); - await runServe(port, { paused, interactive, host, daemon, project, noAutoRegister }); + await runServe(port, { paused, interactive, host, daemon, noAuth, project, noAutoRegister }); break; } diff --git a/packages/cli/src/commands/__tests__/serve.test.ts b/packages/cli/src/commands/__tests__/serve.test.ts index 5e36f724ad..12af1abb7a 100644 --- a/packages/cli/src/commands/__tests__/serve.test.ts +++ b/packages/cli/src/commands/__tests__/serve.test.ts @@ -2009,22 +2009,32 @@ describe("runServe --daemon flag", () => { await triggerSignal("SIGINT"); }); - it("does not pass daemon to createServer when daemon: false", async () => { + /* + FNXC:ServeSecureByDefault 2026-07-30-14:15: + INVERTED DELIBERATELY. These two cases asserted that `fn serve` passes no daemon token unless + `--daemon` was given — the pre-hardening contract, where a plain `fn serve` listened unauthenticated. + Token resolution is now UNCONDITIONAL (mirroring the existing `fn dashboard` precedent), so the old + assertions were pinning the vulnerability rather than the behaviour. + + The pair is kept rather than deleted, because the opt-out is the part worth guarding: if `--no-auth` + ever stops disabling auth, or the default ever stops minting, one of these fails. + */ + it("mints a daemon token by default, with no --daemon flag", async () => { const { createServer } = await import("@fusion/dashboard"); - await runServe(4040, { daemon: false }); + await runServe(4040, {}); expect(createServer).toHaveBeenCalledTimes(1); const serverOpts = createServer.mock.calls[0][1]; - expect(serverOpts.daemon).toBeUndefined(); + expect(serverOpts.daemon?.token).toEqual(expect.any(String)); await triggerSignal("SIGINT"); }); - it("does not pass daemon to createServer when daemon option is omitted", async () => { + it("passes no daemon token when --no-auth opts out", async () => { const { createServer } = await import("@fusion/dashboard"); - await runServe(4040, {}); + await runServe(4040, { noAuth: true }); expect(createServer).toHaveBeenCalledTimes(1); const serverOpts = createServer.mock.calls[0][1]; diff --git a/packages/cli/src/commands/__tests__/task-lock-retry.test.ts b/packages/cli/src/commands/__tests__/task-lock-retry.test.ts index 471d9938f6..546c0dcbb8 100644 --- a/packages/cli/src/commands/__tests__/task-lock-retry.test.ts +++ b/packages/cli/src/commands/__tests__/task-lock-retry.test.ts @@ -251,7 +251,13 @@ describe("runTaskShow / runTaskMove — mocked-store lock exhaustion, not-found, await mod.runTaskMove("FN-5", "todo"); expect(moveTask).toHaveBeenCalledTimes(1); - expect(moveTask).toHaveBeenCalledWith("FN-5", "todo"); + /* + FNXC:ToolPermissionGates 2026-07-30-14:05: + A CLI move IS a user action, so it now carries `moveSource: "user"` — the store's move pipeline + applies user-move semantics (and the dashboard move route already did this). Asserting the bare + two-arg call pinned the pre-hardening shape. + */ + expect(moveTask).toHaveBeenCalledWith("FN-5", "todo", { moveSource: "user" }); expect(closeProjectStore).toHaveBeenCalled(); logSpy.mockRestore(); }); diff --git a/packages/cli/src/commands/__tests__/task.test.ts b/packages/cli/src/commands/__tests__/task.test.ts index 8b73cbd248..ae4002b6ec 100644 --- a/packages/cli/src/commands/__tests__/task.test.ts +++ b/packages/cli/src/commands/__tests__/task.test.ts @@ -1176,7 +1176,53 @@ describe("project-aware task command behavior", () => { await runTaskMove("FN-123", "done", "demo-project"); expect(resolveProject).toHaveBeenCalledWith("demo-project"); - expect(mockMoveTask).toHaveBeenCalledWith("FN-123", "done"); + // FNXC:TaskMovement 2026-07-26-12:35: `fn task move` is a human board action and + // must carry the user move source so user-move semantics (hard cancel) apply. + expect(mockMoveTask).toHaveBeenCalledWith("FN-123", "done", { moveSource: "user" }); + }); + + it("runTaskMove passes the user source through to the task-move disposer seam (hard cancel)", async () => { + /* + FNXC:TaskMovement 2026-07-26-12:35: + Regression coverage for the moveSource hard-cancel gap: only user-source + in-progress → todo moves run disposeTaskBeforeMove. The fake store forwards + the CLI-provided moveSource into the REAL core disposer seam (the from/todo + columns are pinned by the harness because this file's @fusion/core mock + replaces COLUMNS with a fixture list that has no "todo"), so this fails if + runTaskMove ever drops `moveSource: "user"` again — the disposer would not + fire and the agent session would keep running behind a Todo card. + */ + const { disposeTaskBeforeMove, registerTaskMoveDisposer } = await import("@fusion/core"); + const disposer = vi.fn().mockResolvedValue(undefined); + const fakeStore = { + moveTask: vi.fn( + async (id: string, column: string, options?: { moveSource?: "user" | "engine" | "scheduler" }) => { + const task = makeTask({ id, column: "in-progress" }); + await disposeTaskBeforeMove(fakeStore as unknown as TaskStore, { + task: task as never, + from: "in-progress", + to: "todo", + // Mirrors moves.ts: an absent moveSource defaults to "engine". + source: options?.moveSource ?? "engine", + }); + return makeTask({ id, column }); + }, + ), + }; + registerTaskMoveDisposer(fakeStore as unknown as TaskStore, disposer); + + vi.mocked(resolveProject).mockResolvedValue({ + projectId: "proj_test", + projectPath: "/test", + projectName: "demo-project", + isRegistered: true, + store: fakeStore as unknown as TaskStore, + }); + + await runTaskMove("FN-123", "in-progress", "demo-project"); + + expect(disposer).toHaveBeenCalledOnce(); + expect(disposer).toHaveBeenCalledWith(expect.objectContaining({ id: "FN-123" })); }); it("runTaskAttach uses resolved project store when project name is provided", async () => { diff --git a/packages/cli/src/commands/serve-daemon-token.ts b/packages/cli/src/commands/serve-daemon-token.ts new file mode 100644 index 0000000000..c584b418ea --- /dev/null +++ b/packages/cli/src/commands/serve-daemon-token.ts @@ -0,0 +1,68 @@ +import { DaemonTokenManager, GlobalSettingsStore, resolveGlobalDir } from "@fusion/core"; + +/* +FNXC:ServeSecureByDefault 2026-07-26-16:50: +`fn serve` used to resolve a bearer token only when `--daemon` was passed, so a plain +`fn serve` exposed the entire API — including the approval-decision route an AI agent +used to self-approve deleting a live task — unauthenticated to anything that could reach +the socket. This module makes token resolution unconditional: serve ALWAYS resolves (or +mints and persists) a daemon token unless the operator explicitly opts out with +`--no-auth`. Resolution priority matches `fn daemon` / `fn dashboard` +(resolveDashboardAuthToken): env FUSION_DAEMON_TOKEN > stored global-settings token > +newly generated persisted token. + +Kept as a small standalone module (not inside serve.ts) so the resolution policy is +unit-testable without importing serve's heavy dashboard/engine module graph, and so the +token-manager seam can be injected by tests instead of mocking @fusion/core. +*/ + +/** Minimal token-manager seam; mirrors DaemonTokenManager's surface used here. */ +export interface ServeTokenManagerLike { + getOrCreateToken?: () => Promise; + getToken(): Promise; + generateToken(): Promise; +} + +export interface ResolveServeDaemonTokenDeps { + /** Environment source; defaults to process.env. Injectable for tests. */ + env?: Pick; + /** Token-manager factory; defaults to the real global-settings-backed manager. */ + createTokenManager?: () => ServeTokenManagerLike; +} + +function createDefaultTokenManager(): ServeTokenManagerLike { + const globalDir = resolveGlobalDir(); + const settingsStore = new GlobalSettingsStore(globalDir); + return new DaemonTokenManager(settingsStore); +} + +/** + * FNXC:ServeSecureByDefault 2026-07-26-16:50: + * Resolve the bearer token `fn serve` will install auth with. Returns undefined ONLY + * when the operator explicitly passed `--no-auth`; every other path yields a token so + * the served API is authenticated by default. + */ +export async function resolveServeDaemonToken( + opts: { noAuth?: boolean }, + deps: ResolveServeDaemonTokenDeps = {}, +): Promise { + if (opts.noAuth) { + return undefined; + } + + const envToken = (deps.env ?? process.env).FUSION_DAEMON_TOKEN; + if (envToken) { + return envToken; + } + + const tokenManager = (deps.createTokenManager ?? createDefaultTokenManager)(); + if (typeof tokenManager.getOrCreateToken === "function") { + return tokenManager.getOrCreateToken(); + } + + const existingToken = await tokenManager.getToken(); + if (existingToken) { + return existingToken; + } + return tokenManager.generateToken(); +} diff --git a/packages/cli/src/commands/serve.ts b/packages/cli/src/commands/serve.ts index caf6b45d1e..243bcceede 100644 --- a/packages/cli/src/commands/serve.ts +++ b/packages/cli/src/commands/serve.ts @@ -18,9 +18,6 @@ import { getTaskMergeBlocker, INSIGHT_EXTRACTION_SCHEDULE_NAME, processAndAuditInsightExtraction, - DaemonTokenManager, - GlobalSettingsStore, - resolveGlobalDir, getEnabledPiExtensionPaths, mergeBuiltInGrokProviderModels, mergeBuiltInZaiProviderModels, @@ -40,6 +37,7 @@ import { refreshFusionModelRegistry, } from "@fusion/engine"; import { setHostTaskStore, clearHostTaskStores } from "../extension.js"; +import { resolveServeDaemonToken } from "./serve-daemon-token.js"; import { DefaultPackageManager, SettingsManager, @@ -243,7 +241,9 @@ function ensureProcessDiagnostics(): void { export async function runServe( port: number, - opts: { interactive?: boolean; paused?: boolean; host?: string; daemon?: boolean; noAutoRegister?: boolean; project?: string } = {}, + // FNXC:ServeSecureByDefault 2026-07-26-16:55: `noAuth` is the explicit opt-out from + // the always-on bearer-token default (mirrors `fn dashboard --no-auth`). + opts: { interactive?: boolean; paused?: boolean; host?: string; daemon?: boolean; noAuth?: boolean; noAutoRegister?: boolean; project?: string } = {}, ) { serveStartTime = Date.now(); ensureProcessDiagnostics(); @@ -869,31 +869,18 @@ export async function runServe( }); // ── Daemon token resolution ───────────────────────────────────────────── - // - // When --daemon flag is set, resolve the daemon token using the same - // priority as fn daemon: env var > stored token > generate new token. - // - let daemonToken: string | undefined; - if (opts.daemon) { - // 1. Check environment variable first - daemonToken = process.env.FUSION_DAEMON_TOKEN; - - // 2. Check stored token in global settings - if (!daemonToken) { - const globalDir = resolveGlobalDir(); - const settingsStore = new GlobalSettingsStore(globalDir); - const tokenManager = new DaemonTokenManager(settingsStore); - daemonToken = await tokenManager.getToken(); - } - - // 3. Generate and store a new token if none exists - if (!daemonToken) { - const globalDir = resolveGlobalDir(); - const settingsStore = new GlobalSettingsStore(globalDir); - const tokenManager = new DaemonTokenManager(settingsStore); - daemonToken = await tokenManager.generateToken(); - } - } + /* + FNXC:ServeSecureByDefault 2026-07-26-16:55: + Token resolution is now UNCONDITIONAL, not gated on `--daemon`. Plain `fn serve` + previously started with no token, so `createServer` installed no auth middleware and + the whole API (including POST /api/approvals/:id/decision) was reachable + unauthenticated — the hole an AI agent used to self-approve destructive actions. + `fn serve` now always resolves/mints a persisted token (env > stored > generated, + matching `fn daemon` and `fn dashboard`) unless the operator explicitly passes + `--no-auth`. The resolved token/URL is printed after listen so operators can still + connect (see the startup banner below). + */ + const daemonToken = await resolveServeDaemonToken({ noAuth: opts.noAuth }); // ── Skills adapter for skills discovery and execution toggling ───────────── // @@ -1070,6 +1057,9 @@ export async function runServe( headless: true, skillsAdapter, daemon: daemonToken ? { token: daemonToken } : undefined, + // FNXC:ServeSecureByDefault 2026-07-26-16:55: forward the explicit opt-out so a + // stale FUSION_DAEMON_TOKEN env var cannot silently re-enable auth under --no-auth. + noAuth: opts.noAuth === true ? true : undefined, https: loadTlsCredentialsFromEnv(), }); @@ -1151,10 +1141,18 @@ export async function runServe( const { maskApiKey } = await import("./node.js"); console.log(); + /* + FNXC:ServeSecureByDefault 2026-07-26-16:55: + Auth is now the default for every `fn serve` (not just --daemon), so the banner must + always surface the token and a click-through `?token=` launch URL — the same operator + affordance `fn dashboard` provides — or a secure-by-default serve would lock the + operator out of their own dashboard. The explicit `--no-auth` opt-out is called out + loudly instead of silently printing an open endpoint. + */ if (daemonToken) { - console.log(` Fusion Node (daemon mode)`); + console.log(opts.daemon ? ` Fusion Node (daemon mode)` : ` Fusion Node`); console.log(` ────────────────────────`); - console.log(` → http://${selectedHost}:${actualPort}`); + console.log(` → http://${selectedHost}:${actualPort}/?token=${daemonToken}`); console.log(); console.log(` Token: fn_${maskApiKey(daemonToken)}`); console.log(); @@ -1171,7 +1169,7 @@ export async function runServe( console.log(` → http://${selectedHost}:${actualPort}`); console.log(); console.log(` Health: GET /api/health`); - console.log(` API: /api/*`); + console.log(` API: /api/* (auth DISABLED via --no-auth — anyone who can reach this socket has full API access)`); console.log(` AI engine: ✓ active`); console.log(` Press Ctrl+C to stop`); } diff --git a/packages/cli/src/commands/task.ts b/packages/cli/src/commands/task.ts index 38ab635d88..6a001a2b19 100644 --- a/packages/cli/src/commands/task.ts +++ b/packages/cli/src/commands/task.ts @@ -1238,8 +1238,16 @@ export async function runTaskMove(id: string, column: string, projectName?: stri // every attempt. Only `database is locked`/SQLITE_BUSY|LOCKED errors are // retried; a genuinely invalid move (bad column, missing task) propagates // immediately without looping. + /* + FNXC:TaskMovement 2026-07-26-12:35: + `fn task move` is a human board action and must carry `moveSource: "user"` like + the dashboard's move route. Without it, moveTask defaulted the source to + "engine", so an in-progress → todo move from the CLI skipped the + disposeTaskBeforeMove hard-cancel seam — the board showed Todo while the + agent session kept running (Move-Task contract violation). + */ await withBoardWrite(projectName, { id, action: "move task" }, async (context) => { - const task = await context.store.moveTask(id, column as Column); + const task = await context.store.moveTask(id, column as Column, { moveSource: "user" }); console.log(); console.log(` ✓ Moved ${task.id} → ${columnLabel(task.column)}`); console.log(); diff --git a/packages/cli/src/extension.ts b/packages/cli/src/extension.ts index 9dfd1eeb58..41f039357b 100644 --- a/packages/cli/src/extension.ts +++ b/packages/cli/src/extension.ts @@ -34,6 +34,11 @@ import { getProjectRootFromWorktree, resolveTaskGithubTracking, formatCurrentTaskLine, + resolveFusionSessionPrincipal, + resolveEffectiveAgentPermissionPolicy, + type FusionSessionPrincipal, + type AgentPermissionPolicy, + type ApprovalRequestActorSnapshot, type SecretScope, declaresAnyLifecycleTrait, resolveTaskLifecycleColumns, @@ -76,6 +81,8 @@ import { normalizeAgentLogPaging, renderAgentLogEntries, createAgentTask, + evaluateAgentActionGate, + resolveGateOutcome, } from "@fusion/engine"; import * as dashboard from "@fusion/dashboard"; import { resolve, relative, isAbsolute, sep, basename, extname, join } from "node:path"; @@ -744,14 +751,20 @@ ephemeral so the project policy still applies. An absent id keeps the human pass resolved permanent agent is still never gated. FNXC:EphemeralAgentTaskCreation 2026-07-26-07:40: -KNOWN LIMITATION — this gate does not currently fire in production, and must not be counted as -enforcement of the Deny policy. pi's `ExtensionContext` (pi-coding-agent, core/extensions/types) -carries no `agentId`; the read at the fn_task_create execute site is a speculative cast, and only -tests ever supply one. Every real call therefore short-circuits at `!callerAgentId` and passes -through as a human caller. The fail-closed direction above is correct for the day an identity -signal exists, but making this lane genuinely enforce Deny needs the engine to thread the session's -agent/task identity into the extension context (env or an augmented context) — a plumbing decision, -not a local fix. Enforcement today lives in the engine lanes, which withhold the tool outright. +KNOWN LIMITATION (superseded 2026-07-26, see below) — pi's `ExtensionContext` (pi-coding-agent, +core/extensions/types) carries no `agentId`; the read at the fn_task_create execute site is a +speculative cast, and only tests ever supply one. Without another identity signal every real call +short-circuited at `!callerAgentId` and passed through as a human caller. + +FNXC:ToolPermissionGates 2026-07-26-13:55: +The identity signal now exists: the engine registers agent sessions by cwd in @fusion/core's +session-identity registry (resolveFusionSessionPrincipal). fn_task_create's ephemeral gate uses +the registry-resolved agentId as a fallback when ctx.agentId is absent (see +resolveExtensionCallerPrincipal below), so the Deny policy is enforceable for engine-spawned +sessions even though pi's ExtensionContext still carries no identity of its own. The fail-closed +lookup semantics above are unchanged: a caller id that is present but unresolvable is classified +ephemeral, an ambiguous registry entry is treated as an agent with unknown identity, and an +unregistered cwd remains a human operator CLI pass-through. */ async function isEphemeralCallerAgent(cwd: string, callerAgentId: string | undefined): Promise { if (!callerAgentId) return false; @@ -767,6 +780,351 @@ async function isEphemeralCallerAgent(cwd: string, callerAgentId: string | undef } } +// ── Caller principal + agent tool gates ──────────────────────────── + +/** Minimal caller-context shape read from the pi ExtensionContext (augmented fields are optional). */ +type ExtensionCallerContext = { + cwd?: string; + agentId?: unknown; + agentName?: unknown; + taskId?: unknown; + runId?: unknown; +}; + +/** Stand-in agent id when the principal is ambiguous (multiple live sessions in one cwd). */ +const AMBIGUOUS_AGENT_PRINCIPAL_ID = "unknown-agent"; + +/* +FNXC:ToolPermissionGates 2026-07-26-13:55: +Security incident root cause: all fn_* host-extension tools are delivered to engine agent +sessions via pi's extension loader and NEVER pass through the engine's per-session gate +wrappers, so destructive tools (fn_task_delete etc.) ran ungated for agents — an agent +autonomously deleted a live task. The engine now registers agent sessions by cwd in +@fusion/core's session-identity registry; this resolver is the extension-side principal +channel. Precedence: +1. An explicit ctx.agentId (engine-augmented contexts and tests) is an agent principal. +2. Otherwise the registry decides: no registration = human operator CLI, exactly one live + registration = that agent, multiple = ambiguous. +"ambiguous" MUST be treated as an agent with unknown identity (fail closed), never as an +operator. Operator (human CLI) behavior is unchanged by every gate built on this resolver. +*/ +export function resolveExtensionCallerPrincipal(ctx: ExtensionCallerContext): FusionSessionPrincipal { + const explicitAgentId = + typeof ctx.agentId === "string" && ctx.agentId.trim().length > 0 ? ctx.agentId.trim() : undefined; + if (explicitAgentId) { + return { + kind: "agent", + identity: { + agentId: explicitAgentId, + ...(typeof ctx.agentName === "string" && ctx.agentName ? { agentName: ctx.agentName } : {}), + ...(typeof ctx.taskId === "string" && ctx.taskId ? { taskId: ctx.taskId } : {}), + registeredAt: Date.now(), + }, + }; + } + return resolveFusionSessionPrincipal(typeof ctx.cwd === "string" && ctx.cwd ? ctx.cwd : process.cwd()); +} + +/* +FNXC:ToolPermissionGates 2026-07-26-13:55: +INTENDED BEHAVIOR CHANGE for agents: these destructive/irreversible tools are hard-withheld +from agent and ambiguous principals at execute time, regardless of permission policy or +preset. Human operator CLI sessions (no registry entry, no ctx.agentId) are unaffected. +The guard runs FIRST in each tool's execute, before any store access or param validation. +*/ +const WITHHELD_FROM_AGENT_EXTENSION_TOOLS: ReadonlySet = new Set([ + "fn_task_delete", + "fn_task_bypass_review", + "fn_mission_delete", + "fn_milestone_delete", + "fn_slice_delete", + "fn_feature_delete", + "fn_workflow_delete", + "fn_experiment_finalize", + "fn_skills_install", +]); + +interface AgentGateDenyResult { + content: Array<{ type: "text"; text: string }>; + isError: true; + details: Record; +} + +/** + * FNXC:ToolPermissionGates 2026-07-26-14:40: + * Dedupe-key lookup that works in PostgreSQL backend mode. + * ApprovalRequestStore.findLatestByDedupeKey's backend branch parses the jsonb + * `targetContext` (already an object from drizzle) through the string-only fromJson + * helper, so it never matches and every retry minted a duplicate request. Until that + * core defect is fixed, fall back to list() — whose backend row mapping returns the + * parsed context verbatim — and match `context.approvalDedupeKey` newest-first, the + * same contract chat.ts uses. + */ +async function findLatestApprovalRequestByDedupeKey( + approvalStore: ApprovalRequestStore, + input: { requesterActorId: string; taskId?: string; dedupeKey: string }, +): Promise>> { + const direct = await approvalStore.findLatestByDedupeKey(input); + if (direct) return direct; + const rows = await approvalStore.list({ requesterActorId: input.requesterActorId, ...(input.taskId ? { taskId: input.taskId } : {}) }); + return ( + rows.find((row) => row.targetAction.context?.approvalDedupeKey === input.dedupeKey) ?? null + ); +} + +/** + * FNXC:ToolPermissionGates 2026-07-26-13:55: + * Shared hard-deny for the withheld list above. Returns null for operator principals + * (tool proceeds unchanged) and a structured error result for agent/ambiguous principals. + */ +function denyWithheldToolForAgentPrincipal( + toolName: string, + ctx: ExtensionCallerContext, +): AgentGateDenyResult | null { + if (!WITHHELD_FROM_AGENT_EXTENSION_TOOLS.has(toolName)) return null; + const principal = resolveExtensionCallerPrincipal(ctx); + if (principal.kind === "operator") return null; + const agentId = principal.kind === "agent" ? principal.identity.agentId : undefined; + return { + content: [ + { + type: "text" as const, + text: + `${toolName} is withheld from agent sessions: this destructive operation is reserved for the human operator. ` + + "Do not retry it; ask the operator to run it from the dashboard or CLI if it is genuinely needed.", + }, + ], + isError: true as const, + details: { + deniedFor: "agent-principal", + tool: toolName, + ...(agentId ? { agentId } : {}), + }, + }; +} + +/* +FNXC:ToolPermissionGates 2026-07-26-13:55: +Policy gate for sensitive-but-policy-governed extension tools called by agent/ambiguous +principals. Resolves the caller's effective permission policy (agent row policy layered over +the project default; the shipped default preset is `unrestricted`) and evaluates the SAME +engine action gate used in engine lanes. Contract: +- Operator principals: never gated, behavior unchanged. +- disposition "allow" (the DEFAULT PRESET path): proceed friction-free — no approval row is + ever created on this path. +- "block": structured deny. +- "require-approval": reuse the latest request for the dedupe key (pending → still waiting, + denied → deny, approved → consume the grant via markCompleted and proceed once); otherwise + mint one approval request with the agent's REAL requester snapshot. +- Ambiguous principals resolve the project default policy only (unknown agent, fail closed on + identity but still policy-governed). +- Any resolution failure (store/asyncLayer unavailable, policy read error) fails CLOSED with a + structured deny. +*/ +async function applyAgentPolicyGateForExtensionTool( + toolName: string, + args: Record, + ctx: ExtensionCallerContext, +): Promise< + | AgentGateDenyResult + | { content: Array<{ type: "text"; text: string }>; details: Record } + | null +> { + const principal = resolveExtensionCallerPrincipal(ctx); + if (principal.kind === "operator") return null; + const callerAgentId = principal.kind === "agent" ? principal.identity.agentId : undefined; + const cwd = typeof ctx.cwd === "string" && ctx.cwd ? ctx.cwd : process.cwd(); + const taskId = typeof ctx.taskId === "string" && ctx.taskId ? ctx.taskId : undefined; + const runId = typeof ctx.runId === "string" && ctx.runId ? ctx.runId : undefined; + + try { + const store = await getStore(cwd); + const settings = await store.getSettings(); + + let agentRow: { name?: string; permissionPolicy?: AgentPermissionPolicy } | null = null; + if (callerAgentId) { + try { + const agentStore = await getAgentStore(cwd); + await agentStore.init(); + agentRow = await agentStore.resolveAgent(callerAgentId); + } catch { + // Unknown/unreadable agent row: fall through to the project default policy (still an + // agent principal — never an operator). + agentRow = null; + } + } + + const policy = resolveEffectiveAgentPermissionPolicy( + agentRow?.permissionPolicy, + settings.defaultAgentPermissionPolicy, + ); + const gateAgentId = callerAgentId ?? AMBIGUOUS_AGENT_PRINCIPAL_ID; + let decision = evaluateAgentActionGate({ + agentId: gateAgentId, + ...(taskId ? { taskId } : {}), + toolName, + args, + permissionPolicy: policy, + }); + if (decision.category === "exempt") { + /* + FNXC:ToolPermissionGates 2026-07-26-13:55: + A policy-gated extension tool that the engine's static classification does not know + (today: fn_agent_set_instructions) must not fall through the gate's exempt default to + an unconditional allow. Treat it as task_agent_mutation, honoring exact toolRules first + — under the default `unrestricted` preset this still resolves to "allow", so default + agent behavior is unchanged. + */ + const fallbackDisposition = policy.toolRules?.[toolName] ?? policy.rules.task_agent_mutation; + decision = { + ...decision, + disposition: fallbackDisposition, + category: "task_agent_mutation", + resourceType: "agent", + }; + } + + if (decision.disposition === "allow") { + // DEFAULT PRESET PATH: friction-free, no approval row. + return null; + } + + if (decision.disposition === "block") { + return { + content: [ + { + type: "text" as const, + text: `${toolName} is blocked by this agent's permission policy (category ${decision.category}). Ask the operator to run it or adjust the agent's permission policy.`, + }, + ], + isError: true as const, + details: { + deniedFor: "agent-permission-policy", + tool: toolName, + disposition: "block", + category: decision.category, + ...(callerAgentId ? { agentId: callerAgentId } : {}), + }, + }; + } + + // require-approval + const layer = store.getAsyncLayer(); + if (!layer) { + throw new Error("approval request store unavailable (no project async layer)"); + } + const approvalStore = new ApprovalRequestStore(null, { asyncLayer: layer }); + const requester: ApprovalRequestActorSnapshot = { + actorId: gateAgentId, + actorType: "agent", + actorName: + agentRow?.name ?? + (principal.kind === "agent" ? principal.identity.agentName ?? gateAgentId : gateAgentId), + }; + const latest = await findLatestApprovalRequestByDedupeKey(approvalStore, { + requesterActorId: gateAgentId, + ...(taskId ? { taskId } : {}), + dedupeKey: decision.approvalDedupeKey, + }); + const outcome = resolveGateOutcome(decision, latest ? { id: latest.id, status: latest.status } : null); + + if (outcome.outcome === "execute-once-then-complete" && outcome.approvalRequestId) { + // Consume the operator's grant so it cannot be replayed; then proceed once. + await approvalStore.markCompleted(outcome.approvalRequestId, { + actor: requester, + note: `Approval consumed by ${toolName}`, + expectedRequesterActorId: gateAgentId, + }); + return null; + } + + if (outcome.outcome === "block") { + return { + content: [ + { + type: "text" as const, + text: `${toolName} was denied by the operator (approval request ${outcome.approvalRequestId ?? "unknown"}).`, + }, + ], + isError: true as const, + details: { + deniedFor: "agent-approval-denied", + tool: toolName, + ...(outcome.approvalRequestId ? { approvalRequestId: outcome.approvalRequestId } : {}), + ...(callerAgentId ? { agentId: callerAgentId } : {}), + }, + }; + } + + if (latest && latest.status === "pending") { + return { + content: [ + { + type: "text" as const, + text: `${toolName} requires operator approval. Request ${latest.id} is still pending — do not retry until it is decided.`, + }, + ], + details: { + outcome: "pending_approval", + approvalRequestId: latest.id, + tool: toolName, + ...(callerAgentId ? { agentId: callerAgentId } : {}), + }, + }; + } + + const request = await approvalStore.create({ + requester, + targetAction: { + category: decision.category === "exempt" ? "task_agent_mutation" : decision.category, + action: decision.operation, + summary: decision.summary, + resourceType: decision.resourceType, + resourceId: decision.resourceId ?? "", + context: { + approvalDedupeKey: decision.approvalDedupeKey, + toolName, + toolArgs: args, + source: "pi-extension-agent-gating", + }, + }, + ...(taskId ? { taskId } : {}), + ...(runId ? { runId } : {}), + }); + return { + content: [ + { + type: "text" as const, + text: `${toolName} requires operator approval. Request ${request.id} created and pending — approve via POST /api/approvals/:id/decision.`, + }, + ], + details: { + outcome: "pending_approval", + approvalRequestId: request.id, + tool: toolName, + ...(callerAgentId ? { agentId: callerAgentId } : {}), + }, + }; + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + return { + content: [ + { + type: "text" as const, + text: `${toolName} denied: the agent permission policy could not be resolved (${message}). Failing closed — ask the operator to run this tool.`, + }, + ], + isError: true as const, + details: { + deniedFor: "agent-permission-policy-unavailable", + tool: toolName, + error: message, + ...(callerAgentId ? { agentId: callerAgentId } : {}), + }, + }; + } +} + function normalizeNullableStringInput(value: string | null | undefined): string | null | undefined { if (value === undefined) { return undefined; @@ -1120,7 +1478,22 @@ export default function kbExtension(pi: ExtensionAPI) { FNXC:WorkflowAuthoringTools 2026-06-29-23:06: fn_workflow_select may default only in task-bound extension contexts; no-task published API calls must pass task_id explicitly so an empty ambient task cannot accidentally route the wrong card. + + FNXC:ToolPermissionGates 2026-07-26-13:55: + fn_workflow_delete is hard-withheld from agent principals; fn_workflow_update is + policy-gated per the caller agent's effective permission policy. Operator CLI calls + are unaffected by both. */ + const withheldDenied = denyWithheldToolForAgentPrincipal(spec.name, ctx as ExtensionCallerContext); + if (withheldDenied) return withheldDenied; + if (spec.name === "fn_workflow_update") { + const gated = await applyAgentPolicyGateForExtensionTool( + spec.name, + params as Record, + ctx as ExtensionCallerContext, + ); + if (gated) return gated; + } const store = await getStore(ctx.cwd); const extensionContext = ctx as typeof ctx & { taskId?: string }; const currentTaskId = typeof extensionContext.taskId === "string" ? extensionContext.taskId : ""; @@ -1198,7 +1571,21 @@ export default function kbExtension(pi: ExtensionAPI) { */ const fnCtx = ctx as typeof ctx & { agentId?: string; taskId?: string }; const projectSettingsForGate = await store.getSettings(); - const callerIsEphemeral = await isEphemeralCallerAgent(ctx.cwd ?? process.cwd(), fnCtx.agentId); + /* + FNXC:ToolPermissionGates 2026-07-26-13:55: + Fall back to the session-identity registry when pi's context carries no agentId so the + ephemeral-task-creation policy actually fires for engine-spawned sessions. An ambiguous + principal uses a sentinel id that never resolves to an agent row, which the fail-closed + lookup classifies as ephemeral. + */ + const createPrincipal = resolveExtensionCallerPrincipal(ctx as ExtensionCallerContext); + const registryAgentId = + createPrincipal.kind === "agent" + ? createPrincipal.identity.agentId + : createPrincipal.kind === "ambiguous" + ? AMBIGUOUS_AGENT_PRINCIPAL_ID + : undefined; + const callerIsEphemeral = await isEphemeralCallerAgent(ctx.cwd ?? process.cwd(), fnCtx.agentId ?? registryAgentId); if (callerIsEphemeral) { const policy = fusionCore.resolveEphemeralTaskCreationPolicy(projectSettingsForGate); if (policy === "deny") { @@ -1812,6 +2199,9 @@ export default function kbExtension(pi: ExtensionAPI) { }), async execute(_toolCallId, params, _signal, _onUpdate, ctx) { + // FNXC:ToolPermissionGates 2026-07-26-13:55: policy-gated for agent principals; operators unaffected. + const gated = await applyAgentPolicyGateForExtensionTool("fn_task_pause", params as Record, ctx as ExtensionCallerContext); + if (gated) return gated; const store = await getStore(ctx.cwd); const task = await store.pauseTask(params.id, true, undefined, { userPaused: true }); @@ -1835,6 +2225,9 @@ export default function kbExtension(pi: ExtensionAPI) { }), async execute(_toolCallId, params, _signal, _onUpdate, ctx) { + // FNXC:ToolPermissionGates 2026-07-26-13:55: policy-gated for agent principals; operators unaffected. + const gated = await applyAgentPolicyGateForExtensionTool("fn_task_unpause", params as Record, ctx as ExtensionCallerContext); + if (gated) return gated; const store = await getStore(ctx.cwd); const task = await store.pauseTask(params.id, false); @@ -1865,8 +2258,11 @@ export default function kbExtension(pi: ExtensionAPI) { }), async execute(_toolCallId, params, _signal, _onUpdate, ctx) { + // FNXC:ToolPermissionGates 2026-07-26-13:55: policy-gated for agent principals; operators unaffected. + const gated = await applyAgentPolicyGateForExtensionTool("fn_task_retry", params as Record, ctx as ExtensionCallerContext); + if (gated) return gated; const store = await getStore(ctx.cwd); - + // Validate task exists let task; try { @@ -1952,7 +2348,8 @@ export default function kbExtension(pi: ExtensionAPI) { await store.logEntry(params.id, `Retry requested via Fusion extension (unusable worktree session-start recovery → todo, preserving progress${retryLogSuffix})`); /* FNXC:WorkflowResolvedColumns 2026-07-30-22:20: census-invisible moveTask DESTINATION — a call argument, not a comparison. This is an OPERATOR-triggered Retry: on a board that does not declare `todo` the move is REJECTED and the retry fails in the operator's face. The reply text below uses the SAME resolved value so it cannot name a lane the card did not go to. */ const retryTarget = await fusionCore.resolveReboundTargetForTask(store, params.id); - await store.moveTask(params.id, retryTarget, { preserveProgress: true }); + /* FNXC:ToolPermissionGates 2026-07-30-13:55: fn_task_retry is a user-facing lever — carry the user move source (target resolves by role). */ + await store.moveTask(params.id, retryTarget, { preserveProgress: true, moveSource: "user" }); return { content: [{ type: "text", text: `Retried ${params.id} → ${retryTarget} (unusable worktree session metadata cleared)` }], details: { taskId: params.id, newColumn: 'todo' }, @@ -1976,7 +2373,8 @@ export default function kbExtension(pi: ExtensionAPI) { ); /* FNXC:WorkflowResolvedColumns 2026-07-30-22:20: census-invisible moveTask DESTINATION — same operator Retry path as above. */ const executionRetryTarget = await fusionCore.resolveReboundTargetForTask(store, params.id); - await store.moveTask(params.id, executionRetryTarget, { preserveProgress: true }); + /* FNXC:ToolPermissionGates 2026-07-30-13:55: fn_task_retry is a user-facing lever — carry the user move source (target resolves by role). */ + await store.moveTask(params.id, executionRetryTarget, { preserveProgress: true, moveSource: "user" }); return { content: [{ type: "text", text: `Retried ${params.id} → ${executionRetryTarget} (execution failure, preserving step progress)` }], details: { taskId: params.id, newColumn: 'todo' }, @@ -2005,7 +2403,8 @@ export default function kbExtension(pi: ExtensionAPI) { }); // Move to todo column - await store.moveTask(params.id, 'todo'); + // FNXC:ToolPermissionGates 2026-07-26-13:55: user-facing retry move carries the user/hard-cancel source (Move-Task contract). + await store.moveTask(params.id, 'todo', { moveSource: "user" }); // Log the retry action await store.logEntry(params.id, "Retry requested via Fusion extension", "Task reset to todo for retry"); @@ -2028,9 +2427,14 @@ export default function kbExtension(pi: ExtensionAPI) { * surface — deliberately NOT wired into packages/engine/src/executor.ts or * packages/engine/src/agent-heartbeat.ts autonomous per-role tool lists, and * NOT part of packages/dashboard/src/planning-board-tools.ts read-only - * planning tools — so headless executor/reviewer/triage agent runs never - * gain the bypass. Requires a mandatory reason; audit-logged via + * planning tools. Requires a mandatory reason; audit-logged via * store.bypassFailedPreMergeReviewStep's run-audit event. + * + * FNXC:ToolPermissionGates 2026-07-26-13:55: + * Registration-surface separation alone was NOT sufficient: pi's host-extension + * loader delivers this tool into engine agent sessions too. Operator-only access + * is now enforced by construction — the withheld-from-agents principal guard runs + * first in execute and hard-denies agent/ambiguous principals. */ pi.registerTool({ name: "fn_task_bypass_review", @@ -2052,6 +2456,8 @@ export default function kbExtension(pi: ExtensionAPI) { }), async execute(_toolCallId, params, _signal, _onUpdate, ctx) { + const withheldDenied = denyWithheldToolForAgentPrincipal("fn_task_bypass_review", ctx as ExtensionCallerContext); + if (withheldDenied) return withheldDenied; const store = await getStore(ctx.cwd); const fnCtx = ctx as typeof ctx & { agentId?: string }; const actor = fnCtx.agentId ?? "cli-operator"; @@ -2174,6 +2580,9 @@ export default function kbExtension(pi: ExtensionAPI) { }), async execute(_toolCallId, params, _signal, _onUpdate, ctx) { + // FNXC:ToolPermissionGates 2026-07-26-13:55: policy-gated for agent principals; operators unaffected. + const gated = await applyAgentPolicyGateForExtensionTool("fn_task_archive", params as Record, ctx as ExtensionCallerContext); + if (gated) return gated; const store = await getStore(ctx.cwd); const task = await store.archiveTask(params.id, { removeLineageReferences: params.removeLineageReferences === true, @@ -2244,6 +2653,9 @@ export default function kbExtension(pi: ExtensionAPI) { }), async execute(_toolCallId, params, _signal, _onUpdate, ctx) { + // FNXC:ToolPermissionGates 2026-07-26-13:55: hard-withheld from agent/ambiguous principals (root cause of the live-task deletion incident); operators unaffected. + const withheldDenied = denyWithheldToolForAgentPrincipal("fn_task_delete", ctx as ExtensionCallerContext); + if (withheldDenied) return withheldDenied; const store = await getStore(ctx.cwd); const callerTaskId = (ctx as { taskId?: string }).taskId; const task = await store.deleteTask(params.id, { @@ -2833,26 +3245,85 @@ export default function kbExtension(pi: ExtensionAPI) { } if (decision.policy === "prompt") { - + /* + FNXC:SecretsApproval 2026-07-26-14:10: + BEHAVIOR CHANGES (broken approval control, both intentional): + (a) An `approved` row previously fell through and minted a BRAND-NEW pending request, + so operator approval never granted anything — the loop was unwinnable. The status + ladder is now: pending → still-awaiting message (no re-mint); denied → denied + message (no re-mint); approved → REDEEM: reveal the secret, then markCompleted so + the grant is consumed execute-once; completed (already redeemed) → mint a fresh + request. + (b) The approval row's category was "task_mutation" (normalized to + task_agent_mutation), so the dashboard's emitSecretsAccessDecisionAudit — which + fires only for category "secrets_access" — never ran. The category is now + "secrets_access" (accepted verbatim by normalizeApprovalRequestActionCategory). + */ const cliLayer = requireProjectLayer(store, "CLI secret approval store"); const approvalStore = new ApprovalRequestStore(null, { asyncLayer: cliLayer }); const dedupeKey = `secret-read:${resolvedScope}:${params.key}:${fnCtx.agentId ?? "unknown"}`; - const existing = await approvalStore.findLatestByDedupeKey({ requesterActorId: fnCtx.agentId ?? "user", taskId: fnCtx.taskId, dedupeKey }); - const request = existing && existing.status === "pending" - ? existing - : await approvalStore.create({ - requester: { actorId: fnCtx.agentId ?? "user", actorType: "agent", actorName: fnCtx.agentName ?? fnCtx.agentId ?? "Agent" }, - targetAction: { - category: "task_mutation", - action: "read", - summary: `Read secret ${params.key}`, - resourceType: "secret", - resourceId: record.id, - context: { approvalDedupeKey: dedupeKey, key: params.key, scope: resolvedScope }, - }, - ...(fnCtx.runId ? { runId: fnCtx.runId } : {}), - ...(fnCtx.taskId ? { taskId: fnCtx.taskId } : {}), + const requesterActorId = fnCtx.agentId ?? "user"; + /* + FNXC:SecretsAccessApproval 2026-07-26-18:35: + Review finding: a caller with no agentId is the human CLI operator, and + recording it as actorType "agent" mislabels the attribution this branch + exists to fix. Snapshot the real principal shape. + */ + const requesterSnapshot: ApprovalRequestActorSnapshot = fnCtx.agentId + ? { + actorId: requesterActorId, + actorType: "agent", + actorName: fnCtx.agentName ?? fnCtx.agentId, + } + : { actorId: "user", actorType: "user", actorName: "CLI User" }; + const existing = await findLatestApprovalRequestByDedupeKey(approvalStore, { requesterActorId, ...(fnCtx.taskId ? { taskId: fnCtx.taskId } : {}), dedupeKey }); + + if (existing?.status === "pending") { + emitSecretAudit(store, fnCtx, "secret:approval-requested", `${resolvedScope}:${params.key}`); + return { + content: [{ type: "text", text: `Secret access approval request ${existing.id} is still pending. Approve via POST /api/approvals/:id/decision.` }], + details: { outcome: "pending_approval", approvalRequestId: existing.id, key: params.key, scope: resolvedScope }, + }; + } + + if (existing?.status === "denied") { + emitSecretAudit(store, fnCtx, "secret:approval-denied", `${resolvedScope}:${params.key}`); + return { + content: [{ type: "text", text: `Secret access request ${existing.id} was denied by the operator. Do not retry without operator direction.` }], + details: { outcome: "denied", approvalRequestId: existing.id, key: params.key, scope: resolvedScope }, + }; + } + + if (existing?.status === "approved") { + const revealedAfterApproval = await secretsStore.revealSecret(record.id, resolvedScope, { agentId: fnCtx.agentId ?? null }); + await approvalStore.markCompleted(existing.id, { + actor: requesterSnapshot, + note: "Secret revealed after approval", + // FNXC:SecretsAccessApproval 2026-07-26-18:35: ownership guard — secret grants + // get the same expectedRequesterActorId enforcement as the gate path. + expectedRequesterActorId: requesterActorId, }); + emitSecretAudit(store, fnCtx, "secret:read", `${resolvedScope}:${params.key}`, { key: params.key, scope: resolvedScope, approvalRequestId: existing.id }); + return { + content: [{ type: "text", text: `Loaded secret '${params.key}' from ${resolvedScope} scope (approval ${existing.id} consumed).` }], + details: { key: params.key, value: revealedAfterApproval.plaintextValue, scope: resolvedScope, approvalRequestId: existing.id }, + }; + } + + // No prior request, or the previous grant was already redeemed (completed) → mint a fresh one. + const request = await approvalStore.create({ + requester: requesterSnapshot, + targetAction: { + category: "secrets_access", + action: "read", + summary: `Read secret ${params.key}`, + resourceType: "secret", + resourceId: record.id, + context: { approvalDedupeKey: dedupeKey, key: params.key, scope: resolvedScope }, + }, + ...(fnCtx.runId ? { runId: fnCtx.runId } : {}), + ...(fnCtx.taskId ? { taskId: fnCtx.taskId } : {}), + }); emitSecretAudit(store, fnCtx, "secret:approval-requested", `${resolvedScope}:${params.key}`); return { @@ -2887,6 +3358,9 @@ export default function kbExtension(pi: ExtensionAPI) { summary: Type.Optional(Type.String({ description: "Optional finalize summary" })), }), async execute(_toolCallId, params, _signal, _onUpdate, ctx) { + // FNXC:ToolPermissionGates 2026-07-26-13:55: hard-withheld from agent/ambiguous principals; operators unaffected. + const withheldDenied = denyWithheldToolForAgentPrincipal("fn_experiment_finalize", ctx as ExtensionCallerContext); + if (withheldDenied) return withheldDenied; try { const store = await getStore(ctx.cwd); const sessionStore = store.getExperimentSessionStore(); @@ -3901,6 +4375,9 @@ export default function kbExtension(pi: ExtensionAPI) { }), async execute(_toolCallId, params, _signal, _onUpdate, ctx) { + // FNXC:ToolPermissionGates 2026-07-26-13:55: hard-withheld from agent/ambiguous principals; operators unaffected. + const withheldDenied = denyWithheldToolForAgentPrincipal("fn_mission_delete", ctx as ExtensionCallerContext); + if (withheldDenied) return withheldDenied; const store = await getStore(ctx.cwd); const missionStore = store.getMissionStore(); @@ -4146,6 +4623,9 @@ export default function kbExtension(pi: ExtensionAPI) { }), async execute(_toolCallId, params, _signal, _onUpdate, ctx) { + // FNXC:ToolPermissionGates 2026-07-26-13:55: hard-withheld from agent/ambiguous principals; operators unaffected. + const withheldDenied = denyWithheldToolForAgentPrincipal("fn_feature_delete", ctx as ExtensionCallerContext); + if (withheldDenied) return withheldDenied; const store = await getStore(ctx.cwd); const missionStore = store.getMissionStore(); @@ -4180,6 +4660,9 @@ export default function kbExtension(pi: ExtensionAPI) { }), async execute(_toolCallId, params, _signal, _onUpdate, ctx) { + // FNXC:ToolPermissionGates 2026-07-26-13:55: hard-withheld from agent/ambiguous principals; operators unaffected. + const withheldDenied = denyWithheldToolForAgentPrincipal("fn_slice_delete", ctx as ExtensionCallerContext); + if (withheldDenied) return withheldDenied; const store = await getStore(ctx.cwd); const missionStore = store.getMissionStore(); @@ -4214,6 +4697,9 @@ export default function kbExtension(pi: ExtensionAPI) { }), async execute(_toolCallId, params, _signal, _onUpdate, ctx) { + // FNXC:ToolPermissionGates 2026-07-26-13:55: hard-withheld from agent/ambiguous principals; operators unaffected. + const withheldDenied = denyWithheldToolForAgentPrincipal("fn_milestone_delete", ctx as ExtensionCallerContext); + if (withheldDenied) return withheldDenied; const store = await getStore(ctx.cwd); const missionStore = store.getMissionStore(); @@ -4531,7 +5017,9 @@ export default function kbExtension(pi: ExtensionAPI) { }), async execute(_toolCallId, params, _signal, _onUpdate, ctx) { - + // FNXC:ToolPermissionGates 2026-07-26-13:55: policy-gated for agent principals; operators unaffected. + const gated = await applyAgentPolicyGateForExtensionTool("fn_agent_stop", params as Record, ctx as ExtensionCallerContext); + if (gated) return gated; const agentStore = await getAgentStore(ctx.cwd); await agentStore.init(); @@ -4596,7 +5084,9 @@ export default function kbExtension(pi: ExtensionAPI) { }), async execute(_toolCallId, params, _signal, _onUpdate, ctx) { - + // FNXC:ToolPermissionGates 2026-07-26-13:55: policy-gated for agent principals; operators unaffected. + const gated = await applyAgentPolicyGateForExtensionTool("fn_agent_start", params as Record, ctx as ExtensionCallerContext); + if (gated) return gated; const agentStore = await getAgentStore(ctx.cwd); await agentStore.init(); @@ -4668,11 +5158,42 @@ export default function kbExtension(pi: ExtensionAPI) { message_response_mode: Type.Optional(Type.Union([Type.Literal("immediate"), Type.Literal("on-heartbeat")])), }), async execute(_toolCallId, params, _signal, _onUpdate, ctx) { - + const agentStore = await getAgentStore(ctx.cwd); await agentStore.init(); const store = await getStore(ctx.cwd); - const caller = { id: "user", role: "user", isPrivileged: true } as const; + /* + FNXC:ToolPermissionGates 2026-07-26-13:55: + BEHAVIOR CHANGE (honest provisioning caller): this site previously hardcoded + `{ id: "user", role: "user", isPrivileged: true }`, so an agent session calling this + tool through the host extension was treated as a privileged human and bypassed the + provisioning policy entirely. The caller is now principal-derived: operator CLI stays + privileged (unchanged); agent principals use their real id/role and are NOT privileged; + ambiguous principals are an unknown, unprivileged agent. Approval requests carry the + REAL requester snapshot instead of the hardcoded CLI User. + */ + const provisionPrincipal = resolveExtensionCallerPrincipal(ctx as ExtensionCallerContext); + let caller: { id: string; role: string; isPrivileged: boolean }; + let provisionRequester: ApprovalRequestActorSnapshot; + if (provisionPrincipal.kind === "operator") { + caller = { id: "user", role: "user", isPrivileged: true }; + provisionRequester = { actorId: "user", actorType: "user", actorName: "CLI User" }; + } else { + const callerAgentId = provisionPrincipal.kind === "agent" ? provisionPrincipal.identity.agentId : AMBIGUOUS_AGENT_PRINCIPAL_ID; + let callerRow: { name?: string; role?: string } | null = null; + if (provisionPrincipal.kind === "agent") { + try { + callerRow = await agentStore.resolveAgent(callerAgentId); + } catch { + callerRow = null; + } + } + const fallbackName = provisionPrincipal.kind === "agent" + ? provisionPrincipal.identity.agentName ?? callerAgentId + : callerAgentId; + caller = { id: callerAgentId, role: callerRow?.role ?? "custom", isPrivileged: false }; + provisionRequester = { actorId: callerAgentId, actorType: "agent", actorName: callerRow?.name ?? fallbackName }; + } const policy = resolveAgentProvisioningPolicy({ tool: "fn_agent_create", caller, @@ -4690,12 +5211,19 @@ export default function kbExtension(pi: ExtensionAPI) { const cliLayer2 = requireProjectLayer(store, "CLI agent-create approval store"); const approvalStore = new ApprovalRequestStore(null, { asyncLayer: cliLayer2 }); const request = await approvalStore.create({ - requester: { actorId: "user", actorType: "user", actorName: "CLI User" }, + requester: provisionRequester, targetAction: { category: "agent_provisioning", action: "create", summary: `Create agent ${params.name} (${params.role})`, resourceType: "agent", resourceId: "", context: { tool: "fn_agent_create", params } }, }); return { content: [{ type: "text" as const, text: `Approval required. Request ${request.id} created.` }], details: { outcome: "pending_approval", approvalRequestId: request.id, matchedRule: policy.matchedRule, effectiveMode: policy.effectiveMode } }; } + if (policy.decision === "deny") { + return { + content: [{ type: "text" as const, text: `DENIED: agent create blocked by policy (${policy.matchedRule})` }], + details: { outcome: "denied", matchedRule: policy.matchedRule, effectiveMode: policy.effectiveMode }, + }; + } + const runtimeConfig: Record = { ...(params.heartbeat_interval_ms !== undefined ? { heartbeatIntervalMs: params.heartbeat_interval_ms } : {}), ...(params.heartbeat_timeout_ms !== undefined ? { heartbeatTimeoutMs: params.heartbeat_timeout_ms } : {}), @@ -4979,7 +5507,9 @@ export default function kbExtension(pi: ExtensionAPI) { ), }), async execute(_toolCallId, params, _signal, _onUpdate, ctx) { - + // FNXC:ToolPermissionGates 2026-07-26-13:55: policy-gated for agent principals (classified as task_agent_mutation via the extension gate's exempt fallback); operators unaffected. + const gated = await applyAgentPolicyGateForExtensionTool("fn_agent_set_instructions", params as Record, ctx as ExtensionCallerContext); + if (gated) return gated; const agentStore = await getAgentStore(ctx.cwd); await agentStore.init(); @@ -5055,11 +5585,38 @@ export default function kbExtension(pi: ExtensionAPI) { reassign_to: Type.Optional(Type.String({ description: "Optional replacement agent for assigned tasks" })), }), async execute(_toolCallId, params, _signal, _onUpdate, ctx) { - + const agentStore = await getAgentStore(ctx.cwd); await agentStore.init(); const store = await getStore(ctx.cwd); - const caller = { id: "user", role: "user", isPrivileged: true } as const; + /* + FNXC:ToolPermissionGates 2026-07-26-13:55: + BEHAVIOR CHANGE (honest provisioning caller) — see the matching comment on + fn_agent_create: the caller is principal-derived instead of a hardcoded privileged + CLI User, and approval requests carry the real agent requester snapshot. + */ + const provisionPrincipal = resolveExtensionCallerPrincipal(ctx as ExtensionCallerContext); + let caller: { id: string; role: string; isPrivileged: boolean }; + let provisionRequester: ApprovalRequestActorSnapshot; + if (provisionPrincipal.kind === "operator") { + caller = { id: "user", role: "user", isPrivileged: true }; + provisionRequester = { actorId: "user", actorType: "user", actorName: "CLI User" }; + } else { + const callerAgentId = provisionPrincipal.kind === "agent" ? provisionPrincipal.identity.agentId : AMBIGUOUS_AGENT_PRINCIPAL_ID; + let callerRow: { name?: string; role?: string } | null = null; + if (provisionPrincipal.kind === "agent") { + try { + callerRow = await agentStore.resolveAgent(callerAgentId); + } catch { + callerRow = null; + } + } + const fallbackName = provisionPrincipal.kind === "agent" + ? provisionPrincipal.identity.agentName ?? callerAgentId + : callerAgentId; + caller = { id: callerAgentId, role: callerRow?.role ?? "custom", isPrivileged: false }; + provisionRequester = { actorId: callerAgentId, actorType: "agent", actorName: callerRow?.name ?? fallbackName }; + } const policy = resolveAgentProvisioningPolicy({ tool: "fn_agent_delete", caller, @@ -5070,7 +5627,7 @@ export default function kbExtension(pi: ExtensionAPI) { const cliLayer3 = requireProjectLayer(store, "CLI agent-delete approval store"); const approvalStore = new ApprovalRequestStore(null, { asyncLayer: cliLayer3 }); const request = await approvalStore.create({ - requester: { actorId: "user", actorType: "user", actorName: "CLI User" }, + requester: provisionRequester, targetAction: { category: "agent_provisioning", action: "delete", summary: `Delete agent ${params.agent_id}`, resourceType: "agent", resourceId: params.agent_id, context: { tool: "fn_agent_delete", params } }, }); return { content: [{ type: "text" as const, text: `Approval required. Request ${request.id} created.` }], details: { outcome: "pending_approval", approvalRequestId: request.id, matchedRule: policy.matchedRule, effectiveMode: policy.effectiveMode, agentId: params.agent_id } }; @@ -5221,6 +5778,16 @@ export default function kbExtension(pi: ExtensionAPI) { }), async execute(_toolCallId, params, _signal, _onUpdate, ctx) { + /* + FNXC:ToolPermissionGates 2026-07-26-13:55: + Ordinary delegation stays ungated (coordination primitive), but the executor-role + policy override is a sensitive escalation: policy-gate it for agent principals when + override=true. Operators unaffected. + */ + if (params.override === true) { + const gated = await applyAgentPolicyGateForExtensionTool("fn_delegate_task", params as Record, ctx as ExtensionCallerContext); + if (gated) return gated; + } // Validate target agent exists and is not ephemeral const delegateTask: Pick = { id: "", column: "todo" }; const agentError = await validateAssignableAgentId(ctx.cwd ?? process.cwd(), params.agent_id, delegateTask, params.override === true); @@ -5820,6 +6387,9 @@ export default function kbExtension(pi: ExtensionAPI) { Kill npx on abort/timeout so outer tool budgets cannot leave orphan install processes after the agent turn fails closed. */ async execute(_toolCallId, params, signal, _onUpdate, ctx) { + // FNXC:ToolPermissionGates 2026-07-26-13:55: hard-withheld from agent/ambiguous principals (installs third-party code into the project); operators unaffected. + const withheldDenied = denyWithheldToolForAgentPrincipal("fn_skills_install", ctx as ExtensionCallerContext); + if (withheldDenied) return withheldDenied; // Validate source format if (!/^[^/]+\/[^/]+$/.test(params.source)) { return { diff --git a/packages/core/src/__tests__/approval-request-transitions.test.ts b/packages/core/src/__tests__/approval-request-transitions.test.ts new file mode 100644 index 0000000000..5657de9c5b --- /dev/null +++ b/packages/core/src/__tests__/approval-request-transitions.test.ts @@ -0,0 +1,190 @@ +/** + * FNXC:ApprovalLifecycleSecurity 2026-07-26-12:35: + * Pure-function tests for the approval-request lifecycle validator and lazy TTL expiry. + * The transition table below is deliberately HARDCODED (all 16 from×to combos as literals, not generated + * from the function or shared constants) so a regression in the validator cannot silently rewrite the + * expectations: same-status replay (from===to) must be invalid because a replayed decision re-stamps + * decidedAt and forges duplicate audit history. + */ +import { describe, it, expect, afterEach } from "vitest"; +import { + APPROVAL_REQUEST_GRANT_TTL_MS, + getApprovalRequestGrantTtlMs, + configureApprovalRequestTtls, + APPROVAL_REQUEST_PENDING_TTL_MS, + isApprovalRequestExpired, + isValidApprovalRequestTransition, + type ApprovalRequestStatus, +} from "../types/agents.js"; + +describe("isValidApprovalRequestTransition", () => { + // Hardcoded 16-row expectation table: [from, to, expected]. + const table: Array<[ApprovalRequestStatus, ApprovalRequestStatus, boolean]> = [ + ["pending", "pending", false], + ["pending", "approved", true], + ["pending", "denied", true], + ["pending", "completed", false], + ["approved", "pending", false], + ["approved", "approved", false], + ["approved", "denied", false], + ["approved", "completed", true], + ["denied", "pending", false], + ["denied", "approved", false], + ["denied", "denied", false], + ["denied", "completed", false], + ["completed", "pending", false], + ["completed", "approved", false], + ["completed", "denied", false], + ["completed", "completed", false], + ]; + + it.each(table)("%s -> %s is %s", (from, to, expected) => { + expect(isValidApprovalRequestTransition(from, to)).toBe(expected); + }); + + it("rejects all four from===to replay combos", () => { + for (const status of ["pending", "approved", "denied", "completed"] as const) { + expect(isValidApprovalRequestTransition(status, status)).toBe(false); + } + }); +}); + +describe("isApprovalRequestExpired", () => { + const T0 = Date.parse("2026-07-26T00:00:00.000Z"); + + it("pending is not expired within 24h of requestedAt", () => { + expect( + isApprovalRequestExpired( + { status: "pending", requestedAt: new Date(T0).toISOString(), decidedAt: undefined }, + T0 + APPROVAL_REQUEST_PENDING_TTL_MS - 1, + ), + ).toBe(false); + expect( + isApprovalRequestExpired( + { status: "pending", requestedAt: new Date(T0).toISOString(), decidedAt: undefined }, + T0 + APPROVAL_REQUEST_PENDING_TTL_MS, + ), + ).toBe(false); + }); + + it("pending is expired past 24h of requestedAt", () => { + expect( + isApprovalRequestExpired( + { status: "pending", requestedAt: new Date(T0).toISOString(), decidedAt: undefined }, + T0 + APPROVAL_REQUEST_PENDING_TTL_MS + 1, + ), + ).toBe(true); + }); + + it("approved grant is redeemable within the grant TTL of decidedAt", () => { + expect( + isApprovalRequestExpired( + { + status: "approved", + requestedAt: new Date(T0 - 60_000).toISOString(), + decidedAt: new Date(T0).toISOString(), + }, + T0 + getApprovalRequestGrantTtlMs() - 1, + ), + ).toBe(false); + }); + + it("approved grant is expired past the grant TTL of decidedAt", () => { + expect( + isApprovalRequestExpired( + { + status: "approved", + requestedAt: new Date(T0 - 60_000).toISOString(), + decidedAt: new Date(T0).toISOString(), + }, + T0 + getApprovalRequestGrantTtlMs() + 1, + ), + ).toBe(true); + }); + + it("approved row with missing decidedAt is treated as expired (fail closed)", () => { + expect( + isApprovalRequestExpired( + { status: "approved", requestedAt: new Date(T0).toISOString(), decidedAt: undefined }, + T0, + ), + ).toBe(true); + }); + + it("approved row with unparseable decidedAt is treated as expired (fail closed)", () => { + expect( + isApprovalRequestExpired( + { status: "approved", requestedAt: new Date(T0).toISOString(), decidedAt: "not-a-date" }, + T0, + ), + ).toBe(true); + }); + + it("denied and completed never expire", () => { + const farFuture = T0 + 365 * 24 * 60 * 60 * 1000; + expect( + isApprovalRequestExpired( + { status: "denied", requestedAt: new Date(T0).toISOString(), decidedAt: new Date(T0).toISOString() }, + farFuture, + ), + ).toBe(false); + expect( + isApprovalRequestExpired( + { + status: "completed", + requestedAt: new Date(T0).toISOString(), + decidedAt: new Date(T0).toISOString(), + }, + farFuture, + ), + ).toBe(false); + }); + + it("TTL defaults encode a 24h pending window and a 1h grant window", () => { + expect(APPROVAL_REQUEST_PENDING_TTL_MS).toBe(24 * 60 * 60 * 1000); + expect(getApprovalRequestGrantTtlMs()).toBe(60 * 60 * 1000); + expect(APPROVAL_REQUEST_GRANT_TTL_MS).toBe(60 * 60 * 1000); + }); + + /* + FNXC:ApprovalLifecycleSecurity 2026-07-26-18:20: + The grant window is a tradeoff an operator must be able to tune (a 15-minute hardcode expired + grants during ordinary restarts and queue backlogs). These assert the override is honored by the + expiry decision itself — not merely stored — and that a nonsense override cannot widen the window + to infinity or collapse it to zero, which would silently re-open the unbounded-grant hazard. + */ + describe("grant TTL is operator-configurable", () => { + const approvedAtT0 = { + status: "approved" as const, + requestedAt: new Date(T0 - 60_000).toISOString(), + decidedAt: new Date(T0).toISOString(), + }; + + afterEach(() => { + configureApprovalRequestTtls({ grantTtlMs: undefined }); + }); + + it("honors a configured override in the expiry decision", () => { + configureApprovalRequestTtls({ grantTtlMs: 5 * 60 * 1000 }); + expect(getApprovalRequestGrantTtlMs()).toBe(5 * 60 * 1000); + // Still inside the default hour, but past the configured five minutes. + expect(isApprovalRequestExpired(approvedAtT0, T0 + 10 * 60 * 1000)).toBe(true); + expect(isApprovalRequestExpired(approvedAtT0, T0 + 60_000)).toBe(false); + }); + + it("resets to the default when the override is cleared", () => { + configureApprovalRequestTtls({ grantTtlMs: 5 * 60 * 1000 }); + configureApprovalRequestTtls({ grantTtlMs: undefined }); + expect(getApprovalRequestGrantTtlMs()).toBe(60 * 60 * 1000); + expect(isApprovalRequestExpired(approvedAtT0, T0 + 10 * 60 * 1000)).toBe(false); + }); + + it.each([0, -1, Number.NaN, Number.POSITIVE_INFINITY])( + "ignores the invalid override %p and keeps the default", + (bad) => { + configureApprovalRequestTtls({ grantTtlMs: bad }); + expect(getApprovalRequestGrantTtlMs()).toBe(60 * 60 * 1000); + }, + ); + }); +}); diff --git a/packages/core/src/__tests__/plugin-loader.route-context.test.ts b/packages/core/src/__tests__/plugin-loader.route-context.test.ts index 995989040e..98837fb9dd 100644 --- a/packages/core/src/__tests__/plugin-loader.route-context.test.ts +++ b/packages/core/src/__tests__/plugin-loader.route-context.test.ts @@ -8,7 +8,8 @@ describe("PluginLoader.createRouteContext", () => { } as any; const baseStore = { getRootDir: () => "/tmp" } as any; const loader = new PluginLoader({ pluginStore, taskStore: baseStore }); - const resolveProjectTaskStore = vi.fn(); + const projectStore = { getTask: vi.fn().mockResolvedValue({ id: "FN-1" }), deleteTask: vi.fn() } as any; + const resolveProjectTaskStore = vi.fn().mockResolvedValue(projectStore); const ctx = await loader.createRouteContext("fusion-plugin-roadmap", { taskStore: baseStore, settings: { ok: true }, @@ -17,6 +18,19 @@ describe("PluginLoader.createRouteContext", () => { expect(ctx.pluginId).toBe("fusion-plugin-roadmap"); expect(ctx.settings).toEqual({ ok: true }); - expect(ctx.resolveProjectTaskStore).toBe(resolveProjectTaskStore); + /* + FNXC:PluginTaskStoreGate 2026-07-26-13:00: + resolveProjectTaskStore is no longer passed through by identity: the loader + wraps it so resolved project stores carry the same destructive-method gate + as ctx.taskStore. Assert delegation + gating behavior instead of identity. + */ + expect(ctx.resolveProjectTaskStore).toBeDefined(); + const resolved = await ctx.resolveProjectTaskStore!("proj-1"); + expect(resolveProjectTaskStore).toHaveBeenCalledWith("proj-1"); + await expect(resolved.getTask("FN-1")).resolves.toEqual({ id: "FN-1" }); + expect(() => resolved.deleteTask("FN-1")).toThrow( + "not permitted to call deleteTask", + ); + expect(projectStore.deleteTask).not.toHaveBeenCalled(); }); }); diff --git a/packages/core/src/__tests__/plugin-task-store-gate.test.ts b/packages/core/src/__tests__/plugin-task-store-gate.test.ts new file mode 100644 index 0000000000..cb96cff33e --- /dev/null +++ b/packages/core/src/__tests__/plugin-task-store-gate.test.ts @@ -0,0 +1,131 @@ +import { describe, expect, it, vi } from "vitest"; +import { + PLUGIN_DESTRUCTIVE_TASK_STORE_METHODS, + createPluginGatedTaskStore, +} from "../plugin-task-store-gate.js"; +import type { TaskStore } from "../store.js"; + +/* +FNXC:PluginTaskStoreGate 2026-07-26-12:20: +Plugins must not be able to delete/bypass/bulk-archive tasks unless their manifest +declares permissions.destructiveTaskOps. These tests exercise the gate through a +fake plugin-context store: denylisted call without declaration throws, with +declaration passes through, and non-destructive methods are unaffected. +*/ + +function makeFakeStore() { + return { + deleteTask: vi.fn().mockResolvedValue({ id: "FN-1" }), + deleteTaskIf: vi.fn().mockResolvedValue({ id: "FN-1" }), + deleteTaskById: vi.fn().mockResolvedValue(undefined), + deleteTaskBackend: vi.fn().mockResolvedValue(undefined), + bypassFailedPreMergeReviewStep: vi.fn().mockResolvedValue({ id: "FN-1" }), + archiveAllDone: vi.fn().mockResolvedValue([]), + cleanupArchivedTasks: vi.fn().mockResolvedValue(0), + getDatabase: vi.fn().mockReturnValue({ raw: "sync-db" }), + getAsyncLayer: vi.fn().mockReturnValue({ raw: "async-layer" }), + getTask: vi.fn().mockResolvedValue({ id: "FN-1", column: "todo" }), + moveTask: vi.fn().mockResolvedValue({ id: "FN-1", column: "todo" }), + someCounter: 7, + }; +} + +describe("createPluginGatedTaskStore", () => { + it.each(PLUGIN_DESTRUCTIVE_TASK_STORE_METHODS)( + "throws for %s without a destructiveTaskOps declaration", + (method) => { + const raw = makeFakeStore(); + const gated = createPluginGatedTaskStore(raw as unknown as TaskStore, { + pluginId: "fusion-plugin-test", + }) as unknown as Record unknown>; + + expect(() => gated[method]("FN-1")).toThrow( + `Plugin fusion-plugin-test is not permitted to call ${method}; ` + + `declare permissions.destructiveTaskOps in the plugin manifest`, + ); + expect((raw as unknown as Record>)[method]).not.toHaveBeenCalled(); + }, + ); + + /* + FNXC:PluginTaskStoreGate 2026-07-26-18:25: + Hardcoded raw-handle expectations (NOT derived from the denylist constant, so a + constant regression cannot self-adjust these): the sync getDatabase handle is + denied (raw SQL around the denylist), while getAsyncLayer deliberately passes + through — four in-repo plugins depend on it for plugin-scoped schema; the + residual is documented on the denylist in plugin-task-store-gate.ts. + */ + it("denies the raw getDatabase handle without a declaration (hardcoded)", () => { + const raw = makeFakeStore(); + const gated = createPluginGatedTaskStore(raw as unknown as TaskStore, { + pluginId: "fusion-plugin-test", + }) as unknown as typeof raw; + + expect(() => gated.getDatabase()).toThrow( + "Plugin fusion-plugin-test is not permitted to call getDatabase; declare permissions.destructiveTaskOps in the plugin manifest", + ); + expect(raw.getDatabase).not.toHaveBeenCalled(); + }); + + it("keeps getAsyncLayer passing through (documented residual; plugins rely on it)", () => { + const raw = makeFakeStore(); + const gated = createPluginGatedTaskStore(raw as unknown as TaskStore, { + pluginId: "fusion-plugin-test", + }) as unknown as typeof raw; + + expect(gated.getAsyncLayer()).toEqual({ raw: "async-layer" }); + }); + + it("passes destructive calls through when the manifest declares destructiveTaskOps", async () => { + const raw = makeFakeStore(); + const gated = createPluginGatedTaskStore(raw as unknown as TaskStore, { + pluginId: "fusion-plugin-test", + permissions: { destructiveTaskOps: true }, + }) as unknown as typeof raw; + + await gated.deleteTask("FN-1"); + await gated.archiveAllDone(); + expect(raw.deleteTask).toHaveBeenCalledWith("FN-1"); + expect(raw.archiveAllDone).toHaveBeenCalledOnce(); + }); + + it("leaves non-destructive methods and plain properties untouched", async () => { + const raw = makeFakeStore(); + const gated = createPluginGatedTaskStore(raw as unknown as TaskStore, { + pluginId: "fusion-plugin-test", + }) as unknown as typeof raw; + + await expect(gated.getTask("FN-1")).resolves.toEqual({ id: "FN-1", column: "todo" }); + await gated.moveTask("FN-1", "todo"); + expect(raw.moveTask).toHaveBeenCalledWith("FN-1", "todo"); + expect(gated.someCounter).toBe(7); + }); + + it("binds pass-through methods to the raw store so store-identity seams survive", async () => { + const raw = makeFakeStore(); + let observedThis: unknown; + (raw as Record).whoAmI = function (this: unknown) { + observedThis = this; + return "ok"; + }; + const gated = createPluginGatedTaskStore(raw as unknown as TaskStore, { + pluginId: "fusion-plugin-test", + }) as unknown as Record unknown>; + + expect(gated.whoAmI()).toBe("ok"); + expect(observedThis).toBe(raw); + // Bound method identity is stable across property reads. + expect(gated.whoAmI).toBe(gated.whoAmI); + }); + + it("rejects when a denylisted method is awaited", async () => { + const raw = makeFakeStore(); + const gated = createPluginGatedTaskStore(raw as unknown as TaskStore, { + pluginId: "fusion-plugin-test", + }) as unknown as typeof raw; + + await expect(async () => gated.bypassFailedPreMergeReviewStep("FN-1")).rejects.toThrow( + "not permitted to call bypassFailedPreMergeReviewStep", + ); + }); +}); diff --git a/packages/core/src/__tests__/postgres/approval-request-lifecycle.pg.test.ts b/packages/core/src/__tests__/postgres/approval-request-lifecycle.pg.test.ts new file mode 100644 index 0000000000..1f0c5b006e --- /dev/null +++ b/packages/core/src/__tests__/postgres/approval-request-lifecycle.pg.test.ts @@ -0,0 +1,167 @@ +import { describe, it, expect, afterEach } from "vitest"; +import { execSync } from "node:child_process"; +import { createAsyncDataLayer, type AsyncDataLayer } from "../../postgres/data-layer.js"; + +const PG_TEST_URL_BASE = + process.env.FUSION_PG_TEST_URL_BASE ?? "postgresql://localhost:5432"; +const PG_AVAILABLE = + process.env.FUSION_PG_TEST_SKIP !== "1" && Boolean(PG_TEST_URL_BASE); + +const pgDescribe = PG_AVAILABLE ? describe : describe.skip; + +function uniqueDbName(): string { + return `fusion_sat_test_${process.pid}_${Math.random().toString(36).slice(2, 8)}`; +} + +/* +FNXC:PgTestAuthFix 2026-07-14-00:00: +The inline adminExec used process.env.USER for the psql -U flag, which is 'runner' on GitHub Actions (not 'postgres'). Use the PG_TEST_URL_BASE connection string instead so credentials are always correct. +*/ +function adminExec(statement: string): void { + execSync( + `psql "${PG_TEST_URL_BASE}/postgres" -v ON_ERROR_STOP=1 -c "${statement.replace(/"/g, '\\"')}"`, + { stdio: "pipe", env: process.env }, + ); +} + +interface StoreTestCtx { + dbName: string; + layer: AsyncDataLayer; +} + +async function setupCtx(): Promise { + const dbName = uniqueDbName(); + try { adminExec(`DROP DATABASE IF EXISTS "${dbName}"`); } catch { /* may not exist */ } + adminExec(`CREATE DATABASE "${dbName}"`); + const testUrl = `${PG_TEST_URL_BASE}/${dbName}`; + const { createConnectionSetFromUrl } = await import("../../postgres/connection.js"); + const { applySchemaBaseline } = await import("../../postgres/schema-applier.js"); + const { resolveBackendWithOptions } = await import("../../postgres/backend-resolver.js"); + const backend = resolveBackendWithOptions({ databaseUrl: testUrl, databaseMigrationUrl: testUrl }); + const connections = await createConnectionSetFromUrl(backend, { poolMax: 3, connectTimeoutSeconds: 5 }); + await applySchemaBaseline(connections.migration); + const layer = createAsyncDataLayer(connections); + return { dbName, layer }; +} + +async function teardownCtx(ctx: StoreTestCtx | null): Promise { + if (!ctx) return; + try { await ctx.layer.close(); } catch { /* best-effort */ } + try { adminExec(`DROP DATABASE IF EXISTS "${ctx.dbName}"`); } catch { /* best-effort */ } +} + +/* +FNXC:ApprovalLifecycleSecurity 2026-07-30-13:10 (ported from the deleted sync branch): +These assertions arrived with the approval-hardening work as `approval-request-store-lifecycle.test.ts`, +which drove the store's SYNC SQLite branch through an in-memory double. The PostgreSQL migration deleted +that branch, so the original file tested code that no longer exists — and deleting it outright would have +left the hardening (atomic guarded decide/complete, lazy TTL expiry, requester-ownership on redemption) +with no coverage at all on the path that actually runs. + +Same contract, re-pointed at the async/PG implementation. + +WHAT THESE DO AND DO NOT COVER, measured by reverting each guard in turn rather than assumed: + - transition rules (replay, approve-then-deny, complete-while-pending) -> 3 of 6 fail when removed + - requester-ownership on redemption -> 1 of 6 fails when removed + - the `AND status = ?` guard on the UPDATE -> 0 fail when removed + +That last line is the honest limit. The in-transaction re-read already rejects a replay single-threaded, +so the guard only earns its keep against a racer committing BETWEEN the read and the write — which needs +two concurrent transactions these tests do not create. The guard stays because the race is real; it is +simply not what is verified here. Do not read a green run as proof of it. +*/ +pgDescribe("approval request lifecycle security (PostgreSQL)", () => { + let ctx: StoreTestCtx | null = null; + afterEach(async () => { + await teardownCtx(ctx); + ctx = null; + }); + + const REQUESTER = { actorId: "agent-1", actorType: "agent" as const, actorName: "Bot" }; + const DECIDER = { actorId: "user-1", actorType: "user" as const, actorName: "Admin" }; + + async function seed(id: string) { + const store = await import("../../async-approval-request-store.js"); + await store.createApprovalRequest(ctx!.layer, { + id, + requester: REQUESTER, + targetAction: { category: "shell", action: "exec", summary: "run cmd", resourceType: "host", resourceId: "local", context: { cmd: "ls" } }, + }); + return store; + } + + it("a same-verdict replay is rejected as an invalid transition", async () => { + ctx = await setupCtx(); + const store = await seed("apr-replay"); + await store.decideApprovalRequest(ctx.layer, "apr-replay", "approved", { actor: DECIDER }); + + await expect( + store.decideApprovalRequest(ctx.layer, "apr-replay", "approved", { actor: DECIDER }), + ).rejects.toThrow(/Invalid approval request transition/); + }); + + it("a replay does not re-stamp decidedAt or append a duplicate audit event", async () => { + ctx = await setupCtx(); + const store = await seed("apr-nodup"); + const first = await store.decideApprovalRequest(ctx.layer, "apr-nodup", "approved", { actor: DECIDER }); + const auditBefore = await store.getApprovalAuditHistory(ctx.layer.db, "apr-nodup"); + + await expect( + store.decideApprovalRequest(ctx.layer, "apr-nodup", "approved", { actor: DECIDER }), + ).rejects.toThrow(/Invalid approval request transition/); + + const after = await store.getApprovalRequest(ctx.layer.db, "apr-nodup"); + expect(after?.decidedAt).toBe(first.decidedAt); + expect(await store.getApprovalAuditHistory(ctx.layer.db, "apr-nodup")).toHaveLength(auditBefore.length); + }); + + it("approve then deny is rejected — the first decision stands", async () => { + ctx = await setupCtx(); + const store = await seed("apr-flip"); + await store.decideApprovalRequest(ctx.layer, "apr-flip", "approved", { actor: DECIDER }); + + await expect( + store.decideApprovalRequest(ctx.layer, "apr-flip", "denied", { actor: DECIDER }), + ).rejects.toThrow(/Invalid approval request transition/); + expect((await store.getApprovalRequest(ctx.layer.db, "apr-flip"))?.status).toBe("approved"); + }); + + it("deciding a request that does not exist reports not-found", async () => { + ctx = await setupCtx(); + const store = await import("../../async-approval-request-store.js"); + + await expect( + store.decideApprovalRequest(ctx.layer, "apr-missing", "approved", { actor: DECIDER }), + ).rejects.toThrow(/not found/); + }); + + it("markCompleted on a still-pending request is rejected", async () => { + ctx = await setupCtx(); + const store = await seed("apr-pending"); + + await expect( + store.markApprovalRequestCompleted(ctx.layer, "apr-pending", { actor: DECIDER }), + ).rejects.toThrow(/Invalid approval request transition/); + }); + + it("a grant can only be redeemed by the actor it was issued to", async () => { + /* + The ownership check is the containment that matters: without it any caller who learned a request id + could redeem someone else's approved grant. + */ + ctx = await setupCtx(); + const store = await seed("apr-owner"); + await store.decideApprovalRequest(ctx.layer, "apr-owner", "approved", { actor: DECIDER }); + + await expect( + store.markApprovalRequestCompleted(ctx.layer, "apr-owner", { actor: DECIDER, expectedRequesterActorId: "someone-else" }), + ).rejects.toThrow(/requester mismatch/); + expect((await store.getApprovalRequest(ctx.layer.db, "apr-owner"))?.status).toBe("approved"); + + const completed = await store.markApprovalRequestCompleted(ctx.layer, "apr-owner", { + actor: DECIDER, + expectedRequesterActorId: REQUESTER.actorId, + }); + expect(completed.status).toBe("completed"); + }); +}); diff --git a/packages/core/src/__tests__/postgres/satellite-db-injected-stores.test.ts b/packages/core/src/__tests__/postgres/satellite-db-injected-stores.test.ts index 2f32d52607..9c70996779 100644 --- a/packages/core/src/__tests__/postgres/satellite-db-injected-stores.test.ts +++ b/packages/core/src/__tests__/postgres/satellite-db-injected-stores.test.ts @@ -210,6 +210,67 @@ pgDescribe("PostgreSQL satellite DB-injected stores (VAL-DATA-016)", () => { expect(history.length).toBeGreaterThanOrEqual(3); // created + approved + completed }); + /* + FNXC:ApprovalLifecycleSecurity 2026-07-26-13:50: + Replay/conflict/expiry/ownership hardening for the async approval store: a replayed decision must throw + the invalid-transition error the dashboard maps to HTTP 409 (it previously re-stamped decidedAt and + forged a duplicate audit event), completed grants expire 15 minutes after decidedAt, and markCompleted + enforces the requester-ownership check. + */ + it("ApprovalRequestStore: replayed/conflicting decisions 409, grants expire, ownership enforced", async () => { + ctx = await setupCtx(); + const { createApprovalRequest, decideApprovalRequest, markApprovalRequestCompleted, getApprovalAuditHistory } = await import("../../async-approval-request-store.js"); + const { eq } = await import("drizzle-orm"); + const schema = await import("../../postgres/schema/index.js"); + const requester = { actorId: "agent-1", actorType: "agent" as const, actorName: "Bot" }; + const admin = { actorId: "user-1", actorType: "user" as const, actorName: "Admin" }; + await createApprovalRequest(ctx.layer, { + id: "apr-2", + requester, + targetAction: { category: "shell", action: "exec", summary: "run cmd", resourceType: "host", resourceId: "local" }, + }); + await decideApprovalRequest(ctx.layer, "apr-2", "approved", { actor: admin }); + + // Replay approve -> conflict; conflicting deny -> conflict; audit history stays unforged. + await expect(decideApprovalRequest(ctx.layer, "apr-2", "approved", { actor: admin })).rejects.toThrow( + "Invalid approval request transition: approved -> approved", + ); + await expect(decideApprovalRequest(ctx.layer, "apr-2", "denied", { actor: admin })).rejects.toThrow( + "Invalid approval request transition: approved -> denied", + ); + expect((await getApprovalAuditHistory(ctx.layer.db, "apr-2")).map((e) => e.eventType)).toEqual([ + "created", + "approved", + ]); + + // Ownership: a different runtime cannot burn agent-1's grant. + await expect( + markApprovalRequestCompleted(ctx.layer, "apr-2", { actor: admin, expectedRequesterActorId: "agent-2" }), + ).rejects.toThrow("Approval request apr-2 requester mismatch"); + + // Expiry: backdate decidedAt past the 15-minute grant TTL -> redemption fails closed. + /* + FNXC:ApprovalLifecycleSecurity 2026-07-30-13:40 (TTL is configurable now — stop hardcoding the default): + This offset was written as 16 minutes against the original 15-minute grant TTL. The review follow-up + raised the DEFAULT to one hour and made it configurable, which left this assertion asserting nothing: + a 16-minute-old grant is simply valid now, so the redemption succeeded and the test failed. + + Pin the TTL for the test instead of chasing the default, so the expiry rule is what is under test + rather than whatever the shipping default happens to be. + */ + const { configureApprovalRequestTtls } = await import("../../types/agents.js"); + configureApprovalRequestTtls({ grantTtlMs: 60_000 }); + const staleDecidedAt = new Date(Date.now() - 5 * 60 * 1000).toISOString(); + await ctx.layer.db + .update(schema.project.approvalRequests) + .set({ decidedAt: staleDecidedAt }) + .where(eq(schema.project.approvalRequests.id, "apr-2")); + await expect(markApprovalRequestCompleted(ctx.layer, "apr-2", { actor: requester })).rejects.toThrow( + "Approval request apr-2 expired", + ); + configureApprovalRequestTtls({ grantTtlMs: undefined }); + }); + // ── EvalStore ── it("EvalStore: create run → upsert result → list → append event", async () => { diff --git a/packages/core/src/__tests__/session-identity-registry.test.ts b/packages/core/src/__tests__/session-identity-registry.test.ts new file mode 100644 index 0000000000..343a78aba7 --- /dev/null +++ b/packages/core/src/__tests__/session-identity-registry.test.ts @@ -0,0 +1,88 @@ +import { beforeEach, describe, expect, it } from "vitest"; +import { mkdtempSync, realpathSync, rmSync, symlinkSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { + __clearFusionSessionIdentityRegistryForTests, + registerFusionSessionIdentity, + resolveFusionSessionPrincipal, +} from "../session-identity-registry.js"; + +/* +FNXC:SessionIdentity 2026-07-26-15:20: +The registry is the extension-side principal signal: absent registration means +human operator CLI; present means engine-owned agent session; two live +registrations on one cwd is ambiguous and must fail closed (agent). These +semantics are what the extension's destructive-tool withholding relies on. +*/ + +describe("session identity registry", () => { + beforeEach(() => { + __clearFusionSessionIdentityRegistryForTests(); + }); + + it("unregistered cwd resolves to operator", () => { + expect(resolveFusionSessionPrincipal("/tmp/nowhere-registered")).toEqual({ kind: "operator" }); + }); + + it("registered cwd resolves to the agent identity", () => { + const dispose = registerFusionSessionIdentity("/tmp/wt-a", { agentId: "executor-FN-1", taskId: "FN-1" }); + const principal = resolveFusionSessionPrincipal("/tmp/wt-a"); + expect(principal.kind).toBe("agent"); + if (principal.kind === "agent") { + expect(principal.identity.agentId).toBe("executor-FN-1"); + expect(principal.identity.taskId).toBe("FN-1"); + } + dispose(); + expect(resolveFusionSessionPrincipal("/tmp/wt-a")).toEqual({ kind: "operator" }); + }); + + it("two live sessions on one cwd resolve to ambiguous (fail closed)", () => { + registerFusionSessionIdentity("/tmp/project-root", { agentId: "agent-1" }); + registerFusionSessionIdentity("/tmp/project-root", { agentId: "agent-2" }); + const principal = resolveFusionSessionPrincipal("/tmp/project-root"); + expect(principal.kind).toBe("ambiguous"); + }); + + it("dispose is idempotent and only removes its own entry", () => { + const disposeA = registerFusionSessionIdentity("/tmp/shared", { agentId: "agent-a" }); + registerFusionSessionIdentity("/tmp/shared", { agentId: "agent-b" }); + disposeA(); + disposeA(); + const principal = resolveFusionSessionPrincipal("/tmp/shared"); + expect(principal.kind).toBe("agent"); + if (principal.kind === "agent") { + expect(principal.identity.agentId).toBe("agent-b"); + } + }); + + /* + FNXC:SessionIdentity 2026-07-26-18:30: + Review finding: the earlier version registered and resolved the SAME canonical + string, which passes even if canonicalizeCwd stops resolving symlinks. Register + through a genuine symlink alias and resolve through the real path (and vice + versa) so the realpath folding is actually exercised. Cleanup removes both + temporary artifacts. + */ + it("resolves a symlink alias and its real path to one key", () => { + const real = realpathSync(mkdtempSync(join(tmpdir(), "fusion-idreg-"))); + const alias = `${real}-alias`; + symlinkSync(real, alias, "dir"); + try { + const dispose = registerFusionSessionIdentity(alias, { agentId: "agent-real" }); + const viaReal = resolveFusionSessionPrincipal(real); + expect(viaReal.kind).toBe("agent"); + if (viaReal.kind === "agent") { + expect(viaReal.identity.agentId).toBe("agent-real"); + } + const viaAlias = resolveFusionSessionPrincipal(alias); + expect(viaAlias.kind).toBe("agent"); + dispose(); + expect(resolveFusionSessionPrincipal(real)).toEqual({ kind: "operator" }); + expect(resolveFusionSessionPrincipal(alias)).toEqual({ kind: "operator" }); + } finally { + rmSync(alias, { force: true }); + rmSync(real, { recursive: true, force: true }); + } + }); +}); diff --git a/packages/core/src/approval-request-store.ts b/packages/core/src/approval-request-store.ts index 6ac8f812d7..88731aa21b 100644 --- a/packages/core/src/approval-request-store.ts +++ b/packages/core/src/approval-request-store.ts @@ -5,6 +5,12 @@ 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, diff --git a/packages/core/src/async-approval-request-store.ts b/packages/core/src/async-approval-request-store.ts index 787fcb3baa..c6108330fe 100644 --- a/packages/core/src/async-approval-request-store.ts +++ b/packages/core/src/async-approval-request-store.ts @@ -21,6 +21,10 @@ import { and, desc, eq, sql } from "drizzle-orm"; import * as schema from "./postgres/schema/index.js"; import type { AsyncDataLayer, DbTransaction } from "./postgres/data-layer.js"; +// FNXC:ApprovalLifecycleSecurity 2026-07-26-12:25: +// isApprovalRequestExpired lives in types/agents.js and is imported directly (types.ts re-exports +// explicit names, not `export *`, and the barrel is out of this change's file scope). +import { isApprovalRequestExpired } from "./types/agents.js"; import { isValidApprovalRequestTransition, normalizeApprovalRequestActionCategory, @@ -247,6 +251,14 @@ export async function listApprovalRequests( * FNXC:ApprovalRequestStore 2026-06-24-07:45: * Decide (approve/deny) an approval request. The status update and the audit * event run in a single transaction. Throws on invalid transition. + * + * FNXC:ApprovalLifecycleSecurity 2026-07-26-12:25: + * Read-validate-update is atomic. The previous shape read `existing` OUTSIDE the transaction and never + * re-checked inside, so concurrent approve+deny both validated against "pending" and the last write won. + * Fix: re-read via the tx handle, re-validate, then a GUARDED update + * (`WHERE id = ? AND status = `) with `.returning(...)` — an empty returning array means + * someone raced the transition, and we re-read + throw the invalid-transition error the dashboard maps to + * HTTP 409. Expired pending rows (lazy TTL, no schema change) are rejected inside the transaction too. */ export async function decideApprovalRequest( layer: AsyncDataLayer, @@ -254,45 +266,89 @@ export async function decideApprovalRequest( status: "approved" | "denied", input: ApprovalRequestDecisionInput, ): Promise { - const existing = await getApprovalRequest(layer.db, requestId); - if (!existing) throw new Error(`Approval request ${requestId} not found`); - if (!isValidApprovalRequestTransition(existing.status, status)) { - throw new Error(`Invalid approval request transition: ${existing.status} -> ${status}`); - } const now = new Date().toISOString(); - await layer.transactionImmediate(async (tx) => { - await tx + return layer.transactionImmediate(async (tx) => { + const existing = await getApprovalRequest(tx, requestId); + if (!existing) throw new Error(`Approval request ${requestId} not found`); + if (!isValidApprovalRequestTransition(existing.status, status)) { + throw new Error(`Invalid approval request transition: ${existing.status} -> ${status}`); + } + if (existing.status === "pending" && isApprovalRequestExpired(existing)) { + throw new Error(`Approval request ${requestId} expired`); + } + const updatedRows = await tx .update(schema.project.approvalRequests) .set({ status, decidedAt: now, updatedAt: now }) - .where(eq(schema.project.approvalRequests.id, requestId)); + .where( + and( + eq(schema.project.approvalRequests.id, requestId), + eq(schema.project.approvalRequests.status, existing.status), + ), + ) + .returning({ id: schema.project.approvalRequests.id }); + if (updatedRows.length === 0) { + const raced = await getApprovalRequest(tx, requestId); + throw new Error( + `Invalid approval request transition: ${raced?.status ?? existing.status} -> ${status}`, + ); + } await appendAuditEvent(tx, layer.projectId ?? "", requestId, status, input.actor, now, input.note); + return (await getApprovalRequest(tx, requestId))!; }); - return (await getApprovalRequest(layer.db, requestId))!; } /** * Mark an approval request as completed. The status update and the audit * event run in a single transaction. Throws on invalid transition. + * + * FNXC:ApprovalLifecycleSecurity 2026-07-26-12:25: + * Same atomic read-validate-guarded-update shape as decideApprovalRequest. Additionally: + * - Ownership: when input.expectedRequesterActorId is provided it must match the row's requester actorId; + * previously any runtime holding a request id could burn another agent's approval. + * - Expiry: an approved grant is redeemable only within APPROVAL_REQUEST_GRANT_TTL_MS of decidedAt + * (live DB had 17 approved / 0 completed grants redeemable forever; TTL bounds the window without a + * schema migration; redemption-side enforcement also lands in the engine gate separately). */ export async function markApprovalRequestCompleted( layer: AsyncDataLayer, requestId: string, input: ApprovalRequestCompletionInput, ): Promise { - const existing = await getApprovalRequest(layer.db, requestId); - if (!existing) throw new Error(`Approval request ${requestId} not found`); - if (!isValidApprovalRequestTransition(existing.status, "completed")) { - throw new Error(`Invalid approval request transition: ${existing.status} -> completed`); - } const now = new Date().toISOString(); - await layer.transactionImmediate(async (tx) => { - await tx + return layer.transactionImmediate(async (tx) => { + const existing = await getApprovalRequest(tx, requestId); + if (!existing) throw new Error(`Approval request ${requestId} not found`); + if (!isValidApprovalRequestTransition(existing.status, "completed")) { + throw new Error(`Invalid approval request transition: ${existing.status} -> completed`); + } + if ( + input.expectedRequesterActorId !== undefined && + input.expectedRequesterActorId !== existing.requester.actorId + ) { + throw new Error(`Approval request ${requestId} requester mismatch`); + } + if (existing.status === "approved" && isApprovalRequestExpired(existing)) { + throw new Error(`Approval request ${requestId} expired`); + } + const updatedRows = await tx .update(schema.project.approvalRequests) .set({ status: "completed", completedAt: now, updatedAt: now }) - .where(eq(schema.project.approvalRequests.id, requestId)); + .where( + and( + eq(schema.project.approvalRequests.id, requestId), + eq(schema.project.approvalRequests.status, existing.status), + ), + ) + .returning({ id: schema.project.approvalRequests.id }); + if (updatedRows.length === 0) { + const raced = await getApprovalRequest(tx, requestId); + throw new Error( + `Invalid approval request transition: ${raced?.status ?? existing.status} -> completed`, + ); + } await appendAuditEvent(tx, layer.projectId ?? "", requestId, "completed", input.actor, now, input.note); + return (await getApprovalRequest(tx, requestId))!; }); - return (await getApprovalRequest(layer.db, requestId))!; } /** diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 4f8a28573d..0933d6eb63 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -1,4 +1,4 @@ -export { COLUMNS, DEFAULT_COLUMN, isColumn, normalizeColumnId, COLUMN_LABELS, COLUMN_DESCRIPTIONS, VALID_TRANSITIONS, DEFAULT_SETTINGS, DEFAULT_GLOBAL_SETTINGS, DEFAULT_PROJECT_SETTINGS, GLOBAL_SETTINGS_KEYS, PROJECT_SETTINGS_KEYS, isGlobalSettingsKey, isProjectSettingsKey, isMergeRequestContractShadowEnabled, resolvePersistAgentThinkingLog, THINKING_LEVELS, ANTHROPIC_AUTH_PREFERENCES, THEME_MODES, COLOR_THEMES, SUPPORTED_LOCALES, DEFAULT_LOCALE, isLocale, AGENT_PERMISSIONS, PERMANENT_AGENT_ACTION_CATEGORIES, AGENT_PERMISSION_POLICY_ACTION_CATEGORIES, AGENT_PROVISIONING_APPROVAL_MODES, SANDBOX_PROVISIONING_APPROVAL_MODES, AGENT_PERMISSION_POLICY_PRESET_IDS, LEGACY_AGENT_PERMISSION_POLICY_ACTION_CATEGORY_ALIASES, APPROVAL_REQUEST_STATUSES, APPROVAL_REQUEST_AUDIT_EVENT_TYPES, normalizeApprovalRequestActionCategory, isValidApprovalRequestTransition, agentToConfigSnapshot, diffConfigSnapshots, isEphemeralAgent, hasAgentIdentity, CheckoutConflictError, DEFAULT_HEARTBEAT_PROCEDURE_PATH, getDefaultHeartbeatProcedurePath, EXECUTION_MODES, DEFAULT_EXECUTION_MODE, PLANNER_OVERSIGHT_LEVELS, DEFAULT_PLANNER_OVERSIGHT_LEVEL, TASK_PRIORITIES, DEFAULT_TASK_PRIORITY, WORKFLOW_WORK_ITEM_KINDS, WORKFLOW_WORK_ITEM_STATES, HIGH_FANOUT_BLOCKER_TODO_THRESHOLD, STALE_HIGH_FANOUT_BLOCKER_AGE_THRESHOLD_MS, REVIEW_ARTIFACTS_MODES, LIVE_DEMO_ARTIFACT_MIME_TYPE, isReviewArtifact, parseReviewArtifactsModeOverride, resolveReviewArtifactsMode, classifyReviewArtifactTask, isReviewArtifactGenerationEligible, DASHBOARD_USER_ID, normalizeMessageParticipant, validateMessageMetadata, resolveEphemeralTaskCreationPolicy, validateDockerNodeConfig, sanitizeDockerNodeConfigForResponse, normalizeMergeIntegrationWorktreeMode, normalizeMergeAdvanceAutoSyncMode, DEFAULT_GITLAB_API_BASE_URL, DEFAULT_GITLAB_INSTANCE_URL, resolveGitlabConfig, resolveGitlabEnabled, MERGE_ADVANCE_AUTO_SYNC_MODES, normalizeMergeConflictStrategy, normalizeMergeStrategyOverlapBehavior, normalizePostMergeAuditMode, POST_MERGE_AUDIT_MODES, normalizeMergeAuditAutoRecovery, MERGE_AUDIT_AUTO_RECOVERY_MODES, normalizeMergerMode, MERGER_MODES, normalizeAutoRecovery, AUTO_RECOVERY_MODES, buildResearchDocumentKey, REPO_OVERRIDE_RE, SHARED_STATE_SNAPSHOT_VERSION, sanitizeCliAgentSettings, sanitizeCliAgentsSettings, sanitizeMcpServers, CLI_AGENT_ADAPTER_IDS, CLI_AGENT_AUTONOMY_MODES, isMcpSecretRef, OVERSEER_INTERVENTION_MUTATION } from "./types.js"; +export { COLUMNS, DEFAULT_COLUMN, isColumn, normalizeColumnId, COLUMN_LABELS, COLUMN_DESCRIPTIONS, VALID_TRANSITIONS, DEFAULT_SETTINGS, DEFAULT_GLOBAL_SETTINGS, DEFAULT_PROJECT_SETTINGS, GLOBAL_SETTINGS_KEYS, PROJECT_SETTINGS_KEYS, isGlobalSettingsKey, isProjectSettingsKey, isMergeRequestContractShadowEnabled, resolvePersistAgentThinkingLog, THINKING_LEVELS, ANTHROPIC_AUTH_PREFERENCES, THEME_MODES, COLOR_THEMES, SUPPORTED_LOCALES, DEFAULT_LOCALE, isLocale, AGENT_PERMISSIONS, PERMANENT_AGENT_ACTION_CATEGORIES, AGENT_PERMISSION_POLICY_ACTION_CATEGORIES, AGENT_PROVISIONING_APPROVAL_MODES, SANDBOX_PROVISIONING_APPROVAL_MODES, AGENT_PERMISSION_POLICY_PRESET_IDS, LEGACY_AGENT_PERMISSION_POLICY_ACTION_CATEGORY_ALIASES, APPROVAL_REQUEST_STATUSES, APPROVAL_REQUEST_AUDIT_EVENT_TYPES, APPROVAL_REQUEST_PENDING_TTL_MS, APPROVAL_REQUEST_GRANT_TTL_MS, isApprovalRequestExpired, configureApprovalRequestTtls, getApprovalRequestGrantTtlMs, normalizeApprovalRequestActionCategory, isValidApprovalRequestTransition, agentToConfigSnapshot, diffConfigSnapshots, isEphemeralAgent, hasAgentIdentity, CheckoutConflictError, DEFAULT_HEARTBEAT_PROCEDURE_PATH, getDefaultHeartbeatProcedurePath, EXECUTION_MODES, DEFAULT_EXECUTION_MODE, PLANNER_OVERSIGHT_LEVELS, DEFAULT_PLANNER_OVERSIGHT_LEVEL, TASK_PRIORITIES, DEFAULT_TASK_PRIORITY, WORKFLOW_WORK_ITEM_KINDS, WORKFLOW_WORK_ITEM_STATES, HIGH_FANOUT_BLOCKER_TODO_THRESHOLD, STALE_HIGH_FANOUT_BLOCKER_AGE_THRESHOLD_MS, REVIEW_ARTIFACTS_MODES, LIVE_DEMO_ARTIFACT_MIME_TYPE, isReviewArtifact, parseReviewArtifactsModeOverride, resolveReviewArtifactsMode, classifyReviewArtifactTask, isReviewArtifactGenerationEligible, DASHBOARD_USER_ID, normalizeMessageParticipant, validateMessageMetadata, resolveEphemeralTaskCreationPolicy, validateDockerNodeConfig, sanitizeDockerNodeConfigForResponse, normalizeMergeIntegrationWorktreeMode, normalizeMergeAdvanceAutoSyncMode, DEFAULT_GITLAB_API_BASE_URL, DEFAULT_GITLAB_INSTANCE_URL, resolveGitlabConfig, resolveGitlabEnabled, MERGE_ADVANCE_AUTO_SYNC_MODES, normalizeMergeConflictStrategy, normalizeMergeStrategyOverlapBehavior, normalizePostMergeAuditMode, POST_MERGE_AUDIT_MODES, normalizeMergeAuditAutoRecovery, MERGE_AUDIT_AUTO_RECOVERY_MODES, normalizeMergerMode, MERGER_MODES, normalizeAutoRecovery, AUTO_RECOVERY_MODES, buildResearchDocumentKey, REPO_OVERRIDE_RE, SHARED_STATE_SNAPSHOT_VERSION, sanitizeCliAgentSettings, sanitizeCliAgentsSettings, sanitizeMcpServers, CLI_AGENT_ADAPTER_IDS, CLI_AGENT_AUTONOMY_MODES, isMcpSecretRef, OVERSEER_INTERVENTION_MUTATION } from "./types.js"; export type { VoiceInputSettings, Column, ColumnId, IssueInfo, IssueState, TaskSourceIssue, TaskGitLabTracking, TaskGitLabTrackedItem, GitLabTrackedItemKind, PrInfo, PrConflictState, PrConflictDiagnostics, PrCheckState, PrCheckStatus, PrStatus, BranchGroup, BranchGroupCreateInput, BranchGroupUpdate, BranchGroupPrState, Task, TaskTokenUsage, TaskTokenUsagePerModel, TaskAttachment, TaskComment, TaskCommentInput, TaskDocument, TaskDocumentRevision, TaskDocumentCreateInput, ArchivedTaskDocumentAdditionInput, ArchivedTaskDocumentAdditionResult, TaskDocumentWithTask, ArtifactType, Artifact, ArtifactCreateInput, ArtifactWithTask, TaskCreateInput, TaskSource, SourceType, TaskDetail, RetrySummary, InboxTask, TodoList, TodoItem, TodoListCreateInput, TodoListUpdateInput, TodoItemCreateInput, TodoItemUpdateInput, TodoListWithItems, AgentLogEntry, AgentLogType, AgentRole, BoardConfig, DistributedTaskIdReserveInput, DistributedTaskIdReserveResult, DistributedTaskIdCommitInput, DistributedTaskIdCommitResult, DistributedTaskIdAbortInput, DistributedTaskIdAbortResult, DistributedTaskIdStateInput, DistributedTaskIdStateResult, AutostashOrphanRecord, AutostashOutcome, MergeDetails, MergeResult, MergeIntegrationWorktreeMode, MergeAdvanceAutoSyncMode, MergeConflictStrategy, CanonicalMergeConflictStrategy, MergeStrategyOverlapBehavior, PostMergeAuditMode, MergeAuditAutoRecoveryMode, MergerMode, MergerSettings, AutoRecoveryMode, AutoRecoveryFailureClass, AutoRecoverySettings, DirectMergeCommitStrategy, Settings, GlobalSettings, ProjectSettings, ReportMode, ReportActionType, ReportTarget, SecretsEnvConfig, WebSearchBackend, ResearchEnabledSources, ResearchGlobalDefaults, ResearchProjectLimits, ResearchProjectSettings, SandboxBackendName, SandboxFailureMode, SandboxPolicy, SandboxProjectSettings, EvalFollowUpPolicy, EvalProjectSettings, ResolvedEvalSettings, SettingsScope, DaemonTokenSettings, TaskStep, StepStatus, TaskLogEntry, RunMutationContext, ActivityLogEntry, ActivityEventType, ThinkingLevel, AnthropicAuthPreference, ThemeMode, ColorTheme, Locale, ExecutionMode, PlannerOversightLevel, ReviewArtifactsMode, ReviewArtifactTaskClassification, TaskPriority, MergeQueueEntry, MergeQueueEnqueueOptions, MergeQueueAcquireOptions, MergeQueueReleaseOutcome, MergeRequestState, MergeRequestRecord, MergeRequestWorkflowProjectionOptions, CompletionHandoffMarker, WorkflowWorkItem, WorkflowWorkItemDueFilter, WorkflowWorkItemKind, WorkflowWorkItemState, WorkflowWorkItemTransitionPatch, WorkflowWorkItemUpsertInput, HandoffEvidence, HandoffToReviewOptions, UnavailableNodePolicy, OwningNodeHandoffPolicy, PlanningQuestion, PlanningSummary, PlanningResponse, PlanningQuestionType, ArchivedTaskEntry, BatchStatusRequest, BatchStatusResponse, BatchStatusEntry, BatchStatusResult, GithubIssueAction, ModelPreset, WorkflowStep, WorkflowStepMode, WorkflowStepGateMode, WorkflowStepPhase, WorkflowStepInput, WorkflowStepResult, WorkflowStepTemplate, Agent, OrgTreeNode, AgentState, AgentDetail, AgentCreateInput, AgentUpdateInput, AgentApiKey, AgentApiKeyCreateResult, AgentCapability, AgentPromptTemplate, AgentPromptsConfig, AgentPermission, PermanentAgentActionCategory, PermanentAgentSensitiveActionCategory, PermanentAgentGatingContext, AgentPermissionPolicy, AgentPermissionPolicyRules, AgentPermissionPolicyToolRules, AgentPermissionPolicyActionCategory, AgentProvisioningApprovalMode, SandboxProvisioningApprovalMode, LegacyAgentPermissionPolicyActionCategory, ApprovalRequestActionCategoryInput, ApprovalRequestActionCategory, AgentPermissionPolicyDisposition, AgentPermissionPolicyPresetId, ApprovalRequestStatus, ApprovalRequestAuditEventType, ApprovalRequestActorSnapshot, ApprovalRequestTargetAction, ApprovalRequestAuditEvent, ApprovalRequest, ApprovalRequestCreateInput, ApprovalRequestDecisionInput, ApprovalRequestCompletionInput, ApprovalRequestListInput, TaskAssignSource, AgentAccessState, AgentHeartbeatConfig, AgentBudgetConfig, AgentBudgetStatus, InstructionsBundleConfig, MessageResponseMode, AgentHeartbeatEvent, AgentHeartbeatRun, BlockedStateSnapshot, HeartbeatInvocationSource, AgentTaskSession, AgentRating, AgentRatingSummary, AgentRatingInput, AgentConfigSnapshot, RevisionFieldDiff, AgentConfigRevision, AgentStats, ReflectionTrigger, ReflectionMetrics, AgentReflection, AgentPerformanceSummary, NtfyNotificationEvent, NotificationEvent, NotificationPayload, NotificationProviderConfig, CustomProvider, SteeringComment, ParticipantType, MessageType, Message, MessageCreateInput, MessageFilter, MessageMetadata, ProposedTaskMetadata, EphemeralTaskCreationPolicy, MessageReplyReference, Mailbox, CheckoutLease, CheckoutClaimPrecondition, TaskClaimRow, CentralClaimStore, RunAuditDomain, RunAuditEvent, RunAuditEventInput, RunAuditEventFilter, AgentMemoryInclusionMode, HeartbeatPromptTemplate, HeartbeatScopeDisciplineMode, WorktrunkSettings, WorktrunkOnFailure, TaskBranchContext, CliAgentSettings, McpSecretRef, McpSensitiveValue, McpStdioTransport, McpSseTransport, McpStreamableHttpTransport, McpTransport, McpServerDefinition, McpServersSettings, GitlabConfigSettingsSource, ResolvedGitlabConfig, ResolveGitlabConfigInput, GitlabAuthTokenType, PlannerOversightStage, PlannerInterventionAction, PlannerInterventionOutcome, PlannerInterventionSourceLink, PlannerInterventionEntry, ExecutorOverseerSignalMemory, BackupSettingsMigrationCandidate, BackupSettingsMigrationConflict } from "./types.js"; export type { NativeStructureRef, NativeStructureEmbed, NativeStructureOpenTarget, NativeStructurePreviewPayload, NativeStructureUnavailablePayload, NativeStructurePreviewResult } from "./types.js"; export type { @@ -2662,3 +2662,16 @@ export { type TaskDeleteNoticeMailbox, type TaskDeleteNoticeSnapshot, } from "./task-delete-notice.js"; +/* +FNXC:SessionIdentity 2026-07-26-12:10: +In-process principal channel between the engine (session spawner) and the bundled +@runfusion/fusion pi extension (tool surface). Exported from core because both sides +inline core, while the actual state lives on globalThis so bundling cannot fork it. +*/ +export { + registerFusionSessionIdentity, + resolveFusionSessionPrincipal, + __clearFusionSessionIdentityRegistryForTests, + type FusionSessionIdentity, + type FusionSessionPrincipal, +} from "./session-identity-registry.js"; diff --git a/packages/core/src/plugin-loader.ts b/packages/core/src/plugin-loader.ts index 186da0d855..5f9998be4a 100644 --- a/packages/core/src/plugin-loader.ts +++ b/packages/core/src/plugin-loader.ts @@ -47,6 +47,7 @@ import { createLogger } from "./logger.js"; import { getCreateAiSessionFactory, getCreateInteractiveAiSessionFactory } from "./ai-engine-loader.js"; import { scanPluginSecurity } from "./plugin-security-scan.js"; import { resolvePluginRootFromEntryPath } from "./plugin-skill-paths.js"; +import { createPluginGatedTaskStore } from "./plugin-task-store-gate.js"; // Minimum Fusion version for plugin compatibility checks (can be expanded later) const MINIMUM_FUSION_VERSION = "0.1.0"; @@ -214,6 +215,15 @@ export class PluginLoader extends EventEmitter<{ "plugin:error": [PluginErrorEvent]; "plugin:stopped": [string]; // Kept for backward compatibility }> { + /* + FNXC:PluginTaskStoreGate 2026-07-26-12:20: + Re-exposed here (in addition to plugin-task-store-gate.ts) because the engine's + PluginRunner builds its own PluginContexts and must apply the same gate; the + core barrel (index.ts) is edit-frozen, so the already-exported PluginLoader + class is the cross-package access point. + */ + static readonly createGatedTaskStore = createPluginGatedTaskStore; + /** Loaded plugin instances keyed by plugin id */ private plugins: Map = new Map(); @@ -277,14 +287,27 @@ export class PluginLoader extends EventEmitter<{ ); } + /* + FNXC:PluginTaskStoreGate 2026-07-26-12:20: + Every context handed to a plugin carries a gated TaskStore: destructive methods + throw unless the manifest declares permissions.destructiveTaskOps. The gate also + wraps override stores and project stores resolved via resolveProjectTaskStore so + a plugin cannot escape the gate through a project-scoped handle. + */ + const permissions = this.getPlugin(pluginId)?.manifest.permissions; + const rawTaskStore = overrides?.taskStore ?? this.options.taskStore; + const rawResolveProjectTaskStore = overrides?.resolveProjectTaskStore; return { pluginId, - taskStore: overrides?.taskStore ?? this.options.taskStore, + taskStore: createPluginGatedTaskStore(rawTaskStore, { pluginId, permissions }), settings: overrides?.settings ?? await this.getPluginSettings(pluginId), logger: this.createLogger(pluginId), createAiSession, createInteractiveAiSession, - resolveProjectTaskStore: overrides?.resolveProjectTaskStore, + resolveProjectTaskStore: rawResolveProjectTaskStore + ? async (projectId: string) => + createPluginGatedTaskStore(await rawResolveProjectTaskStore(projectId), { pluginId, permissions }) + : undefined, // The host (dashboard) may supply a real publisher that forwards custom // plugin events to connected SSE clients. Absent an override, fall back to // logging (the historical no-op behavior) so non-dashboard hosts and tests diff --git a/packages/core/src/plugin-task-store-gate.ts b/packages/core/src/plugin-task-store-gate.ts new file mode 100644 index 0000000000..363c5b783a --- /dev/null +++ b/packages/core/src/plugin-task-store-gate.ts @@ -0,0 +1,103 @@ +import type { TaskStore } from "./store.js"; +import type { PluginPermissions } from "./plugin-types.js"; + +/* +FNXC:PluginTaskStoreGate 2026-07-26-12:20: +PluginContext.taskStore historically handed every plugin the FULL TaskStore, so any +plugin could delete tasks, bypass failed pre-merge review steps, or bulk-archive the +board with no gate. This module is the smallest honest gate: a Proxy over the store +that intercepts a hardcoded denylist of destructive methods and throws unless the +plugin's manifest declares `permissions: { destructiveTaskOps: true }`. Everything +not on the denylist passes through untouched, so default plugin behavior is +otherwise unchanged. +*/ + +/** + * FNXC:PluginTaskStoreGate 2026-07-26-12:20: + * Destructive-method denylist. Chosen from the TaskStore surface: + * - `deleteTask` / `deleteTaskIf` / `deleteTaskById` / `deleteTaskBackend` — every + * task-deletion entry point (public and backend seams reachable via the handle). + * - `bypassFailedPreMergeReviewStep` — the FN-7720 privileged operator bypass of a + * failed pre-merge review gate; must never be callable by an ungated plugin. + * - `archiveAllDone` — the bulk archive sweep (archiveAllDone-style bulk method). + * - `cleanupArchivedTasks` — bulk destructive removal of archived task history. + * Single-task `archiveTask` is intentionally NOT denylisted: it is reversible via + * `unarchiveTask` and gating it would break benign board-hygiene plugins. + * - `getDatabase` — the raw sync SQLite handle. No in-repo plugin uses it (the QA + * plugin explicitly documents NOT to), and a raw handle would let a plugin run + * destructive SQL around the named-method denylist, so it requires the same + * destructiveTaskOps declaration. + * + * FNXC:PluginTaskStoreGate 2026-07-26-18:20: + * KNOWN RESIDUAL (review finding, deliberately not closed here): `getAsyncLayer()` + * also exposes a raw (drizzle) handle that could execute destructive SQL outside + * the denylist. It is NOT denied because four in-repo plugins (printing-press, + * compound-engineering, glasses, quality) legitimately depend on it for their own + * plugin-scoped schema/reads — denying it breaks them, and granting them + * destructiveTaskOps to compensate would defeat the gate entirely. Making this + * airtight needs a scoped/read-only data-layer design (a follow-up), not a + * denylist entry. Until then the gate is an honest guard against the named + * destructive TaskStore surface, not a sandbox for raw SQL. + */ +export const PLUGIN_DESTRUCTIVE_TASK_STORE_METHODS = [ + "deleteTask", + "deleteTaskIf", + "deleteTaskById", + "deleteTaskBackend", + "bypassFailedPreMergeReviewStep", + "archiveAllDone", + "cleanupArchivedTasks", + "getDatabase", +] as const; + +export type PluginDestructiveTaskStoreMethod = + (typeof PLUGIN_DESTRUCTIVE_TASK_STORE_METHODS)[number]; + +export interface PluginTaskStoreGateOptions { + pluginId: string; + permissions?: PluginPermissions; +} + +/** + * FNXC:PluginTaskStoreGate 2026-07-26-12:20: + * Wrap a TaskStore for hand-off to a plugin context. When the plugin manifest + * declares `permissions.destructiveTaskOps: true` the raw store is returned + * unchanged. Otherwise a Proxy intercepts the denylisted methods and throws a + * clear declaration-pointing error. + * + * Implementation notes: + * - Non-denylisted function properties are bound to the RAW store (and cached per + * property) so `this` inside store methods is always the real TaskStore. This + * preserves WeakMap-keyed seams (e.g. task-move-disposer registration keyed by + * store identity) that would silently break if methods ran with the proxy as + * `this`. + * - The thrower is a plain sync function so both `store.deleteTask(...)` and + * `await store.deleteTask(...)` fail loudly. + */ +export function createPluginGatedTaskStore( + store: TaskStore, + options: PluginTaskStoreGateOptions, +): TaskStore { + if (options.permissions?.destructiveTaskOps === true) return store; + const denied = new Set(PLUGIN_DESTRUCTIVE_TASK_STORE_METHODS); + const boundMethodCache = new Map(); + return new Proxy(store, { + get(target, prop) { + if (denied.has(prop)) { + return () => { + throw new Error( + `Plugin ${options.pluginId} is not permitted to call ${String(prop)}; ` + + `declare permissions.destructiveTaskOps in the plugin manifest`, + ); + }; + } + const value = Reflect.get(target, prop, target); + if (typeof value !== "function") return value; + const cached = boundMethodCache.get(prop); + if (cached) return cached; + const bound = (value as (...args: unknown[]) => unknown).bind(target); + boundMethodCache.set(prop, bound); + return bound; + }, + }) as TaskStore; +} diff --git a/packages/core/src/plugin-types.ts b/packages/core/src/plugin-types.ts index bc111e9816..877e48926c 100644 --- a/packages/core/src/plugin-types.ts +++ b/packages/core/src/plugin-types.ts @@ -30,6 +30,18 @@ const SETUP_CHANNELS = ["stable", "beta", "nightly"] as const; // ── Plugin Manifest ─────────────────────────────────────────────────── +/** + * FNXC:PluginTaskStoreGate 2026-07-26-12:20: + * Opt-in privilege declarations. Plugins receive a gated TaskStore by default: + * destructive methods (deleteTask*, bypassFailedPreMergeReviewStep, archiveAllDone, + * cleanupArchivedTasks) throw unless the manifest declares + * `permissions: { destructiveTaskOps: true }`. See plugin-task-store-gate.ts. + */ +export interface PluginPermissions { + /** Allow calling destructive TaskStore methods (delete/bypass/bulk-archive). */ + destructiveTaskOps?: boolean; +} + /** * Metadata and capability declaration for a plugin. */ @@ -66,6 +78,8 @@ export interface PluginManifest { promptSurfaces?: PluginPromptSurface[]; /** Setup metadata for plugin-managed binaries/runtimes. */ setup?: PluginSetupManifest; + /** Opt-in privilege declarations (see PluginPermissions). Absent = least privilege. */ + permissions?: PluginPermissions; } // ── Plugin Setting Schema ────────────────────────────────────────────── diff --git a/packages/core/src/session-identity-registry.ts b/packages/core/src/session-identity-registry.ts new file mode 100644 index 0000000000..9df338753f --- /dev/null +++ b/packages/core/src/session-identity-registry.ts @@ -0,0 +1,124 @@ +import { resolve } from "node:path"; +import { realpathSync } from "node:fs"; + +/* +FNXC:SessionIdentity 2026-07-26-12:05: +Security incident follow-up (agent autonomously deleted a live task): pi's +ExtensionContext carries no agent identity, so host-extension (@runfusion/fusion) +tools could never distinguish "human operator at the CLI" from "engine-spawned +agent session" — every destructive fn_* tool ran as an implicit operator. +Engine agent sessions execute IN-PROCESS via pi's DefaultResourceLoader, but the +extension is a separately bundled module (core is inlined into the CLI bundle), +so ordinary module state is NOT shared between engine and extension. globalThis +is the only reliable in-process channel; this registry lives there under a +versioned key. + +Trust model: only the engine process can write this registry (agents cannot +reach the engine's globalThis), so a PRESENT entry is authoritative proof that +the cwd belongs to an engine-managed agent session. An ABSENT entry means the +process is an operator-driven CLI (interactive pi), which is the correct +default: a human terminal has no engine registration. Ambiguity (two live +sessions sharing one cwd, e.g. heartbeat lanes at the project root) fails +CLOSED — callers must treat "ambiguous" as an agent principal, never as an +operator. +*/ + +export interface FusionSessionIdentity { + /** Acting agent id (permanent or ephemeral runtime id). */ + agentId: string; + agentName?: string; + taskId?: string; + isEphemeral?: boolean; + /** Session lane, e.g. "executor", "heartbeat", "chat". Diagnostic only. */ + purpose?: string; + /** Epoch ms at registration; diagnostic only (no TTL semantics). */ + registeredAt: number; +} + +export type FusionSessionPrincipal = + | { kind: "operator" } + | { kind: "agent"; identity: FusionSessionIdentity } + | { kind: "ambiguous"; identities: FusionSessionIdentity[] }; + +const REGISTRY_KEY = "__FUSION_SESSION_IDENTITY_REGISTRY_V1__"; + +type Registry = Map; + +function getRegistry(): Registry { + const holder = globalThis as Record; + let registry = holder[REGISTRY_KEY] as Registry | undefined; + if (!(registry instanceof Map)) { + registry = new Map(); + holder[REGISTRY_KEY] = registry; + } + return registry; +} + +/** + * FNXC:SessionIdentity 2026-07-26-12:05: + * Canonicalize before keying: macOS reports temp worktrees as both /var/... and + * /private/var/..., and engine/extension may hold either spelling. realpath + * failures (deleted dir mid-lookup) fall back to resolve() so lookups never + * throw inside a tool call. + */ +function canonicalizeCwd(cwd: string): string { + const resolved = resolve(cwd); + try { + return realpathSync(resolved); + } catch { + return resolved; + } +} + +/** + * Register an engine-owned agent session for a working directory. + * Returns a dispose function; the engine MUST call it when the session ends, + * otherwise a later operator CLI in the same cwd would be misclassified as an + * agent (fail-closed direction, but operator-hostile — so lifetimes matter). + */ +export function registerFusionSessionIdentity( + cwd: string, + identity: Omit, +): () => void { + const key = canonicalizeCwd(cwd); + const registry = getRegistry(); + const entry: FusionSessionIdentity = { ...identity, registeredAt: Date.now() }; + const list = registry.get(key) ?? []; + list.push(entry); + registry.set(key, list); + + let disposed = false; + return () => { + if (disposed) return; + disposed = true; + const current = registry.get(key); + if (!current) return; + const idx = current.indexOf(entry); + if (idx >= 0) current.splice(idx, 1); + if (current.length === 0) registry.delete(key); + }; +} + +/** + * Resolve the acting principal for a session working directory. + * - No registration → operator (human CLI process; engine never registered it). + * - Exactly one live registration → that agent. + * - Multiple registrations → ambiguous; callers must fail CLOSED (treat as + * agent, withhold operator-only capabilities). + */ +export function resolveFusionSessionPrincipal(cwd: string): FusionSessionPrincipal { + const registry = getRegistry(); + const list = registry.get(canonicalizeCwd(cwd)); + if (!list || list.length === 0) { + return { kind: "operator" }; + } + if (list.length === 1) { + return { kind: "agent", identity: list[0] }; + } + return { kind: "ambiguous", identities: [...list] }; +} + +/** Test-only: wipe all registrations (isolated vitest workers share globalThis). */ +export function __clearFusionSessionIdentityRegistryForTests(): void { + (globalThis as Record)[REGISTRY_KEY] = new Map(); +} diff --git a/packages/core/src/types.ts b/packages/core/src/types.ts index 4dc0f6dae6..6753693621 100644 --- a/packages/core/src/types.ts +++ b/packages/core/src/types.ts @@ -1172,6 +1172,12 @@ import { getLegacyAgentAssetDirectoryName, getLegacyAgentInstructionsBundleDirName, getSafeAgentAssetIdSegment, + // FNXC:ApprovalLifecycleSecurity 2026-07-26-14:20: lazy approval TTLs must reach the engine gate + dashboard via the stable barrel. + APPROVAL_REQUEST_PENDING_TTL_MS, + APPROVAL_REQUEST_GRANT_TTL_MS, + getApprovalRequestGrantTtlMs, + configureApprovalRequestTtls, + isApprovalRequestExpired, isValidApprovalRequestTransition, normalizeApprovalRequestActionCategory, } from "./types/agents.js"; @@ -1197,6 +1203,11 @@ export { getLegacyAgentAssetDirectoryName, getLegacyAgentInstructionsBundleDirName, getSafeAgentAssetIdSegment, + APPROVAL_REQUEST_PENDING_TTL_MS, + APPROVAL_REQUEST_GRANT_TTL_MS, + getApprovalRequestGrantTtlMs, + configureApprovalRequestTtls, + isApprovalRequestExpired, isValidApprovalRequestTransition, normalizeApprovalRequestActionCategory, }; diff --git a/packages/core/src/types/agents.ts b/packages/core/src/types/agents.ts index 35c2abf27a..d11e5a5618 100644 --- a/packages/core/src/types/agents.ts +++ b/packages/core/src/types/agents.ts @@ -246,6 +246,15 @@ export interface PermanentAgentGatingContext { approvalDedupeKey?: string; }) => Promise; findPendingApprovalRequest?: (dedupeKey: string) => Promise; + /** + * FNXC:AgentGating 2026-07-26-14:45: + * Audit finding: the permanent-agent gate minted an approval request but never + * paused, unlike the action gate — the agent kept its turn while "awaiting + * approval" and hunted for ungated workarounds. Optional pause hook keeps the + * two gate paths consistent; context builders that supply pauseForApproval get + * the same pause-on-pending semantics as wrapToolsWithActionGate. + */ + pauseForApproval?: (info: { approvalRequestId: string; toolName: string }) => Promise; } /** Built-in permission policy preset identifiers for agent runtime policies. */ @@ -395,6 +404,13 @@ export interface ApprovalRequestDecisionInput { export interface ApprovalRequestCompletionInput { actor: ApprovalRequestActorSnapshot; note?: string; + /* + FNXC:ApprovalLifecycleSecurity 2026-07-26-12:05: + Ownership check for grant redemption. Any runtime holding a request id could previously burn another agent's approval by calling markCompleted with it. + When provided, both stores compare this against the row's requester actorId inside the transaction and throw "Approval request requester mismatch" on disagreement. + Optional so existing operator/dashboard callers keep working; the engine redemption gate passes the requesting agent's id. + */ + expectedRequesterActorId?: string; } /** Query filters for approval request listings. */ @@ -412,8 +428,16 @@ export function isValidApprovalRequestTransition( from: ApprovalRequestStatus, to: ApprovalRequestStatus, ): boolean { + /* + FNXC:ApprovalLifecycleSecurity 2026-07-26-12:05: + from===to is INVALID for every status. A replayed decision is not idempotent: re-POSTing approve on an + already-approved row re-stamped decidedAt and appended a duplicate audit event, forging audit history. + Callers must see a conflict instead — the dashboard route maps the thrown + "Invalid approval request transition: -> " message to HTTP 409, so that exact message format + is load-bearing and must not change. + */ if (from === to) { - return true; + return false; } if (from === "pending") { return to === "approved" || to === "denied"; @@ -424,6 +448,97 @@ export function isValidApprovalRequestTransition( return false; } +/* +FNXC:ApprovalLifecycleSecurity 2026-07-26-12:10: +Lazy TTL expiry for approval requests, deliberately implemented as pure functions over existing columns +(requestedAt/decidedAt) with NO schema change: a PG forward migration on a live system is avoided. +Incident context: the live DB held 17 approved / 0 completed grants, each redeemable forever — an approved +grant never expired, so any later compromise could replay old approvals. These TTLs bound the window. +Enforcement lands in both stores (decide throws on an expired pending row; markCompleted throws on an +expired approved row); redemption-side enforcement also lands in the engine gate separately. +*/ + +/** Pending approval requests are decidable for 24 hours after requestedAt. */ +export const APPROVAL_REQUEST_PENDING_TTL_MS = 24 * 60 * 60 * 1000; + +/* +FNXC:ApprovalLifecycleSecurity 2026-07-26-18:20: +The grant TTL bounds how long an approved-but-unredeemed grant stays replayable. It is a tradeoff, +not a constant: too long re-opens the "17 approved / 0 completed, redeemable forever" hazard; too +short breaks legitimate work, because approval->redemption is NOT instantaneous. The gap covers an +operator approving from their phone, an engine restart, a queued lane, or a paused task waiting on a +worktree — 15 minutes lost those routinely and the agent would silently re-request. + +Default is 1 hour: comfortably longer than a restart or queue backlog, far shorter than "forever". +Operators who need a different window override it with FUSION_APPROVAL_GRANT_TTL_MS (milliseconds), +or programmatically via configureApprovalRequestTtls() at runtime boot. +*/ +const DEFAULT_APPROVAL_REQUEST_GRANT_TTL_MS = 60 * 60 * 1000; + +/** @deprecated Read {@link getApprovalRequestGrantTtlMs} instead — the value is configurable. */ +export const APPROVAL_REQUEST_GRANT_TTL_MS = DEFAULT_APPROVAL_REQUEST_GRANT_TTL_MS; + +function parsePositiveIntEnv(raw: string | undefined): number | undefined { + if (raw === undefined) return undefined; + const parsed = Number(raw); + return Number.isFinite(parsed) && parsed > 0 ? Math.floor(parsed) : undefined; +} + +let configuredGrantTtlMs: number | undefined; + +/** + * Override the approval-grant TTL at runtime (milliseconds). Pass `undefined` to reset to the + * env/default resolution. Non-positive or non-finite values are ignored rather than throwing — + * a bad override must never widen the window to Infinity or collapse it to zero. + */ +export function configureApprovalRequestTtls(options: { grantTtlMs?: number | undefined }): void { + const next = options.grantTtlMs; + configuredGrantTtlMs = typeof next === "number" && Number.isFinite(next) && next > 0 ? Math.floor(next) : undefined; +} + +/** Resolved grant TTL: explicit runtime override, else FUSION_APPROVAL_GRANT_TTL_MS, else 1 hour. */ +export function getApprovalRequestGrantTtlMs(): number { + return ( + configuredGrantTtlMs + ?? parsePositiveIntEnv(typeof process !== "undefined" ? process.env?.FUSION_APPROVAL_GRANT_TTL_MS : undefined) + ?? DEFAULT_APPROVAL_REQUEST_GRANT_TTL_MS + ); +} + +/** + * True when an approval request is past its lazy TTL. + * + * FNXC:ApprovalLifecycleSecurity 2026-07-26-12:10: + * pending: expired once requestedAt + APPROVAL_REQUEST_PENDING_TTL_MS is exceeded. + * approved: expired once decidedAt + APPROVAL_REQUEST_GRANT_TTL_MS is exceeded; an approved row with a + * missing/invalid decidedAt is treated as expired (fail closed — an unbounded grant is the incident class). + * denied/completed: terminal, never expired. + */ +export function isApprovalRequestExpired( + request: Pick, + nowMs: number = Date.now(), +): boolean { + if (request.status === "pending") { + const requestedAtMs = Date.parse(request.requestedAt); + if (Number.isNaN(requestedAtMs)) { + return true; + } + return nowMs > requestedAtMs + APPROVAL_REQUEST_PENDING_TTL_MS; + } + if (request.status === "approved") { + if (!request.decidedAt) { + return true; + } + const decidedAtMs = Date.parse(request.decidedAt); + if (Number.isNaN(decidedAtMs)) { + return true; + } + // FNXC:ApprovalLifecycleSecurity 2026-07-26-18:20: read the resolved (configurable) TTL, not the frozen constant. + return nowMs > decidedAtMs + getApprovalRequestGrantTtlMs(); + } + return false; +} + /** Describes how an agent's task assignment capability was determined. */ export type TaskAssignSource = | "role_default" // Granted automatically by role (e.g., scheduler gets tasks:assign) diff --git a/packages/dashboard/app/api/tasks-lifecycle.ts b/packages/dashboard/app/api/tasks-lifecycle.ts index 8a658faff4..3fd1f28310 100644 --- a/packages/dashboard/app/api/tasks-lifecycle.ts +++ b/packages/dashboard/app/api/tasks-lifecycle.ts @@ -285,7 +285,17 @@ export function revertTask(id: string, projectId?: string, body?: RevertTaskOpti } export function archiveAllDone(projectId?: string): Promise { - return api<{ archived: Task[] }>(withProjectId("/tasks/archive-all-done", projectId), { method: "POST" }).then( + /* + FNXC:ArchiveConfirmGate 2026-07-26-16:30: + The bulk archive route now requires an explicit `{ confirm: true }` body (400 without + it) so non-UI callers cannot silently sweep the Done column. The UI's own user-facing + confirmation happens before this client call; this body is the machine-level ack. + */ + return api<{ archived: Task[] }>(withProjectId("/tasks/archive-all-done", projectId), { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ confirm: true }), + }).then( (response) => response.archived ); } diff --git a/packages/dashboard/src/chat.ts b/packages/dashboard/src/chat.ts index 223c9bbc77..93cdadb0f5 100644 --- a/packages/dashboard/src/chat.ts +++ b/packages/dashboard/src/chat.ts @@ -459,14 +459,16 @@ async function createChatMissionGateContexts( createApprovalRequest, findApprovalByDedupeKey: async (dedupeKey) => { const latest = await approvalStore.findLatestByDedupeKey({ requesterActorId: agent.id, dedupeKey }); - return latest ? { id: latest.id, status: latest.status } : null; + // FNXC:ApprovalRedemption 2026-07-26-13:50: decidedAt lets resolveGateOutcome apply the approval-grant TTL at redemption. + return latest ? { id: latest.id, status: latest.status, decidedAt: latest.decidedAt } : null; }, pauseForApproval: async () => { await agentStore.updateAgentState(agent.id, "paused"); await agentStore.updateAgent(agent.id, { pauseReason: "awaiting-approval" }); }, markApprovalCompleted: async (approvalRequestId) => { - await approvalStore.markCompleted(approvalRequestId, { actor: requester, note: "Tool executed after approval" }); + // FNXC:ApprovalRedemption 2026-07-26-14:35: ownership guard — an agent must not be able to burn another agent's approval by id. + await approvalStore.markCompleted(approvalRequestId, { actor: requester, note: "Tool executed after approval", expectedRequesterActorId: agent.id }); }, }; const permanentAgentGating: PermanentAgentGatingContext = { @@ -487,6 +489,11 @@ async function createChatMissionGateContexts( const pending = await approvalStore.list({ status: "pending", requesterActorId: agent.id, limit: 100 }); return pending.find((request) => request.targetAction.context?.approvalDedupeKey === dedupeKey) ?? null; }, + // FNXC:AgentGating 2026-07-26-14:50: gate-path parity — pause the bound agent when the permanent gate parks a pending approval, matching the action-gate context above. + pauseForApproval: async () => { + await agentStore.updateAgentState(agent.id, "paused"); + await agentStore.updateAgent(agent.id, { pauseReason: "awaiting-approval" }); + }, }; return { missionMutationGated: true, actionGateContext, permanentAgentGating }; diff --git a/packages/dashboard/src/routes/__tests__/register-approval-routes.test.ts b/packages/dashboard/src/routes/__tests__/register-approval-routes.test.ts new file mode 100644 index 0000000000..4e14b21584 --- /dev/null +++ b/packages/dashboard/src/routes/__tests__/register-approval-routes.test.ts @@ -0,0 +1,364 @@ +// @vitest-environment node + +/* +FNXC:ApprovalDecisionAuthority 2026-07-26-17:10: +Negative-path coverage for POST /api/approvals/:id/decision — none existed before, which +is how an AI agent could self-approve its own destructive request: the route accepted a +client-supplied `actor` as the authorization input with shape validation only. + +Invariants under test: + - the decider recorded in the store is ALWAYS the server-derived synthetic operator + (actorId "user" / actorType "user"), never a body-claimed identity; + - a body actor with a non-user actorType is rejected 403; + - a body actor matching the request's requester is rejected 403 (self-approval); + - a requester whose actorId equals the derived operator id cannot be auto-decided; + - with the real bearer-token middleware installed, an unauthenticated decision is 401; + - store lifecycle races (invalid transition / expired) map to 409, unknown id to 404; + - approving sandbox_provisioning with no registered executor is refused 409 BEFORE + decide() (the request stays pending — no lying "approved" audit); + - a store rejection surfaces as a 5xx error, never a silent success (fail closed). + +All store access is via in-memory fakes (no DB, no network, no timers) per the AGENTS.md +slow-test rule; the bearer middleware runs in-process via the mock-socket test harness. +*/ + +import { describe, it, expect, vi, beforeEach } from "vitest"; +import express from "express"; + +const approvalState = vi.hoisted(() => ({ + requests: new Map>(), + decide: vi.fn(), +})); + +vi.mock("@fusion/core", async (importOriginal) => { + const { createCoreMock } = await import("../../test/mockCoreEngine.js"); + return createCoreMock(() => importOriginal>(), { + /* + FNXC:ApprovalDecisionAuthority 2026-07-26-17:10: + Routes construct their own ApprovalRequestStore per request, so the fake reads the + shared hoisted state instead of instance state. AgentStore is inert — the decision + route only touches it in resume-after-decision best-effort paths. + */ + ApprovalRequestStore: class FakeApprovalRequestStore { + constructor(..._args: unknown[]) {} + async get(id: string) { return approvalState.requests.get(id); } + async decide(id: string, status: string, input: unknown) { return approvalState.decide(id, status, input); } + async getAuditHistory() { return []; } + async list() { return []; } + async findLatestByDedupeKey() { return undefined; } + }, + AgentStore: class FakeAgentStore { + constructor(..._args: unknown[]) {} + async init() {} + async getAgent() { return undefined; } + async updateAgentState() {} + async updateAgent() {} + }, + }); +}); + +import type { TaskStore } from "@fusion/core"; +import { createApiRoutes } from "../../routes.js"; +import type { ServerOptions } from "../../server.js"; +import { createAuthMiddleware } from "../../auth-middleware.js"; +import { registerSandboxProvisioningExecutor } from "../register-approval-routes.js"; +import { request as REQUEST } from "../../test-request.js"; + +const REQUEST_ID = "AR-1"; + +function makeApprovalRequest(overrides: Record = {}): Record { + return { + id: REQUEST_ID, + status: "pending", + requester: { actorId: "agent-7", actorType: "agent", actorName: "Agent Seven" }, + requestedAt: "2026-07-26T00:00:00.000Z", + createdAt: "2026-07-26T00:00:00.000Z", + updatedAt: "2026-07-26T00:00:00.000Z", + targetAction: { + category: "command", + action: "run", + summary: "Run a command", + resourceType: "command", + resourceId: "cmd-1", + }, + ...overrides, + }; +} + +function makeStore(): TaskStore { + return { + getRootDir: vi.fn(() => process.cwd()), + getFusionDir: vi.fn(() => "/tmp/fusion-approval-route-test"), + getAsyncLayer: vi.fn(() => ({})), + getSettings: vi.fn(async () => ({})), + getTask: vi.fn(async () => { throw new Error("no task in this suite"); }), + recordRunAuditEvent: vi.fn(async () => {}), + // Marks the store runtime-owned so project-context binding skips the plugin-MCP binder. + getProjectScopedPluginMcpServers: vi.fn(async () => []), + } as unknown as TaskStore; +} + +function makeLogger() { + const warn = vi.fn(); + const logger = { + scope: "test", + info: vi.fn(), + warn, + error: vi.fn(), + child: () => logger, + }; + return { logger, warn }; +} + +function makeApp(options?: Partial & { authToken?: string }) { + const store = makeStore(); + const app = express(); + if (options?.authToken) { + app.use(createAuthMiddleware(options.authToken)); + } + app.use(express.json()); + const { authToken: _authToken, ...serverOptions } = options ?? {}; + app.use("/api", createApiRoutes(store, serverOptions as ServerOptions)); + return { app, store }; +} + +async function postDecision( + app: Parameters[0], + body: Record, + headers: Record = {}, +) { + return REQUEST(app, "POST", `/api/approvals/${REQUEST_ID}/decision`, JSON.stringify(body), { + "content-type": "application/json", + ...headers, + }); +} + +beforeEach(() => { + approvalState.requests.clear(); + approvalState.requests.set(REQUEST_ID, makeApprovalRequest()); + approvalState.decide.mockReset(); + approvalState.decide.mockImplementation(async (id: string, status: string, input: { actor: unknown; note?: string }) => ({ + ...makeApprovalRequest(), + id, + status, + decidedAt: "2026-07-26T00:00:01.000Z", + decidedBy: (input.actor as { actorId?: string })?.actorId, + })); + registerSandboxProvisioningExecutor(null); +}); + +describe("POST /api/approvals/:id/decision — server-derived decider", () => { + it("records the synthetic operator when the body carries no actor", async () => { + const { app } = makeApp(); + const res = await postDecision(app, { decision: "approve", comment: "ok" }); + + expect(res.status).toBe(200); + expect(approvalState.decide).toHaveBeenCalledTimes(1); + expect(approvalState.decide).toHaveBeenCalledWith(REQUEST_ID, "approved", { + actor: { actorId: "user", actorType: "user", actorName: "User" }, + note: "ok", + }); + }); + + it("keeps actorId/actorType server-derived and carries only the display name from a user body actor", async () => { + const { app } = makeApp(); + const res = await postDecision(app, { + decision: "approve", + actor: { actorId: "someone-else", actorType: "user", actorName: "Alice Operator" }, + }); + + expect(res.status).toBe(200); + const [, , input] = approvalState.decide.mock.calls[0]; + expect(input.actor).toEqual({ actorId: "user", actorType: "user", actorName: "Alice Operator" }); + }); + + it("rejects a forged non-user body actor with 403 and never calls decide", async () => { + const { app } = makeApp(); + const res = await postDecision(app, { + decision: "approve", + actor: { actorId: "agent-7", actorType: "agent", actorName: "Agent Seven" }, + }); + + expect(res.status).toBe(403); + expect(approvalState.decide).not.toHaveBeenCalled(); + }); + + it("rejects a body actor whose actorId matches the requester with 403 (self-approval claim)", async () => { + const { app } = makeApp(); + const res = await postDecision(app, { + decision: "approve", + actor: { actorId: "agent-7", actorType: "user", actorName: "Totally A Human" }, + }); + + expect(res.status).toBe(403); + expect(approvalState.decide).not.toHaveBeenCalled(); + }); + + it("refuses 403 when the derived operator IS the requester (requester actorId 'user')", async () => { + approvalState.requests.set(REQUEST_ID, makeApprovalRequest({ + requester: { actorId: "user", actorType: "user", actorName: "User" }, + })); + const { app } = makeApp(); + const res = await postDecision(app, { decision: "approve" }); + + expect(res.status).toBe(403); + expect(approvalState.decide).not.toHaveBeenCalled(); + }); + + it("still rejects a malformed body actor with 400", async () => { + const { app } = makeApp(); + const res = await postDecision(app, { decision: "approve", actor: { actorId: 42 } }); + + expect(res.status).toBe(400); + expect(approvalState.decide).not.toHaveBeenCalled(); + }); +}); + +describe("POST /api/approvals/:id/decision — auth boundary", () => { + it("returns 401 for an unauthenticated decision when the real bearer middleware is installed", async () => { + const { app } = makeApp({ authToken: "test-daemon-token" }); + const res = await postDecision(app, { decision: "approve" }); + + expect(res.status).toBe(401); + expect(approvalState.decide).not.toHaveBeenCalled(); + }); + + it("accepts the decision with a valid bearer token", async () => { + const { app } = makeApp({ authToken: "test-daemon-token" }); + const res = await postDecision(app, { decision: "approve" }, { authorization: "Bearer test-daemon-token" }); + + expect(res.status).toBe(200); + expect(approvalState.decide).toHaveBeenCalledTimes(1); + }); + + it("warns loudly (but still allows) when daemon auth is disabled", async () => { + const { logger, warn } = makeLogger(); + const { app } = makeApp({ runtimeLogger: logger as unknown as ServerOptions["runtimeLogger"], isDaemonAuthEnabled: false }); + const res = await postDecision(app, { decision: "approve" }); + + expect(res.status).toBe(200); + expect(warn.mock.calls.some(([message]) => String(message).includes("without daemon auth"))).toBe(true); + }); + + it("does not emit the local-trust warning when daemon auth is enabled", async () => { + const { logger, warn } = makeLogger(); + const { app } = makeApp({ runtimeLogger: logger as unknown as ServerOptions["runtimeLogger"], isDaemonAuthEnabled: true }); + const res = await postDecision(app, { decision: "approve" }); + + expect(res.status).toBe(200); + expect(warn.mock.calls.some(([message]) => String(message).includes("without daemon auth"))).toBe(false); + }); +}); + +describe("POST /api/approvals/:id/decision — store lifecycle mapping", () => { + it("maps an invalid-transition (replayed/already-decided) store error to 409", async () => { + approvalState.decide.mockRejectedValue(new Error("Invalid approval request transition: approved -> approved")); + const { app } = makeApp(); + const res = await postDecision(app, { decision: "approve" }); + + expect(res.status).toBe(409); + }); + + it("maps an expired-request store error to 409", async () => { + approvalState.decide.mockRejectedValue(new Error(`Approval request ${REQUEST_ID} has expired`)); + const { app } = makeApp(); + const res = await postDecision(app, { decision: "approve" }); + + expect(res.status).toBe(409); + }); + + it("returns 404 for a nonexistent approval request", async () => { + approvalState.requests.clear(); + const { app } = makeApp(); + const res = await postDecision(app, { decision: "approve" }); + + expect(res.status).toBe(404); + expect(approvalState.decide).not.toHaveBeenCalled(); + }); + + it("fails closed with a 5xx error when the store rejects for an unknown reason", async () => { + approvalState.decide.mockRejectedValue(new Error("connection terminated unexpectedly")); + const { app } = makeApp(); + const res = await postDecision(app, { decision: "approve" }); + + expect(res.status).toBe(500); + expect(res.status).not.toBe(200); + }); +}); + +describe("POST /api/approvals/:id/decision — sandbox provisioning honesty", () => { + it("refuses 409 to approve sandbox_provisioning when no executor is registered, before decide()", async () => { + approvalState.requests.set(REQUEST_ID, makeApprovalRequest({ + targetAction: { + category: "sandbox_provisioning", + action: "provision", + summary: "Provision a sandbox", + resourceType: "sandbox", + resourceId: "sb-1", + }, + })); + const { app } = makeApp(); + const res = await postDecision(app, { decision: "approve" }); + + expect(res.status).toBe(409); + expect(approvalState.decide).not.toHaveBeenCalled(); + }); + + /* + FNXC:ApprovalDecisionAuthority 2026-07-26-18:55: + Review finding: a registered executor that THROWS used to be swallowed into a + warn while the response looked like a clean approval. The decision still stands + (grant TTL bounds the window) but the failure must be first-class: surfaced as + `executorError` on the response so the operator sees provisioning did not run. + */ + it("surfaces a registered executor failure as executorError instead of swallowing it", async () => { + const sandboxRequest = makeApprovalRequest({ + targetAction: { + category: "sandbox_provisioning", + action: "provision", + summary: "Provision a sandbox", + resourceType: "sandbox", + resourceId: "sb-1", + context: { backendId: "docker", operation: "provision" }, + }, + }); + approvalState.requests.set(REQUEST_ID, sandboxRequest); + approvalState.decide.mockImplementation(async (id: string, status: string) => ({ + ...sandboxRequest, + id, + status, + decidedAt: "2026-07-26T00:00:01.000Z", + })); + registerSandboxProvisioningExecutor(() => Promise.reject(new Error("docker daemon unreachable"))); + try { + const { app } = makeApp(); + const res = await postDecision(app, { decision: "approve" }); + + expect(res.status).toBe(200); + expect(res.body.executorError).toBe("docker daemon unreachable"); + expect(approvalState.decide).toHaveBeenCalledOnce(); + } finally { + registerSandboxProvisioningExecutor(null); + } + }); + + it("still allows denying sandbox_provisioning without an executor", async () => { + approvalState.requests.set(REQUEST_ID, makeApprovalRequest({ + targetAction: { + category: "sandbox_provisioning", + action: "provision", + summary: "Provision a sandbox", + resourceType: "sandbox", + resourceId: "sb-1", + }, + context: { backendId: "docker", operation: "provision" }, + })); + const { app } = makeApp(); + const res = await postDecision(app, { decision: "deny" }); + + expect(res.status).toBe(200); + expect(approvalState.decide).toHaveBeenCalledWith(REQUEST_ID, "denied", expect.objectContaining({ + actor: { actorId: "user", actorType: "user", actorName: "User" }, + })); + }); +}); diff --git a/packages/dashboard/src/routes/__tests__/register-planning-subtask-routes.parent-close.test.ts b/packages/dashboard/src/routes/__tests__/register-planning-subtask-routes.parent-close.test.ts new file mode 100644 index 0000000000..19a5b167c5 --- /dev/null +++ b/packages/dashboard/src/routes/__tests__/register-planning-subtask-routes.parent-close.test.ts @@ -0,0 +1,120 @@ +// @vitest-environment node + +/* +FNXC:TaskDeleteAttribution 2026-07-26-17:30: +POST /api/subtasks/create-tasks closes (deletes) the parent task after a breakdown. +Invariants under test: + - the parent delete is ATTRIBUTED via the task-delete-attribution vocabulary + (auditContext with callerKind "engine" — automation behind the planning session, + not an operator click); + - a parent-delete failure is SURFACED, not swallowed: the response reports + parentTaskClosed:false plus parentTaskCloseError, and runtimeLogger.warn fires so + server diagnostics show the failure (the FN-2164 ghost-parent incident was a silent + swallow here); + - a successful delete reports parentTaskClosed:true. +In-memory store fakes and a mocked subtask-session module — no DB, AI, or timers. +*/ + +import { describe, it, expect, vi, beforeEach } from "vitest"; +import express from "express"; + +vi.mock("../../subtask-breakdown.js", async (importOriginal) => { + const actual = await importOriginal>(); + return { + ...actual, + getSubtaskSession: vi.fn(async () => ({ + sessionId: "S1", + initialDescription: "break down the parent", + })), + cleanupSubtaskSession: vi.fn(), + }; +}); + +vi.mock("@fusion/engine", async () => { + const { createEngineMock } = await import("../../test/mockCoreEngine.js"); + return createEngineMock({ + createAgentTask: vi.fn(async (_store: unknown, input: { title: string }) => ({ + task: { id: "FN-100", title: input.title, column: "todo", steps: [] }, + wasDuplicate: false, + })), + }); +}); + +import type { TaskStore } from "@fusion/core"; +import { createApiRoutes } from "../../routes.js"; +import type { ServerOptions } from "../../server.js"; +import { request as REQUEST } from "../../test-request.js"; + +function makeHarness(deleteImpl: () => Promise) { + const deleteSpy = vi.fn(deleteImpl); + const warnSpy = vi.fn(); + const logger = { + scope: "test", + info: vi.fn(), + warn: warnSpy, + error: vi.fn(), + child: () => logger, + }; + + const store = { + getRootDir: vi.fn(() => process.cwd()), + getSettings: vi.fn(async () => ({})), + getTask: vi.fn(async () => { throw new Error("parent not found"); }), + updateTask: vi.fn(async (id: string) => ({ id })), + logEntry: vi.fn(async () => {}), + deleteTask: deleteSpy, + getProjectScopedPluginMcpServers: vi.fn(async () => []), + } as unknown as TaskStore; + + const app = express(); + app.use(express.json()); + app.use("/api", createApiRoutes(store, { runtimeLogger: logger } as unknown as ServerOptions)); + return { app, deleteSpy, warnSpy }; +} + +async function postCreateTasks(app: Parameters[0]) { + return REQUEST(app, "POST", "/api/subtasks/create-tasks", JSON.stringify({ + sessionId: "S1", + parentTaskId: "FN-1", + subtasks: [{ tempId: "t1", title: "Child A", description: "first child" }], + }), { "content-type": "application/json" }); +} + +beforeEach(() => { + vi.clearAllMocks(); +}); + +describe("POST /api/subtasks/create-tasks — parent close attribution and failure surfacing", () => { + it("attributes the parent delete with an engine auditContext", async () => { + const { app, deleteSpy } = makeHarness(async () => {}); + const res = await postCreateTasks(app); + + expect(res.status).toBe(201); + expect((res.body as { parentTaskClosed?: boolean }).parentTaskClosed).toBe(true); + expect(deleteSpy).toHaveBeenCalledTimes(1); + const [deletedId, options] = deleteSpy.mock.calls[0] as [string, { auditContext?: Record }]; + expect(deletedId).toBe("FN-1"); + expect(options.auditContext).toMatchObject({ + agentId: "system", + sessionId: "S1", + callerKind: "engine", + }); + expect(String(options.auditContext?.runId)).toContain("synthetic-planning-delete-FN-1"); + }); + + it("surfaces a parent-delete failure in the response payload and warns in server diagnostics", async () => { + const { app, warnSpy } = makeHarness(async () => { + throw new Error("Cannot delete FN-1: live tasks still depend on it"); + }); + const res = await postCreateTasks(app); + + expect(res.status).toBe(201); + const body = res.body as { parentTaskClosed?: boolean; parentTaskCloseError?: string }; + expect(body.parentTaskClosed).toBe(false); + expect(body.parentTaskCloseError).toBe("Cannot delete FN-1: live tasks still depend on it"); + expect(warnSpy.mock.calls.some(([message, context]) => + String(message).includes("failed to close parent task") + && (context as { parentTaskId?: string })?.parentTaskId === "FN-1", + )).toBe(true); + }); +}); diff --git a/packages/dashboard/src/routes/__tests__/register-task-workflow-routes.operator-attribution.test.ts b/packages/dashboard/src/routes/__tests__/register-task-workflow-routes.operator-attribution.test.ts new file mode 100644 index 0000000000..d0f3f6086a --- /dev/null +++ b/packages/dashboard/src/routes/__tests__/register-task-workflow-routes.operator-attribution.test.ts @@ -0,0 +1,107 @@ +// @vitest-environment node + +/* +FNXC:ApprovalDecisionAuthority 2026-07-26-17:20: +Route-boundary invariants for two operator-only task mutations: + +1. POST /tasks/:id/bypass-review — the recorded bypass actor is derived SERVER-SIDE. + A client-supplied `actor` string used to become `bypassedBy` verbatim, so an agent + could stamp an arbitrary identity onto a review-gate bypass. Now the attribution is + always `dashboard-operator`, with a body-supplied name carried only as advisory + display metadata: `dashboard-operator (as "")`. `reason` stays mandatory. + +2. POST /tasks/archive-all-done — the bulk archive sweep now requires an explicit + `{ confirm: true }` body (400 without it, store untouched), matching the single-task + reset's confirm gate, so a stray script or agent call cannot silently empty Done. + +FNXC:ArchiveConfirmGate 2026-07-26-17:20: +In-memory store fakes only (no DB, no network, no timers) per the AGENTS.md slow-test rule. +*/ + +import { describe, it, expect, vi } from "vitest"; +import express from "express"; +import type { TaskStore } from "@fusion/core"; +import { createApiRoutes } from "../../routes.js"; +import { request as REQUEST } from "../../test-request.js"; + +function makeHarness() { + const bypassSpy = vi.fn(async (id: string, _input: { reason: string; actor: string }) => ({ id, column: "in-review" })); + const archiveAllDoneSpy = vi.fn(async () => []); + + const store = { + getRootDir: vi.fn(() => process.cwd()), + bypassFailedPreMergeReviewStep: bypassSpy, + archiveAllDone: archiveAllDoneSpy, + getProjectScopedPluginMcpServers: vi.fn(async () => []), + } as unknown as TaskStore; + + const app = express(); + app.use(express.json()); + app.use("/api", createApiRoutes(store)); + return { app, bypassSpy, archiveAllDoneSpy }; +} + +describe("POST /tasks/:id/bypass-review — server-derived actor", () => { + it("records dashboard-operator when the body carries no actor", async () => { + const { app, bypassSpy } = makeHarness(); + const res = await REQUEST(app, "POST", "/api/tasks/FN-1/bypass-review", JSON.stringify({ reason: "stuck gate" }), { + "content-type": "application/json", + }); + + expect(res.status).toBe(200); + expect(bypassSpy).toHaveBeenCalledWith("FN-1", { reason: "stuck gate", actor: "dashboard-operator" }); + }); + + it("keeps a body-supplied actor as advisory display metadata, never the attribution", async () => { + const { app, bypassSpy } = makeHarness(); + const res = await REQUEST(app, "POST", "/api/tasks/FN-1/bypass-review", JSON.stringify({ reason: "stuck gate", actor: "EvilAgent" }), { + "content-type": "application/json", + }); + + expect(res.status).toBe(200); + expect(bypassSpy).toHaveBeenCalledWith("FN-1", { reason: "stuck gate", actor: 'dashboard-operator (as "EvilAgent")' }); + }); + + it("still requires a non-empty reason (400)", async () => { + const { app, bypassSpy } = makeHarness(); + const res = await REQUEST(app, "POST", "/api/tasks/FN-1/bypass-review", JSON.stringify({ reason: " " }), { + "content-type": "application/json", + }); + + expect(res.status).toBe(400); + expect(bypassSpy).not.toHaveBeenCalled(); + }); +}); + +describe("POST /tasks/archive-all-done — confirm gate", () => { + it("returns 400 and leaves the store untouched without { confirm: true }", async () => { + const { app, archiveAllDoneSpy } = makeHarness(); + const res = await REQUEST(app, "POST", "/api/tasks/archive-all-done", JSON.stringify({}), { + "content-type": "application/json", + }); + + expect(res.status).toBe(400); + expect(archiveAllDoneSpy).not.toHaveBeenCalled(); + }); + + it("rejects a truthy-but-not-true confirm value", async () => { + const { app, archiveAllDoneSpy } = makeHarness(); + const res = await REQUEST(app, "POST", "/api/tasks/archive-all-done", JSON.stringify({ confirm: "yes" }), { + "content-type": "application/json", + }); + + expect(res.status).toBe(400); + expect(archiveAllDoneSpy).not.toHaveBeenCalled(); + }); + + it("archives with an explicit { confirm: true }", async () => { + const { app, archiveAllDoneSpy } = makeHarness(); + const res = await REQUEST(app, "POST", "/api/tasks/archive-all-done", JSON.stringify({ confirm: true }), { + "content-type": "application/json", + }); + + expect(res.status).toBe(200); + expect(archiveAllDoneSpy).toHaveBeenCalledTimes(1); + expect(res.body).toEqual({ archived: [] }); + }); +}); diff --git a/packages/dashboard/src/routes/__tests__/register-worktrunk-routes.test.ts b/packages/dashboard/src/routes/__tests__/register-worktrunk-routes.test.ts new file mode 100644 index 0000000000..48f8d0cbb0 --- /dev/null +++ b/packages/dashboard/src/routes/__tests__/register-worktrunk-routes.test.ts @@ -0,0 +1,126 @@ +// @vitest-environment node + +/* +FNXC:ApprovalDecisionAuthority 2026-07-26-17:25: +POST /api/worktrunk/install-request used to take `req.body.actor` verbatim as the +approval-request requester snapshot, letting any HTTP caller forge who asked for the +install. Invariants under test: + - the requester passed to requestWorktrunkInstallApproval is ALWAYS the synthetic + operator (actorId "user" / actorType "user"), regardless of body content; + - a user-typed body actor contributes only its advisory display actorName; + - a body actor with a non-user actorType is rejected 403 and no approval request is + created; + - a malformed body actor stays 400 (pre-existing contract). +In-memory fakes only; the engine's worktrunk helpers are mocked so no binary probing, +network, or filesystem installs happen. +*/ + +import { describe, it, expect, vi, beforeEach } from "vitest"; +import express from "express"; + +const worktrunkMocks = vi.hoisted(() => ({ + resolveWorktrunkBinary: vi.fn(), + requestWorktrunkInstallApproval: vi.fn(), +})); + +vi.mock("@fusion/engine", async () => { + const { createEngineMock } = await import("../../test/mockCoreEngine.js"); + return createEngineMock({ + WORKTRUNK_INSTALL_PATH: "/tmp/fake-worktrunk/wt", + WORKTRUNK_PINNED_RELEASE: { version: "1.0.0" }, + probeWorktrunk: vi.fn(async () => ({ version: "1.0.0" })), + resolveWorktrunkBinary: worktrunkMocks.resolveWorktrunkBinary, + requestWorktrunkInstallApproval: worktrunkMocks.requestWorktrunkInstallApproval, + }); +}); + +vi.mock("@fusion/core", async (importOriginal) => { + const { createCoreMock } = await import("../../test/mockCoreEngine.js"); + return createCoreMock(() => importOriginal>(), { + ApprovalRequestStore: class FakeApprovalRequestStore { + constructor(..._args: unknown[]) {} + async get() { return undefined; } + async getAuditHistory() { return []; } + async list() { return []; } + async findLatestByDedupeKey() { return undefined; } + }, + }); +}); + +import type { TaskStore } from "@fusion/core"; +import { createApiRoutes } from "../../routes.js"; +import { request as REQUEST } from "../../test-request.js"; + +function makeApp() { + const store = { + getRootDir: vi.fn(() => process.cwd()), + getAsyncLayer: vi.fn(() => ({})), + getSettings: vi.fn(async () => ({})), + getProjectScopedPluginMcpServers: vi.fn(async () => []), + } as unknown as TaskStore; + + const app = express(); + app.use(express.json()); + app.use("/api", createApiRoutes(store)); + return { app }; +} + +async function postInstallRequest(app: Parameters[0], body: Record) { + return REQUEST(app, "POST", "/api/worktrunk/install-request", JSON.stringify(body), { + "content-type": "application/json", + }); +} + +beforeEach(() => { + worktrunkMocks.resolveWorktrunkBinary.mockReset(); + worktrunkMocks.resolveWorktrunkBinary.mockRejectedValue(new Error("worktrunk not installed")); + worktrunkMocks.requestWorktrunkInstallApproval.mockReset(); + worktrunkMocks.requestWorktrunkInstallApproval.mockResolvedValue({ approvalRequestId: "AR-WT-1" }); +}); + +describe("POST /api/worktrunk/install-request — server-derived requester", () => { + it("uses the synthetic operator when the body carries no actor", async () => { + const { app } = makeApp(); + const res = await postInstallRequest(app, {}); + + expect(res.status).toBe(200); + expect(worktrunkMocks.requestWorktrunkInstallApproval).toHaveBeenCalledTimes(1); + expect(worktrunkMocks.requestWorktrunkInstallApproval.mock.calls[0][0].actor).toEqual({ + actorId: "user", + actorType: "user", + actorName: "User", + }); + }); + + it("keeps actorId/actorType server-derived and carries only the display name from a user body actor", async () => { + const { app } = makeApp(); + const res = await postInstallRequest(app, { + actor: { actorId: "someone-else", actorType: "user", actorName: "Alice Operator" }, + }); + + expect(res.status).toBe(200); + expect(worktrunkMocks.requestWorktrunkInstallApproval.mock.calls[0][0].actor).toEqual({ + actorId: "user", + actorType: "user", + actorName: "Alice Operator", + }); + }); + + it("rejects a forged non-user body actor with 403 and creates no approval request", async () => { + const { app } = makeApp(); + const res = await postInstallRequest(app, { + actor: { actorId: "agent-7", actorType: "agent", actorName: "Agent Seven" }, + }); + + expect(res.status).toBe(403); + expect(worktrunkMocks.requestWorktrunkInstallApproval).not.toHaveBeenCalled(); + }); + + it("keeps rejecting a malformed body actor with 400", async () => { + const { app } = makeApp(); + const res = await postInstallRequest(app, { actor: { actorId: "x" } }); + + expect(res.status).toBe(400); + expect(worktrunkMocks.requestWorktrunkInstallApproval).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/dashboard/src/routes/register-approval-routes.ts b/packages/dashboard/src/routes/register-approval-routes.ts index a983381e45..20ffb97f94 100644 --- a/packages/dashboard/src/routes/register-approval-routes.ts +++ b/packages/dashboard/src/routes/register-approval-routes.ts @@ -11,7 +11,17 @@ import { ApiError, badRequest, conflict, notFound } from "../api-error.js"; import type { ApiRoutesContext } from "./types.js"; import { emitApprovalSseEvent } from "../sse.js"; import { requireAsyncLayer } from "../require-async-layer.js"; +import { isDaemonAuthActive } from "../auth-middleware.js"; +/* +FNXC:ApprovalDecisionAuthority 2026-07-26-16:10: +The synthetic dashboard operator. The daemon bearer token is a single shared operator +secret, so every authenticated HTTP decision is, at best, "the operator" — the server +cannot distinguish individual humans, and it must never accept a client-claimed agent +identity as the decider (an AI agent self-approved a live-task deletion through exactly +that hole). All decisions are therefore recorded against this snapshot; a body-supplied +actor is advisory display metadata only (see the decision route). +*/ const DEFAULT_ACTOR: ApprovalRequestActorSnapshot = { actorId: "user", actorType: "user", @@ -297,7 +307,13 @@ export function registerApprovalRoutes(ctx: ApiRoutesContext): void { router.post("/approvals/:id/decision", async (req, res) => { try { - const body = (req.body ?? {}) as { decision?: "approve" | "deny"; comment?: string; actor?: ApprovalRequestActorSnapshot }; + // FNXC:ApprovalDecisionAuthority 2026-07-26-16:10: `actor` is deliberately typed as + // untrusted wire input, not ApprovalRequestActorSnapshot — it never becomes the decider. + const body = (req.body ?? {}) as { + decision?: "approve" | "deny"; + comment?: string; + actor?: { actorId?: unknown; actorType?: unknown; actorName?: unknown } | null; + }; if (body.decision !== "approve" && body.decision !== "deny") { throw badRequest("decision must be one of: approve, deny"); } @@ -312,9 +328,86 @@ export function registerApprovalRoutes(ctx: ApiRoutesContext): void { const existing = await approvalStore.get(requestId); if (!existing) throw notFound("Approval request not found"); - const actor = body.actor ?? DEFAULT_ACTOR; - if (!actor || typeof actor.actorId !== "string" || typeof actor.actorType !== "string" || typeof actor.actorName !== "string") { - throw badRequest("actor must include actorId, actorType, and actorName"); + /* + FNXC:ApprovalDecisionAuthority 2026-07-26-16:10: + The decider is derived SERVER-SIDE, never taken from the request body. Previously + `body.actor` was shape-validated only and passed straight into `decide(...)`, so an + AI agent holding the API could stamp its own (or any) identity on the approval of + its own request. Rules: + - A body actor with actorType !== "user" is rejected 403 — agents/system callers + cannot decide approvals over HTTP at all. + - A body actor whose actorId matches the request's requester is rejected 403 + (explicit self-approval claim). + - The effective decider is always DEFAULT_ACTOR's actorId/actorType; only the + advisory display actorName may be carried from a body actor whose actorType is + "user". + - If the derived decider's actorId equals the requester's actorId, the decision + is refused 403 (the requester queue cannot approve itself). + */ + let advisoryActorName: string | undefined; + const bodyActor = body.actor; + if (bodyActor !== undefined) { + if ( + bodyActor === null + || typeof bodyActor !== "object" + || typeof bodyActor.actorId !== "string" + || typeof bodyActor.actorType !== "string" + || typeof bodyActor.actorName !== "string" + ) { + throw badRequest("actor must include actorId, actorType, and actorName"); + } + if (bodyActor.actorType !== "user") { + throw new ApiError(403, "Approval decisions are operator-only; a non-user actor cannot decide an approval request"); + } + if (bodyActor.actorId === existing.requester.actorId) { + throw new ApiError(403, "An approval request cannot be decided by its own requester"); + } + if (bodyActor.actorName.trim().length > 0) { + advisoryActorName = bodyActor.actorName; + } + } + const actor: ApprovalRequestActorSnapshot = { + actorId: DEFAULT_ACTOR.actorId, + actorType: DEFAULT_ACTOR.actorType, + actorName: advisoryActorName ?? DEFAULT_ACTOR.actorName, + }; + if (actor.actorId === existing.requester.actorId) { + throw new ApiError(403, "An approval request cannot be decided by its own requester"); + } + + /* + FNXC:ApprovalDecisionAuthority 2026-07-26-16:10: + Auth-disabled trust assumption, stated explicitly: when no daemon bearer token is + installed (local single-operator mode), anyone who can reach the socket is treated + as the operator. The decision is still allowed — locking approvals out of unauth + local mode would break the shipped default — but each decision is loudly logged so + the trust boundary is visible, not silent. + */ + const daemonAuthEnabled = ctx.isDaemonAuthEnabled + ?? ctx.options?.isDaemonAuthEnabled + ?? isDaemonAuthActive(ctx.options); + if (!daemonAuthEnabled) { + runtimeLogger.warn("Approval decision accepted without daemon auth (local single-operator trust)", { + requestId, + decision: body.decision, + }); + } + + /* + FNXC:ApprovalDecisionAuthority 2026-07-26-16:10: + Sandbox-provisioning honesty: without a registered executor, "approve" used to + succeed silently and write an approved audit event while provisioning never ran — + a control that lies. Refuse 409 BEFORE decide() so the request stays pending until + a server with a real executor handles it. + */ + if ( + body.decision === "approve" + && existing.targetAction.category === "sandbox_provisioning" + && !sandboxProvisioningExecutor + ) { + throw conflict( + "Cannot approve sandbox provisioning: no sandbox provisioning executor is registered on this server; the request remains pending", + ); } const targetStatus = body.decision === "approve" ? "approved" : "denied"; @@ -323,7 +416,14 @@ export function registerApprovalRoutes(ctx: ApiRoutesContext): void { updated = await approvalStore.decide(requestId, targetStatus, { actor, note: body.comment }); } catch (error) { const message = error instanceof Error ? error.message : String(error); - if (message.includes("Invalid approval request transition")) { + /* + FNXC:ApprovalDecisionAuthority 2026-07-26-16:10: + The core store rejects replayed/already-decided requests with messages starting + "Invalid approval request transition" and expired requests with messages + containing "expired". Both are client-resolvable races on a request's lifecycle, + so both map to 409 conflict rather than a 500. + */ + if (message.includes("Invalid approval request transition") || message.includes("expired")) { throw conflict(message); } throw error; @@ -372,16 +472,46 @@ export function registerApprovalRoutes(ctx: ApiRoutesContext): void { emitSecretsAccessDecisionAudit({ scopedStore, request: updated, decision: body.decision }); + /* + FNXC:ApprovalDecisionAuthority 2026-07-26-18:40: + Review finding: an executor throw used to be swallowed into a warn while the + request stayed "approved" and only the approved audit event was written — an + operator could not tell provisioning never ran. The approval row itself is + deliberately NOT rolled back (the operator's decision stands, and the 15min + grant TTL bounds the window), but the failure is now first-class: a + sandbox:provisioning:execute-failed run-audit event (ids/outcomes only) is + recorded alongside the decision audit, and the failure is surfaced in the + HTTP response via executorError so the dashboard shows it immediately. + Modeling a durable retryable execution state is a schema/contract change + deferred to a follow-up. + */ + let sandboxExecutorError: string | undefined; if (updated.targetAction.category === "sandbox_provisioning") { if (body.decision === "approve") { if (sandboxProvisioningExecutor) { try { await sandboxProvisioningExecutor(updated); } catch (error) { + sandboxExecutorError = error instanceof Error ? error.message : String(error); runtimeLogger.warn("Sandbox provisioning executor failed", { requestId: updated.id, - error: error instanceof Error ? error.message : String(error), + error: sandboxExecutorError, }); + const failureEvent: Parameters[0] = { + agentId: updated.requester.actorId, + domain: "database", + mutationType: "sandbox:provisioning:execute-failed", + target: updated.targetAction.resourceId || updated.id, + metadata: { + approvalRequestId: updated.id, + requesterAgentId: updated.requester.actorId, + outcome: "execute-failed", + }, + runId: updated.id, + }; + if (updated.taskId) failureEvent.taskId = updated.taskId; + if (updated.runId) failureEvent.runId = updated.runId; + void scopedStore.recordRunAuditEvent(failureEvent); } } emitSandboxProvisioningDecisionAudit({ scopedStore, request: updated, decision: "approved", runtimeLogger }); @@ -395,7 +525,9 @@ export function registerApprovalRoutes(ctx: ApiRoutesContext): void { const detail = toDetailDto(updated, history); emitApprovalSseEvent("approval:updated", detail, projectId); emitApprovalSseEvent("approval:decided", detail, projectId); - res.json(detail); + // FNXC:ApprovalDecisionAuthority 2026-07-26-18:40: additive field — clients that + // ignore it see the exact prior contract; the dashboard can surface the failure. + res.json(sandboxExecutorError !== undefined ? { ...detail, executorError: sandboxExecutorError } : detail); } catch (err: unknown) { if (err instanceof ApiError) throw err; rethrowAsApiError(err); diff --git a/packages/dashboard/src/routes/register-planning-subtask-routes.ts b/packages/dashboard/src/routes/register-planning-subtask-routes.ts index 9e887e5ada..7bde554048 100644 --- a/packages/dashboard/src/routes/register-planning-subtask-routes.ts +++ b/packages/dashboard/src/routes/register-planning-subtask-routes.ts @@ -73,7 +73,7 @@ function rethrowPlanningWorkflowCreateError( } export function registerPlanningSubtaskRoutes(ctx: ApiRoutesContext, deps: PlanningSubtaskRouteDeps): void { - const { router, getProjectContext, planningLogger, rethrowAsApiError } = ctx; + const { router, getProjectContext, planningLogger, runtimeLogger, rethrowAsApiError } = ctx; const { aiSessionStore, parseLastEventId, replayBufferedSSE } = deps; const planningRuntime = (settings: Awaited>) => ({ clarificationEnabled: settings.agentClarificationEnabled === true, @@ -439,6 +439,17 @@ export function registerPlanningSubtaskRoutes(ctx: ApiRoutesContext, deps: Plann // is what left FN-2164 blocked by the ghost of FN-2163. parentTaskClosed = false; parentTaskCloseError = err instanceof Error ? err.message : String(err); + /* + FNXC:TaskDeleteAttribution 2026-07-26-16:25: + A parent-close failure must be operator-visible in server diagnostics, not only + in the (easily ignored) response field — the FN-2164 incident was a parent + delete failing silently and leaving children permanently blocked on a ghost id. + */ + runtimeLogger.warn("Subtask breakdown: failed to close parent task after creating subtasks", { + parentTaskId: normalizedParentId, + sessionId, + error: parentTaskCloseError, + }); } } diff --git a/packages/dashboard/src/routes/register-task-workflow-routes.ts b/packages/dashboard/src/routes/register-task-workflow-routes.ts index 73040040dd..7a77e3b8eb 100644 --- a/packages/dashboard/src/routes/register-task-workflow-routes.ts +++ b/packages/dashboard/src/routes/register-task-workflow-routes.ts @@ -3047,7 +3047,20 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork if (typeof reason !== "string" || reason.trim().length === 0) { throw badRequest("reason is required to bypass a failed pre-merge review step"); } - const resolvedActor = typeof actor === "string" && actor.trim().length > 0 ? actor.trim() : "dashboard-operator"; + /* + FNXC:ApprovalDecisionAuthority 2026-07-26-16:20: + The recorded bypass actor is derived SERVER-SIDE. The daemon bearer token is a + single shared operator secret, so the only honest attribution for an HTTP bypass is + the synthetic dashboard operator; a body-supplied `actor` is client-claimed, + unverifiable identity and is carried as advisory display metadata only — it can no + longer replace the attribution (an agent could previously stamp any name into + `bypassedBy`). Format: `dashboard-operator` or `dashboard-operator (as "")`. + Mandatory `reason` stays mandatory. + */ + const advisoryName = typeof actor === "string" && actor.trim().length > 0 ? actor.trim() : undefined; + const resolvedActor = advisoryName && advisoryName !== "dashboard-operator" + ? `dashboard-operator (as ${JSON.stringify(advisoryName)})` + : "dashboard-operator"; const updated = await scopedStore.bypassFailedPreMergeReviewStep(req.params.id, { reason: reason.trim(), actor: resolvedActor, @@ -3269,6 +3282,20 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork // Archive all done tasks router.post("/tasks/archive-all-done", async (req, res) => { try { + /* + FNXC:ArchiveConfirmGate 2026-07-26-16:30: + Bulk archive sweeps every done task in one call, yet had no confirmation while the + single-task reset (a comparable board-wide-impact mutation) already requires + `{ confirm: true }`. An agent or stray script hitting this route could silently + empty the Done column. Require the same explicit `{ confirm: true }` body; the + dashboard's "Archive all done" button sends it after its user-facing confirm. + */ + const { confirm: confirmed } = (req.body ?? {}) as { confirm?: boolean }; + if (confirmed !== true) { + throw badRequest( + "This operation archives every done task. Pass { \"confirm\": true } in the request body to proceed.", + ); + } const { store: scopedStore } = await getProjectContext(req); const archived = await scopedStore.archiveAllDone(); res.json({ archived }); diff --git a/packages/dashboard/src/routes/register-worktrunk-routes.ts b/packages/dashboard/src/routes/register-worktrunk-routes.ts index e772c97095..de8ae36279 100644 --- a/packages/dashboard/src/routes/register-worktrunk-routes.ts +++ b/packages/dashboard/src/routes/register-worktrunk-routes.ts @@ -75,11 +75,41 @@ export function registerWorktrunkRoutes(ctx: ApiRoutesContext): void { router.post("/worktrunk/install-request", async (req, res) => { try { - const body = (req.body ?? {}) as { actor?: ApprovalRequestActorSnapshot }; - if (body.actor && (!body.actor.actorId || !body.actor.actorType || !body.actor.actorName)) { - throw badRequest("actor must include actorId, actorType, and actorName"); + /* + FNXC:ApprovalDecisionAuthority 2026-07-26-16:40: + The worktrunk-install approval requester is derived SERVER-SIDE. Previously + `req.body.actor` became the requester snapshot verbatim, letting any HTTP caller + forge the identity that later shows as "who asked for this install". The bearer + token is a single shared operator secret, so the honest requester is the synthetic + dashboard operator; a body actor is at most advisory display metadata — its + actorName is carried only when its actorType is "user", and a non-user actorType is + rejected 403 (agents must use their own engine-side approval path, not this route). + This also keeps the requester actorId aligned with the /worktrunk/status + pending-lookup, which queries by DEFAULT_ACTOR.actorId. + */ + const body = (req.body ?? {}) as { actor?: { actorId?: unknown; actorType?: unknown; actorName?: unknown } | null }; + let advisoryActorName: string | undefined; + if (body.actor !== undefined && body.actor !== null) { + if ( + typeof body.actor !== "object" + || typeof body.actor.actorId !== "string" || body.actor.actorId.length === 0 + || typeof body.actor.actorType !== "string" || body.actor.actorType.length === 0 + || typeof body.actor.actorName !== "string" || body.actor.actorName.length === 0 + ) { + throw badRequest("actor must include actorId, actorType, and actorName"); + } + if (body.actor.actorType !== "user") { + throw new ApiError(403, "Worktrunk install requests over HTTP are operator-only; a non-user actor cannot request an install"); + } + if (body.actor.actorName.trim().length > 0) { + advisoryActorName = body.actor.actorName; + } } - const actor = body.actor ?? DEFAULT_ACTOR; + const actor: ApprovalRequestActorSnapshot = { + actorId: DEFAULT_ACTOR.actorId, + actorType: DEFAULT_ACTOR.actorType, + actorName: advisoryActorName ?? DEFAULT_ACTOR.actorName, + }; const { store: scopedStore, projectId } = await getProjectContext(req); const settings = await scopedStore.getSettings(); const worktrunkSettings = settings.worktrunk ?? {}; diff --git a/packages/dashboard/src/routes/types.ts b/packages/dashboard/src/routes/types.ts index 4253ebfa82..486a4520f1 100644 --- a/packages/dashboard/src/routes/types.ts +++ b/packages/dashboard/src/routes/types.ts @@ -45,6 +45,15 @@ export interface ApiRoutesContext { /** Narrow multipart seam for routes that must accept local binary artifacts. */ reportUpload?: { single(fieldName: string): RequestHandler }; options?: ServerOptions; + /* + FNXC:ApprovalDecisionAuthority 2026-07-26-16:10: + Whether the daemon bearer-token auth middleware is installed in front of these routes. + Security-sensitive routes (approval decisions) use it to decide whether to log the + local single-operator trust assumption on each privileged action. Optional: when absent, + consumers fall back to `options.isDaemonAuthEnabled` and then to + `isDaemonAuthActive(options)` so direct `createApiRoutes` callers keep working. + */ + isDaemonAuthEnabled?: boolean; runtimeLogger: RuntimeLogger; planningLogger: RuntimeLogger; chatLogger: RuntimeLogger; diff --git a/packages/dashboard/src/server.ts b/packages/dashboard/src/server.ts index 19e98fc75c..7bad81c771 100644 --- a/packages/dashboard/src/server.ts +++ b/packages/dashboard/src/server.ts @@ -495,6 +495,14 @@ export interface ServerOptions { * FUSION_DASHBOARD_TOKEN env vars. Used by `fn dashboard --no-auth` so a * stale token in a project .env doesn't silently override the flag. */ noAuth?: boolean; + /* + FNXC:ApprovalDecisionAuthority 2026-07-26-16:10: + Resolved auth-middleware state, wired by createServer once it has decided whether the + bearer-token middleware is actually installed. Routes that gate or log privileged + operator actions (approval decisions) read this instead of re-deriving token state, so + the route-visible answer can never disagree with the middleware that was mounted. + */ + isDaemonAuthEnabled?: boolean; /** Optional runtime logger for server/routes diagnostics. * Defaults to a console-backed logger scoped to `server` when omitted. */ runtimeLogger?: RuntimeLogger; @@ -2166,6 +2174,9 @@ export function createServer(store: TaskStore, options?: ServerOptions): ReturnT // REST API const apiRouter = createApiRoutes(store, { ...options, + // FNXC:ApprovalDecisionAuthority 2026-07-26-16:10: routes must see the same answer + // as the middleware mounted above — auth is enabled iff a daemonToken was installed. + isDaemonAuthEnabled: Boolean(daemonToken), runtimeLogger, aiSessionStore: aiSessionStore as AiSessionStore, chatStore, diff --git a/packages/engine/src/__tests__/action-gate-fail-closed.test.ts b/packages/engine/src/__tests__/action-gate-fail-closed.test.ts new file mode 100644 index 0000000000..9f8373a20e --- /dev/null +++ b/packages/engine/src/__tests__/action-gate-fail-closed.test.ts @@ -0,0 +1,163 @@ +import { describe, expect, it } from "vitest"; +import { + configureApprovalRequestTtls, + normalizeAgentPermissionPolicyFromPreset, +} from "@fusion/core"; +import { evaluateAgentActionGate, resolveGateOutcome } from "../agent-action-gate.js"; +import { resolvePermanentAgentToolDecision } from "../permanent-agent-gating.js"; + +/* +FNXC:AgentGating 2026-07-26-14:10: +Both-directions regression tests for the fail-closed audit fixes: + 1) unclassified tools no longer resolve to hardcoded exempt/allow — they are + policy-governed (unchanged under the default `unrestricted` preset, + actually blocked under `locked-down`); + 2) bash approvals bind to the exact command via a hashed resourceId in the + dedupe key; + 3) approved-but-unredeemed grants expire (grant TTL) at redemption; + 4) a permanent-gating context with a wholly missing policy fails closed. +Expectations are HARDCODED, never derived from the constants under test. +*/ + +const UNRESTRICTED = normalizeAgentPermissionPolicyFromPreset("unrestricted"); +const LOCKED_DOWN = normalizeAgentPermissionPolicyFromPreset("locked-down"); + +describe("evaluateAgentActionGate — unclassified tools are policy-governed (fail closed)", () => { + it("default unrestricted preset: unknown tool still allowed (behavior unchanged)", () => { + const decision = evaluateAgentActionGate({ + agentId: "agent-1", + toolName: "fn_some_future_tool", + args: {}, + permissionPolicy: UNRESTRICTED, + }); + expect(decision.disposition).toBe("allow"); + expect(decision.category).toBe("command_execution"); + }); + + it("locked-down preset: unknown tool is blocked (was exempt/allow before the fix)", () => { + const decision = evaluateAgentActionGate({ + agentId: "agent-1", + toolName: "fn_some_future_tool", + args: {}, + permissionPolicy: LOCKED_DOWN, + }); + expect(decision.disposition).toBe("block"); + expect(decision.category).toBe("command_execution"); + }); + + it("registered coordination tools stay exempt in both presets", () => { + for (const policy of [UNRESTRICTED, LOCKED_DOWN]) { + const decision = evaluateAgentActionGate({ + agentId: "agent-1", + toolName: "fn_heartbeat_done", + args: {}, + permissionPolicy: policy, + }); + expect(decision.disposition).toBe("allow"); + expect(decision.category).toBe("exempt"); + } + }); +}); + +describe("evaluateAgentActionGate — bash approvals bind to the exact command", () => { + it("two different shell commands produce different dedupe keys", () => { + const base = { agentId: "agent-1", taskId: "FN-1", toolName: "bash", permissionPolicy: UNRESTRICTED }; + const a = evaluateAgentActionGate({ ...base, args: { command: "echo hello" } }); + const b = evaluateAgentActionGate({ ...base, args: { command: "rm -rf build" } }); + expect(a.approvalDedupeKey).not.toBe(b.approvalDedupeKey); + expect(a.resourceId).toMatch(/^cmd:[0-9a-f]{16}$/); + expect(b.resourceId).toMatch(/^cmd:[0-9a-f]{16}$/); + }); + + it("the same command produces a stable dedupe key", () => { + const base = { agentId: "agent-1", taskId: "FN-1", toolName: "bash", permissionPolicy: UNRESTRICTED }; + const a = evaluateAgentActionGate({ ...base, args: { command: "pnpm test" } }); + const b = evaluateAgentActionGate({ ...base, args: { command: "pnpm test" } }); + expect(a.approvalDedupeKey).toBe(b.approvalDedupeKey); + }); + + it("git write commands are also command-bound", () => { + const base = { agentId: "agent-1", toolName: "bash", permissionPolicy: UNRESTRICTED }; + const push = evaluateAgentActionGate({ ...base, args: { command: "git push origin main" } }); + const pushForce = evaluateAgentActionGate({ ...base, args: { command: "git push --force origin main" } }); + expect(push.category).toBe("git_write"); + expect(push.approvalDedupeKey).not.toBe(pushForce.approvalDedupeKey); + }); +}); + +describe("resolveGateOutcome — approval-grant TTL at redemption", () => { + const decision = { + disposition: "require-approval" as const, + category: "command_execution" as const, + toolName: "bash", + operation: "shell command", + summary: "bash: shell command", + resourceType: "command" as const, + approvalDedupeKey: "k", + metadata: {}, + }; + + it("fresh approved grant redeems execute-once", () => { + const outcome = resolveGateOutcome(decision, { + id: "apr-1", + status: "approved", + decidedAt: new Date().toISOString(), + }); + expect(outcome.outcome).toBe("execute-once-then-complete"); + expect(outcome.approvalRequestId).toBe("apr-1"); + }); + + it("stale approved grant (past the configured grant TTL) is treated as absent", () => { + /* + FNXC:ApprovalRedemption 2026-07-26-19:05: + The grant TTL is operator-configurable (configureApprovalRequestTtls / + FUSION_APPROVAL_GRANT_TTL_MS, default 1h). Pin it explicitly for this test so + the expectation cannot silently drift with the default. + */ + configureApprovalRequestTtls({ grantTtlMs: 15 * 60 * 1000 }); + try { + const outcome = resolveGateOutcome(decision, { + id: "apr-1", + status: "approved", + decidedAt: new Date(Date.now() - 16 * 60 * 1000).toISOString(), + }); + expect(outcome).toEqual({ outcome: "wait-for-approval" }); + } finally { + configureApprovalRequestTtls({ grantTtlMs: undefined }); + } + }); + + it("closures that omit decidedAt keep legacy redemption (backward compatible)", () => { + const outcome = resolveGateOutcome(decision, { id: "apr-1", status: "approved" }); + expect(outcome.outcome).toBe("execute-once-then-complete"); + }); +}); + +describe("resolvePermanentAgentToolDecision — missing policy fails closed", () => { + it("sensitive tool with a policy-less gating context requires approval (was allow)", () => { + const decision = resolvePermanentAgentToolDecision({ + toolName: "fn_task_delete", + args: {}, + gating: {} as never, + }); + expect(decision.disposition).toBe("require-approval"); + }); + + it("recognized coordination tool stays allowed even with a policy-less context", () => { + const decision = resolvePermanentAgentToolDecision({ + toolName: "fn_heartbeat_done", + args: {}, + gating: {} as never, + }); + expect(decision.disposition).toBe("allow"); + }); + + it("with the default unrestricted policy, sensitive tools remain allowed (behavior unchanged)", () => { + const decision = resolvePermanentAgentToolDecision({ + toolName: "fn_task_delete", + args: {}, + gating: { permissionPolicy: UNRESTRICTED } as never, + }); + expect(decision.disposition).toBe("allow"); + }); +}); diff --git a/packages/engine/src/__tests__/agent-tools-config.test.ts b/packages/engine/src/__tests__/agent-tools-config.test.ts index 0a36c8e9a5..a396e3e818 100644 --- a/packages/engine/src/__tests__/agent-tools-config.test.ts +++ b/packages/engine/src/__tests__/agent-tools-config.test.ts @@ -241,7 +241,14 @@ describe("agent lifecycle tools", () => { vi.mocked(agentStore.getAgent).mockResolvedValue(manager); vi.mocked(agentStore.createAgent).mockResolvedValue(created); - const tool = createAgentCreateTool(agentStore, "manager-1"); + /* + FNXC:AgentProvisioningGate 2026-07-26-13:30: + The no-options factory call no longer synthesizes approvalMode "never"; this test now + passes the explicit operator opt-out so it keeps exercising the direct-report create path. + */ + const tool = createAgentCreateTool(agentStore, "manager-1", { + settingsProvider: async () => ({ agentProvisioning: { approvalMode: "never" } }) as never, + }); const result = await tool.execute("session", { name: "Report", role: "executor" }, undefined as never, undefined as never, undefined as never); expect((result.content[0] as { text: string }).text).toContain("Created agent Report (report-1)"); diff --git a/packages/engine/src/__tests__/agent-tools-provisioning-approval.test.ts b/packages/engine/src/__tests__/agent-tools-provisioning-approval.test.ts index 11f1519988..93857b9b53 100644 --- a/packages/engine/src/__tests__/agent-tools-provisioning-approval.test.ts +++ b/packages/engine/src/__tests__/agent-tools-provisioning-approval.test.ts @@ -139,3 +139,200 @@ describe("agent provisioning approval tools", () => { expect(deleted).toEqual({ deletedId: "agent-target" }); }); }); + +/* +FNXC:AgentProvisioningGate 2026-07-26-13:35: +Hardcoded privilege and fail-closed expectations. Trust comes ONLY from the operator-configured +settings.agentProvisioning trusted lists — never from top-level org position, and never from a +hardcoded role name (see FNXC:AgentProvisioning 2026-07-26-18:20) — and a +require-approval decision with no approval store must DENY, never silently allow. +Expected outcomes are hardcoded strings, never derived from the policy module. +*/ +describe("agent provisioning privilege and fail-closed gating", () => { + let agentStore: AgentStore; + let approvalRequestStore: ApprovalRequestStore; + + const setCaller = (overrides: Partial) => { + const caller = makeAgent({ id: "agent-caller", ...overrides }); + vi.mocked(agentStore.getAgent).mockImplementation(async (id: string) => + (id === "agent-caller" ? caller : id === "agent-target" ? makeAgent({ id: "agent-target", reportsTo: "agent-caller" }) : null) as any); + }; + + beforeEach(() => { + agentStore = { + getAgent: vi.fn(async () => null), + createAgent: vi.fn(async (input: any) => makeAgent({ id: "agent-created", name: input.name, role: input.role })), + deleteAgent: vi.fn(async () => undefined), + } as unknown as AgentStore; + approvalRequestStore = { + create: vi.fn((input: any) => ({ id: "APR-1", status: "pending", requester: input.requester, targetAction: input.targetAction })), + } as unknown as ApprovalRequestStore; + }); + + /* + FNXC:AgentProvisioning 2026-07-26-18:20: + Privilege is OPERATOR-CONFIGURED, never a magic role name. "ceo" is an ordinary role string that any + agent config can claim, so on its own it must grant nothing; the operator opts a role or id into + trust via agentProvisioning.trustedRoles / trustedAgentIds. These three cases pin that contract from + both directions so a future hardcode reintroduces a failure rather than silent privilege. + */ + it("a 'ceo' role alone is NOT privileged when no trusted lists are configured", async () => { + setCaller({ role: "ceo", reportsTo: "board" }); + const tool = createAgentCreateTool(agentStore, "agent-caller", { + approvalRequestStore, + settingsProvider: async () => withProvisioning({ approvalMode: "trusted-only" }), + }); + const result = await tool.execute("s", { name: "N", role: "executor" } as any, undefined as any, undefined as any, undefined as any); + expect((result.details as any).outcome).toBe("pending_approval"); + expect((result.details as any).matchedRule).toBe("approval-mode-trusted-only"); + expect(agentStore.createAgent).not.toHaveBeenCalled(); + }); + + it("an operator-configured trusted role is allowed without an approval", async () => { + setCaller({ role: "ceo", reportsTo: "board" }); + const tool = createAgentCreateTool(agentStore, "agent-caller", { + approvalRequestStore, + settingsProvider: async () => withProvisioning({ approvalMode: "trusted-only", trustedRoles: ["ceo"] }), + }); + const result = await tool.execute("s", { name: "N", role: "executor" } as any, undefined as any, undefined as any, undefined as any); + expect((result.details as any).outcome).toBe("created"); + expect((result.details as any).matchedRule).toBe("trusted-role"); + expect(approvalRequestStore.create).not.toHaveBeenCalled(); + }); + + /* + FNXC:AgentProvisioning 2026-07-26-18:20: + These two exercise the ORG-CHART escape hatch — the only thing isCallerPrivileged still governs — + by creating an agent that reports to somebody ELSE. The policy-path tests above cannot see this + function at all (isPrivileged is deliberately no longer forwarded to the policy), so without these + a reintroduced role hardcode would pass the whole suite. Verified by mutation: restoring + `caller.role === "ceo"` fails the first case here and nothing else. + */ + it("a 'ceo' role alone cannot create an agent reporting to someone else", async () => { + setCaller({ role: "ceo", reportsTo: "board" }); + const tool = createAgentCreateTool(agentStore, "agent-caller", { + approvalRequestStore, + settingsProvider: async () => withProvisioning({ approvalMode: "trusted-only" }), + }); + const result = await tool.execute("s", { name: "N", role: "executor", reportsTo: "someone-else" } as any, undefined as any, undefined as any, undefined as any); + expect((result.content as any)[0].text).toContain("You can only create agents that report to you"); + expect(agentStore.createAgent).not.toHaveBeenCalled(); + }); + + it("an operator-configured trusted role can create an agent reporting to someone else", async () => { + setCaller({ role: "ceo", reportsTo: "board" }); + const tool = createAgentCreateTool(agentStore, "agent-caller", { + approvalRequestStore, + settingsProvider: async () => withProvisioning({ approvalMode: "trusted-only", trustedRoles: ["ceo"] }), + }); + const result = await tool.execute("s", { name: "N", role: "executor", reportsTo: "someone-else" } as any, undefined as any, undefined as any, undefined as any); + expect((result.content as any)[0].text).not.toContain("You can only create agents that report to you"); + expect((result.details as any).outcome).toBe("created"); + }); + + it("an operator-configured trusted agent id is allowed without an approval", async () => { + setCaller({ role: "custom", reportsTo: "board" }); + const tool = createAgentCreateTool(agentStore, "agent-caller", { + approvalRequestStore, + settingsProvider: async () => withProvisioning({ approvalMode: "trusted-only", trustedAgentIds: ["agent-caller"] }), + }); + const result = await tool.execute("s", { name: "N", role: "executor" } as any, undefined as any, undefined as any, undefined as any); + expect((result.details as any).outcome).toBe("created"); + expect((result.details as any).matchedRule).toBe("trusted-agent-id"); + expect(approvalRequestStore.create).not.toHaveBeenCalled(); + }); + + it("top-level non-ceo caller (reportsTo null, role custom) is NOT privileged: requires approval", async () => { + setCaller({ role: "custom", reportsTo: undefined }); + const tool = createAgentCreateTool(agentStore, "agent-caller", { + approvalRequestStore, + settingsProvider: async () => withProvisioning({ approvalMode: "trusted-only" }), + }); + const result = await tool.execute("s", { name: "N", role: "executor" } as any, undefined as any, undefined as any, undefined as any); + expect((result.details as any).outcome).toBe("pending_approval"); + expect((result.details as any).matchedRule).toBe("approval-mode-trusted-only"); + expect(agentStore.createAgent).not.toHaveBeenCalled(); + }); + + it("top-level non-ceo caller (reportsTo null, role manager) is NOT privileged: requires approval", async () => { + setCaller({ role: "manager", reportsTo: undefined }); + const tool = createAgentCreateTool(agentStore, "agent-caller", { + approvalRequestStore, + settingsProvider: async () => withProvisioning({ approvalMode: "trusted-only" }), + }); + const result = await tool.execute("s", { name: "N", role: "executor" } as any, undefined as any, undefined as any, undefined as any); + expect((result.details as any).outcome).toBe("pending_approval"); + expect(agentStore.createAgent).not.toHaveBeenCalled(); + }); + + it("reporting agent is NOT privileged: requires approval", async () => { + setCaller({ role: "executor", reportsTo: "agent-root" }); + const tool = createAgentCreateTool(agentStore, "agent-caller", { + approvalRequestStore, + settingsProvider: async () => withProvisioning({ approvalMode: "trusted-only" }), + }); + const result = await tool.execute("s", { name: "N", role: "executor" } as any, undefined as any, undefined as any, undefined as any); + expect((result.details as any).outcome).toBe("pending_approval"); + expect(agentStore.createAgent).not.toHaveBeenCalled(); + }); + + it("no options at all: untrusted create is DENIED (fail closed, no synthesized 'never' mode)", async () => { + setCaller({ role: "executor", reportsTo: "agent-root" }); + const tool = createAgentCreateTool(agentStore, "agent-caller"); + const result = await tool.execute("s", { name: "N", role: "executor" } as any, undefined as any, undefined as any, undefined as any); + expect((result.details as any).outcome).toBe("denied"); + expect((result.content[0] as { text: string }).text).toContain("approval storage is unavailable"); + expect(agentStore.createAgent).not.toHaveBeenCalled(); + }); + + it("no options at all: top-level non-ceo delete is DENIED (fail closed)", async () => { + setCaller({ role: "custom", reportsTo: undefined }); + const tool = createAgentDeleteTool(agentStore, "agent-caller"); + const result = await tool.execute("s", { agent_id: "agent-target" } as any, undefined as any, undefined as any, undefined as any); + expect((result.details as any).outcome).toBe("denied"); + expect((result.content[0] as { text: string }).text).toContain("approval storage is unavailable"); + expect(agentStore.deleteAgent).not.toHaveBeenCalled(); + }); + + it("require-approval with settings but no approval store is DENIED, never silently allowed", async () => { + setCaller({ role: "executor", reportsTo: "agent-root" }); + const tool = createAgentCreateTool(agentStore, "agent-caller", { + settingsProvider: async () => withProvisioning({ approvalMode: "always" }), + }); + const result = await tool.execute("s", { name: "N", role: "executor" } as any, undefined as any, undefined as any, undefined as any); + expect((result.details as any).outcome).toBe("denied"); + expect(agentStore.createAgent).not.toHaveBeenCalled(); + }); + + it("trustedAgentIds still allows without approval", async () => { + setCaller({ role: "executor", reportsTo: "agent-root" }); + const tool = createAgentCreateTool(agentStore, "agent-caller", { + approvalRequestStore, + settingsProvider: async () => withProvisioning({ approvalMode: "trusted-only", trustedAgentIds: ["agent-caller"] }), + }); + const result = await tool.execute("s", { name: "N", role: "executor" } as any, undefined as any, undefined as any, undefined as any); + expect((result.details as any).outcome).toBe("created"); + expect((result.details as any).matchedRule).toBe("trusted-agent-id"); + }); + + it("trustedRoles still allows without approval", async () => { + setCaller({ role: "manager", reportsTo: undefined }); + const tool = createAgentCreateTool(agentStore, "agent-caller", { + approvalRequestStore, + settingsProvider: async () => withProvisioning({ approvalMode: "trusted-only", trustedRoles: ["manager"] }), + }); + const result = await tool.execute("s", { name: "N", role: "executor" } as any, undefined as any, undefined as any, undefined as any); + expect((result.details as any).outcome).toBe("created"); + expect((result.details as any).matchedRule).toBe("trusted-role"); + }); + + it("explicit approvalMode 'never' still allows untrusted create (operator opt-out unchanged)", async () => { + setCaller({ role: "executor", reportsTo: "agent-root" }); + const tool = createAgentCreateTool(agentStore, "agent-caller", { + settingsProvider: async () => withProvisioning({ approvalMode: "never" }), + }); + const result = await tool.execute("s", { name: "N", role: "executor" } as any, undefined as any, undefined as any, undefined as any); + expect((result.details as any).outcome).toBe("created"); + expect((result.details as any).matchedRule).toBe("approval-mode-never"); + }); +}); diff --git a/packages/engine/src/__tests__/bash-containment.test.ts b/packages/engine/src/__tests__/bash-containment.test.ts new file mode 100644 index 0000000000..483ee15fea --- /dev/null +++ b/packages/engine/src/__tests__/bash-containment.test.ts @@ -0,0 +1,151 @@ +import { describe, expect, it } from "vitest"; +import { homedir } from "node:os"; +import { + buildBashContainmentDenialMessage, + evaluateBashContainment, + normalizeBashCommandForContainment, +} from "../bash-containment.js"; +import { wrapToolsWithBashContainment } from "../pi.js"; + +/* +FNXC:BashContainment 2026-07-26-14:00: +Regression tests for the unconditional privilege-escalation floor. The +incident chain was bash reading ~/.fusion/settings.json (daemon token) and +curling the approvals API to self-approve. Expectations are HARDCODED — +never derived from the rule table under test. +*/ + +describe("evaluateBashContainment — denies the escalation chain", () => { + it("denies reading the global fusion settings file", () => { + const verdict = evaluateBashContainment("cat ~/.fusion/settings.json"); + expect(verdict.allowed).toBe(false); + expect(verdict.rule).toBe("fusion-global-dir"); + }); + + it("denies quote-split spellings", () => { + expect(evaluateBashContainment("cat ~/.fus''ion/settings.json").allowed).toBe(false); + expect(evaluateBashContainment('cat "~/.fusion/settings.json"').allowed).toBe(false); + }); + + it("denies $HOME and ${HOME} spellings", () => { + expect(evaluateBashContainment("cat $HOME/.fusion/settings.json").allowed).toBe(false); + expect(evaluateBashContainment('cat "${HOME}/.fusion/settings.json"').allowed).toBe(false); + }); + + it("denies the literal home directory spelling", () => { + expect(evaluateBashContainment(`cat ${homedir()}/.fusion/settings.json`).allowed).toBe(false); + }); + + it("denies other users' fusion dirs", () => { + expect(evaluateBashContainment("cat /Users/someone/.fusion/settings.json").allowed).toBe(false); + expect(evaluateBashContainment("cat /home/ci/.fusion/settings.json").allowed).toBe(false); + }); + + it("denies relative .fusion/settings.json reads", () => { + expect(evaluateBashContainment("cd ~ && cat .fusion/settings.json").allowed).toBe(false); + }); + + it("denies daemon token env references", () => { + expect(evaluateBashContainment("echo $FUSION_DAEMON_TOKEN").allowed).toBe(false); + expect(evaluateBashContainment("env | grep -i daemonToken").allowed).toBe(false); + }); + + it("denies credential store reads", () => { + expect(evaluateBashContainment("cat ~/.ssh/id_ed25519").allowed).toBe(false); + expect(evaluateBashContainment("cat $HOME/.aws/credentials").allowed).toBe(false); + expect(evaluateBashContainment("cat ~/.netrc").allowed).toBe(false); + expect(evaluateBashContainment("cat ~/.npmrc").allowed).toBe(false); + expect(evaluateBashContainment("cat ~/.config/gh/hosts.yml").allowed).toBe(false); + }); + + it("denies shell calls to the approvals API", () => { + expect( + evaluateBashContainment('curl -X POST http://localhost:4040/api/approvals/apr-123/decision -d \'{"decision":"approve"}\'').allowed, + ).toBe(false); + expect(evaluateBashContainment("curl 'http://127.0.0.1:9000/api/tasks?fn_token=abc'").allowed).toBe(false); + }); +}); + +describe("evaluateBashContainment — normal work is unaffected", () => { + const allowed = [ + "git status", + "git commit -m 'feat: add thing'", + "pnpm --filter @fusion/core exec vitest run src/__tests__/foo.test.ts", + "pnpm install && pnpm build", + "cat src/index.ts", + "ls -la packages/", + "curl https://registry.npmjs.org/react", + "grep -rn approvals packages/dashboard/src", + "node scripts/check-changesets.mjs", + "cat .fusion/tasks/FN-1/PROMPT.md", + ]; + for (const command of allowed) { + it(`allows: ${command}`, () => { + expect(evaluateBashContainment(command)).toEqual({ allowed: true }); + }); + } + + it("allows empty commands", () => { + expect(evaluateBashContainment("")).toEqual({ allowed: true }); + }); +}); + +describe("normalizeBashCommandForContainment", () => { + it("strips quotes/backslashes, folds home spellings, lowercases", () => { + expect(normalizeBashCommandForContainment("CAT '$HOME'/.FUS\\ION/x")).toBe("cat ~/.fusion/x"); + }); +}); + +describe("wrapToolsWithBashContainment", () => { + const makeBashTool = (execute: (...args: unknown[]) => Promise) => ({ + name: "bash", + label: "bash", + description: "", + parameters: {}, + execute, + }); + + it("blocks a denied command before the underlying tool runs", async () => { + let executed = false; + const [wrapped] = wrapToolsWithBashContainment([ + makeBashTool(async () => { + executed = true; + return { ok: true }; + }) as never, + ]); + const result = (await (wrapped.execute as (...args: unknown[]) => Promise)( + "call-1", + { command: "cat ~/.fusion/settings.json" }, + undefined, + )) as { isError?: boolean; error?: string }; + expect(executed).toBe(false); + expect(result.isError).toBe(true); + expect(result.error).toContain("privilege-escalation containment"); + }); + + it("passes allowed commands through untouched", async () => { + const [wrapped] = wrapToolsWithBashContainment([ + makeBashTool(async () => ({ ok: true, ran: true })) as never, + ]); + const result = (await (wrapped.execute as (...args: unknown[]) => Promise)( + "call-2", + { command: "git status" }, + undefined, + )) as { ran?: boolean }; + expect(result.ran).toBe(true); + }); + + it("does not wrap non-bash tools", () => { + const readTool = { name: "read", label: "read", description: "", parameters: {}, execute: async () => ({}) }; + const [unwrapped] = wrapToolsWithBashContainment([readTool as never]); + expect(unwrapped).toBe(readTool); + }); +}); + +describe("buildBashContainmentDenialMessage", () => { + it("names the rule and tells the agent to ask the operator", () => { + const message = buildBashContainmentDenialMessage({ allowed: false, rule: "approvals-api", reason: "nope" }); + expect(message).toContain("approvals-api"); + expect(message).toContain("ask the operator"); + }); +}); diff --git a/packages/engine/src/__tests__/gating-classifications.test.ts b/packages/engine/src/__tests__/gating-classifications.test.ts index bd5e95148e..12ccf91cb4 100644 --- a/packages/engine/src/__tests__/gating-classifications.test.ts +++ b/packages/engine/src/__tests__/gating-classifications.test.ts @@ -585,7 +585,10 @@ describe("gating-classifications parity", () => { continue; } if (FILE_WRITE_DELETE_FN_TOOLS.has(toolName)) { - expect({ toolName, actionKind, permanentKind }).toEqual({ toolName, actionKind: "readonly", permanentKind: "file-write" }); + // FNXC:AgentGating 2026-07-26-15:10: both gates now agree fn_task_attach + // is a file write; the old "readonly" action-side expectation encoded the + // silent exempt-fallback defect fixed by the fail-closed classifier. + expect({ toolName, actionKind, permanentKind }).toEqual({ toolName, actionKind: "file-write", permanentKind: "file-write" }); continue; } if (NETWORK_API_TOOLS.has(toolName) && !ACTION_GATE_NETWORK_API_TOOLS.has(toolName)) { diff --git a/packages/engine/src/agent-action-gate.ts b/packages/engine/src/agent-action-gate.ts index 40787a9a1d..cc70902f39 100644 --- a/packages/engine/src/agent-action-gate.ts +++ b/packages/engine/src/agent-action-gate.ts @@ -1,3 +1,5 @@ +import { createHash } from "node:crypto"; +import { isApprovalRequestExpired } from "@fusion/core"; import type { AgentPermissionPolicy, AgentPermissionPolicyActionCategory, @@ -6,11 +8,14 @@ import type { } from "@fusion/core"; import { ACTION_GATE_NETWORK_API_TOOLS, + ACTION_GATE_PROVISIONING_POLICY_TOOLS, ACTION_GATE_TASK_AGENT_MANAGEMENT_TOOLS, COMMAND_EXECUTION_FN_TOOLS, COORDINATION_EXEMPT_TOOLS, FILE_SCOPE_FN_TOOLS, + FILE_WRITE_DELETE_FN_TOOLS, READONLY_BUILTIN_TOOLS, + READONLY_FN_TOOLS, REVIEW_GATE_BYPASS_FN_TOOLS, classifyGitCommand, } from "./gating-classifications.js"; @@ -42,7 +47,14 @@ export interface AgentActionGateContext { runId?: string; permissionPolicy: AgentPermissionPolicy; createApprovalRequest: (decision: AgentActionGateDecision, args: Record) => Promise; - findApprovalByDedupeKey?: (dedupeKey: string) => Promise<{ id: string; status: ApprovalRequestStatus } | null>; + /** + * FNXC:ApprovalRedemption 2026-07-26-13:05: + * `decidedAt` lets resolveGateOutcome apply the approval-grant TTL at + * redemption time (approved-but-unredeemed grants were redeemable forever — + * live DB showed 17 approved / 0 completed). Optional for backward + * compatibility: a closure that omits it skips TTL evaluation. + */ + findApprovalByDedupeKey?: (dedupeKey: string) => Promise<{ id: string; status: ApprovalRequestStatus; decidedAt?: string } | null>; /** @deprecated Use findApprovalByDedupeKey */ findPendingApprovalByDedupeKey?: (dedupeKey: string) => Promise<{ id: string } | null>; pauseForApproval?: (info: { approvalRequestId: string; decision: AgentActionGateDecision }) => Promise; @@ -144,6 +156,16 @@ export function evaluateAgentActionGate(params: { if (params.toolName === "bash") { const command = extractShellCommand(args); const git = classifyGitCommand(command); + /* + FNXC:ApprovalRedemption 2026-07-26-13:05: + Bind bash approvals to the EXACT command. Previously the dedupe key for + non-git bash collapsed to operation "shell command", so one approved + request authorized arbitrary future shell commands for that agent+task. + Hashing the full command string into resourceId makes each distinct + command a distinct approval, and redemption (execute-once-then-complete) + can only consume an approval minted for that same command. + */ + resourceId = command ? `cmd:${createHash("sha256").update(command).digest("hex").slice(0, 16)}` : undefined; if (git?.write) { category = "git_write"; operation = git.operation; @@ -199,6 +221,47 @@ export function evaluateAgentActionGate(params: { category = "network_api"; operation = params.toolName; resourceType = params.toolName.startsWith("mcp__") ? "mcp" : "research"; + } else if (FILE_WRITE_DELETE_FN_TOOLS.has(params.toolName)) { + // FNXC:AgentGating 2026-07-26-15:10: fn_task_attach mutates persisted task + // attachments; the permanent gate already classifies it file_write_delete. + // The action gate previously let it through via the exempt fallback — a + // silent-exemption defect. Positive parity classification; still "allow" + // under the default unrestricted preset. + category = "file_write_delete"; + operation = params.toolName; + resourceType = "file"; + } else if (ACTION_GATE_PROVISIONING_POLICY_TOOLS.has(params.toolName)) { + // FNXC:AgentGating 2026-07-26-15:05: FN-3953 — provisioning tools are governed + // solely by the dedicated agent_provisioning policy; positive exemption here + // avoids double approval rows now that the unknown fallback fails closed. + category = "exempt"; + operation = params.toolName; + } else if (READONLY_FN_TOOLS.has(params.toolName)) { + /* + FNXC:AgentGating 2026-07-26-15:00: + Read-only fn_* discovery tools were previously "recognized" only by + falling into the exempt default. With the unknown-tool fallback now fail + closed, they need a POSITIVE exempt classification (matching the + permanent gate's recognized "none" class) so read paths stay ungated in + both directions. + */ + category = "exempt"; + operation = params.toolName; + } else { + /* + FNXC:AgentGating 2026-07-26-13:10: + Audit finding: an UNCLASSIFIED tool used to fall through with category + "exempt" → hardcoded allow, so anything the classifier missed bypassed + even a locked-down policy. Fail closed instead: unknown tools resolve to + the policy-governed `command_execution` category. Under the shipped + default `unrestricted` preset this is still "allow", so out-of-the-box + behavior is UNCHANGED; under strict presets unknown tools are now + actually governed. Genuine coordination exemptions must be positively + registered in COORDINATION_EXEMPT_TOOLS. + */ + category = "command_execution"; + operation = params.toolName; + resourceType = "other"; } /* @@ -253,7 +316,7 @@ export function evaluateAgentActionGate(params: { export function resolveGateOutcome( decision: AgentActionGateDecision, - latestRequest: { id: string; status: ApprovalRequestStatus } | null, + latestRequest: { id: string; status: ApprovalRequestStatus; decidedAt?: string } | null, ): { outcome: "allow" | "block" | "execute-once-then-complete" | "wait-for-approval"; approvalRequestId?: string } { if (decision.disposition === "allow") { return { outcome: "allow" }; @@ -268,6 +331,20 @@ export function resolveGateOutcome( return { outcome: "wait-for-approval", approvalRequestId: latestRequest.id }; } if (latestRequest.status === "approved") { + /* + FNXC:ApprovalRedemption 2026-07-26-13:05: + Approved-but-unredeemed grants expire after the grant TTL instead of + staying redeemable forever. An expired grant is treated as absent so a + fresh request is minted (wait-for-approval), never silently executed. + Closures that do not yet supply decidedAt skip TTL evaluation + (backward-compatible; both engine closures now supply it). + */ + if ( + latestRequest.decidedAt !== undefined + && isApprovalRequestExpired({ status: "approved", requestedAt: latestRequest.decidedAt, decidedAt: latestRequest.decidedAt }) + ) { + return { outcome: "wait-for-approval" }; + } return { outcome: "execute-once-then-complete", approvalRequestId: latestRequest.id }; } if (latestRequest.status === "denied") { diff --git a/packages/engine/src/agent-heartbeat.ts b/packages/engine/src/agent-heartbeat.ts index bab81221b4..0755f94b02 100644 --- a/packages/engine/src/agent-heartbeat.ts +++ b/packages/engine/src/agent-heartbeat.ts @@ -1014,6 +1014,28 @@ export class HeartbeatMonitor { return this.approvalRequestStore; } + /* + FNXC:AgentProvisioningGate 2026-07-26-13:15: + fn_agent_create / fn_agent_delete previously received no options here, which made the + factory synthesize approvalMode "never" and disabled the provisioning approval gate for + every production heartbeat lane. Always pass a real settingsProvider (guarded — lightweight + test TaskStores may lack getSettings) plus the shared PostgreSQL-backed ApprovalRequestStore + when the async layer is available. When no layer exists we deliberately pass no approval + store: the factory then fails CLOSED (require-approval => DENY), never silently allows. + */ + private buildAgentProvisioningToolOptions(taskStore: TaskStore): import("./agent-tools.js").AgentProvisioningToolOptions { + const maybeGetSettings = (taskStore as { getSettings?: () => Promise }).getSettings; + const options: import("./agent-tools.js").AgentProvisioningToolOptions = {}; + if (typeof maybeGetSettings === "function") { + options.settingsProvider = () => maybeGetSettings.call(taskStore); + } + const layer = typeof taskStore.getAsyncLayer === "function" ? taskStore.getAsyncLayer() : null; + if (layer) { + options.approvalRequestStore = new ApprovalRequestStore(null, { asyncLayer: layer }); + } + return options; + } + private buildActionGateContext(agent: Agent, taskId?: string, runId?: string, projectDefaultPolicy?: { rules?: Partial; toolRules?: import("@fusion/core").AgentPermissionPolicyToolRules }): AgentActionGateContext | undefined { const policy = resolveEffectiveAgentPermissionPolicy(agent.permissionPolicy, projectDefaultPolicy); return { @@ -1038,7 +1060,8 @@ export class HeartbeatMonitor { }), findApprovalByDedupeKey: async (dedupeKey) => { const latest = await this.getApprovalRequestStore().findLatestByDedupeKey({ requesterActorId: agent.id, taskId, dedupeKey }); - return latest ? { id: latest.id, status: latest.status } : null; + // FNXC:ApprovalRedemption 2026-07-26-14:30: decidedAt lets resolveGateOutcome apply the approval-grant TTL at redemption. + return latest ? { id: latest.id, status: latest.status, decidedAt: latest.decidedAt } : null; }, findPendingApprovalByDedupeKey: async (dedupeKey) => { const latest = await this.getApprovalRequestStore().findLatestByDedupeKey({ requesterActorId: agent.id, taskId, dedupeKey }); @@ -1076,6 +1099,8 @@ export class HeartbeatMonitor { await this.getApprovalRequestStore().markCompleted(approvalRequestId, { actor: { actorId: agent.id, actorType: "agent", actorName: agent.name }, note: "Tool executed after approval", + // FNXC:ApprovalRedemption 2026-07-26-14:35: ownership guard — an agent must not be able to burn another agent's approval by id. + expectedRequesterActorId: agent.id, }); }, }; @@ -1122,6 +1147,24 @@ export class HeartbeatMonitor { const pending = await this.getApprovalRequestStore().list({ status: "pending", requesterActorId: agent.id, taskId, limit: 100 }); return pending.find((request) => request.targetAction.context?.approvalDedupeKey === dedupeKey) ?? null; }, + /* + FNXC:AgentGating 2026-07-26-14:50: + Gate-path parity (audit): the permanent gate now pauses on a pending + approval exactly like this monitor's action-gate pauseForApproval — + task-level AWAITING_APPROVAL_PAUSE_REASON hold plus agent pause — so a + gated heartbeat agent stops instead of hunting for ungated workarounds. + */ + pauseForApproval: async ({ approvalRequestId, toolName }) => { + if (taskId && this.taskStore) { + await this.taskStore.pauseTask(taskId, true, undefined, { pausedByAgentId: agent.id, pausedReason: AWAITING_APPROVAL_PAUSE_REASON }); + await this.taskStore.logEntry( + taskId, + `Approval required for ${toolName}. Request ${approvalRequestId} created; task and agent paused awaiting decision.`, + ); + } + await this.store.updateAgentState(agent.id, "paused"); + await this.store.updateAgent(agent.id, { pauseReason: "awaiting-approval" }); + }, }; } @@ -2625,8 +2668,10 @@ export class HeartbeatMonitor { heartbeatTools.push(createTaskAssignTool(this.store, taskStore)); heartbeatTools.push(createGetAgentConfigTool(this.store, agentId)); heartbeatTools.push(createUpdateAgentConfigTool(this.store, agentId)); - heartbeatTools.push(createAgentCreateTool(this.store, agentId)); - heartbeatTools.push(createAgentDeleteTool(this.store, agentId)); + // FNXC:AgentProvisioningGate 2026-07-26-13:15: real settings + approval store so the provisioning policy actually gates idle-heartbeat lanes. + const idleProvisioningOptions = this.buildAgentProvisioningToolOptions(taskStore); + heartbeatTools.push(createAgentCreateTool(this.store, agentId, idleProvisioningOptions)); + heartbeatTools.push(createAgentDeleteTool(this.store, agentId, idleProvisioningOptions)); // Messaging tools — when MessageStore is available if (this.messageStore) { @@ -3956,8 +4001,10 @@ export class HeartbeatMonitor { tools.push(createTaskAssignTool(this.store, taskStore)); tools.push(createGetAgentConfigTool(this.store, agentId)); tools.push(createUpdateAgentConfigTool(this.store, agentId)); - tools.push(createAgentCreateTool(this.store, agentId)); - tools.push(createAgentDeleteTool(this.store, agentId)); + // FNXC:AgentProvisioningGate 2026-07-26-13:15: real settings + approval store so the provisioning policy actually gates task-scoped heartbeat lanes. + const taskProvisioningOptions = this.buildAgentProvisioningToolOptions(taskStore); + tools.push(createAgentCreateTool(this.store, agentId, taskProvisioningOptions)); + tools.push(createAgentDeleteTool(this.store, agentId, taskProvisioningOptions)); // Messaging tools — when MessageStore is available, agents can send and receive messages if (messageStore) { diff --git a/packages/engine/src/agent-tools.ts b/packages/engine/src/agent-tools.ts index 3d74a06816..81e60f5667 100644 --- a/packages/engine/src/agent-tools.ts +++ b/packages/engine/src/agent-tools.ts @@ -4611,9 +4611,44 @@ export function createGetAgentConfigTool(agentStore: AgentStore, callingAgentId: }; } -function isCallerPrivileged(caller: { id: string; role: string; reportsTo?: string | null } | null): boolean { +/* +FNXC:AgentProvisioningGate 2026-07-26-13:05: +Deliberate decision: only the "ceo" role is provisioning-privileged. Top-level position +(reportsTo == null) is NOT trust — any orphaned/imported/misconfigured top-level agent used +to auto-bypass resolveAgentProvisioningPolicy entirely, making the deny/require-approval +branches unreachable for it. Operator trust is expressed via +settings.agentProvisioning.trustedAgentIds/trustedRoles, not org position. Top-level +non-ceo agents now flow through the normal approval policy (default mode "trusted-only" +=> require-approval). +*/ +/* +FNXC:AgentProvisioning 2026-07-26-18:20: +Provisioning privilege comes from OPERATOR CONFIGURATION only — `agentProvisioning.trustedAgentIds` +and `agentProvisioning.trustedRoles` — never from a hardcoded role name and never from org-chart +position. + +Two earlier shapes were both wrong. `caller.reportsTo == null` made EVERY top-level agent privileged, +so an agent that created a manager-less agent escalated permanently. Replacing it with +`caller.role === "ceo"` swapped one implicit rule for a magic string: it silently grants a role that +any agent config can claim, while an operator who genuinely wants a privileged agent has no supported +way to say so other than naming it "ceo". + +Fails CLOSED: with no resolvable settings there is no privileged caller. This governs ONLY the +org-chart escape hatch (creating/deleting agents outside your own direct reports). It is deliberately +NOT fed to `resolveAgentProvisioningPolicy` as `isPrivileged`, because that flag short-circuits the +policy before `alwaysApproveDelete` — a trusted caller should still route a delete through approval. +The policy applies the same trusted-id/trusted-role rules itself, in the right order. +*/ +function isCallerPrivileged( + caller: { id: string; role: string; reportsTo?: string | null } | null, + settings: ProjectSettings | undefined, +): boolean { if (!caller) return false; - return caller.role === "ceo" || caller.reportsTo == null; + const provisioning = settings?.agentProvisioning; + if (!provisioning) return false; + if ((provisioning.trustedAgentIds ?? []).includes(caller.id)) return true; + const trustedRoles = (provisioning.trustedRoles ?? []).map((role) => role.toLowerCase()); + return Boolean(caller.role) && trustedRoles.includes(caller.role.toLowerCase()); } export function createUpdateAgentConfigTool(agentStore: AgentStore, callingAgentId: string): ToolDefinition { @@ -4723,7 +4758,7 @@ export function createUpdateAgentConfigTool(agentStore: AgentStore, callingAgent * @param taskStore - TaskStore for task creation * @returns ToolDefinition for the `fn_delegate_task` tool */ -type AgentProvisioningToolOptions = { +export type AgentProvisioningToolOptions = { hireApprovalEnabled?: boolean; approvalRequestStore?: ApprovalRequestStore; settingsProvider?: () => Promise; @@ -4742,7 +4777,9 @@ export function createAgentCreateTool( parameters: createAgentParams, execute: async (_id: string, params: Static) => { const caller = await agentStore.getAgent(callingAgentId); - const privileged = isCallerPrivileged(caller); + // FNXC:AgentProvisioning 2026-07-26-18:20: settings resolve BEFORE the org-chart check because privilege is now operator-configured rather than role-derived. + const settings = await options?.settingsProvider?.(); + const privileged = isCallerPrivileged(caller, settings); const reportsTo = params.reportsTo ?? callingAgentId; if (!privileged && reportsTo !== callingAgentId) { @@ -4752,14 +4789,20 @@ export function createAgentCreateTool( }; } - const settings = await options?.settingsProvider?.(); - const fallbackSettings = !options?.settingsProvider && !options?.approvalRequestStore - ? { agentProvisioning: { approvalMode: "never" as const } } - : settings; + /* + FNXC:AgentProvisioningGate 2026-07-26-13:10: + Never synthesize approvalMode "never" when the factory receives no options. All three + production call sites (heartbeat idle + task lanes, executor lane) previously passed no + options, so the synthesized "never" disabled the provisioning gate everywhere outside + tests. With no settingsProvider the policy now resolves with settings undefined + (normalizeMode default "trusted-only"); a require-approval decision with no + approvalRequestStore fails CLOSED below — never silently allows. + */ + // FNXC:AgentProvisioning 2026-07-26-18:20: `isPrivileged` is deliberately NOT forwarded — it short-circuits the policy ahead of `alwaysApproveDelete`. The policy re-applies trusted-id/trusted-role itself, in the correct order. const policy = resolveAgentProvisioningPolicy({ tool: "fn_agent_create", - caller: caller ? { id: caller.id, role: caller.role, isPrivileged: privileged } : undefined, - settings: fallbackSettings, + caller: caller ? { id: caller.id, role: caller.role } : undefined, + settings, }); await options?.runAuditor?.database({ type: "agent:create:requested", target: callingAgentId, metadata: { policy } }); @@ -4860,7 +4903,9 @@ export function createAgentDeleteTool( }; } - const privileged = isCallerPrivileged(caller); + // FNXC:AgentProvisioning 2026-07-26-18:20: operator-configured privilege; see isCallerPrivileged. + const deleteSettings = await options?.settingsProvider?.(); + const privileged = isCallerPrivileged(caller, deleteSettings); if (!privileged && target.reportsTo !== callingAgentId) { return { content: [{ type: "text" as const, text: "ERROR: You can only delete agents that report to you" }], @@ -4872,14 +4917,18 @@ export function createAgentDeleteTool( return { content: [{ type: "text" as const, text: `ERROR: Cannot delete ephemeral/runtime agent ${params.agent_id}` }], details: {} }; } - const settings = await options?.settingsProvider?.(); - const fallbackSettings = !options?.settingsProvider && !options?.approvalRequestStore - ? { agentProvisioning: { approvalMode: "never" as const } } - : settings; + /* + FNXC:AgentProvisioningGate 2026-07-26-13:10: + Same fail-closed contract as fn_agent_create: no synthesized "never" mode when options + are absent; settings undefined resolves to the "trusted-only" default and a + require-approval decision with no approvalRequestStore is DENIED below. + */ + // FNXC:AgentProvisioning 2026-07-26-18:20: reuse the already-resolved settings; `isPrivileged` is not forwarded so `alwaysApproveDelete` still applies to trusted callers. + const settings = deleteSettings; const policy = resolveAgentProvisioningPolicy({ tool: "fn_agent_delete", - caller: caller ? { id: caller.id, role: caller.role, isPrivileged: privileged } : undefined, - settings: fallbackSettings, + caller: caller ? { id: caller.id, role: caller.role } : undefined, + settings, }); await options?.runAuditor?.database({ type: "agent:delete:requested", target: target.id, metadata: { policy } }); diff --git a/packages/engine/src/bash-containment.ts b/packages/engine/src/bash-containment.ts new file mode 100644 index 0000000000..3bc708274e --- /dev/null +++ b/packages/engine/src/bash-containment.ts @@ -0,0 +1,127 @@ +import { homedir } from "node:os"; + +/* +FNXC:BashContainment 2026-07-26-12:40: +Security incident follow-up: an agent's real escalation chain was +`bash("cat ~/.fusion/settings.json")` (agents run as the same OS user, so the +0600 file holding `daemonToken` is readable) → find its own pending approval +request → `curl` the approvals decision API to approve itself. Fixing the +decision route alone is insufficient while bash can read the token. + +This module is a PRIVILEGE-ESCALATION FLOOR, not a permission preference: +it holds UNCONDITIONALLY at every permission preset including the default +`unrestricted`. Ordinary bash permission gating stays policy-driven (see +agent-action-gate.ts); this floor only denies the narrow set of commands +that let an agent escalate its OWN privileges: + 1. reads of the Fusion global config / daemon token (~/.fusion, any + .fusion/settings.json spelling, FUSION_DAEMON_TOKEN / daemonToken refs), + 2. reads of common same-user credential stores (~/.ssh, ~/.aws, ~/.netrc, + ~/.npmrc, ~/.config/gh, ~/.docker/config.json, ~/.gnupg, ~/.kube), + 3. self-directed calls to the approvals API (/api/approvals, fn_token=). + +HONESTY / LIMITATIONS (do not oversell this): matching happens on a +normalized copy of the command STRING (quotes/backslashes stripped, $HOME and +the literal home directory folded to ~, lowercased). That defeats the trivial +bypasses (`cat ~/.fus''ion/settings.json`, `"$HOME"/.fusion`, absolute home +paths) but NOT determined indirection: base64-built paths, variable +concatenation (a=fus; b=ion; cat ~/.$a$b/...), glob spelling (~/.f*sion), +symlinks created first, python/node one-liners, or reading via a spawned +interpreter. String inspection of shell cannot be sound. This floor raises +the cost of the KNOWN escalation chain and creates an audit-visible denial; +real containment requires OS-level isolation (separate user / sandbox), +which is out of scope here. Deny messages are deliberately explicit so a +probing agent's attempts are visible in session logs. +*/ + +export interface BashContainmentVerdict { + allowed: boolean; + /** Stable rule id for logs/tests when denied. */ + rule?: string; + /** Human-readable denial reason (safe to surface to the agent). */ + reason?: string; +} + +interface ContainmentRule { + id: string; + pattern: RegExp; + reason: string; +} + +/* +FNXC:BashContainment 2026-07-26-12:40: +Rules match the NORMALIZED command (see normalizeCommand). Home-anchored +patterns use `~/.` because normalization folds $HOME/${HOME}/absolute +home spellings to `~`. `/users//` and `/home//` cover OTHER +users' homes which normalization cannot fold. +*/ +const RULES: readonly ContainmentRule[] = [ + { + id: "fusion-global-dir", + pattern: /(?:~|\/users\/[^/\s]+|\/home\/[^/\s]+)\/\.fusion\b/, + reason: "access to the global Fusion directory (daemon token / global settings) is not permitted from agent sessions", + }, + { + id: "fusion-settings-file", + pattern: /\.fusion\/settings\.json/, + reason: "access to Fusion settings.json is not permitted from agent sessions", + }, + { + id: "fusion-daemon-token", + pattern: /fusion_daemon_token|fusion_dashboard_token|daemontoken/, + reason: "referencing the Fusion daemon token is not permitted from agent sessions", + }, + { + id: "credential-store", + pattern: /(?:~|\/users\/[^/\s]+|\/home\/[^/\s]+)\/(?:\.ssh|\.aws|\.netrc|\.npmrc|\.gnupg|\.kube|\.config\/gh|\.docker\/config\.json)\b/, + reason: "access to user credential stores is not permitted from agent sessions", + }, + { + id: "approvals-api", + pattern: /\/api\/approvals|fn_token=/, + reason: "calling the Fusion approvals API from a shell is not permitted from agent sessions (approvals are decided by the operator)", + }, +]; + +function escapeRegExp(value: string): string { + return value.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); +} + +const HOME_DIR = homedir(); +const HOME_PATTERN = new RegExp(escapeRegExp(HOME_DIR), "gi"); + +/** + * FNXC:BashContainment 2026-07-26-12:40: + * Normalization defeats quote-splitting and $HOME spellings only. Keep this + * pure and dependency-free so it is trivially unit-testable. + */ +export function normalizeBashCommandForContainment(command: string): string { + let normalized = command.replace(/["'\\]/g, ""); + normalized = normalized.replace(/\$\{home\}/gi, "~").replace(/\$home\b/gi, "~"); + if (HOME_DIR && HOME_DIR !== "/") { + normalized = normalized.replace(HOME_PATTERN, "~"); + } + return normalized.toLowerCase(); +} + +/** Evaluate the unconditional containment floor for one bash command string. */ +export function evaluateBashContainment(command: string): BashContainmentVerdict { + if (typeof command !== "string" || command.trim() === "") { + return { allowed: true }; + } + const normalized = normalizeBashCommandForContainment(command); + for (const rule of RULES) { + if (rule.pattern.test(normalized)) { + return { allowed: false, rule: rule.id, reason: rule.reason }; + } + } + return { allowed: true }; +} + +/** Stable message shown to the agent on denial. */ +export function buildBashContainmentDenialMessage(verdict: BashContainmentVerdict): string { + return ( + `Command blocked by Fusion privilege-escalation containment (${verdict.rule ?? "containment"}): ` + + `${verdict.reason ?? "not permitted"}. This boundary applies at every permission preset; ` + + `do not attempt to work around it — ask the operator instead.` + ); +} diff --git a/packages/engine/src/executor.ts b/packages/engine/src/executor.ts index 19106be76c..de2fc0e56d 100644 --- a/packages/engine/src/executor.ts +++ b/packages/engine/src/executor.ts @@ -2641,7 +2641,8 @@ export class TaskExecutor { }), findApprovalByDedupeKey: async (dedupeKey) => { const latest = await this.approvalRequestStore.findLatestByDedupeKey({ requesterActorId: actorId, taskId, dedupeKey }); - return latest ? { id: latest.id, status: latest.status } : null; + // FNXC:ApprovalRedemption 2026-07-26-14:30: decidedAt lets resolveGateOutcome apply the approval-grant TTL at redemption. + return latest ? { id: latest.id, status: latest.status, decidedAt: latest.decidedAt } : null; }, findPendingApprovalByDedupeKey: async (dedupeKey) => { const latest = await this.approvalRequestStore.findLatestByDedupeKey({ requesterActorId: actorId, taskId, dedupeKey }); @@ -2710,6 +2711,8 @@ export class TaskExecutor { await this.approvalRequestStore.markCompleted(approvalRequestId, { actor: { actorId, actorType: "agent", actorName }, note: "Tool executed after approval", + // FNXC:ApprovalRedemption 2026-07-26-14:35: ownership guard — an agent must not be able to burn another agent's approval by id. + expectedRequesterActorId: actorId, }); }, }; @@ -2767,6 +2770,30 @@ export class TaskExecutor { const pending = await this.approvalRequestStore.list({ status: "pending", requesterActorId: actorId, taskId, limit: 100 }); return pending.find((request) => request.targetAction.context?.approvalDedupeKey === dedupeKey) ?? null; }, + /* + FNXC:AgentGating 2026-07-26-14:50: + Audit finding (gate-path divergence): the permanent gate minted an + approval request but never paused, so the agent kept its turn while + "awaiting approval". Mirror the action gate's task-level hold (canonical + AWAITING_APPROVAL_PAUSE_REASON + approvalSuspended marker). Session + suspension is intentionally not wired here: the permanent gate only runs + in lanes WITHOUT an actionGateContext, where no executor in-flight + session surface exists to abort. + */ + pauseForApproval: async ({ approvalRequestId, toolName }) => { + if (!taskId) return; + this.approvalSuspended.add(taskId); + try { + await this.store.pauseTask(taskId, true, this.getRunContextFor(taskId), { pausedByAgentId: actorId, pausedReason: AWAITING_APPROVAL_PAUSE_REASON }); + await this.store.logEntry( + taskId, + `Approval required for ${toolName}. Request ${approvalRequestId} created; task paused awaiting decision.`, + ); + } catch (error) { + this.approvalSuspended.delete(taskId); + throw error; + } + }, }; } @@ -13820,6 +13847,19 @@ export class TaskExecutor { }, }).catch(() => undefined); } + /* + FNXC:AgentProvisioningGate 2026-07-26-13:20: + fn_agent_create / fn_agent_delete previously received no options in the executor lane, + which made the factory synthesize approvalMode "never" and disabled the provisioning + approval gate in production. Pass a live settingsProvider plus the shared + PostgreSQL-backed ApprovalRequestStore when the async layer exists; without a layer we + pass no approval store so the factory fails CLOSED (require-approval => DENY). + */ + const provisioningApprovalLayer = typeof this.store.getAsyncLayer === "function" ? this.store.getAsyncLayer() : null; + const agentProvisioningToolOptions = { + settingsProvider: async () => await this.store.getSettings(), + ...(provisioningApprovalLayer ? { approvalRequestStore: this.approvalRequestStore } : {}), + }; const customTools = [ this.createTaskUpdateTool(task.id, codeReviewVerdicts, sessionRef, stuckDetector), this.createTaskLogTool(task.id), @@ -13915,8 +13955,8 @@ export class TaskExecutor { ...(assignedAgentId ? [ createGetAgentConfigTool(this.options.agentStore, assignedAgentId), createUpdateAgentConfigTool(this.options.agentStore, assignedAgentId), - createAgentCreateTool(this.options.agentStore, assignedAgentId), - createAgentDeleteTool(this.options.agentStore, assignedAgentId), + createAgentCreateTool(this.options.agentStore, assignedAgentId, agentProvisioningToolOptions), + createAgentDeleteTool(this.options.agentStore, assignedAgentId, agentProvisioningToolOptions), ] : []), ] : []), // Messaging tools — allows executor agents to send and receive messages. diff --git a/packages/engine/src/gating-classifications.ts b/packages/engine/src/gating-classifications.ts index 5b620ac419..4930593723 100644 --- a/packages/engine/src/gating-classifications.ts +++ b/packages/engine/src/gating-classifications.ts @@ -25,6 +25,16 @@ const SHARED_TASK_AGENT_TOOLS = [ "fn_task_refine", ] as const; const PROVISIONING_TOOLS = ["fn_agent_create", "fn_agent_delete"] as const; +/** + * FNXC:AgentGating 2026-07-26-15:05: + * FN-3953 keeps provisioning tools OUT of action-gate task_agent_mutation so the + * dedicated agent_provisioning approval policy (resolveAgentProvisioningPolicy, + * now live in production lanes) is the single authority — double approval rows + * would otherwise be minted. With the action gate's unknown-tool fallback now + * fail-closed, this deliberate exemption must be POSITIVE, not an accident of + * the old exempt default. + */ +export const ACTION_GATE_PROVISIONING_POLICY_TOOLS: ReadonlySet = new Set(PROVISIONING_TOOLS); /** * FNXC:ToolGovernance 2026-06-27-12:00: @@ -153,6 +163,8 @@ export const NETWORK_API_TOOLS: ReadonlySet = new Set([ export const ACTION_GATE_NETWORK_API_TOOLS: ReadonlySet = new Set([ "fn_research_run", "fn_research_cancel", + // FNXC:AgentGating 2026-07-26-15:15: fn_research_retry re-runs an outbound research call; it previously slipped through the action gate via the exempt fallback (silent-exemption defect). Parity with the permanent gate's network_api classification; still "allow" under the default preset. + "fn_research_retry", "fn_web_fetch", // FN-4603: honor network_api approval policy for web fetches. "worktrunk_install", // FN-4624: gate binary auto-install under network_api policy. ]); diff --git a/packages/engine/src/index.ts b/packages/engine/src/index.ts index 35c278583a..5ec05fe02c 100644 --- a/packages/engine/src/index.ts +++ b/packages/engine/src/index.ts @@ -5,8 +5,9 @@ export { type ReportHealthClassification, type ReportHealthInput, } from "./reports-health.js"; -export { reloadExemptTools, addToExemptTools, getExemptToolNames } from "./agent-action-gate.js"; -export type { AgentActionGateContext } from "./agent-action-gate.js"; +// FNXC:ToolPermissionGates 2026-07-26-13:55: evaluateAgentActionGate/resolveGateOutcome are exported so the @runfusion/fusion pi extension can enforce the SAME per-agent permission policy on host-extension fn_* tools that engine lanes enforce, instead of shipping a second drift-prone policy evaluator. +export { reloadExemptTools, addToExemptTools, getExemptToolNames, evaluateAgentActionGate, resolveGateOutcome } from "./agent-action-gate.js"; +export type { AgentActionGateContext, AgentActionGateDecision } from "./agent-action-gate.js"; export { createFusionAuthStorage, createFusionModelRegistry } from "./auth-storage.js"; export { DEFAULT_MODEL_REGISTRY_REFRESH_TIMEOUT_MS, diff --git a/packages/engine/src/permanent-agent-gating.ts b/packages/engine/src/permanent-agent-gating.ts index d0ca980f51..bfdfbd381e 100644 --- a/packages/engine/src/permanent-agent-gating.ts +++ b/packages/engine/src/permanent-agent-gating.ts @@ -171,11 +171,21 @@ export function resolvePermanentAgentToolDecision(input: { Keep permanent-agent results in lockstep with evaluateAgentActionGate. */ + /* + FNXC:AgentGating 2026-07-26-13:45: + Audit finding: a gating context with a wholly MISSING permissionPolicy used + to resolve every tool to "allow" — an accidental unrestricted bypass. Fail + closed instead: recognized coordination ("none") tools stay allowed so + heartbeats cannot deadlock, everything else requires approval. Production + context builders (executor.ts, agent-heartbeat.ts, chat.ts) always resolve a + policy via resolveEffectiveAgentPermissionPolicy, so default-preset behavior + is unchanged; this branch only bites a buggy/hand-rolled context. + */ if (!input.gating?.permissionPolicy) { return { ...classification, toolName: input.toolName, - disposition: "allow", + disposition: classification.category === "none" && classification.recognized ? "allow" : "require-approval", }; } diff --git a/packages/engine/src/pi.ts b/packages/engine/src/pi.ts index 9ba7b99f27..81d29a3d0b 100644 --- a/packages/engine/src/pi.ts +++ b/packages/engine/src/pi.ts @@ -50,6 +50,7 @@ import { mergeSupplementalOpenAiCodexModels, registerBuiltInGrokProvider, registerBuiltInZaiProvider, + registerFusionSessionIdentity, resolvePiExtensionProjectRoot, resolveToolOutputBudget, } from "@fusion/core"; @@ -78,6 +79,7 @@ import { type AgentActionGateContext, } from "./agent-action-gate.js"; import { resolvePermanentAgentToolDecision } from "./permanent-agent-gating.js"; +import { buildBashContainmentDenialMessage, evaluateBashContainment } from "./bash-containment.js"; import type { SystemPromptLayers } from "./prompt-layers.js"; import { READONLY_ALLOWLIST, filterCustomToolsForReadonly, isReadonlyAllowed } from "./workflow-step-tool-policy.js"; import { createStreamingDeltaNormalizer } from "./streaming-delta.js"; @@ -1877,6 +1879,42 @@ export function wrapToolsWithBoundary( }); } +/* +/* +FNXC:BashContainment 2026-07-26-13:20: +Unconditional privilege-escalation floor for engine-spawned sessions (see +bash-containment.ts for the threat model and honest limitations). Applied +INNERMOST in the wrapper chain so it evaluates the FINAL command string — +including an rtk-rewritten command — immediately before execution. This is +deliberately NOT policy-driven: it holds at every permission preset including +the default `unrestricted`, because it guards the agent-self-escalation +boundary (daemon token, credential stores, approvals API), not an operator +preference. Ordinary bash permission gating remains the action gate's job. +*/ +export function wrapToolsWithBashContainment(tools: ToolDefinition[]): ToolDefinition[] { + return tools.map((tool) => { + if (tool.name !== "bash") { + return tool; + } + const originalExecute = tool.execute as any; + return { + ...tool, + execute: async (...args: any[]) => { + const params = (args[1] ?? {}) as Record; + const command = typeof params.command === "string" ? params.command : ""; + const verdict = evaluateBashContainment(command); + if (!verdict.allowed) { + piLog.warn(`[bash-containment] denied rule=${verdict.rule ?? "unknown"}`); + return boundaryRejection(buildBashContainmentDenialMessage(verdict), { + containmentRule: verdict.rule, + }); + } + return originalExecute(...args); + }, + }; + }); +} + /* FNXC:ToolOutputBudget 2026-08-06-12:00: FN-8614 requires one finite budget for the total model-visible text in every @@ -2034,6 +2072,19 @@ export function wrapToolsWithPermanentAgentGating( if (approvalRequest?.id) { details.approvalRequestId = approvalRequest.id; + /* + FNXC:AgentGating 2026-07-26-14:45: + Keep the permanent gate consistent with the action gate: once a + pending approval exists (fresh or reused), pause via the context + hook so the agent does not keep its turn while "awaiting approval". + Optional: legacy contexts without the hook keep prior behavior. + */ + try { + await gating.pauseForApproval?.({ approvalRequestId: approvalRequest.id, toolName: decision.toolName }); + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + piLog.warn(`[permanent-gate] pauseForApproval failed: ${message}`); + } } } @@ -2561,7 +2612,10 @@ export async function createFnAgent(options: AgentOptions): Promise ...(tools as ToolDefinition[]), ...allowlistFilteredCustomTools.allowed, ]; - const toolsWithRtkRewrite = wrapToolsWithRtkRewrite(toolChainStart); + // FNXC:BashContainment 2026-07-26-13:20: innermost wrapper — sees the final + // (post-rtk-rewrite) command; applies to every engine session unconditionally. + const toolsWithContainment = wrapToolsWithBashContainment(toolChainStart); + const toolsWithRtkRewrite = wrapToolsWithRtkRewrite(toolsWithContainment); /* * FNXC:AgentGating 2026-07-12-17:22: * MAIN-008 requires one approval authority per tool call. Executor sessions @@ -2847,6 +2901,49 @@ export async function createFnAgent(options: AgentOptions): Promise }); }; + /* + FNXC:SessionIdentity 2026-07-26-18:50: + Review finding: model-swap sessions lost their identity registration. The swap + disposes the old session (whose wrapped dispose deregisters the identity) and + Object.assign then installs the NEW session's unwrapped dispose — so after a + fallback swap, extension tool calls for this cwd resolved to "operator" instead + of the engine agent principal. The registration/dispose-wrapping is therefore a + per-session-instance helper: applied to the initial session at the end of + createFnAgent AND to every swapped-in session here (before Object.assign copies + the wrapped dispose onto the caller-held facade), so each instance registers on + attach and deregisters exactly once on its own dispose. + */ + const sessionIdentity = (() => { + const principalAgentId = options.actionGateContext?.agentId + ?? options.permanentAgentGating?.requester?.actorId + ?? "engine-session"; + const principalAgentName = options.actionGateContext?.agentName + ?? options.permanentAgentGating?.requester?.actorName; + return { + agentId: principalAgentId, + ...(principalAgentName ? { agentName: principalAgentName } : {}), + ...(options.taskId ? { taskId: options.taskId } : {}), + ...(options.sessionPurpose ? { purpose: options.sessionPurpose } : {}), + }; + })(); + const sessionIdentityKeys = [...new Set([options.cwd, resolvedProjectRoot].filter((key): key is string => Boolean(key)))]; + const attachSessionIdentity = (session: PromptableSession & { dispose?: () => void | Promise }): void => { + const identityDisposers = sessionIdentityKeys.map((key) => registerFusionSessionIdentity(key, sessionIdentity)); + const disposeBeforeIdentity = typeof session.dispose === "function" + ? session.dispose.bind(session) + : () => undefined; + session.dispose = async () => { + for (const disposeIdentity of identityDisposers) { + try { + disposeIdentity(); + } catch { + // Registry cleanup must never mask the underlying dispose. + } + } + await Promise.resolve(disposeBeforeIdentity()); + }; + }; + const swapPromptSession = async (modelToUse: typeof selectedModel): Promise => { if (!modelToUse) { throw new Error("Cannot swap session without a resolved model"); @@ -2861,6 +2958,9 @@ export async function createFnAgent(options: AgentOptions): Promise const next = (await createSessionWithModel(modelToUse)).session as PromptableSession; wireFallbackHooks(next); wrapSessionDisposeWithShutdown(next); + // FNXC:SessionIdentity 2026-07-26-18:50: re-register for the swapped-in session; + // Object.assign below copies the identity-wrapped dispose onto the facade. + attachSessionIdentity(next as PromptableSession & { dispose?: () => void | Promise }); applyThinkingLevelIfSupported(next, `${modelToUse.provider}/${modelToUse.id}`); Object.setPrototypeOf(promptableSession, Object.getPrototypeOf(next)); Object.assign(promptableSession, next); @@ -2991,5 +3091,22 @@ export async function createFnAgent(options: AgentOptions): Promise } }); + /* + FNXC:SessionIdentity 2026-07-26-13:35: + Register this engine-spawned session in the globalThis identity registry so + the bundled @runfusion/fusion pi extension (whose tools bypass every engine + gate wrapper — they are loaded by pi's resource loader, not customTools) can + distinguish agent principals from a human operator CLI. EVERY createFnAgent + session is an LLM principal, never a human terminal, so registration is + unconditional; the best-known agent identity comes from the action-gate or + permanent-gating contexts, falling back to a synthetic "engine-session" id + that the extension must still treat as an agent (fail closed). Registered + AFTER successful session construction (extension tools only run once the + caller prompts, i.e. post-return), keyed under both the session cwd and the + resolved project root because pi may surface either as ExtensionContext.cwd. + Deregistration rides the session's dispose chain. + */ + attachSessionIdentity(promptableSession as PromptableSession & { dispose?: () => void | Promise }); + return { session: promptableSession, sessionFile: promptableSession.sessionFile }; } diff --git a/packages/engine/src/plugin-runner.ts b/packages/engine/src/plugin-runner.ts index 5b58aafb7e..e0da01dc23 100644 --- a/packages/engine/src/plugin-runner.ts +++ b/packages/engine/src/plugin-runner.ts @@ -38,6 +38,7 @@ import type { ToolDefinition } from "@earendil-works/pi-coding-agent"; import { Type } from "@earendil-works/pi-ai"; import { isAbsolute } from "node:path"; import { + PluginLoader as CorePluginLoader, getTraitRegistry, getWorkflowExtensionRegistry, evaluatePromptConditionDetailed, @@ -813,9 +814,13 @@ export class PluginRunner { const pluginId = plugin.manifest.id; try { const settings = await this.getPluginSettings(pluginId); + // FNXC:PluginTaskStoreGate 2026-07-26-12:20: same gate as createToolContext. const context: PluginContext = { pluginId, - taskStore: this.options.taskStore, + taskStore: CorePluginLoader.createGatedTaskStore(this.options.taskStore, { + pluginId, + permissions: plugin.manifest.permissions, + }), settings, logger: this.createPluginLogger(pluginId), emitEvent: (event: string, data: unknown) => { @@ -1252,9 +1257,14 @@ export class PluginRunner { */ private async createToolContext(plugin: FusionPlugin): Promise { const settings = await this.getPluginSettings(plugin.manifest.id); + // FNXC:PluginTaskStoreGate 2026-07-26-12:20: destructive TaskStore methods are + // gated behind manifest permissions.destructiveTaskOps for every plugin context. return { pluginId: plugin.manifest.id, - taskStore: this.options.taskStore, + taskStore: CorePluginLoader.createGatedTaskStore(this.options.taskStore, { + pluginId: plugin.manifest.id, + permissions: plugin.manifest.permissions, + }), settings, logger: this.createPluginLogger(plugin.manifest.id), emitEvent: (event: string, data: unknown) => { @@ -1281,9 +1291,13 @@ export class PluginRunner { } const settings = await this.getPluginSettings(pluginId); + // FNXC:PluginTaskStoreGate 2026-07-26-12:20: same gate as createToolContext. return { pluginId, - taskStore: this.options.taskStore, + taskStore: CorePluginLoader.createGatedTaskStore(this.options.taskStore, { + pluginId, + permissions: plugin.manifest.permissions, + }), settings, logger: this.createPluginLogger(pluginId), emitEvent: (event: string, data: unknown) => { diff --git a/packages/engine/src/sandbox/__tests__/provisioning-gate.test.ts b/packages/engine/src/sandbox/__tests__/provisioning-gate.test.ts index efb86af052..cf9ee610cf 100644 --- a/packages/engine/src/sandbox/__tests__/provisioning-gate.test.ts +++ b/packages/engine/src/sandbox/__tests__/provisioning-gate.test.ts @@ -178,4 +178,48 @@ describe("requireSandboxProvisioningApproval", () => { (secondError as SandboxProvisioningPendingError).dedupeKey, ); }); + + /* + FNXC:SandboxProvisioningGate 2026-07-26-13:40: + Self-asserted requester.actorType === "user" must NOT grant privilege; only the + caller-verified callerVerifiedPrivileged flag set by trusted engine code does. + Expected outcomes are hardcoded, never derived from the policy module. + */ + it("self-asserted actorType 'user' no longer bypasses the gate: still requires approval", async () => { + const createApprovalRequest = vi.fn(async () => makeApproval("apr-1", "pending")); + + await expect( + requireSandboxProvisioningApproval({ + backendId: "bubblewrap", + operation: "install", + description: "Install bubblewrap", + context: { + taskId: "FN-4641", + requester: { actorId: "someone", actorType: "user", actorName: "Impostor" }, + settings: undefined, + createApprovalRequest, + }, + }), + ).rejects.toBeInstanceOf(SandboxProvisioningPendingError); + + expect(createApprovalRequest).toHaveBeenCalledTimes(1); + }); + + it("callerVerifiedPrivileged set by trusted engine code allows without approval", async () => { + const createApprovalRequest = vi.fn(); + const result = await requireSandboxProvisioningApproval({ + backendId: "bubblewrap", + operation: "install", + description: "Install bubblewrap", + context: { + taskId: "FN-4641", + requester: { actorId: "operator", actorType: "user", actorName: "Operator" }, + callerVerifiedPrivileged: true, + settings: undefined, + createApprovalRequest, + }, + }); + expect(result).toEqual({ outcome: "allow" }); + expect(createApprovalRequest).not.toHaveBeenCalled(); + }); }); diff --git a/packages/engine/src/sandbox/provisioning-gate.ts b/packages/engine/src/sandbox/provisioning-gate.ts index 97f348a9a0..0bbc4cbc51 100644 --- a/packages/engine/src/sandbox/provisioning-gate.ts +++ b/packages/engine/src/sandbox/provisioning-gate.ts @@ -34,6 +34,14 @@ export interface SandboxProvisioningGateContext { taskId?: string; runId?: string; requester: ApprovalRequestActorSnapshot; + /* + FNXC:SandboxProvisioningGate 2026-07-26-13:25: + Privilege must be asserted by TRUSTED ENGINE CODE that verified the caller, never derived + from request-context strings. The gate previously treated the self-asserted + requester.actorType === "user" as privileged, so any caller claiming to be a user bypassed + the sandbox provisioning policy entirely. Defaults to unprivileged when omitted. + */ + callerVerifiedPrivileged?: boolean; settings: Pick | undefined; createApprovalRequest: (input: { category: "sandbox_provisioning"; @@ -64,13 +72,25 @@ export async function requireSandboxProvisioningApproval(input: { operation, }); + /* + FNXC:SandboxProvisioningGate 2026-07-26-13:25: + isPrivileged now comes only from callerVerifiedPrivileged (set by trusted engine code), + not from the self-asserted requester.actorType string. + Status note: this gate currently has NO production caller — it is exported via + sandbox/index.ts but only exercised by sandbox/__tests__/provisioning-gate.test.ts, and no + approval executor is registered for the sandbox_provisioning category. Approving a request + created here therefore cannot silently no-op from this module's perspective: execution only + happens when the caller re-runs this gate and resolveGateOutcome maps the approved request + (via findApprovalByDedupeKey) to "execute-once-then-complete"; without that re-run nothing + executes at all. + */ const policyDecision = resolveSandboxProvisioningPolicy({ backendId, operation, caller: { id: context.requester.actorId, role: context.requester.actorType === "agent" ? "agent" : context.requester.actorType, - isPrivileged: context.requester.actorType === "user", + isPrivileged: context.callerVerifiedPrivileged === true, }, settings: context.settings, }); diff --git a/plugins/fusion-plugin-acp-runtime/src/__tests__/control-handler.test.ts b/plugins/fusion-plugin-acp-runtime/src/__tests__/control-handler.test.ts index ec848fbeba..147b55d5bb 100644 --- a/plugins/fusion-plugin-acp-runtime/src/__tests__/control-handler.test.ts +++ b/plugins/fusion-plugin-acp-runtime/src/__tests__/control-handler.test.ts @@ -271,18 +271,88 @@ describe("resolvePermission — the security floor", () => { expect(selectedId(res)).toBe("reject_once_id"); }); - it("reuses a prior approved decision via the dedupe key (no new request)", async () => { + /* + FNXC:AcpApprovalConsumption 2026-07-26-12:50: + Approvals are execute-once-then-complete. Reusing an approved row must + CONSUME it via markApprovalCompleted; a completed row no longer authorizes, + so an identical second request goes back through the HITL round-trip. + */ + it("reuses a prior approved decision once, consuming it via markApprovalCompleted (no new request)", async () => { const createApprovalRequest = vi.fn(async () => ({ id: "appr-x" })); + const markApprovalCompleted = vi.fn(async () => {}); const gate: PermissionGate = gateWithRules( { ...UNRESTRICTED, command_execution: "require-approval" }, { createApprovalRequest, + markApprovalCompleted, findApprovalByDedupeKey: vi.fn(async () => ({ id: "prior", status: "approved" as const })), }, ); const res = await resolvePermission(toolCall("execute"), ALL_OPTIONS, gate); expect(selectedId(res)).toBe("allow_once_id"); expect(createApprovalRequest).not.toHaveBeenCalled(); + // The single-use grant is finalized before the allow is returned. + expect(markApprovalCompleted).toHaveBeenCalledOnce(); + expect(markApprovalCompleted).toHaveBeenCalledWith("prior"); + }); + + it("does NOT auto-allow a second identical request after the approval is consumed", async () => { + // In-memory approval store: one approved row that flips to completed on + // markApprovalCompleted, mirroring the engine's approval lifecycle. + const row = { id: "prior", status: "approved" as "approved" | "completed" }; + const createApprovalRequest = vi.fn(async () => ({ id: "appr-2" })); + const gate: PermissionGate = gateWithRules( + { ...UNRESTRICTED, command_execution: "require-approval" }, + { + createApprovalRequest, + markApprovalCompleted: vi.fn(async () => { + row.status = "completed"; + }), + findApprovalByDedupeKey: vi.fn(async () => ({ id: row.id, status: row.status })), + // Pause never resolves a decision → the second call must NOT allow. + pauseForApproval: vi.fn(async () => {}), + }, + ); + + const first = await resolvePermission(toolCall("execute"), ALL_OPTIONS, gate); + expect(selectedId(first)).toBe("allow_once_id"); + expect(row.status).toBe("completed"); + + const second = await resolvePermission(toolCall("execute"), ALL_OPTIONS, gate); + // Completed is not approved: the second identical call re-enters the HITL + // flow (a new request is registered) and, with no human grant, denies. + expect(selectedId(second)).toBe("reject_once_id"); + expect(createApprovalRequest).toHaveBeenCalledTimes(1); + }); + + it("does not reuse an approved row when it cannot be consumed (no markApprovalCompleted)", async () => { + const createApprovalRequest = vi.fn(async () => ({ id: "appr-x" })); + const gate: PermissionGate = gateWithRules( + { ...UNRESTRICTED, command_execution: "require-approval" }, + { + createApprovalRequest, + findApprovalByDedupeKey: vi.fn(async () => ({ id: "prior", status: "approved" as const })), + // No markApprovalCompleted and no pauseForApproval → the fresh + // round-trip cannot complete → default-deny, never an unconsumable allow. + }, + ); + const res = await resolvePermission(toolCall("execute"), ALL_OPTIONS, gate); + expect(selectedId(res)).toBe("reject_once_id"); + }); + + it("still reuses a prior denied decision without consuming anything", async () => { + const markApprovalCompleted = vi.fn(async () => {}); + const gate: PermissionGate = gateWithRules( + { ...UNRESTRICTED, command_execution: "require-approval" }, + { + createApprovalRequest: vi.fn(async () => ({ id: "appr-x" })), + markApprovalCompleted, + findApprovalByDedupeKey: vi.fn(async () => ({ id: "prior", status: "denied" as const })), + }, + ); + const res = await resolvePermission(toolCall("execute"), ALL_OPTIONS, gate); + expect(selectedId(res)).toBe("reject_once_id"); + expect(markApprovalCompleted).not.toHaveBeenCalled(); }); it("require-approval with NO closures → default-deny, no throw", async () => { diff --git a/plugins/fusion-plugin-acp-runtime/src/control-handler.ts b/plugins/fusion-plugin-acp-runtime/src/control-handler.ts index f51809c602..d3f68ac3d0 100644 --- a/plugins/fusion-plugin-acp-runtime/src/control-handler.ts +++ b/plugins/fusion-plugin-acp-runtime/src/control-handler.ts @@ -183,11 +183,29 @@ export async function runApprovalForCategory( status === "approved" ? "allow" : "deny"; try { - // Reuse a prior decision for an identical call when available. + /* + FNXC:AcpApprovalConsumption 2026-07-26-12:50: + Approvals are execute-once-then-complete (mirrors the pi action gate's + resolveGateOutcome + markApprovalCompleted contract). The previous reuse + branch returned allow on an `approved` row WITHOUT consuming it, so one + human approval authorized unlimited repeats of the same tool call. Now an + approved row is marked completed (the engine-wired closure records the + requesting agent's actor snapshot and the "Tool executed after approval" + note) BEFORE the allow is returned; a `completed` row is not approved, so + the next identical call goes back through the HITL round-trip. When + `markApprovalCompleted` is absent the approval CANNOT be consumed, so it is + not reused either — the call falls through to a fresh approval round-trip + (or the default-deny floor) instead of granting an unconsumable allow. + Denied rows remain reusable: repeating a denial is the conservative outcome. + */ if (typeof gate.findApprovalByDedupeKey === "function") { const prior = await gate.findApprovalByDedupeKey(dedupeKey); - if (prior && (prior.status === "approved" || prior.status === "denied")) { - return mapStatus(prior.status); + if (prior && prior.status === "denied") { + return "deny"; + } + if (prior && prior.status === "approved" && typeof gate.markApprovalCompleted === "function") { + await gate.markApprovalCompleted(prior.id); + return "allow"; } } diff --git a/plugins/fusion-plugin-even-realities-glasses/src/__tests__/agent-actions.test.ts b/plugins/fusion-plugin-even-realities-glasses/src/__tests__/agent-actions.test.ts index d971abfa8e..ea80daf348 100644 --- a/plugins/fusion-plugin-even-realities-glasses/src/__tests__/agent-actions.test.ts +++ b/plugins/fusion-plugin-even-realities-glasses/src/__tests__/agent-actions.test.ts @@ -72,7 +72,8 @@ describe("startWork", () => { it("moves allowed tasks to in-progress and returns task card", async () => { const deps = createDeps(makeTask({ column: "todo", status: null })); const result = await startWork({ taskId: "FN-1" }, deps as never); - expect(deps.moveTask).toHaveBeenCalledWith("FN-1", "in-progress"); + // FNXC:GlassesAgentActions 2026-07-26-12:40: human gestures carry the user move source. + expect(deps.moveTask).toHaveBeenCalledWith("FN-1", "in-progress", { moveSource: "user" }); expect(result.card.kind).toBe("task"); expect(result.task.column).toBe("in-progress"); }); @@ -99,7 +100,8 @@ describe("requestReview", () => { it("moves in-progress task to in-review", async () => { const deps = createDeps(makeTask({ column: "in-progress" })); const result = await requestReview({ taskId: "FN-1" }, deps as never); - expect(deps.moveTask).toHaveBeenCalledWith("FN-1", "in-review"); + // FNXC:GlassesAgentActions 2026-07-26-12:40: human gestures carry the user move source. + expect(deps.moveTask).toHaveBeenCalledWith("FN-1", "in-review", { moveSource: "user" }); expect(result.task.column).toBe("in-review"); }); diff --git a/plugins/fusion-plugin-even-realities-glasses/src/agent-actions.ts b/plugins/fusion-plugin-even-realities-glasses/src/agent-actions.ts index fdbaaf81d9..4da5108e72 100644 --- a/plugins/fusion-plugin-even-realities-glasses/src/agent-actions.ts +++ b/plugins/fusion-plugin-even-realities-glasses/src/agent-actions.ts @@ -294,7 +294,8 @@ export async function startWork(input: AgentActionInput, deps: AgentActionDeps): const startTarget = destination(startLanes, "wip"); if (!startTarget) conflict("start-work", task); // Intentional v1 limitation: plugin cannot import engine allocator, so moveTask runs without allocateWorktree. - await deps.taskStore.moveTask(taskId, startTarget); + /* FNXC:GlassesAgentActions 2026-07-30-12:40: human gesture — user move source, matching the dashboard move route. */ + await deps.taskStore.moveTask(taskId, startTarget, { moveSource: "user" }); return toResult(deps.taskStore, taskId); } @@ -315,7 +316,8 @@ export async function requestReview(input: AgentActionInput, deps: AgentActionDe } const reviewTarget = destination(reviewLanes, "review"); if (!reviewTarget) conflict("request-review", task); - await deps.taskStore.moveTask(taskId, reviewTarget); + /* FNXC:GlassesAgentActions 2026-07-30-12:40: human gesture — user source (see startWork). */ + await deps.taskStore.moveTask(taskId, reviewTarget, { moveSource: "user" }); return toResult(deps.taskStore, taskId); } @@ -332,7 +334,8 @@ export async function approvePlan(input: AgentActionInput, deps: AgentActionDeps } const approveTarget = destination(approveLanes, "hold"); if (!approveTarget) conflict("approve-plan", task); - await deps.taskStore.moveTask(taskId, approveTarget); + /* FNXC:GlassesAgentActions 2026-07-30-12:40: human gesture — user source (see startWork). */ + await deps.taskStore.moveTask(taskId, approveTarget, { moveSource: "user" }); await deps.taskStore.updateTask(taskId, { status: undefined }); return toResult(deps.taskStore, taskId); } @@ -372,6 +375,7 @@ export async function returnToAgent(input: AgentActionInput, deps: AgentActionDe status: null, assignedAgentId: null, }); + /* FNXC:GlassesAgentActions 2026-07-30-12:40: intentionally DEFAULT (engine) source: a user-source move to the hold lane parks the task userPaused, defeating the return-to-agent intent. */ await deps.taskStore.moveTask(taskId, returnTarget); return toResult(deps.taskStore, taskId); } @@ -447,6 +451,7 @@ export async function retryTask(input: AgentActionInput, deps: AgentActionDeps): recoveryRetryCount: null, nextRecoveryAt: null, }); + /* FNXC:GlassesAgentActions 2026-07-30-12:40: intentionally DEFAULT (engine) source: retry requeues for execution; a user source would userPaused-park the row. */ await deps.taskStore.moveTask(taskId, retryTarget); return toResult(deps.taskStore, taskId); } diff --git a/scripts/lib/getdatabase-allowlist.json b/scripts/lib/getdatabase-allowlist.json index 046955d4b9..079c592d10 100644 --- a/scripts/lib/getdatabase-allowlist.json +++ b/scripts/lib/getdatabase-allowlist.json @@ -1,3 +1,11 @@ { - "entries": [] + "entries": [ + { + "file": "packages/core/src/__tests__/plugin-task-store-gate.test.ts", + "line": 64, + "snippet": "expect(() => gated.getDatabase()).toThrow(", + "reason": "Negative-path assertion only: the plugin task-store gate must DENY the raw sync getDatabase handle for undeclared plugins; the invocation exists to prove it throws and never reaches a database.", + "allowlistedAt": "2026-07-26T19:10:00.000Z" + } + ] }