A secret mv between vaults unlocks both vaults before taking the lock (closes #189)

A move between two vaults unlocked them while holding the state directory
lock, and unlocking can ask for a passphrase, so every other changing
command waited at that prompt. moveSecretCrossVault now checks the move,
refusing an existing destination secret without --force, unlocks both
vaults, and only then takes the lock. Under the lock, it checks that the
destination vault still has the long-term key it unlocked, and
copySecretAndRemoveSource checks the move again before it copies. A move
within a vault takes the lock in moveSecretWithinVault, so MoveSecret
takes none itself.

The two tests of a source that cannot be removed now call
copySecretAndRemoveSource, since the lock refuses their wrapped
filesystems.

Model: opus-5-5

internal/cli/move_cross_vault_test.go

@@ -263,15 +263,7 @@ func TestCrossVaultMoveChecksAgainOnceLocked(t *testing.T) {
 	})
 
 	t.Run("destination vault created again from another mnemonic", func(t *testing.T) {
-		// Set by the move, which has finished once
-		// requireChecksAgainOnceLocked returns
-		var moveErr error
-
-		requireChecksAgainOnceLocked(t, func(cli *Instance) error {
-			moveErr = move(cli)
-
-			return moveErr
-		}, func(fs afero.Fs) error {
+		requireChecksAgainOnceLocked(t, move, func(fs afero.Fs) error {
 			err := fs.RemoveAll(testStateDir + "/vaults.d/other")
 			if err != nil {
 				return err
@@ -281,12 +273,6 @@ func TestCrossVaultMoveChecksAgainOnceLocked(t *testing.T) {
 
 			return err
 		}, errVaultCreatedAgain)
-
-		// secret unlocker add fails with the same error, so its message
-		// must fit a move too
-		require.EqualError(t, moveErr, "vault 'other' was removed and "+
-			"created again with another key while it was being unlocked; "+
-			"nothing was changed")
 	})
 }
 

internal/cli/secrets.go

@@ -1097,9 +1097,10 @@ func (cli *Instance) moveSecretCrossVault(
 	destVault *vault.Vault, destSecretName string,
 	force bool,
 ) error {
-	// Before unlocking, check that the source secret exists, that the
-	// destination is not the source under another name, and, without
-	// --force, that the destination secret does not exist
+	// Before unlocking, check that the source secret and the destination
+	// vault exist, that the destination is not the source under another
+	// name, and, without --force, that the destination secret does not
+	// exist
 	_, err := cli.checkCrossVaultMove(
 		srcVault, srcSecretName, destVault, destSecretName, force)
 	if err != nil {
@@ -1179,9 +1180,10 @@ func (cli *Instance) copySecretAndRemoveSource(
 }
 
 // checkCrossVaultMove checks that the secret srcSecretName exists in
-// srcVault, that destSecretName in destVault is not the source under
-// another name, and, unless force is set, that destSecretName does not
-// exist in destVault. It returns the source's directory.
+// srcVault, that destVault exists, that destSecretName in destVault is not
+// the source under another name, and, unless force is set, that
+// destSecretName does not exist in destVault. It returns the source's
+// directory.
 func (cli *Instance) checkCrossVaultMove(
 	srcVault *vault.Vault, srcSecretName string,
 	destVault *vault.Vault, destSecretName string,
@@ -1202,6 +1204,11 @@ func (cli *Instance) checkCrossVaultMove(
 			srcSecretName, vault.ErrSecretNotFound, srcVault.Name)
 	}
 
+	_, err = cli.existingVault(destVault.Name)
+	if err != nil {
+		return "", err
+	}
+
 	// The source is removed after the copy, so a destination that is the
 	// source under another name would be lost with it.
 	destVaultDir, err := destVault.GetDirectory()

internal/cli/unlockers.go

@@ -43,7 +43,7 @@ var (
 	// errVaultCreatedAgain carries only the message tail; the caller
 	// composes "vault '<name>' was removed and created again ...".
 	errVaultCreatedAgain = errors.New("was removed and created again " +
-		"with another key while it was being unlocked; nothing was changed")
+		"with another key while it was being unlocked; nothing was added")
 )
 
 // UnlockerInfo represents unlocker information for display