fix:reorder env var redaction checks to scan values first

[kartikangiras]

Summary

Previously shouldRedactEnvironmentVariable would immediately allow variables in ALWAYS_ALLOWED_ENVIRONMENT_VARIABLES (like PATH, HOME, USER) without checking their values for secrets. This could allow API tokens, private keys, or other sensitive credentials to leak if embedded in the value of an allowed by name variable.

Details

The fix reorders the checks to prioritise value scanning: all variables have their values checked against NEVER_ALLOWED_VALUE_PATTERNS before any name based allowlist is applied. This ensures even if a variable name is allowed, a secret value will still be caught and redacted

Related Issues

fixes #15349

How to Validate

npm run test --workspace=packages/core -- environmentSanitization

All 22 tests pass, including the 4 new security regression tests.

Pre-Merge Checklist

• Updated relevant documentation and README (if needed)
• Added/updated tests (if needed)
• Noted breaking changes (if any)
• Validated on required platforms/methods:
  • MacOS
    • npm run
    • npx
    • Docker
    • Podman
    • Seatbelt
  • Windows
    • npm run
    • npx
    • Docker
  • Linux
    • npm run
    • npx
    • Docker

[gemini-code-assist]

Summary of Changes

Hello, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request addresses a security vulnerability where environment variables, despite having allowed names, could inadvertently leak sensitive information if their values contained secrets. The core change involves reordering the redaction logic to first check variable values against known secret patterns, ensuring that any sensitive data is redacted regardless of the variable's name. This significantly enhances the security posture by preventing accidental exposure of credentials or private keys.

Highlights

• Security Fix: Implemented a critical security fix to prevent sensitive data leakage from environment variables by reordering redaction checks.
• Redaction Logic Reordering: The shouldRedactEnvironmentVariable function now prioritizes scanning variable values for secrets before applying any name-based allowlists.
• Enhanced Protection: Ensures that even if an environment variable name is typically allowed (e.g., HOME, PATH), its value will still be redacted if it contains a secret pattern.
• New Test Coverage: Added four new security regression tests to validate the updated value-first redaction logic.

Changelog

• packages/core/src/services/environmentSanitization.test.ts
  • Added a new 'value-first security' describe block with four new test cases.
  • Included tests to verify redaction of ALWAYS_ALLOWED variables containing GitHub tokens or certificates.
  • Added a test for redacting user-allowlisted variables with secret values.
  • Confirmed that GEMINI_CLI_ variables are never redacted, even if their values resemble secrets.
• packages/core/src/services/environmentSanitization.ts
  • Moved the GEMINI_CLI_ variable check to the very beginning of the shouldRedactEnvironmentVariable function for highest priority.
  • Introduced a new section to perform value-based secret pattern matching before any name-based checks.
  • Adjusted comments to reflect the new order of precedence for redaction checks.
  • Removed the redundant value-based redaction check from the end of the function.

Activity

• The author, kartikangiras, has addressed a security vulnerability related to environment variable redaction.
• Four new security regression tests were added and passed, alongside all existing tests.
• The pull request includes a pre-merge checklist indicating validation on MacOS using npm run.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request addresses a critical security vulnerability by reordering the environment variable redaction checks. The previous logic could have allowed sensitive values, such as API keys, to be exposed if they were part of an environment variable whose name was on an allowlist (e.g., PATH or HOME). The fix correctly prioritizes scanning the variable's value for secret patterns before checking its name against any allowlists. The implementation is sound, and the addition of specific regression tests effectively validates that the vulnerability is resolved. The changes are well-executed and improve the security of the application.