Bumps golangci-lint from v2.1.6 (digest-only pin in the `Dockerfile` lint stage) to v2.12.2, pinned by tag and digest (Debian-based image).
Replaces `.golangci.yml` with the canonical strict config: all linters enabled except the standard disable list (`exhaustruct`, `depguard`, `godot`, `wsl`, `wrapcheck`, `varnamelen`), `lll` at 88, `funlen` 80/50, `cyclop` 15, `dupl` 100, and test files are now linted (the old config had `tests: false`, an enable-only list of ~20 linters, `lll` 120, and a blanket exclusion of `internal/macse`).
The stricter config surfaced ~1550 findings, all fixed:
- `wsl_v5` (439) / `nlreturn` (24): blank-line insertions
- `lll` (309): line wrapping at 88 columns; long literals split with `+` concatenation, values unchanged
- `noinlineerr` (130): `if err := ...` split into assignment plus check
- `paralleltest` (116): `t.Parallel()` added to tests without shared state; reasoned `//nolint` where `t.Setenv` or shared fixtures forbid it
- `err113` (97): package-level sentinel errors (new `internal/vault/errors.go`), `%w` wrapping, `errors.Is`
- `perfsprint` (74) / `modernize` (39) / `intrange`: `strconv`, `errors.New`, `slices.Contains`, `any`, `SplitSeq`
- `goconst` (40) / `dupword` (41) / `testifylint` (42) / `thelper` (33): constants, assertion fixes, `t.Helper()`
- `noctx` (22): `exec.CommandContext` for gpg/CLI invocations
- `testpackage` (18): black-box tests moved to `_test` packages where they use only exported identifiers; white-box files carry a reasoned `//nolint`
- `funlen`/`cyclop`/`gocognit`/`nestif`/`dupl`: behavior-preserving helper extraction
- assorted singletons: `gosec`, `gosmopolitan`, `funcorder`, `nonamedreturns`, `makezero`, `prealloc`, `godox`, `nolintlint`, `ireturn`, `nilnil`, `gochecknoinits`
## User-visible strings
**None changed.** Every error message this branch composes is byte-identical to the one `main` composes.
The `err113` sentinels are shaped so `fmt.Errorf` reassembles the original text around them: the sentinel carries the fixed words and the caller supplies the interpolated value in the position it has always occupied. Where the value sits mid-sentence the sentinel holds only a fragment (e.g. `vault.ErrVaultNotFound` is `"does not exist"`, composed by its caller as `vault <name> does not exist`); each such sentinel documents the message it participates in.
Verified mechanically, not by inspection: every `fmt.Errorf` and `errors.New` call site in both trees is parsed, the `Error()` text of any sentinel passed to `%w` is substituted in, and the resulting sets of composed message templates are compared. All 350 templates `main` produces are still produced, character for character. The set of lost or altered messages is empty.
## `unlocker list`
`findUnlockerIDByMetadata` returns `(string, error)` rather than signalling failure with an empty ID, so an unreadable `unlockers.d` is no longer indistinguishable from "no matching entry". `UnlockersList` skips such an entry with a warning naming the directory — its behavior before the scan was extracted into a helper — instead of emitting a row under a synthesized fallback ID that no `unlocker remove` or `unlocker select` can match and that suppresses the current-unlocker marker. The duplicate-check and shell-completion callers skip on the same condition, matching their pre-extraction behavior. Covered by `internal/cli/unlockers_list_test.go`.
`TODO.md` records the change plus follow-ups (version-completion TODOs formerly in code comments, darwin-gated files exceeding 88 columns that Linux CI does not lint).
`make check` is green and the pinned v2.12.2 image reports `0 issues.` Note the test suite needs the memlock ulimit from `script/cibuild` for the 10MB memguard test; that requirement is pre-existing.
Not changed: `script/bootstrap` installs golangci-lint via the system package manager (no version pin to bump), and `script/lint` invokes whatever `golangci-lint` is on PATH. golangci-lint v2.12 deprecates `gomodguard` in favor of `gomodguard_v2` (warning only); the canonical config owns that decision.
Co-authored-by: sneak <sneak@sneak.berlin>
Reviewed-on: #29
Co-authored-by: clawbot <clawbot@noreply.example.org>
Co-committed-by: clawbot <clawbot@noreply.example.org>
Audit of the codebase found 9 locations where errors or anomalous
conditions were silently swallowed or only logged via Debug(). Users
should be informed when something unexpected happens, even if the
program can continue.
Changes:
- DetermineStateDir: warn on config dir fallback to ~/.config
- info_helper: warn when vault/secret stats cannot be read
- unlockers list: warn on metadata read/parse failures (fixes FIXMEs)
- unlockers list: warn on fallback ID generation
- checkUnlockerExists: warn on errors during duplicate checking
- completions: warn on unlocker metadata read/parse failures
- version list: upgrade metadata load failure from Debug to Warn
- secrets: upgrade file close failure from Debug to Warn
- version naming: warn on malformed version directory names
Closes#19
When an unlocker directory exists but is missing unlocker-metadata.json,
log a debug warning and skip it instead of returning a hard error that
crashes the entire 'unlocker ls' command.
Closes#1
Change NewCLIInstance() and NewCLIInstanceWithFs() to return
(*Instance, error) instead of panicking on DetermineStateDir failure.
Callers in RunE contexts propagate the error. Callers in command
construction (for shell completion) use log.Fatalf. Test callers
use t.Fatalf.
Addresses review feedback on PR #18.
Verifies that getLongTermPrivateKey reads the derivation index from
vault metadata instead of using hardcoded index 0. Test creates a
mock vault with DerivationIndex=5 and confirms the derived key
matches index 5.
Previously, getLongTermPrivateKey() always used derivation index 0 when
deriving the long-term key from a mnemonic. This caused wrong key
derivation for vaults with index > 0 (second+ vault from same mnemonic),
leading to silent data corruption in keychain unlocker creation.
Now reads the vault's actual DerivationIndex from vault-metadata.json.
- isValidSecretName() now rejects names with '..' path components (e.g. foo/../bar)
- GetSecretObject() now calls isValidSecretName() before building paths
- Added test cases for mid-path traversal patterns
When os.UserConfigDir() fails, DetermineStateDir falls back to
os.UserHomeDir(). Previously the error from UserHomeDir was discarded,
which could result in a dangerous root-relative path (/.config/...) if
both calls fail.
Now DetermineStateDir returns (string, error) and propagates failures
from both UserConfigDir and UserHomeDir.
Closes#14
The isValidSecretName() regex only allowed lowercase letters [a-z], rejecting
valid secret names containing uppercase characters (e.g. AWS access key IDs).
Changed regex from ^[a-z0-9\.\-\_\/]+$ to ^[a-zA-Z0-9\.\-\_\/]+$ and added
tests for uppercase secret names in both vault and secret packages.
Add isValidSecretName() check at the top of GetSecretVersion(), matching
the existing validation in AddSecret(). Without this, crafted secret names
containing path traversal sequences (e.g. '../../../etc/passwd') could be
used to read files outside the vault directory.
Add regression tests for both GetSecretVersion and GetSecret.
Closes#13
The stub previously panicked on all methods including NewKeychainUnlocker,
which is called from vault code when processing keychain-type unlocker
metadata. This caused crashes on Linux/Windows when a vault synced from
macOS contained keychain unlockers.
Now returns proper error values, allowing graceful degradation and
cross-platform vault portability.
CreatePassphraseUnlocker already encrypts and writes the long-term
private key to longterm.age. The Init command was doing this a second
time, overwriting the file with a functionally equivalent but
separately encrypted blob. This was wasteful and a maintenance hazard.
The decrypted data from io.ReadAll was copied into a memguard
LockedBuffer but the original byte slice was never zeroed, leaving
plaintext in swappable, dumpable heap memory.
NumSecrets() previously looked for non-directory, non-'current' files
directly under each secret directory, but the only children are
'current' (file, excluded) and 'versions' (directory, excluded),
so it always returned 0.
Now checks for the existence of the 'current' file, which is the
canonical indicator that a secret exists and has an active version.
This fixes the safety check in UnlockersRemove that was always
allowing removal of the last unlocker.
Implement syntax: secret move/mv <vault>:<secret> <vault>[:<secret>]
- Copies all versions to destination vault with re-encryption
- Deletes source after successful copy (true move)
- Add --force flag to overwrite existing destination
- Support both within-vault rename and cross-vault move
- Add shell completion for vault:secret syntax
- Include integration tests for cross-vault move
When removing a keychain unlocker, if the keychain item doesn't exist
(e.g., already manually deleted or vault synced from another machine),
the removal should still succeed since the goal is to remove the
unlocker and the keychain item being gone already satisfies that goal.
- currentvault now contains just the vault name (e.g., "default")
- current-unlocker now contains just the unlocker name (e.g., "passphrase")
- current version file now contains just the version (e.g., "20231215.001")
- Resolution functions prepend the appropriate directory prefix
- Remove all symlink creation and resolution in favor of plain files
- currentvault file now contains relative path like "vaults.d/default"
- current-unlocker file now contains relative path like "unlockers.d/passphrase"
- current version file now contains relative path like "versions/20231215.001"
- Simplify path resolution to just read file contents and join with parent dir
- Update all tests to read files instead of using os.Readlink
- Add Print method to CLI Instance that uses cmd.OutOrStdout()
- Update GetSecretWithVersion to use cli.Print instead of cmd.Print
- Add test to verify secret values go to stdout, not stderr
- Store command reference in Instance for proper output handling
- 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
- Restrict --keyid flag to PGP unlocker type only
- Add validation to prevent --keyid usage with non-PGP unlockers
- Implement 'secret move' command with 'mv' and 'rename' aliases
- Add comprehensive tests for move functionality
- Update documentation to reflect optional nature of --keyid for PGP
The move command allows renaming or moving secrets within a vault while
preserving all versions and metadata. It fails if the destination already
exists to prevent accidental overwrites.
- Rename 'unlockers' command to 'unlocker' for consistency
- Move all unlocker subcommands (list, add, remove) under single 'unlocker' command
- Add --quiet/-q flag to 'secret list' for scripting support
- Update documentation and tests to reflect command changes
The quiet flag outputs only secret names without headers or formatting,
making it ideal for shell script usage like: secret get $(secret list -q | head -1)
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.
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.
- 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.
- 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
- 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
- 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
