diff --git a/TODO.md b/TODO.md index ca7c3d7..2f205dd 100644 --- a/TODO.md +++ b/TODO.md @@ -18,6 +18,24 @@ https://git.eeqj.de/sneak/secret/milestone/12 # Completed Steps +- 2026-10-04: When a vault cannot be opened through its current unlocker, + because a file the unlocker needs is missing or damaged, its keychain item + or Secure Enclave key is gone, or the passphrase is wrong, the error now + ends by naming the vault, saying that it still opens with its mnemonic, + and that `secret unlocker add passphrase`, run with `SB_SECRET_MNEMONIC` + set to it, gives the vault a new unlocker; for a vault that is not the + current one, as in `secret move` between vaults, it says to run + `secret vault select` first (https://git.eeqj.de/sneak/secret/issues/47). + Before, it ended with the bare cause. The advice is given only when the + vault metadata records the key the mnemonic derives, so not for a vault + created without a mnemonic, and not when the passphrase could not be read + at all. `secret vault import` is not named: it refuses a vault that has a + long-term key. `secret encrypt` and `secret decrypt` now read the key + secret through `vault.GetSecret`, as `secret get` does, so they give the + same advice; `Secret.GetValue`, the other way to get the long-term key, is + removed. When a secret's `current` file cannot be read, the error says + that `secret version list` lists its versions and `secret version promote` + makes one current. The causes stay wrapped. - 2026-10-04: An unlocker's ID is the name of its directory in `unlockers.d`, so no two unlockers of a vault share one (https://git.eeqj.de/sneak/secret/issues/98). Before, a keychain or Secure @@ -406,8 +424,6 @@ https://git.eeqj.de/sneak/secret/milestone/12 (`secret.IdentityToLockedBuffer` overwrites only the string itself). - Medium priority: - Standardize error messages; stop leaking internals. - - Graceful handling of corrupted or missing key files with recovery - suggestions. - Split oversized CLI functions. - Cleanups: read statedir from environment or default instead of passing it around. diff --git a/internal/cli/crypto.go b/internal/cli/crypto.go index d6da3d6..b420ac3 100644 --- a/internal/cli/crypto.go +++ b/internal/cli/crypto.go @@ -130,7 +130,7 @@ func (cli *Instance) resolveEncryptionKey( } // Secret exists, get the age secret key from it - secretBuffer, err := cli.getSecretValue(vlt, secretObj) + secretBuffer, err := vlt.GetSecret(secretName) if err != nil { return nil, fmt.Errorf("failed to get secret value: %w", err) } @@ -249,7 +249,7 @@ func (cli *Instance) Decrypt(secretName, inputFile, outputFile string) error { } // Get the age secret key from the secret - secretBuffer, err := cli.getSecretValue(vlt, secretObj) + secretBuffer, err := vlt.GetSecret(secretName) if err != nil { return fmt.Errorf("failed to get secret value: %w", err) } @@ -313,20 +313,3 @@ func isValidAgeSecretKey(key string) bool { return err == nil } - -// getSecretValue retrieves the value of a secret with the vault's mnemonic -// when it has one, else with the current unlocker -func (cli *Instance) getSecretValue( - vlt *vault.Vault, secretObj *secret.Secret, -) (*memguard.LockedBuffer, error) { - if vlt.Mnemonic != nil { - return secretObj.GetValue(nil, vlt.Mnemonic) - } - - unlocker, err := vlt.GetCurrentUnlocker() - if err != nil { - return nil, fmt.Errorf("failed to get current unlocker: %w", err) - } - - return secretObj.GetValue(unlocker, nil) -} diff --git a/internal/cli/unlock_failure_test.go b/internal/cli/unlock_failure_test.go new file mode 100644 index 0000000..86e4396 --- /dev/null +++ b/internal/cli/unlock_failure_test.go @@ -0,0 +1,374 @@ +// Unlock Failure Tests +// +// When a vault cannot be opened through its current unlocker, because a +// file the unlocker needs is missing or the passphrase is wrong, the error +// keeps its cause and ends by saying that the mnemonic still opens that +// vault, but only for a vault that the mnemonic does open, and not when the +// passphrase could not be read at all. When a secret's current file is +// missing, the error says how to make a version current again. Each test +// that pins such advice also follows it. + +package cli_test + +import ( + "bytes" + "io" + "os" + "os/exec" + "path/filepath" + "testing" + + "filippo.io/age" + "git.eeqj.de/sneak/secret/internal/cli" + "git.eeqj.de/sneak/secret/internal/secret" + "git.eeqj.de/sneak/secret/internal/vault" + "github.com/awnumar/memguard" + "github.com/spf13/afero" + "github.com/spf13/cobra" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +const ( + // mnemonicAdvice ends the error when the current vault "default", which + // its mnemonic opens, cannot be opened through its current unlocker. + mnemonicAdvice = "; the vault 'default' still opens with its mnemonic: " + + "run 'secret unlocker add passphrase' with SB_SECRET_MNEMONIC set " + + "to the mnemonic to give it a new unlocker" + + // versionAdvice ends the error when a secret's current file cannot be + // read. + versionAdvice = "; this file only names the current version: " + + "'secret version list' lists the secret's versions, and " + + "'secret version promote' makes one of them current" + + // unlockTestVaultDir is the directory of the vault "default" of + // newTwoVaultFs, the current vault, whose secret "x" is "value". + unlockTestVaultDir = testStateDir + "/vaults.d/default" +) + +// currentUnlockerDir returns the directory of the current unlocker of the +// vault in vaultDir on fs. +func currentUnlockerDir(t *testing.T, fs afero.Fs, vaultDir string) string { + t.Helper() + + unlockerName, err := afero.ReadFile(fs, + filepath.Join(vaultDir, "current-unlocker")) + require.NoError(t, err) + + return filepath.Join(vaultDir, "unlockers.d", string(unlockerName)) +} + +// newUnlockTestCLI returns the directory of the current unlocker of the +// vault "default" on fs, a copy of the vaults of newTwoVaultFs, and a CLI +// instance on fs that has the unlock passphrase, as from the environment, +// but not the mnemonic. +func newUnlockTestCLI(t *testing.T, fs afero.Fs) (string, *cli.Instance) { + t.Helper() + + c := cli.NewCLIInstanceWithStateDir(fs, testStateDir) + c.UnlockPassphrase = memguard.NewBufferFromBytes([]byte(testPassphrase)) + t.Cleanup(c.UnlockPassphrase.Destroy) + + return currentUnlockerDir(t, fs, unlockTestVaultDir), c +} + +// discardCmd returns a command whose output is discarded. +func discardCmd() *cobra.Command { + cmd := &cobra.Command{} + cmd.SetOut(io.Discard) + + return cmd +} + +// getSecret returns what `secret get name` prints. +func getSecret(t *testing.T, c *cli.Instance, name string) string { + t.Helper() + + var out bytes.Buffer + + cmd := &cobra.Command{} + cmd.SetOut(&out) + require.NoError(t, c.GetSecret(cmd, name)) + + return out.String() +} + +// TestUnlockFailureNamesMnemonic checks the error of `secret get` when a +// file that opening the vault through its current unlocker needs is +// missing: it keeps the cause, which names the file, and ends with the +// advice that the mnemonic still opens the vault. The test then follows +// that advice: `secret unlocker add passphrase`, with the mnemonic, gives +// the vault a new unlocker, which opens it. +func TestUnlockFailureNamesMnemonic(t *testing.T) { + t.Parallel() + + tests := []struct { + file string // the file removed + inVaultDir bool // the file is the vault's, not the unlocker's + want string // the message before the cause + }{ + { + file: "current-unlocker", + inVaultDir: true, + want: "failed to unlock vault: failed to get long-term key: " + + "failed to get current unlocker: " + + "failed to read current unlocker: ", + }, + { + file: "priv.age", + want: "failed to unlock vault: failed to get long-term key: " + + "failed to get unlocker identity: " + + "failed to read unlocker private key: ", + }, + { + file: "longterm.age", + want: "failed to unlock vault: failed to get long-term key: " + + "failed to read encrypted long-term private key: ", + }, + } + + for _, tt := range tests { + t.Run(tt.file, func(t *testing.T) { + t.Parallel() + + fs := newTwoVaultFs(t) + unlockerDir, c := newUnlockTestCLI(t, fs) + + path := filepath.Join(unlockerDir, tt.file) + + if tt.inVaultDir { + path = filepath.Join(unlockTestVaultDir, tt.file) + } + + require.NoError(t, fs.Remove(path)) + + err := c.GetSecret(discardCmd(), "x") + + var cause *os.PathError + + require.ErrorAs(t, err, &cause) + require.ErrorIs(t, err, os.ErrNotExist) + assert.Equal(t, path, cause.Path) + + require.EqualError(t, err, tt.want+cause.Error()+mnemonicAdvice) + + c.Mnemonic = testMnemonicBuffer(t) + require.NoError(t, c.UnlockersAdd("passphrase", discardCmd())) + + c.Mnemonic = nil + assert.Equal(t, "value", getSecret(t, c, "x")) + }) + } +} + +// TestWrongPassphraseNamesMnemonic checks the error of `secret get` given a +// passphrase that does not decrypt the passphrase unlocker: it keeps age's +// error and ends with the advice that the mnemonic still opens the vault. +func TestWrongPassphraseNamesMnemonic(t *testing.T) { + t.Parallel() + + _, c := newUnlockTestCLI(t, newTwoVaultFs(t)) + c.UnlockPassphrase = memguard.NewBufferFromBytes([]byte("wrong passphrase")) + t.Cleanup(c.UnlockPassphrase.Destroy) + + err := c.GetSecret(discardCmd(), "x") + + var noMatch *age.NoIdentityMatchError + + require.ErrorAs(t, err, &noMatch) + + require.EqualError(t, err, "failed to unlock vault: "+ + "failed to get long-term key: failed to get unlocker identity: "+ + "failed to decrypt unlocker private key: failed to create decryptor: "+ + noMatch.Error()+mnemonicAdvice) +} + +// TestMoveUnlockFailureNamesVault checks the error of `secret move` into +// the vault "work", which is not the current vault, when "work" cannot be +// opened through its current unlocker: the advice names "work" and says to +// select it first, since `secret unlocker add` acts on the current vault. +// The test then follows that advice, and the move succeeds. +func TestMoveUnlockFailureNamesVault(t *testing.T) { + t.Parallel() + + fs := newTwoVaultFs(t) + _, c := newUnlockTestCLI(t, fs) + + path := filepath.Join( + currentUnlockerDir(t, fs, testStateDir+"/vaults.d/work"), "priv.age") + require.NoError(t, fs.Remove(path)) + + err := c.MoveSecret(discardCmd(), "default:x", "work:y", false) + + var cause *os.PathError + + require.ErrorAs(t, err, &cause) + assert.Equal(t, path, cause.Path) + + require.EqualError(t, err, "failed to unlock destination vault 'work': "+ + "failed to get unlocker identity: failed to read unlocker private key: "+ + cause.Error()+"; the vault 'work' still opens with its mnemonic: "+ + "run 'secret vault select work', then 'secret unlocker add passphrase' "+ + "with SB_SECRET_MNEMONIC set to the mnemonic to give it a new unlocker") + + require.NoError(t, c.SelectVault(discardCmd(), "work")) + + c.Mnemonic = testMnemonicBuffer(t) + require.NoError(t, c.UnlockersAdd("passphrase", discardCmd())) + + c.Mnemonic = nil + require.NoError(t, c.MoveSecret(discardCmd(), "default:x", "work:y", false)) + assert.Equal(t, "value", getSecret(t, c, "y")) +} + +// TestPassphraseNotReadNamesNoMnemonic runs `secret get x` on the built +// binary without SB_UNLOCK_PASSPHRASE and with a stdin that is not a +// terminal, so the passphrase cannot be read. The unlocker was not tried, +// and adding one would need a passphrase read the same way, so the error +// is the cause alone, without the advice to use the mnemonic. +func TestPassphraseNotReadNamesNoMnemonic(t *testing.T) { + t.Parallel() + + stateDir := t.TempDir() + + mnemonic := memguard.NewBufferFromBytes([]byte(testMnemonic)) + defer mnemonic.Destroy() + + passphrase := memguard.NewBufferFromBytes([]byte(testPassphrase)) + defer passphrase.Destroy() + + vlt, err := vault.CreateVault( + afero.NewOsFs(), stateDir, "default", mnemonic, passphrase) + require.NoError(t, err) + + value := memguard.NewBufferFromBytes([]byte("value")) + defer value.Destroy() + + require.NoError(t, vlt.AddSecret("x", value, false)) + + //nolint:gosec // G204: test executes the freshly built secret binary + cmd := exec.CommandContext(t.Context(), secretBinaryPath(t), "get", "x") + cmd.Env = []string{ + secret.EnvStateDir + "=" + stateDir, + "PATH=" + os.Getenv("PATH"), + "HOME=" + os.Getenv("HOME"), + } + + output, err := cmd.CombinedOutput() + require.Error(t, err) + + assert.Equal(t, "Error: failed to unlock vault: "+ + "failed to get long-term key: failed to get unlocker identity: "+ + "failed to read passphrase: cannot read passphrase from non-terminal "+ + "stdin (piped input or script). Please set the SB_UNLOCK_PASSPHRASE "+ + "environment variable or run interactively\n", string(output)) +} + +// TestCryptoUnlockFailureNamesMnemonic checks that `secret encrypt` and +// `secret decrypt`, reading the key secret, end with the same advice as +// `secret get` when the vault cannot be opened through its current +// unlocker. +func TestCryptoUnlockFailureNamesMnemonic(t *testing.T) { + t.Parallel() + + tests := []struct { + command string + run func(c *cli.Instance) error + }{ + {"encrypt", func(c *cli.Instance) error { return c.Encrypt("x", "", "") }}, + {"decrypt", func(c *cli.Instance) error { return c.Decrypt("x", "", "") }}, + } + + for _, tt := range tests { + t.Run(tt.command, func(t *testing.T) { + t.Parallel() + + fs := newTwoVaultFs(t) + unlockerDir, c := newUnlockTestCLI(t, fs) + + path := filepath.Join(unlockerDir, "priv.age") + require.NoError(t, fs.Remove(path)) + + err := tt.run(c) + + var cause *os.PathError + + require.ErrorAs(t, err, &cause) + assert.Equal(t, path, cause.Path) + + require.EqualError(t, err, "failed to get secret value: "+ + "failed to unlock vault: failed to get long-term key: "+ + "failed to get unlocker identity: "+ + "failed to read unlocker private key: "+cause.Error()+ + mnemonicAdvice) + }) + } +} + +// TestMissingCurrentFileNamesVersionCommands checks the error of `secret +// get` when the secret's current file is missing: it keeps the cause, which +// names the file, and ends with the advice that says how to make a version +// current again. The test then follows that advice. +func TestMissingCurrentFileNamesVersionCommands(t *testing.T) { + t.Parallel() + + fs := newTwoVaultFs(t) + _, c := newUnlockTestCLI(t, fs) + + secretDir := filepath.Join(unlockTestVaultDir, "secrets.d", "x") + path := filepath.Join(secretDir, "current") + require.NoError(t, fs.Remove(path)) + + err := c.GetSecret(discardCmd(), "x") + + var cause *os.PathError + + require.ErrorAs(t, err, &cause) + require.ErrorIs(t, err, os.ErrNotExist) + assert.Equal(t, path, cause.Path) + + require.EqualError(t, err, "failed to get current version: "+ + "failed to read current version file: "+cause.Error()+versionAdvice) + + versions, err := afero.ReadDir(fs, filepath.Join(secretDir, "versions")) + require.NoError(t, err) + require.Len(t, versions, 1) + + var out bytes.Buffer + + cmd := &cobra.Command{} + cmd.SetOut(&out) + require.NoError(t, c.ListVersions(cmd, "x")) + assert.Contains(t, out.String(), versions[0].Name()) + + require.NoError(t, c.PromoteVersion(cmd, "x", versions[0].Name())) + assert.Equal(t, "value", getSecret(t, c, "x")) +} + +// TestUnlockFailureWithoutLongTermKeyNamesNoMnemonic checks that a vault +// created without a mnemonic, which no mnemonic opens, gets no advice to +// use one: `secret unlocker add passphrase` there fails with the cause +// alone. +func TestUnlockFailureWithoutLongTermKeyNamesNoMnemonic(t *testing.T) { + t.Parallel() + + fs := afero.NewMemMapFs() + + _, err := vault.CreateVault(fs, testStateDir, "keyless", nil, nil) + require.NoError(t, err) + + c := cli.NewCLIInstanceWithStateDir(fs, testStateDir) + c.UnlockPassphrase = memguard.NewBufferFromBytes([]byte(testPassphrase)) + t.Cleanup(c.UnlockPassphrase.Destroy) + + err = c.UnlockersAdd("passphrase", discardCmd()) + + var cause *os.PathError + + require.ErrorAs(t, err, &cause) + + require.EqualError(t, err, "failed to get long-term key: "+ + "failed to get current unlocker: failed to read current unlocker: "+ + cause.Error()) +} diff --git a/internal/secret/passphraseunlocker.go b/internal/secret/passphraseunlocker.go index 2ed41f2..2ae7bfb 100644 --- a/internal/secret/passphraseunlocker.go +++ b/internal/secret/passphraseunlocker.go @@ -1,6 +1,7 @@ package secret import ( + "errors" "fmt" "log/slog" "path/filepath" @@ -10,6 +11,11 @@ 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. +var ErrPassphraseNotRead = errors.New("failed to read passphrase") + // PassphraseUnlocker represents a passphrase-protected unlocker type PassphraseUnlocker struct { Directory string @@ -149,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("failed to read passphrase: %w", err) + return nil, fmt.Errorf("%w: %w", ErrPassphraseNotRead, err) } return secureBuffer, nil diff --git a/internal/secret/secret.go b/internal/secret/secret.go index d36b014..ed9989f 100644 --- a/internal/secret/secret.go +++ b/internal/secret/secret.go @@ -1,26 +1,18 @@ package secret import ( - "encoding/json" "errors" - "fmt" "log/slog" "path/filepath" "strings" "time" "filippo.io/age" - "git.eeqj.de/sneak/secret/pkg/agehd" "github.com/awnumar/memguard" "github.com/spf13/afero" ) var ( - // errSecretNotFound carries only the message tail; callers compose - // "secret not found" around it so the emitted text is - // unchanged. - errSecretNotFound = errors.New("not found") - errUnlockerRequired = errors.New("unlocker required to decrypt secret") errGetEncryptedDataDeprecated = errors.New( "GetEncryptedData is deprecated - use version-specific methods") errGetCurrentVaultNotRegistered = errors.New( @@ -81,73 +73,6 @@ func NewSecret(vault VaultInterface, name string) *Secret { } } -// GetValue retrieves and decrypts the current version's value, with the -// vault's long-term key derived from mnemonic when it is not nil, else -// obtained through unlocker -func (s *Secret) GetValue( - unlocker Unlocker, mnemonic *memguard.LockedBuffer, -) (*memguard.LockedBuffer, error) { - DebugWith("Getting secret value", - slog.String("secret_name", s.Name), - slog.String("vault_name", s.vault.GetName()), - ) - - // Check if secret exists - exists, err := s.Exists() - if err != nil { - Debug("Failed to check if secret exists during GetValue", - "error", err, "secret_name", s.Name) - - return nil, fmt.Errorf("failed to check if secret exists: %w", err) - } - - if !exists { - Debug("Secret not found during GetValue", - "secret_name", s.Name, "vault_name", s.vault.GetName()) - - return nil, fmt.Errorf("secret %s %w", s.Name, errSecretNotFound) - } - - Debug("Secret exists, getting current version", "secret_name", s.Name) - - // Get current version - currentVersion, err := GetCurrentVersion(s.vault.GetFilesystem(), s.Directory) - if err != nil { - Debug("Failed to get current version", "error", err, "secret_name", s.Name) - - return nil, fmt.Errorf("failed to get current version: %w", err) - } - - // Create version object - version := NewVersion(s.vault, s.Name, currentVersion) - - if mnemonic != nil { - return s.getValueViaMnemonic(version, mnemonic.String()) - } - - Debug("Using unlocker for vault access", "secret_name", s.Name) - - // Use the provided unlocker to get the vault's long-term private key - if unlocker == nil { - Debug("No unlocker provided for secret decryption", "secret_name", s.Name) - - return nil, errUnlockerRequired - } - - ltIdentity, err := s.getLongTermIdentityFromUnlocker(unlocker) - if err != nil { - return nil, err - } - - DebugWith("Successfully obtained vault's long-term key", - slog.String("secret_name", s.Name), - slog.String("public_key", ltIdentity.Recipient().String()), - ) - - // Use the long-term key to decrypt the version - return version.GetValue(ltIdentity) -} - // LoadMetadata is deprecated - metadata is now per-version and encrypted func (s *Secret) LoadMetadata() error { Debug("LoadMetadata called but is deprecated in versioned model", @@ -215,124 +140,6 @@ func (s *Secret) Exists() (bool, error) { return true, nil } -// getValueViaMnemonic derives the vault's long-term key from the -// mnemonic and decrypts the version value with it. -func (s *Secret) getValueViaMnemonic( - version *Version, mnemonic string, -) (*memguard.LockedBuffer, error) { - Debug("Using mnemonic for direct long-term key derivation", - "secret_name", s.Name) - - // Get vault directory to read metadata - vaultDir, err := s.vault.GetDirectory() - if err != nil { - Debug("Failed to get vault directory", "error", err, "secret_name", s.Name) - - return nil, fmt.Errorf("failed to get vault directory: %w", err) - } - - // Load vault metadata to get the correct derivation index - metadataPath := filepath.Join(vaultDir, "vault-metadata.json") - - metadataBytes, err := afero.ReadFile(s.vault.GetFilesystem(), metadataPath) - if err != nil { - Debug("Failed to read vault metadata", "error", err, "path", metadataPath) - - return nil, fmt.Errorf("failed to read vault metadata: %w", err) - } - - var metadata VaultMetadata - - err = json.Unmarshal(metadataBytes, &metadata) - if err != nil { - Debug("Failed to parse vault metadata", "error", err, "secret_name", s.Name) - - return nil, fmt.Errorf("failed to parse vault metadata: %w", err) - } - - DebugWith("Using vault derivation index from metadata", - slog.String("secret_name", s.Name), - slog.String("vault_name", s.vault.GetName()), - slog.Uint64("derivation_index", uint64(metadata.DerivationIndex)), - ) - - // Use mnemonic with the vault's derivation index from metadata - ltIdentity, err := agehd.DeriveIdentity(mnemonic, metadata.DerivationIndex) - if err != nil { - Debug("Failed to derive long-term key from mnemonic for secret", - "error", err, "secret_name", s.Name) - - return nil, fmt.Errorf( - "failed to derive long-term key from mnemonic: %w", err) - } - - Debug("Successfully derived long-term key from mnemonic", "secret_name", s.Name) - - // Use the long-term key to decrypt the version - return version.GetValue(ltIdentity) -} - -// getLongTermIdentityFromUnlocker uses the unlocker to obtain and parse -// the vault's long-term private key. -func (s *Secret) getLongTermIdentityFromUnlocker( - unlocker Unlocker, -) (*age.X25519Identity, error) { - DebugWith("Getting vault's long-term key using unlocker", - slog.String("secret_name", s.Name), - slog.String("unlocker_type", unlocker.GetType()), - slog.String("unlocker_id", unlocker.GetID()), - ) - - // Step 1: Use the unlocker to get the vault's long-term private key - unlockIdentity, err := unlocker.GetIdentity() - if err != nil { - Debug("Failed to get unlocker identity", - "error", err, "secret_name", s.Name, - "unlocker_type", unlocker.GetType()) - - return nil, fmt.Errorf("failed to get unlocker identity: %w", err) - } - - // Read the encrypted long-term private key from the unlocker directory - encryptedLtPrivKeyPath := filepath.Join(unlocker.GetDirectory(), "longterm.age") - Debug("Reading encrypted long-term private key", "path", encryptedLtPrivKeyPath) - - encryptedLtPrivKey, err := afero.ReadFile( - s.vault.GetFilesystem(), encryptedLtPrivKeyPath) - if err != nil { - Debug("Failed to read encrypted long-term private key", - "error", err, "path", encryptedLtPrivKeyPath) - - return nil, fmt.Errorf( - "failed to read encrypted long-term private key: %w", err) - } - - // Decrypt the encrypted long-term private key using the unlocker - Debug("Decrypting long-term private key using unlocker", "secret_name", s.Name) - - ltPrivKeyBuffer, err := DecryptWithIdentity(encryptedLtPrivKey, unlockIdentity) - if err != nil { - Debug("Failed to decrypt long-term private key", - "error", err, "secret_name", s.Name) - - return nil, fmt.Errorf("failed to decrypt long-term private key: %w", err) - } - defer ltPrivKeyBuffer.Destroy() - - // Parse the long-term private key - Debug("Parsing long-term private key", "secret_name", s.Name) - - ltIdentity, err := age.ParseX25519Identity(ltPrivKeyBuffer.String()) - if err != nil { - Debug("Failed to parse long-term private key", - "error", err, "secret_name", s.Name) - - return nil, fmt.Errorf("failed to parse long-term private key: %w", err) - } - - return ltIdentity, nil -} - // GetCurrentVault gets the current vault from the file system // This function is a wrapper around the actual implementation in the vault package // and exists to break the import cycle. diff --git a/internal/secret/secret_test.go b/internal/secret/secret_test.go index 68bef3c..ac05ec2 100644 --- a/internal/secret/secret_test.go +++ b/internal/secret/secret_test.go @@ -2,7 +2,6 @@ package secret import ( - "encoding/json" "errors" "os" "path/filepath" @@ -13,7 +12,6 @@ import ( "git.eeqj.de/sneak/secret/pkg/agehd" "github.com/awnumar/memguard" "github.com/spf13/afero" - "github.com/stretchr/testify/require" ) // testMnemonicValue is the standard BIP39 test vector mnemonic. @@ -321,46 +319,3 @@ func TestPerSecretKeyFunctionality(t *testing.T) { t.Logf("Secret.Exists() works correctly") }) } - -// TestSecretGetValueWithMnemonicUsesVaultDerivationIndex checks that -// GetValue, given the mnemonic, derives the long-term key at the derivation -// index in the vault's metadata. At index 0 it could not decrypt the secret, -// which was encrypted to the key at index 1. -func TestSecretGetValueWithMnemonicUsesVaultDerivationIndex(t *testing.T) { - t.Parallel() - - fs := afero.NewMemMapFs() - vaultDir := "/test-config/vaults.d/test-vault" - - mnemonic := memguard.NewBufferFromBytes([]byte(testMnemonicValue)) - defer mnemonic.Destroy() - - vlt := &MockVault{ - name: "test-vault", - fs: fs, - directory: vaultDir, - derivationIndex: 1, - mnemonic: mnemonic, - } - - metadata, err := json.Marshal(VaultMetadata{DerivationIndex: vlt.derivationIndex}) - require.NoError(t, err) - require.NoError(t, fs.MkdirAll(vaultDir, DirPerms)) - - err = afero.WriteFile( - fs, filepath.Join(vaultDir, "vault-metadata.json"), metadata, FilePerms) - require.NoError(t, err) - - secretName, secretValue := "x", "value" - - err = vlt.AddSecret(secretName, - memguard.NewBufferFromBytes([]byte(secretValue)), false) - require.NoError(t, err) - - value, err := NewSecret(vlt, secretName).GetValue(nil, mnemonic) - require.NoError(t, err) - - defer value.Destroy() - - require.Equal(t, secretValue, value.String()) -} diff --git a/internal/secret/version.go b/internal/secret/version.go index 17cca2b..fe9fd0a 100644 --- a/internal/secret/version.go +++ b/internal/secret/version.go @@ -557,13 +557,18 @@ func VersionExists(fs afero.Fs, secretDir string, version string) (bool, error) } // GetCurrentVersion returns the version that the "current" file points to -// The file contains just the version name (e.g., "20231215.001") +// The file contains just the version name (e.g., "20231215.001"). If it +// cannot be read, the error says how to make a version current again: the +// versions themselves are not in the file. func GetCurrentVersion(fs afero.Fs, secretDir string) (string, error) { currentPath := filepath.Join(secretDir, "current") fileData, err := afero.ReadFile(fs, currentPath) if err != nil { - return "", fmt.Errorf("failed to read current version file: %w", err) + return "", fmt.Errorf("failed to read current version file: %w; "+ + "this file only names the current version: 'secret version list' "+ + "lists the secret's versions, and 'secret version promote' makes "+ + "one of them current", err) } version := strings.TrimSpace(string(fileData)) diff --git a/internal/vault/vault.go b/internal/vault/vault.go index a4f3fe1..262fdb5 100644 --- a/internal/vault/vault.go +++ b/internal/vault/vault.go @@ -1,6 +1,7 @@ package vault import ( + "errors" "fmt" "log/slog" "path/filepath" @@ -98,7 +99,8 @@ func (v *Vault) GetOrDeriveLongTermKey() (*age.X25519Identity, error) { if err != nil { secret.Debug("Failed to get current unlocker", "error", err, "vault_name", v.Name) - return nil, fmt.Errorf("failed to get current unlocker: %w", err) + return nil, v.withMnemonicAdvice( + fmt.Errorf("failed to get current unlocker: %w", err)) } secret.DebugWith("Retrieved current unlocker for vault unlock", @@ -112,7 +114,7 @@ func (v *Vault) GetOrDeriveLongTermKey() (*age.X25519Identity, error) { // Other unlockers return their own identity, used to decrypt longterm.age. ltIdentity, err := v.unlockLongTermKey(unlocker) if err != nil { - return nil, err + return nil, v.withMnemonicAdvice(err) } secret.DebugWith("Successfully obtained long-term identity via unlocker", @@ -295,3 +297,36 @@ func (v *Vault) unlockLongTermKey( return ltIdentity, nil } + +// withMnemonicAdvice returns err, a failure to get the long-term key through +// the current unlocker, with advice added: that the mnemonic still opens the +// vault, and how to give it a new unlocker. The advice is added only when the +// vault metadata records the key that the mnemonic derives; a vault created +// without a mnemonic records none, and without its metadata the key cannot +// be derived. It is not added when the passphrase could not be read: the +// unlocker was not tried, and adding one would need a passphrase read the +// same way. +func (v *Vault) withMnemonicAdvice(err error) error { + if errors.Is(err, secret.ErrPassphraseNotRead) { + return err + } + + vaultDir, _ := v.GetDirectory() + + metadata, metadataErr := LoadVaultMetadata(v.fs, vaultDir) + if metadataErr != nil || metadata.PublicKeyHash == "" { + return err + } + + // 'secret unlocker add' acts on the current vault only. + steps := "'secret unlocker add passphrase'" + + current, currentErr := GetCurrentVault(v.fs, v.stateDir) + if currentErr != nil || current.Name != v.Name { + steps = fmt.Sprintf("'secret vault select %s', then %s", v.Name, steps) + } + + return fmt.Errorf("%w; the vault '%s' still opens with its mnemonic: run "+ + "%s with %s set to the mnemonic to give it a new unlocker", + err, v.Name, steps, secret.EnvMnemonic) +}