Skip to content

Commit a9aecf3

Browse files
authored
fix: use file-read-data instead of file-read* in macOS Seatbelt deny rules (#20)
Seatbelt ignores wildcard denies (file-read*) when a specific allow (file-read-data) covers the same path. This made all denyRead rules on macOS completely ineffective — .env files and user-configured denyRead paths were readable despite deny rules in the profile. Fixes #18
1 parent 99ae165 commit a9aecf3

File tree

3 files changed

+192
-9
lines changed

3 files changed

+192
-9
lines changed

internal/sandbox/integration_macos_test.go

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,87 @@ func TestMacOS_SeatbeltAllowsTmpGreywall(t *testing.T) {
163163
assertFileExists(t, testFile)
164164
}
165165

166+
// ============================================================================
167+
// Sensitive File Read Blocking Tests
168+
// ============================================================================
169+
170+
// TestMacOS_SeatbeltBlocksEnvFileRead verifies that .env files cannot be read
171+
// even though the workspace directory has a broad file-read-data allow.
172+
// This is the regression test for the Seatbelt deny file-read* vs file-read-data bug:
173+
// Seatbelt ignores wildcard denies (file-read*) when a specific allow (file-read-data)
174+
// covers the same path. Deny rules must use file-read-data to be effective.
175+
func TestMacOS_SeatbeltBlocksEnvFileRead(t *testing.T) {
176+
skipIfAlreadySandboxed(t)
177+
178+
workspace := createTempWorkspace(t)
179+
createTestFile(t, workspace, ".env", "SECRET_KEY=supersecret")
180+
createTestFile(t, workspace, "README.md", "hello")
181+
182+
cfg := testConfigWithWorkspace(workspace)
183+
cfg.Filesystem.DefaultDenyRead = boolPtr(true)
184+
cfg.Filesystem.AllowRead = []string{workspace}
185+
cfg.Filesystem.DenyRead = []string{filepath.Join(workspace, ".env")}
186+
187+
// .env should be blocked
188+
result := runUnderSandbox(t, cfg, "cat "+filepath.Join(workspace, ".env"), workspace)
189+
assertBlocked(t, result)
190+
191+
// Regular files should still be readable
192+
result = runUnderSandbox(t, cfg, "cat "+filepath.Join(workspace, "README.md"), workspace)
193+
assertAllowed(t, result)
194+
assertContains(t, result.Stdout, "hello")
195+
}
196+
197+
// TestMacOS_SeatbeltBlocksEnvVariantsRead verifies that .env.* variants are also blocked.
198+
func TestMacOS_SeatbeltBlocksEnvVariantsRead(t *testing.T) {
199+
skipIfAlreadySandboxed(t)
200+
201+
workspace := createTempWorkspace(t)
202+
createTestFile(t, workspace, ".env.local", "LOCAL_SECRET=abc")
203+
createTestFile(t, workspace, ".env.production", "PROD_SECRET=xyz")
204+
205+
cfg := testConfigWithWorkspace(workspace)
206+
cfg.Filesystem.DefaultDenyRead = boolPtr(true)
207+
cfg.Filesystem.AllowRead = []string{workspace}
208+
cfg.Filesystem.DenyRead = []string{
209+
filepath.Join(workspace, ".env.local"),
210+
filepath.Join(workspace, ".env.production"),
211+
}
212+
213+
result := runUnderSandbox(t, cfg, "cat "+filepath.Join(workspace, ".env.local"), workspace)
214+
assertBlocked(t, result)
215+
216+
result = runUnderSandbox(t, cfg, "cat "+filepath.Join(workspace, ".env.production"), workspace)
217+
assertBlocked(t, result)
218+
}
219+
220+
// TestMacOS_SeatbeltBlocksUserDenyReadPaths verifies that user-configured
221+
// denyRead paths are blocked even in legacy mode (defaultDenyRead=false).
222+
func TestMacOS_SeatbeltBlocksUserDenyReadPaths(t *testing.T) {
223+
skipIfAlreadySandboxed(t)
224+
225+
workspace := createTempWorkspace(t)
226+
secretDir := filepath.Join(workspace, "secrets")
227+
if err := os.MkdirAll(secretDir, 0o750); err != nil {
228+
t.Fatalf("failed to create secrets dir: %v", err)
229+
}
230+
createTestFile(t, workspace, "secrets/api_key.txt", "my-api-key")
231+
createTestFile(t, workspace, "public.txt", "public data")
232+
233+
cfg := testConfigWithWorkspace(workspace)
234+
cfg.Filesystem.DefaultDenyRead = boolPtr(false) // legacy mode
235+
cfg.Filesystem.DenyRead = []string{secretDir}
236+
237+
// Secret file should be blocked
238+
result := runUnderSandbox(t, cfg, "cat "+filepath.Join(secretDir, "api_key.txt"), workspace)
239+
assertBlocked(t, result)
240+
241+
// Public file should still be readable
242+
result = runUnderSandbox(t, cfg, "cat public.txt", workspace)
243+
assertAllowed(t, result)
244+
assertContains(t, result.Stdout, "public data")
245+
}
246+
166247
// ============================================================================
167248
// Network Blocking Tests
168249
// ============================================================================

internal/sandbox/macos.go

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -229,19 +229,21 @@ func generateReadRules(defaultDenyRead bool, cwd string, allowPaths, denyPaths [
229229
}
230230
}
231231

232-
// Deny sensitive files within CWD (Seatbelt evaluates deny before allow)
232+
// Deny sensitive files within CWD.
233+
// Must use file-read-data (not file-read*) because Seatbelt ignores
234+
// wildcard denies when a specific allow (file-read-data) covers the same path.
233235
if cwd != "" {
234236
for _, f := range SensitiveProjectFiles {
235237
p := filepath.Join(cwd, f)
236238
rules = append(rules,
237-
"(deny file-read*",
239+
"(deny file-read-data",
238240
fmt.Sprintf(" (literal %s)", escapePath(p)),
239241
fmt.Sprintf(" (with message %q))", logTag),
240242
)
241243
}
242244
// Also deny .env.* pattern via regex
243245
rules = append(rules,
244-
"(deny file-read*",
246+
"(deny file-read-data",
245247
fmt.Sprintf(" (regex %s)", escapePath("^"+regexp.QuoteMeta(cwd)+"/\\.env\\..*$")),
246248
fmt.Sprintf(" (with message %q))", logTag),
247249
)
@@ -252,23 +254,21 @@ func generateReadRules(defaultDenyRead bool, cwd string, allowPaths, denyPaths [
252254
}
253255

254256
// In both modes, deny specific paths (denyRead takes precedence).
255-
// Note: We use file-read* (not file-read-data) so denied paths are fully hidden.
256-
// In defaultDenyRead mode, this overrides the global file-read-metadata allow,
257-
// meaning denied paths can't even be listed or stat'd - more restrictive than
258-
// default mode where denied paths are still visible but unreadable.
257+
// Must use file-read-data (not file-read*) because Seatbelt ignores
258+
// wildcard denies when a specific allow (file-read-data) covers the same path.
259259
for _, pathPattern := range denyPaths {
260260
normalized := NormalizePath(pathPattern)
261261

262262
if ContainsGlobChars(normalized) {
263263
regex := GlobToRegex(normalized)
264264
rules = append(rules,
265-
"(deny file-read*",
265+
"(deny file-read-data",
266266
fmt.Sprintf(" (regex %s)", escapePath(regex)),
267267
fmt.Sprintf(" (with message %q))", logTag),
268268
)
269269
} else {
270270
rules = append(rules,
271-
"(deny file-read*",
271+
"(deny file-read-data",
272272
fmt.Sprintf(" (subpath %s)", escapePath(normalized)),
273273
fmt.Sprintf(" (with message %q))", logTag),
274274
)

internal/sandbox/macos_test.go

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -264,6 +264,108 @@ func TestMacOS_DefaultDenyRead(t *testing.T) {
264264
}
265265
}
266266

267+
// TestMacOS_DenyReadUsesExactOperation verifies that deny read rules use file-read-data
268+
// (not file-read*) in the generated Seatbelt profile. Seatbelt ignores wildcard denies
269+
// (file-read*) when a specific allow (file-read-data) covers the same path.
270+
func TestMacOS_DenyReadUsesExactOperation(t *testing.T) {
271+
tests := []struct {
272+
name string
273+
defaultDenyRead bool
274+
denyRead []string
275+
cwd string
276+
}{
277+
{
278+
name: "defaultDenyRead with sensitive project files",
279+
defaultDenyRead: true,
280+
denyRead: nil,
281+
cwd: "/home/user/project",
282+
},
283+
{
284+
name: "defaultDenyRead with user denyRead paths",
285+
defaultDenyRead: true,
286+
denyRead: []string{"/home/user/secrets", "/home/user/.ssh/id_*"},
287+
cwd: "/home/user/project",
288+
},
289+
{
290+
name: "legacy mode with user denyRead paths",
291+
defaultDenyRead: false,
292+
denyRead: []string{"/home/user/secrets"},
293+
cwd: "/home/user/project",
294+
},
295+
}
296+
297+
for _, tt := range tests {
298+
t.Run(tt.name, func(t *testing.T) {
299+
params := MacOSSandboxParams{
300+
Command: "echo test",
301+
DefaultDenyRead: tt.defaultDenyRead,
302+
Cwd: tt.cwd,
303+
ReadDenyPaths: tt.denyRead,
304+
}
305+
306+
profile := GenerateSandboxProfile(params)
307+
308+
// Must NOT contain "deny file-read*" (wildcard deny is ineffective)
309+
if strings.Contains(profile, "(deny file-read*") {
310+
t.Errorf("profile must NOT use 'deny file-read*' (wildcard deny is ignored by Seatbelt when a specific allow covers the same path)\nProfile:\n%s", profile)
311+
}
312+
313+
// Must contain "deny file-read-data" for deny rules to work
314+
if tt.defaultDenyRead || len(tt.denyRead) > 0 {
315+
if !strings.Contains(profile, "(deny file-read-data") {
316+
t.Errorf("profile must use 'deny file-read-data' for deny rules to be effective\nProfile:\n%s", profile)
317+
}
318+
}
319+
})
320+
}
321+
}
322+
323+
// TestMacOS_DenyReadSensitiveProjectFiles verifies that the generated profile
324+
// contains deny rules for all sensitive project files (.env, .env.local, etc.).
325+
func TestMacOS_DenyReadSensitiveProjectFiles(t *testing.T) {
326+
cwd := "/home/user/project"
327+
params := MacOSSandboxParams{
328+
Command: "cat .env",
329+
DefaultDenyRead: true,
330+
Cwd: cwd,
331+
}
332+
333+
profile := GenerateSandboxProfile(params)
334+
335+
for _, f := range SensitiveProjectFiles {
336+
expected := fmt.Sprintf(`(deny file-read-data
337+
(literal %q)`, cwd+"/"+f)
338+
if !strings.Contains(profile, expected) {
339+
t.Errorf("profile missing deny rule for sensitive file %q\nExpected to contain: %s", f, expected)
340+
}
341+
}
342+
343+
// Also check .env.* regex pattern
344+
if !strings.Contains(profile, `(deny file-read-data
345+
(regex`) {
346+
t.Errorf("profile missing regex deny rule for .env.* pattern")
347+
}
348+
}
349+
350+
// TestMacOS_DenyReadUserPaths verifies that user-configured denyRead paths
351+
// appear in the generated profile with file-read-data (not file-read*).
352+
func TestMacOS_DenyReadUserPaths(t *testing.T) {
353+
params := MacOSSandboxParams{
354+
Command: "echo test",
355+
DefaultDenyRead: false,
356+
Cwd: "/home/user/project",
357+
ReadDenyPaths: []string{"/home/user/secrets"},
358+
}
359+
360+
profile := GenerateSandboxProfile(params)
361+
362+
expected := `(deny file-read-data
363+
(subpath "/home/user/secrets")`
364+
if !strings.Contains(profile, expected) {
365+
t.Errorf("profile missing deny rule for user denyRead path\nExpected: %s\nProfile:\n%s", expected, profile)
366+
}
367+
}
368+
267369
// TestExpandMacOSTmpPaths verifies that /tmp and /private/tmp paths are properly mirrored.
268370
func TestExpandMacOSTmpPaths(t *testing.T) {
269371
tests := []struct {

0 commit comments

Comments
 (0)