fix: Sandbox policy tuning for tmp write access (#145)

* fix: Sandbox policy tuning for tmp write access

* fix: Remove numbers from test

* Update sandbox/profiles/pnpm-restrictive.yml

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Abhisek Datta <abhisek.datta@gmail.com>

* fix: Sandbox E2E test to consider Linux bubblewrap tmpfs mount

* Update sandbox/profiles/pnpm-restrictive.yml

Co-authored-by: Sahil Bansal <bansalsahil315@gmail.com>
Signed-off-by: Abhisek Datta <abhisek.datta@gmail.com>

* fix: Migrate deny rules from pnpm to npm policy

---------

Signed-off-by: Abhisek Datta <abhisek.datta@gmail.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Sahil Bansal <bansalsahil315@gmail.com>

.github/workflows/pmg-e2e.yml

@@ -443,6 +443,11 @@ jobs:
           node-version: 20
           check-latest: true
 
+      - name: Setup PNPM
+        uses: pnpm/action-setup@a7487c7e89a18df4991f7f222e4898a00d66ddda # v4
+        with:
+          version: 10
+
       - name: Build PMG
         run: make
 
@@ -465,6 +470,9 @@ jobs:
       - name: Run Sandbox E2E Test
         run: pmg --sandbox --sandbox-enforce npm exec -- node test/sandbox-e2e.js
 
+      - name: Run Package Manager E2E Test
+        run: pmg --sandbox --sandbox-enforce npm exec -- node test/pm-e2e.js
+
   sandbox-e2e-linux:
     name: Sandbox E2E - Linux (Bubblewrap)
     runs-on: ubuntu-latest
@@ -487,6 +495,11 @@ jobs:
           node-version: 20
           check-latest: true
 
+      - name: Setup PNPM
+        uses: pnpm/action-setup@a7487c7e89a18df4991f7f222e4898a00d66ddda # v4
+        with:
+          version: 10
+
       - name: Install Bubblewrap
         run: sudo apt-get update && sudo apt-get install -y bubblewrap
 
@@ -522,3 +535,6 @@ jobs:
 
       - name: Run Sandbox E2E Test
         run: pmg --sandbox --sandbox-enforce --sandbox-profile npm-restrictive npm exec -- node test/sandbox-e2e.js
+
+      - name: Run Package Manager E2E Test
+        run: pmg --sandbox --sandbox-enforce --sandbox-profile npm-restrictive npm exec -- node test/pm-e2e.js

config/config.template.yml

@@ -97,7 +97,7 @@ sandbox:
 
     pnpm:
       enabled: true
-      profile: npm-restrictive
+      profile: pnpm-restrictive
 
     npx:
       enabled: true

sandbox/profiles/npm-restrictive.yml

@@ -45,7 +45,9 @@ filesystem:
     #         1. Creating the node_modules directory itself
     #         2. Writing any files/directories inside it
     # Temporary directories for shell scripts and package managers
+    # Note: On macOS, /tmp is a symlink to /private/tmp, so we need both
     - /tmp/**
+    - /private/tmp/**
     - /var/tmp/**
     # Project directories
     - ${CWD}/node_modules/**
@@ -75,6 +77,9 @@ filesystem:
     - /usr/**
     - /bin/**
     - /sbin/**
+    # Additional deny rules for extra security
+    - ${CWD}/.env
+    - ${CWD}/.env.*
 
 network:
   # MacOS sandbox-exec does not support network restrictions, so we allow all outbound traffic

sandbox/profiles/pnpm-restrictive.yml

@@ -0,0 +1,25 @@
+name: pnpm-restrictive
+description: Profile for pnpm with write access to current directory
+inherits: npm-restrictive
+
+package_managers:
+  - pnpm
+
+filesystem:
+  allow_write:
+    # pnpm needs write access here
+    - ${HOME}/Library/pnpm/.tools/**
+    - ${HOME}/.pnpm-store/**
+
+    # `pnpm i` creates the tmp files in local dir, at least on MacOS
+    - ${CWD}/_tmp_*
+
+    # pnpm self-update (or likely update) creates temporary package.json files
+    # for writing. This is likely for atomic update using filesystem rename operation
+    # which guarantees atomicity
+    - ${CWD}/package.json.*
+
+    # Need access for dependency resolution
+    - ${CWD}/.pnpm-store
+
+

sandbox/profiles/pypi-restrictive.yml

@@ -32,6 +32,11 @@ filesystem:
     #       For example, ${CWD}/.venv/** allows both:
     #         1. Creating the .venv directory itself
     #         2. Writing any files/directories inside it
+    # Temporary directories for shell scripts and package managers
+    # Note: On macOS, /tmp is a symlink to /private/tmp, so we need both
+    - /tmp/**
+    - /private/tmp/**
+    - /var/tmp/**
     - ${CWD}/.venv/**
     - ${CWD}/venv/**
     - ${HOME}/.cache/pip/**

sandbox/util/dangerous.go

@@ -58,16 +58,23 @@ func GetMandatoryDenyPatterns(allowGitConfig bool) []string {
 		}
 	}
 
-	// Git hooks are ALWAYS blocked for security (can execute arbitrary code)
+	// Git hooks are blocked in CWD and HOME for security (can execute arbitrary code)
+	// We don't use global globs like **/.git/hooks to allow legitimate temp dir operations
+	// (e.g., npx cloning repos to /tmp)
 	patterns = append(patterns, filepath.Join(cwd, ".git/hooks"))
 	patterns = append(patterns, filepath.Join(cwd, ".git/hooks/**"))
-	patterns = append(patterns, "**/.git/hooks")
-	patterns = append(patterns, "**/.git/hooks/**")
 
-	// Git config is conditionally blocked
+	if home != "" {
+		patterns = append(patterns, filepath.Join(home, ".git/hooks"))
+		patterns = append(patterns, filepath.Join(home, ".git/hooks/**"))
+	}
+
+	// Git config is conditionally blocked in CWD and HOME
 	if !allowGitConfig {
 		patterns = append(patterns, filepath.Join(cwd, ".git/config"))
-		patterns = append(patterns, "**/.git/config")
+		if home != "" {
+			patterns = append(patterns, filepath.Join(home, ".git/config"))
+		}
 	}
 
 	return patterns

sandbox/util/dangerous_test.go

@@ -22,19 +22,36 @@ func TestGetMandatoryDenyPatterns(t *testing.T) {
 		assert.Contains(t, patterns, "**/.docker/config.json")
 	})
 
