🛡️ Sentinel: [CRITICAL] Fix environment leakage in unit test gate#325
Conversation
- Restrict environment variables in `scripts/03_unit_test_gate.py` to a safe allowlist (PATH, PYTHONPATH, etc.) to prevent sensitive data leakage. - Harden static analysis with additional dangerous patterns (posix, nt, __subclasses__, etc.). - Fix code wrapping in unit test execution to use proper indentation. - Fix `save_jsonl` to handle relative paths in the current directory. - Fix `NameError` in `heidi_engine/telemetry.py` discovered during test verification. - Update Sentinel security journal with findings.
|
👋 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. |
![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 implements security hardening for the unit test gate by filtering environment variables to prevent sensitive data leakage and expanding the list of prohibited Python patterns. It also includes a fix for directory creation in save_jsonl, removes a cache optimization in the telemetry engine, and documents a recent vulnerability. Feedback was provided regarding a local import in scripts/03_unit_test_gate.py that should be moved to the module level to adhere to PEP 8 style conventions.
| test_file = os.path.join(temp_dir, "test_code.py") | ||
|
|
||
| # Wrap code to capture output safely | ||
| import textwrap |
There was a problem hiding this comment.
This import is local to the function. According to PEP 8, imports should usually be at the top of the file. Moving import textwrap to the module level improves code organization and follows standard Python style conventions.
References
- PEP 8 recommends that imports are always put at the top of the file, just after any module comments and docstrings, and before module globals and constants. (link)
1 participant
🚨 Severity: CRITICAL
💡 Vulnerability:
scripts/03_unit_test_gate.pywas executing generated code with the full parent environment, allowing potential exfiltration of sensitive variables likeOPENAI_API_KEY.🎯 Impact: Maliciously generated code could steal credentials or other sensitive information from the training environment.
🔧 Fix: Implemented environment variable filtering using an allowlist for the test subprocess. Hardened regex-based static analysis and fixed code execution template bugs.
✅ Verification: Verified with a reproduction script that
OPENAI_API_KEYis no longer accessible within the executed code blocks. Ran full test suite (21 passed).PR created automatically by Jules for task 11563154516534909711 started by @heidi-dang