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 <vincentkoc@ieee.org>
This commit is contained in:
Drew Wagner
2026-03-06 18:18:13 -05:00
committed by GitHub
parent 03b9abab84
commit ae96a81916
6 changed files with 76 additions and 2 deletions

View File

@@ -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.

View File

@@ -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", () => {

View File

@@ -348,8 +348,16 @@ function buildServerArgs(opts: AcpClientOptions): string[] {
export function resolveAcpClientSpawnEnv(
baseEnv: NodeJS.ProcessEnv = process.env,
options?: { stripKeys?: ReadonlySet<string> },
): 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<AcpC
const entryPath = resolveSelfEntryPath();
const serverCommand = opts.serverCommand ?? (entryPath ? process.execPath : "openclaw");
const effectiveArgs = opts.serverCommand || !entryPath ? serverArgs : [entryPath, ...serverArgs];
const spawnEnv = resolveAcpClientSpawnEnv();
const { getActiveSkillEnvKeys } = await import("../agents/skills/env-overrides.runtime.js");
const spawnEnv = resolveAcpClientSpawnEnv(process.env, {
stripKeys: getActiveSkillEnvKeys(),
});
const spawnInvocation = resolveAcpClientSpawnInvocation(
{ serverCommand, serverArgs: effectiveArgs },
{

View File

@@ -12,6 +12,7 @@ import {
buildWorkspaceSkillSnapshot,
loadWorkspaceSkillEntries,
} from "./skills.js";
import { getActiveSkillEnvKeys } from "./skills/env-overrides.js";
const fixtureSuite = createFixtureSuite("openclaw-skills-suite-");
let tempHome: TempHomeEnv | null = null;
@@ -256,9 +257,11 @@ describe("applySkillEnvOverrides", () => {
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);
}
});
});

View File

@@ -0,0 +1 @@
export { getActiveSkillEnvKeys } from "./env-overrides.js";

View File

@@ -12,6 +12,19 @@ const log = createSubsystemLogger("env-overrides");
type EnvUpdate = { key: string; prev: string | undefined };
type SkillConfig = NonNullable<ReturnType<typeof resolveSkillConfig>>;
/**
* 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<string>();
/** Returns a snapshot of env var keys currently injected by skill overrides. */
export function getActiveSkillEnvKeys(): ReadonlySet<string> {
return activeSkillEnvKeys;
}
type SanitizedSkillEnvOverrides = {
allowed: Record<string, string>;
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 {