From 5ed0b74b34f3265abcc5c16c61d5a94d6be0183d Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Wed, 22 Jul 2026 07:01:57 -0500 Subject: [PATCH] fix(runtime): scope PAPERCLIP_ env-binding strip to reserved keys (#9974) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Agent runs get their environment from user/adapter/project/routine env bindings resolved by the server heartbeat, plus `PAPERCLIP_*` runtime vars (identity, wake, workspace, API access) injected by the harness > - The heartbeat stripped **every** `PAPERCLIP_`-prefixed binding before resolution, so legitimately user-named keys (e.g. cloud provider token bindings like `PAPERCLIP_CLOUD_PROD_PROVIDER_RAILWAY_*`) were silently dropped and never reached the run env > - At the same time, several adapters honored an explicitly configured `PAPERCLIP_API_KEY` over the harness-minted run token, which is exactly the one key config must never control > - This pull request replaces the blanket prefix strip with a precise three-rule policy: never accept `PAPERCLIP_API_KEY` from config, always let harness-assigned runtime vars win, and let every other `PAPERCLIP_*`-named user binding flow through > - The benefit is that user secrets with a `PAPERCLIP_`-style name work like any other binding, while runtime identity and API credentials stay fully harness-controlled ## Linked Issues or Issue Description **Bug description** (no public issue exists): - **What happened:** Env bindings whose key starts with `PAPERCLIP_` (e.g. a cloud provider token a user deliberately named `PAPERCLIP_CLOUD_PROD_PROVIDER_RAILWAY_TOKEN`) were silently stripped by the server before secret resolution, so the spawned agent never received them. No error, no access event — the variable just never appeared. - **Expected behavior:** A user-named `PAPERCLIP_*` binding should reach the run env unless the harness itself uses that key. Only `PAPERCLIP_API_KEY` should be categorically rejected, and harness-assigned runtime vars (`PAPERCLIP_RUN_ID`, `PAPERCLIP_AGENT_ID`, wake/workspace vars, …) should always win over config. - **Steps to reproduce:** Configure an agent/project env binding named `PAPERCLIP_` (plain or secret_ref), run a heartbeat, and inspect the spawned process env — the key is absent. - **Deployment mode:** local server, any local adapter. Related prior PRs (different, save-time/API-layer blanket-ban approach; this PR supersedes that direction with a runtime allow-except-reserved policy): Refs #8239, Refs #8439. ## What Changed - `server/src/services/heartbeat.ts`: the pre-resolution strip now removes only `PAPERCLIP_API_KEY` (hard denylist) instead of every `PAPERCLIP_`-prefixed binding; other `PAPERCLIP_*` keys flow into binding resolution. Low-trust inline-sensitive-env checks now also cover those keys. - `packages/adapter-utils/src/server-utils.ts`: new `isForbiddenConfigEnvKey()` helper; the shared `refreshPaperclipWorkspaceEnvForExecution` merge drops `PAPERCLIP_API_KEY` from config and keeps harness-assigned `PAPERCLIP_*` keys authoritative. - `packages/adapter-utils/src/acpx-engine/execute.ts`: removed the explicit-`PAPERCLIP_API_KEY`-from-config allowance; the run token (`authToken`) is now always applied; config `PAPERCLIP_API_KEY` is ignored. - All local adapters (`claude-local`, `codex-local`, `cursor-local`, `gemini-local`, `grok-local`, `opencode-local`, `pi-local`) plus `cursor-cloud`, `hermes`, and the server `process` adapter: removed `hasExplicitApiKey`-style allowances so the harness token always wins, and guarded the remaining unguarded env-merge loops (claude-local inline loop, process adapter) with the same policy. - Tests updated/added: heartbeat binding-strip test now asserts the three-rule policy; adapter-utils merge tests assert the `PAPERCLIP_API_KEY` ban and `PAPERCLIP_*` pass-through; acpx engine tests moved credential fixtures to `authToken` and assert config `PAPERCLIP_API_KEY` is ignored while other `PAPERCLIP_*` config keys forward and still bust the session fingerprint on rotation. ## Verification - `pnpm vitest run packages/adapter-utils/src/server-utils.test.ts packages/adapter-utils/src/acpx-engine/execute.test.ts` — 127 passed - `pnpm vitest run server/src/__tests__/heartbeat-project-env.test.ts server/src/__tests__/heartbeat-local-environment.test.ts server/src/__tests__/claude-local-execute.test.ts server/src/__tests__/codex-local-execute.test.ts server/src/__tests__/cursor-local-execute.test.ts server/src/__tests__/gemini-local-execute.test.ts` — 68 passed - Adapter package execute suites and the server tests touching API-key fixtures (`heartbeat-run-log`, `redaction`, `effective-run-config-fingerprints`, `agent-permissions-routes`) — green. Three pre-existing sandbox/SSH fixture failures reproduce identically on clean `master` on this host and are unrelated. - `pnpm --filter typecheck` for server, adapter-utils, and all nine touched adapter packages — all pass. ## Risks - Behavioral change: a deployment that relied on configuring a static `PAPERCLIP_API_KEY` in adapter config env loses that override — by design; the harness-minted run token is now the only source. When no run token exists, no API key is injected at all. - `PAPERCLIP_*`-named user bindings now reach binding resolution and run envs; a key that collides with a harness runtime var is still discarded at merge time, so runtime identity/wake/workspace vars cannot be spoofed. - Low risk otherwise: no migrations, no API surface changes. ## Model Used - Claude Fable 5 (`claude-fable-5`, Anthropic Claude 5 family, Mythos-class tier), extended thinking enabled, agentic tool use (file edits, shell, test runner) via Claude Agent SDK. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Paperclip Co-authored-by: Claude Fable 5 --- .../src/acpx-engine/execute.test.ts | 55 +++++++------------ .../adapter-utils/src/acpx-engine/execute.ts | 13 +++-- .../adapter-utils/src/server-utils.test.ts | 23 ++++++-- packages/adapter-utils/src/server-utils.ts | 16 +++++- .../claude-local/src/server/execute.ts | 14 +++-- .../codex-local/src/server/execute.ts | 4 +- .../cursor-cloud/src/server/execute.ts | 5 +- .../cursor-local/src/server/execute.ts | 4 +- .../gemini-local/src/server/execute.ts | 4 +- .../adapters/grok-local/src/server/execute.ts | 4 +- .../hermes/src/server/execute.onspawn.test.ts | 18 ++++++ .../adapters/hermes/src/server/execute.ts | 4 +- .../opencode-local/src/server/execute.ts | 4 +- .../adapters/pi-local/src/server/execute.ts | 4 +- .../__tests__/heartbeat-project-env.test.ts | 20 +++++-- server/src/adapters/process/execute.ts | 16 +++++- server/src/adapters/utils.ts | 2 + server/src/services/heartbeat.ts | 30 ++++++---- 18 files changed, 149 insertions(+), 91 deletions(-) diff --git a/packages/adapter-utils/src/acpx-engine/execute.test.ts b/packages/adapter-utils/src/acpx-engine/execute.test.ts index 56e41dc6fe..c737e67127 100644 --- a/packages/adapter-utils/src/acpx-engine/execute.test.ts +++ b/packages/adapter-utils/src/acpx-engine/execute.test.ts @@ -633,16 +633,8 @@ describe("shared ACPX engine runtime behavior", () => { stateDir, }; - await runExecutor({ - ...baseConfig, - agent: "custom-a", - env: { PAPERCLIP_API_KEY: "old-key" }, - }); - await runExecutor({ - ...baseConfig, - agent: "custom-b", - env: { PAPERCLIP_API_KEY: "new-key" }, - }); + await runExecutor({ ...baseConfig, agent: "custom-a" }, { authToken: "old-key" }); + await runExecutor({ ...baseConfig, agent: "custom-b" }, { authToken: "new-key" }); const wrappers = await fs.readdir(path.join(stateDir, "wrappers")); expect(wrappers.filter((name) => name.endsWith(".sh"))).toHaveLength(2); @@ -677,6 +669,10 @@ describe("shared ACPX engine runtime behavior", () => { OPENROUTER_API_KEY: "resolved-secret-value", // Reserved-namespace config keys must not clobber runtime identity/wake. PAPERCLIP_TASK_ID: "attacker-issue", + // PAPERCLIP_API_KEY is never accepted from config. + PAPERCLIP_API_KEY: "config-key", + // A PAPERCLIP_*-named key the harness does not assign flows through. + PAPERCLIP_CLOUD_PROVIDER_TOKEN: "cloud-token", }, }, { @@ -694,6 +690,11 @@ describe("shared ACPX engine runtime behavior", () => { // Runtime PAPERCLIP_TASK_ID (from the wake context) wins over config. expect(env).toContain("PAPERCLIP_TASK_ID='issue-real'"); expect(env).not.toContain("attacker-issue"); + // The harness-minted run token is the only PAPERCLIP_API_KEY source. + expect(env).toContain("PAPERCLIP_API_KEY='runtime-secret-token'"); + expect(env).not.toContain("config-key"); + // A PAPERCLIP_*-named user key the harness does not assign passes through. + expect(env).toContain("PAPERCLIP_CLOUD_PROVIDER_TOKEN='cloud-token'"); }); it("busts the session fingerprint when resolved adapter env changes but not across wakes", async () => { @@ -731,17 +732,17 @@ describe("shared ACPX engine runtime behavior", () => { const stateDir = path.join(root, "state"); const baseConfig = { agentCommand: "node ./fake-acp.js", stateDir }; - // An explicitly configured PAPERCLIP_API_KEY is stable per-run config (not a - // per-wake runtime var): rotating it must invalidate a warm/resumable session - // so the next launch sources the new key, even across an otherwise-identical - // wake context. + // A configured PAPERCLIP_*-named value the harness does not assign (e.g. a + // cloud provider token binding) is stable per-run config: rotating it must + // invalidate a warm/resumable session so the next launch sources the new + // value, even across an otherwise-identical wake context. const context = { taskId: "issue-1", wakeReason: "issue_assigned" }; const withKey = await runExecutor( - { ...baseConfig, env: { PAPERCLIP_API_KEY: "explicit-key-1" } }, + { ...baseConfig, env: { PAPERCLIP_CLOUD_PROVIDER_TOKEN: "explicit-key-1" } }, { context }, ); const rotatedKey = await runExecutor( - { ...baseConfig, env: { PAPERCLIP_API_KEY: "explicit-key-2" } }, + { ...baseConfig, env: { PAPERCLIP_CLOUD_PROVIDER_TOKEN: "explicit-key-2" } }, { context }, ); @@ -812,11 +813,7 @@ describe("shared ACPX engine runtime behavior", () => { stateDir, }; - await runExecutor({ - ...baseConfig, - agent: "custom-a", - env: { PAPERCLIP_API_KEY: "old-key" }, - }); + await runExecutor({ ...baseConfig, agent: "custom-a" }, { authToken: "old-key" }); const oldDate = new Date(Date.now() - 16 * 60 * 1000); await Promise.all( (await fs.readdir(wrappersDir)) @@ -824,11 +821,7 @@ describe("shared ACPX engine runtime behavior", () => { .map((name) => fs.utimes(path.join(wrappersDir, name), oldDate, oldDate)), ); - await runExecutor({ - ...baseConfig, - agent: "custom-b", - env: { PAPERCLIP_API_KEY: "new-key" }, - }); + await runExecutor({ ...baseConfig, agent: "custom-b" }, { authToken: "new-key" }); const wrappers = await fs.readdir(wrappersDir); expect(wrappers.filter((name) => name.endsWith(".sh"))).toHaveLength(1); @@ -846,14 +839,8 @@ describe("shared ACPX engine runtime behavior", () => { stateDir, }; - await runExecutor({ - ...baseConfig, - env: { PAPERCLIP_API_KEY: "first-key" }, - }); - await runExecutor({ - ...baseConfig, - env: { PAPERCLIP_API_KEY: "second-key" }, - }); + await runExecutor(baseConfig, { authToken: "first-key" }); + await runExecutor(baseConfig, { authToken: "second-key" }); const envFileNames = (await fs.readdir(path.join(stateDir, "wrappers"))).filter((name) => name.endsWith(".env")); expect(envFileNames).toHaveLength(2); diff --git a/packages/adapter-utils/src/acpx-engine/execute.ts b/packages/adapter-utils/src/acpx-engine/execute.ts index d23841e35f..7078a51490 100644 --- a/packages/adapter-utils/src/acpx-engine/execute.ts +++ b/packages/adapter-utils/src/acpx-engine/execute.ts @@ -33,6 +33,7 @@ import { ensureAbsoluteDirectory, ensurePathInEnv, ensurePaperclipSkillSymlink, + isForbiddenConfigEnvKey, isPaperclipRuntimeEnvKey, joinPromptSections, materializePaperclipSkillCopy, @@ -1044,8 +1045,6 @@ async function buildRuntime(input: { await fs.mkdir(stateDir, { recursive: true }); const envConfig = parseObject(config.env); - const hasExplicitApiKey = - typeof envConfig.PAPERCLIP_API_KEY === "string" && envConfig.PAPERCLIP_API_KEY.trim().length > 0; const env: Record = { ...buildPaperclipEnv(agent), PAPERCLIP_RUN_ID: runId }; const wakeTaskId = (typeof context.taskId === "string" && context.taskId.trim()) || @@ -1100,14 +1099,16 @@ async function buildRuntime(input: { for (const [key, value] of Object.entries(shapedEnvConfig)) { if (typeof value !== "string") continue; // Runtime PAPERCLIP_* always wins over config: skip a PAPERCLIP_* key that - // Paperclip has already assigned this run. A PAPERCLIP_* key Paperclip did - // NOT set (e.g. an explicitly configured PAPERCLIP_API_KEY, applied here) is - // stable per-run config, so it applies and feeds the fingerprint hash below. + // Paperclip has already assigned this run. PAPERCLIP_API_KEY is never + // accepted from config — the harness-minted run token is the only source. + // A PAPERCLIP_* key Paperclip did NOT set is stable per-run config, so it + // applies and feeds the fingerprint hash below. + if (isForbiddenConfigEnvKey(key)) continue; if (isPaperclipRuntimeEnvKey(key) && key in env) continue; env[key] = value; resolvedAdapterEnv[key] = value; } - if (!hasExplicitApiKey && authToken) env.PAPERCLIP_API_KEY = authToken; + if (authToken) env.PAPERCLIP_API_KEY = authToken; // For the claude agent, set model via ANTHROPIC_MODEL at startup rather than // via session/set_config_option — the ACP server's set_config_option handler // validates the value against its internal available-models list and rejects diff --git a/packages/adapter-utils/src/server-utils.test.ts b/packages/adapter-utils/src/server-utils.test.ts index 4ce82bbbe6..7dccd3fa59 100644 --- a/packages/adapter-utils/src/server-utils.test.ts +++ b/packages/adapter-utils/src/server-utils.test.ts @@ -2147,6 +2147,22 @@ describe("refreshPaperclipWorkspaceEnvForExecution", () => { it("applies a configured PAPERCLIP_* key only when Paperclip has not set it", () => { const env: Record = {}; + refreshPaperclipWorkspaceEnvForExecution({ + env, + envConfig: { + PAPERCLIP_CLOUD_PROVIDER_TOKEN: "cloud-token", + }, + workspaceCwd: null, + }); + + // Paperclip did not assign this PAPERCLIP_*-named key for the run, so the + // configured value flows through to the spawned process. + expect(env.PAPERCLIP_CLOUD_PROVIDER_TOKEN).toBe("cloud-token"); + }); + + it("never accepts PAPERCLIP_API_KEY from config env", () => { + const env: Record = {}; + refreshPaperclipWorkspaceEnvForExecution({ env, envConfig: { @@ -2155,10 +2171,9 @@ describe("refreshPaperclipWorkspaceEnvForExecution", () => { workspaceCwd: null, }); - // Paperclip did not assign PAPERCLIP_API_KEY before the merge, so an - // explicitly configured value is allowed through (adapters apply the run - // token here only when no explicit key was configured). - expect(env.PAPERCLIP_API_KEY).toBe("explicit-key"); + // The harness-minted run token is the only PAPERCLIP_API_KEY source; + // a configured value is dropped even when Paperclip has not set one. + expect(env.PAPERCLIP_API_KEY).toBeUndefined(); }); }); diff --git a/packages/adapter-utils/src/server-utils.ts b/packages/adapter-utils/src/server-utils.ts index d3e6b4e73a..164d80fbe1 100644 --- a/packages/adapter-utils/src/server-utils.ts +++ b/packages/adapter-utils/src/server-utils.ts @@ -114,6 +114,14 @@ const REDACTED_LOG_VALUE = "***REDACTED***"; export function isPaperclipRuntimeEnvKey(key: string): boolean { return key.startsWith("PAPERCLIP_"); } + +// PAPERCLIP_API_KEY is never accepted from adapter/user config env: the +// harness-minted run token is the only source of Paperclip API identity. +// Other PAPERCLIP_*-named config keys are allowed as long as Paperclip has +// not assigned the same key for the run (runtime vars always win). +export function isForbiddenConfigEnvKey(key: string): boolean { + return key === "PAPERCLIP_API_KEY"; +} const PAPERCLIP_SKILL_ROOT_RELATIVE_CANDIDATES = [ "../../skills", "../../../../../skills", @@ -2044,9 +2052,11 @@ export function refreshPaperclipWorkspaceEnvForExecution(input: { // runtime variable. Non-PAPERCLIP_* keys (plain values and resolved // secret_ref values) always forward to the spawned process; a PAPERCLIP_* // key from config only applies when Paperclip has NOT already assigned it - // for this run (e.g. an explicitly configured PAPERCLIP_API_KEY that the - // adapter applies after this merge). This keeps runtime identity, wake, and - // workspace vars authoritative regardless of what a config binding sets. + // for this run. PAPERCLIP_API_KEY is never accepted from config — the + // harness-minted run token is the only source. This keeps runtime + // identity, wake, and workspace vars authoritative regardless of what a + // config binding sets. + if (isForbiddenConfigEnvKey(key)) continue; if (isPaperclipRuntimeEnvKey(key) && key in input.env) continue; input.env[key] = value; } diff --git a/packages/adapters/claude-local/src/server/execute.ts b/packages/adapters/claude-local/src/server/execute.ts index b2d5848aed..fa02bfb7e2 100644 --- a/packages/adapters/claude-local/src/server/execute.ts +++ b/packages/adapters/claude-local/src/server/execute.ts @@ -36,6 +36,8 @@ import { buildInvocationEnvForLogs, ensureAbsoluteDirectory, ensurePathInEnv, + isForbiddenConfigEnvKey, + isPaperclipRuntimeEnvKey, refreshPaperclipWorkspaceEnvForExecution, renderTemplate, renderPaperclipWakePrompt, @@ -202,8 +204,6 @@ async function buildClaudeRuntimeConfig(input: ClaudeExecutionInput): Promise 0; const env: Record = { ...buildPaperclipEnv(agent) }; env.PAPERCLIP_RUN_ID = runId; @@ -287,10 +287,16 @@ async function buildClaudeRuntimeConfig(input: ClaudeExecutionInput): Promise 0; const env: Record = { ...paperclipBaseEnv }; env.PAPERCLIP_RUN_ID = runId; const wakeTaskId = @@ -776,7 +774,7 @@ export async function execute(ctx: AdapterExecutionContext): Promise 0) env.PAPERCLIP_LINKED_ISSUE_IDS = linkedIssueIds.join(","); if (wakePayloadJson) env.PAPERCLIP_WAKE_PAYLOAD_JSON = wakePayloadJson; if (issueWorkMode) env.PAPERCLIP_ISSUE_WORK_MODE = issueWorkMode; - if (!trimNullable(env.PAPERCLIP_API_KEY) && authToken) { + if (authToken) { env.PAPERCLIP_API_KEY = authToken; } diff --git a/packages/adapters/cursor-local/src/server/execute.ts b/packages/adapters/cursor-local/src/server/execute.ts index b2b5cac742..587f3f45fd 100644 --- a/packages/adapters/cursor-local/src/server/execute.ts +++ b/packages/adapters/cursor-local/src/server/execute.ts @@ -237,8 +237,6 @@ export async function execute(ctx: AdapterExecutionContext): Promise 0; let env: Record = { ...buildPaperclipEnv(agent) }; env.PAPERCLIP_RUN_ID = runId; const wakeTaskId = @@ -303,7 +301,7 @@ export async function execute(ctx: AdapterExecutionContext): Promise 0; const env: Record = { ...buildPaperclipEnv(agent) }; env.PAPERCLIP_RUN_ID = runId; const wakeTaskId = @@ -312,7 +310,7 @@ export async function execute(ctx: AdapterExecutionContext): Promise 0; const env: Record = { ...buildPaperclipEnv(agent) }; env.PAPERCLIP_RUN_ID = runId; const wakeTaskId = @@ -292,7 +290,7 @@ export async function execute(ctx: AdapterExecutionContext): Promise { }; expect(opts.onSpawn).toBeDefined(); }); + + it("does not inherit PAPERCLIP_API_KEY without a harness token", async () => { + const previousApiKey = process.env.PAPERCLIP_API_KEY; + process.env.PAPERCLIP_API_KEY = "parent-process-key"; + + try { + const { ctx } = makeCtx(); + await execute(ctx as any); + + const mocked = vi.mocked(serverUtils.runChildProcess); + const lastCall = mocked.mock.calls[mocked.mock.calls.length - 1]; + const opts = lastCall[3] as { env: Record }; + expect(opts.env.PAPERCLIP_API_KEY).toBeUndefined(); + } finally { + if (previousApiKey === undefined) delete process.env.PAPERCLIP_API_KEY; + else process.env.PAPERCLIP_API_KEY = previousApiKey; + } + }); }); diff --git a/packages/adapters/hermes/src/server/execute.ts b/packages/adapters/hermes/src/server/execute.ts index fd4c7d03bd..f7b743abff 100644 --- a/packages/adapters/hermes/src/server/execute.ts +++ b/packages/adapters/hermes/src/server/execute.ts @@ -465,7 +465,9 @@ export async function execute( if (ctx.runId) env.PAPERCLIP_RUN_ID = ctx.runId; - // BUG FIX: Inject authToken as PAPERCLIP_API_KEY (matches adapter-claude-local behavior) + // PAPERCLIP_API_KEY is never accepted from config — the harness-minted run + // token is the only source of Paperclip API identity. + delete env.PAPERCLIP_API_KEY; if ((ctx as any).authToken) env.PAPERCLIP_API_KEY = (ctx as any).authToken; // BUG FIX: Read task context from ctx.context (wake context), not ctx.config (adapter config) diff --git a/packages/adapters/opencode-local/src/server/execute.ts b/packages/adapters/opencode-local/src/server/execute.ts index 4f05a6611f..91f953f2c3 100644 --- a/packages/adapters/opencode-local/src/server/execute.ts +++ b/packages/adapters/opencode-local/src/server/execute.ts @@ -252,8 +252,6 @@ export async function execute(ctx: AdapterExecutionContext): Promise 0; const env: Record = { ...buildPaperclipEnv(agent) }; env.PAPERCLIP_RUN_ID = runId; const wakeTaskId = @@ -307,7 +305,7 @@ export async function execute(ctx: AdapterExecutionContext): Promise 0; const env: Record = { ...buildPaperclipEnv(agent) }; env.PAPERCLIP_RUN_ID = runId; @@ -316,7 +314,7 @@ export async function execute(ctx: AdapterExecutionContext): Promise { }); }); - it("drops Paperclip runtime-owned env before resolving environment, agent, project, and routine overlays", async () => { + it("drops PAPERCLIP_API_KEY bindings but forwards other PAPERCLIP_-named env to resolution", async () => { const resolveAdapterConfigForRuntime = vi.fn(async (_companyId, config: Record) => ({ config: { ...config, @@ -158,24 +158,24 @@ describe("resolveExecutionRunAdapterConfig", () => { environmentId: "environment-1", environmentEnv: { PAPERCLIP_API_KEY: "environment-api-key", - PAPERCLIP_AGENT_ID: "environment-agent", + PAPERCLIP_CLOUD_PROVIDER_TOKEN_ENV: "environment-cloud", ENV_ONLY: "environment-only", }, executionRunConfig: { env: { PAPERCLIP_API_KEY: { type: "secret_ref", secretId: "secret-api-key", version: "latest" }, - PAPERCLIP_AGENT_ID: "spoofed-agent", + PAPERCLIP_CLOUD_PROVIDER_TOKEN_AGENT: "agent-cloud", AGENT_ONLY: "agent-only", }, }, projectEnv: { PAPERCLIP_API_KEY: "project-api-key", - PAPERCLIP_COMPANY_ID: "spoofed-company", + PAPERCLIP_CLOUD_PROVIDER_TOKEN_PROJECT: "project-cloud", PROJECT_ONLY: "project-only", }, routineEnv: { PAPERCLIP_API_KEY: "routine-api-key", - PAPERCLIP_RUN_ID: "spoofed-run", + PAPERCLIP_CLOUD_PROVIDER_TOKEN_ROUTINE: "routine-cloud", ROUTINE_ONLY: "routine-only", }, routineId: "routine-1", @@ -186,26 +186,34 @@ describe("resolveExecutionRunAdapterConfig", () => { }); expect(resolveEnvBindings.mock.calls[0]?.[1]).toEqual({ + PAPERCLIP_CLOUD_PROVIDER_TOKEN_ENV: "environment-cloud", ENV_ONLY: "environment-only", }); expect(resolveAdapterConfigForRuntime.mock.calls[0]?.[1]).toEqual({ env: { + PAPERCLIP_CLOUD_PROVIDER_TOKEN_AGENT: "agent-cloud", AGENT_ONLY: "agent-only", }, }); expect(resolveEnvBindings.mock.calls[1]?.[1]).toEqual({ + PAPERCLIP_CLOUD_PROVIDER_TOKEN_PROJECT: "project-cloud", PROJECT_ONLY: "project-only", }); expect(resolveEnvBindings.mock.calls[2]?.[1]).toEqual({ + PAPERCLIP_CLOUD_PROVIDER_TOKEN_ROUTINE: "routine-cloud", ROUTINE_ONLY: "routine-only", }); expect(result.resolvedConfig.env).toEqual({ + PAPERCLIP_CLOUD_PROVIDER_TOKEN_ENV: "environment-cloud", ENV_ONLY: "environment-only", + PAPERCLIP_CLOUD_PROVIDER_TOKEN_AGENT: "agent-cloud", AGENT_ONLY: "agent-only", + PAPERCLIP_CLOUD_PROVIDER_TOKEN_PROJECT: "project-cloud", PROJECT_ONLY: "project-only", + PAPERCLIP_CLOUD_PROVIDER_TOKEN_ROUTINE: "routine-cloud", ROUTINE_ONLY: "routine-only", }); - expect(JSON.stringify(result.resolvedConfig.env)).not.toContain("PAPERCLIP_"); + expect(JSON.stringify(result.resolvedConfig.env)).not.toContain("PAPERCLIP_API_KEY"); }); it("skips project env resolution when the project has no bindings", async () => { diff --git a/server/src/adapters/process/execute.ts b/server/src/adapters/process/execute.ts index c16d37422f..6f4c6ba893 100644 --- a/server/src/adapters/process/execute.ts +++ b/server/src/adapters/process/execute.ts @@ -5,6 +5,8 @@ import { asStringArray, parseObject, buildPaperclipEnv, + isForbiddenConfigEnvKey, + isPaperclipRuntimeEnvKey, buildInvocationEnvForLogs, ensurePathInEnv, resolveCommandForLogs, @@ -23,10 +25,20 @@ export async function execute(ctx: AdapterExecutionContext): Promise | null { +function stripForbiddenEnvBindings(envValue: unknown): Record | null { const record = parseObject(envValue); const filtered = Object.fromEntries( - Object.entries(record).filter(([key]) => !isPaperclipRuntimeEnvKey(key)), + Object.entries(record).filter(([key]) => !FORBIDDEN_ENV_BINDING_KEYS.has(key)), ); return Object.keys(filtered).length > 0 ? filtered : null; } -function stripPaperclipRuntimeEnvFromAdapterConfig(config: Record): Record { +function stripForbiddenEnvFromAdapterConfig(config: Record): Record { if (!Object.prototype.hasOwnProperty.call(config, "env")) return config; return { ...config, - env: stripPaperclipRuntimeEnvBindings(config.env) ?? {}, + env: stripForbiddenEnvBindings(config.env) ?? {}, }; } function assertLowTrustEnvConfigAllowed(envValue: unknown, source: string) { - const record = stripPaperclipRuntimeEnvBindings(envValue); + const record = stripForbiddenEnvBindings(envValue); if (!record) return; for (const [key, rawBinding] of Object.entries(record)) { const parsed = envBindingSchema.safeParse(rawBinding); @@ -673,10 +679,10 @@ export async function resolveExecutionRunAdapterConfig(input: { remediation: string; }; }) { - const executionRunConfig = stripPaperclipRuntimeEnvFromAdapterConfig(input.executionRunConfig); - const environmentEnv = stripPaperclipRuntimeEnvBindings(input.environmentEnv); - const projectEnv = stripPaperclipRuntimeEnvBindings(input.projectEnv); - const routineEnv = stripPaperclipRuntimeEnvBindings(input.routineEnv); + const executionRunConfig = stripForbiddenEnvFromAdapterConfig(input.executionRunConfig); + const environmentEnv = stripForbiddenEnvBindings(input.environmentEnv); + const projectEnv = stripForbiddenEnvBindings(input.projectEnv); + const routineEnv = stripForbiddenEnvBindings(input.routineEnv); const agentEnv = parseObject(executionRunConfig.env); const lowTrustAllowedBindingIds = input.trustPreset?.kind === "low_trust_review" ? input.trustPreset.boundary.allowedSecretBindingIds ?? []