Ask before removing a secret, version, vault or unlocker (closes #39)

secret rm, secret version rm, secret vault remove and secret unlocker
remove ask [y/N] on a terminal, naming what they remove, and go ahead
only on y or yes. Without --force, a command whose stdin is not a
terminal fails at once. --force, now also on rm and version rm,
removes without asking; it replaces the old refusals to remove a vault
with secrets or the last unlocker without --force. The checks run and
the question is asked before the state directory lock is taken; under
the lock the checks run again, and nothing is removed if they would
ask a different question.

Model: opus-5-5

internal/cli/vault.go

@@ -31,8 +31,6 @@ var (
 	errPassphraseEnvNotSet = errors.New(
 		"SB_UNLOCK_PASSPHRASE environment variable not set")
 	errCannotRemoveLastVault = errors.New("cannot remove the last vault")
-	errVaultContainsSecrets  = errors.New(
-		"contains secrets; use --force to remove")
 )
 
 func newVaultCmd() *cobra.Command {
@@ -156,9 +154,12 @@ func newVaultRemoveCmd() *cobra.Command {
 		Use:     "remove <name>",
 		Aliases: []string{"rm"},
 		Short:   "Remove a vault",
-		Long: `Remove a vault. Requires --force if the vault contains ` +
-			`secrets. Will automatically switch to another vault if ` +
-			`removing the currently selected one.`,
+		Long: `Remove a vault and all its secrets. Asks for ` +
+			`confirmation first, naming how many secrets the vault ` +
+			`holds; when stdin is not a terminal, fails unless --force ` +
+			`is given. Will automatically switch to another vault if ` +
+			`removing the currently selected one. The last vault ` +
+			`cannot be removed.`,
 		Args:              cobra.ExactArgs(1),
 		ValidArgsFunction: getVaultNamesCompletionFunc(cli.fs, cli.stateDir),
 		RunE: func(cmd *cobra.Command, args []string) error {
@@ -173,7 +174,8 @@ func newVaultRemoveCmd() *cobra.Command {
 		},
 	}
 
-	cmd.Flags().BoolP("force", "f", false, "Force removal even if vault contains secrets")
+	cmd.Flags().BoolP("force", "f", false,
+		"Remove without asking for confirmation, even a vault that contains secrets")
 
 	return cmd
 }
@@ -537,27 +539,27 @@ func (cli *Instance) importMnemonic(cmd *cobra.Command, vaultName string) error
 	return nil
 }
 
-// vaultHasSecrets reports whether the vault directory contains any secrets
-func (cli *Instance) vaultHasSecrets(vaultDir string) (bool, error) {
+// countVaultSecrets returns the number of secrets in the vault directory
+func (cli *Instance) countVaultSecrets(vaultDir string) (int, error) {
 	secretsDir := filepath.Join(vaultDir, "secrets.d")
 
 	exists, err := afero.DirExists(cli.fs, secretsDir)
 	if err != nil {
-		return false, fmt.Errorf("failed to check secrets directory %s: %w",
+		return 0, fmt.Errorf("failed to check secrets directory %s: %w",
 			secretsDir, err)
 	}
 
 	if !exists {
-		return false, nil
+		return 0, nil
 	}
 
 	entries, err := afero.ReadDir(cli.fs, secretsDir)
 	if err != nil {
-		return false, fmt.Errorf("failed to read secrets directory %s: %w",
+		return 0, fmt.Errorf("failed to read secrets directory %s: %w",
 			secretsDir, err)
 	}
 
-	return len(entries) > 0, nil
+	return len(entries), nil
 }
 
 // switchAwayFromVault selects another vault as current before removal
@@ -586,88 +588,107 @@ func (cli *Instance) switchAwayFromVault(
 	return nil
 }
 
-// RemoveVault removes a vault, holding the state directory lock while
-// removeVault runs
+// RemoveVault removes a vault and all its secrets, after asking the user
+// to confirm unless force is set.
 func (cli *Instance) RemoveVault(cmd *cobra.Command, name string, force bool) error {
 	err := vault.ValidateVaultName(name)
 	if err != nil {
 		return err
 	}
 
-	release, err := vault.LockStateDir(cli.fs, cli.stateDir)
+	var found vaultToRemove
+
+	release, err := cli.askThenLock(cmd, force, func() (string, error) {
+		var err error
+
+		found, err = cli.findVaultToRemove(name)
+
+		return found.question, err
+	})
 	if err != nil {
 		return err
 	}
 	defer release()
 
-	return cli.removeVault(cmd, name, force)
-}
-
-// removeVault removes a vault with safety checks
-func (cli *Instance) removeVault(cmd *cobra.Command, name string, force bool) error {
-	// Get list of all vaults
-	vaults, err := vault.ListVaults(cli.fs, cli.stateDir)
-	if err != nil {
-		return fmt.Errorf("failed to list vaults: %w", err)
-	}
-
-	// Check if vault exists
-	if !slices.Contains(vaults, name) {
-		return fmt.Errorf("vault '%s' %w", name, errVaultDoesNotExist)
-	}
-
-	// Don't allow removing the last vault
-	if len(vaults) == 1 {
-		return errCannotRemoveLastVault
-	}
-
-	// Check if this is the current vault
-	currentVault, err := vault.GetCurrentVault(cli.fs, cli.stateDir)
-	if err != nil {
-		return fmt.Errorf("failed to get current vault: %w", err)
-	}
-
-	isCurrentVault := currentVault.GetName() == name
-
-	// Load the vault to check for secrets
-	vlt := vault.NewVault(cli.fs, cli.stateDir, name)
-
-	vaultDir, err := vlt.GetDirectory()
-	if err != nil {
-		return fmt.Errorf("failed to get vault directory: %w", err)
-	}
-
-	// Check if vault has secrets
-	hasSecrets, err := cli.vaultHasSecrets(vaultDir)
-	if err != nil {
-		return err
-	}
-
-	// Require --force if vault has secrets
-	if hasSecrets && !force {
-		return fmt.Errorf("vault '%s' %w", name, errVaultContainsSecrets)
-	}
-
 	// If removing current vault, switch to another vault first
-	if isCurrentVault {
-		err = cli.switchAwayFromVault(cmd, vaults, name)
+	if found.isCurrent {
+		err = cli.switchAwayFromVault(cmd, found.vaults, name)
 		if err != nil {
 			return err
 		}
 	}
 
 	// Remove the vault directory
-	err = secret.RemoveDirAtomic(cli.fs, vaultDir)
+	err = secret.RemoveDirAtomic(cli.fs, found.dir)
 	if err != nil {
 		return fmt.Errorf("failed to remove vault directory: %w", err)
 	}
 
 	cmd.Printf("Removed vault '%s'\n", name)
 
-	if hasSecrets {
+	if found.secrets > 0 {
 		cmd.Printf("Warning: Vault contained secrets that have been " +
 			"permanently deleted\n")
 	}
 
 	return nil
 }
+
+// vaultToRemove is what removing a vault removes, as findVaultToRemove
+// found it.
+type vaultToRemove struct {
+	// dir is the vault's directory, which holds all its secrets.
+	dir     string
+	secrets int
+	// vaults lists every vault, this one included, and isCurrent is set
+	// when this one is the current vault.
+	vaults    []string
+	isCurrent bool
+	// question names what is removed, for the user to confirm.
+	question string
+}
+
+// findVaultToRemove checks that the vault exists and is not the last one,
+// and counts its secrets.
+func (cli *Instance) findVaultToRemove(name string) (vaultToRemove, error) {
+	vaults, err := vault.ListVaults(cli.fs, cli.stateDir)
+	if err != nil {
+		return vaultToRemove{}, fmt.Errorf("failed to list vaults: %w", err)
+	}
+
+	if !slices.Contains(vaults, name) {
+		return vaultToRemove{},
+			fmt.Errorf("vault '%s' %w", name, errVaultDoesNotExist)
+	}
+
+	if len(vaults) == 1 {
+		return vaultToRemove{}, errCannotRemoveLastVault
+	}
+
+	currentVault, err := vault.GetCurrentVault(cli.fs, cli.stateDir)
+	if err != nil {
+		return vaultToRemove{},
+			fmt.Errorf("failed to get current vault: %w", err)
+	}
+
+	vaultDir, err := vault.NewVault(cli.fs, cli.stateDir, name).GetDirectory()
+	if err != nil {
+		return vaultToRemove{},
+			fmt.Errorf("failed to get vault directory: %w", err)
+	}
+
+	secrets, err := cli.countVaultSecrets(vaultDir)
+	if err != nil {
+		return vaultToRemove{}, err
+	}
+
+	return vaultToRemove{
+		dir:       vaultDir,
+		secrets:   secrets,
+		vaults:    vaults,
+		isCurrent: currentVault.GetName() == name,
+		question: fmt.Sprintf(
+			"Permanently remove vault '%s' and its %d secret(s)?",
+			name, secrets),
+	}, nil
+}