Skip to content

Commit b48b528

Browse files
pgondhi987jacobtomlinson
pgondhi987
and
jacobtomlinson
authored
fix(skills): block UV_PYTHON in workspace dotenv and harden uv installer env [AI] (#59178)
* fix: address issue * fix: finalize issue changes * fix: address PR review feedback * Changelog: note uv installer env hardening --------- Co-authored-by: Jacob Tomlinson <[email protected]>
1 parent 8b5e80f commit b48b528

9 files changed

Lines changed: 156 additions & 2 deletions

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ Docs: https://docs.openclaw.ai
1414

1515
### Fixes
1616

17+
- Skills/uv install: block workspace `.env` from overriding `UV_PYTHON` and strip related interpreter override keys from uv skill-install subprocesses so repository-controlled env files cannot steer the selected Python runtime. (#59178) Thanks @pgondhi987.
1718
- Telegram/reactions: preserve `reactionNotifications: "own"` across gateway restarts by persisting sent-message ownership state instead of treating cold cache as a permissive fallback. (#59207) Thanks @samzong.
1819
- Gateway/startup: detect PID recycling in gateway lock files on Windows and macOS, and add startup progress so stale lock conflicts no longer block healthy restarts. (#59843) Thanks @TonyDerek-dot.
1920
- MS Teams/DM media: download inline images in 1:1 chats via Graph API so Teams DM image attachments stop failing to load. (#52212) Thanks @Ted-developer.

‎apps/macos/Sources/OpenClaw/HostEnvSecurityPolicy.generated.swift‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,7 @@ enum HostEnvSecurityPolicy {
110110
"PIP_TRUSTED_HOST",
111111
"UV_INDEX",
112112
"UV_INDEX_URL",
113+
"UV_PYTHON",
113114
"UV_EXTRA_INDEX_URL",
114115
"UV_DEFAULT_INDEX",
115116
"DOCKER_HOST",

‎src/agents/skills-install-fallback.test.ts‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import fs from "node:fs/promises";
22
import os from "node:os";
33
import path from "node:path";
44
import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } from "vitest";
5+
import { captureEnv } from "../test-utils/env.js";
56
import {
67
hasBinaryMock,
78
runCommandWithTimeoutMock,
@@ -199,4 +200,52 @@ describe("skills-install fallback edge cases", () => {
199200
// Verify NO curl command was attempted (no auto-install)
200201
expect(runCommandWithTimeoutMock).not.toHaveBeenCalled();
201202
});
203+
204+
it("preserves system uv/python env vars when running uv installs", async () => {
205+
mockAvailableBinaries(["uv"]);
206+
runCommandWithTimeoutMock.mockResolvedValueOnce({
207+
code: 0,
208+
stdout: "ok",
209+
stderr: "",
210+
signal: null,
211+
killed: false,
212+
});
213+
214+
const envSnapshot = captureEnv([
215+
"UV_PYTHON",
216+
"UV_INDEX_URL",
217+
"PIP_INDEX_URL",
218+
"PYTHONPATH",
219+
"VIRTUAL_ENV",
220+
]);
221+
try {
222+
process.env.UV_PYTHON = "/tmp/attacker-python";
223+
process.env.UV_INDEX_URL = "https://example.invalid/simple";
224+
process.env.PIP_INDEX_URL = "https://example.invalid/pip";
225+
process.env.PYTHONPATH = "/tmp/attacker-pythonpath";
226+
process.env.VIRTUAL_ENV = "/tmp/attacker-venv";
227+
228+
const result = await installSkill({
229+
workspaceDir,
230+
skillName: "py-tool",
231+
installId: "deps",
232+
timeoutMs: 10_000,
233+
});
234+
235+
expect(result.ok).toBe(true);
236+
expect(runCommandWithTimeoutMock).toHaveBeenCalledWith(
237+
["uv", "tool", "install", "example-package"],
238+
expect.objectContaining({
239+
timeoutMs: 10_000,
240+
}),
241+
);
242+
const firstCall = runCommandWithTimeoutMock.mock.calls[0] as
243+
| [string[], { timeoutMs?: number; env?: Record<string, string | undefined> }]
244+
| undefined;
245+
const envArg = firstCall?.[1]?.env;
246+
expect(envArg).toBeUndefined();
247+
} finally {
248+
envSnapshot.restore();
249+
}
250+
});
202251
});

‎src/agents/skills-install-uv-python-env-override.poc.test.ts‎

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
import fs from "node:fs/promises";
2+
import os from "node:os";
3+
import path from "node:path";
4+
import { afterEach, describe, expect, it } from "vitest";
5+
import { loadWorkspaceDotEnvFile } from "../infra/dotenv.js";
6+
import { captureEnv } from "../test-utils/env.js";
7+
import { installSkill } from "./skills-install.js";
8+
9+
describe("workspace .env UV_PYTHON handling for uv skill installs", () => {
10+
let envSnapshot: ReturnType<typeof captureEnv> | undefined;
11+
12+
afterEach(async () => {
13+
envSnapshot?.restore();
14+
envSnapshot = undefined;
15+
});
16+
17+
it.runIf(process.platform !== "win32")(
18+
"does not propagate UV_PYTHON from workspace dotenv into uv tool install execution",
19+
async () => {
20+
const base = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-poc-uv-python-"));
21+
const cwdDir = path.join(base, "cwd");
22+
const binDir = path.join(base, "bin");
23+
const markerPath = path.join(base, "uv-python-marker.txt");
24+
const fakeUvPath = path.join(binDir, "uv");
25+
try {
26+
await fs.mkdir(cwdDir, { recursive: true });
27+
await fs.mkdir(binDir, { recursive: true });
28+
await fs.mkdir(path.join(cwdDir, "skills", "uv-skill"), { recursive: true });
29+
30+
await fs.writeFile(
31+
path.join(cwdDir, "skills", "uv-skill", "SKILL.md"),
32+
[
33+
"---",
34+
"name: uv-skill",
35+
"description: uv install PoC",
36+
'metadata: {"openclaw":{"install":[{"id":"deps","kind":"uv","package":"httpie==3.2.2"}]}}',
37+
"---",
38+
"",
39+
"# uv-skill",
40+
"",
41+
].join("\n"),
42+
"utf8",
43+
);
44+
45+
await fs.writeFile(
46+
fakeUvPath,
47+
[
48+
"#!/bin/sh",
49+
'printf "%s\\n" "$UV_PYTHON" > "$OPENCLAW_POC_MARKER_PATH"',
50+
"exit 0",
51+
"",
52+
].join("\n"),
53+
"utf8",
54+
);
55+
await fs.chmod(fakeUvPath, 0o755);
56+
57+
const attackerPython = path.join(base, "attacker-python");
58+
await fs.writeFile(path.join(cwdDir, ".env"), `UV_PYTHON=${attackerPython}\n`, "utf8");
59+
60+
envSnapshot = captureEnv(["PATH", "UV_PYTHON", "OPENCLAW_POC_MARKER_PATH"]);
61+
delete process.env.UV_PYTHON;
62+
process.env.OPENCLAW_POC_MARKER_PATH = markerPath;
63+
process.env.PATH = `${binDir}${path.delimiter}${process.env.PATH ?? ""}`;
64+
65+
loadWorkspaceDotEnvFile(path.join(cwdDir, ".env"), { quiet: true });
66+
expect(process.env.UV_PYTHON).toBeUndefined();
67+
68+
const result = await installSkill({
69+
workspaceDir: cwdDir,
70+
skillName: "uv-skill",
71+
installId: "deps",
72+
timeoutMs: 10_000,
73+
});
74+
75+
expect(result.ok).toBe(true);
76+
await expect(fs.readFile(markerPath, "utf8")).resolves.toBe("\n");
77+
} finally {
78+
await fs.rm(base, { recursive: true, force: true });
79+
}
80+
},
81+
);
82+
});

‎src/agents/skills-install.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -507,13 +507,14 @@ export async function installSkill(params: SkillInstallRequest): Promise<SkillIn
507507
argv[0] = brewExe;
508508
}
509509

510-
let env: NodeJS.ProcessEnv | undefined;
510+
const envOverrides: NodeJS.ProcessEnv = {};
511511
if (spec.kind === "go" && brewExe) {
512512
const brewBin = await resolveBrewBinDir(timeoutMs, brewExe);
513513
if (brewBin) {
514-
env = { GOBIN: brewBin };
514+
envOverrides.GOBIN = brewBin;
515515
}
516516
}
517+
const env = Object.keys(envOverrides).length > 0 ? envOverrides : undefined;
517518

518519
return withWarnings(await executeInstallCommand({ argv, timeoutMs, env }), warnings);
519520
}

‎src/infra/dotenv.test.ts‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,8 @@ describe("loadDotEnv", () => {
109109
"OPENCLAW_CONFIG_PATH=./evil-config.json",
110110
"ANTHROPIC_BASE_URL=https://evil.example.com/v1",
111111
"HTTP_PROXY=http://evil-proxy:8080",
112+
"UV_PYTHON=./attacker-python",
113+
"uv_python=./attacker-python-lower",
112114
].join("\n"),
113115
);
114116
await writeEnvFile(path.join(stateDir, ".env"), "BAR=from-global\n");
@@ -119,6 +121,8 @@ describe("loadDotEnv", () => {
119121
delete process.env.OPENCLAW_CONFIG_PATH;
120122
delete process.env.ANTHROPIC_BASE_URL;
121123
delete process.env.HTTP_PROXY;
124+
delete process.env.UV_PYTHON;
125+
delete process.env.uv_python;
122126

123127
loadDotEnv({ quiet: true });
124128

@@ -129,6 +133,8 @@ describe("loadDotEnv", () => {
129133
expect(process.env.OPENCLAW_CONFIG_PATH).toBeUndefined();
130134
expect(process.env.ANTHROPIC_BASE_URL).toBeUndefined();
131135
expect(process.env.HTTP_PROXY).toBeUndefined();
136+
expect(process.env.UV_PYTHON).toBeUndefined();
137+
expect(process.env.uv_python).toBeUndefined();
132138
});
133139
});
134140
});
@@ -460,6 +466,8 @@ describe("loadCliDotEnv", () => {
460466
`OPENCLAW_BUNDLED_PLUGINS_DIR=${bundledPluginsDir}`,
461467
"NODE_OPTIONS=--require ./evil.js",
462468
"ANTHROPIC_BASE_URL=https://evil.example.com/v1",
469+
"UV_PYTHON=./attacker-python",
470+
"uv_python=./attacker-python-lower",
463471
].join("\n"),
464472
);
465473
await writeEnvFile(path.join(stateDir, ".env"), "BAR=from-global\n");
@@ -470,6 +478,8 @@ describe("loadCliDotEnv", () => {
470478
delete process.env.OPENCLAW_BUNDLED_PLUGINS_DIR;
471479
delete process.env.NODE_OPTIONS;
472480
delete process.env.ANTHROPIC_BASE_URL;
481+
delete process.env.UV_PYTHON;
482+
delete process.env.uv_python;
473483
delete process.env.BAR;
474484

475485
loadCliDotEnv({ quiet: true });
@@ -481,6 +491,8 @@ describe("loadCliDotEnv", () => {
481491
expect(process.env.OPENCLAW_BUNDLED_PLUGINS_DIR).toBeUndefined();
482492
expect(process.env.NODE_OPTIONS).toBeUndefined();
483493
expect(process.env.ANTHROPIC_BASE_URL).toBeUndefined();
494+
expect(process.env.UV_PYTHON).toBeUndefined();
495+
expect(process.env.uv_python).toBeUndefined();
484496
});
485497
});
486498
});

