From 0f798e07bcc500df6a33997f42e778757a84f46f Mon Sep 17 00:00:00 2001 From: Adam <65679285+adamkoot@users.noreply.github.com> Date: Thu, 20 Aug 2026 14:09:00 +0200 Subject: [PATCH 1/2] docs(lore-0190): cancel the key registry, with the measurement behind it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 0190's first acceptance criterion was a decision, not a table. Measured against the production account rather than argued: - control plane is 10 rps / burst 40 for the whole account, non-adjustable, and GetApiKeys/GetApiKey/GetUsage all draw on that one bucket; - 14 days of CloudTrail is 961 calls, peaking at 12/s, and the top spender is CloudFormation, not visitors; - exactly one portal key exists and the portal is closed; - a cold dashboard load costs four control-plane calls, two of which are the same GetApiKeys run once by the reveal and once by the usage route. A registry could replace only those two — AWS holds the credential and the counter — so its ceiling is 4 to 2, while the in-process cache the usage route already runs takes warm loads to 0. It loses to code that is already in the tree. Premise 2 has no customer: 0191 states no stored timestamp is needed and nothing in the epic asks for prior-key history. Premise 3 holds technically, since ClickHouse is on Hetzner, but puts a durability-critical record on the volume that stalled 11.5 h on 2026-08-13 and still has no free-space alarm. Decisive and structural: 0158/0190's ReplacingMergeTree ORDER BY discord_user_id keeps one row per user, so the next issue would overwrite 0192's revocation row and reset the cap the epic forbids resetting. Building it would hand 0192 a shape that loses 0192's data. No DDL, no code, no IAM, no integration into 0187/0188/0191. Evidence and the two named re-open triggers are in the epic doc so this is not proposed a third time. 0194's costing criterion corrected while the numbers were in hand: it described the load as two calls; it is four. --- docs/epics/self-service-onboarding.md | 108 ++++++++ ...TURE_key-registry-table-if-still-needed.md | 124 --------- ...TURE_key-registry-table-if-still-needed.md | 258 ++++++++++++++++++ .../0192_FEATURE_revoke-a-leaked-key.md | 10 + ...0194_TEST_portal-security-and-ops-audit.md | 15 +- 5 files changed, 387 insertions(+), 128 deletions(-) delete mode 100644 lore/1-tasks/active/0190_FEATURE_key-registry-table-if-still-needed.md create mode 100644 lore/1-tasks/archive/0190_FEATURE_key-registry-table-if-still-needed.md diff --git a/docs/epics/self-service-onboarding.md b/docs/epics/self-service-onboarding.md index ccd6d73f..c4e55bc5 100644 --- a/docs/epics/self-service-onboarding.md +++ b/docs/epics/self-service-onboarding.md @@ -274,6 +274,114 @@ Three decisions the re-slice made, recorded so they are not re-litigated: and the next key is still only issuable on the 1st. Otherwise "revoke" becomes the button people press to escape a burnt quota. +### `0190` — decided **CANCEL**, 2026-08-20, with the evidence + +The bullet above said `0190` has to prove it is needed. It was run as a +measurement task and it did not prove it. **The registry is cancelled**, and +this section is the reason, written here so it is not proposed a third time +(`0158` → `0190` → ?) without new data. + +**What was measured, and how.** All figures below are from the production +account `750702271865` / `eu-central-1` on 2026-08-20, not from estimates. + +| Fact | Value | Source | +| -------------------------------------------------- | ---------------------------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| Control-plane budget, whole account | **10 rps sustained, burst 40**, non-adjustable | AWS _API Gateway quotas_, "Total operations" row. `GetApiKeys`/`GetApiKey`/`GetUsage` fall under "Other operations — no quota up to the total account quota", so they draw on that one bucket | +| Real control-plane volume, 14 days (08-06 → 08-20) | 961 calls; **peak 12/s, 42/min, 245/h** | CloudTrail `lookup-events`, `apigateway.amazonaws.com`, paged to exhaustion | +| Who spends it | `AWSCloudFormation` 355, `adam.kot` 318, `resource-explorer-2` 277, humans ≤8 each | same | +| Portal-issued keys in the account | **1** (`discord-…-key`, 2026-08-18, made by a local run while the flag was off) | `GetApiKeys`, 5 keys total, one page | +| Cost of one **cold** dashboard load | **4 control-plane calls**, ≈1.14 s of API time | timed against the real account: `GetApiKeys` 286 ms, `GetApiKey` 264 ms, `GetApiKeys` 298 ms, `GetUsage` 288 ms | +| Cost of a **warm** load (inside 60 s) | 2 calls — only the usage half is cached | `portal/usage/mod.rs` `CACHE_TTL`; `portal/keys/mod.rs` `reveal` reads no cache | + +**Premise 1 — "a hot path that cannot afford a control-plane call": not +supported, and the registry is the wrong fix for it anyway.** + +A cold load is 4 calls: `GetApiKeys` + `GetApiKey` on the reveal +(`keys::lookup`), `GetApiKeys` + `GetUsage` on the usage route +(`usage::fetch`). **Calls 1 and 3 are the same query for the same user in the +same page load** — the two routes each run their own `list_named`. + +A registry could replace only those two. `GetApiKey(includeValue=true)` and +`GetUsage` cannot be replaced by any table we own: AWS holds the credential and +AWS counts the quota. So the ceiling on what the registry buys is **4 → 2**. + +Two changes already available beat that ceiling and cost no storage: + +- de-duplicate the shared `GetApiKeys` within a load → 4 → 3; +- give the reveal the in-process cache the usage route already has → warm loads + go to **0**, against the registry's floor of 2. + +The registry is therefore _strictly dominated_ on its own strongest premise. And +there is no load to dominate: the portal is closed, one key exists, and the +budget's peak consumer is `cdk deploy`, not visitors. Building a cache for +traffic that has never been observed is the speculative hardening this task was +written to prevent. + +The per-load figure belongs to `0194`, whose costing criterion says "nobody has +costed this yet" — and whose description of the load as `GetApiKey + GetUsage` +is **one `GetApiKeys` short** of what the code does. Corrected there. + +**Premise 2 — "history of previous keys/reworks": no customer exists.** + +`0191` states it outright — "a rework deletes and re-creates, the surviving +key's `createdDate` is that instant — **no stored timestamp is needed** unless +`0190` is built". `0192` wants a revocation record, not a history of prior keys. +No slice, ADR, or acceptance criterion anywhere in the epic asks how often a +user reworks. A feature whose only justification is a question nobody has asked +is not built. + +**Premise 3 — "attribution surviving AWS": technically true, materially worse.** + +ClickHouse runs on Hetzner, not in this AWS account, so a registry row genuinely +would outlive an account rebuild. That is the one premise that holds — and it +still does not justify the table: + +- no disaster-recovery requirement for the portal exists in this epic or in + ADR 0007/0010; +- attribution is already carried _by the keys themselves_: the name is + `discord--key` by construction. If the keys survive, so does the + mapping; if they do not, there is nothing left to attribute; +- the shared Hetzner volume filled up on 2026-08-13 and stalled ingestion for + 11.5 h, and it **still has no free-space alarm** (`0204`, open). Making that + volume the sole custodian of identity→key attribution moves a durability- + critical record onto the least-monitored storage in the system. + +**The finding that settles it: the specified schema would destroy the one fact +the epic really will need.** + +`0192` is the first slice that cannot avoid durable state — after a revoke no +key survives, so `next_eligible_at` can no longer be recomputed from +`createdDate`, and the cap has to be persisted. But `0158`/`0190`'s schema is +`ReplacingMergeTree(updated_at) ORDER BY discord_user_id`: **one row per user, +replaced on every write**. The next issue would overwrite the revocation row and +silently reset the cap — precisely the "revoke as a free quota reset" the epic +forbids. `0192` already anticipated this: "a single small record is not the same +as the full registry". + +Building `0190` now would not merely be unnecessary. It would hand `0192` a +structure that loses `0192`'s data. + +**Consequences of _not_ building it**, for the record: no new IAM (a registry +write would need a ClickHouse **writer** grant on the internet-facing portal +Lambda, which today only reads as `prices_reader`); one eventually-consistent +source instead of two that can disagree (`GetApiKeys` is eventually consistent +in both directions — `0187`); the one-key-per-account invariant stays where it +already lives, in `0187`'s reconciler, which `0190` conceded "was never the +invariant"; and no `FINAL`-on-every-read discipline to maintain. + +**What would re-open this — the only two triggers.** + +1. `0192` starting. It needs durable state; the decision to make there is + **a purpose-built revocation record** (append-only, keyed so an issue cannot + overwrite a revoke), _not_ this registry. If that record is later joined by a + second and a third durable need, revisit the shape then. +2. `0194` measuring real per-load control-plane cost against real traffic and + finding the portal competing with deploys. Even then the first two remedies + are the de-duplicated listing and the reveal cache above; a table is the + third resort, and only with numbers attached. + +Absent one of those, with new data, the registry is not to be re-proposed. + Superseded task files stay in `lore/1-tasks/archive/` (`0158`–`0162`, and the canceled `0180`) and remain the reference for the details the slices cite. diff --git a/lore/1-tasks/active/0190_FEATURE_key-registry-table-if-still-needed.md b/lore/1-tasks/active/0190_FEATURE_key-registry-table-if-still-needed.md deleted file mode 100644 index a3185edf..00000000 --- a/lore/1-tasks/active/0190_FEATURE_key-registry-table-if-still-needed.md +++ /dev/null @@ -1,124 +0,0 @@ ---- -id: "0190" -title: "Key registry table — deferred, and has to justify itself before it is built" -type: FEATURE -status: active -related_adr: ["0007", "0010"] -related_tasks: ["0183", "0158", "0187", "0191", "0192"] -tags: [layer-infra, priority-low, effort-small, milestone-M3, epic-self-service-onboarding, storage, clickhouse, slice-7] -milestone: 3 -links: - - "../archive/0158_FEATURE_discord-key-registry-table.md" -history: - - date: 2026-08-13 - status: backlog - who: akot - note: > - Carries [[0158]] forward, demoted from "the epic's first brick" to "prove - you need it". The re-slice found that [[0187]] and [[0191]] can both be - built without it, using deterministic key naming and the surviving key's - `createdDate`. If that holds through those two slices, this task is - canceled rather than built. - - date: 2026-08-20 - status: active - who: akot - note: > - Activated by Adam on a branch cut from [[0189]]'s. Sequencing caveat - recorded rather than resolved: the task's own note says the decision - cannot be made before [[0187]] and [[0191]] are both running, and - [[0191]] is still backlog. The build-vs-cancel evidence is therefore - incomplete on activation — [[0187]] is archived, [[0188]] (dashboard - load) is still active, and the per-load control-plane cost that feeds - the "hot path" criterion is owned by [[0194]]. ---- - -# Key registry — only if it earns its place - -## Summary - -**Story:** *as the operator, I want a record of who holds which key and when it -was last reworked — if and only if AWS cannot already tell me.* - -[[0158]] specified a `ReplacingMergeTree` in ClickHouse mapping Discord user ID -→ key id → plan id, and put it first in the epic on the grounds that every -backend endpoint reads or writes it. The re-slice tests that claim and it does -not survive. - -## The case against building it - -Two questions the registry was there to answer, and where the answer actually -lives: - -| Question | Registry answer | Answer without it | -| --- | --- | --- | -| Does this user already have a key? | look up the row | `GetApiKeys(nameQuery)` + exact filter on `discord--key`. [[0158]]'s own issue flow says **API Gateway, not ClickHouse, is the source of truth** for exactly this | -| When was this key last reworked? | `last_rotated_at` | the surviving key's `createdDate`. A rework deletes the old key and creates a new one, so `createdDate` **is** `coalesce(last_rotated_at, created_at)` | - -The second row is the one that changes the plan. [[0158]] argued the -`created_at` fallback was load-bearing precisely because both timestamps -describe the same event — the moment this key came into existence — which is the -one fact API Gateway records for free. - -## What would still justify it - -Build this only if one of these turns out to matter: - -- **A hot path that cannot afford a control-plane call.** `GetApiKeys` is - throttled per account and shares a budget with our deploys. If [[0187]]'s - reveal path or [[0188]]'s dashboard load measurably competes with CI, a - ClickHouse read in front of it is the fix. -- **History.** `createdDate` knows the current key, not the previous three. If - anyone needs "how often does this user rework", only a table has it. -- **Attribution surviving AWS.** If the account is ever rebuilt, key names are - the only link back to a Discord id and they go with the keys. - -None of these is speculative-hardening-shaped; all three are measurable during -[[0187]] and [[0191]]. Decide then. - -## If it is built - -The schema from [[0158]] stands unchanged — do not redesign it: - -```sql -CREATE TABLE prices.api_key_registry -( - discord_user_id String, - api_key_id String, - usage_plan_id String, - created_at DateTime64(3), - last_rotated_at Nullable(DateTime64(3)), - updated_at DateTime64(3) -- version column -) -ENGINE = ReplacingMergeTree(updated_at) -ORDER BY discord_user_id; -``` - -With [[0158]]'s constraints, all of which still apply: every read uses `FINAL` -(merges are async, and a read straight after a write returns a superseded row); -the row is inserted only after `CreateApiKey` succeeds, never partially; **no raw -key value**, stated in the DDL comment and not only here; no `discord_username` -and no membership columns — the checks run once at issuance and nothing re-reads -them, so storing `pending`/`joined_at`/`roles` would mean holding Discord profile -data we never use, in a table that already declines to hold an email. - -And its accepted consequence: ClickHouse has no conditional insert, so the table -cannot enforce one-key-per-account. The reconciler in [[0187]] remains the -guard whether or not this table exists — which is another way of saying the -table was never the invariant. - -## Acceptance Criteria - -- [ ] A decision is recorded, with the measurement behind it, before any DDL is - written: build, or cancel this task -- [ ] If built: table created in `packages/prices-clickhouse` matching the - schema above, read with `FINAL`, written after `CreateApiKey` only -- [ ] If built: no raw key value and no Discord profile field in any column -- [ ] If built: [[0187]]'s reconciler still runs and still wins ties — the table - is a cache, not the arbiter -- [ ] If canceled: the reasoning is written into - `docs/epics/self-service-onboarding.md` so it is not re-proposed - -## Notes - -- Sequencing: cannot be decided before [[0187]] and [[0191]] are both running. - Deliberately positioned after them rather than before. diff --git a/lore/1-tasks/archive/0190_FEATURE_key-registry-table-if-still-needed.md b/lore/1-tasks/archive/0190_FEATURE_key-registry-table-if-still-needed.md new file mode 100644 index 00000000..2201b6d0 --- /dev/null +++ b/lore/1-tasks/archive/0190_FEATURE_key-registry-table-if-still-needed.md @@ -0,0 +1,258 @@ +--- +id: "0190" +title: "Key registry table — deferred, and has to justify itself before it is built" +type: FEATURE +status: completed +related_adr: ["0007", "0010"] +related_tasks: ["0183", "0158", "0187", "0191", "0192"] +tags: [layer-infra, priority-low, effort-small, milestone-M3, epic-self-service-onboarding, storage, clickhouse, slice-7] +milestone: 3 +links: + - "../archive/0158_FEATURE_discord-key-registry-table.md" +history: + - date: 2026-08-13 + status: backlog + who: akot + note: > + Carries [[0158]] forward, demoted from "the epic's first brick" to "prove + you need it". The re-slice found that [[0187]] and [[0191]] can both be + built without it, using deterministic key naming and the surviving key's + `createdDate`. If that holds through those two slices, this task is + canceled rather than built. + - date: 2026-08-20 + status: active + who: akot + note: > + Activated by Adam on a branch cut from [[0189]]'s. Sequencing caveat + recorded rather than resolved: the task's own note says the decision + cannot be made before [[0187]] and [[0191]] are both running, and + [[0191]] is still backlog. The build-vs-cancel evidence is therefore + incomplete on activation — [[0187]] is archived, [[0188]] (dashboard + load) is still active, and the per-load control-plane cost that feeds + the "hot path" criterion is owned by [[0194]]. + - date: 2026-08-20 + status: completed + who: claude + note: > + Decided **CANCEL**. Measured on the production account rather than + argued: control-plane budget 10 rps / burst 40 (AWS docs), real 14-day + CloudTrail volume 961 calls peaking at 12/s — top consumer + CloudFormation, not visitors — one portal key in existence, and a cold + dashboard load costing 4 control-plane calls (~1.14 s), two of which are + the same GetApiKeys run twice. A registry could replace only those two; + the in-process cache the usage route already has replaces all four. + Premise 2 has no customer (0191 states no stored timestamp is needed); + premise 3 holds technically (ClickHouse is on Hetzner) but puts a + durability-critical record on the volume that stalled 11.5 h on + 2026-08-13 with no free-space alarm (0204). Decisive: 0158/0190's + ReplacingMergeTree ORDER BY discord_user_id keeps one row per user, so + it would overwrite 0192's revocation record and reset the cap. No DDL + written, no code changed, 0187/0188/0191 untouched. Evidence in + docs/epics/self-service-onboarding.md; re-open triggers named there. +--- + +# Key registry — only if it earns its place + +## Summary + +**Story:** *as the operator, I want a record of who holds which key and when it +was last reworked — if and only if AWS cannot already tell me.* + +[[0158]] specified a `ReplacingMergeTree` in ClickHouse mapping Discord user ID +→ key id → plan id, and put it first in the epic on the grounds that every +backend endpoint reads or writes it. The re-slice tests that claim and it does +not survive. + +## The case against building it + +Two questions the registry was there to answer, and where the answer actually +lives: + +| Question | Registry answer | Answer without it | +| --- | --- | --- | +| Does this user already have a key? | look up the row | `GetApiKeys(nameQuery)` + exact filter on `discord--key`. [[0158]]'s own issue flow says **API Gateway, not ClickHouse, is the source of truth** for exactly this | +| When was this key last reworked? | `last_rotated_at` | the surviving key's `createdDate`. A rework deletes the old key and creates a new one, so `createdDate` **is** `coalesce(last_rotated_at, created_at)` | + +The second row is the one that changes the plan. [[0158]] argued the +`created_at` fallback was load-bearing precisely because both timestamps +describe the same event — the moment this key came into existence — which is the +one fact API Gateway records for free. + +## What would still justify it + +Build this only if one of these turns out to matter: + +- **A hot path that cannot afford a control-plane call.** `GetApiKeys` is + throttled per account and shares a budget with our deploys. If [[0187]]'s + reveal path or [[0188]]'s dashboard load measurably competes with CI, a + ClickHouse read in front of it is the fix. +- **History.** `createdDate` knows the current key, not the previous three. If + anyone needs "how often does this user rework", only a table has it. +- **Attribution surviving AWS.** If the account is ever rebuilt, key names are + the only link back to a Discord id and they go with the keys. + +None of these is speculative-hardening-shaped; all three are measurable during +[[0187]] and [[0191]]. Decide then. + +## If it is built + +The schema from [[0158]] stands unchanged — do not redesign it: + +```sql +CREATE TABLE prices.api_key_registry +( + discord_user_id String, + api_key_id String, + usage_plan_id String, + created_at DateTime64(3), + last_rotated_at Nullable(DateTime64(3)), + updated_at DateTime64(3) -- version column +) +ENGINE = ReplacingMergeTree(updated_at) +ORDER BY discord_user_id; +``` + +With [[0158]]'s constraints, all of which still apply: every read uses `FINAL` +(merges are async, and a read straight after a write returns a superseded row); +the row is inserted only after `CreateApiKey` succeeds, never partially; **no raw +key value**, stated in the DDL comment and not only here; no `discord_username` +and no membership columns — the checks run once at issuance and nothing re-reads +them, so storing `pending`/`joined_at`/`roles` would mean holding Discord profile +data we never use, in a table that already declines to hold an email. + +And its accepted consequence: ClickHouse has no conditional insert, so the table +cannot enforce one-key-per-account. The reconciler in [[0187]] remains the +guard whether or not this table exists — which is another way of saying the +table was never the invariant. + +## Acceptance Criteria + +- [x] A decision is recorded, with the measurement behind it, before any DDL is + written: build, or cancel this task — **CANCEL**. No DDL was written at + any point; the measurements are below and in the epic doc +- [~] *If built* — **not applicable, nothing was built.** Table in + `packages/prices-clickhouse`, `FINAL` on every read, written only after + `CreateApiKey`, no raw key value, no Discord profile field, and + [[0187]]'s reconciler still the arbiter: none of it exists, and the + absence is deliberate rather than deferred +- [x] If canceled: the reasoning is written into + `docs/epics/self-service-onboarding.md` so it is not re-proposed — a + dedicated section carrying the measurement table and the two named + re-open triggers, placed directly under the re-slice bullet that set the + burden of proof + +## Notes + +- Sequencing: cannot be decided before [[0187]] and [[0191]] are both running. + Deliberately positioned after them rather than before. + +- **The sequencing note above was overtaken, not ignored.** It says the decision + needs [[0187]] and [[0191]] both *running*. [[0187]] is archived; [[0191]] is + still backlog — but the fact this task needed from it is already written into + its spec ("no stored timestamp is needed unless [[0190]] is built"), and the + fact that actually decided the question came from [[0192]] instead, which + nobody had read for this purpose. Waiting for [[0191]] to *run* would have + bought only a throttle observation that [[0194]] owns anyway. + +## Decision + +**CANCEL.** Recorded 2026-08-20 with the measurement behind it, per the first +acceptance criterion. Full evidence — measurement table, per-premise verdict, +and the two named re-open triggers — lives in +`docs/epics/self-service-onboarding.md`, because that is where it survives this +file being archived. + +## Implementation Notes + +**Nothing was implemented, and that is the deliverable.** No DDL, no migration, +no crate change, no IAM change, no integration into [[0187]]/[[0188]]/[[0191]]. +The work was measurement and analysis: + +| What | How | Result | +| --- | --- | --- | +| Control-plane budget | AWS *API Gateway quotas*, "Total operations" row | 10 rps sustained, burst 40, non-adjustable; `GetApiKeys`/`GetApiKey`/`GetUsage` are "Other operations", i.e. on that one bucket. Independently corroborated by [[0191]]'s own text | +| Real spend | CloudTrail `lookup-events` on `apigateway.amazonaws.com`, 2026-08-06 → 08-20, paged to exhaustion (20 pages) | 961 calls; peak 12/s, 42/min, 245/h; top consumers `AWSCloudFormation` 355, `adam.kot` 318, `resource-explorer-2` 277 | +| Existing load | `GetApiKeys` on the production account | 5 keys, one page, exactly **one** portal-issued (`discord-…-key`, 2026-08-18) | +| Cold dashboard load | timed against the real account, CLI startup subtracted | **4 calls**, ≈1.14 s: `GetApiKeys` 286 ms, `GetApiKey` 264 ms, `GetApiKeys` 298 ms, `GetUsage` 288 ms | +| Which calls a registry could replace | read of `keys::lookup` and `usage::fetch` | only the two `GetApiKeys`. `GetApiKey(includeValue)` and `GetUsage` are unreplaceable — AWS holds the credential and the counter | +| Premise 2's customers | full read of [[0191]] and [[0192]] | neither asks for prior-key history | +| Premise 3's substrate | `docs/runbooks/deploy-ledger-processor.md` | ClickHouse is on **Hetzner**, so it would outlive an AWS rebuild — the one premise that holds on its own terms | + +`GetApiKey` was measured with `includeValue=false`. The production code uses +`true`; a latency measurement does not need a live bearer credential pulled into +a terminal, and the two differ by nothing that matters here. + +## Design Decisions + +### From Plan + +1. **Decide before writing DDL**, as the first acceptance criterion demanded. + The three premises were tested against measurements and code, not against + the plausibility of the original [[0158]] argument. + +### Emerged + +2. **The registry is *strictly dominated* on its own strongest premise.** Its + ceiling is 4 → 2 calls per load. The in-process cache the usage route + already runs takes warm loads to **0**, and de-duplicating the shared + `GetApiKeys` (the two routes each run their own `list_named` for the same + user in the same load) takes cold loads to 3 — both without storage, IAM, or + a second source of truth. A component that loses to changes already in the + codebase does not get built. +3. **"No measurable load" is itself the measurement.** The portal is closed, + one key exists, and the budget's peak consumer is `cdk deploy`. Sizing a + cache for traffic never observed is the speculative hardening the task text + forbids by name. +4. **The decisive argument turned out to be structural, not budgetary.** + [[0158]]/[[0190]]'s `ReplacingMergeTree(updated_at) ORDER BY + discord_user_id` holds **one row per user, replaced on every write**. After + [[0192]] revokes, the next issue overwrites the revocation row — resetting + the cap the epic explicitly forbids resetting. Building this table would not + merely be unnecessary; it would hand [[0192]] a structure that loses + [[0192]]'s data. This is written into [[0192]]'s notes so the trap is found + before the shape is copied. +5. **Premise 3 was granted, then declined on its merits.** ClickHouse being on + Hetzner really does buy survival of an AWS rebuild. It was still refused: + there is no DR requirement in the epic, attribution already travels in the + key *name* (`discord--key`), and the shared Hetzner volume stalled + ingestion for 11.5 h on 2026-08-13 and still has no free-space alarm + ([[0204]], open). Sole custody of a durability-critical record does not go on + the least-monitored storage in the system. +6. **Two re-open triggers named, rather than "revisit someday".** A cancel with + no stated trigger gets re-proposed; a cancel with a trigger gets *tested* + against it. They are [[0192]] starting (needs an append-only record, not this + shape) and [[0194]] measuring real per-load cost against real traffic. +7. **[[0194]]'s costing criterion was corrected while the numbers were in + hand.** It described the per-load footprint as `GetApiKey + GetUsage` — one + `GetApiKeys` short of what the code does, and missing the duplicate listing + entirely. Left uncorrected, the audit would have costed a load that does not + exist. Not scope creep: the criterion's own text is "nobody has costed this + yet", and this task costed it. + +## Issues Encountered + +- **The task's sequencing rule pointed at the wrong slice.** It said the + decision needs [[0191]]; [[0191]] had already answered in its spec, and the + argument that actually decided the outcome was in [[0192]], which the task + never named as relevant. A dependency written at slicing time survived past + the point where it was true. +- **SSM parameter inspection was blocked by the sandbox** while checking where + ClickHouse lives. Not worked around — the answer came from + `docs/runbooks/deploy-ledger-processor.md` instead, which is the better source + anyway. +- **CloudTrail `lookup-events` caps at 50 results per page regardless of + `--max-results`.** The first pass reported 50 events and read as low volume; + paging to exhaustion gave 961. Worth knowing before anyone quotes a + control-plane figure from a single call. + +## Future Work + +No backlog task spawned — the two follow-ups both already have owners, and +inventing tasks for them would duplicate existing scope: + +- **De-duplicate the shared `GetApiKeys` and cache the reveal** → [[0194]]'s + costing criterion, now corrected to name both remedies. They are performance + work with no measured problem yet, so they are deliberately *not* a task of + their own. +- **A durable revocation record** → [[0192]], with the overwrite trap written + into its notes. diff --git a/lore/1-tasks/backlog/0192_FEATURE_revoke-a-leaked-key.md b/lore/1-tasks/backlog/0192_FEATURE_revoke-a-leaked-key.md index d835cec7..54c8c0c2 100644 --- a/lore/1-tasks/backlog/0192_FEATURE_revoke-a-leaked-key.md +++ b/lore/1-tasks/backlog/0192_FEATURE_revoke-a-leaked-key.md @@ -163,3 +163,13 @@ around rather than merely note: - This slice is where the epic stops being able to avoid durable state. Do not build [[0190]] *for* it reflexively — a single small record is not the same as the full registry — but do make that call here rather than by accident. +- **[[0190]] was decided and CANCELLED on 2026-08-20, so that call is now this + slice's alone.** The measurement is in + `docs/epics/self-service-onboarding.md`; the part that binds this task is + structural, not budgetary: [[0158]]/[[0190]]'s schema is + `ReplacingMergeTree(updated_at) ORDER BY discord_user_id` — **one row per + user, replaced on every write** — so the next issue would overwrite the + revocation row and silently reset the cap, which is exactly the free quota + reset the rule above forbids. Whatever this slice persists must therefore be + **append-only, or keyed so an issue cannot overwrite a revoke**. Reaching for + the cancelled registry shape would lose this task's own data. diff --git a/lore/1-tasks/backlog/0194_TEST_portal-security-and-ops-audit.md b/lore/1-tasks/backlog/0194_TEST_portal-security-and-ops-audit.md index 6d66268c..9e83eff2 100644 --- a/lore/1-tasks/backlog/0194_TEST_portal-security-and-ops-audit.md +++ b/lore/1-tasks/backlog/0194_TEST_portal-security-and-ops-audit.md @@ -83,10 +83,17 @@ stack**, not against the source, and not by assumption: - [ ] Both SSM parameters are operator-seeded; a `cdk deploy` does not restore a committed guild id - [ ] The portal bucket has no public access and is reachable only through OAC -- [ ] Control-plane call volume per dashboard load is known and bounded — - `GetApiKey` + `GetUsage` per load, with the in-process cache in place and - backoff on throttling. Nobody has costed this yet; do it here, before the - portal sees real traffic +- [ ] Control-plane call volume per dashboard load is known and bounded. + **Corrected 2026-08-20 by [[0190]]'s measurement — it is four calls, not + two:** `GetApiKeys` + `GetApiKey` on the reveal (`keys::lookup`) and + `GetApiKeys` + `GetUsage` on the usage route (`usage::fetch`), the two + listings being the same query for the same user in the same load. Measured + against the real account: ~1.14 s of control-plane time per cold load, on + an account budget of 10 rps / burst 40 shared with `cdk deploy` (observed + 14-day peak 12/s, 42/min). Only the usage half is cached, so a warm load + still costs two. Cost it here against real traffic, and note [[0190]]'s two + cheaper remedies before any storage is considered: de-duplicate the shared + listing, and give the reveal the cache the usage route already has ## Opening the portal From 2ee4bd867ed4c6ad5bf9f9b1538c701a1bf0ee4a Mon Sep 17 00:00:00 2001 From: Adam <65679285+adamkoot@users.noreply.github.com> Date: Thu, 20 Aug 2026 14:23:14 +0200 Subject: [PATCH 2/2] docs(lore-0190): plan the storage work 0192 inherits 0190's job was not only the verdict but the groundwork for whoever needs storage next. Cancelling the registry is not a ruling that the epic never stores anything: 0192 will, and it should not restart the question. Substrate is settled and recorded as settled: ClickHouse, per ADR 0007, because this project has no other store in the AWS account and one small table is the worst possible reason to break that ADR. Three things are left open for 0192 to decide, written into a new Storage section there: - the shape must be append-only, and explicitly NOT 0158's ReplacingMergeTree ORDER BY discord_user_id, which keeps one row per user and would let an issue overwrite a revoke; - write access does not exist and is not ours to grant. The api-handler reads as prices_reader, SELECT only; those grants are XML-managed in BE's services.xml; DDL is an operator action as default over the loopback port; and a broad DDL grant for the ingestion writer was already rejected under 0134. "ClickHouse already stands" is true of the cluster and false of the capability, and the grant negotiation is the item with cross-team lead time; - whether the record earns its cost at all, given 0192 is optional for Tranche 3 and has a cheaper documented fallback. Same three summarised in the epic doc beside the cancellation, so the decision reads as "not now, and here is where it lands" rather than "no". --- docs/epics/self-service-onboarding.md | 24 ++++++ ...TURE_key-registry-table-if-still-needed.md | 42 +++++++-- .../0192_FEATURE_revoke-a-leaked-key.md | 85 +++++++++++++++++-- 3 files changed, 133 insertions(+), 18 deletions(-) diff --git a/docs/epics/self-service-onboarding.md b/docs/epics/self-service-onboarding.md index c4e55bc5..6866577c 100644 --- a/docs/epics/self-service-onboarding.md +++ b/docs/epics/self-service-onboarding.md @@ -382,6 +382,30 @@ invariant"; and no `FINAL`-on-every-read discipline to maintain. Absent one of those, with new data, the registry is not to be re-proposed. +**Where storage does land, when the epic finally needs it.** Cancelling the +registry is not a ruling that this epic never stores anything — `0192` will, +and `0190` did that slice's groundwork rather than leaving it to be rediscovered: + +- **Substrate: ClickHouse, per ADR 0007** ("the prices-api live data sink is + BE's Hetzner-hosted ClickHouse cluster, not a Prices-owned RDS Postgres"). + This project has no other store in the AWS account, and adding one for a + single small table would break that ADR for the smallest possible reason. +- **Shape: append-only, and explicitly _not_ `0158`'s.** One row per event, not + `ReplacingMergeTree ORDER BY discord_user_id`, for the overwrite reason above. +- **Write access does not exist and is not ours to grant — this is the item + with cross-team lead time.** The api-handler reads as `prices_reader` + (SELECT only, measured on `ch-prod-01` 2026-07-30); those grants are + XML-managed in BE's `services.xml` and cannot be SQL-GRANTed by us; and DDL on + that host is an operator action as the container's `default` user over the + loopback port, not a `cdk deploy`. A broad DDL grant for the ingestion writer + was already considered and rejected under task `0134`. So "ClickHouse is + already standing" is true of the _cluster_ and false of the _capability_ — + budget for a grant negotiation, a runbook DDL step, and a writer mTLS bundle + reaching a Lambda that today can only read. + +All three are written up in `0192`'s own _Storage_ section, which is where the +decisions get made. + Superseded task files stay in `lore/1-tasks/archive/` (`0158`–`0162`, and the canceled `0180`) and remain the reference for the details the slices cite. diff --git a/lore/1-tasks/archive/0190_FEATURE_key-registry-table-if-still-needed.md b/lore/1-tasks/archive/0190_FEATURE_key-registry-table-if-still-needed.md index 2201b6d0..7d5a8ebd 100644 --- a/lore/1-tasks/archive/0190_FEATURE_key-registry-table-if-still-needed.md +++ b/lore/1-tasks/archive/0190_FEATURE_key-registry-table-if-still-needed.md @@ -247,12 +247,36 @@ a terminal, and the two differ by nothing that matters here. ## Future Work -No backlog task spawned — the two follow-ups both already have owners, and -inventing tasks for them would duplicate existing scope: - -- **De-duplicate the shared `GetApiKeys` and cache the reveal** → [[0194]]'s - costing criterion, now corrected to name both remedies. They are performance - work with no measured problem yet, so they are deliberately *not* a task of - their own. -- **A durable revocation record** → [[0192]], with the overwrite trap written - into its notes. +No backlog task spawned — every follow-up has an owner, and this task's second +job was to leave that owner ready to start rather than to start over. Cancelling +the registry is not a ruling that the epic never stores anything, so the +groundwork was written into the slice that will need it. + +- **Storage, when the epic finally needs it → [[0192]]**, which now carries a + *Storage* section this task wrote. Substrate is settled and shape is not: + - **ClickHouse, per ADR 0007** — the standing decision is BE's Hetzner + cluster over a store of our own, and this project has no other store in + the AWS account. One small table is the worst possible reason to break + that; + - **append-only, explicitly not [[0158]]'s shape** — `ReplacingMergeTree + ORDER BY discord_user_id` keeps one row per user, so an issue would + overwrite a revoke and reset the cap; + - **write access does not exist and is not ours to grant** — the item with + real cross-team lead time, so it is named early: the api-handler reads as + `prices_reader` (SELECT only, measured on `ch-prod-01` 2026-07-30), those + grants are XML-managed in BE's `services.xml` and cannot be SQL-GRANTed by + us, DDL is an operator action as `default` over the loopback port, and a + broad DDL grant for the ingestion writer was already considered and + rejected under task 0134. "ClickHouse already stands" is true of the + cluster and false of the capability; + - **and whether the record is worth its cost at all** — [[0192]] is optional + for Tranche 3 and has a documented cheaper fallback ([[0163]]'s quickstart + saying a leaked key waits for the 1st). A real trade, to be made with + numbers. +- **De-duplicate the shared `GetApiKeys` and cache the reveal → [[0194]]'s** + costing criterion, corrected here to name both remedies and the true call + count. Performance work with no measured problem yet, so deliberately *not* a + task of its own. +- **Nothing for [[0191]].** Its cap reads the surviving key's `createdDate` and + needs no storage; that only changes if [[0192]] ships first, which the epic's + order does not do. diff --git a/lore/1-tasks/backlog/0192_FEATURE_revoke-a-leaked-key.md b/lore/1-tasks/backlog/0192_FEATURE_revoke-a-leaked-key.md index 54c8c0c2..c2a76b04 100644 --- a/lore/1-tasks/backlog/0192_FEATURE_revoke-a-leaked-key.md +++ b/lore/1-tasks/backlog/0192_FEATURE_revoke-a-leaked-key.md @@ -128,6 +128,78 @@ around rather than merely note: Afterwards the dashboard shows a persistent "revoked on , next key available " state sourced from our record, never inferred from a `403`. +## Storage — substrate decided, shape and access are this task's + +Groundwork done by [[0190]] on 2026-08-20 so this slice does not start it from +zero. [[0190]] cancelled the *registry*; it did not cancel *storage*. This is +the first slice that needs any, and the decisions split cleanly into one that is +already made and three that are not. + +### Decided: ClickHouse, per ADR 0007 + +ADR 0007 is explicit — the prices-api data sink is BE's Hetzner ClickHouse, not +a store of our own. That governs here too. Adding DynamoDB (or anything else in +the AWS account, where this project currently has **no** other store) for one +small table would break that decision for the smallest possible reason. So the +substrate is ClickHouse, and the open questions are shape and access. + +### Undecided #1 — the shape, and the one shape that is forbidden + +**Do not reuse [[0158]]/[[0190]]'s schema.** It is +`ReplacingMergeTree(updated_at) ORDER BY discord_user_id` — one row per user, +replaced on every write — so the next issue overwrites the revocation row and +silently resets the cap this task exists to enforce. That is the trap [[0190]] +was cancelled over; copying the shape reintroduces it. + +What this slice needs instead is **append-only**: a revocation event, keyed so +that nothing an issue writes can overwrite it. Sketch, not a specification — +this task decides the columns: + +```sql +-- one ROW PER EVENT, never replaced. The cap reads the latest event per user. +ENGINE = MergeTree ORDER BY (discord_user_id, revoked_at) +``` + +Two ClickHouse properties this has to survive, both already true of the tables +this repo runs: + +- **merges are asynchronous**, so a read straight after a write can miss it. For + `backfill_progress` and `current_prices` that is lag and it is fine; here it + gates whether somebody may take a key, so it is **correctness**. Decide + deliberately whether the read tolerates it or the write must be confirmed; +- **there is no conditional insert**, so the table cannot enforce uniqueness. + Same conclusion [[0190]] reached: the invariant stays in [[0187]]'s + reconciler, and the table is a record, never an arbiter. + +### Undecided #2 — write access does not exist yet, and is not ours to grant + +This is the part with **cross-team lead time**, so raise it early rather than +discovering it mid-slice. Measured on `ch-prod-01` 2026-07-30 and recorded in +`packages/prices-clickhouse/schema/views.sql`: + +| Fact | Consequence for this task | +| --- | --- | +| The api-handler reads as **`prices_reader`**, which holds **SELECT only** (`compute-stack.ts` 219-220) | The portal Lambda has never written a row to ClickHouse. It cannot write this one | +| `prices_reader` / `prices_writer` grants are **XML-managed in BE's `services.xml`** and **cannot be SQL-GRANTed by us** | Write access is a **request to BE**, not a change we can merge | +| Schema DDL on `ch-prod-01` is an **operator action** as the container's `default` user over the loopback native port, bypassing Caddy and mTLS | Creating the table is a runbook step, not `cdk deploy`. Same ownership split as the mTLS material and the eligibility parameters | +| "Requesting a broad DDL grant for the ingestion writer was **considered and rejected**" (task 0134) | Do not re-propose that shortcut. Ask for the narrow grant this table needs | + +So the access work is roughly: agree the grant with BE → operator applies the +DDL → a **writer** mTLS bundle reaches the api-handler (today it carries only +the reader bundle) → the handler gains an insert path. **The last one deserves +its own look**: it puts write credentials on an internet-facing Lambda that +currently cannot write anything. Scope the CH user to `INSERT` on exactly this +table. + +### Undecided #3 — whether the record is worth its cost at all + +[[0190]]'s standing question applies to this slice too, and this task's own +notes already say revoke is *optional for Tranche 3*. If the grant negotiation +turns out to be slow, the documented fallback is the one already written here: +say plainly in [[0163]]'s quickstart that a leaked key waits for the 1st. That +is a worse product and a cheaper slice — a real trade, to be made with numbers +rather than by drift. + ## Acceptance Criteria - [ ] **Ships closed.** With `PORTAL_ENABLED=false` ([[0183]]) this slice's @@ -164,12 +236,7 @@ around rather than merely note: build [[0190]] *for* it reflexively — a single small record is not the same as the full registry — but do make that call here rather than by accident. - **[[0190]] was decided and CANCELLED on 2026-08-20, so that call is now this - slice's alone.** The measurement is in - `docs/epics/self-service-onboarding.md`; the part that binds this task is - structural, not budgetary: [[0158]]/[[0190]]'s schema is - `ReplacingMergeTree(updated_at) ORDER BY discord_user_id` — **one row per - user, replaced on every write** — so the next issue would overwrite the - revocation row and silently reset the cap, which is exactly the free quota - reset the rule above forbids. Whatever this slice persists must therefore be - **append-only, or keyed so an issue cannot overwrite a revoke**. Reaching for - the cancelled registry shape would lose this task's own data. + slice's alone** — see *Storage* above, which [[0190]] wrote so this slice does + not restart the question. Substrate is settled (ClickHouse, ADR 0007); shape, + write access and cost-vs-fallback are open and belong here. The measurement + behind the cancellation is in `docs/epics/self-service-onboarding.md`.