Conversation
|
Hi @jacob314, thank you so much for your contribution to Gemini CLI! We really appreciate the time and effort you've put into this. We're making some updates to our contribution process to improve how we track and review changes. Please take a moment to review our recent discussion post: Improving Our Contribution Process & Introducing New Guidelines. Key Update: Starting January 26, 2026, the Gemini CLI project will require all pull requests to be associated with an existing issue. Any pull requests not linked to an issue by that date will be automatically closed. Thank you for your understanding and for being a part of our community! |
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 introduces a fundamental enhancement to shell command execution by implementing persistent shell sessions. This change allows the shell environment, including variables, current working directory, and aliases, to be maintained between commands, providing a more natural and efficient interactive experience. It addresses limitations where each command previously ran in an isolated shell, losing context. The new functionality is configurable and enabled by default, ensuring that users benefit from a more consistent and powerful shell interaction. Highlights
Changelog
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 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 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
|
|
Size Change: +18.9 kB (+0.07%) Total Size: 26 MB
ℹ️ View Unchanged
|
![gemini-code-assist[bot]](https://avatars.gh.lumenfield.work/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces support for persistent shell sessions, a significant feature for maintaining state across shell commands. During the review, a critical command injection vulnerability was identified in the PersistentShellSession class where the current working directory (cwd) is unsafely interpolated into shell command strings, potentially allowing an attacker to execute arbitrary commands. This violates established guidelines for sanitizing user-provided file paths in file system operations. Additionally, a critical bug was found in the interactive shell command processor causing incorrect exit code reporting, and the persistent session's lack of resilience to exit commands can lead to unexpected state loss. Addressing these issues is crucial to ensure the feature is robust, reliable, and secure.
| if (isPersistent) { | ||
| commandToExecute = `{ ${command} }; __code=$?; pwd > "${pwdFilePath}"; (exit $__code)`; | ||
| } else { | ||
| commandToExecute = `{ ${command} }; __code=$?; pwd > "${pwdFilePath}"; exit $__code`; | ||
| } |
There was a problem hiding this comment.
The command wrapping logic for persistent shells creates an incompatibility with the underlying PersistentShellSession. The commandToExecute is constructed with (exit $__code) to prevent the PTY from closing, but this command is then passed to PersistentShellSession, which has its own mechanism for capturing exit codes using markers and $?. The (exit) command runs in a subshell and does not set $? in the parent shell where PersistentShellSession is trying to read it. As a result, PersistentShellSession will likely always receive an exit code of 0, incorrectly reporting all commands as successful. This dual-wrapping approach breaks exit code reporting.
| wrappedCmd = `Set-Location "${cwd}"; Write-Output ("${start1}" + "${start2}"); try { ${command} } finally { Write-Output ("${prefix1}" + "${prefix2}$LASTEXITCODE${this.endMarkerSuffix}") }\r\n`; | ||
| } else if (this.shellType === 'cmd') { | ||
| wrappedCmd = `call echo ${start1}^${start2} & pushd "${cwd}" && ${command} & set __code=%errorlevel% & popd & call echo ${prefix1}^${prefix2}%__code%${this.endMarkerSuffix}\r\n`; | ||
| } else { | ||
| // bash/zsh | ||
| wrappedCmd = `echo "${start1}""${start2}"; cd "${cwd}" && { ${command.trim().replace(/;$/, '')}; }; echo "${prefix1}""${prefix2}$?${this.endMarkerSuffix}"\n`; |
There was a problem hiding this comment.
A critical command injection vulnerability exists here: the cwd (current working directory) parameter is unsafely interpolated into a shell command string using double quotes. This allows command substitution, enabling an attacker to achieve arbitrary command execution if they can influence the directory path. To remediate, use the escapeShellArg utility to properly escape the cwd string.
Furthermore, the current implementation for executing commands in bash/zsh uses a command group { ... }, which makes the session vulnerable to exit commands. If exit is issued, the entire persistent shell process terminates, leading to state loss and defeating the purpose of a persistent shell. The session should be made resilient to exit commands to ensure reliable state persistence.
References
- Sanitize user-provided file paths used in file system operations to prevent path traversal vulnerabilities.
- Utility functions that perform file system operations should validate their path inputs internally to prevent path traversal vulnerabilities, rather than relying solely on callers to perform validation.
gemini-cli
bot
added
the
🔒 maintainer only
![github-advanced-security[bot]](https://avatars.gh.lumenfield.work/in/57789?s=60&v=4){kind=small}
| const startMarker1 = commandCall1 | ||
| .match(/"___GEMINI""(_START_MARKER_[a-z0-9]+___)"/)[0] | ||
| .replace(/"/g, '') | ||
| .replace(/___GEMINI/, '___GEMINI'); |
Check warning
Code scanning / CodeQL
Replacement of a substring with itself Medium
| const startMarker2 = commandCall2 | ||
| .match(/"___GEMINI""(_START_MARKER_[a-z0-9]+___)"/)[0] | ||
| .replace(/"/g, '') | ||
| .replace(/___GEMINI/, '___GEMINI'); |
Check warning
Code scanning / CodeQL
Replacement of a substring with itself Medium
1 participant
Summary
Support aliases
Massively speed up running shell commands
Support setting env variables within shell commands with the expected behavior
Details
Related Issues
Fixes #21461
How to Validate
Define an alias in your profile
Validate you can now use it inside gemini cli
set env variables in one shell command and use them in the next
Possible bugs:
make sure things are still ok if you send shells to the background, execute and exit shell commands such as vim that use the alternate buffer.
Make sure shells are sized correctly and don't end up with stale size information due to the reuse.