diff --git a/src/agent/tools.ts b/src/agent/tools.ts index db3ed89f0..f1ceb50dc 100644 --- a/src/agent/tools.ts +++ b/src/agent/tools.ts @@ -743,6 +743,7 @@ export async function createAgentToolset( let mcpTrustStore: ProjectTrustStore = projectTrust ?? { trustedPluginPaths: [], trustedMcpFingerprints: [], + trustedGrantFingerprints: [], }; const untrustedLocalError = `Not trusted for this project (see ${SETTINGS_DIR_NAME}/trust.json)`; diff --git a/src/config.test.ts b/src/config.test.ts index 5565c0f17..aaa4050c0 100644 --- a/src/config.test.ts +++ b/src/config.test.ts @@ -363,7 +363,11 @@ describe("loadConfig", () => { filterMcpServersForConnect(servers, { source: "local", cwd: "/repo/without-trust-grant", - store: { trustedPluginPaths: [], trustedMcpFingerprints: [] }, + store: { + trustedPluginPaths: [], + trustedMcpFingerprints: [], + trustedGrantFingerprints: [], + }, }), ).resolves.toEqual([BUILTIN_EXA_MCP]); }); diff --git a/src/exec/runner.ts b/src/exec/runner.ts index 3ee04b08b..71b47d94e 100644 --- a/src/exec/runner.ts +++ b/src/exec/runner.ts @@ -568,6 +568,9 @@ export async function runExec(config: Config): Promise { onPersistNotice: (text) => { stderr.write(`${text}\n`); }, + onPendingProjectGrants: (text) => { + stderr.write(`${text}\n`); + }, interactive, skipPermissions: config.dangerouslySkipPermissions, auto: config.auto, diff --git a/src/permission/project-approvals-trust.test.ts b/src/permission/project-approvals-trust.test.ts new file mode 100644 index 000000000..cbcdce418 --- /dev/null +++ b/src/permission/project-approvals-trust.test.ts @@ -0,0 +1,308 @@ +import { describe, expect, test } from "bun:test"; +import { mkdir, mkdtemp, realpath, symlink, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { generateSessionId } from "../session/index.js"; +import { loadSeededApprovals } from "../session/runtime-assembly.js"; +import { runWithSubAgentIdentity } from "../subagent/identity-context.js"; +import { trustProjectGrants } from "../trust/project-trust.js"; +import { createPermissionGate } from "./gate.js"; +import type { Approval, PermissionRequest } from "./types.js"; +import { + formatPendingProjectApprovals, + loadPendingProjectApprovals, + loadProjectApprovals, + saveProjectApproval, +} from "./store.js"; + +const PLANTED = [ + { tool: "run_shell", pattern: "git push --force origin main" }, + { tool: "write_file", pattern: "*.ts" }, +] as const; + +async function plantProjectApprovals( + cwd: string, + entries: readonly unknown[] = PLANTED, +): Promise { + const dir = join(cwd, ".corbits"); + await mkdir(dir, { recursive: true }); + await writeFile( + join(dir, "permissions.json"), + JSON.stringify({ version: 1, approvals: entries }), + ); +} + +async function driveGate(cwd: string, sessionId: string, home: string) { + const asked: string[] = []; + const gate = createPermissionGate({ + cwd, + interactive: true, + skipPermissions: false, + reactorGated: false, + requestApproval: async (request: PermissionRequest) => { + asked.push(`${request.tool}:${request.subject}`); + return { allow: true }; + }, + approvals: await loadSeededApprovals(cwd, sessionId, home), + }); + return { gate, asked }; +} + +const PUSH = { + id: "push", + name: "run_shell", + arguments: { command: "git push --force origin main" }, +} as const; + +const WRITE = { + id: "write", + name: "write_file", + arguments: { path: "notes.ts" }, +} as const; + +describe("CL-7782: project approvals require grant trust", () => { + test("untrusted directory contributes zero project approvals through the real gate", async () => { + const base = await mkdtemp(join(tmpdir(), "cl-7782-untrusted-")); + const home = join(base, "home"); + const cwd = join(base, "repo"); + await plantProjectApprovals(cwd); + const sessionId = generateSessionId(); + + expect(await loadProjectApprovals(cwd, home)).toEqual([]); + const seeded = await loadSeededApprovals(cwd, sessionId, home); + for (const entry of PLANTED) { + expect( + seeded.some( + (approval) => + approval.tool === entry.tool && approval.pattern === entry.pattern, + ), + ).toBe(false); + } + + const { gate, asked } = await driveGate(cwd, sessionId, home); + expect((await gate.evaluate({ ...PUSH })).allowed).toBe(true); + expect((await gate.evaluate({ ...WRITE })).allowed).toBe(true); + expect(asked).toEqual([ + "run_shell:git push --force origin main", + "write_file:notes.ts", + ]); + }); + + test("trusted and confirmed directory grants apply without asking", async () => { + const base = await mkdtemp(join(tmpdir(), "cl-7782-trusted-")); + const home = join(base, "home"); + const cwd = join(base, "repo"); + await plantProjectApprovals(cwd); + await trustProjectGrants(cwd, [...PLANTED], home); + const sessionId = generateSessionId(); + + expect(await loadProjectApprovals(cwd, home)).toHaveLength(2); + + const { gate, asked } = await driveGate(cwd, sessionId, home); + expect((await gate.evaluate({ ...PUSH })).allowed).toBe(true); + expect((await gate.evaluate({ ...WRITE })).allowed).toBe(true); + expect(asked).toEqual([]); + }); + + test("trust is keyed by realpath: a symlinked checkout cannot inherit or confer", async () => { + const base = await mkdtemp(join(tmpdir(), "cl-7782-link-")); + const home = join(base, "home"); + const target = join(base, "real-repo"); + await plantProjectApprovals(target); + const link = join(base, "linked-repo"); + await symlink(target, link); + expect(await realpath(link)).toBe(await realpath(target)); + + await trustProjectGrants(link, [...PLANTED], home); + expect(await loadProjectApprovals(target, home)).toHaveLength(2); + + const twin = join(base, "twin-repo"); + await plantProjectApprovals(twin); + expect(await loadProjectApprovals(twin, home)).toEqual([]); + }); + + test("first encounter surfaces the would-be grants instead of dropping them silently", async () => { + const base = await mkdtemp(join(tmpdir(), "cl-7782-pending-")); + const home = join(base, "home"); + const cwd = join(base, "repo"); + await plantProjectApprovals(cwd); + + const pending = await loadPendingProjectApprovals(cwd, home); + expect(pending).toHaveLength(2); + const notice = formatPendingProjectApprovals(pending); + for (const entry of PLANTED) { + expect(notice).toContain(entry.pattern); + } + + await trustProjectGrants(cwd, [...PLANTED], home); + expect(await loadPendingProjectApprovals(cwd, home)).toEqual([]); + expect(formatPendingProjectApprovals([])).toBe(""); + }); + + test("hand-removing an entry revokes its confirmation: a byte-identical replant re-surfaces", async () => { + const base = await mkdtemp(join(tmpdir(), "cl-7782-replant-")); + const home = join(base, "home"); + const cwd = join(base, "repo"); + await plantProjectApprovals(cwd); + await trustProjectGrants(cwd, [...PLANTED], home); + expect(await loadProjectApprovals(cwd, home)).toHaveLength(2); + + // Hand-edit the first entry out of the file without removeProjectApproval. + await plantProjectApprovals(cwd, [PLANTED[1]]); + expect(await loadProjectApprovals(cwd, home)).toEqual([{ ...PLANTED[1] }]); + expect(await loadPendingProjectApprovals(cwd, home)).toEqual([]); + + // Replant the identical bytes: the removed entry surfaces, not applies. + await plantProjectApprovals(cwd); + expect(await loadProjectApprovals(cwd, home)).toEqual([{ ...PLANTED[1] }]); + expect(await loadPendingProjectApprovals(cwd, home)).toEqual([ + { ...PLANTED[0] }, + ]); + }); + + test("a gate-minted project grant ({tool, pattern, cwd}) survives save → reload and still applies", async () => { + const base = await mkdtemp(join(tmpdir(), "cl-7782-cwd-")); + const home = join(base, "home"); + const cwd = join(base, "repo"); + + // Mint through the real gate so the entry has the production shape. + const minted: Approval[] = []; + const mintGate = createPermissionGate({ + cwd, + interactive: true, + skipPermissions: false, + reactorGated: false, + requestApproval: async () => ({ + allow: true, + persist: { + id: "exact", + label: "Always allow", + pattern: "npm test", + grant: "project", + }, + }), + persist: (approval, scope) => { + expect(scope).toBe("project"); + minted.push(approval); + }, + approvals: await loadSeededApprovals(cwd, generateSessionId(), home), + }); + const NPM_TEST = { + id: "npm-test", + name: "run_shell", + arguments: { command: "npm test" }, + } as const; + expect((await mintGate.evaluate({ ...NPM_TEST })).allowed).toBe(true); + expect(minted).toEqual([{ tool: "run_shell", pattern: "npm test", cwd }]); + + // Production persist path, then reload: the cwd must round-trip, not strip. + const grant = minted[0]; + if (grant === undefined) throw new Error("gate minted no project grant"); + await saveProjectApproval(cwd, grant, home); + const reloaded = await loadProjectApprovals(cwd, home); + expect(reloaded).toEqual(minted); + + // The reloaded entry still applies: a fresh seeded gate asks nothing. + const { gate, asked } = await driveGate(cwd, generateSessionId(), home); + expect((await gate.evaluate({ ...NPM_TEST })).allowed).toBe(true); + expect(asked).toEqual([]); + }); + + test("confirming a planted entry through the pending flow converges the file to the minted shape", async () => { + const base = await mkdtemp(join(tmpdir(), "cl-7782-converge-")); + const home = join(base, "home"); + const cwd = join(base, "repo"); + await plantProjectApprovals(cwd, [ + { tool: "run_shell", pattern: "npm test" }, + ]); + expect(await loadPendingProjectApprovals(cwd, home)).toEqual([ + { tool: "run_shell", pattern: "npm test" }, + ]); + + // What the gate persist does when the operator confirms the pending entry + // with a project-scope persist: mint {tool, pattern, cwd} and write it. + await saveProjectApproval( + cwd, + { tool: "run_shell", pattern: "npm test", cwd }, + home, + ); + + // The planted twin is displaced by the minted shape — nothing lingers as + // pending, and the grant applies without asking. + expect(await loadProjectApprovals(cwd, home)).toEqual([ + { tool: "run_shell", pattern: "npm test", cwd }, + ]); + expect(await loadPendingProjectApprovals(cwd, home)).toEqual([]); + + const { gate, asked } = await driveGate(cwd, generateSessionId(), home); + expect( + ( + await gate.evaluate({ + id: "npm-test", + name: "run_shell", + arguments: { command: "npm test" }, + }) + ).allowed, + ).toBe(true); + expect(asked).toEqual([]); + }); + + test("stripping cwd from a confirmed entry re-surfaces as pending and never cross-repo auto-allows", async () => { + const base = await mkdtemp(join(tmpdir(), "cl-7782-cwd-strip-")); + const home = join(base, "home"); + const cwd = join(base, "repo"); + const other = join(base, "other"); + await mkdir(other, { recursive: true }); + + // Operator confirms {tool, pattern, cwd} through the production path. + await saveProjectApproval( + cwd, + { tool: "run_shell", pattern: "npm test", cwd }, + home, + ); + expect(await loadProjectApprovals(cwd, home)).toEqual([ + { tool: "run_shell", pattern: "npm test", cwd }, + ]); + + // Hand-edit drops the cwd key: byte-identical to a planted entry, but the + // confirmation was bound to the cwd-bearing shape, so trust must not + // follow the stripped bytes. + await plantProjectApprovals(cwd, [ + { tool: "run_shell", pattern: "npm test" }, + ]); + expect(await loadProjectApprovals(cwd, home)).toEqual([]); + expect(await loadPendingProjectApprovals(cwd, home)).toEqual([ + { tool: "run_shell", pattern: "npm test" }, + ]); + + // The real gate, seeded after the strip: neither the same-repo request + // nor a cross-repo request (different request cwd) auto-allows. + const asked: string[] = []; + const gate = createPermissionGate({ + cwd, + interactive: true, + skipPermissions: false, + reactorGated: false, + requestApproval: async (request: PermissionRequest) => { + asked.push(`${request.tool}:${request.subject}`); + return { allow: false }; + }, + approvals: await loadSeededApprovals(cwd, generateSessionId(), home), + }); + const NPM_TEST = { + id: "npm-test", + name: "run_shell", + arguments: { command: "npm test" }, + } as const; + expect((await gate.evaluate({ ...NPM_TEST })).allowed).toBe(false); + expect( + ( + await runWithSubAgentIdentity( + { description: "other", cwd: other }, + () => gate.evaluate({ ...NPM_TEST }), + ) + ).allowed, + ).toBe(false); + expect(asked).toEqual(["run_shell:npm test", "run_shell:npm test"]); + }); +}); diff --git a/src/permission/store.test.ts b/src/permission/store.test.ts index ac5d40ddf..975dd1625 100644 --- a/src/permission/store.test.ts +++ b/src/permission/store.test.ts @@ -4,6 +4,7 @@ import { tmpdir } from "node:os"; import { join } from "node:path"; import { loadGlobalApprovals, + loadPendingProjectApprovals, loadProjectApprovals, loadProviderModelApprovals, saveGlobalApproval, @@ -15,44 +16,71 @@ import { } from "./store.js"; let dir: string; +let home: string; beforeEach(async () => { dir = await mkdtemp(join(tmpdir(), "perm-store-")); + home = await mkdtemp(join(tmpdir(), "perm-store-home-")); }); afterEach(async () => { await rm(dir, { recursive: true, force: true }); + await rm(home, { recursive: true, force: true }); }); describe("project store", () => { test("round-trips approvals at /.corbits/permissions.json", async () => { - expect(await loadProjectApprovals(dir)).toEqual([]); - await saveProjectApproval(dir, { tool: "run_shell", pattern: "npm *" }); - await saveProjectApproval(dir, { tool: "write_file", pattern: "src/*" }); - expect(await loadProjectApprovals(dir)).toEqual([ + expect(await loadProjectApprovals(dir, home)).toEqual([]); + await saveProjectApproval( + dir, + { tool: "run_shell", pattern: "npm *" }, + home, + ); + await saveProjectApproval( + dir, + { tool: "write_file", pattern: "src/*" }, + home, + ); + expect(await loadProjectApprovals(dir, home)).toEqual([ { tool: "run_shell", pattern: "npm *" }, { tool: "write_file", pattern: "src/*" }, ]); }); test("drops an over-broad pattern at load so a poisoned file cannot over-grant", async () => { - await saveProjectApproval(dir, { tool: "run_shell", pattern: "*" }); - await saveProjectApproval(dir, { tool: "run_shell", pattern: "npm *" }); - expect(await loadProjectApprovals(dir)).toEqual([ + await saveProjectApproval(dir, { tool: "run_shell", pattern: "*" }, home); + await saveProjectApproval( + dir, + { tool: "run_shell", pattern: "npm *" }, + home, + ); + expect(await loadProjectApprovals(dir, home)).toEqual([ { tool: "run_shell", pattern: "npm *" }, ]); }); test("removeProjectApproval drops only the matching entry", async () => { - await saveProjectApproval(dir, { tool: "run_shell", pattern: "npm *" }); - await saveProjectApproval(dir, { tool: "write_file", pattern: "src/*" }); - await removeProjectApproval(dir, { tool: "run_shell", pattern: "npm *" }); - expect(await loadProjectApprovals(dir)).toEqual([ + await saveProjectApproval( + dir, + { tool: "run_shell", pattern: "npm *" }, + home, + ); + await saveProjectApproval( + dir, + { tool: "write_file", pattern: "src/*" }, + home, + ); + await removeProjectApproval( + dir, + { tool: "run_shell", pattern: "npm *" }, + home, + ); + expect(await loadProjectApprovals(dir, home)).toEqual([ { tool: "write_file", pattern: "src/*" }, ]); }); - test("drops a hand-edited entry missing a required field, keeping valid entries", async () => { + test("an unconfirmed hand-written file contributes zero approvals but stays visible as pending", async () => { const settingsDir = join(dir, ".corbits"); await mkdir(settingsDir, { recursive: true }); await writeFile( @@ -65,7 +93,8 @@ describe("project store", () => { ], }), ); - expect(await loadProjectApprovals(dir)).toEqual([ + expect(await loadProjectApprovals(dir, home)).toEqual([]); + expect(await loadPendingProjectApprovals(dir, home)).toEqual([ { tool: "run_shell", pattern: "npm *" }, ]); }); diff --git a/src/permission/store.ts b/src/permission/store.ts index d828d98aa..e052d4c3d 100644 --- a/src/permission/store.ts +++ b/src/permission/store.ts @@ -8,6 +8,12 @@ import { type } from "arktype"; import type { Approval } from "./types.js"; import { sessionDir } from "../session/index.js"; import { SETTINGS_DIR_NAME } from "../branding.js"; +import { + isProjectGrantTrusted, + reconcileProjectGrants, + trustProjectGrants, + untrustProjectGrants, +} from "../trust/project-trust.js"; // Approvals are remembered per session, alongside the run state. function storePath(cwd: string, sessionId: string, home?: string): string { @@ -62,6 +68,20 @@ function parseApprovalList(raw: unknown): Approval[] { } function sameApproval(a: Approval, b: Approval): boolean { + // Removal equality deliberately ignores cwd: revocation targets arrive + // cwd-less (see admin.ts toApproval), so a strict comparison would silently + // keep a confined twin live. Removing the file entry is the revocation; + // the next load's reconcile prunes the cwd-bound fingerprint with it. + return ( + a.tool === b.tool && + a.pattern === b.pattern && + a.providerModel === b.providerModel + ); +} + +// Equality on every confirmed dimension except cwd: a planted file entry and +// the gate's minted confirmation of it differ only in cwd. +function sameGrantModuloCwd(a: Approval, b: Approval): boolean { return ( a.tool === b.tool && a.pattern === b.pattern && @@ -124,30 +144,100 @@ export async function loadApprovals( return readApprovalsField(storePath(cwd, sessionId, home), "approvals"); } -export async function loadProjectApprovals(cwd: string): Promise { - return readApprovalsField(projectStorePath(cwd), "approvals"); +// CL-7782: the project approvals file is repo content — committable, +// copyable, plantable — so its entries are NOT approvals until the operator +// confirms each one (see trustedGrantFingerprints in ../trust/project-trust). +// An untrusted directory therefore contributes zero approvals here; entries +// still awaiting confirmation are visible via loadPendingProjectApprovals so +// the first encounter shows what the file would grant instead of silently +// dropping it. DECISION: project trust does not imply grant trust — plugins +// and MCP servers trusted for a directory confer no approval coverage; grants +// require their own confirmation. This is the safer default because a grant +// auto-allows future tool calls with no further prompt, while plugin/MCP +// trust only permits code to load or a server to connect. +// REVOCATION: trust follows the file. Each load reconciles the trust record +// against the entries currently on disk and drops fingerprints with no +// corresponding entry, so hand-removing an entry revokes its confirmation +// just like removeProjectApproval does — a byte-identical replant re-surfaces +// as pending instead of applying silently. +export async function loadProjectApprovals( + cwd: string, + home?: string, +): Promise { + const onDisk = await readApprovalsField(projectStorePath(cwd), "approvals"); + const trust = await reconcileProjectGrants(cwd, onDisk, home); + return onDisk.filter((approval) => isProjectGrantTrusted(trust, approval)); +} + +/** + * Project-file entries the operator has not confirmed yet: the first-encounter + * surfacing source. Non-empty means "this directory ships a permissions file + * you have not reviewed" — callers should show formatPendingProjectApprovals + * output to the operator rather than apply or silently ignore the file. + */ +export async function loadPendingProjectApprovals( + cwd: string, + home?: string, +): Promise { + const onDisk = await readApprovalsField(projectStorePath(cwd), "approvals"); + if (onDisk.length === 0) return []; + const trust = await reconcileProjectGrants(cwd, onDisk, home); + return onDisk.filter((approval) => !isProjectGrantTrusted(trust, approval)); +} + +/** Operator-facing rendering of unconfirmed project-file entries. */ +export function formatPendingProjectApprovals(pending: Approval[]): string { + if (pending.length === 0) return ""; + const lines = pending.map( + (approval) => + ` - ${approval.tool}: "${approval.pattern}"${approval.providerModel ? ` (only with ${approval.providerModel})` : ""}`, + ); + return [ + "This directory contains a project approvals file with entries you have not confirmed:", + ...lines, + "Nothing from this file is applied until you confirm each entry.", + ].join("\n"); } export async function saveProjectApproval( cwd: string, approval: Approval, + home?: string, ): Promise { - return chainObjectWrite(projectStorePath(cwd), (current) => ({ + await chainObjectWrite(projectStorePath(cwd), (current) => ({ ...current, - approvals: [...parseApprovalList(current.approvals), approval], + approvals: [ + // A planted entry carries no cwd; confirming it through the pending flow + // mints {tool, pattern, cwd} and writes that shape back here. Displace + // its twin instead of stacking a duplicate that would linger as pending + // forever — the dropped twin never applied, so nothing confirmed is + // lost. A save without cwd keeps the plain append path and never + // displaces a confined entry. + ...parseApprovalList(current.approvals).filter( + (entry) => + approval.cwd === undefined || !sameGrantModuloCwd(entry, approval), + ), + approval, + ], })); + // The only production writer is the interactive grant path (an operator + // answering a prompt with a project-scope persist), so writing an entry is + // itself the confirmation its fingerprint needs. + await trustProjectGrants(cwd, [approval], home); } export async function removeProjectApproval( cwd: string, target: Approval, + home?: string, ): Promise { - return chainObjectWrite(projectStorePath(cwd), (current) => ({ + await chainObjectWrite(projectStorePath(cwd), (current) => ({ ...current, approvals: parseApprovalList(current.approvals).filter( (a) => !sameApproval(a, target), ), })); + await untrustProjectGrants(cwd, [target], home); } export async function loadGlobalApprovals( diff --git a/src/session/assemble-runtime.ts b/src/session/assemble-runtime.ts index 3b00855ff..c96faf507 100644 --- a/src/session/assemble-runtime.ts +++ b/src/session/assemble-runtime.ts @@ -243,6 +243,12 @@ export interface SessionGateArgs { /** Read at persist time so a live model switch stores under the pair in use. */ getActiveProviderModel: () => string; onPersistNotice?: ((text: string) => void) | undefined; + /** + * First encounter with an unconfirmed project approvals file: the runners + * surface what the file would grant (exec: stderr, TUI: persist notice). + * Entries stay gated regardless of delivery. + */ + onPendingProjectGrants?: ((text: string) => void) | undefined; interactive: boolean; skipPermissions: boolean; auto?: boolean | undefined; @@ -265,7 +271,12 @@ export interface SessionGate { export async function assembleSessionGate( args: SessionGateArgs, ): Promise { - const seededApprovals = await loadSeededApprovals(args.cwd, args.sessionId); + const seededApprovals = await loadSeededApprovals( + args.cwd, + args.sessionId, + undefined, + { onPendingProjectGrants: args.onPendingProjectGrants }, + ); const gate = createPermissionGate({ approvals: seededApprovals, telemetry: args.telemetry, diff --git a/src/session/runtime-assembly.test.ts b/src/session/runtime-assembly.test.ts index 9f6f661c0..ad97c2046 100644 --- a/src/session/runtime-assembly.test.ts +++ b/src/session/runtime-assembly.test.ts @@ -165,10 +165,14 @@ describe("loadSeededApprovals merge order", () => { approvals: [{ tool: "run_shell", pattern: "session npm *" }], }), ); - await permissionStore.saveProjectApproval(cwd, { - tool: "run_shell", - pattern: "project npm *", - }); + await permissionStore.saveProjectApproval( + cwd, + { + tool: "run_shell", + pattern: "project npm *", + }, + home, + ); const seeded = await loadSeededApprovals(cwd, sessionId, home); diff --git a/src/session/runtime-assembly.ts b/src/session/runtime-assembly.ts index 31b358875..c1f1ad078 100644 --- a/src/session/runtime-assembly.ts +++ b/src/session/runtime-assembly.ts @@ -37,8 +37,10 @@ import { } from "../plugins/loader.js"; import { isPluginModuleEnabled } from "../plugins/register.js"; import { + formatPendingProjectApprovals, loadApprovals, loadGlobalApprovals, + loadPendingProjectApprovals, loadProjectApprovals, loadProviderModelApprovals, saveGlobalApproval, @@ -128,14 +130,32 @@ export async function loadSeededApprovals( cwd: string, sessionId: string, home?: string, + opts?: { onPendingProjectGrants?: ((text: string) => void) | undefined }, ): Promise { const sessionApprovals = await loadApprovals(cwd, sessionId, home); - const [projectApprovals, globalApprovals, providerModelApprovals] = - await Promise.all([ - loadProjectApprovals(cwd), - loadGlobalApprovals(), - loadProviderModelApprovals(), - ]); + const [ + projectApprovals, + globalApprovals, + providerModelApprovals, + pendingProjectApprovals, + ] = await Promise.all([ + loadProjectApprovals(cwd, home), + loadGlobalApprovals(), + loadProviderModelApprovals(), + loadPendingProjectApprovals(cwd, home), + ]); + // First encounter with an unconfirmed project approvals file: show the + // operator what it would grant. The entries stay out of the seeded set + // (loadProjectApprovals gates them) — this notice is the only trace. + const pendingNotice = formatPendingProjectApprovals(pendingProjectApprovals); + if (pendingNotice !== "") { + persistLogger.warn("Unconfirmed project approvals in {cwd}", { cwd }); + try { + opts?.onPendingProjectGrants?.(pendingNotice); + } catch { + // Notice delivery is best-effort; the grants stay gated regardless. + } + } return [ ...sessionApprovals, ...projectApprovals, diff --git a/src/trust/project-trust.test.ts b/src/trust/project-trust.test.ts index 290e80779..0c95088ac 100644 --- a/src/trust/project-trust.test.ts +++ b/src/trust/project-trust.test.ts @@ -52,14 +52,22 @@ describe("project trust store", () => { filterMcpServersForConnect(servers, { source: "global", cwd: "/repo/under/test", - store: { trustedPluginPaths: [], trustedMcpFingerprints: [] }, + store: { + trustedPluginPaths: [], + trustedMcpFingerprints: [], + trustedGrantFingerprints: [], + }, }), ).resolves.toEqual(servers); await expect( filterMcpServersForConnect(servers, { source: "local", cwd: "/repo/under/test", - store: { trustedPluginPaths: [], trustedMcpFingerprints: [] }, + store: { + trustedPluginPaths: [], + trustedMcpFingerprints: [], + trustedGrantFingerprints: [], + }, }), ).resolves.toEqual([]); }); @@ -228,6 +236,7 @@ describe("project trust store", () => { expect(result.store).toEqual({ trustedPluginPaths: [], trustedMcpFingerprints: [], + trustedGrantFingerprints: [], }); }); }); diff --git a/src/trust/project-trust.ts b/src/trust/project-trust.ts index 97ac52502..3afe52927 100644 --- a/src/trust/project-trust.ts +++ b/src/trust/project-trust.ts @@ -18,6 +18,7 @@ const logger = getLogger([LOG_NAMESPACE_ROOT, "trust"]); const ProjectTrustRecordSchema = type({ "trustedPluginPaths?": "unknown[]", "trustedMcpFingerprints?": "unknown[]", + "trustedGrantFingerprints?": "unknown[]", "repo?": "string", }); @@ -34,11 +35,18 @@ export interface ProjectTrustStore { trustedPluginPaths: string[]; /** MCP fingerprints (see mcpServerFingerprint) trusted for this project. */ trustedMcpFingerprints: string[]; + /** + * Grant fingerprints (see projectGrantFingerprint) confirmed for this + * project's approvals file. Trusting the project never implies trusting its + * grants: each entry requires its own operator confirmation (CL-7782). + */ + trustedGrantFingerprints: string[]; } const emptyStore = (): ProjectTrustStore => ({ trustedPluginPaths: [], trustedMcpFingerprints: [], + trustedGrantFingerprints: [], }); /** @@ -156,6 +164,11 @@ export async function readProjectTrustStore( "trustedMcpFingerprints", path, ); + const trustedGrantFingerprints = extractStringArrayField( + validated.trustedGrantFingerprints, + "trustedGrantFingerprints", + path, + ); // Guard against a stale/copied record keyed to a different repo path: the // file records the repo it was written for and must match this cwd. A // missing or non-string `repo` is invalid too — without it, a hand-edited @@ -186,6 +199,7 @@ export async function readProjectTrustStore( store: { trustedPluginPaths: absolutePluginPaths, trustedMcpFingerprints: [...trustedMcpFingerprints], + trustedGrantFingerprints: [...trustedGrantFingerprints], }, }; } @@ -307,6 +321,139 @@ export async function trustMcpServer( }); } +/** + * Stable fingerprint for one project-approval entry: tool + pattern, with the + * provider-model binding folded in when set, so switching models invalidates a + * prior confirmation exactly the way the gate's providerModel check does. + * The entry's cwd is folded in too (absent → ""): Approval has four enforced + * dimensions and a cwd-less grant matches any request cwd (see + * cwdMatchesGrant), so a fingerprint that ignored cwd would let a hand-edit + * dropping `cwd` from a confirmed entry keep its confirmation and silently + * widen a repo-confined grant to cross-repo. A cwd-less planted entry still + * fingerprints the same way at trust and load time, so confirming it through + * the pending flow matches the minted shape (saveProjectApproval converges + * the file to that shape on write). + */ +export function projectGrantFingerprint(approval: { + tool: string; + pattern: string; + providerModel?: string; + cwd?: string; +}): string { + const payload = JSON.stringify({ + tool: approval.tool, + pattern: approval.pattern, + providerModel: approval.providerModel ?? "", + cwd: approval.cwd ?? "", + }); + return createHash("sha256").update(payload).digest("hex"); +} + +export function isProjectGrantTrusted( + store: ProjectTrustStore, + approval: { + tool: string; + pattern: string; + providerModel?: string; + cwd?: string; + }, +): boolean { + return store.trustedGrantFingerprints.includes( + projectGrantFingerprint(approval), + ); +} + +/** + * Record the operator's confirmation of project-approval entries: trusting the + * project (plugins, MCP) never implies trusting its grants — these + * fingerprints are only written when the operator persists a grant to the + * project scope (the interactive grant path whose saveProjectApproval write is + * itself the confirmation), never by the mere existence of the file. A planted + * entry becomes trusted the next time the operator answers its per-call prompt + * with a project-scope persist; there is no separate first-encounter review + * writer. + */ +export async function trustProjectGrants( + cwd: string, + approvals: { + tool: string; + pattern: string; + providerModel?: string; + cwd?: string; + }[], + home: string = homedir(), +): Promise { + const fps = approvals.map(projectGrantFingerprint); + return enqueueMutation(projectTrustPath(cwd, home), async () => { + const store = await loadProjectTrust(cwd, home); + const missing = fps.filter( + (fp) => !store.trustedGrantFingerprints.includes(fp), + ); + if (missing.length > 0) { + store.trustedGrantFingerprints = [ + ...store.trustedGrantFingerprints, + ...missing, + ]; + await saveProjectTrust(cwd, store, home); + } + return store; + }); +} + +/** Drop confirmations for removed entries so a replanted file re-surfaces. */ +export async function untrustProjectGrants( + cwd: string, + approvals: { + tool: string; + pattern: string; + providerModel?: string; + cwd?: string; + }[], + home: string = homedir(), +): Promise { + const fps = new Set(approvals.map(projectGrantFingerprint)); + return enqueueMutation(projectTrustPath(cwd, home), async () => { + const store = await loadProjectTrust(cwd, home); + const kept = store.trustedGrantFingerprints.filter((fp) => !fps.has(fp)); + if (kept.length !== store.trustedGrantFingerprints.length) { + store.trustedGrantFingerprints = kept; + await saveProjectTrust(cwd, store, home); + } + return store; + }); +} + +/** + * Revocation by absence: drop trusted fingerprints with no corresponding + * on-disk entry. untrustProjectGrants only runs on the removeProjectApproval + * path, so a hand-edit that deletes an entry from the file would otherwise + * leave its fingerprint trusted and a byte-identical replant would apply + * silently. Loaders reconcile first, so trust follows the file: removing an + * entry revokes its confirmation whether or not the removal went through the + * store, and replanting it re-surfaces as pending. + */ +export async function reconcileProjectGrants( + cwd: string, + onDisk: { + tool: string; + pattern: string; + providerModel?: string; + cwd?: string; + }[], + home: string = homedir(), +): Promise { + const live = new Set(onDisk.map(projectGrantFingerprint)); + return enqueueMutation(projectTrustPath(cwd, home), async () => { + const store = await loadProjectTrust(cwd, home); + const kept = store.trustedGrantFingerprints.filter((fp) => live.has(fp)); + if (kept.length !== store.trustedGrantFingerprints.length) { + store.trustedGrantFingerprints = kept; + await saveProjectTrust(cwd, store, home); + } + return store; + }); +} + /** * Filter MCP servers that may connect. Global-source servers are always allowed. * Local-source servers require a trust fingerprint (or an interactive grant callback). diff --git a/src/tui/runner/session.ts b/src/tui/runner/session.ts index 56803aa52..82e63501d 100644 --- a/src/tui/runner/session.ts +++ b/src/tui/runner/session.ts @@ -189,6 +189,8 @@ export async function assembleTUISession( getActiveProviderModel: () => `${state.config.providerName}:${state.config.model}`, onPersistNotice: (text) => state.approvalPersistNotice.notify?.(text), + onPendingProjectGrants: (text) => + state.approvalPersistNotice.notify?.(text), interactive: true, skipPermissions: config.dangerouslySkipPermissions, auto: config.auto, diff --git a/tests/unit/project-trust.test.ts b/tests/unit/project-trust.test.ts index 1586cc59f..a4d121d4d 100644 --- a/tests/unit/project-trust.test.ts +++ b/tests/unit/project-trust.test.ts @@ -137,6 +137,7 @@ describe("project-trust", () => { expect(result.store).toEqual({ trustedPluginPaths: [], trustedMcpFingerprints: [], + trustedGrantFingerprints: [], }); } finally { await cleanup(); @@ -154,6 +155,7 @@ describe("project-trust", () => { expect(result.store).toEqual({ trustedPluginPaths: [], trustedMcpFingerprints: [], + trustedGrantFingerprints: [], }); } finally { await cleanup(); @@ -171,6 +173,7 @@ describe("project-trust", () => { repo: cwd, trustedPluginPaths: "nope", trustedMcpFingerprints: [], + trustedGrantFingerprints: [], }), "utf8", ); @@ -179,6 +182,7 @@ describe("project-trust", () => { expect(result.store).toEqual({ trustedPluginPaths: [], trustedMcpFingerprints: [], + trustedGrantFingerprints: [], }); } finally { await cleanup(); @@ -196,6 +200,7 @@ describe("project-trust", () => { expect(result.store).toEqual({ trustedPluginPaths: [], trustedMcpFingerprints: [], + trustedGrantFingerprints: [], }); } finally { await cleanup(); @@ -213,6 +218,7 @@ describe("project-trust", () => { repo: 7, trustedPluginPaths: [], trustedMcpFingerprints: [], + trustedGrantFingerprints: [], }), "utf8", ); @@ -221,6 +227,7 @@ describe("project-trust", () => { expect(result.store).toEqual({ trustedPluginPaths: [], trustedMcpFingerprints: [], + trustedGrantFingerprints: [], }); } finally { await cleanup(); diff --git a/tests/unit/tui/agent-tools.test.ts b/tests/unit/tui/agent-tools.test.ts index 64a436c26..61028f156 100644 --- a/tests/unit/tui/agent-tools.test.ts +++ b/tests/unit/tui/agent-tools.test.ts @@ -511,7 +511,11 @@ test("late connect of an untrusted local-source server does not spawn", async () onOperatorGate: async () => ({ kind: "cancel" }), mcpServers: [localStdioServer], mcpServersSource: "local", - projectTrust: { trustedPluginPaths: [], trustedMcpFingerprints: [] }, + projectTrust: { + trustedPluginPaths: [], + trustedMcpFingerprints: [], + trustedGrantFingerprints: [], + }, }); await toolset.connectMCPServer(localStdioServer, { @@ -537,7 +541,11 @@ test("late connect of an untrusted local-source server fail-closes when requestM onOperatorGate: async () => ({ kind: "cancel" }), mcpServers: [localStdioServer], mcpServersSource: "local", - projectTrust: { trustedPluginPaths: [], trustedMcpFingerprints: [] }, + projectTrust: { + trustedPluginPaths: [], + trustedMcpFingerprints: [], + trustedGrantFingerprints: [], + }, requestMcpTrust: async () => { trustAsks += 1; return false; @@ -566,6 +574,7 @@ test("late connect of a trusted local-source server still connects", async () => projectTrust: { trustedPluginPaths: [], trustedMcpFingerprints: [mcpServerFingerprint(localStdioServer)], + trustedGrantFingerprints: [], }, }); @@ -588,7 +597,11 @@ test("late connect of a global-source HTTP server does not require trust", async onOperatorGate: async () => ({ kind: "cancel" }), mcpServers: [globalHttpServer], mcpServersSource: "global", - projectTrust: { trustedPluginPaths: [], trustedMcpFingerprints: [] }, + projectTrust: { + trustedPluginPaths: [], + trustedMcpFingerprints: [], + trustedGrantFingerprints: [], + }, }); await toolset.connectMCPServer(globalHttpServer, { @@ -611,7 +624,11 @@ test("startup connectMCP still fail-closes untrusted local servers", async () => onOperatorGate: async () => ({ kind: "cancel" }), mcpServers: [localStdioServer], mcpServersSource: "local", - projectTrust: { trustedPluginPaths: [], trustedMcpFingerprints: [] }, + projectTrust: { + trustedPluginPaths: [], + trustedMcpFingerprints: [], + trustedGrantFingerprints: [], + }, }); await toolset.connectMCP({