Sanitize subprocess environments in tools_config.py#703
Conversation
This commit introduces environment sanitization for all subprocess calls within `hermes_cli/tools_config.py` to prevent sensitive environment variables (like API keys and tokens) from leaking into external tools (npm, pip, Chromium/cua-driver installers). Key changes: - Imported `_sanitize_subprocess_env` from `tools.environments.local`. - Updated `_pip_install` to sanitize `uv_env` and standard `pip` calls. - Applied sanitization to `npm install`, Chromium install, Camofox install, and `cua-driver` setup calls. - Ensured `os.environ.copy()` is used to prevent side effects. 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 integrates environment sanitization for subprocesses in hermes_cli/tools_config.py using _sanitize_subprocess_env across several installation and setup commands. The review feedback recommends a performance improvement in _pip_install to sanitize the environment once at the start of the function and reuse it, rather than repeatedly copying and sanitizing os.environ for each subprocess invocation.
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.
| """ | ||
| venv_root = Path(sys.executable).parent.parent | ||
| uv_env = {**os.environ, "VIRTUAL_ENV": str(venv_root)} | ||
| uv_env = _sanitize_subprocess_env(os.environ.copy(), {"VIRTUAL_ENV": str(venv_root)}) |
There was a problem hiding this comment.
Instead of calling _sanitize_subprocess_env(os.environ.copy()) multiple times throughout _pip_install, we can sanitize the environment once at the beginning of the function and reuse it. This avoids redundant environment copying and blocklist filtering overhead.
| uv_env = _sanitize_subprocess_env(os.environ.copy(), {"VIRTUAL_ENV": str(venv_root)}) | |
| base_env = _sanitize_subprocess_env(os.environ.copy()) | |
| uv_env = {**base_env, "VIRTUAL_ENV": str(venv_root)} |
| 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.
Reuse the base_env dictionary defined at the start of the function to avoid redundant sanitization overhead.
| env=_sanitize_subprocess_env(os.environ.copy()), | |
| env=base_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 base_env dictionary defined at the start of the function to avoid redundant sanitization overhead.
| env=_sanitize_subprocess_env(os.environ.copy()), | |
| env=base_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 base_env dictionary defined at the start of the function to avoid redundant sanitization overhead.
| env=_sanitize_subprocess_env(os.environ.copy()), | |
| env=base_env, |
This commit introduces environment sanitization for all subprocess calls within `hermes_cli/tools_config.py` to prevent sensitive environment variables (like API keys and tokens) from leaking into external tools. To avoid circular dependency or initialization order issues that could affect environment context during tests, `_sanitize_subprocess_env` is imported lazily within the functions. Key changes: - Updated `_pip_install` to sanitize `uv_env` and standard `pip` calls. - Applied sanitization to `npm install`, Chromium install, Camofox install, and `cua-driver` setup calls in `_run_post_setup`. - Ensured `os.environ.copy()` is used to prevent side effects. Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
1 participant
What
Sanitized all
subprocess.runcalls inhermes_cli/tools_config.pyusing the_sanitize_subprocess_envhelper to prevent credential leakage.Why
External tools and installers called by the configuration wizard (e.g.,
npm,pip,curl | bash) should not have access to the agent's internal environment variables, especially secrets likeOPENAI_API_KEYorGH_TOKEN.Verification
grep -n "subprocess.run" hermes_cli/tools_config.pyto identify all call sites.envparameter with the sanitizer.tests/hermes_cli/test_tools_config.pyand other relevant tests; all passed.Fixes #696
PR created automatically by Jules for task 6334960680090751462 started by @badMade