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 <keyid>-pgp to pgp-<keyid>
- 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
This commit is contained in:
Jeffrey Paul 2025-07-21 18:57:58 +02:00
parent 09b3a1fcdc
commit 7af1e6efa8
2 changed files with 143 additions and 11 deletions

View File

@ -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 <type>",
Use: "add <type> [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
}

View File

@ -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: <keyid>-pgp
// Generate ID using GPG key ID: pgp-<keyid>
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)
}