fix: Avoid bind mount for non-existentent deny protection

sandbox/platform/bubblewrap_translator_linux.go

@@ -224,14 +224,7 @@ func (t *bubblewrapPolicyTranslator) translateFilesystem(policy *sandbox.Sandbox
 		args = append(args, writeArgs...)
 	}
 
-	// Collect allowed write paths for deny rule processing
-	// This is used to determine if non-existent deny paths need to be blocked
-	allowedWritePaths := make([]string, 0, len(writeBoundPaths))
-	for path := range writeBoundPaths {
-		allowedWritePaths = append(allowedWritePaths, path)
-	}
-
-	// 3. Process deny_write rules (mount /dev/null to prevent creation)
+	// 3. Process deny_write rules (mount /dev/null to prevent access)
 	allowGitConfig := utils.SafelyGetValue(policy.AllowGitConfig)
 	denyPatterns := append([]string{}, policy.Filesystem.DenyWrite...)
 
@@ -246,7 +239,7 @@ func (t *bubblewrapPolicyTranslator) translateFilesystem(policy *sandbox.Sandbox
 			continue
 		}
 
-		denyArgs, err := t.processDenyRule(expanded, allowedWritePaths)
+		denyArgs, err := t.processDenyRule(expanded)
 		if err != nil {
 			log.Debugf("Deny rule '%s' skipped: %v", expanded, err)
 			continue
@@ -437,10 +430,9 @@ func (t *bubblewrapPolicyTranslator) processWriteRule(path string, boundPaths ma
 	return args, nil
 }
 
-// processDenyRule handles deny rules by mounting /dev/null to prevent file creation.
+// processDenyRule handles deny rules by mounting /dev/null to prevent file access.
 // This technique is borrowed from Anthropic's sandbox-runtime.
-// allowedWritePaths is used to determine if non-existent paths need to be blocked.
-func (t *bubblewrapPolicyTranslator) processDenyRule(path string, allowedWritePaths []string) ([]string, error) {
+func (t *bubblewrapPolicyTranslator) processDenyRule(path string) ([]string, error) {
 	args := []string{}
 
 	// For glob patterns, expand and deny each path
@@ -481,18 +473,14 @@ func (t *bubblewrapPolicyTranslator) processDenyRule(path string, allowedWritePa
 				args = append(args, "--ro-bind", "/dev/null", path)
 			}
 		} else if os.IsNotExist(err) {
-			// File doesn't exist - check if it's within an allowed write path
-			// If so, we need to block creation by mounting /dev/null at first non-existent component
-			if isWithinAllowedWritePath(path, allowedWritePaths) {
-				firstNonExistent := t.findFirstNonExistentPath(path)
-				if firstNonExistent != "" {
-					args = append(args, "--ro-bind", "/dev/null", firstNonExistent)
-					log.Debugf("Deny rule: blocking creation of '%s' by mounting /dev/null at '%s'", path, firstNonExistent)
-				}
-			} else {
-				// Not within allowed write paths - already protected by deny-by-default
-				log.Debugf("Deny rule: skipping '%s' - not within allowed write paths (protected by deny-by-default)", path)
-			}
+			// File doesn't exist - skip it
+			// IMPORTANT: We cannot use --ro-bind /dev/null for non-existent paths because
+			// bwrap creates the file on the host filesystem as a mount point, which leaves
+			// empty files (.env, .aws, etc.) in the user's directory after sandbox exits.
+			// Non-existent files are harmless (no secrets to leak), and blocking creation
+			// in writable directories isn't critical since an attacker creating an empty
+			// .env is not a security threat.
+			log.Debugf("Deny rule: skipping non-existent path '%s' (bwrap would create empty file as mount point)", path)
 		}
 	}
 
@@ -522,19 +510,6 @@ func (t *bubblewrapPolicyTranslator) findFirstNonExistentPath(path string) strin
 	return ""
 }
 
-// isWithinAllowedWritePath checks if a target path is within any of the allowed write paths.
-// This is used to determine if a non-existent deny path needs to be blocked.
-func isWithinAllowedWritePath(targetPath string, allowedWritePaths []string) bool {
-	targetPath = filepath.Clean(targetPath)
-	for _, allowedPath := range allowedWritePaths {
-		allowedPath = filepath.Clean(allowedPath)
-		if targetPath == allowedPath || strings.HasPrefix(targetPath, allowedPath+"/") {
-			return true
-		}
-	}
-	return false
-}
-
 // expandGlobPattern expands a glob pattern to a list of concrete paths.
 // Implements depth limiting and path count limiting to prevent DoS.
 // Returns (paths, useFallback, error) where useFallback indicates if

