diff --git a/sandbox/platform/bubblewrap_config_linux.go b/sandbox/platform/bubblewrap_config_linux.go index 2c32ac8..ce8ba3e 100644 --- a/sandbox/platform/bubblewrap_config_linux.go +++ b/sandbox/platform/bubblewrap_config_linux.go @@ -261,18 +261,20 @@ func getMandatoryDenyPatterns() []string { // shouldUnshareNetwork determines whether to isolate network based on policy. // Returns true if network should be completely isolated (--unshare-net). func (c *bubblewrapConfig) shouldUnshareNetwork(hasAllowRules bool, hasDenyAll bool) bool { - // If policy explicitly denies all network ("*:*"), isolate + // If there are allow rules, don't isolate network + // Note: bubblewrap can't do per-host filtering, so allow rules mean "allow network" + // The allow_outbound rules serve as documentation of intended access + if hasAllowRules { + return false + } + + // No allow rules - check if we should deny all if hasDenyAll { return true } - // If no allow rules and default is to isolate, unshare - if !hasAllowRules && c.unshareNetworkByDefault { - return true - } - - // Otherwise, allow network (no --unshare-net) - return false + // No allow rules and no deny-all - use default behavior + return c.unshareNetworkByDefault } // getEssentialSystemPaths returns essential system paths for read-only binding. diff --git a/sandbox/platform/bubblewrap_translator_linux.go b/sandbox/platform/bubblewrap_translator_linux.go index df3be29..8a7ba11 100644 --- a/sandbox/platform/bubblewrap_translator_linux.go +++ b/sandbox/platform/bubblewrap_translator_linux.go @@ -159,9 +159,11 @@ func (t *bubblewrapPolicyTranslator) addIsolationNamespaces(policy *sandbox.Sand // // Strategy: // 1. Start with essential system paths (added separately) -// 2. Add user-specified allow_read paths (read-only bind mounts) -// 3. Add user-specified allow_write paths (read-write bind mounts) -// 4. Handle deny patterns by mounting /dev/null (prevents creation) +// 2. Add user-specified allow_read paths FIRST (read-only bind mounts) +// This establishes the base filesystem view (e.g., "/" for full access) +// 3. Add user-specified allow_write paths SECOND (read-write bind mounts) +// These OVERRIDE earlier read-only binds (bwrap: later mounts win) +// 4. Handle deny patterns by mounting /dev/null or read-only for directories // 5. Add mandatory deny patterns func (t *bubblewrapPolicyTranslator) translateFilesystem(policy *sandbox.SandboxPolicy) ([]string, error) { args := []string{} @@ -174,7 +176,13 @@ func (t *bubblewrapPolicyTranslator) translateFilesystem(policy *sandbox.Sandbox boundPaths[path] = true } - // 1. Process allow_read rules (read-only bind mounts) + // Mark tmpdir as already bound (will be handled by addTmpdirSupport()) + // This prevents conflicts from policy patterns like /tmp/** + tmpDir := os.TempDir() + boundPaths[tmpDir] = true + + // 1. Process allow_read rules FIRST (read-only bind mounts) + // This establishes the base read-only filesystem view (including "/" if specified) for _, pattern := range policy.Filesystem.AllowRead { expanded, err := util.ExpandVariables(pattern) if err != nil { @@ -190,7 +198,11 @@ func (t *bubblewrapPolicyTranslator) translateFilesystem(policy *sandbox.Sandbox args = append(args, readArgs...) } - // 2. Process allow_write rules (read-write bind mounts) + // 2. Process allow_write rules SECOND (read-write bind mounts) + // These OVERRIDE earlier read-only binds (bwrap: later mounts win) + // Use a separate map so we don't skip paths that need write access + writeBoundPaths := make(map[string]bool) + writeBoundPaths[tmpDir] = true // tmpdir handled by addTmpdirSupport for _, pattern := range policy.Filesystem.AllowWrite { expanded, err := util.ExpandVariables(pattern) if err != nil { @@ -198,7 +210,7 @@ func (t *bubblewrapPolicyTranslator) translateFilesystem(policy *sandbox.Sandbox continue } - writeArgs, err := t.processWriteRule(expanded, boundPaths) + writeArgs, err := t.processWriteRule(expanded, writeBoundPaths) if err != nil { log.Warnf("Failed to process allow_write rule '%s': %v", expanded, err) continue @@ -210,7 +222,7 @@ func (t *bubblewrapPolicyTranslator) translateFilesystem(policy *sandbox.Sandbox allowGitConfig := utils.SafelyGetValue(policy.AllowGitConfig) denyPatterns := append([]string{}, policy.Filesystem.DenyWrite...) - // Add mandatory deny patterns + // Add mandatory deny patterns (credentials - these get completely hidden) mandatoryDenies := util.GetMandatoryDenyPatterns(allowGitConfig) denyPatterns = append(denyPatterns, mandatoryDenies...) @@ -230,6 +242,69 @@ func (t *bubblewrapPolicyTranslator) translateFilesystem(policy *sandbox.Sandbox args = append(args, denyArgs...) } + // 4. Process mandatory credential directories - completely hide them with tmpfs + // This blocks both read AND write access (more secure than read-only mount) + hiddenDirs := make(map[string]bool) // Track to avoid duplicates + for _, pattern := range mandatoryDenies { + expanded, err := util.ExpandVariables(pattern) + if err != nil { + continue + } + + var dirsToHide []string + if util.ContainsGlob(expanded) { + // Expand glob pattern to find matching directories + matches, err := filepath.Glob(expanded) + if err != nil { + continue + } + dirsToHide = matches + } else { + dirsToHide = []string{expanded} + } + + for _, dir := range dirsToHide { + if hiddenDirs[dir] { + continue + } + if info, err := os.Stat(dir); err == nil && info.IsDir() { + args = append(args, "--tmpfs", dir) + hiddenDirs[dir] = true + log.Debugf("Hiding credential directory '%s' with tmpfs", dir) + } + } + } + + // 5. Process deny_exec rules (mount /dev/null over executables) + for _, exePath := range policy.Process.DenyExec { + expanded, err := util.ExpandVariables(exePath) + if err != nil { + log.Warnf("Failed to expand variables in deny_exec pattern '%s': %v", exePath, err) + continue + } + + // Handle glob patterns (e.g., /usr/bin/python*) + if util.ContainsGlob(expanded) { + matches, err := filepath.Glob(expanded) + if err != nil { + log.Warnf("Failed to expand deny_exec glob '%s': %v", expanded, err) + continue + } + for _, match := range matches { + if info, err := os.Stat(match); err == nil && !info.IsDir() { + args = append(args, "--ro-bind", "/dev/null", match) + log.Debugf("Blocked execution of '%s'", match) + } + } + } else { + // Literal path + if info, err := os.Stat(expanded); err == nil && !info.IsDir() { + args = append(args, "--ro-bind", "/dev/null", expanded) + log.Debugf("Blocked execution of '%s'", expanded) + } + } + } + return args, nil } @@ -239,6 +314,13 @@ func (t *bubblewrapPolicyTranslator) processReadRule(path string, boundPaths map // Check if path contains glob pattern if util.ContainsGlob(path) { + // Check if the base directory is already bound + baseDir := t.extractParentDir(path) + if boundPaths[baseDir] { + log.Debugf("Skipping pattern '%s' - base directory '%s' already bound", path, baseDir) + return args, nil + } + // Expand glob pattern to concrete paths with fallback detection paths, useFallback, err := t.expandGlobPattern(path, t.config.maxGlobDepth, t.config.maxGlobPaths) if err != nil { @@ -280,6 +362,13 @@ 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) + if boundPaths[baseDir] { + log.Debugf("Skipping pattern '%s' - base directory '%s' already bound", path, baseDir) + return args, nil + } + // Expand glob pattern to concrete paths with fallback detection paths, useFallback, err := t.expandGlobPattern(path, t.config.maxGlobDepth, t.config.maxGlobPaths) if err != nil { @@ -301,14 +390,25 @@ func (t *bubblewrapPolicyTranslator) processWriteRule(path string, boundPaths ma } else { // Fine-grained: bind individual paths for _, p := range paths { - if !boundPaths[p] { - args = append(args, "--bind-try", p, p) - boundPaths[p] = true + // Check if path exists - if not, bind parent directory instead + // This allows creating new directories (e.g., node_modules/** when node_modules doesn't exist) + pathToBind := p + if _, err := os.Stat(p); os.IsNotExist(err) { + parentDir := filepath.Dir(p) + if parentDir != "" && parentDir != "." && parentDir != "/" { + pathToBind = parentDir + log.Debugf("Path '%s' doesn't exist, binding parent '%s' as writable to allow creation", p, parentDir) + } + } + + if !boundPaths[pathToBind] { + args = append(args, "--bind-try", pathToBind, pathToBind) + boundPaths[pathToBind] = true } else { - // Path already bound as read-only, upgrade to read-write - // This is a limitation of the simple approach - we'd need to track - // and replace the previous bind. For now, log warning. - log.Warnf("Path '%s' already bound, cannot upgrade to read-write", p) + // Path already bound, add another bind to upgrade to read-write + // bwrap: later mounts override earlier ones + args = append(args, "--bind-try", pathToBind, pathToBind) + log.Debugf("Path '%s' already bound, adding write bind to override", pathToBind) } } } @@ -340,20 +440,36 @@ func (t *bubblewrapPolicyTranslator) processDenyRule(path string) ([]string, err } for _, p := range paths { - // Mount /dev/null to prevent access - args = append(args, "--ro-bind", "/dev/null", p) + info, err := os.Stat(p) + if err == nil { + if info.IsDir() { + // For directories, mount as read-only to prevent writes + // This overrides any previous writable bind of parent directories + args = append(args, "--ro-bind-try", p, p) + log.Debugf("Deny rule: mounted directory '%s' as read-only", p) + } else { + // For files, mount /dev/null to prevent access + args = append(args, "--ro-bind", "/dev/null", p) + } + } } } else { // For literal paths, check if they exist - if _, err := os.Stat(path); err == nil { - // File exists - mount /dev/null over it - args = append(args, "--ro-bind", "/dev/null", path) - } else if os.IsNotExist(err) { - // File doesn't exist - find first non-existent ancestor and block it - nonExistentPath := t.findFirstNonExistentPath(path) - if nonExistentPath != "" { - args = append(args, "--ro-bind", "/dev/null", nonExistentPath) + if info, err := os.Stat(path); err == nil { + if info.IsDir() { + // For directories, mount as read-only to prevent writes + // This overrides any previous writable bind of parent directories + args = append(args, "--ro-bind-try", path, path) + log.Debugf("Deny rule: mounted directory '%s' as read-only", path) + } else { + // File exists - mount /dev/null over it + args = append(args, "--ro-bind", "/dev/null", path) } + } else if os.IsNotExist(err) { + // File doesn't exist - skip blocking it + // Rationale: bwrap cannot create mount points in read-only parent directories. + // Non-existent files are already protected by deny-by-default (not in allow_write). + log.Debugf("Deny rule: skipping non-existent path '%s' (already protected by deny-by-default)", path) } }