Resolve open design questions: write-scoped grants, rollback, purge

Per-question resolution, per best practice rather than deferral:

- Push authorization: closed the least-privilege gap where read implied
  write. vault_grants gained can_write (default true, so nothing
  existing changes); enforced on push and grant (granting others is
  itself a mutation of vault membership, so it needs write too, not
  just read). 'keep grant --read-only' creates a read-only grant.

- Version history: not full history (conflates "undo a typo'd push"
  with "this leaked, stop retaining it" into one mechanism). Retains
  exactly one previous version as a rollback safety net
  (vaults.prev_ciphertext/prev_nonce + a vault_grants_previous mirror
  table so recipients can unwrap it), plus 'keep push --purge' for
  compromise-driven rotations that explicitly skips retention and
  wipes any existing previous version too.

- Per-secret-key granularity: resolved by NOT building it — documented
  the escape hatch (split into more vaults) instead of adding partial-
  decrypt complexity for a problem the existing primitive solves.

One more real bug caught during verification: the CLI's --previous flag
initially signed a path including its query string, but the server
verifies against req.originalUrl with the query stripped — a mismatch
that would have made every --previous request fail signature
verification. Fixed by splitting the signed path from the request URL
in signedFetch, signing only the former.

Verified end-to-end with three independent identities: read-only grant
correctly blocked from push and from granting others, write access and
read-only status both preserved correctly across a rotation, previous-
version pull working for a routine push and correctly unavailable to
every recipient after a purge push. Also re-verified against a fresh
Docker build.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
Fredrik Johansson
2026-07-12 19:50:08 +02:00
parent 67000b66ee
commit 001623c8e2
7 changed files with 284 additions and 101 deletions

View File

@@ -234,18 +234,43 @@ which would have silently mismatched a client that signs the full request
path. Fixed by verifying against `req.originalUrl` (path portion only)
instead, which stays consistent regardless of router nesting.
## Open questions
## Resolved design decisions
- **Version history**: keep only the latest payload per vault (what got
built), or retain N previous versions for `keep pull --version=N`
rollback? Skipped for v1, revisit if a real rollback need shows up.
- **Push authorization**: does having a *read* grant (`vault_grants` row)
imply push rights too, or is push a separate capability? Went with
"read implies write" for v1 — a `can_write` flag on `vault_grants` is
the natural extension if that turns out to be wrong.
- **Per-secret-key granularity**: v1 treats a vault as one opaque
`.env`-shaped blob (matches the actual pain point this was built for:
whole files get copied around, not individual keys). If a use case
emerges for "grant access to just one value, not the whole bundle,"
that's a genuinely different data model (per-key rows, each separately
wrapped) — don't half-build it speculatively.
These were open questions at initial implementation; resolved as follows.
### Version history: one previous version, not full history
Full N-version history (what Vault/AWS Secrets Manager do) conflates two
different needs that deserve different treatment: "I pushed a typo'd
value, let me roll back" wants a safety net; "this credential leaked"
wants the old value to stop existing anywhere retrievable, which a kept
version directly undermines. Resolution: retain exactly the immediately
previous payload as a rollback safety net (`keep pull --previous`), plus
an explicit hard-rotate mode (`keep push --purge`) that skips retention
entirely for compromise-driven rotations. Not full history — the two
real use cases are served without the complexity (or the confused
security story) of an unbounded log of past secret values.
### Push authorization: read no longer implies write
The original "read implies write" v1 simplification was a real
least-privilege gap: an automated deploy identity that only ever needs
to *pull* a vault also had the power to overwrite it — a compromised
read-only consumer could push garbage values or silently exclude other
recipients from the next rotation. Fixed rather than deferred, since the
cost of closing it is low and the cost of retrofitting it later (once
grants without write intent already exist) is higher: `vault_grants`
gained a `can_write` column (`1` by default, so nothing existing
changes behavior), enforced in the push handler, with `keep grant
--read-only` to create a read-only grant explicitly.
### Per-secret-key granularity: resolved by not building it
The right answer for "different recipients need different subsets of
secrets" is splitting into more vaults (`myapp/db`, `myapp/api`, each
with their own recipient list), not partial-access ACLs within one
blob. A vault staying the permission boundary keeps the mental model
simple and auditable; per-key wrapping would solve the same problem
with materially more complexity for no real gain over just making more
vaults. No code change — this documents the escape hatch instead of
leaving a dangling TODO.