Files
secret/TODO.md
T
sneak b9d2797840
check / check (push) Failing after 1s
Skip corrupt unlocker metadata in unlocker select and remove (closes #72)
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
2026-10-04 10:09:52 +00:00

266 lines
15 KiB
Markdown

# Workflow
* branch (from `main`)
* do the work in Next Step
* move Next Step to the top of Completed Steps
* move the top item of Future Steps into Next Step
* commit (`TODO.md` changes in the same commit as the work)
* merge to `main` if the branch is not protected, otherwise open a PR
* push
# Status
pre-1.0. No git tags. TODO.md carries open 1.0 security blockers. Work in
flight on branch secure-enclave-unlocker (clean tree as of 2026-07-06).
# Next Step
Bring the repo into policy compliance in one commit:
- Add fmt-check and hooks targets to the Makefile (test/lint/fmt/check/
docker already exist).
- Add REPO_POLICIES.md and .editorconfig.
- Add .gitea/workflows/check.yml running make check.
- Verify Dockerfile base images are pinned by sha256.
# Completed Steps
- 2026-10-04: `secret unlocker select` and `secret unlocker remove`
skip, with the warning `unlocker list` gives, an unlocker directory
whose metadata file cannot be checked for, read or parsed, instead of
failing when it sorts before the unlocker asked for. Such a directory,
or one without a metadata file, is removed by its directory name, the
name the warning gives; only the directory is removed, since its type
is unknown. 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 always does, since it may be the
only working unlocker, so in a vault with secrets it needs `--force`.
- 2026-10-04: A failed command prints its error once, without the usage
text after it (https://git.eeqj.de/sneak/secret/issues/41). Usage is
still printed for a command called wrongly: wrong number of arguments,
unknown flag, bad flag value, missing required flag, or flags that
break a flag group (mutually exclusive, required together, one
required). The root command's `PersistentPreRunE` turns usage off.
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` would have hidden usage for all of these.
- 2026-10-04: `secret get` keeps the secret in locked memory until it
writes it out (https://git.eeqj.de/sneak/secret/issues/37):
`Vault.GetSecret` and `Vault.GetSecretVersion` return a
`*memguard.LockedBuffer`, which every caller destroys, and `secret get`
writes its bytes straight to stdout, still with no trailing newline.
Before, the value was copied into ordinary memory that nothing wiped,
and `get --version` also wrote it to the debug log.
- 2026-10-04: The `Makefile` no longer sets `DOCKER_HOST`, so its docker
targets use the local docker daemon, or whatever `DOCKER_HOST` the
environment sets. `make build` calls the new `script/build`, which
stamps the version (`VERSION` from the environment, else
`git describe`) and the git commit as before. `build`, `clean`,
`install` and `docker-run` are in `.PHONY`; `make install` depends on
`build`. The `vet` target is gone: `script/test` runs `go vet` first.
- 2026-10-04: `.gitignore` is the org's standard file, which ignores
`.env`, `.env.*`, `*.pem` and `*.key` and editor and OS files, plus
this repo's `/secret`, `*.log`, `*.test` and `settings.local.json`
(https://git.eeqj.de/sneak/secret/issues/40). `.dockerignore` also
leaves out `node_modules`; `.git` stays in the build context for the
version stamp.
- 2026-10-04: `secret init` refuses when the default vault exists, and
`secret vault create NAME` when `NAME` does, with "vault NAME already
exists", before writing anything. The check is in `vault.CreateVault`,
which both commands call while holding the state directory lock, so two
creates of one vault at once cannot both pass the check. Before, either
command replaced the vault's metadata, passphrase unlocker and
`longterm.age`, so none of its secrets could be decrypted any more. Both
commands now ask for the unlocker passphrase before creating the vault,
so one stopped at that prompt leaves no vault behind.
- 2026-10-04: The `internal/cli` tests are back to about their time
before the state directory lock
(https://git.eeqj.de/sneak/secret/issues/80). The test that each
changing command waits for the lock releases it as soon as it sees the
command waiting there, instead of after a fixed 100 ms. The two vaults
with passphrase unlockers that the path and move tests start from are
made once and copied for each test.
- 2026-10-04: `secret mv` rejects a move whose destination is the source
under another name, such as `foo` for `Foo` on a case-insensitive
filesystem (the macOS default) or a name reached through a symbolic
link, before changing anything, with or without `--force`, within a
vault and between vaults; before, `--force` removed the destination and
so deleted the secret. A rename that changes only letter case works on a
case-sensitive filesystem as before.
- 2026-10-04: Lint runs only in docker: `script/lint` builds
`Dockerfile.lint`, where golangci-lint is a build step rebuilt on
every run (`--no-cache-filter`), so an unchanged tree is linted too;
the module download stays cached. `script/bootstrap` no longer
installs golangci-lint, and the `Dockerfile` lint stage calls it
directly instead of `make lint`. `golangci-lint config verify` is not
run: it fetches its schema live over unpinned HTTPS.
- 2026-10-04: A PGP unlocker whose metadata has no usable GPG key ID
no longer panics: `GetID()` warns with the unlocker's directory and
returns `pgp-unknown`. `ListUnlockers` skips, with a warning, an
unlocker whose metadata file cannot be checked for, read or parsed
instead of failing, so `secret unlocker list` still lists the others;
the listing's ID lookup no longer warns about that directory again.
- 2026-10-03: `secret mv` rejects a move whose destination is the
source (`mv --force x x`, `mv --force work:x work:`, or an empty
destination, which defaults to the source name) before changing
anything; before, `--force` removed the destination first and so
deleted the secret. Every vault name given with `vault:` must be one
of the existing vaults by exact name, so `work:x work/:x` is rejected
instead of being taken for a move between two vaults. A move within a
named vault no longer makes that vault the current one, whether it
succeeds or fails.
- 2026-10-03: Commands that change the state directory hold one lock
(`flock` on `lock` in the state directory; a mutex on the in-memory
test filesystem), so concurrent commands no longer lose versions or
race on the current pointers. Every file is written through
`secret.WriteFileAtomic` (temporary file, sync, rename), so no file
is ever half-written and `current`, `currentvault` and
`current-unlocker` never go missing. New versions, new secrets and
cross-vault copies are built in a temporary directory and renamed
into place, and removals rename out of the way first, so a version
or secret is never half-added and never half-removed. An
interrupted command can still leave:
- a broken unlocker, when it was replacing one: an unlocker added
under the directory name of an existing one is rewritten file by
file. That happens to a passphrase unlocker added to a vault that
has one, and to a PGP, keychain or Secure Enclave unlocker added
on the same host and day as another of its type
(https://git.eeqj.de/sneak/secret/issues/71);
- from `init` or `vault create` killed after the passphrase prompt
but before the unlocker is written, a vault with no unlocker,
which `vault create` has already made the current vault;
- from an unlocker add stopped before its metadata is written, a
directory that `unlocker list` warns about and `unlocker rm`
removes only by its directory name;
- data under a `.tmp-` name in the state directory: a secret or
version being added, or the secret, version, unlocker or vault
being removed, encrypted keys included. Nothing deletes it; it
must be deleted by hand
(https://git.eeqj.de/sneak/secret/issues/75).
- 2026-10-03: The checks run before changing a vault now stop with an
error naming the path and cause when they cannot read what they
inspect, instead of reading the failure as "nothing there": the
duplicate check before `unlocker add pgp` (an unreadable
`unlockers.d` or unlocker metadata file), the secret count that
guards removing the last unlocker and removing a vault, and the
existing long-term key check before `vault import`.
- 2026-10-03: `version rm`, `version promote` and `get --version`
accept a version only if it is one of the versions `version list`
lists for that secret, compared as typed before any path is built
(`secret.VersionExists`), and touch nothing otherwise. An empty
`--version` is rejected instead of meaning the current version.
Before, `secret version rm x ../../..` deleted the whole vault,
`secret version rm x ..` the secret, and `.` or `""` every version.
- 2026-10-03: Key material is wiped on every exit: `Entry()` returns
the exit code after its deferred `memguard.Purge()` has run, and only
`main` calls `os.Exit`. SIGINT and SIGTERM go through memguard's
handler, which wipes every buffer before exiting; when the process is
in the terminal's foreground process group it first restores the
terminal settings from startup, so an interrupted passphrase prompt no
longer leaves echo off.
- 2026-10-03: Every command that builds a path from a secret name
checks the name first with `vault.ValidateSecretName` and touches
nothing when it is invalid: `rm`, `mv` (both names, within a vault
and between vaults, before switching the current vault), `import`,
`version list`/`promote`/`rm`, `encrypt` and `decrypt`. The error
and `README.md` state the naming rule. Before, `secret rm ..`
deleted the whole vault and `secret rm .` every secret in it.
- 2026-10-03: The keychain unlocker's age key passphrase stays in
locked memory: it is generated into a locked buffer, and the
keychain JSON is written and read by `KeychainData` code in
`internal/secret/keychaindata.go` (tested on Linux) without
`encoding/json` holding it; the JSON field names are unchanged.
- 2026-10-02: A plain `docker build .` builds again: the size tests
skip a case that needs more locked memory than the process can
lock, and run every case under `script/cibuild`. The image stamps the
`VERSION` build argument, else `git describe --tags --always`, into
`Version`, and fails if `.git` is present but yields no version;
`make build` stamps `git describe` too, not a fixed `0.1.0`.
`.dockerignore` keeps `.git/config` out; `script/docker` is the
canonical copy.
- 2026-08-07: Updated golangci-lint to v2.12.2 with the canonical
`.golangci.yml` (all linters enabled minus the standard disable
list, `lll` 88, tests linted); bumped the `Dockerfile` lint-stage
image to the tagged v2.12.2 Debian digest; fixed all ~1550 new
findings across `internal/` and `pkg/` (line wrapping, `wsl_v5`
blank lines, sentinel errors for `err113`, `t.Parallel()` where
safe, `_test` package conversions, complexity/`dupl` helper
extraction) on branch `golangci-v2.12.2`. Reworked after review:
the `err113` sentinels in `internal/vault`, `internal/secret`,
`internal/cli` and `pkg/bip85` were reshaped so every composed
error message is byte-identical to `main`, and
`findUnlockerIDByMetadata` now returns an error so `unlocker list`
skips an unreadable `unlockers.d` entry with a warning instead of
emitting a fabricated fallback ID.
- 2026-07-07 Adopted scripts-to-rule-them-all: `script/` entrypoints,
Makefile shims, README Entrypoints section
- 2026-03-11: Secure Enclave unlocker for hardware-backed secret
protection, plus review fixes (stub panics, derivation index, tests,
README) on branch secure-enclave-unlocker.
- 2026-02-28: Repo cleanup, removed stale .cursorrules and coverage.out.
- Audit fix wave (issues #1, #2, #3, #13, #14): skip unlockers with
missing metadata, allow uppercase secret names, fix hardcoded
derivation index, validate names in GetSecretVersion against path
traversal, return errors instead of panicking, add Warn() on silent
anomalies.
- Memory security hardening: LockedBuffer used through encrypt/decrypt
paths (Save/EncryptWithPassphrase/GetValue/gpg helpers), deprecated
bare-[]byte APIs removed.
- Per-secret keypair architecture, vault package refactor, versioning
with --version, comprehensive test suite with in-memory filesystem.
- Debug logging system (slog, GODEBUG flag, TTY-aware output).
- Renamed SEP unlocker to Keychain, reorganized import commands.
- 2025-05-28: Initial implementation (vault, age encryption, mnemonic,
CLI).
# Future Steps
- Compliance (after Next Step lands): keep main green under the new
.gitea workflow; run make check before every merge.
- Implement version-number shell completion for the second arg of
`secret version promote` and `secret version rm`
(`internal/cli/version.go`; was an in-code TODO removed for godox).
- Cover mnemonic-vs-xprv identity consistency in
`pkg/agehd/agehd_test.go` `TestMnemonicVsXPRVConsistency` (was an
in-code FIXME removed for godox).
- Darwin-gated files (`internal/secret/keychainunlocker.go`,
`seunlocker_darwin.go`, `internal/macse/macse_darwin.go`, related
tests) are not linted on the Linux CI runner and still contain lines
over the new 88-column limit; they will surface if lint ever runs on
macOS.
- Merge secure-enclave-unlocker to main once review is done.
- 1.0 critical security blockers (from repo TODO.md):
- Command injection: GPG key IDs passed unescaped to exec.Command
(pgpunlocker.go:323-327); data.String() passed unescaped to the
security command (keychainunlocker.go:472-476).
- Memory security: age identity .String() creates unprotected
copies (keychainunlocker.go:356, pgpunlocker.go:256,
version.go:155); age secret key held in a plain string in
cli/crypto.go:86,91,113; private keys exposed via buffer.Bytes()
to GPGEncryptFunc and EncryptWithPassphrase.
- Input validation: no maximum secret size (DoS).
- Timing attacks: bytes.Equal passphrase compare (cli/init.go:
209-216); non-constant-time public key compare (vault.go:95-100).
- High priority:
- Secure temporary file handling and cleanup.
- Initialize a default unlock key at vault creation.
- Confirmation prompts for destructive operations (keys rm, vault
deletion).
- Add secret rm and vault deletion commands.
- Medium priority:
- Standardize error messages; stop leaking internals.
- Graceful handling of corrupted or missing key files with recovery
suggestions.
- Validate GPG key existence before creating PGP unlock keys.
- Split oversized CLI functions.
- Document env var security (SB_UNLOCK_PASSPHRASE,
SB_SECRET_MNEMONIC); clear after use.
- mlock/munlock for sensitive allocations.
- Cleanups: read statedir from environment or default instead of
passing it around.
- Enhancements: help examples, shell completion, colored output,
--quiet flag, name suggestions on miss, audit logging, hardware
integration tests (Keychain, GPG), naming consistency, vault
export/import, batch operations, search, secret metadata
(descriptions, tags).