fix(core): block $VAR and ${VAR} variable expansion bypass (GHSA-wpqr-6v78-jr5g)#28403
fix(core): block $VAR and ${VAR} variable expansion bypass (GHSA-wpqr-6v78-jr5g)#28403thalha-a9 wants to merge 7 commits into
Conversation
…titution
The existing detectBashSubstitution() only blocked $() command
substitution and backticks, but allowed $VAR and ${VAR} variable
expansion to pass through undetected. This is a bypass of the
fix for GHSA-wpqr-6v78-jr5g.
An attacker-controlled prompt could instruct the model to run
commands like: echo $GITHUB_TOKEN, echo ${GEMINI_API_KEY}, or
write env vars to GITHUB_ENV — exfiltrating secrets from CI
environments without triggering any security gate.
Changes:
- shell-utils.ts: Extend the $ character check in
detectBashSubstitution() to also block ${...} (brace
expansion) and $IDENTIFIER (bare variable names starting
with [a-zA-Z_]). Harmless special variables ($?, $$,
$0-$9) remain unblocked.
- shell-utils.test.ts: Add 19 test cases covering blocked
variable expansion, existing command substitution, safe
commands, and edge cases (escaped \$, trailing $,
single-quoted $VAR).
- gemini-automated-issue-dedup.yml: Defense-in-depth
hardening — add author_association guard on issues:opened
trigger and clear GITHUB_TOKEN in the Gemini CLI
environment to prevent token access from within the
sandboxed execution.
Summary of ChangesHello, I'm Gemini Code Assist1! 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 (GHSA-wpqr-6v78-jr5g) where the existing shell command validation was insufficient. By only blocking command substitution like Highlights
Ignored Files
Using Gemini Code AssistThe 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
Customization To customize the 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 Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. 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. Footnotes
|
|
📊 PR Size: size/L
|
There was a problem hiding this comment.
Code Review
This pull request updates detectBashSubstitution in shell-utils.ts to block variable expansion ($VAR and ${VAR}) and command substitution, and adds corresponding unit tests. The review feedback correctly identifies a critical security gap where detectPowerShellSubstitution (used on Windows) was not updated, leaving Windows environments vulnerable to variable expansion attacks. Additionally, the reviewer suggests expanding the test suite to run these variable expansion checks against both linux and win32 platforms to ensure comprehensive coverage.
…form test coverage
detectPowerShellSubstitution() was missed in the original fix, leaving
Windows environments exploitable via $env:VAR and ${env:VAR} syntax.
Also restructures variable-expansion tests to run against both bash
and PowerShell platforms, since the prior tests only exercised the
bash path due to a global platform stub — which is why this gap
wasn't caught initially.
- shell-utils.ts: extend detectPowerShellSubstitution() to block
${...} and $IDENTIFIER, mirroring the bash fix
- shell-utils.test.ts: dual-platform assertions for shared cases,
new PowerShell-specific $env:/${env:} test cases, vi.stubEnv/
unstubAllEnvs for ComSpec isolation (matches existing file
convention)
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request enhances shell utility functions to detect and block command substitutions and variable expansions in both Bash and PowerShell environments, accompanied by comprehensive unit tests. The review feedback correctly identifies that overly restrictive blocking of variables starting with letters or underscores will cause false positives by blocking harmless automatic variables and constants (such as $_ in Bash, and $true, $false, $null, and $_ in PowerShell). It is recommended to apply the provided code suggestions to explicitly allow these safe, built-in variables.
…detection Addresses reviewer feedback that blocking all $IDENTIFIER patterns over-blocks harmless automatic variables. After capturing the full variable name, check against an explicit allowlist before blocking: - Bash: $_ (last argument) allowed; $_SECRET, $_1 etc. still blocked - PowerShell: $true, $false, $null, $args, $_ allowed; $trueVar, $_FOO, $TOKEN etc. still blocked Adds 13 test cases confirming exact-match semantics — partial matches like $nullify or $_SECRET are correctly blocked.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces the detectCommandSubstitution utility to detect and block unsafe command substitutions and variable expansions in bash and PowerShell, while permitting safe automatic variables. The review feedback correctly points out that PowerShell automatic variables are case-insensitive, meaning the current case-sensitive check will incorrectly block mixed-case or uppercase variables like $True. It is recommended to normalize the variable name to lowercase before checking and to add corresponding test cases to verify this behavior.
…riables PowerShell automatic variables ($True, $FALSE, $Null, $ARGS) are case-insensitive. Normalize varName to lowercase before checking against the allowlist so mixed-case variants are not incorrectly blocked. Adds 5 additional test cases for mixed/uppercase variants.
…nstants Move BASH_SAFE_VARS, PS_SAFE_VARS, and VAR_NAME_RE out of the hot path so they are not re-created on every $ character encountered. Adds one test for $_ followed by a space (word-boundary correctness).
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request enhances the command substitution detection logic in shell-utils.ts by blocking variable expansions (such as ${VAR} and $VAR) for both Bash and PowerShell, while allowing a predefined set of safe automatic variables. It also adds comprehensive unit tests to verify these rules. The feedback highlights a critical vulnerability bypass in the PowerShell detection where scope-qualified variables (e.g., $true:SECRET) could bypass the block because the prefix matches a safe variable. Suggestions are provided to fix this bypass in the implementation and to add a corresponding test case.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request enhances the detectCommandSubstitution utility in packages/core/src/utils/shell-utils.ts to block variable expansions and subexpressions for both Bash and PowerShell environments, while permitting a defined set of safe automatic variables (such as $_, $true, $false, $null, and $args). Comprehensive unit tests have been added in packages/core/src/utils/shell-utils.test.ts to verify these detection rules across different platforms. I have no feedback to provide.
Summary
detectBashSubstitution()anddetectPowerShellSubstitution()that allowed variable expansion patterns to bypass the security gate added for GHSA-wpqr-6v78-jr5ggemini-automated-issue-dedup.ymlworkflowFixes #28418
Changes
packages/core/src/utils/shell-utils.ts${VAR}and$VARin both bash and PowerShell detection, with allowlist for safe automatic variablespackages/core/src/utils/shell-utils.test.ts.github/workflows/gemini-automated-issue-dedup.ymlauthor_associationguard + clearGITHUB_TOKENin sandboxed envTest plan