Make an unlocker's ID the name of its directory (closes #98)

Keychain and Secure Enclave unlocker IDs were the creation time to the
minute plus the host name, and passphrase unlocker IDs the time to the
minute, so two created within one minute shared an ID, and `unlocker
select`, `unlocker remove` and the selection after `unlocker add` acted on
the older one. Every unlocker's ID is now its directory name, unique in
its vault. `vault.ListUnlockers` returns each unlocker's metadata keyed by
that name, so `unlocker list` and shell completion no longer find IDs by
matching metadata. PGP unlocker IDs were `pgp-<fingerprint>`; a second
PGP unlocker for one key is refused by comparing fingerprints in metadata.

Model: opus-5-5

internal/cli/unlockers_list_test.go

@@ -1,25 +1,13 @@
 // Unlocker List Tests
 //
-// Tests for `secret unlocker list` behavior when the unlockers.d directory,
-// or an unlocker's metadata in it, cannot be read while the listing is
-// being rendered:
+// Tests for `secret unlocker list` behavior when an unlocker's metadata
+// cannot be read or used:
 //
-//   - TestUnlockersListSkipsUnreadableUnlockersDir: an unreadable
-//     unlockers.d yields no rows rather than rows bearing synthesized IDs.
-//   - TestUnlockersListSkipsOnlyUnreadableEntries: a readable entry is
-//     still listed, with its real ID and its current-unlocker marker,
-//     when a later entry's scan fails.
 //   - TestUnlockersListToleratesCorruptMetadata: one unlocker's corrupt
 //     metadata does not stop the others from being listed.
 //   - TestUnlockersListSkipsUnreadableMetadata: an unlocker whose metadata
 //     file cannot be checked for or read is left out, and the other is
 //     still listed.
-//
-// The listing resolves each unlocker's real ID by rescanning unlockers.d
-// after the vault has already enumerated it. If that rescan fails the ID
-// is unknowable, so the entry must be skipped: a synthesized ID matches
-// no `unlocker remove` or `unlocker select` argument and would also
-// suppress the current-unlocker marker.
 
 //nolint:testpackage // white-box test of unexported internals
 package cli
