carlos / platform Public

docs: bucket-writer audit — every write call site ruled for N boxes

Paul Campbell pushed by paul@keymail.dev 2 months ago 3f880495b4645c90c88efc03831547b98ac1b759 parent 02f449a

1 file changed, +279 −0

docs/superpowers/specs/2026-08-20-bucket-writer-audit.md +279 −0

diff --git a/docs/superpowers/specs/2026-08-20-bucket-writer-audit.md b/docs/superpowers/specs/2026-08-20-bucket-writer-audit.md

new file mode 100644

index 000000000..b5c7d361c

--- /dev/null

+++ b/docs/superpowers/specs/2026-08-20-bucket-writer-audit.md

@@ -0,0 +1,279 @@

+# Bucket-writer audit — every deployment-bucket write call site, ruled for N boxes (#238 stage 1)

+Status: stage-1 audit deliverable for

+`2026-08-20-multi-region-ingress-design.md` Part 1 (the writer map). Surveys

+the CURRENT tree (multi-region-writer-map-certs branch, after Task 1's

+per-box snapshot keys, commits b466af4+02f449a).

+**Recipe** (task-2-brief.md, verbatim):

+```

+grep -rn "\.Put(\|\.Delete(\|putReplacing(" internal/ --include='*.go' | grep -v _test

+```

+79 matches. Swept beyond that recipe for other store-write idioms:

+`s3.PutObject`, `PutObjectInput`, `DeleteObject(s)`, `.Upload(`, and a

+`.Write(` scan on store-shaped receivers (see Self-review). Grouped below by

+function/key, one row per distinct write site — where a CAS retry pattern

+puts the same key twice (a create attempt + one retry), both grep line

+numbers are cited on one row, matching the brief's own citation style

