Skip to content

Commit d24f695

Browse files
rodrigourozrodrigouroz
committed
fix(acp): preserve custom server auth env and share stripping list
Use a shared provider auth env-var list for ACP child process scrubbing so both the CLI ACP path and the acpx runtime strip the same credentials, including non-_API_KEY tokens like GITHUB_TOKEN and HF_TOKEN. Only apply provider auth env stripping on the default OpenClaw bridge path. Explicit custom ACP servers keep their env-based auth contract.
1 parent d4781d6 commit d24f695

File tree

7 files changed

+122
-71
lines changed

7 files changed

+122
-71
lines changed

‎extensions/acpx/src/runtime-internals/process.test.ts‎

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { spawn } from "node:child_process";
22
import { mkdir, mkdtemp, rm, writeFile } from "node:fs/promises";
33
import { tmpdir } from "node:os";
44
import path from "node:path";
5-
import { afterEach, describe, expect, it } from "vitest";
5+
import { afterEach, describe, expect, it, vi } from "vitest";
66
import { createWindowsCmdShimFixture } from "../../../shared/windows-cmd-shim-test-fixtures.js";
77
import {
88
resolveSpawnCommand,
@@ -28,6 +28,7 @@ async function createTempDir(): Promise<string> {
2828
}
2929

3030
afterEach(async () => {
31+
vi.unstubAllEnvs();
3132
while (tempDirs.length > 0) {
3233
const dir = tempDirs.pop();
3334
if (!dir) {
@@ -289,4 +290,36 @@ describe("spawnAndCollect", () => {
289290
const result = await resultPromise;
290291
expect(result.error?.name).toBe("AbortError");
291292
});
293+
294+
it("strips shared provider auth env vars from spawned acpx children", async () => {
295+
vi.stubEnv("OPENAI_API_KEY", "openai-secret");
296+
vi.stubEnv("GITHUB_TOKEN", "gh-secret");
297+
vi.stubEnv("HF_TOKEN", "hf-secret");
298+
vi.stubEnv("OPENCLAW_API_KEY", "keep-me");
299+
300+
const result = await spawnAndCollect({
301+
command: process.execPath,
302+
args: [
303+
"-e",
304+
"process.stdout.write(JSON.stringify({openai:process.env.OPENAI_API_KEY,github:process.env.GITHUB_TOKEN,hf:process.env.HF_TOKEN,openclaw:process.env.OPENCLAW_API_KEY,shell:process.env.OPENCLAW_SHELL}))",
305+
],
306+
cwd: process.cwd(),
307+
});
308+
309+
expect(result.code).toBe(0);
310+
expect(result.error).toBeNull();
311+
312+
const parsed = JSON.parse(result.stdout) as {
313+
openai?: string;
314+
github?: string;
315+
hf?: string;
316+
openclaw?: string;
317+
shell?: string;
318+
};
319+
expect(parsed.openai).toBeUndefined();
320+
expect(parsed.github).toBeUndefined();
321+
expect(parsed.hf).toBeUndefined();
322+
expect(parsed.openclaw).toBe("keep-me");
323+
expect(parsed.shell).toBe("acp");
324+
});
292325
});

‎extensions/acpx/src/runtime-internals/process.ts‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import type {
77
} from "openclaw/plugin-sdk/acpx";
88
import {
99
applyWindowsSpawnProgramPolicy,
10+
listKnownProviderAuthEnvVarNames,
1011
materializeWindowsSpawnProgram,
1112
resolveWindowsSpawnProgramCandidate,
1213
} from "openclaw/plugin-sdk/acpx";
@@ -136,16 +137,12 @@ export function spawnWithResolvedCommand(
136137
options,
137138
);
138139

139-
// Strip provider API keys from child environment to prevent ACP agents
140-
// (e.g. Codex CLI) from inheriting credentials they shouldn't use.
141-
// Without this, leaked OPENAI_API_KEY causes Codex to overwrite its
142-
// OAuth config in ~/.codex/auth.json with apikey mode.
143-
const childEnv: Record<string, string | undefined> = {
140+
const childEnv: NodeJS.ProcessEnv = {
144141
...process.env,
145142
OPENCLAW_SHELL: "acp",
146143
};
147-
for (const key of Object.keys(childEnv)) {
148-
if (key.endsWith("_API_KEY") && key !== "OPENCLAW_API_KEY") {
144+
for (const key of listKnownProviderAuthEnvVarNames()) {
145+
if (key !== "OPENCLAW_API_KEY") {
149146
delete childEnv[key];
150147
}
151148
}

‎src/acp/client.test.ts‎

Lines changed: 30 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,10 @@ import {
77
resolveAcpClientSpawnEnv,
88
resolveAcpClientSpawnInvocation,
99
resolvePermissionRequest,
10+
shouldStripProviderAuthEnvVarsForAcpServer,
1011
} from "./client.js";
1112
import { extractAttachmentsFromPrompt, extractTextFromPrompt } from "./event-mapper.js";
1213

13-
const envVar = (...parts: string[]) => parts.join("_");
14-
1514
function makePermissionRequest(
1615
overrides: Partial<RequestPermissionRequest> = {},
1716
): RequestPermissionRequest {
@@ -63,52 +62,50 @@ describe("resolveAcpClientSpawnEnv", () => {
6362
expect(env.OPENCLAW_SHELL).toBe("acp-client");
6463
});
6564

66-
it("strips skill-injected env keys when stripKeys is provided", () => {
67-
const openAiApiKeyEnv = envVar("OPENAI", "API", "KEY");
68-
const elevenLabsApiKeyEnv = envVar("ELEVENLABS", "API", "KEY");
69-
const anthropicApiKeyEnv = envVar("ANTHROPIC", "API", "KEY");
70-
const stripKeys = new Set([openAiApiKeyEnv, elevenLabsApiKeyEnv]);
65+
it("strips provider auth env vars for the default OpenClaw bridge", () => {
66+
const stripKeys = new Set(["OPENAI_API_KEY", "GITHUB_TOKEN", "HF_TOKEN"]);
7167
const env = resolveAcpClientSpawnEnv(
7268
{
69+
OPENAI_API_KEY: "openai-secret",
70+
GITHUB_TOKEN: "gh-secret",
71+
HF_TOKEN: "hf-secret",
72+
OPENCLAW_API_KEY: "keep-me",
7373
PATH: "/usr/bin",
74-
[openAiApiKeyEnv]: "openai-test-value", // pragma: allowlist secret
75-
[elevenLabsApiKeyEnv]: "elevenlabs-test-value", // pragma: allowlist secret
76-
[anthropicApiKeyEnv]: "anthropic-test-value", // pragma: allowlist secret
7774
},
7875
{ stripKeys },
7976
);
8077

78+
expect(env.OPENAI_API_KEY).toBeUndefined();
79+
expect(env.GITHUB_TOKEN).toBeUndefined();
80+
expect(env.HF_TOKEN).toBeUndefined();
81+
expect(env.OPENCLAW_API_KEY).toBe("keep-me");
8182
expect(env.PATH).toBe("/usr/bin");
8283
expect(env.OPENCLAW_SHELL).toBe("acp-client");
83-
expect(env.ANTHROPIC_API_KEY).toBe("anthropic-test-value");
84-
expect(env.OPENAI_API_KEY).toBeUndefined();
85-
expect(env.ELEVENLABS_API_KEY).toBeUndefined();
8684
});
8785

88-
it("does not modify the original baseEnv when stripping keys", () => {
89-
const openAiApiKeyEnv = envVar("OPENAI", "API", "KEY");
90-
const baseEnv: NodeJS.ProcessEnv = {
91-
[openAiApiKeyEnv]: "openai-original", // pragma: allowlist secret
92-
PATH: "/usr/bin",
93-
};
94-
const stripKeys = new Set([openAiApiKeyEnv]);
95-
resolveAcpClientSpawnEnv(baseEnv, { stripKeys });
86+
it("preserves provider auth env vars for explicit custom ACP servers", () => {
87+
const env = resolveAcpClientSpawnEnv({
88+
OPENAI_API_KEY: "openai-secret",
89+
GITHUB_TOKEN: "gh-secret",
90+
HF_TOKEN: "hf-secret",
91+
OPENCLAW_API_KEY: "keep-me",
92+
});
9693

97-
expect(baseEnv.OPENAI_API_KEY).toBe("openai-original");
94+
expect(env.OPENAI_API_KEY).toBe("openai-secret");
95+
expect(env.GITHUB_TOKEN).toBe("gh-secret");
96+
expect(env.HF_TOKEN).toBe("hf-secret");
97+
expect(env.OPENCLAW_API_KEY).toBe("keep-me");
98+
expect(env.OPENCLAW_SHELL).toBe("acp-client");
9899
});
100+
});
99101

100-
it("preserves OPENCLAW_SHELL even when stripKeys contains it", () => {
101-
const openAiApiKeyEnv = envVar("OPENAI", "API", "KEY");
102-
const env = resolveAcpClientSpawnEnv(
103-
{
104-
OPENCLAW_SHELL: "skill-overridden",
105-
[openAiApiKeyEnv]: "openai-leaked", // pragma: allowlist secret
106-
},
107-
{ stripKeys: new Set(["OPENCLAW_SHELL", openAiApiKeyEnv]) },
108-
);
102+
describe("shouldStripProviderAuthEnvVarsForAcpServer", () => {
103+
it("strips provider auth env vars for the default bridge", () => {
104+
expect(shouldStripProviderAuthEnvVarsForAcpServer()).toBe(true);
105+
});
109106

110-
expect(env.OPENCLAW_SHELL).toBe("acp-client");
111-
expect(env.OPENAI_API_KEY).toBeUndefined();
107+
it("preserves provider auth env vars for explicit custom ACP servers", () => {
108+
expect(shouldStripProviderAuthEnvVarsForAcpServer("custom-acp-server")).toBe(false);
112109
});
113110
});
114111

‎src/acp/client.ts‎

Lines changed: 20 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import {
2020
resolveWindowsSpawnProgram,
2121
} from "../plugin-sdk/windows-spawn.js";
2222
import { DANGEROUS_ACP_TOOLS } from "../security/dangerous-tools.js";
23+
import { listKnownProviderAuthEnvVarNames } from "../secrets/provider-env-vars.js";
2324

2425
const SAFE_AUTO_APPROVE_TOOL_IDS = new Set(["read", "search", "web_search", "memory_search"]);
2526
const TRUSTED_SAFE_TOOL_ALIASES = new Set(["search"]);
@@ -346,20 +347,25 @@ function buildServerArgs(opts: AcpClientOptions): string[] {
346347
return args;
347348
}
348349

350+
export type AcpClientSpawnEnvOptions = {
351+
stripKeys?: Iterable<string>;
352+
};
353+
349354
export function resolveAcpClientSpawnEnv(
350355
baseEnv: NodeJS.ProcessEnv = process.env,
351-
options?: { stripKeys?: ReadonlySet<string> },
356+
options: AcpClientSpawnEnvOptions = {},
352357
): NodeJS.ProcessEnv {
353-
const env: NodeJS.ProcessEnv = { ...baseEnv };
354-
if (options?.stripKeys) {
355-
for (const key of options.stripKeys) {
356-
delete env[key];
357-
}
358+
const env = { ...baseEnv, OPENCLAW_SHELL: "acp-client" };
359+
for (const key of options.stripKeys ?? []) {
360+
delete env[key];
358361
}
359-
env.OPENCLAW_SHELL = "acp-client";
360362
return env;
361363
}
362364

365+
export function shouldStripProviderAuthEnvVarsForAcpServer(serverCommand?: string): boolean {
366+
return !serverCommand;
367+
}
368+
363369
type AcpSpawnRuntime = {
364370
platform: NodeJS.Platform;
365371
env: NodeJS.ProcessEnv;
@@ -458,14 +464,13 @@ export async function createAcpClient(opts: AcpClientOptions = {}): Promise<AcpC
458464
const entryPath = resolveSelfEntryPath();
459465
const serverCommand = opts.serverCommand ?? (entryPath ? process.execPath : "openclaw");
460466
const effectiveArgs = opts.serverCommand || !entryPath ? serverArgs : [entryPath, ...serverArgs];
461-
const { getActiveSkillEnvKeys } = await import("../agents/skills/env-overrides.runtime.js");
462-
const { listKnownProviderEnvApiKeyNames } = await import("../agents/model-auth-env-vars.js");
463-
const stripKeys = new Set(getActiveSkillEnvKeys());
464-
// Strip provider API keys so they don't leak to ACP child processes.
465-
// Without this, env vars like OPENAI_API_KEY cause Codex CLI to overwrite
466-
// its OAuth credentials in ~/.codex/auth.json with apikey mode.
467-
for (const key of listKnownProviderEnvApiKeyNames()) {
468-
stripKeys.add(key);
467+
const stripKeys = new Set<string>();
468+
if (shouldStripProviderAuthEnvVarsForAcpServer(opts.serverCommand)) {
469+
for (const key of listKnownProviderAuthEnvVarNames()) {
470+
if (key !== "OPENCLAW_API_KEY") {
471+
stripKeys.add(key);
472+
}
473+
}
469474
}
470475
const spawnEnv = resolveAcpClientSpawnEnv(process.env, { stripKeys });
471476
const spawnInvocation = resolveAcpClientSpawnInvocation(

‎src/plugin-sdk/acpx.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,3 +32,4 @@ export {
3232
materializeWindowsSpawnProgram,
3333
resolveWindowsSpawnProgramCandidate,
3434
} from "./windows-spawn.js";
35+
export { listKnownProviderAuthEnvVarNames } from "../secrets/provider-env-vars.js";

‎src/plugin-sdk/subpaths.test.ts‎

Lines changed: 5 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -98,26 +98,16 @@ describe("plugin-sdk subpath exports", () => {
9898
expect(typeof msteamsSdk.loadOutboundMediaFromUrl).toBe("function");
9999
});
100100

101+
it("exports acpx helpers", async () => {
102+
const acpxSdk = await import("openclaw/plugin-sdk/acpx");
103+
expect(typeof acpxSdk.listKnownProviderAuthEnvVarNames).toBe("function");
104+
});
105+
101106
it("resolves bundled extension subpaths", async () => {
102107
for (const { id, load } of bundledExtensionSubpathLoaders) {
103108
const mod = await load();
104109
expect(typeof mod).toBe("object");
105110
expect(mod, `subpath ${id} should resolve`).toBeTruthy();
106111
}
107112
});
108-
109-
it("keeps the newly added bundled plugin-sdk contracts available", async () => {
110-
const bluebubbles = await import("openclaw/plugin-sdk/bluebubbles");
111-
expect(typeof bluebubbles.parseFiniteNumber).toBe("function");
112-
113-
const mattermost = await import("openclaw/plugin-sdk/mattermost");
114-
expect(typeof mattermost.parseStrictPositiveInteger).toBe("function");
115-
116-
const nextcloudTalk = await import("openclaw/plugin-sdk/nextcloud-talk");
117-
expect(typeof nextcloudTalk.waitForAbortSignal).toBe("function");
118-
119-
const twitch = await import("openclaw/plugin-sdk/twitch");
120-
expect(typeof twitch.DEFAULT_ACCOUNT_ID).toBe("string");
121-
expect(typeof twitch.normalizeAccountId).toBe("function");
122-
});
123113
});

‎src/secrets/provider-env-vars.ts‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,34 @@ export const PROVIDER_ENV_VARS: Record<string, readonly string[]> = {
2525
byteplus: ["BYTEPLUS_API_KEY"],
2626
};
2727

28+
const EXTRA_PROVIDER_AUTH_ENV_VARS = [
29+
"VOYAGE_API_KEY",
30+
"GROQ_API_KEY",
31+
"DEEPGRAM_API_KEY",
32+
"CEREBRAS_API_KEY",
33+
"NVIDIA_API_KEY",
34+
"COPILOT_GITHUB_TOKEN",
35+
"GH_TOKEN",
36+
"GITHUB_TOKEN",
37+
"ANTHROPIC_OAUTH_TOKEN",
38+
"CHUTES_OAUTH_TOKEN",
39+
"CHUTES_API_KEY",
40+
"QWEN_OAUTH_TOKEN",
41+
"QWEN_PORTAL_API_KEY",
42+
"MINIMAX_OAUTH_TOKEN",
43+
"OLLAMA_API_KEY",
44+
"VLLM_API_KEY",
45+
] as const;
46+
47+
export function listKnownProviderAuthEnvVarNames(): string[] {
48+
return [
49+
...new Set([
50+
...Object.values(PROVIDER_ENV_VARS).flatMap((keys) => keys),
51+
...EXTRA_PROVIDER_AUTH_ENV_VARS,
52+
]),
53+
];
54+
}
55+
2856
export function listKnownSecretEnvVarNames(): string[] {
2957
return [...new Set(Object.values(PROVIDER_ENV_VARS).flatMap((keys) => keys))];
3058
}

0 commit comments

Comments
 (0)