Skip to content

fix(core): allow explicit write permissions to override governance file protections in sandboxes#25338

Merged
galz10 merged 18 commits intomainfrom
galzahavi/fix/git-commit
Apr 16, 2026
Merged

fix(core): allow explicit write permissions to override governance file protections in sandboxes#25338
galz10 merged 18 commits intomainfrom
galzahavi/fix/git-commit

Conversation

@galz10
Copy link
Copy Markdown
Collaborator

@galz10 galz10 commented Apr 13, 2026

Summary

This PR fixes an issue where strict read-only protections for governance files (like .git directories) overrode explicit write permissions. By updating the sandbox arguments builder for both macOS (Seatbelt) and Linux (bwrap), explicit write allowances now correctly take precedence, allowing users to modify these protected paths when intentionally configured.

Details

Previously, git worktree read-only rules or other governance file denials were being enforced unconditionally, conflicting with any explicit policyWrite bindings set by the user or system configuration.

  • Linux (bwrap): Reordered the generation of --bind-try for explicit write paths so they occur after the --ro-bind-try configurations for governance files and git worktrees. This allows bwrap to prioritize the later explicit write rules.
  • macOS (Seatbelt): Added checks before generating (deny file-write* ...) rules for governance files and git worktrees. If the path or a parent path is explicitly present in resolvedPaths.policyWrite, the deny rule is skipped for that path.
  • Updated relevant test cases to verify the precedence.

Related Issues

How to Validate

  1. Unit Tests: Run the updated Linux sandbox tests (if on Linux) to verify the bwrap args order.
    npm run test -w @google/gemini-cli-core -- src/sandbox/linux/bwrapArgsBuilder.test.ts
  2. Manual (Linux/macOS):
    • Run a sandboxed process attempting to write to a .git worktree file without explicit write permissions (should be denied).
    • Re-run the process but provide explicit write policy access to the .git directory (should succeed).

Pre-Merge Checklist

  • Updated relevant documentation and README (if needed)
  • Added/updated tests (if needed)
  • Noted breaking changes (if any)
  • Validated on required platforms/methods:
    • MacOS
      • npm run
      • npx
      • Docker
      • Podman
      • Seatbelt
    • Windows
      • npm run
      • npx
      • Docker
    • Linux
      • npm run
      • npx
      • Docker

@galz10 galz10 requested a review from a team as a code owner April 13, 2026 20:40
@gemini-code-assist
Copy link
Copy Markdown
Contributor

Summary of Changes

Hello, 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 addresses an issue where strict read-only protections for sensitive governance files and git worktrees were unconditionally overriding user-defined write permissions in sandboxed environments. By adjusting the argument generation logic for both Linux (bwrap) and macOS (Seatbelt), the system now correctly respects explicit write policies. Additionally, a minor improvement was made to the PolicyEngine to ensure that YOLO mode correctly maintains its intended behavior when encountering commands flagged as dangerous.

Highlights

  • Linux Sandbox (bwrap) Precedence: Reordered the generation of bwrap arguments to ensure explicit write permissions are applied after governance file protections, allowing them to take precedence.
  • macOS Sandbox (Seatbelt) Overrides: Updated the Seatbelt profile generator to check for explicit write permissions before applying deny rules for governance files and git worktrees.
  • Policy Engine YOLO Mode: Added a safety override in the PolicyEngine to ensure that YOLO mode preserves ALLOW decisions even when commands are flagged as dangerous by heuristics.
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.

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request modifies the policy engine and sandbox builders to allow explicit write permissions to override default protections for sensitive directories (like .git) and ensures YOLO mode preserves ALLOW decisions for dangerous commands. Critical security feedback highlights that allowing policyWrite to override governance and worktree protections creates a sandbox bypass risk, potentially leading to remote code execution via git hooks. Furthermore, the macOS Seatbelt implementation requires improvements for consistent symbolic link resolution and more robust path matching logic.

Comment thread packages/core/src/sandbox/linux/bwrapArgsBuilder.ts Outdated
Comment thread packages/core/src/sandbox/macos/seatbeltArgsBuilder.ts Outdated
Comment thread packages/core/src/sandbox/macos/seatbeltArgsBuilder.ts Outdated
Comment thread packages/core/src/sandbox/macos/seatbeltArgsBuilder.ts Outdated
@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 13, 2026

Size Change: +3.55 kB (+0.01%)

Total Size: 33.6 MB

