Skip to content

Commit 4600ab0

Browse files
abhisekCopilotSahilb315
authored
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>
1 parent b332e1d commit 4600ab0

File tree

9 files changed

+354
-24
lines changed

9 files changed

+354
-24
lines changed

.github/workflows/pmg-e2e.yml

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -443,6 +443,11 @@ jobs:
443443
node-version: 20
444444
check-latest: true
445445

446+
- name: Setup PNPM
447+
uses: pnpm/action-setup@a7487c7e89a18df4991f7f222e4898a00d66ddda # v4
448+
with:
449+
version: 10
450+
446451
- name: Build PMG
447452
run: make
448453

@@ -465,6 +470,9 @@ jobs:
465470
- name: Run Sandbox E2E Test
466471
run: pmg --sandbox --sandbox-enforce npm exec -- node test/sandbox-e2e.js
467472

473+
- name: Run Package Manager E2E Test
474+
run: pmg --sandbox --sandbox-enforce npm exec -- node test/pm-e2e.js
475+
468476
sandbox-e2e-linux:
469477
name: Sandbox E2E - Linux (Bubblewrap)
470478
runs-on: ubuntu-latest
@@ -487,6 +495,11 @@ jobs:
487495
node-version: 20
488496
check-latest: true
489497

498+
- name: Setup PNPM
499+
uses: pnpm/action-setup@a7487c7e89a18df4991f7f222e4898a00d66ddda # v4
500+
with:
501+
version: 10
502+
490503
- name: Install Bubblewrap
491504
run: sudo apt-get update && sudo apt-get install -y bubblewrap
492505

@@ -522,3 +535,6 @@ jobs:
522535
523536
- name: Run Sandbox E2E Test
524537
run: pmg --sandbox --sandbox-enforce --sandbox-profile npm-restrictive npm exec -- node test/sandbox-e2e.js
538+
539+
- name: Run Package Manager E2E Test
540+
run: pmg --sandbox --sandbox-enforce --sandbox-profile npm-restrictive npm exec -- node test/pm-e2e.js

config/config.template.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ sandbox:
9797

9898
pnpm:
9999
enabled: true
100-
profile: npm-restrictive
100+
profile: pnpm-restrictive
101101

102102
npx:
103103
enabled: true

sandbox/profiles/npm-restrictive.yml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,9 @@ filesystem:
4545
# 1. Creating the node_modules directory itself
4646
# 2. Writing any files/directories inside it
4747
# Temporary directories for shell scripts and package managers
48+
# Note: On macOS, /tmp is a symlink to /private/tmp, so we need both
4849
- /tmp/**
50+
- /private/tmp/**
4951
- /var/tmp/**
5052
# Project directories
5153
- ${CWD}/node_modules/**
@@ -75,6 +77,9 @@ filesystem:
7577
- /usr/**
7678
- /bin/**
7779
- /sbin/**
80+
# Additional deny rules for extra security
81+
- ${CWD}/.env
82+
- ${CWD}/.env.*
7883

7984
network:
8085
# MacOS sandbox-exec does not support network restrictions, so we allow all outbound traffic
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
name: pnpm-restrictive
2+
description: Profile for pnpm with write access to current directory
3+
inherits: npm-restrictive
4+
5+
package_managers:
6+
- pnpm
7+
8+
filesystem:
9+
allow_write:
10+
# pnpm needs write access here
11+
- ${HOME}/Library/pnpm/.tools/**
12+
- ${HOME}/.pnpm-store/**
13+
14+
# `pnpm i` creates the tmp files in local dir, at least on MacOS
15+
- ${CWD}/_tmp_*
16+
17+
# pnpm self-update (or likely update) creates temporary package.json files
18+
# for writing. This is likely for atomic update using filesystem rename operation
19+
# which guarantees atomicity
20+
- ${CWD}/package.json.*
21+
22+
# Need access for dependency resolution
23+
- ${CWD}/.pnpm-store
24+
25+

sandbox/profiles/pypi-restrictive.yml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,11 @@ filesystem:
3232
# For example, ${CWD}/.venv/** allows both:
3333
# 1. Creating the .venv directory itself
3434
# 2. Writing any files/directories inside it
35+
# Temporary directories for shell scripts and package managers
36+
# Note: On macOS, /tmp is a symlink to /private/tmp, so we need both
37+
- /tmp/**
38+
- /private/tmp/**
39+
- /var/tmp/**
3540
- ${CWD}/.venv/**
3641
- ${CWD}/venv/**
3742
- ${HOME}/.cache/pip/**

sandbox/util/dangerous.go

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -58,16 +58,23 @@ func GetMandatoryDenyPatterns(allowGitConfig bool) []string {
5858
}
5959
}
6060

61-
// Git hooks are ALWAYS blocked for security (can execute arbitrary code)
61+
// Git hooks are blocked in CWD and HOME for security (can execute arbitrary code)
62+
// We don't use global globs like **/.git/hooks to allow legitimate temp dir operations
63+
// (e.g., npx cloning repos to /tmp)
6264
patterns = append(patterns, filepath.Join(cwd, ".git/hooks"))
6365
patterns = append(patterns, filepath.Join(cwd, ".git/hooks/**"))
64-
patterns = append(patterns, "**/.git/hooks")
65-
patterns = append(patterns, "**/.git/hooks/**")
6666