@@ -48,18 +36,16 @@ const (
 	// listTestVaultName is the name of that synthetic vault.
 	listTestVaultName = "default"
 
-	// listTestGPGKeyID is the GPG key ID recorded in the readable PGP
-	// unlocker's metadata. The unlocker's real ID is derived from it, and
-	// differs from the timestamp-derived fallback ID.
+	// listTestGPGKeyID is the GPG key ID recorded, with a letter appended,
+	// in the PGP unlockers' metadata.
 	listTestGPGKeyID = "DEADBEEFDEADBEEF"
 
 	// listTestUnlockerDirOne and listTestUnlockerDirTwo are the unlocker
-	// directory names under unlockers.d.
+	// directory names under unlockers.d, and so the unlockers' IDs.
 	listTestUnlockerDirOne = "host-pgp-2026-08-09"
 	listTestUnlockerDirTwo = "host-pgp-2026-08-10"
 
-	// listTestUnlockersDirName is the directory the listing rescans to
-	// resolve unlocker IDs.
+	// listTestUnlockersDirName is the directory holding the unlockers.
 	listTestUnlockersDirName = "unlockers.d"
 
 	// listTestMetadataFileName is the per-unlocker metadata file name.
@@ -74,25 +60,16 @@ const (
 // a successful open of unlockers.d.
 var errUnlockersDirUnreadable = errors.New("permission denied")
 
-// unlockersDirFailFs makes unlockers.d unreadable once it has been opened
-// successfully openBudget times. This reproduces the directory becoming
-// unreadable (permission change, partially restored backup, EIO) between
-// the vault's own enumeration and the per-entry rescan that resolves
-// unlocker IDs.
+// unlockersDirFailFs fails every open of unlockers.d, as when the
+// directory cannot be read.
 type unlockersDirFailFs struct {
 	afero.Fs
-
-	openBudget int
-	opens      int
 }
 
 //nolint:ireturn // afero.File is the interface required by afero.Fs
 func (f *unlockersDirFailFs) Open(name string) (afero.File, error) {
 	if filepath.Base(name) == listTestUnlockersDirName {
-		f.opens++
-		if f.opens > f.openBudget {
-			return nil, errUnlockersDirUnreadable
-		}
+		return nil, errUnlockersDirUnreadable
 	}
 
 	//nolint:wrapcheck // test double must return the wrapped Fs error as-is
@@ -142,8 +119,8 @@ func (f *metadataStatFailFs) Stat(name string) (os.FileInfo, error) {
 	return f.Fs.Stat(name)
 }
 
-// writePGPUnlocker writes a PGP unlocker directory with metadata that
-// yields the real ID "pgp-<keyID>".
+// writePGPUnlocker writes a PGP unlocker directory named dirName, with
+// metadata recording the GPG key ID keyID.
 func writePGPUnlocker(
 	t *testing.T, fs afero.Fs, unlockersDir, dirName string,
 	createdAt time.Time, keyID string,
@@ -224,44 +201,6 @@ func listUnlockersJSON(t *testing.T, fs afero.Fs) []UnlockerInfo {
 	return decoded.Unlockers
 }
 
-// TestUnlockersListSkipsUnreadableUnlockersDir asserts that an unlockers.d
-// which becomes unreadable after the vault enumerated it produces no rows,
-// rather than rows carrying fabricated fallback IDs.
-func TestUnlockersListSkipsUnreadableUnlockersDir(t *testing.T) {
-	t.Parallel()
-
-	base := newListTestVault(t, 1)
-	// Budget of one: the vault's own ListUnlockers scan succeeds, the
-	// per-entry rescan that resolves the ID fails.
-	fs := &unlockersDirFailFs{Fs: base, openBudget: 1}
-
-	unlockers := listUnlockersJSON(t, fs)
-
-	assert.Empty(t, unlockers,
-		"an unreadable unlockers.d must yield no rows, not fabricated IDs")
-}
-
-// TestUnlockersListSkipsOnlyUnreadableEntries asserts that a readable
-// entry survives with its real ID and current-unlocker marker when a later
-// entry's rescan fails.
-func TestUnlockersListSkipsOnlyUnreadableEntries(t *testing.T) {
-	t.Parallel()
-
-	base := newListTestVault(t, 2)
-	// Budget of two: ListUnlockers plus the first entry's rescan succeed,
-	// the second entry's rescan fails.
-	fs := &unlockersDirFailFs{Fs: base, openBudget: 2}
-
-	unlockers := listUnlockersJSON(t, fs)
-
-	require.Len(t, unlockers, 1,
-		"only the entry whose directory was readable may be listed")
-	assert.Equal(t, "pgp-"+listTestGPGKeyID+"A", unlockers[0].ID,
-		"the surviving row must carry the real unlocker ID")
-	assert.True(t, unlockers[0].IsCurrent,
-		"the current-unlocker marker must survive the skip")
-}
-
 // TestUnlockersListReadableEntriesAreListed is the control case: with a
 // fully readable unlockers.d every entry is listed with its real ID.
 func TestUnlockersListReadableEntriesAreListed(t *testing.T) {
@@ -272,20 +211,21 @@ func TestUnlockersListReadableEntriesAreListed(t *testing.T) {
 	unlockers := listUnlockersJSON(t, base)
 
 	require.Len(t, unlockers, 2)
-	assert.Equal(t, "pgp-"+listTestGPGKeyID+"A", unlockers[0].ID)
-	assert.Equal(t, "pgp-"+listTestGPGKeyID+"B", unlockers[1].ID)
+	assert.Equal(t, listTestUnlockerDirOne, unlockers[0].ID)
+	assert.Equal(t, listTestUnlockerDirTwo, unlockers[1].ID)
 	assert.True(t, unlockers[0].IsCurrent)
 	assert.False(t, unlockers[1].IsCurrent)
 }
 
 // TestUnlockersListToleratesCorruptMetadata asserts that one unlocker with
 // corrupt metadata does not stop the listing. Metadata that is not JSON
-// leaves that unlocker out; PGP metadata without a usable GPG key ID lists
-// it as "pgp-unknown". The healthy unlocker is listed with its real ID.
+// leaves that unlocker out; PGP metadata without a usable GPG key ID, and
+// metadata of an unknown type, are still listed, under the directory name
+// like any other. The healthy unlocker is listed with its real ID.
 func TestUnlockersListToleratesCorruptMetadata(t *testing.T) {
 	t.Parallel()
 
-	healthyID := "pgp-" + listTestGPGKeyID + "A"
+	healthyID := listTestUnlockerDirOne
 
 	tests := []struct {
 		name     string
@@ -300,12 +240,17 @@ func TestUnlockersListToleratesCorruptMetadata(t *testing.T) {
 		{
 			name:     "GPG key ID of the wrong type",
 			metadata: `{"type": "pgp", "gpgKeyId": 42}`,
-			wantIDs:  []string{healthyID, "pgp-unknown"},
+			wantIDs:  []string{healthyID, listTestUnlockerDirTwo},
 		},
 		{
 			name:     "GPG key ID missing",
 			metadata: `{"type": "pgp"}`,
-			wantIDs:  []string{healthyID, "pgp-unknown"},
+			wantIDs:  []string{healthyID, listTestUnlockerDirTwo},
+		},
+		{
+			name:     "unknown type",
+			metadata: `{"type": "unknown"}`,
+			wantIDs:  []string{healthyID, listTestUnlockerDirTwo},
 		},
 	}
 
@@ -371,7 +316,7 @@ func TestUnlockersListSkipsUnreadableMetadata(t *testing.T) {
 
 			require.Len(t, unlockers, 1,
 				"only the unlocker with usable metadata may be listed")
-			assert.Equal(t, "pgp-"+listTestGPGKeyID+"B", unlockers[0].ID,
+			assert.Equal(t, listTestUnlockerDirTwo, unlockers[0].ID,
 				"the listed row must carry the real unlocker ID")
 		})
 	}