+(`internal/usage/edge.go:40` for a Put that's actually on line 93).

+**Ruling vocabulary**: `console-only` | `per-box keys` | `housekeeper` |

+`elected already` | `NEEDS RULING`, plus two escapes this sweep needed and

+used sparingly (see notes on each row): `N/A` (matched the grep but isn't a

+deployment-bucket write at all) and a spelled-out ruling where the site is

+safe but doesn't fit the five-word vocabulary cleanly.

+## Table

+|---|---|---|---|

+| `internal/appconfig/generation.go:92` (`TouchGeneration`) | `apps/<acct>/<app>/config-gen.json` | console (`apiconfig.go`) **+ CLI direct** (`carlos env sync`, `openStore()`) | console-only (CLI direct-bucket path is CAS create-or-replace-only and idempotent; see "CLI direct-bucket" note below) |

+| `internal/host/fleetbind.go:156,171` (`WriteFleetBinding`) | `fleetBindingKey(fleet, host)` — no platform-box component | every box (activator `OnWake` / agent startup) | **not lease, not per-box; already CAS+`takeover`-guard-arbitrated for concurrent writers by design** — see note below, not literally one of the five rulings |

+| `internal/host/metrics_loop.go:435` (`pruneStaleRaw`) | `metrics/boxes/<box>/raw/*` — this box's own | every box | per-box keys (already) |

+| `internal/host/snapshot_loop.go:105,126,134,144,230` | `control/registry-snapshot(s)/<box>.{db,json}` + generations | every box | per-box keys — **Task 1, done** (b466af4+02f449a) |

+| `internal/edge/storedoor.go:190` (PUT) | `fleets/<fleet>/...` or `apps/<sqid>/<app>/logs/...` (ACL-scoped: `aclWrite`) | every box serving the store door, **proxying** the connecting fleet member's write | every box, but the write identity is the fleet member and CAS is enforced by the same shared `release.Store` regardless of which box proxies — safe as-is, no per-box key needed |

+| `internal/dbverify/dbverify.go:144` (`Publish`) | `control/restore-verify.json` — **one fixed key, no box component** | every box, via `carlos-hibernate-verify.timer` (systemd timer, every box — see `internal/host/units/carlos-hibernate-verify.timer`) | **NEEDS RULING** — see Findings §1 |

+| `internal/release/prune.go:129,136` (`DeleteVersion`) | `releases/<acct>/<app>/<version>/*` | every box (`host/retention.go` `retentionPass`) | **housekeeper (Task 3 pending)** — matches brief's known-expected row |

+| `internal/domains/reconcile.go` → `internal/release/trash.go:237,241` (`PurgeApp`) | `releases/<acct>/<app>/*` (whole trashed app) + `apps/<acct>/<app>/deleted.json` marker | **every box** (`host/agent.go` → `domains.Reconcile`'s auto-purge step) **and** console (`ownership.go`) | doc-asserted race-proof for N boxes already (see Findings §2) — ruled safe, flagged for controller confirmation, not literally NEEDS RULING |

+| `internal/release/release.go:598` (`putImmutable`, called by `Ship`) | `releases/<acct>/<app>/<version>/manifest.json` (content-addressed, create-only) | console (`apicli.go`, `server.go`) **+ CLI direct** (`carlos ship`, `openStore()`, `cli.go:1883`) | console-only (CLI direct path is create-only/idempotent-on-identical-bytes; see CLI direct-bucket note) |

+| `internal/release/release.go:722` (`Promote`) | `apps/<acct>/<app>/channels/<channel>.json` | console (`apicli.go`, `server.go`) **+ CLI direct** (`carlos promote`, `PromoteOnLadder`, `cli.go:2083`) | console-only (+ CLI direct, CAS read-etag-then-Put) |

+| `internal/release/store.go:104,108` (`prefixStore.Put/Delete`) | pass-through — rebases the key onto a fleet's own prefix, then delegates to the wrapped `Store` | n/a — wrapper, not a distinct write | N/A (wrapper; the underlying write is whichever real store the fleet box holds — customer-fleets namespacing, out of platform-box scope) |

+| `internal/provision/provision.go:548` (`PutManifest`) | `apps/<acct>/<app>/instances.json` | console — **no HTTP caller found**; only caller is CLI direct (`instances_cli.go:202`, `carlos instances enable`, store-mode) | console-only in spirit (CLI direct-bucket, CAS upsert-with-retry) — see CLI direct-bucket note |

+| `internal/domains/mint.go:199` (`MintAccount`) | `accounts/<sqid>/account.json` (create-only) | console (`server.go`) **+ CLI direct** (`carlos accounts create`, `accounts.go:146`) | console-only (+ CLI direct, create-only CAS — id-allocation loser re-lists, never overwrites) |

+| `internal/edge/wildcard.go:483` (`w.cache.Put`, wildcard cert PEM) | `autocert.Cache` key `wildcardPEMKey(parent)` | every box that runs the wildcard manager | **N/A today** — see Findings §3 (cache backend is local disk; bucket ("s3") backend is reserved/unimplemented) |

+## CLI direct-bucket note

+Several rows above are reachable from BOTH the console process and a

+`carlos` CLI verb that opens the bucket directly (`openStore()` in `cli.go`,

+`accounts.go`, `apps.go`, `appconfig_cli.go`, `instances_cli.go`) —

+`ship`/`promote`/`rollback`/`target`/`accounts create`/`env`/`instances

+enable`/`apps delete`/`apps restore`. This matches CLAUDE.md's own note that

+"direct-bucket promotes (pinfra scripts) don't bump [`control/serial`]" —

+`scripts/ship-app.sh`/`promote-app.sh` are exactly this path.

+None of these change the writer map's `console-only` ruling for N platform

+boxes: the CLI path is an ad hoc, external invocation (an operator's laptop,

+a pinfra script, a first-boot bootstrap) — never a process running

+persistently on one of the N boxes, so it does not scale with box count the

+way a box's own converge loop would. Every one of these functions already

+uses the codebase's standard CAS discipline (create-only, or read-etag-then-

+Put with a bounded retry), so two concurrent invocations — whether both CLI,

+both console, or one of each — resolve the same way a single console

+already resolves two concurrent browser tabs. Called out per-row rather than

+silently folded into "console" because the spec's Part 1 table's literal

+wording ("console stays the single control-plane writer") undersells the

+actual call graph, and a future reader tracing one of these keys should not

+have to rediscover the CLI path themselves.

+## Findings (surprises, worth the controller's attention)

+### 1. `control/restore-verify.json` — NEEDS RULING

+`internal/dbverify/dbverify.go` publishes ONE fixed key with no per-box

+component, and the doc comment places it "beside `control/serial` and

+`control/registry-snapshot.json`" — i.e. explicitly aware of the same

+key-collision family Task 1 fixed for snapshots, yet it wasn't migrated.

+The write path (`internal/ops/ops.go`'s `runHibernateVerify`) does

+`Load` (Get) → `Merge` (fold this run's verdicts into the prior record,

+in-process) → `Publish`. `Publish`'s retry loop (`internal/dbverify/dbverify.go:120-144`)

+re-fetches the ETAG on each of 3 attempts but does **not** re-fetch or

+re-merge the BODY — the marshaled `rec` is fixed once, before the loop. If

+two boxes' `carlos-hibernate-verify.timer` runs race (confirmed installed

+on every box: `internal/host/units/carlos-hibernate-verify.timer`), the

+loser's retry re-reads a fresh etag and blindly overwrites the winner's

+just-published record with its own OLDER merge — silently dropping

+whatever the winner had just added, with no error surfaced (the retry

+"succeeds").

+This is the same fixed-key shape #158 fixed for snapshots, but with a worse

+failure mode: #158's collision surfaced as an outright overwrite each box

+could at least notice was contended; this one merges first, so the loser's

+write looks locally correct and the data loss is invisible until someone

+diffs `Hosts` against what a box's own log claimed to publish. I'm not

+confident whether the right fix is (a) box-scoped keys

+(`control/restore-verify/<box>.json`, mirroring snapshots, with a reader

+that folds them for display) or (b) keeping one merged key but making

+`Publish` re-Load-and-re-Merge inside each retry attempt rather than

+retrying a stale body — that's a design call, not something to invent here.

+Flagging for the controller before Task 3/4 lock in the housekeeper-lease

+boundary, since this key doesn't fit "deployment-global duty" (nothing

+elects one issuer) OR "per-box data" (results are meant to be read as one

+fleet-wide table) cleanly.

