From 6140e840945efa394f6bb8b55c1d8b2ee4609997 Mon Sep 17 00:00:00 2001 From: Vasanthdev2004 Date: Mon, 27 Jul 2026 21:53:50 +0530 Subject: [PATCH 1/2] fix(sandbox): deny reads of git's credential stores (#815) git's credential-store backend keeps host passwords and personal access tokens in cleartext, at ~/.git-credentials or ~/.config/git/credentials depending on whether the user is on the XDG layout. Neither was on the deny list, so any command the agent runs could read them. Scoped to those two files deliberately. #815 also covers SSH private keys and the GPG keyring, and those are a harder trade: denying ~/.ssh stops a sandboxed git push over SSH from working, and excluding that directory is not even sufficient, since an IdentityFile directive can point key material anywhere. These two cost nothing by comparison, because git reads them to authenticate rather than to know who the user is, so a sandboxed git keeps working and simply cannot act as the user. The XDG one denies the file rather than the directory around it, which is the opposite of the choice made for ~/.config/zero and is deliberate: ~/.config/git also holds the global git config, which userGitConfigReadPaths grants on purpose so a sandboxed git can read user.name and aliases instead of failing outright. Denying the directory would take that away. Tests assert both stores are denied, that the git config beside the XDG one stays readable, and that an explicit read grant still re-includes them like every other entry. Dropping either entry fails the first; widening the XDG entry to the directory fails the config assertion. --- internal/sandbox/profile.go | 14 ++++++ internal/sandbox/profile_test.go | 79 ++++++++++++++++++++++++++++++++ 2 files changed, 93 insertions(+) create mode 100644 internal/sandbox/profile_test.go diff --git a/internal/sandbox/profile.go b/internal/sandbox/profile.go index 8eca4c9d7..47947316c 100644 --- a/internal/sandbox/profile.go +++ b/internal/sandbox/profile.go @@ -212,6 +212,14 @@ func credentialDenyReadPathsForEnvironment(env credentialPathEnvironment, allowR candidates = append(candidates, filepath.Join(home, ".aws"), filepath.Join(home, ".azure"), + // git's credential store backend, which holds host passwords and + // personal access tokens in cleartext. Denied rather than the whole + // of ~/.ssh, because these cost nothing functionally: git reads them + // through a credential helper for authentication, not for identity, + // so a sandboxed git still works and simply cannot authenticate as + // the user. SSH key material is a harder trade and is tracked + // separately (#815). + filepath.Join(home, ".git-credentials"), ) } npmUserConfig := strings.TrimSpace(env.NPMUserConfig) @@ -233,6 +241,12 @@ func credentialDenyReadPathsForEnvironment(env credentialPathEnvironment, allowR if configHome != "" { candidates = append(candidates, filepath.Join(configHome, "zero"), + // The XDG location of the same git credential store. Denying the file + // rather than the directory is deliberate: ~/.config/git also holds + // the global git config, which userGitConfigReadPaths grants on + // purpose so a sandboxed git can read user.name and aliases instead + // of failing outright. + filepath.Join(configHome, "git", "credentials"), ) } cloudSDKConfig := strings.TrimSpace(env.CloudSDKConfig) diff --git a/internal/sandbox/profile_test.go b/internal/sandbox/profile_test.go new file mode 100644 index 000000000..04a2a94f3 --- /dev/null +++ b/internal/sandbox/profile_test.go @@ -0,0 +1,79 @@ +package sandbox + +import ( + "os" + "path/filepath" + "testing" +) + +// git's credential store holds host passwords and personal access tokens in +// cleartext, in one of two locations depending on whether the user is on the +// XDG layout. Neither was denied, so a sandboxed command could read them +// (#815). +// +// Scoped to the credential files on purpose. Denying ~/.ssh as well would stop +// a sandboxed git push over SSH from working, which is a functional trade that +// issue tracks separately; these two cost nothing, because git reads them for +// authentication rather than identity. +func TestCredentialDenyReadPathsCoversGitCredentialStores(t *testing.T) { + home := t.TempDir() + configHome := filepath.Join(home, ".config") + if err := os.MkdirAll(filepath.Join(configHome, "git"), 0o700); err != nil { + t.Fatal(err) + } + // The builder only emits paths that exist, so these have to be real. + gitCredentials := filepath.Join(home, ".git-credentials") + xdgCredentials := filepath.Join(configHome, "git", "credentials") + gitConfig := filepath.Join(configHome, "git", "config") + for _, path := range []string{gitCredentials, xdgCredentials, gitConfig} { + if err := os.WriteFile(path, []byte("x"), 0o600); err != nil { + t.Fatal(err) + } + } + + denied := credentialDenyReadPathsForEnvironment(credentialPathEnvironment{ + Home: home, + ConfigHome: configHome, + }, nil) + covered := func(target string) bool { + norm := normalizeProfilePath(target) + for _, entry := range denied { + if entry == norm || pathWithinRoot(entry, norm) { + return true + } + } + return false + } + + if !covered(gitCredentials) { + t.Fatalf("~/.git-credentials is readable; deny list = %v", denied) + } + if !covered(xdgCredentials) { + t.Fatalf("~/.config/git/credentials is readable; deny list = %v", denied) + } + // The global git config must stay readable. userGitConfigReadPaths grants it + // deliberately so a sandboxed git can read user.name and aliases rather than + // failing with "unable to access", so denying the directory instead of the + // credential file would break that on purpose-built setups. + if covered(gitConfig) { + t.Fatalf("~/.config/git/config was denied; a sandboxed git loses its identity config") + } +} + +// An explicit read grant still wins, matching how every other entry behaves: a +// user who deliberately scopes a workspace over one of these paths is not +// overridden by the default deny. +func TestGitCredentialDenyYieldsToExplicitAllowRead(t *testing.T) { + home := t.TempDir() + gitCredentials := filepath.Join(home, ".git-credentials") + if err := os.WriteFile(gitCredentials, []byte("x"), 0o600); err != nil { + t.Fatal(err) + } + + denied := credentialDenyReadPathsForEnvironment(credentialPathEnvironment{Home: home}, []string{home}) + for _, entry := range denied { + if entry == normalizeProfilePath(gitCredentials) { + t.Fatalf("explicit allowRead %q did not re-include the credential store", home) + } + } +} From afbe47f3826309c4a66cf009182f3d103447546a Mon Sep 17 00:00:00 2001 From: Vasanthdev2004 Date: Tue, 28 Jul 2026 11:50:47 +0530 Subject: [PATCH 2/2] test(sandbox): assert the credential deny exists before the grant lifts it The allow-read case only checked that the credential store was absent from the deny list once an explicit grant covered it. A deny rule that was never added satisfies that just as well, so the test passed with the production change reverted and proved nothing about the interaction it was named for. Check both runs: denied with no grant, absent with one. The first leg is what establishes there is something for the grant to override. --- internal/sandbox/profile_test.go | 24 +++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/internal/sandbox/profile_test.go b/internal/sandbox/profile_test.go index 04a2a94f3..6ce88a218 100644 --- a/internal/sandbox/profile_test.go +++ b/internal/sandbox/profile_test.go @@ -63,17 +63,31 @@ func TestCredentialDenyReadPathsCoversGitCredentialStores(t *testing.T) { // An explicit read grant still wins, matching how every other entry behaves: a // user who deliberately scopes a workspace over one of these paths is not // overridden by the default deny. +// +// Asserted from both sides on purpose. Checking only that the path is absent +// under an allowRead is satisfied just as well by a deny rule that was never +// added, so the run without allowRead has to establish that there is something +// there for the grant to override. func TestGitCredentialDenyYieldsToExplicitAllowRead(t *testing.T) { home := t.TempDir() gitCredentials := filepath.Join(home, ".git-credentials") if err := os.WriteFile(gitCredentials, []byte("x"), 0o600); err != nil { t.Fatal(err) } - - denied := credentialDenyReadPathsForEnvironment(credentialPathEnvironment{Home: home}, []string{home}) - for _, entry := range denied { - if entry == normalizeProfilePath(gitCredentials) { - t.Fatalf("explicit allowRead %q did not re-include the credential store", home) + target := normalizeProfilePath(gitCredentials) + listed := func(entries []string) bool { + for _, entry := range entries { + if entry == target { + return true + } } + return false + } + + if !listed(credentialDenyReadPathsForEnvironment(credentialPathEnvironment{Home: home}, nil)) { + t.Fatalf("~/.git-credentials is not denied without an allowRead; nothing for the grant to override") + } + if listed(credentialDenyReadPathsForEnvironment(credentialPathEnvironment{Home: home}, []string{home})) { + t.Fatalf("explicit allowRead %q did not re-include the credential store", home) } }