From d19d0d4b27055dd3795c0a060589b93c4a6a5c4c Mon Sep 17 00:00:00 2001 From: Alex Nork Date: Tue, 12 May 2026 11:13:53 -0400 Subject: [PATCH 1/2] feat(security): enforce credential allowedTools policy before proxied bash sessions --- .../__tests__/shell-credential-ref.test.ts | 98 ++++++++++++++++++- assistant/src/tools/terminal/shell.ts | 44 +++++++++ 2 files changed, 139 insertions(+), 3 deletions(-) diff --git a/assistant/src/__tests__/shell-credential-ref.test.ts b/assistant/src/__tests__/shell-credential-ref.test.ts index fd69b4c72c2..c451f03561e 100644 --- a/assistant/src/__tests__/shell-credential-ref.test.ts +++ b/assistant/src/__tests__/shell-credential-ref.test.ts @@ -93,6 +93,7 @@ afterAll(() => { describe("shell tool credential ref resolution", () => { test("service/field ref resolves to UUID and reaches session creation", async () => { const meta = upsertCredentialMetadata("fal", "api_key", { + allowedTools: ["bash"], injectionTemplates: [ { hostPattern: "*.fal.ai", @@ -120,7 +121,9 @@ describe("shell tool credential ref resolution", () => { }); test("UUID ref remains supported", async () => { - const meta = upsertCredentialMetadata("github", "token"); + const meta = upsertCredentialMetadata("github", "token", { + allowedTools: ["bash"], + }); await shellTool.execute( { @@ -156,7 +159,9 @@ describe("shell tool credential ref resolution", () => { }); test("mixed known+unknown refs fails fast (no partial execution)", async () => { - upsertCredentialMetadata("fal", "api_key"); + upsertCredentialMetadata("fal", "api_key", { + allowedTools: ["bash"], + }); const result = await shellTool.execute( { @@ -175,7 +180,9 @@ describe("shell tool credential ref resolution", () => { }); test("duplicate refs are deduped", async () => { - const meta = upsertCredentialMetadata("fal", "api_key"); + const meta = upsertCredentialMetadata("fal", "api_key", { + allowedTools: ["bash"], + }); await shellTool.execute( { @@ -209,4 +216,89 @@ describe("shell tool credential ref resolution", () => { expect(result.isError).toBeFalsy(); expect(mockGetOrStartSession).not.toHaveBeenCalled(); }); + + test("credential with allowedTools excluding bash is denied for proxied shell", async () => { + upsertCredentialMetadata("vercel", "api_token", { + allowedTools: ["publish_page"], + injectionTemplates: [ + { + hostPattern: "api.vercel.com", + injectionType: "header", + headerName: "Authorization", + valuePrefix: "Bearer ", + }, + ], + }); + + const result = await shellTool.execute( + { + command: "curl https://api.vercel.com/v1/projects", + activity: "test", + network_mode: "proxied", + credential_ids: ["vercel/api_token"], + }, + ctx, + ); + + expect(result.isError).toBe(true); + expect(result.content).toContain("credential tool policy denied"); + expect(result.content).toContain("not bash"); + // Must not call getOrStartSession — policy denial happens before session creation + expect(mockGetOrStartSession).not.toHaveBeenCalled(); + }); + + test("credential with allowedTools including bash starts proxied session", async () => { + const meta = upsertCredentialMetadata("deploy_svc", "api_key", { + allowedTools: ["bash"], + injectionTemplates: [ + { + hostPattern: "*.deploy-svc.io", + injectionType: "header", + headerName: "Authorization", + valuePrefix: "Bearer ", + }, + ], + }); + + await shellTool.execute( + { + command: "echo deploy", + activity: "test", + network_mode: "proxied", + credential_ids: ["deploy_svc/api_key"], + }, + ctx, + ); + + // Session should be created with the resolved credential ID + expect(mockGetOrStartSession).toHaveBeenCalledTimes(1); + const callArgs = mockGetOrStartSession.mock.calls[0]; + expect(callArgs[1]).toEqual([meta.credentialId]); + }); + + test("mixed allowed and denied credentials fail the whole command before session creation", async () => { + upsertCredentialMetadata("allowed_svc", "token", { + allowedTools: ["bash"], + }); + upsertCredentialMetadata("denied_svc", "token", { + allowedTools: ["publish_page"], + }); + + const result = await shellTool.execute( + { + command: "echo mixed", + activity: "test", + network_mode: "proxied", + credential_ids: ["allowed_svc/token", "denied_svc/token"], + }, + ctx, + ); + + expect(result.isError).toBe(true); + expect(result.content).toContain("credential tool policy denied"); + expect(result.content).toContain("denied_svc/token"); + expect(result.content).toContain("not bash"); + // Must not call getOrStartSession — even one denied credential blocks the whole command + expect(mockGetOrStartSession).not.toHaveBeenCalled(); + }); }); diff --git a/assistant/src/tools/terminal/shell.ts b/assistant/src/tools/terminal/shell.ts index 02409ebe286..e691ec9efd6 100644 --- a/assistant/src/tools/terminal/shell.ts +++ b/assistant/src/tools/terminal/shell.ts @@ -17,7 +17,9 @@ import { registerBackgroundTool, removeBackgroundTool, } from "../background-tool-registry.js"; +import { getCredentialMetadataById } from "../credentials/metadata-store.js"; import { resolveCredentialRef } from "../credentials/resolve.js"; +import { isToolAllowed } from "../credentials/tool-policy.js"; import { getOrStartSession, getSessionEnv, @@ -214,6 +216,48 @@ class ShellTool implements Tool { }, "Credential refs resolved", ); + + // ------------------------------------------------------------------- + // Tool policy enforcement — deny any credential that does not + // explicitly allow "bash" in its allowedTools metadata. This check + // runs after resolution/dedup and before proxy session creation so + // that a denied credential never reaches getOrStartSession. + // ------------------------------------------------------------------- + const deniedCredentials: { credentialId: string; reason: string }[] = []; + for (const credId of credentialIds) { + const meta = getCredentialMetadataById(credId); + if (!meta) { + // Should not happen — we just resolved these IDs — but fail-closed. + deniedCredentials.push({ + credentialId: credId, + reason: "metadata not found", + }); + continue; + } + if (!isToolAllowed("bash", meta.allowedTools)) { + const tools = meta.allowedTools ?? []; + deniedCredentials.push({ + credentialId: credId, + reason: + tools.length === 0 + ? `credential ${meta.service}/${meta.field} has no allowed tools` + : `credential ${meta.service}/${meta.field} allows [${tools.join(", ")}] but not bash`, + }); + } + } + if (deniedCredentials.length > 0) { + log.warn( + { denied: deniedCredentials }, + "Credential tool policy denied for proxied bash", + ); + const reasons = deniedCredentials + .map((d) => `${d.credentialId}: ${d.reason}`) + .join("; "); + return { + content: `Error: credential tool policy denied — ${reasons}. Each credential must include "bash" in its allowed tools to be used in a proxied shell session.`, + isError: true, + }; + } } else { credentialIds.push(...rawCredentialRefs); } From 6d396d461a7e092d7ae2a28b998a58ef6c1fcb6e Mon Sep 17 00:00:00 2001 From: Alex Nork Date: Tue, 12 May 2026 11:20:24 -0400 Subject: [PATCH 2/2] fix: add credential metadata mocks to shell-tool-proxy-mode tests Co-Authored-By: Claude Opus 4.6 (1M context) --- .../src/__tests__/shell-tool-proxy-mode.test.ts | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/assistant/src/__tests__/shell-tool-proxy-mode.test.ts b/assistant/src/__tests__/shell-tool-proxy-mode.test.ts index dd590962750..5d69f4b2ff0 100644 --- a/assistant/src/__tests__/shell-tool-proxy-mode.test.ts +++ b/assistant/src/__tests__/shell-tool-proxy-mode.test.ts @@ -136,6 +136,20 @@ mock.module("../tools/credentials/resolve.js", () => ({ resolveCredentialRef: (ref: string) => ({ credentialId: ref }), })); +mock.module("../tools/credentials/metadata-store.js", () => ({ + getCredentialMetadataById: (id: string) => ({ + service: "test", + field: id, + allowedTools: ["bash"], + allowedDomains: [], + }), +})); + +mock.module("../tools/credentials/tool-policy.js", () => ({ + isToolAllowed: (toolName: string, allowedTools: string[]) => + Array.isArray(allowedTools) && allowedTools.includes(toolName), +})); + mock.module("../tools/network/script-proxy/logging.js", () => ({ buildCredentialRefTrace: ( rawRefs: string[],