fix(sandbox): bind parent dir for globstar allow_write on bwrap (#321)

* fix(sandbox): bind parent dir for globstar allow_write on bwrap

Fine-grained per-path mounts under read-only project binds broke pip
install into in-project .venv directories. Always mount the parent tree
for ** write rules instead.

Fixes #315

* test(sandbox): tighten globstar bind assertions and ensure ~/.npm exists for e2e

Strengthen TestBubblewrapAllowWriteGlobstarBindsParentOnly to verify the
parent dir is writably bound and the child path is read-only bound, not
just substring presence. Pre-create ~/.npm in the e2e harness so
bubblewrap --bind-try does not skip the npm cache dir on fresh runners.

* switch pnpm to /tmp in sandbox e2e

* test(sandbox): update glob ** test for parent-bind semantics

Globstar allow_write now binds the parent dir only (e2e740d), so the
test should assert the parent is writably bound and child subdirs are
not individually bound, instead of substring-matching subdir names.

* fix(sandbox): bind correct base dir for in-pattern globstar allow_write

Globstar allow_write previously used extractGlobParentDir, which walks past
the first ** and yields the wrong root for patterns like /a/b/**/d/**/e.
Introduce extractGlobstarWriteBaseDir, which takes the prefix before the
first /**, and use it in processWriteRule. Also dedup the coarse-fallback
parent-bind loop to mirror the read-rule fallback.
This commit is contained in:
Sahil Bansal
2026-06-07 10:03:23 +05:30
committed by GitHub
parent f3e00a7f6e
commit 872c5d663c
4 changed files with 165 additions and 13 deletions
@@ -529,8 +529,21 @@ func (t *bubblewrapPolicyTranslator) processWriteRule(path string, boundPaths ma
// Check if path contains glob pattern
if util.ContainsGlob(path) {
// Check if the base directory is already bound (e.g., /tmp already bound, skip /tmp/**)
baseDir := t.extractParentDir(path)
// Globstar write rules always bind the parent directory read-write. Per-path
// binds interact badly with earlier read-only parent mounts (e.g. ${CWD}/**)
// and miss files beyond maxGlobDepth — see https://github.com/safedep/pmg/issues/315.
// Base dir is the path prefix before the first "/**" (see extractGlobstarWriteBaseDir).
if strings.Contains(path, "**") {
baseDir = extractGlobstarWriteBaseDir(path)
args = append(args, "--bind-try", baseDir, baseDir)
boundPaths[baseDir] = true
log.Debugf("Globstar allow_write: bound parent directory '%s' (read-write)", baseDir)
return args, nil
}
// Check if the base directory is already bound (e.g., /tmp already bound, skip /tmp/**)
if boundPaths[baseDir] {
log.Debugf("Skipping pattern '%s' - base directory '%s' already bound", path, baseDir)
return args, nil
@@ -550,8 +563,7 @@ func (t *bubblewrapPolicyTranslator) processWriteRule(path string, boundPaths ma
boundPaths[parentDir] = true
log.Debugf("Coarse-grained fallback: bound parent directory '%s' (read-write)", parentDir)
} else {
// Path already bound, skip (likely already bound as read-only from essential paths)
log.Debugf("Parent directory '%s' already bound, skipping duplicate bind", parentDir)
log.Debugf("Parent directory '%s' already bound for write, skipping duplicate bind", parentDir)
}
}
} else {
@@ -274,13 +274,9 @@ func TestBubblewrapTranslatorGlobPatterns(t *testing.T) {
},
assert: func(t *testing.T, args []string, err error) {
require.NoError(t, err)
argsStr := argSliceToString(args)
// Should include the base directory
assert.Contains(t, argsStr, tmpDir)
// Should include subdirectories
assert.Contains(t, argsStr, "subdir1")
assert.Contains(t, argsStr, "subdir2")
assertWriteBind(t, args, tmpDir)
assertNoWriteBind(t, args, filepath.Join(tmpDir, "subdir1"))
assertNoWriteBind(t, args, filepath.Join(tmpDir, "subdir2"))
},
},
{
@@ -775,6 +771,42 @@ func TestGlobFallbackThresholdGlobstar(t *testing.T) {
assert.Less(t, len(args), 300, "Coarse-grained fallback should prevent argument explosion")
}
// TestBubblewrapAllowWriteGlobstarBindsParentOnly verifies globstar allow_write rules
// bind the parent tree (e.g. .venv) instead of per-file mounts that break pip in-project venvs.
// See https://github.com/safedep/pmg/issues/315
func TestBubblewrapAllowWriteGlobstarBindsParentOnly(t *testing.T) {
tmpDir := t.TempDir()
venvDir := filepath.Join(tmpDir, ".venv")
binDir := filepath.Join(venvDir, "bin")
require.NoError(t, os.MkdirAll(binDir, 0755))
pythonPath := filepath.Join(binDir, "python")
require.NoError(t, os.Symlink("/usr/bin/python3", pythonPath))
// Populate enough shallow files that old logic would fine-grain bind without hitting fallback.
for i := 0; i < 30; i++ {
path := filepath.Join(venvDir, fmt.Sprintf("file%d.txt", i))
require.NoError(t, os.WriteFile(path, []byte("x"), 0644))
}
config := newDefaultBubblewrapConfig()
translator := newBubblewrapPolicyTranslator(config)
policy := &sandbox.SandboxPolicy{
Name: "test-venv-write",
Filesystem: sandbox.FilesystemPolicy{
AllowRead: []string{tmpDir + "/**"},
AllowWrite: []string{venvDir + "/**"},
},
}
args, err := translator.translate(policy)
require.NoError(t, err)
assertWriteBind(t, args, venvDir)
assertNoWriteBind(t, args, pythonPath)
assertReadBind(t, args, pythonPath)
}
// TestGlobNoFallbackSmallPattern tests that small patterns don't trigger fallback
func TestGlobNoFallbackSmallPattern(t *testing.T) {
tmpDir := t.TempDir()
@@ -905,6 +937,57 @@ func TestExtractParentDir(t *testing.T) {
}
}
func TestExtractGlobstarWriteBaseDir(t *testing.T) {
cases := []struct {
name string
pattern string
expected string
}{
{
name: "suffix globstar (profiles)",
pattern: "/home/user/.venv/**",
expected: "/home/user/.venv",
},
{
name: "in-pattern globstars",
pattern: "/a/b/**/d/**/e",
expected: "/a/b",
},
{
name: "middle globstar with file suffix",
pattern: "/usr/lib/**/*.so",
expected: "/usr/lib",
},
}
for _, tt := range cases {
t.Run(tt.name, func(t *testing.T) {
assert.Equal(t, tt.expected, extractGlobstarWriteBaseDir(tt.pattern))
})
}
}
func TestBubblewrapGlobstarWriteMultiSegmentBind(t *testing.T) {
tmpDir := t.TempDir()
baseDir := filepath.Join(tmpDir, "a", "b")
require.NoError(t, os.MkdirAll(baseDir, 0755))
pattern := filepath.Join(tmpDir, "a", "b", "**", "d", "**", "e")
config := newDefaultBubblewrapConfig()
translator := newBubblewrapPolicyTranslator(config)
policy := &sandbox.SandboxPolicy{
Name: "test-multi-globstar-write",
Filesystem: sandbox.FilesystemPolicy{
AllowWrite: []string{pattern},
},
}
args, err := translator.translate(policy)
require.NoError(t, err)
assertWriteBind(t, args, baseDir)
}
// Helper function to convert arg slice to string for easier assertion
func argSliceToString(args []string) string {
result := ""
@@ -1054,6 +1137,25 @@ func assertReadBind(t *testing.T, args []string, path string) {
t.Fatalf("expected --ro-bind %q %q, not found in args: %v", path, path, args)
}
func assertWriteBind(t *testing.T, args []string, path string) {
t.Helper()
for i := 0; i+2 < len(args); i++ {
if (args[i] == "--bind" || args[i] == "--bind-try") && args[i+1] == path && args[i+2] == path {
return
}
}
t.Fatalf("expected --bind-try %q %q, not found in args: %v", path, path, args)
}
func assertNoWriteBind(t *testing.T, args []string, path string) {
t.Helper()
for i := 0; i+2 < len(args); i++ {
if (args[i] == "--bind" || args[i] == "--bind-try") && args[i+1] == path && args[i+2] == path {
t.Fatalf("unexpected writable bind at %q in args: %v", path, args)
}
}
}
func assertReadOnlyBindAfterWritableBind(t *testing.T, args []string, readOnlyPath string, writablePath string) {
t.Helper()
+16
View File
@@ -108,3 +108,19 @@ func extractGlobParentDir(pattern string) string {
}
return pattern
}
// extractGlobstarWriteBaseDir returns the directory to bind read-write for an
// allow_write globstar pattern. It uses the path before the first "/**" so
// suffix-only profiles (${CWD}/.venv/**) and in-pattern globstars (/a/b/**/d/**/e)
// both bind the intended tree root (/a/b for the latter). Falls back to
// extractGlobParentDir when the pattern has ** but no "/**" segment.
func extractGlobstarWriteBaseDir(pattern string) string {
if i := strings.Index(pattern, "/**"); i >= 0 {
base := strings.TrimSuffix(pattern[:i], string(filepath.Separator))
if base == "" {
return string(filepath.Separator)
}
return base
}
return extractGlobParentDir(pattern)
}
+25 -3
View File
@@ -47,15 +47,37 @@ function cleanup(dir) {
}
}
// npm-restrictive allow_write includes /tmp/**. Use cache/store paths under /tmp so
// bubblewrap can bind /tmp read-write without pre-creating ~/.npm or ~/.cache/pnpm.
function pmEnv(pm) {
const base = path.join(os.tmpdir(), 'pmg-e2e');
fs.mkdirSync(base, { recursive: true });
const env = { ...process.env };
env.npm_config_cache = path.join(base, 'npm-cache');
fs.mkdirSync(env.npm_config_cache, { recursive: true });
if (pm === 'pnpm') {
env.PNPM_HOME = path.join(base, 'pnpm-home');
env.npm_config_store_dir = path.join(base, 'pnpm-store');
fs.mkdirSync(env.PNPM_HOME, { recursive: true });
fs.mkdirSync(env.npm_config_store_dir, { recursive: true });
env.PATH = `${env.PNPM_HOME}${path.delimiter}${env.PATH}`;
}
return env;
}
function testPackageManager(pm) {
const testDir = createTempDir(`pmg-e2e-${pm}-`);
const env = pmEnv(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 });
const result = exec(initCmd, { cwd: testDir, env });
if (!result.success) {
console.log(` ❌ FAIL: ${result.error}`);
return false;
@@ -77,7 +99,7 @@ function testPackageManager(pm) {
const addCmd = pm === 'npm'
? `npm install ${depsList}`
: `pnpm add ${depsList}`;
const result = exec(addCmd, { cwd: testDir });
const result = exec(addCmd, { cwd: testDir, env });
if (!result.success) {
console.log(` ❌ FAIL: ${result.error}`);
return false;
@@ -122,7 +144,7 @@ function testPackageManager(pm) {
// Reinstall
const installCmd = pm === 'npm' ? 'npm install' : 'pnpm install';
const result = exec(installCmd, { cwd: testDir });
const result = exec(installCmd, { cwd: testDir, env });
if (!result.success) {
console.log(` ❌ FAIL: ${result.error}`);
return false;