From 7af1e6efa86bfe45e7fb18a4ace06a431573ef80 Mon Sep 17 00:00:00 2001 From: sneak Date: Mon, 21 Jul 2025 18:57:58 +0200 Subject: [PATCH] Improve PGP unlocker ergonomics - Support 'secret unlockers add pgp [keyid]' positional argument syntax - Automatically detect and use default GPG key when no key is specified - Change PGP unlocker ID format from -pgp to pgp- - Check if PGP key is already added before creating duplicate unlocker - Add getDefaultGPGKey() that checks gpgconf first, then falls back to first secret key - Export ResolveGPGKeyFingerprint() for use in CLI - Add checkUnlockerExists() helper to verify unlocker IDs The new behavior: - 'secret unlockers add pgp' uses default GPG key - 'secret unlockers add pgp KEYID' uses specified key - 'secret unlockers add pgp --keyid=KEYID' also works - Errors if key is already added or no default key exists --- internal/cli/unlockers.go | 144 +++++++++++++++++++++++++++++++-- internal/secret/pgpunlocker.go | 10 +-- 2 files changed, 143 insertions(+), 11 deletions(-) diff --git a/internal/cli/unlockers.go b/internal/cli/unlockers.go index 019bba2..847375c 100644 --- a/internal/cli/unlockers.go +++ b/internal/cli/unlockers.go @@ -4,6 +4,7 @@ import ( "encoding/json" "fmt" "os" + "os/exec" "path/filepath" "strings" "time" @@ -15,9 +16,44 @@ import ( "github.com/spf13/cobra" ) -// Import from init.go +// getDefaultGPGKey returns the default GPG key ID if available +func getDefaultGPGKey() (string, error) { + // First try to get the configured default key using gpgconf + cmd := exec.Command("gpgconf", "--list-options", "gpg") + output, err := cmd.Output() + if err == nil { + lines := strings.Split(string(output), "\n") + for _, line := range lines { + fields := strings.Split(line, ":") + if len(fields) > 9 && fields[0] == "default-key" && fields[9] != "" { + // The default key is in field 10 (index 9) + return fields[9], nil + } + } + } -// ... existing imports ... + // If no default key is configured, get the first secret key + cmd = exec.Command("gpg", "--list-secret-keys", "--with-colons") + output, err = cmd.Output() + if err != nil { + return "", fmt.Errorf("failed to list GPG keys: %w", err) + } + + // Parse output to find the first usable secret key + lines := strings.Split(string(output), "\n") + for _, line := range lines { + // sec line indicates a secret key + if strings.HasPrefix(line, "sec:") { + fields := strings.Split(line, ":") + // Field 5 contains the key ID + if len(fields) > 4 && fields[4] != "" { + return fields[4], nil + } + } + } + + return "", fmt.Errorf("no GPG secret keys found") +} func newUnlockersCmd() *cobra.Command { cmd := &cobra.Command{ @@ -55,13 +91,19 @@ func newUnlockersListCmd() *cobra.Command { func newUnlockersAddCmd() *cobra.Command { cmd := &cobra.Command{ - Use: "add ", + Use: "add [keyid]", Short: "Add a new unlocker", Long: `Add a new unlocker of the specified type (passphrase, keychain, pgp).`, - Args: cobra.ExactArgs(1), + Args: cobra.RangeArgs(1, 2), //nolint:mnd // Command accepts 1 or 2 arguments RunE: func(cmd *cobra.Command, args []string) error { cli := NewCLIInstance() + // For PGP type, check if keyid is provided as positional argument + if args[0] == "pgp" && len(args) == 2 { + // Override any flag value with the positional argument + _ = cmd.Flags().Set("keyid", args[1]) + } + return cli.UnlockersAdd(args[0], cmd) }, } @@ -300,14 +342,38 @@ func (cli *Instance) UnlockersAdd(unlockerType string, cmd *cobra.Command) error return nil case "pgp": - // Get GPG key ID from flag or environment variable + // Get GPG key ID from flag, environment, or default key var gpgKeyID string if flagKeyID, _ := cmd.Flags().GetString("keyid"); flagKeyID != "" { gpgKeyID = flagKeyID } else if envKeyID := os.Getenv(secret.EnvGPGKeyID); envKeyID != "" { gpgKeyID = envKeyID } else { - return fmt.Errorf("GPG key ID required: use --keyid flag or set SB_GPG_KEY_ID environment variable") + // Try to get the default GPG key + defaultKeyID, err := getDefaultGPGKey() + if err != nil { + return fmt.Errorf("no GPG key specified and no default key found: %w", err) + } + gpgKeyID = defaultKeyID + 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) + } + + // Resolve the GPG key ID to its fingerprint + fingerprint, err := secret.ResolveGPGKeyFingerprint(gpgKeyID) + if err != nil { + return fmt.Errorf("failed to resolve GPG key fingerprint: %w", err) + } + + // Check if this GPG key is already added + expectedID := fmt.Sprintf("pgp-%s", fingerprint) + if err := cli.checkUnlockerExists(vlt, expectedID); err != nil { + return fmt.Errorf("GPG key %s is already added as an unlocker", gpgKeyID) } pgpUnlocker, err := secret.CreatePGPUnlocker(cli.fs, cli.stateDir, gpgKeyID) @@ -380,3 +446,69 @@ func (cli *Instance) UnlockerSelect(unlockerID string) error { return vlt.SelectUnlocker(unlockerID) } + +// checkUnlockerExists checks if an unlocker with the given ID exists +func (cli *Instance) checkUnlockerExists(vlt *vault.Vault, unlockerID string) error { + // Get the list of unlockers and check if any match the ID + unlockers, err := vlt.ListUnlockers() + if err != nil { + return nil // If we can't list unlockers, assume it doesn't exist + } + + // Get vault directory to construct unlocker instances + vaultDir, err := vlt.GetDirectory() + if err != nil { + return nil + } + + // Check each unlocker's ID + for _, metadata := range unlockers { + // Construct the unlocker based on type to get its ID + unlockersDir := filepath.Join(vaultDir, "unlockers.d") + files, err := afero.ReadDir(cli.fs, unlockersDir) + if err != nil { + continue + } + + for _, file := range files { + if !file.IsDir() { + continue + } + + unlockerDir := filepath.Join(unlockersDir, file.Name()) + metadataPath := filepath.Join(unlockerDir, "unlocker-metadata.json") + + // Check if this matches our metadata + metadataBytes, err := afero.ReadFile(cli.fs, metadataPath) + if err != nil { + continue + } + + var diskMetadata secret.UnlockerMetadata + if err := json.Unmarshal(metadataBytes, &diskMetadata); err != nil { + continue + } + + // Match by type and creation time + if diskMetadata.Type == metadata.Type && diskMetadata.CreatedAt.Equal(metadata.CreatedAt) { + var unlocker secret.Unlocker + switch metadata.Type { + case "passphrase": + unlocker = secret.NewPassphraseUnlocker(cli.fs, unlockerDir, diskMetadata) + case "keychain": + unlocker = secret.NewKeychainUnlocker(cli.fs, unlockerDir, diskMetadata) + case "pgp": + unlocker = secret.NewPGPUnlocker(cli.fs, unlockerDir, diskMetadata) + } + + if unlocker != nil && unlocker.GetID() == unlockerID { + return fmt.Errorf("unlocker already exists") + } + + break + } + } + } + + return nil +} diff --git a/internal/secret/pgpunlocker.go b/internal/secret/pgpunlocker.go index c1281e2..1fedb17 100644 --- a/internal/secret/pgpunlocker.go +++ b/internal/secret/pgpunlocker.go @@ -128,7 +128,7 @@ func (p *PGPUnlocker) GetDirectory() string { // GetID implements Unlocker interface - generates ID from GPG key ID func (p *PGPUnlocker) GetID() string { - // Generate ID using GPG key ID: -pgp + // Generate ID using GPG key ID: pgp- gpgKeyID, err := p.GetGPGKeyID() if err != nil { // The vault metadata is corrupt - this is a fatal error @@ -136,7 +136,7 @@ func (p *PGPUnlocker) GetID() string { panic(fmt.Sprintf("PGP unlocker metadata is corrupt or missing GPG key ID: %v", err)) } - return fmt.Sprintf("%s-pgp", gpgKeyID) + return fmt.Sprintf("pgp-%s", gpgKeyID) } // Remove implements Unlocker interface - removes the PGP unlocker @@ -267,7 +267,7 @@ func CreatePGPUnlocker(fs afero.Fs, stateDir string, gpgKeyID string) (*PGPUnloc } // Step 9: Resolve the GPG key ID to its full fingerprint - fingerprint, err := resolveGPGKeyFingerprint(gpgKeyID) + fingerprint, err := ResolveGPGKeyFingerprint(gpgKeyID) if err != nil { return nil, fmt.Errorf("failed to resolve GPG key fingerprint: %w", err) } @@ -313,8 +313,8 @@ func validateGPGKeyID(keyID string) error { return nil } -// resolveGPGKeyFingerprint resolves any GPG key identifier to its full fingerprint -func resolveGPGKeyFingerprint(keyID string) (string, error) { +// ResolveGPGKeyFingerprint resolves any GPG key identifier to its full fingerprint +func ResolveGPGKeyFingerprint(keyID string) (string, error) { if err := validateGPGKeyID(keyID); err != nil { return "", fmt.Errorf("invalid GPG key ID: %w", err) }