+### 2. `release.PurgeApp` has two callers, one of them every-box — ruled safe, flagged for confirmation

+`internal/domains/reconcile.go`'s auto-purge step (run from

+`internal/host/agent.go`'s `domains.Reconcile` pass, **every box, every

+converge tick**) calls `release.PurgeApp` for any app trashed past

+`TrashCooloff` — not console-only, and not in the writer map's table at

+all. Unlike the retention-prune row (which the spec explicitly moves behind

+a housekeeper lease because "five concurrent pruners racing a promote is a

+bug class we refuse to have"), this call site's own comment argues it is

+already safe for concurrent boxes: `PurgeApp` re-reads the trash marker and

+re-checks the production flag immediately before destroying, both guards

+fail toward keeping data, deletes are idempotent, and the marker is deleted

+last — "concurrent boxes converge because every step tolerates already-gone

+and the marker goes last" (`internal/domains/reconcile.go:260-267`).

+I'm ruling this one safe as-is (idempotent deletes racing harmlessly, no

+CAS content to clobber the way a merge-then-retry does), distinct from

+finding #1 — but it's a second every-box destructive-delete path on

+`releases/**` that the writer map's table doesn't mention, so Task 3's

+housekeeper-lease scope should explicitly decide whether this stays outside

+the lease (my read: yes, it's structurally different from keep-newest-N

+prune) or gets folded in for uniformity.

+### 3. Wildcard cert issuance (`internal/edge/wildcard.go`) is not a bucket writer today — but is the exact seam Part 2 will need

+The grep caught `w.cache.Put(...)` twice (wildcard cert PEM, ACME account

+key). `w.cache` is a `golang.org/x/crypto/acme/autocert.Cache`, and

+`internal/edge/certcache.go`'s `newCertCache` resolves it from

+`CARLOS_CERT_CACHE`: unset/`"dir"` → `autocert.DirCache` (local disk,

+per-box, never touches the deployment bucket); `"s3"` → **refuses to start,

+"reserved, not implemented yet."** So today, every box that has

+`PLATFORM_APPS_DOMAIN`/`CARLOS_WILDCARD_DOMAINS` set mints its own wildcard

+certs independently against its own local disk — not a deployment-bucket

+write, so out of this audit's scope by the letter of the brief, but it is

+functionally the same shared-ACME-account collision problem Part 2's

+housekeeper is designed to solve, on a code path Part 2's design doesn't