67-
// Git config is conditionally blocked
67+
if home != "" {
68+
patterns = append(patterns, filepath.Join(home, ".git/hooks"))
69+
patterns = append(patterns, filepath.Join(home, ".git/hooks/**"))
70+
}
71+
72+
// Git config is conditionally blocked in CWD and HOME
6873
if !allowGitConfig {
6974
patterns = append(patterns, filepath.Join(cwd, ".git/config"))
70-
patterns = append(patterns, "**/.git/config")
75+
if home != "" {
76+
patterns = append(patterns, filepath.Join(home, ".git/config"))
77+
}
7178
}
7279

7380
return patterns

sandbox/util/dangerous_test.go

Lines changed: 33 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,19 +22,36 @@ func TestGetMandatoryDenyPatterns(t *testing.T) {
2222
assert.Contains(t, patterns, "**/.docker/config.json")
2323
})
2424

25-
t.Run("always blocks git hooks", func(t *testing.T) {
25+
t.Run("always blocks git hooks in CWD and HOME", func(t *testing.T) {
26+
cwd, err := os.Getwd()
27+
assert.NoError(t, err)
28+
29+
home, err := os.UserHomeDir()
30+
assert.NoError(t, err)
31+
2632
patterns := GetMandatoryDenyPatterns(false)
2733

28-
// Should block git hooks
29-
assert.Contains(t, patterns, "**/.git/hooks")
30-
assert.Contains(t, patterns, "**/.git/hooks/**")
34+
// Should block git hooks in CWD
35+
assert.Contains(t, patterns, filepath.Join(cwd, ".git/hooks"))
36+
assert.Contains(t, patterns, filepath.Join(cwd, ".git/hooks/**"))
37+
38+
// Should block git hooks in HOME
39+
assert.Contains(t, patterns, filepath.Join(home, ".git/hooks"))
40+
assert.Contains(t, patterns, filepath.Join(home, ".git/hooks/**"))
3141
})
3242

3343
t.Run("blocks git config when allowGitConfig is false", func(t *testing.T) {
44+
cwd, err := os.Getwd()
45+
assert.NoError(t, err)
46+
47+
home, err := os.UserHomeDir()
48+
assert.NoError(t, err)
49+
3450
patterns := GetMandatoryDenyPatterns(false)
3551

36-
// Should block git config
37-
assert.Contains(t, patterns, "**/.git/config")
52+
// Should block git config in CWD and HOME
53+
assert.Contains(t, patterns, filepath.Join(cwd, ".git/config"))
54+
assert.Contains(t, patterns, filepath.Join(home, ".git/config"))
3855
})
3956

4057
t.Run("allows git config when allowGitConfig is true", func(t *testing.T) {
@@ -76,4 +93,14 @@ func TestGetMandatoryDenyPatterns(t *testing.T) {
7693
// Should include pattern for .env.* files
7794
assert.Contains(t, patterns, "**/.env.*")
7895
})
96+
97+
t.Run("does not use global globs for git operations", func(t *testing.T) {
98+
patterns := GetMandatoryDenyPatterns(false)
99+
100+
// Should NOT contain global globs for git hooks/config
101+
// This allows legitimate git operations in temp directories (e.g., npx cloning repos)
102+
assert.NotContains(t, patterns, "**/.git/hooks")
103+
assert.NotContains(t, patterns, "**/.git/hooks/**")
104+
assert.NotContains(t, patterns, "**/.git/config")
105+
})
79106
}

test/pm-e2e.js

Lines changed: 178 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,178 @@
1+
const fs = require('fs');
2+
const { execSync } = require('child_process');
3+
const path = require('path');
4+
const os = require('os');
5+
6+
// Package managers to test
7+
const PACKAGE_MANAGERS = ['npm', 'pnpm'];
8+
9+
// Well-known dependencies to install
10+
const TEST_DEPENDENCIES = ['lodash'];
11+
12+
const results = { passed: 0, failed: 0, tests: [] };
13+
14+
function test(name, fn) {
15+
try {
16+
const result = fn();
17+
results.tests.push({ name, status: result ? 'PASS' : 'FAIL', error: null });
18+
result ? results.passed++ : results.failed++;
19+
} catch (e) {
20+
results.tests.push({ name, status: 'ERROR', error: e.message });
21+
results.failed++;
22+
}
23+
}
24+
25+
function exec(cmd, options = {}) {
26+
try {
27+
const output = execSync(cmd, {
28+
encoding: 'utf8',
29+
timeout: 120000, // 2 minutes
30+
...options
31+
});
32+
return { success: true, output };
33+
} catch (e) {
34+
return { success: false, error: e.message, output: e.stdout || '' };
35+
}
36+
}
37+
38+
function createTempDir(prefix) {
39+
return fs.mkdtempSync(path.join(os.tmpdir(), prefix));
40+
}
41+
42+
function cleanup(dir) {
43+
try {
44+
fs.rmSync(dir, { recursive: true, force: true });
45+
} catch (e) {
46+
// Ignore cleanup errors
47+
}
48+
}
49+
50+
function testPackageManager(pm) {
51+
const testDir = createTempDir(`pmg-e2e-${pm}-`);
52+
console.log(`\n Test directory: ${testDir}`);
53+
54+
try {
55+
// Initialize project
56+
test(`${pm}: Initialize project`, () => {
57+
const initCmd = pm === 'npm' ? 'npm init -y' : 'pnpm init';
58+
const result = exec(initCmd, { cwd: testDir });
59+
if (!result.success) {
60+
console.log(` ❌ FAIL: ${result.error}`);
61+
return false;
62+
}
63+
64+
const packageJsonExists = fs.existsSync(path.join(testDir, 'package.json'));
65+
if (packageJsonExists) {
66+
console.log(` ✅ PASS: Project initialized`);
67+
return true;
68+
}
69+
console.log(` ❌ FAIL: package.json not created`);
70+
return false;
71+
});
72+
73+
// Add dependencies
74+
const depsStr = TEST_DEPENDENCIES.join(', ');
75+
test(`${pm}: Add dependencies (${depsStr})`, () => {
76+
const depsList = TEST_DEPENDENCIES.join(' ');
77+
const addCmd = pm === 'npm'
78+
? `npm install ${depsList}`
79+
: `pnpm add ${depsList}`;
80+
const result = exec(addCmd, { cwd: testDir });
81+
if (!result.success) {
82+
console.log(` ❌ FAIL: ${result.error}`);
83+
return false;
84+
}
85+
86+
// Verify dependencies were added to package.json
87+
const packageJson = JSON.parse(fs.readFileSync(path.join(testDir, 'package.json'), 'utf8'));
88+
const missingDeps = TEST_DEPENDENCIES.filter(
89+
dep => !packageJson.dependencies || !packageJson.dependencies[dep]
90+
);
91+
if (missingDeps.length === 0) {
92+
console.log(` ✅ PASS: Dependencies added`);
93+
return true;
94+
}
95+
console.log(` ❌ FAIL: Missing dependencies in package.json: ${missingDeps.join(', ')}`);
96+
return false;
97+
});
98+
99+
// Verify node_modules exists
100+
test(`${pm}: Verify node_modules created`, () => {
101+
const nodeModulesExists = fs.existsSync(path.join(testDir, 'node_modules'));
102+
if (!nodeModulesExists) {
103+
console.log(` ❌ FAIL: node_modules missing`);
104+
return false;
105+
}
106+
const missingDeps = TEST_DEPENDENCIES.filter(
107+
dep => !fs.existsSync(path.join(testDir, 'node_modules', dep))
108+
);
109+
if (missingDeps.length === 0) {
110+
console.log(` ✅ PASS: node_modules and dependencies exist`);
111+
return true;
112+
}
113+
console.log(` ❌ FAIL: Missing in node_modules: ${missingDeps.join(', ')}`);
114+
return false;
115+
});
116+
117+
// Clean install (remove node_modules and reinstall)
118+
test(`${pm}: Clean install`, () => {
119+
// Remove node_modules
120+
const nodeModulesPath = path.join(testDir, 'node_modules');
121+
fs.rmSync(nodeModulesPath, { recursive: true, force: true });
122+
123+
// Reinstall
124+
const installCmd = pm === 'npm' ? 'npm install' : 'pnpm install';
125+
const result = exec(installCmd, { cwd: testDir });
126+
if (!result.success) {
127+
console.log(` ❌ FAIL: ${result.error}`);
128+
return false;
129+
}
130+
131+
// Verify node_modules recreated with all dependencies
132+
const missingDeps = TEST_DEPENDENCIES.filter(
133+
dep => !fs.existsSync(path.join(testDir, 'node_modules', dep))
134+
);
135+
if (missingDeps.length === 0) {
136+
console.log(` ✅ PASS: Clean install successful`);
137+
return true;
138+
}
139+
console.log(` ❌ FAIL: Dependencies not reinstalled: ${missingDeps.join(', ')}`);
140+
return false;
141+
});
142+
143+
} finally {
144+
cleanup(testDir);
145+
console.log(` Cleaned up: ${testDir}`);
146+
}
147+
}
148+
149+
// Main
150+
console.log('=== PMG Package Manager E2E Tests ===\n');
151+
console.log('This script tests basic npm/pnpm flows to ensure sandbox compatibility.\n');
152+
153+
for (const pm of PACKAGE_MANAGERS) {
154+
// Check if package manager is available
155+
const checkResult = exec(`which ${pm}`);
156+
if (!checkResult.success) {
157+
console.log(`--- Skipping ${pm} (not installed) ---`);
158+
continue;
159+
}
160+
161+
console.log(`--- Testing ${pm.toUpperCase()} ---`);
162+
testPackageManager(pm);
163+
}
164+
165+
// Summary
166+
console.log('\n=== SUMMARY ===');
167+
console.log(`Passed: ${results.passed}/${results.tests.length}`);
168+
console.log(`Failed: ${results.failed}/${results.tests.length}`);
169+
170+
if (results.failed > 0) {
171+
console.log('\nFailed tests:');
172+
results.tests.filter(t => t.status !== 'PASS').forEach(t => {
173+
console.log(` - ${t.name}: ${t.status} ${t.error || ''}`);
174+
});
175+
process.exit(1);
176+
}
177+
178+
console.log('\nAll tests passed!');

0 commit comments

Comments
 (0)