‎src/infra/dotenv.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ const BLOCKED_WORKSPACE_DOTENV_KEYS = new Set([
3838
"OPENAI_API_KEY",
3939
"OPENAI_API_KEYS",
4040
"PI_CODING_AGENT_DIR",
41+
"UV_PYTHON",
4142
]);
4243

4344
const BLOCKED_WORKSPACE_DOTENV_SUFFIXES = ["_BASE_URL"];

‎src/infra/host-env-security-policy.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,7 @@
103103
"PIP_TRUSTED_HOST",
104104
"UV_INDEX",
105105
"UV_INDEX_URL",
106+
"UV_PYTHON",
106107
"UV_EXTRA_INDEX_URL",
107108
"UV_DEFAULT_INDEX",
108109
"DOCKER_HOST",

‎src/infra/host-env-security.test.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -262,6 +262,7 @@ describe("sanitizeHostExecEnv", () => {
262262
PIP_TRUSTED_HOST: "example.invalid",
263263
UV_INDEX: "https://example.invalid/simple",
264264
UV_INDEX_URL: "https://example.invalid/simple",
265+
UV_PYTHON: "/tmp/evil-uv-python",
265266
UV_DEFAULT_INDEX: "https://example.invalid/simple",
266267
UV_EXTRA_INDEX_URL: "https://example.invalid/simple",
267268
DOCKER_HOST: "tcp://example.invalid:2376",
@@ -334,6 +335,7 @@ describe("sanitizeHostExecEnv", () => {
334335
expect(env.PIP_TRUSTED_HOST).toBeUndefined();
335336
expect(env.UV_INDEX).toBeUndefined();
336337
expect(env.UV_INDEX_URL).toBeUndefined();
338+
expect(env.UV_PYTHON).toBeUndefined();
337339
expect(env.UV_DEFAULT_INDEX).toBeUndefined();
338340
expect(env.UV_EXTRA_INDEX_URL).toBeUndefined();
339341
expect(env.DOCKER_HOST).toBeUndefined();
@@ -504,6 +506,7 @@ describe("isDangerousHostEnvOverrideVarName", () => {
504506
expect(isDangerousHostEnvOverrideVarName("PIP_EXTRA_INDEX_URL")).toBe(true);
505507
expect(isDangerousHostEnvOverrideVarName("UV_INDEX")).toBe(true);
506508
expect(isDangerousHostEnvOverrideVarName("UV_INDEX_URL")).toBe(true);
509+
expect(isDangerousHostEnvOverrideVarName("uv_python")).toBe(true);
507510
expect(isDangerousHostEnvOverrideVarName("uv_default_index")).toBe(true);
508511
expect(isDangerousHostEnvOverrideVarName("UV_EXTRA_INDEX_URL")).toBe(true);
509512
expect(isDangerousHostEnvOverrideVarName("DOCKER_HOST")).toBe(true);
@@ -556,6 +559,7 @@ describe("sanitizeHostExecEnvWithDiagnostics", () => {
556559
PIP_TRUSTED_HOST: "example.invalid",
557560
UV_INDEX: "https://example.invalid/simple",
558561
UV_INDEX_URL: "https://example.invalid/simple",
562+
UV_PYTHON: "/tmp/evil-uv-python",
559563
UV_DEFAULT_INDEX: "https://example.invalid/simple",
560564
UV_EXTRA_INDEX_URL: "https://example.invalid/simple",
561565
DOCKER_HOST: "tcp://example.invalid:2376",
@@ -635,6 +639,7 @@ describe("sanitizeHostExecEnvWithDiagnostics", () => {
635639
"UV_EXTRA_INDEX_URL",
636640
"UV_INDEX",
637641
"UV_INDEX_URL",
642+
"UV_PYTHON",
638643
"VIRTUAL_ENV",
639644
"YARN_RC_FILENAME",
640645
]);
@@ -653,6 +658,7 @@ describe("sanitizeHostExecEnvWithDiagnostics", () => {
653658
expect(result.env.PIP_TRUSTED_HOST).toBeUndefined();
654659
expect(result.env.UV_INDEX).toBeUndefined();
655660
expect(result.env.UV_INDEX_URL).toBeUndefined();
661+
expect(result.env.UV_PYTHON).toBeUndefined();
656662
expect(result.env.UV_DEFAULT_INDEX).toBeUndefined();
657663
expect(result.env.UV_EXTRA_INDEX_URL).toBeUndefined();
658664
expect(result.env.GIT_SSL_NO_VERIFY).toBeUndefined();

0 commit comments

Comments
 (0)