Refuse to create a vault that already exists (closes #74)
check / check (push) Waiting to run
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. The lock tests set up the vault "work" instead of "default", which init now refuses to create again. Model: opus-5-5
This commit is contained in:
@@ -25,6 +25,15 @@ Bring the repo into policy compliance in one commit:
|
||||
|
||||
# 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. A
|
||||
vault left without an unlocker by an `init` or `vault create` stopped at
|
||||
the passphrase prompt is refused like any other.
|
||||
- 2026-10-03: `secret mv` rejects a move whose destination is the
|
||||
source (`mv --force x x`, `mv --force work:x work:`, or an empty
|
||||
destination, which defaults to the source name) before changing
|
||||
|
||||
@@ -0,0 +1,90 @@
|
||||
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.
|
||||
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))
|
||||
|
||||
// Without the mnemonic, reading a secret goes through the
|
||||
// vault's passphrase unlocker.
|
||||
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))
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -274,11 +274,12 @@ func stateDirModTimes(t *testing.T, fs afero.Fs) map[string]int64 {
|
||||
}
|
||||
|
||||
// setupEveryCommand makes what each command in
|
||||
// TestChangingCommandsWaitForLock needs: the current vault "default" with
|
||||
// two versions of "test/secret", the vault "other" without a long-term key,
|
||||
// for vault import, and the file testInput. If withUnlocker is set, it also
|
||||
// gives "default" a passphrase unlocker, which is slow. It returns the older
|
||||
// version and the unlocker's ID.
|
||||
// TestChangingCommandsWaitForLock needs: the current vault "work" with two
|
||||
// versions of "test/secret", the vault "other" without a long-term key, for
|
||||
// vault import, and the file testInput. There is no vault "default", which
|
||||
// init creates. If withUnlocker is set, it also gives "work" a passphrase
|
||||
// unlocker, which is slow. It returns the older version and the unlocker's
|
||||
// ID.
|
||||
func setupEveryCommand(
|
||||
t *testing.T, fs afero.Fs, withUnlocker bool,
|
||||
) (string, string) {
|
||||
@@ -291,7 +292,7 @@ func setupEveryCommand(
|
||||
require.NoError(t, err)
|
||||
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)
|
||||
|
||||
addTestSecret(t, vlt, []byte("older"), false)
|
||||
|
||||
@@ -26,6 +26,10 @@ var (
|
||||
// as "vault <name> 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 = errors.New("value buffer is nil")
|
||||
|
||||
|
||||
@@ -191,7 +191,11 @@ func processMnemonicForVault(
|
||||
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) {
|
||||
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)
|
||||
|
||||
// Create vault directory structure
|
||||
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)
|
||||
|
||||
// Create main vault directory
|
||||
err := fs.MkdirAll(vaultDir, secret.DirPerms)
|
||||
err = fs.MkdirAll(vaultDir, secret.DirPerms)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("failed to create vault directory: %w", err)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user