Compare commits
1
Commits
next
...
af7246c608
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
af7246c608 |
@@ -18,6 +18,21 @@ https://git.eeqj.de/sneak/secret/milestone/12
|
|||||||
|
|
||||||
# Completed Steps
|
# 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`,
|
- 2026-10-04: README's Storage Architecture, `secret version promote`,
|
||||||
Technical Details and Testing text matches the code
|
Technical Details and Testing text matches the code
|
||||||
(https://git.eeqj.de/sneak/secret/issues/102). `current` and
|
(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).
|
(`secret.IdentityToLockedBuffer` overwrites only the string itself).
|
||||||
- Medium priority:
|
- Medium priority:
|
||||||
- Standardize error messages; stop leaking internals.
|
- Standardize error messages; stop leaking internals.
|
||||||
- Graceful handling of corrupted or missing key files with recovery
|
|
||||||
suggestions.
|
|
||||||
- Split oversized CLI functions.
|
- Split oversized CLI functions.
|
||||||
- Cleanups: read statedir from environment or default instead of
|
- Cleanups: read statedir from environment or default instead of
|
||||||
passing it around.
|
passing it around.
|
||||||
|
|||||||
+2
-19
@@ -130,7 +130,7 @@ func (cli *Instance) resolveEncryptionKey(
|
|||||||
}
|
}
|
||||||
|
|
||||||
// Secret exists, get the age secret key from it
|
// Secret exists, get the age secret key from it
|
||||||
secretBuffer, err := cli.getSecretValue(vlt, secretObj)
|
secretBuffer, err := vlt.GetSecret(secretName)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, fmt.Errorf("failed to get secret value: %w", err)
|
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
|
// Get the age secret key from the secret
|
||||||
secretBuffer, err := cli.getSecretValue(vlt, secretObj)
|
secretBuffer, err := vlt.GetSecret(secretName)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return fmt.Errorf("failed to get secret value: %w", err)
|
return fmt.Errorf("failed to get secret value: %w", err)
|
||||||
}
|
}
|
||||||
@@ -313,20 +313,3 @@ func isValidAgeSecretKey(key string) bool {
|
|||||||
|
|
||||||
return err == nil
|
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
|
// 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) {
|
func GetCurrentVersion(fs afero.Fs, secretDir string) (string, error) {
|
||||||
currentPath := filepath.Join(secretDir, "current")
|
currentPath := filepath.Join(secretDir, "current")
|
||||||
|
|
||||||
fileData, err := afero.ReadFile(fs, currentPath)
|
fileData, err := afero.ReadFile(fs, currentPath)
|
||||||
if err != nil {
|
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))
|
version := strings.TrimSpace(string(fileData))
|
||||||
|
|||||||
+22
-2
@@ -98,7 +98,8 @@ func (v *Vault) GetOrDeriveLongTermKey() (*age.X25519Identity, error) {
|
|||||||
if err != nil {
|
if err != nil {
|
||||||
secret.Debug("Failed to get current unlocker", "error", err, "vault_name", v.Name)
|
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",
|
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.
|
// Other unlockers return their own identity, used to decrypt longterm.age.
|
||||||
ltIdentity, err := v.unlockLongTermKey(unlocker)
|
ltIdentity, err := v.unlockLongTermKey(unlocker)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, err
|
return nil, v.withMnemonicAdvice(err)
|
||||||
}
|
}
|
||||||
|
|
||||||
secret.DebugWith("Successfully obtained long-term identity via unlocker",
|
secret.DebugWith("Successfully obtained long-term identity via unlocker",
|
||||||
@@ -295,3 +296,22 @@ func (v *Vault) unlockLongTermKey(
|
|||||||
|
|
||||||
return ltIdentity, nil
|
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