Sanitize subprocess environment in tools_config.py#705
Conversation
This PR updates `hermes_cli/tools_config.py` to use the `_sanitize_subprocess_env` helper for all `subprocess.run` calls. This prevents sensitive Hermes-managed API keys and secrets from leaking into subprocesses spawned during tool installation and setup. Specifically: - Updated `_pip_install` to sanitize the environment while preserving the `VIRTUAL_ENV` variable. - Updated all `subprocess.run` calls in `_run_post_setup` (npm install, npx/chromium install, cua-driver install, etc.) to use sanitized environment. - Updated `.jules/sentinel.md` to record the vulnerability and prevention. Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
🔎 Lint report:
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request secures subprocess executions in hermes_cli/tools_config.py by sanitizing environment variables to prevent credential leakage, documenting this fix in .jules/sentinel.md. The review feedback suggests optimizing the implementation in _pip_install by reusing the already constructed and sanitized uv_env dictionary across multiple subprocess.run calls instead of repeatedly copying and sanitizing the environment.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| probe = subprocess.run( | ||
| pip_cmd + ["--version"], | ||
| capture_output=True, text=True, timeout=15, | ||
| env=_sanitize_subprocess_env(os.environ.copy()), | ||
| ) |
There was a problem hiding this comment.
Instead of repeatedly calling _sanitize_subprocess_env(os.environ.copy()) which performs redundant environment copying and filtering, we can reuse the already constructed uv_env dictionary. This also ensures that the VIRTUAL_ENV environment variable is consistently set for all fallback pip commands, matching the behavior of the uv execution path.
| probe = subprocess.run( | |
| pip_cmd + ["--version"], | |
| capture_output=True, text=True, timeout=15, | |
| env=_sanitize_subprocess_env(os.environ.copy()), | |
| ) | |
| probe = subprocess.run( | |
| pip_cmd + ["--version"], | |
| capture_output=True, text=True, timeout=15, | |
| env=uv_env, | |
| ) |
| subprocess.run( | ||
| [sys.executable, "-m", "ensurepip", "--upgrade", "--default-pip"], | ||
| capture_output=True, text=True, timeout=120, check=True, | ||
| env=_sanitize_subprocess_env(os.environ.copy()), | ||
| ) |
There was a problem hiding this comment.
Reuse the uv_env dictionary here as well to avoid redundant environment sanitization and ensure consistent virtual environment context.
| subprocess.run( | |
| [sys.executable, "-m", "ensurepip", "--upgrade", "--default-pip"], | |
| capture_output=True, text=True, timeout=120, check=True, | |
| env=_sanitize_subprocess_env(os.environ.copy()), | |
| ) | |
| subprocess.run( | |
| [sys.executable, "-m", "ensurepip", "--upgrade", "--default-pip"], | |
| capture_output=True, text=True, timeout=120, check=True, | |
| env=uv_env, | |
| ) |
| return subprocess.run( | ||
| pip_cmd + ["install", *args], | ||
| capture_output=capture_output, text=True, timeout=timeout, | ||
| env=_sanitize_subprocess_env(os.environ.copy()), | ||
| ) |
There was a problem hiding this comment.
Reuse the uv_env dictionary here to avoid redundant environment sanitization and ensure consistent virtual environment context.
| return subprocess.run( | |
| pip_cmd + ["install", *args], | |
| capture_output=capture_output, text=True, timeout=timeout, | |
| env=_sanitize_subprocess_env(os.environ.copy()), | |
| ) | |
| return subprocess.run( | |
| pip_cmd + ["install", *args], | |
| capture_output=capture_output, text=True, timeout=timeout, | |
| env=uv_env, | |
| ) |
There was a problem hiding this comment.
Pull request overview
Updates Hermes CLI tool setup/install subprocess invocations to consistently sanitize the child-process environment, reducing the risk of Hermes-managed secrets leaking into package managers or install scripts.
Changes:
- Added
tools.environments.local._sanitize_subprocess_envusage across allsubprocess.runcalls inhermes_cli/tools_config.py. - Ensured the
uv-based install path preserves required venv context (VIRTUAL_ENV) while still sanitizing the environment. - Documented the remediation in
.jules/sentinel.md.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| hermes_cli/tools_config.py | Passes a sanitized env into every subprocess.run used by post-setup hooks and pip/uv install helpers. |
| .jules/sentinel.md | Adds an entry documenting the environment-sanitization hardening in tools setup. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
What
This PR updates
hermes_cli/tools_config.pyto use the_sanitize_subprocess_envhelper for allsubprocess.runcalls. This prevents sensitive Hermes-managed API keys and secrets from leaking into subprocesses spawned during tool installation and setup (e.g.,npm install,pip install,cua-driverinstallation scripts).Why
Spawning subprocesses without sanitizing the environment poses a security risk, as sensitive credentials from the main process can be exposed to child processes or external scripts.
Verification
subprocess.runcalls inhermes_cli/tools_config.pynow include theenvparameter with_sanitize_subprocess_env.tests/hermes_cli/test_tools_config.py,tests/hermes_cli/test_post_setup_gating.py,tests/hermes_cli/test_managed_installs.py, andtests/hermes_cli/test_tui_npm_install.py. All tests passed.Result
.jules/sentinel.mdto document this fix.Fixes #697
PR created automatically by Jules for task 17047939641955675881 started by @badMade