fix(cli): isolate settings directory in sandbox containers - #29216
fix(cli): isolate settings directory in sandbox containers#29216jvargassanchez-dot wants to merge 4 commits into
Conversation
Isolate the user settings directory mounted into sandbox containers by preparing a session-scoped temporary directory containing only non-sensitive configuration (settings, commands, skills, policies, keybindings, and trusted folders) while explicitly omitting credentials and authentication token stores. Clean up the temporary directory upon container exit.
|
📊 PR Size: size/L
|
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 improves the security posture of the Gemini CLI by isolating the user configuration directory when running in containerized sandboxes. By creating a temporary, filtered copy of the settings directory, the CLI ensures that sensitive credentials and session data remain on the host while still providing the container with necessary configuration files. This change mitigates the risk of credential leakage within untrusted execution environments. 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
|
![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 sandbox security by mounting an isolated settings directory instead of exposing raw host credentials directly to the container. It introduces helper functions to identify and filter out sensitive files (such as OAuth credentials, history, and temp files) before copying the configuration to a temporary directory. Feedback was provided regarding the sensitivity check base.includes('credential'), which is too broad and may cause false positives on user-defined scripts or files containing the word "credential" in their name. A more precise suffix-matching approach was suggested.
gemini-cli
Bot
added
the
status/need-issue
Use precise suffix matching for credential and token stores to prevent false positives on user scripts and utilities containing 'credential' in their filenames.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces an isolated settings directory for the Docker sandbox to prevent exposing sensitive host credentials (such as OAuth tokens and account files) to untrusted containers. It filters out sensitive files and directories when copying settings to a temporary location. The review feedback correctly identifies a bug where generic directory names like 'history', 'tmp', and 'bin' are filtered out recursively, which would cause false positives for nested directories (e.g., within user commands or skills). The reviewer suggests restricting these generic exclusions to direct children of the root settings directory and adding corresponding unit tests.
…ctory Ensure that directory names such as 'history', 'tmp', and 'bin' are only excluded when they are direct children of the root settings directory, preventing false positives on nested directories within custom commands or skills.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request enhances sandbox security by mounting an isolated settings directory instead of directly exposing host credentials to the Docker container. It introduces a filtering mechanism to exclude sensitive files (such as OAuth tokens and history) when copying settings to a temporary directory, along with corresponding tests. The review feedback recommends expanding this filter to cover other common sensitive file patterns like .env, .key, .pem, and key.json, and updating the unit tests accordingly.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request enhances sandbox security by copying user settings to an isolated temporary directory and filtering out sensitive credential files before mounting them into the Docker container. The review feedback highlights a security concern regarding world-readable permissions on the created temporary directory, suggesting restricting it to 0700. It also recommends expanding the credential-filtering logic to cover singular variations of sensitive filenames (such as token and cred.json) and adding corresponding unit tests.
| const isolatedDir = fs.mkdtempSync( | ||
| path.join(baseTmpDir, 'gemini-sandbox-settings-'), | ||
| ); |
There was a problem hiding this comment.
The temporary directory isolatedDir is created in a shared location (e.g., /tmp) using fs.mkdtempSync without restricting its permissions. By default, this directory is created with world-readable permissions (usually 0755 or 0775 depending on the process's umask). Since this directory is populated with copied user settings (which can contain sensitive custom commands, skills, policies, keybindings, and trusted folders), any other local user on the same machine can read these files. To prevent local information disclosure, restrict the permissions of the temporary directory immediately after creation to 0700 (read, write, and execute by the owner only) using fs.chmodSync(isolatedDir, 0o700).
| const isolatedDir = fs.mkdtempSync( | |
| path.join(baseTmpDir, 'gemini-sandbox-settings-'), | |
| ); | |
| const isolatedDir = fs.mkdtempSync( | |
| path.join(baseTmpDir, 'gemini-sandbox-settings-'), | |
| ); | |
| fs.chmodSync(isolatedDir, 0o700); |
References
- When creating temporary files or directories in global temporary directories (e.g.,
/tmp), usefs.mkdtempSync()to generate securely named, uniquely named temporary directories. This mitigates symlink attacks where an attacker could pre-create a symlink with a predictable name to truncate arbitrary files.
| if ( | ||
| base.endsWith('.credentials') || | ||
| base.endsWith('credentials') || | ||
| base.endsWith('credentials.json') || | ||
| base.endsWith('tokens.json') || | ||
| base.endsWith('creds.json') || | ||
| base === '.env' || | ||
| base.endsWith('.env') || | ||
| base.endsWith('.key') || | ||
| base.endsWith('.pem') || | ||
| base.endsWith('.p12') || | ||
| base.endsWith('key.json') | ||
| ) { |
There was a problem hiding this comment.
The current implementation checks for plural tokens.json and creds.json but misses singular variations like token.json, token, and cred.json. This could lead to sensitive credentials or OAuth tokens being leaked into the sandbox container if they are named using singular forms (e.g., github-token.json or user_cred.json). Adding singular forms to the check improves security coverage.
if (
base.endsWith('.credentials') ||
base.endsWith('credentials') ||
base.endsWith('credentials.json') ||
base.endsWith('tokens.json') ||
base.endsWith('token.json') ||
base.endsWith('token') ||
base.endsWith('creds.json') ||
base.endsWith('cred.json') ||
base === '.env' ||
base.endsWith('.env') ||
base.endsWith('.key') ||
base.endsWith('.pem') ||
base.endsWith('.p12') ||
base.endsWith('key.json')
) {| it('should identify tokens, credentials, and sensitive directories', () => { | ||
| expect( | ||
| isCredentialOrSensitivePath('/home/user/.gemini/custom-tokens.json'), | ||
| ).toBe(true); | ||
| expect( | ||
| isCredentialOrSensitivePath('/home/user/.gemini/api.credentials'), | ||
| ).toBe(true); | ||
| expect( | ||
| isCredentialOrSensitivePath('/home/user/.gemini/user_creds.json'), | ||
| ).toBe(true); |
There was a problem hiding this comment.
Add test cases to verify that singular variations like token.json, token, and cred.json are correctly identified as sensitive paths.
it('should identify tokens, credentials, and sensitive directories', () => {
expect(
isCredentialOrSensitivePath('/home/user/.gemini/custom-tokens.json'),
).toBe(true);
expect(
isCredentialOrSensitivePath('/home/user/.gemini/token.json'),
).toBe(true);
expect(
isCredentialOrSensitivePath('/home/user/.gemini/github-token'),
).toBe(true);
expect(
isCredentialOrSensitivePath('/home/user/.gemini/api.credentials'),
).toBe(true);
expect(
isCredentialOrSensitivePath('/home/user/.gemini/user_creds.json'),
).toBe(true);
expect(
isCredentialOrSensitivePath('/home/user/.gemini/user_cred.json'),
).toBe(true);1 participant
Summary
When running Gemini CLI inside a container sandbox (Docker/Podman), the user configuration directory was previously mounted directly from the host's
~/.geminifolder. This could inadvertently expose sensitive local credentials (such as OAuth tokens, account credentials, and authentication stores) within the container's execution boundary.This change isolates the mounted settings directory by creating a session-scoped temporary directory on the host that includes only non-sensitive configuration (e.g.,
settings.json, custom commands, skills, policies, keybindings, and trusted folders) while strictly omitting credential stores. The temporary settings directory is automatically cleaned up upon container process completion.Details
Selective Settings Copy & Credential Omission:
prepareIsolatedSettingsDirandisCredentialOrSensitivePathinpackages/cli/src/utils/sandboxUtils.ts.oauth_creds.json,google_accounts.json,gemini-credentials.json,mcp-oauth-tokens.json,a2a-oauth-tokens.json, and wildcard credential/token patterns).history/,tmp/,bin/).settings.json,commands/,skills/,policies/,keybindings.json,trustedFolders.json).Container Volume Isolation:
packages/cli/src/utils/sandbox.tsto mount the isolated directory into/home/node/.gemini(and the containerized host settings mirror) instead of mounting the host's raw.geminidirectory.workdir, temp directory,~/.config/gcloud, andGOOGLE_APPLICATION_CREDENTIALSfor Vertex AI ADC workflows).Lifecycle Cleanup:
isolatedSettingsDirfor automatic cleanup viafs.rmSyncin thecleanup()handler executed on process exit,SIGINT, andSIGTERM.Testing:
packages/cli/src/utils/sandbox.test.tsverifying thatstart_sandboxmounts the isolated directory and does not mount raw host.geminifolders.packages/cli/src/utils/sandboxUtils.test.tstesting credential identification and isolation logic.Related Issues
How to Validate
Pre-Merge Checklist