fix(core): enhance workspace path boundary checks and symlink resolution in command safety and file discovery - #29170
Conversation
|
📊 PR Size: size/XL
|
|
/gemini review |
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 significantly hardens the sandbox security by implementing strict workspace boundary checks across file discovery, directory listing, and command execution heuristics. It ensures that file-reading utilities and symbolic links cannot be used to escape the project root, providing a more secure environment for executing commands on both POSIX and Windows systems. Highlights
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
|
There was a problem hiding this comment.
Code Review
This pull request enhances the sandbox environment by introducing workspace path boundary validation to prevent path traversal and sandbox escapes during command execution. It updates the policy engine and platform-specific sandbox managers to track the working directory and workspace root, validating that arguments for file-reading commands (such as cat, ls, grep, rg, and sed) remain strictly within the workspace. The review feedback highlights several critical security vulnerabilities in these new checks, including tilde expansion bypasses (e.g., ~root), option parsing bypasses in grep and rg (such as --file or options with =), and a Windows-specific bypass where absolute paths starting with / are misinterpreted as flags. Addressing these vulnerabilities is essential to ensure the robustness of the sandbox boundaries.
There was a problem hiding this comment.
Code Review
This pull request introduces workspace path boundary validation to prevent executed commands from escaping the workspace root, updating the policy engine, sandbox managers, and command safety utilities for both POSIX and Windows. While the changes significantly improve security, several critical issues and vulnerabilities were identified in the review. These include potential false positives/negatives when the workspace root or working directory contains symbolic links, a path traversal vulnerability in PolicyEngine.applyShellHeuristics via the dir_path parameter, and bypasses in grep, ripgrep, and Windows parameter parsing where options like --file or colon-separated parameters (e.g., -Path:) escape validation.
81fb855 to
8bd51b5
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request enhances sandbox security by propagating the working directory (cwd) and workspace root to command safety checks, preventing path traversal and escaping via symbolic links or directory manipulation. It also updates the file discovery service to ignore symlinks pointing outside the project root. While these changes significantly improve security, the code review highlights critical vulnerabilities: path-escaping bypasses in the grep and ripgrep validation logic due to pattern-skipping heuristics, a lack of validation ensuring the resolved working directory (effectiveCwd) remains within the workspace boundary, and a missing argument update at a call site in WindowsSandboxManager.ts.
…ion in command safety and file discovery
8bd51b5 to
6b835c1
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces workspace path boundary validation across the policy engine, sandbox managers, and file discovery service to prevent command execution and file access from escaping the workspace root. The review feedback highlights several critical security vulnerabilities and improvement opportunities in the path validation logic. Specifically, the cd command validation can be bypassed using switches or empty arguments, the find command validation breaks early on options (bypassing subsequent path checks), and Windows switch path validation fails to check relative paths starting with alphanumeric characters. Additionally, the reviewer recommends replacing isWithinRoot with isSubpath to ensure robust, case-insensitive path boundary checks on Windows and macOS.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request enhances sandbox security by introducing workspace path boundary validation for shell commands across Linux, macOS, and Windows environments, ensuring that commands and symlinks do not escape the workspace. It also updates the FileDiscoveryService to ignore symlinks pointing outside the project root. The review feedback highlights critical security and performance improvements, including addressing path traversal bypasses in command-line options and Windows switches, adding the sort command to the file-reading allowlist to prevent arbitrary file access, and caching the resolved project root in FileDiscoveryService to avoid repeated synchronous file system I/O.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces workspace path boundary validation to prevent executed commands from escaping the workspace root. It updates the policy engine, sandbox managers, and command safety utilities to resolve and validate path arguments for file-reading commands (such as cat, ls, grep, find, and cd) against the workspace boundary, including resolving symbolic links. Additionally, it updates the file discovery service to ignore external symlinks and adds comprehensive tests. The review feedback highlights a potential issue in the symlink traversal loop within both POSIX and Windows command safety utilities, where direct string comparison against the workspace root could fail due to casing differences or parent symlinks; using isSubpath is recommended to safely terminate the loop.
Note: Security Review did not run due to the size of the PR.
…, and cached realProjectRoot
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces robust workspace path boundary validation across Unix and Windows sandboxes to prevent directory traversal and path escaping. It updates the policy engine, sandbox managers, and file discovery services to resolve and validate working directories and path arguments against the workspace root, and adds comprehensive tests. The review feedback highlights two critical security improvements: limiting the length of Windows switch patterns in the cd safety check to prevent path bypasses (e.g., /windows), and forcing an ASK_USER decision in the policy engine if a command's effective working directory escapes the workspace boundary.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request enhances workspace boundary enforcement by validating that command arguments and working directories do not escape the workspace root, including checks for symlinks pointing outside the project. It updates the policy engine, sandbox managers, and file discovery services across POSIX and Windows platforms, and adds comprehensive test coverage. A security review comment highlights a potential bypass vulnerability where file paths starting with a hyphen (e.g., ./-secret) could be incorrectly skipped as command-line options, and suggests checking for file existence to ensure robust path validation.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request enhances security and path validation across the policy engine, sandbox managers, and file discovery services. It introduces robust checks to ensure that commands and path arguments (including symbolic links and working directories) do not escape the designated workspace boundary, forcing user confirmation (ASK_USER) when an escape is detected unless YOLO mode is active. Additionally, it updates the SandboxManager interfaces and implementations for Linux, macOS, and Windows to support directory-aware safety checks, adds comprehensive unit tests for these boundary validations, and integrates path access validation into the ls tool. I have no further feedback to provide as there are no review comments.
Note: Security Review did not run due to the size of the PR.
|
✅ 68 tests passed successfully on gemini-3-flash-preview. 🧠 Model Steering GuidanceThis PR modifies files that affect the model's behavior (prompts, tools, or instructions).
This is an automated guidance message triggered by steering logic signatures. |
Summary
Enhances workspace boundary enforcement and symbolic link resolution across command safety heuristics, file discovery services, and directory listing tools on POSIX and Windows systems.
Details
isPathEscapingWorkspace) toisKnownSafeCommandandisSafeToCallWithExecin@google/gemini-cli-core(packages/core/src/sandbox/utils/commandSafety.ts). Inspects target file and directory arguments for file-reading utilities (such ascat,head,tail,ls,grep,find,sed, etc.) to ensure arguments do not traverse outside the workspace root or dereference through symbolic links to external targets. Unresolved shell variables and user directory expansions (~) that cannot be validated statically are treated as non-safe.isDangerousCommandincommandSafety.tsto identifylninvocations that specify symbolic link flags (-s,--symbolic) so that confirmation is required before creating new symbolic links.packages/core/src/sandbox/windows/commandSafety.ts) for utilities likedir,type,attrib,more,findstr,get-childitem, andget-content.SandboxManagerinterface and implementations (LocalSandboxManager,LinuxSandboxManager,MacOsSandboxManager,WindowsSandboxManager) to accept and propagate the current working directory (cwd) alongside workspace root, ensuring thatPolicyEngine.applyShellHeuristicspasses the effective directory context to safety evaluators.FileDiscoveryService(packages/core/src/services/fileDiscoveryService.ts) to verify symbolic link targets withisWithinRootand ignore links pointing outside the project root.ls) Tool: AddedvalidatePathAccesschecks when resolving directory entries inpackages/core/src/tools/ls.tsto ensure entries outside the workspace boundary are excluded.packages/core/src/sandbox/utils/commandSafety.test.ts,packages/core/src/services/fileDiscoveryService.symlink.test.ts, and verified againstls.test.tsandpolicy-engine.test.ts.Related Issues
Fixes b/439919800
How to Validate
npm test --workspace @google/gemini-cli-core -- src/sandbox/utils/commandSafety.test.ts src/services/fileDiscoveryService.symlink.test.ts src/tools/ls.test.ts src/policy/policy-engine.test.tsnpm run lint:ci && npm run typechecknpm test --workspace @google/gemini-cli-corePre-Merge Checklist