fix: Allow explicit override over deny patterns (#177)

* fix: Allow explicit override over deny patterns

* test: Non-glob expansion for overrides is expected

sandbox/executor/apply.go

@@ -156,22 +156,26 @@ func ApplySandbox(ctx context.Context, cmd *exec.Cmd, pmName string, opts ...app
 }
 
 // applyRuntimeOverrides applies --sandbox-allow overrides to the policy.
-// Overrides are additive — they only append to allow lists, never modify deny lists.
-// Warnings are logged for conflicts with deny rules and mandatory deny patterns.
+// Overrides append to allow lists and remove exact matches from corresponding deny lists
+// so that deny rules don't shadow the explicit override. Only full-path exact matches are
+// removed — glob and wildcard deny patterns are never modified to stay secure by default.
 func applyRuntimeOverrides(policy *sandbox.SandboxPolicy, overrides []config.SandboxAllowOverride) {
 	for _, override := range overrides {
 		switch override.Type {
 		case config.SandboxAllowRead:
 			log.Infof("Sandbox override: allowing read access to %s", override.Value)
 			policy.Filesystem.AllowRead = append(policy.Filesystem.AllowRead, override.Value)
+			policy.Filesystem.DenyRead = removeExactMatch(policy.Filesystem.DenyRead, override.Value)
 
 		case config.SandboxAllowWrite:
 			log.Infof("Sandbox override: allowing write access to %s", override.Value)
 			policy.Filesystem.AllowWrite = append(policy.Filesystem.AllowWrite, override.Value)
+			policy.Filesystem.DenyWrite = removeExactMatch(policy.Filesystem.DenyWrite, override.Value)
 
 		case config.SandboxAllowExec:
 			log.Infof("Sandbox override: allowing execution of %s", override.Value)
 			policy.Process.AllowExec = append(policy.Process.AllowExec, override.Value)
+			policy.Process.DenyExec = removeExactMatch(policy.Process.DenyExec, override.Value)
 
 		case config.SandboxAllowNetConnect:
 			log.Infof("Sandbox override: allowing outbound connection to %s", override.Value)
@@ -188,6 +192,23 @@ func applyRuntimeOverrides(policy *sandbox.SandboxPolicy, overrides []config.San
 	}
 }
 
+// removeExactMatch removes entries from the slice that exactly match the given value.
+// Glob patterns and wildcards in the slice are never matched. Only literal string
+// equality is used. This keeps broad deny rules intact while allowing targeted overrides.
+func removeExactMatch(slice []string, value string) []string {
+	result := make([]string, 0, len(slice))
+	for _, entry := range slice {
+		if entry == value {
+			log.Infof("Sandbox override: removing conflicting deny rule for %s", value)
+			continue
+		}
+
+		result = append(result, entry)
+	}
+
+	return result
+}
+
 // logSandboxOverridesToEventLog records sandbox allow overrides in the audit event log.
 func logSandboxOverridesToEventLog(profileName string, overrides []config.SandboxAllowOverride) {
 	entries := make([]map[string]string, 0, len(overrides))

sandbox/executor/apply_test.go

@@ -1,6 +1,8 @@
 package executor
 
 import (
+	"os"
+	"path/filepath"
 	"testing"
 
 	"github.com/safedep/dry/utils"
@@ -136,7 +138,7 @@ func TestApplyRuntimeOverrides_EmptyOverrides(t *testing.T) {
 	assert.Equal(t, []string{"/existing"}, policy.Filesystem.AllowWrite)
 }
 
-func TestApplyRuntimeOverrides_DenyListsUnmodified(t *testing.T) {
+func TestApplyRuntimeOverrides_DenyListsUnmodifiedWhenNoConflict(t *testing.T) {
 	policy := &sandbox.SandboxPolicy{
 		Filesystem: sandbox.FilesystemPolicy{
 			DenyWrite: []string{"/protected"},
@@ -157,9 +159,93 @@ func TestApplyRuntimeOverrides_DenyListsUnmodified(t *testing.T) {
 
 	applyRuntimeOverrides(policy, overrides)
 
-	// Deny lists should never be modified by overrides
+	// Deny lists should be unchanged when overrides don't conflict
 	assert.Equal(t, []string{"/protected"}, policy.Filesystem.DenyWrite)
 	assert.Equal(t, []string{"/usr/bin/curl"}, policy.Process.DenyExec)
 	assert.Equal(t, []string{"*:*"}, policy.Network.DenyOutbound)
 }
 
+func TestApplyRuntimeOverrides_RemovesExactDenyConflict(t *testing.T) {
+	policy := &sandbox.SandboxPolicy{
+		Filesystem: sandbox.FilesystemPolicy{
+			DenyRead:  []string{"/secret", "/other"},
+			DenyWrite: []string{"/protected", "/tmp/data"},
+		},
+		Process: sandbox.ProcessPolicy{
+			DenyExec: []string{"/usr/bin/curl", "/bin/bash"},
+		},
+	}
+
+	overrides := []config.SandboxAllowOverride{
+		{Type: config.SandboxAllowRead, Value: "/secret", Raw: "read=/secret"},
+		{Type: config.SandboxAllowWrite, Value: "/protected", Raw: "write=/protected"},
+		{Type: config.SandboxAllowExec, Value: "/bin/bash", Raw: "exec=/bin/bash"},
+	}
+
+	applyRuntimeOverrides(policy, overrides)
+
+	// Exact matches should be removed from deny lists
+	assert.Equal(t, []string{"/other"}, policy.Filesystem.DenyRead)
+	assert.Equal(t, []string{"/tmp/data"}, policy.Filesystem.DenyWrite)
+	assert.Equal(t, []string{"/usr/bin/curl"}, policy.Process.DenyExec)
+
+	// Allow lists should have the overrides
+	assert.Contains(t, policy.Filesystem.AllowRead, "/secret")
+	assert.Contains(t, policy.Filesystem.AllowWrite, "/protected")
+	assert.Contains(t, policy.Process.AllowExec, "/bin/bash")
+}
+
+func TestApplyRuntimeOverrides_PreservesGlobDenyPatterns(t *testing.T) {
+	policy := &sandbox.SandboxPolicy{
+		Filesystem: sandbox.FilesystemPolicy{
+			DenyRead:  []string{"/etc/**"},
+			DenyWrite: []string{"/usr/**"},
+		},
+		Process: sandbox.ProcessPolicy{
+			DenyExec: []string{"/usr/bin/*"},
+		},
+	}
+
+	overrides := []config.SandboxAllowOverride{
+		{Type: config.SandboxAllowRead, Value: "/etc/hosts", Raw: "read=/etc/hosts"},
+		{Type: config.SandboxAllowWrite, Value: "/usr/local/bin/tool", Raw: "write=/usr/local/bin/tool"},
+		{Type: config.SandboxAllowExec, Value: "/usr/bin/git", Raw: "exec=/usr/bin/git"},
+	}
+
+	applyRuntimeOverrides(policy, overrides)
+
+	// Glob/wildcard deny patterns must NOT be removed — only exact matches are removed
+	assert.Equal(t, []string{"/etc/**"}, policy.Filesystem.DenyRead)
+	assert.Equal(t, []string{"/usr/**"}, policy.Filesystem.DenyWrite)
+	assert.Equal(t, []string{"/usr/bin/*"}, policy.Process.DenyExec)
+}
+
+func TestApplyRuntimeOverrides_VariableDenyNotRemovedByAbsoluteOverride(t *testing.T) {
+	// Known limitation: deny entries using ${CWD} or ${HOME} variables are NOT
+	// removed by overrides that resolve to absolute paths. removeExactMatch uses
+	// literal string comparison, so "${CWD}/blocked.txt" != "/actual/cwd/blocked.txt".
+	// The override still adds the path to the allow list, but the unexpanded deny
+	// entry remains and will take precedence once the translator expands it.
+	cwd, err := os.Getwd()
+	assert.NoError(t, err)
+
+	absolutePath := filepath.Join(cwd, "blocked.txt")
+
+	policy := &sandbox.SandboxPolicy{
+		Filesystem: sandbox.FilesystemPolicy{
+			DenyWrite: []string{"${CWD}/blocked.txt"},
+		},
+	}
+
+	applyRuntimeOverrides(policy, []config.SandboxAllowOverride{
+		{Type: config.SandboxAllowWrite, Value: absolutePath, Raw: "write=./blocked.txt"},
+	})
+
+	// The override is added to the allow list
+	assert.Contains(t, policy.Filesystem.AllowWrite, absolutePath)
+
+	// But the ${CWD} deny entry is NOT removed because the strings don't match literally.
+	// This means the deny rule will still shadow the allow after variable expansion.
+	assert.Equal(t, []string{"${CWD}/blocked.txt"}, policy.Filesystem.DenyWrite)
+}
+