feat: add defaultDenyRead mode for strict filesystem isolation (#24)

This commit is contained in:
JY Tan
2026-02-01 15:11:40 -08:00
committed by GitHub
parent cef3576076
commit 7679fecf06
9 changed files with 430 additions and 11 deletions

View File

@@ -37,10 +37,12 @@ type NetworkConfig struct {
// FilesystemConfig defines filesystem restrictions. // FilesystemConfig defines filesystem restrictions.
type FilesystemConfig struct { type FilesystemConfig struct {
DenyRead []string `json:"denyRead"` DefaultDenyRead bool `json:"defaultDenyRead,omitempty"` // If true, deny reads by default except system paths and AllowRead
AllowWrite []string `json:"allowWrite"` AllowRead []string `json:"allowRead"` // Paths to allow reading (used when DefaultDenyRead is true)
DenyWrite []string `json:"denyWrite"` DenyRead []string `json:"denyRead"`
AllowGitConfig bool `json:"allowGitConfig,omitempty"` AllowWrite []string `json:"allowWrite"`
DenyWrite []string `json:"denyWrite"`
AllowGitConfig bool `json:"allowGitConfig,omitempty"`
} }
// CommandConfig defines command restrictions. // CommandConfig defines command restrictions.
@@ -179,6 +181,9 @@ func (c *Config) Validate() error {
} }
} }
if slices.Contains(c.Filesystem.AllowRead, "") {
return errors.New("filesystem.allowRead contains empty path")
}
if slices.Contains(c.Filesystem.DenyRead, "") { if slices.Contains(c.Filesystem.DenyRead, "") {
return errors.New("filesystem.denyRead contains empty path") return errors.New("filesystem.denyRead contains empty path")
} }
@@ -427,7 +432,11 @@ func Merge(base, override *Config) *Config {
}, },
Filesystem: FilesystemConfig{ Filesystem: FilesystemConfig{
// Boolean fields: true if either enables it
DefaultDenyRead: base.Filesystem.DefaultDenyRead || override.Filesystem.DefaultDenyRead,
// Append slices // Append slices
AllowRead: mergeStrings(base.Filesystem.AllowRead, override.Filesystem.AllowRead),
DenyRead: mergeStrings(base.Filesystem.DenyRead, override.Filesystem.DenyRead), DenyRead: mergeStrings(base.Filesystem.DenyRead, override.Filesystem.DenyRead),
AllowWrite: mergeStrings(base.Filesystem.AllowWrite, override.Filesystem.AllowWrite), AllowWrite: mergeStrings(base.Filesystem.AllowWrite, override.Filesystem.AllowWrite),
DenyWrite: mergeStrings(base.Filesystem.DenyWrite, override.Filesystem.DenyWrite), DenyWrite: mergeStrings(base.Filesystem.DenyWrite, override.Filesystem.DenyWrite),

View File

@@ -111,6 +111,15 @@ func TestConfigValidate(t *testing.T) {
}, },
wantErr: true, wantErr: true,
}, },
{
name: "empty allowRead path",
config: Config{
Filesystem: FilesystemConfig{
AllowRead: []string{""},
},
},
wantErr: true,
},
{ {
name: "empty denyRead path", name: "empty denyRead path",
config: Config{ config: Config{
@@ -453,6 +462,50 @@ func TestMerge(t *testing.T) {
} }
}) })
t.Run("merge defaultDenyRead and allowRead", func(t *testing.T) {
base := &Config{
Filesystem: FilesystemConfig{
DefaultDenyRead: true,
AllowRead: []string{"/home/user/project"},
},
}
override := &Config{
Filesystem: FilesystemConfig{
AllowRead: []string{"/home/user/other"},
},
}
result := Merge(base, override)
if !result.Filesystem.DefaultDenyRead {
t.Error("expected DefaultDenyRead to be true (from base)")
}
if len(result.Filesystem.AllowRead) != 2 {
t.Errorf("expected 2 allowRead paths, got %d: %v", len(result.Filesystem.AllowRead), result.Filesystem.AllowRead)
}
})
t.Run("merge defaultDenyRead from override", func(t *testing.T) {
base := &Config{
Filesystem: FilesystemConfig{
DefaultDenyRead: false,
},
}
override := &Config{
Filesystem: FilesystemConfig{
DefaultDenyRead: true,
AllowRead: []string{"/home/user/project"},
},
}
result := Merge(base, override)
if !result.Filesystem.DefaultDenyRead {
t.Error("expected DefaultDenyRead to be true (from override)")
}
if len(result.Filesystem.AllowRead) != 1 {
t.Errorf("expected 1 allowRead path, got %d", len(result.Filesystem.AllowRead))
}
})
t.Run("override ports", func(t *testing.T) { t.Run("override ports", func(t *testing.T) {
base := &Config{ base := &Config{
Network: NetworkConfig{ Network: NetworkConfig{

View File

@@ -53,6 +53,102 @@ func GetDefaultWritePaths() []string {
return paths return paths
} }
// GetDefaultReadablePaths returns paths that should remain readable when defaultDenyRead is enabled.
// These are essential system paths needed for most programs to run.
//
// Note on user tooling paths: Version managers like nvm, pyenv, etc. require read access to their
// entire installation directories (not just bin/) because runtimes need to load libraries and
// modules from these paths. For example, Node.js needs to read ~/.nvm/versions/.../lib/ to load
// globally installed packages. This is a trade-off between functionality and strict isolation.
// Users who need tighter control can use denyRead to block specific subpaths within these directories.
func GetDefaultReadablePaths() []string {
home, _ := os.UserHomeDir()
paths := []string{
// Core system paths
"/bin",
"/sbin",
"/usr",
"/lib",
"/lib64",
// System configuration (needed for DNS, SSL, locale, etc.)
"/etc",
// Proc filesystem (needed for process info)
"/proc",
// Sys filesystem (needed for system info)
"/sys",
// Device nodes
"/dev",
// macOS specific
"/System",
"/Library",
"/Applications",
"/private/etc",
"/private/var/db",
"/private/var/run",
// Linux distributions may have these
"/opt",
"/run",
// Temp directories (needed for many operations)
"/tmp",
"/private/tmp",
// Common package manager paths
"/usr/local",
"/opt/homebrew",
"/nix",
"/snap",
}
// User-installed tooling paths. These version managers and language runtimes need
// read access to their full directories (not just bin/) to function properly.
// Runtimes load libraries, modules, and configs from within these directories.
if home != "" {
paths = append(paths,
// Node.js version managers (need lib/ for global packages)
filepath.Join(home, ".nvm"),
filepath.Join(home, ".fnm"),
filepath.Join(home, ".volta"),
filepath.Join(home, ".n"),
// Python version managers (need lib/ for installed packages)
filepath.Join(home, ".pyenv"),
filepath.Join(home, ".local/pipx"),
// Ruby version managers (need lib/ for gems)
filepath.Join(home, ".rbenv"),
filepath.Join(home, ".rvm"),
// Rust (bin only - cargo doesn't need full .cargo for execution)
filepath.Join(home, ".cargo/bin"),
filepath.Join(home, ".rustup"),
// Go (bin only)
filepath.Join(home, "go/bin"),
filepath.Join(home, ".go"),
// User local binaries (bin only)
filepath.Join(home, ".local/bin"),
filepath.Join(home, "bin"),
// Bun (bin only)
filepath.Join(home, ".bun/bin"),
// Deno (bin only)
filepath.Join(home, ".deno/bin"),
)
}
return paths
}
// GetMandatoryDenyPatterns returns glob patterns for paths that must always be protected. // GetMandatoryDenyPatterns returns glob patterns for paths that must always be protected.
func GetMandatoryDenyPatterns(cwd string, allowGitConfig bool) []string { func GetMandatoryDenyPatterns(cwd string, allowGitConfig bool) []string {
var patterns []string var patterns []string

View File

@@ -355,8 +355,52 @@ func WrapCommandLinuxWithOptions(cfg *config.Config, command string, bridge *Lin
} }
} }
// Start with read-only root filesystem (default deny writes) defaultDenyRead := cfg != nil && cfg.Filesystem.DefaultDenyRead
bwrapArgs = append(bwrapArgs, "--ro-bind", "/", "/")
if defaultDenyRead {
// In defaultDenyRead mode, we only bind essential system paths read-only
// and user-specified allowRead paths. Everything else is inaccessible.
if opts.Debug {
fmt.Fprintf(os.Stderr, "[fence:linux] DefaultDenyRead mode enabled - binding only essential system paths\n")
}
// Bind essential system paths read-only
// Skip /dev, /proc, /tmp as they're mounted with special options below
for _, systemPath := range GetDefaultReadablePaths() {
if systemPath == "/dev" || systemPath == "/proc" || systemPath == "/tmp" ||
systemPath == "/private/tmp" {
continue
}
if fileExists(systemPath) {
bwrapArgs = append(bwrapArgs, "--ro-bind", systemPath, systemPath)
}
}
// Bind user-specified allowRead paths
if cfg != nil && cfg.Filesystem.AllowRead != nil {
boundPaths := make(map[string]bool)
expandedPaths := ExpandGlobPatterns(cfg.Filesystem.AllowRead)
for _, p := range expandedPaths {
if fileExists(p) && !strings.HasPrefix(p, "/dev/") && !strings.HasPrefix(p, "/proc/") && !boundPaths[p] {
boundPaths[p] = true
bwrapArgs = append(bwrapArgs, "--ro-bind", p, p)
}
}
// Add non-glob paths
for _, p := range cfg.Filesystem.AllowRead {
normalized := NormalizePath(p)
if !ContainsGlobChars(normalized) && fileExists(normalized) &&
!strings.HasPrefix(normalized, "/dev/") && !strings.HasPrefix(normalized, "/proc/") && !boundPaths[normalized] {
boundPaths[normalized] = true
bwrapArgs = append(bwrapArgs, "--ro-bind", normalized, normalized)
}
}
}
} else {
// Default mode: bind entire root filesystem read-only
bwrapArgs = append(bwrapArgs, "--ro-bind", "/", "/")
}
// Mount special filesystems // Mount special filesystems
// Use --dev-bind for /dev instead of --dev to preserve host device permissions // Use --dev-bind for /dev instead of --dev to preserve host device permissions

View File

@@ -35,6 +35,8 @@ type MacOSSandboxParams struct {
AllowAllUnixSockets bool AllowAllUnixSockets bool
AllowLocalBinding bool AllowLocalBinding bool
AllowLocalOutbound bool AllowLocalOutbound bool
DefaultDenyRead bool
ReadAllowPaths []string
ReadDenyPaths []string ReadDenyPaths []string
WriteAllowPaths []string WriteAllowPaths []string
WriteDenyPaths []string WriteDenyPaths []string
@@ -143,13 +145,54 @@ func getTmpdirParent() []string {
} }
// generateReadRules generates filesystem read rules for the sandbox profile. // generateReadRules generates filesystem read rules for the sandbox profile.
func generateReadRules(denyPaths []string, logTag string) []string { func generateReadRules(defaultDenyRead bool, allowPaths, denyPaths []string, logTag string) []string {
var rules []string var rules []string
// Allow all reads by default if defaultDenyRead {
rules = append(rules, "(allow file-read*)") // When defaultDenyRead is enabled:
// 1. Allow file-read-metadata globally (needed for directory traversal, stat, etc.)
// 2. Allow file-read-data only for system paths + user-specified allowRead paths
// This lets programs see what files exist but not read their contents.
// Deny specific paths // Allow metadata operations globally (stat, readdir, etc.) and root dir (for path resolution)
rules = append(rules, "(allow file-read-metadata)")
rules = append(rules, `(allow file-read-data (literal "/"))`)
// Allow reading data from essential system paths
for _, systemPath := range GetDefaultReadablePaths() {
rules = append(rules,
"(allow file-read-data",
fmt.Sprintf(" (subpath %s))", escapePath(systemPath)),
)
}
// Allow reading data from user-specified paths
for _, pathPattern := range allowPaths {
normalized := NormalizePath(pathPattern)
if ContainsGlobChars(normalized) {
regex := GlobToRegex(normalized)
rules = append(rules,
"(allow file-read-data",
fmt.Sprintf(" (regex %s))", escapePath(regex)),
)
} else {
rules = append(rules,
"(allow file-read-data",
fmt.Sprintf(" (subpath %s))", escapePath(normalized)),
)
}
}
} else {
// Allow all reads by default
rules = append(rules, "(allow file-read*)")
}
// In both modes, deny specific paths (denyRead takes precedence).
// Note: We use file-read* (not file-read-data) so denied paths are fully hidden.
// In defaultDenyRead mode, this overrides the global file-read-metadata allow,
// meaning denied paths can't even be listed or stat'd - more restrictive than
// default mode where denied paths are still visible but unreadable.
for _, pathPattern := range denyPaths { for _, pathPattern := range denyPaths {
normalized := NormalizePath(pathPattern) normalized := NormalizePath(pathPattern)
@@ -494,7 +537,7 @@ func GenerateSandboxProfile(params MacOSSandboxParams) string {
// Read rules // Read rules
profile.WriteString("; File read\n") profile.WriteString("; File read\n")
for _, rule := range generateReadRules(params.ReadDenyPaths, logTag) { for _, rule := range generateReadRules(params.DefaultDenyRead, params.ReadAllowPaths, params.ReadDenyPaths, logTag) {
profile.WriteString(rule + "\n") profile.WriteString(rule + "\n")
} }
profile.WriteString("\n") profile.WriteString("\n")
@@ -566,6 +609,8 @@ func WrapCommandMacOS(cfg *config.Config, command string, httpPort, socksPort in
AllowAllUnixSockets: cfg.Network.AllowAllUnixSockets, AllowAllUnixSockets: cfg.Network.AllowAllUnixSockets,
AllowLocalBinding: allowLocalBinding, AllowLocalBinding: allowLocalBinding,
AllowLocalOutbound: allowLocalOutbound, AllowLocalOutbound: allowLocalOutbound,
DefaultDenyRead: cfg.Filesystem.DefaultDenyRead,
ReadAllowPaths: cfg.Filesystem.AllowRead,
ReadDenyPaths: cfg.Filesystem.DenyRead, ReadDenyPaths: cfg.Filesystem.DenyRead,
WriteAllowPaths: allowPaths, WriteAllowPaths: allowPaths,
WriteDenyPaths: cfg.Filesystem.DenyWrite, WriteDenyPaths: cfg.Filesystem.DenyWrite,

View File

@@ -115,6 +115,8 @@ func buildMacOSParamsForTest(cfg *config.Config) MacOSSandboxParams {
AllowAllUnixSockets: cfg.Network.AllowAllUnixSockets, AllowAllUnixSockets: cfg.Network.AllowAllUnixSockets,
AllowLocalBinding: allowLocalBinding, AllowLocalBinding: allowLocalBinding,
AllowLocalOutbound: allowLocalOutbound, AllowLocalOutbound: allowLocalOutbound,
DefaultDenyRead: cfg.Filesystem.DefaultDenyRead,
ReadAllowPaths: cfg.Filesystem.AllowRead,
ReadDenyPaths: cfg.Filesystem.DenyRead, ReadDenyPaths: cfg.Filesystem.DenyRead,
WriteAllowPaths: allowPaths, WriteAllowPaths: allowPaths,
WriteDenyPaths: cfg.Filesystem.DenyWrite, WriteDenyPaths: cfg.Filesystem.DenyWrite,
@@ -177,6 +179,89 @@ func TestMacOS_ProfileNetworkSection(t *testing.T) {
} }
} }
// TestMacOS_DefaultDenyRead verifies that the defaultDenyRead option properly restricts filesystem reads.
func TestMacOS_DefaultDenyRead(t *testing.T) {
tests := []struct {
name string
defaultDenyRead bool
allowRead []string
wantContainsBlanketAllow bool
wantContainsMetadataAllow bool
wantContainsSystemAllows bool
wantContainsUserAllowRead bool
}{
{
name: "default mode - blanket allow read",
defaultDenyRead: false,
allowRead: nil,
wantContainsBlanketAllow: true,
wantContainsMetadataAllow: false, // No separate metadata allow needed
wantContainsSystemAllows: false, // No need for explicit system allows
wantContainsUserAllowRead: false,
},
{
name: "defaultDenyRead enabled - metadata allow, system data allows",
defaultDenyRead: true,
allowRead: nil,
wantContainsBlanketAllow: false,
wantContainsMetadataAllow: true, // Should have file-read-metadata for traversal
wantContainsSystemAllows: true, // Should have explicit system path allows
wantContainsUserAllowRead: false,
},
{
name: "defaultDenyRead with allowRead paths",
defaultDenyRead: true,
allowRead: []string{"/home/user/project"},
wantContainsBlanketAllow: false,
wantContainsMetadataAllow: true,
wantContainsSystemAllows: true,
wantContainsUserAllowRead: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
params := MacOSSandboxParams{
Command: "echo test",
HTTPProxyPort: 8080,
SOCKSProxyPort: 1080,
DefaultDenyRead: tt.defaultDenyRead,
ReadAllowPaths: tt.allowRead,
}
profile := GenerateSandboxProfile(params)
// Check for blanket "(allow file-read*)" without path restrictions
// This appears at the start of read rules section in default mode
hasBlanketAllow := strings.Contains(profile, "(allow file-read*)\n")
if hasBlanketAllow != tt.wantContainsBlanketAllow {
t.Errorf("blanket file-read allow = %v, want %v", hasBlanketAllow, tt.wantContainsBlanketAllow)
}
// Check for file-read-metadata allow (for directory traversal in defaultDenyRead mode)
hasMetadataAllow := strings.Contains(profile, "(allow file-read-metadata)")
if hasMetadataAllow != tt.wantContainsMetadataAllow {
t.Errorf("file-read-metadata allow = %v, want %v", hasMetadataAllow, tt.wantContainsMetadataAllow)
}
// Check for system path allows (e.g., /usr, /bin) - should use file-read-data in strict mode
hasSystemAllows := strings.Contains(profile, `(subpath "/usr")`) ||
strings.Contains(profile, `(subpath "/bin")`)
if hasSystemAllows != tt.wantContainsSystemAllows {
t.Errorf("system path allows = %v, want %v\nProfile:\n%s", hasSystemAllows, tt.wantContainsSystemAllows, profile)
}
// Check for user-specified allowRead paths
if tt.wantContainsUserAllowRead && len(tt.allowRead) > 0 {
hasUserAllow := strings.Contains(profile, tt.allowRead[0])
if !hasUserAllow {
t.Errorf("user allowRead path %q not found in profile", tt.allowRead[0])
}
}
})
}
}
// TestExpandMacOSTmpPaths verifies that /tmp and /private/tmp paths are properly mirrored. // TestExpandMacOSTmpPaths verifies that /tmp and /private/tmp paths are properly mirrored.
func TestExpandMacOSTmpPaths(t *testing.T) { func TestExpandMacOSTmpPaths(t *testing.T) {
tests := []struct { tests := []struct {

View File

@@ -0,0 +1,29 @@
{
"extends": "code",
"filesystem": {
// Deny reads by default, only system paths and allowRead are accessible
"defaultDenyRead": true,
"allowRead": [
// Current working directory
".",
// macOS preferences (needed by many apps)
"~/Library/Preferences",
// AI coding tool configs (need to read their own settings)
"~/.claude",
"~/.claude.json",
"~/.codex",
"~/.cursor",
"~/.opencode",
"~/.gemini",
"~/.factory",
// XDG config directory
"~/.config",
// Cache directories (some tools read from cache)
"~/.cache"
]
}
}

View File

@@ -43,6 +43,7 @@ var templateDescriptions = map[string]string{
"git-readonly": "Blocks destructive commands like git push, rm -rf, etc.", "git-readonly": "Blocks destructive commands like git push, rm -rf, etc.",
"code": "Production-ready config for AI coding agents (Claude Code, Codex, Copilot, etc.)", "code": "Production-ready config for AI coding agents (Claude Code, Codex, Copilot, etc.)",
"code-relaxed": "Like 'code' but allows direct network for apps that ignore HTTP_PROXY (cursor-agent, opencode)", "code-relaxed": "Like 'code' but allows direct network for apps that ignore HTTP_PROXY (cursor-agent, opencode)",
"code-strict": "Like 'code' but denies reads by default; only allows reading the current project directory and essential system paths",
} }
// List returns all available template names sorted alphabetically. // List returns all available template names sorted alphabetically.

View File

@@ -126,6 +126,63 @@ func TestCodeTemplate(t *testing.T) {
} }
} }
func TestCodeStrictTemplate(t *testing.T) {
cfg, err := Load("code-strict")
if err != nil {
t.Fatalf("failed to load code-strict template: %v", err)
}
// Should inherit AllowPty from code template
if !cfg.AllowPty {
t.Error("code-strict should inherit AllowPty=true from code")
}
// Should have defaultDenyRead enabled
if !cfg.Filesystem.DefaultDenyRead {
t.Error("code-strict should have DefaultDenyRead=true")
}
// Should have allowRead with current directory
if len(cfg.Filesystem.AllowRead) == 0 {
t.Error("code-strict should have allowRead paths")
}
hasCurrentDir := false
for _, path := range cfg.Filesystem.AllowRead {
if path == "." {
hasCurrentDir = true
break
}
}
if !hasCurrentDir {
t.Error("code-strict should allow reading current directory")
}
// Should inherit allowWrite from code
if len(cfg.Filesystem.AllowWrite) == 0 {
t.Error("code-strict should inherit allowWrite from code")
}
// Should inherit denyWrite from code
if len(cfg.Filesystem.DenyWrite) == 0 {
t.Error("code-strict should inherit denyWrite from code")
}
// Should inherit allowed domains from code
if len(cfg.Network.AllowedDomains) == 0 {
t.Error("code-strict should inherit allowed domains from code")
}
// Should inherit denied commands from code
if len(cfg.Command.Deny) == 0 {
t.Error("code-strict should inherit denied commands from code")
}
// Extends should be cleared after resolution
if cfg.Extends != "" {
t.Error("extends should be cleared after loading")
}
}
func TestCodeRelaxedTemplate(t *testing.T) { func TestCodeRelaxedTemplate(t *testing.T) {
cfg, err := Load("code-relaxed") cfg, err := Load("code-relaxed")
if err != nil { if err != nil {