fix(cli): use newline in shell command wrapping to avoid breaking heredocs

[cocosheng-g]

Summary

This pull request enhances the security, reliability, and cross-platform robustness of shell command execution.

Key Changes

• Secure Temporary Directories: Replaced predictable temporary filenames with unique directories created via fs.mkdtempSync(). This mitigates symlink attacks, resource leaks, and prevents race conditions.
• Command Injection Prevention: Introduced escapeShellArg to safely escape file paths before interpolating them into shell wrappers.
• Robust Command Wrapping: Commands are now wrapped using literal newlines (e.g., {\n${command}\n}) instead of trailing semicolons. This properly supports heredocs, trailing comments, and commands ending with a backslash.
• Cross-Platform Fixes: Ensured POSIX utility outputs (like pgrep) are parsed robustly by explicitly splitting on \n and trimming each line to handle potential \r (CRLF) carriage returns.
• Test Suite Resiliency: Completely overhauled the shell tests by removing brittle global node:fs mocks that previously broke WASM parsing and sandbox heuristic checks on CI. Tests now dynamically capture and assert against the true generated temporary paths using resilient string matching.

fix #25536

[cocosheng-g]

/gemini review

[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 resolves a critical issue in the CLI's shell command wrapping mechanism that previously caused heredoc commands to fail. By adjusting the command separator from a semicolon to a newline, the system now correctly processes heredocs, significantly enhancing the reliability of shell command execution within the application.

Highlights

• Bug Fix: Fixed a bug where shell commands wrapped by the CLI would fail if they ended with a heredoc delimiter, due to an incorrectly appended semicolon.
• Command Wrapping Logic Update: Modified the shell command wrapping logic to use a newline character ('\n') instead of a semicolon (';') before the closing brace, ensuring proper heredoc termination and valid command separation for bash.
• New Test Cases: Added new unit tests to validate the correct handling of heredoc commands in both the CLI's useExecutionLifecycle hook and the core ShellTool.

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.

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

[github-actions]

Size Change: +629 B (0%)

Total Size: 33.7 MB

Filename	Size	Change
./bundle/chunk-4KZPUKLD.js	0 B	-674 kB (removed)	🏆
./bundle/chunk-6XKGCZRJ.js	0 B	-2.72 MB (removed)	🏆
./bundle/chunk-CSFLEDXW.js	0 B	-49.2 kB (removed)	🏆
./bundle/chunk-HJIAMJCZ.js	0 B	-3.8 kB (removed)	🏆
./bundle/chunk-KPMH5OHL.js	0 B	-14.5 MB (removed)	🏆
./bundle/chunk-OMLPW664.js	0 B	-3.43 kB (removed)	🏆
./bundle/core-KCJ6UZXS.js	0 B	-46.7 kB (removed)	🏆
./bundle/devtoolsService-UNSTBT2H.js	0 B	-28.4 kB (removed)	🏆
./bundle/gemini-JKLW4OY5.js	0 B	-576 kB (removed)	🏆
./bundle/interactiveCli-DXPKS52Q.js	0 B	-1.3 MB (removed)	🏆
./bundle/liteRtServerManager-IULQPK32.js	0 B	-2.08 kB (removed)	🏆
./bundle/oauth2-provider-L2LXI7YB.js	0 B	-9.16 kB (removed)	🏆
./bundle/chunk-DH7XIIIE.js	3.8 kB	+3.8 kB (new file)	🆕
./bundle/chunk-DLRSKX4P.js	14.5 MB	+14.5 MB (new file)	🆕
./bundle/chunk-E5IB4HBC.js	49.2 kB	+49.2 kB (new file)	🆕
./bundle/chunk-KWNML3AY.js	2.72 MB	+2.72 MB (new file)	🆕
./bundle/chunk-PXVJLG72.js	3.43 kB	+3.43 kB (new file)	🆕
./bundle/chunk-VE7A3IXJ.js	674 kB	+674 kB (new file)	🆕
./bundle/core-37ITZ5YW.js	46.7 kB	+46.7 kB (new file)	🆕
./bundle/devtoolsService-MDROW7DQ.js	28.4 kB	+28.4 kB (new file)	🆕
./bundle/gemini-6X6ENSC5.js	576 kB	+576 kB (new file)	🆕
./bundle/interactiveCli-YBY7BE7S.js	1.3 MB	+1.3 MB (new file)	🆕
./bundle/liteRtServerManager-U5U3UR6J.js	2.08 kB	+2.08 kB (new file)	🆕
./bundle/oauth2-provider-VGJ335JC.js	9.16 kB	+9.16 kB (new file)	🆕

ℹ️ View Unchanged

Filename	Size	Change
./bundle/bundled/third_party/index.js	8 MB	0 B
./bundle/chunk-34MYV7JD.js	2.45 kB	0 B
./bundle/chunk-5AUYMPVF.js	858 B	0 B
./bundle/chunk-5PS3AYFU.js	1.18 kB	0 B
./bundle/chunk-664ZODQF.js	124 kB	0 B
./bundle/chunk-DAHVX5MI.js	206 kB	0 B
./bundle/chunk-IUUIT4SU.js	56.5 kB	0 B
./bundle/chunk-RJTRUG2J.js	39.8 kB	0 B
./bundle/chunk-YIPDXML5.js	1.97 MB	0 B
./bundle/cleanup-VY5PDR3D.js	0 B	-932 B (removed)	🏆
./bundle/devtools-36NN55EP.js	696 kB	0 B
./bundle/dist-T73EYRDX.js	356 B	0 B
./bundle/events-XB7DADIJ.js	418 B	0 B
./bundle/examples/hooks/scripts/on-start.js	188 B	0 B
./bundle/examples/mcp-server/example.js	1.43 kB	0 B
./bundle/gemini.js	4.97 kB	0 B
./bundle/getMachineId-bsd-TXG52NKR.js	1.55 kB	0 B
./bundle/getMachineId-darwin-7OE4DDZ6.js	1.55 kB	0 B
./bundle/getMachineId-linux-SHIFKOOX.js	1.34 kB	0 B
./bundle/getMachineId-unsupported-5U5DOEYY.js	1.06 kB	0 B
./bundle/getMachineId-win-6KLLGOI4.js	1.72 kB	0 B
./bundle/memoryDiscovery-NXEEFR74.js	980 B	0 B
./bundle/multipart-parser-KPBZEGQU.js	11.7 kB	0 B
./bundle/node_modules/@google/gemini-cli-devtools/dist/client/main.js	222 kB	0 B
./bundle/node_modules/@google/gemini-cli-devtools/dist/src/_client-assets.js	229 kB	0 B
./bundle/node_modules/@google/gemini-cli-devtools/dist/src/index.js	13.4 kB	0 B
./bundle/node_modules/@google/gemini-cli-devtools/dist/src/types.js	132 B	0 B
./bundle/sandbox-macos-permissive-open.sb	890 B	0 B
./bundle/sandbox-macos-permissive-proxied.sb	1.31 kB	0 B
./bundle/sandbox-macos-restrictive-open.sb	3.36 kB	0 B
./bundle/sandbox-macos-restrictive-proxied.sb	3.56 kB	0 B
./bundle/sandbox-macos-strict-open.sb	4.82 kB	0 B
./bundle/sandbox-macos-strict-proxied.sb	5.02 kB	0 B
./bundle/src-QVCVGIUX.js	47 kB	0 B
./bundle/start-N46YKBAM.js	0 B	-622 B (removed)	🏆
./bundle/tree-sitter-7U6MW5PS.js	274 kB	0 B
./bundle/tree-sitter-bash-34ZGLXVX.js	1.84 MB	0 B
./bundle/cleanup-4ACZZU5W.js	932 B	+932 B (new file)	🆕
./bundle/start-R7F3ON3B.js	622 B	+622 B (new file)	🆕

_{compressed-size-action}

[gemini-code-assist]

Code Review

This pull request modifies the shell command wrapping logic to use newlines instead of semicolons, which improves support for heredocs and multi-line commands. Feedback identifies a security vulnerability due to predictable temporary file paths and suggests using fs.mkdtempSync() as per repository rules. Additionally, it recommends adding a newline after the opening brace of the command wrapper to prevent potential issues with backslashes or comments, along with updating the relevant tests.

[gemini-code-assist]

Code Review

This pull request improves shell command wrapping in the useExecutionLifecycle hook and shell tools to correctly handle heredocs. By replacing conditional semicolon appending with a mandatory newline before the closing brace or parenthesis, the changes ensure that heredoc delimiters are properly recognized by the shell. New test cases were added to both the CLI and Core packages to verify this behavior. I have no feedback to provide as no review comments were present.

[cocosheng-g]

/gemini review

[gemini-code-assist]

Code Review

This pull request enhances the security and reliability of shell command execution by utilizing unique temporary directories for capturing the working directory and process IDs. It also improves command wrapping to correctly handle heredocs and trailing backslashes by using newlines. Review feedback identifies a non-deterministic test in shell.test.ts caused by the lack of mocking for fsPromises.mkdtemp, and a potential command injection vulnerability where temporary file paths are interpolated into shell strings without adequate escaping.

I am having trouble creating individual review comments. Click here to see my feedback.

packages/core/src/tools/shell.test.ts (512)

The test assertions in this file rely on a hardcoded temporary directory name (gemini-shell-abcdef). However, the implementation in shell.ts now uses fsPromises.mkdtemp, which generates a directory with a random suffix, making this test non-deterministic and likely to fail.

To fix this, please mock fsPromises.mkdtemp to return a predictable path, similar to how fs.mkdtempSync is mocked in useExecutionLifecycle.test.tsx.

You can add the following to your test setup:

import * as fsPromises from 'node:fs/promises';
vi.mock('node:fs/promises');

// In beforeEach:
vi.mocked(fsPromises.mkdtemp).mockResolvedValue(
  path.join(os.tmpdir(), 'gemini-shell-abcdef')
);

This will ensure your tests are reliable and correctly verify the new implementation.

References

1. In tests, prefer using hardcoded literal values instead of importing constants to ensure tests are self-contained and less brittle.

packages/cli/src/ui/hooks/useExecutionLifecycle.ts (373)

The pwdFilePath variable is interpolated into a shell command string using double quotes: pwd > "${pwdFilePath}". While double quotes prevent word splitting (handling spaces), they do not prevent command substitution in shells like bash. Since pwdFilePath is derived from os.tmpdir() (which is influenced by the TMPDIR environment variable), an attacker who can control environment variables could achieve command injection by including $(...) or backticks in the temporary directory path.

Although this is a CLI tool where the user typically has full shell access, this vulnerability could be exploited in restricted environments (e.g., shared systems, web-based terminals, or restricted CI/CD pipelines) where a user might be able to set environment variables but is otherwise restricted from running arbitrary commands. Additionally, a similar unquoted usage exists in packages/core/src/tools/shell.ts (line 114), which is also vulnerable to word splitting if the path contains spaces.

References

1. When creating temporary files or directories in global temporary directories (e.g., /tmp), use fs.mkdtempSync() to generate securely named, uniquely named temporary directories to mitigate symlink attacks.

[cocosheng-g]

/gemini review

[gemini-code-assist]

Code Review

This pull request enhances the security and reliability of shell command execution by replacing predictable temporary filenames with unique directories created via mkdtemp. It also introduces shell argument escaping and improves the wrapping of commands to correctly handle heredocs and trailing backslashes. The review feedback identifies potential resource leaks where temporary directories are created outside of try-finally blocks, and recommends using fs.mkdtempSync instead of fsPromises.mkdtemp to align with security best practices for global temporary directories.

[cocosheng-g]

/gemini review

[gemini-code-assist]

Code Review

This pull request improves the security and robustness of shell command execution by using fs.mkdtempSync for temporary directories and escapeShellArg for file paths, replacing the previous crypto-based random filename approach. It also refactors the command wrapping logic in useExecutionLifecycle and ShellToolInvocation to ensure proper cleanup of temporary resources. Feedback was provided regarding the parsing of pgrep output, suggesting that lines should be trimmed to handle potential carriage returns when splitting by \n to avoid regex mismatches.

[cocosheng-g]

/gemini review

[github-actions]

🛑 Action Required: Evaluation Approval

Steering changes have been detected in this PR. To prevent regressions, a maintainer must approve the evaluation run before this PR can be merged.

Maintainers:

1. Go to the Workflow Run Summary.
2. Click the yellow 'Review deployments' button.
3. Select the 'eval-gate' environment and click 'Approve'.

Once approved, the evaluation results will be posted here automatically.