feat(sandbox): implement secret visibility lockdown for env files#23712
feat(sandbox): implement secret visibility lockdown for env files#23712DavidAPierce wants to merge 7 commits intomainfrom
Conversation
|
Hi @DavidAPierce, 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 significantly enhances the security posture of the sandbox environments by implementing a robust system to prevent unauthorized access to sensitive files, such as 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 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. Footnotes
|
![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 enhances sandboxing capabilities across Linux, macOS, and Windows to protect sensitive files (like .env files) and enforce explicitly forbidden paths. It introduces new utility functions for identifying secret files and integrates these protections into the platform-specific sandbox managers. Specifically, the Linux sandbox uses bwrap to mask secrets and forbidden paths, the macOS sandbox updates seatbelt profiles with regex-based deny rules, and the Windows sandbox uses icacls to set integrity levels for secret files and a C# helper to enforce forbidden paths. Key security issues identified include a symlink attack vulnerability in the Linux sandbox's temporary mask file creation, a path normalization bypass in the Windows sandbox's CheckForbidden method using 8.3 short names, and silent error handling in the Linux sandbox when finding and masking secret files, which should be logged. Additionally, a configuration change to disable auto-updates in the test rig is included.
| const maskPath = join(os.tmpdir(), `gemini-cli-mask-${process.pid}`); | ||
| fs.writeFileSync(maskPath, ''); |
There was a problem hiding this comment.
Similar to the seccomp BPF file, this temporary mask file is created with a predictable name in a global directory, making it susceptible to symlink attacks. An attacker could use this to truncate arbitrary files that the user has write access to.
There was a problem hiding this comment.
I think this is valid comment.
There was a problem hiding this comment.
Addressed.
I refactored the creation of the seccomp BPF file, the Linux mask file, the Linux mask directory, and the Windows forbidden paths manifest to use fs.mkdtempSync(). This creates securely generated, uniquely named temporary
directories (e.g., /tmp/gemini-cli-mask-file-XXX/mask) instead of placing files directly into the global /tmp directory with easily guessable names (/tmp/gemini-cli-mask-file-).
By doing this, we completely mitigate the risk of an attacker pre-creating a symlink with that predictable name to truncate arbitrary files on the host system. I've also verified that the tests still pass successfully with
this updated logic.
| } | ||
|
|
||
| private static void CheckForbidden(string path, List<string> forbiddenPaths) { | ||
| string fullPath = Path.GetFullPath(path).ToLower(); |
There was a problem hiding this comment.
The CheckForbidden method relies on Path.GetFullPath for path normalization, which does not resolve 8.3 short names (e.g., PROGRA~1 for Program Files) on Windows. An attacker can bypass the forbiddenPaths check by providing the short name of a restricted file or directory. Since the subsequent file access calls in the helper (and the sandboxed process) will resolve and accept these short names, the security policy can be circumvented.
To fix this, you should resolve all paths to their long-name equivalents before performing the comparison. This can be achieved by using the GetLongPathName Win32 API via P/Invoke.
| } catch { | ||
| // Ignore errors finding secrets | ||
| } |
There was a problem hiding this comment.
This try...catch block silently ignores any errors that occur while finding and masking secret files. If an error occurs (e.g., the find command is not available or fails for some reason), the secret protection will fail silently, which poses a security risk. It's better to log this failure for debugging purposes, similar to how it's handled in the WindowsSandboxManager.
You will also need to import debugLogger from ../../utils/debugLogger.js.
} catch (e) {
// Ignore errors finding secrets, but log them for debugging.
debugLogger.log(
'LinuxSandboxManager: Failed to secure secret files',
e,
);
}References
- When catching exceptions, log the detailed error for debugging instead of providing only a generic error message.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
Size Change: +6.43 kB (+0.02%) Total Size: 26.3 MB
ℹ️ View Unchanged
|
gemini-cli
bot
added
the
status/need-issue
Code ReviewScope: Pull Request #23712 This PR implements a strict visibility lockdown for Metadata Review
Concerns (Action Required)Linux (
Windows (
Nits (Suggestions)
|
…lock' into sandbox_visibility_lock
|
Updated PR to merge with incoming changes and address comments. Summary Details & Review Remediation Linux (Bubblewrap)
Windows (Low Integrity & Helper)
macOS (Seatbelt)
Unified Secret Service
Related Issues How to Validate
Pre-Merge Checklist
|
| const escapedBase = escapeRegex(resolvedBase); | ||
| if (secret.pattern.endsWith('*')) { | ||
| // .env.* -> .env\..+ (match .env followed by dot and something) | ||
| const basePattern = secret.pattern.slice(0, -1).replace(/\./g, '\\.'); |
Check failure
Code scanning / CodeQL
Incomplete string escaping or encoding High
| regexPattern = `^${escapedBase}/.*${basePattern}[^/]+$`; | ||
| } else { | ||
| // .env -> \.env$ | ||
| const basePattern = secret.pattern.replace(/\./g, '\\.'); |
Check failure
Code scanning / CodeQL
Incomplete string escaping or encoding High
3 participants
Summary
This PR implements a strict visibility lockdown for secret files (specifically .env and .env.*) across all supported sandbox environments (Linux, macOS, and Windows). This ensures that sensitive credentials and API keys
are effectively invisible and inaccessible to any tools running within the sandbox.
Details
Related Issues
Fixes https://github.com/google-gemini/maintainers-gemini-cli/issues/1560
How to Validate
Pre-Merge Checklist