fix(security): stop /api/streets/activate handing out licence keys by email - #427
Merged
Merged
Conversation
… email `GET /api/streets/activate?email=<address>` returned that address's full Ed25519-signed licence key with no authentication of any kind. The `email` parameter was caller-supplied and never verified against anything, so a single unauthenticated request yielded a working licence for any customer whose email was known or guessed. Those keys are verified OFFLINE against the app's embedded public key, so a stolen one keeps working indefinitely with nothing to revoke it. The 200-vs-404 split was also a customer-list oracle. CORS was not a control — a plain HTTP client sends no Origin — and there is no middleware, no Turnstile and no state store (site/wrangler.toml declares zero KV/D1 bindings) to gate it. #421 deleted the stale Vercel duplicate of this handler but did not touch the live Cloudflare Pages one, which is the actual vulnerability. The fix keeps recovery working without ever putting the key on the wire to an unverified caller: - site/functions/api/streets/activate.js:356-470 — the GET handler now splits into two explicit paths. `?session_id=` is UNCHANGED (a checkout session id is unguessable and re-verified against Stripe, so it is proof of purchase). `?email=` never returns the key: it mails it to the address on file and answers a constant 202 whether or not the address matched. The response is sent BEFORE the Stripe lookup runs (scheduled on waitUntil) so neither the body nor the latency reveals whether an address is a customer. - site/lib/recovery-email.js — new: Resend delivery (same provider paddle-webhook already uses, raw fetch, no new dependency). Only ever mails an address that is already a Stripe customer holding a licence, so it cannot be used to mail arbitrary third parties. Expired licences get a "renew" notice, not a key. - Requires RESEND_API_KEY + RESEND_FROM_EMAIL in the Cloudflare Pages env. When they are absent the endpoint fails honestly with 503 + "contact support" rather than falling back to returning the key — that fallback IS the vulnerability. - json() now sets Cache-Control: no-store so nothing caches a key-bearing reply. Consumers updated to the new contract: - src-tauri/src/settings_commands_license.rs:326-443 — recover_license_by_email handles 202 (reason "emailed"), 400 and 503; it no longer auto-activates, because the server no longer gives it a key. - src/components/settings/LicenseSection.tsx, LicenseRecoveryBanner.tsx — "emailed" renders as an informational state, not an error. - src/locales/*/ui.json — 3 new keys across all 13 locales, recovery copy updated to say the key is emailed and never shown in-app. - site/src/signal/success.njk — the web form now says "Email me my key" and handles 202/503. - site/test-e2e-stripe.mjs — the assertions that encoded the vulnerable behaviour are now regression tests for the fix: no key in the body, and byte-identical responses for a known vs unknown address. - NETWORK.md — the documented behaviour of the only 4DA-operated endpoint. - site/src/privacy.njk, docs/legal/PRIVACY-POLICY.md — disclose Resend, the new processor this change introduces. (Stripe is still absent from both tables — a pre-existing gap, not introduced here.) Live-verified against `wrangler pages dev` with throwaway credentials: a known and an unknown address return byte-identical 202 bodies with no licence key, a malformed address returns 400 on shape alone, and the response carries Cache-Control: no-store. Also unblocks the file-size gate: #423 grew src-tauri/src/analysis_rerank.rs to 1032 lines (past the 1000 hard limit) without an exception entry, leaving main red and blocking every local commit. Added as an explicitly TEMPORARY entry that names #423 as owing the split — not a blessing to keep growing the file. Not fixed here: the email path is still unmetered. It can only mail existing customers (bounded to inbox nuisance, not open relay) and a proper limiter needs per-IP counters with no KV/D1 binding available — a Cloudflare WAF rate-limiting rule on the path is the zero-code mitigation. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AUeKTKwNmdow8yUk3q8RB2
runyourempire
enabled auto-merge (squash)
August 14, 2026 13:49
runyourempire
added a commit
that referenced
this pull request
Aug 15, 2026
…pply-chain blind spot (#433) ## The premise, verified first An audit lane claimed the quick-xml suppression in `deny.toml:96-112` / `.cargo/audit.toml:15-29` had gone stale. It rested on this justification: > "NO consumer in our tree has a released version against >=0.41 yet" **Confirmed false.** Read straight out of the registry index (`rust_version` and `deps` per published version): | consumer | we had | latest | quick-xml req | zip req | |---|---|---|---|---| | `calamine` | **0.25.0** | 0.36.1 | `^0.31` → **`^0.41`** | `^1.0` → `^8.6` | | `docx-rs` | **0.4.20** | 0.4.22 | `^0.36` → **`^0.41`** (since 0.4.21) | `^0.6.3` → `^8.6` | | `plist` | **1.9.0** | 1.10.0 | `^0.39.2` → **`^0.41`** | — | All three shipped support. The ignores were suppressing a live, fixable advisory pair on a parser that reads **user-supplied `.xlsx` / `.docx`**. ## What changed **quick-xml (RUSTSEC-2026-0194 / -0195) — resolved, not re-justified.** Bumping the three consumers collapses `quick-xml` **0.31.0 + 0.36.2 + 0.39.4 → a single 0.41.0**. Both advisories stop firing on their own, so both ignores are **deleted** from `deny.toml` *and* `.cargo/audit.toml` (they had diverged; both were checked). `RUSTSEC-2023-0071` (`rsa`) left alone as instructed. **`office.rs` needed no edit** — and that is a verified claim, not an absence of errors: - `sheet_names()` and `worksheet_range()` have byte-identical signatures in 0.25 and 0.36. - `Data` still has exactly the same nine variants with the same payloads. `cell_to_string` matches it **exhaustively with no wildcard arm**, so an added variant could not have compiled. - `ExcelDateTime`'s `Display` impl is byte-identical (`write!(f, "{}", self.value)`), so `DateTime` cells format the same. - Same for `docx-rs`: `TableChild` / `TableRowChild` are destructured irrefutably, so a new variant there could not have compiled either. The documented decompression-bomb weakness (the 100 MB cap is on the **compressed** size) is untouched — separate work, not regressed. **zip — partial.** `zip 1.1.4` retired as hoped. **`zip 0.6.6` did not** — it is our own direct `zip = "0.6"`, so retiring it is an 8-major API migration across `osv/cache.rs`, `extractors/archive.rs` and `embeddings_providers/fastembed.rs`. No advisory attaches to it, so it is staleness, not exposure. Left as follow-up rather than smuggled into a security PR. Tree is now `zip` 0.6.6 (ours) + 4.6.1 (tauri-plugin-updater) + 8.6.0 (calamine/docx-rs). **`relay/` — 5 vulnerabilities → 0.** A TLS-terminating server with no Dependabot entry, no cargo-audit, no CI. | crate | change | advisory | |---|---|---| | `rustls-webpki` | 0.103.9 → **0.103.14** | RUSTSEC-2026-0049 / -0098 / -0099 / -0104 (cert validation) | | `spin` | 0.9.8 → **0.9.9** | 0.9.8 was **yanked** | | `anyhow` | 1.0.102 → 1.0.104 | RUSTSEC-2026-0190 | | `event-listener` | 5.4.1 → 5.4.2 | RUSTSEC-2026-0221 | | `rand` | 0.8.5 → 0.8.7 | RUSTSEC-2026-0097 | `rsa 0.9.10` remains with no fix available, and is recorded in a new `relay/.cargo/audit.toml` with evidence that it is **not in the build graph**: it reaches `Cargo.lock` only via sqlx's optional `mysql` backend, which relay never enables — `cargo tree -i rsa` and `cargo tree -i sqlx-mysql` both report *nothing to print*. **Coverage, so it stops recurring.** `dependabot.yml` gains a `cargo` entry for `/relay` (not a `src-tauri` workspace member, so the existing entry never saw it), and `nightly-audit.yml`'s cargo-audit step now loops every `Cargo.lock` in the repo. **Workflow footprint is deliberately limited to those two files** — `validate.yml` is being reshaped by peer PRs and is untouched here. **`relay/Dockerfile`.** `cargo build --release --locked 2>/dev/null || cargo build --release` silently dropped lockfile enforcement and swallowed the reason. Fallback removed. Its base image also had to move **1.82 → 1.95**: the lockfile already required 1.88 via `time 0.3.47` (`jsonwebtoken` → `simple_asn1`), so that image could not have built this crate at all — the fallback was hiding a hard failure, not surviving a soft one. ## Two things found on the way **1. `main` was un-committable — independently confirmed, now fixed by #430.** `scripts/check-file-sizes.cjs` exits 1 on `src-tauri/src/analysis_rerank.rs` (**1032 lines against a 1000 hard limit**, arrived with #423). The gate scans the whole repo rather than staged paths, so `.husky/pre-commit` failed for *every* terminal on *every* commit — including this one. I hit it, diagnosed it, and fixed it the same way a peer did in **#430** (lift the test module into a sibling `analysis_rerank_tests.rs` via `#[path]`, 1032 → 866). #430 landed first, so **that commit has been dropped from this branch by rebase** — this PR now contains only the dependency work. Recording it here as an independent second confirmation of both the diagnosis and the chosen fix. **2. `cargo clippy --all-targets -- -D warnings` does not pass on `main`** (255 pre-existing errors at my branch point, ~all `unwrap_used`/`expect_used` in test code). This is **not** the gate — CI runs `cargo clippy ${{ matrix.cargo-features }} -- -D warnings` *without* `--all-targets`, so the numbers below are from the CI-equivalent invocation. Reported as an observation, not touched. ## Verification | check | result | |---|---| | `src-tauri` `cargo audit` | **exit 0** — zero vulnerabilities, zero warnings | | `src-tauri` `cargo deny check` | **exit 0** — `advisories ok, bans ok, licenses ok, sources ok` | | `relay` `cargo audit` | **exit 0** (was 5 vulns + 4 warnings + 1 yanked) | | `relay` `cargo check --locked --all-targets` | clean | | `cargo clippy -- -D warnings` (CI-equivalent, default) | **exit 0** | | `cargo clippy --features experimental -- -D warnings` | **exit 0** | | `cargo fmt --check` | **exit 0** | | `cargo test --lib` | **4300 passed, 0 failed, 10 ignored** | All re-run after rebasing onto `c1fd348c` (#425, #426, #427, #429, #430, #431 all landed mid-flight). `--features team-sync` and `--features enterprise` fail to compile — **pre-existing rot on `main`** (`chacha20poly1305::aead::OsRng` unresolved, then cascading `__cmd__*` macro failures), which is what #424 exists to repair. My lockfile diff touches no crypto crate. #424 is still open as of this push, and the CI clippy matrix on `main` still carries only the `default` and `experimental` legs — so the two legs verified above are exactly the gate. ### The extractor tests were `#[ignore]`d and had never run There are no `.xlsx`/`.docx` fixtures anywhere in the repo, so `test_real_docx_extraction` / `test_real_xlsx_extraction` were no-ops that returned early. To gain real confidence in an 11-minor-version parser bump I generated **real OOXML documents** — shared strings, an inline string, numeric and boolean cells, paragraphs and a table — confirmed both `#[ignore]`d tests pass against them, and separately asserted the extracted text matches the pre-bump formatting contract exactly: ``` === Sheet: Budget === Hello from 4DA Item | Cost Second paragraph Widget | 42 A1 | B1 Gadget | 3.50 | TRUE ``` That exercises every arm of `cell_to_string` that a document can reach (shared/inline string, integral float → `42`, fractional float → `3.50`, bool → `TRUE`) plus the docx paragraph and table paths. The scratch harness was deleted; **no test-file changes ship in this PR**. ## Deliberately left - **`zip 0.6.6`** — direct dep, 8-major API migration, no advisory. Follow-up. - **`office.rs` decompression bomb** — the 100 MB cap is on the compressed size. Out of scope, not regressed. - **`--all-targets` clippy backlog** — pre-existing, not the CI gate. - **`validate.yml`** — peer-owned right now, untouched on purpose. - **`--features team-sync` / `enterprise`** — pre-existing rot, #424's job. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
runyourempire
added a commit
that referenced
this pull request
Aug 17, 2026
…ce key Buying Signal minted a key into Stripe customer metadata and emailed nothing. The ONLY way a customer ever saw it was /signal/success rendering it — and that page polls the webhook 4 times over ~8 seconds before giving up. Close the tab inside that window, or hit a slow webhook, and the key existed but the buyer never had it. The advertised fallback was worse than absent. "Email me my key" on that page called an endpoint that answered 503, because RESEND_API_KEY/RESEND_FROM_EMAIL were never set on the Pages project. That 503 is CORRECT code — #427 closed a real vulnerability (the endpoint used to return any address's key to anyone who asked) and deliberately refuses to fall back to returning the key. But closing the hole replaced a working-if-unsafe recovery path with one that was never switched on, and nothing anywhere reported the difference. Verified against production: the project has STRIPE_SECRET_KEY, STRIPE_WEBHOOK_SECRET, LICENSE_PRIVATE_KEY_HEX, SITE_URL and the three price IDs — and neither RESEND_*. So the delivery model was: one page load, no durable copy, and a dead fallback. WHAT CHANGES 1. The key is emailed at purchase and at renewal (`deliverLicenseEmail`). The success page stops being the delivery mechanism and becomes a convenience. It NEVER throws. This runs off a paid Stripe webhook, and `generateAndStoreLicense` is not idempotent: a throw returns non-2xx, Stripe retries, and the retry MINTS A SECOND VALID KEY with a fresh expiry. A failed email must degrade to "use the page or recovery", never to "issue another entitlement". Awaited rather than backgrounded, because it cannot throw, so the log line is guaranteed written before we answer 200. An unprovisioned mailer logs at error level on every single sale rather than skipping silently. That is the entire lesson of the 503: correct behaviour nobody could see is indistinguishable from no behaviour at all. 2. Renewals say so. A renewal silently replaces the key the customer holds — the old one dies at its original expiry. Previously the first they learned of it was the app rejecting their key. 3. The email footer matches why it arrived. There was one shared footer reading "someone asked 4DA to recover the licence for this address", which on a purchase confirmation lands as an account-compromise warning at the happiest moment of the funnel. Purchase / renewal / recovery now each say the true thing, with matching subjects. 4. `streets_*` metadata keys become `signal_*`. STREETS was retired in June 2026. Every entitlement key still carried its name, so the LIVE licensing path read as a dead feature — and that is not hypothetical: this endpoint was believed retired for exactly that reason, and the 503 was read as expected behaviour of something switched off. Reads accept EITHER prefix, preferring `signal_`, so no customer record is orphaned and no migration is needed. Contained entirely to site/ — nothing in src-tauri/, src/ or mcp-4da-server/ ever read these keys. 5. `scripts/check-pages-secrets.cjs` — a gate for the class of bug, not the instance. A missing runtime variable is invisible to every test (not a code property) and to every CI job (cannot read a Pages secret). The only thing that can catch it is asking the live project, so that is what this does. It reads names only; Cloudflare never returns values. "Could not check" exits 2, never 0. Conflating unknown with fine is the same defect the script exists to catch. Run against production it currently exits 1 and names both missing RESEND_* variables with the fix command. Deliberately NOT in `validate`/CI: it needs a Cloudflare token, so in CI it would be permanently inconclusive — which would either fail every build or, worse, get read as a pass. It is `pnpm run ops:pages-secrets`. ALSO FIXED, UNRELATED BUT BLOCKING `check-retired-claims` was failing on main. #478's changelog quotes the retired claims verbatim to document what its republish removed — a legitimate historical quotation, which is exactly what the gate's `retired-ok:` escape hatch is for. It just never got the marker, and the violation spanned two lines so the one-line lookbehind could not cover it from above; reflowed onto one line with an inline marker (HTML comments do not render, so the changelog is unchanged to a reader). And the gate is now wired into `repo-guards`. `.ai/RULES.md` and `AGENTS.md` have been telling every agent this claim is "enforced by scripts/check-retired-claims.cjs" while nothing in CI invoked it — the claim of enforcement was itself unenforced, which is how it broke on main unnoticed. VERIFICATION 164 script tests pass, up from 141. All 23 new ones were confirmed to fail without their fix and pass with it: - neutering delivery to the old silent no-op -> 7 of 8 delivery tests red - the namespace tests pin the legacy fallback rather than assuming it: a LEGACY-only record still resolves, a legacy terminal status is still terminal (reading only signal_* would silently re-grant revoked access), the current prefix wins when both are present, and a legacy first-seen stamp survives the rename instead of being restamped. File-size gate clean. `check-retired-claims` clean. `node --check` clean on every changed JS file. YAML parses and the new step is inside `repo-guards`, the job with no path filter. STILL AN OPERATOR ACTION Setting RESEND_API_KEY + RESEND_FROM_EMAIL on the Pages project (the domain must be verified in Resend first). Until then this commit changes the failure from "silent, and the page is the only copy" to "loud in the logs of every sale, and `ops:pages-secrets` exits 1 naming it" — but no email is sent, because there is nothing to send it with. NOT DONE, DELIBERATELY Rate limiting on the recovery path, and event-id dedup on the mint path. Both need a KV namespace on the Pages project, which is infrastructure this commit cannot conjure — naming a binding that does not exist fails closed at request time on the live payment path. Dedup is the more valuable of the two: without it a duplicate `checkout.session.completed` or `invoice.paid` delivery mints a second valid key. Both remain as the dispatch comment already documents. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
runyourempire
added a commit
that referenced
this pull request
Aug 18, 2026
…ce key (#485) ## A browser tab was the only thing delivering a paid licence key Buying Signal minted a key into Stripe customer metadata and **emailed nothing**. The only way a customer ever saw it was `/signal/success` rendering it — and that page polls the webhook 4 times over ~8 seconds before giving up. Close the tab inside that window, or hit a slow webhook, and the key existed but the buyer never had it. The advertised fallback was worse than absent. The **"Email me my key"** button on that page called an endpoint answering **503**, because `RESEND_API_KEY` / `RESEND_FROM_EMAIL` were never set on the Pages project. Verified against production: it has `STRIPE_SECRET_KEY`, `STRIPE_WEBHOOK_SECRET`, `LICENSE_PRIVATE_KEY_HEX`, `SITE_URL` and the three price IDs — and neither `RESEND_*`. That 503 is *correct code*. #427 closed a real vulnerability — the endpoint used to return any address's key to anyone who asked — and deliberately refuses to fall back to returning the key. But closing the hole replaced a working-if-unsafe recovery path with one that was never switched on, and nothing reported the difference. ## What changes **1 · The key is emailed at purchase and renewal.** The success page becomes a convenience instead of the delivery mechanism. It **never throws**. This runs off a paid Stripe webhook and `generateAndStoreLicense` is not idempotent: a throw returns non-2xx, Stripe retries, and the retry **mints a second valid key** with a fresh expiry. A failed email must degrade to "use the page or recovery", never to "issue another entitlement". Awaited rather than backgrounded precisely *because* it cannot throw, so the log line is guaranteed written before we answer 200. An unprovisioned mailer logs at **error level on every single sale** rather than skipping silently. That is the whole lesson of the 503: correct behaviour nobody can see is indistinguishable from no behaviour at all. **2 · Renewals say so.** A renewal silently replaces the key the customer is holding — the old one dies at its original expiry. Previously the first they learned of it was the app rejecting their key. **3 · The footer matches why the mail arrived.** There was one shared footer reading *"someone asked 4DA to recover the licence for this address"*, which on a purchase confirmation lands as an account-compromise warning at the happiest moment of the funnel. Purchase / renewal / recovery now each say the true thing, with matching subjects. **4 · `streets_*` metadata keys become `signal_*`.** STREETS was retired in June 2026, but every entitlement key still carried its name — so the *live* licensing path read as a dead feature. That is not hypothetical: this endpoint was believed retired for exactly that reason, and its 503 was read as expected behaviour of something switched off. Reads accept **either** prefix, preferring `signal_`, so no customer record is orphaned and no migration is needed. Contained entirely to `site/` — nothing in `src-tauri/`, `src/` or `mcp-4da-server/` ever read these keys. **5 · `scripts/check-pages-secrets.cjs` — a gate for the class, not the instance.** A missing runtime variable is invisible to every test (it isn't a code property) and to every CI job (CI can't read a Pages secret). The only thing that can catch it is asking the live project, so that's what this does — names only; Cloudflare never returns values. *"Could not check" exits 2, never 0.* Conflating unknown with fine is the same defect the script exists to catch. Run against production it currently **exits 1** and names both missing variables with the fix command. Deliberately **not** in `validate`/CI — it needs a token, so there it would be permanently inconclusive, which would either fail every build or get read as a pass. It's `pnpm run ops:pages-secrets`. ## The delivery fix had this bug in it, twice over **A `Date` expiry would have sent no email at all.** `buildLicenseEmail` did `expiresAt.slice(0, 10)`, but the webhook path holds a `Date` — `generateAndStoreLicense` returns `new Date(...)` — while only the recovery path reads an ISO string out of Stripe metadata. `.slice()` on a `Date` throws, and the no-throw contract above would have caught that throw and degraded it to a logged `'error'`. So the headline path of this PR would have silently sent **nothing**, in exactly the failure mode the PR exists to end, and the tests written first would not have caught it because they all passed strings. `formatExpiry()` now normalises both shapes, and a junk expiry omits the line rather than costing the buyer their key. The first version of the maintenance-timestamp fix in this same batch had the mirror-image defect — `unwrap_or(0)` would have written "never ran" instead of failing to write — which is why both are called out rather than quietly corrected. ## The activate button did nothing, in the most-used mail client there is Real delivered mail proved it: the email linked straight to `4da://activate?key=...`, and **Gmail removes custom-scheme hrefs outright** — browser and mobile apps alike. The button rendered as a button and carried no href at all. Nothing was wrong with the app. The `4da` scheme is declared in `tauri.conf.json`, the plugin is initialised in `lib.rs`, `app_setup.rs` validates and emits `deep-link-activate`, and on a machine with 4DA installed the handler is registered correctly (`HKCU\Software\Classes\4da` → `fourda.exe "%1"`, `URL Protocol` set). The link simply never reached the OS. The email now points at **`https://4da.ai/activate`**, which no mail client sanitises, and that page performs the `4da://` handoff from an ordinary click on an ordinary web page — where the restriction does not apply, because it is a mail-client sanitiser and not a browser policy. The same desktop-handoff bridge Slack and Zoom use. Shipped in this PR so the button is never live against a 404. **The key travels in the fragment, not the query string.** A fragment is never transmitted, keeping the licence key out of Cloudflare's request logs and out of any Referer header. That mattered more than it first appears: `base.njk` injects PostHog into *every* page unconditionally, and a client-side script can read `location.hash` even though the server cannot — so `/activate` would have handed licence keys to a third-party analytics service. Added a `noAnalytics` front-matter flag and set it there. Verified both directions against the build: `activate.html` has **0** posthog references while index, privacy and success still have 14, 13 and 10. The page degrades honestly instead of looking broken: it does **not** fire the protocol navigation on load — an unprompted handoff on a machine without the app is exactly what makes these pages feel dead — and it always shows the key, a copy button, and the `Settings → License` instructions with a download link. A link carrying no key says so and names the likely cause. Links to `/activate`, not `/activate.html`: Pages 308-redirects the extension form to the clean URL, and a redirect is not somewhere to route a URL whose meaning lives in its fragment. ## Privacy policy: Stripe was not in it Checking whether the policy covered emailing at purchase (its Resend row scoped it to *"licence recovery only, when you ask for it"* — no longer true) surfaced that **Stripe appeared nowhere in `privacy.njk`**. It is the payment processor: it takes the buyer's email and card details, and the issued licence key and expiry are stored against the Stripe customer record — which is our **only** server-side copy, since the app verifies offline. Three more in the same document: - **PostHog** was absent from the third-party table, and §3.1 hedged that we *"may use privacy-respecting analytics ... if used"*. It is injected by `base.njk` on every page and `posthog.capture()` is called from `index.njk`. - The **CCPA table had two false rows**, not merely thin ones: identifiers *"Only via Store purchases"* and commercial information *"Only via Store"*. Signal checkout collects an email address and produces billing history. Added a financial-information row recording that card details reach Stripe/Shopify directly and never us. - **Retention covered App, Website and Store but not subscriptions.** Added §7.3, including the consequence a buyer needs before requesting deletion: that Stripe record is the only copy of their key we hold, so erasing it ends our ability to re-send it. **Keygen stays.** It looked stale beside offline verification, but `src-tauri/src/settings/license/keygen.rs` is live, so the row is correct. ## The sole required status check could pass a job that did not succeed Caught live on this PR, and it outweighs the delivery fix it turned up beside. `Validate Success` reported: ``` dependency results: success,success,success,success,abandoned,success ``` That `abandoned` is `Relay`. The gate was a **denylist** — it exited 1 only on `failure` or `cancelled` — so `abandoned` fell through to the success branch. Relay's conclusion was `failure`, the run's own conclusion was `failure`, and yet the one job configured as **the required status check** reported SUCCESS and the PR read `mergeable=MERGEABLE`. A human clicking merge would have landed a PR whose Relay leg never ran. GitHub emits more job results than the four that are documented, so a denylist here is unsound by construction: every value nobody thought of is a pass. Inverted to an **allowlist** — only `success` and `skipped` pass (`skipped` because the path-filtered legs legitimately skip when a change does not touch them) and everything else, known or not, blocks the merge. An empty results string now fails too, rather than being a gate that measured nothing and reported success. **Not introduced here.** The case statement dates to `bb9a843d` (#141); adding `relay` to `needs` is merely what made a job report a value outside the two it happened to check. It has been able to pass a non-succeeding job for as long as it has existed — the same failure shape as the stale-green NOTICE gate below, on the check that guards every merge. Verified against every result value: the live string above now fails naming `abandoned`, `success,skipped,success` still passes, and `failure` / `cancelled` / `timed_out` / `neutral` / empty all fail. Workflow YAML parses with the `needs` list unchanged. ## Also fixed, unrelated but blocking **`check-retired-claims` was failing on `main`.** #478's changelog quotes the retired claims verbatim to document what its republish removed — a legitimate historical quotation, which is exactly what the gate's `retired-ok:` escape hatch is for. It just never got the marker, and the violation spanned two lines so the one-line lookbehind couldn't cover it from above. Reflowed onto one line with an inline marker; HTML comments don't render, so the changelog is unchanged to a reader. **And the gate is now wired into `repo-guards`.** `.ai/RULES.md` and `AGENTS.md` have been telling every agent this is *"enforced by `scripts/check-retired-claims.cjs`"* while nothing in CI invoked it — the claim of enforcement was itself unenforced, which is how it broke on main unnoticed. **`NOTICE` was stale on `main`, and how it got there matters more than the diff.** `@tanstack/react-virtual` 3.14.5→3.14.9 and `@tanstack/virtual-core` 3.17.3→3.17.7 were missing. #460 added the NOTICE gate on 15 Aug; #450 merged on 17 Aug on a CI run **created before that gate existed**, passing in 13 seconds. `strict_required_status_checks_policy: false` permits a stale run to satisfy a required check, so the gate was never invoked and the next PR inherits both the breakage and the blame. Regenerated — but the class fix is enabling strict required status checks, which is an operator action on the ruleset. ## Verification **169 script tests pass, 0 fail** — up from 141 on `main`. All 28 new ones were confirmed to fail without their fix and pass with it: - neutering delivery back to the old silent no-op turns **7 of 8** delivery tests red - the two expiry tests fail against the original `.slice()` implementation - the three activation-link tests pin the regression rather than the implementation: the href must be https and must never be `4da://` again, the key must be in the fragment and never in the query string, and a key containing `+`, `/` and `=` must survive the round trip — a raw `+` would decode back as a space and hand the buyer a broken key - the namespace tests pin the legacy fallback rather than assuming it: a legacy-only record still resolves; a legacy *terminal* status is still terminal (reading only `signal_*` would silently re-grant revoked access); the current prefix wins when both are present; and a legacy first-seen stamp survives the rename instead of being restamped Site build clean — `privacy.html` renders at 41,384 bytes with Stripe, PostHog, the Signal retention section and the card-details disclosure all present, and both false CCPA claims gone. File-size gate clean · `check-retired-claims` clean · `node --check` clean on every changed JS file · YAML parses with the new step inside `repo-guards`, the job with no path filter. ## Still an operator action **Setting `RESEND_API_KEY` + `RESEND_FROM_EMAIL` on the Pages project.** The domain must be verified in Resend first, and `_dmarc.4da.ai` does not currently exist — worth adding in the same pass, since a licence key filtered to spam is indistinguishable to the buyer from not being sent. Current DNS is `v=spf1 include:spf.improvmx.com ~all` with MX on ImprovMX for inbound forwarding; Resend's free tier allows one verified domain. Until the variables are set, this PR changes the failure from *silent, with the page as the only copy* to *loud in the logs of every sale, with `ops:pages-secrets` exiting 1 and naming it* — but no email is sent, because there is nothing to send it with. **Enabling strict required status checks** on the ruleset, per the NOTICE finding above. ## Not done, deliberately Rate limiting on recovery, and **event-id dedup on the mint path**. Both need a KV namespace, and naming a binding that doesn't exist fails closed at request time on the live payment path. An earlier draft of this section claimed that namespace was infrastructure this PR could not conjure. That was not checked, and it is false: the available Cloudflare token carries `workers_kv (write)`, and `wrangler kv namespace list` returns `[]` — reachable, simply never created. The blocker is a missing decision, not a missing capability. It is still deliberately out of scope here, for a different and better reason: **dedup is a change to the live payment path and deserves its own tests and its own review**, not a late addition to a PR already in flight. It is also the more valuable of the two — without it a duplicate `checkout.session.completed` or `invoice.paid` delivery mints a second valid key, which is the same class of defect as the one this PR fixes. It should be the next piece of work, and there is now nothing infrastructural standing in the way. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01Fq96xWyPQjx2bCCzWtsnC9 --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Closes an unauthenticated licence-key disclosure in the live Signal licence endpoint, without breaking licence recovery for real customers.
The vulnerability
site/functions/api/streets/activate.jsserves two GET paths. Thesession_idpath is correct: a Stripe checkout session id is high-entropy, unguessable, and re-verified against Stripe, so holding one is proof of purchase.The
emailpath was not. It took the address straight off the query string, looked the customer up, and returned that customer's full licence key in the response body — no authentication, no proof the caller owned the address, nothing:Impact, in order of severity:
200vs404answered "is this person a paying 4DA subscriber?" for any address the caller cared to try.Originheader at all. There is no_middleware.js, no Turnstile, andsite/wrangler.tomldeclares zero KV/D1 bindings, so no rate limiter existed or could have existed without new infrastructure.The fix
The
emailpath now never puts the key on the wire to an unverified caller. It mails the key to the address on file and returns the same202either way.site/functions/api/streets/activate.js:356-470— the GET handler splits into two named functions with explicitly different trust properties.handleSessionLookupis byte-for-byte the old verified behaviour (untouched on purpose).handleEmailRecoveryvalidates the address shape, checks that outbound mail is provisioned, then responds before doing the Stripe lookup — the lookup and send are scheduled onwaitUntil. That makes the response constant in both body and latency, so the oracle is closed on every path rather than just the obvious one.site/lib/recovery-email.js(new) — Resend delivery via rawfetch, the same provider and patternpaddle-webhook/api/paddle.ts:363already uses; no new dependency. It only ever mails an address that is already a Stripe customer holding a licence, so the endpoint cannot be turned into an open relay against arbitrary third parties. Expired licences get a "renew at 4da.ai/signal" notice instead of a key. It never throws and never returns the key to the caller.RESEND_API_KEYandRESEND_FROM_EMAILin the Cloudflare Pages environment (see operator actions below). If they are unset, the endpoint returns503with "contact support@4da.ai from your purchase email" — uniformly, before any Stripe call. It does not fall back to returning the key; that fallback is the vulnerability.json()now setsCache-Control: no-storeso no browser, proxy or CDN retains a key-bearing response from thesession_idpath either.Consumers updated to the new contract
Legitimate recovery had three consumers and all three were carried across rather than broken:
src-tauri/src/settings_commands_license.rs:326-443—recover_license_by_emailnow handles202(reason: "emailed"),400and503. It no longer auto-activates, because the server no longer hands it a key; the doc comment explains why the 200 arm is now unreachable.src/components/settings/LicenseSection.tsx,src/components/LicenseRecoveryBanner.tsx—emailedrenders as an informational (gold) state, not a red error. It is the success case.src/locales/*/ui.json— 3 new keys across all 13 locales, plus updated recovery copy that says the key is emailed and is never shown in-app.site/src/signal/success.njk— the public form's button is now "Email me my key", with a note explaining why, and it handles202/503.Privacy disclosure
This change introduces Resend as a processor of a customer's email address and licence key, so it is added to the third-party tables in
site/src/privacy.njkanddocs/legal/PRIVACY-POLICY.md. #421 had already corrected the Vercel→Cloudflare drift in both, so that part is not re-done here.Type of change
Checklist
pnpm run lint— 0 errorstsc --noEmit— cleanpnpm run test— 1362 tests / 125 files passcargo fmt --checkandcargo clippy --lib— cleannode scripts/validate-translations.cjs— 0 errorsnode scripts/check-file-sizes.cjs— passes (see the note below about fix(pipeline): four silent production failures — dead sources, notification storm, lobotomised reranker, lying metrics #423)npx @11ty/eleventy— site buildsTesting
Live-verified against
wrangler pages devwith throwaway credentials (a fake Stripe key is sufficient — the whole point is that the response does not depend on the lookup):?email=a plausible address202, body{"delivery":"email","message":"If that address has a 4DA licence…"}?email=a different address202, byte-identical body?email=malformed400, rejected on shape alone400Cache-Control: no-store, nolicense_keyfield anywheresite/test-e2e-stripe.mjspreviously asserted the vulnerable behaviour (data.license_key === licenseKey). Those assertions are now regression tests for the fix, covering the two properties that matter:license_keyfield, in the live, cancelled and expired cases;That script needs live Stripe test keys, so it is for the operator to run against a preview deployment — it is not wired into CI.
Operator actions required before this is fully live
RESEND_API_KEYandRESEND_FROM_EMAIL(e.g.4DA <licenses@4da.ai>, on a Resend-verified domain) in Cloudflare Pages →4da-site→ Settings → Environment variables. Until they are set, email recovery honestly returns "contact support" instead of working.wrangler pages deploy.4da-homeis not still serving the old copy. Audit remediation: remove ~53k lines of dead code, fix legal/tier accuracy, close enforcement gaps #421 removedsite/api/**from the repo, but if that project still has a live deployment on a*.vercel.appURL withSTRIPE_SECRET_KEYandLICENSE_PRIVATE_KEY_HEXpopulated, the old vulnerable endpoint is still reachable there regardless of what the repo says.LICENSE_PRIVATE_KEY_HEXif there is reason to think keys were harvested while the endpoint was open. This invalidates every issued key and forces re-issue, so it is a judgement call, not automatic.Note for the #423 author — file-size gate was left red
src-tauri/src/analysis_rerank.rswent from 739 → 1032 lines in #423, past the 1000-line hard error inscripts/check-file-sizes.cjs, with no exception entry added. That leftmainred on the gate and blocked every local commit via the pre-commit hook. I added an explicitly TEMPORARY entry so this PR could be committed at all, with a comment naming #423 as owing the split and instructing that the line be deleted afterwards. It is not a blessing to keep growing the file — please split the reranker and remove the entry.Residual risk, deliberately not fixed here
/api/streets/activate, configurable from the dashboard. This is noted in a comment at the top of the email path.POSTwould fix that but would break already-shipped desktop builds that callGET, so the compatible shape was kept.🤖 Generated with Claude Code
https://claude.ai/code/session_01AUeKTKwNmdow8yUk3q8RB2