From ae96a8191649c5d1d44c6e06f8503015216cd880 Mon Sep 17 00:00:00 2001 From: Drew Wagner <42811278+taw0002@users.noreply.github.com> Date: Fri, 6 Mar 2026 18:18:13 -0500 Subject: [PATCH] fix: strip skill-injected env vars from ACP harness spawn env (#36280) (#36316) * fix: strip skill-injected env vars from ACP harness spawn env Skill apiKey entries (e.g., openai-image-gen with primaryEnv=OPENAI_API_KEY) are set on process.env during agent runs and only reverted after the run completes. ACP harnesses like Codex CLI inherit these vars, causing them to silently use API billing instead of their own auth (e.g., OAuth). The fix tracks which env vars are actively injected by skill overrides in a module-level Set (activeSkillEnvKeys) and strips them in resolveAcpClientSpawnEnv() before spawning ACP child processes. Fixes #36280 * ACP: type spawn env for stripped keys * Skills: cover active env key lifecycle * Changelog: note ACP skill env isolation * ACP: preserve shell marker after env stripping --------- Co-authored-by: Vincent Koc --- CHANGELOG.md | 1 + src/acp/client.test.ts | 43 ++++++++++++++++++++++ src/acp/client.ts | 15 +++++++- src/agents/skills.test.ts | 3 ++ src/agents/skills/env-overrides.runtime.ts | 1 + src/agents/skills/env-overrides.ts | 15 ++++++++ 6 files changed, 76 insertions(+), 2 deletions(-) create mode 100644 src/agents/skills/env-overrides.runtime.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 8fc4a7cd81b..dd664c4dadc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -206,6 +206,7 @@ Docs: https://docs.openclaw.ai - Skills/nano-banana-pro resolution override: respect explicit `--resolution` values during image editing and only auto-detect output size from input images when the flag is omitted. (#36880) Thanks @shuofengzhang and @vincentkoc. - Skills/openai-image-gen CLI validation: validate `--background` and `--style` inputs early, normalize supported values, and warn when those flags are ignored for incompatible models. (#36762) Thanks @shuofengzhang and @vincentkoc. - Skills/openai-image-gen output formats: validate `--output-format` values early, normalize aliases like `jpg -> jpeg`, and warn when the flag is ignored for incompatible models. (#36648) Thanks @shuofengzhang and @vincentkoc. +- ACP/skill env isolation: strip skill-injected API keys from ACP harness child-process environments so tools like Codex CLI keep their own auth flow instead of inheriting billed provider keys from active skills. (#36316) Thanks @taw0002 and @vincentkoc. - WhatsApp media upload caps: make outbound media sends and auto-replies honor `channels.whatsapp.mediaMaxMb` with per-account overrides so inbound and outbound limits use the same channel config. Thanks @vincentkoc. - Windows/Plugin install: when OpenClaw runs on Windows via Bun and `npm-cli.js` is not colocated with the runtime binary, fall back to `npm.cmd`/`npx.cmd` through the existing `cmd.exe` wrapper so `openclaw plugins install` no longer fails with `spawn EINVAL`. (#38056) Thanks @0xlin2023. - Telegram/send retry classification: retry grammY `Network request ... failed after N attempts` envelopes in send flows without reclassifying plain `Network request ... failed!` wrappers as transient, restoring the intended retry path while keeping broad send-context message matching tight. (#38056) Thanks @0xlin2023. diff --git a/src/acp/client.test.ts b/src/acp/client.test.ts index 72958ca57c2..bb5340115a1 100644 --- a/src/acp/client.test.ts +++ b/src/acp/client.test.ts @@ -60,6 +60,49 @@ describe("resolveAcpClientSpawnEnv", () => { }); expect(env.OPENCLAW_SHELL).toBe("acp-client"); }); + + it("strips skill-injected env keys when stripKeys is provided", () => { + const stripKeys = new Set(["OPENAI_API_KEY", "ELEVENLABS_API_KEY"]); + const env = resolveAcpClientSpawnEnv( + { + PATH: "/usr/bin", + OPENAI_API_KEY: "sk-leaked-from-skill", + ELEVENLABS_API_KEY: "el-leaked", + ANTHROPIC_API_KEY: "sk-keep-this", + }, + { stripKeys }, + ); + + expect(env.PATH).toBe("/usr/bin"); + expect(env.OPENCLAW_SHELL).toBe("acp-client"); + expect(env.ANTHROPIC_API_KEY).toBe("sk-keep-this"); + expect(env.OPENAI_API_KEY).toBeUndefined(); + expect(env.ELEVENLABS_API_KEY).toBeUndefined(); + }); + + it("does not modify the original baseEnv when stripping keys", () => { + const baseEnv: NodeJS.ProcessEnv = { + OPENAI_API_KEY: "sk-original", + PATH: "/usr/bin", + }; + const stripKeys = new Set(["OPENAI_API_KEY"]); + resolveAcpClientSpawnEnv(baseEnv, { stripKeys }); + + expect(baseEnv.OPENAI_API_KEY).toBe("sk-original"); + }); + + it("preserves OPENCLAW_SHELL even when stripKeys contains it", () => { + const env = resolveAcpClientSpawnEnv( + { + OPENCLAW_SHELL: "skill-overridden", + OPENAI_API_KEY: "sk-leaked", + }, + { stripKeys: new Set(["OPENCLAW_SHELL", "OPENAI_API_KEY"]) }, + ); + + expect(env.OPENCLAW_SHELL).toBe("acp-client"); + expect(env.OPENAI_API_KEY).toBeUndefined(); + }); }); describe("resolveAcpClientSpawnInvocation", () => { diff --git a/src/acp/client.ts b/src/acp/client.ts index 0cf9a194d88..54be5ffc455 100644 --- a/src/acp/client.ts +++ b/src/acp/client.ts @@ -348,8 +348,16 @@ function buildServerArgs(opts: AcpClientOptions): string[] { export function resolveAcpClientSpawnEnv( baseEnv: NodeJS.ProcessEnv = process.env, + options?: { stripKeys?: ReadonlySet }, ): NodeJS.ProcessEnv { - return { ...baseEnv, OPENCLAW_SHELL: "acp-client" }; + const env: NodeJS.ProcessEnv = { ...baseEnv }; + if (options?.stripKeys) { + for (const key of options.stripKeys) { + delete env[key]; + } + } + env.OPENCLAW_SHELL = "acp-client"; + return env; } type AcpSpawnRuntime = { @@ -450,7 +458,10 @@ export async function createAcpClient(opts: AcpClientOptions = {}): Promise { try { expect(process.env.ENV_KEY).toBe("injected"); + expect(getActiveSkillEnvKeys().has("ENV_KEY")).toBe(true); } finally { restore(); expect(process.env.ENV_KEY).toBeUndefined(); + expect(getActiveSkillEnvKeys().has("ENV_KEY")).toBe(false); } }); }); diff --git a/src/agents/skills/env-overrides.runtime.ts b/src/agents/skills/env-overrides.runtime.ts new file mode 100644 index 00000000000..ab8c4b305fb --- /dev/null +++ b/src/agents/skills/env-overrides.runtime.ts @@ -0,0 +1 @@ +export { getActiveSkillEnvKeys } from "./env-overrides.js"; diff --git a/src/agents/skills/env-overrides.ts b/src/agents/skills/env-overrides.ts index 83bb559bc7c..b56d02070df 100644 --- a/src/agents/skills/env-overrides.ts +++ b/src/agents/skills/env-overrides.ts @@ -12,6 +12,19 @@ const log = createSubsystemLogger("env-overrides"); type EnvUpdate = { key: string; prev: string | undefined }; type SkillConfig = NonNullable>; +/** + * Tracks env var keys that are currently injected by skill overrides. + * Used by ACP harness spawn to strip skill-injected keys so they don't + * leak to child processes (e.g., OPENAI_API_KEY leaking to Codex CLI). + * @see https://github.com/openclaw/openclaw/issues/36280 + */ +const activeSkillEnvKeys = new Set(); + +/** Returns a snapshot of env var keys currently injected by skill overrides. */ +export function getActiveSkillEnvKeys(): ReadonlySet { + return activeSkillEnvKeys; +} + type SanitizedSkillEnvOverrides = { allowed: Record; blocked: string[]; @@ -135,12 +148,14 @@ function applySkillConfigEnvOverrides(params: { } updates.push({ key: envKey, prev: process.env[envKey] }); process.env[envKey] = envValue; + activeSkillEnvKeys.add(envKey); } } function createEnvReverter(updates: EnvUpdate[]) { return () => { for (const update of updates) { + activeSkillEnvKeys.delete(update.key); if (update.prev === undefined) { delete process.env[update.key]; } else {