-	t.Run("always blocks git hooks", func(t *testing.T) {
+	t.Run("always blocks git hooks in CWD and HOME", func(t *testing.T) {
+		cwd, err := os.Getwd()
+		assert.NoError(t, err)
+
+		home, err := os.UserHomeDir()
+		assert.NoError(t, err)
+
 		patterns := GetMandatoryDenyPatterns(false)
 
-		// Should block git hooks
+		// Should block git hooks in CWD
-		assert.Contains(t, patterns, "**/.git/hooks")
+		assert.Contains(t, patterns, filepath.Join(cwd, ".git/hooks"))
-		assert.Contains(t, patterns, "**/.git/hooks/**")
+		assert.Contains(t, patterns, filepath.Join(cwd, ".git/hooks/**"))
+
+		// Should block git hooks in HOME
+		assert.Contains(t, patterns, filepath.Join(home, ".git/hooks"))
+		assert.Contains(t, patterns, filepath.Join(home, ".git/hooks/**"))
 	})
 
 	t.Run("blocks git config when allowGitConfig is false", func(t *testing.T) {
+		cwd, err := os.Getwd()
+		assert.NoError(t, err)
+
+		home, err := os.UserHomeDir()
+		assert.NoError(t, err)
+
 		patterns := GetMandatoryDenyPatterns(false)
 
-		// Should block git config
+		// Should block git config in CWD and HOME
-		assert.Contains(t, patterns, "**/.git/config")
+		assert.Contains(t, patterns, filepath.Join(cwd, ".git/config"))
+		assert.Contains(t, patterns, filepath.Join(home, ".git/config"))
 	})
 
 	t.Run("allows git config when allowGitConfig is true", func(t *testing.T) {
@@ -76,4 +93,14 @@ func TestGetMandatoryDenyPatterns(t *testing.T) {
 		// Should include pattern for .env.* files
 		assert.Contains(t, patterns, "**/.env.*")
 	})
+
+	t.Run("does not use global globs for git operations", func(t *testing.T) {
+		patterns := GetMandatoryDenyPatterns(false)
+
+		// Should NOT contain global globs for git hooks/config
+		// This allows legitimate git operations in temp directories (e.g., npx cloning repos)
+		assert.NotContains(t, patterns, "**/.git/hooks")
+		assert.NotContains(t, patterns, "**/.git/hooks/**")
+		assert.NotContains(t, patterns, "**/.git/config")
+	})
 }

test/pm-e2e.js

@@ -0,0 +1,178 @@
+const fs = require('fs');
+const { execSync } = require('child_process');
+const path = require('path');
+const os = require('os');
+
+// Package managers to test
+const PACKAGE_MANAGERS = ['npm', 'pnpm'];
+
+// Well-known dependencies to install
+const TEST_DEPENDENCIES = ['lodash'];
+
+const results = { passed: 0, failed: 0, tests: [] };
+
+function test(name, fn) {
+  try {
+    const result = fn();
+    results.tests.push({ name, status: result ? 'PASS' : 'FAIL', error: null });
+    result ? results.passed++ : results.failed++;
+  } catch (e) {
+    results.tests.push({ name, status: 'ERROR', error: e.message });
+    results.failed++;
+  }
+}
+
+function exec(cmd, options = {}) {
+  try {
+    const output = execSync(cmd, {
+      encoding: 'utf8',
+      timeout: 120000, // 2 minutes
+      ...options
+    });
+    return { success: true, output };
+  } catch (e) {
+    return { success: false, error: e.message, output: e.stdout || '' };
+  }
+}
+
+function createTempDir(prefix) {
+  return fs.mkdtempSync(path.join(os.tmpdir(), prefix));
+}
+
+function cleanup(dir) {
+  try {
+    fs.rmSync(dir, { recursive: true, force: true });
+  } catch (e) {
+    // Ignore cleanup errors
+  }
+}
+
+function testPackageManager(pm) {
+  const testDir = createTempDir(`pmg-e2e-${pm}-`);
+  console.log(`\n  Test directory: ${testDir}`);
+
+  try {
+    // Initialize project
+    test(`${pm}: Initialize project`, () => {
+      const initCmd = pm === 'npm' ? 'npm init -y' : 'pnpm init';
+      const result = exec(initCmd, { cwd: testDir });
+      if (!result.success) {
+        console.log(`    ❌ FAIL: ${result.error}`);
+        return false;
+      }
+
+      const packageJsonExists = fs.existsSync(path.join(testDir, 'package.json'));
+      if (packageJsonExists) {
+        console.log(`    ✅ PASS: Project initialized`);
+        return true;
+      }
+      console.log(`    ❌ FAIL: package.json not created`);
+      return false;
+    });
+
+    // Add dependencies
+    const depsStr = TEST_DEPENDENCIES.join(', ');
+    test(`${pm}: Add dependencies (${depsStr})`, () => {
+      const depsList = TEST_DEPENDENCIES.join(' ');
+      const addCmd = pm === 'npm'
+        ? `npm install ${depsList}`
+        : `pnpm add ${depsList}`;
+      const result = exec(addCmd, { cwd: testDir });
+      if (!result.success) {
+        console.log(`    ❌ FAIL: ${result.error}`);
+        return false;
+      }
+
+      // Verify dependencies were added to package.json
+      const packageJson = JSON.parse(fs.readFileSync(path.join(testDir, 'package.json'), 'utf8'));
+      const missingDeps = TEST_DEPENDENCIES.filter(
+        dep => !packageJson.dependencies || !packageJson.dependencies[dep]
+      );
+      if (missingDeps.length === 0) {
+        console.log(`    ✅ PASS: Dependencies added`);
+        return true;
+      }
+      console.log(`    ❌ FAIL: Missing dependencies in package.json: ${missingDeps.join(', ')}`);
+      return false;
+    });
+
+    // Verify node_modules exists
+    test(`${pm}: Verify node_modules created`, () => {
+      const nodeModulesExists = fs.existsSync(path.join(testDir, 'node_modules'));
+      if (!nodeModulesExists) {
+        console.log(`    ❌ FAIL: node_modules missing`);
+        return false;
+      }
+      const missingDeps = TEST_DEPENDENCIES.filter(
+        dep => !fs.existsSync(path.join(testDir, 'node_modules', dep))
+      );
+      if (missingDeps.length === 0) {
+        console.log(`    ✅ PASS: node_modules and dependencies exist`);
+        return true;
+      }
+      console.log(`    ❌ FAIL: Missing in node_modules: ${missingDeps.join(', ')}`);
+      return false;
+    });
+
+    // Clean install (remove node_modules and reinstall)
+    test(`${pm}: Clean install`, () => {
+      // Remove node_modules
+      const nodeModulesPath = path.join(testDir, 'node_modules');
+      fs.rmSync(nodeModulesPath, { recursive: true, force: true });
+
+      // Reinstall
+      const installCmd = pm === 'npm' ? 'npm install' : 'pnpm install';
+      const result = exec(installCmd, { cwd: testDir });
+      if (!result.success) {
+        console.log(`    ❌ FAIL: ${result.error}`);
+        return false;
+      }
+
+      // Verify node_modules recreated with all dependencies
+      const missingDeps = TEST_DEPENDENCIES.filter(
+        dep => !fs.existsSync(path.join(testDir, 'node_modules', dep))
+      );
+      if (missingDeps.length === 0) {
+        console.log(`    ✅ PASS: Clean install successful`);
+        return true;
+      }
+      console.log(`    ❌ FAIL: Dependencies not reinstalled: ${missingDeps.join(', ')}`);
+      return false;
+    });
+
+  } finally {
+    cleanup(testDir);
+    console.log(`  Cleaned up: ${testDir}`);
+  }
+}
+
+// Main
+console.log('=== PMG Package Manager E2E Tests ===\n');
+console.log('This script tests basic npm/pnpm flows to ensure sandbox compatibility.\n');
+
+for (const pm of PACKAGE_MANAGERS) {
+  // Check if package manager is available
+  const checkResult = exec(`which ${pm}`);
+  if (!checkResult.success) {
+    console.log(`--- Skipping ${pm} (not installed) ---`);
+    continue;
+  }
+
+  console.log(`--- Testing ${pm.toUpperCase()} ---`);
+  testPackageManager(pm);
+}
+
+// Summary
+console.log('\n=== SUMMARY ===');
+console.log(`Passed: ${results.passed}/${results.tests.length}`);
+console.log(`Failed: ${results.failed}/${results.tests.length}`);
+
+if (results.failed > 0) {
+  console.log('\nFailed tests:');
+  results.tests.filter(t => t.status !== 'PASS').forEach(t => {
+    console.log(`  - ${t.name}: ${t.status} ${t.error || ''}`);
+  });
+  process.exit(1);
+}
+
+console.log('\nAll tests passed!');

test/sandbox-e2e.js

@@ -45,7 +45,7 @@ console.log('=== PMG Sandbox Policy Violation Tests ===\n');
 // ============================================
 console.log('--- Tests that SHOULD be BLOCKED ---\n');
 
-// Test 1: Read ~/.ssh (should be blocked)
+// Read ~/.ssh (should be blocked)
 test('BLOCK: Read ~/.ssh directory', () => {
   const result = isDirectoryBlocked(path.join(home, '.ssh'));
   if (result.skip) {
@@ -60,7 +60,7 @@ test('BLOCK: Read ~/.ssh directory', () => {
   return false;
 });
 
-// Test 2: Read ~/.aws (should be blocked)
+// Read ~/.aws (should be blocked)
 test('BLOCK: Read ~/.aws directory', () => {
   const result = isDirectoryBlocked(path.join(home, '.aws'));
   if (result.skip) {
@@ -75,7 +75,7 @@ test('BLOCK: Read ~/.aws directory', () => {
   return false;
 });
 
-// Test 3: Read ~/.kube (should be blocked)
+// Read ~/.kube (should be blocked)
 test('BLOCK: Read ~/.kube directory', () => {
   const result = isDirectoryBlocked(path.join(home, '.kube'));
   if (result.skip) {
@@ -90,7 +90,7 @@ test('BLOCK: Read ~/.kube directory', () => {
   return false;
 });
 
-// Test 4: Read ~/.gcloud (should be blocked)
+// Read ~/.gcloud (should be blocked)
 test('BLOCK: Read ~/.gcloud directory', () => {
   const result = isDirectoryBlocked(path.join(home, '.gcloud'));
   if (result.skip) {
@@ -105,7 +105,7 @@ test('BLOCK: Read ~/.gcloud directory', () => {
   return false;
 });
 
-// Test 5: Write to /etc (should be blocked)
+// Write to /etc (should be blocked)
 test('BLOCK: Write to /etc', () => {
   try {
     fs.writeFileSync('/etc/test-pmg-sandbox', 'test');
@@ -122,7 +122,7 @@ test('BLOCK: Write to /etc', () => {
   }
 });
 
-// Test 6: Write to /usr (should be blocked)
+// Write to /usr (should be blocked)
 test('BLOCK: Write to /usr', () => {
   try {
     fs.writeFileSync('/usr/test-pmg-sandbox', 'test');
@@ -135,7 +135,7 @@ test('BLOCK: Write to /usr', () => {
   }
 });
 
-// Test 7: Execute curl (should be blocked by policy)
+// Execute curl (should be blocked by policy)
 test('BLOCK: Execute /usr/bin/curl', () => {
   try {
     const result = spawnSync('/usr/bin/curl', ['--version'], { timeout: 5000 });
@@ -151,12 +151,79 @@ test('BLOCK: Execute /usr/bin/curl', () => {
   }
 });
 
+// .git/hooks in CWD should be protected from persistent writes.
+// Two valid sandbox strategies:
+//   macOS/seatbelt: write is denied outright (EPERM)
+//   Linux/bwrap:    tmpfs hides real directory; writes go to ephemeral tmpfs
+// Both prevent malicious hooks from persisting to the real filesystem.
+test('BLOCK: .git/hooks in CWD is protected', () => {
+  const gitHooksDir = path.join(process.cwd(), '.git', 'hooks');
+
+  if (!fs.existsSync(path.join(process.cwd(), '.git'))) {
+    console.log('  ⚠️  SKIP: .git does not exist in CWD');
+    return true;
+  }
+
+  // Count original hooks visible before any write attempt.
+  // On Linux/bwrap with tmpfs, the real hooks are hidden (directory appears empty).
+  let originalCount = 0;
+  try {
+    originalCount = fs.readdirSync(gitHooksDir).length;
+  } catch (e) {
+    if (e.code === 'ENOENT') {
+      console.log('  ⚠️  SKIP: .git/hooks does not exist');
+      return true;
+    }
+    // Read denied entirely — protected
+    console.log(`  ✅ PASS: .git/hooks read blocked (${e.code})`);
+    return true;
+  }
+
+  // If the directory is empty, tmpfs is hiding the real hooks
+  if (originalCount === 0) {
+    console.log('  ✅ PASS: .git/hooks hidden by tmpfs (real hooks not visible)');
+    return true;
+  }
+
+  // Directory is visible with contents — must be macOS/seatbelt.
+  // Verify writes are denied.
+  const testHookPath = path.join(gitHooksDir, 'test-pmg-sandbox-hook');
+  const cleanup = () => {
+    try {
+      if (fs.existsSync(testHookPath)) {
+        fs.unlinkSync(testHookPath);
+      }
+    } catch (e) {
+      // Ignore cleanup errors
+    }
+  };
+
+  try {
+    fs.writeFileSync(testHookPath, '#!/bin/sh\necho "malicious hook"');
+    cleanup();
+    console.log('  ❌ FAIL: Could write to .git/hooks');
+    return false;
+  } catch (e) {
+    cleanup();
+    if (e.code === 'EPERM' || e.code === 'EACCES') {
+      console.log('  ✅ PASS: .git/hooks write blocked');
+      return true;
+    }
+    if (e.code === 'ENOENT') {
+      console.log('  ⚠️  SKIP: .git/hooks does not exist');
+      return true;
+    }
+    console.log(`  ✅ PASS: .git/hooks write blocked (${e.code})`);
+    return true;
+  }
+});
+
 // ============================================
 // TESTS THAT SHOULD BE ALLOWED
 // ============================================
 console.log('\n--- Tests that SHOULD be ALLOWED ---\n');
 
-// Test 8: Read current directory
+// Read current directory
 test('ALLOW: Read current directory', () => {
   try {
     fs.readdirSync('.');
@@ -168,7 +235,7 @@ test('ALLOW: Read current directory', () => {
   }
 });
 
-// Test 9: Write to TMPDIR (allowed by policy)
+// Write to TMPDIR (allowed by policy)
 test('ALLOW: Write to TMPDIR', () => {
   try {
     const tmpFile = path.join(os.tmpdir(), 'test-pmg-sandbox-write.txt');
@@ -182,7 +249,7 @@ test('ALLOW: Write to TMPDIR', () => {
   }
 });
 
-// Test 10: Read node_modules (if exists)
+// Read node_modules (if exists)
 test('ALLOW: Read node_modules', () => {
   try {
     fs.readdirSync('node_modules');
@@ -198,7 +265,7 @@ test('ALLOW: Read node_modules', () => {
   }
 });
 
-// Test 11: Read system libraries
+// Read system libraries
 test('ALLOW: Read /usr/lib', () => {
   try {
     fs.readdirSync('/usr/lib');
@@ -210,7 +277,7 @@ test('ALLOW: Read /usr/lib', () => {
   }
 });
 
-// Test 12: Network access (DNS + HTTP)
+// Network access (DNS + HTTP)
 test('ALLOW: Network DNS resolution', () => {
   try {
     require('dns').lookup('registry.npmjs.org', (err) => { });