Skip to content

Commit a10a698

Browse files
committed
fix: use file-read-data instead of file-read* in macOS Seatbelt deny rules
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 9615dfa commit a10a698

File tree

3 files changed

+185
-9
lines changed

3 files changed

+185
-9
lines changed

internal/sandbox/integration_macos_test.go

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,80 @@ 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+
185+
// .env should be blocked
186+
result := runUnderSandbox(t, cfg, "cat .env", workspace)
187+
assertBlocked(t, result)
188+
189+
// Regular files should still be readable
190+
result = runUnderSandbox(t, cfg, "cat README.md", workspace)
191+
assertAllowed(t, result)
192+
assertContains(t, result.Stdout, "hello")
193+
}
194+
195+
// TestMacOS_SeatbeltBlocksEnvVariantsRead verifies that .env.* variants are also blocked.
196+
func TestMacOS_SeatbeltBlocksEnvVariantsRead(t *testing.T) {
197+
skipIfAlreadySandboxed(t)
198+
199+
workspace := createTempWorkspace(t)
200+
createTestFile(t, workspace, ".env.local", "LOCAL_SECRET=abc")
201+
createTestFile(t, workspace, ".env.production", "PROD_SECRET=xyz")
202+
203+
cfg := testConfigWithWorkspace(workspace)
204+
cfg.Filesystem.DefaultDenyRead = boolPtr(true)
205+
206+
result := runUnderSandbox(t, cfg, "cat .env.local", workspace)
207+
assertBlocked(t, result)
208+
209+
result = runUnderSandbox(t, cfg, "cat .env.production", workspace)
210+
assertBlocked(t, result)
211+
}
212+
213+
// TestMacOS_SeatbeltBlocksUserDenyReadPaths verifies that user-configured
214+
// denyRead paths are blocked even in legacy mode (defaultDenyRead=false).
215+
func TestMacOS_SeatbeltBlocksUserDenyReadPaths(t *testing.T) {
216+
skipIfAlreadySandboxed(t)
217+
218+
workspace := createTempWorkspace(t)
219+
secretDir := filepath.Join(workspace, "secrets")
220+
if err := os.MkdirAll(secretDir, 0o750); err != nil {
221+
t.Fatalf("failed to create secrets dir: %v", err)
222+
}
223+
createTestFile(t, workspace, "secrets/api_key.txt", "my-api-key")
224+
createTestFile(t, workspace, "public.txt", "public data")
225+
226+
cfg := testConfigWithWorkspace(workspace)
227+
cfg.Filesystem.DefaultDenyRead = boolPtr(false) // legacy mode
228+
cfg.Filesystem.DenyRead = []string{secretDir}
229+
230+
// Secret file should be blocked
231+
result := runUnderSandbox(t, cfg, "cat "+filepath.Join(secretDir, "api_key.txt"), workspace)
232+
assertBlocked(t, result)
233+
234+
// Public file should still be readable
235+
result = runUnderSandbox(t, cfg, "cat public.txt", workspace)
236+
assertAllowed(t, result)
237+
assertContains(t, result.Stdout, "public data")
238+
}
239+
166240
// ============================================================================
167241
// Network Blocking Tests
168242
// ============================================================================

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)