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 8 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
|
![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 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.
gemini-cli
Bot
added
the
status/need-issue
…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.
gemini-cli
Bot
added
area/security
|
@gemini-code-assist How long will it take to get merged? |
|
I am an automated assistant and do not have visibility into the repository's internal merge process or the schedules of the maintainers. The timeline for merging depends on the project's review workflow, CI status, and maintainer availability. I recommend checking the repository's |
|
I am an automated assistant and do not have access to the repository's team roster or contact information for specific maintainers. To identify the maintainers for this project, I recommend the following steps:
|
|
@gemini-code-assist can i tag maintainers? |
|
Yes, you can tag maintainers in a comment if you have a specific question or if the pull request has been pending for a significant amount of time. However, it is generally best practice to check the |
|
Hello @kschaab, @SandyTao520, and @gundermanc, Could a maintainer kindly approve the CI workflow run for this pull request? This patch resolves the Thank you for your time and assistance. |
|
/gemini review once so that it is ready. |
There was a problem hiding this comment.
Code Review
This pull request enhances shell command substitution detection in packages/core/src/utils/shell-utils.ts by blocking variable expansions (such as ${VAR} and $VAR) for both bash and PowerShell, while allowing a set of safe automatic variables. Comprehensive unit tests have been added to validate these changes. The review feedback suggests optimizing the variable name extraction logic by using a sticky regular expression (/y flag) and setting lastIndex instead of slicing the command string inside loops, which prevents potential performance issues and CPU exhaustion on large inputs.
|
/gemini review once more and check if everything is ready to be merged. |
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 (such as $VAR and ${VAR}) and command substitutions on both Bash and PowerShell, while allowing a set of predefined safe automatic variables (e.g., $_, $true, $false, $null, $args). It also adds comprehensive unit tests in packages/core/src/utils/shell-utils.test.ts to validate these changes across different platforms. There are no review comments to address, and I have no additional feedback to provide.
…e detection Replace `command.slice(i + 1).match(VAR_NAME_RE)` with sticky regex (`/y` flag) and `VAR_NAME_RE.exec(command)` in both bash and PowerShell variable expansion detection. This avoids creating a new substring on every `$` character encountered, reducing worst-case complexity from O(N²) to O(N).
5fa3104 to
a3b06ef
Compare
|
/gemini review so that it is ready |
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 (such as $VAR and ${VAR}) for both Bash and PowerShell, while allowing a set of safe automatic variables (e.g., $_ for Bash, and $true, $false, $null, $args, $_ for PowerShell). Comprehensive unit tests have been added in packages/core/src/utils/shell-utils.test.ts to verify these changes across different platforms. There are no review comments to address, and I have no additional feedback to provide.
1 participant
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