diff --git a/CHANGELOG.md b/CHANGELOG.md index 59dc5912c39..b968bb5b82b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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. diff --git a/src/agents/skills-install-fallback.test.ts b/src/agents/skills-install-fallback.test.ts index 3f05b66db95..7a6105f0a90 100644 --- a/src/agents/skills-install-fallback.test.ts +++ b/src/agents/skills-install-fallback.test.ts @@ -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({ diff --git a/src/agents/skills-install.ts b/src/agents/skills-install.ts index 59dadb3cc0a..39568863ca8 100644 --- a/src/agents/skills-install.ts +++ b/src/agents/skills-install.ts @@ -234,11 +234,6 @@ async function resolveBrewBinDir(timeoutMs: number, brewExe?: string): Promise { 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>, + run: () => Promise, + ) { + 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) { + 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")); + }); }); }); diff --git a/src/infra/brew.ts b/src/infra/brew.ts index 0b600ee7cdc..8f87c927947 100644 --- a/src/infra/brew.ts +++ b/src/infra/brew.ts @@ -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"); diff --git a/src/infra/dotenv.test.ts b/src/infra/dotenv.test.ts index 2abe07764f4..ef17726deb2 100644 --- a/src/infra/dotenv.test.ts +++ b/src/infra/dotenv.test.ts @@ -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", diff --git a/src/infra/dotenv.ts b/src/infra/dotenv.ts index d15654668fa..c1e42288490 100644 --- a/src/infra/dotenv.ts +++ b/src/infra/dotenv.ts @@ -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", diff --git a/src/infra/path-env.test.ts b/src/infra/path-env.test.ts index c568665f6c6..cfdf86c8080 100644 --- a/src/infra/path-env.test.ts +++ b/src/infra/path-env.test.ts @@ -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); + }); });