Skip to content

🛡️ Sentinel: HIGH Fix Information Exposure in Unit Test Gate - #468

Open
heidi-dang wants to merge 1 commit into
feat/bootstrap-scaffoldfrom
sentinel/fix-info-exposure-10952334476949179404
Open

🛡️ Sentinel: HIGH Fix Information Exposure in Unit Test Gate#468
heidi-dang wants to merge 1 commit into
feat/bootstrap-scaffoldfrom
sentinel/fix-info-exposure-10952334476949179404

Conversation

@heidi-dang

Copy link
Copy Markdown
Owner

🚨 Severity: HIGH
💡 Vulnerability: Information Exposure in scripts/03_unit_test_gate.py and heidi_engine/telemetry.py.

  • The unit test gate leaked the entire host environment (including OPENAI_API_KEY) to untrusted generated code.
  • Telemetry redaction was missing support for newer sk-proj- OpenAI keys.
  • A NameError in telemetry.py could crash the status server.

🎯 Impact:

  • Untrusted code generated by LLMs could steal host API keys if the unit test gate is enabled.
  • Sensitive project-specific keys could be leaked in logs or the telemetry API.
  • The telemetry dashboard could fail to load due to server crashes.

🔧 Fix:

  • Whitelisted environment variables (PATH, PYTHONPATH, HOME) in the unit test gate subprocess.
  • Updated secret redaction regex to support hyphens in OpenAI keys.
  • Removed broken cache logic in telemetry.py to resolve the NameError.
  • Fixed IndentationError in unit test wrapper and os.makedirs errors.

✅ Verification:

  • Verified environment isolation with reproduction scripts.
  • Verified key redaction with testing script.
  • Ran full test suite: ./.venv3/bin/python3 -m pytest tests/ (21 passed).

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

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

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

Copy link
Copy Markdown

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

Comment thread heidi_engine/telemetry.py
(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]"),

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

security-high high

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.

Suggested change
(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"),

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

security-high high

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.

Suggested change
(r"sk-[a-zA-Z0-9\-]{48,}", "openai_key"),
(r"sk-[\w\-]{48,}", "openai_key"),

Comment on lines +265 to +269
safe_env = {
"PATH": os.environ.get("PATH", ""),
"PYTHONPATH": temp_dir,
"HOME": os.environ.get("HOME", "/tmp"),
}

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 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]

Comment on lines 217 to 222
# 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

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

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