From 3d67002cefde85b27daa7cbe09abac2fcbe408c6 Mon Sep 17 00:00:00 2001 From: "Trinity Agent (trinity)" Date: Fri, 9 Oct 2026 13:30:51 -0400 Subject: [PATCH 01/10] refactor(mcp): credential tools are advertised only to the keys the backend admits (#3435) export_credentials, import_credentials and get_credential_encryption_key carry a per-tool canAccess allow-list {user, system} (fails closed, #848), descriptions that name who may call each, and a backend 403 returned as a typed {success:false, human_only|admin_only|not_authorized} envelope through accessDenied so the audit row reads denied (#2807). 400/404/503 stay thrown. No backend change. Mutations verified: deny-check allow-list and an unstamped envelope each turn agents.credentials.test.ts red. Co-Authored-By: Claude Opus 5.5 --- src/mcp-server/src/access.ts | 10 +- .../src/tools/agents.credentials.test.ts | 165 ++++++++++++++++++ src/mcp-server/src/tools/agents.ts | 78 +++++++-- 3 files changed, 239 insertions(+), 14 deletions(-) create mode 100644 src/mcp-server/src/tools/agents.credentials.test.ts diff --git a/src/mcp-server/src/access.ts b/src/mcp-server/src/access.ts index 299a5a546..adc618aaa 100644 --- a/src/mcp-server/src/access.ts +++ b/src/mcp-server/src/access.ts @@ -207,6 +207,10 @@ const ENT629 = "abilityai/trinity-enterprise#629 — the backend route is owner-equivalent for an agent key (Invariant #8); MCP gate pending that ruling"; const ADMIN_ONLY = "backend rejects agent principals (require_admin / assert_admin / reject_agent_principal, #1890)"; +const CREDENTIAL_HUMAN_ONLY = + "backend: get_owned_agent_by_name + reject_agent_principal (trinity-enterprise#69 Part 2) — owner or admin, human-only; hidden from agent sessions by a per-tool canAccess (#3435)"; +const KEY_ADMIN_ONLY = + "backend require_admin (#1890): admin + human-only; hidden from agent sessions by a per-tool canAccess (#3435)"; const TEARDOWN_HUMAN_ONLY = "backend fence: the gated route requires role 'creator' AND a HUMAN caller — reject_agent_principal plus a credential-kind refusal, because one call removes N agents without delete_agent's per-agent spawn-scope check (abilityai/trinity-enterprise#454)"; const REMINDER_SELF_GATE = "backend self-gate: reminders.py::_self_gate refuses an agent key naming another agent"; @@ -255,11 +259,11 @@ export const TOOL_ACCESS_POLICY: Readonly> = { list_templates: { kind: "none", why: "no agent target" }, get_credential_status: { kind: "baselined", owner: ENT629 }, inject_credentials: { kind: "baselined", owner: ADMIN_ONLY }, - export_credentials: { kind: "baselined", owner: ADMIN_ONLY }, - import_credentials: { kind: "baselined", owner: ADMIN_ONLY }, + export_credentials: { kind: "baselined", owner: CREDENTIAL_HUMAN_ONLY }, + import_credentials: { kind: "baselined", owner: CREDENTIAL_HUMAN_ONLY }, export_agent_data: { kind: "baselined", owner: ENT629 }, import_agent_data: { kind: "baselined", owner: ENT629 }, - get_credential_encryption_key: { kind: "none", why: "no agent target" }, + get_credential_encryption_key: { kind: "none", why: "no agent target; " + KEY_ADMIN_ONLY }, get_agent_ssh_access: { kind: "baselined", owner: ADMIN_ONLY }, deploy_local_agent: { kind: "none", why: "creates the agent; `name` is the new agent's name, not a target" }, initialize_github_sync: { kind: "baselined", owner: ENT629 }, diff --git a/src/mcp-server/src/tools/agents.credentials.test.ts b/src/mcp-server/src/tools/agents.credentials.test.ts new file mode 100644 index 000000000..0de469e44 --- /dev/null +++ b/src/mcp-server/src/tools/agents.credentials.test.ts @@ -0,0 +1,165 @@ +/** + * #3435 — the three credential-portability tools say who may call them. + * + * `export_credentials` / `import_credentials` sit behind the backend's + * `get_owned_agent_by_name` + `reject_agent_principal` (owner or admin, human + * only); `get_credential_encryption_key` behind `require_admin` (admin, human + * only). An agent-scoped key can never pass either gate, so: + * + * - each tool carries its own `canAccess` ALLOW-list `{user, system}` — what + * is advertised equals what the backend admits, and an unknown scope fails + * closed (#848); + * - a backend 403 comes back as a typed `{success:false, …}` envelope stamped + * `denied` on the audit row (#2807) instead of a thrown string; + * - 400 / 404 / 503 stay thrown, unchanged (ruling TD-4). + * + * Drives the real tool execute() with a fake TrinityClient. + * Runner: node:test → `node --import tsx --test src/tools/*.test.ts`. + */ +import { describe, it, beforeEach, afterEach } from "node:test"; +import { strict as assert } from "node:assert"; + +import { createAgentTools } from "./agents.js"; +import { ApiError, type TrinityClient } from "../client.js"; +import { configureAudit, withAudit } from "../audit.js"; + +type Tool = { + name: string; + description: string; + canAccess?: (auth: unknown) => boolean; + execute: (a: unknown, c: unknown) => Promise; +}; + +const TOOLS = ["export_credentials", "import_credentials", "get_credential_encryption_key"] as const; +const HUMAN_ONLY = "This operation is human-only; agent-scoped keys cannot perform it"; + +function build(answer: () => Promise) { + const calls: string[] = []; + const fake: Partial = { + getBaseUrl: () => "http://localhost:8000", + exportCredentials: async (name: string) => { calls.push(`export:${name}`); return answer() as never; }, + importCredentials: async (name: string) => { calls.push(`import:${name}`); return answer() as never; }, + getEncryptionKey: async () => { calls.push("key"); return answer() as never; }, + }; + const all = createAgentTools(fake as TrinityClient, false) as unknown as Record; + const byName = Object.fromEntries(Object.values(all).map((t) => [t.name, t])) as Record; + return { byName, calls }; +} + +const args = (tool: string) => (tool === "get_credential_encryption_key" ? {} : { name: "acme-bot" }); + +describe("#3435 credential tools: advertisement", () => { + const { byName } = build(async () => ({})); + + it("each tool declares its own canAccess", () => { + for (const t of TOOLS) assert.equal(typeof byName[t].canAccess, "function", `${t} has no canAccess`); + }); + + it("is listed for user and system keys — what the backend admits", () => { + for (const t of TOOLS) { + for (const scope of ["user", "system"]) { + assert.equal(byName[t].canAccess!({ scope }), true, `${t} must be listed for ${scope}`); + } + } + }); + + it("is hidden from agent, connector, anonymous, an unknown scope and no scope (fails closed)", () => { + for (const t of TOOLS) { + for (const auth of [{ scope: "agent" }, { scope: "connector" }, { scope: "anonymous" }, + { scope: "portal_delegate" }, { scope: "some_future_tier" }, {}, undefined, null]) { + assert.equal(byName[t].canAccess!(auth), false, `${t} must be hidden for ${JSON.stringify(auth)}`); + } + } + }); + + it("each description names who may call it, and stays under the 2,048 cap", () => { + for (const t of TOOLS) { + assert.match(byName[t].description, /agent-scoped keys? (?:can never|cannot)/i, `${t}: no caller class`); + assert.ok(byName[t].description.length <= 2048, `${t}: ${byName[t].description.length} chars`); + } + assert.match(byName.get_credential_encryption_key.description, /administrator/i); + for (const t of ["export_credentials", "import_credentials"]) { + assert.match(byName[t].description, /owner or (?:an )?admin/i); + } + }); +}); + +describe("#3435 credential tools: execution", () => { + it("a user / system / keyless-dev session reaches the backend and gets the raw result back", async () => { + for (const session of [{ scope: "user" }, { scope: "system" }, undefined]) { + const { byName, calls } = build(async () => ({ status: "ok", files_imported: [".env"] })); + for (const t of TOOLS) { + const out = await byName[t].execute(args(t), { session }); + assert.deepEqual(JSON.parse(out), { status: "ok", files_imported: [".env"] }); + } + assert.deepEqual(calls, ["export:acme-bot", "import:acme-bot", "key"]); + } + }); + + it("a human-only 403 becomes a typed envelope, not a thrown string", async () => { + const { byName } = build(async () => { throw new ApiError(403, JSON.stringify({ detail: HUMAN_ONLY })); }); + for (const t of TOOLS) { + const out = JSON.parse(await byName[t].execute(args(t), { session: { scope: "user" } })); + assert.equal(out.success, false); + assert.equal(out.human_only, true); + assert.equal(out.error, HUMAN_ONLY); + } + }); + + it("an admin-gate 403 is admin_only; any other 403 is not_authorized", async () => { + const admin = build(async () => { throw new ApiError(403, JSON.stringify({ detail: "Admin access required" })); }); + const out = JSON.parse(await admin.byName.get_credential_encryption_key.execute({}, { session: { scope: "user" } })); + assert.deepEqual(out, { success: false, error: "Admin access required", admin_only: true }); + + const other = build(async () => { throw new ApiError(403, "not json"); }); + const o2 = JSON.parse(await other.byName.import_credentials.execute(args("import_credentials"), {})); + assert.equal(o2.success, false); + assert.equal(o2.not_authorized, true); + assert.match(o2.error, /not json/); + }); + + it("400, 404 and 503 stay thrown, and so does a non-API failure (TD-4)", async () => { + for (const status of [400, 404, 503]) { + const { byName } = build(async () => { throw new ApiError(status, JSON.stringify({ detail: "x" })); }); + for (const t of TOOLS) { + await assert.rejects(() => byName[t].execute(args(t), {}), (e: unknown) => e instanceof ApiError && e.status === status); + } + } + const { byName } = build(async () => { throw new Error("socket hang up"); }); + await assert.rejects(() => byName.export_credentials.execute(args("export_credentials"), {}), /socket hang up/); + }); +}); + +describe("#3435 a 403 refusal is audited as denied (#2807)", () => { + const realFetch = globalThis.fetch; + let rows: Array<{ details: Record }>; + + beforeEach(() => { + configureAudit({ apiUrl: "http://audit.test", secret: "test-secret" }); + rows = []; + globalThis.fetch = (async (input: unknown, init?: RequestInit) => { + if (String(input).endsWith("/api/internal/audit")) { + rows.push(JSON.parse(String(init?.body))); + return new Response(JSON.stringify({ status: "logged" }), { status: 200 }); + } + throw new Error(`unstubbed fetch: ${String(input)}`); + }) as typeof fetch; + }); + afterEach(() => { + globalThis.fetch = realFetch; + }); + + it("the row says refused for each tool", async () => { + const { byName } = build(async () => { throw new ApiError(403, JSON.stringify({ detail: HUMAN_ONLY })); }); + for (const t of TOOLS) { + const wrapped = withAudit(t, byName[t].execute as never); + await wrapped(args(t) as never, { session: { scope: "user", userId: "u1" } } as never); + } + assert.equal(rows.length, 3); + for (const row of rows) { + assert.equal(row.details.success, false, JSON.stringify(row.details)); + assert.equal(row.details.denied, true); + assert.equal(row.details.error, HUMAN_ONLY); + } + }); +}); diff --git a/src/mcp-server/src/tools/agents.ts b/src/mcp-server/src/tools/agents.ts index d2117ae4c..e9aa2b6b5 100644 --- a/src/mcp-server/src/tools/agents.ts +++ b/src/mcp-server/src/tools/agents.ts @@ -5,7 +5,7 @@ */ import { z } from "zod"; -import { TrinityClient } from "../client.js"; +import { ApiError, TrinityClient } from "../client.js"; import type { McpAuthContext, StartAgentResult } from "../types.js"; import { accessDenied } from "../access.js"; import { deriveMcpIdempotencyKey } from "./chat.js"; @@ -38,6 +38,38 @@ export function skillsDeliveryLines(result: StartAgentResult): string[] { return lines; } +/** + * #3435: the scopes that may see the three credential-portability tools. An + * ALLOW-list (#848 — a null or unknown scope fails closed), equal to what the + * backend admits: import/export take `get_owned_agent_by_name` + + * `reject_agent_principal`, the key takes `require_admin`; both refuse every + * agent-scoped key, so an agent session is never shown a tool it cannot call. + */ +const CREDENTIAL_TOOL_SCOPES: ReadonlySet = new Set(["user", "system"]); +const credentialToolAccess = (auth: any): boolean => CREDENTIAL_TOOL_SCOPES.has(auth?.scope ?? ""); + +/** + * #3435: a backend 403 on a credential tool, as a typed envelope stamped + * `denied` on the audit row (#2807). Only the backend `detail` is copied — never + * a body. Every other failure (400/404/503, transport) is rethrown unchanged. + */ +function credentialRefusal(error: unknown, context: { session?: McpAuthContext } | undefined): string { + if (!(error instanceof ApiError) || error.status !== 403) throw error; + let detail = error.message; + try { + const parsed = JSON.parse(error.body); + if (typeof parsed?.detail === "string") detail = parsed.detail; + } catch { + // not JSON — keep the message + } + const flag = /human-only/i.test(detail) + ? "human_only" + : /admin access required/i.test(detail) + ? "admin_only" + : "not_authorized"; + return accessDenied(context, { success: false, error: detail, [flag]: true }); +} + /** * Create agent management tools with the given client * @param client - Base Trinity client (provides base URL, no auth when requireApiKey=true) @@ -624,18 +656,26 @@ export function createAgentTools( "Reads .env and .mcp.json from the agent, encrypts them, and writes .credentials.enc " + "to the agent's workspace. This file can be committed to git for portable credential storage. " + "Requires CREDENTIAL_ENCRYPTION_KEY to be configured on the backend. " + - "The agent must be running.", + "The agent must be running. " + + "WHO MAY CALL: the agent's owner or an admin, with a user-scoped key or a signed-in session; " + + "agent-scoped keys can never call it (human-only), so agent sessions are not shown it. " + + "A refusal returns {success:false, error, human_only|not_authorized}.", parameters: z.object({ name: z.string().describe("The name of the agent to export credentials from"), }), + canAccess: credentialToolAccess, execute: async ({ name }: { name: string }, context?: { session?: McpAuthContext }) => { const authContext = context?.session; const apiClient = getClient(authContext); console.log(`[export_credentials] Exporting credentials from agent '${name}'`); - const result = await apiClient.exportCredentials(name); - return JSON.stringify(result, null, 2); + try { + const result = await apiClient.exportCredentials(name); + return JSON.stringify(result, null, 2); + } catch (error) { + return credentialRefusal(error, context); + } }, }, @@ -649,19 +689,27 @@ export function createAgentTools( "Reads .credentials.enc from the agent's workspace, decrypts it, and writes " + "the credential files (.env, .mcp.json, etc.) to the workspace. " + "This is useful after cloning an agent's git repo. " + - "Requires CREDENTIAL_ENCRYPTION_KEY to be configured on the backend. " + - "The agent must be running.", + "Requires CREDENTIAL_ENCRYPTION_KEY to be configured on the backend (the same key " + + "that encrypted the file). The agent must be running. " + + "WHO MAY CALL: the agent's owner or an admin, with a user-scoped key or a signed-in session; " + + "agent-scoped keys can never call it (human-only), so agent sessions are not shown it. " + + "A refusal returns {success:false, error, human_only|not_authorized}.", parameters: z.object({ name: z.string().describe("The name of the agent to import credentials into"), }), + canAccess: credentialToolAccess, execute: async ({ name }: { name: string }, context?: { session?: McpAuthContext }) => { const authContext = context?.session; const apiClient = getClient(authContext); console.log(`[import_credentials] Importing credentials into agent '${name}'`); - const result = await apiClient.importCredentials(name); - return JSON.stringify(result, null, 2); + try { + const result = await apiClient.importCredentials(name); + return JSON.stringify(result, null, 2); + } catch (error) { + return credentialRefusal(error, context); + } }, }, @@ -732,13 +780,21 @@ export function createAgentTools( "This allows local agents to encrypt/decrypt .credentials.enc files themselves, " + "enabling portable agents that work both locally and on Trinity. " + "Returns the key as a hex string (64 chars for AES-256). " + - "SECURITY: Store securely and never commit to git.", + "SECURITY: Store securely and never commit to git. " + + "WHO MAY CALL: an administrator only, with a user-scoped key or a signed-in session; " + + "agent-scoped keys can never call it (human-only), so agent sessions are not shown it. " + + "A refusal returns {success:false, error, admin_only|human_only}.", parameters: z.object({}), + canAccess: credentialToolAccess, execute: async (_params: unknown, context?: { session?: McpAuthContext }) => { const authContext = context?.session; const apiClient = getClient(authContext); - const result = await apiClient.getEncryptionKey(); - return JSON.stringify(result, null, 2); + try { + const result = await apiClient.getEncryptionKey(); + return JSON.stringify(result, null, 2); + } catch (error) { + return credentialRefusal(error, context); + } }, }, From 3948030ea4a947b0fc03d321a2c0d486e8018b7f Mon Sep 17 00:00:00 2001 From: "Trinity Agent (trinity)" Date: Fri, 9 Oct 2026 13:32:53 -0400 Subject: [PATCH 02/10] refactor(mcp): pin credential-tool visibility over the real transport, with a shrink-only admin-only guard (#3435) Boots the real server in key mode: a user key lists the three credential tools, an agent key neither lists nor can call them (not found). Every other ADMIN_ONLY row is either hidden from agent sessions or in a shrink-only baseline of nine (follow-up per ruling TD-3). Dropping one tool's canAccess turns it red. Co-Authored-By: Claude Opus 5.5 --- src/mcp-server/src/tool-visibility.test.ts | 120 +++++++++++++++++++++ 1 file changed, 120 insertions(+) diff --git a/src/mcp-server/src/tool-visibility.test.ts b/src/mcp-server/src/tool-visibility.test.ts index 2d2cb6f9c..3e902c44b 100644 --- a/src/mcp-server/src/tool-visibility.test.ts +++ b/src/mcp-server/src/tool-visibility.test.ts @@ -47,12 +47,16 @@ import { describe, it } from "node:test"; import { strict as assert } from "node:assert"; import { createServer } from "node:net"; +import { createServer as createHttpServer, type Server } from "node:http"; +import type { AddressInfo } from "node:net"; import { FastMCP } from "fastmcp"; import { Client } from "@modelcontextprotocol/sdk/client/index.js"; import { StreamableHTTPClientTransport } from "@modelcontextprotocol/sdk/client/streamableHttp.js"; +import { TOOL_ACCESS_POLICY } from "./access.js"; import { + createServer as createTrinityServer, OPERATOR_SCOPES, makeOperatorOnly, connectorOnly, @@ -470,3 +474,119 @@ describe("ent#500 get_agent_assignments is operator-scope only", () => { } }); }); + +describe("#3435 human-only credential tools are not advertised to agent sessions", () => { + // export_credentials / import_credentials (owner or admin, human-only) and + // get_credential_encryption_key (admin, human-only) can never be called with + // an agent-scoped key: the backend refuses every one. Each carries its own + // canAccess allow-list {user, system}. This boots the REAL server in key + // mode and drives it over the real transport with a user key and an agent + // key — listing AND calling, because advertisement alone is not a control. + const CREDENTIAL_TOOLS = ["export_credentials", "get_credential_encryption_key", "import_credentials"]; + + /** + * The other tools whose backend gate refuses every agent key (the `ADMIN_ONLY` + * owner in access.ts) but which are still advertised to agent sessions. + * SHRINK-ONLY: an entry that becomes hidden fails until it is removed here, + * and a new admin-only tool that is advertised to agents fails until it gets + * a canAccess. Never add an entry to make a new tool pass. Follow-up: convert + * the rest (#3435 ruling TD-3). + */ + const ADVERTISED_TO_AGENTS_BASELINE = [ + "get_agent_ssh_access", + "get_nevermined_payments", + "inject_credentials", + "register_a2a_endpoint", + "remove_a2a_endpoint", + "rename_agent", + "set_a2a_inbound_allowlist", + "set_agent_a2a_exposure", + "toggle_nevermined", + ]; + const ADMIN_ONLY_OWNER = /^backend rejects agent principals/; + + let backend: Server; + let mcpServer: { stop: () => Promise }; + let mcpUrl: URL; + + async function boot(): Promise { + backend = createHttpServer((req, res) => { + req.resume(); + req.on("end", () => { + res.writeHead(200, { "Content-Type": "application/json" }); + if (req.url === "/api/mcp/validate") { + const agent = String(req.headers.authorization ?? "").includes("agent_key"); + return res.end(JSON.stringify({ + valid: true, key_id: agent ? "key-agent-1" : "key-user-1", user_id: "owner", + user_email: "owner@example.com", key_name: "vis", scope: agent ? "agent" : "user", + ...(agent ? { agent_name: "alpha" } : {}), + })); + } + return res.end("{}"); + }); + }); + await new Promise((r) => backend.listen(0, "127.0.0.1", () => r())); + const backendPort = (backend.address() as AddressInfo).port; + const mcpPort = await freePort(); + const { server } = await createTrinityServer({ + trinityApiUrl: `http://127.0.0.1:${backendPort}`, + requireApiKey: true, + port: mcpPort, + }); + await server.start({ transportType: "httpStream", httpStream: { port: mcpPort, host: "127.0.0.1" } }); + mcpServer = server; + mcpUrl = new URL(`http://127.0.0.1:${mcpPort}/mcp`); + } + + async function asKey(key: string, body: (client: Client) => Promise): Promise { + const client = new Client({ name: `vis-3435-${key}`, version: "1.0.0" }); + await client.connect(new StreamableHTTPClientTransport(mcpUrl, { + requestInit: { headers: { Authorization: `Bearer trinity_mcp_${key}` } }, + })); + try { + return await body(client); + } finally { + await client.close().catch(() => {}); + } + } + + it("a user key lists them; an agent key neither lists nor can call them", async () => { + await boot(); + try { + const userTools = await asKey("user_key", async (c) => (await c.listTools()).tools.map((t) => t.name)); + const agentTools = await asKey("agent_key", async (c) => (await c.listTools()).tools.map((t) => t.name)); + assert.ok(agentTools.length > 50, `only ${agentTools.length} tools for the agent session — the census is not seeing the server`); + for (const name of CREDENTIAL_TOOLS) { + assert.ok(userTools.includes(name), `${name} must be listed for a user key`); + assert.ok(!agentTools.includes(name), `${name} must NOT be listed for an agent key`); + } + + await asKey("agent_key", async (c) => { + for (const name of CREDENTIAL_TOOLS) { + await assert.rejects( + () => c.callTool({ name, arguments: name === "get_credential_encryption_key" ? {} : { name: "alpha" } }), + (err: unknown) => /not found|unknown tool|method not found/i.test(String((err as Error)?.message ?? err)), + `${name}: an agent key calling it by name must be rejected as not found`, + ); + } + }); + + // Shrink-only guard over every admin-only row: hidden from agents, or baselined. + const guarded = Object.entries(TOOL_ACCESS_POLICY) + .filter(([, p]) => p.kind === "baselined" && ADMIN_ONLY_OWNER.test(p.owner)) + .map(([name]) => name) + .filter((name) => userTools.includes(name)); + const advertised = guarded.filter((name) => agentTools.includes(name)).sort(); + assert.deepEqual( + advertised.filter((n) => !ADVERTISED_TO_AGENTS_BASELINE.includes(n)), + [], + "an admin-only tool is advertised to agent sessions — give it a canAccess allow-list (the #3435 shape)", + ); + const stale = ADVERTISED_TO_AGENTS_BASELINE.filter((n) => userTools.includes(n) && !agentTools.includes(n)); + assert.deepEqual(stale, [], "baseline entries now hidden from agents — remove them (shrink-only)"); + } finally { + await mcpServer?.stop().catch(() => {}); + await new Promise((r) => backend.close(() => r())); + } + }); +}); From b874108a5066d92f4963ec63f54af43f1172a3ba Mon Sep 17 00:00:00 2001 From: "Trinity Agent (trinity)" Date: Fri, 9 Oct 2026 13:34:59 -0400 Subject: [PATCH 03/10] refactor(mcp): pin the backend gates behind the three credential tools by principal (#3435) A pin of current behaviour, no backend change: import/export admit an owner or admin human and refuse every agent key with the exact human-only 403 the MCP mapper keys on (uniform 404 for a non-owner); the key route admits an admin human only (403 non-admin, agent, ops scope; 503 when the key is unset). Explicit models.User principals. Mutation: removing reject_agent_principal from the import handler turns 2 cases red. Co-Authored-By: Claude Opus 5.5 --- tests/registry.json | 13 ++ tests/unit/test_3435_credential_tool_gates.py | 180 ++++++++++++++++++ 2 files changed, 193 insertions(+) create mode 100644 tests/unit/test_3435_credential_tool_gates.py diff --git a/tests/registry.json b/tests/registry.json index 1c11866de..4ab70b08d 100644 --- a/tests/registry.json +++ b/tests/registry.json @@ -5326,6 +5326,19 @@ "unit" ], "description": "#3406: the backend half of the contract the browser's 401 verdict reads. get_current_user rejects a dead token and OAuth2PasswordBearer rejects a missing one WITH `WWW-Authenticate: Bearer`; the five public-link session routes (intro, history, session clear, chat, terminate) answer an expired or missing visitor session with a bare 401. Handlers driven directly with the link check, rate limit and session lookup stubbed. No Docker." + }, + { + "file": "unit/test_3435_credential_tool_gates.py", + "feature": "#3435", + "added": "2026-10-09", + "categories": [ + "backend", + "auth", + "credentials", + "mcp", + "unit" + ], + "description": "#3435: pins the EXISTING backend gates behind the export_credentials / import_credentials / get_credential_encryption_key MCP tools, by principal (real models.User objects): import/export admit an owner or admin human (JWT, user or system key), refuse every agent key with the exact human-only 403 the MCP mapper keys on, and give a non-owner the uniform 404; the key route admits an admin human, refuses a non-admin 403 'Admin access required', an agent key 403 human-only, an ops-scope key 403, and answers 503 when CREDENTIAL_ENCRYPTION_KEY is unset. Real router + real owner/admin dependencies; DB ownership, Docker and the encryption service stubbed. No Docker." } ] } diff --git a/tests/unit/test_3435_credential_tool_gates.py b/tests/unit/test_3435_credential_tool_gates.py new file mode 100644 index 000000000..832fa6e78 --- /dev/null +++ b/tests/unit/test_3435_credential_tool_gates.py @@ -0,0 +1,180 @@ +"""The backend gates behind the three credential MCP tools, by principal (#3435). + +`export_credentials` / `import_credentials` / `get_credential_encryption_key` +are advertised by the MCP server only to `user` and `system` keys (a per-tool +`canAccess` allow-list). That is correct only while the backend refuses every +agent-scoped key on all three routes — this file pins those EXISTING gates so +the MCP advertisement and the backend admission cannot drift apart silently: + + POST /api/agents/{name}/credentials/export|import + get_owned_agent_by_name (owner or admin, else a uniform 404, #186) + + reject_agent_principal (agent key -> 403 "human-only", ent#69 Part 2) + GET /api/credentials/encryption-key + require_admin (admin role, human, scope in {None, user, system}; #1890, #2323) + + 503 when CREDENTIAL_ENCRYPTION_KEY is unset + +A pin of current behaviour — no backend code changes with it. Principals are +real `models.User` objects with `role`, `agent_name`, `mcp_scope` declared +explicitly: a MagicMock caller carries a truthy `agent_name` and would read as +an agent key, and a stand-in lacking `mcp_scope` fails closed at the admin gate +— both a red (or green) for the wrong reason. The MCP mapper keys on the +literal "human-only" in the 403 detail, so that string is pinned exactly. +""" + +from unittest.mock import AsyncMock, MagicMock + +import pytest +from fastapi import FastAPI +from fastapi.testclient import TestClient + +import dependencies +import routers.credentials as rc +import services.credential_encryption as ce +from dependencies import get_current_user +from models import User + +_AGENT = "acme-bot" +_HUMAN_ONLY = "This operation is human-only; agent-scoped keys cannot perform it" + + +def _owner_jwt(): + return User(id=1, username="owner", role="user", agent_name=None, mcp_scope=None) + + +def _owner_user_key(): + return User(id=1, username="owner", role="user", agent_name=None, mcp_scope="user") + + +def _admin_jwt(): + return User(id=2, username="admin", role="admin", agent_name=None, mcp_scope=None) + + +def _admin_user_key(): + return User(id=2, username="admin", role="admin", agent_name=None, mcp_scope="user") + + +def _admin_system_key(): + return User(id=2, username="admin", role="admin", agent_name=None, mcp_scope="system") + + +def _agent_key_of_owner(): + # An agent-scoped key resolves to its OWNER carrying the owner's role. + return User(id=1, username="owner", role="user", agent_name="bot", mcp_scope="agent") + + +def _agent_key_of_admin(): + return User(id=2, username="admin", role="admin", agent_name="bot", mcp_scope="agent") + + +def _stranger(): + return User(id=3, username="stranger", role="user", agent_name=None, mcp_scope="user") + + +def _ops_key_of_admin(): + return User(id=2, username="admin", role="admin", agent_name=None, mcp_scope="ops") + + +@pytest.fixture +def harness(monkeypatch): + """Real router, real owner/admin dependencies; only the principal, the DB + ownership lookups, Docker and the encryption service are stubbed.""" + state = {"user": _owner_jwt()} + app = FastAPI() + app.include_router(rc.router) + app.dependency_overrides[get_current_user] = lambda: state["user"] + + monkeypatch.setattr(dependencies.db, "get_agent_owner", lambda name: "owner" if name == _AGENT else None) + monkeypatch.setattr( + dependencies.db, "can_user_share_agent", lambda username, name: username in ("owner", "admin") + ) + container = MagicMock() + monkeypatch.setattr(rc, "get_agent_container", lambda name: container) + monkeypatch.setattr( + rc, "get_agent_status_from_container", lambda c: MagicMock(status="running") + ) + monkeypatch.setattr(rc.platform_audit_service, "log", AsyncMock(return_value=None)) + monkeypatch.setattr(rc.credential_requirements_service, "invalidate_report_cache", lambda name: None) + service = MagicMock() + service.export_to_agent = AsyncMock(return_value=("/home/developer/.credentials.enc", 2)) + service.import_to_agent = AsyncMock(return_value={".env": "K=V", ".mcp.json": "{}"}) + monkeypatch.setattr(ce, "get_credential_encryption_service", lambda: service) + monkeypatch.setenv("CREDENTIAL_ENCRYPTION_KEY", "ab" * 32) + + def as_(user): + state["user"] = user + return TestClient(app) + + return as_, service + + +ROUTES = [ + ("export", f"/api/agents/{_AGENT}/credentials/export", "export_to_agent"), + ("import", f"/api/agents/{_AGENT}/credentials/import", "import_to_agent"), +] + + +class TestImportExportGate: + @pytest.mark.parametrize("op,path,method", ROUTES) + @pytest.mark.parametrize("principal", [_owner_jwt, _owner_user_key, _admin_jwt, _admin_user_key, _admin_system_key]) + def test_owner_or_admin_human_reaches_the_service(self, harness, op, path, method, principal): + as_, service = harness + r = as_(principal()).post(path) + assert r.status_code == 200, r.text + getattr(service, method).assert_awaited_once_with(_AGENT) + + @pytest.mark.parametrize("op,path,method", ROUTES) + @pytest.mark.parametrize("principal", [_agent_key_of_owner, _agent_key_of_admin]) + def test_agent_key_is_refused_human_only_even_for_its_owner(self, harness, op, path, method, principal): + as_, service = harness + r = as_(principal()).post(path) + assert r.status_code == 403 + assert r.json()["detail"] == _HUMAN_ONLY + getattr(service, method).assert_not_awaited() + + @pytest.mark.parametrize("op,path,method", ROUTES) + def test_non_owner_gets_the_uniform_404(self, harness, op, path, method): + as_, service = harness + r = as_(_stranger()).post(path) + assert r.status_code == 404 + assert r.json()["detail"] == "Agent not found" + getattr(service, method).assert_not_awaited() + + +class TestEncryptionKeyGate: + PATH = "/api/credentials/encryption-key" + + @pytest.mark.parametrize("principal", [_admin_jwt, _admin_user_key, _admin_system_key]) + def test_admin_human_gets_the_key(self, harness, principal): + as_, _ = harness + r = as_(principal()).get(self.PATH) + assert r.status_code == 200, r.text + assert r.json()["key"] == "ab" * 32 + assert r.json()["algorithm"] == "AES-256-GCM" + + @pytest.mark.parametrize("principal", [_owner_jwt, _owner_user_key]) + def test_non_admin_human_is_refused(self, harness, principal): + as_, _ = harness + r = as_(principal()).get(self.PATH) + assert r.status_code == 403 + assert r.json()["detail"] == "Admin access required" + + @pytest.mark.parametrize("principal", [_agent_key_of_owner, _agent_key_of_admin]) + def test_agent_key_is_refused_human_only_even_for_an_admin_owner(self, harness, principal): + as_, _ = harness + r = as_(principal()).get(self.PATH) + assert r.status_code == 403 + assert r.json()["detail"] == _HUMAN_ONLY + assert "key" not in r.json() + + def test_a_scope_outside_the_admin_allowlist_is_refused(self, harness): + as_, _ = harness + r = as_(_ops_key_of_admin()).get(self.PATH) + assert r.status_code == 403 + assert "key" not in r.json() + + def test_unset_key_is_503(self, harness, monkeypatch): + as_, _ = harness + monkeypatch.delenv("CREDENTIAL_ENCRYPTION_KEY", raising=False) + r = as_(_admin_jwt()).get(self.PATH) + assert r.status_code == 503 + assert r.json()["detail"] == "Credential encryption key not configured" From f7c9bc29b627492bba12cf0241a074dc3aa3938d Mon Sep 17 00:00:00 2001 From: "Trinity Agent (trinity)" Date: Fri, 9 Oct 2026 13:35:34 -0400 Subject: [PATCH 04/10] refactor(mcp): document who may call the credential tools (#3435) User-docs MCP list and FAQ migration entry name the caller class per tool (the FAQ gains the target-side step: set CREDENTIAL_ENCRYPTION_KEY in .env and restart before importing). The feature flow's stale export/import handler snippets now show the real dependency chain, and its MCP section and the mcp-server architecture row describe the per-tool allow-list. Co-Authored-By: Claude Opus 5.5 --- docs/memory/architecture/mcp-server.md | 2 +- .../feature-flows/credential-injection.md | 66 +++++++------------ .../credentials/credential-management.md | 8 ++- .../faq/credentials-and-subscriptions.md | 2 +- 4 files changed, 29 insertions(+), 49 deletions(-) diff --git a/docs/memory/architecture/mcp-server.md b/docs/memory/architecture/mcp-server.md index 80bc1f208..5258000ff 100644 --- a/docs/memory/architecture/mcp-server.md +++ b/docs/memory/architecture/mcp-server.md @@ -22,7 +22,7 @@ FastMCP, Streamable HTTP transport, port 8080. API-key auth via `Authorization: | Module | Tools | Description | |--------|-------|-------------| -| `agents.ts` (22) | `list_agents`, `get_agent`, `get_agent_info`, `get_agent_compatibility_report`, `create_agent`, `rename_agent`, `delete_agent`, `start_agent`, `stop_agent`, `list_templates`, `get_credential_status`, `inject_credentials`, `export_credentials`, `import_credentials`, `get_credential_encryption_key`, `get_agent_ssh_access`, `deploy_local_agent`, `initialize_github_sync`, `get_agent_github_pat_status`, `set_agent_github_pat`, `export_agent_data`, `import_agent_data` | Agent lifecycle, credentials, SSH, local deploy (#2060: `deploy_local_agent` requires an embedded `.trinity-manifest.json` — set in tool code — verified server-side; deterministic Idempotency-Key; description states the token-bound payload ceiling and directs large agents at the `trinity` CLI), GitHub sync, per-agent PAT (#347), runtime-data export/import (#1169), compatibility report (#668) | +| `agents.ts` (22) | `list_agents`, `get_agent`, `get_agent_info`, `get_agent_compatibility_report`, `create_agent`, `rename_agent`, `delete_agent`, `start_agent`, `stop_agent`, `list_templates`, `get_credential_status`, `inject_credentials`, `export_credentials`, `import_credentials`, `get_credential_encryption_key`, `get_agent_ssh_access`, `deploy_local_agent`, `initialize_github_sync`, `get_agent_github_pat_status`, `set_agent_github_pat`, `export_agent_data`, `import_agent_data` | Agent lifecycle, credentials, SSH, local deploy (#2060: `deploy_local_agent` requires an embedded `.trinity-manifest.json` — set in tool code — verified server-side; deterministic Idempotency-Key; description states the token-bound payload ceiling and directs large agents at the `trinity` CLI), GitHub sync, per-agent PAT (#347), runtime-data export/import (#1169), compatibility report (#668). `export_credentials` / `import_credentials` / `get_credential_encryption_key` carry a per-tool `canAccess` allow-list `{user, system}` (what their human-only backend gates admit — hidden from agent sessions) and return a backend 403 as a stamped `{success:false, …}` envelope (#3435); the other `ADMIN_ONLY` rows still advertised to agents are a shrink-only baseline in `tool-visibility.test.ts` | | `chat.ts` (4) | `chat_with_agent`, `get_chat_history`, `get_agent_logs`, `fan_out` | Chat (enforces sharing rules), history, logs; `fan_out` = parallel dispatch of one message to several agents (FANOUT-001). Both sync modes apply `MCP_CHAT_TIMEOUT_MS` (default 25000) — sequential `/chat` (#914) and parallel `/task` (#2661); on abort the client queries `/api/agents/{name}/executions` and returns `{status:"queued_timeout", execution_id, message}` so callers poll instead of duplicate-queueing. **Attribution must be provable, never newest-wins**: `/task` runs N tasks concurrently, so `triggered_by`/`source_mcp_key_id`/window are identical across one caller's rows — the matcher also keys on the call's own `message`, takes its trigger set per call site (`/chat` must NOT accept `self_task`), derives the window from the timeout (a fixed 30s made raising the documented knob a silent kill-switch), and returns **nothing when >1 candidate survives**. A duplicate under the same `Idempotency-Key` is answered from the 409's `execution_id` instead of an opaque `API error (409)`. The recovery read is separately bounded (`MCP_RECOVERY_TIMEOUT_MS`, no 401-reauth) because it spends what is left of the gateway budget. **`fan_out` is the third route (#2670)**, and the one that exceeds the gateway most reliably — a batch runs longer than any single task in it. Its receipt names a `fan_out_id`, not an `execution_id`: N rows share one id, so a single execution id could only name an arbitrary member. Ambiguity is therefore measured on the BATCH (>1 distinct `fan_out_id` ⇒ no receipt), and status is deliberately NOT filtered — by abort time a batch is normally a mix of finished and running rows, so requiring non-terminal ones would drop the batches furthest along. Poll `get_fan_out_result`, which folds `schedule_executions` rather than the idempotency snapshot (that snapshot is written by `complete()`, i.e. only once the batch is over, so it cannot answer what is happening now). **Chain-depth refusal (#2806):** `client.chat`/`task`/`fanOut` turn a 403 carrying `detail.error == "inter_agent_depth_exceeded"` into a typed `DepthRefusal` (`parseDepthRefusal`) instead of throwing; `runAgentChat` (every branch) and `fan_out` return it as `{status: "inter_agent_depth_exceeded", agent, depth, max_depth, retryable: false, message}`. A 403 without the code still throws. **Delegation contract (ent#568):** `src/delegation_contract.ts` is the one text the platform prompt's §Agent Collaboration also carries (byte parity: `tests/unit/test_ent568_delegation_contract.py`); `chat_with_agent` and every `chat_with_` publish it verbatim, `fan_out` / `send_message` repeat its rule sentence, and each description stays ≤ 2,048 chars because Claude Code cuts MCP tool descriptions there (`CLAUDE_CODE_MAX_MCP_DESCRIPTION_LENGTH`; per-mode detail lives in parameter descriptions, which are not cut). Every `execution_id` receipt a `chat_with_*` caller gets carries `readNotResend` — the MCP server's own `queued_timeout` and 409 replay, and the backend's `accepted`/`queued`/sync-`/task` `queued_timeout` snapshots, whose REST-only "poll GET …" line `runAgentChat` rewrites with a liveness-neutral lead; `fan_out_timeout` keeps its `get_fan_out_result` line. **Parent forwarding (ent#224/#3232):** `resolveReportBack` decides once per call whether `parent_execution_id` goes on the `/task` body (parallel and #946 pull-routed branches; sequential `/chat` never carries one) and which `report_back` fields the result gets. Async dispatches send the platform header turn (#2392) by default; a typed `execution_id="manual"` opts out; a self-task with `inject_result` is excluded; sync parallel is opt-in by a typed id; the header turn wins over a typed id, which is forwarded only if `isWellFormedExecutionId` accepts it. `callerTurn` (the forwarded `X-Trinity-Execution-Id`) is computed separately and never cleared by the opt-out. One `[Report-Back #3232]` log line per call records route, arm, source and verdict. `EXECUTION_ID_PARAM_DESCRIPTION` is the one parameter text shared with `dynamic-agents.ts`. Kill switch `MCP_REPORT_BACK_ENABLED` (default `true`): `createServer` reads it, passes it to `createChatTools`, and returns it so `index.ts` hands it to `startExposedToolsReconciler` (`ReconcilerOptions.reportBackEnabled` is required, so `tsc` fails if a start site forgets it). Second consumer of `parent_execution_id`: on agent images without the #2392 header the backend skill gate reads it as the requester execution (validated there as the caller's own). | | `schedules.ts` (8) | `list_agent_schedules`, `create_agent_schedule`, `get_agent_schedule`, `update_agent_schedule`, `delete_agent_schedule`, `toggle_agent_schedule`, `trigger_agent_schedule`, `get_schedule_executions` | Schedule CRUD and execution history | | `executions.ts` (5) | `list_recent_executions`, `get_execution_result`, `get_fan_out_result`, `get_agent_activity_summary`, `search_executions` | Execution queries, async result polling, activity monitoring (MCP-007). `get_execution_result` returns `compact_metadata` parsed (#2958), so a long turn reads as a one-off auto-compaction rather than a degraded agent. `get_fan_out_result` polls a BATCH — `GET /api/agents/{name}/fan-out/{fan_out_id}`, the only surface that answers while it is still running (#2670). `search_executions` (ent#653) proxies the entitlement-gated `/api/enterprise/execution-search` route (`mode=substring` or `mode=regex`, the latter PostgreSQL-only): a per-tool `canAccess` ALLOW-LIST of `system`/`user` scope (agent-scoped sessions never see it, and the backend refuses them regardless) and an honest `available:false` envelope when the module is absent (404) or unentitled (403) — never "no results" | diff --git a/docs/memory/feature-flows/credential-injection.md b/docs/memory/feature-flows/credential-injection.md index a1aa93bce..efebda978 100644 --- a/docs/memory/feature-flows/credential-injection.md +++ b/docs/memory/feature-flows/credential-injection.md @@ -183,26 +183,16 @@ const exportToGit = async () => { ### Backend (`src/backend/routers/credentials.py`) ```python -@router.post("/agents/{agent_name}/credentials/export", response_model=CredentialExportResponse) -async def export_credentials(agent_name: str, current_user: User = Depends(get_current_user)): - """Export agent credentials to encrypted file in workspace.""" - encryption_service = CredentialEncryptionService() - - # Read credential files from agent - file_paths = ['.env', '.mcp.json'] - files = await encryption_service.read_agent_credential_files(agent_name, file_paths) - - # Encrypt and write to agent - encrypted = encryption_service.encrypt(files) - await encryption_service.write_agent_credential_files( - agent_name, {'.credentials.enc': encrypted} - ) - - return CredentialExportResponse( - status="success", - files_exported=len(files), - encrypted_file=".credentials.enc" - ) +@router.post("/agents/{agent_name}/credentials/export") +async def export_credentials( + request: Request, + agent_name: str = Depends(get_owned_agent_by_name), # owner or admin, else uniform 404 (#186); connector -> 403 + current_user: User = Depends(get_current_user) +): + reject_agent_principal(current_user) # agent-scoped key -> 403 "human-only" (ent#69 Part 2) + # container missing -> 404; not running -> 400 + encrypted_file, files_exported = await get_credential_encryption_service().export_to_agent(agent_name) + # audited (SEC-001); agent server unreachable -> 503 ``` ### Encryption Service (`src/backend/services/credential_encryption.py`) @@ -260,30 +250,16 @@ const importFromGit = async () => { ### Backend (`src/backend/routers/credentials.py`) ```python -@router.post("/agents/{agent_name}/credentials/import", response_model=CredentialImportResponse) -async def import_credentials(agent_name: str, current_user: User = Depends(get_current_user)): - """Import credentials from encrypted file in agent workspace.""" - encryption_service = CredentialEncryptionService() - - # Read encrypted file - enc_files = await encryption_service.read_agent_credential_files( - agent_name, ['.credentials.enc'] - ) - - if '.credentials.enc' not in enc_files: - raise HTTPException(status_code=404, detail="No .credentials.enc file found") - - # Decrypt - files = encryption_service.decrypt(enc_files['.credentials.enc']) - - # Write decrypted files to agent - await encryption_service.write_agent_credential_files(agent_name, files) - - return CredentialImportResponse( - status="success", - files_imported=list(files.keys()), - message=f"Imported {len(files)} file(s)" - ) +@router.post("/agents/{agent_name}/credentials/import") +async def import_credentials( + request: Request, + agent_name: str = Depends(get_owned_agent_by_name), # owner or admin, else uniform 404 (#186); connector -> 403 + current_user: User = Depends(get_current_user) +): + reject_agent_principal(current_user) # agent-scoped key -> 403 "human-only" (ent#69 Part 2) + # container missing -> 404; not running -> 400 + files = await get_credential_encryption_service().import_to_agent(agent_name) + # missing .credentials.enc / wrong key -> 400; agent server unreachable -> 503; audited (SEC-001) ``` --- @@ -330,6 +306,8 @@ async def decrypt_and_inject(request: InternalDecryptInjectRequest): ## MCP Tools +**Who sees which tool (#3435).** `export_credentials`, `import_credentials` and `get_credential_encryption_key` each carry a per-tool `canAccess` allow-list `{user, system}` — exactly the scopes their backend gates admit — so an agent-scoped session neither lists nor can call them (FastMCP answers "not found"). Their descriptions name the caller class (owner or admin for export/import, administrator for the key). A backend 403 comes back as `{success:false, error, human_only|admin_only|not_authorized}` through `accessDenied`, so the audit row reads `denied` (#2807); 400/404/503 are still thrown. No backend gate changed. `inject_credentials` is still advertised to agent sessions (shrink-only baseline in `tool-visibility.test.ts`). + ### inject_credentials ```typescript // src/mcp-server/src/tools/agents.ts diff --git a/docs/user-docs/credentials/credential-management.md b/docs/user-docs/credentials/credential-management.md index e73d11ddf..caba84b1a 100644 --- a/docs/user-docs/credentials/credential-management.md +++ b/docs/user-docs/credentials/credential-management.md @@ -170,9 +170,11 @@ The inject, export, import, checklist, and drift routes are owner-only **and hum - `get_credential_status(name)` -- Check credential file status. - `inject_credentials(name, credentials)` -- Inject credentials into the agent. -- `export_credentials(name)` -- Export credentials to encrypted file. -- `import_credentials(name)` -- Import credentials from encrypted file. -- `get_credential_encryption_key()` -- Retrieve the encryption key. +- `export_credentials(name)` -- Export credentials to encrypted file. The agent's owner or an admin only, with a user-scoped key. +- `import_credentials(name)` -- Import credentials from encrypted file. The agent's owner or an admin only, with a user-scoped key. +- `get_credential_encryption_key()` -- Retrieve the encryption key. An administrator only, with a user-scoped key. + +These three are human-only: an agent-scoped key can never call them, so they are not listed in an agent's MCP session at all. If one of them is refused, the tool returns `{"success": false, "error": "…"}` with `human_only`, `admin_only` or `not_authorized` set, rather than an error string. - `list_available_credentials()` / `fetch_credential(name)` -- Read a granted vault credential at runtime (see [Credential Vault](#credential-vault)). ## See Also diff --git a/docs/user-docs/faq/credentials-and-subscriptions.md b/docs/user-docs/faq/credentials-and-subscriptions.md index e38aaf3a2..281c08d76 100644 --- a/docs/user-docs/faq/credentials-and-subscriptions.md +++ b/docs/user-docs/faq/credentials-and-subscriptions.md @@ -140,7 +140,7 @@ The encryption key (`CREDENTIAL_ENCRYPTION_KEY`) rotates online, with no downtim ## How do I move an agent's credentials to another Trinity instance? -Export the credentials to a `.credentials.enc` file (**Export to Git** on the Credentials tab, or the `export_credentials` MCP tool), bring that file to the agent's workspace on the target instance, and import it there (**Import from Git** on the Credentials tab, or the `import_credentials` MCP tool). The catch is the encryption key: the file only decrypts if the target instance uses the same `CREDENTIAL_ENCRYPTION_KEY` as the source. An admin can retrieve the key from the source instance (via the encryption-key API endpoint or the `get_credential_encryption_key` MCP tool) and configure it on the target before importing. See [Credential Management](../credentials/credential-management.md). +Export the credentials to a `.credentials.enc` file (**Export to Git** on the Credentials tab, or the `export_credentials` MCP tool), bring that file to the agent's workspace on the target instance, and import it there (**Import from Git** on the Credentials tab, or the `import_credentials` MCP tool). The catch is the encryption key: the file only decrypts if the target instance uses the same `CREDENTIAL_ENCRYPTION_KEY` as the source. An admin can retrieve the key from the source instance (via the encryption-key API endpoint or the `get_credential_encryption_key` MCP tool). On the target, set `CREDENTIAL_ENCRYPTION_KEY=` in the instance's `.env` and restart the backend, then import. All three MCP tools are human-only: call them with a user-scoped MCP key (the agent's owner or an admin for export and import, an administrator for the key) — an agent's own key can never call them, and they are not listed in an agent's session. See [Credential Management](../credentials/credential-management.md). ## How do I know which credentials an agent actually needs? From b475f4ccad3f853035c621566a892cf78f10676e Mon Sep 17 00:00:00 2001 From: "Trinity Agent (trinity)" Date: Fri, 9 Oct 2026 13:59:45 -0400 Subject: [PATCH 05/10] refactor(mcp): a non-JSON 403 on a credential tool returns a fixed sentence, never the body (I1) (#3435) Co-Authored-By: Claude Opus 5.5 --- .../src/tools/agents.credentials.test.ts | 17 +++++++++++++---- src/mcp-server/src/tools/agents.ts | 7 ++++--- 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/src/mcp-server/src/tools/agents.credentials.test.ts b/src/mcp-server/src/tools/agents.credentials.test.ts index 0de469e44..cb8baf949 100644 --- a/src/mcp-server/src/tools/agents.credentials.test.ts +++ b/src/mcp-server/src/tools/agents.credentials.test.ts @@ -111,11 +111,20 @@ describe("#3435 credential tools: execution", () => { const out = JSON.parse(await admin.byName.get_credential_encryption_key.execute({}, { session: { scope: "user" } })); assert.deepEqual(out, { success: false, error: "Admin access required", admin_only: true }); - const other = build(async () => { throw new ApiError(403, "not json"); }); + const other = build(async () => { throw new ApiError(403, JSON.stringify({ detail: "Forbidden" })); }); const o2 = JSON.parse(await other.byName.import_credentials.execute(args("import_credentials"), {})); - assert.equal(o2.success, false); - assert.equal(o2.not_authorized, true); - assert.match(o2.error, /not json/); + assert.deepEqual(o2, { success: false, error: "Forbidden", not_authorized: true }); + }); + + it("a non-JSON 403 body is never copied into the envelope — a fixed sentence instead", async () => { + const MARKER = "PROXY-PAGE-MARKER-3435"; + const body = `

403 Forbidden

${MARKER}

`; + const { byName } = build(async () => { throw new ApiError(403, body); }); + for (const t of TOOLS) { + const raw = await byName[t].execute(args(t), { session: { scope: "user" } }); + assert.ok(!raw.includes(MARKER), `${t}: the 403 body leaked into the envelope: ${raw}`); + assert.deepEqual(JSON.parse(raw), { success: false, error: "Access denied (HTTP 403)", not_authorized: true }); + } }); it("400, 404 and 503 stay thrown, and so does a non-API failure (TD-4)", async () => { diff --git a/src/mcp-server/src/tools/agents.ts b/src/mcp-server/src/tools/agents.ts index e9aa2b6b5..45765c192 100644 --- a/src/mcp-server/src/tools/agents.ts +++ b/src/mcp-server/src/tools/agents.ts @@ -51,16 +51,17 @@ const credentialToolAccess = (auth: any): boolean => CREDENTIAL_TOOL_SCOPES.has( /** * #3435: a backend 403 on a credential tool, as a typed envelope stamped * `denied` on the audit row (#2807). Only the backend `detail` is copied — never - * a body. Every other failure (400/404/503, transport) is rethrown unchanged. + * a body: a 403 without a JSON `detail` (a proxy or WAF page) gets a fixed + * sentence. Every other failure (400/404/503, transport) is rethrown unchanged. */ function credentialRefusal(error: unknown, context: { session?: McpAuthContext } | undefined): string { if (!(error instanceof ApiError) || error.status !== 403) throw error; - let detail = error.message; + let detail = "Access denied (HTTP 403)"; try { const parsed = JSON.parse(error.body); if (typeof parsed?.detail === "string") detail = parsed.detail; } catch { - // not JSON — keep the message + // not JSON — keep the fixed sentence; the body is never echoed } const flag = /human-only/i.test(detail) ? "human_only" From cf47dd7b0fe806c834821cdd688f3962eccd0386 Mon Sep 17 00:00:00 2001 From: "Trinity Agent (trinity)" Date: Fri, 9 Oct 2026 14:00:01 -0400 Subject: [PATCH 06/10] refactor(mcp): credential tool descriptions and docs name the system-scoped key the allow-list admits (I2) (#3435) Co-Authored-By: Claude Opus 5.5 --- docs/user-docs/credentials/credential-management.md | 6 +++--- docs/user-docs/faq/credentials-and-subscriptions.md | 2 +- src/mcp-server/src/tools/agents.credentials.test.ts | 1 + src/mcp-server/src/tools/agents.ts | 6 +++--- 4 files changed, 8 insertions(+), 7 deletions(-) diff --git a/docs/user-docs/credentials/credential-management.md b/docs/user-docs/credentials/credential-management.md index caba84b1a..8faeb80b1 100644 --- a/docs/user-docs/credentials/credential-management.md +++ b/docs/user-docs/credentials/credential-management.md @@ -170,9 +170,9 @@ The inject, export, import, checklist, and drift routes are owner-only **and hum - `get_credential_status(name)` -- Check credential file status. - `inject_credentials(name, credentials)` -- Inject credentials into the agent. -- `export_credentials(name)` -- Export credentials to encrypted file. The agent's owner or an admin only, with a user-scoped key. -- `import_credentials(name)` -- Import credentials from encrypted file. The agent's owner or an admin only, with a user-scoped key. -- `get_credential_encryption_key()` -- Retrieve the encryption key. An administrator only, with a user-scoped key. +- `export_credentials(name)` -- Export credentials to encrypted file. The agent's owner or an admin only, with a user-scoped or system-scoped key. +- `import_credentials(name)` -- Import credentials from encrypted file. The agent's owner or an admin only, with a user-scoped or system-scoped key. +- `get_credential_encryption_key()` -- Retrieve the encryption key. An administrator only, with a user-scoped or system-scoped key. These three are human-only: an agent-scoped key can never call them, so they are not listed in an agent's MCP session at all. If one of them is refused, the tool returns `{"success": false, "error": "…"}` with `human_only`, `admin_only` or `not_authorized` set, rather than an error string. - `list_available_credentials()` / `fetch_credential(name)` -- Read a granted vault credential at runtime (see [Credential Vault](#credential-vault)). diff --git a/docs/user-docs/faq/credentials-and-subscriptions.md b/docs/user-docs/faq/credentials-and-subscriptions.md index 281c08d76..0bdd0af53 100644 --- a/docs/user-docs/faq/credentials-and-subscriptions.md +++ b/docs/user-docs/faq/credentials-and-subscriptions.md @@ -140,7 +140,7 @@ The encryption key (`CREDENTIAL_ENCRYPTION_KEY`) rotates online, with no downtim ## How do I move an agent's credentials to another Trinity instance? -Export the credentials to a `.credentials.enc` file (**Export to Git** on the Credentials tab, or the `export_credentials` MCP tool), bring that file to the agent's workspace on the target instance, and import it there (**Import from Git** on the Credentials tab, or the `import_credentials` MCP tool). The catch is the encryption key: the file only decrypts if the target instance uses the same `CREDENTIAL_ENCRYPTION_KEY` as the source. An admin can retrieve the key from the source instance (via the encryption-key API endpoint or the `get_credential_encryption_key` MCP tool). On the target, set `CREDENTIAL_ENCRYPTION_KEY=` in the instance's `.env` and restart the backend, then import. All three MCP tools are human-only: call them with a user-scoped MCP key (the agent's owner or an admin for export and import, an administrator for the key) — an agent's own key can never call them, and they are not listed in an agent's session. See [Credential Management](../credentials/credential-management.md). +Export the credentials to a `.credentials.enc` file (**Export to Git** on the Credentials tab, or the `export_credentials` MCP tool), bring that file to the agent's workspace on the target instance, and import it there (**Import from Git** on the Credentials tab, or the `import_credentials` MCP tool). The catch is the encryption key: the file only decrypts if the target instance uses the same `CREDENTIAL_ENCRYPTION_KEY` as the source. An admin can retrieve the key from the source instance (via the encryption-key API endpoint or the `get_credential_encryption_key` MCP tool). On the target, set `CREDENTIAL_ENCRYPTION_KEY=` in the instance's `.env` and restart the backend, then import. All three MCP tools are human-only: call them with a user-scoped or system-scoped MCP key (the agent's owner or an admin for export and import, an administrator for the key) — an agent's own key can never call them, and they are not listed in an agent's session. See [Credential Management](../credentials/credential-management.md). ## How do I know which credentials an agent actually needs? diff --git a/src/mcp-server/src/tools/agents.credentials.test.ts b/src/mcp-server/src/tools/agents.credentials.test.ts index cb8baf949..171120b90 100644 --- a/src/mcp-server/src/tools/agents.credentials.test.ts +++ b/src/mcp-server/src/tools/agents.credentials.test.ts @@ -75,6 +75,7 @@ describe("#3435 credential tools: advertisement", () => { it("each description names who may call it, and stays under the 2,048 cap", () => { for (const t of TOOLS) { assert.match(byName[t].description, /agent-scoped keys? (?:can never|cannot)/i, `${t}: no caller class`); + assert.match(byName[t].description, /user-scoped or system-scoped key/i, `${t}: must name both admitted key scopes`); assert.ok(byName[t].description.length <= 2048, `${t}: ${byName[t].description.length} chars`); } assert.match(byName.get_credential_encryption_key.description, /administrator/i); diff --git a/src/mcp-server/src/tools/agents.ts b/src/mcp-server/src/tools/agents.ts index 45765c192..da1dc1686 100644 --- a/src/mcp-server/src/tools/agents.ts +++ b/src/mcp-server/src/tools/agents.ts @@ -658,7 +658,7 @@ export function createAgentTools( "to the agent's workspace. This file can be committed to git for portable credential storage. " + "Requires CREDENTIAL_ENCRYPTION_KEY to be configured on the backend. " + "The agent must be running. " + - "WHO MAY CALL: the agent's owner or an admin, with a user-scoped key or a signed-in session; " + + "WHO MAY CALL: the agent's owner or an admin, with a user-scoped or system-scoped key, or a signed-in session; " + "agent-scoped keys can never call it (human-only), so agent sessions are not shown it. " + "A refusal returns {success:false, error, human_only|not_authorized}.", parameters: z.object({ @@ -692,7 +692,7 @@ export function createAgentTools( "This is useful after cloning an agent's git repo. " + "Requires CREDENTIAL_ENCRYPTION_KEY to be configured on the backend (the same key " + "that encrypted the file). The agent must be running. " + - "WHO MAY CALL: the agent's owner or an admin, with a user-scoped key or a signed-in session; " + + "WHO MAY CALL: the agent's owner or an admin, with a user-scoped or system-scoped key, or a signed-in session; " + "agent-scoped keys can never call it (human-only), so agent sessions are not shown it. " + "A refusal returns {success:false, error, human_only|not_authorized}.", parameters: z.object({ @@ -782,7 +782,7 @@ export function createAgentTools( "enabling portable agents that work both locally and on Trinity. " + "Returns the key as a hex string (64 chars for AES-256). " + "SECURITY: Store securely and never commit to git. " + - "WHO MAY CALL: an administrator only, with a user-scoped key or a signed-in session; " + + "WHO MAY CALL: an administrator only, with a user-scoped or system-scoped key, or a signed-in session; " + "agent-scoped keys can never call it (human-only), so agent sessions are not shown it. " + "A refusal returns {success:false, error, admin_only|human_only}.", parameters: z.object({}), From 611e4ce06ab948897e58d26f3900d105c702f933 Mon Sep 17 00:00:00 2001 From: "Trinity Agent (trinity)" Date: Fri, 9 Oct 2026 14:00:19 -0400 Subject: [PATCH 07/10] docs(mcp): the human-only note follows the MCP tool list instead of splitting it (I3) (#3435) Co-Authored-By: Claude Opus 5.5 --- docs/user-docs/credentials/credential-management.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/user-docs/credentials/credential-management.md b/docs/user-docs/credentials/credential-management.md index 8faeb80b1..5529083e7 100644 --- a/docs/user-docs/credentials/credential-management.md +++ b/docs/user-docs/credentials/credential-management.md @@ -174,9 +174,10 @@ The inject, export, import, checklist, and drift routes are owner-only **and hum - `import_credentials(name)` -- Import credentials from encrypted file. The agent's owner or an admin only, with a user-scoped or system-scoped key. - `get_credential_encryption_key()` -- Retrieve the encryption key. An administrator only, with a user-scoped or system-scoped key. -These three are human-only: an agent-scoped key can never call them, so they are not listed in an agent's MCP session at all. If one of them is refused, the tool returns `{"success": false, "error": "…"}` with `human_only`, `admin_only` or `not_authorized` set, rather than an error string. - `list_available_credentials()` / `fetch_credential(name)` -- Read a granted vault credential at runtime (see [Credential Vault](#credential-vault)). +`export_credentials`, `import_credentials` and `get_credential_encryption_key` are human-only: an agent-scoped key can never call them, so they are not listed in an agent's MCP session at all. If one of them is refused, the tool returns `{"success": false, "error": "…"}` with `human_only`, `admin_only` or `not_authorized` set, rather than an error string. + ## See Also - [Agent Configuration](../agents/agent-configuration.md) From 9ae9c3a875fee2568ac204c5d24abc31823c5345 Mon Sep 17 00:00:00 2001 From: "Trinity Agent (trinity)" Date: Fri, 9 Oct 2026 14:00:31 -0400 Subject: [PATCH 08/10] docs(mcp): keep the MCP tool list tight after moving the human-only note (I3) (#3435) Co-Authored-By: Claude Opus 5.5 --- docs/user-docs/credentials/credential-management.md | 1 - 1 file changed, 1 deletion(-) diff --git a/docs/user-docs/credentials/credential-management.md b/docs/user-docs/credentials/credential-management.md index 5529083e7..860b740a3 100644 --- a/docs/user-docs/credentials/credential-management.md +++ b/docs/user-docs/credentials/credential-management.md @@ -173,7 +173,6 @@ The inject, export, import, checklist, and drift routes are owner-only **and hum - `export_credentials(name)` -- Export credentials to encrypted file. The agent's owner or an admin only, with a user-scoped or system-scoped key. - `import_credentials(name)` -- Import credentials from encrypted file. The agent's owner or an admin only, with a user-scoped or system-scoped key. - `get_credential_encryption_key()` -- Retrieve the encryption key. An administrator only, with a user-scoped or system-scoped key. - - `list_available_credentials()` / `fetch_credential(name)` -- Read a granted vault credential at runtime (see [Credential Vault](#credential-vault)). `export_credentials`, `import_credentials` and `get_credential_encryption_key` are human-only: an agent-scoped key can never call them, so they are not listed in an agent's MCP session at all. If one of them is refused, the tool returns `{"success": false, "error": "…"}` with `human_only`, `admin_only` or `not_authorized` set, rather than an error string. From c547fb4ebb66898f5add60a72c116bb1224b7f6c Mon Sep 17 00:00:00 2001 From: "Trinity Agent (trinity)" Date: Fri, 9 Oct 2026 14:37:05 -0400 Subject: [PATCH 09/10] test(mcp): credential gate pins patch the module the running route reads (#3435) test_inject_assigned_credentials.py replaces sys.modules["services.credential_encryption"] at collection time, and the export/import handlers resolve get_credential_encryption_service through a function-local import at request time. In a shared xdist worker the stub this file installed on its import-time alias was never read, the real service ran, and every 200-path case got a 503 ("Failed to connect to agent"). The fixture now patches the module currently in sys.modules, overrides the exact get_current_user object the router's Depends() holds, and stubs the db the ownership dependency closes over. Same principals, real router and dependencies, same 25 cases; no backend change. Co-Authored-By: Claude Opus 5.5 --- tests/unit/test_3435_credential_tool_gates.py | 24 +++++++++++++------ 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/tests/unit/test_3435_credential_tool_gates.py b/tests/unit/test_3435_credential_tool_gates.py index 832fa6e78..d1895a48b 100644 --- a/tests/unit/test_3435_credential_tool_gates.py +++ b/tests/unit/test_3435_credential_tool_gates.py @@ -21,16 +21,14 @@ literal "human-only" in the 403 detail, so that string is pinned exactly. """ +import importlib from unittest.mock import AsyncMock, MagicMock import pytest from fastapi import FastAPI from fastapi.testclient import TestClient -import dependencies import routers.credentials as rc -import services.credential_encryption as ce -from dependencies import get_current_user from models import User _AGENT = "acme-bot" @@ -77,15 +75,24 @@ def _ops_key_of_admin(): @pytest.fixture def harness(monkeypatch): """Real router, real owner/admin dependencies; only the principal, the DB - ownership lookups, Docker and the encryption service are stubbed.""" + ownership lookups, Docker and the encryption service are stubbed. + + Every stub lands on the object the running route actually reads, never on + a name this file bound at import: sibling unit files replace + `sys.modules["services.credential_encryption"]` at collection time, and the + handlers resolve that module by a function-local import at request time — + patching an import-time alias then leaves the real service running.""" state = {"user": _owner_jwt()} app = FastAPI() app.include_router(rc.router) - app.dependency_overrides[get_current_user] = lambda: state["user"] + # The exact dependency object the router's Depends() holds. + app.dependency_overrides[rc.get_current_user] = lambda: state["user"] - monkeypatch.setattr(dependencies.db, "get_agent_owner", lambda name: "owner" if name == _AGENT else None) + # The `db` the ownership dependency closes over. + owner_db = rc.get_owned_agent_by_name.__globals__["db"] + monkeypatch.setattr(owner_db, "get_agent_owner", lambda name: "owner" if name == _AGENT else None) monkeypatch.setattr( - dependencies.db, "can_user_share_agent", lambda username, name: username in ("owner", "admin") + owner_db, "can_user_share_agent", lambda username, name: username in ("owner", "admin") ) container = MagicMock() monkeypatch.setattr(rc, "get_agent_container", lambda name: container) @@ -97,6 +104,9 @@ def harness(monkeypatch): service = MagicMock() service.export_to_agent = AsyncMock(return_value=("/home/developer/.credentials.enc", 2)) service.import_to_agent = AsyncMock(return_value={".env": "K=V", ".mcp.json": "{}"}) + # The module object `from services.credential_encryption import ...` inside + # the handlers resolves to now (sys.modules), not an import-time alias. + ce = importlib.import_module("services.credential_encryption") monkeypatch.setattr(ce, "get_credential_encryption_service", lambda: service) monkeypatch.setenv("CREDENTIAL_ENCRYPTION_KEY", "ab" * 32) From ef7d22fa735aa8b2d408e1300a224b9cdb339e4d Mon Sep 17 00:00:00 2001 From: trinity-ability <309458136+trinity-ability@users.noreply.github.com> Date: Sat, 10 Oct 2026 09:58:09 +0100 Subject: [PATCH 10/10] docs(faq): import a foreign credentials file via the secondary key, never by replacing the target's key (#3435) The migration entry told operators to set the source instance's key as CREDENTIAL_ENCRYPTION_KEY on the target. On a target that already holds encrypted secrets that strands every one of them. The decrypt path already falls back to CREDENTIAL_ENCRYPTION_KEY_SECONDARY, so that is the safe slot. Co-Authored-By: Claude Opus 5.5 --- docs/user-docs/faq/credentials-and-subscriptions.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/user-docs/faq/credentials-and-subscriptions.md b/docs/user-docs/faq/credentials-and-subscriptions.md index 0bdd0af53..7d7dc5e37 100644 --- a/docs/user-docs/faq/credentials-and-subscriptions.md +++ b/docs/user-docs/faq/credentials-and-subscriptions.md @@ -140,7 +140,7 @@ The encryption key (`CREDENTIAL_ENCRYPTION_KEY`) rotates online, with no downtim ## How do I move an agent's credentials to another Trinity instance? -Export the credentials to a `.credentials.enc` file (**Export to Git** on the Credentials tab, or the `export_credentials` MCP tool), bring that file to the agent's workspace on the target instance, and import it there (**Import from Git** on the Credentials tab, or the `import_credentials` MCP tool). The catch is the encryption key: the file only decrypts if the target instance uses the same `CREDENTIAL_ENCRYPTION_KEY` as the source. An admin can retrieve the key from the source instance (via the encryption-key API endpoint or the `get_credential_encryption_key` MCP tool). On the target, set `CREDENTIAL_ENCRYPTION_KEY=` in the instance's `.env` and restart the backend, then import. All three MCP tools are human-only: call them with a user-scoped or system-scoped MCP key (the agent's owner or an admin for export and import, an administrator for the key) — an agent's own key can never call them, and they are not listed in an agent's session. See [Credential Management](../credentials/credential-management.md). +Export the credentials to a `.credentials.enc` file (**Export to Git** on the Credentials tab, or the `export_credentials` MCP tool), bring that file to the agent's workspace on the target instance, and import it there (**Import from Git** on the Credentials tab, or the `import_credentials` MCP tool). The catch is the encryption key: the file only decrypts if the target instance can decrypt with the source's `CREDENTIAL_ENCRYPTION_KEY`. An admin can retrieve the key from the source instance (via the encryption-key API endpoint or the `get_credential_encryption_key` MCP tool). On a target that already has its own key, do **not** replace it — everything that instance has already encrypted would stop decrypting. Set the source key as `CREDENTIAL_ENCRYPTION_KEY_SECONDARY=` (a decrypt-only fallback) in the target's `.env` and restart the backend, then import; remove the secondary once the import is done. Only on a brand-new target with nothing encrypted yet can you set it as `CREDENTIAL_ENCRYPTION_KEY` itself. All three MCP tools are human-only: call them with a user-scoped or system-scoped MCP key (the agent's owner or an admin for export and import, an administrator for the key) — an agent's own key can never call them, and they are not listed in an agent's session. See [Credential Management](../credentials/credential-management.md). ## How do I know which credentials an agent actually needs?