Give each failure one error value (closes #113)
check / check (push) Failing after 3s

internal/cli's copies of vault.ErrSecretNotFound, ErrVaultNotFound,
ErrVersionNotFound and ErrSecretExists are removed; the commands wrap the
vault errors. Every error of secret.ReadPassphrase wraps
ErrPassphraseNotRead, so its callers no longer add those words.
ResolveGPGKeyFingerprint returns ErrGPGKeyNotFound for a key the keyring
lacks, recognised by gpg's status line. storeInKeychain returns
errNilDataBuffer. bip85's ErrPasswordTooShort and ErrEncodedTooShort go
with their unreachable checks. Tests that matched these errors' text use
errors.Is.

Model: opus-5-5
This commit is contained in:
2026-10-04 21:39:24 +00:00
parent 176095e3d1
commit a83743383e
19 changed files with 159 additions and 97 deletions
+1 -1
View File
@@ -188,7 +188,7 @@ func TestStopAtPassphrasePromptLeavesNothing(t *testing.T) {
err := tt.run(c)
require.ErrorContains(t, err, "failed to read passphrase")
require.ErrorIs(t, err, secret.ErrPassphraseNotRead)
require.Equal(t, before, snapshotStateDir(t, tt.fs))
})
}
+1 -2
View File
@@ -17,7 +17,6 @@ import (
var (
errNotAgeSecretKey = errors.New(
"does not contain a valid age secret key")
errSecretDoesNotExist = errors.New("does not exist")
)
// newCryptoCmd builds an encrypt/decrypt command with input/output flags
@@ -245,7 +244,7 @@ func (cli *Instance) Decrypt(secretName, inputFile, outputFile string) error {
}
if !exists {
return fmt.Errorf("secret '%s' %w", secretName, errSecretDoesNotExist)
return fmt.Errorf("secret '%s' %w", secretName, vault.ErrSecretNotFound)
}
// Get the age secret key from the secret
+62
View File
@@ -0,0 +1,62 @@
package cli_test
import (
"testing"
"git.eeqj.de/sneak/secret/internal/cli"
"git.eeqj.de/sneak/secret/internal/vault"
"github.com/spf13/cobra"
)
// TestMissingSecretOrVaultErrors checks that a command that finds no such
// secret or vault returns the vault package's error for it, as `secret get`
// does, and leaves the vaults unchanged. "default" is the current vault, and
// both vaults hold the secret "x".
func TestMissingSecretOrVaultErrors(t *testing.T) {
t.Parallel()
before := snapshotStateDir(t, newTwoVaultFs(t))
tests := []struct {
command string
want error
run func(c *cli.Instance) error
}{
{
"rm --force nosuch", vault.ErrSecretNotFound,
func(c *cli.Instance) error {
return c.RemoveSecret(&cobra.Command{}, "nosuch", true)
},
},
{
"version rm --force nosuch", vault.ErrSecretNotFound,
func(c *cli.Instance) error {
return c.RemoveVersion(&cobra.Command{}, "nosuch", "20260101.001", true)
},
},
{
"mv --force work:nosuch default", vault.ErrSecretNotFound,
func(c *cli.Instance) error {
return c.MoveSecret(&cobra.Command{}, "work:nosuch", "default", true)
},
},
{
"decrypt nosuch", vault.ErrSecretNotFound,
func(c *cli.Instance) error { return c.Decrypt("nosuch", "", "") },
},
{
"vault rm --force nosuch", vault.ErrVaultNotFound,
func(c *cli.Instance) error {
return c.RemoveVault(&cobra.Command{}, "nosuch", true)
},
},
}
for _, tt := range tests {
t.Run(tt.command, func(t *testing.T) {
t.Parallel()
requireRejectedAndUnchanged(t, before, tt.want, tt.run)
})
}
}
+8 -12
View File
@@ -1216,9 +1216,8 @@ func test12bMoveSecret(t *testing.T, testMnemonic string, runSecret func(...stri
// Test error cases
// Try to move non-existent secret
output, err = runSecret("move", "test/nonexistent", "test/destination")
require.Error(t, err, "move non-existent should fail")
assert.Contains(t, output, "not found", "should indicate source not found")
_, err = runSecret("move", "test/nonexistent", "test/destination")
require.ErrorIs(t, err, vault.ErrSecretNotFound, "move non-existent should fail")
// Try to move to existing destination
_, err = runSecretWithStdin("dest-value", map[string]string{
@@ -1226,9 +1225,8 @@ func test12bMoveSecret(t *testing.T, testMnemonic string, runSecret func(...stri
}, "add", "test/existing-dest")
require.NoError(t, err, "add test/existing-dest should succeed")
output, err = runSecret("move", "test/renamed", "test/existing-dest")
require.Error(t, err, "move to existing destination should fail")
assert.Contains(t, output, "already exists", "should indicate destination exists")
_, err = runSecret("move", "test/renamed", "test/existing-dest")
require.ErrorIs(t, err, vault.ErrSecretExists, "move to existing destination should fail")
// Verify the source wasn't removed since move failed
getOutput, err = runSecretWithEnv(map[string]string{
@@ -1933,12 +1931,11 @@ func test23ErrorHandling(t *testing.T, tempDir, secretPath, testMnemonic string,
// Import to non-existent vault with test passphrase
testPassphrase := "test-passphrase-123" // Define testPassphrase locally
output, err := runSecretWithEnv(map[string]string{
_, err = runSecretWithEnv(map[string]string{
secret.EnvMnemonic: testMnemonic,
secret.EnvUnlockPassphrase: testPassphrase,
}, "vault", "import", "nonexistent")
require.Error(t, err, "import to non-existent vault should fail")
assert.Contains(t, output, "does not exist", "should indicate vault doesn't exist")
require.ErrorIs(t, err, vault.ErrVaultNotFound, "import to non-existent vault should fail")
// Get specific version that doesn't exist
_, err = runSecretWithEnv(map[string]string{
@@ -1947,11 +1944,10 @@ func test23ErrorHandling(t *testing.T, tempDir, secretPath, testMnemonic string,
require.ErrorIs(t, err, vault.ErrVersionNotFound, "get non-existent version should fail")
// Promote non-existent version
output, err = runSecretWithEnv(map[string]string{
_, err = runSecretWithEnv(map[string]string{
secret.EnvMnemonic: testMnemonic,
}, "version", "promote", "database/password", "99999999.999")
require.Error(t, err, "promote non-existent version should fail")
assert.Contains(t, output, "not found", "should indicate version not found")
require.ErrorIs(t, err, vault.ErrVersionNotFound, "promote non-existent version should fail")
}
func test24EnvironmentVariables(t *testing.T, tempDir, secretPath, testMnemonic, testPassphrase string) {
+14 -9
View File
@@ -45,15 +45,6 @@ func TestRejectedMoveWithinVaultLeavesStateUnchanged(t *testing.T) {
{`mv --force work:x ""`, workX, "", true, ontoItself},
// "work" is a vault name, so the destination is work:x.
{"mv --force work:x work", workX, "work", true, ontoItself},
{
"mv work:nosuch work:y", "work:nosuch", "work:y", false,
"secret 'nosuch' not found",
},
// Only an existing vault is used.
{
"mv --force nosuch:x nosuch:y", "nosuch:x", "nosuch:y", true,
"vault 'nosuch' does not exist",
},
}
for _, tt := range tests {
@@ -70,6 +61,20 @@ func TestRejectedMoveWithinVaultLeavesStateUnchanged(t *testing.T) {
})
}
// A missing secret, and a missing vault: only an existing vault is used.
for source, want := range map[string]error{
"work:nosuch": vault.ErrSecretNotFound,
"nosuch:x": vault.ErrVaultNotFound,
} {
t.Run("mv --force "+source+" work:y", func(t *testing.T) {
t.Parallel()
requireRejectedAndUnchanged(t, before, want, func(c *cli.Instance) error {
return c.MoveSecret(&cobra.Command{}, source, "work:y", true)
})
})
}
// Each of these spells "work" a second way. The spelling is not a valid
// vault name, so the move is not taken for a move between two vaults,
// which would delete the destination, here the source.
+2 -12
View File
@@ -297,18 +297,8 @@ func TestInvalidVersionLeavesVaultsUnchanged(t *testing.T) {
for _, tt := range commands {
for _, version := range []string{"", ".", "..", "../../..", "a/b"} {
t.Run(fmt.Sprintf("%s %q", tt.command, version), func(t *testing.T) {
fs := newFsFromSnapshot(t, before)
err := tt.run(cli.NewCLIInstanceWithStateDir(fs, testStateDir), version)
require.Equal(t, before, snapshotStateDir(t, fs))
// Compared as text: `version rm` and `version promote` return
// internal/cli's own error of this text, which errors.Is does
// not match to vault.ErrVersionNotFound.
want := fmt.Errorf("version '%s' %w '%s'",
version, vault.ErrVersionNotFound, "x")
require.EqualError(t, err, want.Error())
requireRejectedAndUnchanged(t, before, vault.ErrVersionNotFound,
func(c *cli.Instance) error { return tt.run(c, version) })
})
}
}
+6 -9
View File
@@ -35,10 +35,6 @@ var (
errSecretTooLarge = errors.New("secret too large: exceeds 100MB limit")
errSecretFileTooLarge = errors.New(
"secret file too large: exceeds 100MB limit")
errSecretNotFound = errors.New("not found")
errSecretExistsNoForce = errors.New(
"already exists (use --force to overwrite)")
errVaultDoesNotExist = errors.New("does not exist")
errCrossVaultSourceUnqualified = errors.New(
"source must specify vault (e.g., vault:secret) for cross-vault move")
errMoveOntoItself = errors.New("cannot be moved onto itself")
@@ -776,7 +772,7 @@ func (cli *Instance) findSecretToRemove(
if !exists {
return secretToRemove{},
fmt.Errorf("secret '%s' %w", secretName, errSecretNotFound)
fmt.Errorf("secret '%s' %w", secretName, vault.ErrSecretNotFound)
}
// A secret without a versions directory has no versions, and can
@@ -907,7 +903,7 @@ func (cli *Instance) existingVault(name string) (*vault.Vault, error) {
}
if !slices.Contains(vaults, name) {
return nil, fmt.Errorf("vault '%s' %w", name, errVaultDoesNotExist)
return nil, fmt.Errorf("vault '%s' %w", name, vault.ErrVaultNotFound)
}
return vault.NewVault(cli.fs, cli.stateDir, name), nil
@@ -938,7 +934,7 @@ func (cli *Instance) moveSecretWithinVault(
}
if !exists {
return fmt.Errorf("secret '%s' %w", source, errSecretNotFound)
return fmt.Errorf("secret '%s' %w", source, vault.ErrSecretNotFound)
}
destEncoded := strings.ReplaceAll(dest, "/", "%")
@@ -963,7 +959,8 @@ func (cli *Instance) moveSecretWithinVault(
if exists {
if !force {
return fmt.Errorf("secret '%s' %w", dest, errSecretExistsNoForce)
return fmt.Errorf("secret '%s' %w (use --force to overwrite)",
dest, vault.ErrSecretExists)
}
err = secret.RemoveDirAtomic(cli.fs, destDir)
@@ -1028,7 +1025,7 @@ func (cli *Instance) moveSecretCrossVault(
exists, err := afero.DirExists(cli.fs, srcSecretDir)
if err != nil || !exists {
return fmt.Errorf("secret '%s' %w in vault '%s'",
srcSecretName, errSecretNotFound, srcVault.Name)
srcSecretName, vault.ErrSecretNotFound, srcVault.Name)
}
// The source is removed after the copy, so a destination that is the
+1 -1
View File
@@ -474,7 +474,7 @@ func (cli *Instance) addPassphraseUnlocker(cmd *cobra.Command) error {
// Use secure passphrase input with confirmation
passphraseBuffer, err = readSecurePassphrase("Enter passphrase for unlocker: ")
if err != nil {
return fmt.Errorf("failed to read passphrase: %w", err)
return err
}
defer passphraseBuffer.Destroy()
}
+2 -1
View File
@@ -5,6 +5,7 @@ import (
"path/filepath"
"testing"
"git.eeqj.de/sneak/secret/internal/secret"
"git.eeqj.de/sneak/secret/internal/vault"
"github.com/awnumar/memguard"
"github.com/spf13/afero"
@@ -98,7 +99,7 @@ func TestAddPGPUnlockerUnknownKey(t *testing.T) {
err := instance.addPGPUnlocker(cmd)
require.ErrorContains(t, err, "failed to resolve GPG key fingerprint")
require.ErrorIs(t, err, secret.ErrGPGKeyNotFound)
assertDirEntries(t, base,
filepath.Join(testVaultDir(listTestVaultName), listTestUnlockersDirName),
listTestUnlockerDirOne)
+3 -3
View File
@@ -250,7 +250,7 @@ func (cli *Instance) resolvePassphrase() (*memguard.LockedBuffer, func(), error)
// Use secure passphrase input with confirmation
passphraseBuffer, err := readSecurePassphrase("Enter passphrase for unlocker: ")
if err != nil {
return nil, nil, fmt.Errorf("failed to read passphrase: %w", err)
return nil, nil, err
}
return passphraseBuffer, passphraseBuffer.Destroy, nil
@@ -353,7 +353,7 @@ func (cli *Instance) vaultImportPreflight(
if !exists {
return "", "", "", fmt.Errorf("vault '%s' %w",
vaultName, errVaultDoesNotExist)
vaultName, vault.ErrVaultNotFound)
}
// Check if vault already has a public key
@@ -644,7 +644,7 @@ func (cli *Instance) findVaultToRemove(name string) (vaultToRemove, error) {
if !slices.Contains(vaults, name) {
return vaultToRemove{},
fmt.Errorf("vault '%s' %w", name, errVaultDoesNotExist)
fmt.Errorf("vault '%s' %w", name, vault.ErrVaultNotFound)
}
if len(vaults) == 1 {
+4 -5
View File
@@ -23,7 +23,6 @@ const (
// Sentinel errors for version operations
var (
errVersionNotFound = errors.New("not found for secret")
errCannotRemoveCurrentVersion = errors.New("promote another version first")
)
@@ -156,7 +155,7 @@ func (cli *Instance) ListVersions(cmd *cobra.Command, secretName string) error {
if !exists {
secret.Debug("Secret not found", "secret_name", secretName)
return fmt.Errorf("secret '%s' %w", secretName, errSecretNotFound)
return fmt.Errorf("secret '%s' %w", secretName, vault.ErrSecretNotFound)
}
// List all versions
@@ -289,7 +288,7 @@ func (cli *Instance) PromoteVersion(
if !exists {
return fmt.Errorf("version '%s' %w '%s'",
version, errVersionNotFound, secretName)
version, vault.ErrVersionNotFound, secretName)
}
// Update the current symlink using the proper function
@@ -374,7 +373,7 @@ func (cli *Instance) findVersionToRemove(
if !exists {
return versionToRemove{},
fmt.Errorf("secret '%s' %w", secretName, errSecretNotFound)
fmt.Errorf("secret '%s' %w", secretName, vault.ErrSecretNotFound)
}
// Check if version exists
@@ -386,7 +385,7 @@ func (cli *Instance) findVersionToRemove(
if !exists {
return versionToRemove{}, fmt.Errorf("version '%s' %w '%s'",
version, errVersionNotFound, secretName)
version, vault.ErrVersionNotFound, secretName)
}
// Get current version
+2 -2
View File
@@ -171,7 +171,7 @@ func TestListVersionsNonExistentSecret(t *testing.T) {
// Try to list versions of non-existent secret
err := cli.ListVersions(cmd, "nonexistent/secret")
require.ErrorIs(t, err, errSecretNotFound)
require.ErrorIs(t, err, vault.ErrSecretNotFound)
}
func TestPromoteVersionCommand(t *testing.T) {
@@ -265,7 +265,7 @@ func TestPromoteNonExistentVersion(t *testing.T) {
// Try to promote non-existent version
err = cli.PromoteVersion(cmd, "test/secret", "20991231.999")
require.ErrorIs(t, err, errVersionNotFound)
require.ErrorIs(t, err, vault.ErrVersionNotFound)
}
func TestGetSecretWithVersion(t *testing.T) {
+6 -5
View File
@@ -166,19 +166,20 @@ func DecryptWithPassphrase(
// ReadPassphrase reads a passphrase securely from the terminal without echoing
// This version is for unlocking and doesn't require confirmation
// Returns a LockedBuffer containing the passphrase for secure memory handling
// Returns a LockedBuffer containing the passphrase for secure memory handling.
// Every error it returns wraps ErrPassphraseNotRead.
func ReadPassphrase(prompt string) (*memguard.LockedBuffer, error) {
// Check if stdin is a terminal
if !term.IsTerminal(syscall.Stdin) {
// Not a terminal - never read passphrases from piped input
// for security reasons
return nil, errStdinNotTerminal
return nil, fmt.Errorf("%w: %w", ErrPassphraseNotRead, errStdinNotTerminal)
}
// stdin is a terminal, check if stderr is also a terminal for
// interactive prompting
if !term.IsTerminal(syscall.Stderr) {
return nil, errStderrNotTerminal
return nil, fmt.Errorf("%w: %w", ErrPassphraseNotRead, errStderrNotTerminal)
}
// Both stdin and stderr are terminals - use secure password reading
@@ -186,14 +187,14 @@ func ReadPassphrase(prompt string) (*memguard.LockedBuffer, error) {
passphrase, err := term.ReadPassword(syscall.Stdin)
if err != nil {
return nil, fmt.Errorf("failed to read passphrase: %w", err)
return nil, fmt.Errorf("%w: %w", ErrPassphraseNotRead, err)
}
// Print newline to stderr since ReadPassword doesn't echo
fmt.Fprintln(os.Stderr)
if len(passphrase) == 0 {
return nil, errEmptyPassphrase
return nil, fmt.Errorf("%w: %w", ErrPassphraseNotRead, errEmptyPassphrase)
}
// Create a secure buffer and copy the passphrase
+1 -1
View File
@@ -15,7 +15,7 @@ import (
// storeInKeychain stores data in the macOS keychain using keybase/go-keychain
func storeInKeychain(itemName string, data *memguard.LockedBuffer) error {
if data == nil {
return fmt.Errorf("data buffer is nil")
return errNilDataBuffer
}
if err := validateKeychainItemName(itemName); err != nil {
return fmt.Errorf("invalid keychain item name: %w", err)
+1 -2
View File
@@ -130,8 +130,7 @@ func TestKeychainNilData(t *testing.T) {
// Test storing nil data
err := storeInKeychain("test-item", nil)
assert.Error(t, err, "Expected error when storing nil data")
assert.Contains(t, err.Error(), "data buffer is nil")
require.ErrorIs(t, err, errNilDataBuffer)
}
func TestKeychainLargeData(t *testing.T) {
+4 -4
View File
@@ -11,9 +11,9 @@ import (
"github.com/spf13/afero"
)
// ErrPassphraseNotRead is wrapped in the error of a passphrase unlocker
// that could not read its passphrase from the terminal, for example because
// there is none. The unlocker itself was not tried.
// ErrPassphraseNotRead is wrapped in every error of ReadPassphrase: there
// is no terminal to read the passphrase from, reading it failed, or it was
// empty. A passphrase unlocker that fails with it was not tried.
var ErrPassphraseNotRead = errors.New("failed to read passphrase")
// PassphraseUnlocker represents a passphrase-protected unlocker
@@ -155,7 +155,7 @@ func (p *PassphraseUnlocker) getPassphrase() (*memguard.LockedBuffer, error) {
if err != nil {
Debug("Failed to read passphrase", "error", err, "unlocker_id", p.GetID())
return nil, fmt.Errorf("%w: %w", ErrPassphraseNotRead, err)
return nil, err
}
return secureBuffer, nil
+16 -2
View File
@@ -18,6 +18,10 @@ import (
"github.com/spf13/afero"
)
// gpgNoPublicKeyStatus is the status line gpg writes when it has no key for
// the ID it was asked to list: 9 is gpg's error code for "No public key".
const gpgNoPublicKeyStatus = "[GNUPG:] ERROR keylist.getkey 9\n"
var (
errGPGKeyIDEmpty = errors.New("GPG key ID cannot be empty")
errInvalidGPGKeyID = errors.New("invalid GPG key ID format")
@@ -25,6 +29,10 @@ var (
errNilDataBuffer = errors.New("data buffer is nil")
)
// ErrGPGKeyNotFound is returned by ResolveGPGKeyFingerprint for a key ID
// that matches no key in the GPG keyring.
var ErrGPGKeyNotFound = errors.New("GPG key not found")
// Variables to allow overriding in tests
var (
// GPGEncryptFunc is the function used for GPG encryption
@@ -367,14 +375,20 @@ func ResolveGPGKeyFingerprint(keyID string) (string, error) {
return "", fmt.Errorf("invalid GPG key ID: %w", err)
}
// Use GPG to get the full fingerprint for the key
// Use GPG to get the full fingerprint for the key. --status-fd 1 adds
// gpg's status lines to the output.
cmd := exec.CommandContext( //nolint:gosec // G204: keyID validated above
context.Background(),
"gpg", "--list-keys", "--with-colons", "--fingerprint", keyID,
"gpg", "--status-fd", "1",
"--list-keys", "--with-colons", "--fingerprint", keyID,
)
output, err := cmd.Output()
if err != nil {
if strings.Contains(string(output), gpgNoPublicKeyStatus) {
return "", fmt.Errorf("%w: %s", ErrGPGKeyNotFound, keyID)
}
return "", fmt.Errorf("failed to resolve GPG key fingerprint: %w", err)
}