feat(sandbox): dynamic Linux sandbox expansion and worktree support (#23692)

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

packages/core/src/sandbox/macos/MacOsSandboxManager.test.ts

@@ -38,7 +38,7 @@ describe('MacOsSandboxManager', () => {
     manager = new MacOsSandboxManager({ workspace: mockWorkspace });
 
     // Mock the seatbelt args builder to isolate manager tests
-    vi.spyOn(seatbeltArgsBuilder, 'buildSeatbeltArgs').mockResolvedValue([
+    vi.spyOn(seatbeltArgsBuilder, 'buildSeatbeltArgs').mockReturnValue([
       '-p',
       '(mock profile)',
       '-D',

packages/core/src/sandbox/macos/MacOsSandboxManager.ts

@@ -24,8 +24,9 @@ import {
   isKnownSafeCommand,
   isDangerousCommand,
   isStrictlyApproved,
-} from './commandSafety.js';
+} from '../utils/commandSafety.js';
 import { type SandboxPolicyManager } from '../../policy/sandboxPolicyManager.js';
+import { verifySandboxOverrides } from '../utils/commandUtils.js';
 
 export interface MacOsSandboxOptions extends GlobalSandboxOptions {
   /** The current sandbox mode behavior from config. */
@@ -70,17 +71,7 @@ export class MacOsSandboxManager implements SandboxManager {
     const allowOverrides = this.options.modeConfig?.allowOverrides ?? true;
 
     // Reject override attempts in plan mode
-    if (!allowOverrides && req.policy?.additionalPermissions) {
-      const perms = req.policy.additionalPermissions;
-      if (
-        perms.network ||
-        (perms.fileSystem?.write && perms.fileSystem.write.length > 0)
-      ) {
-        throw new Error(
-          'Sandbox request rejected: Cannot override readonly/network restrictions in Plan mode.',
-        );
-      }
-    }
+    verifySandboxOverrides(allowOverrides, req.policy);
 
     // If not in readonly mode OR it's a strictly approved pipeline, allow workspace writes
     const isApproved = allowOverrides
@@ -120,7 +111,7 @@ export class MacOsSandboxManager implements SandboxManager {
         false,
     };
 
-    const sandboxArgs = await buildSeatbeltArgs({
+    const sandboxArgs = buildSeatbeltArgs({
       workspace: this.options.workspace,
       allowedPaths: [...(req.policy?.allowedPaths || [])],
       forbiddenPaths: req.policy?.forbiddenPaths,

packages/core/src/sandbox/macos/commandSafety.ts

@@ -1,513 +0,0 @@
-/**
- * @license
- * Copyright 2026 Google LLC
- * SPDX-License-Identifier: Apache-2.0
- */
-import { parse as shellParse } from 'shell-quote';
-import {
-  extractStringFromParseEntry,
-  initializeShellParsers,
-  splitCommands,
-  stripShellWrapper,
-} from '../../utils/shell-utils.js';
-
-/**
- * Determines if a command is strictly approved for execution on macOS.
- * A command is approved if it's composed entirely of tools explicitly listed in `approvedTools`
- * OR if it's composed of known safe, read-only POSIX commands.
- *
- * @param command - The full command string to execute.
- * @param args - The arguments for the command.
- * @param approvedTools - A list of explicitly approved tool names (e.g., ['npm', 'git']).
- * @returns true if the command is strictly approved, false otherwise.
- */
-export async function isStrictlyApproved(
-  command: string,
-  args: string[],
-  approvedTools?: string[],
-): Promise<boolean> {
-  const tools = approvedTools ?? [];
-
-  await initializeShellParsers();
-
-  const fullCmd = [command, ...args].join(' ');
-  const stripped = stripShellWrapper(fullCmd);
-
-  const pipelineCommands = splitCommands(stripped);
-
-  // Fallback for simple commands or parsing failures
-  if (pipelineCommands.length === 0) {
-    // For simple commands, we check the root command.
-    // If it's explicitly approved OR it's a known safe POSIX command, we allow it.
-    return tools.includes(command) || isKnownSafeCommand([command, ...args]);
-  }
-
-  // Check every segment of the pipeline
-  return pipelineCommands.every((cmdString) => {
-    const trimmed = cmdString.trim();
-    if (!trimmed) return true;
-
-    const parsedArgs = shellParse(trimmed).map(extractStringFromParseEntry);
-    if (parsedArgs.length === 0) return true;
-
-    const root = parsedArgs[0];
-    // The segment is approved if the root tool is in the allowlist OR if the whole segment is safe.
-    return tools.includes(root) || isKnownSafeCommand(parsedArgs);
-  });
-}
-
-/**
- * Checks if a command with its arguments is known to be safe to execute
- * without requiring user confirmation. This is primarily used to allow
- * harmless, read-only commands to run silently in the macOS sandbox.
- *
- * It handles raw command execution as well as wrapped commands like `bash -c "..."` or `bash -lc "..."`.
- * For wrapped commands, it parses the script and ensures all individual
- * sub-commands are in the known-safe list and no dangerous shell operators
- * (like subshells or redirection) are used.
- *
- * @param args - The command and its arguments (e.g., ['ls', '-la'])
- * @returns true if the command is considered safe, false otherwise.
- */
-export function isKnownSafeCommand(args: string[]): boolean {
-  if (!args || args.length === 0) {
-    return false;
-  }
-
-  // Normalize zsh to bash
-  const normalizedArgs = args.map((a) => (a === 'zsh' ? 'bash' : a));
-
-  if (isSafeToCallWithExec(normalizedArgs)) {
-    return true;
-  }
-
-  // Support `bash -lc "..."`
-  if (
-    normalizedArgs.length === 3 &&
-    normalizedArgs[0] === 'bash' &&
-    (normalizedArgs[1] === '-lc' || normalizedArgs[1] === '-c')
-  ) {
-    try {
-      const script = normalizedArgs[2];
-
-      // Basic check for dangerous operators that could spawn subshells or redirect output
-      // We allow &&, ||, |, ; but explicitly block subshells () and redirection >, >>, <
-      if (/[()<>]/g.test(script)) {
-        return false;
-      }
-
-      const commands = splitCommands(script);
-      if (commands.length === 0) return false;
-
-      return commands.every((cmd) => {
-        const trimmed = cmd.trim();
-        if (!trimmed) return true;
-
-        const parsed = shellParse(trimmed).map(extractStringFromParseEntry);
-        if (parsed.length === 0) return true;
-
-        return isSafeToCallWithExec(parsed);
-      });
-    } catch {
-      return false;
-    }
-  }
-
-  return false;
-}
-
-/**
- * Core validation logic that checks a single command and its arguments
- * against an allowlist of known safe operations. It performs deep validation
- * for specific tools like `base64`, `find`, `rg`, `git`, and `sed` to ensure
- * unsafe flags (like `--output`, `-exec`, or mutating options) are not used.
- *
- * @param args - The command and its arguments.
- * @returns true if the command is strictly read-only and safe.
- */
-function isSafeToCallWithExec(args: string[]): boolean {
-  if (!args || args.length === 0) return false;
-  const cmd = args[0];
-
-  const safeCommands = new Set([
-    'cat',
-    'cd',
-    'cut',
-    'echo',
-    'expr',
-    'false',
-    'grep',
-    'head',
-    'id',
-    'ls',
-    'nl',
-    'paste',
-    'pwd',
-    'rev',
-    'seq',
-    'stat',
-    'tail',
-    'tr',
-    'true',
-    'uname',
-    'uniq',
-    'wc',
-    'which',
-    'whoami',
-    'numfmt',
-    'tac',
-  ]);
-
-  if (safeCommands.has(cmd)) {
-    return true;
-  }
-
-  if (cmd === 'base64') {
-    const unsafeOptions = new Set(['-o', '--output']);
-    return !args
-      .slice(1)
-      .some(
-        (arg) =>
-          unsafeOptions.has(arg) ||
-          arg.startsWith('--output=') ||
-          (arg.startsWith('-o') && arg !== '-o'),
-      );
-  }
-
-  if (cmd === 'find') {
-    const unsafeOptions = new Set([
-      '-exec',
-      '-execdir',
-      '-ok',
-      '-okdir',
-      '-delete',
-      '-fls',
-      '-fprint',
-      '-fprint0',
-      '-fprintf',
-    ]);
-    return !args.some((arg) => unsafeOptions.has(arg));
-  }
-
-  if (cmd === 'rg') {
-    const unsafeWithArgs = new Set(['--pre', '--hostname-bin']);
-    const unsafeWithoutArgs = new Set(['--search-zip', '-z']);
-
-    return !args.some((arg) => {
-      if (unsafeWithoutArgs.has(arg)) return true;
-      for (const opt of unsafeWithArgs) {
-        if (arg === opt || arg.startsWith(opt + '=')) return true;
-      }
-      return false;
-    });
-  }
-
-  if (cmd === 'git') {
-    if (gitHasConfigOverrideGlobalOption(args)) {
-      return false;
-    }
-
-    const { idx, subcommand } = findGitSubcommand(args, [
-      'status',
-      'log',
-      'diff',
-      'show',
-      'branch',
-    ]);
-    if (!subcommand) {
-      return false;
-    }
-
-    const subcommandArgs = args.slice(idx + 1);
-
-    if (['status', 'log', 'diff', 'show'].includes(subcommand)) {
-      return gitSubcommandArgsAreReadOnly(subcommandArgs);
-    }
-
-    if (subcommand === 'branch') {
-      return (
-        gitSubcommandArgsAreReadOnly(subcommandArgs) &&
-        gitBranchIsReadOnly(subcommandArgs)
-      );
-    }
-
-    return false;
-  }
-
-  if (cmd === 'sed') {
-    // Special-case sed -n {N|M,N}p
-    if (args.length <= 4 && args[1] === '-n' && isValidSedNArg(args[2])) {
-      return true;
-    }
-    return false;
-  }
-
-  return false;
-}
-
-/**
- * Helper to identify which git subcommand is being executed, skipping over
- * global git options like `-c` or `--git-dir`.
- *
- * @param args - The full git command arguments.
- * @param subcommands - A list of subcommands to look for.
- * @returns An object containing the index of the subcommand and its name.
- */
-function findGitSubcommand(
-  args: string[],
-  subcommands: string[],
-): { idx: number; subcommand: string | null } {
-  let skipNext = false;
-
-  for (let idx = 1; idx < args.length; idx++) {
-    if (skipNext) {
-      skipNext = false;
-      continue;
-    }
-
-    const arg = args[idx];
-
-    if (
-      arg.startsWith('--config-env=') ||
-      arg.startsWith('--exec-path=') ||
-      arg.startsWith('--git-dir=') ||
-      arg.startsWith('--namespace=') ||
-      arg.startsWith('--super-prefix=') ||
-      arg.startsWith('--work-tree=') ||
-      ((arg.startsWith('-C') || arg.startsWith('-c')) && arg.length > 2)
-    ) {
-      continue;
-    }
-
-    if (
-      arg === '-C' ||
-      arg === '-c' ||
-      arg === '--config-env' ||
-      arg === '--exec-path' ||
-      arg === '--git-dir' ||
-      arg === '--namespace' ||
-      arg === '--super-prefix' ||
-      arg === '--work-tree'
-    ) {
-      skipNext = true;
-      continue;
-    }
-
-    if (arg === '--' || arg.startsWith('-')) {
-      continue;
-    }
-
-    if (subcommands.includes(arg)) {
-      return { idx, subcommand: arg };
-    }
-
-    return { idx: -1, subcommand: null };
-  }
-
-  return { idx: -1, subcommand: null };
-}
-
-/**
- * Checks if a git command contains global configuration override flags
- * (e.g., `-c` or `--config-env`) which could be used maliciously to
- * execute arbitrary code via git config.
- *
- * @param args - The git command arguments.
- * @returns true if config overrides are present.
- */
-function gitHasConfigOverrideGlobalOption(args: string[]): boolean {
-  return args.some(
-    (arg) =>
-      arg === '-c' ||
-      arg === '--config-env' ||
-      (arg.startsWith('-c') && arg.length > 2) ||
-      arg.startsWith('--config-env='),
-  );
-}
-
-/**
- * Validates that the arguments for safe git subcommands (like `status`, `log`,
- * `diff`, `show`) do not contain flags that could cause mutations or execute
- * arbitrary commands (e.g., `--output`, `--exec`).
- *
- * @param args - Arguments passed to the git subcommand.
- * @returns true if the arguments only represent read-only operations.
- */
-function gitSubcommandArgsAreReadOnly(args: string[]): boolean {
-  const unsafeFlags = new Set([
-    '--output',
-    '--ext-diff',
-    '--textconv',
-    '--exec',
-    '--paginate',
-  ]);
-
-  return !args.some(
-    (arg) =>
-      unsafeFlags.has(arg) ||
-      arg.startsWith('--output=') ||
-      arg.startsWith('--exec='),
-  );
-}
-
-/**
- * Validates that `git branch` is only used for read operations
- * (e.g., listing branches) rather than creating, deleting, or renaming branches.
- *
- * @param args - Arguments passed to `git branch`.
- * @returns true if it's purely a listing/read-only branch command.
- */
-function gitBranchIsReadOnly(args: string[]): boolean {
-  if (args.length === 0) return true;
-
-  let sawReadOnlyFlag = false;
-  for (const arg of args) {
-    if (
-      [
-        '--list',
-        '-l',
-        '--show-current',
-        '-a',
-        '--all',
-        '-r',
-        '--remotes',
-        '-v',
-        '-vv',
-        '--verbose',
-      ].includes(arg)
-    ) {
-      sawReadOnlyFlag = true;
-    } else if (arg.startsWith('--format=')) {
-      sawReadOnlyFlag = true;
-    } else {
-      return false;
-    }
-  }
-  return sawReadOnlyFlag;
-}
-
-/**
- * Ensures that a `sed` command argument is a valid line-printing instruction
- * (e.g., `10p` or `5,10p`), preventing unsafe script execution in `sed`.
- *
- * @param arg - The script argument passed to `sed -n`.
- * @returns true if it's a valid, safe print command.
- */
-function isValidSedNArg(arg: string | undefined): boolean {
-  if (!arg) return false;
-
-  if (!arg.endsWith('p')) return false;
-  const core = arg.slice(0, -1);
-
-  const parts = core.split(',');
-  if (parts.length === 1) {
-    const num = parts[0];
-    return num.length > 0 && /^\d+$/.test(num);
-  } else if (parts.length === 2) {
-    const a = parts[0];
-    const b = parts[1];
-    return a.length > 0 && b.length > 0 && /^\d+$/.test(a) && /^\d+$/.test(b);
-  }
-
-  return false;
-}
-
-/**
- * Checks if a command with its arguments is explicitly known to be dangerous
- * and should be blocked or require strict user confirmation. This catches
- * destructive commands like `rm -rf`, `sudo`, and commands with execution
- * flags like `find -exec`.
- *
- * @param args - The command and its arguments.
- * @returns true if the command is identified as dangerous, false otherwise.
- */
-export function isDangerousCommand(args: string[]): boolean {
-  if (!args || args.length === 0) {
-    return false;
-  }
-
-  const cmd = args[0];
-
-  if (cmd === 'rm') {
-    return args[1] === '-f' || args[1] === '-rf' || args[1] === '-fr';
-  }
-
-  if (cmd === 'sudo') {
-    return isDangerousCommand(args.slice(1));
-  }
-
-  if (cmd === 'find') {
-    const unsafeOptions = new Set([
-      '-exec',
-      '-execdir',
-      '-ok',
-      '-okdir',
-      '-delete',
-      '-fls',
-      '-fprint',
-      '-fprint0',
-      '-fprintf',
-    ]);
-    return args.some((arg) => unsafeOptions.has(arg));
-  }
-
-  if (cmd === 'rg') {
-    const unsafeWithArgs = new Set(['--pre', '--hostname-bin']);
-    const unsafeWithoutArgs = new Set(['--search-zip', '-z']);
-
-    return args.some((arg) => {
-      if (unsafeWithoutArgs.has(arg)) return true;
-      for (const opt of unsafeWithArgs) {
-        if (arg === opt || arg.startsWith(opt + '=')) return true;
-      }
-      return false;
-    });
-  }
-
-  if (cmd === 'git') {
-    if (gitHasConfigOverrideGlobalOption(args)) {
-      return true;
-    }
-
-    const { idx, subcommand } = findGitSubcommand(args, [
-      'status',
-      'log',
-      'diff',
-      'show',
-      'branch',
-    ]);
-    if (!subcommand) {
-      // It's a git command we don't recognize as explicitly safe.
-      return false;
-    }
-
-    const subcommandArgs = args.slice(idx + 1);
-
-    if (['status', 'log', 'diff', 'show'].includes(subcommand)) {
-      return !gitSubcommandArgsAreReadOnly(subcommandArgs);
-    }
-
-    if (subcommand === 'branch') {
-      return !(
-        gitSubcommandArgsAreReadOnly(subcommandArgs) &&
-        gitBranchIsReadOnly(subcommandArgs)
-      );
-    }
-
-    return false;
-  }
-
-  if (cmd === 'base64') {
-    const unsafeOptions = new Set(['-o', '--output']);
-    return args
-      .slice(1)
-      .some(
-        (arg) =>
-          unsafeOptions.has(arg) ||
-          arg.startsWith('--output=') ||
-          (arg.startsWith('-o') && arg !== '-o'),
-      );
-  }
-
-  return false;
-}

packages/core/src/sandbox/macos/seatbeltArgsBuilder.test.ts

@@ -3,25 +3,31 @@
  * Copyright 2026 Google LLC
  * SPDX-License-Identifier: Apache-2.0
  */
-import { describe, it, expect, vi, beforeEach } from 'vitest';
+import { describe, it, expect, vi, afterEach } from 'vitest';
 import { buildSeatbeltArgs } from './seatbeltArgsBuilder.js';
-import * as sandboxManager from '../../services/sandboxManager.js';
+import * as fsUtils from '../utils/fsUtils.js';
 import fs from 'node:fs';
 import os from 'node:os';
 
+vi.mock('../utils/fsUtils.js', async () => {
+  const actual = await vi.importActual('../utils/fsUtils.js');
+  return {
+    ...actual,
+    tryRealpath: vi.fn((p) => p),
+    resolveGitWorktreePaths: vi.fn(() => ({})),
+  };
+});
+
 describe('seatbeltArgsBuilder', () => {
-  beforeEach(() => {
+  afterEach(() => {
     vi.restoreAllMocks();
   });
 
   describe('buildSeatbeltArgs', () => {
-    it('should build a strict allowlist profile allowing the workspace via param', async () => {
-      // Mock tryRealpath to just return the path for testing
-      vi.spyOn(sandboxManager, 'tryRealpath').mockImplementation(
-        async (p) => p,
-      );
+    it('should build a strict allowlist profile allowing the workspace via param', () => {
+      vi.mocked(fsUtils.tryRealpath).mockImplementation((p) => p);
 
-      const args = await buildSeatbeltArgs({
+      const args = buildSeatbeltArgs({
         workspace: '/Users/test/workspace',
       });
 
@@ -38,11 +44,9 @@ describe('seatbeltArgsBuilder', () => {
       expect(args).toContain(`TMPDIR=${os.tmpdir()}`);
     });
 
-    it('should allow network when networkAccess is true', async () => {
-      vi.spyOn(sandboxManager, 'tryRealpath').mockImplementation(
-        async (p) => p,
-      );
-      const args = await buildSeatbeltArgs({
+    it('should allow network when networkAccess is true', () => {
+      vi.mocked(fsUtils.tryRealpath).mockImplementation((p) => p);
+      const args = buildSeatbeltArgs({
         workspace: '/test',
         networkAccess: true,
       });
@@ -51,10 +55,8 @@ describe('seatbeltArgsBuilder', () => {
     });
 
     describe('governance files', () => {
-      it('should inject explicit deny rules for governance files', async () => {
-        vi.spyOn(sandboxManager, 'tryRealpath').mockImplementation(async (p) =>
-          p.toString(),
-        );
+      it('should inject explicit deny rules for governance files', () => {
+        vi.mocked(fsUtils.tryRealpath).mockImplementation((p) => p.toString());
         vi.spyOn(fs, 'existsSync').mockReturnValue(true);
         vi.spyOn(fs, 'lstatSync').mockImplementation(
           (p) =>
@@ -64,35 +66,29 @@ describe('seatbeltArgsBuilder', () => {
             }) as unknown as fs.Stats,
         );
 
-        const args = await buildSeatbeltArgs({
-          workspace: '/Users/test/workspace',
+        const args = buildSeatbeltArgs({
+          workspace: '/test/workspace',
         });
         const profile = args[1];
 
-        // .gitignore should be a literal deny
         expect(args).toContain('-D');
-        expect(args).toContain(
-          'GOVERNANCE_FILE_0=/Users/test/workspace/.gitignore',
-        );
+        expect(args).toContain('GOVERNANCE_FILE_0=/test/workspace/.gitignore');
         expect(profile).toContain(
           '(deny file-write* (literal (param "GOVERNANCE_FILE_0")))',
         );
 
-        // .git should be a subpath deny
-        expect(args).toContain('GOVERNANCE_FILE_2=/Users/test/workspace/.git');
+        expect(args).toContain('GOVERNANCE_FILE_2=/test/workspace/.git');
         expect(profile).toContain(
           '(deny file-write* (subpath (param "GOVERNANCE_FILE_2")))',
         );
       });
 
-      it('should protect both the symlink and the real path if they differ', async () => {
-        vi.spyOn(sandboxManager, 'tryRealpath').mockImplementation(
-          async (p) => {
-            if (p === '/test/workspace/.gitignore')
-              return '/test/real/.gitignore';
-            return p.toString();
-          },
-        );
+      it('should protect both the symlink and the real path if they differ', () => {
+        vi.mocked(fsUtils.tryRealpath).mockImplementation((p) => {
+          if (p === '/test/workspace/.gitignore')
+            return '/test/real/.gitignore';
+          return p.toString();
+        });
         vi.spyOn(fs, 'existsSync').mockReturnValue(true);
         vi.spyOn(fs, 'lstatSync').mockImplementation(
           () =>
@@ -102,7 +98,7 @@ describe('seatbeltArgsBuilder', () => {
             }) as unknown as fs.Stats,
         );
 
-        const args = await buildSeatbeltArgs({ workspace: '/test/workspace' });
+        const args = buildSeatbeltArgs({ workspace: '/test/workspace' });
         const profile = args[1];
 
         expect(args).toContain('GOVERNANCE_FILE_0=/test/workspace/.gitignore');
@@ -117,15 +113,13 @@ describe('seatbeltArgsBuilder', () => {
     });
 
     describe('allowedPaths', () => {
-      it('should parameterize allowed paths and normalize them', async () => {
-        vi.spyOn(sandboxManager, 'tryRealpath').mockImplementation(
-          async (p) => {
-            if (p === '/test/symlink') return '/test/real_path';
-            return p;
-          },
-        );
+      it('should parameterize allowed paths and normalize them', () => {
+        vi.mocked(fsUtils.tryRealpath).mockImplementation((p) => {
+          if (p === '/test/symlink') return '/test/real_path';
+          return p;
+        });
 
-        const args = await buildSeatbeltArgs({
+        const args = buildSeatbeltArgs({
           workspace: '/test',
           allowedPaths: ['/custom/path1', '/test/symlink'],
         });
@@ -141,12 +135,10 @@ describe('seatbeltArgsBuilder', () => {
     });
 
     describe('forbiddenPaths', () => {
-      it('should parameterize forbidden paths and explicitly deny them', async () => {
-        vi.spyOn(sandboxManager, 'tryRealpath').mockImplementation(
-          async (p) => p,
-        );
+      it('should parameterize forbidden paths and explicitly deny them', () => {
+        vi.mocked(fsUtils.tryRealpath).mockImplementation((p) => p);
 
-        const args = await buildSeatbeltArgs({
+        const args = buildSeatbeltArgs({
           workspace: '/test',
           forbiddenPaths: ['/secret/path'],
         });
@@ -161,22 +153,21 @@ describe('seatbeltArgsBuilder', () => {
         );
       });
 
-      it('resolves forbidden symlink paths to their real paths', async () => {
-        vi.spyOn(sandboxManager, 'tryRealpath').mockImplementation(
-          async (p) => {
-            if (p === '/test/symlink') return '/test/real_path';
-            return p;
-          },
-        );
+      it('resolves forbidden symlink paths to their real paths', () => {
+        vi.mocked(fsUtils.tryRealpath).mockImplementation((p) => {
+          if (p === '/test/symlink' || p === '/test/missing-dir') {
+            return '/test/real_path';
+          }
+          return p;
+        });
 
-        const args = await buildSeatbeltArgs({
+        const args = buildSeatbeltArgs({
           workspace: '/test',
           forbiddenPaths: ['/test/symlink'],
         });
 
         const profile = args[1];
 
-        // The builder should resolve the symlink and explicitly deny the real target path
         expect(args).toContain('-D');
         expect(args).toContain('FORBIDDEN_PATH_0=/test/real_path');
         expect(profile).toContain(
@@ -184,12 +175,10 @@ describe('seatbeltArgsBuilder', () => {
         );
       });
 
-      it('explicitly denies non-existent forbidden paths to prevent creation', async () => {
-        vi.spyOn(sandboxManager, 'tryRealpath').mockImplementation(
-          async (p) => p,
-        );
+      it('explicitly denies non-existent forbidden paths to prevent creation', () => {
+        vi.mocked(fsUtils.tryRealpath).mockImplementation((p) => p);
 
-        const args = await buildSeatbeltArgs({
+        const args = buildSeatbeltArgs({
           workspace: '/test',
           forbiddenPaths: ['/test/missing-dir/missing-file.txt'],
         });
@@ -205,12 +194,10 @@ describe('seatbeltArgsBuilder', () => {
         );
       });
 
-      it('should override allowed paths if a path is also in forbidden paths', async () => {
-        vi.spyOn(sandboxManager, 'tryRealpath').mockImplementation(
-          async (p) => p,
-        );
+      it('should override allowed paths if a path is also in forbidden paths', () => {
+        vi.mocked(fsUtils.tryRealpath).mockImplementation((p) => p);
 
-        const args = await buildSeatbeltArgs({
+        const args = buildSeatbeltArgs({
           workspace: '/test',
           allowedPaths: ['/custom/path1'],
           forbiddenPaths: ['/custom/path1'],
@@ -226,8 +213,6 @@ describe('seatbeltArgsBuilder', () => {
         expect(profile).toContain(allowString);
         expect(profile).toContain(denyString);
 
-        // Verify ordering: The explicit deny must appear AFTER the explicit allow in the profile string
-        // Seatbelt rules are evaluated in order where the latest rule matching a path wins
         const allowIndex = profile.indexOf(allowString);
         const denyIndex = profile.indexOf(denyString);
         expect(denyIndex).toBeGreaterThan(allowIndex);

packages/core/src/sandbox/macos/seatbeltArgsBuilder.ts

@@ -15,8 +15,8 @@ import {
   type SandboxPermissions,
   sanitizePaths,
   GOVERNANCE_FILES,
-  tryRealpath,
 } from '../../services/sandboxManager.js';
+import { tryRealpath, resolveGitWorktreePaths } from '../utils/fsUtils.js';
 
 /**
  * Options for building macOS Seatbelt arguments.
@@ -44,13 +44,11 @@ export interface SeatbeltArgsOptions {
  * Returns arguments up to the end of sandbox-exec configuration (e.g. ['-p', '<profile>', '-D', ...])
  * Does not include the final '--' separator or the command to run.
  */
-export async function buildSeatbeltArgs(
-  options: SeatbeltArgsOptions,
-): Promise<string[]> {
+export function buildSeatbeltArgs(options: SeatbeltArgsOptions): string[] {
   let profile = BASE_SEATBELT_PROFILE + '\n';
   const args: string[] = [];
 
-  const workspacePath = await tryRealpath(options.workspace);
+  const workspacePath = tryRealpath(options.workspace);
   args.push('-D', `WORKSPACE=${workspacePath}`);
   args.push('-D', `WORKSPACE_RAW=${options.workspace}`);
   profile += `(allow file-read* (subpath (param "WORKSPACE_RAW")))\n`;
@@ -67,7 +65,7 @@ export async function buildSeatbeltArgs(
   // (Seatbelt evaluates rules in order, later rules win for same path).
   for (let i = 0; i < GOVERNANCE_FILES.length; i++) {
     const governanceFile = path.join(workspacePath, GOVERNANCE_FILES[i].path);
-    const realGovernanceFile = await tryRealpath(governanceFile);
+    const realGovernanceFile = tryRealpath(governanceFile);
 
     // Determine if it should be treated as a directory (subpath) or a file (literal).
     // .git is generally a directory, while ignore files are literals.
@@ -92,42 +90,20 @@ export async function buildSeatbeltArgs(
   }
 
   // Auto-detect and support git worktrees by granting read and write access to the underlying git directory
-  try {
-    const gitPath = path.join(workspacePath, '.git');
-    const gitStat = fs.lstatSync(gitPath);
-    if (gitStat.isFile()) {
-      const gitContent = fs.readFileSync(gitPath, 'utf8');
-      const match = gitContent.match(/^gitdir:\s*(.+)$/m);
-      if (match && match[1]) {
-        let worktreeGitDir = match[1].trim();
-        if (!path.isAbsolute(worktreeGitDir)) {
-          worktreeGitDir = path.resolve(workspacePath, worktreeGitDir);
-        }
-        const resolvedWorktreeGitDir = await tryRealpath(worktreeGitDir);
-
-        // Grant write access to the worktree's specific .git directory
-        args.push('-D', `WORKTREE_GIT_DIR=${resolvedWorktreeGitDir}`);
-        profile += `(allow file-read* file-write* (subpath (param "WORKTREE_GIT_DIR")))\n`;
-
-        // Grant write access to the main repository's .git directory (objects, refs, etc. are shared)
-        // resolvedWorktreeGitDir is usually like: /path/to/main-repo/.git/worktrees/worktree-name
-        const mainGitDir = await tryRealpath(
-          path.dirname(path.dirname(resolvedWorktreeGitDir)),
-        );
-        if (mainGitDir && mainGitDir.endsWith('.git')) {
-          args.push('-D', `MAIN_GIT_DIR=${mainGitDir}`);
-          profile += `(allow file-read* file-write* (subpath (param "MAIN_GIT_DIR")))\n`;
-        }
-      }
-    }
-  } catch (_e) {
-    // Ignore if .git doesn't exist, isn't readable, etc.
+  const { worktreeGitDir, mainGitDir } = resolveGitWorktreePaths(workspacePath);
+  if (worktreeGitDir) {
+    args.push('-D', `WORKTREE_GIT_DIR=${worktreeGitDir}`);
+    profile += `(allow file-read* file-write* (subpath (param "WORKTREE_GIT_DIR")))\n`;
+  }
+  if (mainGitDir) {
+    args.push('-D', `MAIN_GIT_DIR=${mainGitDir}`);
+    profile += `(allow file-read* file-write* (subpath (param "MAIN_GIT_DIR")))\n`;
   }
 
-  const tmpPath = await tryRealpath(os.tmpdir());
+  const tmpPath = tryRealpath(os.tmpdir());
   args.push('-D', `TMPDIR=${tmpPath}`);
 
-  const nodeRootPath = await tryRealpath(
+  const nodeRootPath = tryRealpath(
     path.dirname(path.dirname(process.execPath)),
   );
   args.push('-D', `NODE_ROOT=${nodeRootPath}`);
@@ -142,7 +118,7 @@ export async function buildSeatbeltArgs(
     for (const p of paths) {
       if (!p.trim()) continue;
       try {
-        let resolved = await tryRealpath(p);
+        let resolved = tryRealpath(p);
 
         // If this is a 'bin' directory (like /usr/local/bin or homebrew/bin),
         // also grant read access to its parent directory so that symlinked
@@ -165,8 +141,10 @@ export async function buildSeatbeltArgs(
 
   // Handle allowedPaths
   const allowedPaths = sanitizePaths(options.allowedPaths) || [];
+  const resolvedAllowedPaths: string[] = [];
   for (let i = 0; i < allowedPaths.length; i++) {
-    const allowedPath = await tryRealpath(allowedPaths[i]);
+    const allowedPath = tryRealpath(allowedPaths[i]);
+    resolvedAllowedPaths.push(allowedPath);
     args.push('-D', `ALLOWED_PATH_${i}=${allowedPath}`);
     profile += `(allow file-read* file-write* (subpath (param "ALLOWED_PATH_${i}")))\n`;
   }
@@ -176,7 +154,7 @@ export async function buildSeatbeltArgs(
     const { read, write } = options.additionalPermissions.fileSystem;
     if (read) {
       for (let i = 0; i < read.length; i++) {
-        const resolved = await tryRealpath(read[i]);
+        const resolved = tryRealpath(read[i]);
         const paramName = `ADDITIONAL_READ_${i}`;
         args.push('-D', `${paramName}=${resolved}`);
         let isFile = false;
@@ -194,7 +172,7 @@ export async function buildSeatbeltArgs(
     }
     if (write) {
       for (let i = 0; i < write.length; i++) {
-        const resolved = await tryRealpath(write[i]);
+        const resolved = tryRealpath(write[i]);
         const paramName = `ADDITIONAL_WRITE_${i}`;
         args.push('-D', `${paramName}=${resolved}`);
         let isFile = false;
@@ -215,7 +193,7 @@ export async function buildSeatbeltArgs(
   // Handle forbiddenPaths
   const forbiddenPaths = sanitizePaths(options.forbiddenPaths) || [];
   for (let i = 0; i < forbiddenPaths.length; i++) {
-    const forbiddenPath = await tryRealpath(forbiddenPaths[i]);
+    const forbiddenPath = tryRealpath(forbiddenPaths[i]);
     args.push('-D', `FORBIDDEN_PATH_${i}=${forbiddenPath}`);
     profile += `(deny file-read* file-write* (subpath (param "FORBIDDEN_PATH_${i}")))\n`;
   }