Skip to content
12 changes: 9 additions & 3 deletions internal/agent/loop.go
Original file line number Diff line number Diff line change
Expand Up @@ -1617,9 +1617,15 @@ func sandboxRestrictedShellRetryCandidate(call ToolCall, args map[string]any, re

func sandboxDeniedShellResult(result tools.Result) bool {
if result.ExecutionOutcome != nil {
// Typed denials must be handled by their exact capability path. Never
// turn a structured narrow denial into the legacy unrestricted retry.
return false
// Only a typed platform denial that explicitly requests unrestricted
// recovery may enter the legacy retry path. Narrow capability denials
// remain on their exact capability path.
denial := result.ExecutionOutcome.Denial
return denial != nil &&
denial.Source == execution.DenialSourcePlatformSandbox &&
denial.Capability.Kind == execution.CapabilityUnrestricted &&
denial.Recoverable &&
denial.NextAction == execution.DenialNextActionRequestApproval
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}
return result.Status == tools.StatusError && result.Meta[tools.SandboxLikelyDeniedMeta] == "true"
}
Expand Down
28 changes: 28 additions & 0 deletions internal/agent/loop_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,22 @@ func TestTypedExecutionOutcomeOverridesLegacySandboxHeuristics(t *testing.T) {
t.Fatal("narrow protected-metadata denial must not become an unrestricted retry")
}

nonRecoverableDenial := protectedDenial
nonRecoverableDenial.ExecutionOutcome = &execution.Outcome{
State: execution.StateDenied,
Kind: execution.OutcomeEnforcementDenied,
Denial: &execution.Denial{
Capability: execution.Capability{Kind: execution.CapabilityUnrestricted, Scope: "host"},
Source: execution.DenialSourcePlatformSandbox,
Reason: "sandbox enforcement failed closed",
Recoverable: false,
NextAction: execution.DenialNextActionRequestApproval,
},
}
if sandboxRestrictedShellRetryCandidate(call, nil, nonRecoverableDenial, options) {
t.Fatal("non-recoverable sandbox denial must not become an unrestricted retry")
}

networkDenial := protectedDenial
networkDenial.ExecutionOutcome = &execution.Outcome{
State: execution.StateDenied,
Expand Down Expand Up @@ -288,6 +304,18 @@ func (tool *sandboxDeniedRetryTool) Run(_ context.Context, args map[string]any)
return tools.Result{
Status: tools.StatusError,
Output: "touch: cannot touch '/home/user/.npm/cache': Read-only file system",
ExecutionOutcome: &execution.Outcome{
State: execution.StateDenied,
Kind: execution.OutcomeEnforcementDenied,
Exit: &execution.Exit{Code: 1},
Denial: &execution.Denial{
Capability: execution.Capability{Kind: execution.CapabilityUnrestricted, Scope: "host"},
Source: execution.DenialSourcePlatformSandbox,
Reason: "sandbox blocked command execution",
Recoverable: true,
NextAction: execution.DenialNextActionRequestApproval,
},
},
Meta: map[string]string{
"exit_code": "1",
tools.SandboxLikelyDeniedMeta: "true",
Expand Down
94 changes: 92 additions & 2 deletions internal/sandbox/manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"path/filepath"
"reflect"
"runtime"
"strings"
"testing"
)

Expand Down Expand Up @@ -378,7 +379,31 @@ func TestCredentialDenyReadPathsIn(t *testing.T) {
home := t.TempDir()
awsDir := filepath.Join(home, ".aws")
gcloudDir := filepath.Join(home, ".config", "gcloud")
if err := mkdirAll(awsDir, gcloudDir); err != nil {
npmrc := filepath.Join(home, ".npmrc")
ghHosts := filepath.Join(home, ".config", "gh", "hosts.yml")
netrc := filepath.Join(home, ".netrc")
dockerConfig := filepath.Join(home, ".docker", "config.json")
kubeConfig := filepath.Join(home, ".kube", "config")
zeroConfig := filepath.Join(home, ".config", "zero")
if err := mkdirAll(
awsDir,
gcloudDir,
filepath.Dir(ghHosts),
filepath.Dir(dockerConfig),
filepath.Dir(kubeConfig),
filepath.Dir(zeroConfig),
); err != nil {
t.Fatal(err)
}
for _, path := range []string{npmrc, ghHosts, netrc, dockerConfig, kubeConfig, zeroConfig} {
if path == zeroConfig {
continue
}
if err := os.WriteFile(path, []byte("{}"), 0o600); err != nil {
t.Fatal(err)
}
}
if err := os.MkdirAll(zeroConfig, 0o755); err != nil {
t.Fatal(err)
}
keyFile := filepath.Join(home, "sa-key.json")
Expand All @@ -387,7 +412,17 @@ func TestCredentialDenyReadPathsIn(t *testing.T) {
}

paths := credentialDenyReadPathsIn(home, keyFile, nil)
for _, want := range normalizeProfilePaths([]string{awsDir, gcloudDir, keyFile}) {
for _, want := range normalizeProfilePaths([]string{
awsDir,
gcloudDir,
keyFile,
npmrc,
ghHosts,
netrc,
dockerConfig,
kubeConfig,
zeroConfig,
}) {
if !stringSliceContains(paths, want) {
t.Errorf("credential deny paths = %#v, want %q included", paths, want)
}
Expand Down Expand Up @@ -418,3 +453,58 @@ func TestCredentialDenyReadPathsIn(t *testing.T) {
t.Errorf("credential deny paths without home = %#v, want key file included", homeless)
}
}

func TestCredentialDenyReadPathsForEnvironmentHonorsConfigOverrides(t *testing.T) {
root := t.TempDir()
configHome := filepath.Join(root, "xdg")
npmrc := filepath.Join(root, "npm", "userconfig")
ghConfigDir := filepath.Join(root, "gh")
cloudSDKConfig := filepath.Join(root, "gcloud")
netrc := filepath.Join(root, "netrc")
dockerConfigDir := filepath.Join(root, "docker")
kubeConfigA := filepath.Join(root, "kube", "a")
kubeConfigB := filepath.Join(root, "kube", "b")
zeroConfig := filepath.Join(configHome, "zero")
filePaths := []string{
npmrc,
filepath.Join(ghConfigDir, "hosts.yml"),
netrc,
filepath.Join(dockerConfigDir, "config.json"),
kubeConfigA,
kubeConfigB,
zeroConfig,
}
if err := os.MkdirAll(cloudSDKConfig, 0o755); err != nil {
t.Fatal(err)
}
for _, path := range filePaths {
if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil {
t.Fatal(err)
}
if path == zeroConfig {
if err := os.MkdirAll(path, 0o755); err != nil {
t.Fatal(err)
}
continue
}
if err := os.WriteFile(path, []byte("{}"), 0o600); err != nil {
t.Fatal(err)
}
}

got := credentialDenyReadPathsForEnvironment(credentialPathEnvironment{
ConfigHome: configHome,
CloudSDKConfig: cloudSDKConfig,
NPMUserConfig: npmrc,
GHConfigDir: ghConfigDir,
Netrc: netrc,
DockerConfigDir: dockerConfigDir,
KubeConfig: strings.Join([]string{kubeConfigA, kubeConfigB}, string(filepath.ListSeparator)),
}, nil)
wantPaths := append(filePaths, cloudSDKConfig)
for _, want := range normalizeProfilePaths(wantPaths) {
if !stringSliceContains(got, want) {
t.Errorf("credential deny paths = %#v, want override %q included", got, want)
}
}
}
103 changes: 91 additions & 12 deletions internal/sandbox/profile.go
Original file line number Diff line number Diff line change
Expand Up @@ -145,9 +145,10 @@
}

// credentialDenyReadPaths returns default deny-read entries for well-known
// credential stores (~/.aws, ~/.config/gcloud, ~/.azure, and the file
// GOOGLE_APPLICATION_CREDENTIALS points to) so sandboxed commands cannot read
// cloud secrets under the read-all workspace posture. Two deliberate limits:
// credential stores so sandboxed commands cannot read secrets under the
// read-all workspace posture. This includes tool configuration files that can
// carry credentials and are now discoverable through the preserved caller
// environment. Two deliberate limits:
//
// - Windows is skipped: a non-empty profile DenyRead switches the Windows
// runner onto the capability-SID/ACL deny path and away from the
Expand All @@ -166,23 +167,100 @@
// A failed home lookup only drops the home-based candidates; the
// GOOGLE_APPLICATION_CREDENTIALS target must be protected regardless.
home, _ := os.UserHomeDir()
return credentialDenyReadPathsIn(home, os.Getenv("GOOGLE_APPLICATION_CREDENTIALS"), policy.AllowRead)
return credentialDenyReadPathsForEnvironment(credentialPathEnvironment{
Home: home,
ConfigHome: os.Getenv("XDG_CONFIG_HOME"),
CloudSDKConfig: os.Getenv("CLOUDSDK_CONFIG"),
GoogleCredentials: os.Getenv("GOOGLE_APPLICATION_CREDENTIALS"),
NPMUserConfig: firstNonEmpty(os.Getenv("NPM_CONFIG_USERCONFIG"), os.Getenv("npm_config_userconfig")),
GHConfigDir: os.Getenv("GH_CONFIG_DIR"),
Netrc: os.Getenv("NETRC"),
DockerConfigDir: os.Getenv("DOCKER_CONFIG"),
KubeConfig: os.Getenv("KUBECONFIG"),
}, policy.AllowRead)
}

// credentialDenyReadPathsIn is the pure core of credentialDenyReadPaths,
// separated so tests can exercise it against a synthetic home directory.
func credentialDenyReadPathsIn(home string, googleCredentials string, allowRead []string) []string {

Check failure on line 185 in internal/sandbox/profile.go

View workflow job for this annotation

GitHub Actions / Security & code health

unreachable func: credentialDenyReadPathsIn
return credentialDenyReadPathsForEnvironment(credentialPathEnvironment{
Home: home,
GoogleCredentials: googleCredentials,
}, allowRead)
}

type credentialPathEnvironment struct {
Home string
ConfigHome string
CloudSDKConfig string
GoogleCredentials string
NPMUserConfig string
GHConfigDir string
Netrc string
DockerConfigDir string
KubeConfig string
}

func credentialDenyReadPathsForEnvironment(env credentialPathEnvironment, allowRead []string) []string {
var candidates []string
if home = strings.TrimSpace(home); home != "" {
home := strings.TrimSpace(env.Home)
configHome := strings.TrimSpace(env.ConfigHome)
if configHome == "" && home != "" {
configHome = filepath.Join(home, ".config")
}
if home != "" {
candidates = append(candidates,
filepath.Join(home, ".aws"),
filepath.Join(home, ".config", "gcloud"),
filepath.Join(home, ".azure"),
)
}
if target := strings.TrimSpace(googleCredentials); target != "" {
npmUserConfig := strings.TrimSpace(env.NPMUserConfig)
if npmUserConfig == "" && home != "" {
npmUserConfig = filepath.Join(home, ".npmrc")
}
netrc := strings.TrimSpace(env.Netrc)
if netrc == "" && home != "" {
netrc = filepath.Join(home, ".netrc")
}
dockerConfigDir := strings.TrimSpace(env.DockerConfigDir)
if dockerConfigDir == "" && home != "" {
dockerConfigDir = filepath.Join(home, ".docker")
}
candidates = append(candidates, npmUserConfig, netrc)
if dockerConfigDir != "" {
candidates = append(candidates, filepath.Join(dockerConfigDir, "config.json"))
}
if configHome != "" {
candidates = append(candidates,
filepath.Join(configHome, "zero"),
)
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
cloudSDKConfig := strings.TrimSpace(env.CloudSDKConfig)
if cloudSDKConfig == "" && configHome != "" {
cloudSDKConfig = filepath.Join(configHome, "gcloud")
}
if cloudSDKConfig != "" {
candidates = append(candidates, cloudSDKConfig)
}
ghConfigDir := strings.TrimSpace(env.GHConfigDir)
if ghConfigDir == "" && configHome != "" {
ghConfigDir = filepath.Join(configHome, "gh")
}
if ghConfigDir != "" {
candidates = append(candidates, filepath.Join(ghConfigDir, "hosts.yml"))
}
if target := strings.TrimSpace(env.GoogleCredentials); target != "" {
candidates = append(candidates, target)
}
kubeConfig := strings.TrimSpace(env.KubeConfig)
if kubeConfig == "" && home != "" {
kubeConfig = filepath.Join(home, ".kube", "config")
}
for _, path := range filepath.SplitList(kubeConfig) {
if path = strings.TrimSpace(path); path != "" {
candidates = append(candidates, path)
}
}
allowRoots := normalizeProfilePaths(allowRead)
out := make([]string, 0, len(candidates))
for _, path := range normalizeProfilePaths(candidates) {
Expand All @@ -206,11 +284,12 @@

// userGitConfigReadPaths returns the user's global git config FILES so a
// sandboxed git can read identity and config (user.name/email, aliases) instead
// of failing with "unable to access ~/.gitconfig". It is deliberately the config
// files only — not the ~/.config/git directory, which can hold an XDG credential
// store — so credentials and the rest of HOME stay unreadable. Granted at the
// macOS-seatbelt read rule (not the cross-platform PermissionProfile) so the
// HOME-dependent paths don't leak into the platform-agnostic policy snapshot.
// of failing with "unable to access ~/.gitconfig". On macOS, where reads are
// allow-listed, granting only these files avoids exposing the surrounding
// configuration directory. Linux uses a read-all profile with explicit
// credential deny rules. The paths are granted at the macOS seatbelt read rule
// rather than the cross-platform PermissionProfile so HOME-dependent paths
// don't leak into the platform-agnostic policy snapshot.
func userGitConfigReadPaths() []string {
home, err := os.UserHomeDir()
if err != nil || strings.TrimSpace(home) == "" {
Expand Down
30 changes: 8 additions & 22 deletions internal/sandbox/runtime_state.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,10 @@ var fallbackSandboxRuntimes = struct {
}{roots: make(map[string]string)}

type SandboxRuntime struct {
Root string `json:"root,omitempty"`
Home string `json:"home,omitempty"`
Cache string `json:"cache,omitempty"`
Config string `json:"config,omitempty"`
Data string `json:"data,omitempty"`
State string `json:"state,omitempty"`
Temp string `json:"temp,omitempty"`
Root string `json:"root,omitempty"`
Cache string `json:"cache,omitempty"`
Data string `json:"data,omitempty"`
Temp string `json:"temp,omitempty"`
}

func prepareSandboxRuntime(workspaceRoot string) (SandboxRuntime, func(), error) {
Expand Down Expand Up @@ -75,21 +72,15 @@ func prepareSandboxRuntime(workspaceRoot string) (SandboxRuntime, func(), error)
}
}()
runtimeState := SandboxRuntime{
Root: root,
Home: filepath.Join(root, "home"),
Cache: filepath.Join(root, "cache"),
Config: filepath.Join(root, "config"),
Data: filepath.Join(root, "data"),
State: filepath.Join(root, "state"),
Temp: filepath.Join(root, "tmp"),
Root: root,
Cache: filepath.Join(root, "cache"),
Data: filepath.Join(root, "data"),
Temp: filepath.Join(root, "tmp"),
}
directories := []string{
runtimeState.Root,
runtimeState.Home,
runtimeState.Cache,
runtimeState.Config,
runtimeState.Data,
runtimeState.State,
runtimeState.Temp,
filepath.Join(runtimeState.Cache, "npm"),
filepath.Join(runtimeState.Cache, "yarn"),
Expand Down Expand Up @@ -207,16 +198,11 @@ func sandboxRuntimeEnvironment(env []string, runtimeState *SandboxRuntime) []str
return env
}
overrides := []string{
"HOME=" + runtimeState.Home,
"XDG_CACHE_HOME=" + runtimeState.Cache,
"XDG_CONFIG_HOME=" + runtimeState.Config,
"XDG_DATA_HOME=" + runtimeState.Data,
"XDG_STATE_HOME=" + runtimeState.State,
"TMPDIR=" + runtimeState.Temp,
"TMP=" + runtimeState.Temp,
"TEMP=" + runtimeState.Temp,
"npm_config_cache=" + filepath.Join(runtimeState.Cache, "npm"),
"NPM_CONFIG_USERCONFIG=" + filepath.Join(runtimeState.Config, "npmrc"),
"YARN_CACHE_FOLDER=" + filepath.Join(runtimeState.Cache, "yarn"),
"COREPACK_HOME=" + filepath.Join(runtimeState.Cache, "corepack"),
"PIP_CACHE_DIR=" + filepath.Join(runtimeState.Cache, "pip"),
Expand Down
Loading
Loading