Make an unlocker's ID the name of its directory (closes #98)
check / check (push) Failing after 2s
check / check (push) Failing after 2s
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
This commit was merged in pull request #109.
This commit is contained in:
+21
-146
@@ -6,6 +6,7 @@ import (
|
||||
"errors"
|
||||
"fmt"
|
||||
"log"
|
||||
"maps"
|
||||
"os"
|
||||
"os/exec"
|
||||
"path/filepath"
|
||||
@@ -313,91 +314,8 @@ func newUnlockerSelectCmd() *cobra.Command {
|
||||
}
|
||||
}
|
||||
|
||||
// unlockerIDFromDir constructs an unlocker of the given metadata type
|
||||
// rooted at unlockerDir and returns its ID. Returns "" for unknown types
|
||||
// and, when includeSecureEnclave is false, for secure enclave unlockers.
|
||||
func unlockerIDFromDir(
|
||||
fs afero.Fs, unlockerDir string, metadata secret.UnlockerMetadata,
|
||||
includeSecureEnclave bool,
|
||||
) string {
|
||||
// Create the appropriate unlocker instance
|
||||
var unlocker secret.Unlocker
|
||||
|
||||
switch metadata.Type {
|
||||
case unlockerTypePassphrase:
|
||||
unlocker = secret.NewPassphraseUnlocker(fs, unlockerDir, metadata)
|
||||
case unlockerTypeKeychain:
|
||||
unlocker = secret.NewKeychainUnlocker(fs, unlockerDir, metadata)
|
||||
case unlockerTypePGP:
|
||||
unlocker = secret.NewPGPUnlocker(fs, unlockerDir, metadata)
|
||||
case unlockerTypeSecureEnclave:
|
||||
if includeSecureEnclave {
|
||||
unlocker = secret.NewSecureEnclaveUnlocker(fs, unlockerDir, metadata)
|
||||
}
|
||||
}
|
||||
|
||||
if unlocker == nil {
|
||||
return ""
|
||||
}
|
||||
|
||||
return unlocker.GetID()
|
||||
}
|
||||
|
||||
// findUnlockerIDByMetadata scans unlockersDir for the directory whose
|
||||
// stored metadata matches the given type and creation time and returns
|
||||
// the matching unlocker's ID. It returns ("", nil) when the directory is
|
||||
// readable but holds no match, and a non-nil error when the directory
|
||||
// itself cannot be read. Callers must distinguish the two: an unreadable
|
||||
// directory means the unlocker's real ID is unknowable, so the entry has
|
||||
// to be skipped rather than reported under a synthesized ID.
|
||||
//
|
||||
// A metadata file that cannot be read or parsed is skipped without a
|
||||
// warning: every caller gets metadata from vault.ListUnlockers first,
|
||||
// which has already warned about that directory.
|
||||
func findUnlockerIDByMetadata(
|
||||
fs afero.Fs, unlockersDir string, metadata secret.UnlockerMetadata,
|
||||
includeSecureEnclave bool,
|
||||
) (string, error) {
|
||||
files, err := afero.ReadDir(fs, unlockersDir)
|
||||
if err != nil {
|
||||
return "", fmt.Errorf(
|
||||
"failed to read unlockers directory %s: %w", unlockersDir, err,
|
||||
)
|
||||
}
|
||||
|
||||
for _, file := range files {
|
||||
if !file.IsDir() {
|
||||
continue
|
||||
}
|
||||
|
||||
unlockerDir := filepath.Join(unlockersDir, file.Name())
|
||||
metadataPath := filepath.Join(unlockerDir, "unlocker-metadata.json")
|
||||
|
||||
// Check if this is the right unlocker by comparing metadata
|
||||
metadataBytes, err := afero.ReadFile(fs, metadataPath)
|
||||
if err != nil {
|
||||
continue
|
||||
}
|
||||
|
||||
var diskMetadata secret.UnlockerMetadata
|
||||
|
||||
err = json.Unmarshal(metadataBytes, &diskMetadata)
|
||||
if err != nil {
|
||||
continue
|
||||
}
|
||||
|
||||
// Match by type and creation time
|
||||
if diskMetadata.Type == metadata.Type &&
|
||||
diskMetadata.CreatedAt.Equal(metadata.CreatedAt) {
|
||||
return unlockerIDFromDir(fs, unlockerDir, diskMetadata,
|
||||
includeSecureEnclave), nil
|
||||
}
|
||||
}
|
||||
|
||||
return "", nil
|
||||
}
|
||||
|
||||
// UnlockersList lists unlockers in the current vault
|
||||
// UnlockersList lists unlockers in the current vault, each under its ID,
|
||||
// the name of its directory in unlockers.d
|
||||
func (cli *Instance) UnlockersList(jsonOutput bool) error {
|
||||
// Get current vault
|
||||
vlt, err := vault.GetCurrentVault(cli.fs, cli.stateDir)
|
||||
@@ -413,58 +331,23 @@ func (cli *Instance) UnlockersList(jsonOutput bool) error {
|
||||
currentUnlockerID = currentUnlocker.GetID()
|
||||
}
|
||||
|
||||
// Get the metadata first
|
||||
unlockerMetadataList, err := vlt.ListUnlockers()
|
||||
unlockerMetadata, err := vlt.ListUnlockers()
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
// Load actual unlocker objects to get the proper IDs
|
||||
var unlockers []UnlockerInfo
|
||||
|
||||
for _, metadata := range unlockerMetadataList {
|
||||
// Create unlocker instance to get the proper ID
|
||||
vaultDir, err := vlt.GetDirectory()
|
||||
if err != nil {
|
||||
secret.Warn("Could not get vault directory while listing unlockers",
|
||||
"error", err)
|
||||
for _, unlockerID := range slices.Sorted(maps.Keys(unlockerMetadata)) {
|
||||
metadata := unlockerMetadata[unlockerID]
|
||||
|
||||
continue
|
||||
}
|
||||
|
||||
// Find the unlocker directory by type and created time
|
||||
unlockersDir := filepath.Join(vaultDir, "unlockers.d")
|
||||
|
||||
unlockerID, err := findUnlockerIDByMetadata(
|
||||
cli.fs, unlockersDir, metadata, true,
|
||||
)
|
||||
if err != nil {
|
||||
secret.Warn("Could not read unlockers directory, skipping unlocker",
|
||||
"unlockers_dir", unlockersDir, "error", err)
|
||||
|
||||
continue
|
||||
}
|
||||
|
||||
// Get the proper ID using the unlocker's ID() method
|
||||
var properID string
|
||||
if unlockerID != "" {
|
||||
properID = unlockerID
|
||||
} else {
|
||||
// Generate ID as fallback
|
||||
properID = fmt.Sprintf("%s-%s",
|
||||
metadata.CreatedAt.Format("2006-01-02.15.04"), metadata.Type)
|
||||
secret.Warn("Could not create unlocker instance, using fallback ID",
|
||||
"fallback_id", properID, "type", metadata.Type)
|
||||
}
|
||||
|
||||
unlockerInfo := UnlockerInfo{
|
||||
ID: properID,
|
||||
unlockers = append(unlockers, UnlockerInfo{
|
||||
ID: unlockerID,
|
||||
Type: metadata.Type,
|
||||
CreatedAt: metadata.CreatedAt,
|
||||
Flags: metadata.Flags,
|
||||
IsCurrent: properID == currentUnlockerID,
|
||||
}
|
||||
unlockers = append(unlockers, unlockerInfo)
|
||||
IsCurrent: unlockerID == currentUnlockerID,
|
||||
})
|
||||
}
|
||||
|
||||
if jsonOutput {
|
||||
@@ -697,9 +580,7 @@ func (cli *Instance) addPGPUnlocker(cmd *cobra.Command) error {
|
||||
}
|
||||
|
||||
// Check if this GPG key is already added
|
||||
expectedID := "pgp-" + fingerprint
|
||||
|
||||
exists, err := cli.checkUnlockerExists(vlt, expectedID)
|
||||
exists, err := cli.pgpUnlockerExists(vlt, fingerprint)
|
||||
if err != nil {
|
||||
return fmt.Errorf(
|
||||
"could not check whether GPG key %s is already an unlocker: %w",
|
||||
@@ -804,13 +685,7 @@ func (cli *Instance) findUnlockerToRemove(
|
||||
}
|
||||
|
||||
if len(unlockers) == 1 {
|
||||
lastID, err := findUnlockerIDByMetadata(
|
||||
cli.fs, unlockersDir, unlockers[0], true)
|
||||
if err != nil {
|
||||
return unlockerToRemove{}, err
|
||||
}
|
||||
|
||||
found.last = lastID == unlockerID
|
||||
_, found.last = unlockers[unlockerID]
|
||||
}
|
||||
|
||||
// unlockerID may instead name a directory left out of the list. If its
|
||||
@@ -889,16 +764,16 @@ func (cli *Instance) UnlockerSelect(unlockerID string) error {
|
||||
return vlt.SelectUnlocker(unlockerID)
|
||||
}
|
||||
|
||||
// checkUnlockerExists reports whether the vault already has an unlocker
|
||||
// with the given ID. It returns an error, and no answer, when unlockers.d
|
||||
// or an unlocker's metadata file cannot be read; the caller must then not
|
||||
// create the unlocker. It reads unlockers.d itself because
|
||||
// vault.ListUnlockers skips an unlocker it cannot read, which suits
|
||||
// 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;
|
||||
// the caller must then not create the unlocker. It reads unlockers.d itself
|
||||
// because vault.ListUnlockers skips an unlocker it cannot read, which suits
|
||||
// `unlocker list` but not this check: the skipped unlocker may be the
|
||||
// duplicate. A directory whose metadata file is missing or corrupt is not
|
||||
// a working unlocker and is passed over.
|
||||
func (cli *Instance) checkUnlockerExists(
|
||||
vlt *vault.Vault, unlockerID string,
|
||||
func (cli *Instance) pgpUnlockerExists(
|
||||
vlt *vault.Vault, fingerprint string,
|
||||
) (bool, error) {
|
||||
vaultDir, err := vlt.GetDirectory()
|
||||
if err != nil {
|
||||
@@ -937,14 +812,14 @@ func (cli *Instance) checkUnlockerExists(
|
||||
)
|
||||
}
|
||||
|
||||
var metadata secret.UnlockerMetadata
|
||||
var metadata secret.PGPUnlockerMetadata
|
||||
|
||||
err = json.Unmarshal(metadataBytes, &metadata)
|
||||
if err != nil {
|
||||
continue
|
||||
}
|
||||
|
||||
if unlockerIDFromDir(cli.fs, unlockerDir, metadata, true) == unlockerID {
|
||||
if metadata.Type == unlockerTypePGP && metadata.GPGKeyID == fingerprint {
|
||||
return true, nil
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user