From bc3b05dce4b2ecd3534d6f233c9a00ad37afb9ae Mon Sep 17 00:00:00 2001 From: pgondhi987 Date: Mon, 30 Mar 2026 17:01:04 +0530 Subject: [PATCH] fix(infra): block BROWSER, GIT_EDITOR, GIT_SEQUENCE_EDITOR from inherited host env (#57559) --- .../HostEnvSecurityPolicy.generated.swift | 3 + src/infra/host-env-security-policy.json | 3 + src/infra/host-env-security.test.ts | 107 ++++++++++++++++++ 3 files changed, 113 insertions(+) diff --git a/apps/macos/Sources/OpenClaw/HostEnvSecurityPolicy.generated.swift b/apps/macos/Sources/OpenClaw/HostEnvSecurityPolicy.generated.swift index 1862621183d..3b59a2d12ae 100644 --- a/apps/macos/Sources/OpenClaw/HostEnvSecurityPolicy.generated.swift +++ b/apps/macos/Sources/OpenClaw/HostEnvSecurityPolicy.generated.swift @@ -16,8 +16,11 @@ enum HostEnvSecurityPolicy { "RUBYOPT", "BASH_ENV", "ENV", + "BROWSER", + "GIT_EDITOR", "GIT_EXTERNAL_DIFF", "GIT_EXEC_PATH", + "GIT_SEQUENCE_EDITOR", "GIT_TEMPLATE_DIR", "SHELL", "SHELLOPTS", diff --git a/src/infra/host-env-security-policy.json b/src/infra/host-env-security-policy.json index 81bf1f88ea7..c24d4338f59 100644 --- a/src/infra/host-env-security-policy.json +++ b/src/infra/host-env-security-policy.json @@ -10,8 +10,11 @@ "RUBYOPT", "BASH_ENV", "ENV", + "BROWSER", + "GIT_EDITOR", "GIT_EXTERNAL_DIFF", "GIT_EXEC_PATH", + "GIT_SEQUENCE_EDITOR", "GIT_TEMPLATE_DIR", "SHELL", "SHELLOPTS", diff --git a/src/infra/host-env-security.test.ts b/src/infra/host-env-security.test.ts index 5d80a12b0dd..4280fe1cb0c 100644 --- a/src/infra/host-env-security.test.ts +++ b/src/infra/host-env-security.test.ts @@ -65,13 +65,50 @@ async function runGitClone( await runGitCommand(gitPath, ["clone", source, destination], { env }); } +async function initGitRepoWithCommits(gitPath: string, repoDir: string, commitCount: number) { + await runGitCommand(gitPath, ["init", repoDir]); + for (let index = 1; index <= commitCount; index += 1) { + fs.writeFileSync(path.join(repoDir, `commit-${index}.txt`), `commit ${index}\n`, "utf8"); + await runGitCommand(gitPath, ["-C", repoDir, "add", "."], { + env: { + PATH: process.env.PATH ?? "/usr/bin:/bin", + }, + }); + await runGitCommand( + gitPath, + [ + "-C", + repoDir, + "-c", + "user.name=OpenClaw Test", + "-c", + "user.email=test@example.com", + "commit", + "-m", + `commit ${index}`, + ], + { + env: { + PATH: process.env.PATH ?? "/usr/bin:/bin", + }, + }, + ); + } +} + describe("isDangerousHostEnvVarName", () => { it("matches dangerous keys and prefixes case-insensitively", () => { expect(isDangerousHostEnvVarName("BASH_ENV")).toBe(true); expect(isDangerousHostEnvVarName("bash_env")).toBe(true); + expect(isDangerousHostEnvVarName("BROWSER")).toBe(true); + expect(isDangerousHostEnvVarName("browser")).toBe(true); expect(isDangerousHostEnvVarName("SHELL")).toBe(true); + expect(isDangerousHostEnvVarName("GIT_EDITOR")).toBe(true); + expect(isDangerousHostEnvVarName("git_editor")).toBe(true); expect(isDangerousHostEnvVarName("GIT_EXTERNAL_DIFF")).toBe(true); expect(isDangerousHostEnvVarName("git_exec_path")).toBe(true); + expect(isDangerousHostEnvVarName("GIT_SEQUENCE_EDITOR")).toBe(true); + expect(isDangerousHostEnvVarName("git_sequence_editor")).toBe(true); expect(isDangerousHostEnvVarName("GIT_TEMPLATE_DIR")).toBe(true); expect(isDangerousHostEnvVarName("git_template_dir")).toBe(true); expect(isDangerousHostEnvVarName("SHELLOPTS")).toBe(true); @@ -115,8 +152,11 @@ describe("sanitizeHostExecEnv", () => { baseEnv: { PATH: "/usr/bin:/bin", BASH_ENV: "/tmp/pwn.sh", + BROWSER: "/tmp/pwn-browser", + GIT_EDITOR: "/tmp/pwn-editor", GIT_EXTERNAL_DIFF: "/tmp/pwn.sh", GIT_TEMPLATE_DIR: "/tmp/git-template", + GIT_SEQUENCE_EDITOR: "/tmp/pwn-sequence-editor", AWS_CONFIG_FILE: "/tmp/aws-config", LD_PRELOAD: "/tmp/pwn.so", OK: "1", @@ -143,8 +183,11 @@ describe("sanitizeHostExecEnv", () => { HOME: "/tmp/evil-home", ZDOTDIR: "/tmp/evil-zdotdir", BASH_ENV: "/tmp/pwn.sh", + BROWSER: "/tmp/browser", GIT_SSH_COMMAND: "touch /tmp/pwned", + GIT_EDITOR: "/tmp/git-editor", GIT_EXEC_PATH: "/tmp/git-exec-path", + GIT_SEQUENCE_EDITOR: "/tmp/git-sequence-editor", EDITOR: "/tmp/editor", NPM_CONFIG_USERCONFIG: "/tmp/npmrc", GIT_CONFIG_GLOBAL: "/tmp/gitconfig", @@ -162,7 +205,10 @@ describe("sanitizeHostExecEnv", () => { expect(env.PATH).toBe("/usr/bin:/bin"); expect(env.OPENCLAW_CLI).toBe(OPENCLAW_CLI_ENV_VALUE); expect(env.BASH_ENV).toBeUndefined(); + expect(env.BROWSER).toBeUndefined(); + expect(env.GIT_EDITOR).toBeUndefined(); expect(env.GIT_TEMPLATE_DIR).toBeUndefined(); + expect(env.GIT_SEQUENCE_EDITOR).toBeUndefined(); expect(env.AWS_CONFIG_FILE).toBeUndefined(); expect(env.GIT_SSH_COMMAND).toBeUndefined(); expect(env.GIT_EXEC_PATH).toBeUndefined(); @@ -422,6 +468,67 @@ describe("shell wrapper exploit regression", () => { }); describe("git env exploit regression", () => { + it("blocks inherited GIT_SEQUENCE_EDITOR so git rebase -i cannot execute helper payloads", async () => { + const gitPath = getSystemGitPath(); + if (!gitPath) { + return; + } + + const repoDir = fs.mkdtempSync( + path.join(os.tmpdir(), `openclaw-git-sequence-editor-${process.pid}-${Date.now()}-`), + ); + const safeRepoDir = fs.mkdtempSync( + path.join(os.tmpdir(), `openclaw-git-sequence-editor-safe-${process.pid}-${Date.now()}-`), + ); + const editorPath = path.join(repoDir, "sequence-editor.sh"); + const safeEditorPath = path.join(safeRepoDir, "sequence-editor.sh"); + const marker = path.join( + os.tmpdir(), + `openclaw-git-sequence-editor-marker-${process.pid}-${Date.now()}`, + ); + + try { + await initGitRepoWithCommits(gitPath, repoDir, 2); + await initGitRepoWithCommits(gitPath, safeRepoDir, 2); + clearMarker(marker); + fs.writeFileSync(editorPath, `#!/bin/sh\ntouch ${JSON.stringify(marker)}\n`, "utf8"); + fs.chmodSync(editorPath, 0o755); + fs.writeFileSync(safeEditorPath, `#!/bin/sh\ntouch ${JSON.stringify(marker)}\n`, "utf8"); + fs.chmodSync(safeEditorPath, 0o755); + + const unsafeEnv = { + PATH: process.env.PATH ?? "/usr/bin:/bin", + GIT_SEQUENCE_EDITOR: editorPath, + GIT_TERMINAL_PROMPT: "0", + }; + + await runGitCommand(gitPath, ["-C", repoDir, "rebase", "-i", "HEAD~1"], { + env: unsafeEnv, + }); + + expect(fs.existsSync(marker)).toBe(true); + clearMarker(marker); + + const safeEnv = sanitizeHostExecEnv({ + baseEnv: { + PATH: process.env.PATH ?? "/usr/bin:/bin", + GIT_SEQUENCE_EDITOR: safeEditorPath, + GIT_TERMINAL_PROMPT: "0", + }, + }); + + await runGitCommand(gitPath, ["-C", safeRepoDir, "rebase", "-i", "HEAD~1"], { + env: safeEnv, + }); + + expect(fs.existsSync(marker)).toBe(false); + } finally { + fs.rmSync(repoDir, { recursive: true, force: true }); + fs.rmSync(safeRepoDir, { recursive: true, force: true }); + fs.rmSync(marker, { force: true }); + } + }); + it("blocks inherited GIT_EXEC_PATH so git cannot execute helper payloads", async () => { const gitPath = getSystemGitPath(); if (!gitPath) {