From fa31f1cad2c2c7d14ec0b2e2be7058d3c08d774f Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Thu, 19 Feb 2026 06:43:22 +0000 Subject: [PATCH] refactor(cli): reuse allowlist mutation flow in approvals CLI --- src/cli/exec-approvals-cli.test.ts | 36 +++++++++ src/cli/exec-approvals-cli.ts | 114 ++++++++++++++++++----------- 2 files changed, 106 insertions(+), 44 deletions(-) diff --git a/src/cli/exec-approvals-cli.test.ts b/src/cli/exec-approvals-cli.test.ts index cfff096a11..73d511da05 100644 --- a/src/cli/exec-approvals-cli.test.ts +++ b/src/cli/exec-approvals-cli.test.ts @@ -24,6 +24,10 @@ const localSnapshot = { file: { version: 1, agents: {} }, }; +function resetLocalSnapshot() { + localSnapshot.file = { version: 1, agents: {} }; +} + vi.mock("./gateway-rpc.js", () => ({ callGatewayFromCli: (method: string, opts: unknown, params?: unknown) => callGatewayFromCli(method, opts, params), @@ -64,6 +68,7 @@ describe("exec approvals CLI", () => { }; it("routes get command to local, gateway, and node modes", async () => { + resetLocalSnapshot(); resetRuntimeCapture(); callGatewayFromCli.mockClear(); @@ -91,6 +96,7 @@ describe("exec approvals CLI", () => { }); it("defaults allowlist add to wildcard agent", async () => { + resetLocalSnapshot(); resetRuntimeCapture(); callGatewayFromCli.mockClear(); @@ -116,4 +122,34 @@ describe("exec approvals CLI", () => { }), ); }); + + it("removes wildcard allowlist entry and prunes empty agent", async () => { + resetLocalSnapshot(); + localSnapshot.file = { + version: 1, + agents: { + "*": { + allowlist: [{ pattern: "/usr/bin/uname", lastUsedAt: Date.now() }], + }, + }, + }; + resetRuntimeCapture(); + callGatewayFromCli.mockClear(); + + const saveExecApprovals = vi.mocked(execApprovals.saveExecApprovals); + saveExecApprovals.mockClear(); + + const program = createProgram(); + await program.parseAsync(["approvals", "allowlist", "remove", "/usr/bin/uname"], { + from: "user", + }); + + expect(saveExecApprovals).toHaveBeenCalledWith( + expect.objectContaining({ + version: 1, + agents: undefined, + }), + ); + expect(runtimeErrors).toHaveLength(0); + }); }); diff --git a/src/cli/exec-approvals-cli.ts b/src/cli/exec-approvals-cli.ts index b09938a2a1..291617df74 100644 --- a/src/cli/exec-approvals-cli.ts +++ b/src/cli/exec-approvals-cli.ts @@ -292,6 +292,36 @@ async function loadWritableAllowlistAgent(opts: ExecApprovalsCliOpts): Promise<{ return { nodeId, source, targetLabel, baseHash, file, agentKey, agent, allowlistEntries }; } +type WritableAllowlistAgentContext = Awaited> & { + trimmedPattern: string; +}; + +async function runAllowlistMutation( + pattern: string, + opts: ExecApprovalsCliOpts, + mutate: (context: WritableAllowlistAgentContext) => boolean | Promise, +): Promise { + try { + const trimmedPattern = requireTrimmedNonEmpty(pattern, "Pattern required."); + const context = await loadWritableAllowlistAgent(opts); + const shouldSave = await mutate({ ...context, trimmedPattern }); + if (!shouldSave) { + return; + } + await saveSnapshotTargeted({ + opts, + source: context.source, + nodeId: context.nodeId, + file: context.file, + baseHash: context.baseHash, + targetLabel: context.targetLabel, + }); + } catch (err) { + defaultRuntime.error(formatCliError(err)); + defaultRuntime.exit(1); + } +} + export function registerExecApprovalsCli(program: Command) { const formatExample = (cmd: string, desc: string) => ` ${theme.command(cmd)}\n ${theme.muted(desc)}`; @@ -393,22 +423,20 @@ export function registerExecApprovalsCli(program: Command) { .option("--gateway", "Force gateway approvals", false) .option("--agent ", 'Agent id (defaults to "*")') .action(async (pattern: string, opts: ExecApprovalsCliOpts) => { - try { - const trimmed = requireTrimmedNonEmpty(pattern, "Pattern required."); - const { nodeId, source, targetLabel, baseHash, file, agentKey, agent, allowlistEntries } = - await loadWritableAllowlistAgent(opts); - if (allowlistEntries.some((entry) => normalizeAllowlistEntry(entry) === trimmed)) { - defaultRuntime.log("Already allowlisted."); - return; - } - allowlistEntries.push({ pattern: trimmed, lastUsedAt: Date.now() }); - agent.allowlist = allowlistEntries; - file.agents = { ...file.agents, [agentKey]: agent }; - await saveSnapshotTargeted({ opts, source, nodeId, file, baseHash, targetLabel }); - } catch (err) { - defaultRuntime.error(formatCliError(err)); - defaultRuntime.exit(1); - } + await runAllowlistMutation( + pattern, + opts, + ({ trimmedPattern, file, agent, agentKey, allowlistEntries }) => { + if (allowlistEntries.some((entry) => normalizeAllowlistEntry(entry) === trimmedPattern)) { + defaultRuntime.log("Already allowlisted."); + return false; + } + allowlistEntries.push({ pattern: trimmedPattern, lastUsedAt: Date.now() }); + agent.allowlist = allowlistEntries; + file.agents = { ...file.agents, [agentKey]: agent }; + return true; + }, + ); }); nodesCallOpts(allowlistAdd); @@ -419,34 +447,32 @@ export function registerExecApprovalsCli(program: Command) { .option("--gateway", "Force gateway approvals", false) .option("--agent ", 'Agent id (defaults to "*")') .action(async (pattern: string, opts: ExecApprovalsCliOpts) => { - try { - const trimmed = requireTrimmedNonEmpty(pattern, "Pattern required."); - const { nodeId, source, targetLabel, baseHash, file, agentKey, agent, allowlistEntries } = - await loadWritableAllowlistAgent(opts); - const nextEntries = allowlistEntries.filter( - (entry) => normalizeAllowlistEntry(entry) !== trimmed, - ); - if (nextEntries.length === allowlistEntries.length) { - defaultRuntime.log("Pattern not found."); - return; - } - if (nextEntries.length === 0) { - delete agent.allowlist; - } else { - agent.allowlist = nextEntries; - } - if (isEmptyAgent(agent)) { - const agents = { ...file.agents }; - delete agents[agentKey]; - file.agents = Object.keys(agents).length > 0 ? agents : undefined; - } else { - file.agents = { ...file.agents, [agentKey]: agent }; - } - await saveSnapshotTargeted({ opts, source, nodeId, file, baseHash, targetLabel }); - } catch (err) { - defaultRuntime.error(formatCliError(err)); - defaultRuntime.exit(1); - } + await runAllowlistMutation( + pattern, + opts, + ({ trimmedPattern, file, agent, agentKey, allowlistEntries }) => { + const nextEntries = allowlistEntries.filter( + (entry) => normalizeAllowlistEntry(entry) !== trimmedPattern, + ); + if (nextEntries.length === allowlistEntries.length) { + defaultRuntime.log("Pattern not found."); + return false; + } + if (nextEntries.length === 0) { + delete agent.allowlist; + } else { + agent.allowlist = nextEntries; + } + if (isEmptyAgent(agent)) { + const agents = { ...file.agents }; + delete agents[agentKey]; + file.agents = Object.keys(agents).length > 0 ? agents : undefined; + } else { + file.agents = { ...file.agents, [agentKey]: agent }; + } + return true; + }, + ); }); nodesCallOpts(allowlistRemove); }