1
0
forked from sneak/secret
Commit Graph

67 Commits

Author SHA1 Message Date
75c3d22b62 Fix vault creation to require mnemonic and set up initial unlocker
- Vault creation now prompts for mnemonic if not in environment
- Automatically creates passphrase unlocker during vault creation
- Prevents 'missing public key' error when adding secrets to new vaults
- Updates tests to reflect new vault creation flow
2025-07-26 21:58:57 +02:00
40ea47b2a1 Add missing changes from feature branch
- Update Makefile to run lint and vet before tests
- Add install target to Makefile
- Fix keychainunlocker_stub.go for non-Darwin platforms
2025-07-22 12:51:02 +02:00
a09fa89f30 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.
2025-07-21 22:05:23 +02:00
7af1e6efa8 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
2025-07-21 18:57:58 +02:00
09b3a1fcdc 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.
2025-07-21 17:48:47 +02:00
816f53f819 Replace shell-based keychain implementation with keybase/go-keychain library
- Replaced exec.Command calls to /usr/bin/security with native keybase/go-keychain API
- Added comprehensive test suite for keychain operations
- Fixed binary data storage in tests using hex encoding
- Updated macse tests to skip with explanation about ADE requirements
- All tests passing with CGO_ENABLED=1
2025-07-21 15:58:41 +02:00
e53161188c Fix remaining memory security issues
- Fixed gpgDecryptDefault to return *memguard.LockedBuffer instead of []byte
- Updated GPGDecryptFunc signature and all implementations
- Confirmed getSecretValue already returns LockedBuffer (was fixed earlier)
- Improved passphrase string handling by removing intermediate variables
- Note: String conversion for passphrases is unavoidable due to age library API
- All GPG decrypted data is now immediately protected in memory
2025-07-15 09:08:51 +02:00
63cc06b93c Fix DecryptWithIdentity to return LockedBuffer
- Changed DecryptWithIdentity to return *memguard.LockedBuffer instead of []byte
- Updated all callers throughout the codebase to handle LockedBuffer
- This ensures decrypted data is protected in memory immediately after decryption
- Fixed all usages in vault, secret, version, and unlocker implementations
- Removed duplicate buffer creation and unnecessary memory clearing
2025-07-15 09:04:34 +02:00
8ec3fc877d Fix GetValue methods to return LockedBuffer internally
- Changed Secret.GetValue and Version.GetValue to return *memguard.LockedBuffer
- Updated all internal callers to handle LockedBuffer properly
- For backward compatibility, vault.GetSecret still returns []byte but makes a copy
- This ensures secret values are protected in memory during decryption
- Updated tests to handle LockedBuffer returns
- Fixed CLI getSecretValue to use LockedBuffer throughout
2025-07-15 08:59:23 +02:00
819902f385 Fix gpgEncryptDefault to accept LockedBuffer for data parameter
- Changed GPGEncryptFunc signature to accept *memguard.LockedBuffer instead of []byte
- Updated gpgEncryptDefault implementation to use LockedBuffer
- Updated all callers including tests to pass LockedBuffer
- This ensures GPG encryption data is protected in memory
- Fixed linter issue with line length
2025-07-15 08:46:33 +02:00
292564c6e7 Fix storeInKeychain to accept LockedBuffer for data parameter
- Changed storeInKeychain to accept *memguard.LockedBuffer instead of []byte
- Updated caller in CreateKeychainUnlocker to create LockedBuffer before storing
- This ensures keychain data is protected in memory before being stored
- Added proper buffer cleanup with defer Destroy()
2025-07-15 08:44:09 +02:00
eef2332823 Fix EncryptWithPassphrase to accept LockedBuffer for data parameter
- Changed EncryptWithPassphrase to accept *memguard.LockedBuffer instead of []byte
- Updated all callers to pass LockedBuffer:
  - CreatePassphraseUnlocker in vault/unlockers.go
  - Keychain unlocker in keychainunlocker.go
  - Tests in passphrase_test.go
