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; }