Filename Size Change
./bundle/chunk-L455LL7A.js 0 B -3.43 MB (removed) 🏆
./bundle/chunk-NP635TS7.js 0 B -3.8 kB (removed) 🏆
./bundle/chunk-SIHSECUW.js 0 B -14.5 MB (removed) 🏆
./bundle/core-IHSY2JWR.js 0 B -46.7 kB (removed) 🏆
./bundle/devtoolsService-W5UW2YUH.js 0 B -28.4 kB (removed) 🏆
./bundle/gemini-YWUUBXJE.js 0 B -553 kB (removed) 🏆
./bundle/interactiveCli-ULYYZCLS.js 0 B -1.29 MB (removed) 🏆
./bundle/oauth2-provider-4VAOQ5CV.js 0 B -9.16 kB (removed) 🏆
./bundle/chunk-KI6IEWSS.js 3.43 MB +3.43 MB (new file) 🆕
./bundle/chunk-LNZJ52VM.js 14.5 MB +14.5 MB (new file) 🆕
./bundle/chunk-SHM2KOKM.js 3.8 kB +3.8 kB (new file) 🆕
./bundle/core-XBLYYXT5.js 46.7 kB +46.7 kB (new file) 🆕
./bundle/devtoolsService-UH3OWAPU.js 28.4 kB +28.4 kB (new file) 🆕
./bundle/gemini-MH4KAPSJ.js 553 kB +553 kB (new file) 🆕
./bundle/interactiveCli-XCPZLTIS.js 1.29 MB +1.29 MB (new file) 🆕
./bundle/oauth2-provider-JJ5EKNAM.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-MJYPYSCS.js 1.97 MB 0 B
./bundle/chunk-RJTRUG2J.js 39.8 kB 0 B
./bundle/cleanup-GFFSSJTB.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/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-5ZT36IJ2.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/tree-sitter-7U6MW5PS.js 274 kB 0 B
./bundle/tree-sitter-bash-34ZGLXVX.js 1.84 MB 0 B
./bundle/cleanup-7S2SKLUK.js 932 B +932 B (new file) 🆕

compressed-size-action

@gemini-cli gemini-cli bot added the priority/p1 Important and should be addressed in the near term. label Apr 13, 2026
@ehedlund
Copy link
Copy Markdown
Contributor

Can you add unit tests as well as coverage in sandboxManager.integration.test.ts?

Comment thread packages/core/src/sandbox/macos/seatbeltArgsBuilder.ts Outdated
Comment thread packages/core/src/sandbox/macos/seatbeltArgsBuilder.ts
@galz10 galz10 requested a review from ehedlund April 15, 2026 21:28
galz10 and others added 6 commits April 16, 2026 17:01
…nance files

Refactored bubblewrap argument generation in LinuxSandboxManager to improve reliability and security. Specifically:
- Added automatic write access to .git when running git commands in a writable workspace.
- Allowed implicit write access to .gitignore and .geminiignore when the workspace is writable.
- Improved mount ordering by destination path length to prevent hierarchical masking issues.
- Added integration tests to verify write access for governance and git files.
Refactored `LinuxSandboxManager` to use `shell-utils` for more reliable
command root identification, ensuring `.git` write permissions are
correctly applied even for complex shell-wrapped commands. Updated
integration tests to verify this behavior.
…nance files

Refactored macOS sandbox to improve write access management for git and governance files, aligning with the Linux implementation. Specifically:
- Implemented automatic .git write permission in MacOsSandboxManager when git commands are detected in a writable workspace.
- Updated seatbeltArgsBuilder to implicitly allow write access to .gitignore and .geminiignore when the workspace is writable.
- Utilized shell-utils for robust git command detection.
…h escaping

- Implemented automatic .git write access in WindowsSandboxManager for git commands in writable workspaces.
- Fixed backslash escaping issues in sandbox integration tests on Windows by using double quotes for paths in sh -c commands.
- Updated Windows command detection to use shell-utils for consistency with Linux and macOS.
@galz10 galz10 enabled auto-merge April 16, 2026 20:26
@galz10 galz10 added this pull request to the merge queue Apr 16, 2026
Merged via the queue into main with commit fe89042 Apr 16, 2026
27 checks passed
@galz10 galz10 deleted the galzahavi/fix/git-commit branch April 16, 2026 21:30
@wissamblue69-dotcom
Copy link
Copy Markdown

Summary

This PR fixes an issue where strict read-only protections for governance files (like .git directories) overrode explicit write permissions. By updating the sandbox arguments builder for both macOS (Seatbelt) and Linux (bwrap), explicit write allowances now correctly take precedence, allowing users to modify these protected paths when intentionally configured.

Details

Previously, git worktree read-only rules or other governance file denials were being enforced unconditionally, conflicting with any explicit policyWrite bindings set by the user or system configuration.

  • Linux (bwrap): Reordered the generation of --bind-try for explicit write paths so they occur after the --ro-bind-try configurations for governance files and git worktrees. This allows bwrap to prioritize the later explicit write rules.
  • macOS (Seatbelt): Added checks before generating (deny file-write* ...) rules for governance files and git worktrees. If the path or a parent path is explicitly present in resolvedPaths.policyWrite, the deny rule is skipped for that path.
  • Updated relevant test cases to verify the precedence.

Related Issues

How to Validate

  1. Unit Tests: Run the updated Linux sandbox tests (if on Linux) to verify the bwrap args order.
    npm run test -w @google/gemini-cli-core -- src/sandbox/linux/bwrapArgsBuilder.test.ts
  2. Manual (Linux/macOS):
    • Run a sandboxed process attempting to write to a .git worktree file without explicit write permissions (should be denied).
    • Re-run the process but provide explicit write policy access to the .git directory (should succeed).

Pre-Merge Checklist

  • Updated relevant documentation and README (if needed)
  • Added/updated tests (if needed)
  • Noted breaking changes (if any)
  • Validated on required platforms/methods:
    • MacOS
      • npm run
      • npx
      • Docker
      • Podman
      • Seatbelt
    • Windows
      • npm run
      • npx
      • Docker
    • Linux
      • npm run
      • npx
      • Docker

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

priority/p1 Important and should be addressed in the near term.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants