Refuse to create a vault that already exists (closes #74)
check / check (push) Waiting to run

vault.CreateVault now checks for the vault before writing anything and
fails with "vault NAME already exists" (vault.ErrVaultExists). secret
init and secret vault create call it while holding the state directory
lock, so two creates at once cannot both pass the check. Before, either
command over an existing vault replaced its metadata, passphrase
unlocker and longterm.age, so none of its secrets could be decrypted.

Both commands now ask for the unlocker passphrase before creating the
vault, so one stopped at that prompt leaves no vault without an
unlocker behind, which they would then refuse to create again.

The lock tests set up the vault "work" instead of "default", which init
now refuses to create again.

Model: opus-5-5
This commit was merged in pull request #82.
This commit is contained in:
2026-10-04 08:25:40 +02:00
parent 5ec59862ff
commit fb4481b4f7
7 changed files with 204 additions and 26 deletions
+12 -3
View File
@@ -25,6 +25,15 @@ Bring the repo into policy compliance in one commit:
# Completed Steps # Completed Steps
- 2026-10-04: `secret init` refuses when the default vault exists, and
`secret vault create NAME` when `NAME` does, with "vault NAME already
exists", before writing anything. The check is in `vault.CreateVault`,
which both commands call while holding the state directory lock, so two
creates of one vault at once cannot both pass the check. Before, either
command replaced the vault's metadata, passphrase unlocker and
`longterm.age`, so none of its secrets could be decrypted any more. Both
commands now ask for the unlocker passphrase before creating the vault,
so one stopped at that prompt leaves no vault behind.
- 2026-10-04: The `internal/cli` tests are back to about their time - 2026-10-04: The `internal/cli` tests are back to about their time
before the state directory lock before the state directory lock
(https://git.eeqj.de/sneak/secret/issues/80). The test that each (https://git.eeqj.de/sneak/secret/issues/80). The test that each
@@ -78,9 +87,9 @@ Bring the repo into policy compliance in one commit:
has one, and to a PGP, keychain or Secure Enclave unlocker added has one, and to a PGP, keychain or Secure Enclave unlocker added
on the same host and day as another of its type on the same host and day as another of its type
(https://git.eeqj.de/sneak/secret/issues/71); (https://git.eeqj.de/sneak/secret/issues/71);
- from `vault create` stopped at the passphrase prompt, a new vault - from `init` or `vault create` killed after the passphrase prompt
with no unlocker that is already the current vault; from `init` but before the unlocker is written, a vault with no unlocker,
stopped there, the default vault with no unlocker; which `vault create` has already made the current vault;
- from an unlocker add stopped before its metadata is written, a - from an unlocker add stopped before its metadata is written, a
directory that `unlocker list` warns about and `unlocker rm` directory that `unlocker list` warns about and `unlocker rm`
cannot remove; cannot remove;
+148
View File
@@ -0,0 +1,148 @@
package cli_test
import (
"testing"
"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/require"
)
// TestCreateExistingVaultChangesNothing is a regression test for
// https://git.eeqj.de/sneak/secret/issues/74, where running `secret init`
// a second time, or `secret vault create` with the name of an existing
// vault, replaced that vault's keys, so that none of its secrets could be
// decrypted any more. Each must refuse, change nothing, and leave every
// vault's secret readable through its passphrase unlocker.
//
//nolint:paralleltest // t.Setenv forbids parallel subtests
func TestCreateExistingVaultChangesNothing(t *testing.T) {
t.Setenv(secret.EnvMnemonic, testMnemonic)
t.Setenv(secret.EnvUnlockPassphrase, testPassphrase)
// `secret init`, `secret vault create work`, `secret vault select
// default`, and the secret "x" in each vault. "work" is then not the
// current vault, which creating it again must not change.
fs := afero.NewMemMapFs()
c := cli.NewCLIInstanceWithStateDir(fs, testStateDir)
cmd := &cobra.Command{}
require.NoError(t, c.Init(cmd))
require.NoError(t, c.CreateVault(cmd, "work"))
require.NoError(t, c.SelectVault(cmd, "default"))
vaults, err := vault.ListVaults(fs, testStateDir)
require.NoError(t, err)
require.Len(t, vaults, 2)
for _, name := range vaults {
value := memguard.NewBufferFromBytes([]byte("value"))
err := vault.NewVault(fs, testStateDir, name).AddSecret("x", value, false)
require.NoError(t, err)
}
before := snapshotStateDir(t, fs)
tests := []struct {
command string
want string
run func(c *cli.Instance) error
}{
{
"init",
"failed to create default vault: vault default already exists",
func(c *cli.Instance) error { return c.Init(cmd) },
},
{
"vault create default",
"vault default already exists",
func(c *cli.Instance) error { return c.CreateVault(cmd, "default") },
},
{
"vault create work",
"vault work already exists",
func(c *cli.Instance) error { return c.CreateVault(cmd, "work") },
},
}
for _, tt := range tests {
t.Run(tt.command, func(t *testing.T) {
fs := newFsFromSnapshot(t, before)
err := tt.run(cli.NewCLIInstanceWithStateDir(fs, testStateDir))
require.EqualError(t, err, tt.want)
require.Equal(t, before, snapshotStateDir(t, fs))
})
}
// Every case left the state directory exactly as recorded in before, so
// reading each vault's secret once from it shows that it still decrypts
// after each case. Without the mnemonic, reading a secret goes through
// the vault's passphrase unlocker, which is slow.
t.Setenv(secret.EnvMnemonic, "")
for _, name := range vaults {
value, err := vault.NewVault(fs, testStateDir, name).GetSecret("x")
require.NoError(t, err)
require.Equal(t, "value", string(value))
}
}
// TestStopAtPassphrasePromptLeavesNothing is a regression test for the
// review of https://git.eeqj.de/sneak/secret/pulls/82: `secret init` or
// `secret vault create` stopped at the passphrase prompt left a vault with
// no unlocker, which neither command would then create again. Each must ask
// for the passphrase before writing anything.
//
//nolint:paralleltest // t.Setenv forbids parallel subtests
func TestStopAtPassphrasePromptLeavesNothing(t *testing.T) {
t.Setenv(secret.EnvMnemonic, testMnemonic)
// Without the passphrase in the environment, both commands prompt for
// it, which fails because the tests do not run in a terminal.
t.Setenv(secret.EnvUnlockPassphrase, "")
// An empty state directory for `secret init`, and one holding the vault
// "default" for `secret vault create work`.
empty := afero.NewMemMapFs()
require.NoError(t, empty.MkdirAll(testStateDir, secret.DirPerms))
withDefault := afero.NewMemMapFs()
_, err := vault.CreateVault(withDefault, testStateDir, "default")
require.NoError(t, err)
cmd := &cobra.Command{}
tests := []struct {
command string
fs afero.Fs
run func(c *cli.Instance) error
}{
{
"init",
empty,
func(c *cli.Instance) error { return c.Init(cmd) },
},
{
"vault create work",
withDefault,
func(c *cli.Instance) error { return c.CreateVault(cmd, "work") },
},
}
for _, tt := range tests {
t.Run(tt.command, func(t *testing.T) {
before := snapshotStateDir(t, tt.fs)
err := tt.run(cli.NewCLIInstanceWithStateDir(tt.fs, testStateDir))
require.ErrorContains(t, err, "failed to read passphrase")
require.Equal(t, before, snapshotStateDir(t, tt.fs))
})
}
}
+8 -7
View File
@@ -160,6 +160,14 @@ func (cli *Instance) initialize(cmd *cobra.Command) error {
errInvalidMnemonicPhrase) errInvalidMnemonicPhrase)
} }
// Ask for the unlocker passphrase before creating the vault, so that
// stopping at the prompt leaves no vault without an unlocker behind
passphraseBuffer, err := resolvePassphrase()
if err != nil {
return err
}
defer passphraseBuffer.Destroy()
// Set mnemonic in environment for CreateVault to use // Set mnemonic in environment for CreateVault to use
restoreMnemonicEnv := setMnemonicEnv(mnemonicStr) restoreMnemonicEnv := setMnemonicEnv(mnemonicStr)
defer restoreMnemonicEnv() defer restoreMnemonicEnv()
@@ -175,13 +183,6 @@ func (cli *Instance) initialize(cmd *cobra.Command) error {
// Unlock the vault with the derived long-term key // Unlock the vault with the derived long-term key
vlt.Unlock(ltIdentity) vlt.Unlock(ltIdentity)
// Prompt for passphrase for unlocker
passphraseBuffer, err := resolvePassphrase()
if err != nil {
return err
}
defer passphraseBuffer.Destroy()
// Create passphrase-protected unlocker // Create passphrase-protected unlocker
secret.Debug("Creating passphrase-protected unlocker") secret.Debug("Creating passphrase-protected unlocker")
+7 -6
View File
@@ -271,11 +271,12 @@ func stateDirModTimes(t *testing.T, fs afero.Fs) map[string]int64 {
} }
// setupEveryCommand makes what each command in // setupEveryCommand makes what each command in
// TestChangingCommandsWaitForLock needs: the current vault "default" with // TestChangingCommandsWaitForLock needs: the current vault "work" with two
// two versions of "test/secret", the vault "other" without a long-term key, // versions of "test/secret", the vault "other" without a long-term key, for
// for vault import, and the file testInput. If withUnlocker is set, it also // vault import, and the file testInput. There is no vault "default", which
// gives "default" a passphrase unlocker, which is slow. It returns the older // init creates. If withUnlocker is set, it also gives "work" a passphrase
// version and the unlocker's ID. // unlocker, which is slow. It returns the older version and the unlocker's
// ID.
func setupEveryCommand( func setupEveryCommand(
t *testing.T, fs afero.Fs, withUnlocker bool, t *testing.T, fs afero.Fs, withUnlocker bool,
) (string, string) { ) (string, string) {
@@ -288,7 +289,7 @@ func setupEveryCommand(
require.NoError(t, err) require.NoError(t, err)
require.NoError(t, fs.Remove(filepath.Join(otherDir, "pub.age"))) require.NoError(t, fs.Remove(filepath.Join(otherDir, "pub.age")))
vlt, err := vault.CreateVault(fs, testStateDir, "default") vlt, err := vault.CreateVault(fs, testStateDir, "work")
require.NoError(t, err) require.NoError(t, err)
addTestSecret(t, vlt, []byte("older"), false) addTestSecret(t, vlt, []byte("older"), false)
+8 -7
View File
@@ -309,6 +309,14 @@ func (cli *Instance) CreateVault(cmd *cobra.Command, name string) error {
return errInvalidMnemonicPhrase return errInvalidMnemonicPhrase
} }
// Ask for the unlocker passphrase before creating the vault, so that
// stopping at the prompt leaves no vault without an unlocker behind
passphraseBuffer, err := resolvePassphrase()
if err != nil {
return err
}
defer passphraseBuffer.Destroy()
// Set mnemonic in environment for CreateVault to use // Set mnemonic in environment for CreateVault to use
restoreMnemonicEnv := setMnemonicEnv(mnemonicStr) restoreMnemonicEnv := setMnemonicEnv(mnemonicStr)
defer restoreMnemonicEnv() defer restoreMnemonicEnv()
@@ -336,13 +344,6 @@ func (cli *Instance) CreateVault(cmd *cobra.Command, name string) error {
// Unlock the vault with the derived long-term key // Unlock the vault with the derived long-term key
vlt.Unlock(ltIdentity) vlt.Unlock(ltIdentity)
// Get or prompt for passphrase
passphraseBuffer, err := resolvePassphrase()
if err != nil {
return err
}
defer passphraseBuffer.Destroy()
// Create passphrase-protected unlocker // Create passphrase-protected unlocker
secret.Debug("Creating passphrase-protected unlocker") secret.Debug("Creating passphrase-protected unlocker")
+4
View File
@@ -26,6 +26,10 @@ var (
// as "vault <name> does not exist". // as "vault <name> does not exist".
ErrVaultNotFound = errors.New("does not exist") ErrVaultNotFound = errors.New("does not exist")
// ErrVaultExists indicates that a vault to be created already exists.
// Composed as "vault <name> already exists".
ErrVaultExists = errors.New("already exists")
// ErrNilValueBuffer indicates a nil value buffer was supplied. // ErrNilValueBuffer indicates a nil value buffer was supplied.
ErrNilValueBuffer = errors.New("value buffer is nil") ErrNilValueBuffer = errors.New("value buffer is nil")
+17 -3
View File
@@ -191,7 +191,11 @@ func processMnemonicForVault(
return derivationIndex, publicKeyHash, familyHash, nil return derivationIndex, publicKeyHash, familyHash, nil
} }
// CreateVault creates a new vault // CreateVault creates a new vault and selects it as the current vault. It
// refuses a vault that already exists before writing anything: creating it
// again would replace its keys, and its secrets could no longer be
// decrypted. The commands that call it hold the state directory lock, so no
// other command can create the vault between the check and the writes.
func CreateVault(fs afero.Fs, stateDir string, name string) (*Vault, error) { func CreateVault(fs afero.Fs, stateDir string, name string) (*Vault, error) {
secret.Debug("Creating new vault", "name", name, "state_dir", stateDir) secret.Debug("Creating new vault", "name", name, "state_dir", stateDir)
@@ -207,12 +211,22 @@ func CreateVault(fs afero.Fs, stateDir string, name string) (*Vault, error) {
secret.Debug("Vault name validation passed", "vault_name", name) secret.Debug("Vault name validation passed", "vault_name", name)
// Create vault directory structure
vaultDir := filepath.Join(stateDir, "vaults.d", name) vaultDir := filepath.Join(stateDir, "vaults.d", name)
exists, err := afero.DirExists(fs, vaultDir)
if err != nil {
return nil, fmt.Errorf("failed to check if vault exists: %w", err)
}
if exists {
return nil, fmt.Errorf("vault %s %w", name, ErrVaultExists)
}
// Create vault directory structure
secret.Debug("Creating vault directory structure", "vault_dir", vaultDir) secret.Debug("Creating vault directory structure", "vault_dir", vaultDir)
// Create main vault directory // Create main vault directory
err := fs.MkdirAll(vaultDir, secret.DirPerms) err = fs.MkdirAll(vaultDir, secret.DirPerms)
if err != nil { if err != nil {
return nil, fmt.Errorf("failed to create vault directory: %w", err) return nil, fmt.Errorf("failed to create vault directory: %w", err)
} }