fix(infra): block BROWSER, GIT_EDITOR, GIT_SEQUENCE_EDITOR from inherited host env (#57559)

This commit is contained in:
pgondhi987 2026-03-30 17:01:04 +05:30 committed by GitHub
parent c4fa8635d0
commit bc3b05dce4
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 113 additions and 0 deletions

View File

@ -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",

View File

@ -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",

View File

@ -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) {