Skip to content

Commit fefbdf8

Browse files
ampagentPierrunoYT
ampagent
and
PierrunoYT
committed
fix(sandbox): fail closed for daemon tokens
Amp-Thread-ID: https://ampcode.com/threads/T-019fb8cf-a980-7568-80f1-fe34faaaecae Co-authored-by: Pierre Bruno <pierrebruno@hotmail.ch>
1 parent d402545 commit fefbdf8

4 files changed

Lines changed: 198 additions & 6 deletions

File tree

‎internal/sandbox/manager.go‎

Lines changed: 59 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -228,8 +228,15 @@ func (manager SandboxManager) BuildExecutionRequest(request SandboxManagerReques
228228
if request.ValidateExecution && preference == SandboxPreferenceRequire && backend.SupportLevel() != BackendSupportNative {
229229
return SandboxExecutionRequest{}, nativeSandboxUnavailableError(backend)
230230
}
231-
if request.ValidateExecution && preference != SandboxPreferenceForbid && backend.SupportLevel() != BackendSupportNative && policyHasExplicitDeny(policy) {
232-
return SandboxExecutionRequest{}, errors.New("native sandbox unavailable: configured deny_read or deny_write rules cannot be enforced")
231+
protectedCredentials := protectedCredentialPaths()
232+
protectedCredentialNeedsNative := policy.Mode != ModeDisabled && len(protectedCredentials) > 0
233+
if request.ValidateExecution && preference != SandboxPreferenceForbid && backend.SupportLevel() != BackendSupportNative &&
234+
(policyHasExplicitDeny(policy) || protectedCredentialNeedsNative) {
235+
return SandboxExecutionRequest{}, errors.New("native sandbox unavailable: configured deny rules or protected credentials cannot be enforced")
236+
}
237+
if request.ValidateExecution && preference != SandboxPreferenceForbid && policy.Mode != ModeDisabled && manager.goos == "darwin" &&
238+
protectedCredentialInWritableMacOSRoot(profile, protectedCredentials) {
239+
return SandboxExecutionRequest{}, errors.New("macOS sandbox cannot protect the remote token file inside a shell-writable root; move the token file outside the workspace and temporary directories")
233240
}
234241
// Windows: the FULL OS sandbox needs a one-time elevated `zero sandbox setup`
235242
// (it applies WFP network filters + workspace ACLs and writes a marker).
@@ -290,6 +297,56 @@ func policyHasExplicitDeny(policy Policy) bool {
290297
return len(normalizeProfilePaths(policy.DenyRead)) > 0 || len(normalizeProfilePaths(policy.DenyWrite)) > 0
291298
}
292299

300+
func protectedCredentialInWritableMacOSRoot(profile PermissionProfile, protected []string) bool {
301+
if len(protected) == 0 {
302+
return false
303+
}
304+
if profile.FileSystem.Kind == FileSystemUnrestricted {
305+
return true
306+
}
307+
if profile.FileSystem.Kind != FileSystemRestricted {
308+
return false
309+
}
310+
writeRoots := make([]string, 0, len(profile.FileSystem.WriteRoots)+len(sandboxWritableSubpaths))
311+
for _, root := range profile.FileSystem.WriteRoots {
312+
writeRoots = append(writeRoots, normalizeProfilePath(root.Root))
313+
}
314+
if profile.FileSystem.AllowTemp {
315+
writeRoots = append(writeRoots, normalizeProfilePaths(sandboxWritableSubpaths)...)
316+
}
317+
for _, credential := range protected {
318+
credential = filepath.Clean(strings.TrimSpace(credential))
319+
if credential == "." || credential == "" {
320+
continue
321+
}
322+
for _, root := range writeRoots {
323+
if pathWithinMacOSRoot(root, credential) {
324+
return true
325+
}
326+
}
327+
}
328+
return false
329+
}
330+
331+
func pathWithinMacOSRoot(root, candidate string) bool {
332+
if pathWithinRoot(root, candidate) || pathWithinRoot(strings.ToLower(root), strings.ToLower(candidate)) {
333+
return true
334+
}
335+
rootInfo, err := os.Stat(root)
336+
if err != nil {
337+
return false
338+
}
339+
for current := candidate; ; current = filepath.Dir(current) {
340+
if info, err := os.Stat(current); err == nil && os.SameFile(rootInfo, info) {
341+
return true
342+
}
343+
parent := filepath.Dir(current)
344+
if parent == current {
345+
return false
346+
}
347+
}
348+
}
349+
293350
func (manager SandboxManager) BuildCommandPlan(request SandboxManagerRequest) (CommandPlan, error) {
294351
execRequest, err := manager.BuildExecutionRequest(request)
295352
if err != nil {

‎internal/sandbox/manager_test.go‎

Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -250,6 +250,126 @@ func TestSandboxManagerDegradesUnavailableCommandPlan(t *testing.T) {
250250
}
251251
}
252252

253+
func TestSandboxManagerRejectsUnavailableBackendForProtectedToken(t *testing.T) {
254+
workspace, _ := protectedTokenFixture(t)
255+
policy := DefaultPolicy()
256+
backend := Backend{Name: BackendUnavailable, Platform: "linux", Fallback: true, Message: "native sandbox unavailable"}
257+
_, err := NewSandboxManager(SandboxManagerOptions{GOOS: "linux", Backend: backend}).BuildCommandPlan(SandboxManagerRequest{
258+
WorkspaceRoot: workspace,
259+
Command: CommandSpec{Name: "/bin/sh", Args: []string{"-c", "true"}, Dir: workspace},
260+
Policy: policy,
261+
Profile: PermissionProfileFromPolicy(workspace, policy, nil),
262+
Preference: SandboxPreferenceAuto,
263+
ValidateExecution: true,
264+
})
265+
if err == nil || !strings.Contains(err.Error(), "protected credentials cannot be enforced") {
266+
t.Fatalf("BuildCommandPlan error = %v, want protected-credential enforcement failure", err)
267+
}
268+
}
269+
270+
func TestSandboxManagerRejectsMacOSTokenInsideWritableWorkspace(t *testing.T) {
271+
t.Setenv(daemonRemoteTokenEnv, "")
272+
t.Setenv(daemonRemoteTokenFileEnv, "/workspace/bridge-token")
273+
workspace := "/workspace"
274+
policy := DefaultPolicy()
275+
backend := Backend{
276+
Name: BackendMacOSSeatbelt,
277+
Available: true,
278+
Executable: "/usr/bin/sandbox-exec",
279+
Platform: "darwin",
280+
CommandWrapping: true,
281+
NativeIsolation: true,
282+
}
283+
_, err := NewSandboxManager(SandboxManagerOptions{GOOS: "darwin", Backend: backend}).BuildCommandPlan(SandboxManagerRequest{
284+
WorkspaceRoot: workspace,
285+
Command: CommandSpec{Name: "/bin/sh", Args: []string{"-c", "true"}, Dir: workspace},
286+
Policy: policy,
287+
Profile: PermissionProfileFromPolicy(workspace, policy, nil),
288+
Preference: SandboxPreferenceAuto,
289+
ValidateExecution: true,
290+
})
291+
if err == nil || !strings.Contains(err.Error(), "inside a shell-writable root") {
292+
t.Fatalf("BuildCommandPlan error = %v, want macOS writable-token failure", err)
293+
}
294+
}
295+
296+
func TestProtectedCredentialInWritableMacOSRootMatchesSeatbeltWrites(t *testing.T) {
297+
restricted := PermissionProfile{FileSystem: FileSystemPolicy{
298+
Kind: FileSystemRestricted,
299+
WriteRoots: []WritableRoot{{Root: "/Users/Test/Workspace"}},
300+
}}
301+
if !protectedCredentialInWritableMacOSRoot(restricted, []string{"/users/test/workspace/token"}) {
302+
t.Fatal("case-variant token under a macOS write root should be rejected")
303+
}
304+
if protectedCredentialInWritableMacOSRoot(restricted, []string{"/Users/Test/Credentials/token"}) {
305+
t.Fatal("token outside every macOS write root should remain allowed")
306+
}
307+
restricted.FileSystem.AllowTemp = true
308+
if !protectedCredentialInWritableMacOSRoot(restricted, []string{"/private/tmp/bridge-token"}) {
309+
t.Fatal("token under an allowed temporary root should be rejected")
310+
}
311+
unrestricted := PermissionProfile{FileSystem: FileSystemPolicy{Kind: FileSystemUnrestricted}}
312+
if !protectedCredentialInWritableMacOSRoot(unrestricted, []string{"/credentials/token"}) {
313+
t.Fatal("an unrestricted macOS filesystem makes every token path shell-writable")
314+
}
315+
316+
writable := t.TempDir()
317+
targetDir := t.TempDir()
318+
target := filepath.Join(targetDir, "token")
319+
if err := os.WriteFile(target, []byte("secret"), 0o600); err != nil {
320+
t.Fatal(err)
321+
}
322+
link := filepath.Join(writable, "token-link")
323+
if err := os.Symlink(target, link); err != nil {
324+
t.Skipf("symlinks unavailable: %v", err)
325+
}
326+
symlinkProfile := PermissionProfile{FileSystem: FileSystemPolicy{
327+
Kind: FileSystemRestricted,
328+
WriteRoots: []WritableRoot{{Root: writable}},
329+
}}
330+
if !protectedCredentialInWritableMacOSRoot(symlinkProfile, []string{link, target}) {
331+
t.Fatal("selected symlink inside a write root must be rejected even when its target is outside")
332+
}
333+
}
334+
335+
func TestSandboxManagerLeavesProtectedTokenShellOpenWhenSandboxDisabled(t *testing.T) {
336+
t.Setenv(daemonRemoteTokenEnv, "")
337+
t.Setenv(daemonRemoteTokenFileEnv, "/workspace/bridge-token")
338+
policy := DefaultPolicy()
339+
policy.Mode = ModeDisabled
340+
backend := Backend{Name: BackendUnavailable, Platform: "darwin", Fallback: true, Message: "native sandbox unavailable"}
341+
request, err := NewSandboxManager(SandboxManagerOptions{GOOS: "darwin", Backend: backend}).BuildExecutionRequest(SandboxManagerRequest{
342+
WorkspaceRoot: "/workspace",
343+
Command: CommandSpec{Name: "/bin/sh", Args: []string{"-c", "true"}, Dir: "/workspace"},
344+
Policy: policy,
345+
Profile: PermissionProfileFromPolicy("/workspace", policy, nil),
346+
Preference: SandboxPreferenceAuto,
347+
ValidateExecution: true,
348+
})
349+
if err != nil {
350+
t.Fatalf("BuildExecutionRequest disabled policy: %v", err)
351+
}
352+
if request.EnforcementLevel != EnforcementDisabled || request.CommandWrapped {
353+
t.Fatalf("disabled request = %#v, want intentionally open shell", request)
354+
}
355+
356+
policy = DefaultPolicy()
357+
request, err = NewSandboxManager(SandboxManagerOptions{GOOS: "darwin", Backend: backend}).BuildExecutionRequest(SandboxManagerRequest{
358+
WorkspaceRoot: "/workspace",
359+
Command: CommandSpec{Name: "/bin/sh", Args: []string{"-c", "true"}, Dir: "/workspace"},
360+
Policy: policy,
361+
Profile: PermissionProfileFromPolicy("/workspace", policy, nil),
362+
Preference: SandboxPreferenceForbid,
363+
ValidateExecution: true,
364+
})
365+
if err != nil {
366+
t.Fatalf("BuildExecutionRequest forbidden sandbox preference: %v", err)
367+
}
368+
if request.EnforcementLevel != EnforcementDisabled || request.CommandWrapped {
369+
t.Fatalf("forbidden-sandbox request = %#v, want intentionally open shell", request)
370+
}
371+
}
372+
253373
func TestSandboxManagerSelectsPlatformBackend(t *testing.T) {
254374
tests := []struct {
255375
name string

‎internal/sandbox/runner.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1001,10 +1001,10 @@ func seatbeltPlatformRuntimeRules() string {
10011001
` (literal "/dev/zero"))`,
10021002
`(allow file-read-data file-test-existence file-write-data (subpath "/dev/fd"))`,
10031003
`(allow file-read* file-test-existence file-write-data file-ioctl (literal "/dev/dtracehelper"))`,
1004-
`(allow file-read* file-test-existence file-write* (subpath "/tmp"))`,
1005-
`(allow file-read* file-write* (subpath "/private/tmp"))`,
1006-
`(allow file-read* file-write* (subpath "/var/tmp"))`,
1007-
`(allow file-read* file-write* (subpath "/private/var/tmp"))`,
1004+
`(allow file-read* file-test-existence (subpath "/tmp"))`,
1005+
`(allow file-read* (subpath "/private/tmp"))`,
1006+
`(allow file-read* (subpath "/var/tmp"))`,
1007+
`(allow file-read* (subpath "/private/var/tmp"))`,
10081008
`(allow file-read* file-test-existence`,
10091009
` (literal "/System/Library/CoreServices")`,
10101010
` (literal "/System/Library/CoreServices/.SystemVersionPlatform.plist")`,

‎internal/sandbox/runner_test.go‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -515,6 +515,21 @@ func TestSeatbeltProfileProtectsMetadataAndDenyOrdering(t *testing.T) {
515515
}
516516
}
517517

518+
func TestSeatbeltTemporaryWritesFollowPermissionProfile(t *testing.T) {
519+
runtimeRules := seatbeltPlatformRuntimeRules()
520+
if strings.Contains(runtimeRules, `file-write* (subpath "/tmp")`) {
521+
t.Fatal("platform runtime rules must not bypass FileSystemPolicy.AllowTemp")
522+
}
523+
withoutTemp := seatbeltWriteRule(FileSystemPolicy{Kind: FileSystemRestricted})
524+
if strings.Contains(withoutTemp, `(subpath "/tmp")`) {
525+
t.Fatal("restricted profile with AllowTemp=false unexpectedly permits /tmp writes")
526+
}
527+
withTemp := seatbeltWriteRule(FileSystemPolicy{Kind: FileSystemRestricted, AllowTemp: true})
528+
if !strings.Contains(withTemp, `(subpath "/tmp")`) {
529+
t.Fatal("restricted profile with AllowTemp=true must permit /tmp writes")
530+
}
531+
}
532+
518533
// TestSeatbeltProfileAllowsGitWritesExceptHooksAndConfig locks in the fix for
519534
// git subprocesses (fetch, commit, add, ...) failing under the sandbox: the
520535
// default profile must stop write-denying the whole .git tree and only carve

0 commit comments

Comments
 (0)