From 0ecae2cd7efb1d09f2354c6904fd20f266834571 Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Wed, 15 Jul 2026 11:26:47 -0500 Subject: [PATCH] fix(adapters): forward resolved adapter env to local agents, keep runtime env authoritative (#9617) 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 > - Local coding adapters (Claude, Codex) run each agent heartbeat in a spawned child process; Paperclip resolves adapter-configured env — including `secret_ref` bindings — into the env for that process > - The resolved adapter env was not being forwarded reliably: config env could overwrite Paperclip's own runtime vars, and a warm/resumable ACP session could keep serving stale env because its fingerprint ignored the resolved env > - This mattered because a configured API key or other secret could be silently absent from the agent shell, and a resumed session would never pick up an updated value — while a config binding could also override runtime identity/wake vars > - This pull request keeps Paperclip-managed `PAPERCLIP_*` runtime env authoritative over config, and folds a stable hash of the applied adapter env into the session fingerprint so an env change forces a fresh launch > - The benefit is that adapter-configured env (plain values and resolved secrets) reliably reaches the agent process, updates are picked up on the next launch, and runtime identity can never be clobbered by config ## Linked Issues or Issue Description No public GitHub issue exists, so the underlying bug is described inline following the bug-report template. **What happened** Env keys configured on a local adapter (plain values and `secret_ref` bindings, resolved server-side into plain strings) did not reliably reach the spawned agent process. Two distinct gaps: (1) when merging config env into the process env, a config key in the reserved `PAPERCLIP_*` namespace could overwrite a Paperclip-managed runtime variable (identity, wake, workspace, API access); (2) a warm-handle / resumable ACP session computed its reuse fingerprint from `secretManifestHash` only, which misses plain-value edits and same-version secret rotations — so a resumed session kept serving stale env and never re-launched with updated values. **Expected behavior** Non-`PAPERCLIP_*` adapter env (plain + resolved secret values) is forwarded to the agent process; a change to any applied forwarded value invalidates a warm/resumable session so the next launch sources the latest env; configured `PAPERCLIP_*` entries can never override Paperclip runtime env, while an explicitly configured `PAPERCLIP_API_KEY` (stable per-run config) is still honored and its rotation also busts the session. **Steps to reproduce** Configure an adapter with an env key (e.g. a `secret_ref` API key) and a resumable ACP session. On resume, the updated env value is not sourced; separately, a `PAPERCLIP_*` config key overrides the runtime value. **Deployment mode** Local adapters (Claude / Codex) via the shared adapter-utils execution path. ## What Changed - `packages/adapter-utils/src/server-utils.ts`: add `isPaperclipRuntimeEnvKey` and, in `refreshPaperclipWorkspaceEnvForExecution` (used by all local adapters), skip a `PAPERCLIP_*` config key when Paperclip has already assigned it this run; all other keys still forward. - `packages/adapter-utils/src/acpx-engine/execute.ts`: apply the same `PAPERCLIP_*` non-override rule (via the shared helper) when merging config env, capture the applied config env in `resolvedAdapterEnv`, and fold a stable `adapterEnvHash` of it into the session fingerprint so an env change forces a fresh launch. Per-wake `PAPERCLIP_*` runtime vars are assigned earlier and never enter that map, so they stay out of the hash; stable configured `PAPERCLIP_*` values (e.g. an explicit `PAPERCLIP_API_KEY`) are included so rotating one busts the session. - Added unit/integration tests for plain + secret forwarding, `PAPERCLIP_*` non-override, the explicit-API-key path, fingerprint refresh-on-env-change vs. stable-across-wakes, and rotation of a configured `PAPERCLIP_API_KEY`. ## Verification - `npx vitest run packages/adapter-utils/src/acpx-engine/execute.test.ts packages/adapter-utils/src/server-utils.test.ts` → 2 files, 111 tests passing (includes the new cases). - Tests assert: forwarded plain/secret values appear in the spawned wrapper `.env`; a `PAPERCLIP_*` config key does not override the runtime value; changing an applied forwarded env value (including a rotated `PAPERCLIP_API_KEY`) changes `configFingerprint`, while a new wake with the same config env keeps it stable. ## Risks Low risk. Behavior change is limited to (a) config env no longer overriding `PAPERCLIP_*` runtime vars — a security-positive tightening — and (b) a resumable session re-launching when its applied config env changes, which is the intended fix. Per-wake `PAPERCLIP_*` churn is deliberately excluded from the fingerprint so normal sessions still resume across heartbeats. Existing sessions get a new fingerprint once on first deploy (the added `adapterEnvHash` field), which is expected. No secret values are logged (key-name redaction in the adapter plus manifest-driven redaction of `meta.env`). ## Model Used Claude Opus 4.8 (Anthropic), model id `claude-opus-4-8`, extended reasoning with tool use. ## 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 - [ ] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] 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 --------- Co-authored-by: Paperclip --- .../src/acpx-engine/execute.test.ts | 89 +++++++++++++++++++ .../adapter-utils/src/acpx-engine/execute.ts | 27 +++++- .../adapter-utils/src/server-utils.test.ts | 44 +++++++++ packages/adapter-utils/src/server-utils.ts | 15 ++++ 4 files changed, 174 insertions(+), 1 deletion(-) diff --git a/packages/adapter-utils/src/acpx-engine/execute.test.ts b/packages/adapter-utils/src/acpx-engine/execute.test.ts index c50de41e89..b900189be9 100644 --- a/packages/adapter-utils/src/acpx-engine/execute.test.ts +++ b/packages/adapter-utils/src/acpx-engine/execute.test.ts @@ -557,6 +557,95 @@ describe("shared ACPX engine runtime behavior", () => { expect(env).not.toContain("old-key"); }); + it("forwards resolved adapter env (plain + secret) to the wrapper without overriding runtime vars", async () => { + const root = await makeTempRoot(); + const stateDir = path.join(root, "state"); + + await runExecutor( + { + agentCommand: "node ./fake-acp.js", + stateDir, + env: { + OOGA_BOOGA_123: "plain-value", + // Server-resolved secret_ref values arrive here as plain strings. + OPENROUTER_API_KEY: "resolved-secret-value", + // Reserved-namespace config keys must not clobber runtime identity/wake. + PAPERCLIP_TASK_ID: "attacker-issue", + }, + }, + { + authToken: "runtime-secret-token", + context: { taskId: "issue-real", wakeReason: "issue_assigned" }, + }, + ); + + const wrappers = await fs.readdir(path.join(stateDir, "wrappers")); + const envPath = path.join(stateDir, "wrappers", wrappers.find((name) => name.endsWith(".env"))!); + const env = await fs.readFile(envPath, "utf8"); + + expect(env).toContain("OOGA_BOOGA_123='plain-value'"); + expect(env).toContain("OPENROUTER_API_KEY='resolved-secret-value'"); + // 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"); + }); + + it("busts the session fingerprint when resolved adapter env changes but not across wakes", async () => { + const root = await makeTempRoot(); + const stateDir = path.join(root, "state"); + const baseConfig = { agentCommand: "node ./fake-acp.js", stateDir }; + + const first = await runExecutor( + { ...baseConfig, env: { OPENROUTER_API_KEY: "value-1" } }, + { context: { taskId: "issue-1", wakeReason: "issue_assigned" } }, + ); + const changedEnv = await runExecutor( + { ...baseConfig, env: { OPENROUTER_API_KEY: "value-2" } }, + { context: { taskId: "issue-1", wakeReason: "issue_assigned" } }, + ); + const sameEnvNewWake = await runExecutor( + { ...baseConfig, env: { OPENROUTER_API_KEY: "value-1" } }, + { context: { taskId: "issue-1", wakeReason: "comment", wakeCommentId: "c-9" } }, + ); + + const fp = (r: { result: { sessionParams?: unknown } }) => + (r.result.sessionParams as { configFingerprint?: string } | undefined)?.configFingerprint; + + // A changed forwarded env value invalidates warm-handle / session reuse so + // the next launch sources the latest env. + expect(fp(first)).toBeDefined(); + expect(fp(changedEnv)).not.toBe(fp(first)); + // A new heartbeat with the same config env keeps the fingerprint stable, so + // per-wake PAPERCLIP_* churn does not needlessly reset the session. + expect(fp(sameEnvNewWake)).toBe(fp(first)); + }); + + it("busts the session fingerprint when a stable configured PAPERCLIP_* value rotates", async () => { + const root = await makeTempRoot(); + 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. + const context = { taskId: "issue-1", wakeReason: "issue_assigned" }; + const withKey = await runExecutor( + { ...baseConfig, env: { PAPERCLIP_API_KEY: "explicit-key-1" } }, + { context }, + ); + const rotatedKey = await runExecutor( + { ...baseConfig, env: { PAPERCLIP_API_KEY: "explicit-key-2" } }, + { context }, + ); + + const fp = (r: { result: { sessionParams?: unknown } }) => + (r.result.sessionParams as { configFingerprint?: string } | undefined)?.configFingerprint; + + expect(fp(withKey)).toBeDefined(); + expect(fp(rotatedKey)).not.toBe(fp(withKey)); + }); + it("shapes ACPX wrapper workspace env for remote execution identities", async () => { const root = await makeTempRoot(); const stateDir = path.join(root, "state"); diff --git a/packages/adapter-utils/src/acpx-engine/execute.ts b/packages/adapter-utils/src/acpx-engine/execute.ts index e1d58a1ea2..1d2583d70b 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, + isPaperclipRuntimeEnvKey, joinPromptSections, materializePaperclipSkillCopy, parseObject, @@ -1044,8 +1045,24 @@ async function buildRuntime(input: { executionCwd: shapedWorkspaceEnv.workspaceCwd, executionTargetIsRemote, }); + // Resolved adapter env (plain + server-resolved secret_ref values) that we + // forward to the spawned agent process. Captured so a stable hash of it can be + // folded into the session fingerprint below — a change here must invalidate a + // warm/resumable session so the next launch picks up the latest env. Only + // user/adapter-configured env flows through this loop; per-wake PAPERCLIP_* + // runtime vars (PAPERCLIP_RUN_ID, wake/approval ids, ...) were assigned to + // `env` above and are never present in shapedEnvConfig, so they inherently + // stay out of the hash and don't reset the session every heartbeat. + const resolvedAdapterEnv: Record = {}; for (const [key, value] of Object.entries(shapedEnvConfig)) { - if (typeof value === "string") env[key] = value; + 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. + if (isPaperclipRuntimeEnvKey(key) && key in env) continue; + env[key] = value; + resolvedAdapterEnv[key] = value; } if (!hasExplicitApiKey && authToken) env.PAPERCLIP_API_KEY = authToken; // For the claude agent, set model via ANTHROPIC_MODEL at startup rather than @@ -1217,6 +1234,14 @@ async function buildRuntime(input: { : null, mcpServers: mcpIdentity, secretManifestHash: shortHash(secretManifest), + // Fold the resolved adapter env (all applied user-configured values — + // plain, secret_ref, and stable PAPERCLIP_* config such as an explicit + // PAPERCLIP_API_KEY) into the fingerprint so a change to any forwarded value + // invalidates a warm handle / resumable session and forces a fresh launch + // that sources the latest env. secretManifestHash alone misses plain-value + // edits and same-version secret rotations. Per-wake runtime vars never enter + // resolvedAdapterEnv, so they don't churn the fingerprint every heartbeat. + adapterEnvHash: shortHash(resolvedAdapterEnv), }); const taskKey = asString(input.ctx.runtime.taskKey, "") || wakeTaskId || workspaceId || "default"; const sessionKey = `paperclip:${agent.companyId}:${agent.id}:${taskKey}:${fingerprint}`; diff --git a/packages/adapter-utils/src/server-utils.test.ts b/packages/adapter-utils/src/server-utils.test.ts index 0b59b30919..4a5f8c8f85 100644 --- a/packages/adapter-utils/src/server-utils.test.ts +++ b/packages/adapter-utils/src/server-utils.test.ts @@ -2053,6 +2053,50 @@ describe("refreshPaperclipWorkspaceEnvForExecution", () => { }, ]); }); + + it("forwards resolved adapter env but never overrides Paperclip runtime env", () => { + const env: Record = { + PAPERCLIP_RUN_ID: "run-1", + PAPERCLIP_TASK_ID: "issue-1", + PAPERCLIP_API_URL: "http://runtime:3100", + }; + + refreshPaperclipWorkspaceEnvForExecution({ + env, + envConfig: { + // Plain non-PAPERCLIP key. + OOGA_BOOGA_123: "plain-value", + // Server-resolved secret_ref value arrives as a plain string here. + OPENROUTER_API_KEY: "resolved-secret-value", + // Reserved-namespace keys must not clobber runtime identity/wake vars. + PAPERCLIP_TASK_ID: "attacker-issue", + PAPERCLIP_API_URL: "http://evil:9999", + }, + workspaceCwd: null, + }); + + expect(env.OOGA_BOOGA_123).toBe("plain-value"); + expect(env.OPENROUTER_API_KEY).toBe("resolved-secret-value"); + expect(env.PAPERCLIP_TASK_ID).toBe("issue-1"); + expect(env.PAPERCLIP_API_URL).toBe("http://runtime:3100"); + }); + + it("applies a configured PAPERCLIP_* key only when Paperclip has not set it", () => { + const env: Record = {}; + + refreshPaperclipWorkspaceEnvForExecution({ + env, + envConfig: { + PAPERCLIP_API_KEY: "explicit-key", + }, + 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"); + }); }); describe("appendWithByteCap", () => { diff --git a/packages/adapter-utils/src/server-utils.ts b/packages/adapter-utils/src/server-utils.ts index 69c5757bdc..acb4d305ed 100644 --- a/packages/adapter-utils/src/server-utils.ts +++ b/packages/adapter-utils/src/server-utils.ts @@ -107,6 +107,13 @@ const DEFAULT_PAPERCLIP_INSTANCE_ID = "default"; const PATH_SEGMENT_RE = /^[a-zA-Z0-9_-]+$/; const SENSITIVE_ENV_KEY = /(key|token|secret|password|passwd|authorization|cookie)/i; const REDACTED_LOG_VALUE = "***REDACTED***"; + +// PAPERCLIP_* is the reserved runtime namespace: these vars are generated by +// Paperclip per run (identity, wake, workspace, API access). Adapter/user +// config env must never override them. +export function isPaperclipRuntimeEnvKey(key: string): boolean { + return key.startsWith("PAPERCLIP_"); +} const PAPERCLIP_SKILL_ROOT_RELATIVE_CANDIDATES = [ "../../skills", "../../../../../skills", @@ -1954,6 +1961,14 @@ export function refreshPaperclipWorkspaceEnvForExecution(input: { executionTargetIsRemote: input.executionTargetIsRemote, }); for (const [key, value] of Object.entries(shapedEnvConfig)) { + // Adapter/user-configured env must never override a Paperclip-managed + // 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. + if (isPaperclipRuntimeEnvKey(key) && key in input.env) continue; input.env[key] = value; }