From 06a0587a74b3c15038e77325e9858c52895eb1bc Mon Sep 17 00:00:00 2001 From: secret-mars Date: Sun, 10 May 2026 22:40:48 +0000 Subject: [PATCH] hygiene(cache-invariants): extract to single source + posture-marker + structural test (#723) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extracts the 3 cache-key invariants from #697 umbrella body into a single canonical doc + 1-line pointer comments + structural enforcement via CACHE_INVARIANTS:POSTURE markers. Surfaced via @steel-yeti's Cycle 26 advisory on #722 + @secret-mars's v162 elevation; filed as #723 by @whoabuddy. Lands before Step 3.2 (#725 PR) so the 4-route × 3-place duplication never materializes. ## Changes - **NEW** `lib/inbox/CACHE_INVARIANTS.md` — canonical 3-invariant prose with agent-news#802 cross-reference + history (#697 → #722 → #723 → #725) + per-route compliance checklist. - **EDIT** `lib/inbox/d1-reads.ts` — replace ~17-line invariant file-header block with a 2-line pointer. - **EDIT** `app/api/inbox/[address]/route.ts` — replace ~26-line inline invariant block in GET handler with a 4-line pointer + add `CACHE_INVARIANTS:POSTURE=public-only-get` marker. - **EDIT** `app/api/inbox/[address]/[messageId]/route.ts` — add `CACHE_INVARIANTS:POSTURE=public-only-get` marker. - **EDIT** `app/api/outbox/[address]/route.ts` — add `CACHE_INVARIANTS:POSTURE=public-only-get` marker. - **NEW** `lib/inbox/__tests__/cache-invariants-enforcement.test.ts` — ~95 LOC structural enforcement test: asserts each inbox/outbox route file declares a posture marker, and if posture is `auth-required-get` then file must include `Cache-Control: private` (Invariant 2). Catches the agent-news#802 unauthenticated-HIT bug class via lint rather than runtime. ## Why posture-marker, not auth-import detection Initial proposal was to detect any auth import (verifyBitcoinSignature etc.) and require co-occurring `Cache-Control: private`. That false- positives on these route files because POST/PATCH legitimately import `verifyBitcoinSignature` for sender auth on writes, but the GET handler is public-only. The agent-news#802 bug class only applies to GET-side cache HITs, so the load-bearing check is at the GET-handler posture. The magic-comment marker lets each route file declare that posture explicitly + forces explicit declaration when a new route is added (the test fails if the marker is missing). ## Behavior preservation Net diff: +127 LOC (canonical doc + test) / -38 LOC (inline blocks removed). No runtime behavior changes — the invariants are documented + structurally enforced, the routes themselves are unchanged. Closes #723. --- app/api/inbox/[address]/[messageId]/route.ts | 4 + app/api/inbox/[address]/route.ts | 28 ++--- app/api/outbox/[address]/route.ts | 4 + lib/inbox/CACHE_INVARIANTS.md | 62 +++++++++ .../cache-invariants-enforcement.test.ts | 118 ++++++++++++++++++ lib/inbox/d1-reads.ts | 22 +--- 6 files changed, 203 insertions(+), 35 deletions(-) create mode 100644 lib/inbox/CACHE_INVARIANTS.md create mode 100644 lib/inbox/__tests__/cache-invariants-enforcement.test.ts diff --git a/app/api/inbox/[address]/[messageId]/route.ts b/app/api/inbox/[address]/[messageId]/route.ts index 0f861498..5b110235 100644 --- a/app/api/inbox/[address]/[messageId]/route.ts +++ b/app/api/inbox/[address]/[messageId]/route.ts @@ -1,3 +1,7 @@ +// CACHE_INVARIANTS:POSTURE=public-only-get +// See lib/inbox/CACHE_INVARIANTS.md — GET handler is public; PATCH (mark-read) +// uses verifyBitcoinSignature for caller auth on writes but does not cache. + import { NextRequest, NextResponse } from "next/server"; import { getCloudflareContext } from "@opennextjs/cloudflare"; import { createLogger, createConsoleLogger, isLogsRPC } from "@/lib/logging"; diff --git a/app/api/inbox/[address]/route.ts b/app/api/inbox/[address]/route.ts index 1f309d28..2ce83edb 100644 --- a/app/api/inbox/[address]/route.ts +++ b/app/api/inbox/[address]/route.ts @@ -1,3 +1,7 @@ +// CACHE_INVARIANTS:POSTURE=public-only-get +// See lib/inbox/CACHE_INVARIANTS.md — GET handler is public; POST has its own +// auth for sender verification but does not cache, so Invariant 2 does not apply. + import { NextRequest, NextResponse } from "next/server"; import { getCloudflareContext } from "@opennextjs/cloudflare"; import { invalidateAgentListCache } from "@/lib/cache"; @@ -203,27 +207,13 @@ export async function GET( // the data; we are simply swapping the read source. // // This endpoint is FULLY PUBLIC — there is no auth gate on the inbox list GET. - // Cache-key invariants from #697 umbrella apply to any future auth'd branch: - // - // Invariant 1 (auth'd vs public branch separation): This endpoint currently - // has only a public branch. When an auth'd branch is added, its cache key - // MUST include a verified-address-hash suffix OR be excluded from any shared - // cache, so a public caller cannot receive an auth'd cached response. - // - // Invariant 2 (auth'd branch must set Cache-Control: private, no-store): - // The current public path does not set this header. Any future PR that adds - // an auth'd branch MUST add Cache-Control: private, no-store on that branch. - // This is not needed today because the public projection has no per-user data. // - // Invariant 3 (pre-gate cache safety): No cache lookup is performed before - // the auth gate because there is no auth gate yet. A future PR adding auth - // MUST ensure the auth gate runs before any cache lookup to prevent the - // agent-news#802 unauthenticated-HIT bug class (public caller receiving an - // auth'd cached response). This is safe by construction on this PR because - // there is no caching at all on this route today. + // Cache-key invariants: see lib/inbox/CACHE_INVARIANTS.md + // (auth'd-branch separation / private no-store / pre-gate cache safety) // - // See: https://github.com/aibtcdev/landing-page/issues/721 - // See: https://github.com/aibtcdev/landing-page/issues/697 + // See: https://github.com/aibtcdev/landing-page/issues/721 (Step 3.1 spec) + // See: https://github.com/aibtcdev/landing-page/issues/697 (Phase 2.5 umbrella) + // See: https://github.com/aibtcdev/landing-page/issues/723 (cache-invariant extraction) // This route only supports received messages for the inbox list GET. // The 'view' param is preserved for response-shape compatibility and diff --git a/app/api/outbox/[address]/route.ts b/app/api/outbox/[address]/route.ts index 93533ceb..fb91013b 100644 --- a/app/api/outbox/[address]/route.ts +++ b/app/api/outbox/[address]/route.ts @@ -1,3 +1,7 @@ +// CACHE_INVARIANTS:POSTURE=public-only-get +// See lib/inbox/CACHE_INVARIANTS.md — GET handler is public; POST (reply) uses +// verifyBitcoinSignature for caller auth on writes but does not cache. + import { NextRequest, NextResponse } from "next/server"; import { getCloudflareContext } from "@opennextjs/cloudflare"; import { createLogger, createConsoleLogger, isLogsRPC } from "@/lib/logging"; diff --git a/lib/inbox/CACHE_INVARIANTS.md b/lib/inbox/CACHE_INVARIANTS.md new file mode 100644 index 00000000..bd34e437 --- /dev/null +++ b/lib/inbox/CACHE_INVARIANTS.md @@ -0,0 +1,62 @@ +# Inbox / Outbox Cache-Key Invariants + +> Canonical source for the 3 cache-key invariants applied to `/api/inbox/*` and `/api/outbox/*` routes during the Phase 2.5 cutover series. Referenced by 1-line pointer comments in each route file + `lib/inbox/d1-reads.ts`. +> +> **History:** Surfaced via [#697 umbrella body](https://github.com/aibtcdev/landing-page/issues/697); first codified in-code on [#722 (Step 3.1)](https://github.com/aibtcdev/landing-page/pull/722); extracted to single source via [#723 (this doc)](https://github.com/aibtcdev/landing-page/issues/723) before duplication reached 4-route × 3-place = 12 instances. Steel-yeti Cycle 26 advisory framed the duplication risk; @secret-mars elevated; @whoabuddy filed the extraction issue. + +## Invariant 1 — Auth'd vs public branch separation + +Routes that have **both** a public branch (no auth required) **and** an auth'd branch (caller proves ownership of `[address]` via BIP-322 / SIP-018) MUST use one of: + +- **(a) Cache-key exclusion** — the auth'd branch is excluded from any shared cache layer (e.g., `Cache-Control: private, no-store` headers + skip the CF cache layer entirely). +- **(b) Verified-address-hash suffix** — the auth'd branch's cache key includes a hash of the verified caller address so a public caller cannot receive an auth'd cached response. + +The current state of routes flipped to D1 (#722) is **public-only**; both invariants are satisfied by construction. When an auth'd branch is added in a future PR, this invariant MUST be checked at PR-review time. + +## Invariant 2 — Auth'd branch must set `Cache-Control: private, no-store` + +If a route has an auth'd branch, that branch MUST set `Cache-Control: private, no-store` to prevent any intermediate cache (browser, CDN, reverse proxy) from storing the response. This complements Invariant 1's cache-key separation: even if the cache key is unique per verified caller, storing per-user data in a shared cache is unsafe — Invariant 1 says "don't cross-pollinate the cache key," Invariant 2 says "don't let intermediaries cache it at all." + +Routes that are currently public-only do not set this header; that is correct (public data on a unique URL has no per-user component to protect). Any future PR that **adds** an auth'd branch MUST add this header on the auth'd branch before merging. + +## Invariant 3 — Pre-gate cache safety + +Never serve a cache HIT **before** the BIP-322 / SIP-018 auth gate runs. + +This prevents the `agent-news#802` unauthenticated-HIT bug class: a public caller's first request populates a cache entry for an address; a subsequent (still public) caller for the same address receives a cached response that bypassed the auth gate, leaking content that should only be visible to the verified caller. + +Mitigation: any cache lookup MUST be gated by a successful auth verification first. The lookup order is: + +``` +1. parse request → extract address + auth signature +2. run BIP-322 / SIP-018 verification +3. if auth fails: serve public response (no cache lookup) OR 401 +4. if auth succeeds: now check the auth'd-branch cache +5. return cached HIT, or compute and cache +``` + +Step 4 must come **after** step 2, never before. Routes that are currently public-only have no auth gate, so step 2 is a no-op; this remains safe as long as the route stays public-only. + +## Compliance checklist for any route under `/api/inbox/*` or `/api/outbox/*` + +When adding or modifying these routes, the PR review should confirm: + +- [ ] **Invariant 1 check** — does this route have an auth'd branch? If yes, is the cache key separated from the public branch (excluded OR address-hash-suffixed)? +- [ ] **Invariant 2 check** — if there is an auth'd branch, does it set `Cache-Control: private, no-store`? +- [ ] **Invariant 3 check** — if there is any cache lookup, does the auth gate run before the lookup (not after)? + +For routes that are currently public-only (the post-#722 state of all inbox/outbox GETs), checks 1+2+3 are trivially satisfied. The structural enforcement test (`lib/inbox/__tests__/cache-invariants-enforcement.test.ts`) catches the most common violation shape: auth-related imports appearing in a route file without `Cache-Control: private` strings somewhere in the same file. + +## Cross-references + +- [agent-news#802](https://github.com/aibtcdev/agent-news/issues/802) — historical unauthenticated-HIT incident this invariant family prevents +- [aibtcdev/landing-page#697](https://github.com/aibtcdev/landing-page/issues/697) — Phase 2.5 umbrella where the invariants were ratified +- [aibtcdev/landing-page#722](https://github.com/aibtcdev/landing-page/pull/722) — first codified-in-code (route file inline + `lib/inbox/d1-reads.ts` header) +- [aibtcdev/landing-page#723](https://github.com/aibtcdev/landing-page/issues/723) — extraction to this single canonical source (this PR) +- [aibtcdev/landing-page#725](https://github.com/aibtcdev/landing-page/issues/725) — Step 3.2 spec; adopts 1-line pointer comments instead of inline block once #723 lands + +## When this doc changes + +- New route family added under `/api/inbox/*` or `/api/outbox/*` → add to the structural enforcement test's `INBOX_ROUTE_FILES` list. +- Auth gate mechanism changes (e.g., new signature scheme) → update Invariant 3's lookup order + the structural test's `AUTH_IMPORT_PATTERNS` list. +- Invariant 1/2/3 prose changes → update the matching content + re-run the test suite + cross-reference the changing PR. diff --git a/lib/inbox/__tests__/cache-invariants-enforcement.test.ts b/lib/inbox/__tests__/cache-invariants-enforcement.test.ts new file mode 100644 index 00000000..b0c872f1 --- /dev/null +++ b/lib/inbox/__tests__/cache-invariants-enforcement.test.ts @@ -0,0 +1,118 @@ +/** + * Structural enforcement of cache-key invariants from lib/inbox/CACHE_INVARIANTS.md. + * + * Catches the agent-news#802 unauthenticated-HIT bug class via lint rather than + * runtime, by requiring each route file under /api/inbox/* and /api/outbox/* to + * declare its cache-invariant posture via a magic comment marker, then asserting + * the declared posture is consistent with the file contents (Invariant 2). + * + * ## Why a posture marker instead of auth-import detection + * + * These route files mix multiple HTTP methods (GET / POST / PATCH) where POST + * and PATCH legitimately import `verifyBitcoinSignature` for sender auth on + * write paths, but the GET handler is public-only. A string-match test that + * fires on "any auth import + no Cache-Control: private" would false-positive + * on those mixed-handler files. The magic-comment marker lets each route file + * declare its GET-path posture explicitly, which is the load-bearing invariant + * (agent-news#802 was a GET-side cache HIT before auth, not a POST issue). + * + * ## Marker format + * + * Each enforced route file MUST include a single line of the form: + * + * // CACHE_INVARIANTS:POSTURE= + * + * where `` is one of: + * + * - `public-only-get` GET handler has no auth gate; Invariants 1+2+3 are + * satisfied trivially. POST/PATCH/DELETE may still + * use auth — that's fine; the agent-news#802 bug class + * only applies to GET-side cache HITs. + * - `auth-required-get` GET handler has an auth gate (BIP-322 / SIP-018 / + * session). MUST set `Cache-Control: private, no-store` + * somewhere in the file for Invariant 2 compliance. + * + * When a route's GET handler posture changes (e.g., adding auth to a previously + * public GET), update the marker AND add `Cache-Control: private, no-store` + * on the auth'd response paths in the same PR. + * + * See: lib/inbox/CACHE_INVARIANTS.md (Invariants 1, 2, 3) + * See: https://github.com/aibtcdev/landing-page/issues/723 (this enforcement) + * See: https://github.com/aibtcdev/agent-news/issues/802 (incident class) + */ + +import { describe, it, expect } from "vitest"; +import { readFileSync, existsSync } from "fs"; +import { join } from "path"; + +type Posture = "public-only-get" | "auth-required-get"; + +const POSTURE_PATTERN = /\/\/\s*CACHE_INVARIANTS:POSTURE=([a-z-]+)/; + +const CACHE_CONTROL_PRIVATE_PATTERNS: RegExp[] = [ + /Cache-Control['"\s:]+private/, + /['"]Cache-Control['"]\s*:\s*['"]private/, +]; + +/** + * Routes under the inbox/outbox surface that this test enforces against. + * Files that don't exist yet are skipped silently so the test passes during + * cutover (e.g., Step 3.x routes that may not have landed at test-run time). + */ +const INBOX_ROUTE_FILES: string[] = [ + "app/api/inbox/[address]/route.ts", + "app/api/inbox/[address]/[messageId]/route.ts", + "app/api/outbox/[address]/route.ts", +]; + +function extractPosture(content: string): Posture | null { + const match = content.match(POSTURE_PATTERN); + if (!match) return null; + const value = match[1]; + if (value === "public-only-get" || value === "auth-required-get") { + return value; + } + return null; +} + +function hasCacheControlPrivate(content: string): boolean { + return CACHE_CONTROL_PRIVATE_PATTERNS.some((p) => p.test(content)); +} + +describe("CACHE_INVARIANTS structural enforcement", () => { + for (const relPath of INBOX_ROUTE_FILES) { + describe(relPath, () => { + const fullPath = join(process.cwd(), relPath); + const fileExists = existsSync(fullPath); + const content = fileExists ? readFileSync(fullPath, "utf-8") : ""; + + it("must declare CACHE_INVARIANTS:POSTURE marker", () => { + if (!fileExists) return; + const posture = extractPosture(content); + expect( + posture, + `${relPath} is missing the CACHE_INVARIANTS:POSTURE marker. ` + + `Add a comment of the form: ` + + `\`// CACHE_INVARIANTS:POSTURE=public-only-get\` or ` + + `\`// CACHE_INVARIANTS:POSTURE=auth-required-get\`. ` + + `See lib/inbox/CACHE_INVARIANTS.md for the posture definitions.` + ).not.toBeNull(); + }); + + it("if posture=auth-required-get, must set Cache-Control: private (Invariant 2)", () => { + if (!fileExists) return; + const posture = extractPosture(content); + if (posture !== "auth-required-get") return; + expect( + hasCacheControlPrivate(content), + `${relPath} declares posture=auth-required-get but does not set ` + + `Cache-Control: private. Invariant 2 violation ` + + `(see lib/inbox/CACHE_INVARIANTS.md). Either add ` + + `\`Cache-Control: private, no-store\` on the auth'd response paths ` + + `OR change the posture marker to public-only-get if the GET handler ` + + `is in fact public.` + ).toBe(true); + }); + }); + } +}); diff --git a/lib/inbox/d1-reads.ts b/lib/inbox/d1-reads.ts index 2980ca46..5db58cb4 100644 --- a/lib/inbox/d1-reads.ts +++ b/lib/inbox/d1-reads.ts @@ -1,5 +1,5 @@ /** - * D1 read helpers for GET /api/inbox/[address]. + * D1 read helpers for GET /api/inbox/[address] and sibling routes. * * Phase 2.5 Step 3.1 — flip inbox-list GET from KV reads to D1 SELECTs. * KV writes are NOT removed here (that is Step 4). @@ -15,22 +15,12 @@ * computing the count via live SELECT COUNT(*) WHERE read_at IS NULL, * replacing the stale cached KV counter. * - * Cache-key invariants (from #697 umbrella, non-negotiable): - * 1. Auth'd vs public branch separation — no auth'd branch exists on this - * endpoint yet (the GET is fully public). When an auth'd branch is added - * in a future PR, its cache key MUST include a verified-address-hash suffix - * OR be excluded from any shared cache. This file documents that invariant - * so the constraint survives future diffs. - * 2. Auth'd branch responses MUST set Cache-Control: private, no-store. - * The current (public-only) path does not set this header; adding an - * auth'd branch without adding this header would violate invariant 2. - * 3. Pre-gate cache safety — never serve a cache HIT before the auth gate runs. - * Not currently at risk (no auth gate on this GET), but a follow-up PR - * adding auth MUST gate any cache lookup behind signature verification to - * avoid the agent-news#802 unauthenticated-HIT bug class. + * Cache-key invariants: see lib/inbox/CACHE_INVARIANTS.md + * (auth'd-branch separation / private no-store / pre-gate cache safety) * - * See: https://github.com/aibtcdev/landing-page/issues/721 - * See: https://github.com/aibtcdev/landing-page/issues/697 + * See: https://github.com/aibtcdev/landing-page/issues/721 (Step 3.1 spec) + * See: https://github.com/aibtcdev/landing-page/issues/697 (Phase 2.5 umbrella) + * See: https://github.com/aibtcdev/landing-page/issues/723 (cache-invariant extraction) */ import type { InboxMessage, OutboxReply } from "./types";