Ask for the mnemonic and passphrase before taking the state directory lock (closes #152)

init, vault create, unlocker add, and add, import and generate secret
with --force asked for the mnemonic or passphrase, or unlocked the
vault, while holding the state directory lock, so every other command
that changes anything waited at the prompt. They now ask and unlock
first and lock after. What they checked before asking is checked again
under the lock: unlocker add fails if the vault was removed meanwhile
and refuses a GPG key added meanwhile. A forced add of a secret that
another command created meanwhile lets go of the lock, gets the key,
and takes the lock again. secret.CreatePGPUnlocker now takes the vault,
already unlocked; SetMnemonic and SetUnlockPassphrase, used only by it,
are gone.

Not done: secret move between vaults still unlocks both under the lock;
the issue does not name it.

Model: opus-5-5

internal/cli/unlockers.go

@@ -404,12 +404,6 @@ 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)
@@ -423,6 +417,34 @@ func (cli *Instance) UnlockersAdd(unlockerType string, cmd *cobra.Command) error
 	}
 }
 
+// unlockThenLock gets the long-term key of vlt, the vault an unlocker is
+// added to, then takes the state directory lock and returns the function
+// that releases it. Getting the key can ask for a passphrase, so it comes
+// before the lock, as the question of askThenLock does, and no other
+// command waits while the user answers. Once the lock is held, it fails if
+// another command has removed the vault in the meantime, since writing the
+// unlocker would create the vault's directory again, holding nothing else.
+func (cli *Instance) unlockThenLock(vlt *vault.Vault) (func(), error) {
+	_, err := vlt.GetOrDeriveLongTermKey()
+	if err != nil {
+		return nil, fmt.Errorf("failed to get long-term key: %w", err)
+	}
+
+	release, err := vault.LockStateDir(cli.fs, cli.stateDir)
+	if err != nil {
+		return nil, err
+	}
+
+	_, err = cli.existingVault(vlt.GetName())
+	if err != nil {
+		release()
+
+		return nil, err
+	}
+
+	return release, nil
+}
+
 // autoSelectUnlocker selects the newly created unlocker as current,
 // printing a warning if selection fails
 func autoSelectUnlocker(cmd *cobra.Command, vlt *vault.Vault, unlockerID string) {
@@ -442,9 +464,6 @@ func (cli *Instance) addPassphraseUnlocker(cmd *cobra.Command) error {
 		return fmt.Errorf("failed to get current vault: %w", err)
 	}
 
-	// For passphrase unlockers, we don't need the vault to be unlocked
-	// The CreatePassphraseUnlocker method will handle getting the
-	// long-term key
 	vlt.Mnemonic, vlt.UnlockPassphrase = cli.Mnemonic, cli.UnlockPassphrase
 
 	// The new passphrase is asked for even when SB_UNLOCK_PASSPHRASE is
@@ -464,6 +483,12 @@ func (cli *Instance) addPassphraseUnlocker(cmd *cobra.Command) error {
 		defer passphraseBuffer.Destroy()
 	}
 
+	release, err := cli.unlockThenLock(vlt)
+	if err != nil {
+		return err
+	}
+	defer release()
+
 	passphraseUnlocker, err := vlt.CreatePassphraseUnlocker(passphraseBuffer)
 	if err != nil {
 		return err
@@ -502,12 +527,13 @@ func (cli *Instance) addPGPUnlocker(cmd *cobra.Command) error {
 		cmd.Printf("Using default GPG key: %s\n", gpgKeyID)
 	}
 
-	// Check if this key is already added as an unlocker
 	vlt, err := vault.GetCurrentVault(cli.fs, cli.stateDir)
 	if err != nil {
 		return fmt.Errorf("failed to get current vault: %w", err)
 	}
 
+	vlt.Mnemonic, vlt.UnlockPassphrase = cli.Mnemonic, cli.UnlockPassphrase
+
 	// Resolve the GPG key ID to its fingerprint, once: the duplicate check
 	// and the new unlocker's metadata both use this result
 	fingerprint, err := secret.ResolveGPGKeyFingerprint(gpgKeyID)
@@ -515,21 +541,26 @@ func (cli *Instance) addPGPUnlocker(cmd *cobra.Command) error {
 		return fmt.Errorf("failed to resolve GPG key fingerprint: %w", err)
 	}
 
-	// Check if this GPG key is already added
-	exists, err := cli.pgpUnlockerExists(vlt, fingerprint)
+	// Refuse a key that is already an unlocker before asking for anything,
+	// and again once the lock is held, since another command may have added
+	// it in the meantime
+	err = cli.checkPGPKeyNotAdded(vlt, gpgKeyID, fingerprint)
 	if err != nil {
-		return fmt.Errorf(
-			"could not check whether GPG key %s is already an unlocker: %w",
-			gpgKeyID, err,
-		)
+		return err
 	}
 
-	if exists {
-		return fmt.Errorf("GPG key %s %w", gpgKeyID, errGPGKeyAlreadyUnlocker)
+	release, err := cli.unlockThenLock(vlt)
+	if err != nil {
+		return err
+	}
+	defer release()
+
+	err = cli.checkPGPKeyNotAdded(vlt, gpgKeyID, fingerprint)
+	if err != nil {
+		return err
 	}
 
-	pgpUnlocker, err := secret.CreatePGPUnlocker(cli.fs, cli.stateDir,
-		gpgKeyID, fingerprint, cli.Mnemonic, cli.UnlockPassphrase)
+	pgpUnlocker, err := secret.CreatePGPUnlocker(vlt, gpgKeyID, fingerprint)
 	if err != nil {
 		return err
 	}
@@ -704,6 +735,27 @@ func (cli *Instance) UnlockerSelect(unlockerID string) error {
 	return vlt.SelectUnlocker(unlockerID)
 }
 
+// checkPGPKeyNotAdded fails when vlt already has a PGP unlocker for the GPG
+// key gpgKeyID, whose fingerprint is fingerprint, or when that cannot be
+// checked.
+func (cli *Instance) checkPGPKeyNotAdded(
+	vlt *vault.Vault, gpgKeyID, fingerprint string,
+) error {
+	exists, err := cli.pgpUnlockerExists(vlt, fingerprint)
+	if err != nil {
+		return fmt.Errorf(
+			"could not check whether GPG key %s is already an unlocker: %w",
+			gpgKeyID, err,
+		)
+	}
+
+	if exists {
+		return fmt.Errorf("GPG key %s %w", gpgKeyID, errGPGKeyAlreadyUnlocker)
+	}
+
+	return nil
+}
+
 // pgpUnlockerExists reports whether the vault already has a PGP unlocker
 // for the GPG key with the given fingerprint. It returns an error, and no
 // answer, when unlockers.d or an unlocker's metadata file cannot be read;