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. errUnsupportedUnlockerType is removed for
errInvalidUnlockerType, which names the same failure. 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 22:09:27 +00:00
parent 176095e3d1
commit c0b02b3dcb
19 changed files with 175 additions and 99 deletions
+23
View File
@@ -18,6 +18,29 @@ https://git.eeqj.de/sneak/secret/milestone/12
# Completed Steps # Completed Steps
- 2026-10-04: A failure returns the same error value whichever command hits
it (https://git.eeqj.de/sneak/secret/issues/113). `internal/cli` no longer
keeps its own copies of `vault.ErrSecretNotFound`, `ErrVaultNotFound`,
`ErrVersionNotFound` and `ErrSecretExists`: `secret mv`, `rm`, `decrypt`,
`vault import`, `vault remove` and `version list`, `promote` and `rm` wrap
the `vault` errors. `errUnsupportedUnlockerType` is removed: `secret
unlocker add` gives `errInvalidUnlockerType` for an unknown type, whichever
check rejects it. Messages are unchanged, except that `secret decrypt`
of a missing secret says "not found", as `secret get` does, not "does not
exist". Every error of `secret.ReadPassphrase` wraps
`secret.ErrPassphraseNotRead`, which supplies the words "failed to read
passphrase" that its callers used to add themselves; so two passphrases
that differ now give only "passphrases do not match", the words now follow
"failed to read mnemonic:" and "failed to read passphrase confirmation:",
and a terminal read error no longer repeats them. A GPG key the keyring
does not hold gives `secret.ErrGPGKeyNotFound`, found by gpg's status line
for "No public key"; before, the message repeated "failed to resolve GPG
key fingerprint" and ended in gpg's exit status. The keychain unlocker
returns `errNilDataBuffer` for nil data; this and its test build only on
macOS with cgo and were only read. `bip85.ErrPasswordTooShort` and
`ErrEncodedTooShort` are removed with their checks: 64 bytes of entropy
always give 86 Base64 or 80 Base85 characters, the most a password length
may ask for. Tests that matched these errors' text use `errors.Is`.
- 2026-10-04: Tests check which error a failure returns with `errors.Is`, - 2026-10-04: Tests check which error a failure returns with `errors.Is`,
not by matching words of its message not by matching words of its message
(https://git.eeqj.de/sneak/secret/issues/49). Every exported error that (https://git.eeqj.de/sneak/secret/issues/49). Every exported error that
+1 -1
View File
@@ -188,7 +188,7 @@ func TestStopAtPassphrasePromptLeavesNothing(t *testing.T) {
err := tt.run(c) 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)) require.Equal(t, before, snapshotStateDir(t, tt.fs))
}) })
} }
+1 -2
View File
@@ -17,7 +17,6 @@ import (
var ( var (
errNotAgeSecretKey = errors.New( errNotAgeSecretKey = errors.New(
"does not contain a valid age secret key") "does not contain a valid age secret key")
errSecretDoesNotExist = errors.New("does not exist")
) )
// newCryptoCmd builds an encrypt/decrypt command with input/output flags // 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 { 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 // 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 // Test error cases
// Try to move non-existent secret // Try to move non-existent secret
output, err = runSecret("move", "test/nonexistent", "test/destination") _, err = runSecret("move", "test/nonexistent", "test/destination")
require.Error(t, err, "move non-existent should fail") require.ErrorIs(t, err, vault.ErrSecretNotFound, "move non-existent should fail")
assert.Contains(t, output, "not found", "should indicate source not found")
// Try to move to existing destination // Try to move to existing destination
_, err = runSecretWithStdin("dest-value", map[string]string{ _, 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") }, "add", "test/existing-dest")
require.NoError(t, err, "add test/existing-dest should succeed") require.NoError(t, err, "add test/existing-dest should succeed")
output, err = runSecret("move", "test/renamed", "test/existing-dest") _, err = runSecret("move", "test/renamed", "test/existing-dest")
require.Error(t, err, "move to existing destination should fail") require.ErrorIs(t, err, vault.ErrSecretExists, "move to existing destination should fail")
assert.Contains(t, output, "already exists", "should indicate destination exists")
// Verify the source wasn't removed since move failed // Verify the source wasn't removed since move failed
getOutput, err = runSecretWithEnv(map[string]string{ 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 // Import to non-existent vault with test passphrase
testPassphrase := "test-passphrase-123" // Define testPassphrase locally testPassphrase := "test-passphrase-123" // Define testPassphrase locally
output, err := runSecretWithEnv(map[string]string{ _, err = runSecretWithEnv(map[string]string{
secret.EnvMnemonic: testMnemonic, secret.EnvMnemonic: testMnemonic,
secret.EnvUnlockPassphrase: testPassphrase, secret.EnvUnlockPassphrase: testPassphrase,
}, "vault", "import", "nonexistent") }, "vault", "import", "nonexistent")
require.Error(t, err, "import to non-existent vault should fail") require.ErrorIs(t, err, vault.ErrVaultNotFound, "import to non-existent vault should fail")
assert.Contains(t, output, "does not exist", "should indicate vault doesn't exist")
// Get specific version that doesn't exist // Get specific version that doesn't exist
_, err = runSecretWithEnv(map[string]string{ _, 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") require.ErrorIs(t, err, vault.ErrVersionNotFound, "get non-existent version should fail")
// Promote non-existent version // Promote non-existent version
output, err = runSecretWithEnv(map[string]string{ _, err = runSecretWithEnv(map[string]string{
secret.EnvMnemonic: testMnemonic, secret.EnvMnemonic: testMnemonic,
}, "version", "promote", "database/password", "99999999.999") }, "version", "promote", "database/password", "99999999.999")
require.Error(t, err, "promote non-existent version should fail") require.ErrorIs(t, err, vault.ErrVersionNotFound, "promote non-existent version should fail")
assert.Contains(t, output, "not found", "should indicate version not found")
} }
func test24EnvironmentVariables(t *testing.T, tempDir, secretPath, testMnemonic, testPassphrase string) { func test24EnvironmentVariables(t *testing.T, tempDir, secretPath, testMnemonic, testPassphrase string) {
+27 -9
View File
@@ -45,15 +45,6 @@ func TestRejectedMoveWithinVaultLeavesStateUnchanged(t *testing.T) {
{`mv --force work:x ""`, workX, "", true, ontoItself}, {`mv --force work:x ""`, workX, "", true, ontoItself},
// "work" is a vault name, so the destination is work:x. // "work" is a vault name, so the destination is work:x.
{"mv --force work:x work", workX, "work", true, ontoItself}, {"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 { for _, tt := range tests {
@@ -70,6 +61,33 @@ func TestRejectedMoveWithinVaultLeavesStateUnchanged(t *testing.T) {
}) })
} }
missing := []struct {
command string
source, dest string
force bool
want error
}{
{
"mv work:nosuch work:y", "work:nosuch", "work:y", false,
vault.ErrSecretNotFound,
},
// Only an existing vault is used.
{
"mv --force nosuch:x nosuch:y", "nosuch:x", "nosuch:y", true,
vault.ErrVaultNotFound,
},
}
for _, tt := range missing {
t.Run(tt.command, func(t *testing.T) {
t.Parallel()
requireRejectedAndUnchanged(t, before, tt.want, func(c *cli.Instance) error {
return c.MoveSecret(&cobra.Command{}, tt.source, tt.dest, tt.force)
})
})
}
// Each of these spells "work" a second way. The spelling is not a valid // 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, // vault name, so the move is not taken for a move between two vaults,
// which would delete the destination, here the source. // 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 _, tt := range commands {
for _, version := range []string{"", ".", "..", "../../..", "a/b"} { for _, version := range []string{"", ".", "..", "../../..", "a/b"} {
t.Run(fmt.Sprintf("%s %q", tt.command, version), func(t *testing.T) { t.Run(fmt.Sprintf("%s %q", tt.command, version), func(t *testing.T) {
fs := newFsFromSnapshot(t, before) requireRejectedAndUnchanged(t, before, vault.ErrVersionNotFound,
func(c *cli.Instance) error { return tt.run(c, version) })
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())
}) })
} }
} }
+6 -9
View File
@@ -35,10 +35,6 @@ var (
errSecretTooLarge = errors.New("secret too large: exceeds 100MB limit") errSecretTooLarge = errors.New("secret too large: exceeds 100MB limit")
errSecretFileTooLarge = errors.New( errSecretFileTooLarge = errors.New(
"secret file too large: exceeds 100MB limit") "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( errCrossVaultSourceUnqualified = errors.New(
"source must specify vault (e.g., vault:secret) for cross-vault move") "source must specify vault (e.g., vault:secret) for cross-vault move")
errMoveOntoItself = errors.New("cannot be moved onto itself") errMoveOntoItself = errors.New("cannot be moved onto itself")
@@ -776,7 +772,7 @@ func (cli *Instance) findSecretToRemove(
if !exists { if !exists {
return secretToRemove{}, 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 // 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) { 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 return vault.NewVault(cli.fs, cli.stateDir, name), nil
@@ -938,7 +934,7 @@ func (cli *Instance) moveSecretWithinVault(
} }
if !exists { if !exists {
return fmt.Errorf("secret '%s' %w", source, errSecretNotFound) return fmt.Errorf("secret '%s' %w", source, vault.ErrSecretNotFound)
} }
destEncoded := strings.ReplaceAll(dest, "/", "%") destEncoded := strings.ReplaceAll(dest, "/", "%")
@@ -963,7 +959,8 @@ func (cli *Instance) moveSecretWithinVault(
if exists { if exists {
if !force { 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) err = secret.RemoveDirAtomic(cli.fs, destDir)
@@ -1028,7 +1025,7 @@ func (cli *Instance) moveSecretCrossVault(
exists, err := afero.DirExists(cli.fs, srcSecretDir) exists, err := afero.DirExists(cli.fs, srcSecretDir)
if err != nil || !exists { if err != nil || !exists {
return fmt.Errorf("secret '%s' %w in vault '%s'", 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 // The source is removed after the copy, so a destination that is the
+2 -3
View File
@@ -47,7 +47,6 @@ var (
// composes "GPG key <id> is already added as an unlocker". // composes "GPG key <id> is already added as an unlocker".
errGPGKeyAlreadyUnlocker = errors.New( errGPGKeyAlreadyUnlocker = errors.New(
"is already added as an unlocker") "is already added as an unlocker")
errUnsupportedUnlockerType = errors.New("unsupported unlocker type")
) )
// UnlockerInfo represents unlocker information for display // UnlockerInfo represents unlocker information for display
@@ -439,7 +438,7 @@ func (cli *Instance) UnlockersAdd(unlockerType string, cmd *cobra.Command) error
} }
return fmt.Errorf("%w: %s (supported: %s)", return fmt.Errorf("%w: %s (supported: %s)",
errUnsupportedUnlockerType, unlockerType, supportedTypes) errInvalidUnlockerType, unlockerType, supportedTypes)
} }
} }
@@ -474,7 +473,7 @@ func (cli *Instance) addPassphraseUnlocker(cmd *cobra.Command) error {
// Use secure passphrase input with confirmation // Use secure passphrase input with confirmation
passphraseBuffer, err = readSecurePassphrase("Enter passphrase for unlocker: ") passphraseBuffer, err = readSecurePassphrase("Enter passphrase for unlocker: ")
if err != nil { if err != nil {
return fmt.Errorf("failed to read passphrase: %w", err) return err
} }
defer passphraseBuffer.Destroy() defer passphraseBuffer.Destroy()
} }
+2 -1
View File
@@ -5,6 +5,7 @@ import (
"path/filepath" "path/filepath"
"testing" "testing"
"git.eeqj.de/sneak/secret/internal/secret"
"git.eeqj.de/sneak/secret/internal/vault" "git.eeqj.de/sneak/secret/internal/vault"
"github.com/awnumar/memguard" "github.com/awnumar/memguard"
"github.com/spf13/afero" "github.com/spf13/afero"
@@ -98,7 +99,7 @@ func TestAddPGPUnlockerUnknownKey(t *testing.T) {
err := instance.addPGPUnlocker(cmd) err := instance.addPGPUnlocker(cmd)
require.ErrorContains(t, err, "failed to resolve GPG key fingerprint") require.ErrorIs(t, err, secret.ErrGPGKeyNotFound)
assertDirEntries(t, base, assertDirEntries(t, base,
filepath.Join(testVaultDir(listTestVaultName), listTestUnlockersDirName), filepath.Join(testVaultDir(listTestVaultName), listTestUnlockersDirName),
listTestUnlockerDirOne) listTestUnlockerDirOne)
+3 -3
View File
@@ -250,7 +250,7 @@ func (cli *Instance) resolvePassphrase() (*memguard.LockedBuffer, func(), error)
// Use secure passphrase input with confirmation // Use secure passphrase input with confirmation
passphraseBuffer, err := readSecurePassphrase("Enter passphrase for unlocker: ") passphraseBuffer, err := readSecurePassphrase("Enter passphrase for unlocker: ")
if err != nil { if err != nil {
return nil, nil, fmt.Errorf("failed to read passphrase: %w", err) return nil, nil, err
} }
return passphraseBuffer, passphraseBuffer.Destroy, nil return passphraseBuffer, passphraseBuffer.Destroy, nil
@@ -353,7 +353,7 @@ func (cli *Instance) vaultImportPreflight(
if !exists { if !exists {
return "", "", "", fmt.Errorf("vault '%s' %w", return "", "", "", fmt.Errorf("vault '%s' %w",
vaultName, errVaultDoesNotExist) vaultName, vault.ErrVaultNotFound)
} }
// Check if vault already has a public key // 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) { if !slices.Contains(vaults, name) {
return vaultToRemove{}, return vaultToRemove{},
fmt.Errorf("vault '%s' %w", name, errVaultDoesNotExist) fmt.Errorf("vault '%s' %w", name, vault.ErrVaultNotFound)
} }
if len(vaults) == 1 { if len(vaults) == 1 {
+4 -5
View File
@@ -23,7 +23,6 @@ const (
// Sentinel errors for version operations // Sentinel errors for version operations
var ( var (
errVersionNotFound = errors.New("not found for secret")
errCannotRemoveCurrentVersion = errors.New("promote another version first") errCannotRemoveCurrentVersion = errors.New("promote another version first")
) )
@@ -156,7 +155,7 @@ func (cli *Instance) ListVersions(cmd *cobra.Command, secretName string) error {
if !exists { if !exists {
secret.Debug("Secret not found", "secret_name", secretName) 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 // List all versions
@@ -289,7 +288,7 @@ func (cli *Instance) PromoteVersion(
if !exists { if !exists {
return fmt.Errorf("version '%s' %w '%s'", return fmt.Errorf("version '%s' %w '%s'",
version, errVersionNotFound, secretName) version, vault.ErrVersionNotFound, secretName)
} }
// Update the current symlink using the proper function // Update the current symlink using the proper function
@@ -374,7 +373,7 @@ func (cli *Instance) findVersionToRemove(
if !exists { if !exists {
return versionToRemove{}, return versionToRemove{},
fmt.Errorf("secret '%s' %w", secretName, errSecretNotFound) fmt.Errorf("secret '%s' %w", secretName, vault.ErrSecretNotFound)
} }
// Check if version exists // Check if version exists
@@ -386,7 +385,7 @@ func (cli *Instance) findVersionToRemove(
if !exists { if !exists {
return versionToRemove{}, fmt.Errorf("version '%s' %w '%s'", return versionToRemove{}, fmt.Errorf("version '%s' %w '%s'",
version, errVersionNotFound, secretName) version, vault.ErrVersionNotFound, secretName)
} }
// Get current version // Get current version
+2 -2
View File
@@ -171,7 +171,7 @@ func TestListVersionsNonExistentSecret(t *testing.T) {
// Try to list versions of non-existent secret // Try to list versions of non-existent secret
err := cli.ListVersions(cmd, "nonexistent/secret") err := cli.ListVersions(cmd, "nonexistent/secret")
require.ErrorIs(t, err, errSecretNotFound) require.ErrorIs(t, err, vault.ErrSecretNotFound)
} }
func TestPromoteVersionCommand(t *testing.T) { func TestPromoteVersionCommand(t *testing.T) {
@@ -265,7 +265,7 @@ func TestPromoteNonExistentVersion(t *testing.T) {
// Try to promote non-existent version // Try to promote non-existent version
err = cli.PromoteVersion(cmd, "test/secret", "20991231.999") err = cli.PromoteVersion(cmd, "test/secret", "20991231.999")
require.ErrorIs(t, err, errVersionNotFound) require.ErrorIs(t, err, vault.ErrVersionNotFound)
} }
func TestGetSecretWithVersion(t *testing.T) { 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 // ReadPassphrase reads a passphrase securely from the terminal without echoing
// This version is for unlocking and doesn't require confirmation // 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) { func ReadPassphrase(prompt string) (*memguard.LockedBuffer, error) {
// Check if stdin is a terminal // Check if stdin is a terminal
if !term.IsTerminal(syscall.Stdin) { if !term.IsTerminal(syscall.Stdin) {
// Not a terminal - never read passphrases from piped input // Not a terminal - never read passphrases from piped input
// for security reasons // 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 // stdin is a terminal, check if stderr is also a terminal for
// interactive prompting // interactive prompting
if !term.IsTerminal(syscall.Stderr) { 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 // 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) passphrase, err := term.ReadPassword(syscall.Stdin)
if err != nil { 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 // Print newline to stderr since ReadPassword doesn't echo
fmt.Fprintln(os.Stderr) fmt.Fprintln(os.Stderr)
if len(passphrase) == 0 { if len(passphrase) == 0 {
return nil, errEmptyPassphrase return nil, fmt.Errorf("%w: %w", ErrPassphraseNotRead, errEmptyPassphrase)
} }
// Create a secure buffer and copy the passphrase // 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 // storeInKeychain stores data in the macOS keychain using keybase/go-keychain
func storeInKeychain(itemName string, data *memguard.LockedBuffer) error { func storeInKeychain(itemName string, data *memguard.LockedBuffer) error {
if data == nil { if data == nil {
return fmt.Errorf("data buffer is nil") return errNilDataBuffer
} }
if err := validateKeychainItemName(itemName); err != nil { if err := validateKeychainItemName(itemName); err != nil {
return fmt.Errorf("invalid keychain item name: %w", err) 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 // Test storing nil data
err := storeInKeychain("test-item", nil) err := storeInKeychain("test-item", nil)
assert.Error(t, err, "Expected error when storing nil data") require.ErrorIs(t, err, errNilDataBuffer)
assert.Contains(t, err.Error(), "data buffer is nil")
} }
func TestKeychainLargeData(t *testing.T) { func TestKeychainLargeData(t *testing.T) {
+4 -4
View File
@@ -11,9 +11,9 @@ import (
"github.com/spf13/afero" "github.com/spf13/afero"
) )
// ErrPassphraseNotRead is wrapped in the error of a passphrase unlocker // ErrPassphraseNotRead is wrapped in every error of ReadPassphrase: there
// that could not read its passphrase from the terminal, for example because // is no terminal to read the passphrase from, reading it failed, or it was
// there is none. The unlocker itself was not tried. // empty. A passphrase unlocker that fails with it was not tried.
var ErrPassphraseNotRead = errors.New("failed to read passphrase") var ErrPassphraseNotRead = errors.New("failed to read passphrase")
// PassphraseUnlocker represents a passphrase-protected unlocker // PassphraseUnlocker represents a passphrase-protected unlocker
@@ -155,7 +155,7 @@ func (p *PassphraseUnlocker) getPassphrase() (*memguard.LockedBuffer, error) {
if err != nil { if err != nil {
Debug("Failed to read passphrase", "error", err, "unlocker_id", p.GetID()) 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 return secureBuffer, nil
+16 -2
View File
@@ -18,6 +18,10 @@ import (
"github.com/spf13/afero" "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 ( var (
errGPGKeyIDEmpty = errors.New("GPG key ID cannot be empty") errGPGKeyIDEmpty = errors.New("GPG key ID cannot be empty")
errInvalidGPGKeyID = errors.New("invalid GPG key ID format") errInvalidGPGKeyID = errors.New("invalid GPG key ID format")
@@ -25,6 +29,10 @@ var (
errNilDataBuffer = errors.New("data buffer is nil") 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 // Variables to allow overriding in tests
var ( var (
// GPGEncryptFunc is the function used for GPG encryption // 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) 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 cmd := exec.CommandContext( //nolint:gosec // G204: keyID validated above
context.Background(), context.Background(),
"gpg", "--list-keys", "--with-colons", "--fingerprint", keyID, "gpg", "--status-fd", "1",
"--list-keys", "--with-colons", "--fingerprint", keyID,
) )
output, err := cmd.Output() output, err := cmd.Output()
if err != nil { 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) return "", fmt.Errorf("failed to resolve GPG key fingerprint: %w", err)
} }
+4 -26
View File
@@ -59,16 +59,6 @@ var (
// ErrInvalidBase85PwdLen is returned when the Base85 password length // ErrInvalidBase85PwdLen is returned when the Base85 password length
// is out of range. // is out of range.
ErrInvalidBase85PwdLen = errors.New("pwdLen must be between 10 and 80") ErrInvalidBase85PwdLen = errors.New("pwdLen must be between 10 and 80")
// ErrPasswordTooShort is returned when the derived material is
// shorter than the requested password length. It carries only the
// middle of the message, which the caller composes as
// "derived password length <n> is shorter than requested length <m>",
// so the emitted text is unchanged.
ErrPasswordTooShort = errors.New("is shorter than requested length")
// ErrEncodedTooShort is returned when the encoded material is shorter
// than the requested password length. Composed as
// "encoded length <n> is less than requested length <m>".
ErrEncodedTooShort = errors.New("is less than requested length")
) )
// Version bytes for extended keys // Version bytes for extended keys
@@ -381,14 +371,8 @@ func DeriveBase64Password(
// Remove any padding // Remove any padding
encodedStr = strings.TrimRight(encodedStr, "=") encodedStr = strings.TrimRight(encodedStr, "=")
// Slice to the desired password length // Slice to the desired password length: 64 bytes of entropy leave 86
if len(encodedStr) < int(pwdLen) { // characters, the most pwdLen allows
return "", fmt.Errorf(
"derived password length %d %w %d",
len(encodedStr), ErrPasswordTooShort, pwdLen,
)
}
return encodedStr[:pwdLen], nil return encodedStr[:pwdLen], nil
} }
@@ -411,14 +395,8 @@ func DeriveBase85Password(
// Base85 encode all 64 bytes of entropy using the RFC1924 character set // Base85 encode all 64 bytes of entropy using the RFC1924 character set
encoded := encodeBase85WithRFC1924Charset(entropy) encoded := encodeBase85WithRFC1924Charset(entropy)
// Slice to the desired password length // Slice to the desired password length: 64 bytes of entropy give 80
if len(encoded) < int(pwdLen) { // characters, the most pwdLen allows
return "", fmt.Errorf(
"encoded length %d %w %d",
len(encoded), ErrEncodedTooShort, pwdLen,
)
}
return encoded[:pwdLen], nil return encoded[:pwdLen], nil
} }