Skip to content

Sanitize subprocess environments in tools_config.py#702

Draft
badMade wants to merge 1 commit into
mainfrom
security-sanitize-subprocess-env-15309540465303758137
Draft

Sanitize subprocess environments in tools_config.py#702
badMade wants to merge 1 commit into
mainfrom
security-sanitize-subprocess-env-15309540465303758137

Conversation

@badMade
Copy link
Copy Markdown
Owner

@badMade badMade commented Jun 3, 2026

This PR implements environment sanitization for all subprocess.run calls in hermes_cli/tools_config.py. It uses _sanitize_subprocess_env from tools.environments.local to prevent sensitive API keys and other managed secrets from leaking into child processes (installers, version checks, etc.).

Key changes:

  • Added top-level import of _sanitize_subprocess_env.
  • Updated _pip_install to sanitize the environment for uv and pip calls.
  • Updated _run_post_setup to sanitize environments for npm, Chromium, and cua-driver related calls.
  • Modernized the cua-driver installation logic by converting it to a list-based command, removing shell=True, and adding suggested parameters (capture_output, text, cwd, timeout).

Verification:

  • Ran PYTHONPATH=. uv run --with pytest-asyncio --with pytest-xdist pytest tests/hermes_cli/test_tools_config.py - all 65 tests passed.
  • Manually verified that all subprocess.run call sites in hermes_cli/tools_config.py now include the env= argument with sanitized environment.

Fixes #694


PR created automatically by Jules for task 15309540465303758137 started by @badMade

Implemented environment sanitization for all subprocess.run calls in hermes_cli/tools_config.py using _sanitize_subprocess_env. This prevents sensitive API keys and managed secrets from leaking into child processes. Also modernized the cua-driver installation logic.

Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
@google-labs-jules
Copy link
Copy Markdown

👋 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 @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.


For security, I will only act on instructions from the user who triggered this task.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jun 3, 2026

🔎 Lint report: security-sanitize-subprocess-env-15309540465303758137 vs origin/main

ruff

Total: 0 on HEAD, 0 on base (➖ 0)

🆕 New issues: none

✅ Fixed issues: none

Unchanged: 0 pre-existing issues carried over.

ty (type checker)

Total: 8253 on HEAD, 8253 on base (➖ 0)

🆕 New issues (3):

Rule Count
invalid-argument-type 3
First entries
run_agent.py:13576: [invalid-argument-type] invalid-argument-type: Argument to function `len` is incorrect: Expected `Sized`, found `(str & ~AlwaysFalsy) | (dict[Unknown, Unknown] & ~AlwaysFalsy) | (Any & ~AlwaysFalsy) | ... omitted 3 union elements`
run_agent.py:13573: [invalid-argument-type] invalid-argument-type: Argument to function `_is_oauth_token` is incorrect: Expected `str`, found `str | dict[Unknown, Unknown] | Any | ... omitted 3 union elements`
run_agent.py:7317: [invalid-argument-type] invalid-argument-type: Argument to function `build_anthropic_client` is incorrect: Expected `str`, found `str | dict[Unknown, Unknown] | Any | ... omitted 3 union elements`

✅ Fixed issues (3):

Rule Count
invalid-argument-type 3
First entries
run_agent.py:13573: [invalid-argument-type] invalid-argument-type: Argument to function `_is_oauth_token` is incorrect: Expected `str`, found `str | dict[Unknown | str, Unknown | str | dict[str, str]] | Any | ... omitted 3 union elements`
run_agent.py:13576: [invalid-argument-type] invalid-argument-type: Argument to function `len` is incorrect: Expected `Sized`, found `(str & ~AlwaysFalsy) | (dict[Unknown | str, Unknown | str | dict[str, str]] & ~AlwaysFalsy) | (Any & ~AlwaysFalsy) | ... omitted 3 union elements`
run_agent.py:7317: [invalid-argument-type] invalid-argument-type: Argument to function `build_anthropic_client` is incorrect: Expected `str`, found `str | dict[Unknown | str, Unknown | str | dict[str, str]] | Any | ... omitted 3 union elements`

Unchanged: 4354 pre-existing issues carried over.

Diagnostics are surfaced as warnings — this check never fails the build.

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates various subprocess calls in hermes_cli/tools_config.py to sanitize environment variables using _sanitize_subprocess_env. Additionally, it refactors the cua-driver installation command to avoid using shell=True. Feedback suggests adding set -o pipefail to the cua-driver installation pipeline to prevent masking potential failures in the curl command.

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.

Comment on lines +804 to +808
install_cmd = [
"/bin/bash",
"-c",
"curl -fsSL https://raw.githubusercontent.com/trycua/cua/main/libs/cua-driver/scripts/install.sh | /bin/bash"
]
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

When executing a shell pipeline (like curl ... | /bin/bash) via subprocess.run, the exit status of the pipeline is determined by the last command in the pipeline (here, /bin/bash). If curl fails due to network or DNS issues, /bin/bash will receive an empty stream and exit with 0, masking the failure and causing result.returncode to be 0. Using set -o pipefail ensures that the pipeline fails if any command in the pipeline fails, allowing proper error detection.

Suggested change
install_cmd = [
"/bin/bash",
"-c",
"curl -fsSL https://raw.githubusercontent.com/trycua/cua/main/libs/cua-driver/scripts/install.sh | /bin/bash"
]
install_cmd = [
"/bin/bash",
"-c",
"set -o pipefail; curl -fsSL https://raw.githubusercontent.com/trycua/cua/main/libs/cua-driver/scripts/install.sh | /bin/bash"
]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant