diff --git a/TODO.md b/TODO.md index b2bb723..86da1ea 100644 --- a/TODO.md +++ b/TODO.md @@ -18,6 +18,19 @@ https://git.eeqj.de/sneak/secret/milestone/12 # Completed Steps +- 2026-10-04: `secret init` and `secret vault create` create a vault whole or + not at all (https://git.eeqj.de/sneak/secret/issues/105). + `vault.CreateVault` now takes the unlocker passphrase too, writes the vault + directory with its metadata, long-term public key and passphrase unlocker, + `longterm.age` included, into a temporary directory, renames that into + `vaults.d` once it is complete, and only then makes the vault current. + Before, either command killed after the passphrase prompt but before the + unlocker was written left a vault with no unlocker, which `vault create` had + already made current and which neither command would create again. Killed + part-way now, it leaves no vault, and the next command that takes the lock + deletes the temporary directory; or, killed between the rename and making + the vault current, a complete vault that is not current, which + `secret vault select` makes current. - 2026-10-04: What a command killed part-way left under a `.tmp-` name (https://git.eeqj.de/sneak/secret/issues/75), the temporary directories of `secret.TempDirFor` and the temporary files of @@ -242,11 +255,7 @@ https://git.eeqj.de/sneak/secret/milestone/12 `current-unlocker` never go missing. New versions, new secrets and cross-vault copies are built in a temporary directory and renamed into place, and removals rename out of the way first, so a version - or secret is never half-added and never half-removed. An - interrupted command can still leave, from `init` or `vault create` - killed after the passphrase prompt but before the unlocker is - written, a vault with no unlocker, which `vault create` has already - made the current vault. + or secret is never half-added and never half-removed. - 2026-10-03: The checks run before changing a vault now stop with an error naming the path and cause when they cannot read what they inspect, instead of reading the failure as "nothing there": the diff --git a/internal/cli/confirm_test.go b/internal/cli/confirm_test.go index 8569e70..9385580 100644 --- a/internal/cli/confirm_test.go +++ b/internal/cli/confirm_test.go @@ -63,10 +63,10 @@ func newConfirmTestVaults( fs := &afero.MemMapFs{} mnemonic := testMnemonicBuffer(t) - _, err := vault.CreateVault(fs, testStateDir, "other", mnemonic) + _, err := vault.CreateVault(fs, testStateDir, "other", mnemonic, nil) require.NoError(t, err) - vlt, err := vault.CreateVault(fs, testStateDir, "work", mnemonic) + vlt, err := vault.CreateVault(fs, testStateDir, "work", mnemonic, nil) require.NoError(t, err) addTestSecret(t, vlt, []byte("older"), false) diff --git a/internal/cli/create_vault_test.go b/internal/cli/create_vault_test.go index b778851..2217ffb 100644 --- a/internal/cli/create_vault_test.go +++ b/internal/cli/create_vault_test.go @@ -2,7 +2,9 @@ package cli_test import ( "bytes" + "io" "os" + "strings" "testing" "git.eeqj.de/sneak/secret/internal/cli" @@ -155,7 +157,7 @@ func TestStopAtPassphrasePromptLeavesNothing(t *testing.T) { require.NoError(t, empty.MkdirAll(testStateDir, secret.DirPerms)) withDefault := afero.NewMemMapFs() - _, err := vault.CreateVault(withDefault, testStateDir, "default", mnemonic) + _, err := vault.CreateVault(withDefault, testStateDir, "default", mnemonic, nil) require.NoError(t, err) cmd := &cobra.Command{} @@ -193,3 +195,166 @@ func TestStopAtPassphrasePromptLeavesNothing(t *testing.T) { }) } } + +// TestStopDuringCreateLeavesWholeVaultOrNone is a regression test for +// https://git.eeqj.de/sneak/secret/issues/105: `secret init` or `secret vault +// create` killed after the passphrase prompt but before the unlocker was +// written left a vault with no unlocker, which neither command would then +// create again. After the prompt, each command changes the state directory +// only through vault.CreateVault. The test makes that call as the command +// does and records the state directory before each change it makes, and once +// after it returns: what a stop at that point leaves. Each must hold either +// no vault, and not name it current, or exactly the finished vault, which +// opens with the passphrase through its current unlocker. The command, run +// again on each as after a killed command, must then create the vault, or +// refuse the one there, and delete what the stop left under a temporary name. +// +//nolint:paralleltest // commands on the in-memory filesystem share one lock +func TestStopDuringCreateLeavesWholeVaultOrNone(t *testing.T) { + mnemonic := testMnemonicBuffer(t) + passphrase := memguard.NewBufferFromBytes([]byte(testPassphrase)) + t.Cleanup(passphrase.Destroy) + + cmd := &cobra.Command{} + cmd.SetOut(io.Discard) + + tests := []struct { + command string + name string + exists string + run func(c *cli.Instance) error + }{ + { + "init", "default", + "failed to create default vault: vault default already exists", + func(c *cli.Instance) error { return c.Init(cmd) }, + }, + { + "vault create work", "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) { + // `secret init` starts from an empty state directory, `secret + // vault create work` from one holding the vault "default" + fs := afero.NewMemMapFs() + require.NoError(t, fs.MkdirAll(testStateDir, secret.DirPerms)) + + if tt.name != "default" { + _, err := vault.CreateVault(fs, testStateDir, "default", mnemonic, nil) + require.NoError(t, err) + } + + var stops []map[string]string + + record := func() { stops = append(stops, snapshotStateDir(t, fs)) } + + _, err := vault.CreateVault(hookFs{Fs: fs, before: record}, + testStateDir, tt.name, mnemonic, passphrase) + require.NoError(t, err) + record() + + vaultDir := testStateDir + "/vaults.d/" + tt.name + finished := entriesUnder(stops[len(stops)-1], vaultDir) + + opener := vault.NewVault(fs, testStateDir, tt.name) + opener.UnlockPassphrase = passphrase + + key, err := opener.UnlockVault() + require.NoError(t, err) + require.Equal(t, finished[vaultDir+"/pub.age"], key.Recipient().String()) + + for i, stop := range stops { + stopped := newFsFromSnapshot(t, stop) + c := cli.NewCLIInstanceWithStateDir(stopped, testStateDir) + c.Mnemonic = mnemonic + c.UnlockPassphrase = passphrase + + if _, there := stop[vaultDir+"/"]; there { + require.Equal(t, finished, entriesUnder(stop, vaultDir), + "stop %d left a partial vault", i) + require.EqualError(t, tt.run(c), tt.exists, "stop %d", i) + } else { + require.NotEqual(t, tt.name, stop[testStateDir+"/currentvault"], + "stop %d made a missing vault current", i) + require.NoError(t, tt.run(c), "stop %d", i) + } + + for path := range snapshotStateDir(t, stopped) { + require.NotContains(t, path, ".tmp-", "stop %d", i) + } + } + }) + } +} + +// entriesUnder returns the entries of a tree recorded by snapshotStateDir +// that are under dir. +func entriesUnder(tree map[string]string, dir string) map[string]string { + entries := map[string]string{} + + for path, content := range tree { + if strings.HasPrefix(path, dir+"/") { + entries[path] = content + } + } + + return entries +} + +// hookFs passes every call through to Fs, but first calls before for each +// call that can change the filesystem. +type hookFs struct { + afero.Fs + + before func() +} + +//nolint:ireturn // implements afero.Fs +func (h hookFs) Create(name string) (afero.File, error) { + h.before() + + return h.Fs.Create(name) +} + +//nolint:ireturn // implements afero.Fs +func (h hookFs) OpenFile( + name string, flag int, perm os.FileMode, +) (afero.File, error) { + h.before() + + return h.Fs.OpenFile(name, flag, perm) +} + +func (h hookFs) Mkdir(name string, perm os.FileMode) error { + h.before() + + return h.Fs.Mkdir(name, perm) +} + +func (h hookFs) MkdirAll(path string, perm os.FileMode) error { + h.before() + + return h.Fs.MkdirAll(path, perm) +} + +func (h hookFs) Remove(name string) error { + h.before() + + return h.Fs.Remove(name) +} + +func (h hookFs) RemoveAll(path string) error { + h.before() + + return h.Fs.RemoveAll(path) +} + +func (h hookFs) Rename(oldname, newname string) error { + h.before() + + return h.Fs.Rename(oldname, newname) +} diff --git a/internal/cli/init.go b/internal/cli/init.go index a451b37..1b9142e 100644 --- a/internal/cli/init.go +++ b/internal/cli/init.go @@ -6,13 +6,10 @@ import ( "log" "log/slog" "os" - "path/filepath" "strings" - "filippo.io/age" "git.eeqj.de/sneak/secret/internal/secret" "git.eeqj.de/sneak/secret/internal/vault" - "git.eeqj.de/sneak/secret/pkg/agehd" "github.com/awnumar/memguard" "github.com/spf13/cobra" "github.com/tyler-smith/go-bip39" @@ -70,43 +67,6 @@ func (cli *Instance) promptMnemonic() (*memguard.LockedBuffer, func(), error) { return mnemonicBuffer, mnemonicBuffer.Destroy, nil } -// setupDefaultVault creates the default vault and derives its long-term -// identity from the mnemonic -func (cli *Instance) setupDefaultVault( - stateDir string, mnemonic *memguard.LockedBuffer, -) (*vault.Vault, *age.X25519Identity, error) { - // Create the default vault - it will handle key derivation internally - secret.Debug("Creating default vault") - - vlt, err := vault.CreateVault(cli.fs, cli.stateDir, "default", mnemonic) - if err != nil { - secret.Debug("Failed to create default vault", "error", err) - - return nil, nil, fmt.Errorf("failed to create default vault: %w", err) - } - - // Get the vault metadata to retrieve the derivation index - vaultDir := filepath.Join(stateDir, "vaults.d", "default") - - metadata, err := vault.LoadVaultMetadata(cli.fs, vaultDir) - if err != nil { - secret.Debug("Failed to load vault metadata", "error", err) - - return nil, nil, fmt.Errorf("failed to load vault metadata: %w", err) - } - - // Derive the long-term key using the same index that CreateVault used - ltIdentity, err := agehd.DeriveIdentity(mnemonic.String(), metadata.DerivationIndex) - if err != nil { - secret.Debug("Failed to derive long-term key", "error", err) - - return nil, nil, fmt.Errorf( - "failed to derive long-term key from mnemonic: %w", err) - } - - return vlt, ltIdentity, nil -} - // Init initializes the secret manager, holding the state directory lock // while initialize runs func (cli *Instance) Init(cmd *cobra.Command) error { @@ -173,34 +133,31 @@ func (cli *Instance) initialize(cmd *cobra.Command) error { } defer cleanupPassphrase() - // Create the default vault and derive its long-term key - vlt, ltIdentity, err := cli.setupDefaultVault(stateDir, mnemonic) + // Create the default vault with its passphrase unlocker + secret.Debug("Creating default vault") + + vlt, err := vault.CreateVault(cli.fs, cli.stateDir, "default", + mnemonic, passphraseBuffer) + if err != nil { + secret.Debug("Failed to create default vault", "error", err) + + return fmt.Errorf("failed to create default vault: %w", err) + } + + ltIdentity, err := vlt.GetOrDeriveLongTermKey() + if err != nil { + return fmt.Errorf("failed to get long-term key: %w", err) + } + + unlocker, err := vlt.GetCurrentUnlocker() if err != nil { return err } - ltPubKey := ltIdentity.Recipient().String() - - // Unlock the vault with the derived long-term key - vlt.Unlock(ltIdentity) - - // Create passphrase-protected unlocker - secret.Debug("Creating passphrase-protected unlocker") - - passphraseUnlocker, err := vlt.CreatePassphraseUnlocker(passphraseBuffer) - if err != nil { - secret.Debug("Failed to create unlocker", "error", err) - - return fmt.Errorf("failed to create unlocker: %w", err) - } - - // Note: CreatePassphraseUnlocker already encrypts and writes the long-term - // private key to longterm.age, so no need to do it again here. - if cmd != nil { cmd.Printf("\nDefault vault created and configured\n") - cmd.Printf("Long-term public key: %s\n", ltPubKey) - cmd.Printf("Unlocker ID: %s\n", passphraseUnlocker.GetID()) + cmd.Printf("Long-term public key: %s\n", ltIdentity.Recipient().String()) + cmd.Printf("Unlocker ID: %s\n", unlocker.GetID()) cmd.Println("\nYour secret manager is ready to use!") cmd.Println("Note: When using SB_SECRET_MNEMONIC environment variable,") cmd.Println("unlockers are not required for secret operations.") diff --git a/internal/cli/integration_test.go b/internal/cli/integration_test.go index a77ae0e..d6a54e9 100644 --- a/internal/cli/integration_test.go +++ b/internal/cli/integration_test.go @@ -2563,7 +2563,7 @@ func secretRmCommand(ctx context.Context, t *testing.T) (*exec.Cmd, string) { mnemonic := memguard.NewBufferFromBytes([]byte(testMnemonic)) defer mnemonic.Destroy() - vlt, err := vault.CreateVault(afero.NewOsFs(), stateDir, "default", mnemonic) + vlt, err := vault.CreateVault(afero.NewOsFs(), stateDir, "default", mnemonic, nil) require.NoError(t, err) value := memguard.NewBufferFromBytes([]byte("value")) diff --git a/internal/cli/leftovers_test.go b/internal/cli/leftovers_test.go index 04d5a1e..6c044d3 100644 --- a/internal/cli/leftovers_test.go +++ b/internal/cli/leftovers_test.go @@ -28,7 +28,7 @@ func TestLeftoversRemovedByNextChangingCommand(t *testing.T) { fs := newTwoVaultFs(t) - _, err := vault.CreateVault(fs, testStateDir, ".tmp-0", nil) + _, err := vault.CreateVault(fs, testStateDir, ".tmp-0", nil, nil) require.NoError(t, err) require.NoError(t, vault.SelectVault(fs, testStateDir, "default")) diff --git a/internal/cli/lock_test.go b/internal/cli/lock_test.go index abe7b8b..5919d9c 100644 --- a/internal/cli/lock_test.go +++ b/internal/cli/lock_test.go @@ -110,7 +110,7 @@ func TestConcurrentAddsKeepEveryVersion(t *testing.T) { {"real", afero.NewOsFs(), t.TempDir()}, } { t.Run(tc.name, func(t *testing.T) { - _, err := vault.CreateVault(tc.fs, tc.stateDir, "default", mnemonic) + _, err := vault.CreateVault(tc.fs, tc.stateDir, "default", mnemonic, nil) require.NoError(t, err) // One add creates the secret; the others find that it exists @@ -185,7 +185,7 @@ func (r *readNotifier) Read(p []byte) (int, error) { //nolint:paralleltest // times commands against the in-memory lock all tests share func TestEncryptPipedIntoAdd(t *testing.T) { fs := afero.NewMemMapFs() - _, err := vault.CreateVault(fs, testStateDir, "default", testMnemonicBuffer(t)) + _, err := vault.CreateVault(fs, testStateDir, "default", testMnemonicBuffer(t), nil) require.NoError(t, err) require.NoError(t, afero.WriteFile(fs, testInput, []byte("piped"), 0o600)) @@ -292,14 +292,14 @@ func setupEveryCommand( mnemonic := testMnemonicBuffer(t) - other, err := vault.CreateVault(fs, testStateDir, "other", mnemonic) + other, err := vault.CreateVault(fs, testStateDir, "other", mnemonic, nil) require.NoError(t, err) otherDir, err := other.GetDirectory() require.NoError(t, err) require.NoError(t, fs.Remove(filepath.Join(otherDir, "pub.age"))) - vlt, err := vault.CreateVault(fs, testStateDir, "work", mnemonic) + vlt, err := vault.CreateVault(fs, testStateDir, "work", mnemonic, nil) require.NoError(t, err) addTestSecret(t, vlt, []byte("older"), false) @@ -487,7 +487,7 @@ func TestEncryptWithExistingKeyTakesNoLock(t *testing.T) { mnemonic := testMnemonicBuffer(t) fs := afero.NewMemMapFs() - _, err := vault.CreateVault(fs, testStateDir, "default", mnemonic) + _, err := vault.CreateVault(fs, testStateDir, "default", mnemonic, nil) require.NoError(t, err) require.NoError(t, afero.WriteFile(fs, testInput, []byte("input"), 0o600)) @@ -525,7 +525,7 @@ func TestEncryptWithExistingKeyTakesNoLock(t *testing.T) { //nolint:paralleltest // times commands against the in-memory lock all tests share func TestEncryptStreamsUnlocked(t *testing.T) { fs := afero.NewMemMapFs() - _, err := vault.CreateVault(fs, testStateDir, "default", testMnemonicBuffer(t)) + _, err := vault.CreateVault(fs, testStateDir, "default", testMnemonicBuffer(t), nil) require.NoError(t, err) require.NoError(t, afero.WriteFile(fs, testInput, []byte("streamed"), 0o600)) diff --git a/internal/cli/move_test.go b/internal/cli/move_test.go index d77ca29..c1169f1 100644 --- a/internal/cli/move_test.go +++ b/internal/cli/move_test.go @@ -156,10 +156,12 @@ func TestMoveOntoSameSecretUnderAnotherNameIsRejected(t *testing.T) { vaultsDir := filepath.Join(stateDir, "vaults.d") // "default" is created last, so it is the current vault. - _, err := vault.CreateVault(fs, stateDir, "other", testMnemonicBuffer(t)) + _, err := vault.CreateVault(fs, stateDir, "other", + testMnemonicBuffer(t), nil) require.NoError(t, err) - vlt, err := vault.CreateVault(fs, stateDir, "default", testMnemonicBuffer(t)) + vlt, err := vault.CreateVault(fs, stateDir, "default", + testMnemonicBuffer(t), nil) require.NoError(t, err) err = vlt.AddSecret("x", memguard.NewBufferFromBytes([]byte("value")), false) @@ -205,7 +207,7 @@ func TestForcedCaseOnlyMoveOnCaseSensitiveFilesystem(t *testing.T) { fs := afero.NewOsFs() stateDir := t.TempDir() - vlt, err := vault.CreateVault(fs, stateDir, "default", testMnemonicBuffer(t)) + vlt, err := vault.CreateVault(fs, stateDir, "default", testMnemonicBuffer(t), nil) require.NoError(t, err) err = vlt.AddSecret("Foo", memguard.NewBufferFromBytes([]byte("upper")), false) diff --git a/internal/cli/path_traversal_test.go b/internal/cli/path_traversal_test.go index 090ff10..a7d472a 100644 --- a/internal/cli/path_traversal_test.go +++ b/internal/cli/path_traversal_test.go @@ -68,7 +68,7 @@ func newTwoVaultFs(t *testing.T) afero.Fs { mnemonic := testMnemonicBuffer(t) for _, name := range []string{"work", "default"} { - vlt, err := vault.CreateVault(fs, testStateDir, name, mnemonic) + vlt, err := vault.CreateVault(fs, testStateDir, name, mnemonic, nil) require.NoError(t, err) err = vlt.AddSecret("x", memguard.NewBufferFromBytes([]byte("value")), false) diff --git a/internal/cli/secrets_size_test.go b/internal/cli/secrets_size_test.go index 2c7b345..3df84b8 100644 --- a/internal/cli/secrets_size_test.go +++ b/internal/cli/secrets_size_test.go @@ -71,7 +71,8 @@ func newSizeTestVault(t *testing.T) (afero.Fs, *vault.Vault) { fs := afero.NewMemMapFs() // Create vault - _, err := vault.CreateVault(fs, testStateDir, testVaultName, testMnemonicBuffer(t)) + _, err := vault.CreateVault(fs, testStateDir, testVaultName, + testMnemonicBuffer(t), nil) require.NoError(t, err) // Set current vault diff --git a/internal/cli/unlockers_add_test.go b/internal/cli/unlockers_add_test.go index 66f5c9e..da070d6 100644 --- a/internal/cli/unlockers_add_test.go +++ b/internal/cli/unlockers_add_test.go @@ -47,7 +47,7 @@ func TestAddPGPUnlocker(t *testing.T) { t.Run(test.name, func(t *testing.T) { fs := afero.NewMemMapFs() vlt, err := vault.CreateVault(fs, listTestStateDir, listTestVaultName, - testMnemonicBuffer(t)) + testMnemonicBuffer(t), nil) require.NoError(t, err) err = vlt.AddSecret(addTestSecretName, diff --git a/internal/cli/vault.go b/internal/cli/vault.go index 4cd699c..e3a45be 100644 --- a/internal/cli/vault.go +++ b/internal/cli/vault.go @@ -293,40 +293,26 @@ func (cli *Instance) CreateVault(cmd *cobra.Command, name string) error { } defer cleanupPassphrase() - // Create the vault - it will handle key derivation internally - vlt, err := vault.CreateVault(cli.fs, cli.stateDir, name, mnemonic) + // Create the vault with its passphrase unlocker + vlt, err := vault.CreateVault(cli.fs, cli.stateDir, name, + mnemonic, passphraseBuffer) if err != nil { return err } - // Get the vault metadata to retrieve the derivation index - vaultDir := filepath.Join(cli.stateDir, "vaults.d", name) - - metadata, err := vault.LoadVaultMetadata(cli.fs, vaultDir) + ltIdentity, err := vlt.GetOrDeriveLongTermKey() if err != nil { - return fmt.Errorf("failed to load vault metadata: %w", err) + return fmt.Errorf("failed to get long-term key: %w", err) } - // Derive the long-term key using the same index that CreateVault used - ltIdentity, err := agehd.DeriveIdentity(mnemonicStr, metadata.DerivationIndex) + unlocker, err := vlt.GetCurrentUnlocker() if err != nil { - return fmt.Errorf("failed to derive long-term key from mnemonic: %w", err) - } - - // Unlock the vault with the derived long-term key - vlt.Unlock(ltIdentity) - - // Create passphrase-protected unlocker - secret.Debug("Creating passphrase-protected unlocker") - - passphraseUnlocker, err := vlt.CreatePassphraseUnlocker(passphraseBuffer) - if err != nil { - return fmt.Errorf("failed to create unlocker: %w", err) + return err } cmd.Printf("Created vault '%s'\n", vlt.GetName()) cmd.Printf("Long-term public key: %s\n", ltIdentity.Recipient().String()) - cmd.Printf("Unlocker ID: %s\n", passphraseUnlocker.GetID()) + cmd.Printf("Unlocker ID: %s\n", unlocker.GetID()) return nil } diff --git a/internal/cli/version_test.go b/internal/cli/version_test.go index b71cdec..2e81e60 100644 --- a/internal/cli/version_test.go +++ b/internal/cli/version_test.go @@ -73,7 +73,8 @@ func setupTestVault(t *testing.T, fs afero.Fs) { t.Helper() // Create vault - vlt, err := vault.CreateVault(fs, testStateDir, "default", testMnemonicBuffer(t)) + vlt, err := vault.CreateVault(fs, testStateDir, "default", + testMnemonicBuffer(t), nil) require.NoError(t, err) // Derive and store long-term key from mnemonic diff --git a/internal/secret/atomic_test.go b/internal/secret/atomic_test.go index cf9792c..f9b107b 100644 --- a/internal/secret/atomic_test.go +++ b/internal/secret/atomic_test.go @@ -235,7 +235,7 @@ func newVaultWithSecret( ) *vault.Vault { t.Helper() - vlt, err := vault.CreateVault(fs, stateDir, name, testMnemonicBuffer(t)) + vlt, err := vault.CreateVault(fs, stateDir, name, testMnemonicBuffer(t), nil) require.NoError(t, err) buffer := memguard.NewBufferFromBytes([]byte(value)) @@ -352,7 +352,7 @@ func TestLongestNames(t *testing.T) { fs := afero.NewOsFs() name := strings.Repeat("a", longestName) - vlt, err := vault.CreateVault(fs, t.TempDir(), name, testMnemonicBuffer(t)) + vlt, err := vault.CreateVault(fs, t.TempDir(), name, testMnemonicBuffer(t), nil) require.NoError(t, err) value := memguard.NewBufferFromBytes([]byte("long")) @@ -646,7 +646,7 @@ func TestPassphraseUnlockerGetsKeyFirst(t *testing.T) { // No mnemonic, and no current unlocker to get the key from base := afero.NewMemMapFs() - _, err := vault.CreateVault(base, testVaultStateDir, testVaultName, nil) + _, err := vault.CreateVault(base, testVaultStateDir, testVaultName, nil, nil) require.NoError(t, err) fs := hookFs{Fs: base, before: func(_, path string) error { @@ -678,7 +678,7 @@ func TestPassphraseUnlockerIsWholeOrAbsent(t *testing.T) { base, stateDir := tfs.open(t) vlt, err := vault.CreateVault(base, stateDir, testVaultName, - testMnemonicBuffer(t)) + testMnemonicBuffer(t), nil) require.NoError(t, err) vaultDir, err := vlt.GetDirectory() @@ -727,7 +727,7 @@ func TestPassphraseUnlockerReplacementKeepsVaultOpen(t *testing.T) { base, stateDir := tfs.open(t) vlt, err := vault.CreateVault(base, stateDir, testVaultName, - testMnemonicBuffer(t)) + testMnemonicBuffer(t), nil) require.NoError(t, err) ltIdentity, err := vlt.GetOrDeriveLongTermKey() diff --git a/internal/secret/pgpunlock_test.go b/internal/secret/pgpunlock_test.go index 164948a..064a174 100644 --- a/internal/secret/pgpunlock_test.go +++ b/internal/secret/pgpunlock_test.go @@ -330,7 +330,7 @@ func testCreatePGPUnlocker( mnemonic := testMnemonicBuffer(t) // Create a test vault directory structure - vlt, err := vault.CreateVault(fs, stateDir, vaultName, mnemonic) + vlt, err := vault.CreateVault(fs, stateDir, vaultName, mnemonic, nil) if err != nil { t.Fatalf("Failed to create vault: %v", err) } diff --git a/internal/secret/pgpunlocker_test.go b/internal/secret/pgpunlocker_test.go index fcdf67e..e8c8a11 100644 --- a/internal/secret/pgpunlocker_test.go +++ b/internal/secret/pgpunlocker_test.go @@ -48,7 +48,7 @@ func TestCreatePGPUnlockerFailureWritesNothing(t *testing.T) { installFakeGPG(t) base := afero.NewMemMapFs() - vlt, err := vault.CreateVault(base, testVaultStateDir, testVaultName, nil) + vlt, err := vault.CreateVault(base, testVaultStateDir, testVaultName, nil, nil) require.NoError(t, err) fs := hookFs{Fs: base, before: func(_, path string) error { @@ -87,7 +87,7 @@ func TestPGPUnlockerAddedTwiceKeepsFirst(t *testing.T) { fs := afero.NewMemMapFs() mnemonic := testMnemonicBuffer(t) - _, err := vault.CreateVault(fs, testVaultStateDir, testVaultName, mnemonic) + _, err := vault.CreateVault(fs, testVaultStateDir, testVaultName, mnemonic, nil) require.NoError(t, err) first, err := secret.CreatePGPUnlocker( diff --git a/internal/vault/errors.go b/internal/vault/errors.go index a4f0e27..32805d1 100644 --- a/internal/vault/errors.go +++ b/internal/vault/errors.go @@ -31,6 +31,11 @@ var ( // Composed as "vault already exists". ErrVaultExists = errors.New("already exists") + // ErrUnlockerWithoutMnemonic indicates that CreateVault was given a + // passphrase for an unlocker but no mnemonic to derive the long-term key + // it unlocks. Composed as "vault needs a mnemonic for an unlocker". + ErrUnlockerWithoutMnemonic = errors.New("needs a mnemonic for an unlocker") + // ErrNilValueBuffer indicates a nil value buffer was supplied. ErrNilValueBuffer = errors.New("value buffer is nil") diff --git a/internal/vault/integration_test.go b/internal/vault/integration_test.go index b8bea4a..4c28f0c 100644 --- a/internal/vault/integration_test.go +++ b/internal/vault/integration_test.go @@ -99,7 +99,7 @@ func testCurrentVaultFileHandling(t *testing.T, fs afero.Fs, tempDir string) { // Create a test vault vlt, err := vault.CreateVault(fs, stateDir, testVaultName, - testMnemonicBuffer(t)) + testMnemonicBuffer(t), nil) if err != nil { t.Fatalf("Failed to create vault: %v", err) } @@ -147,7 +147,7 @@ func testDeepPathSecrets(t *testing.T, fs afero.Fs, tempDir string) { // Create a test vault - CreateVault writes the public key derived from // the mnemonic vlt, err := vault.CreateVault(fs, stateDir, testVaultName, - testMnemonicBuffer(t)) + testMnemonicBuffer(t), nil) if err != nil { t.Fatalf("Failed to create vault: %v", err) } @@ -223,7 +223,7 @@ func testKeyCaching(t *testing.T, fs afero.Fs, tempDir string) { // Create a test vault - CreateVault writes the public key derived from // the mnemonic vlt, err := vault.CreateVault(fs, stateDir, testVaultName, - testMnemonicBuffer(t)) + testMnemonicBuffer(t), nil) if err != nil { t.Fatalf("Failed to create vault: %v", err) } @@ -324,7 +324,7 @@ func testVaultNameValidation(t *testing.T, fs afero.Fs, tempDir string) { } for _, name := range validNames { - _, err := vault.CreateVault(fs, stateDir, name, testMnemonicBuffer(t)) + _, err := vault.CreateVault(fs, stateDir, name, testMnemonicBuffer(t), nil) if err != nil { t.Errorf("Failed to create vault with valid name %q: %v", name, err) } @@ -340,7 +340,7 @@ func testVaultNameValidation(t *testing.T, fs afero.Fs, tempDir string) { } for _, name := range invalidNames { - _, err := vault.CreateVault(fs, stateDir, name, testMnemonicBuffer(t)) + _, err := vault.CreateVault(fs, stateDir, name, testMnemonicBuffer(t), nil) if err == nil { t.Errorf("Expected error creating vault with invalid name %q, "+ "but got none", name) @@ -361,7 +361,7 @@ func testMultipleVaults(t *testing.T, fs afero.Fs, tempDir string) { // Create three vaults vaultNames := []string{"vault1", "vault2", "vault3"} for _, name := range vaultNames { - _, err := vault.CreateVault(fs, stateDir, name, testMnemonicBuffer(t)) + _, err := vault.CreateVault(fs, stateDir, name, testMnemonicBuffer(t), nil) if err != nil { t.Fatalf("Failed to create vault %s: %v", name, err) } @@ -411,12 +411,12 @@ func testVaultIsolation(t *testing.T, fs afero.Fs, tempDir string) { // Create two vaults - CreateVault writes the public key derived from // the mnemonic - vault1, err := vault.CreateVault(fs, stateDir, "vault1", testMnemonicBuffer(t)) + vault1, err := vault.CreateVault(fs, stateDir, "vault1", testMnemonicBuffer(t), nil) if err != nil { t.Fatalf("Failed to create vault1: %v", err) } - vault2, err := vault.CreateVault(fs, stateDir, "vault2", testMnemonicBuffer(t)) + vault2, err := vault.CreateVault(fs, stateDir, "vault2", testMnemonicBuffer(t), nil) if err != nil { t.Fatalf("Failed to create vault2: %v", err) } diff --git a/internal/vault/integration_version_test.go b/internal/vault/integration_version_test.go index 9330c97..00e2b8b 100644 --- a/internal/vault/integration_version_test.go +++ b/internal/vault/integration_version_test.go @@ -49,7 +49,7 @@ func TestVersionIntegrationWorkflow(t *testing.T) { fs := afero.NewMemMapFs() // Create vault without a long-term key, which is set up below - vault, err := CreateVault(fs, testStateDir, "test", nil) + vault, err := CreateVault(fs, testStateDir, "test", nil, nil) require.NoError(t, err) // Derive and store long-term key from mnemonic diff --git a/internal/vault/management.go b/internal/vault/management.go index c1ab9af..f909374 100644 --- a/internal/vault/management.go +++ b/internal/vault/management.go @@ -8,6 +8,7 @@ import ( "strings" "time" + "filippo.io/age" "git.eeqj.de/sneak/secret/internal/secret" "git.eeqj.de/sneak/secret/pkg/agehd" "github.com/awnumar/memguard" @@ -152,16 +153,17 @@ func ListVaults(fs afero.Fs, stateDir string) ([]string, error) { } // processMnemonicForVault handles mnemonic processing for vault creation. -// It returns the derivation index, public key hash, and family hash. +// It returns the long-term key, nil when there is no mnemonic, and the +// derivation index, public key hash, and family hash. func processMnemonicForVault( fs afero.Fs, stateDir, vaultDir, vaultName string, mnemonicBuffer *memguard.LockedBuffer, -) (uint32, string, string, error) { +) (*age.X25519Identity, uint32, string, string, error) { if mnemonicBuffer == nil { secret.Debug("No mnemonic given, vault created without long-term key", "vault", vaultName) // Use 0 for derivation index when no mnemonic is provided - return 0, "", "", nil + return nil, 0, "", "", nil } mnemonic := mnemonicBuffer.String() @@ -171,13 +173,14 @@ func processMnemonicForVault( // Get the next available derivation index for this mnemonic derivationIndex, err := GetNextDerivationIndex(fs, stateDir, mnemonic) if err != nil { - return 0, "", "", fmt.Errorf("failed to get next derivation index: %w", err) + return nil, 0, "", "", + fmt.Errorf("failed to get next derivation index: %w", err) } // Derive the long-term key using the actual derivation index ltIdentity, err := agehd.DeriveIdentity(mnemonic, derivationIndex) if err != nil { - return 0, "", "", fmt.Errorf("failed to derive long-term key: %w", err) + return nil, 0, "", "", fmt.Errorf("failed to derive long-term key: %w", err) } // Write the public key @@ -187,7 +190,8 @@ func processMnemonicForVault( err = secret.WriteFileAtomic(fs, ltPubKeyPath, []byte(ltPubKey)) if err != nil { - return 0, "", "", fmt.Errorf("failed to write long-term public key: %w", err) + return nil, 0, "", "", + fmt.Errorf("failed to write long-term public key: %w", err) } secret.Debug("Wrote long-term public key", "path", ltPubKeyPath) @@ -199,24 +203,33 @@ func processMnemonicForVault( // This is used to identify which vaults belong to the same mnemonic family identity0, err := agehd.DeriveIdentity(mnemonic, 0) if err != nil { - return 0, "", "", fmt.Errorf("failed to derive identity for index 0: %w", err) + return nil, 0, "", "", + fmt.Errorf("failed to derive identity for index 0: %w", err) } familyHash := ComputeDoubleSHA256([]byte(identity0.Recipient().String())) - return derivationIndex, publicKeyHash, familyHash, nil + return ltIdentity, derivationIndex, publicKeyHash, familyHash, nil } // CreateVault creates a new vault and selects it as the current vault. When // mnemonic is not nil, the vault's long-term key is derived from it, and the // returned vault has it as its Mnemonic; when it is nil, the vault has no -// long-term key until one is imported. 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. +// long-term key until one is imported. When passphrase is not nil, the vault +// gets a passphrase unlocker protected by it, as its current unlocker; that +// needs a mnemonic. 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. +// +// The vault is written whole into a temporary directory, which is renamed +// into vaults.d only once complete, and only then selected: a crash at any +// point leaves either no vault or a complete one, and what it leaves under +// a temporary name the next command that takes the lock deletes. func CreateVault( - fs afero.Fs, stateDir string, name string, mnemonic *memguard.LockedBuffer, + fs afero.Fs, stateDir string, name string, + mnemonic, passphrase *memguard.LockedBuffer, ) (*Vault, error) { secret.Debug("Creating new vault", "name", name, "state_dir", stateDir) @@ -240,51 +253,19 @@ func CreateVault( return nil, fmt.Errorf("vault %s %w", name, ErrVaultExists) } - // Create vault directory structure + if passphrase != nil && mnemonic == nil { + return nil, fmt.Errorf("vault %s %w", name, ErrUnlockerWithoutMnemonic) + } + secret.Debug("Creating vault directory structure", "vault_dir", vaultDir) - // Create main vault directory - err = fs.MkdirAll(vaultDir, secret.DirPerms) - if err != nil { - return nil, fmt.Errorf("failed to create vault directory: %w", err) - } - - // Create secrets directory - secretsDir := filepath.Join(vaultDir, "secrets.d") - - err = fs.MkdirAll(secretsDir, secret.DirPerms) - if err != nil { - return nil, fmt.Errorf("failed to create secrets directory: %w", err) - } - - // Create unlockers directory - unlockersDir := filepath.Join(vaultDir, "unlockers.d") - - err = fs.MkdirAll(unlockersDir, secret.DirPerms) - if err != nil { - return nil, fmt.Errorf("failed to create unlockers directory: %w", err) - } - - // Process mnemonic if available - derivationIndex, publicKeyHash, familyHash, err := processMnemonicForVault( - fs, stateDir, vaultDir, name, mnemonic) + err = secret.WriteDir(fs, vaultDir, func(dir string) error { + return writeVaultFiles(fs, stateDir, dir, name, mnemonic, passphrase) + }) if err != nil { return nil, err } - // Save vault metadata - metadata := &Metadata{ - CreatedAt: time.Now(), - DerivationIndex: derivationIndex, - PublicKeyHash: publicKeyHash, - MnemonicFamilyHash: familyHash, - } - - err = SaveVaultMetadata(fs, vaultDir, metadata) - if err != nil { - return nil, fmt.Errorf("failed to save vault metadata: %w", err) - } - // Select the newly created vault as current secret.Debug("Selecting newly created vault as current", "name", name) @@ -302,6 +283,47 @@ func CreateVault( return vlt, nil } +// writeVaultFiles writes the files of the new vault name into vaultDir: its +// secrets and unlockers directories, its long-term public key and metadata, +// and, when passphrase is not nil, a passphrase unlocker as its current one. +func writeVaultFiles( + fs afero.Fs, stateDir, vaultDir, name string, + mnemonic, passphrase *memguard.LockedBuffer, +) error { + for _, subdir := range []string{"secrets.d", "unlockers.d"} { + err := fs.MkdirAll(filepath.Join(vaultDir, subdir), secret.DirPerms) + if err != nil { + return fmt.Errorf("failed to create %s directory: %w", subdir, err) + } + } + + ltIdentity, derivationIndex, publicKeyHash, familyHash, err := + processMnemonicForVault(fs, stateDir, vaultDir, name, mnemonic) + if err != nil { + return err + } + + metadata := &Metadata{ + CreatedAt: time.Now(), + DerivationIndex: derivationIndex, + PublicKeyHash: publicKeyHash, + MnemonicFamilyHash: familyHash, + } + + err = SaveVaultMetadata(fs, vaultDir, metadata) + if err != nil { + return fmt.Errorf("failed to save vault metadata: %w", err) + } + + if passphrase == nil { + return nil + } + + _, err = writePassphraseUnlocker(fs, vaultDir, ltIdentity, passphrase) + + return err +} + // SelectVault selects the given vault as the current vault func SelectVault(fs afero.Fs, stateDir string, name string) error { secret.Debug("Selecting vault", "vault_name", name, "state_dir", stateDir) diff --git a/internal/vault/metadata_test.go b/internal/vault/metadata_test.go index 70f4c08..2343868 100644 --- a/internal/vault/metadata_test.go +++ b/internal/vault/metadata_test.go @@ -304,7 +304,7 @@ func TestWorkflowMismatch(t *testing.T) { fs := afero.NewOsFs() // Test Case 1: Create vault WITH mnemonic (like init command) - _, err := vault.CreateVault(fs, tempDir, "default", testMnemonicBuffer(t)) + _, err := vault.CreateVault(fs, tempDir, "default", testMnemonicBuffer(t), nil) if err != nil { t.Fatalf("Failed to create vault with mnemonic: %v", err) } @@ -321,7 +321,7 @@ func TestWorkflowMismatch(t *testing.T) { metadata1.DerivationIndex, metadata1.PublicKeyHash) // Test Case 2: Create vault WITHOUT mnemonic, then import (work vault) - _, err = vault.CreateVault(fs, tempDir, "work", nil) + _, err = vault.CreateVault(fs, tempDir, "work", nil, nil) if err != nil { t.Fatalf("Failed to create vault without mnemonic: %v", err) } diff --git a/internal/vault/path_traversal_test.go b/internal/vault/path_traversal_test.go index 65b3319..e88a147 100644 --- a/internal/vault/path_traversal_test.go +++ b/internal/vault/path_traversal_test.go @@ -18,7 +18,7 @@ func TestGetSecretVersionRejectsPathTraversal(t *testing.T) { fs := afero.NewMemMapFs() vlt, err := vault.CreateVault(fs, testStateDir, testVaultName, - testMnemonicBuffer(t)) + testMnemonicBuffer(t), nil) require.NoError(t, err) // Add a legitimate secret so the vault is set up @@ -57,7 +57,7 @@ func TestGetSecretRejectsPathTraversal(t *testing.T) { fs := afero.NewMemMapFs() vlt, err := vault.CreateVault(fs, testStateDir, testVaultName, - testMnemonicBuffer(t)) + testMnemonicBuffer(t), nil) require.NoError(t, err) _, err = vlt.GetSecret("../../../etc/passwd") @@ -73,7 +73,7 @@ func TestGetSecretObjectRejectsPathTraversal(t *testing.T) { fs := afero.NewMemMapFs() vlt, err := vault.CreateVault(fs, testStateDir, testVaultName, - testMnemonicBuffer(t)) + testMnemonicBuffer(t), nil) require.NoError(t, err) maliciousNames := []string{ diff --git a/internal/vault/secrets_version_test.go b/internal/vault/secrets_version_test.go index df7cec2..9a85aad 100644 --- a/internal/vault/secrets_version_test.go +++ b/internal/vault/secrets_version_test.go @@ -66,7 +66,7 @@ func createTestVaultWithKey(t *testing.T, fs afero.Fs) *Vault { t.Helper() // Create vault without a long-term key, which is set up below - vault, err := CreateVault(fs, testStateDir, "test", nil) + vault, err := CreateVault(fs, testStateDir, "test", nil, nil) require.NoError(t, err) // Derive and store long-term key from mnemonic diff --git a/internal/vault/unlockers.go b/internal/vault/unlockers.go index 4075066..b9df96d 100644 --- a/internal/vault/unlockers.go +++ b/internal/vault/unlockers.go @@ -390,8 +390,31 @@ func (v *Vault) CreatePassphraseUnlocker( return nil, err } + unlocker, err := writePassphraseUnlocker(v.fs, vaultDir, ltIdentity, passphrase) + if err != nil { + return nil, err + } + + for _, oldDir := range oldDirs { + err = secret.RemoveDirAtomic(v.fs, oldDir) + if err != nil { + return nil, fmt.Errorf( + "created and selected the new passphrase unlocker: %w", err) + } + } + + return unlocker, nil +} + +// writePassphraseUnlocker writes a new passphrase unlocker of the long-term +// key ltIdentity into the vault directory vaultDir, in a directory of its own, +// and makes it the vault's current unlocker. +func writePassphraseUnlocker( + fs afero.Fs, vaultDir string, ltIdentity *age.X25519Identity, + passphrase *memguard.LockedBuffer, +) (*secret.PassphraseUnlocker, error) { createdAt := time.Now() - unlockerDir := filepath.Join(unlockersDir, unlockerTypePassphrase+"-"+ + unlockerDir := filepath.Join(vaultDir, "unlockers.d", unlockerTypePassphrase+"-"+ createdAt.UTC().Format(secret.UnlockerTimeFormat)) // Generate new age keypair for unlocker @@ -422,8 +445,8 @@ func (v *Vault) CreatePassphraseUnlocker( } // Write the unlocker's files, the metadata last - err = secret.WriteDir(v.fs, unlockerDir, func(dir string) error { - return v.writeUnlockerFiles(dir, unlockerIdentity, passphrase, + err = secret.WriteDir(fs, unlockerDir, func(dir string) error { + return writeUnlockerFiles(fs, dir, unlockerIdentity, passphrase, encryptedLtPrivKey, metadataBytes) }) if err != nil { @@ -434,21 +457,13 @@ func (v *Vault) CreatePassphraseUnlocker( // passphrase unlocker created in the same minute has the same ID. currentUnlockerPath := filepath.Join(vaultDir, "current-unlocker") - err = secret.WriteFileAtomic(v.fs, currentUnlockerPath, + err = secret.WriteFileAtomic(fs, currentUnlockerPath, []byte(filepath.Base(unlockerDir))) if err != nil { return nil, fmt.Errorf("failed to select new unlocker: %w", err) } - for _, oldDir := range oldDirs { - err = secret.RemoveDirAtomic(v.fs, oldDir) - if err != nil { - return nil, fmt.Errorf( - "created and selected the new passphrase unlocker: %w", err) - } - } - - return secret.NewPassphraseUnlocker(v.fs, unlockerDir, metadata), nil + return secret.NewPassphraseUnlocker(fs, unlockerDir, metadata), nil } // passphraseUnlockerDirs returns the directories in unlockersDir that hold @@ -514,7 +529,8 @@ func (v *Vault) readUnlockerMetadata(unlockerDir string) (UnlockerMetadata, erro // writeUnlockerFiles writes the files of a passphrase unlocker into // unlockerDir: its public key, its passphrase-encrypted private key, the // long-term private key encrypted to it, and its metadata, last. -func (v *Vault) writeUnlockerFiles( +func writeUnlockerFiles( + fs afero.Fs, unlockerDir string, unlockerIdentity *age.X25519Identity, passphrase *memguard.LockedBuffer, @@ -523,7 +539,7 @@ func (v *Vault) writeUnlockerFiles( // Write public key pubKeyPath := filepath.Join(unlockerDir, "pub.age") - err := secret.WriteFileAtomic(v.fs, pubKeyPath, + err := secret.WriteFileAtomic(fs, pubKeyPath, []byte(unlockerIdentity.Recipient().String())) if err != nil { return fmt.Errorf("failed to write unlocker public key: %w", err) @@ -541,18 +557,18 @@ func (v *Vault) writeUnlockerFiles( // Write encrypted private key privKeyPath := filepath.Join(unlockerDir, "priv.age") - err = secret.WriteFileAtomic(v.fs, privKeyPath, encryptedPrivKey) + err = secret.WriteFileAtomic(fs, privKeyPath, encryptedPrivKey) if err != nil { return fmt.Errorf("failed to write encrypted unlocker private key: %w", err) } - err = secret.WriteFileAtomic(v.fs, + err = secret.WriteFileAtomic(fs, filepath.Join(unlockerDir, "longterm.age"), encryptedLtPrivKey) if err != nil { return fmt.Errorf("failed to write encrypted long-term private key: %w", err) } - err = secret.WriteFileAtomic(v.fs, + err = secret.WriteFileAtomic(fs, filepath.Join(unlockerDir, "unlocker-metadata.json"), metadataBytes) if err != nil { return fmt.Errorf("failed to write unlocker metadata: %w", err) diff --git a/internal/vault/vault_test.go b/internal/vault/vault_test.go index 0cab776..1a5d547 100644 --- a/internal/vault/vault_test.go +++ b/internal/vault/vault_test.go @@ -2,6 +2,7 @@ package vault_test import ( "bytes" + "errors" "path/filepath" "slices" "testing" @@ -72,7 +73,7 @@ func testCreateVault(t *testing.T, fs afero.Fs) { t.Helper() vlt, err := vault.CreateVault(fs, testStateDir, testVaultName, - testMnemonicBuffer(t)) + testMnemonicBuffer(t), nil) if err != nil { t.Fatalf("Failed to create vault: %v", err) } @@ -298,7 +299,7 @@ func TestListUnlockers_SkipsMissingMetadata(t *testing.T) { // Create vault vlt, err := vault.CreateVault(fs, testStateDir, testVaultName, - testMnemonicBuffer(t)) + testMnemonicBuffer(t), nil) if err != nil { t.Fatalf("Failed to create vault: %v", err) } @@ -344,3 +345,29 @@ func TestListUnlockers_SkipsMissingMetadata(t *testing.T) { } } } + +// TestCreateVaultUnlockerNeedsMnemonic checks that CreateVault, given a +// passphrase for an unlocker but no mnemonic to derive the long-term key from, +// fails without writing anything. +func TestCreateVaultUnlockerNeedsMnemonic(t *testing.T) { + t.Parallel() + + fs := afero.NewMemMapFs() + + passphrase := memguard.NewBufferFromBytes([]byte(testPassphrase)) + defer passphrase.Destroy() + + _, err := vault.CreateVault(fs, testStateDir, testVaultName, nil, passphrase) + if !errors.Is(err, vault.ErrUnlockerWithoutMnemonic) { + t.Fatalf("Expected ErrUnlockerWithoutMnemonic, got %v", err) + } + + exists, err := afero.Exists(fs, testStateDir) + if err != nil { + t.Fatalf("Failed to check for the state directory: %v", err) + } + + if exists { + t.Errorf("CreateVault wrote the state directory") + } +}