diff --git a/internal/sandbox/integration_linux_test.go b/internal/sandbox/integration_linux_test.go index b57417c..da948a3 100644 --- a/internal/sandbox/integration_linux_test.go +++ b/internal/sandbox/integration_linux_test.go @@ -426,6 +426,52 @@ func TestLinux_DenyReadTakesPrecedenceOverMandatoryDangerousPath(t *testing.T) { assertBlocked(t, result) } +// TestLinux_DenyReadTakesPrecedenceOverSamePathDenyWrite verifies that masking a +// directory still wins when the same directory also appears in denyWrite. +func TestLinux_DenyReadTakesPrecedenceOverSamePathDenyWrite(t *testing.T) { + skipIfAlreadySandboxed(t) + + workspace := createTempWorkspace(t) + fakeHome := createTempWorkspace(t) + t.Setenv("HOME", fakeHome) + + sshDir := filepath.Join(fakeHome, ".ssh") + if err := os.MkdirAll(sshDir, 0o700); err != nil { + t.Fatalf("failed to create ssh dir: %v", err) + } + keyPath := createTestFile(t, sshDir, "id_rsa", "secret key") + + cfg := testConfigWithWorkspace(workspace) + cfg.Filesystem.DenyRead = []string{"~/.ssh"} + cfg.Filesystem.DenyWrite = []string{"~/.ssh"} + + result := runUnderLinuxSandboxDirect(t, cfg, "cat "+keyPath, workspace) + assertBlocked(t, result) +} + +// TestLinux_DenyReadMaskedAncestorBeatsChildDenyWrite verifies that a later +// denyWrite self-bind cannot puncture a directory already masked by denyRead. +func TestLinux_DenyReadMaskedAncestorBeatsChildDenyWrite(t *testing.T) { + skipIfAlreadySandboxed(t) + + workspace := createTempWorkspace(t) + fakeHome := createTempWorkspace(t) + t.Setenv("HOME", fakeHome) + + sshDir := filepath.Join(fakeHome, ".ssh") + if err := os.MkdirAll(sshDir, 0o700); err != nil { + t.Fatalf("failed to create ssh dir: %v", err) + } + keyPath := createTestFile(t, sshDir, "id_rsa", "secret key") + + cfg := testConfigWithWorkspace(workspace) + cfg.Filesystem.DenyRead = []string{"~/.ssh"} + cfg.Filesystem.DenyWrite = []string{"~/.ssh/id_rsa"} + + result := runUnderLinuxSandboxDirect(t, cfg, "cat "+keyPath, workspace) + assertBlocked(t, result) +} + // ============================================================================ // Network Blocking Tests // ============================================================================ diff --git a/internal/sandbox/linux.go b/internal/sandbox/linux.go index e807098..3d49ba2 100644 --- a/internal/sandbox/linux.go +++ b/internal/sandbox/linux.go @@ -280,17 +280,7 @@ func isSymlink(path string) bool { return info.Mode()&os.ModeSymlink != 0 } -// canMountOver returns true if bwrap can safely mount over this path. -// Returns false for symlinks (target may not exist in sandbox) and -// other special cases that could cause mount failures. -func canMountOver(path string) bool { - if isSymlink(path) { - return false - } - return fileExists(path) -} - -// resolvePathForMount canonicalizes a path before a self-bind mount. +// resolvePathForMount canonicalizes a path before mounting over it. // bubblewrap can fail when destination paths include symlink components // (common on usr-merged distros, e.g. /bin -> /usr/bin), so always prefer the // fully-resolved path. @@ -1065,116 +1055,7 @@ func WrapCommandLinuxWithOptions(cfg *config.Config, command string, bridge *Lin } } - // Track explicit denyRead paths so they always keep precedence over - // mandatory dangerous-path write protection. - denyReadPaths := make(map[string]bool) - - // Handle denyRead paths - hide them - // For directories: use --tmpfs to replace with empty tmpfs - // For files: use --ro-bind /dev/null to mask with empty file - // Skip symlinks: they may point outside the sandbox and cause mount errors - if cfg != nil && cfg.Filesystem.DenyRead != nil { - expandedDenyRead := ExpandGlobPatterns(cfg.Filesystem.DenyRead) - for _, p := range expandedDenyRead { - denyReadPaths[p] = true - if canMountOver(p) { - if isDirectory(p) { - bwrapArgs = append(bwrapArgs, "--tmpfs", p) - } else { - // Mask file with /dev/null (appears as empty, unreadable) - bwrapArgs = append(bwrapArgs, "--ro-bind", "/dev/null", p) - } - } - } - - // Add non-glob paths - for _, p := range cfg.Filesystem.DenyRead { - normalized := NormalizePath(p) - if !ContainsGlobChars(normalized) { - denyReadPaths[normalized] = true - } - if !ContainsGlobChars(normalized) && canMountOver(normalized) { - if isDirectory(normalized) { - bwrapArgs = append(bwrapArgs, "--tmpfs", normalized) - } else { - bwrapArgs = append(bwrapArgs, "--ro-bind", "/dev/null", normalized) - } - } - } - } - - // Apply mandatory dangerous-path write protection. - // In defaultDenyRead mode, never rebind the real path because that would - // make hidden files readable; mask with /dev/null or empty tmpfs instead. - // - // getMandatoryDenyPaths covers: cwd-level files, home dir files, and a - // depth-limited walk (DefaultMaxDangerousFileDepth levels) to find dangerous - // files in subdirectories without full tree walks that hang on large dirs. - allowGitConfig := cfg != nil && cfg.Filesystem.AllowGitConfig - mandatoryDeny := getMandatoryDenyPaths(cwd, allowGitConfig) - - // Deduplicate - seen := make(map[string]bool) - for _, p := range mandatoryDeny { - if denyReadPaths[p] { - // Respect explicit denyRead precedence. - continue - } - mountPath, ok := resolvePathForMount(p) - if !ok || denyReadPaths[mountPath] { - continue - } - if !seen[mountPath] { - seen[mountPath] = true - seen[p] = true - if defaultDenyRead { - if isDirectory(mountPath) { - bwrapArgs = append(bwrapArgs, "--tmpfs", mountPath) - } else { - bwrapArgs = append(bwrapArgs, "--ro-bind", "/dev/null", mountPath) - } - } else { - bwrapArgs = append(bwrapArgs, "--ro-bind", mountPath, mountPath) - } - } - } - - // Handle explicit denyWrite paths (make them read-only) - if cfg != nil && cfg.Filesystem.DenyWrite != nil { - expandedDenyWrite := ExpandGlobPatterns(cfg.Filesystem.DenyWrite) - for _, p := range expandedDenyWrite { - if fileExists(p) && !seen[p] { - seen[p] = true - bwrapArgs = append(bwrapArgs, "--ro-bind", p, p) - } - } - // Add non-glob paths - for _, p := range cfg.Filesystem.DenyWrite { - normalized := NormalizePath(p) - if !ContainsGlobChars(normalized) && fileExists(normalized) && !seen[normalized] { - seen[normalized] = true - bwrapArgs = append(bwrapArgs, "--ro-bind", normalized, normalized) - } - } - } - - // Runtime executable deny (applies to child processes). - // This masks resolved executable paths so execve fails even when launched - // from an allowed wrapper process (e.g., agent subprocesses). - for _, p := range deniedExecPaths { - mountPath, ok := resolvePathForMount(p) - if !ok { - if opts.Debug { - fmt.Fprintf(os.Stderr, "[fence:linux] Skipping runtime exec deny mount for %s (unmountable)\n", p) - } - continue - } - if !seen[mountPath] { - seen[mountPath] = true - seen[p] = true - bwrapArgs = append(bwrapArgs, "--ro-bind", "/dev/null", mountPath) - } - } + bwrapArgs = appendLinuxLatePolicyMounts(bwrapArgs, cfg, cwd, defaultDenyRead, deniedExecPaths, opts.Debug) // Bind the outbound Unix sockets into the sandbox (need to be writable) if bridge != nil { diff --git a/internal/sandbox/linux_mount_planner.go b/internal/sandbox/linux_mount_planner.go new file mode 100644 index 0000000..673c852 --- /dev/null +++ b/internal/sandbox/linux_mount_planner.go @@ -0,0 +1,250 @@ +//go:build linux + +package sandbox + +import ( + "fmt" + "os" + "path/filepath" + "slices" + "strings" + + "github.com/Use-Tusk/fence/internal/config" +) + +type linuxLateMountKind int + +const ( + linuxLateMountReadOnly linuxLateMountKind = iota + linuxLateMountMaskFile + linuxLateMountMaskDir +) + +type linuxLateMount struct { + Path string + Kind linuxLateMountKind +} + +type linuxLateMountPlanner struct { + mounts []linuxLateMount +} + +func newLinuxLateMountPlanner() *linuxLateMountPlanner { + return &linuxLateMountPlanner{} +} + +func (p *linuxLateMountPlanner) Add(path string, kind linuxLateMountKind) { + path = filepath.Clean(path) + if path == "" || path == "." { + return + } + + if p.hasStrictAncestor(path, linuxLateMountMaskDir) { + return + } + + for i, existing := range p.mounts { + if existing.Path != path { + continue + } + if linuxLateMountPriority(existing.Kind) >= linuxLateMountPriority(kind) { + return + } + + p.mounts[i].Kind = kind + switch kind { + case linuxLateMountMaskDir: + p.removeDescendants(path, func(mount linuxLateMount) bool { + return true + }) + case linuxLateMountReadOnly: + p.removeDescendants(path, func(mount linuxLateMount) bool { + return mount.Kind == linuxLateMountReadOnly + }) + } + return + } + + if kind == linuxLateMountReadOnly && p.hasStrictAncestor(path, linuxLateMountReadOnly) { + return + } + + switch kind { + case linuxLateMountMaskDir: + p.removeDescendants(path, func(mount linuxLateMount) bool { + return true + }) + case linuxLateMountReadOnly: + p.removeDescendants(path, func(mount linuxLateMount) bool { + return mount.Kind == linuxLateMountReadOnly + }) + } + + p.mounts = append(p.mounts, linuxLateMount{Path: path, Kind: kind}) +} + +func (p *linuxLateMountPlanner) hasStrictAncestor(path string, kind linuxLateMountKind) bool { + for _, mount := range p.mounts { + if mount.Kind != kind || mount.Path == path { + continue + } + if linuxPathContains(mount.Path, path) { + return true + } + } + return false +} + +func (p *linuxLateMountPlanner) removeDescendants(path string, shouldRemove func(linuxLateMount) bool) { + filtered := p.mounts[:0] + for _, mount := range p.mounts { + if mount.Path != path && linuxPathContains(path, mount.Path) && shouldRemove(mount) { + continue + } + filtered = append(filtered, mount) + } + p.mounts = filtered +} + +func (p *linuxLateMountPlanner) Mounts() []linuxLateMount { + mounts := slices.Clone(p.mounts) + slices.SortFunc(mounts, func(a, b linuxLateMount) int { + depthA := linuxLateMountDepth(a.Path) + depthB := linuxLateMountDepth(b.Path) + if depthA != depthB { + return depthA - depthB + } + return strings.Compare(a.Path, b.Path) + }) + return mounts +} + +func appendLinuxLateMounts(args []string, mounts []linuxLateMount) []string { + for _, mount := range mounts { + switch mount.Kind { + case linuxLateMountMaskDir: + args = append(args, "--tmpfs", mount.Path) + case linuxLateMountMaskFile: + args = append(args, "--ro-bind", "/dev/null", mount.Path) + case linuxLateMountReadOnly: + args = append(args, "--ro-bind", mount.Path, mount.Path) + } + } + return args +} + +func linuxLateMountPriority(kind linuxLateMountKind) int { + switch kind { + case linuxLateMountMaskDir: + return 3 + case linuxLateMountMaskFile: + return 2 + case linuxLateMountReadOnly: + return 1 + default: + return 0 + } +} + +func linuxLateMountDepth(path string) int { + path = filepath.Clean(path) + trimmed := strings.Trim(path, string(filepath.Separator)) + if trimmed == "" { + return 0 + } + return strings.Count(trimmed, string(filepath.Separator)) + 1 +} + +func linuxPathContains(ancestor, path string) bool { + ancestor = filepath.Clean(ancestor) + path = filepath.Clean(path) + + if ancestor == path { + return true + } + if ancestor == string(filepath.Separator) { + return strings.HasPrefix(path, string(filepath.Separator)) + } + return strings.HasPrefix(path, ancestor+string(filepath.Separator)) +} + +func collectResolvedLinuxLateMountPaths(patterns []string) []string { + if len(patterns) == 0 { + return nil + } + + var paths []string + seen := make(map[string]bool) + for _, path := range ExpandGlobPatterns(patterns) { + mountPath, ok := resolvePathForMount(path) + if !ok { + continue + } + mountPath = filepath.Clean(mountPath) + if seen[mountPath] { + continue + } + seen[mountPath] = true + paths = append(paths, mountPath) + } + return paths +} + +// appendLinuxLatePolicyMounts plans the final policy overlays with subtree-aware +// precedence so masked directories cannot be punctured by later self-binds. +func appendLinuxLatePolicyMounts( + bwrapArgs []string, + cfg *config.Config, + cwd string, + defaultDenyRead bool, + deniedExecPaths []string, + debug bool, +) []string { + planner := newLinuxLateMountPlanner() + + if cfg != nil { + for _, mountPath := range collectResolvedLinuxLateMountPaths(cfg.Filesystem.DenyRead) { + if isDirectory(mountPath) { + planner.Add(mountPath, linuxLateMountMaskDir) + } else { + planner.Add(mountPath, linuxLateMountMaskFile) + } + } + } + + allowGitConfig := cfg != nil && cfg.Filesystem.AllowGitConfig + for _, path := range getMandatoryDenyPaths(cwd, allowGitConfig) { + mountPath, ok := resolvePathForMount(path) + if !ok { + continue + } + if defaultDenyRead { + if isDirectory(mountPath) { + planner.Add(mountPath, linuxLateMountMaskDir) + } else { + planner.Add(mountPath, linuxLateMountMaskFile) + } + continue + } + planner.Add(mountPath, linuxLateMountReadOnly) + } + + if cfg != nil { + for _, mountPath := range collectResolvedLinuxLateMountPaths(cfg.Filesystem.DenyWrite) { + planner.Add(mountPath, linuxLateMountReadOnly) + } + } + + for _, path := range deniedExecPaths { + mountPath, ok := resolvePathForMount(path) + if !ok { + if debug { + fmt.Fprintf(os.Stderr, "[fence:linux] Skipping runtime exec deny mount for %s (unmountable)\n", path) + } + continue + } + planner.Add(mountPath, linuxLateMountMaskFile) + } + + return appendLinuxLateMounts(bwrapArgs, planner.Mounts()) +} diff --git a/internal/sandbox/linux_test.go b/internal/sandbox/linux_test.go index e832d87..c4ac7d1 100644 --- a/internal/sandbox/linux_test.go +++ b/internal/sandbox/linux_test.go @@ -432,3 +432,166 @@ func TestWrapCommandLinuxWithOptions_RootBindPrecedesSpecialMounts(t *testing.T) t.Fatalf("expected root bind to appear before device passthroughs: %s", cmd) } } + +func TestLinuxLateMountPlanner_MaskedAncestorWinsOverChildReadOnly(t *testing.T) { + planner := newLinuxLateMountPlanner() + planner.Add("/tmp/secret", linuxLateMountMaskDir) + planner.Add("/tmp/secret/id_rsa", linuxLateMountReadOnly) + + mounts := planner.Mounts() + if len(mounts) != 1 { + t.Fatalf("expected a single mount, got %#v", mounts) + } + if mounts[0].Path != "/tmp/secret" || mounts[0].Kind != linuxLateMountMaskDir { + t.Fatalf("expected masked ancestor to win, got %#v", mounts) + } +} + +func TestLinuxLateMountPlanner_ReadOnlyAncestorKeepsChildMask(t *testing.T) { + planner := newLinuxLateMountPlanner() + planner.Add("/tmp/tools", linuxLateMountReadOnly) + planner.Add("/tmp/tools/python3", linuxLateMountMaskFile) + + mounts := planner.Mounts() + if len(mounts) != 2 { + t.Fatalf("expected parent read-only mount and child mask, got %#v", mounts) + } + if mounts[0].Path != "/tmp/tools" || mounts[0].Kind != linuxLateMountReadOnly { + t.Fatalf("expected read-only parent mount first, got %#v", mounts) + } + if mounts[1].Path != "/tmp/tools/python3" || mounts[1].Kind != linuxLateMountMaskFile { + t.Fatalf("expected child mask to remain after parent read-only mount, got %#v", mounts) + } +} + +func TestLinuxLateMountPlanner_ReadOnlyAncestorPrunesChildReadOnly(t *testing.T) { + planner := newLinuxLateMountPlanner() + planner.Add("/tmp/secret/id_rsa", linuxLateMountReadOnly) + planner.Add("/tmp/secret", linuxLateMountReadOnly) + + mounts := planner.Mounts() + if len(mounts) != 1 { + t.Fatalf("expected redundant child read-only mount to be removed, got %#v", mounts) + } + if mounts[0].Path != "/tmp/secret" || mounts[0].Kind != linuxLateMountReadOnly { + t.Fatalf("expected parent read-only mount to remain, got %#v", mounts) + } +} + +func TestWrapCommandLinuxWithOptions_DenyReadDirectoryWinsOverSamePathDenyWrite(t *testing.T) { + if _, err := exec.LookPath("bwrap"); err != nil { + t.Skip("bwrap not available") + } + + workspace := t.TempDir() + secretDir := filepath.Join(workspace, "secret") + if err := os.MkdirAll(secretDir, 0o700); err != nil { + t.Fatalf("failed to create secret dir: %v", err) + } + + cfg := &config.Config{ + Filesystem: config.FilesystemConfig{ + DenyRead: []string{secretDir}, + DenyWrite: []string{secretDir}, + }, + } + + cmd, err := WrapCommandLinuxWithOptions(cfg, "echo ok", nil, nil, LinuxSandboxOptions{ + UseLandlock: false, + UseSeccomp: false, + UseEBPF: false, + ShellMode: ShellModeDefault, + }) + if err != nil { + t.Fatalf("WrapCommandLinuxWithOptions failed: %v", err) + } + + maskFragment := ShellQuote([]string{"--tmpfs", secretDir}) + rebindFragment := ShellQuote([]string{"--ro-bind", secretDir, secretDir}) + if !strings.Contains(cmd, maskFragment) { + t.Fatalf("expected denyRead directory mask in command: %s", cmd) + } + if strings.Contains(cmd, rebindFragment) { + t.Fatalf("did not expect denyWrite to rebind a masked directory: %s", cmd) + } +} + +func TestWrapCommandLinuxWithOptions_DenyReadDirectoryWinsOverChildDenyWrite(t *testing.T) { + if _, err := exec.LookPath("bwrap"); err != nil { + t.Skip("bwrap not available") + } + + workspace := t.TempDir() + secretDir := filepath.Join(workspace, "secret") + if err := os.MkdirAll(secretDir, 0o700); err != nil { + t.Fatalf("failed to create secret dir: %v", err) + } + secretFile := filepath.Join(secretDir, "id_rsa") + if err := os.WriteFile(secretFile, []byte("secret"), 0o600); err != nil { + t.Fatalf("failed to create secret file: %v", err) + } + + cfg := &config.Config{ + Filesystem: config.FilesystemConfig{ + DenyRead: []string{secretDir}, + DenyWrite: []string{secretFile}, + }, + } + + cmd, err := WrapCommandLinuxWithOptions(cfg, "echo ok", nil, nil, LinuxSandboxOptions{ + UseLandlock: false, + UseSeccomp: false, + UseEBPF: false, + ShellMode: ShellModeDefault, + }) + if err != nil { + t.Fatalf("WrapCommandLinuxWithOptions failed: %v", err) + } + + maskFragment := ShellQuote([]string{"--tmpfs", secretDir}) + rebindFragment := ShellQuote([]string{"--ro-bind", secretFile, secretFile}) + if !strings.Contains(cmd, maskFragment) { + t.Fatalf("expected denyRead directory mask in command: %s", cmd) + } + if strings.Contains(cmd, rebindFragment) { + t.Fatalf("did not expect denyWrite child rebind under a masked directory: %s", cmd) + } +} + +func TestWrapCommandLinuxWithOptions_DenyReadFileWinsOverSamePathDenyWrite(t *testing.T) { + if _, err := exec.LookPath("bwrap"); err != nil { + t.Skip("bwrap not available") + } + + workspace := t.TempDir() + secretFile := filepath.Join(workspace, "secret.txt") + if err := os.WriteFile(secretFile, []byte("secret"), 0o600); err != nil { + t.Fatalf("failed to create secret file: %v", err) + } + + cfg := &config.Config{ + Filesystem: config.FilesystemConfig{ + DenyRead: []string{secretFile}, + DenyWrite: []string{secretFile}, + }, + } + + cmd, err := WrapCommandLinuxWithOptions(cfg, "echo ok", nil, nil, LinuxSandboxOptions{ + UseLandlock: false, + UseSeccomp: false, + UseEBPF: false, + ShellMode: ShellModeDefault, + }) + if err != nil { + t.Fatalf("WrapCommandLinuxWithOptions failed: %v", err) + } + + maskFragment := ShellQuote([]string{"--ro-bind", "/dev/null", secretFile}) + rebindFragment := ShellQuote([]string{"--ro-bind", secretFile, secretFile}) + if !strings.Contains(cmd, maskFragment) { + t.Fatalf("expected denyRead file mask in command: %s", cmd) + } + if strings.Contains(cmd, rebindFragment) { + t.Fatalf("did not expect denyWrite to rebind a masked file: %s", cmd) + } +}