On an unchanged tree docker served every check step of the Dockerfile
from its build cache, so a second script/cibuild ran no lint, tests or
build and still succeeded.
script/cibuild now passes the current time as the CHECK_EPOCH build
argument. The lint and build stages each declare it after their module
download and before `COPY . .`. A build argument whose value changes
makes every RUN step after its declaration miss the cache, so the
checks run on each build while the base images, the apk install and the
module downloads stay cached.
Model: opus-5-5
CreatePGPUnlocker looked up the GPG key's fingerprint, and the keychain
unlocker got the long-term key, only after writing part of the unlocker,
so a failure there left a directory with no metadata. Both now do every
step that can fail before writing anything. `secret unlocker add pgp`
looks the fingerprint up once, for its duplicate check, and passes it to
CreatePGPUnlocker to record. All four unlocker types write their files
through the new secret.WriteDir, which builds a new directory in a
temporary directory, renames it into place when complete and removes it
on a failure. A directory that already exists, as when an unlocker
replaces one of the same name, is written in place and never removed.
Model: opus-5-5
findUnlockerByID failed on the first unlocker directory whose metadata
could not be checked, read or parsed, so `secret unlocker select` and
`remove` failed when one sorted before the unlocker asked for. It now
skips such a directory with the warning ListUnlockers gives, through
the code both now share. A skipped directory is removed by its
directory name, with RemoveDirAtomic, and cannot be selected. Removing
one whose metadata file is missing or corrupt never counts as removing
the last unlocker; removing one whose metadata file cannot be checked
for or read does, since it may be the only working unlocker.
Model: opus-5-5
A failed command printed the whole usage text after its error, burying
it. The root command's PersistentPreRunE now turns usage off, so an
error from running the command is printed once on its own. Wrong arity,
an unknown flag, a bad flag value, a missing required flag and broken
flag groups still get usage: cobra checks arguments and flag values
before that hook but required flags and flag groups only after it, so
the hook checks those two first. Root SilenceUsage was not used: in
this cobra version it hides usage for argument and flag errors too.
Cobra still prints the error; Entry is unchanged.
Model: opus-5-5
Vault.GetSecret and Vault.GetSecretVersion return the decrypted value
as a *memguard.LockedBuffer instead of copying it into an ordinary
[]byte that nothing wiped. Every caller destroys the buffer, and
`secret get` writes its bytes straight to stdout, still with no
trailing newline. Instance.Print, which formatted through fmt and had
no other callers, is removed, and so is a debug log line in
`get --version` that held the plaintext value.
Model: opus-5-5
The Makefile exported DOCKER_HOST pointing at one private machine, so
every docker call made through make, `make lint` and `make check`
included, failed everywhere else. The line is gone: docker uses the
local daemon, or a DOCKER_HOST set in the environment.
`make build` now calls the new `script/build`, which stamps the version
and commit as the Makefile did. A VERSION set in the environment now
wins over `git describe`, not only one given as `make build VERSION=x`.
build, clean, install and docker-run are phony; install depends on
build. The vet target is removed: `script/test` runs `go vet` first.
Model: opus-5-5
Adding a PGP unlocker checked unlockers.d for a duplicate and, when the
directory or an unlocker's metadata file could not be read, reported no
duplicate and went on. The check now reads unlockers.d itself and stops
with an error naming the path and cause; `unlocker list` keeps skipping
unlockers it cannot read.
The same flaw guarded removing the last unlocker and removing a vault
(an unreadable secrets directory counted as no secrets) and vault
import (an unreadable pub.age counted as no long-term key). Those now
stop with an error too.
`vault rm` and `unlocker rm` keep the state directory lock and now do
their work in an unexported function, as `vault import` does.
Model: opus-5-5
.gitignore had no secret patterns at all. It is now the org's standard
file, which ignores .env, .env.*, *.pem and *.key and editor and OS
files, plus this repo's /secret (anchored, so internal/secret/ is not
matched), *.log, *.test and settings.local.json. The stale
.cursorrules and coverage.out entries are gone. No tracked file
matches the new patterns.
.dockerignore also leaves out node_modules and ends with a newline.
.git stays in the build context because the build stamps the version
with git describe; .git/config stays excluded.
Model: opus-5-5
vault.CreateVault now checks for the vault before writing anything and
fails with "vault NAME already exists" (vault.ErrVaultExists). secret
init and secret vault create call it while holding the state directory
lock, so two creates at once cannot both pass the check. Before, either
command over an existing vault replaced its metadata, passphrase
unlocker and longterm.age, so none of its secrets could be decrypted.
Both commands now ask for the unlocker passphrase before creating the
vault, so one stopped at that prompt leaves no vault without an
unlocker behind, which they would then refuse to create again.
The lock tests set up the vault "work" instead of "default", which init
now refuses to create again.
Model: opus-5-5
The test that each changing command waits for the state directory lock
slept a fixed 100 ms per command. It now polls the goroutine stacks until
the command is parked in vault.LockStateDir, checks the state directory
is unchanged, and releases the lock; a command that takes no lock still
fails by finishing first.
newTwoVaultFs creates its two vaults, each with a passphrase unlocker,
once, and returns a fresh copy of them on every call, so the six path and
move tests no longer each pay for two passphrase key derivations.
Model: opus-5-5
On a case-insensitive filesystem (the macOS default) "Foo" and "foo" name
one secret, so `secret mv --force Foo foo` removed the destination, which
was the source, and lost the secret with every version. Between vaults the
copy replaced the source, and removing the source then removed the copy.
Both kinds of move now compare the two secret directories with
os.SameFile before changing anything and reject the move if they are one,
with or without --force. The tests give one secret two names with
symbolic links on the real filesystem.
Model: opus-5-5
script/lint builds the new Dockerfile.lint, where golangci-lint runs as
a build step. The lint stage is rebuilt on every run, so an unchanged
tree is linted too; the module download stays cached. script/bootstrap
no longer installs golangci-lint. The Dockerfile lint stage calls
golangci-lint directly, since make lint now starts a docker build.
golangci-lint config verify is not run: it fetches its schema live over
unpinned HTTPS.
Model: opus-5-5
PGPUnlocker.GetID() panicked when its metadata could not be read or
parsed, which took down `secret unlocker list` for every unlocker. It
now warns with the unlocker's directory and returns `pgp-unknown`;
metadata with an empty GPG key ID counts as corrupt too.
ListUnlockers now skips, with a warning, an unlocker whose metadata
file cannot be checked for, read or parsed, as it already did for a
missing one. The listing's ID lookup skips such a directory without
warning again.
This is the first half of the issue only. Passing the mnemonic in
memory moved to #60.
Model: opus-5-5
Co-authored-by: clawbot <sneak+clawbot@sneak.cloud>
`secret mv --force x x` deleted the secret: a move within one vault
removes an existing destination before renaming the source onto it. The
same happened for `work:x work:`, `work:x work` and `work:x ""`, and for
`work:x work/:x`, which named one vault two ways and so was taken for a
move between vaults.
A move whose two names are the same is now rejected before anything
changes. Every vault named with `vault:` must be one of the existing
vaults by exact name, checked before choosing between the two kinds of
move. A move within a named vault no longer makes it the current vault.
The test runs each rejected move on a copy of two in-memory vaults and
requires the exact error and an unchanged state directory.
Model: opus-5-5
Each command that changes the state directory holds one lock: flock(2)
on `lock` in the state directory, dropped by the kernel if the process
dies, or a process-wide mutex on the in-memory test filesystem. It
covers the state directory, not each vault, because `currentvault`,
`vault create` and cross-vault moves span vaults, and a lock file in a
vault would be deleted by `vault remove` under a waiting command.
Files go through `secret.WriteFileAtomic`; versions, new secrets and
cross-vault copies are built in a temporary directory and renamed into
place; removals rename out of the way first. Left for later: replacing
an unlocker (#71) and deleting
what an interrupted command leaves under a `.tmp-` name
(#75).
Model: opus-5-5
Co-authored-by: clawbot <sneak+clawbot@sneak.cloud>
version rm, version promote and get --version joined the version
argument into a path unchecked, so "", ".", "..", "../../.." removed or
read every version, the secret, the vault or directories above it.
A version is now accepted only if it is one of the versions
ListVersions lists for the secret, compared by name before any path is
built (secret.VersionExists, used by all three). An empty --version is
rejected instead of meaning the current version: GetSecretVersion no
longer treats "" as current, and GetSecret looks the current version
up itself.
Model: opus-5-5
Entry() now returns the exit code and only main calls os.Exit, so the
deferred memguard.Purge() in Entry() runs on success and on error;
before, os.Exit(1) skipped every deferred Destroy().
SIGINT and SIGTERM go through memguard's handler, which wipes every
buffer and exits with status 1. The passphrase prompt turns terminal
echo off until its read returns, and the handler exits before that, so
the handler first restores the terminal settings saved at startup, but
only when this process is in the terminal's foreground process group: a
background process that changes the terminal is stopped instead of
exiting.
Model: opus-5-5
`secret rm ..` deleted the whole vault; `secret rm .` and `secret rm ""`
deleted every secret. rm, mv, the version commands, encrypt and decrypt
built paths from the name unchecked; import checked it only after
reading the source file.
Each now calls vault.ValidateSecretName, which wraps the existing rule,
on the name as given, before building any path; MoveSecret checks both
names before switching the current vault. Its error and README.md state
the rule. The test-only copy of the rule in internal/secret is removed.
The regression test runs each rejected command on a copy of two
in-memory vaults and requires the exact error and an unchanged state
directory.
Model: opus-5-5
The passphrase protecting the keychain unlocker's age key was a plain
string passed through encoding/json, leaving copies in ordinary memory
when an unlocker was created and each time one was used.
It is now generated into a locked buffer, and KeychainData, moved to
keychaindata.go, which is not darwin-only so its tests run on Linux,
writes and reads the keychain JSON itself: encode copies the parts
straight into a locked buffer, and decodeKeychainData takes the
passphrase from a json.RawMessage that it wipes. The JSON field names
are unchanged. keychainunlocker.go only calls this code and stores the
item from the locked buffer without a string copy.
Model: opus-5-5
The size tests skip a case whose secret needs more locked memory than
the process can lock, found by locking a buffer of that size: memguard
panics otherwise, and a plain `docker build .` runs under an 8 MiB
RLIMIT_MEMLOCK. script/cibuild, or any process allowed to lock past the
limit, runs every case.
The build stage stamps the VERSION build argument, else
`git describe --tags --always`, and fails when .git is present but
yields no version. `make build` stamps `git describe` too instead of
the fixed 0.1.0. .dockerignore keeps .git/config out; script/docker is
now the canonical copy.
Model: opus-5-5
Co-authored-by: clawbot <sneak+clawbot@sneak.cloud>
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>
Adds `.editorconfig`, copied verbatim from `sneak/dnswatcher` (`root = true`; space/4/lf/utf-8/trim/final-newline, with `[Makefile]` overriding to tab). secret had none. Resolves#27.
`make check` is green on the host (`0 issues`, all packages pass). Note: `docker build .` fails only on a pre-existing, environmental `memguard` `mlock` / `RLIMIT_MEMLOCK` issue in the 10MB-secret test — it reproduces identically on pristine `origin/main` with this file removed, and secret's real CI sets the memlock ulimit for it. A static config file cannot affect Go tests, so this change is clean; the container ulimit is a separate pre-existing concern.
Left open for review (not merged).
Co-authored-by: sneak <sneak@sneak.berlin>
Reviewed-on: #28
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.
sneak
clawbot
