Make another unlocker current when the current one is removed (closes #137)
check / check (push) Canceled after 0s

Removing the vault's current unlocker left current-unlocker naming it,
so every command that opened the vault failed although another
unlocker remained, and the error pointed at the mnemonic.
RemoveUnlocker now makes the first other unlocker, in the order
`secret unlocker list` shows, current before the removal and returns
its ID, which `secret unlocker remove` prints; removing the last
unlocker also removes current-unlocker. When current-unlocker names an
unlocker that does not exist and the vault has others, the error wraps
ErrUnlockerNotFound and says to run `secret unlocker select`, without
the mnemonic advice.

Unverified: no test covers that advice for a vault other than the
current one, which gets `secret vault select` first.

Model: opus-5-5
This commit is contained in:
2026-10-14 18:16:59 +00:00
parent fd6c5df1a8
commit b9c552c3cc
8 changed files with 253 additions and 25 deletions
+104 -7
View File
@@ -5,8 +5,10 @@ import (
"errors"
"fmt"
"log/slog"
"maps"
"os"
"path/filepath"
"slices"
"strings"
"time"
@@ -52,6 +54,11 @@ func (v *Vault) GetCurrentUnlocker() (secret.Unlocker, error) {
return nil, err
}
err = v.checkCurrentUnlockerExists(unlockerDir)
if err != nil {
return nil, err
}
secret.DebugWith("Resolved unlocker directory",
slog.String("unlocker_dir", unlockerDir),
slog.String("vault_name", v.Name),
@@ -119,6 +126,36 @@ func (v *Vault) resolveUnlockerDirectory(currentUnlockerPath string) (string, er
return absolutePath, nil
}
// checkCurrentUnlockerExists fails with ErrUnlockerNotFound, saying to
// select another unlocker, when unlockerDir, the unlocker directory that
// current-unlocker names, does not exist and the vault has other unlockers.
// When the vault has none, it returns nil: reading the missing unlocker then
// fails, with the advice that the mnemonic still opens the vault.
func (v *Vault) checkCurrentUnlockerExists(unlockerDir string) error {
exists, err := afero.DirExists(v.fs, unlockerDir)
if err != nil {
return fmt.Errorf("failed to check for the current unlocker: %w", err)
}
if exists {
return nil
}
unlockers, err := v.ListUnlockers()
if err != nil {
return err
}
if len(unlockers) == 0 {
return nil
}
return fmt.Errorf("unlocker with ID %s %w; the vault '%s' has other "+
"unlockers: run %s with the ID of one of them to make it the current "+
"unlocker", filepath.Base(unlockerDir), ErrUnlockerNotFound, v.Name,
v.withVaultSelect("'secret unlocker select'"))
}
// findUnlockerByID finds an unlocker by its ID and returns the unlocker
// instance and its directory path. A directory that ListUnlockers skips is
// skipped here too, with the same warning. Such a directory has no ID: if
@@ -282,10 +319,16 @@ func (v *Vault) HasUnlocker(unlockerID string) (bool, error) {
// RemoveUnlocker removes an unlocker from this vault. An unlocker
// directory that ListUnlockers skips is removed by its directory name; its
// type is unknown, so only the directory is removed.
func (v *Vault) RemoveUnlocker(unlockerID string) error {
//
// It returns the ID of the unlocker it made current, or "" when it made none
// current. If the removed unlocker is the current one, the vault's first
// other unlocker in ID order, the order `secret unlocker list` shows, is
// made current before the removal; with no other, current-unlocker is
// removed after it.
func (v *Vault) RemoveUnlocker(unlockerID string) (string, error) {
vaultDir, err := v.GetDirectory()
if err != nil {
return err
return "", err
}
// Find the unlocker directory and create the unlocker instance
@@ -294,19 +337,73 @@ func (v *Vault) RemoveUnlocker(unlockerID string) error {
// Find the unlocker by ID
unlocker, unlockerDir, err := v.findUnlockerByID(unlockersDir, unlockerID)
if err != nil {
return err
return "", err
}
if unlockerDir == "" {
return fmt.Errorf("unlocker with ID %s %w", unlockerID, ErrUnlockerNotFound)
return "", fmt.Errorf("unlocker with ID %s %w", unlockerID, ErrUnlockerNotFound)
}
currentUnlockerPath := filepath.Join(vaultDir, "current-unlocker")
currentDir, err := v.resolveUnlockerDirectory(currentUnlockerPath)
if err != nil && !errors.Is(err, os.ErrNotExist) {
return "", err
}
isCurrent := currentDir == unlockerDir
newCurrentID := ""
if isCurrent {
newCurrentID, err = v.selectOtherUnlocker(unlockerID)
if err != nil {
return "", err
}
}
if unlocker == nil {
return secret.RemoveDirAtomic(v.fs, unlockerDir)
err = secret.RemoveDirAtomic(v.fs, unlockerDir)
} else {
err = unlocker.Remove()
}
// Use the unlocker's Remove method
return unlocker.Remove()
if err != nil {
return "", err
}
if isCurrent && newCurrentID == "" {
err = v.fs.Remove(currentUnlockerPath)
if err != nil {
return "", fmt.Errorf("failed to remove current-unlocker file: %w", err)
}
}
return newCurrentID, nil
}
// selectOtherUnlocker makes the vault's first unlocker in ID order other
// than unlockerID the current unlocker and returns its ID. It returns ""
// when the vault has no other unlocker.
func (v *Vault) selectOtherUnlocker(unlockerID string) (string, error) {
unlockers, err := v.ListUnlockers()
if err != nil {
return "", err
}
for _, id := range slices.Sorted(maps.Keys(unlockers)) {
if id == unlockerID {
continue
}
err = v.SelectUnlocker(id)
if err != nil {
return "", err
}
return id, nil
}
return "", nil
}
// SelectUnlocker selects an unlocker as current for this vault