Give each failure one error value (closes #113)

internal/cli drops its copies of vault.ErrSecretNotFound, ErrVaultNotFound,
ErrVersionNotFound and ErrSecretExists and of the secret package's keychain
and Secure Enclave errors, and its second error for an unknown unlocker
type, an invalid mnemonic, a length below 1, an unsupported secret type and
an oversized secret. vault.ErrNilValueBuffer becomes
secret.ErrNilValueBuffer. Every error of secret.ReadPassphrase wraps
ErrPassphraseNotRead. ResolveGPGKeyFingerprint returns ErrGPGKeyNotFound for
a key the keyring lacks. storeInKeychain returns errNilDataBuffer. bip85's
ErrPasswordTooShort and ErrEncodedTooShort go with their unreachable checks,
as does the macOS check in macOS-only code. Tests that matched these
errors' text use errors.Is.

Model: opus-5-5

TODO.md

@@ -25,22 +25,36 @@ https://git.eeqj.de/sneak/secret/milestone/12
   `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`.
+  check rejects it. Off macOS, adding a keychain or Secure Enclave unlocker
+  returns the `secret` package's error for it, not an `internal/cli` copy; on
+  macOS, the check that the system is macOS is gone, as it could never fail.
+  `secret vault import` gives `errInvalidMnemonicPhrase` for an invalid
+  mnemonic, as `init` and `vault create` do. `secret generate secret` gives
+  `errLengthTooSmall` for a length below 1 wherever it is checked, and
+  `errUnsupportedSecretType` for `--type mnemonic` too. `secret import` of a
+  file over 100MB wraps `errSecretTooLarge`, as `secret add` returns it.
+  `vault.ErrNilValueBuffer` is replaced by `secret.ErrNilValueBuffer`, which
+  `secret` already returned under another name. Messages are unchanged,
+  except that `secret decrypt` of a missing secret says "not found", as
+  `secret get` does, not "does not exist"; `vault import` of an invalid
+  mnemonic says "invalid BIP39 mnemonic phrase"; `--type mnemonic` says
+  "unsupported type: mnemonic (use 'secret generate mnemonic' instead)"; and
+  a file too large to import says
+  `failed to read secret from file <path>: secret too large: exceeds 100MB limit`.
+  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`,
   not by matching words of its message
   (https://git.eeqj.de/sneak/secret/issues/49). Every exported error that

internal/cli/generate.go

@@ -21,10 +21,6 @@ const (
 // Sentinel errors for secret generation
 var (
 	errLengthTooSmall        = errors.New("length must be at least 1")
-	errLengthNotPositive        = errors.New("length must be positive")
-	errMnemonicTypeNotSupported = errors.New(
-		"mnemonic type not supported for secret generation, " +
-			"use 'secret generate mnemonic' instead")
 	errUnsupportedSecretType = errors.New("unsupported type")
 )
 
@@ -148,7 +144,8 @@ func (cli *Instance) GenerateSecret(
 	case "alnum":
 		secretValue, err = generateRandomAlnum(length)
 	case "mnemonic":
-		return errMnemonicTypeNotSupported
+		return fmt.Errorf("%w: mnemonic (use 'secret generate mnemonic' instead)",
+			errUnsupportedSecretType)
 	default:
 		return fmt.Errorf("%w: %s (supported: base58, alnum)",
 			errUnsupportedSecretType, secretType)
@@ -204,8 +201,8 @@ func generateRandomAlnum(length int) (string, error) {
 // generateRandomString generates a random string of the specified length
 // using the given character set
 func generateRandomString(length int, charset string) (string, error) {
-	if length <= 0 {
-		return "", errLengthNotPositive
+	if length < 1 {
+		return "", errLengthTooSmall
 	}
 
 	result := make([]byte, length)

internal/cli/input_errors_test.go

@@ -0,0 +1,67 @@
+//nolint:testpackage // white-box test of unexported internals
+package cli
+
+import (
+	"testing"
+
+	"git.eeqj.de/sneak/secret/internal/vault"
+	"github.com/awnumar/memguard"
+	"github.com/spf13/afero"
+	"github.com/stretchr/testify/require"
+)
+
+// TestInvalidMnemonicError checks that every command that takes a mnemonic
+// returns errInvalidMnemonicPhrase for one that is not valid BIP39. The vault
+// "other" has no long-term key, as vault import needs.
+func TestInvalidMnemonicError(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		command string
+		run     func(c *Instance) error
+	}{
+		{"secret init", func(c *Instance) error { return c.Init(c.cmd) }},
+		{"secret vault create work", func(c *Instance) error {
+			return c.CreateVault(c.cmd, "work")
+		}},
+		{"secret vault import other", func(c *Instance) error {
+			return c.VaultImport(c.cmd, "other")
+		}},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.command, func(t *testing.T) {
+			t.Parallel()
+
+			fs := afero.NewMemMapFs()
+			_, err := vault.CreateVault(fs, listTestStateDir, "other", nil, nil)
+			require.NoError(t, err)
+
+			instance, _ := newTestInstance(fs)
+			instance.Mnemonic = memguard.NewBufferFromBytes([]byte("not a mnemonic"))
+			t.Cleanup(instance.Mnemonic.Destroy)
+
+			require.ErrorIs(t, tt.run(instance), errInvalidMnemonicPhrase)
+		})
+	}
+}
+
+// TestGenerateSecretErrors checks that `secret generate secret` gives one
+// error for a length below 1 and one for a type it cannot generate.
+func TestGenerateSecretErrors(t *testing.T) {
+	t.Parallel()
+
+	instance, cmd := newTestInstance(afero.NewMemMapFs())
+
+	err := instance.GenerateSecret(cmd, "x", 0, "base58", false)
+	require.ErrorIs(t, err, errLengthTooSmall)
+
+	_, err = generateRandomString(0, "ab")
+	require.ErrorIs(t, err, errLengthTooSmall)
+
+	err = instance.GenerateSecret(cmd, "x", defaultSecretLength, "mnemonic", false)
+	require.ErrorIs(t, err, errUnsupportedSecretType)
+
+	err = instance.GenerateSecret(cmd, "x", defaultSecretLength, "hex", false)
+	require.ErrorIs(t, err, errUnsupportedSecretType)
+}

internal/cli/secrets.go

@@ -33,8 +33,6 @@ const (
 // Sentinel errors for secret operations
 var (
 	errSecretTooLarge              = errors.New("secret too large: exceeds 100MB limit")
-	errSecretFileTooLarge = errors.New(
-		"secret file too large: exceeds 100MB limit")
 	errCrossVaultSourceUnqualified = errors.New(
 		"source must specify vault (e.g., vault:secret) for cross-vault move")
 	errMoveOntoItself = errors.New("cannot be moved onto itself")
@@ -669,10 +667,6 @@ func (cli *Instance) ImportSecret(
 
 	buffers, totalSize, err := readSecretFromReader(file)
 	if err != nil {
-		if errors.Is(err, errSecretTooLarge) {
-			return errSecretFileTooLarge
-		}
-
 		return fmt.Errorf("failed to read secret from file %s: %w", sourceFile, err)
 	}
 	defer destroyBuffers(buffers)

internal/cli/secrets_size_test.go

@@ -289,7 +289,7 @@ func TestImportSecretVariousSizes(t *testing.T) {
 		{
 			name:    "101MB file - should fail",
 			size:    101 * 1024 * 1024,
-			wantErr: errSecretFileTooLarge,
+			wantErr: errSecretTooLarge,
 		},
 	}
 

internal/cli/unlockers.go

@@ -39,10 +39,6 @@ var (
 	errInvalidUnlockerType = errors.New("invalid unlocker type")
 	errKeyIDOnlyForPGP     = errors.New(
 		"--keyid flag is only valid for PGP unlockers")
-	errKeychainMacOSOnly = errors.New(
-		"keychain unlockers are only supported on macOS")
-	errSecureEnclaveMacOSOnly = errors.New(
-		"secure enclave unlockers are only supported on macOS")
 	// errGPGKeyAlreadyUnlocker carries only the message tail; the caller
 	// composes "GPG key <id> is already added as an unlocker".
 	errGPGKeyAlreadyUnlocker = errors.New(
@@ -493,10 +489,6 @@ func (cli *Instance) addPassphraseUnlocker(cmd *cobra.Command) error {
 
 // addKeychainUnlocker creates a macOS Keychain unlocker in the current vault
 func (cli *Instance) addKeychainUnlocker(cmd *cobra.Command) error {
-	if runtime.GOOS != platformDarwin {
-		return errKeychainMacOSOnly
-	}
-
 	keychainUnlocker, err := secret.CreateKeychainUnlocker(
 		cli.fs, cli.stateDir, cli.Mnemonic, cli.UnlockPassphrase)
 	if err != nil {
@@ -524,10 +516,6 @@ func (cli *Instance) addKeychainUnlocker(cmd *cobra.Command) error {
 // addSecureEnclaveUnlocker creates a Secure Enclave unlocker in the
 // current vault
 func (cli *Instance) addSecureEnclaveUnlocker(cmd *cobra.Command) error {
-	if runtime.GOOS != platformDarwin {
-		return errSecureEnclaveMacOSOnly
-	}
-
 	seUnlocker, err := secret.CreateSecureEnclaveUnlocker(
 		cli.fs, cli.stateDir, cli.Mnemonic, cli.UnlockPassphrase)
 	if err != nil {

internal/cli/vault.go

@@ -23,7 +23,6 @@ import (
 var (
 	errMnemonicEmpty         = errors.New("mnemonic cannot be empty")
 	errInvalidMnemonicPhrase = errors.New("invalid BIP39 mnemonic phrase")
-	errInvalidMnemonic       = errors.New("invalid BIP39 mnemonic")
 	errVaultHasLongTermKey   = errors.New(
 		"already has a long-term key configured")
 	errMnemonicEnvNotSet = errors.New(
@@ -381,7 +380,7 @@ func (cli *Instance) vaultImportPreflight(
 	secret.Debug("Validating BIP39 mnemonic", "word_count", len(mnemonicWords))
 
 	if !bip39.IsMnemonicValid(mnemonic) {
-		return "", "", "", errInvalidMnemonic
+		return "", "", "", errInvalidMnemonicPhrase
 	}
 
 	return vaultDir, pubKeyPath, mnemonic, nil

internal/secret/keychainunlocker.go

@@ -11,7 +11,6 @@ import (
 	"os"
 	"path/filepath"
 	"regexp"
-	"runtime"
 	"time"
 
 	"filippo.io/age"
@@ -39,8 +38,6 @@ const (
 var keychainItemNameRegex = regexp.MustCompile(`^[A-Za-z0-9._-]+$`)
 
 var (
-	errNotMacOS = errors.New(
-		"keychain unlockers are only supported on macOS")
 	errKeychainItemNameEmpty      = errors.New("keychain item name cannot be empty")
 	errInvalidKeychainItemName    = errors.New("invalid keychain item name format")
 	errUnsupportedCurrentUnlocker = errors.New(
@@ -394,12 +391,6 @@ func deriveLongTermPrivateKey(
 func CreateKeychainUnlocker(
 	fs afero.Fs, stateDir string, mnemonic, passphrase *memguard.LockedBuffer,
 ) (*KeychainUnlocker, error) {
-	// Check if we're on macOS
-	err := checkMacOSAvailable()
-	if err != nil {
-		return nil, err
-	}
-
 	// Get current vault using the GetCurrentVault function from the same package
 	vault, err := GetCurrentVault(fs, stateDir)
 	if err != nil {
@@ -555,15 +546,6 @@ func writeKeychainUnlocker(
 	}, nil
 }
 
-// checkMacOSAvailable verifies that we're running on macOS
-func checkMacOSAvailable() error {
-	if runtime.GOOS != "darwin" {
-		return fmt.Errorf("%w, current OS: %s", errNotMacOS, runtime.GOOS)
-	}
-
-	return nil
-}
-
 // validateKeychainItemName validates that a keychain item name is safe for
 // command execution
 func validateKeychainItemName(itemName string) error {

internal/secret/seunlocker_darwin.go

@@ -216,11 +216,6 @@ func CreateSecureEnclaveUnlocker(
 	stateDir string,
 	mnemonic, passphrase *memguard.LockedBuffer,
 ) (*SecureEnclaveUnlocker, error) {
-	err := checkMacOSAvailable()
-	if err != nil {
-		return nil, err
-	}
-
 	vault, err := GetCurrentVault(fs, stateDir)
 	if err != nil {
 		return nil, fmt.Errorf("failed to get current vault: %w", err)

internal/secret/version.go

@@ -22,10 +22,10 @@ const (
 	maxVersionsPerDay = 999
 )
 
-var (
-	errMaxVersionsPerDay = errors.New("exceeded maximum versions per day (999)")
-	errNilValueBuffer    = errors.New("value buffer is nil")
-)
+var errMaxVersionsPerDay = errors.New("exceeded maximum versions per day (999)")
+
+// ErrNilValueBuffer is returned when a secret's value is given as nil.
+var ErrNilValueBuffer = errors.New("value buffer is nil")
 
 // VersionMetadata contains information about a secret version
 type VersionMetadata struct {
@@ -138,7 +138,7 @@ func GenerateVersionName(fs afero.Fs, secretDir string) (string, error) {
 // process dies part-way.
 func (sv *Version) Save(value *memguard.LockedBuffer) error {
 	if value == nil {
-		return errNilValueBuffer
+		return ErrNilValueBuffer
 	}
 
 	DebugWith("Saving secret version",

internal/vault/errors.go

@@ -36,9 +36,6 @@ var (
 	// it unlocks. Composed as "vault <name> needs a mnemonic for an unlocker".
 	ErrUnlockerWithoutMnemonic = errors.New("needs a mnemonic for an unlocker")
 
-	// ErrNilValueBuffer indicates a nil value buffer was supplied.
-	ErrNilValueBuffer = errors.New("value buffer is nil")
-
 	// ErrInvalidSecretName indicates a secret name that breaks the naming
 	// rule: only ASCII letters, digits, '.', '-', '_' and '/'; not empty;
 	// no leading '.' or '/', no trailing '/', no '//', no '..' path segment.

internal/vault/errors_test.go

@@ -60,7 +60,7 @@ func TestVaultErrors(t *testing.T) {
 		}, vault.ErrVaultNotFound},
 		{"add a nil value", func(vlt *vault.Vault) error {
 			return vlt.AddSecret(missingName, nil, false)
-		}, vault.ErrNilValueBuffer},
+		}, secret.ErrNilValueBuffer},
 		{"get a missing secret", func(vlt *vault.Vault) error {
 			_, err := vlt.GetSecret(missingName)
 

internal/vault/secrets.go

@@ -130,7 +130,7 @@ func ValidateSecretName(name string) error {
 // AddSecret adds a secret to this vault
 func (v *Vault) AddSecret(name string, value *memguard.LockedBuffer, force bool) error {
 	if value == nil {
-		return ErrNilValueBuffer
+		return secret.ErrNilValueBuffer
 	}
 
 	secret.DebugWith("Adding secret to vault",