fix(core): strip execution-affecting GIT_* env vars in getSafeGitEnv - #29008
fix(core): strip execution-affecting GIT_* env vars in getSafeGitEnv#29008chelsealong wants to merge 3 commits into
Conversation
Project .env files loaded into process.env for trusted workspaces can set GIT_EXEC_PATH, GIT_SSH_COMMAND, GIT_PROXY_COMMAND, and similar variables that control which binaries/helpers git executes. getSafeGitEnv() only stripped GIT_CONFIG_* variables, so these leaked through to every internal git invocation (checkpointing, worktrees, extension installs, grep's git ls-files usage), letting a malicious-but-trusted repo run arbitrary commands via ordinary git calls. Fixes google-gemini#29003
…ice too getSafeGitEnv() covers internal git operations (checkpointing, worktrees, extension installs, grep's git fallback), but ordinary shell commands run through ShellExecutionService.prepareExecution(), which builds its own environment via sanitizeEnvironment() and never routes through getSafeGitEnv(). Since default (non-strict) sanitization leaves GIT_* vars untouched, a trusted-but-malicious project .env could still set GIT_EXEC_PATH/GIT_SSH_COMMAND/etc. and get them inherited by any shell command that happens to invoke git (e.g. `git status`), per google-gemini#29003's third named vector. This strips the same execution-affecting GIT_* vars from the shell-execution environment.
|
📊 PR Size: size/M
|
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a security vulnerability where Git-related environment variables could be exploited to achieve arbitrary code execution. By ensuring that specific Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces security enhancements by stripping execution-affecting GIT_* environment variables (such as GIT_EXEC_PATH and GIT_SSH_COMMAND) from the environment before executing shell commands, preventing potential arbitrary command execution from malicious repositories. Corresponding unit tests have been added to verify this behavior. The review feedback highlights two important improvements: first, adding GIT_CONFIG_PARAMETERS to the list of stripped variables to close a potential command injection vector, and second, updating the test file to use an empty string instead of undefined when unsetting environment variables with vi.stubEnv to align with the repository's testing conventions.
| export const EXECUTION_AFFECTING_GIT_ENV_VARS = new Set([ | ||
| 'GIT_EXEC_PATH', | ||
| 'GIT_PROXY_COMMAND', | ||
| 'GIT_SSH_COMMAND', | ||
| 'GIT_SSH_VARIANT', | ||
| 'GIT_ALTERNATE_OBJECT_DIRECTORIES', | ||
| 'GIT_TEMPLATE_DIR', | ||
| 'GIT_REPLACE_REF_BASE', | ||
| 'GIT_CEILING_DIRECTORIES', | ||
| ]); |
There was a problem hiding this comment.
The EXECUTION_AFFECTING_GIT_ENV_VARS set defines the list of git environment variables that are stripped from the environment before executing shell commands to prevent arbitrary command execution. However, it does not include GIT_CONFIG_PARAMETERS.
In shellExecutionService.ts, all environment variables starting with GIT_CONFIG_ (which includes GIT_CONFIG_PARAMETERS) are explicitly collected and preserved in the execution environment. Since GIT_CONFIG_PARAMETERS is not stripped, a malicious repository could supply a .env file containing GIT_CONFIG_PARAMETERS (e.g., setting core.pager or core.sshCommand to a malicious executable), leading to arbitrary code execution when Gemini CLI runs any shell command that invokes git.
To remediate this, add 'GIT_CONFIG_PARAMETERS' to the EXECUTION_AFFECTING_GIT_ENV_VARS set.
| export const EXECUTION_AFFECTING_GIT_ENV_VARS = new Set([ | |
| 'GIT_EXEC_PATH', | |
| 'GIT_PROXY_COMMAND', | |
| 'GIT_SSH_COMMAND', | |
| 'GIT_SSH_VARIANT', | |
| 'GIT_ALTERNATE_OBJECT_DIRECTORIES', | |
| 'GIT_TEMPLATE_DIR', | |
| 'GIT_REPLACE_REF_BASE', | |
| 'GIT_CEILING_DIRECTORIES', | |
| ]); | |
| export const EXECUTION_AFFECTING_GIT_ENV_VARS = new Set([ | |
| 'GIT_EXEC_PATH', | |
| 'GIT_PROXY_COMMAND', | |
| 'GIT_SSH_COMMAND', | |
| 'GIT_SSH_VARIANT', | |
| 'GIT_ALTERNATE_OBJECT_DIRECTORIES', | |
| 'GIT_TEMPLATE_DIR', | |
| 'GIT_REPLACE_REF_BASE', | |
| 'GIT_CEILING_DIRECTORIES', | |
| 'GIT_CONFIG_PARAMETERS', | |
| ]); |
| vi.stubEnv('GITHUB_SHA', undefined); | ||
| vi.stubEnv('SURFACE', undefined); |
There was a problem hiding this comment.
According to the repository's testing conventions, to "unset" an environment variable when using vi.stubEnv, you should use an empty string '' instead of undefined to avoid potential test leakage or unexpected behavior in Vitest.
| vi.stubEnv('GITHUB_SHA', undefined); | |
| vi.stubEnv('SURFACE', undefined); | |
| vi.stubEnv('GITHUB_SHA', ''); | |
| vi.stubEnv('SURFACE', ''); |
References
- When testing code that depends on environment variables, use vi.stubEnv('NAME', 'value') and to 'unset' a variable, use an empty string vi.stubEnv('NAME', '').
GIT_CONFIG_PARAMETERS was preserved by shellExecutionService's GIT_CONFIG_* allow list but not stripped by EXECUTION_AFFECTING_GIT_ENV_VARS, letting a malicious .env set core.sshCommand/core.pager via that variable. Also switch the GITHUB_SHA/SURFACE test stubs to '' per repo convention for vi.stubEnv.
|
Addressed both review comments: added |
Summary
Fixes #29003.
Gemini CLI loads a trusted project's
.envfile intoprocess.env. Theissue names three internal call sites that route git operations through an
environment derived from
process.env, and reports that all three onlystrip
GIT_CONFIG_*/GIT_CONFIG_PARAMETERS, leaving execution-affectingGIT_*variables such asGIT_EXEC_PATH,GIT_SSH_COMMAND, andGIT_PROXY_COMMANDto pass straight through:getSafeGitEnv()inpackages/core/src/utils/gitUtils.tsGitService.getShadowRepoEnv()(already chains intogetSafeGitEnv())ShellExecutionService.prepareExecution()inpackages/core/src/services/shellExecutionService.ts, which builds itsown environment via
sanitizeEnvironment()and does not callgetSafeGitEnv()at allA trusted-but-malicious repository could ship a
.envsetting thesevariables and get arbitrary code execution the moment Gemini CLI ran any
ordinary, non-model git call (checkpointing, worktrees, extension
install/update,
grep'sgit ls-filesfallback) or any ordinary shellcommand that happens to invoke git (e.g.
git status) — no explicit userapproval step involved.
This PR extends sanitization in both places:
getSafeGitEnv()now also strips:GIT_EXEC_PATHGIT_PROXY_COMMANDGIT_SSH_COMMANDGIT_SSH_VARIANTGIT_ALTERNATE_OBJECT_DIRECTORIESGIT_TEMPLATE_DIRGIT_REPLACE_REF_BASEGIT_CEILING_DIRECTORIESGitService.getShadowRepoEnv(), extension install/update, andgrep'sgit fallback all route through this function, so they're covered
transitively.
ShellExecutionService.prepareExecution()now deletes the same set ofvariables (exported from
gitUtils.tsasEXECUTION_AFFECTING_GIT_ENV_VARS) from the environment it builds forevery shell command, closing the gap for the shell-execution path.
Test plan
Added
packages/core/src/utils/gitUtils.test.ts, asserting the vars aboveare stripped from
getSafeGitEnv()'s returned environment while unrelatedvariables (e.g.
PATH) and the existingGIT_CONFIG_*stripping behaviorare preserved.
Added a test to
packages/core/src/services/shellExecutionService.test.ts("should strip execution-affecting GIT_* variables from the spawned
environment") asserting the same variables are absent from the environment
passed to
child_process.spawn, while unrelated variables (e.g.PATH)are preserved. This test explicitly unsets
GITHUB_SHA/SURFACEso itexercises the default (non-strict) sanitization path — the actual
vulnerable path for ordinary local usage — rather than being masked by the
stricter CI-only sanitization mode.
Confirmed both new tests fail without their respective source fix:
And both pass with their fixes restored (150/150 across the directly
related suites):
Also ran the full core package test suite:
The 19 failures are pre-existing and unrelated to this change: they
require the
bwrap(bubblewrap) sandbox binary, which is not installed inthis environment.
Also ran
npm run lint(repo-wide,--max-warnings 0) andtsc --noEmitforpackages/core— both clean.AI assistance disclosure
This change was implemented with AI assistance (an autonomous Claude-based
coding agent), with the diff, tests, and test output reviewed before
submission.