+mention. The `certcache.go` comment already anticipated this ("Edge boxes

+in a pool must eventually SHARE issued certs... an S3 backend is a drop-in

+behind this constructor") — worth Task 4/5 checking whether the wildcard

+path is meant to keep using this same `autocert.Cache` seam (in which case

+implementing `CARLOS_CERT_CACHE=s3` might be a natural way to land both

+per-host HTTP-01 AND wildcard DNS-01 through the housekeeper in one

+mechanism) or whether wildcard stays on per-box `DirCache` forever

+(acceptable only if wildcard and the new `control/certs/**` machinery never

+need to agree on a cert for the same hostname).

+### 4. `WriteFleetBinding` — CAS-arbitrated, not lease/per-box, ruled safe

+`internal/host/fleetbind.go` writes `fleetBindingKey(fleet, host)` — a

+single key with no platform-box component, from every box's activator

+(`OnWake`, `takeover=true`) and agent-startup pass (`takeover=false`). This

+doesn't fit the writer map's four ruling categories: it isn't console-only,

+isn't lease-elected, and the key doesn't carry a box suffix so it isn't

+"per-box" in the snapshot/metrics/usage sense either. It's a fourth shape —

+a CAS-guarded, semantically-arbitrated shared key (`bindingBlocked` +

+`takeover` decide who wins, not "last writer" or "one elected box") — and

+the code's own doc comment reasons carefully about exactly the concurrent-

+platform-box case ("a box republishes to the SAME key on every wake and

+once more at startup"). Ruled safe as designed; flagged because it's a

+pattern the writer map's four-row vocabulary doesn't have a name for, and a

+future writer-map revision might want one.

+## Self-review — could a writer exist that none of the greps caught?

+1. **Listed every `release.Store` implementor and read every method that

+ mutates.** Three implementations: `DirStore` and `S3Store`

+ (`internal/release/store.go`, `Put`/`Delete` — both caught, since every

+ caller goes through the interface methods the grep already matches),

+ `prefixStore` (pure delegation, not an independent writer — see its row

+ above), and the fleet-api HTTP client (`internal/fleetapi/fleetapi.go`'s

+ `store.Put`/`store.Delete`, method *definitions*, which the grep's

+ call-site pattern `\.Put(\|\.Delete(` correctly does not match — but

+ every byte it sends lands on the store-door server side, which the grep

+ DID catch at `internal/edge/storedoor.go:190,211`, so nothing behind

+ this client escapes the audit).

+2. **Swept for direct S3 client usage bypassing the `Store` interface

+ entirely**, in case some package talks to `*s3.Client` itself instead of

+ going through `release.Store`. Found two: `internal/release/store.go`

+ (that's `S3Store`'s own implementation — already covered) and

+ `internal/objstore/aws.go`/`provisioner.go` (`CreateBucket`,

+ `DeleteBucket`, `DeleteObjects`, `PutUserPolicy`, etc.) — confirmed this

+ is the TENANT object-storage feature's own bucket (a member's

+ provisioned S3 bucket, issue #55), never the deployment control bucket,

+ so it's out of scope for a deployment-bucket writer map and isn't given

+ a row above.

+3. **Swept `.Write(` on store-shaped receivers** per the brief's

+ instruction, in case some writer uses `io.Writer`-shaped code instead of

+ `Store.Put`. Every hit was a `http.ResponseWriter`/hash/gzip/file write —

+ none touch the deployment bucket (`internal/host/restart.go` and

+ `internal/release/bake.go`'s `f.Write(raw)` are local box-side files, not

+ bucket keys).

+4. **Checked every `release.Store`-typed function for callers outside what

+ the grep's line already showed**, rather than trusting the first caller

+ found — this is how findings §1, §2, and the CLI-direct-bucket note were

+ caught: `dbverify.Publish` looked console-adjacent from its own package

+ doc comment until tracing to `internal/ops/ops.go` showed a systemd timer

+ installed fleet-wide; `release.PurgeApp` looked console-only until

+ `internal/domains/reconcile.go` showed a second, every-box caller;

+ several `release.*`/`appconfig.*`/`domains.*` functions I initially

+ assumed were console-exclusive (based on `deploy.go` going through the

+ console HTTP API) turned out to have a second CLI direct-bucket caller

+ via `openStore()` once I grepped for it explicitly across the CLI's root

+ package.

+5. **Did not find** any additional deployment-bucket writer beyond the

+ above. The two areas most likely to hide one — a second `Store`

+ implementation, and non-`Store`-interface S3 calls — were both

+ enumerated and both came back accounted for (fleet-api client

+ delegates to the store door; objstore's S3 calls are a different

+ bucket entirely).

+## Cross-check against the spec's Part 1 table

+| Spec row | Audit outcome |

+|---|---|

+| `apps/**`, `domains/*`, `control/serial`, restart-gen — console | Confirmed, with the CLI-direct-bucket caveat above (does not change the ruling) |

+| `control/registry-snapshot*` — per-box keys | Confirmed done (Task 1) |

+| `status/**` — lease-elected publisher | Confirmed (`status/fleet.json` elected; `status/boxes/<label>.json` already per-box) |

+| `releases/**` retention prune — housekeeper (Task 3) | Confirmed current state is every-box (`host/retention.go`); ruled `housekeeper (Task 3 pending)` as instructed. **Plus** a second every-box destructive path on `releases/**` the table doesn't mention — Finding §2 |

+| usage metering day objects — audit box-scoping | Confirmed already box-scoped (`usage/edge/<box>/days/`) |

+| `control/certs/**`, `control/acme-challenges/**` — housekeeper (Part 2) | Not yet built (correctly absent from this tree) — but Finding §3 flags a live, unbuilt-for-N-boxes adjacent cert-issuance path (wildcard) worth Part 2/Task 4 awareness |

+| `control/edges/<box>.json` — each box, own key | Not yet built (Part 3, correctly absent) |

+| `control/edge-mesh-key` — housekeeper create-only | Not yet built (Part 3, correctly absent) |

+No writer discovered outside the spec's table lacks a ruling here — two

+(§1, `control/restore-verify.json`) genuinely need the controller's

+decision; the rest (§2, §3, §4) are ruled with reasoning attached.