[jerryfane]

Closes #45. Part of #46. Assignment: keyring seat brief 2026-10-14 (owner-approved via Jarvis); owner chose Cloudflare Containers (B1) on #46, so this is built as one process per person driven by a supervisor (the person's Durable Object at Cloudflare; any process manager on a host). Design notes: /root/fleet-tools/state/keyring-seat/design-45.md.

What

keyring instance --admin-listen 127.0.0.1:P --admin-token-sha256 HEX [--state FILE] [--audit FILE]

• Admin API on loopback only, Authorization: Bearer <admin token> on every call (only the hash is given). Until create/unlock it serves nothing else.
• Sealed state: network, services, roles (token hashes), consent settings and key values in one AES-256-GCM blob; AAD binds it to the instance id (a blob relabeled as another instance, or opened with another instance's key, is refused with one generic error). The 32-byte data key comes from the supervisor and is never written by keyring. --state keeps the blob on disk (mode 600, atomic write); without it the blob is memory-only and the supervisor fetches it via GET /state (B1, B1: keep the encrypted instance state in the Durable Object, not on the container disk #49).
• Calls: create, unlock (from file or a passed blob), status (no values, no token hashes), state, network, consent, services, roles (token returned once; rotation endpoint), keys, rotate (reseal with a new data key), delete (shred + remove state file, forget state and key, stop listeners, process exits).
• Every change: copy → apply → policy.Config.Validate → bind new listeners first → seal → write → swap into the proxy → close old listeners. A refused change leaves version, file and running proxy untouched. Version +1 per change.
• Consent links (Consent links: let the key's owner connect a service themselves (OAuth sign-in or one-time secure form) #44) store keys through the same path, so they land in the sealed state.
• Proxy and consent code unchanged; instance reuses server.Handler and consent.Server.

Why one process per person (issue asks to justify): OS address-space separation between persons, the proxy/policy code runs unchanged, and it is exactly B1's one-container-per-person model.

Idle footprint (built binary, 3 instances, Linux x64, Go 1.24.13, one service + one role each): VmRSS ≈ 12 MB, 8 threads, 0 CPU ticks over 30 s idle.

Unchanged (hard rule)

keyring serve, file mode, dashboard, SIGHUP and the Mac install are untouched; the only change outside internal/instance and cmd/keyring/instance.go is three new flags and one new case in cmd/keyring/main.go.

Tests

• go test ./... -race (Go 1.24.13): all ok.
• New internal/instance/instance_test.go: create → use → rotate role token → rotate data key (old key opens nothing new; file matches) → delete (file gone, proxy closed, status locked, /state 409, unlock refused, audit clean); restart from a sealed blob with no disk (wrong key 403, right key restores the proxy); two instances cannot read or use each other's keys (tokens cross-refused 401, each upstream gets only its own value, keys don't open each other's state, relabeled state refused); admin token required, wildcard admin refused; refused changes (bad service, unknown fields, service in use, unknown service in role, bad key name, empty value, busy port, admin port, wildcard, second create, bad data key, create over an existing state file from a second process) leave version, file and proxy untouched; network move rebinds and applies new sources; consent link value lands in the sealed state, not the audit.
• test-check prove: inconclusive (new package). Instead 16 hand mutants of instance.go/state.go, all red: AAD ignores id, open ignores id mismatch, no admin token check, rotate keeps old key, wipe keeps file / listeners / state, role token not rotated, status leaks values, bind failure ignored, admin address allowed for proxy, moved listener kept, commit without validate, create overwrites state file, consent bypasses sealed state, no file write. (First two were green against the initial tests; the relabel and id assertions were added, then red.)
• Smoke (built binary, /tmp, cleaned up): 3 instances created via API; alice's call proxied, alice's token at bob 401; alice SIGTERM → restart → locked (proxy closed) → wrong key 403 → unlock → proxy works; rotate → restart → old key 403, new key 200; DELETE → process exits 0, state file gone, proxy closed, bob unaffected; SIGTERM exits 0; 22 audit lines, no value/token/data key; sealed files hold no value or key name.

Not in this PR

#49 stale-blob version check on the DO side; #50 counts/links/audit across a container sleep; #51 container listen mode; #52 router proof. Interface agreed with phobos-dev (gitmoot message 313693).

Review focus

Crypto (GCM nonce, AAD binding, key handling and lifetimes), isolation between instances, commit atomicity under refused changes and concurrent consent writes, admin API auth, and that keyring serve is unchanged.

[jerryfane]

Agent: g7-review
Runtime: omp
Job: local-review-review-router-18de809a09a8a606-1

Decision: changes_requested
Summary: Crypto and isolation hold up: AES-256-GCM with fresh random nonces, AAD bound to the instance id, open() refuses wrong-key/relabeled blobs with one undifferentiated error, the data key is never persisted (only sealed state, mode 600, via fileutil.WriteAtomic), in.key is cleared on wipe/rotate, and the cross-instance test plus my overlay runs confirm instances cannot open or spend each other's state, tokens, or keys. Commit is correctly ordered (validate → seal → bind → write → swap → serve) and refused changes provably leave version, state file, and running listeners untouched. Admin auth is bearer+SHA256+constant-time compare, loopback-only. 'keyring serve', file mode, and dashboard are untouched (diff is purely additive). Three issues: (1) GET /status exposes consent.api_token_sha256 — the code deliberately strips role token hashes via roleView but serializes s.Consent verbatim, contradicting the README's 'no token hashes' promise; confirmed live. (2) A wiped instance can be resurrected: unlock (or create) only checks in.state==nil and ignores the closed wiped channel, so DELETE /instance is not terminal — the resurrected instance then answers 410 'already deleted' to a second wipe. Confirmed live. (3) Listener shutdown is racy: srv.Close() does not wait for srv.Serve to register the listener, so Close can return while the port is still bound; observed as a one-off EADDRINUSE flake in TestRestartFromSealedState (passed 10x on rerun). Minor: Options.Audit nil is never defaulted (nil-write panic via server.Handler on first proxied call) and AuditName is effectively required though undocumented — embedder traps only.
Severity: P2

Findings

• P2
  • description: GET /status leaks the consent API token hash
  • evidence: internal/instance/instance.go:489-491 status writes "consent": s.Consent verbatim; internal/policy/policy.go:95 gives Consent.APITokenSHA256 the json tag api_token_sha256, while roles are deliberately projected through roleView without TokenSHA256 (instance.go:481-488). Executed proof: overlay test issued PUT /consent with api_token_sha256=sha256('router')=74c956... then GET /status; response body contained "api_token_sha256":"74c95604043427f0bee1d0e16bfa53afd537f736ad0073c4cc4e1ccb3a82b5dc". Violates README's claim that status exposes no token hashes; the hash is brute-forceable offline against weak consent API tokens.
  • file: internal/instance/instance.go:489
• P2
  • description: POST /unlock and POST /create accept calls after DELETE /instance, resurrecting a wiped instance that can no longer be wiped
  • evidence: instance.go:439-441 unlock checks only in.state==nil; wipe (instance.go:683-703) nils state and closes in.wiped, but nothing later consults the channel. Executed proof: create → GET /state → DELETE /instance (200 deleted:true; second DELETE correctly 410) → POST /unlock with the sealed blob returned 200 {version:1} and GET /status showed unlocked:true; a subsequent DELETE /instance returns 410 'already deleted', so the resurrected instance can never be wiped. Same for POST /create after wipe.
  • file: internal/instance/instance.go:439
• P2
  • description: Instance.Close/commit closeListeners can return before the TCP listener is released
  • evidence: closeListeners → r.srv.Close() (instance.go:184) while serve() launches go srv.Serve(l) (instance.go:172). If Close runs before Serve's internal listener registration, the socket stays bound until the Serve goroutine runs and returns; net/http gives no rendezvous. Executed proof: first run of 'CGO_ENABLED=0 go test ./...' failed TestRestartFromSealedState at instance_test.go:265 with 'proxy on 127.0.0.1:42865: listen ... bind: address already in use' (x.in.Close() had returned but the socket was still open); the test passed on all 10 subsequent runs, i.e. a genuine race, not a deterministic failure. In production a supervisor restart or rapid PUT /network can hit the same refused bind. Fix: store the net.Listener in running and Close it directly, or wait for Serve to exit.
  • file: internal/instance/instance.go:184
• P3
  • description: Options.Audit nil is not defaulted and AuditName is an undocumented hard requirement
  • evidence: instance.go:80-82 defaults opt.Log to io.Discard but never opt.Audit; writeAudit (instance.go:105) and server.Handler.writeAudit (server.go:643) call opt.Audit.Write unconditionally, and config() rejects empty AuditName ('audit_log is empty', observed as create:400 before setting AuditName). runInstance always supplies both, so only embedders (e.g. the cited Durable Object use case) hit a nil-writer panic or an unexplained 400.
  • file: internal/instance/instance.go:83

Tests Run

• CGO_ENABLED=0 go build ./... — PASS (plain 'go build' fails sandbox-wide: cgo cannot read /usr/include/stdc-predef.h)
• CGO_ENABLED=0 go vet ./... — PASS
• CGO_ENABLED=0 go test ./... — first run: FAIL, TestRestartFromSealedState EADDRINUSE at instance_test.go:265 (race, see finding 3); all other packages ok
• CGO_ENABLED=0 go test ./internal/instance/ -count=10 — PASS (flake did not reproduce in 10 runs)
• CGO_ENABLED=0 go test ./... -count=1 (final) — PASS all packages
• overlay test (no repo edits) TestStatusLeaksConsentTokenHash — FAILED as designed: GET /status returned api_token_sha256, confirming finding 1
• overlay test TestUnlockAfterWipeResurrects — unlock after DELETE /instance returned 200 and status unlocked:true; second wipe returned 410, confirming finding 2
• go test -race — NOT RUNNABLE: -race requires cgo and the sandbox denies C header access (checks_blocked: sandbox)

Raw runtime output was retained in local Gitmoot state and is not posted here.

[jerryfane]

Review round 1 (job local-review-review-router-18de809a09a8a606-1, g7-review, devin/swe-2, head 5149470): changes_requested, three P2 and one P3. All fixed at f5ba532, each with a check that fails on 5149470 and passes now (3 runs on old code; new code 30 runs under -race):

• P2 instance.go:489 status shows the consent API token hash: status now clears api_token_sha256. Check in TestConsentLinkStoresIntoTheSealedState.
• P2 instance.go:439 create/unlock after DELETE /instance brought the instance back: both answer 410 after a delete, as does a second delete. Check in TestCreateUseRotateDelete (unlock with the supervisor's old sealed state, create, second delete).
• P2 instance.go:184 a listener could stay bound after close: closeListeners now closes the net.Listener itself before srv.Close. TestNetworkMoveRebindsTheProxy moves the proxy back and forth 20 times; on 5149470 it failed with "address already in use" in 3 of 3 runs.
• P3 instance.go:83 Audit nil / AuditName: New refuses a nil audit writer, and AuditName defaults to "audit". Check in TestAdminNeedsTheToken.
  go test ./... -race (Go 1.24.13): all ok.

[jerryfane]

Agent: g7-review
Runtime: omp
Job: local-review-review-router-18de81bcfd03b52e-1

Decision: approved
Summary: Re-reviewed delta 5149470..f5ba532 (instance.go +49/-, instance_test.go +24/-). All four prior findings are fixed and verified at runtime: (f1) status now projects Consent through a copy with APITokenSHA256 blanked (instance.go:509-514) — executed GET /status returned "api_token_sha256":"" with no hash present; (f2) create/unlock check isWiped() (instance.go:414,454) and return 410 — executed POST /unlock and POST /create after DELETE /instance both returned 410, second DELETE 410, status shows unlocked:false; (f3) running now stores the net.Listener and closeListeners closes r.l before srv.Close (instance.go:66-72,194-197) — port is freed synchronously; executed 10 rapid PUT /network move-back cycles all succeeded, and the previously racy TestRestartFromSealedState plus full suite passed -count=3; (f4) New rejects nil Audit and defaults AuditName to "audit" (instance.go:84-89) — executed: New without Audit errors, New with Audit but no AuditName succeeds. Production caller runInstance always passes a non-nil Audit (cmd/keyring/instance.go:21,30-33). Original scope re-confirmed: seal/data-key handling, commit atomicity, admin auth, keyring serve path all unchanged in this delta; only listeners bookkeeping, wipe gating, status projection, and Options validation changed. Race detector unavailable (-race requires cgo; /usr/include/stdc-predef.h permission denied); compensated with -count=5 instance runs and -count=3 full suite, all green.

Findings

• P2
  • continues_uid: feat: keyring instance: one person's keyring with sealed state and admin API (#45) #53-f1
  • detail: Fixed at this head. status() now copies *s.Consent and sets c.APITokenSHA256="" before serializing (instance.go:509-514). Executed: PUT /consent with api_token_sha256=sha256('router'), then GET /status returned "api_token_sha256":""; body contains no token hash. Grep confirms s.Consent is serialized nowhere else.
  • file: internal/instance/instance.go:512
  • state: answered
• P2
  • continues_uid: feat: keyring instance: one person's keyring with sealed state and admin API (#45) #53-f2
  • detail: Fixed at this head. create and unlock both call in.isWiped() under in.mu and return errWipedHTTP (410) (instance.go:414-417, 454-457); every other commit path requires in.state != nil which wipe nils. Executed: create → GET /state → DELETE /instance → POST /unlock with sealed blob = 410, POST /create = 410, second DELETE = 410, GET /status = {"unlocked":false}.
  • file: internal/instance/instance.go:414
  • state: answered
• P2
  • continues_uid: feat: keyring instance: one person's keyring with sealed state and admin API (#45) #53-f3
  • detail: Fixed at this head. running carries the net.Listener (instance.go:71) and closeListeners calls r.l.Close() before r.srv.Close() (instance.go:196-197), freeing the port synchronously even if Serve never registered it. Executed: 10 rapid PUT /network move-back cycles in one test all succeeded; CGO_ENABLED=0 go test -count=5 ./internal/instance and -count=3 ./... all pass; the flaky bind failure did not recur.
  • file: internal/instance/instance.go:196
  • state: answered
• P3
  • continues_uid: feat: keyring instance: one person's keyring with sealed state and admin API (#45) #53-f4
  • detail: Fixed at this head. New returns 'an audit writer is required' for nil opt.Audit and defaults AuditName to 'audit' when empty (instance.go:84-89). Executed: New without Audit rejected; New with Audit but empty AuditName accepted; sole production caller runInstance always supplies a writer (stdout or the audit file).
  • file: internal/instance/instance.go:84
  • state: answered

Tests Run

• CGO_ENABLED=0 go build ./... (at f5ba532) — ok
• CGO_ENABLED=0 go vet ./... — ok
• CGO_ENABLED=0 go test ./... — all packages ok
• CGO_ENABLED=0 go test -count=5 ./internal/instance/ — ok
• CGO_ENABLED=0 go test -count=3 ./... — all packages ok
• CGO_ENABLED=0 go test -v -run TestReviewSmoke ./internal/instance/ (throwaway harness driving New+AdminHandler over real sockets: status token-hash check, 10 rebind cycles, unlock/create/double-delete after wipe = 410, nil-Audit rejection, AuditName default) — PASS
• go test -race — not runnable: -race requires cgo and /usr/include/stdc-predef.h is permission-denied in this sandbox
  Diagnostics: workflow advance failed: review approval held: approval is bound to head f5ba532 but the observed pull request head is 5149470; holding until the observed head catches up, because an approval at a genuinely superseded head must not clear changes_requested

Raw runtime output was retained in local Gitmoot state and is not posted here.

[jerryfane]

Merge gate (keyring seat): gitmoot review level = level3_required at f5ba532. Independent review by g7-review (devin/swe-2; the implementer is anthropic/claude-opus), job local-review-review-router-18de81bcfd03b52e-1, approved at that exact head. Round 1 was local-review-review-router-18de809a09a8a606-1: three P2 and one P3, all fixed with checks that fail on the old head. CI is green, and the seat ran go test ./... -race (Go 1.24.13), also green. No tag or release, so the Mac is not affected.