From 276fea01214cba6c4e527b7ce5db5a0fb4568b10 Mon Sep 17 00:00:00 2001 From: Ruslan Shaydullin Date: Tue, 7 Jul 2026 10:33:30 +0500 Subject: [PATCH] fix(provenance): treat only not-exist as keyring absence in defaultKeyring MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A stat error other than 'not exist' (e.g. a permission problem) meant the file may well be present, but defaultKeyring() skipped past it: an unreadable pubring.gpg silently lost precedence to pubring.kbx, and the surfaced error could point at the wrong file. Treat only fs.ErrNotExist as absence — for any other stat error return that path unchanged so the real error surfaces when the keyring is opened. Suggested by the Copilot review on #32281. Signed-off-by: Ruslan Shaydullin --- pkg/cmd/dependency_build.go | 16 ++++++++++------ pkg/cmd/dependency_build_test.go | 29 +++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 6 deletions(-) diff --git a/pkg/cmd/dependency_build.go b/pkg/cmd/dependency_build.go index ba3cefcdd..3ccda1b9f 100644 --- a/pkg/cmd/dependency_build.go +++ b/pkg/cmd/dependency_build.go @@ -19,6 +19,7 @@ import ( "errors" "fmt" "io" + "io/fs" "os" "path/filepath" @@ -93,21 +94,24 @@ func newDependencyBuildCmd(out io.Writer) *cobra.Command { // defaultKeyring returns the expanded path to the default keyring. // -// The legacy pubring.gpg file is preferred when present. If it is absent and -// the keybox file maintained by modern GnuPG (2.1+) exists, that is used -// instead. When neither exists, the legacy path is returned so that error -// messages keep pointing at the traditional default. +// The legacy pubring.gpg file is preferred and treated as absent only when +// stat fails with "not exist" — any other stat error (e.g. permissions) +// keeps the legacy path so the real error surfaces when the file is opened. +// If the legacy file is absent, the keybox file maintained by modern GnuPG +// (2.1+) takes over under the same rule. When neither exists, the legacy +// path is returned so that error messages keep pointing at the traditional +// default. func defaultKeyring() string { gnupgHome := filepath.Join(homedir.HomeDir(), ".gnupg") if v, ok := os.LookupEnv("GNUPGHOME"); ok { gnupgHome = v } legacy := filepath.Join(gnupgHome, "pubring.gpg") - if _, err := os.Stat(legacy); err == nil { + if _, err := os.Stat(legacy); !errors.Is(err, fs.ErrNotExist) { return legacy } keybox := filepath.Join(gnupgHome, "pubring.kbx") - if _, err := os.Stat(keybox); err == nil { + if _, err := os.Stat(keybox); !errors.Is(err, fs.ErrNotExist) { return keybox } return legacy diff --git a/pkg/cmd/dependency_build_test.go b/pkg/cmd/dependency_build_test.go index d53232421..bb90d4473 100644 --- a/pkg/cmd/dependency_build_test.go +++ b/pkg/cmd/dependency_build_test.go @@ -19,6 +19,7 @@ import ( "fmt" "os" "path/filepath" + "runtime" "strings" "testing" @@ -194,6 +195,34 @@ func TestDefaultKeyring(t *testing.T) { }) } + t.Run("stat error other than not-exist keeps the legacy path", func(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("directory permissions are not enforced on Windows") + } + if os.Geteuid() == 0 { + t.Skip("root bypasses directory permissions") + } + + parent := t.TempDir() + dir := filepath.Join(parent, ".gnupg") + if err := os.MkdirAll(dir, 0o700); err != nil { + t.Fatal(err) + } + touch(t, filepath.Join(dir, "pubring.kbx")) + t.Setenv("GNUPGHOME", dir) + + // Make the directory unsearchable so stat on both keyrings fails + // with a permission error rather than "not exist". + if err := os.Chmod(dir, 0o000); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { os.Chmod(dir, 0o700) }) + + if got, want := defaultKeyring(), filepath.Join(dir, "pubring.gpg"); got != want { + t.Errorf("expected %q, got %q", want, got) + } + }) + t.Run("no GNUPGHOME falls back to the home directory", func(t *testing.T) { home := t.TempDir() t.Setenv("HOME", home)