Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 28 additions & 10 deletions common/pkg/secrets/passdriver/passdriver.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,9 +102,12 @@ func (d *Driver) List() (secrets []string, err error) {
return nil, fmt.Errorf("failed to read secret directory: %w", err)
}
for _, f := range files {
fileName := f.Name()
withoutSuffix := fileName[:len(fileName)-len(".gpg")]
secrets = append(secrets, withoutSuffix)
if f.IsDir() {
continue
}
if name, ok := strings.CutSuffix(f.Name(), ".gpg"); ok {
secrets = append(secrets, name)
}
}
sort.Strings(secrets)
return secrets, nil
Expand All @@ -117,8 +120,16 @@ func (d *Driver) Lookup(id string) ([]byte, error) {
if err != nil {
return nil, err
}
// Check file existence before invoking GPG. GPG exits non-zero both when
// the file is missing and when decryption fails (e.g. expired agent cache)
if err := fileutils.Exists(key); err != nil {
if errors.Is(err, os.ErrNotExist) {
return nil, fmt.Errorf("%s: %w", id, define.ErrNoSuchSecret)
}
return nil, fmt.Errorf("accessing secret %s: %w", id, err)
}
if err := d.gpg(context.TODO(), nil, out, "--decrypt", key); err != nil {
return nil, fmt.Errorf("%s: %w", id, define.ErrNoSuchSecret)
return nil, fmt.Errorf("decrypting secret %s: %w", id, err)
}
if out.Len() == 0 {
return nil, fmt.Errorf("%s: %w", id, define.ErrNoSuchSecret)
Expand All @@ -128,14 +139,14 @@ func (d *Driver) Lookup(id string) ([]byte, error) {

// Store saves the bytes associated with an ID. An error is returned if the ID already exists.
func (d *Driver) Store(id string, data []byte) error {
if _, err := d.Lookup(id); err == nil {
return fmt.Errorf("%s: %w", id, define.ErrSecretIDExists)
}
in := bytes.NewReader(data)
key, err := d.getPath(id)
if err != nil {
return err
}
if err := fileutils.Exists(key); err == nil {
return fmt.Errorf("%s: %w", id, define.ErrSecretIDExists)
}
in := bytes.NewReader(data)
return d.gpg(context.TODO(), in, nil, "--encrypt", "-r", d.KeyID, "-o", key)
}

Expand All @@ -162,8 +173,15 @@ func (d *Driver) gpg(ctx context.Context, in io.Reader, out io.Writer, args ...s
cmd.Env = os.Environ()
cmd.Stdin = in
cmd.Stdout = out
cmd.Stderr = io.Discard
return cmd.Run()
var errBuf bytes.Buffer
cmd.Stderr = &errBuf
if err := cmd.Run(); err != nil {
if stderr := strings.TrimSpace(errBuf.String()); stderr != "" {
return fmt.Errorf("%w: %s", err, stderr)
}
return err
}
return nil
}

func (d *Driver) getPath(id string) (string, error) {
Expand Down
55 changes: 55 additions & 0 deletions common/pkg/secrets/passdriver/passdriver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ package passdriver
import (
"context"
"fmt"
"os"
"path/filepath"
"testing"

"github.com/stretchr/testify/require"
Expand Down Expand Up @@ -170,3 +172,56 @@ func TestDelete(t *testing.T) {
})
}
}

// TestLookupDecryptFailureNotMasked reproduces the core bug from issue 28938:
// a GPG decrypt failure must not be reported as ErrNoSuchSecret.
func TestLookupDecryptFailureNotMasked(t *testing.T) {
driver := setupDriver(t)

err := driver.Store("corrupted", []byte("secret-data"))
require.NoError(t, err)

// Corrupt the .gpg file so GPG cannot decrypt it.
key, err := driver.getPath("corrupted")
require.NoError(t, err)
err = os.WriteFile(key, []byte("not-valid-gpg-data"), 0o644)
require.NoError(t, err)

// Lookup must fail with a decrypt error, not ErrNoSuchSecret.
_, err = driver.Lookup("corrupted")
require.Error(t, err)
require.NotErrorIs(t, err, define.ErrNoSuchSecret,
"Lookup must not report 'no such secret' when the .gpg file exists but decryption fails")
}

// TestListFiltersNonGpgEntries verifies that List skips directories and
// non-.gpg files instead of panicking or returning garbage IDs.
func TestListFiltersNonGpgEntries(t *testing.T) {
driver := setupDriver(t)
require.NoError(t, driver.Store("valid", []byte("data")))

// Add a non-.gpg file and a subdirectory to the store root.
err := os.WriteFile(filepath.Join(driver.Root, "readme.txt"), []byte("hi"), 0o644)
require.NoError(t, err)
err = os.Mkdir(filepath.Join(driver.Root, "subdir"), 0o755)
require.NoError(t, err)

list, err := driver.List()
require.NoError(t, err)
require.Equal(t, []string{"valid"}, list,
"List should only return IDs from .gpg files, skipping directories and other files")
}

// TestStoreExistenceCheckWithoutGpg verifies that Store detects duplicate IDs
// via os.Stat rather than a GPG decrypt, and returns ErrSecretIDExists.
func TestStoreExistenceCheckWithoutGpg(t *testing.T) {
driver := setupDriver(t)

err := driver.Store("exists", []byte("data"))
require.NoError(t, err)

// Storing the same ID again must fail with ErrSecretIDExists.
err = driver.Store("exists", []byte("other"))
require.Error(t, err)
require.ErrorIs(t, err, define.ErrSecretIDExists)
}
Loading