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

packages/core/src/sandbox/linux/LinuxSandboxManager.ts

@@ -25,7 +25,6 @@ import {
 import {
   isStrictlyApproved,
   verifySandboxOverrides,
-  getCommandName,
 } from '../utils/commandUtils.js';
 import { assertValidPathString } from '../../utils/paths.js';
 import {
@@ -40,6 +39,11 @@ import {
 import { isErrnoException } from '../utils/fsUtils.js';
 import { handleReadWriteCommands } from '../utils/sandboxReadWriteUtils.js';
 import { buildBwrapArgs } from './bwrapArgsBuilder.js';
+import {
+  getCommandRoots,
+  initializeShellParsers,
+  stripShellWrapper,
+} from '../../utils/shell-utils.js';
 
 let cachedBpfPath: string | undefined;
 
@@ -218,7 +222,15 @@ export class LinuxSandboxManager implements SandboxManager {
       args = ['-c', 'cat > "$1"', '_', ...args];
     }
 
-    const commandName = await getCommandName({ ...req, command, args });
+    await initializeShellParsers();
+    const fullCmd = [command, ...args].join(' ');
+    const stripped = stripShellWrapper(fullCmd);
+    const roots = getCommandRoots(stripped).filter(
+      (r) => r !== 'shopt' && r !== 'set',
+    );
+    const commandName = roots.length > 0 ? roots[0] : join(command);
+    const isGitCommand = roots.includes('git');
+
     const isApproved = allowOverrides
       ? await isStrictlyApproved(
           { ...req, command, args },
@@ -253,6 +265,15 @@ export class LinuxSandboxManager implements SandboxManager {
         false,
     };
 
+    // If the workspace is writable and we're running a git command,
+    // automatically allow write access to the .git directory.
+    if (workspaceWrite && isGitCommand) {
+      const gitDir = join(this.options.workspace, '.git');
+      if (!mergedAdditional.fileSystem!.write!.includes(gitDir)) {
+        mergedAdditional.fileSystem!.write!.push(gitDir);
+      }
+    }
+
     const { command: finalCommand, args: finalArgs } = handleReadWriteCommands(
       req,
       mergedAdditional,

packages/core/src/sandbox/linux/bwrapArgsBuilder.test.ts

@@ -115,14 +115,14 @@ describe.skipIf(os.platform() === 'win32')('buildBwrapArgs', () => {
       workspace,
       workspace,
       '--ro-bind',
+      `${workspace}/.git`,
+      `${workspace}/.git`,
+      '--ro-bind',
       `${workspace}/.gitignore`,
       `${workspace}/.gitignore`,
       '--ro-bind',
       `${workspace}/.geminiignore`,
       `${workspace}/.geminiignore`,
-      '--ro-bind',
-      `${workspace}/.git`,
-      `${workspace}/.git`,
     ]);
   });
 

packages/core/src/sandbox/linux/bwrapArgsBuilder.ts

@@ -14,6 +14,7 @@ import {
 import { isErrnoException } from '../utils/fsUtils.js';
 import { spawnAsync } from '../../utils/shell-utils.js';
 import { debugLogger } from '../../utils/debugLogger.js';
+import { toPathKey } from '../../utils/paths.js';
 
 /**
  * Options for building bubblewrap (bwrap) arguments.
@@ -63,58 +64,101 @@ export async function buildBwrapArgs(
     '/tmp',
   );
 
-  const bindFlag = workspaceWrite ? '--bind-try' : '--ro-bind-try';
+  type MountType =
+    | '--bind'
+    | '--ro-bind'
+    | '--bind-try'
+    | '--ro-bind-try'
+    | '--symlink';
 
-  bwrapArgs.push(bindFlag, workspace.original, workspace.original);
+  type Mount =
+    | {
+        type: MountType;
+        src: string;
+        dest: string;
+      }
+    | { type: '--tmpfs-ro'; dest: string };
+
+  const mounts: Mount[] = [];
+
+  const bindFlag: MountType = workspaceWrite ? '--bind-try' : '--ro-bind-try';
+  mounts.push({
+    type: bindFlag,
+    src: workspace.original,
+    dest: workspace.original,
+  });
   if (workspace.resolved !== workspace.original) {
-    bwrapArgs.push(bindFlag, workspace.resolved, workspace.resolved);
+    mounts.push({
+      type: bindFlag,
+      src: workspace.resolved,
+      dest: workspace.resolved,
+    });
   }
 
   for (const includeDir of resolvedPaths.globalIncludes) {
-    bwrapArgs.push('--ro-bind-try', includeDir, includeDir);
+    mounts.push({ type: '--ro-bind-try', src: includeDir, dest: includeDir });
   }
 
   for (const allowedPath of resolvedPaths.policyAllowed) {
     if (fs.existsSync(allowedPath)) {
-      bwrapArgs.push('--bind-try', allowedPath, allowedPath);
+      mounts.push({ type: '--bind-try', src: allowedPath, dest: allowedPath });
     } else {
-      // If the path doesn't exist, we still want to allow access to its parent
-      // to enable creating it.
       const parent = dirname(allowedPath);
-      bwrapArgs.push(
-        isReadOnlyCommand ? '--ro-bind-try' : '--bind-try',
-        parent,
-        parent,
-      );
+      mounts.push({
+        type: isReadOnlyCommand ? '--ro-bind-try' : '--bind-try',
+        src: parent,
+        dest: parent,
+      });
     }
   }
 
   for (const p of resolvedPaths.policyRead) {
-    bwrapArgs.push('--ro-bind-try', p, p);
+    mounts.push({ type: '--ro-bind-try', src: p, dest: p });
   }
 
+  // Collect explicit additional write permissions.
   for (const p of resolvedPaths.policyWrite) {
-    bwrapArgs.push('--bind-try', p, p);
+    mounts.push({ type: '--bind-try', src: p, dest: p });
   }
 
+  const policyWriteKeys = new Set(resolvedPaths.policyWrite.map(toPathKey));
+
   for (const file of GOVERNANCE_FILES) {
     const filePath = join(workspace.original, file.path);
     const realPath = join(workspace.resolved, file.path);
-    bwrapArgs.push('--ro-bind', filePath, filePath);
-    if (realPath !== filePath) {
-      bwrapArgs.push('--ro-bind', realPath, realPath);
+
+    const isExplicitlyWritable =
+      policyWriteKeys.has(toPathKey(filePath)) ||
+      policyWriteKeys.has(toPathKey(realPath));
+
+    // If the workspace is writable, we allow editing .gitignore and .geminiignore by default.
+    // .git remains protected unless explicitly requested (e.g. for git commands).
+    const isImplicitlyWritable = workspaceWrite && file.path !== '.git';
+
+    if (!isExplicitlyWritable && !isImplicitlyWritable) {
+      mounts.push({ type: '--ro-bind', src: filePath, dest: filePath });
+      if (realPath !== filePath) {
+        mounts.push({ type: '--ro-bind', src: realPath, dest: realPath });
+      }
     }
   }
 
-  // Grant read-only access to git worktrees/submodules. We do this last in order to
-  // ensure that these rules aren't overwritten by broader write policies.
+  // Grant read-only access to git worktrees/submodules.
   if (resolvedPaths.gitWorktree) {
     const { worktreeGitDir, mainGitDir } = resolvedPaths.gitWorktree;
-    if (worktreeGitDir) {
-      bwrapArgs.push('--ro-bind-try', worktreeGitDir, worktreeGitDir);
+    if (worktreeGitDir && !policyWriteKeys.has(toPathKey(worktreeGitDir))) {
+      mounts.push({
+        type: '--ro-bind-try',
+        src: worktreeGitDir,
+        dest: worktreeGitDir,
+      });
     }
-    if (mainGitDir) {
-      bwrapArgs.push('--ro-bind-try', mainGitDir, mainGitDir);
+    if (mainGitDir && !policyWriteKeys.has(toPathKey(mainGitDir))) {
+      mounts.push({
+        type: '--ro-bind-try',
+        src: mainGitDir,
+        dest: mainGitDir,
+      });
     }
   }
 
@@ -123,37 +167,23 @@ export async function buildBwrapArgs(
     try {
       const stat = fs.statSync(p);
       if (stat.isDirectory()) {
-        bwrapArgs.push('--tmpfs', p, '--remount-ro', p);
+        mounts.push({ type: '--tmpfs-ro', dest: p });
       } else {
-        bwrapArgs.push('--ro-bind', '/dev/null', p);
+        mounts.push({ type: '--ro-bind', src: '/dev/null', dest: p });
       }
     } catch (e: unknown) {
       if (isErrnoException(e) && e.code === 'ENOENT') {
-        bwrapArgs.push('--symlink', '/dev/null', p);
+        mounts.push({ type: '--symlink', src: '/dev/null', dest: p });
       } else {
         debugLogger.warn(
           `Failed to secure forbidden path ${p}: ${e instanceof Error ? e.message : String(e)}`,
         );
-        bwrapArgs.push('--ro-bind', '/dev/null', p);
+        mounts.push({ type: '--ro-bind', src: '/dev/null', dest: p });
       }
     }
   }
 
   // Mask secret files (.env, .env.*)
-  const secretArgs = await getSecretFilesArgs(resolvedPaths, maskFilePath);
-  bwrapArgs.push(...secretArgs);
-
-  return bwrapArgs;
-}
-
-/**
- * Generates bubblewrap arguments to mask secret files.
- */
-async function getSecretFilesArgs(
-  resolvedPaths: ResolvedSandboxPaths,
-  maskPath: string,
-): Promise<string[]> {
-  const args: string[] = [];
   const searchDirs = new Set([
     resolvedPaths.workspace.original,
     resolvedPaths.workspace.resolved,
@@ -164,9 +194,6 @@ async function getSecretFilesArgs(
 
   for (const dir of searchDirs) {
     try {
-      // Use the native 'find' command for performance and to catch nested secrets.
-      // We limit depth to 3 to keep it fast while covering common nested structures.
-      // We use -prune to skip heavy directories efficiently while matching dotfiles.
       const findResult = await spawnAsync('find', [
         dir,
         '-maxdepth',
@@ -203,7 +230,7 @@ async function getSecretFilesArgs(
       const files = findResult.stdout.toString().split('\0');
       for (const file of files) {
         if (file.trim()) {
-          args.push('--bind', maskPath, file.trim());
+          mounts.push({ type: '--bind', src: maskFilePath, dest: file.trim() });
         }
       }
     } catch (e) {
@@ -213,5 +240,19 @@ async function getSecretFilesArgs(
       );
     }
   }
-  return args;
+
+  // Sort mounts by destination path length to ensure parents are bound before children.
+  // This prevents hierarchical masking where a parent mount would hide a child mount.
+  mounts.sort((a, b) => a.dest.length - b.dest.length);
+
+  // Emit final bwrap arguments
+  for (const m of mounts) {
+    if (m.type === '--tmpfs-ro') {
+      bwrapArgs.push('--tmpfs', m.dest, '--remount-ro', m.dest);
+    } else {
+      bwrapArgs.push(m.type, m.src, m.dest);
+    }
+  }
+
+  return bwrapArgs;
 }