vultr/openclaw
1
0
Fork 0
mirror of https://github.com/openclaw/openclaw.git synced 2026-05-06 05:20:43 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(infra): block ambient Homebrew env vars from brew resolution (#74463)

Browse Source 
* fix: address issue

* fix: address issue

* fix: address PR review feedback

* fix: address PR review feedback

* fix: address codex review feedback

* docs: add changelog entry for PR merge
This commit is contained in:
Pavan Kumar Gondhi
2026-05-01 18:23:34 +05:30
committed by GitHub
parent 94b4b3c644
commit f86953f354
8 changed files with 226 additions and 104 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
CHANGELOG.md
View File

@@ -22,6 +22,7 @@ Docs: https://docs.openclaw.ai
### Fixes
- fix(infra): block ambient Homebrew env vars from brew resolution. (#74463) Thanks @pgondhi987.
- Thinking/providers: resolve bundled provider thinking profiles through lightweight provider policy artifacts when startup-lazy providers are not active, so OpenAI Codex GPT-5.x keeps xhigh available in Gateway session validation. Fixes #74796. Thanks @maxschachere.
- Plugins/TTS: keep bundled speech-provider discovery available on cold package Gateway paths and add bundled plugin matrix runtime probes for health, readiness, RPC, TTS discovery, and post-ready runtime-deps watchdog coverage. Refs #75283. Thanks @vincentkoc.
- Google Meet/Twilio: show delegated voice call ID, DTMF, and intro-greeting state in `googlemeet doctor`, and avoid claiming DTMF was sent when no Meet PIN sequence was configured. Refs #72478. Thanks @DougButdorf.

59
src/agents/skills-install-fallback.test.ts
View File

@@ -165,6 +165,65 @@ describe("skills-install fallback edge cases", () => {
expect(runCommandWithTimeoutMock).not.toHaveBeenCalled();
});
it("does not use HOMEBREW_PREFIX as a brew bin fallback for go installs", async () => {
const envSnapshot = captureEnv(["HOMEBREW_PREFIX"]);
try {
const maliciousPrefix = path.join(workspaceDir, "evil-brew");
process.env.HOMEBREW_PREFIX = maliciousPrefix;
mockAvailableBinaries([]);
skillsInstallTesting.setDepsForTest({
hasBinary: (bin: string) => hasBinaryMock(bin),
resolveBrewExecutable: () => "/safe/homebrew/bin/brew",
});
runCommandWithTimeoutMock.mockResolvedValue({
code: 0,
stdout: "ok",
stderr: "",
signal: null,
killed: false,
});
runCommandWithTimeoutMock.mockResolvedValueOnce({
code: 0,
stdout: "installed go",
stderr: "",
signal: null,
killed: false,
});
runCommandWithTimeoutMock.mockResolvedValueOnce({
code: 1,
stdout: "",
stderr: "prefix unavailable",
signal: null,
killed: false,
});
const result = await installSkill({
workspaceDir,
skillName: "go-tool-single",
installId: "deps",
});
expect(result.ok).toBe(true);
expect(runCommandWithTimeoutMock).toHaveBeenNthCalledWith(
1,
["/safe/homebrew/bin/brew", "install", "go"],
expect.objectContaining({ timeoutMs: 300_000 }),
);
expect(runCommandWithTimeoutMock).toHaveBeenNthCalledWith(
2,
["/safe/homebrew/bin/brew", "--prefix"],
expect.objectContaining({ timeoutMs: 30_000 }),
);
const finalCall = runCommandWithTimeoutMock.mock.calls.at(-1) as
| [string[], { env?: NodeJS.ProcessEnv }]
| undefined;
expect(finalCall?.[0]).toEqual(["go", "install", "example.com/tool@latest"]);
expect(finalCall?.[1]?.env?.GOBIN).not.toBe(path.join(maliciousPrefix, "bin"));
} finally {
envSnapshot.restore();
}
});
it("preserves system uv/python env vars when running uv installs", async () => {
mockAvailableBinaries(["uv"]);
runCommandWithTimeoutMock.mockResolvedValueOnce({

5
src/agents/skills-install.ts
View File

@@ -234,11 +234,6 @@ async function resolveBrewBinDir(timeoutMs: number, brewExe?: string): Promise<s
}
}
const envPrefix = process.env.HOMEBREW_PREFIX?.trim();
if (envPrefix) {
return path.join(envPrefix, "bin");
}
for (const candidate of ["/opt/homebrew/bin", "/usr/local/bin"]) {
try {
if (fs.existsSync(candidate)) {

178
src/infra/brew.test.ts
View File

@@ -4,6 +4,8 @@ import { describe, expect, it } from "vitest";
import { withTempDir } from "../test-helpers/temp-dir.js";
import { resolveBrewExecutable, resolveBrewPathDirs } from "./brew.js";
const HOMEBREW_ENV_KEYS = ["HOMEBREW_BREW_FILE", "HOMEBREW_PREFIX"] as const;
describe("brew helpers", () => {
async function writeExecutable(filePath: string) {
await fs.mkdir(path.dirname(filePath), { recursive: true });
@@ -11,71 +13,105 @@ describe("brew helpers", () => {
await fs.chmod(filePath, 0o755);
}
async function withHomebrewEnv(
values: Partial<Record<(typeof HOMEBREW_ENV_KEYS)[number], string>>,
run: () => Promise<void>,
) {
const previous = Object.fromEntries(
HOMEBREW_ENV_KEYS.map((key) => [key, process.env[key]]),
) as Record<(typeof HOMEBREW_ENV_KEYS)[number], string | undefined>;
try {
for (const key of HOMEBREW_ENV_KEYS) {
const value = values[key];
if (value === undefined) {
delete process.env[key];
} else {
process.env[key] = value;
}
}
await run();
} finally {
for (const key of HOMEBREW_ENV_KEYS) {
const value = previous[key];
if (value === undefined) {
delete process.env[key];
} else {
process.env[key] = value;
}
}
}
}
async function withPathEnv(value: string | undefined, run: () => Promise<void>) {
const previous = process.env.PATH;
try {
if (value === undefined) {
delete process.env.PATH;
} else {
process.env.PATH = value;
}
await run();
} finally {
if (previous === undefined) {
delete process.env.PATH;
} else {
process.env.PATH = previous;
}
}
}
it("resolves brew from ~/.linuxbrew/bin when executable exists", async () => {
await withTempDir({ prefix: "openclaw-brew-" }, async (tmp) => {
const homebrewBin = path.join(tmp, ".linuxbrew", "bin");
const brewPath = path.join(homebrewBin, "brew");
await writeExecutable(brewPath);
const env: NodeJS.ProcessEnv = {};
expect(resolveBrewExecutable({ homeDir: tmp, env })).toBe(brewPath);
await withPathEnv("", async () => {
expect(resolveBrewExecutable({ homeDir: tmp })).toBe(brewPath);
});
});
});
it("prefers HOMEBREW_PREFIX/bin/brew when present", async () => {
it("resolves brew from absolute PATH entries for non-standard installs", async () => {
await withTempDir({ prefix: "openclaw-brew-" }, async (tmp) => {
const customBin = path.join(tmp, "custom-homebrew", "bin");
const customBrew = path.join(customBin, "brew");
await writeExecutable(customBrew);
await withPathEnv(customBin, async () => {
expect(resolveBrewExecutable({ homeDir: path.join(tmp, "home") })).toBe(customBrew);
});
});
});
it("ignores HOMEBREW_BREW_FILE and HOMEBREW_PREFIX by default", async () => {
await withTempDir({ prefix: "openclaw-brew-" }, async (tmp) => {
const explicit = path.join(tmp, "custom", "brew");
const prefix = path.join(tmp, "prefix");
const prefixBin = path.join(prefix, "bin");
const prefixBrew = path.join(prefixBin, "brew");
await writeExecutable(prefixBrew);
const homebrewBin = path.join(tmp, ".linuxbrew", "bin");
const homebrewBrew = path.join(homebrewBin, "brew");
await writeExecutable(homebrewBrew);
const env: NodeJS.ProcessEnv = { HOMEBREW_PREFIX: prefix };
expect(resolveBrewExecutable({ homeDir: tmp, env })).toBe(prefixBrew);
});
});
it("prefers HOMEBREW_BREW_FILE over prefix and trims value", async () => {
await withTempDir({ prefix: "openclaw-brew-" }, async (tmp) => {
const explicit = path.join(tmp, "custom", "brew");
const prefix = path.join(tmp, "prefix");
const prefixBrew = path.join(prefix, "bin", "brew");
await writeExecutable(explicit);
await writeExecutable(prefixBrew);
await writeExecutable(homebrewBrew);
const env: NodeJS.ProcessEnv = {
HOMEBREW_BREW_FILE: ` ${explicit} `,
HOMEBREW_PREFIX: prefix,
};
expect(resolveBrewExecutable({ homeDir: tmp, env })).toBe(explicit);
});
});
it("falls back to prefix when HOMEBREW_BREW_FILE is missing or not executable", async () => {
await withTempDir({ prefix: "openclaw-brew-" }, async (tmp) => {
const explicit = path.join(tmp, "custom", "brew");
const prefix = path.join(tmp, "prefix");
const prefixBrew = path.join(prefix, "bin", "brew");
let brewFile = explicit;
if (process.platform === "win32") {
// Windows doesn't enforce POSIX executable bits, so use a missing path
// to verify fallback behavior deterministically.
brewFile = path.join(tmp, "custom", "missing-brew");
} else {
await fs.mkdir(path.dirname(explicit), { recursive: true });
await fs.writeFile(explicit, "#!/bin/sh\necho no\n", "utf-8");
await fs.chmod(explicit, 0o644);
}
await writeExecutable(prefixBrew);
const env: NodeJS.ProcessEnv = {
HOMEBREW_BREW_FILE: brewFile,
HOMEBREW_PREFIX: prefix,
};
expect(resolveBrewExecutable({ homeDir: tmp, env })).toBe(prefixBrew);
await withHomebrewEnv(
{
HOMEBREW_BREW_FILE: explicit,
HOMEBREW_PREFIX: prefix,
},
async () => {
const env: NodeJS.ProcessEnv = {
HOMEBREW_BREW_FILE: explicit,
HOMEBREW_PREFIX: prefix,
};
await withPathEnv("", async () => {
expect(resolveBrewExecutable({ homeDir: tmp, env })).toBe(homebrewBrew);
});
expect(resolveBrewPathDirs({ homeDir: tmp, env })).not.toContain(prefixBin);
},
);
});
});
@@ -85,37 +121,39 @@ describe("brew helpers", () => {
const brewPath = path.join(homebrewBin, "brew");
await writeExecutable(brewPath);
const env: NodeJS.ProcessEnv = {
HOMEBREW_BREW_FILE: " ",
HOMEBREW_PREFIX: "\t",
};
await withHomebrewEnv(
{
HOMEBREW_BREW_FILE: " ",
HOMEBREW_PREFIX: "\t",
},
async () => {
await withPathEnv("", async () => {
expect(resolveBrewExecutable({ homeDir: tmp })).toBe(brewPath);
});
expect(resolveBrewExecutable({ homeDir: tmp, env })).toBe(brewPath);
const dirs = resolveBrewPathDirs({ homeDir: tmp, env });
expect(dirs).not.toContain(path.join("", "bin"));
expect(dirs).not.toContain(path.join("", "sbin"));
const dirs = resolveBrewPathDirs({ homeDir: tmp });
expect(dirs).not.toContain(path.join("", "bin"));
expect(dirs).not.toContain(path.join("", "sbin"));
},
);
});
});
it("always includes the standard macOS brew dirs after linuxbrew candidates", () => {
const env: NodeJS.ProcessEnv = {
HOMEBREW_BREW_FILE: " ",
HOMEBREW_PREFIX: " ",
};
const dirs = resolveBrewPathDirs({ homeDir: "/home/test", env });
const dirs = resolveBrewPathDirs({ homeDir: "/home/test" });
expect(dirs.slice(-2)).toEqual(["/opt/homebrew/bin", "/usr/local/bin"]);
});
it("includes Linuxbrew bin/sbin in path candidates", () => {
const env: NodeJS.ProcessEnv = { HOMEBREW_PREFIX: "/custom/prefix" };
const dirs = resolveBrewPathDirs({ homeDir: "/home/test", env });
expect(dirs).toContain(path.join("/custom/prefix", "bin"));
expect(dirs).toContain(path.join("/custom/prefix", "sbin"));
expect(dirs).toContain("/home/linuxbrew/.linuxbrew/bin");
expect(dirs).toContain("/home/linuxbrew/.linuxbrew/sbin");
expect(dirs).toContain(path.join("/home/test", ".linuxbrew", "bin"));
expect(dirs).toContain(path.join("/home/test", ".linuxbrew", "sbin"));
it("includes Linuxbrew bin/sbin in path candidates without env prefixes", async () => {
await withHomebrewEnv({ HOMEBREW_PREFIX: "/custom/prefix" }, async () => {
const dirs = resolveBrewPathDirs({ homeDir: "/home/test" });
expect(dirs).not.toContain(path.join("/custom/prefix", "bin"));
expect(dirs).not.toContain(path.join("/custom/prefix", "sbin"));
expect(dirs).toContain("/home/linuxbrew/.linuxbrew/bin");
expect(dirs).toContain("/home/linuxbrew/.linuxbrew/sbin");
expect(dirs).toContain(path.join("/home/test", ".linuxbrew", "bin"));
expect(dirs).toContain(path.join("/home/test", ".linuxbrew", "sbin"));
});
});
});

56
src/infra/brew.ts
View File

@@ -11,26 +11,33 @@ function isExecutable(filePath: string): boolean {
}
}
function normalizePathValue(value: unknown): string | undefined {
if (typeof value !== "string") {
return undefined;
type BrewResolutionOptions = {
homeDir?: string;
/**
* @deprecated No-op compatibility field for plugin SDK callers. Homebrew
* env vars are ignored for resolution because workspace env can be untrusted.
*/
env?: NodeJS.ProcessEnv;
};
function resolveBrewFromPath(pathEnv = process.env.PATH): string | undefined {
for (const dir of (pathEnv ?? "").split(path.delimiter)) {
const trimmed = dir.trim();
if (!trimmed || !path.isAbsolute(trimmed)) {
continue;
}
const candidate = path.join(trimmed, "brew");
if (isExecutable(candidate)) {
return candidate;
}
}
const trimmed = value.trim();
return trimmed ? trimmed : undefined;
return undefined;
}
export function resolveBrewPathDirs(opts?: {
homeDir?: string;
env?: NodeJS.ProcessEnv;
}): string[] {
export function resolveBrewPathDirs(opts?: BrewResolutionOptions): string[] {
const homeDir = opts?.homeDir ?? os.homedir();
const env = opts?.env ?? process.env;
const dirs: string[] = [];
const prefix = normalizePathValue(env.HOMEBREW_PREFIX);
if (prefix) {
dirs.push(path.join(prefix, "bin"), path.join(prefix, "sbin"));
}
// Linuxbrew defaults.
dirs.push(path.join(homeDir, ".linuxbrew", "bin"));
@@ -43,25 +50,16 @@ export function resolveBrewPathDirs(opts?: {
return dirs;
}
export function resolveBrewExecutable(opts?: {
homeDir?: string;
env?: NodeJS.ProcessEnv;
}): string | undefined {
export function resolveBrewExecutable(opts?: BrewResolutionOptions): string | undefined {
const homeDir = opts?.homeDir ?? os.homedir();
const env = opts?.env ?? process.env;
const pathBrew = resolveBrewFromPath();
if (pathBrew) {
return pathBrew;
}
const candidates: string[] = [];
const brewFile = normalizePathValue(env.HOMEBREW_BREW_FILE);
if (brewFile) {
candidates.push(brewFile);
}
const prefix = normalizePathValue(env.HOMEBREW_PREFIX);
if (prefix) {
candidates.push(path.join(prefix, "bin", "brew"));
}
// Linuxbrew defaults.
candidates.push(path.join(homeDir, ".linuxbrew", "bin", "brew"));
candidates.push("/home/linuxbrew/.linuxbrew/bin/brew");

8
src/infra/dotenv.test.ts
View File

@@ -212,6 +212,8 @@ describe("loadDotEnv", () => {
"EXAMPLE_API_HOST=https://evil-api.example.com",
"MINIMAX_API_HOST=https://evil.example.com",
"HTTP_PROXY=http://evil-proxy:8080",
"HOMEBREW_BREW_FILE=./evil-brew/bin/brew",
"HOMEBREW_PREFIX=./evil-brew",
"UV_PYTHON=./attacker-python",
"uv_python=./attacker-python-lower",
].join("\n"),
@@ -226,6 +228,8 @@ describe("loadDotEnv", () => {
delete process.env.EXAMPLE_API_HOST;
delete process.env.MINIMAX_API_HOST;
delete process.env.HTTP_PROXY;
delete process.env.HOMEBREW_BREW_FILE;
delete process.env.HOMEBREW_PREFIX;
delete process.env.UV_PYTHON;
delete process.env.uv_python;
@@ -240,6 +244,8 @@ describe("loadDotEnv", () => {
expect(process.env.EXAMPLE_API_HOST).toBeUndefined();
expect(process.env.MINIMAX_API_HOST).toBeUndefined();
expect(process.env.HTTP_PROXY).toBeUndefined();
expect(process.env.HOMEBREW_BREW_FILE).toBeUndefined();
expect(process.env.HOMEBREW_PREFIX).toBeUndefined();
expect(process.env.UV_PYTHON).toBeUndefined();
expect(process.env.uv_python).toBeUndefined();
});
@@ -650,6 +656,8 @@ describe("workspace .env blocklist completeness", () => {
"OPENCLAW_ALLOW_INSECURE_PRIVATE_WS",
"OPENCLAW_BROWSER_EXECUTABLE_PATH",
"EXAMPLE_API_HOST",
"HOMEBREW_BREW_FILE",
"HOMEBREW_PREFIX",
"IRC_HOST",
"MATTERMOST_URL",
"MATRIX_HOMESERVER",

2
src/infra/dotenv.ts
View File

@@ -24,6 +24,8 @@ const BLOCKED_WORKSPACE_DOTENV_KEYS = new Set([
"CLAWHUB_URL",
"HTTP_PROXY",
"HTTPS_PROXY",
"HOMEBREW_BREW_FILE",
"HOMEBREW_PREFIX",
"IRC_HOST",
"MATTERMOST_URL",
"MATRIX_HOMESERVER",

21
src/infra/path-env.test.ts
View File

@@ -345,4 +345,25 @@ describe("ensureOpenClawCliOnPath", () => {
const updated = bootstrapPath(params);
expectPathsAfter(updated, anchor, expectedPaths);
});
it("does not append HOMEBREW_PREFIX from process env", () => {
const { tmp, appCli } = setupAppCliRoot("case-homebrew-env-ignored");
const maliciousPrefix = path.join(tmp, "evil-brew");
const maliciousBin = path.join(maliciousPrefix, "bin");
const maliciousSbin = path.join(maliciousPrefix, "sbin");
setDir(maliciousBin);
setDir(maliciousSbin);
resetBootstrapEnv("/usr/bin:/bin");
process.env.HOMEBREW_PREFIX = maliciousPrefix;
const updated = bootstrapPath({
execPath: appCli,
cwd: tmp,
homeDir: tmp,
platform: "linux",
});
expect(updated).not.toContain(maliciousBin);
expect(updated).not.toContain(maliciousSbin);
});
});