sandbox/platform/bubblewrap_translator_linux_test.go

@@ -506,35 +506,11 @@ func TestBubblewrapTranslatorProcessDenyRule(t *testing.T) {
 		assert.Contains(t, argsStr, testFile)
 	})
 
-	t.Run("non-existent file outside allowed write paths is skipped", func(t *testing.T) {
-		// Use a path that is truly outside any allowed write path
-		// Note: /tmp is always in allowedWritePaths due to tmpdir support
-		// So we use a path under /nonexistent which doesn't exist
-		nonExistentPath := "/nonexistent/completely/fake/path/.env"
-
-		policy := &sandbox.SandboxPolicy{
-			Filesystem: sandbox.FilesystemPolicy{
-				DenyWrite: []string{nonExistentPath},
-				// No AllowWrite - path is not within any allowed write area
-			},
-		}
-
-		config := newDefaultBubblewrapConfig()
-		translator := newBubblewrapPolicyTranslator(config)
-		args, err := translator.translate(policy)
-		require.NoError(t, err)
-
-		argsStr := argSliceToString(args)
-
-		// Non-existent file should NOT be in args (protected by deny-by-default)
-		// Path is not within allowed write paths, so no need to block it
-		assert.NotContains(t, argsStr, nonExistentPath)
-	})
-
-	t.Run("non-existent file within allowed write path is blocked", func(t *testing.T) {
+	t.Run("non-existent file is skipped to avoid creating empty files", func(t *testing.T) {
 		tmpDir := t.TempDir()
 
-		// Non-existent path WITHIN an allowed write path
+		// Non-existent file - should be skipped because using --ro-bind /dev/null
+		// on non-existent paths causes bwrap to create the file as a mount point
 		nonExistentPath := filepath.Join(tmpDir, ".env")
 
 		policy := &sandbox.SandboxPolicy{
@@ -551,29 +527,21 @@ func TestBubblewrapTranslatorProcessDenyRule(t *testing.T) {
 
 		argsStr := argSliceToString(args)
 
-		// Non-existent file within allowed write path should be blocked
-		// by mounting /dev/null at the first non-existent component
-		assert.Contains(t, argsStr, "--ro-bind")
-		assert.Contains(t, argsStr, "/dev/null")
-		assert.Contains(t, argsStr, nonExistentPath)
+		// Non-existent file should NOT be in args
+		// Using --ro-bind /dev/null on non-existent paths creates empty files
+		assert.NotContains(t, argsStr, nonExistentPath)
 	})
 
-	t.Run("deeply nested non-existent path is blocked at first missing component", func(t *testing.T) {
+	t.Run("existing directory is mounted read-only", func(t *testing.T) {
 		tmpDir := t.TempDir()
 
-		// Create a subdirectory that exists
-		existingDir := filepath.Join(tmpDir, "existing")
-		require.NoError(t, os.MkdirAll(existingDir, 0755))
-
-		// Deeply nested non-existent path: existing/nonexistent/deep/.env
-		// First non-existent component is "nonexistent"
-		nonExistentPath := filepath.Join(existingDir, "nonexistent", "deep", ".env")
-		firstNonExistent := filepath.Join(existingDir, "nonexistent")
+		// Create a directory
+		testDir := filepath.Join(tmpDir, "secrets")
+		require.NoError(t, os.MkdirAll(testDir, 0755))
 
 		policy := &sandbox.SandboxPolicy{
 			Filesystem: sandbox.FilesystemPolicy{
-				AllowWrite: []string{tmpDir}, // Allow writes to tmpDir
-				DenyWrite:  []string{nonExistentPath},
+				DenyWrite: []string{testDir},
 			},
 		}
 
@@ -584,10 +552,9 @@ func TestBubblewrapTranslatorProcessDenyRule(t *testing.T) {
 
 		argsStr := argSliceToString(args)
 
-		// Should mount /dev/null at the first non-existent component
-		assert.Contains(t, argsStr, "--ro-bind")
-		assert.Contains(t, argsStr, "/dev/null")
-		assert.Contains(t, argsStr, firstNonExistent)
+		// Existing directory should be mounted read-only
+		assert.Contains(t, argsStr, "--ro-bind-try")
+		assert.Contains(t, argsStr, testDir)
 	})
 }