Skip to content

🛡️ Sentinel: [security enhancement] Harden unit test gate and fix telemetry bug#323

Open
heidi-dang wants to merge 1 commit into
feat/bootstrap-scaffoldfrom
sentinel/harden-test-gate-17690827714301133543
Open

🛡️ Sentinel: [security enhancement] Harden unit test gate and fix telemetry bug#323
heidi-dang wants to merge 1 commit into
feat/bootstrap-scaffoldfrom
sentinel/harden-test-gate-17690827714301133543

Conversation

@heidi-dang
Copy link
Copy Markdown
Owner

🚨 Severity: MEDIUM
💡 Vulnerability: Environment variable leakage and missing sandbox escape checks in the unit test gate script.
🎯 Impact: Sensitive keys (like OPENAI_API_KEY) could be exposed to generated code being tested. Basic sandbox escapes using __subclasses__ were also possible.
🔧 Fix:

  • Implemented a strict environment variable allowlist (PATH, PYTHONPATH, LANG, PYTHONIOENCODING) for the test subprocess in scripts/03_unit_test_gate.py.
  • Added __subclasses__, __globals__, and __builtins__ to the DANGEROUS_PATTERNS list.
  • Used textwrap.indent to ensure injected code is correctly formatted in the test wrapper.
  • Fixed a NameError in heidi_engine/telemetry.py caused by a redundant and broken cache check using an undefined variable.
    ✅ Verification: Created and ran a custom verification script verify_hardening.py to confirm that sensitive environment variables are not leaked and dangerous patterns are blocked. Ran the full test suite with pytest and confirmed all 21 tests passed.

PR created automatically by Jules for task 17690827714301133543 started by @heidi-dang

- Harden `scripts/03_unit_test_gate.py` by filtering environment variables and blocking sandbox escape patterns.
- Fix indentation of injected code in `scripts/03_unit_test_gate.py`.
- Fix `NameError` in `heidi_engine/telemetry.py` by removing a redundant and broken cache check.
@google-labs-jules
Copy link
Copy Markdown
Contributor

👋 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.

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 focuses on security hardening for the unit test gatekeeper by implementing environment isolation and blocking Python introspection attributes to prevent sandbox escapes. It also removes a state cache optimization in the telemetry module and updates the security sentinel log. Feedback suggests refining the environment variable allowlist by removing redundant keys and ensuring cross-platform compatibility through case-insensitive key matching.

try:
# BOLT SECURITY: Filter environment to prevent leakage of sensitive keys (e.g. OPENAI_API_KEY)
# to the generated code being tested.
allowed_env_keys = {"PATH", "PYTHONPATH", "LANG", "PYTHONIOENCODING"}
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

The PYTHONPATH key in allowed_env_keys is redundant because it is explicitly overwritten on line 269. Removing it from the allowlist improves clarity and ensures that the host's PYTHONPATH is intentionally ignored in favor of the isolated temp_dir. Additionally, consider moving allowed_env_keys to a module-level constant to avoid recreating the set on every function call.

Suggested change
allowed_env_keys = {"PATH", "PYTHONPATH", "LANG", "PYTHONIOENCODING"}
allowed_env_keys = {"PATH", "LANG", "PYTHONIOENCODING"}

# BOLT SECURITY: Filter environment to prevent leakage of sensitive keys (e.g. OPENAI_API_KEY)
# to the generated code being tested.
allowed_env_keys = {"PATH", "PYTHONPATH", "LANG", "PYTHONIOENCODING"}
safe_env = {k: v for k, v in os.environ.items() if k in allowed_env_keys}
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

Environment variable keys can be case-insensitive on some platforms (like Windows). The current set lookup is case-sensitive, which might cause essential variables like Path (instead of PATH) to be filtered out, potentially breaking the subprocess execution. Using k.upper() for the check ensures cross-platform robustness.

Suggested change
safe_env = {k: v for k, v in os.environ.items() if k in allowed_env_keys}
safe_env = {k: v for k, v in os.environ.items() if k.upper() in allowed_env_keys}

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant