🛡️ Sentinel: HIGH Fix Information Exposure in Unit Test Gate - #468
🛡️ Sentinel: HIGH Fix Information Exposure in Unit Test Gate#468heidi-dang wants to merge 1 commit into
Conversation
- Restrict environment in unit test gate to prevent secret leakage. - Update OpenAI key patterns to support sk-proj- keys. - Fix NameError in telemetry state loading. - Fix IndentationError and directory creation bugs. - All tests passed.
|
👋 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. |
There was a problem hiding this comment.
Code Review
This pull request documents recent vulnerabilities in .jules/sentinel.md, removes a broken cache check in telemetry.py, updates OpenAI key regex patterns, ensures os.makedirs is only called on non-empty directory paths, and restricts the execution environment in 03_unit_test_gate.py to prevent secret leakage. Feedback on these changes suggests updating the OpenAI key regexes to include underscores (_) to correctly match modern keys, passing system-specific and locale environment variables to safe_env to ensure cross-platform stability and proper UTF-8 encoding, and simplifying the unit test execution by removing the complex wrapper code in favor of direct execution.
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.
| (r"ghp_[a-zA-Z0-9]{36}", "[GITHUB_TOKEN]"), | ||
| (r"glpat-[a-zA-Z0-9\-]{20,}", "[GITLAB_TOKEN]"), | ||
| (r"sk-[a-zA-Z0-9]{20,}", "[OPENAI_KEY]"), | ||
| (r"sk-[a-zA-Z0-9\-]{20,}", "[OPENAI_KEY]"), |
There was a problem hiding this comment.
The updated regex sk-[a-zA-Z0-9\-]{20,} does not include the underscore character (_). However, modern OpenAI project keys (starting with sk-proj-) frequently contain underscores (e.g., sk-proj-xNyYT9WJ6sNs_...). Because _ is not matched, the regex will either fail to match the key entirely (if the segment before the first underscore is under 20 characters) or only partially redact it, leaking the rest of the key in the logs. Using [\w\-] (which includes _) resolves this issue safely.
| (r"sk-[a-zA-Z0-9\-]{20,}", "[OPENAI_KEY]"), | |
| (r"sk-[\w\-]{20,}", "[OPENAI_KEY]"), |
| (r"glpat-[a-zA-Z0-9\-]{20,}", "gitlab_token"), | ||
| # OpenAI API keys | ||
| (r"sk-[a-zA-Z0-9]{48,}", "openai_key"), | ||
| (r"sk-[a-zA-Z0-9\-]{48,}", "openai_key"), |
There was a problem hiding this comment.
Similar to the telemetry regex, the pattern sk-[a-zA-Z0-9\-]{48,} misses the underscore character (_), which is common in modern sk-proj- OpenAI keys. This can cause the secret scanner to miss keys containing underscores, allowing them to be written to the cleaned dataset. Using [\w\-] ensures all characters (including underscores and hyphens) are matched.
| (r"sk-[a-zA-Z0-9\-]{48,}", "openai_key"), | |
| (r"sk-[\w\-]{48,}", "openai_key"), |
| safe_env = { | ||
| "PATH": os.environ.get("PATH", ""), | ||
| "PYTHONPATH": temp_dir, | ||
| "HOME": os.environ.get("HOME", "/tmp"), | ||
| } |
There was a problem hiding this comment.
The safe_env dictionary is highly restricted, which is excellent for security. However, on Windows platforms, Python subprocesses and certain standard library modules (like subprocess or socket) require system-specific environment variables such as SystemRoot, SystemDrive, or COMSPEC to function correctly. Without them, the test execution might fail with obscure errors. Additionally, passing locale-related variables (LANG, LC_ALL, LC_CTYPE) ensures that Python uses UTF-8 encoding for standard I/O, preventing potential UnicodeEncodeError when user code prints non-ASCII characters. We can safely pass these variables from os.environ if they exist.
safe_env = {
"PATH": os.environ.get("PATH", ""),
"PYTHONPATH": temp_dir,
"HOME": os.environ.get("HOME", "/tmp"),
}
# Pass through system-specific variables required for Python/OS stability and UTF-8 encoding
for var in ["SYSTEMROOT", "SYSTEMDRIVE", "COMSPEC", "LANG", "LC_ALL", "LC_CTYPE"]:
if var in os.environ:
safe_env[var] = os.environ[var]| # Wrap code to capture output safely | ||
| # We use textwrap.indent to ensure the user code is correctly indented within the try block | ||
| indented_code = textwrap.indent(code, " ") | ||
| wrapped_code = f""" | ||
| import sys | ||
| import io |
There was a problem hiding this comment.
The wrapper code that redirects sys.stdout and sys.stderr using StringIO and wraps the execution in a try/except block adds significant complexity (requiring textwrap.indent to avoid indentation errors) and exposes internal variables (like original_stdout and sys) to the untrusted user code. Since subprocess.run is already called with capture_output=True, it automatically captures all stdout and stderr from the subprocess. We can simplify this entire execution by writing the user's code directly to test_file without any wrapper. If the subprocess exits with returncode == 0, the execution succeeded; otherwise, it failed. This would eliminate the need for indentation wrapping, simplify the code, and improve isolation.
🚨 Severity: HIGH
💡 Vulnerability: Information Exposure in
scripts/03_unit_test_gate.pyandheidi_engine/telemetry.py.OPENAI_API_KEY) to untrusted generated code.sk-proj-OpenAI keys.NameErrorintelemetry.pycould crash the status server.🎯 Impact:
🔧 Fix:
PATH,PYTHONPATH,HOME) in the unit test gate subprocess.telemetry.pyto resolve theNameError.os.makedirserrors.✅ Verification:
./.venv3/bin/python3 -m pytest tests/(21 passed).PR created automatically by Jules for task 10952334476949179404 started by @heidi-dang