Skip to content

Commit 7679fec

Browse files
jy-tanjy-tan
authored
feat: add defaultDenyRead mode for strict filesystem isolation (#24)
1 parent cef3576 commit 7679fec

File tree

9 files changed

+430
-11
lines changed

9 files changed

+430
-11
lines changed

‎internal/config/config.go‎

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -37,10 +37,12 @@ type NetworkConfig struct {
3737

3838
// FilesystemConfig defines filesystem restrictions.
3939
type FilesystemConfig struct {
40-
DenyRead []string `json:"denyRead"`
41-
AllowWrite []string `json:"allowWrite"`
42-
DenyWrite []string `json:"denyWrite"`
43-
AllowGitConfig bool `json:"allowGitConfig,omitempty"`
40+
DefaultDenyRead bool `json:"defaultDenyRead,omitempty"` // If true, deny reads by default except system paths and AllowRead
41+
AllowRead []string `json:"allowRead"` // Paths to allow reading (used when DefaultDenyRead is true)
42+
DenyRead []string `json:"denyRead"`
43+
AllowWrite []string `json:"allowWrite"`
44+
DenyWrite []string `json:"denyWrite"`
45+
AllowGitConfig bool `json:"allowGitConfig,omitempty"`
4446
}
4547

4648
// CommandConfig defines command restrictions.
@@ -179,6 +181,9 @@ func (c *Config) Validate() error {
179181
}
180182
}
181183

184+
if slices.Contains(c.Filesystem.AllowRead, "") {
185+
return errors.New("filesystem.allowRead contains empty path")
186+
}
182187
if slices.Contains(c.Filesystem.DenyRead, "") {
183188
return errors.New("filesystem.denyRead contains empty path")
184189
}
@@ -427,7 +432,11 @@ func Merge(base, override *Config) *Config {
427432
},
428433

429434
Filesystem: FilesystemConfig{
435+
// Boolean fields: true if either enables it
436+
DefaultDenyRead: base.Filesystem.DefaultDenyRead || override.Filesystem.DefaultDenyRead,
437+
430438
// Append slices
439+
AllowRead: mergeStrings(base.Filesystem.AllowRead, override.Filesystem.AllowRead),
431440
DenyRead: mergeStrings(base.Filesystem.DenyRead, override.Filesystem.DenyRead),
432441
AllowWrite: mergeStrings(base.Filesystem.AllowWrite, override.Filesystem.AllowWrite),
433442
DenyWrite: mergeStrings(base.Filesystem.DenyWrite, override.Filesystem.DenyWrite),

‎internal/config/config_test.go‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,15 @@ func TestConfigValidate(t *testing.T) {
111111
},
112112
wantErr: true,
113113
},
114+
{
115+
name: "empty allowRead path",
116+
config: Config{
117+
Filesystem: FilesystemConfig{
118+
AllowRead: []string{""},
119+
},
120+
},
121+
wantErr: true,
122+
},
114123
{
115124
name: "empty denyRead path",
116125
config: Config{
@@ -453,6 +462,50 @@ func TestMerge(t *testing.T) {
453462
}
454463
})
455464

465+
t.Run("merge defaultDenyRead and allowRead", func(t *testing.T) {
466+
base := &Config{
467+
Filesystem: FilesystemConfig{
468+
DefaultDenyRead: true,
469+
AllowRead: []string{"/home/user/project"},
470+
},
471+
}
472+
override := &Config{
473+
Filesystem: FilesystemConfig{
474+
AllowRead: []string{"/home/user/other"},
475+
},
476+
}
477+
result := Merge(base, override)
478+
479+
if !result.Filesystem.DefaultDenyRead {
480+
t.Error("expected DefaultDenyRead to be true (from base)")
481+
}
482+
if len(result.Filesystem.AllowRead) != 2 {
483+
t.Errorf("expected 2 allowRead paths, got %d: %v", len(result.Filesystem.AllowRead), result.Filesystem.AllowRead)
484+
}
485+
})
486+
487+
t.Run("merge defaultDenyRead from override", func(t *testing.T) {
488+
base := &Config{
489+
Filesystem: FilesystemConfig{
490+
DefaultDenyRead: false,
491+
},
492+
}
493+
override := &Config{
494+
Filesystem: FilesystemConfig{
495+
DefaultDenyRead: true,
496+
AllowRead: []string{"/home/user/project"},
497+
},
498+
}
499+
result := Merge(base, override)
500+
501+
if !result.Filesystem.DefaultDenyRead {
502+
t.Error("expected DefaultDenyRead to be true (from override)")
503+
}
504+
if len(result.Filesystem.AllowRead) != 1 {
505+
t.Errorf("expected 1 allowRead path, got %d", len(result.Filesystem.AllowRead))
506+
}
507+
})
508+
456509
t.Run("override ports", func(t *testing.T) {
457510
base := &Config{
458511
Network: NetworkConfig{

‎internal/sandbox/dangerous.go‎

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,102 @@ func GetDefaultWritePaths() []string {
5353
return paths
5454
}
5555

56+
// GetDefaultReadablePaths returns paths that should remain readable when defaultDenyRead is enabled.
57+
// These are essential system paths needed for most programs to run.
58+
//
59+
// Note on user tooling paths: Version managers like nvm, pyenv, etc. require read access to their
60+
// entire installation directories (not just bin/) because runtimes need to load libraries and
61+
// modules from these paths. For example, Node.js needs to read ~/.nvm/versions/.../lib/ to load
62+
// globally installed packages. This is a trade-off between functionality and strict isolation.
63+
// Users who need tighter control can use denyRead to block specific subpaths within these directories.
64+
func GetDefaultReadablePaths() []string {
65+
home, _ := os.UserHomeDir()
66+
67+
paths := []string{
68+
// Core system paths
69+
"/bin",
70+
"/sbin",
71+
"/usr",
72+
"/lib",
73+
"/lib64",
74+
75+
// System configuration (needed for DNS, SSL, locale, etc.)
76+
"/etc",
77+
78+
// Proc filesystem (needed for process info)
79+
"/proc",
80+
81+
// Sys filesystem (needed for system info)
82+
"/sys",
83+
84+
// Device nodes
85+
"/dev",
86+
87+
// macOS specific
88+
"/System",
89+
"/Library",
90+
"/Applications",
91+
"/private/etc",
92+
"/private/var/db",
93+
"/private/var/run",
94+
95+
// Linux distributions may have these
96+
"/opt",
97+
"/run",
98+
99+
// Temp directories (needed for many operations)
100+
"/tmp",
101+
"/private/tmp",
102+
103+
// Common package manager paths
104+
"/usr/local",
105+
"/opt/homebrew",
106+
"/nix",
107+
"/snap",
108+
}
109+
110+
// User-installed tooling paths. These version managers and language runtimes need
111+
// read access to their full directories (not just bin/) to function properly.
112+
// Runtimes load libraries, modules, and configs from within these directories.
113+
if home != "" {
114+
paths = append(paths,
115+
// Node.js version managers (need lib/ for global packages)
116+
filepath.Join(home, ".nvm"),
117+
filepath.Join(home, ".fnm"),
118+
filepath.Join(home, ".volta"),
119+
filepath.Join(home, ".n"),
120+
121+
// Python version managers (need lib/ for installed packages)
122+
filepath.Join(home, ".pyenv"),
123+
filepath.Join(home, ".local/pipx"),
124+
125+
// Ruby version managers (need lib/ for gems)
126+
filepath.Join(home, ".rbenv"),
127+
filepath.Join(home, ".rvm"),
128+
129+
// Rust (bin only - cargo doesn't need full .cargo for execution)
130+
filepath.Join(home, ".cargo/bin"),
131+
filepath.Join(home, ".rustup"),
132+
133+
// Go (bin only)
134+
filepath.Join(home, "go/bin"),
135+
filepath.Join(home, ".go"),
136+
137+
// User local binaries (bin only)
138+
filepath.Join(home, ".local/bin"),
139+
filepath.Join(home, "bin"),
140+
141+
// Bun (bin only)
142+
filepath.Join(home, ".bun/bin"),
143+
144+
// Deno (bin only)
145+
filepath.Join(home, ".deno/bin"),
146+
)
147+
}
148+
149+
return paths
150+
}
151+
56152
// GetMandatoryDenyPatterns returns glob patterns for paths that must always be protected.
57153
func GetMandatoryDenyPatterns(cwd string, allowGitConfig bool) []string {
58154
var patterns []string

‎internal/sandbox/linux.go‎

Lines changed: 46 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -355,8 +355,52 @@ func WrapCommandLinuxWithOptions(cfg *config.Config, command string, bridge *Lin
355355
}
356356
}
357357

358-
// Start with read-only root filesystem (default deny writes)
359-
bwrapArgs = append(bwrapArgs, "--ro-bind", "/", "/")
358+
defaultDenyRead := cfg != nil && cfg.Filesystem.DefaultDenyRead
359+
360+
if defaultDenyRead {
361+
// In defaultDenyRead mode, we only bind essential system paths read-only
362+
// and user-specified allowRead paths. Everything else is inaccessible.
363+
if opts.Debug {
364+
fmt.Fprintf(os.Stderr, "[fence:linux] DefaultDenyRead mode enabled - binding only essential system paths\n")
365+
}
366+
367+
// Bind essential system paths read-only
368+
// Skip /dev, /proc, /tmp as they're mounted with special options below
369+
for _, systemPath := range GetDefaultReadablePaths() {
370+
if systemPath == "/dev" || systemPath == "/proc" || systemPath == "/tmp" ||
371+
systemPath == "/private/tmp" {
372+
continue
373+
}
374+
if fileExists(systemPath) {
375+
bwrapArgs = append(bwrapArgs, "--ro-bind", systemPath, systemPath)
376+
}
377+
}
378+
379+
// Bind user-specified allowRead paths
380+
if cfg != nil && cfg.Filesystem.AllowRead != nil {
381+
boundPaths := make(map[string]bool)
382+
383+
expandedPaths := ExpandGlobPatterns(cfg.Filesystem.AllowRead)
384+
for _, p := range expandedPaths {
385+
if fileExists(p) && !strings.HasPrefix(p, "/dev/") && !strings.HasPrefix(p, "/proc/") && !boundPaths[p] {
386+
boundPaths[p] = true
387+
bwrapArgs = append(bwrapArgs, "--ro-bind", p, p)
388+
}
389+
}
390+
// Add non-glob paths
391+
for _, p := range cfg.Filesystem.AllowRead {
392+
normalized := NormalizePath(p)
393+
if !ContainsGlobChars(normalized) && fileExists(normalized) &&
394+
!strings.HasPrefix(normalized, "/dev/") && !strings.HasPrefix(normalized, "/proc/") && !boundPaths[normalized] {
395+
boundPaths[normalized] = true
396+
bwrapArgs = append(bwrapArgs, "--ro-bind", normalized, normalized)
397+
}
398+
}
399+
}
400+
} else {
401+
// Default mode: bind entire root filesystem read-only
402+
bwrapArgs = append(bwrapArgs, "--ro-bind", "/", "/")
403+
}
360404

361405
// Mount special filesystems
362406
// Use --dev-bind for /dev instead of --dev to preserve host device permissions

‎internal/sandbox/macos.go‎

Lines changed: 50 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,8 @@ type MacOSSandboxParams struct {
3535
AllowAllUnixSockets bool
3636
AllowLocalBinding bool
3737
AllowLocalOutbound bool
38+
DefaultDenyRead bool
39+
ReadAllowPaths []string
3840
ReadDenyPaths []string
3941
WriteAllowPaths []string
4042
WriteDenyPaths []string
@@ -143,13 +145,54 @@ func getTmpdirParent() []string {
143145
}
144146

145147
// generateReadRules generates filesystem read rules for the sandbox profile.
146-
func generateReadRules(denyPaths []string, logTag string) []string {
148+
func generateReadRules(defaultDenyRead bool, allowPaths, denyPaths []string, logTag string) []string {
147149
var rules []string
148150

149-
// Allow all reads by default
150-
rules = append(rules, "(allow file-read*)")
151+
if defaultDenyRead {
152+
// When defaultDenyRead is enabled:
153+
// 1. Allow file-read-metadata globally (needed for directory traversal, stat, etc.)
154+
// 2. Allow file-read-data only for system paths + user-specified allowRead paths
155+
// This lets programs see what files exist but not read their contents.
151156

152-
// Deny specific paths
157+
// Allow metadata operations globally (stat, readdir, etc.) and root dir (for path resolution)
158+
rules = append(rules, "(allow file-read-metadata)")
159+
rules = append(rules, `(allow file-read-data (literal "/"))`)
160+
161+
// Allow reading data from essential system paths
162+
for _, systemPath := range GetDefaultReadablePaths() {
163+
rules = append(rules,
164+
"(allow file-read-data",
165+
fmt.Sprintf(" (subpath %s))", escapePath(systemPath)),
166+
)
167+
}
168+
169+
// Allow reading data from user-specified paths
170+
for _, pathPattern := range allowPaths {
171+
normalized := NormalizePath(pathPattern)
172+
173+
if ContainsGlobChars(normalized) {
174+
regex := GlobToRegex(normalized)
175+
rules = append(rules,
176+
"(allow file-read-data",
177+
fmt.Sprintf(" (regex %s))", escapePath(regex)),
178+
)
179+
} else {
180+
rules = append(rules,
181+
"(allow file-read-data",
182+
fmt.Sprintf(" (subpath %s))", escapePath(normalized)),
183+
)
184+
}
185+
}
186+
} else {
187+
// Allow all reads by default
188+
rules = append(rules, "(allow file-read*)")
189+
}
190+
191+
// In both modes, deny specific paths (denyRead takes precedence).
192+
// Note: We use file-read* (not file-read-data) so denied paths are fully hidden.
193+
// In defaultDenyRead mode, this overrides the global file-read-metadata allow,
194+
// meaning denied paths can't even be listed or stat'd - more restrictive than
195+
// default mode where denied paths are still visible but unreadable.
153196
for _, pathPattern := range denyPaths {
154197
normalized := NormalizePath(pathPattern)
155198

@@ -494,7 +537,7 @@ func GenerateSandboxProfile(params MacOSSandboxParams) string {
494537

495538
// Read rules
496539
profile.WriteString("; File read\n")
497-
for _, rule := range generateReadRules(params.ReadDenyPaths, logTag) {
540+
for _, rule := range generateReadRules(params.DefaultDenyRead, params.ReadAllowPaths, params.ReadDenyPaths, logTag) {
498541
profile.WriteString(rule + "\n")
499542
}
500543
profile.WriteString("\n")
@@ -566,6 +609,8 @@ func WrapCommandMacOS(cfg *config.Config, command string, httpPort, socksPort in
566609
AllowAllUnixSockets: cfg.Network.AllowAllUnixSockets,
567610
AllowLocalBinding: allowLocalBinding,
568611
AllowLocalOutbound: allowLocalOutbound,
612+
DefaultDenyRead: cfg.Filesystem.DefaultDenyRead,
613+
ReadAllowPaths: cfg.Filesystem.AllowRead,
569614
ReadDenyPaths: cfg.Filesystem.DenyRead,
570615
WriteAllowPaths: allowPaths,
571616
WriteDenyPaths: cfg.Filesystem.DenyWrite,

0 commit comments

Comments
 (0)