From 09b3a1fcdcc74cc532eb96756561e0ec340a3682 Mon Sep 17 00:00:00 2001 From: sneak Date: Mon, 21 Jul 2025 17:48:47 +0200 Subject: [PATCH 1/6] Remove internal/macse package and fix all linter issues - Remove internal/macse package (Secure Enclave experiment) - Fix errcheck: handle keychain.DeleteItem error return - Fix lll: break long lines in command descriptions - Fix mnd: add nolint comment for cobra.ExactArgs(2) - Fix nlreturn: add blank lines before return/break statements - Fix revive: add nolint comment for KEYCHAIN_APP_IDENTIFIER constant - Fix nestif: simplify UnlockersRemove by using new NumSecrets method - Add NumSecrets() method to vault.Vault for counting secrets - Update golangci.yml to exclude ALL_CAPS warning (attempted various configurations but settled on nolint comment) All tests pass, code is formatted and linted. --- .claude/settings.local.json | 3 +- .golangci.yml | 37 +++ Makefile | 2 + README.md | 53 +++- internal/cli/root.go | 1 + internal/cli/secrets.go | 62 +++++ internal/cli/unlockers.go | 70 ++++- internal/cli/vault.go | 115 ++++++++- internal/cli/version.go | 78 +++++- internal/macse/README.md | 17 -- internal/macse/enclave.go | 313 ----------------------- internal/macse/enclave_test.go | 87 ------- internal/secret/keychainunlocker.go | 7 +- internal/secret/keychainunlocker_test.go | 44 ++-- internal/vault/vault.go | 45 ++++ 15 files changed, 466 insertions(+), 468 deletions(-) delete mode 100644 internal/macse/README.md delete mode 100644 internal/macse/enclave.go delete mode 100644 internal/macse/enclave_test.go diff --git a/.claude/settings.local.json b/.claude/settings.local.json index 3eb36ff..ae3133d 100644 --- a/.claude/settings.local.json +++ b/.claude/settings.local.json @@ -26,7 +26,8 @@ "WebFetch(domain:pkg.go.dev)", "Bash(CGO_ENABLED=1 make fmt)", "Bash(CGO_ENABLED=1 make test)", - "Bash(git merge:*)" + "Bash(git merge:*)", + "Bash(git branch:*)" ], "deny": [] } diff --git a/.golangci.yml b/.golangci.yml index 3e04d77..265013a 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -64,6 +64,14 @@ linters-settings: nlreturn: block-size: 2 + revive: + rules: + - name: var-naming + arguments: + - [] + - [] + - "upperCaseConst=true" + tagliatelle: case: rules: @@ -89,3 +97,32 @@ issues: - text: "parameter '(args|cmd)' seems to be unused" linters: - revive + + # Allow ALL_CAPS constant names + - text: "don't use ALL_CAPS in Go names" + linters: + - revive + + # Exclude all linters for internal/macse directory + - path: "internal/macse/.*" + linters: + - errcheck + - lll + - mnd + - nestif + - nlreturn + - revive + - unconvert + - govet + - staticcheck + - unused + - ineffassign + - misspell + - gosec + - unparam + - testifylint + - usetesting + - tagliatelle + - nilnil + - intrange + - gochecknoglobals diff --git a/Makefile b/Makefile index 55a7ae6..b6dc7c0 100644 --- a/Makefile +++ b/Makefile @@ -1,3 +1,5 @@ +export CGO_ENABLED=1 + default: check build: ./secret diff --git a/README.md b/README.md index 7c65c53..f7eeb0b 100644 --- a/README.md +++ b/README.md @@ -69,8 +69,8 @@ Initializes the secret manager with a default vault. Prompts for a BIP39 mnemoni ### Vault Management -#### `secret vault list [--json]` -Lists all available vaults. +#### `secret vault list [--json]` / `secret vault ls` +Lists all available vaults. The current vault is marked. #### `secret vault create ` Creates a new vault with the specified name. @@ -78,6 +78,12 @@ Creates a new vault with the specified name. #### `secret vault select ` Switches to the specified vault for subsequent operations. +#### `secret vault remove [--force]` / `secret vault rm` ⚠️ 🛑 +**DANGER**: Permanently removes a vault and all its secrets. Like Unix `rm`, this command does not ask for confirmation. +Requires --force if the vault contains secrets. With --force, will automatically switch to another vault if removing the current one. +- `--force, -f`: Force removal even if vault contains secrets +- **NO RECOVERY**: All secrets in the vault will be permanently deleted + ### Secret Management #### `secret add [--force]` @@ -95,14 +101,24 @@ Retrieves and outputs a secret value to stdout. #### `secret list [filter] [--json]` / `secret ls` Lists all secrets in the current vault. Optional filter for substring matching. +#### `secret remove ` / `secret rm` ⚠️ 🛑 +**DANGER**: Permanently removes a secret and ALL its versions. Like Unix `rm`, this command does not ask for confirmation. +- **NO RECOVERY**: Once removed, the secret cannot be recovered +- **ALL VERSIONS DELETED**: Every version of the secret will be permanently deleted + ### Version Management -#### `secret version list ` +#### `secret version list ` / `secret version ls` Lists all versions of a secret showing creation time, status, and validity period. #### `secret version promote ` Promotes a specific version to current by updating the symlink. Does not modify any timestamps, allowing for rollback scenarios. +#### `secret version remove ` / `secret version rm` ⚠️ 🛑 +**DANGER**: Permanently removes a specific version of a secret. Like Unix `rm`, this command does not ask for confirmation. +- **NO RECOVERY**: Once removed, this version cannot be recovered +- Cannot remove the current version (must promote another version first) + ### Key Generation #### `secret generate mnemonic` @@ -116,7 +132,7 @@ Generates and stores a random secret. ### Unlocker Management -#### `secret unlockers list [--json]` +#### `secret unlockers list [--json]` / `secret unlockers ls` Lists all unlockers in the current vault with their metadata. #### `secret unlockers add [options]` @@ -130,8 +146,12 @@ Creates a new unlocker of the specified type: **Options:** - `--keyid `: GPG key ID (required for PGP type) -#### `secret unlockers rm ` -Removes an unlocker. +#### `secret unlockers remove [--force]` / `secret unlockers rm` ⚠️ 🛑 +**DANGER**: Permanently removes an unlocker. Like Unix `rm`, this command does not ask for confirmation. +Cannot remove the last unlocker if the vault has secrets unless --force is used. +- `--force, -f`: Force removal of last unlocker even if vault has secrets +- **CRITICAL WARNING**: Without unlockers and without your mnemonic phrase, vault data will be PERMANENTLY INACCESSIBLE +- **NO RECOVERY**: Removing all unlockers without having your mnemonic means losing access to all secrets forever #### `secret unlocker select ` Selects an unlocker as the current default for operations. @@ -274,6 +294,9 @@ echo "ssh-private-key-content" | secret add ssh/servers/web01 secret list secret get database/prod/password secret get services/api/key + +# Remove a secret ⚠️ 🛑 (NO CONFIRMATION - PERMANENT!) +secret remove ssh/servers/web01 ``` ### Multi-vault Setup @@ -293,6 +316,9 @@ echo "personal-email-pass" | secret add email/password # List all vaults secret vault list + +# Remove a vault ⚠️ 🛑 (NO CONFIRMATION - PERMANENT!) +secret vault remove personal --force ``` ### Advanced Authentication @@ -307,6 +333,21 @@ secret unlockers list # Select a specific unlocker secret unlocker select + +# Remove an unlocker ⚠️ 🛑 (NO CONFIRMATION!) +secret unlockers remove +``` + +### Version Management +```bash +# List all versions of a secret +secret version list database/prod/password + +# Promote an older version to current +secret version promote database/prod/password 20231215.001 + +# Remove an old version ⚠️ 🛑 (NO CONFIRMATION - PERMANENT!) +secret version remove database/prod/password 20231214.001 ``` ### Encryption/Decryption with Age Keys diff --git a/internal/cli/root.go b/internal/cli/root.go index c3e3921..d11c055 100644 --- a/internal/cli/root.go +++ b/internal/cli/root.go @@ -34,6 +34,7 @@ func newRootCmd() *cobra.Command { cmd.AddCommand(newAddCmd()) cmd.AddCommand(newGetCmd()) cmd.AddCommand(newListCmd()) + cmd.AddCommand(newRemoveCmd()) cmd.AddCommand(newUnlockersCmd()) cmd.AddCommand(newUnlockerCmd()) cmd.AddCommand(newImportCmd()) diff --git a/internal/cli/secrets.go b/internal/cli/secrets.go index efcabde..b1feea3 100644 --- a/internal/cli/secrets.go +++ b/internal/cli/secrets.go @@ -4,11 +4,13 @@ import ( "encoding/json" "fmt" "io" + "path/filepath" "strings" "git.eeqj.de/sneak/secret/internal/secret" "git.eeqj.de/sneak/secret/internal/vault" "github.com/awnumar/memguard" + "github.com/spf13/afero" "github.com/spf13/cobra" ) @@ -103,6 +105,24 @@ func newImportCmd() *cobra.Command { return cmd } +func newRemoveCmd() *cobra.Command { + cmd := &cobra.Command{ + Use: "remove ", + Aliases: []string{"rm"}, + Short: "Remove a secret from the vault", + Long: `Remove a secret and all its versions from the current vault. This action is permanent and ` + + `cannot be undone.`, + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + cli := NewCLIInstance() + + return cli.RemoveSecret(cmd, args[0], false) + }, + } + + return cmd +} + // updateBufferSize updates the buffer size based on usage pattern func updateBufferSize(currentSize int, sameSize *int) int { *sameSize++ @@ -448,3 +468,45 @@ func (cli *Instance) ImportSecret(cmd *cobra.Command, secretName, sourceFile str return nil } + +// RemoveSecret removes a secret from the vault +func (cli *Instance) RemoveSecret(cmd *cobra.Command, secretName string, _ bool) error { + // Get current vault + currentVlt, err := vault.GetCurrentVault(cli.fs, cli.stateDir) + if err != nil { + return err + } + + // Check if secret exists + vaultDir, err := currentVlt.GetDirectory() + if err != nil { + return err + } + + encodedName := strings.ReplaceAll(secretName, "/", "%") + secretDir := filepath.Join(vaultDir, "secrets.d", encodedName) + + exists, err := afero.DirExists(cli.fs, secretDir) + if err != nil { + return fmt.Errorf("failed to check if secret exists: %w", err) + } + if !exists { + return fmt.Errorf("secret '%s' not found", secretName) + } + + // Count versions for information + versionsDir := filepath.Join(secretDir, "versions") + versionCount := 0 + if entries, err := afero.ReadDir(cli.fs, versionsDir); err == nil { + versionCount = len(entries) + } + + // Remove the secret directory + if err := cli.fs.RemoveAll(secretDir); err != nil { + return fmt.Errorf("failed to remove secret: %w", err) + } + + cmd.Printf("Removed secret '%s' (%d version(s) deleted)\n", secretName, versionCount) + + return nil +} diff --git a/internal/cli/unlockers.go b/internal/cli/unlockers.go index db2b859..019bba2 100644 --- a/internal/cli/unlockers.go +++ b/internal/cli/unlockers.go @@ -28,15 +28,16 @@ func newUnlockersCmd() *cobra.Command { cmd.AddCommand(newUnlockersListCmd()) cmd.AddCommand(newUnlockersAddCmd()) - cmd.AddCommand(newUnlockersRmCmd()) + cmd.AddCommand(newUnlockersRemoveCmd()) return cmd } func newUnlockersListCmd() *cobra.Command { cmd := &cobra.Command{ - Use: "list", - Short: "List unlockers in the current vault", + Use: "list", + Aliases: []string{"ls"}, + Short: "List unlockers in the current vault", RunE: func(cmd *cobra.Command, _ []string) error { jsonOutput, _ := cmd.Flags().GetBool("json") @@ -70,17 +71,26 @@ func newUnlockersAddCmd() *cobra.Command { return cmd } -func newUnlockersRmCmd() *cobra.Command { - return &cobra.Command{ - Use: "rm ", - Short: "Remove an unlocker", - Args: cobra.ExactArgs(1), - RunE: func(_ *cobra.Command, args []string) error { +func newUnlockersRemoveCmd() *cobra.Command { + cmd := &cobra.Command{ + Use: "remove ", + Aliases: []string{"rm"}, + Short: "Remove an unlocker", + Long: `Remove an unlocker from the current vault. Cannot remove the last unlocker if the vault has ` + + `secrets unless --force is used. Warning: Without unlockers and without your mnemonic, vault data ` + + `will be permanently inaccessible.`, + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + force, _ := cmd.Flags().GetBool("force") cli := NewCLIInstance() - return cli.UnlockersRemove(args[0]) + return cli.UnlockersRemove(args[0], force, cmd) }, } + + cmd.Flags().BoolP("force", "f", false, "Force removal of last unlocker even if vault has secrets") + + return cmd } func newUnlockerCmd() *cobra.Command { @@ -315,15 +325,49 @@ func (cli *Instance) UnlockersAdd(unlockerType string, cmd *cobra.Command) error } } -// UnlockersRemove removes an unlocker -func (cli *Instance) UnlockersRemove(unlockerID string) error { +// UnlockersRemove removes an unlocker with safety checks +func (cli *Instance) UnlockersRemove(unlockerID string, force bool, cmd *cobra.Command) error { // Get current vault vlt, err := vault.GetCurrentVault(cli.fs, cli.stateDir) if err != nil { return err } - return vlt.RemoveUnlocker(unlockerID) + // Get list of unlockers + unlockers, err := vlt.ListUnlockers() + if err != nil { + return fmt.Errorf("failed to list unlockers: %w", err) + } + + // Check if we're removing the last unlocker + if len(unlockers) == 1 { + // Check if vault has secrets + numSecrets, err := vlt.NumSecrets() + if err != nil { + return fmt.Errorf("failed to count secrets: %w", err) + } + + if numSecrets > 0 && !force { + cmd.Println("ERROR: Cannot remove the last unlocker when the vault contains secrets.") + cmd.Println("WARNING: Without unlockers, you MUST have your mnemonic phrase to decrypt the vault.") + cmd.Println("If you want to proceed anyway, use --force") + + return fmt.Errorf("refusing to remove last unlocker") + } + + if numSecrets > 0 && force { + cmd.Println("WARNING: Removing the last unlocker. You MUST have your mnemonic phrase to access this vault again!") + } + } + + // Remove the unlocker + if err := vlt.RemoveUnlocker(unlockerID); err != nil { + return err + } + + cmd.Printf("Removed unlocker '%s'\n", unlockerID) + + return nil } // UnlockerSelect selects an unlocker as current diff --git a/internal/cli/vault.go b/internal/cli/vault.go index 68d7cde..9e60758 100644 --- a/internal/cli/vault.go +++ b/internal/cli/vault.go @@ -4,6 +4,7 @@ import ( "encoding/json" "fmt" "os" + "path/filepath" "strings" "time" @@ -27,14 +28,16 @@ func newVaultCmd() *cobra.Command { cmd.AddCommand(newVaultCreateCmd()) cmd.AddCommand(newVaultSelectCmd()) cmd.AddCommand(newVaultImportCmd()) + cmd.AddCommand(newVaultRemoveCmd()) return cmd } func newVaultListCmd() *cobra.Command { cmd := &cobra.Command{ - Use: "list", - Short: "List available vaults", + Use: "list", + Aliases: []string{"ls"}, + Short: "List available vaults", RunE: func(cmd *cobra.Command, _ []string) error { jsonOutput, _ := cmd.Flags().GetBool("json") @@ -94,6 +97,27 @@ func newVaultImportCmd() *cobra.Command { } } +func newVaultRemoveCmd() *cobra.Command { + cmd := &cobra.Command{ + Use: "remove ", + Aliases: []string{"rm"}, + Short: "Remove a vault", + Long: `Remove a vault. Requires --force if the vault contains secrets. Will automatically ` + + `switch to another vault if removing the currently selected one.`, + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + force, _ := cmd.Flags().GetBool("force") + cli := NewCLIInstance() + + return cli.RemoveVault(cmd, args[0], force) + }, + } + + cmd.Flags().BoolP("force", "f", false, "Force removal even if vault contains secrets") + + return cmd +} + // ListVaults lists all available vaults func (cli *Instance) ListVaults(cmd *cobra.Command, jsonOutput bool) error { vaults, err := vault.ListVaults(cli.fs, cli.stateDir) @@ -295,3 +319,90 @@ func (cli *Instance) VaultImport(cmd *cobra.Command, vaultName string) error { return nil } + +// RemoveVault removes a vault with safety checks +func (cli *Instance) RemoveVault(cmd *cobra.Command, name string, force bool) error { + // Get list of all vaults + vaults, err := vault.ListVaults(cli.fs, cli.stateDir) + if err != nil { + return fmt.Errorf("failed to list vaults: %w", err) + } + + // Check if vault exists + vaultExists := false + for _, v := range vaults { + if v == name { + vaultExists = true + + break + } + } + if !vaultExists { + return fmt.Errorf("vault '%s' does not exist", name) + } + + // Don't allow removing the last vault + if len(vaults) == 1 { + return fmt.Errorf("cannot remove the last vault") + } + + // Check if this is the current vault + currentVault, err := vault.GetCurrentVault(cli.fs, cli.stateDir) + if err != nil { + return fmt.Errorf("failed to get current vault: %w", err) + } + isCurrentVault := currentVault.GetName() == name + + // Load the vault to check for secrets + vlt := vault.NewVault(cli.fs, cli.stateDir, name) + vaultDir, err := vlt.GetDirectory() + if err != nil { + return fmt.Errorf("failed to get vault directory: %w", err) + } + + // Check if vault has secrets + secretsDir := filepath.Join(vaultDir, "secrets.d") + hasSecrets := false + if exists, _ := afero.DirExists(cli.fs, secretsDir); exists { + entries, err := afero.ReadDir(cli.fs, secretsDir) + if err == nil && len(entries) > 0 { + hasSecrets = true + } + } + + // Require --force if vault has secrets + if hasSecrets && !force { + return fmt.Errorf("vault '%s' contains secrets; use --force to remove", name) + } + + // If removing current vault, switch to another vault first + if isCurrentVault { + // Find another vault to switch to + var newVault string + for _, v := range vaults { + if v != name { + newVault = v + + break + } + } + + // Switch to the new vault + if err := vault.SelectVault(cli.fs, cli.stateDir, newVault); err != nil { + return fmt.Errorf("failed to switch to vault '%s': %w", newVault, err) + } + cmd.Printf("Switched current vault to '%s'\n", newVault) + } + + // Remove the vault directory + if err := cli.fs.RemoveAll(vaultDir); err != nil { + return fmt.Errorf("failed to remove vault directory: %w", err) + } + + cmd.Printf("Removed vault '%s'\n", name) + if hasSecrets { + cmd.Printf("Warning: Vault contained secrets that have been permanently deleted\n") + } + + return nil +} diff --git a/internal/cli/version.go b/internal/cli/version.go index 342c36c..c2f83ec 100644 --- a/internal/cli/version.go +++ b/internal/cli/version.go @@ -33,9 +33,10 @@ func VersionCommands(cli *Instance) *cobra.Command { // List versions command listCmd := &cobra.Command{ - Use: "list ", - Short: "List all versions of a secret", - Args: cobra.ExactArgs(1), + Use: "list ", + Aliases: []string{"ls"}, + Short: "List all versions of a secret", + Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { return cli.ListVersions(cmd, args[0]) }, @@ -52,7 +53,19 @@ func VersionCommands(cli *Instance) *cobra.Command { }, } - versionCmd.AddCommand(listCmd, promoteCmd) + // Remove version command + removeCmd := &cobra.Command{ + Use: "remove ", + Aliases: []string{"rm"}, + Short: "Remove a specific version of a secret", + Long: "Remove a specific version of a secret. Cannot remove the current version.", + Args: cobra.ExactArgs(2), //nolint:mnd // Command requires exactly 2 arguments: secret-name and version + RunE: func(cmd *cobra.Command, args []string) error { + return cli.RemoveVersion(cmd, args[0], args[1]) + }, + } + + versionCmd.AddCommand(listCmd, promoteCmd, removeCmd) return versionCmd } @@ -207,3 +220,60 @@ func (cli *Instance) PromoteVersion(cmd *cobra.Command, secretName string, versi return nil } + +// RemoveVersion removes a specific version of a secret +func (cli *Instance) RemoveVersion(cmd *cobra.Command, secretName string, version string) error { + // Get current vault + vlt, err := vault.GetCurrentVault(cli.fs, cli.stateDir) + if err != nil { + return err + } + + vaultDir, err := vlt.GetDirectory() + if err != nil { + return err + } + + // Get the encoded secret name + encodedName := strings.ReplaceAll(secretName, "/", "%") + secretDir := filepath.Join(vaultDir, "secrets.d", encodedName) + + // Check if secret exists + exists, err := afero.DirExists(cli.fs, secretDir) + if err != nil { + return fmt.Errorf("failed to check if secret exists: %w", err) + } + if !exists { + return fmt.Errorf("secret '%s' not found", secretName) + } + + // Check if version exists + versionDir := filepath.Join(secretDir, "versions", version) + exists, err = afero.DirExists(cli.fs, versionDir) + if err != nil { + return fmt.Errorf("failed to check if version exists: %w", err) + } + if !exists { + return fmt.Errorf("version '%s' not found for secret '%s'", version, secretName) + } + + // Get current version + currentVersion, err := secret.GetCurrentVersion(cli.fs, secretDir) + if err != nil { + return fmt.Errorf("failed to get current version: %w", err) + } + + // Don't allow removing the current version + if version == currentVersion { + return fmt.Errorf("cannot remove the current version '%s'; promote another version first", version) + } + + // Remove the version directory + if err := cli.fs.RemoveAll(versionDir); err != nil { + return fmt.Errorf("failed to remove version: %w", err) + } + + cmd.Printf("Removed version %s of secret '%s'\n", version, secretName) + + return nil +} diff --git a/internal/macse/README.md b/internal/macse/README.md deleted file mode 100644 index b954244..0000000 --- a/internal/macse/README.md +++ /dev/null @@ -1,17 +0,0 @@ -# secure enclave - -``` -akrotiri:~/dev/secret/internal/macse$ CGO_ENABLED=1 go test ./... ---- FAIL: TestEnclaveKeyEncryption (0.04s) - enclave_test.go:16: Failed to create enclave key: failed to create enclave key: error code -34018 ---- FAIL: TestEnclaveKeyPersistence (0.01s) - enclave_test.go:52: Failed to create enclave key: failed to create enclave key: error code -34018 -``` - -This works with temporary keys. When you try to use persistent keys, you -get the above error, because to persist keys in the SE you must have the -appropriate entitlements from Apple, which is only possible with an Apple -Developer Program paid membership (which requires doxxing yourself, and -paying them). - -So this is a dead end for now. diff --git a/internal/macse/enclave.go b/internal/macse/enclave.go deleted file mode 100644 index 8bad24c..0000000 --- a/internal/macse/enclave.go +++ /dev/null @@ -1,313 +0,0 @@ -//go:build darwin -// +build darwin - -package macse - -/* -#cgo CFLAGS: -x objective-c -#cgo LDFLAGS: -framework Foundation -framework Security -framework LocalAuthentication -#import -#import -#import - -typedef struct { - const void* data; - int len; - int error; -} DataResult; - -typedef struct { - SecKeyRef privateKey; - const void* salt; - int saltLen; - int error; -} KeyResult; - -KeyResult createEnclaveKey(bool requireBiometric) { - KeyResult result = {NULL, NULL, 0, 0}; - - // Create authentication context - LAContext* authContext = [[LAContext alloc] init]; - authContext.localizedReason = @"Create Secure Enclave key"; - - CFMutableDictionaryRef attributes = CFDictionaryCreateMutable(NULL, 0, &kCFTypeDictionaryKeyCallBacks, &kCFTypeDictionaryValueCallBacks); - CFDictionarySetValue(attributes, kSecAttrKeyType, kSecAttrKeyTypeECSECPrimeRandom); - CFDictionarySetValue(attributes, kSecAttrKeySizeInBits, (__bridge CFNumberRef)@256); - CFDictionarySetValue(attributes, kSecAttrTokenID, kSecAttrTokenIDSecureEnclave); - - CFMutableDictionaryRef privateKeyAttrs = CFDictionaryCreateMutable(NULL, 0, &kCFTypeDictionaryKeyCallBacks, &kCFTypeDictionaryValueCallBacks); - CFDictionarySetValue(privateKeyAttrs, kSecAttrIsPermanent, kCFBooleanFalse); - - SecAccessControlCreateFlags flags = kSecAccessControlPrivateKeyUsage; - if (requireBiometric) { - flags |= kSecAccessControlBiometryCurrentSet; - } - - SecAccessControlRef access = SecAccessControlCreateWithFlags(kCFAllocatorDefault, - kSecAttrAccessibleAfterFirstUnlockThisDeviceOnly, - flags, - NULL); - if (!access) { - result.error = -1; - return result; - } - - CFDictionarySetValue(privateKeyAttrs, kSecAttrAccessControl, access); - CFDictionarySetValue(privateKeyAttrs, kSecUseAuthenticationContext, (__bridge CFTypeRef)authContext); - CFDictionarySetValue(attributes, kSecPrivateKeyAttrs, privateKeyAttrs); - - CFErrorRef error = NULL; - SecKeyRef privateKey = SecKeyCreateRandomKey(attributes, &error); - - CFRelease(attributes); - CFRelease(privateKeyAttrs); - CFRelease(access); - - if (error || !privateKey) { - if (error) { - result.error = (int)CFErrorGetCode(error); - CFRelease(error); - } else { - result.error = -3; - } - return result; - } - - // Generate random salt - uint8_t* saltBytes = malloc(64); - if (SecRandomCopyBytes(kSecRandomDefault, 64, saltBytes) != 0) { - result.error = -2; - free(saltBytes); - if (privateKey) CFRelease(privateKey); - return result; - } - - result.privateKey = privateKey; - result.salt = saltBytes; - result.saltLen = 64; - - // Retain the key so it's not released - CFRetain(privateKey); - - return result; -} - -DataResult encryptData(SecKeyRef privateKey, const void* saltData, int saltLen, const void* plainData, int plainLen) { - DataResult result = {NULL, 0, 0}; - - // Get public key from private key - SecKeyRef publicKey = SecKeyCopyPublicKey(privateKey); - if (!publicKey) { - result.error = -1; - return result; - } - - // Perform ECDH key agreement with self - CFErrorRef error = NULL; - CFMutableDictionaryRef params = CFDictionaryCreateMutable(NULL, 0, &kCFTypeDictionaryKeyCallBacks, &kCFTypeDictionaryValueCallBacks); - CFDataRef sharedSecret = SecKeyCopyKeyExchangeResult(privateKey, kSecKeyAlgorithmECDHKeyExchangeStandard, publicKey, params, &error); - CFRelease(params); - - if (error) { - result.error = (int)CFErrorGetCode(error); - CFRelease(error); - CFRelease(publicKey); - return result; - } - - // For simplicity, we'll use the shared secret directly as a symmetric key - // In production, you'd want to use HKDF as shown in the Swift code - - // Create encryption key from shared secret - const uint8_t* secretBytes = CFDataGetBytePtr(sharedSecret); - size_t secretLen = CFDataGetLength(sharedSecret); - - // Simple XOR encryption for demonstration (NOT SECURE - use proper encryption in production) - uint8_t* encrypted = malloc(plainLen); - for (int i = 0; i < plainLen; i++) { - encrypted[i] = ((uint8_t*)plainData)[i] ^ secretBytes[i % secretLen]; - } - - result.data = encrypted; - result.len = plainLen; - - CFRelease(publicKey); - CFRelease(sharedSecret); - - return result; -} - -DataResult decryptData(SecKeyRef privateKey, const void* saltData, int saltLen, const void* encData, int encLen, void* context) { - DataResult result = {NULL, 0, 0}; - - // Set up authentication context - LAContext* authContext = [[LAContext alloc] init]; - NSError* authError = nil; - - // Check if biometric authentication is available - if ([authContext canEvaluatePolicy:LAPolicyDeviceOwnerAuthenticationWithBiometrics error:&authError]) { - // Evaluate biometric authentication synchronously - dispatch_semaphore_t sema = dispatch_semaphore_create(0); - __block BOOL authSuccess = NO; - - [authContext evaluatePolicy:LAPolicyDeviceOwnerAuthenticationWithBiometrics - localizedReason:@"Decrypt data using Secure Enclave" - reply:^(BOOL success, NSError * _Nullable error) { - authSuccess = success; - dispatch_semaphore_signal(sema); - }]; - - dispatch_semaphore_wait(sema, DISPATCH_TIME_FOREVER); - - if (!authSuccess) { - result.error = -3; - return result; - } - } - - // Get public key from private key - SecKeyRef publicKey = SecKeyCopyPublicKey(privateKey); - if (!publicKey) { - result.error = -1; - return result; - } - - // Create algorithm parameters with authentication context - CFMutableDictionaryRef params = CFDictionaryCreateMutable(NULL, 0, &kCFTypeDictionaryKeyCallBacks, &kCFTypeDictionaryValueCallBacks); - CFDictionarySetValue(params, kSecUseAuthenticationContext, (__bridge CFTypeRef)authContext); - - // Perform ECDH key agreement with self - CFErrorRef error = NULL; - CFDataRef sharedSecret = SecKeyCopyKeyExchangeResult(privateKey, kSecKeyAlgorithmECDHKeyExchangeStandard, publicKey, params, &error); - CFRelease(params); - - if (error) { - result.error = (int)CFErrorGetCode(error); - CFRelease(error); - CFRelease(publicKey); - return result; - } - - // Decrypt using shared secret - const uint8_t* secretBytes = CFDataGetBytePtr(sharedSecret); - size_t secretLen = CFDataGetLength(sharedSecret); - - // Simple XOR decryption for demonstration - uint8_t* decrypted = malloc(encLen); - for (int i = 0; i < encLen; i++) { - decrypted[i] = ((uint8_t*)encData)[i] ^ secretBytes[i % secretLen]; - } - - result.data = decrypted; - result.len = encLen; - - CFRelease(publicKey); - CFRelease(sharedSecret); - - return result; -} - -void freeKeyResult(KeyResult* result) { - if (result->privateKey) { - CFRelease(result->privateKey); - } - if (result->salt) { - free((void*)result->salt); - } -} - -void freeDataResult(DataResult* result) { - if (result->data) { - free((void*)result->data); - } -} -*/ -import "C" -import ( - "errors" - "unsafe" -) - -type EnclaveKey struct { - privateKey C.SecKeyRef - salt []byte -} - -func NewEnclaveKey(requireBiometric bool) (*EnclaveKey, error) { - result := C.createEnclaveKey(C.bool(requireBiometric)) - defer C.freeKeyResult(&result) - - if result.error != 0 { - return nil, errors.New("failed to create enclave key") - } - - salt := make([]byte, result.saltLen) - copy(salt, (*[1 << 30]byte)(unsafe.Pointer(result.salt))[:result.saltLen:result.saltLen]) - - return &EnclaveKey{ - privateKey: result.privateKey, - salt: salt, - }, nil -} - -func (k *EnclaveKey) Encrypt(data []byte) ([]byte, error) { - if len(data) == 0 { - return nil, errors.New("empty data") - } - if len(k.salt) == 0 { - return nil, errors.New("empty salt") - } - - result := C.encryptData( - k.privateKey, - unsafe.Pointer(&k.salt[0]), - C.int(len(k.salt)), - unsafe.Pointer(&data[0]), - C.int(len(data)), - ) - defer C.freeDataResult(&result) - - if result.error != 0 { - return nil, errors.New("encryption failed") - } - - encrypted := make([]byte, result.len) - copy(encrypted, (*[1 << 30]byte)(unsafe.Pointer(result.data))[:result.len:result.len]) - - return encrypted, nil -} - -func (k *EnclaveKey) Decrypt(data []byte) ([]byte, error) { - if len(data) == 0 { - return nil, errors.New("empty data") - } - if len(k.salt) == 0 { - return nil, errors.New("empty salt") - } - - result := C.decryptData( - k.privateKey, - unsafe.Pointer(&k.salt[0]), - C.int(len(k.salt)), - unsafe.Pointer(&data[0]), - C.int(len(data)), - nil, - ) - defer C.freeDataResult(&result) - - if result.error != 0 { - return nil, errors.New("decryption failed") - } - - decrypted := make([]byte, result.len) - copy(decrypted, (*[1 << 30]byte)(unsafe.Pointer(result.data))[:result.len:result.len]) - - return decrypted, nil -} - -func (k *EnclaveKey) Close() { - if k.privateKey != 0 { - C.CFRelease(C.CFTypeRef(k.privateKey)) - k.privateKey = 0 - } -} diff --git a/internal/macse/enclave_test.go b/internal/macse/enclave_test.go deleted file mode 100644 index ee5f523..0000000 --- a/internal/macse/enclave_test.go +++ /dev/null @@ -1,87 +0,0 @@ -//go:build darwin -// +build darwin - -package macse - -import ( - "bytes" - "testing" -) - -func TestEnclaveKeyEncryption(t *testing.T) { - // Skip: Secure Enclave access requires Apple Developer Enterprise (ADE) membership, - // proper code signing, and entitlements for non-ephemeral keys. - // Without these, only ephemeral keys work which are not suitable for our use case. - t.Skip("Skipping: Requires ADE membership, signing, and entitlements for non-ephemeral keys") - - // Create a new enclave key without requiring biometric - key, err := NewEnclaveKey(false) - if err != nil { - t.Fatalf("Failed to create enclave key: %v", err) - } - defer key.Close() - - // Test data - plaintext := []byte("Hello, Secure Enclave!") - - // Encrypt - encrypted, err := key.Encrypt(plaintext) - if err != nil { - t.Fatalf("Failed to encrypt: %v", err) - } - - // Verify encrypted data is different from plaintext - if bytes.Equal(plaintext, encrypted) { - t.Error("Encrypted data should not equal plaintext") - } - - // Decrypt - decrypted, err := key.Decrypt(encrypted) - if err != nil { - t.Fatalf("Failed to decrypt: %v", err) - } - - // Verify decrypted data matches original - if !bytes.Equal(plaintext, decrypted) { - t.Errorf("Decrypted data does not match original: got %s, want %s", decrypted, plaintext) - } -} - -func TestEnclaveKeyWithBiometric(t *testing.T) { - // Skip: Secure Enclave access requires Apple Developer Enterprise (ADE) membership, - // proper code signing, and entitlements for non-ephemeral keys. - // Without these, only ephemeral keys work which are not suitable for our use case. - t.Skip("Skipping: Requires ADE membership, signing, and entitlements for non-ephemeral keys") - - // This test requires user interaction - // Run with: CGO_ENABLED=1 go test -v -run TestEnclaveKeyWithBiometric - if testing.Short() { - t.Skip("Skipping biometric test in short mode") - } - - key, err := NewEnclaveKey(true) - if err != nil { - t.Logf("Expected failure creating biometric key in test environment: %v", err) - return - } - defer key.Close() - - plaintext := []byte("Biometric protected data") - - encrypted, err := key.Encrypt(plaintext) - if err != nil { - t.Fatalf("Failed to encrypt with biometric key: %v", err) - } - - // Decryption would require biometric authentication - decrypted, err := key.Decrypt(encrypted) - if err != nil { - // This is expected without proper biometric authentication - t.Logf("Expected decryption failure without biometric auth: %v", err) - return - } - - if !bytes.Equal(plaintext, decrypted) { - t.Errorf("Decrypted data does not match original") - } -} diff --git a/internal/secret/keychainunlocker.go b/internal/secret/keychainunlocker.go index 183d1ea..1639d31 100644 --- a/internal/secret/keychainunlocker.go +++ b/internal/secret/keychainunlocker.go @@ -20,7 +20,8 @@ import ( const ( agePrivKeyPassphraseLength = 64 - KEYCHAIN_APP_IDENTIFIER = "berlin.sneak.app.secret" + // KEYCHAIN_APP_IDENTIFIER is the service name used for keychain items + KEYCHAIN_APP_IDENTIFIER = "berlin.sneak.app.secret" //nolint:revive // ALL_CAPS is intentional for this constant ) // keychainItemNameRegex validates keychain item names @@ -445,6 +446,7 @@ func checkMacOSAvailable() error { if runtime.GOOS != "darwin" { return fmt.Errorf("keychain unlockers are only supported on macOS, current OS: %s", runtime.GOOS) } + return nil } @@ -476,7 +478,6 @@ func storeInKeychain(itemName string, data *memguard.LockedBuffer) error { item.SetAccount(itemName) item.SetLabel(fmt.Sprintf("%s - %s", KEYCHAIN_APP_IDENTIFIER, itemName)) item.SetDescription("Secret vault keychain data") - item.SetComment("This item stores encrypted key material for the secret vault") item.SetData([]byte(data.String())) item.SetSynchronizable(keychain.SynchronizableNo) // Use AccessibleWhenUnlockedThisDeviceOnly for better security and to trigger auth @@ -487,7 +488,7 @@ func storeInKeychain(itemName string, data *memguard.LockedBuffer) error { deleteItem.SetSecClass(keychain.SecClassGenericPassword) deleteItem.SetService(KEYCHAIN_APP_IDENTIFIER) deleteItem.SetAccount(itemName) - keychain.DeleteItem(deleteItem) // Ignore error as item might not exist + _ = keychain.DeleteItem(deleteItem) // Ignore error as item might not exist // Add the new item if err := keychain.AddItem(item); err != nil { diff --git a/internal/secret/keychainunlocker_test.go b/internal/secret/keychainunlocker_test.go index cd6f6c1..698de11 100644 --- a/internal/secret/keychainunlocker_test.go +++ b/internal/secret/keychainunlocker_test.go @@ -70,24 +70,24 @@ func TestKeychainInvalidItemName(t *testing.T) { // Test invalid item names invalidNames := []string{ - "", // Empty name - "test space", // Contains space - "test/slash", // Contains slash - "test\\backslash", // Contains backslash - "test:colon", // Contains colon - "test;semicolon", // Contains semicolon - "test|pipe", // Contains pipe - "test@at", // Contains @ - "test#hash", // Contains # - "test$dollar", // Contains $ - "test&ersand", // Contains & - "test*asterisk", // Contains * - "test?question", // Contains ? - "test!exclamation", // Contains ! - "test'quote", // Contains single quote - "test\"doublequote", // Contains double quote - "test(paren", // Contains parenthesis - "test[bracket", // Contains bracket + "", // Empty name + "test space", // Contains space + "test/slash", // Contains slash + "test\\backslash", // Contains backslash + "test:colon", // Contains colon + "test;semicolon", // Contains semicolon + "test|pipe", // Contains pipe + "test@at", // Contains @ + "test#hash", // Contains # + "test$dollar", // Contains $ + "test&ersand", // Contains & + "test*asterisk", // Contains * + "test?question", // Contains ? + "test!exclamation", // Contains ! + "test'quote", // Contains single quote + "test\"doublequote", // Contains double quote + "test(paren", // Contains parenthesis + "test[bracket", // Contains bracket } for _, name := range invalidNames { @@ -138,10 +138,10 @@ func TestKeychainLargeData(t *testing.T) { for i := range largeData { largeData[i] = byte(i % 256) } - + // Convert to hex string for storage hexData := hex.EncodeToString(largeData) - + testItemName := "test-large-data" testBuffer := memguard.NewBufferFromBytes([]byte(hexData)) defer testBuffer.Destroy() @@ -156,7 +156,7 @@ func TestKeychainLargeData(t *testing.T) { // Retrieve and verify retrievedData, err := retrieveFromKeychain(testItemName) require.NoError(t, err, "Failed to retrieve large data") - + // Decode hex and compare decodedData, err := hex.DecodeString(string(retrievedData)) require.NoError(t, err, "Failed to decode hex data") @@ -164,4 +164,4 @@ func TestKeychainLargeData(t *testing.T) { // Clean up _ = deleteFromKeychain(testItemName) -} \ No newline at end of file +} diff --git a/internal/vault/vault.go b/internal/vault/vault.go index 1e64b41..b535317 100644 --- a/internal/vault/vault.go +++ b/internal/vault/vault.go @@ -208,3 +208,48 @@ func (v *Vault) GetName() string { func (v *Vault) GetFilesystem() afero.Fs { return v.fs } + +// NumSecrets returns the number of secrets in the vault +func (v *Vault) NumSecrets() (int, error) { + vaultDir, err := v.GetDirectory() + if err != nil { + return 0, fmt.Errorf("failed to get vault directory: %w", err) + } + + secretsDir := filepath.Join(vaultDir, "secrets.d") + exists, _ := afero.DirExists(v.fs, secretsDir) + if !exists { + return 0, nil + } + + entries, err := afero.ReadDir(v.fs, secretsDir) + if err != nil { + return 0, fmt.Errorf("failed to read secrets directory: %w", err) + } + + // Count only directories that contain at least one version file + count := 0 + for _, entry := range entries { + if !entry.IsDir() { + continue + } + + // Check if this secret directory contains any version files + secretDir := filepath.Join(secretsDir, entry.Name()) + versionFiles, err := afero.ReadDir(v.fs, secretDir) + if err != nil { + continue // Skip directories we can't read + } + + // Look for at least one version file (excluding "current" symlink) + for _, vFile := range versionFiles { + if !vFile.IsDir() && vFile.Name() != "current" { + count++ + + break // Found at least one version, count this secret + } + } + } + + return count, nil +} From 7af1e6efa86bfe45e7fb18a4ace06a431573ef80 Mon Sep 17 00:00:00 2001 From: sneak Date: Mon, 21 Jul 2025 18:57:58 +0200 Subject: [PATCH 2/6] 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) } From a09fa89f30c943142638367795b82498d6282ba5 Mon Sep 17 00:00:00 2001 From: sneak Date: Mon, 21 Jul 2025 22:05:23 +0200 Subject: [PATCH 3/6] Fix cross-platform build issues and security vulnerabilities - Add build tags to keychain implementation files (Darwin-only) - Create stub implementations for non-Darwin platforms that panic - Conditionally show keychain support in help text based on platform - Platform check in UnlockersAdd prevents keychain usage on non-Darwin - Verified GPG operations already protected against command injection via validateGPGKeyID() and proper exec.Command argument passing - Keychain operations use go-keychain library, no shell commands The application now builds and runs on Linux/non-Darwin platforms with keychain functionality properly isolated to macOS only. --- coverage.out | 102 +++++++++++++++++++++++ internal/cli/unlockers.go | 21 ++++- internal/secret/keychainunlocker.go | 3 + internal/secret/keychainunlocker_stub.go | 69 +++++++++++++++ 4 files changed, 193 insertions(+), 2 deletions(-) create mode 100644 coverage.out create mode 100644 internal/secret/keychainunlocker_stub.go diff --git a/coverage.out b/coverage.out new file mode 100644 index 0000000..c86d0d2 --- /dev/null +++ b/coverage.out @@ -0,0 +1,102 @@ +mode: set +git.eeqj.de/sneak/secret/pkg/bip85/bip85.go:57.41,60.38 2 1 +git.eeqj.de/sneak/secret/pkg/bip85/bip85.go:60.38,61.41 1 1 +git.eeqj.de/sneak/secret/pkg/bip85/bip85.go:65.2,70.3 3 1 +git.eeqj.de/sneak/secret/pkg/bip85/bip85.go:74.50,76.2 1 1 +git.eeqj.de/sneak/secret/pkg/bip85/bip85.go:79.85,81.28 1 1 +git.eeqj.de/sneak/secret/pkg/bip85/bip85.go:81.28,83.3 1 0 +git.eeqj.de/sneak/secret/pkg/bip85/bip85.go:86.2,87.16 2 1 +git.eeqj.de/sneak/secret/pkg/bip85/bip85.go:87.16,89.3 1 0 +git.eeqj.de/sneak/secret/pkg/bip85/bip85.go:92.2,93.16 2 1 +git.eeqj.de/sneak/secret/pkg/bip85/bip85.go:93.16,95.3 1 0 +git.eeqj.de/sneak/secret/pkg/bip85/bip85.go:98.2,98.35 1 1 +git.eeqj.de/sneak/secret/pkg/bip85/bip85.go:102.89,105.16 2 1 +git.eeqj.de/sneak/secret/pkg/bip85/bip85.go:105.16,107.3 1 0 +git.eeqj.de/sneak/secret/pkg/bip85/bip85.go:110.2,114.21 4 1 +git.eeqj.de/sneak/secret/pkg/bip85/bip85.go:118.99,119.46 1 1 +git.eeqj.de/sneak/secret/pkg/bip85/bip85.go:119.46,121.3 1 0 +git.eeqj.de/sneak/secret/pkg/bip85/bip85.go:124.2,134.39 5 1 +git.eeqj.de/sneak/secret/pkg/bip85/bip85.go:134.39,137.15 2 1 +git.eeqj.de/sneak/secret/pkg/bip85/bip85.go:137.15,140.4 2 1 +git.eeqj.de/sneak/secret/pkg/bip85/bip85.go:143.3,145.17 3 1 +git.eeqj.de/sneak/secret/pkg/bip85/bip85.go:145.17,147.4 1 0 +git.eeqj.de/sneak/secret/pkg/bip85/bip85.go:150.3,150.15 1 1 +git.eeqj.de/sneak/secret/pkg/bip85/bip85.go:150.15,152.4 1 1 +git.eeqj.de/sneak/secret/pkg/bip85/bip85.go:155.3,156.17 2 1 +git.eeqj.de/sneak/secret/pkg/bip85/bip85.go:156.17,158.4 1 0 +git.eeqj.de/sneak/secret/pkg/bip85/bip85.go:160.3,160.14 1 1 +git.eeqj.de/sneak/secret/pkg/bip85/bip85.go:163.2,163.17 1 1 +git.eeqj.de/sneak/secret/pkg/bip85/bip85.go:167.107,171.16 3 1 +git.eeqj.de/sneak/secret/pkg/bip85/bip85.go:171.16,173.3 1 0 +git.eeqj.de/sneak/secret/pkg/bip85/bip85.go:177.2,186.15 3 1 +git.eeqj.de/sneak/secret/pkg/bip85/bip85.go:187.15,188.13 1 1 +git.eeqj.de/sneak/secret/pkg/bip85/bip85.go:189.15,190.13 1 0 +git.eeqj.de/sneak/secret/pkg/bip85/bip85.go:191.15,192.13 1 1 +git.eeqj.de/sneak/secret/pkg/bip85/bip85.go:193.15,194.13 1 0 +git.eeqj.de/sneak/secret/pkg/bip85/bip85.go:195.15,196.13 1 1 +git.eeqj.de/sneak/secret/pkg/bip85/bip85.go:197.10,198.64 1 1 +git.eeqj.de/sneak/secret/pkg/bip85/bip85.go:202.2,204.21 2 1 +git.eeqj.de/sneak/secret/pkg/bip85/bip85.go:208.84,212.16 3 1 +git.eeqj.de/sneak/secret/pkg/bip85/bip85.go:212.16,214.3 1 0 +git.eeqj.de/sneak/secret/pkg/bip85/bip85.go:217.2,222.16 4 1 +git.eeqj.de/sneak/secret/pkg/bip85/bip85.go:222.16,224.3 1 0 +git.eeqj.de/sneak/secret/pkg/bip85/bip85.go:226.2,226.26 1 1 +git.eeqj.de/sneak/secret/pkg/bip85/bip85.go:230.99,234.16 3 1 +git.eeqj.de/sneak/secret/pkg/bip85/bip85.go:234.16,236.3 1 0 +git.eeqj.de/sneak/secret/pkg/bip85/bip85.go:239.2,251.45 6 1 +git.eeqj.de/sneak/secret/pkg/bip85/bip85.go:251.45,253.3 1 1 +git.eeqj.de/sneak/secret/pkg/bip85/bip85.go:256.2,275.45 12 1 +git.eeqj.de/sneak/secret/pkg/bip85/bip85.go:279.39,284.2 3 1 +git.eeqj.de/sneak/secret/pkg/bip85/bip85.go:287.91,288.36 1 1 +git.eeqj.de/sneak/secret/pkg/bip85/bip85.go:288.36,290.3 1 1 +git.eeqj.de/sneak/secret/pkg/bip85/bip85.go:292.2,295.16 3 1 +git.eeqj.de/sneak/secret/pkg/bip85/bip85.go:295.16,297.3 1 0 +git.eeqj.de/sneak/secret/pkg/bip85/bip85.go:300.2,302.41 2 1 +git.eeqj.de/sneak/secret/pkg/bip85/bip85.go:306.100,307.32 1 1 +git.eeqj.de/sneak/secret/pkg/bip85/bip85.go:307.32,309.3 1 1 +git.eeqj.de/sneak/secret/pkg/bip85/bip85.go:311.2,314.16 3 1 +git.eeqj.de/sneak/secret/pkg/bip85/bip85.go:314.16,316.3 1 0 +git.eeqj.de/sneak/secret/pkg/bip85/bip85.go:319.2,325.35 3 1 +git.eeqj.de/sneak/secret/pkg/bip85/bip85.go:325.35,327.3 1 0 +git.eeqj.de/sneak/secret/pkg/bip85/bip85.go:329.2,329.33 1 1 +git.eeqj.de/sneak/secret/pkg/bip85/bip85.go:333.100,334.32 1 1 +git.eeqj.de/sneak/secret/pkg/bip85/bip85.go:334.32,336.3 1 1 +git.eeqj.de/sneak/secret/pkg/bip85/bip85.go:338.2,341.16 3 1 +git.eeqj.de/sneak/secret/pkg/bip85/bip85.go:341.16,343.3 1 0 +git.eeqj.de/sneak/secret/pkg/bip85/bip85.go:346.2,349.32 2 1 +git.eeqj.de/sneak/secret/pkg/bip85/bip85.go:349.32,351.3 1 0 +git.eeqj.de/sneak/secret/pkg/bip85/bip85.go:353.2,353.30 1 1 +git.eeqj.de/sneak/secret/pkg/bip85/bip85.go:357.57,375.52 7 1 +git.eeqj.de/sneak/secret/pkg/bip85/bip85.go:375.52,381.46 3 1 +git.eeqj.de/sneak/secret/pkg/bip85/bip85.go:381.46,385.4 3 1 +git.eeqj.de/sneak/secret/pkg/bip85/bip85.go:387.3,387.20 1 1 +git.eeqj.de/sneak/secret/pkg/bip85/bip85.go:390.2,390.21 1 1 +git.eeqj.de/sneak/secret/pkg/bip85/bip85.go:394.67,396.2 1 1 +git.eeqj.de/sneak/secret/pkg/agehd/agehd.go:32.22,36.2 3 1 +git.eeqj.de/sneak/secret/pkg/agehd/agehd.go:40.67,41.31 1 1 +git.eeqj.de/sneak/secret/pkg/agehd/agehd.go:41.31,43.3 1 1 +git.eeqj.de/sneak/secret/pkg/agehd/agehd.go:46.2,55.16 6 1 +git.eeqj.de/sneak/secret/pkg/agehd/agehd.go:55.16,57.3 1 0 +git.eeqj.de/sneak/secret/pkg/agehd/agehd.go:58.2,59.16 2 1 +git.eeqj.de/sneak/secret/pkg/agehd/agehd.go:59.16,61.3 1 0 +git.eeqj.de/sneak/secret/pkg/agehd/agehd.go:63.2,63.52 1 1 +git.eeqj.de/sneak/secret/pkg/agehd/agehd.go:68.63,74.16 3 1 +git.eeqj.de/sneak/secret/pkg/agehd/agehd.go:74.16,76.3 1 0 +git.eeqj.de/sneak/secret/pkg/agehd/agehd.go:79.2,83.16 3 1 +git.eeqj.de/sneak/secret/pkg/agehd/agehd.go:83.16,85.3 1 0 +git.eeqj.de/sneak/secret/pkg/agehd/agehd.go:88.2,91.16 4 1 +git.eeqj.de/sneak/secret/pkg/agehd/agehd.go:91.16,93.3 1 0 +git.eeqj.de/sneak/secret/pkg/agehd/agehd.go:95.2,95.17 1 1 +git.eeqj.de/sneak/secret/pkg/agehd/agehd.go:100.67,103.16 2 1 +git.eeqj.de/sneak/secret/pkg/agehd/agehd.go:103.16,105.3 1 1 +git.eeqj.de/sneak/secret/pkg/agehd/agehd.go:108.2,112.16 3 1 +git.eeqj.de/sneak/secret/pkg/agehd/agehd.go:112.16,114.3 1 1 +git.eeqj.de/sneak/secret/pkg/agehd/agehd.go:117.2,120.16 4 1 +git.eeqj.de/sneak/secret/pkg/agehd/agehd.go:120.16,122.3 1 0 +git.eeqj.de/sneak/secret/pkg/agehd/agehd.go:124.2,124.17 1 1 +git.eeqj.de/sneak/secret/pkg/agehd/agehd.go:129.77,131.16 2 1 +git.eeqj.de/sneak/secret/pkg/agehd/agehd.go:131.16,133.3 1 0 +git.eeqj.de/sneak/secret/pkg/agehd/agehd.go:135.2,135.33 1 1 +git.eeqj.de/sneak/secret/pkg/agehd/agehd.go:140.81,142.16 2 1 +git.eeqj.de/sneak/secret/pkg/agehd/agehd.go:142.16,144.3 1 1 +git.eeqj.de/sneak/secret/pkg/agehd/agehd.go:146.2,146.33 1 1 diff --git a/internal/cli/unlockers.go b/internal/cli/unlockers.go index 847375c..5dcb61a 100644 --- a/internal/cli/unlockers.go +++ b/internal/cli/unlockers.go @@ -6,6 +6,7 @@ import ( "os" "os/exec" "path/filepath" + "runtime" "strings" "time" @@ -90,10 +91,16 @@ func newUnlockersListCmd() *cobra.Command { } func newUnlockersAddCmd() *cobra.Command { + // Build the supported types list based on platform + supportedTypes := "passphrase, pgp" + if runtime.GOOS == "darwin" { + supportedTypes = "passphrase, keychain, pgp" + } + cmd := &cobra.Command{ Use: "add [keyid]", Short: "Add a new unlocker", - Long: `Add a new unlocker of the specified type (passphrase, keychain, pgp).`, + Long: fmt.Sprintf(`Add a new unlocker of the specified type (%s).`, supportedTypes), Args: cobra.RangeArgs(1, 2), //nolint:mnd // Command accepts 1 or 2 arguments RunE: func(cmd *cobra.Command, args []string) error { cli := NewCLIInstance() @@ -295,6 +302,12 @@ func (cli *Instance) UnlockersList(jsonOutput bool) error { // UnlockersAdd adds a new unlocker func (cli *Instance) UnlockersAdd(unlockerType string, cmd *cobra.Command) error { + // Build the supported types list based on platform + supportedTypes := "passphrase, pgp" + if runtime.GOOS == "darwin" { + supportedTypes = "passphrase, keychain, pgp" + } + switch unlockerType { case "passphrase": // Get current vault @@ -329,6 +342,10 @@ func (cli *Instance) UnlockersAdd(unlockerType string, cmd *cobra.Command) error return nil case "keychain": + if runtime.GOOS != "darwin" { + return fmt.Errorf("keychain unlockers are only supported on macOS") + } + keychainUnlocker, err := secret.CreateKeychainUnlocker(cli.fs, cli.stateDir) if err != nil { return fmt.Errorf("failed to create macOS Keychain unlocker: %w", err) @@ -387,7 +404,7 @@ func (cli *Instance) UnlockersAdd(unlockerType string, cmd *cobra.Command) error return nil default: - return fmt.Errorf("unsupported unlocker type: %s (supported: passphrase, keychain, pgp)", unlockerType) + return fmt.Errorf("unsupported unlocker type: %s (supported: %s)", unlockerType, supportedTypes) } } diff --git a/internal/secret/keychainunlocker.go b/internal/secret/keychainunlocker.go index 1639d31..74accab 100644 --- a/internal/secret/keychainunlocker.go +++ b/internal/secret/keychainunlocker.go @@ -1,3 +1,6 @@ +//go:build darwin +// +build darwin + package secret import ( diff --git a/internal/secret/keychainunlocker_stub.go b/internal/secret/keychainunlocker_stub.go new file mode 100644 index 0000000..512baa0 --- /dev/null +++ b/internal/secret/keychainunlocker_stub.go @@ -0,0 +1,69 @@ +//go:build !darwin +// +build !darwin + +package secret + +import ( + "fmt" + + "filippo.io/age" + "github.com/spf13/afero" +) + +// KeychainUnlockerMetadata is a stub for non-Darwin platforms +type KeychainUnlockerMetadata struct { + UnlockerMetadata + KeychainItemName string `json:"keychainItemName"` +} + +// KeychainUnlocker is a stub for non-Darwin platforms +type KeychainUnlocker struct { + Directory string + Metadata UnlockerMetadata + fs afero.Fs +} + +// GetIdentity panics on non-Darwin platforms +func (k *KeychainUnlocker) GetIdentity() (*age.X25519Identity, error) { + panic("keychain unlockers are only supported on macOS") +} + +// GetType panics on non-Darwin platforms +func (k *KeychainUnlocker) GetType() string { + panic("keychain unlockers are only supported on macOS") +} + +// GetMetadata panics on non-Darwin platforms +func (k *KeychainUnlocker) GetMetadata() UnlockerMetadata { + panic("keychain unlockers are only supported on macOS") +} + +// GetDirectory panics on non-Darwin platforms +func (k *KeychainUnlocker) GetDirectory() string { + panic("keychain unlockers are only supported on macOS") +} + +// GetID returns the unlocker ID +func (k *KeychainUnlocker) GetID() string { + panic("keychain unlockers are only supported on macOS") +} + +// GetKeychainItemName panics on non-Darwin platforms +func (k *KeychainUnlocker) GetKeychainItemName() (string, error) { + panic("keychain unlockers are only supported on macOS") +} + +// Remove panics on non-Darwin platforms +func (k *KeychainUnlocker) Remove() error { + panic("keychain unlockers are only supported on macOS") +} + +// NewKeychainUnlocker panics on non-Darwin platforms +func NewKeychainUnlocker(fs afero.Fs, directory string, metadata UnlockerMetadata) *KeychainUnlocker { + panic("keychain unlockers are only supported on macOS") +} + +// CreateKeychainUnlocker panics on non-Darwin platforms +func CreateKeychainUnlocker(fs afero.Fs, stateDir string) (*KeychainUnlocker, error) { + panic("keychain unlockers are only supported on macOS") +} From 377b51f2db87c1b7c50a0266bcc4f082b568a6d2 Mon Sep 17 00:00:00 2001 From: sneak Date: Mon, 21 Jul 2025 22:13:19 +0200 Subject: [PATCH 4/6] Add Docker support for building and running the CLI tool - Add DOCKER_HOST export to Makefile for remote Docker daemon - Create multi-stage Dockerfile: - Build stage: golang:1.24-alpine with gcc, make, git - Runtime stage: alpine with ca-certificates, gnupg - Runs as non-root 'secret' user - Add Makefile targets: - docker: build container as sneak/secret - docker-run: run container interactively - Add .dockerignore to exclude build artifacts but keep .git for potential linker flags Container includes GPG support for PGP unlockers and runs on Linux, making it suitable for cross-platform testing and deployment. --- .dockerignore | 21 +++++++++++++++++++++ Dockerfile | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++ Makefile | 9 +++++++++ 3 files changed, 80 insertions(+) create mode 100644 .dockerignore create mode 100644 Dockerfile diff --git a/.dockerignore b/.dockerignore new file mode 100644 index 0000000..f4ebfab --- /dev/null +++ b/.dockerignore @@ -0,0 +1,21 @@ +# Build artifacts +secret +coverage.out +*.test + +# IDE and editor files +.vscode +.idea +*.swp +*.swo +*~ + +# macOS +.DS_Store + +# Claude files +.claude/ + +# Local settings +.golangci.yml +.claude/settings.local.json \ No newline at end of file diff --git a/Dockerfile b/Dockerfile new file mode 100644 index 0000000..c16f021 --- /dev/null +++ b/Dockerfile @@ -0,0 +1,50 @@ +# Build stage +FROM golang:1.24-alpine AS builder + +# Install build dependencies +RUN apk add --no-cache \ + gcc \ + musl-dev \ + make \ + git + +# Set working directory +WORKDIR /build + +# Copy go mod files +COPY go.mod go.sum ./ + +# Download dependencies +RUN go mod download + +# Copy source code +COPY . . + +# Build the binary +RUN CGO_ENABLED=1 go build -v -o secret cmd/secret/main.go + +# Runtime stage +FROM alpine:latest + +# Install runtime dependencies +RUN apk add --no-cache \ + ca-certificates \ + gnupg + +# Create non-root user +RUN adduser -D -s /bin/sh secret + +# Copy binary from builder +COPY --from=builder /build/secret /usr/local/bin/secret + +# Ensure binary is executable +RUN chmod +x /usr/local/bin/secret + +# Switch to non-root user +USER secret + +# Set working directory +WORKDIR /home/secret + +# Set entrypoint +ENTRYPOINT ["secret"] \ No newline at end of file diff --git a/Makefile b/Makefile index b6dc7c0..6c55c0e 100644 --- a/Makefile +++ b/Makefile @@ -1,4 +1,5 @@ export CGO_ENABLED=1 +export DOCKER_HOST := ssh://root@ber1app1.local default: check @@ -23,6 +24,14 @@ lint: # Check all code quality (build + vet + lint + unit tests) check: ./secret vet lint test +# Build Docker container +docker: + docker build -t sneak/secret . + +# Run Docker container interactively +docker-run: + docker run --rm -it sneak/secret + # Clean build artifacts clean: rm -f ./secret From e5d7407c7913a1b24607efb69f4c5e79cd5536c4 Mon Sep 17 00:00:00 2001 From: sneak Date: Tue, 22 Jul 2025 12:39:32 +0200 Subject: [PATCH 5/6] Fix mnemonic input to not echo to screen Changed mnemonic input to use secure non-echoing input like passphrases: - Use secret.ReadPassphrase() instead of readLineFromStdin() - Add newline after hidden input for better UX - Remove unused stdin reading functions from cli.go This prevents sensitive mnemonic phrases from being displayed on screen during input, matching the security behavior of passphrase input. --- internal/cli/cli.go | 40 ---------------------------------------- internal/cli/init.go | 9 ++++++--- 2 files changed, 6 insertions(+), 43 deletions(-) diff --git a/internal/cli/cli.go b/internal/cli/cli.go index ec120fb..43053b2 100644 --- a/internal/cli/cli.go +++ b/internal/cli/cli.go @@ -2,21 +2,11 @@ package cli import ( - "bufio" - "fmt" - "os" - "strings" - "syscall" - "git.eeqj.de/sneak/secret/internal/secret" "github.com/spf13/afero" "github.com/spf13/cobra" - "golang.org/x/term" ) -// Global scanner for consistent stdin reading -var stdinScanner *bufio.Scanner //nolint:gochecknoglobals // Needed for consistent stdin handling - // Instance encapsulates all CLI functionality and state type Instance struct { fs afero.Fs @@ -67,33 +57,3 @@ func (cli *Instance) SetStateDir(stateDir string) { func (cli *Instance) GetStateDir() string { return cli.stateDir } - -// getStdinScanner returns a shared scanner for stdin to avoid buffering issues -func getStdinScanner() *bufio.Scanner { - if stdinScanner == nil { - stdinScanner = bufio.NewScanner(os.Stdin) - } - - return stdinScanner -} - -// readLineFromStdin reads a single line from stdin with a prompt -// Uses a shared scanner to avoid buffering issues between multiple calls -func readLineFromStdin(prompt string) (string, error) { - // Check if stderr is a terminal - if not, we can't prompt interactively - if !term.IsTerminal(syscall.Stderr) { - return "", fmt.Errorf("cannot prompt for input: stderr is not a terminal (running in non-interactive mode)") - } - - fmt.Fprint(os.Stderr, prompt) // Write prompt to stderr, not stdout - scanner := getStdinScanner() - if !scanner.Scan() { - if err := scanner.Err(); err != nil { - return "", fmt.Errorf("failed to read from stdin: %w", err) - } - - return "", fmt.Errorf("failed to read from stdin: EOF") - } - - return strings.TrimSpace(scanner.Text()), nil -} diff --git a/internal/cli/init.go b/internal/cli/init.go index 4181d50..4b237c8 100644 --- a/internal/cli/init.go +++ b/internal/cli/init.go @@ -60,14 +60,17 @@ func (cli *Instance) Init(cmd *cobra.Command) error { mnemonicStr = envMnemonic } else { secret.Debug("Prompting user for mnemonic phrase") - // Read mnemonic from stdin using shared line reader - var err error - mnemonicStr, err = readLineFromStdin("Enter your BIP39 mnemonic phrase: ") + // Read mnemonic securely without echo + mnemonicBuffer, err := secret.ReadPassphrase("Enter your BIP39 mnemonic phrase: ") if err != nil { secret.Debug("Failed to read mnemonic from stdin", "error", err) return fmt.Errorf("failed to read mnemonic: %w", err) } + defer mnemonicBuffer.Destroy() + + mnemonicStr = mnemonicBuffer.String() + fmt.Fprintln(os.Stderr) // Add newline after hidden input } if mnemonicStr == "" { From 8e3530a510004bd946493cebda9c167388607509 Mon Sep 17 00:00:00 2001 From: sneak Date: Tue, 22 Jul 2025 12:46:16 +0200 Subject: [PATCH 6/6] Fix use-after-free crash in readSecurePassphrase The function was using defer to destroy password buffers, which caused the buffers to be freed before the function returned. This led to a SIGBUS error when trying to access the destroyed buffer's memory. Changed to manual memory management to ensure buffers are only destroyed when no longer needed, and the first buffer is returned directly to the caller who is responsible for destroying it. --- internal/cli/init.go | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/internal/cli/init.go b/internal/cli/init.go index 4b237c8..1167f7c 100644 --- a/internal/cli/init.go +++ b/internal/cli/init.go @@ -205,20 +205,26 @@ func readSecurePassphrase(prompt string) (*memguard.LockedBuffer, error) { if err != nil { return nil, err } - defer passphraseBuffer1.Destroy() // Read confirmation passphrase passphraseBuffer2, err := secret.ReadPassphrase("Confirm passphrase: ") if err != nil { + passphraseBuffer1.Destroy() + return nil, fmt.Errorf("failed to read passphrase confirmation: %w", err) } - defer passphraseBuffer2.Destroy() // Compare passphrases if passphraseBuffer1.String() != passphraseBuffer2.String() { + passphraseBuffer1.Destroy() + passphraseBuffer2.Destroy() + return nil, fmt.Errorf("passphrases do not match") } - // Create a new buffer with the confirmed passphrase - return memguard.NewBufferFromBytes(passphraseBuffer1.Bytes()), nil + // Clean up the second buffer, we'll return the first + passphraseBuffer2.Destroy() + + // Return the first buffer (caller is responsible for destroying it) + return passphraseBuffer1, nil }