- Removed intermediate dataBuffer creation since data is now already protected
- This ensures sensitive data is protected in memory throughout encryption
2025-07-15 08:42:46 +02:00
e82d428b05 Remove deprecated Secret.Save function
- Removed unused deprecated Save(value []byte, force bool) function
- This function accepted unprotected secret data which was a security issue
- All code now uses vault.AddSecret directly with LockedBuffer
- Updated TODO.md to reflect completion of this security fix
2025-07-15 08:40:35 +02:00
7596049828 uses protected memory buffers now for all secrets in ram 2025-07-15 08:32:33 +02:00
d3ca006886 Merge branch 'main' into fix-memory-security 2025-07-15 07:36:13 +02:00
7c5e78db17 fix: update JSON fields from snake_case to camelCase and make tests quiet by default
- Update all JSON field references in tests from snake_case to camelCase
- Update vault list JSON output to use currentVault instead of current_vault
- Make integration tests quiet by default unless run with -v flag
- Fix tests that were using exec.Command to use in-process execution helpers
- Tests now only show debug output when explicitly requested or on failure
2025-07-15 07:35:48 +02:00
c9774e89e0 WIP: refactor to use memguard for secure memory handling
- Add memguard dependency
- Update ReadPassphrase to return LockedBuffer
- Update EncryptWithPassphrase/DecryptWithPassphrase to accept LockedBuffer
- Remove string wrapper functions
- Update all callers to create LockedBuffers at entry points
- Update interfaces and mock implementations
2025-07-15 07:23:58 +02:00
f9938135c6 fix: resolve all remaining linter issues (staticcheck, tagliatelle, lll)
- Fix staticcheck QF1011: Remove explicit type declaration for io.Writer variables
- Fix tagliatelle: Change all JSON tags from snake_case to camelCase
  - created_at → createdAt
  - keychain_item_name → keychainItemName
  - age_public_key → agePublicKey
  - age_priv_key_passphrase → agePrivKeyPassphrase
  - encrypted_longterm_key → encryptedLongtermKey
  - derivation_index → derivationIndex
  - public_key_hash → publicKeyHash
  - mnemonic_family_hash → mnemonicFamilyHash
  - gpg_key_id → gpgKeyId
- Fix lll: Break long function signature line to stay under 120 character limit

All linter issues have been resolved. The codebase now passes all linter checks.
2025-07-15 06:33:25 +02:00
386a27c0b6 fix: resolve all revive linter issues
Added missing package comments:
- cmd/secret/main.go
- internal/cli/cli.go
- internal/secret/constants.go
- internal/vault/management.go
- pkg/bip85/bip85.go

Fixed comment format issues for exported items:
- EnvStateDir, EnvMnemonic, EnvUnlockPassphrase, EnvGPGKeyID in constants.go
- Metadata, UnlockerMetadata, SecretMetadata, Configuration in metadata.go
- AppBIP39, AppHDWIF, AppXPRV in bip85.go

Replaced unused parameters with underscore (_):
- generate.go:39 - parameter 'args'
- init.go:30 - parameter 'args'
- unlockers.go:39,77,102 - parameter 'args' or 'cmd'
- vault.go:37 - parameter 'args'
- management.go:34 - parameter 'target'
2025-07-15 06:06:48 +02:00
080a3dc253 fix: resolve all nlreturn linter errors
Add blank lines before return statements in all files to satisfy
the nlreturn linter. This improves code readability by providing
visual separation before return statements.

