Lock the state directory and write vault files atomically (closes #34)

Each command that changes the state directory holds one lock: flock(2)
on `lock` in the state directory, dropped by the kernel if the process
dies, or a process-wide mutex on the in-memory test filesystem. It
covers the state directory, not each vault, because `currentvault`,
`vault create` and cross-vault moves span vaults, and a lock file in a
vault would be deleted by `vault remove` under a waiting command.

Files go through `secret.WriteFileAtomic`; versions, new secrets and
cross-vault copies are built in a temporary directory and renamed into
place; removals rename out of the way first. Left for later: replacing
an unlocker (#71) and deleting
what an interrupted command leaves under a `.tmp-` name
(#75).

Model: opus-5-5

internal/cli/crypto.go

@@ -72,10 +72,18 @@ func newDecryptCmd() *cobra.Command {
 
 // resolveEncryptionKey returns a secure buffer holding the age secret key
 // for the named secret, generating and storing a new key if the secret
-// does not exist. The caller must destroy the returned buffer.
+// does not exist. The caller must destroy the returned buffer. It holds the
+// state directory lock itself, so that Encrypt streams its input and output
+// unlocked and cannot block a secret command at the other end of a pipe.
 func (cli *Instance) resolveEncryptionKey(
 	vlt *vault.Vault, secretName string,
 ) (*memguard.LockedBuffer, error) {
+	release, err := vault.LockStateDir(cli.fs, cli.stateDir)
+	if err != nil {
+		return nil, err
+	}
+	defer release()
+
 	// Check if secret exists
 	secretObj := secret.NewSecret(vlt, secretName)
 

internal/cli/generate.go

@@ -155,6 +155,12 @@ func (cli *Instance) GenerateSecret(
 		return fmt.Errorf("failed to generate random secret: %w", err)
 	}
 
+	release, err := vault.LockStateDir(cli.fs, cli.stateDir)
+	if err != nil {
+		return err
+	}
+	defer release()
+
 	// Store the secret in the vault
 	vlt, err := vault.GetCurrentVault(cli.fs, cli.stateDir)
 	if err != nil {

internal/cli/init.go

@@ -103,8 +103,21 @@ func (cli *Instance) setupDefaultVault(
 	return vlt, ltIdentity, nil
 }
 
-// Init initializes the secret manager
+// Init initializes the secret manager, holding the state directory lock
+// while initialize runs
 func (cli *Instance) Init(cmd *cobra.Command) error {
+	release, err := vault.LockStateDir(cli.fs, cli.stateDir)
+	if err != nil {
+		return err
+	}
+	defer release()
+
+	return cli.initialize(cmd)
+}
+
+// initialize creates the state directory, the default vault and its first
+// unlocker
+func (cli *Instance) initialize(cmd *cobra.Command) error {
 	secret.Debug("Starting secret manager initialization")
 
 	// Create state directory

internal/cli/lock_test.go

@@ -0,0 +1,493 @@
+//nolint:testpackage // sets the unexported fields of Instance
+package cli
+
+import (
+	"io"
+	"os"
+	"path/filepath"
+	"strconv"
+	"strings"
+	"sync"
+	"testing"
+	"time"
+
+	"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/assert"
+	"github.com/stretchr/testify/require"
+)
+
+const (
+	// lockWait is how long a test waits for something that must happen
+	// once the lock is free.
+	lockWait = 10 * time.Second
+
+	// heldWait is how long a test watches a command that must wait for the
+	// lock. A command that takes no lock changes the state directory well
+	// within it.
+	heldWait = 100 * time.Millisecond
+
+	// testPassphrase protects the passphrase unlockers the tests create.
+	testPassphrase = "test-passphrase"
+
+	// testInput is a file outside the state directory that commands read.
+	testInput = "/input"
+)
+
+// lockInBackground starts taking the state directory lock and returns a
+// channel that delivers the function releasing it once it has been taken.
+func lockInBackground(t *testing.T, fs afero.Fs) <-chan func() {
+	t.Helper()
+
+	taken := make(chan func(), 1)
+
+	go func() {
+		release, err := vault.LockStateDir(fs, testStateDir)
+		if assert.NoError(t, err) {
+			taken <- release
+		}
+	}()
+
+	return taken
+}
+
+// addAtOnce runs one add of the secret name per value, all at once, and
+// returns their errors.
+func addAtOnce(
+	fs afero.Fs, stateDir, name string, force bool, values []string,
+) []error {
+	errs := make(chan error, len(values))
+
+	for _, value := range values {
+		go func() {
+			cli := NewCLIInstanceWithStateDir(fs, stateDir)
+			cli.cmd = &cobra.Command{}
+			cli.cmd.SetIn(strings.NewReader(value))
+
+			errs <- cli.AddSecret(name, force)
+		}()
+	}
+
+	results := make([]error, 0, len(values))
+	for range values {
+		results = append(results, <-errs)
+	}
+
+	return results
+}
+
+// numbered returns count distinct values starting with prefix.
+func numbered(prefix string, count int) []string {
+	values := make([]string, 0, count)
+	for i := range count {
+		values = append(values, prefix+"-"+strconv.Itoa(i))
+	}
+
+	return values
+}
+
+// TestConcurrentAddsKeepEveryVersion runs adds of one secret at once, on
+// the in-memory and on the real filesystem. Without the state directory
+// lock, adds of a new secret all find it absent and replace each other, and
+// forced adds read the same highest version number and overwrite each
+// other's version. With it they behave as if run one after another.
+//
+//nolint:paralleltest // t.Setenv forbids parallel subtests
+func TestConcurrentAddsKeepEveryVersion(t *testing.T) {
+	t.Setenv(secret.EnvMnemonic, testMnemonic)
+
+	const adds = 8
+
+	for _, tc := range []struct {
+		name     string
+		fs       afero.Fs
+		stateDir string
+	}{
+		{"memory", afero.NewMemMapFs(), testStateDir},
+		{"real", afero.NewOsFs(), t.TempDir()},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			_, err := vault.CreateVault(tc.fs, tc.stateDir, "default")
+			require.NoError(t, err)
+
+			// One add creates the secret; the others find that it exists
+			created := 0
+
+			for _, err := range addAtOnce(tc.fs, tc.stateDir, "shared", false,
+				numbered("create", adds)) {
+				if err == nil {
+					created++
+				} else {
+					require.ErrorIs(t, err, vault.ErrSecretExists)
+				}
+			}
+
+			require.Equal(t, 1, created, "exactly one add creates the secret")
+
+			// Every forced add stores a version of its own
+			for _, err := range addAtOnce(tc.fs, tc.stateDir, "shared", true,
+				numbered("force", adds)) {
+				require.NoError(t, err)
+			}
+
+			vlt, err := vault.GetCurrentVault(tc.fs, tc.stateDir)
+			require.NoError(t, err)
+
+			vaultDir, err := vlt.GetDirectory()
+			require.NoError(t, err)
+
+			versions, err := secret.ListVersions(tc.fs,
+				filepath.Join(vaultDir, "secrets.d", "shared"))
+			require.NoError(t, err)
+			require.Len(t, versions, adds+1, "one version per successful add")
+
+			values := make(map[string]bool, len(versions))
+
+			for _, version := range versions {
+				value, err := vlt.GetSecretVersion("shared", version)
+				require.NoError(t, err)
+
+				values[string(value)] = true
+			}
+
+			assert.Len(t, values, adds+1, "every add stored its own value")
+		})
+	}
+}
+
+// readNotifier passes reads through to Reader and closes reading at the
+// first one.
+type readNotifier struct {
+	io.Reader
+
+	reading chan struct{}
+	once    sync.Once
+}
+
+func (r *readNotifier) Read(p []byte) (int, error) {
+	r.once.Do(func() { close(r.reading) })
+
+	return r.Reader.Read(p)
+}
+
+// TestEncryptPipedIntoAdd runs `secret encrypt key | secret add name` in
+// one process, starting encrypt once add is reading its input. Had add
+// taken the state directory lock before reading, it would hold the lock
+// while waiting for encrypt's output, and encrypt would wait for the lock
+// to store its key: neither would finish.
+func TestEncryptPipedIntoAdd(t *testing.T) {
+	t.Setenv(secret.EnvMnemonic, testMnemonic)
+
+	fs := afero.NewMemMapFs()
+	_, err := vault.CreateVault(fs, testStateDir, "default")
+	require.NoError(t, err)
+	require.NoError(t, afero.WriteFile(fs, testInput, []byte("piped"), 0o600))
+
+	pipeReader, pipeWriter := io.Pipe()
+	// If the test gives up, this makes add's read fail, so that both
+	// commands return and release the lock the other tests use
+	t.Cleanup(func() { _ = pipeReader.Close() })
+
+	const commands = 2
+
+	input := &readNotifier{Reader: pipeReader, reading: make(chan struct{})}
+	results := make(chan error, commands)
+
+	go func() {
+		add := NewCLIInstanceWithStateDir(fs, testStateDir)
+		add.cmd = &cobra.Command{}
+		add.cmd.SetIn(input)
+
+		results <- add.AddSecret("encrypted", false)
+	}()
+
+	go func() {
+		<-input.reading
+
+		encrypt := NewCLIInstanceWithStateDir(fs, testStateDir)
+		encrypt.cmd = &cobra.Command{}
+		encrypt.cmd.SetOut(pipeWriter)
+
+		err := encrypt.Encrypt("key", testInput, "")
+		// Ends add's input, as the end of the pipe does
+		_ = pipeWriter.CloseWithError(err)
+
+		results <- err
+	}()
+
+	timeout := time.After(lockWait)
+
+	for range commands {
+		select {
+		case err := <-results:
+			require.NoError(t, err)
+		case <-timeout:
+			t.Fatal("secret encrypt piped into secret add never finished")
+		}
+	}
+}
+
+// TestFailedCommandReleasesLock checks that a command failing after it
+// took the state directory lock leaves the lock free for the next command.
+func TestFailedCommandReleasesLock(t *testing.T) {
+	t.Parallel()
+
+	fs := afero.NewMemMapFs()
+	cli := NewCLIInstanceWithStateDir(fs, testStateDir)
+
+	// Fails once it holds the lock: there is no current vault
+	err := cli.RemoveSecret(&cobra.Command{}, "missing", false)
+	require.Error(t, err)
+
+	select {
+	case release := <-lockInBackground(t, fs):
+		release()
+	case <-time.After(lockWait):
+		t.Fatal("the failed command left the state directory locked")
+	}
+}
+
+// stateDirModTimes returns the modification time of every file and
+// directory under the test state directory. Any change a command makes, even
+// rewriting a file with the same content, changes it.
+func stateDirModTimes(t *testing.T, fs afero.Fs) map[string]int64 {
+	t.Helper()
+
+	modTimes := make(map[string]int64)
+
+	err := afero.Walk(fs, testStateDir,
+		func(path string, info os.FileInfo, err error) error {
+			if err != nil {
+				return err
+			}
+
+			modTimes[path] = info.ModTime().UnixNano()
+
+			return nil
+		})
+	require.NoError(t, err)
+
+	return modTimes
+}
+
+// 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.
+func setupEveryCommand(
+	t *testing.T, fs afero.Fs, withUnlocker bool,
+) (string, string) {
+	t.Helper()
+
+	other, err := vault.CreateVault(fs, testStateDir, "other")
+	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, "default")
+	require.NoError(t, err)
+
+	addTestSecret(t, vlt, []byte("older"), false)
+	addTestSecret(t, vlt, []byte("newer"), true)
+
+	vaultDir, err := vlt.GetDirectory()
+	require.NoError(t, err)
+
+	versions, err := secret.ListVersions(fs,
+		filepath.Join(vaultDir, "secrets.d", "test%secret"))
+	require.NoError(t, err)
+	require.Len(t, versions, 2)
+
+	unlockerID := ""
+
+	if withUnlocker {
+		passphrase := memguard.NewBufferFromBytes([]byte(testPassphrase))
+		defer passphrase.Destroy()
+
+		unlocker, err := vlt.CreatePassphraseUnlocker(passphrase)
+		require.NoError(t, err)
+
+		unlockerID = unlocker.GetID()
+	}
+
+	require.NoError(t, afero.WriteFile(fs, testInput, []byte("input"), 0o600))
+
+	// Newest first
+	return versions[1], unlockerID
+}
+
+// requireWaitsForLock runs a command, given what setupEveryCommand made,
+// while holding the state directory lock. The command must neither finish
+// nor change anything while the lock is held, and must succeed once it is
+// released.
+func requireWaitsForLock(
+	t *testing.T,
+	withUnlocker bool,
+	run func(cli *Instance, olderVersion, unlockerID string) error,
+) {
+	t.Helper()
+
+	fs := afero.NewMemMapFs()
+	olderVersion, unlockerID := setupEveryCommand(t, fs, withUnlocker)
+	before := stateDirModTimes(t, fs)
+
+	release, err := vault.LockStateDir(fs, testStateDir)
+	require.NoError(t, err)
+
+	// Released at most once, and also if the test fails while holding it,
+	// so that later tests can take it
+	release = sync.OnceFunc(release)
+	defer release()
+
+	cli := NewCLIInstanceWithStateDir(fs, testStateDir)
+	cli.cmd = &cobra.Command{}
+	cli.cmd.SetIn(strings.NewReader("value"))
+	cli.cmd.SetOut(io.Discard)
+
+	done := make(chan error, 1)
+
+	go func() { done <- run(cli, olderVersion, unlockerID) }()
+
+	select {
+	case err := <-done:
+		t.Fatalf("finished while the lock was held, with error %v", err)
+	case <-time.After(heldWait):
+	}
+
+	assert.Equal(t, before, stateDirModTimes(t, fs),
+		"changed the state directory while the lock was held")
+
+	release()
+
+	select {
+	case err := <-done:
+		require.NoError(t, err)
+	case <-time.After(lockWait):
+		t.Fatal("did not finish once the lock was released")
+	}
+}
+
+// TestChangingCommandsWaitForLock checks that each command that changes the
+// state directory waits for its lock.
+//
+//nolint:paralleltest // t.Setenv forbids parallel subtests
+func TestChangingCommandsWaitForLock(t *testing.T) {
+	t.Setenv(secret.EnvMnemonic, testMnemonic)
+	t.Setenv(secret.EnvUnlockPassphrase, testPassphrase)
+
+	for _, tc := range []struct {
+		name         string
+		withUnlocker bool
+		run          func(cli *Instance, olderVersion, unlockerID string) error
+	}{
+		{"add", false, func(cli *Instance, _, _ string) error {
+			return cli.AddSecret("added", false)
+		}},
+		{"import", false, func(cli *Instance, _, _ string) error {
+			return cli.ImportSecret(cli.cmd, "imported", testInput, false)
+		}},
+		{"generate secret", false, func(cli *Instance, _, _ string) error {
+			return cli.GenerateSecret(cli.cmd, "generated", 16, "base58", false)
+		}},
+		{"encrypt", false, func(cli *Instance, _, _ string) error {
+			return cli.Encrypt("key", testInput, "")
+		}},
+		{"rm", false, func(cli *Instance, _, _ string) error {
+			return cli.RemoveSecret(cli.cmd, "test/secret", false)
+		}},
+		{"move", false, func(cli *Instance, _, _ string) error {
+			return cli.MoveSecret(cli.cmd, "test/secret", "moved", false)
+		}},
+		{"version promote", false, func(cli *Instance, olderVersion, _ string) error {
+			return cli.PromoteVersion(cli.cmd, "test/secret", olderVersion)
+		}},
+		{"version rm", false, func(cli *Instance, olderVersion, _ string) error {
+			return cli.RemoveVersion(cli.cmd, "test/secret", olderVersion)
+		}},
+		{"vault create", false, func(cli *Instance, _, _ string) error {
+			return cli.CreateVault(cli.cmd, "created")
+		}},
+		{"vault select", false, func(cli *Instance, _, _ string) error {
+			return cli.SelectVault(cli.cmd, "other")
+		}},
+		{"vault import", false, func(cli *Instance, _, _ string) error {
+			return cli.VaultImport(cli.cmd, "other")
+		}},
+		{"vault rm", false, func(cli *Instance, _, _ string) error {
+			return cli.RemoveVault(cli.cmd, "other", false)
+		}},
+		{"unlocker add", false, func(cli *Instance, _, _ string) error {
+			return cli.UnlockersAdd("passphrase", cli.cmd)
+		}},
+		{"unlocker rm", true, func(cli *Instance, _, unlockerID string) error {
+			return cli.UnlockersRemove(unlockerID, true, cli.cmd)
+		}},
+		{"unlocker select", true, func(cli *Instance, _, unlockerID string) error {
+			return cli.UnlockerSelect(unlockerID)
+		}},
+		{"init", false, func(cli *Instance, _, _ string) error {
+			return cli.Init(cli.cmd)
+		}},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			requireWaitsForLock(t, tc.withUnlocker, tc.run)
+		})
+	}
+}
+
+// TestEncryptStreamsUnlocked checks that secret encrypt has released the
+// state directory lock by the time it writes its output. Holding it while
+// streaming would stall every other changing command for as long as the
+// stream lasts, and forever when the other end of the pipe is one of them.
+func TestEncryptStreamsUnlocked(t *testing.T) {
+	t.Setenv(secret.EnvMnemonic, testMnemonic)
+
+	fs := afero.NewMemMapFs()
+	_, err := vault.CreateVault(fs, testStateDir, "default")
+	require.NoError(t, err)
+	require.NoError(t, afero.WriteFile(fs, testInput, []byte("streamed"), 0o600))
+
+	outputReader, outputWriter := io.Pipe()
+	done := make(chan error, 1)
+
+	go func() {
+		encrypt := NewCLIInstanceWithStateDir(fs, testStateDir)
+		encrypt.cmd = &cobra.Command{}
+		encrypt.cmd.SetOut(outputWriter)
+
+		err := encrypt.Encrypt("key", testInput, "")
+		_ = outputWriter.CloseWithError(err)
+
+		done <- err
+	}()
+
+	// The first byte of output: encrypt is streaming now, and blocked
+	// writing until it is read
+	_, err = io.ReadFull(outputReader, make([]byte, 1))
+	require.NoError(t, err)
+
+	taken := lockInBackground(t, fs)
+
+	select {
+	case release := <-taken:
+		release()
+	case <-time.After(lockWait):
+		// Let encrypt finish, so that it releases the lock, then free it
+		// again for the tests that follow
+		_, _ = io.Copy(io.Discard, outputReader)
+
+		(<-taken)()
+		t.Fatal("secret encrypt held the lock while streaming")
+	}
+
+	_, err = io.Copy(io.Discard, outputReader)
+	require.NoError(t, err)
+	require.NoError(t, <-done)
+}

internal/cli/secrets.go

@@ -377,6 +377,15 @@ func (cli *Instance) AddSecret(secretName string, force bool) error {
 	valueBuffer := combineBuffers(buffers, totalSize)
 	defer valueBuffer.Destroy()
 
+	// Locked only now that stdin has been read: in `secret encrypt key |
+	// secret add name`, holding the lock while reading would leave each
+	// command waiting for the other.
+	release, err := vault.LockStateDir(cli.fs, cli.stateDir)
+	if err != nil {
+		return err
+	}
+	defer release()
+
 	// Add the secret to the vault
 	secret.Debug("Calling vault.AddSecret", "secret_name", secretName,
 		"value_length", valueBuffer.Size(), "force", force)
@@ -635,6 +644,14 @@ func (cli *Instance) ImportSecret(
 	valueBuffer := combineBuffers(buffers, totalSize)
 	defer valueBuffer.Destroy()
 
+	// Locked only now that the file has been read, as in AddSecret: the
+	// file may be a pipe written by another secret command.
+	release, err := vault.LockStateDir(cli.fs, cli.stateDir)
+	if err != nil {
+		return err
+	}
+	defer release()
+
 	// Store the secret in the vault
 	err = vlt.AddSecret(secretName, valueBuffer, force)
 	if err != nil {
@@ -649,6 +666,12 @@ func (cli *Instance) ImportSecret(
 
 // RemoveSecret removes a secret from the vault
 func (cli *Instance) RemoveSecret(cmd *cobra.Command, secretName string, _ bool) error {
+	release, err := vault.LockStateDir(cli.fs, cli.stateDir)
+	if err != nil {
+		return err
+	}
+	defer release()
+
 	// Get current vault
 	currentVlt, err := vault.GetCurrentVault(cli.fs, cli.stateDir)
 	if err != nil {
@@ -683,7 +706,7 @@ func (cli *Instance) RemoveSecret(cmd *cobra.Command, secretName string, _ bool)
 	}
 
 	// Remove the secret directory
-	err = cli.fs.RemoveAll(secretDir)
+	err = secret.RemoveDirAtomic(cli.fs, secretDir)
 	if err != nil {
 		return fmt.Errorf("failed to remove secret: %w", err)
 	}
@@ -698,6 +721,12 @@ func (cli *Instance) RemoveSecret(cmd *cobra.Command, secretName string, _ bool)
 func (cli *Instance) MoveSecret(
 	cmd *cobra.Command, source, dest string, force bool,
 ) error {
+	release, err := vault.LockStateDir(cli.fs, cli.stateDir)
+	if err != nil {
+		return err
+	}
+	defer release()
+
 	// Parse source and destination
 	srcVaultName, srcSecretName, srcQualified := ParseVaultSecretRef(source)
 	destVaultName, destSecretName, destQualified := ParseVaultSecretRef(dest)
@@ -792,7 +821,7 @@ func (cli *Instance) moveSecretWithinVault(
 			return fmt.Errorf("secret '%s' %w", dest, errSecretExistsNoForce)
 		}
 
-		err = cli.fs.RemoveAll(destDir)
+		err = secret.RemoveDirAtomic(cli.fs, destDir)
 		if err != nil {
 			return fmt.Errorf("failed to remove existing destination: %w", err)
 		}
@@ -872,7 +901,7 @@ func (cli *Instance) moveSecretCrossVault(
 	}
 
 	// Delete source secret
-	err = cli.fs.RemoveAll(srcSecretDir)
+	err = secret.RemoveDirAtomic(cli.fs, srcSecretDir)
 	if err != nil {
 		// Copy succeeded but delete failed - warn but don't fail
 		cmd.Printf("Warning: copied secret but failed to remove source: %v\n", err)

internal/cli/unlockers.go

@@ -534,6 +534,12 @@ func (cli *Instance) printUnlockersTable(unlockers []UnlockerInfo) error {
 
 // UnlockersAdd adds a new unlocker
 func (cli *Instance) UnlockersAdd(unlockerType string, cmd *cobra.Command) error {
+	release, err := vault.LockStateDir(cli.fs, cli.stateDir)
+	if err != nil {
+		return err
+	}
+	defer release()
+
 	switch unlockerType {
 	case unlockerTypePassphrase:
 		return cli.addPassphraseUnlocker(cmd)
@@ -714,6 +720,12 @@ func (cli *Instance) addPGPUnlocker(cmd *cobra.Command) error {
 func (cli *Instance) UnlockersRemove(
 	unlockerID string, force bool, cmd *cobra.Command,
 ) error {
+	release, err := vault.LockStateDir(cli.fs, cli.stateDir)
+	if err != nil {
+		return err
+	}
+	defer release()
+
 	// Get current vault
 	vlt, err := vault.GetCurrentVault(cli.fs, cli.stateDir)
 	if err != nil {
@@ -763,6 +775,12 @@ func (cli *Instance) UnlockersRemove(
 
 // UnlockerSelect selects an unlocker as current
 func (cli *Instance) UnlockerSelect(unlockerID string) error {
+	release, err := vault.LockStateDir(cli.fs, cli.stateDir)
+	if err != nil {
+		return err
+	}
+	defer release()
+
 	// Get current vault
 	vlt, err := vault.GetCurrentVault(cli.fs, cli.stateDir)
 	if err != nil {

internal/cli/vault.go

@@ -267,6 +267,12 @@ func resolvePassphrase() (*memguard.LockedBuffer, error) {
 func (cli *Instance) CreateVault(cmd *cobra.Command, name string) error {
 	secret.Debug("Creating new vault", "name", name, "state_dir", cli.stateDir)
 
+	release, err := vault.LockStateDir(cli.fs, cli.stateDir)
+	if err != nil {
+		return err
+	}
+	defer release()
+
 	// Get or prompt for mnemonic
 	var mnemonicStr string
 
@@ -354,7 +360,13 @@ func (cli *Instance) CreateVault(cmd *cobra.Command, name string) error {
 
 // SelectVault selects a vault as the current one
 func (cli *Instance) SelectVault(cmd *cobra.Command, name string) error {
-	err := vault.SelectVault(cli.fs, cli.stateDir, name)
+	release, err := vault.LockStateDir(cli.fs, cli.stateDir)
+	if err != nil {
+		return err
+	}
+	defer release()
+
+	err = vault.SelectVault(cli.fs, cli.stateDir, name)
 	if err != nil {
 		return err
 	}
@@ -442,8 +454,21 @@ func updateVaultImportMetadata(
 	return nil
 }
 
-// VaultImport imports a mnemonic into a specific vault
+// VaultImport imports a mnemonic into a specific vault, holding the state
+// directory lock while importMnemonic runs
 func (cli *Instance) VaultImport(cmd *cobra.Command, vaultName string) error {
+	release, err := vault.LockStateDir(cli.fs, cli.stateDir)
+	if err != nil {
+		return err
+	}
+	defer release()
+
+	return cli.importMnemonic(cmd, vaultName)
+}
+
+// importMnemonic gives the vault a long-term key derived from the mnemonic
+// and a passphrase unlocker
+func (cli *Instance) importMnemonic(cmd *cobra.Command, vaultName string) error {
 	secret.Debug("Importing mnemonic into vault",
 		"vault_name", vaultName, "state_dir", cli.stateDir)
 
@@ -478,7 +503,7 @@ func (cli *Instance) VaultImport(cmd *cobra.Command, vaultName string) error {
 	secret.Debug("Storing long-term public key",
 		"pubkey", ltPublicKey, "vault_dir", vaultDir)
 
-	err = afero.WriteFile(cli.fs, pubKeyPath, []byte(ltPublicKey), secret.FilePerms)
+	err = secret.WriteFileAtomic(cli.fs, pubKeyPath, []byte(ltPublicKey))
 	if err != nil {
 		return fmt.Errorf("failed to store long-term public key: %w", err)
 	}
@@ -577,6 +602,12 @@ func (cli *Instance) switchAwayFromVault(
 
 // RemoveVault removes a vault with safety checks
 func (cli *Instance) RemoveVault(cmd *cobra.Command, name string, force bool) error {
+	release, err := vault.LockStateDir(cli.fs, cli.stateDir)
+	if err != nil {
+		return err
+	}
+	defer release()
+
 	// Get list of all vaults
 	vaults, err := vault.ListVaults(cli.fs, cli.stateDir)
 	if err != nil {
@@ -626,7 +657,7 @@ func (cli *Instance) RemoveVault(cmd *cobra.Command, name string, force bool) er
 	}
 
 	// Remove the vault directory
-	err = cli.fs.RemoveAll(vaultDir)
+	err = secret.RemoveDirAtomic(cli.fs, vaultDir)
 	if err != nil {
 		return fmt.Errorf("failed to remove vault directory: %w", err)
 	}

internal/cli/version.go

@@ -239,6 +239,12 @@ func formatVersionTime(t *time.Time) string {
 func (cli *Instance) PromoteVersion(
 	cmd *cobra.Command, secretName string, version string,
 ) error {
+	release, err := vault.LockStateDir(cli.fs, cli.stateDir)
+	if err != nil {
+		return err
+	}
+	defer release()
+
 	// Get current vault
 	vlt, err := vault.GetCurrentVault(cli.fs, cli.stateDir)
 	if err != nil {
@@ -282,6 +288,12 @@ func (cli *Instance) PromoteVersion(
 func (cli *Instance) RemoveVersion(
 	cmd *cobra.Command, secretName string, version string,
 ) error {
+	release, err := vault.LockStateDir(cli.fs, cli.stateDir)
+	if err != nil {
+		return err
+	}
+	defer release()
+
 	// Get current vault
 	vlt, err := vault.GetCurrentVault(cli.fs, cli.stateDir)
 	if err != nil {
@@ -333,7 +345,7 @@ func (cli *Instance) RemoveVersion(
 	}
 
 	// Remove the version directory
-	err = cli.fs.RemoveAll(versionDir)
+	err = secret.RemoveDirAtomic(cli.fs, versionDir)
 	if err != nil {
 		return fmt.Errorf("failed to remove version: %w", err)
 	}