Say the mnemonic still opens a vault its unlocker cannot (closes #47)
check / check (push) Failing after 2s
check / check (push) Failing after 2s
When the vault cannot be opened through its current unlocker, the error now ends by saying that the vault still opens with its mnemonic, and that 'secret unlocker add passphrase' run with SB_SECRET_MNEMONIC set gives it a new unlocker; only when the vault metadata records the key the mnemonic derives. 'secret encrypt' and 'secret decrypt' read the key secret through vault.GetSecret, as 'secret get' does, so they say it too. An unreadable 'current' file's error names 'secret version list' and 'secret version promote'. Causes stay wrapped. Model: opus-5-5
This commit is contained in:
@@ -18,6 +18,21 @@ https://git.eeqj.de/sneak/secret/milestone/12
|
||||
|
||||
# Completed Steps
|
||||
|
||||
- 2026-10-04: When the 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 saying that the vault 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
|
||||
(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.
|
||||
`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. 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: README's Storage Architecture, `secret version promote`,
|
||||
Technical Details and Testing text matches the code
|
||||
(https://git.eeqj.de/sneak/secret/issues/102). `current` and
|
||||
@@ -388,8 +403,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.
|
||||
|
||||
+2
-19
@@ -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)
|
||||
}
|
||||
|
||||
@@ -0,0 +1,282 @@
|
||||
// Unlock Failure Tests
|
||||
//
|
||||
// When the 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 the
|
||||
// vault, but only for a vault that the mnemonic does open. 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"
|
||||
"path/filepath"
|
||||
"testing"
|
||||
|
||||
"filippo.io/age"
|
||||
"git.eeqj.de/sneak/secret/internal/cli"
|
||||
"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 a vault that its mnemonic opens
|
||||
// cannot be opened through its current unlocker.
|
||||
mnemonicAdvice = "; the vault 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"
|
||||
)
|
||||
|
||||
// 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()
|
||||
|
||||
unlockerName, err := afero.ReadFile(fs,
|
||||
filepath.Join(unlockTestVaultDir, "current-unlocker"))
|
||||
require.NoError(t, err)
|
||||
|
||||
c := cli.NewCLIInstanceWithStateDir(fs, testStateDir)
|
||||
c.UnlockPassphrase = memguard.NewBufferFromBytes([]byte(testPassphrase))
|
||||
t.Cleanup(c.UnlockPassphrase.Destroy)
|
||||
|
||||
return filepath.Join(unlockTestVaultDir, "unlockers.d", string(unlockerName)), 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)
|
||||
}
|
||||
|
||||
// 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())
|
||||
}
|
||||
@@ -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))
|
||||
|
||||
+22
-2
@@ -98,7 +98,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 +113,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 +296,22 @@ 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.
|
||||
func (v *Vault) withMnemonicAdvice(err error) error {
|
||||
vaultDir, _ := v.GetDirectory()
|
||||
|
||||
metadata, metadataErr := LoadVaultMetadata(v.fs, vaultDir)
|
||||
if metadataErr != nil || metadata.PublicKeyHash == "" {
|
||||
return err
|
||||
}
|
||||
|
||||
return fmt.Errorf("%w; the vault still opens with its mnemonic: run "+
|
||||
"'secret unlocker add passphrase' with %s set to the mnemonic "+
|
||||
"to give it a new unlocker", err, secret.EnvMnemonic)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user