Skip to content

Commit 81a5e6c

Browse files
authored
fix(sandbox): preserve user config and retry denied writes (#801)
* fix(sandbox): preserve user config and retry denied writes Keep user identity and configuration paths discoverable while retaining managed cache and temporary storage. Classify native sandbox failures as typed recoverable denials so approved commands can retry outside the sandbox. Add regression coverage for global Git ignores, typed denial handling, and permission retry behavior. * fix(sandbox): tighten denial retry safeguards * test(sandbox): assert denial contract * fix(sandbox): trust typed denials and restore Cargo state Stop promoting command output keywords into recoverable unrestricted denials, so only structured adapter facts can enter the approved unsandboxed retry path. Restore the managed CARGO_HOME directory and environment override so Cargo remains writable while the caller's real home stays read-only. * fix(sandbox): mask preserved credential configs Preserving caller configuration made credential-bearing tool files discoverable under Linux's read-all sandbox profile. Add default and environment-overridden npm, GitHub CLI, netrc, Docker, Kubernetes, and Zero config paths to the credential deny set, with regression coverage. Clarify the typed-denial retry and platform read-model comments without changing the approval behavior. * fix(sandbox): protect custom gcloud config CLOUDSDK_CONFIG can relocate gcloud credentials outside the XDG-derived default. Include the configured directory in the credential deny profile and cover the override with a regression test. * fix(sandbox): deny zero credential directory * test(sandbox): keep zero credential directory deny path expectations
1 parent 5d1869e commit 81a5e6c

7 files changed

Lines changed: 391 additions & 61 deletions

File tree

‎internal/agent/loop.go‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1617,9 +1617,15 @@ func sandboxRestrictedShellRetryCandidate(call ToolCall, args map[string]any, re
16171617

16181618
func sandboxDeniedShellResult(result tools.Result) bool {
16191619
if result.ExecutionOutcome != nil {
1620-
// Typed denials must be handled by their exact capability path. Never
1621-
// turn a structured narrow denial into the legacy unrestricted retry.
1622-
return false
1620+
// Only a typed platform denial that explicitly requests unrestricted
1621+
// recovery may enter the legacy retry path. Narrow capability denials
1622+
// remain on their exact capability path.
1623+
denial := result.ExecutionOutcome.Denial
1624+
return denial != nil &&
1625+
denial.Source == execution.DenialSourcePlatformSandbox &&
1626+
denial.Capability.Kind == execution.CapabilityUnrestricted &&
1627+
denial.Recoverable &&
1628+
denial.NextAction == execution.DenialNextActionRequestApproval
16231629
}
16241630
return result.Status == tools.StatusError && result.Meta[tools.SandboxLikelyDeniedMeta] == "true"
16251631
}

‎internal/agent/loop_test.go‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,22 @@ func TestTypedExecutionOutcomeOverridesLegacySandboxHeuristics(t *testing.T) {
6464
t.Fatal("narrow protected-metadata denial must not become an unrestricted retry")
6565
}
6666

67+
nonRecoverableDenial := protectedDenial
68+
nonRecoverableDenial.ExecutionOutcome = &execution.Outcome{
69+
State: execution.StateDenied,
70+
Kind: execution.OutcomeEnforcementDenied,
71+
Denial: &execution.Denial{
72+
Capability: execution.Capability{Kind: execution.CapabilityUnrestricted, Scope: "host"},
73+
Source: execution.DenialSourcePlatformSandbox,
74+
Reason: "sandbox enforcement failed closed",
75+
Recoverable: false,
76+
NextAction: execution.DenialNextActionRequestApproval,
77+
},
78+
}
79+
if sandboxRestrictedShellRetryCandidate(call, nil, nonRecoverableDenial, options) {
80+
t.Fatal("non-recoverable sandbox denial must not become an unrestricted retry")
81+
}
82+
6783
networkDenial := protectedDenial
6884
networkDenial.ExecutionOutcome = &execution.Outcome{
6985
State: execution.StateDenied,
@@ -288,6 +304,18 @@ func (tool *sandboxDeniedRetryTool) Run(_ context.Context, args map[string]any)
288304
return tools.Result{
289305
Status: tools.StatusError,
290306
Output: "touch: cannot touch '/home/user/.npm/cache': Read-only file system",
307+
ExecutionOutcome: &execution.Outcome{
308+
State: execution.StateDenied,
309+
Kind: execution.OutcomeEnforcementDenied,
310+
Exit: &execution.Exit{Code: 1},
311+
Denial: &execution.Denial{
312+
Capability: execution.Capability{Kind: execution.CapabilityUnrestricted, Scope: "host"},
313+
Source: execution.DenialSourcePlatformSandbox,
314+
Reason: "sandbox blocked command execution",
315+
Recoverable: true,
316+
NextAction: execution.DenialNextActionRequestApproval,
317+
},
318+
},
291319
Meta: map[string]string{
292320
"exit_code": "1",
293321
tools.SandboxLikelyDeniedMeta: "true",

‎internal/sandbox/manager_test.go‎

Lines changed: 92 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"path/filepath"
77
"reflect"
88
"runtime"
9+
"strings"
910
"testing"
1011
)
1112

@@ -378,7 +379,31 @@ func TestCredentialDenyReadPathsIn(t *testing.T) {
378379
home := t.TempDir()
379380
awsDir := filepath.Join(home, ".aws")
380381
gcloudDir := filepath.Join(home, ".config", "gcloud")
381-
if err := mkdirAll(awsDir, gcloudDir); err != nil {
382+
npmrc := filepath.Join(home, ".npmrc")
383+
ghHosts := filepath.Join(home, ".config", "gh", "hosts.yml")
384+
netrc := filepath.Join(home, ".netrc")
385+
dockerConfig := filepath.Join(home, ".docker", "config.json")
386+
kubeConfig := filepath.Join(home, ".kube", "config")
387+
zeroConfig := filepath.Join(home, ".config", "zero")
388+
if err := mkdirAll(
389+
awsDir,
390+
gcloudDir,
391+
filepath.Dir(ghHosts),
392+
filepath.Dir(dockerConfig),
393+
filepath.Dir(kubeConfig),
394+
filepath.Dir(zeroConfig),
395+
); err != nil {
396+
t.Fatal(err)
397+
}
398+
for _, path := range []string{npmrc, ghHosts, netrc, dockerConfig, kubeConfig, zeroConfig} {
399+
if path == zeroConfig {
400+
continue
401+
}
402+
if err := os.WriteFile(path, []byte("{}"), 0o600); err != nil {
403+
t.Fatal(err)
404+
}
405+
}
406+
if err := os.MkdirAll(zeroConfig, 0o755); err != nil {
382407
t.Fatal(err)
383408
}
384409
keyFile := filepath.Join(home, "sa-key.json")
@@ -387,7 +412,17 @@ func TestCredentialDenyReadPathsIn(t *testing.T) {
387412
}
388413

389414
paths := credentialDenyReadPathsIn(home, keyFile, nil)
390-
for _, want := range normalizeProfilePaths([]string{awsDir, gcloudDir, keyFile}) {
415+
for _, want := range normalizeProfilePaths([]string{
416+
awsDir,
417+
gcloudDir,
418+
keyFile,
419+
npmrc,
420+
ghHosts,
421+
netrc,
422+
dockerConfig,
423+
kubeConfig,
424+
zeroConfig,
425+
}) {
391426
if !stringSliceContains(paths, want) {
392427
t.Errorf("credential deny paths = %#v, want %q included", paths, want)
393428
}
@@ -418,3 +453,58 @@ func TestCredentialDenyReadPathsIn(t *testing.T) {
418453
t.Errorf("credential deny paths without home = %#v, want key file included", homeless)
419454
}
420455
}
456+
457+
func TestCredentialDenyReadPathsForEnvironmentHonorsConfigOverrides(t *testing.T) {
458+
root := t.TempDir()
459+
configHome := filepath.Join(root, "xdg")
460+
npmrc := filepath.Join(root, "npm", "userconfig")
461+
ghConfigDir := filepath.Join(root, "gh")
462+
cloudSDKConfig := filepath.Join(root, "gcloud")
463+
netrc := filepath.Join(root, "netrc")
464+
dockerConfigDir := filepath.Join(root, "docker")
465+
kubeConfigA := filepath.Join(root, "kube", "a")
466+
kubeConfigB := filepath.Join(root, "kube", "b")
467+
zeroConfig := filepath.Join(configHome, "zero")
468+
filePaths := []string{
469+
npmrc,
470+
filepath.Join(ghConfigDir, "hosts.yml"),
471+
netrc,
472+
filepath.Join(dockerConfigDir, "config.json"),
473+
kubeConfigA,
474+
kubeConfigB,
475+
zeroConfig,
476+
}
477+
if err := os.MkdirAll(cloudSDKConfig, 0o755); err != nil {
478+
t.Fatal(err)
479+
}
480+
for _, path := range filePaths {
481+
if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil {
482+
t.Fatal(err)
483+
}
484+
if path == zeroConfig {
485+
if err := os.MkdirAll(path, 0o755); err != nil {
486+
t.Fatal(err)
487+
}
488+
continue
489+
}
490+
if err := os.WriteFile(path, []byte("{}"), 0o600); err != nil {
491+
t.Fatal(err)
492+
}
493+
}
494+
495+
got := credentialDenyReadPathsForEnvironment(credentialPathEnvironment{
496+
ConfigHome: configHome,
497+
CloudSDKConfig: cloudSDKConfig,
498+
NPMUserConfig: npmrc,
499+
GHConfigDir: ghConfigDir,
500+
Netrc: netrc,
501+
DockerConfigDir: dockerConfigDir,
502+
KubeConfig: strings.Join([]string{kubeConfigA, kubeConfigB}, string(filepath.ListSeparator)),
503+
}, nil)
504+
wantPaths := append(filePaths, cloudSDKConfig)
505+
for _, want := range normalizeProfilePaths(wantPaths) {
506+
if !stringSliceContains(got, want) {
507+
t.Errorf("credential deny paths = %#v, want override %q included", got, want)
508+
}
509+
}
510+
}

‎internal/sandbox/profile.go‎

Lines changed: 91 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -145,9 +145,10 @@ func permissionProfileReadRoots(workspaceRoot string, policy Policy, scope *Scop
145145
}
146146

147147
// credentialDenyReadPaths returns default deny-read entries for well-known
148-
// credential stores (~/.aws, ~/.config/gcloud, ~/.azure, and the file
149-
// GOOGLE_APPLICATION_CREDENTIALS points to) so sandboxed commands cannot read
150-
// cloud secrets under the read-all workspace posture. Two deliberate limits:
148+
// credential stores so sandboxed commands cannot read secrets under the
149+
// read-all workspace posture. This includes tool configuration files that can
150+
// carry credentials and are now discoverable through the preserved caller
151+
// environment. Two deliberate limits:
151152
//
152153
// - Windows is skipped: a non-empty profile DenyRead switches the Windows
153154
// runner onto the capability-SID/ACL deny path and away from the
@@ -166,23 +167,100 @@ func credentialDenyReadPaths(policy Policy) []string {
166167
// A failed home lookup only drops the home-based candidates; the
167168
// GOOGLE_APPLICATION_CREDENTIALS target must be protected regardless.
168169
home, _ := os.UserHomeDir()
169-
return credentialDenyReadPathsIn(home, os.Getenv("GOOGLE_APPLICATION_CREDENTIALS"), policy.AllowRead)
170+
return credentialDenyReadPathsForEnvironment(credentialPathEnvironment{
171+
Home: home,
172+
ConfigHome: os.Getenv("XDG_CONFIG_HOME"),
173+
CloudSDKConfig: os.Getenv("CLOUDSDK_CONFIG"),
174+
GoogleCredentials: os.Getenv("GOOGLE_APPLICATION_CREDENTIALS"),
175+
NPMUserConfig: firstNonEmpty(os.Getenv("NPM_CONFIG_USERCONFIG"), os.Getenv("npm_config_userconfig")),
176+
GHConfigDir: os.Getenv("GH_CONFIG_DIR"),
177+
Netrc: os.Getenv("NETRC"),
178+
DockerConfigDir: os.Getenv("DOCKER_CONFIG"),
179+
KubeConfig: os.Getenv("KUBECONFIG"),
180+
}, policy.AllowRead)
170181
}
171182

172183
// credentialDenyReadPathsIn is the pure core of credentialDenyReadPaths,
173184
// separated so tests can exercise it against a synthetic home directory.
174185
func credentialDenyReadPathsIn(home string, googleCredentials string, allowRead []string) []string {
186+
return credentialDenyReadPathsForEnvironment(credentialPathEnvironment{
187+
Home: home,
188+
GoogleCredentials: googleCredentials,
189+
}, allowRead)
190+
}
191+
192+
type credentialPathEnvironment struct {
193+
Home string
194+
ConfigHome string
195+
CloudSDKConfig string
196+
GoogleCredentials string
197+
NPMUserConfig string
198+
GHConfigDir string
199+
Netrc string
200+
DockerConfigDir string
201+
KubeConfig string
202+
}
203+
204+
func credentialDenyReadPathsForEnvironment(env credentialPathEnvironment, allowRead []string) []string {
175205
var candidates []string
176-
if home = strings.TrimSpace(home); home != "" {
206+
home := strings.TrimSpace(env.Home)
207+
configHome := strings.TrimSpace(env.ConfigHome)
208+
if configHome == "" && home != "" {
209+
configHome = filepath.Join(home, ".config")
210+
}
211+
if home != "" {
177212
candidates = append(candidates,
178213
filepath.Join(home, ".aws"),
179-
filepath.Join(home, ".config", "gcloud"),
180214
filepath.Join(home, ".azure"),
181215
)
182216
}
183-
if target := strings.TrimSpace(googleCredentials); target != "" {
217+
npmUserConfig := strings.TrimSpace(env.NPMUserConfig)
218+
if npmUserConfig == "" && home != "" {
219+
npmUserConfig = filepath.Join(home, ".npmrc")
220+
}
221+
netrc := strings.TrimSpace(env.Netrc)
222+
if netrc == "" && home != "" {
223+
netrc = filepath.Join(home, ".netrc")
224+
}
225+
dockerConfigDir := strings.TrimSpace(env.DockerConfigDir)
226+
if dockerConfigDir == "" && home != "" {
227+
dockerConfigDir = filepath.Join(home, ".docker")
228+
}
229+
candidates = append(candidates, npmUserConfig, netrc)
230+
if dockerConfigDir != "" {
231+
candidates = append(candidates, filepath.Join(dockerConfigDir, "config.json"))
232+
}
233+
if configHome != "" {
234+
candidates = append(candidates,
235+
filepath.Join(configHome, "zero"),
236+
)
237+
}
238+
cloudSDKConfig := strings.TrimSpace(env.CloudSDKConfig)
239+
if cloudSDKConfig == "" && configHome != "" {
240+
cloudSDKConfig = filepath.Join(configHome, "gcloud")
241+
}
242+
if cloudSDKConfig != "" {
243+
candidates = append(candidates, cloudSDKConfig)
244+
}
245+
ghConfigDir := strings.TrimSpace(env.GHConfigDir)
246+
if ghConfigDir == "" && configHome != "" {
247+
ghConfigDir = filepath.Join(configHome, "gh")
248+
}
249+
if ghConfigDir != "" {
250+
candidates = append(candidates, filepath.Join(ghConfigDir, "hosts.yml"))
251+
}
252+
if target := strings.TrimSpace(env.GoogleCredentials); target != "" {
184253
candidates = append(candidates, target)
185254
}
255+
kubeConfig := strings.TrimSpace(env.KubeConfig)
256+
if kubeConfig == "" && home != "" {
257+
kubeConfig = filepath.Join(home, ".kube", "config")
258+
}
259+
for _, path := range filepath.SplitList(kubeConfig) {
260+
if path = strings.TrimSpace(path); path != "" {
261+
candidates = append(candidates, path)
262+
}
263+
}
186264
allowRoots := normalizeProfilePaths(allowRead)
187265
out := make([]string, 0, len(candidates))
188266
for _, path := range normalizeProfilePaths(candidates) {
@@ -206,11 +284,12 @@ func credentialDenyReadPathsIn(home string, googleCredentials string, allowRead
206284

207285
// userGitConfigReadPaths returns the user's global git config FILES so a
208286
// sandboxed git can read identity and config (user.name/email, aliases) instead
209-
// of failing with "unable to access ~/.gitconfig". It is deliberately the config
210-
// files only — not the ~/.config/git directory, which can hold an XDG credential
211-
// store — so credentials and the rest of HOME stay unreadable. Granted at the
212-
// macOS-seatbelt read rule (not the cross-platform PermissionProfile) so the
213-
// HOME-dependent paths don't leak into the platform-agnostic policy snapshot.
287+
// of failing with "unable to access ~/.gitconfig". On macOS, where reads are
288+
// allow-listed, granting only these files avoids exposing the surrounding
289+
// configuration directory. Linux uses a read-all profile with explicit
290+
// credential deny rules. The paths are granted at the macOS seatbelt read rule
291+
// rather than the cross-platform PermissionProfile so HOME-dependent paths
292+
// don't leak into the platform-agnostic policy snapshot.
214293
func userGitConfigReadPaths() []string {
215294
home, err := os.UserHomeDir()
216295
if err != nil || strings.TrimSpace(home) == "" {

‎internal/sandbox/runtime_state.go‎

Lines changed: 8 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -27,13 +27,10 @@ var fallbackSandboxRuntimes = struct {
2727
}{roots: make(map[string]string)}
2828

2929
type SandboxRuntime struct {
30-
Root string `json:"root,omitempty"`
31-
Home string `json:"home,omitempty"`
32-
Cache string `json:"cache,omitempty"`
33-
Config string `json:"config,omitempty"`
34-
Data string `json:"data,omitempty"`
35-
State string `json:"state,omitempty"`
36-
Temp string `json:"temp,omitempty"`
30+
Root string `json:"root,omitempty"`
31+
Cache string `json:"cache,omitempty"`
32+
Data string `json:"data,omitempty"`
33+
Temp string `json:"temp,omitempty"`
3734
}
3835

3936
func prepareSandboxRuntime(workspaceRoot string) (SandboxRuntime, func(), error) {
@@ -75,21 +72,15 @@ func prepareSandboxRuntime(workspaceRoot string) (SandboxRuntime, func(), error)
7572
}
7673
}()
7774
runtimeState := SandboxRuntime{
78-
Root: root,
79-
Home: filepath.Join(root, "home"),
80-
Cache: filepath.Join(root, "cache"),
81-
Config: filepath.Join(root, "config"),
82-
Data: filepath.Join(root, "data"),
83-
State: filepath.Join(root, "state"),
84-
Temp: filepath.Join(root, "tmp"),
75+
Root: root,
76+
Cache: filepath.Join(root, "cache"),
77+
Data: filepath.Join(root, "data"),
78+
Temp: filepath.Join(root, "tmp"),
8579
}
8680
directories := []string{
8781
runtimeState.Root,
88-
runtimeState.Home,
8982
runtimeState.Cache,
90-
runtimeState.Config,
9183
runtimeState.Data,
92-
runtimeState.State,
9384
runtimeState.Temp,
9485
filepath.Join(runtimeState.Cache, "npm"),
9586
filepath.Join(runtimeState.Cache, "yarn"),
@@ -207,16 +198,11 @@ func sandboxRuntimeEnvironment(env []string, runtimeState *SandboxRuntime) []str
207198
return env
208199
}
209200
overrides := []string{
210-
"HOME=" + runtimeState.Home,
211201
"XDG_CACHE_HOME=" + runtimeState.Cache,
212-
"XDG_CONFIG_HOME=" + runtimeState.Config,
213-
"XDG_DATA_HOME=" + runtimeState.Data,
214-
"XDG_STATE_HOME=" + runtimeState.State,
215202
"TMPDIR=" + runtimeState.Temp,
216203
"TMP=" + runtimeState.Temp,
217204
"TEMP=" + runtimeState.Temp,
218205
"npm_config_cache=" + filepath.Join(runtimeState.Cache, "npm"),
219-
"NPM_CONFIG_USERCONFIG=" + filepath.Join(runtimeState.Config, "npmrc"),
220206
"YARN_CACHE_FOLDER=" + filepath.Join(runtimeState.Cache, "yarn"),
221207
"COREPACK_HOME=" + filepath.Join(runtimeState.Cache, "corepack"),
222208
"PIP_CACHE_DIR=" + filepath.Join(runtimeState.Cache, "pip"),

0 commit comments

Comments
 (0)