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 {