mirror of
https://github.com/moltbot/moltbot.git
synced 2026-04-23 14:45:46 +00:00
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 <jtomlinson@nvidia.com>
This commit is contained in:
@@ -14,6 +14,7 @@ Docs: https://docs.openclaw.ai
|
||||
|
||||
### Fixes
|
||||
|
||||
- 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.
|
||||
- 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.
|
||||
- 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.
|
||||
- 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.
|
||||
|
||||
@@ -110,6 +110,7 @@ enum HostEnvSecurityPolicy {
|
||||
"PIP_TRUSTED_HOST",
|
||||
"UV_INDEX",
|
||||
"UV_INDEX_URL",
|
||||
"UV_PYTHON",
|
||||
"UV_EXTRA_INDEX_URL",
|
||||
"UV_DEFAULT_INDEX",
|
||||
"DOCKER_HOST",
|
||||
|
||||
@@ -2,6 +2,7 @@ import fs from "node:fs/promises";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { captureEnv } from "../test-utils/env.js";
|
||||
import {
|
||||
hasBinaryMock,
|
||||
runCommandWithTimeoutMock,
|
||||
@@ -199,4 +200,52 @@ describe("skills-install fallback edge cases", () => {
|
||||
// Verify NO curl command was attempted (no auto-install)
|
||||
expect(runCommandWithTimeoutMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("preserves system uv/python env vars when running uv installs", async () => {
|
||||
mockAvailableBinaries(["uv"]);
|
||||
runCommandWithTimeoutMock.mockResolvedValueOnce({
|
||||
code: 0,
|
||||
stdout: "ok",
|
||||
stderr: "",
|
||||
signal: null,
|
||||
killed: false,
|
||||
});
|
||||
|
||||
const envSnapshot = captureEnv([
|
||||
"UV_PYTHON",
|
||||
"UV_INDEX_URL",
|
||||
"PIP_INDEX_URL",
|
||||
"PYTHONPATH",
|
||||
"VIRTUAL_ENV",
|
||||
]);
|
||||
try {
|
||||
process.env.UV_PYTHON = "/tmp/attacker-python";
|
||||
process.env.UV_INDEX_URL = "https://example.invalid/simple";
|
||||
process.env.PIP_INDEX_URL = "https://example.invalid/pip";
|
||||
process.env.PYTHONPATH = "/tmp/attacker-pythonpath";
|
||||
process.env.VIRTUAL_ENV = "/tmp/attacker-venv";
|
||||
|
||||
const result = await installSkill({
|
||||
workspaceDir,
|
||||
skillName: "py-tool",
|
||||
installId: "deps",
|
||||
timeoutMs: 10_000,
|
||||
});
|
||||
|
||||
expect(result.ok).toBe(true);
|
||||
expect(runCommandWithTimeoutMock).toHaveBeenCalledWith(
|
||||
["uv", "tool", "install", "example-package"],
|
||||
expect.objectContaining({
|
||||
timeoutMs: 10_000,
|
||||
}),
|
||||
);
|
||||
const firstCall = runCommandWithTimeoutMock.mock.calls[0] as
|
||||
| [string[], { timeoutMs?: number; env?: Record<string, string | undefined> }]
|
||||
| undefined;
|
||||
const envArg = firstCall?.[1]?.env;
|
||||
expect(envArg).toBeUndefined();
|
||||
} finally {
|
||||
envSnapshot.restore();
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
82
src/agents/skills-install-uv-python-env-override.poc.test.ts
Normal file
82
src/agents/skills-install-uv-python-env-override.poc.test.ts
Normal file
@@ -0,0 +1,82 @@
|
||||
import fs from "node:fs/promises";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import { afterEach, describe, expect, it } from "vitest";
|
||||
import { loadWorkspaceDotEnvFile } from "../infra/dotenv.js";
|
||||
import { captureEnv } from "../test-utils/env.js";
|
||||
import { installSkill } from "./skills-install.js";
|
||||
|
||||
describe("workspace .env UV_PYTHON handling for uv skill installs", () => {
|
||||
let envSnapshot: ReturnType<typeof captureEnv> | undefined;
|
||||
|
||||
afterEach(async () => {
|
||||
envSnapshot?.restore();
|
||||
envSnapshot = undefined;
|
||||
});
|
||||
|
||||
it.runIf(process.platform !== "win32")(
|
||||
"does not propagate UV_PYTHON from workspace dotenv into uv tool install execution",
|
||||
async () => {
|
||||
const base = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-poc-uv-python-"));
|
||||
const cwdDir = path.join(base, "cwd");
|
||||
const binDir = path.join(base, "bin");
|
||||
const markerPath = path.join(base, "uv-python-marker.txt");
|
||||
const fakeUvPath = path.join(binDir, "uv");
|
||||
try {
|
||||
await fs.mkdir(cwdDir, { recursive: true });
|
||||
await fs.mkdir(binDir, { recursive: true });
|
||||
await fs.mkdir(path.join(cwdDir, "skills", "uv-skill"), { recursive: true });
|
||||
|
||||
await fs.writeFile(
|
||||
path.join(cwdDir, "skills", "uv-skill", "SKILL.md"),
|
||||
[
|
||||
"---",
|
||||
"name: uv-skill",
|
||||
"description: uv install PoC",
|
||||
'metadata: {"openclaw":{"install":[{"id":"deps","kind":"uv","package":"httpie==3.2.2"}]}}',
|
||||
"---",
|
||||
"",
|
||||
"# uv-skill",
|
||||
"",
|
||||
].join("\n"),
|
||||
"utf8",
|
||||
);
|
||||
|
||||
await fs.writeFile(
|
||||
fakeUvPath,
|
||||
[
|
||||
"#!/bin/sh",
|
||||
'printf "%s\\n" "$UV_PYTHON" > "$OPENCLAW_POC_MARKER_PATH"',
|
||||
"exit 0",
|
||||
"",
|
||||
].join("\n"),
|
||||
"utf8",
|
||||
);
|
||||
await fs.chmod(fakeUvPath, 0o755);
|
||||
|
||||
const attackerPython = path.join(base, "attacker-python");
|
||||
await fs.writeFile(path.join(cwdDir, ".env"), `UV_PYTHON=${attackerPython}\n`, "utf8");
|
||||
|
||||
envSnapshot = captureEnv(["PATH", "UV_PYTHON", "OPENCLAW_POC_MARKER_PATH"]);
|
||||
delete process.env.UV_PYTHON;
|
||||
process.env.OPENCLAW_POC_MARKER_PATH = markerPath;
|
||||
process.env.PATH = `${binDir}${path.delimiter}${process.env.PATH ?? ""}`;
|
||||
|
||||
loadWorkspaceDotEnvFile(path.join(cwdDir, ".env"), { quiet: true });
|
||||
expect(process.env.UV_PYTHON).toBeUndefined();
|
||||
|
||||
const result = await installSkill({
|
||||
workspaceDir: cwdDir,
|
||||
skillName: "uv-skill",
|
||||
installId: "deps",
|
||||
timeoutMs: 10_000,
|
||||
});
|
||||
|
||||
expect(result.ok).toBe(true);
|
||||
await expect(fs.readFile(markerPath, "utf8")).resolves.toBe("\n");
|
||||
} finally {
|
||||
await fs.rm(base, { recursive: true, force: true });
|
||||
}
|
||||
},
|
||||
);
|
||||
});
|
||||
@@ -507,13 +507,14 @@ export async function installSkill(params: SkillInstallRequest): Promise<SkillIn
|
||||
argv[0] = brewExe;
|
||||
}
|
||||
|
||||
let env: NodeJS.ProcessEnv | undefined;
|
||||
const envOverrides: NodeJS.ProcessEnv = {};
|
||||
if (spec.kind === "go" && brewExe) {
|
||||
const brewBin = await resolveBrewBinDir(timeoutMs, brewExe);
|
||||
if (brewBin) {
|
||||
env = { GOBIN: brewBin };
|
||||
envOverrides.GOBIN = brewBin;
|
||||
}
|
||||
}
|
||||
const env = Object.keys(envOverrides).length > 0 ? envOverrides : undefined;
|
||||
|
||||
return withWarnings(await executeInstallCommand({ argv, timeoutMs, env }), warnings);
|
||||
}
|
||||
|
||||
@@ -109,6 +109,8 @@ describe("loadDotEnv", () => {
|
||||
"OPENCLAW_CONFIG_PATH=./evil-config.json",
|
||||
"ANTHROPIC_BASE_URL=https://evil.example.com/v1",
|
||||
"HTTP_PROXY=http://evil-proxy:8080",
|
||||
"UV_PYTHON=./attacker-python",
|
||||
"uv_python=./attacker-python-lower",
|
||||
].join("\n"),
|
||||
);
|
||||
await writeEnvFile(path.join(stateDir, ".env"), "BAR=from-global\n");
|
||||
@@ -119,6 +121,8 @@ describe("loadDotEnv", () => {
|
||||
delete process.env.OPENCLAW_CONFIG_PATH;
|
||||
delete process.env.ANTHROPIC_BASE_URL;
|
||||
delete process.env.HTTP_PROXY;
|
||||
delete process.env.UV_PYTHON;
|
||||
delete process.env.uv_python;
|
||||
|
||||
loadDotEnv({ quiet: true });
|
||||
|
||||
@@ -129,6 +133,8 @@ describe("loadDotEnv", () => {
|
||||
expect(process.env.OPENCLAW_CONFIG_PATH).toBeUndefined();
|
||||
expect(process.env.ANTHROPIC_BASE_URL).toBeUndefined();
|
||||
expect(process.env.HTTP_PROXY).toBeUndefined();
|
||||
expect(process.env.UV_PYTHON).toBeUndefined();
|
||||
expect(process.env.uv_python).toBeUndefined();
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -460,6 +466,8 @@ describe("loadCliDotEnv", () => {
|
||||
`OPENCLAW_BUNDLED_PLUGINS_DIR=${bundledPluginsDir}`,
|
||||
"NODE_OPTIONS=--require ./evil.js",
|
||||
"ANTHROPIC_BASE_URL=https://evil.example.com/v1",
|
||||
"UV_PYTHON=./attacker-python",
|
||||
"uv_python=./attacker-python-lower",
|
||||
].join("\n"),
|
||||
);
|
||||
await writeEnvFile(path.join(stateDir, ".env"), "BAR=from-global\n");
|
||||
@@ -470,6 +478,8 @@ describe("loadCliDotEnv", () => {
|
||||
delete process.env.OPENCLAW_BUNDLED_PLUGINS_DIR;
|
||||
delete process.env.NODE_OPTIONS;
|
||||
delete process.env.ANTHROPIC_BASE_URL;
|
||||
delete process.env.UV_PYTHON;
|
||||
delete process.env.uv_python;
|
||||
delete process.env.BAR;
|
||||
|
||||
loadCliDotEnv({ quiet: true });
|
||||
@@ -481,6 +491,8 @@ describe("loadCliDotEnv", () => {
|
||||
expect(process.env.OPENCLAW_BUNDLED_PLUGINS_DIR).toBeUndefined();
|
||||
expect(process.env.NODE_OPTIONS).toBeUndefined();
|
||||
expect(process.env.ANTHROPIC_BASE_URL).toBeUndefined();
|
||||
expect(process.env.UV_PYTHON).toBeUndefined();
|
||||
expect(process.env.uv_python).toBeUndefined();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -38,6 +38,7 @@ const BLOCKED_WORKSPACE_DOTENV_KEYS = new Set([
|
||||
"OPENAI_API_KEY",
|
||||
"OPENAI_API_KEYS",
|
||||
"PI_CODING_AGENT_DIR",
|
||||
"UV_PYTHON",
|
||||
]);
|
||||
|
||||
const BLOCKED_WORKSPACE_DOTENV_SUFFIXES = ["_BASE_URL"];
|
||||
|
||||
@@ -103,6 +103,7 @@
|
||||
"PIP_TRUSTED_HOST",
|
||||
"UV_INDEX",
|
||||
"UV_INDEX_URL",
|
||||
"UV_PYTHON",
|
||||
"UV_EXTRA_INDEX_URL",
|
||||
"UV_DEFAULT_INDEX",
|
||||
"DOCKER_HOST",
|
||||
|
||||
@@ -262,6 +262,7 @@ describe("sanitizeHostExecEnv", () => {
|
||||
PIP_TRUSTED_HOST: "example.invalid",
|
||||
UV_INDEX: "https://example.invalid/simple",
|
||||
UV_INDEX_URL: "https://example.invalid/simple",
|
||||
UV_PYTHON: "/tmp/evil-uv-python",
|
||||
UV_DEFAULT_INDEX: "https://example.invalid/simple",
|
||||
UV_EXTRA_INDEX_URL: "https://example.invalid/simple",
|
||||
DOCKER_HOST: "tcp://example.invalid:2376",
|
||||
@@ -334,6 +335,7 @@ describe("sanitizeHostExecEnv", () => {
|
||||
expect(env.PIP_TRUSTED_HOST).toBeUndefined();
|
||||
expect(env.UV_INDEX).toBeUndefined();
|
||||
expect(env.UV_INDEX_URL).toBeUndefined();
|
||||
expect(env.UV_PYTHON).toBeUndefined();
|
||||
expect(env.UV_DEFAULT_INDEX).toBeUndefined();
|
||||
expect(env.UV_EXTRA_INDEX_URL).toBeUndefined();
|
||||
expect(env.DOCKER_HOST).toBeUndefined();
|
||||
@@ -504,6 +506,7 @@ describe("isDangerousHostEnvOverrideVarName", () => {
|
||||
expect(isDangerousHostEnvOverrideVarName("PIP_EXTRA_INDEX_URL")).toBe(true);
|
||||
expect(isDangerousHostEnvOverrideVarName("UV_INDEX")).toBe(true);
|
||||
expect(isDangerousHostEnvOverrideVarName("UV_INDEX_URL")).toBe(true);
|
||||
expect(isDangerousHostEnvOverrideVarName("uv_python")).toBe(true);
|
||||
expect(isDangerousHostEnvOverrideVarName("uv_default_index")).toBe(true);
|
||||
expect(isDangerousHostEnvOverrideVarName("UV_EXTRA_INDEX_URL")).toBe(true);
|
||||
expect(isDangerousHostEnvOverrideVarName("DOCKER_HOST")).toBe(true);
|
||||
@@ -556,6 +559,7 @@ describe("sanitizeHostExecEnvWithDiagnostics", () => {
|
||||
PIP_TRUSTED_HOST: "example.invalid",
|
||||
UV_INDEX: "https://example.invalid/simple",
|
||||
UV_INDEX_URL: "https://example.invalid/simple",
|
||||
UV_PYTHON: "/tmp/evil-uv-python",
|
||||
UV_DEFAULT_INDEX: "https://example.invalid/simple",
|
||||
UV_EXTRA_INDEX_URL: "https://example.invalid/simple",
|
||||
DOCKER_HOST: "tcp://example.invalid:2376",
|
||||
@@ -635,6 +639,7 @@ describe("sanitizeHostExecEnvWithDiagnostics", () => {
|
||||
"UV_EXTRA_INDEX_URL",
|
||||
"UV_INDEX",
|
||||
"UV_INDEX_URL",
|
||||
"UV_PYTHON",
|
||||
"VIRTUAL_ENV",
|
||||
"YARN_RC_FILENAME",
|
||||
]);
|
||||
@@ -653,6 +658,7 @@ describe("sanitizeHostExecEnvWithDiagnostics", () => {
|
||||
expect(result.env.PIP_TRUSTED_HOST).toBeUndefined();
|
||||
expect(result.env.UV_INDEX).toBeUndefined();
|
||||
expect(result.env.UV_INDEX_URL).toBeUndefined();
|
||||
expect(result.env.UV_PYTHON).toBeUndefined();
|
||||
expect(result.env.UV_DEFAULT_INDEX).toBeUndefined();
|
||||
expect(result.env.UV_EXTRA_INDEX_URL).toBeUndefined();
|
||||
expect(result.env.GIT_SSL_NO_VERIFY).toBeUndefined();
|
||||
|
||||
Reference in New Issue
Block a user