feat(sandbox): implement native bubblewrap (bwrap) sandbox for Linux#22399
feat(sandbox): implement native bubblewrap (bwrap) sandbox for Linux#22399Lyonk71 wants to merge 11 commits intogoogle-gemini:mainfrom
Conversation
Implement a lightweight, non-container sandbox using bubblewrap for Linux. The outer sandbox wraps the entire CLI process with namespace isolation, while BwrapSandboxManager provides nested per-tool isolation (network disabled) through the injectable SandboxManager interface — requiring zero changes to shellExecutionService.
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 significant enhancement to the sandboxing capabilities on Linux by integrating 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
|
![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 introduces a native Linux sandbox using bubblewrap (bwrap) to provide a two-layer isolation model for the main CLI process and individual tool executions. However, a critical command injection vulnerability has been identified in the 'outer' sandbox implementation in packages/cli/src/utils/sandbox.ts due to improper handling of nodeOptions during command string construction, which could lead to arbitrary command execution within the sandbox. Additionally, there is a path traversal vulnerability that could allow a sandbox escape by mounting sensitive system directories, and missing propagation of authentication credentials (API keys and ADC) into the sandbox, which would prevent the CLI from functioning correctly. Addressing these issues is crucial for the security and functionality of this otherwise strong addition to the project's security posture.
packages/cli/src/utils/sandbox.ts
Outdated
| const innerCmd = [ | ||
| 'SANDBOX=bwrap', | ||
| `NODE_OPTIONS="${nodeOptions}"`, | ||
| ...finalArgv.map((arg) => quote([arg])), | ||
| ].join(' '); | ||
|
|
||
| const args = [...bwrapArgs, '--', shell, '-c', innerCmd]; |
There was a problem hiding this comment.
The innerCmd string is constructed by concatenating nodeOptions into a shell command without proper quoting or sanitization. Specifically, nodeOptions is embedded within double quotes at line 1304. If any argument in nodeArgs (which populates nodeOptions) contains a double quote, it can break out of the environment variable assignment and execute arbitrary shell commands inside the sandbox. Although this execution occurs within the sandbox, it still represents a command injection vulnerability that could be exploited if nodeArgs are influenced by untrusted sources (e.g., project-specific configurations).
To remediate this, avoid constructing a shell command string and using shell -c. Instead, pass the environment variables directly to the spawn call via the env option, or use bwrap's --setenv flag to set environment variables for the sandboxed process. If a shell must be used, ensure all components of the command string are properly quoted using a library like shell-quote.
References
- This rule emphasizes the need to sanitize environments to prevent bypasses or injections, which is directly relevant to the
nodeOptionsinjection issue. - This rule also highlights the importance of sanitizing environments to prevent malicious actions, reinforcing the need for proper handling of
nodeOptionsto avoid command injection.
There was a problem hiding this comment.
@gemini-code-assist - updates pushed. Does this look correct now?
There was a problem hiding this comment.
Yes, the updates look correct now. You've addressed the command injection vulnerability by:
- Using
bwrap's--setenvflag to passNODE_OPTIONSdirectly to the sandboxed process, avoiding shell interpolation for this sensitive environment variable. - Employing the
shell-quotelibrary'squotefunction to properly escapecliArgswhen constructing theinnerCmdstring, which is then passed toshell -c.
These changes significantly improve the security of the sandbox by preventing malicious input in nodeOptions or cliArgs from executing arbitrary commands.
|
Thank you for the detailed explanation regarding the |
Add GEMINI_API_KEY, GOOGLE_API_KEY, GOOGLE_APPLICATION_CREDENTIALS, and other Gemini/Google env vars to the safeEnvKeys allowlist, matching parity with the Docker/Podman sandbox. Also mount the ADC file read-only when GOOGLE_APPLICATION_CREDENTIALS is set.
Replace shell string interpolation of SANDBOX and NODE_OPTIONS with bwrap's --setenv flag, eliminating any theoretical command injection via crafted nodeOptions values.
Lyonk71
force-pushed
the
feat/bwrap-sandbox-v2
branch
from
c39be78 to
3bf9782
Compare
Move duplicated bwrap namespace flags, path binding loops, and node binary binding logic into bwrapUtils.ts. The security-relevant differences (network isolation, writable mounts, env filtering) stay inline in each sandbox.
Cover namespace isolation flags, path binding helpers, node binary binding, environment sanitization, network isolation, and non-Linux fallback behavior.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a new sandbox implementation for Linux using bubblewrap (bwrap), providing a lightweight and performant alternative to Docker/Podman. The changes are well-structured, with clear separation between the outer CLI sandbox and the nested, more restrictive tool sandbox. My review focuses on ensuring consistency and correctness in the new implementation. I've identified a couple of areas for improvement: ensuring feature parity for environment variable configuration across all sandbox types, and improving the robustness of the Linux distribution detection logic.
| // Filter environment variables to prevent leakage of host credentials | ||
| const safeEnvKeys = [ | ||
| 'PATH', | ||
| 'TERM', | ||
| 'COLORTERM', | ||
| 'LANG', | ||
| 'LC_ALL', | ||
| 'HOME', | ||
| 'USER', | ||
| 'LOGNAME', | ||
| 'SHELL', | ||
| 'PWD', | ||
| 'EDITOR', | ||
| 'VISUAL', | ||
| 'DEBUG', | ||
| 'DEBUG_PORT', | ||
| 'NODE_ENV', | ||
| 'NODE_OPTIONS', | ||
| 'GEMINI_SANDBOX', | ||
| 'GEMINI_SANDBOX_IMAGE', | ||
| 'GEMINI_SANDBOX_IMAGE_DEFAULT', | ||
| 'GEMINI_DIR', | ||
| 'GEMINI_CLI_HOME', | ||
| 'GEMINI_CLI_NO_RELAUNCH', | ||
| 'SANDBOX_PORTS', | ||
| 'SANDBOX_SET_UID_GID', | ||
| 'SANDBOX_MOUNTS', | ||
| 'SANDBOX_FLAGS', | ||
| 'SEATBELT_PROFILE', | ||
| 'HTTP_PROXY', | ||
| 'HTTPS_PROXY', | ||
| 'NO_PROXY', | ||
| 'http_proxy', | ||
| 'https_proxy', | ||
| 'no_proxy', | ||
| 'NO_BROWSER', | ||
| 'GEMINI_API_KEY', | ||
| 'GOOGLE_API_KEY', | ||
| 'GOOGLE_APPLICATION_CREDENTIALS', | ||
| 'GOOGLE_GEMINI_BASE_URL', | ||
| 'GOOGLE_VERTEX_BASE_URL', | ||
| 'GOOGLE_GENAI_USE_VERTEXAI', | ||
| 'GOOGLE_GENAI_USE_GCA', | ||
| 'GOOGLE_CLOUD_PROJECT', | ||
| 'GOOGLE_CLOUD_LOCATION', | ||
| 'GEMINI_MODEL', | ||
| ]; | ||
|
|
||
| const sandboxEnv: NodeJS.ProcessEnv = {}; | ||
| for (const key of safeEnvKeys) { | ||
| if (process.env[key] !== undefined) { | ||
| sandboxEnv[key] = process.env[key]; | ||
| } | ||
| } |
There was a problem hiding this comment.
The bwrap sandbox implementation does not handle the SANDBOX_ENV environment variable, which is supported by the Docker/Podman sandbox implementations. This creates an inconsistency where users might expect to pass custom environment variables to any sandbox type using SANDBOX_ENV.
To ensure feature parity, please add support for SANDBOX_ENV to the bwrap sandbox. You can parse the variable and add the key-value pairs to the sandboxEnv object, which is then passed to the spawn call.
There was a problem hiding this comment.
@gemini-code-assist - SANDBOX_ENV exists for Docker/Podman because containers start with a blank environment and need every variable explicitly passed in. bwrap runs on the host OS, so it takes the opposite approach — we inherit the host environment filtered through a safeEnvKeys allowlist to pass through only the minimum necessary set. Adding SANDBOX_ENV here would be redundant since the vars are already available on the host.
There was a problem hiding this comment.
Thanks for the clarification, Lyonk71. I understand that bwrap inherits the host environment and then filters it through safeEnvKeys, which is a different mechanism from Docker/Podman starting with a blank environment. However, this difference in mechanism actually highlights why SANDBOX_ENV is still necessary for feature parity.
The safeEnvKeys list acts as an allowlist. If a user wants to pass a custom environment variable (e.g., MY_API_KEY=12345) that is not already present in safeEnvKeys, it will not be propagated into the bwrap sandbox, even if it exists on the host. This limits the user's ability to inject specific, ad-hoc environment variables for their tools or scripts running within the sandbox.
SANDBOX_ENV in Docker/Podman serves precisely this purpose: to allow users to explicitly specify additional environment variables to be included in the sandbox, potentially overriding existing ones or adding new ones not in the default set. Without this, the bwrap sandbox lacks the flexibility that Docker/Podman sandboxes offer in terms of environment customization.
To achieve true feature parity and allow users to pass custom environment variables to the bwrap sandbox, I recommend adding support for parsing SANDBOX_ENV and incorporating those variables into the sandboxEnv object. This would ensure that users have a consistent way to configure their sandbox environment across different sandbox implementations.
I've updated the suggestion to include SANDBOX_ENV in the safeEnvKeys and to parse its contents, adding or overriding variables in sandboxEnv accordingly. This is a high priority issue as it impacts the functionality and flexibility of the bwrap sandbox for users who rely on custom environment variables.
| // Filter environment variables to prevent leakage of host credentials | |
| const safeEnvKeys = [ | |
| 'PATH', | |
| 'TERM', | |
| 'COLORTERM', | |
| 'LANG', | |
| 'LC_ALL', | |
| 'HOME', | |
| 'USER', | |
| 'LOGNAME', | |
| 'SHELL', | |
| 'PWD', | |
| 'EDITOR', | |
| 'VISUAL', | |
| 'DEBUG', | |
| 'DEBUG_PORT', | |
| 'NODE_ENV', | |
| 'NODE_OPTIONS', | |
| 'GEMINI_SANDBOX', | |
| 'GEMINI_SANDBOX_IMAGE', | |
| 'GEMINI_SANDBOX_IMAGE_DEFAULT', | |
| 'GEMINI_DIR', | |
| 'GEMINI_CLI_HOME', | |
| 'GEMINI_CLI_NO_RELAUNCH', | |
| 'SANDBOX_PORTS', | |
| 'SANDBOX_SET_UID_GID', | |
| 'SANDBOX_MOUNTS', | |
| 'SANDBOX_FLAGS', | |
| 'SEATBELT_PROFILE', | |
| 'HTTP_PROXY', | |
| 'HTTPS_PROXY', | |
| 'NO_PROXY', | |
| 'http_proxy', | |
| 'https_proxy', | |
| 'no_proxy', | |
| 'NO_BROWSER', | |
| 'GEMINI_API_KEY', | |
| 'GOOGLE_API_KEY', | |
| 'GOOGLE_APPLICATION_CREDENTIALS', | |
| 'GOOGLE_GEMINI_BASE_URL', | |
| 'GOOGLE_VERTEX_BASE_URL', | |
| 'GOOGLE_GENAI_USE_VERTEXAI', | |
| 'GOOGLE_GENAI_USE_GCA', | |
| 'GOOGLE_CLOUD_PROJECT', | |
| 'GOOGLE_CLOUD_LOCATION', | |
| 'GEMINI_MODEL', | |
| ]; | |
| const sandboxEnv: NodeJS.ProcessEnv = {}; | |
| for (const key of safeEnvKeys) { | |
| if (process.env[key] !== undefined) { | |
| sandboxEnv[key] = process.env[key]; | |
| } | |
| } | |
| // Filter environment variables to prevent leakage of host credentials | |
| const safeEnvKeys = [ | |
| 'PATH', | |
| 'TERM', | |
| 'COLORTERM', | |
| 'LANG', | |
| 'LC_ALL', | |
| 'HOME', | |
| 'USER', | |
| 'LOGNAME', | |
| 'SHELL', | |
| 'PWD', | |
| 'EDITOR', | |
| 'VISUAL', | |
| 'DEBUG', | |
| 'DEBUG_PORT', | |
| 'NODE_ENV', | |
| 'NODE_OPTIONS', | |
| 'GEMINI_SANDBOX', | |
| 'GEMINI_SANDBOX_IMAGE', | |
| 'GEMINI_SANDBOX_IMAGE_DEFAULT', | |
| 'GEMINI_DIR', | |
| 'GEMINI_CLI_HOME', | |
| 'GEMINI_CLI_NO_RELAUNCH', | |
| 'SANDBOX_PORTS', | |
| 'SANDBOX_SET_UID_GID', | |
| 'SANDBOX_MOUNTS', | |
| 'SANDBOX_FLAGS', | |
| 'SEATBELT_PROFILE', | |
| 'HTTP_PROXY', | |
| 'HTTPS_PROXY', | |
| 'NO_PROXY', | |
| 'http_proxy', | |
| 'https_proxy', | |
| 'no_proxy', | |
| 'NO_BROWSER', | |
| 'GEMINI_API_KEY', | |
| 'GOOGLE_API_KEY', | |
| 'GOOGLE_APPLICATION_CREDENTIALS', | |
| 'GOOGLE_GEMINI_BASE_URL', | |
| 'GOOGLE_VERTEX_BASE_URL', | |
| 'GOOGLE_GENAI_USE_VERTEXAI', | |
| 'GOOGLE_GENAI_USE_GCA', | |
| 'GOOGLE_CLOUD_PROJECT', | |
| 'GOOGLE_CLOUD_LOCATION', | |
| 'GEMINI_MODEL', | |
| 'SANDBOX_ENV', // Add SANDBOX_ENV to the allowlist | |
| ]; | |
| const sandboxEnv: NodeJS.ProcessEnv = {}; | |
| for (const key of safeEnvKeys) { | |
| if (process.env[key] !== undefined) { | |
| sandboxEnv[key] = process.env[key]; | |
| } | |
| } | |
| // Process SANDBOX_ENV for additional environment variables | |
| if (process.env['SANDBOX_ENV']) { | |
| const customEnvVars = process.env['SANDBOX_ENV'].split(','); | |
| for (const entry of customEnvVars) { | |
| const parts = entry.split('='); | |
| const key = parts[0].trim(); | |
| if (key) { | |
| if (parts.length > 1) { | |
| // Key=Value pair | |
| sandboxEnv[key] = parts.slice(1).join('=').trim(); | |
| } else if (process.env[key] !== undefined) { | |
| // Key only, copy from host environment | |
| sandboxEnv[key] = process.env[key]; | |
| } | |
| } | |
| } | |
| } |
| // Filter environment variables to prevent leakage of host credentials | |
| const safeEnvKeys = [ | |
| 'PATH', | |
| 'TERM', | |
| 'COLORTERM', | |
| 'LANG', | |
| 'LC_ALL', | |
| 'HOME', | |
| 'USER', | |
| 'LOGNAME', | |
| 'SHELL', | |
| 'PWD', | |
| 'EDITOR', | |
| 'VISUAL', | |
| 'DEBUG', | |
| 'DEBUG_PORT', | |
| 'NODE_ENV', | |
| 'NODE_OPTIONS', | |
| 'GEMINI_SANDBOX', | |
| 'GEMINI_SANDBOX_IMAGE', | |
| 'GEMINI_SANDBOX_IMAGE_DEFAULT', | |
| 'GEMINI_DIR', | |
| 'GEMINI_CLI_HOME', | |
| 'GEMINI_CLI_NO_RELAUNCH', | |
| 'SANDBOX_PORTS', | |
| 'SANDBOX_SET_UID_GID', | |
| 'SANDBOX_MOUNTS', | |
| 'SANDBOX_FLAGS', | |
| 'SEATBELT_PROFILE', | |
| 'HTTP_PROXY', | |
| 'HTTPS_PROXY', | |
| 'NO_PROXY', | |
| 'http_proxy', | |
| 'https_proxy', | |
| 'no_proxy', | |
| 'NO_BROWSER', | |
| 'GEMINI_API_KEY', | |
| 'GOOGLE_API_KEY', | |
| 'GOOGLE_APPLICATION_CREDENTIALS', | |
| 'GOOGLE_GEMINI_BASE_URL', | |
| 'GOOGLE_VERTEX_BASE_URL', | |
| 'GOOGLE_GENAI_USE_VERTEXAI', | |
| 'GOOGLE_GENAI_USE_GCA', | |
| 'GOOGLE_CLOUD_PROJECT', | |
| 'GOOGLE_CLOUD_LOCATION', | |
| 'GEMINI_MODEL', | |
| ]; | |
| const sandboxEnv: NodeJS.ProcessEnv = {}; | |
| for (const key of safeEnvKeys) { | |
| if (process.env[key] !== undefined) { | |
| sandboxEnv[key] = process.env[key]; | |
| } | |
| } | |
| // Filter environment variables to prevent leakage of host credentials | |
| const safeEnvKeys = [ | |
| 'PATH', | |
| 'TERM', | |
| 'COLORTERM', | |
| 'LANG', | |
| 'LC_ALL', | |
| 'HOME', | |
| 'USER', | |
| 'LOGNAME', | |
| 'SHELL', | |
| 'PWD', | |
| 'EDITOR', | |
| 'VISUAL', | |
| 'DEBUG', | |
| 'DEBUG_PORT', | |
| 'NODE_ENV', | |
| 'NODE_OPTIONS', | |
| 'GEMINI_SANDBOX', | |
| 'GEMINI_SANDBOX_IMAGE', | |
| 'GEMINI_SANDBOX_IMAGE_DEFAULT', | |
| 'GEMINI_DIR', | |
| 'GEMINI_CLI_HOME', | |
| 'GEMINI_CLI_NO_RELAUNCH', | |
| 'SANDBOX_PORTS', | |
| 'SANDBOX_SET_UID_GID', | |
| 'SANDBOX_MOUNTS', | |
| 'SANDBOX_FLAGS', | |
| 'SEATBELT_PROFILE', | |
| 'HTTP_PROXY', | |
| 'HTTPS_PROXY', | |
| 'NO_PROXY', | |
| 'http_proxy', | |
| 'https_proxy', | |
| 'no_proxy', | |
| 'NO_BROWSER', | |
| 'GEMINI_API_KEY', | |
| 'GOOGLE_API_KEY', | |
| 'GOOGLE_APPLICATION_CREDENTIALS', | |
| 'GOOGLE_GEMINI_BASE_URL', | |
| 'GOOGLE_VERTEX_BASE_URL', | |
| 'GOOGLE_GENAI_USE_VERTEXAI', | |
| 'GOOGLE_GENAI_USE_GCA', | |
| 'GOOGLE_CLOUD_PROJECT', | |
| 'GOOGLE_CLOUD_LOCATION', | |
| 'GEMINI_MODEL', | |
| 'SANDBOX_ENV', // Add SANDBOX_ENV to the allowlist | |
| ]; | |
| const sandboxEnv: NodeJS.ProcessEnv = {}; | |
| for (const key of safeEnvKeys) { | |
| if (process.env[key] !== undefined) { | |
| sandboxEnv[key] = process.env[key]; | |
| } | |
| } | |
| // Process SANDBOX_ENV for additional environment variables | |
| if (process.env['SANDBOX_ENV']) { | |
| const customEnvVars = process.env['SANDBOX_ENV'].split(','); | |
| for (const entry of customEnvVars) { | |
| const parts = entry.split('='); | |
| const key = parts[0].trim(); | |
| if (key) { | |
| if (parts.length > 1) { | |
| // Key=Value pair | |
| sandboxEnv[key] = parts.slice(1).join('=').trim(); | |
| } else if (process.env[key] !== undefined) { | |
| // Key only, copy from host environment | |
| sandboxEnv[key] = process.env[key]; | |
| } | |
| } | |
| } | |
| } |
There was a problem hiding this comment.
@gemini-code-assist - You're being verbose, and repeating yourself, but I think you are missing the point. For any system specific envars, we're already covered with safeEnvKeys. SANDBOX_ENV isn't relevant because our sandboxed tools don't have internet access like podman does. The reason is so that a malicious prompt injection can't pawn our local machine. It's a tradeoff we made to have a more secure sandbox at the expense of some flexibility. It's hard to consider a SANDBOX_ENV value that is not:
- A system envar (already covered)
- Used for remote systems (locked down)
We'd be adding complexity for parity theater that doesn't improve the product.
There was a problem hiding this comment.
Thanks for elaborating on the security considerations, Lyonk71. I understand the design choice to prioritize security by disabling network access for tools within the bwrap sandbox using --unshare-net, and that this is a deliberate trade-off.
My point regarding SANDBOX_ENV is not about network access or remote systems, but rather about environment customization and feature parity for local variables. While safeEnvKeys effectively filters existing host environment variables, it doesn't provide a mechanism for users to inject new, custom environment variables that are not already on that allowlist.
Consider a scenario where a user wants to pass a specific configuration flag, like MY_TOOL_CONFIG=debug_mode, to a tool running inside the bwrap sandbox. If MY_TOOL_CONFIG is not in safeEnvKeys, it will not be passed, even if it's a purely local variable with no security implications related to network access. Docker/Podman sandboxes allow this kind of custom variable injection via SANDBOX_ENV.
Adding support for SANDBOX_ENV to bwrap would allow users to define such custom, local environment variables, enhancing the flexibility and configurability of the sandbox without compromising the existing network isolation. The bwrap --unshare-net flag would still prevent network access for tools, regardless of what is passed via SANDBOX_ENV.
This change aims to provide a consistent user experience across different sandbox implementations for environment variable management, which is a key aspect of feature parity, and does not introduce new network vulnerabilities. The suggested code ensures that SANDBOX_ENV is processed and its contents are added to the sandboxEnv object, respecting the existing security model.
…spec Replace regex substring matching with proper parsing of the ID and ID_LIKE fields from /etc/os-release. This correctly handles multi-value ID_LIKE entries like "rhel fedora" without false positives on partial substring matches.
|
Hi there! Thank you for your interest in contributing to Gemini CLI. To ensure we maintain high code quality and focus on our prioritized roadmap, we have updated our contribution policy (see Discussion #17383). We only guarantee review and consideration of pull requests for issues that are explicitly labeled as 'help wanted'. All other community pull requests are subject to closure after 14 days if they do not align with our current focus areas. For this reason, we strongly recommend that contributors only submit pull requests against issues explicitly labeled as 'help-wanted'. This pull request is being closed as it has been open for 14 days without a 'help wanted' designation. We encourage you to find and contribute to existing 'help wanted' issues in our backlog! Thank you for your understanding and for being part of our community! |
1 participant
Summary
This PR implements a native, high-performance Linux sandbox using
bubblewrap(bwrap). It provides a hardened, zero-dependency environment for Linux distributions (like Fedora), replacing the need for rootless Podman/Docker for local isolation.Supersedes #22380, rebased on top of #22231 (SandboxManager integration).
Details
BwrapSandboxManager).SandboxManagerinterface — zero changes toshellExecutionService.ts. TheBwrapSandboxManageris automatically selected viacreateSandboxManager()when running inside a bwrap outer sandbox./usr,/bin,/lib, etc.) and the project workspace using a "Skeleton" mount approach.~/.ssh,~/.aws, etc.) by masking them with tmpfs or omitting them from binds./tmp,/run/user, and/dev/shmmounts to prevent host service escapes (D-Bus, Wayland, X11).How to Validate
GEMINI_SANDBOX=bwrap npm run start -- -s.ls ~/.ssh-> should fail or be empty.curl https://google.com-> should fail in tools (nested --unshare-net).ls /tmp-> should be a private, empty tmpfs.cd packages/cli && npx vitest run src/config/sandboxConfig.test.tsPre-Merge Checklist