diff --git a/.env.prod.example b/.env.prod.example index 27d8513..b034491 100644 --- a/.env.prod.example +++ b/.env.prod.example @@ -34,10 +34,19 @@ AP_USER_ORIGIN=https://coves.social # 32-byte key-encryption key sealing per-actor signing keys at rest # (AES-256-GCM). Generate with: openssl rand -hex 32 -# Losing it means losing every bridged repo's signing keys — back it up. +# Losing it means losing every repo's signing keys — back it up. # THERE IS NO ROTATION PATH. Nothing in the codebase re-seals existing -# ciphertext under a new KEK; changing this value orphans every sealed key -# (every bridged identity and the PLC escrow rotation key) with no recovery. +# ciphertext under a new KEK; changing this value orphans every sealed key, +# across THREE tables, with no recovery: +# - bridged_actors.signing_key every bridged (Lemmy-origin) identity's +# escrowed secp256k1 atproto repo key +# - service_keys.key_material the 'plc-rotation' row: the PLC escrow +# key, the only DID recovery path +# - ap_actors.rsa_key_sealed EVERY NATIVE COVES USER'S AP signing +# key (v2 / task 13). Easy to omit from a +# rotation or backup plan written before +# v2 existed — and it is the entire +# native-user population. # See DEPLOY.md "Not implemented" before you touch it. BRIDGE_KEK=CHANGE_ME diff --git a/DEPLOY.md b/DEPLOY.md index 0e9b0e3..5d3e97d 100644 --- a/DEPLOY.md +++ b/DEPLOY.md @@ -124,12 +124,38 @@ transaction, so intent past the gate is never dropped (`cmd/tidepool/main.go:695-713`). Raising `OUTBOUND_WORKERS` later drains whatever accumulated, immediately. Budget for that. -**`OUTBOUND_DISABLED` parks, it does not fail.** A blocked delivery stays -`pending` and resumes when the switch clears; it is never poisoned and never -cancelled (`internal/outbound/worker.go:235-242`, -`internal/outbound/switches.go:5-12`). Engaging a kill switch loses nothing. -`OUTBOUND_DRY_RUN` parks the same way, after translating and logging -(`worker.go:243-248`). +**`OUTBOUND_DISABLED` parks, it does not fail — but parking is not free.** A +blocked delivery stays `pending` and resumes when the switch clears; `park` +itself never poisons and never cancels (`internal/outbound/worker.go:242-244`, +`internal/outbound/switches.go:5-12`). `OUTBOUND_DRY_RUN` parks the same way, +before any signing or POST (`worker.go:245-249`). + +⚠️ **A park still consumes the delivery's retry budget, and fast.** `ClaimNext` +does `attempts = attempts + 1` on every claim +(`internal/store/outbound_deliveries.go:143`), and `park` settles through +`Release`, whose `SET` clause updates `claimed_until`, `next_attempt_at`, +`last_error_class`, `response_excerpt`, `last_status_code` and `updated_at` — +and **never resets `attempts`** (`outbound_deliveries.go:218-221`). A parked +delivery is rescheduled `parkDelay = 5 * time.Second` out (`worker.go:560`), so +with workers running, each ordering key's head delivery is re-claimed and +re-parked roughly every five seconds and its +`DefaultMaxDeliveryAttempts = 8` budget (`worker.go:78`) is exhausted in about +**40 seconds**. Nothing poisons *while* parked — `park` never calls `poison` — +but once the switch clears, the **first** retryable failure finds +`Attempts >= maxAttempts` and poisons immediately instead of retrying +(`worker.go:512-518`). An hour-long global kill switch leaves every head +delivery with hundreds of attempts and zero retries left. + +It is also a continuous write load: one `UPDATE` per parked head roughly every +five seconds, per ordering key, for as long as the switch is engaged. + +**The remedy is `POST /admin/outbound/redrive`.** `RedrivePoisoned` sets +`attempts = 0` along with `state = 'pending'` +(`internal/store/outbound_deliveries.go:613-617`), so a redrive restores a full +budget. It only matches `state = 'poisoned'` (`:619`), so it repairs the damage +after a delivery has already fallen over, rather than preventing it. **To park +everything, prefer `OUTBOUND_WORKERS=0`** — see [Rollback](#rollback). The +underlying defect is tracked in `FOLLOWUPS.md`. Scope matching, from `config.go:519-525`: @@ -142,6 +168,43 @@ Scope matching, from `config.go:519-525`: There is no allowlist form of any of these. See [Staged rollout](#5-staged-rollout) for what that means for a canary. +**Every switch in that table — and `OUTBOUND_DISABLED` and `OUTBOUND_DRY_RUN` +with them — is INERT while `OUTBOUND_WORKERS=0`.** `ConfigSwitches` is only +constructed inside `if cfg.OutboundWorkers > 0` +(`cmd/tidepool/main.go:716-736`), so with no worker there is nothing holding a +switch and nothing consulting one. Setting `OUTBOUND_DISABLED=true` while +workers are 0 changes nothing and confirms nothing; it is not a second belt. +Conversely, that is also why `OUTBOUND_WORKERS=0` costs nothing: no worker +exists to claim, park, or spend attempts. + +### Confirming a switch actually engaged + +`internal/outbound/metrics.go:10-13` publishes four counters, all under +`/admin/metrics`: + +| Metric | Bumped when | +|---|---| +| `tidepool_outbound_delivered` | a delivery reached the peer (incl. Lemmy's duplicate-ack) | +| `tidepool_outbound_poisoned` | a delivery hit a terminal failure | +| `tidepool_outbound_cancelled` | the **worker** cancelled a claimed delivery on consent state — its only call site (`worker.go:292`) | +| `tidepool_outbound_parked` | a kill-switch, dry-run, **or causal-wait** deferral | + +`tidepool_outbound_parked` is the only **positive** confirmation that a kill +switch engaged: `by_state` still reads `pending` for a parked delivery, exactly +as it does for one merely waiting its turn, so the queue view cannot tell you. +Read it as a rate, not a level — a parked head is re-parked every ~5s, so the +counter climbs continuously while a switch is held. That climb *is* the budget +being spent. + +Two caveats on it. It is shared with `parkCausal`, so a nonzero `parked` with no +switch engaged means causal waits, not an operator action. And +`tidepool_outbound_cancelled` rises **on its own** during rollout: the worker +cancels a claimed delivery whenever the actor is disabled, delivery-paused, or +opted out (`internal/outbound/worker.go:281-296`). That is the counter's only +increment site, so a climbing `cancelled` is consent doing its job and is never +evidence that somebody ran `POST /admin/outbound/cancel` — the admin cancel does +not touch this metric, and shows up only in `by_state`. + --- ## 3. The admin surface at incident time @@ -179,6 +242,12 @@ of `actor` or `community` (`follow.go:236-239`). store is wired unconditionally (`cmd/tidepool/main.go:454`), so an empty `by_state` map is the truth about the queue, not a symptom of misconfiguration. +**It is also the whole route.** `by_state` counts are *all* it returns +(`internal/ingest/follow.go:172-183`) — no rows, no ids, no reasons. There is +no admin endpoint that exposes `last_error_class` or `response_excerpt`, so +"inspect, fix, then redrive" means psql on the box; the query is in +[Step 3](#step-3--widen). Plan for that before the incident, not during it. + ```sh T="Authorization: Bearer $ADMIN_TOKEN" curl -s -H "$T" localhost:8091/admin/outbound # queue depth by state @@ -203,8 +272,23 @@ the Coves side. on the **Coves** hostname, served by the Tidepool process. Tidepool's Host router sends requests whose `Host` is `coves.social` to the persona surface, and everything under `tdpl.io` to the bridge (`internal/personas/hostrouter.go:89-115`). -Caddy currently proxies `coves.social` entirely to the AppView, so **none of -those AP paths reach Tidepool today.** +**None of those AP paths reach Tidepool today** — but not because Caddy sends +the whole hostname to the AppView. The existing `coves.social` site block +(`~/Code/coves/Caddyfile`, block opens at `:70`; routing runs `:72-117`) +already splits the hostname four ways: + +| Path | Today's handler | +|---|---| +| `/.well-known/*` | static `file_server` over `/srv` (`Caddyfile:72`) | +| `/client-metadata.json` | static `file_server` over `/srv` (`Caddyfile:79`) | +| `/img/*` | 301 to `img.coves.social` (`Caddyfile:97`) | +| everything else | catch-all `reverse_proxy appview:8080` (`Caddyfile:102`) | + +Only the catch-all reaches the AppView. That matters for the before/after diff: +`GET /.well-known/webfinger` today returns a **static-file-server 404** from +`/srv`, not an AppView response, and `GET /nodeinfo/2.0` and `/ap/*` fall to the +AppView. So the signature to look for before the change is a bare 404 on +webfinger — and after it, a Tidepool JRD. The apex needs a content-negotiated split, and Tidepool deliberately cannot do it itself: `internal/personas/instance.go:32-34` states in its own comment that @@ -405,6 +489,17 @@ Two reading rules for those keys: for "storage could not be read" (`internal/consume/metrics.go:26-30`), chosen because a `0` would claim the backlog is empty at exactly the moment nobody can tell. +- **The `tidepool_divergence_*` gauges use the same `-1` convention**, for the + same reason (`internal/ingest/divergence.go:166-175`, `divergenceUnswept`). + Because they are published at package init, they are present from process + start — reading `-1` until the startup sweep completes, and again for any + class a failed sweep never wrote. `-1` is "never measured", `0` is "measured, + nothing diverging"; do not alert on them as if both meant healthy. +- **The four `tidepool_outbound_*` counters are absent until the first delivery + worker touches them** — they are `expvar.NewInt` at package init in + `internal/outbound/metrics.go:10-13`, so the keys exist once the package is + linked, but they sit at 0 while `OUTBOUND_WORKERS=0`. A flat 0 across all four + at this step is exactly right. Check the rejections before letting anything out. A misconfigured `ADMISSION_MAX_PER_AUTHOR_PER_COMMUNITY`, a stale community mapping, or a @@ -436,10 +531,21 @@ denylist.** The reconciler will subscribe it and it will start federating on the next sweep. Whenever the follow list grows during a canary, extend `OUTBOUND_DISABLED_COMMUNITIES` in the same change. -Optionally precede this with `OUTBOUND_DRY_RUN=true` for one cycle: every -delivery is translated and logged, nothing is POSTed, and the parked deliveries -resume when you clear it. That validates the translator against real records -without touching a peer. +**`OUTBOUND_DRY_RUN=true` is a weaker instrument than it sounds, and it is not +free.** What it does: the worker claims a delivery, logs it, and parks before +signing or POSTing (`internal/outbound/worker.go:245-249`). What it does **not** +do is validate the translator. **Translation happens at ENQUEUE time**, inside +the consumer's gate transaction (`cmd/tidepool/main.go:703-710`), and the worker +POSTs the stored payload verbatim (`worker.go:309`, `:319`). By the time a +delivery is claimable its payload has already been translated and persisted — so +the moment `CONSUMER_ENABLED=true`, the translator has already run on everything, +dry run or not. Step 1 is where you check its output (read +`outbound_activities.payload` in psql), not here. + +And because dry-run parks, it carries the same budget cost as any other park: +each head delivery is re-claimed every ~5s and burns its 8 attempts in ~40 +seconds (see [§2](#2-the-v2-flag-topology)). A "one cycle" dry run is measured +in seconds, not hours, and wants a `redrive` after it. ### On announcement throttling — what actually exists @@ -450,7 +556,7 @@ Decision 19 asks for "deliberate throttling of initial actor announcements". `docker-compose.prod.yml`, because the public `plc.directory` 429s mint bursts during community backfill) gate **inbound** DID minting only. They are wired into `ingest.NewMintGate`, whose sole consumer is the materializer's minter -(`cmd/tidepool/main.go:271-276`, `:314`) — the path where an unseen *Lemmy* +(`cmd/tidepool/main.go:271-276`, `:315`) — the path where an unseen *Lemmy* author gets an atproto DID. **There is no rate limiter on the outbound side.** `internal/outbound` contains @@ -492,6 +598,27 @@ What each one means: - **`by_state.poisoned` climbing** — deliveries exhausting their retries. Inspect, fix, then `redrive` **scoped** to the affected community. + + **There is no admin inspect surface for the "why".** `GET /admin/outbound` + returns `by_state` and nothing else (`internal/ingest/follow.go:172-183`) — a + count, no rows, no reasons. The columns that carry the diagnosis, + `last_error_class` and `response_excerpt`, are written by the worker but + exposed on no route; reading them means psql on the box: + + ```sh + docker exec -i tidepool-prod-postgres psql -U tidepool -d tidepool -c " + SELECT ordering_key, last_error_class, last_status_code, attempts, + left(response_excerpt, 200) AS excerpt, updated_at + FROM outbound_deliveries + WHERE state = 'poisoned' + ORDER BY updated_at DESC LIMIT 50;" + ``` + + Check `attempts` in that output before concluding the peer rejected anything: + a delivery whose budget was spent by a held kill switch (see + [§2](#2-the-v2-flag-topology)) poisons on its first real failure with an + `attempts` far above 8 and an error class that describes one attempt, not + eight. - **echo drop counters rising steadily** — expected and healthy: our own content arriving back from Lemmy and being correctly refused. A counter at **zero** while native content is flowing is the alarming case; it means @@ -514,13 +641,27 @@ step being one `.env` edit plus `up -d tidepool`: 1. **`OUTBOUND_DISABLED_COMMUNITIES=`** — park one community. Everything else keeps flowing; the parked deliveries resume when you clear - it. -2. **`OUTBOUND_DISABLED=true`** — park everything outbound. The consumer keeps - running and keeps recording intent; nothing reaches any peer. This is the - big red button and it is **lossless**. -3. **`OUTBOUND_WORKERS=0`** — stop the workers entirely. Equivalent effect to - (2) for delivery; prefer (2), because a parked delivery carries a recorded - reason and a stopped worker does not. + it. First because it is the *narrowest*, not because it is free: it is a + park, so it spends that community's head delivery's retry budget at the same + ~5s cadence as (3). Redrive that community after clearing it. +2. **`OUTBOUND_WORKERS=0`** — stop the workers entirely. This is the big red + button for delivery. The consumer keeps running and keeps recording intent; + nothing reaches any peer. `NewWorker` is only called inside + `if cfg.OutboundWorkers > 0` (`cmd/tidepool/main.go:716`), so at 0 there is + no worker to claim anything: nothing is re-claimed, no `attempts` are spent, + and no rows are written. It costs nothing and it is genuinely lossless. +3. **`OUTBOUND_DISABLED=true`** — park everything outbound. Same *observable* + effect as (2) — nothing reaches any peer — but **it is not free, and it is + ranked below (2) for that reason.** The workers keep running, so every + ordering key's head delivery is re-claimed and re-parked every ~5s and burns + its 8-attempt budget in ~40 seconds (see [§2](#2-the-v2-flag-topology)). The + deliveries this switch exists to protect are exactly the ones left with no + retries. Reach for it only when you need the *scope* it gives you and a + whole-worker stop is too blunt — and expect to `redrive` afterwards, which + is the only thing that resets `attempts` + (`internal/store/outbound_deliveries.go:613-617`). The one thing (3) buys + over (2) is that each parked row carries a recorded `last_error_class` / + `response_excerpt`; a stopped worker records nothing. 4. **`CONSUMER_ENABLED=false`** — stop consuming. Intent stops being recorded. The consumer resumes from its stored cursor when re-enabled, so this is recoverable, but it is the only step that stops *observing*, and @@ -555,25 +696,60 @@ re-seals existing ciphertext under a new key: the binary has exactly two subcommands, `tidepool` and `tidepool migrate` (`cmd/tidepool/main.go:69-78`). -Blast radius of losing or changing it: every per-actor RSA signing key for -every bridged identity is sealed under it, plus the PLC **escrow rotation -key** (`internal/identity/keys.go:140-144`). Change the KEK and every one of -those ciphertexts becomes undecryptable — no bridged actor can sign, and the -escrow key that could recover the DIDs is itself sealed under the key you just -replaced. Approximately 950 identities. There is no recovery. +Blast radius of losing or changing it — **three** tables, not two: + +1. **`bridged_actors.signing_key`** — the escrowed **secp256k1 atproto** repo + signing key of every bridged (Lemmy-origin) identity + (`internal/db/migrations/002_create_bridged_actors.sql:14`, + `internal/identity/keys.go:86-96`). Approximately 950 of them. +2. **`service_keys.key_material`, row `plc-rotation`** — the PLC **escrow + rotation key** (`internal/identity/keys.go:140-144`), the one thing that + could recover the DIDs, itself sealed under the key you just replaced. Note + the column is `key_material`, not `private_key_pem`; migration 013 renamed + it precisely because only this row is ciphertext — the sibling + `service-actor` row is **plaintext** PKCS#8 PEM and is *not* KEK-sealed + (`internal/db/migrations/013_rename_service_key_column.sql`). +3. **`ap_actors.rsa_key_sealed`** — **every NATIVE Coves user's ActivityPub RSA + signing key**, sealed under the same KEK under its own AAD prefix + (`internal/db/migrations/017_ap_actors.sql:46`, + `internal/identity/keys.go:39-53`). Written at mint time in + `internal/personas/personas.go:185`, through the same `identity.Custodian` + handed to `personas.New` at `cmd/tidepool/main.go:521-527`. + +**(3) is the v2 one, and it is the one a rotation plan will forget**, because +it did not exist when this section was first written. A rotation built to +handle only `bridged_actors` and `service_keys` would leave every native user +unable to sign a single outbound activity — the exact population v2 exists to +serve. Change the KEK and all three sets of ciphertext become undecryptable. +There is no recovery. *Naming trap:* `LoadOrCreateRotationKey` is **not** KEK rotation. It loads or generates the did:plc escrow/recovery key — an atproto identity concept — which is itself sealed under the KEK. Do not read that symbol as evidence that rotation is implemented. -What a real rotation would require, none of which exists: a key-version -column or KEK-id alongside each sealed blob; a dual-read custodian that tries -the new KEK then the old; an online re-seal pass over `bridged_actors` and -`service_keys`; and a cutover that retires the old KEK only after the pass -completes. The ciphertext does carry a one-byte version prefix -(`internal/identity/keys.go:127`), which is a hook someone could build on, but -nothing reads it as a key selector today. +What a real rotation would require: + +- **A key-version column or KEK-id alongside each sealed blob.** *Partly + present, and this is worth knowing before anyone designs it from scratch.* + `ap_actors` **already has one**: `rsa_key_version INT NOT NULL` + (`internal/db/migrations/017_ap_actors.sql:47`), and that migration's own + comment says it is there so "rotation [is] definable without a schema change" + (`:23-24`). It is stamped from `currentRSAKeyVersion = 1` + (`internal/personas/personas.go:25`, applied at `:185`) and read back, but + **nothing uses it as a selector** — no code branches on it to choose a KEK. + So on `ap_actors` the schema work is done and only the logic is missing. + `bridged_actors.signing_key` and `service_keys.key_material` genuinely have + no version column; those two need the migration as well. +- **A dual-read custodian** that tries the new KEK then the old. +- **An online re-seal pass** over all three tables above — `ap_actors` + included, which is the one a v1-era plan omits. +- **A cutover** that retires the old KEK only after the pass completes. + +The ciphertext also carries a one-byte version prefix +(`internal/identity/keys.go:127`) — but that is the *envelope format* version, +checked for equality and rejected otherwise, not a key id. Neither it nor +`rsa_key_version` selects a key today. Per-actor **RSA** rotation is equally undefined: rotating an actor's key means republishing `publicKey` in its actor document and having every peer that @@ -585,7 +761,7 @@ somewhere that survives the loss of the server.** ### Backup and restore -**No procedure exists, and no tooling.** `docker-compose.prod.yml:46` mounts +**No procedure exists, and no tooling.** `docker-compose.prod.yml:53` mounts `./backups:/backups` into the Postgres container. Nothing writes to it. There is no cron, no `pg_dump` wrapper, no restore drill, and no documented RPO/RTO. @@ -593,8 +769,11 @@ What is at risk, in order of irreplaceability: 1. **`BRIDGE_KEK`** — lives in `/opt/tidepool/.env`, not in Postgres, and is not covered by any database backup. Losing it is unrecoverable (above). -2. **`bridged_actors` / `service_keys`** — the sealed signing keys. Losing - these loses the identities even if the KEK survives. +2. **`bridged_actors` / `service_keys` / `ap_actors`** — the sealed signing + keys, for bridged identities, the PLC escrow key, **and every native Coves + user** respectively. Losing these loses the identities even if the KEK + survives. `ap_actors` is easy to omit from a v1-era backup scope; it is the + whole native-user population. 3. **Repo blocks and commits** — the atproto repos themselves. Re-derivable from upstream only by re-bridging, which mints new DIDs; the old at-uris do not come back. diff --git a/FOLLOWUPS.md b/FOLLOWUPS.md index 0b361d6..d01be9d 100644 --- a/FOLLOWUPS.md +++ b/FOLLOWUPS.md @@ -63,6 +63,68 @@ task documents and git history rather than this list. ## Outbound delivery (task 15) +- **DEFECT, NOT DONE — a PARK spends the poison budget, so the kill switch + destroys the retries of exactly the deliveries it exists to protect.** + Found 2026-08-14 while fact-checking `DEPLOY.md`; the docs and the two park + doc-comments have been corrected to describe it, and **nothing about the + behaviour was changed.** It needs its own test-first subtask. + + *Mechanism.* `ClaimNext` does `attempts = attempts + 1` on every claim + (`internal/store/outbound_deliveries.go:143`). `park` and `parkCausal` settle + through `Release`, whose `SET` clause updates `claimed_until`, + `next_attempt_at`, `last_error_class`, `response_excerpt`, `last_status_code` + and `updated_at` — and never resets `attempts` + (`outbound_deliveries.go:218-221`). `parkDelay = 5 * time.Second` + (`internal/outbound/worker.go:560`) and `DefaultMaxDeliveryAttempts = 8` + (`worker.go:78`), so with `OUTBOUND_DISABLED=true` and workers running, each + ordering key's head delivery is re-claimed and re-parked about every five + seconds and its entire retry budget is gone in roughly **40 seconds**. + + *Why it is not caught by "park never poisons".* It isn't — `park` genuinely + never calls `poison`, which is what made the old comments read as true. The + damage lands later: `releaseOrPoison` poisons on the FIRST retryable failure + once `Attempts >= maxAttempts` (`worker.go:512-518`). So an hour-long kill + switch leaves head deliveries with hundreds of attempts and zero retries, and + the next transient 5xx or dial timeout poisons them immediately. `parkCausal` + shares the mechanism but is materially safer: `causalStatus` bounds the causal + wait by WALL CLOCK from `delivery.CreatedAt`, never by attempt count, so a + held child's outcome is still decided by elapsed time. It is exposed only for + a genuine delivery failure after the parent lands. + + *Remedy that exists today, and its limit.* `RedrivePoisoned` sets + `attempts = 0` (`outbound_deliveries.go:613-617`), but it matches + `state = 'poisoned'` only (`:619`) — it repairs after the fall, and cannot + pre-empt it. `OUTBOUND_WORKERS=0` avoids the whole problem (no worker is + constructed, `cmd/tidepool/main.go:716`) and `DEPLOY.md` §5 now ranks it above + `OUTBOUND_DISABLED` for that reason. + + *Shape of a real fix — this is the design question, not a settled plan.* + `Release` is a single statement serving two callers that mean opposite + things, and it cannot tell them apart: a **retryable failure** (where holding + the incremented `attempts` is exactly right — that is the backoff working) + from a **park by an operator switch or a causal hold** (where it is not; the + delivery never reached the wire and nothing was learned about the peer). Two + candidate directions, with the trade to be argued in the subtask: + - a **park-specific release** — a sibling statement, or a flag on `Release`, + that writes `attempts = attempts - 1` / `attempts = $n` so a park is + attempt-neutral. Cheapest and most local, but adds a second write path + through the most fencing-sensitive statement in the package, and a park + that "un-counts" must not be able to underflow or to un-count a real + attempt on a re-claim race. + - **reset on unpark** — leave the increment and clear `attempts` when a + delivery next passes the switch gate. Keeps `Release` single-purpose, but + the reset then lives on the hot path and has to distinguish "was parked" + from "was retried", which today is only knowable from `last_error_class` + (`switch_parked` / `dry_run` / the causal classes) — i.e. it would make an + error-class string load-bearing for a correctness decision. + + A RED test should pin the operator-visible fact rather than the column: with + the switch engaged, drive N claim/park cycles well past `maxAttempts`, clear + the switch, then fail the delivery once retryably and assert it is + **rescheduled, not poisoned**. Note that any fix must keep `parkCausal`'s + wall-clock poison reachable — a naive "parks never advance anything" change + must not also disarm the causal budget. + - **outbound_deliveries.ClaimNext lacks a standalone `seq` index.** The loose-scan CTE builds the head set via the `(ordering_key, seq)` partial index, but the outer `c.seq = ANY(ARRAY(...)) FOR UPDATE` re-check has no @@ -291,10 +353,19 @@ is not building them; they stay open here: - **No `BRIDGE_KEK` / per-actor RSA rotation path.** Nothing re-seals existing ciphertext under a new KEK, and the binary's only subcommand is `migrate` - (no args = serve). Changing the KEK orphans every bridged identity's signing key - *and* the PLC escrow rotation key sealed under it (~950 identities, no - recovery). Would need a key-version selector on each sealed blob, a - dual-read custodian, an online re-seal pass, and a cutover. + (no args = serve). Changing the KEK orphans sealed key material in **three** + tables, not two: `bridged_actors.signing_key` (~950 bridged identities' + escrowed secp256k1 repo keys), `service_keys.key_material` row `plc-rotation` + (the PLC escrow key, the only DID recovery path), and — added by v2, and the + one a pre-v2 plan omits — `ap_actors.rsa_key_sealed`, **every native Coves + user's AP signing key** (`internal/db/migrations/017_ap_actors.sql:46`). No + recovery for any of it. Would need a key-version selector on each sealed + blob, a dual-read custodian, an online re-seal pass over all three, and a + cutover. Partial credit on the first: `ap_actors.rsa_key_version` already + exists (`017_ap_actors.sql:47`, stamped from `currentRSAKeyVersion = 1` at + `internal/personas/personas.go:25`) and is deliberately there so rotation is + definable without a schema change — but nothing reads it as a selector, and + the other two tables have no version column at all. - **No backup or restore procedure.** `docker-compose.prod.yml` mounts `./backups` into the Postgres container and nothing writes to it. Note `BRIDGE_KEK` lives in `.env` and is not covered by any database backup at diff --git a/README.md b/README.md index 554a135..ab43d70 100644 --- a/README.md +++ b/README.md @@ -295,15 +295,15 @@ Two classes, and the difference matters at boot: | `FOLLOW_LIST_PATH` | *(optional)* | declarative follow list (see below); unset = the `/admin` API is the only subscription control | | `FOLLOW_LIST_INTERVAL` | `15m` | follow-list reconciler sweep cadence | | `DIVERGENCE_INTERVAL` | `15m` | cadence of the reconciliation sweep (task 17e) that compares atproto state against outbound state and publishes the `tidepool_divergence_*` gauges. **Not an on/off switch:** the sweep is wired unconditionally, runs once at startup before its first tick, and `0` is refused — it is read-only (it reports, never repairs), which is what makes an always-on schedule safe. `GET /admin/divergence` runs one on demand | -| `DIVERGENCE_ACCEPTANCE_STALE_AFTER` | `12h` | how long a pending delivery may sit before the sweep reports its acceptance as **stale**. The report's one crying-wolf knob — shorter and every in-flight post is a finding, longer and a queue that stopped this morning is not in tonight's report. 12h is derived from the retry schedule (~2–3h to poison) plus the causal wait budget (6h), not picked | +| `DIVERGENCE_ACCEPTANCE_STALE_AFTER` | `12h` | how long a pending delivery may sit before the sweep reports its acceptance as **stale**. The report's one crying-wolf knob — shorter and every in-flight post is a finding, longer and a queue that stopped this morning is not in tonight's report. 12h is derived from the causal wait budget (6h, the binding envelope) with the retry schedule inside it — 8 attempts of 30s doubling sum to 3810s, so **~63 minutes** to poison, not the "2–3h" this row claimed before 2026-08-14 — not picked | | `CONSUMER_ENABLED` | **off** | turns on the Jetstream consumer (task 14): native users' opt-outs, profiles, posts, comments and votes flowing outward. Default off because it writes durable outbound state, and because a deployment that has not been canaried should not start accumulating it — not because the seams behind it are stubbed. They are wired: with it on, the **real** enqueuer persists outbound intent, the acceptance engine admits postv2 and writes community-signed acceptances, and opt-out `deleteRemote` / confirmed account deletions actually purge at peers. It also gates two other things — the `OUTBOUND_WORKERS` AND, and whether `/admin/admissions*` exists at all | | `JETSTREAM_URL` | *(optional)* | the self-hosted Jetstream the consumer subscribes to (`ws://` or `wss://`); **required** when `CONSUMER_ENABLED`, and validated at boot whenever set so a typo fails fast instead of becoming a reconnect loop. May be staged ahead of the flag | | `OUTBOUND_WORKERS` | `0` (**off**) | how many delivery workers run. **There is no `OUTBOUND_ENABLED`:** delivery starts only when this is `>0` *and* `CONSUMER_ENABLED`. With the consumer on and this at `0`, outbound intent still accumulates durably and nothing is POSTed — which is the intended staging step, not a broken state. Raising it drains the accumulated backlog immediately | -| `OUTBOUND_DISABLED` | off | global delivery kill switch. A blocked delivery is **parked** — it stays `pending` and resumes when the switch clears — never poisoned, never cancelled. Engaging it loses nothing; it stops the wire | +| `OUTBOUND_DISABLED` | off | global delivery kill switch. A blocked delivery is **parked** — it stays `pending` and resumes when the switch clears — and park itself never poisons or cancels. **But it is not free:** `ClaimNext` increments `attempts` on every claim and `Release` never resets it, so with workers running each ordering key's head is re-claimed every ~5s and its 8-attempt budget is gone in ~40s; the first real failure after the switch clears then poisons instead of retrying. Only `redrive` resets `attempts`. To stop everything, **`OUTBOUND_WORKERS=0` is the cheaper switch** — no worker is constructed, so nothing is spent. Also note every switch here is **inert** while `OUTBOUND_WORKERS=0`. See `DEPLOY.md` §2 | | `OUTBOUND_DISABLED_HOSTS` | *(empty)* | comma-separated inbox **hosts** to park. Lowercased on load and compared case-insensitively — a kill switch must fail closed on case | | `OUTBOUND_DISABLED_COMMUNITIES` | *(empty)* | comma-separated community **AP ids** to park (`https://lemmy.world/c/comicstrips`), matched **exactly and case-sensitively** against the delivery's ordering key. There is no allowlist form: a one-community canary is spelled by disabling every other community | | `OUTBOUND_DISABLED_ACTORS` | *(empty)* | comma-separated actor **DIDs** to park, exact match | -| `OUTBOUND_DRY_RUN` | off | translate and log every delivery, POST nothing. Parks like the kill switches, so nothing is lost — the difference is that the translation ran and is in the log | +| `OUTBOUND_DRY_RUN` | off | log every claimed delivery and POST nothing; the worker parks **before** signing. It does **not** validate the translator — translation happens at *enqueue* time inside the consumer's gate transaction and the worker POSTs the stored payload verbatim, so the translator has already run on everything the moment `CONSUMER_ENABLED=true`. Parks like the kill switches, and carries the same `attempts` cost (above): keep a dry run to seconds, then `redrive` | | `ADMISSION_MAX_PER_AUTHOR_PER_COMMUNITY` | `50` | acceptance-engine flood cap: how many posts one native author may have accepted into one bridged community. `0` = unlimited. Tidepool signs the community's acceptance, so this bounds what it vouches for | ## The admin API @@ -325,7 +325,7 @@ through Caddy. | `GET /admin/divergence` | run one reconciliation sweep synchronously and return the report | always wired in a normal deployment | | `GET /admin/outbound` | delivery queue depth by state (`pending`/`poisoned`/`cancelled`/…) | — the store is always wired, so this answers even with the consumer off and workers at 0. An empty queue then is the truth, not a misconfiguration | | `POST /admin/outbound/redrive` | reset poisoned deliveries to pending | — | -| `POST /admin/outbound/cancel` | park an actor's or a community's pending deliveries as cancelled | — | +| `POST /admin/outbound/cancel` | **terminally** cancel an actor's or a community's pending deliveries (consent withdrawal, community removal). Not a pause: `cancelled` is terminal and `redrive` revives only `poisoned` rows, so this is not the reversible sibling of a kill-switch **park** | — | | `GET /admin/admissions` | list acceptance decisions with `status`, `decisionCode`, `evaluatedCid`; filter by `?status=`/`?community=` | **404** — these routes are registered **only** when `CONSUMER_ENABLED`. A 404 here means the consumer is off, not that the endpoint is broken | | `POST /admin/admissions/readmit` | force re-admit a rejected post | **404**, same reason | | `GET /admin/metrics` | expvar counters filtered to the `tidepool*` prefix (never Go's `cmdline`/`memstats`) | — | diff --git a/internal/ingest/divergence.go b/internal/ingest/divergence.go index 627059a..5487462 100644 --- a/internal/ingest/divergence.go +++ b/internal/ingest/divergence.go @@ -70,14 +70,22 @@ const divergenceSweepTimeout = 2 * time.Minute // or deciding whether the report is crying wolf, has to be able to find it. // // TWELVE HOURS, derived rather than picked. Two envelopes make a pending -// delivery legitimately old: the retry schedule (DefaultMaxDeliveryAttempts=8 -// steps of DefaultBackoffBase=30s doubling to a one-hour cap, so roughly two to -// three hours before a failing delivery poisons) and the causal wait -// (DefaultCausalWaitBudget=6h, during which a reply sits pending for a parent -// that has not been accepted). A window inside either one reports the system -// working. Twelve hours clears both with room, and stays well inside "noticed -// the same day" — a queue that stopped moving this morning is in the report -// before the day ends. +// delivery legitimately old, and the SECOND one dominates: +// +// - The retry schedule: DefaultMaxDeliveryAttempts=8 with +// DefaultBackoffBase=30s doubling (capped at 1h). The waits between the 8 +// attempts are 30s, 1m, 2m, 4m, 8m, 16m, 32m — the cap never binds — which +// sums to 3810s, so a continuously failing delivery poisons after roughly +// ONE HOUR, not the "two to three hours" this comment claimed until +// 2026-08-14. (Recompute this if either constant moves; it is the +// derivation an operator re-checks.) +// - The causal wait: DefaultCausalWaitBudget=6h, during which a reply sits +// pending for a parent that has not been accepted. +// +// A window inside either one reports the system working, so the binding figure +// is the 6h causal wait. Twelve hours clears it with 2x room, and stays well +// inside "noticed the same day" — a queue that stopped moving this morning is +// in the report before the day ends. const DefaultAcceptanceStaleAfter = 12 * time.Hour // Divergence classes. The class is what an operator triages on, so it names the diff --git a/internal/outbound/worker.go b/internal/outbound/worker.go index 1f753f5..60fca98 100644 --- a/internal/outbound/worker.go +++ b/internal/outbound/worker.go @@ -561,8 +561,23 @@ const parkDelay = 5 * time.Second // park holds a kill-switched or dry-run delivery: it stays pending, scheduled a // REAL delay into the future so it is not instantly re-claimable, and never -// poisons. Release does not touch the attempt counter, so a park is not a -// failure and does not itself advance the poison budget. +// poisons — park does not call poison, and no state here is terminal. +// +// IT DOES, HOWEVER, ADVANCE THE POISON BUDGET, and this comment used to claim +// the opposite. Release's SET clause does not touch `attempts` — but ClaimNext +// already did `attempts = attempts + 1` to get here (store/outbound_deliveries.go), +// and nothing ever puts it back. So a delivery parked for parkDelay is +// re-claimed ~5s later, +1 again, and a kill switch held for ~40s exhausts +// maxAttempts. The delivery does not poison while parked, but the first +// retryable failure AFTER the switch clears sees Attempts >= maxAttempts in +// releaseOrPoison and poisons instead of retrying. +// +// Only RedrivePoisoned resets attempts to 0, and only for rows already in +// 'poisoned' — i.e. the repair exists but runs after the fall, not instead of +// it. Fixing this means teaching the store to distinguish "retryable failure, +// back off" (increment is correct) from "parked by a switch" (it is not); +// see FOLLOWUPS.md, "Outbound delivery (task 15)". Documented deliberately +// rather than patched here: it needs a failing test first. func (w *Worker) park(ctx context.Context, delivery *store.OutboundDelivery, class, reason string) error { next := time.Now().Add(parkDelay) _, _, err := w.deliveries.Release(ctx, delivery.ActivityID, delivery.TargetInbox, class, reason, 0, next, *delivery.ClaimedUntil) @@ -576,8 +591,13 @@ func (w *Worker) park(ctx context.Context, delivery *store.OutboundDelivery, cla // parkCausal holds a causally-ineligible delivery WITHOUT a future delay: a held // child must become claimable the instant its bridge-origin parent is accepted // (in practice the parent, a lower-seq delivery on the same serial line, is -// delivered first, so this rarely re-fires). Like park it never poisons and does -// not advance the poison budget; the causal wait is bounded by wall clock. +// delivered first, so this rarely re-fires). Like park it never poisons — but, +// like park, it DOES advance the poison budget, because ClaimNext incremented +// attempts and Release never puts it back (see park's doc above; this comment +// previously asserted the opposite). What keeps that from mattering here is the +// wall-clock bound: causalStatus poisons on the CausalWaitBudget deadline, not +// on the attempt count, so a held child's outcome is decided by elapsed time +// even after its retry budget is spent. func (w *Worker) parkCausal(ctx context.Context, delivery *store.OutboundDelivery, class, reason string) error { _, _, err := w.deliveries.Release(ctx, delivery.ActivityID, delivery.TargetInbox, class, reason, 0, time.Now(), *delivery.ClaimedUntil) if err != nil {