Changes made across 24 files:
- internal/cli/*.go
- internal/secret/*.go
- internal/vault/*.go
- pkg/agehd/agehd.go
- pkg/bip85/bip85.go

All 143 nlreturn issues have been resolved.
2025-07-15 06:00:32 +02:00
811ddee3b7 fix: resolve all nestif linter errors
- Extract getLongTermPrivateKey helper function to reduce nesting in keychainunlocker.go and pgpunlocker.go
- Add getPassphrase helper method to reduce nesting in passphraseunlocker.go
- Refactor version serial extraction to use early returns in version.go
- Extract resolveRelativeSymlink and tryResolveOsSymlink helpers in management.go
- Add processMnemonicForVault helper to reduce nesting in vault creation
- Extract resolveUnlockerDirectory and readUnlockerPathFromFile helpers in unlockers.go
- Add findUnlockerByID helper to reduce duplicate code in RemoveUnlocker and SelectUnlocker

All tests pass after refactoring.
2025-07-15 05:47:16 +02:00
95ba80f618 fix: resolve gochecknoglobals, gosec, lll, and mnd linter errors
- Add nolint comments for BIP85 standard constants (MainNetPrivateKey, TestNetPrivateKey)
- Handle error return from shake.Write() in NewBIP85DRNG
- Fix line length issue by moving nolint comment to separate line
- Add nolint comment for cobra.ExactArgs(2) magic number
- Replace magic number 32 with named constant x25519KeySize in agehd package
2025-07-09 12:49:59 -07:00
d710323bd0 fix: add nolint comments for necessary global variables in internal/secret
Add nolint:gochecknoglobals comments for legitimate global variables:
- debugEnabled and debugLogger: Package-wide debug state management
- GPGEncryptFunc and GPGDecryptFunc: Required for test mocking
- getCurrentVaultFunc: Required to break import cycle between packages
2025-07-09 12:47:51 -07:00
38b450cbcf fix: resolve mnd and nestif linter errors
- Added constants to replace magic numbers:
  - agePrivKeyPassphraseLength = 64
  - versionNameParts = 2
  - maxVersionsPerDay = 999
- Refactored crypto.go to reduce nesting complexity:
  - Inverted if condition to handle non-existent secret first
  - Extracted getSecretValue helper method
2025-07-09 07:05:07 -07:00
eb19fa4b97 fix: replace unused parameters with underscores (revive)
- Replace unused function parameters with _ in test files
- Affects version_test.go, debug_test.go, and pgpunlock_test.go
2025-06-20 12:50:16 -07:00
bdcddadf90 fix: resolve exported type stuttering issues (revive)
- Rename VaultMetadata to Metadata in internal/vault package to avoid stuttering
- Rename BIP85DRNG to DRNG in pkg/bip85 package to avoid stuttering
- Update all references in code and tests
2025-06-20 12:47:06 -07:00
4062242063 fix: break long error messages to meet line length limits
Split long error messages at logical points to comply with 120 character
line length limit while maintaining readability.
2025-06-20 09:51:26 -07:00
abcc7b6c3a fix: resolve gosec integer overflow and unconvert issues
- Fix G115 integer overflow by converting uint32 to int comparison
- Remove unnecessary int() conversions for syscall constants
- syscall.Stdin/Stderr/Stdout are already int type
2025-06-20 09:50:00 -07:00
9e35bf21a3 fix: more nlreturn and testifylint issues
- Add blank lines before return statements
- Use require.Error instead of assert.Error for error assertions
- Keep exact float64 comparisons as-is (they are integers from JSON)
2025-06-20 09:40:17 -07:00
2a1e0337fd fix: add blank lines before return statements (nlreturn)
Fix nlreturn linting issues by adding blank lines before return
statements in cli and secret packages.
2025-06-20 09:37:56 -07:00
c450e1c13d fix: replace remaining os.Setenv with t.Setenv in tests
Replace all os.Setenv calls with t.Setenv in test functions to ensure
proper test environment cleanup and better test isolation. This leaves
only legitimate application code and helper functions using os.Setenv.
2025-06-20 09:22:01 -07:00
08a42b16dd fix: replace os.Setenv with t.Setenv in tests (usetesting)
Replace os.Setenv calls with t.Setenv in test functions to ensure
proper test environment cleanup and better test isolation.
2025-06-20 09:13:01 -07:00
4fe49ca8d0 Fix unused parameter errors in secret test mock implementations
Rename unused force and passphrase parameters to _ in MockVault
interface implementations as they are required by the interface
2025-06-20 08:52:19 -07:00
8ca7796d04 Fix unused parameter errors in debug.go slog.Handler interface
Rename unused parameters to _ in WithAttrs and WithGroup methods
as these are required by the slog.Handler interface
2025-06-20 08:51:13 -07:00
e5b18202f3 Fix revive package stuttering errors
- Rename SecretMetadata to Metadata in secret package
- Rename SecretVersion to Version in secret package
- Update NewSecretVersion to NewVersion function
- Update all references across the codebase including:
  - vault package aliases
  - CLI usage
  - test files
  - method receivers and signatures
2025-06-20 08:48:17 -07:00
434b73d834 Fix intrange and G101 linting issues
- Convert for loops to use Go 1.22+ integer ranges in generate.go and helpers.go
- Disable G101 false positives for test vectors and environment variable names
- Add file-level gosec disable for bip85_test.go containing BIP85 test vectors
- Add targeted nolint comments for legitimate test data and constants
2025-06-20 08:08:01 -07:00
985d79d3c0 fix: resolve critical security vulnerabilities in debug logging and command execution
- Remove sensitive data from debug logs (vault/secrets.go, secret/version.go)
- Add input validation for GPG key IDs and keychain item names
- Resolve GPG key IDs to full fingerprints before storing in metadata
- Add comprehensive test coverage for validation functions
- Add golangci-lint configuration with additional linters

Security improvements:
- Debug logs no longer expose decrypted secret values or private keys
- GPG and keychain commands now validate input to prevent injection attacks
- All validation uses precompiled regex patterns for performance
2025-06-20 07:50:26 -07:00
004dce5472 passes tests now! 2025-06-20 07:24:48 -07:00
0b31fba663 latest from ai, it broke the tests 2025-06-20 05:40:20 -07:00
03e0ee2f95 refactor: remove confusing dual ID method pattern from Unlocker interface - Removed redundant ID() method from Unlocker interface - Removed ID field from UnlockerMetadata struct - Modified GetID() to generate IDs dynamically based on unlocker type and data - Updated vault package to create unlocker instances when searching by ID - Fixed all tests and CLI code to remove ID field references - IDs are now consistently generated from unlocker data, preventing redundancy 2025-06-11 15:21:20 -07:00
9adf0c0803 refactor: fix redundant metadata fields across the codebase - Removed VaultMetadata.Name (redundant with directory structure) - Removed SecretMetadata.Name (redundant with Secret.Name field) - Removed AgePublicKey and AgeRecipient from PGPUnlockerMetadata - Removed AgePublicKey from KeychainUnlockerMetadata - Changed PGP and Keychain unlockers to store recipient in pub.txt instead of pub.age - Fixed all tests to reflect these changes - Follows DRY principle and prevents data inconsistency 2025-06-09 17:44:10 -07:00
e9d03987f9 refactor: remove redundant SecretName and Version fields from VersionMetadata - Removed SecretName and Version fields that were redundant with directory structure and parent SecretVersion struct - Updated tests to remove references to deleted fields - Follows DRY principle and prevents potential data inconsistency 2025-06-09 17:26:57 -07:00
2e3fc475cf fix: Use vault metadata derivation index for environment mnemonic - Fixed bug where GetValue() used hardcoded index 0 instead of vault metadata - Added test31 to verify environment mnemonic respects vault derivation index - Rewrote test19DisasterRecovery to actually test manual recovery process - Removed all test skip statements as requested 2025-06-09 17:21:02 -07:00
02be4b2a55 Fix integration tests: correct vault derivation index and debug test failures 2025-06-09 04:54:45 -07:00
d76a4cbf4d fix tests 2025-06-08 22:13:22 -07:00
fbda2d91af add secret versioning support 2025-06-08 22:07:19 -07:00
f59ee4d2d6 'unlock keys' renamed to 'unlockers' 2025-05-30 07:29:02 -07:00
0bf8e71b52 fix: resolve ineffectual assignment lint error in pgpunlock_test.go 2025-05-29 16:36:10 -07:00
34d6870e6a feat: add derivation index to vault metadata for unique keys - Add VaultMetadata fields: DerivationIndex, LongTermKeyHash, MnemonicHash - Implement GetNextDerivationIndex() to track and increment indices for same mnemonics - Update init and import commands to use proper derivation indices - Add ComputeDoubleSHA256() for hash calculations - Save vault metadata on creation with all derivation information - Add comprehensive tests for metadata functionality. This ensures multiple vaults using the same mnemonic will derive different long-term keys by using incremented derivation indices. The mnemonic is double SHA256 hashed and stored to track which vaults share mnemonics. Fixes TODO item #5 2025-05-29 16:23:29 -07:00
1a1b11c5a3 Add comprehensive PGP unlock key testing with non-interactive GPG support 2025-05-29 15:05:58 -07:00