chore(api): enforce sebuf contract + migrate drifting endpoints (#3207)#3242
chore(api): enforce sebuf contract + migrate drifting endpoints (#3207)#3242SebastienMelki merged 26 commits intomainfrom
Conversation
Adds api/api-route-exceptions.json as the single source of truth for non-proto /api/ endpoints, with scripts/enforce-sebuf-api-contract.mjs gating every PR via npm run lint:api-contract. Fixes the root-only blind spot in the prior allowlist (tests/edge-functions.test.mjs), which only scanned top-level *.js files and missed nested paths and .ts endpoints — the gap that let api/supply-chain/v1/country-products.ts and friends drift under proto domain URL prefixes unchallenged. Checks both directions: every api/<domain>/v<N>/[rpc].ts must pair with a generated service_server.ts (so a deleted proto fails CI), and every generated service must have an HTTP gateway (no orphaned generated code). Manifest entries require category + reason + owner, with removal_issue mandatory for temporary categories (deferred, migration-pending) and forbidden for permanent ones. .github/CODEOWNERS pins the manifest to @SebastienMelki so new exceptions don't slip through review. The manifest only shrinks: migration-pending entries (19 today) will be removed as subsequent commits in this PR land each migration.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
…Snapshot (#3207) The proto VesselSnapshot was carrying density + disruptions but the frontend also needed sequence, relay status, and candidate_reports to drive the position-callback system. Those only lived on the raw relay passthrough, so the client had to keep hitting /api/ais-snapshot whenever callbacks were registered and fall back to the proto RPC only when the relay URL was gone. This commit pushes all three missing fields through the proto contract and collapses the dual-fetch-path into one proto client call. Proto changes (proto/worldmonitor/maritime/v1/): - VesselSnapshot gains sequence, status, candidate_reports. - GetVesselSnapshotRequest gains include_candidates (query: include_candidates). Handler (server/worldmonitor/maritime/v1/get-vessel-snapshot.ts): - Forwards include_candidates to ?candidates=... on the relay. - Separate 5-min in-memory caches for the candidates=on and candidates=off variants; they have very different payload sizes and should not share a slot. - Per-request in-flight dedup preserved per-variant. Frontend (src/services/maritime/index.ts): - fetchSnapshotPayload now calls MaritimeServiceClient.getVesselSnapshot directly with includeCandidates threaded through. The raw-relay path, SNAPSHOT_PROXY_URL, DIRECT_RAILWAY_SNAPSHOT_URL and LOCAL_SNAPSHOT_FALLBACK are gone — production already routed via Vercel, the "direct" branch only ever fired on localhost, and the proto gateway covers both. - New toLegacyCandidateReport helper mirrors toDensityZone/toDisruptionEvent. api/ais-snapshot.js deleted; manifest entry removed. Only reduced the codegen scope to worldmonitor.maritime.v1 (buf generate --path) — regenerating the full tree drops // @ts-nocheck from every client/server file and surfaces pre-existing type errors across 30+ unrelated services, which is not in scope for this PR. Shape-diff vs legacy payload: - disruptions / density: proto carries the same fields, just with the GeoCoordinates wrapper and enum strings (remapped client-side via existing toDisruptionEvent / toDensityZone helpers). - sequence, status.{connected,vessels,messages}: now populated from the proto response — was hardcoded to 0/false in the prior proto fallback. - candidateReports: same shape; optional numeric fields come through as 0 instead of undefined, which the legacy consumer already handled.
…ctionEntity (#3207) The proto docstring already claimed "OFAC + OpenSanctions" coverage but the handler only fuzzy-matched a local OFAC Redis index — narrower than the legacy /api/sanctions-entity-search, which proxied OpenSanctions live (the source advertised in docs/api-proxies.mdx). Deleting the legacy without expanding the handler would have been a silent coverage regression for external consumers. Handler changes (server/worldmonitor/sanctions/v1/lookup-entity.ts): - Primary path: live search against api.opensanctions.org/search/default with an 8s timeout and the same User-Agent the legacy edge fn used. - Fallback path: the existing OFAC local fuzzy match, kept intact for when OpenSanctions is unreachable / rate-limiting. - Response source field flips between 'opensanctions' (happy path) and 'ofac' (fallback) so clients can tell which index answered. - Query validation tightened: rejects q > 200 chars (matches legacy cap). Rate limiting: - Added /api/sanctions/v1/lookup-entity to ENDPOINT_RATE_POLICIES at 30/min per IP — matches the legacy createIpRateLimiter budget. The gateway already enforces per-endpoint policies via checkEndpointRateLimit. Docs: - docs/api-proxies.mdx — dropped the /api/sanctions-entity-search row (plus the orphaned /api/ais-snapshot row left over from the previous commit in this PR). - docs/panels/sanctions-pressure.mdx — points at the new RPC URL and describes the OpenSanctions-primary / OFAC-fallback semantics. api/sanctions-entity-search.js deleted; manifest entry removed.
…ts (#3207) Legacy /api/military-flights read a pre-baked Redis blob written by the seed-military-flights cron and returned flights in a flat app-friendly shape (lat/lon, lowercase enums, lastSeenMs). The proto RPC takes a bbox, fetches OpenSky live, classifies server-side, and returns nested GeoCoordinates + MILITARY_*_TYPE_* enum strings + lastSeenAt — same data, different contract. fetchFromRedis in src/services/military-flights.ts was doing nothing sebuf-aware. Renamed it to fetchViaProto and rewrote to: - Instantiate MilitaryServiceClient against getRpcBaseUrl(). - Iterate MILITARY_QUERY_REGIONS (PACIFIC + WESTERN) in parallel — same regions the desktop OpenSky path and the seed cron already use, so dashboard coverage tracks the analytic pipeline. - Dedup by hexCode across regions. - Map proto → app shape via new mapProtoFlight helper plus three reverse enum maps (AIRCRAFT_TYPE_REVERSE, OPERATOR_REVERSE, CONFIDENCE_REVERSE). The seed cron (scripts/seed-military-flights.mjs) stays put: it feeds regional-snapshot mobility, cross-source signals, correlation, and the health freshness check (api/health.js: 'military:flights:v1'). None of those read the legacy HTTP endpoint; they read the Redis key directly. The proto handler uses its own per-bbox cache keys under the same prefix, so dashboard traffic no longer races the seed cron's blob — the two paths diverge by a small refresh lag, which is acceptable. Docs: dropped the /api/military-flights row from docs/api-proxies.mdx. api/military-flights.js deleted; manifest entry removed. Shape-diff vs legacy: - f.location.{latitude,longitude} → f.lat, f.lon - f.aircraftType: MILITARY_AIRCRAFT_TYPE_TANKER → 'tanker' via reverse map - f.operator: MILITARY_OPERATOR_USAF → 'usaf' via reverse map - f.confidence: MILITARY_CONFIDENCE_LOW → 'low' via reverse map - f.lastSeenAt (number) → f.lastSeen (Date) - f.enrichment → f.enriched (with field renames) - Extra fields registration / aircraftModel / origin / destination / firstSeenAt now flow through where proto populates them.
…#3207) Caught by tsconfig.api.json typecheck in the pre-push hook (not covered by the plain tsc --noEmit run that ran before I pushed the ais-snapshot commit). The chokepoint status handler calls getVesselSnapshot internally with a static no-auth request — now required to include the new includeCandidates bool from the proto extension. Passing false: server-internal callers don't need per-vessel reports.
The ais-snapshot migration replaced the single cachedSnapshot/cacheTimestamp pair with a per-variant cache so candidates-on and candidates-off payloads don't evict each other. Pre-push hook surfaced that tests/server-handlers still asserted the old variable names. Rewriting the assertions to match the new shape while preserving the invariants they actually guard: - Freshness check against slot TTL. - Cache read before relay call. - Per-slot in-flight dedup. - Stale-serve on relay failure (result ?? slot.snapshot).
…3207) I ran 'buf generate --path worldmonitor/maritime/v1' to scope the proto regen to the one service I was changing (to avoid the toolchain drift that drops @ts-nocheck from 60+ unrelated files — separate issue). But the repo convention is the 'make generate' target, which runs buf and then sed-prepends '// @ts-nocheck' to every generated .ts file. My scoped command skipped the sed step. The proto-check CI enforces the sed output, so the two maritime files need the directive restored.
Commit 2 complete — 3 migrations landed. cc @koala73The original plan said these were "decomm-only" (proto + handler already exist, just delete the legacy). Checking the code, they weren't — shape/coverage mismatches would have silently broken behavior. Did real migrations instead, split across three commits for reviewability:
Plus three small followups:
Manifest state63 → 60 One question for youThe |
… edge fns (#3207) Both endpoints were already ported to IntelligenceService: - getCompanyEnrichment (/api/intelligence/v1/get-company-enrichment) - listCompanySignals (/api/intelligence/v1/list-company-signals) No frontend callers of the legacy /api/enrichment/* paths exist. Removes: - api/enrichment/company.js, signals.js, _domain.js - api-route-exceptions.json migration-pending entries (58 remain) - docs/api-proxies.mdx rows for /api/enrichment/{company,signals} - docs/architecture.mdx reference updated to the IntelligenceService RPCs Verified: typecheck, typecheck:api, lint:api-contract (89 files / 58 entries), lint:boundaries, tests/edge-functions.test.mjs (136 pass), tests/enrichment-caching.test.mjs (14 pass — still guards the intelligence/v1 handlers), make generate is zero-diff. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
@koala73 Commit 3 landed — Scope: decomm-only. Both
No frontend callers of the legacy paths existed, so the original "create new Deleted: Verified:
Next up: Commit 4 — |
…ice (#3207) New leads/v1 sebuf service with two POST RPCs: - SubmitContact → /api/leads/v1/submit-contact - RegisterInterest → /api/leads/v1/register-interest Handler logic ported 1:1 from api/contact.js + api/register-interest.js: - Turnstile verification (desktop sources bypass, preserved) - Honeypot (website field) silently accepts without upstream calls - Free-email-domain gate on SubmitContact (422 ApiError) - validateEmail (disposable/offensive/typo-TLD/MX) on RegisterInterest - Convex writes via ConvexHttpClient (contactMessages:submit, registerInterest:register) - Resend notification + confirmation emails (HTML templates unchanged) Shared helpers moved to server/_shared/: - turnstile.ts (getClientIp + verifyTurnstile) - email-validation.ts (disposable/offensive/MX checks) Rate limits preserved via ENDPOINT_RATE_POLICIES: - submit-contact: 3/hour per IP (was in-memory 3/hr) - register-interest: 5/hour per IP (was in-memory 5/hr; desktop sources previously capped at 2/hr via shared in-memory map — now 5/hr like everyone else, accepting the small regression in exchange for Upstash-backed global limiting) Callers updated: - pro-test/src/App.tsx contact form → new submit-contact path - src-tauri/sidecar/local-api-server.mjs cloud-fallback rewrites /api/register-interest → /api/leads/v1/register-interest when proxying; keeps local path for older desktop builds - src/services/runtime.ts isKeyFreeApiTarget allows both old and new paths through the WORLDMONITOR_API_KEY-optional gate Tests: - tests/contact-handler.test.mjs rewritten to call submitContact handler directly; asserts on ValidationError / ApiError - tests/email-validation.test.mjs + tests/turnstile.test.mjs point at the new server/_shared/ modules Deleted: api/contact.js, api/register-interest.js, api/_ip-rate-limit.js, api/_turnstile.js, api/_email-validation.js, api/_turnstile.test.mjs. Manifest entries removed (58 → 56). Docs updated (api-platform, api-commerce, usage-rate-limits). Verified: npm run typecheck + typecheck:api + lint:api-contract (88 files / 56 entries) + lint:boundaries pass; full test:data (5852 tests) passes; make generate is zero-diff. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Updates the enterprise contact form to POST to /api/leads/v1/submit-contact (old path /api/contact removed in the previous commit). Bundle is rebuilt from pro-test/src/App.tsx source change in 9ccd309. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
@koala73 Commit 4 landed — WhatNew
Handler logic ported 1:1 — Turnstile verification, honeypot, free-email-domain gate, Rate limitsPreserved via
One minor regression: the legacy handler capped desktop-source signups at 2/hr using the shared in-memory rate-limit map ( Callers updated
Tests
Shared helpers moved
Verified
Next upCommit 5 — |
There was a problem hiding this comment.
Why this PR?
Closes #3207. Lands the sebuf-contract guardrail (manifest + CI + CODEOWNERS) alongside 5 migrations (ais-snapshot, sanctions-entity-search, military-flights, enrichment/*, contact + register-interest). Manifest shrinks 63 → 56. The bidirectional walker in scripts/enforce-sebuf-api-contract.mjs closes the root-only-.js blind spot in tests/edge-functions.test.mjs that let country-products.ts drift under the proto prefix. Guardrail-first ordering is correct; migrations ship with self-contained shape-diff commits.
HIGH — must fix before merge
1. Sanctions endpoint rate-limit policy is dead code (path typo)
server/_shared/rate-limit.ts:83-90registers the 30/min preserved budget under key/api/sanctions/v1/lookup-entity.- Generated route from the proto is
/api/sanctions/v1/lookup-sanction-entity(src/generated/server/worldmonitor/sanctions/v1/service_server.ts:174-176). hasEndpointRatePolicy/getEndpointRatelimitare exact-stringpathnamelookups. No match → fall through to the generic global limiter (Ratelimit.slidingWindow(600, '60 s')at rate-limit.ts:14).
Net effect: live OpenSanctions proxy endpoint has 600/min per IP instead of the advertised 30/min. Materially weakens abuse protection on the one endpoint in this PR where it matters most (external upstream, unauthenticated search).
Fix: rename the key to /api/sanctions/v1/lookup-sanction-entity (or rename the proto RPC to LookupEntity so the path matches). Consider a cheap unit test that asserts every ENDPOINT_RATE_POLICIES key resolves to a registered route in the gateway's route table — would have caught this at review time.
2. military-flights dashboard loses the stale-seed fallback
- Legacy
api/military-flights.js:7-36cascadedmilitary:flights:v1→military:flights:stale:v1before returning null. - New
server/worldmonitor/military/v1/list-military-flights.ts:76-152goes straight to live OpenSky/relay with a 2-mincachedFetchJsonand returnsnullon miss. fetchViaProto+fetchMilitaryFlights(src/services/military-flights.ts:163-204, 517-520) treat empty as failure.
A relay or OpenSky hiccup now shows an empty map where the legacy code served stale seeded data. Fix: have the proto handler read military:flights:stale:v1 when the live fetch returns null (keeps the two-tier fallback the legacy had). Worth adding a MilitaryServiceImpl integration test that mocks relay failure and asserts the stale path is consulted.
3. military-flights hex lookup broken by casing mismatch
- Legacy seeder uppercased:
hexCode: icao24.toUpperCase()(mainsrc/services/military-flights.ts:193). getFlightByHexstill uppercases the input:f.hexCode === hexCode.toUpperCase()(src/services/military-flights.ts:547).- New proto handler preserves OpenSky's lowercase
icao24:id: icao24, hexCode: icao24(server/worldmonitor/military/v1/list-military-flights.ts:120/122). mapProtoFlightpass-through:hexCode: pf.hexCode(src/services/military-flights.ts:128).
After merge, getFlightByHex silently returns undefined for every call. Fix: either uppercase in the proto handler (hexCode: icao24.toUpperCase()) or normalize both sides in the client (mapProtoFlight uppercase + getFlightByHex keep .toUpperCase() of input). Pick one canonical case and document it in the proto comment.
4. register-interest desktop Turnstile-bypass now gets 5/hr instead of 2/hr
Legacy had a nested harder cap for source: 'desktop-settings':
if (isDesktopSource) {
const entry = rateLimiter.getEntry(ip);
if (entry && entry.count > 2) return jsonResponse({ error: 'Rate limit exceeded' }, 429, cors);
}The new handler relies on the shared 5/hr endpoint limit. Since source is attacker-controlled, anyone can send source: 'desktop-settings' to skip Turnstile and enjoy 5/hr per IP. PR body acknowledges this as "small regression" — flagging explicitly because the endpoint hits Convex + Resend per success. Either:
- Second Upstash limiter at 2/hr keyed
${ip}:desktopinside the handler whenisDesktopSource. - Require a desktop shared secret header before trusting
source(also fixes the pre-existing unsigned-sourceweakness).
MEDIUM
5. Turnstile missingSecretPolicy inconsistency
Preserved from legacy, but now that both handlers share server/_shared/turnstile.ts:
submit-contact.ts→'allow-in-development'(rejects in prod when secret missing)register-interest.ts→ default'allow'(silently passes in prod when secret missing)
Cleanest fix: flip the shared default to 'allow-in-development' so missing env = safe in dev, strict in prod for both. submit-contact's explicit override then becomes redundant.
6. Silent enum fallbacks in maritime client (src/services/maritime/index.ts:32-38)
type: DISRUPTION_TYPE_REVERSE[proto.type] || 'gap_spike',
severity: SEVERITY_REVERSE[proto.severity] || 'low',AIS_DISRUPTION_TYPE_UNSPECIFIED (or any future enum value) gets silently mislabeled as gap_spike / low. Handler doesn't produce UNSPECIFIED today, so inert — but the codebase has a history here (see classification-default-unknown skill). Map unknown → a distinct placeholder or filter the row.
LOW / NIT
- Copy drift:
server/worldmonitor/leads/v1/register-interest.tsemail template keeps435+ Sources. PR #3241 (merged yesterday) bumped marketing copy to500+. This file is being rewritten — natural spot to bump, or extract to a shared template constant. as anyon Convex mutation names:'contactMessages:submit' as any/'registerInterest:register' as anycarried over from legacy. Convex generates typedapi.*handles — follow-up issue.- Static-string test assertions in
tests/server-handlers.test.mjs: grep-on-source asserts pass if any rename happens to reuse the substring. Pre-existing pattern, flagging per thestatic-analysis-test-fragilityskill.
Also good to add
A tiny CI check asserting every ENDPOINT_RATE_POLICIES key is a real gateway route would catch future rename-drift like finding #1. One-liner against the same route table the gateway uses.
Test plan before merge
-
/api/sanctions/v1/lookup-sanction-entity?q=putin— verify 30/min limit actually fires on the 31st request (after finding 1 fix). -
/api/military/v1/list-military-flights— kill the relay, expect stale Redis data (after finding 2 fix), not an empty response. -
getFlightByHex('A1B2C3')vsgetFlightByHex('a1b2c3')against the live proto handler — expect hit on both (after finding 3 fix). -
/api/leads/v1/submit-contactwithemail: 'x@gmail.com'— expect 422 withmessagefield (pro-test readsdata.message || data.error). -
/api/leads/v1/register-interest6× from same IP — expect 429 on 6th. -
/api/leads/v1/register-interestwithsource: 'desktop-settings'6× — confirm behavior matches finding 4 decision. -
/api/maritime/v1/get-vessel-snapshot?include_candidates=truethen?include_candidates=false— confirm slots don't evict each other. - Old desktop builds hitting
/api/register-intereston sidecar → cloud fallback rewrites path correctly. -
npm run lint:api-contractwith bogusapi/test-drift.jsstill fails with the advertised remedy text.
Guardrail + shape of the PR is good. Three HIGH items above (1–3) are the gating ones; 4 is worth discussing; 5–6 can ride in follow-ups.
…3207 # Conflicts: # public/pro/assets/index-HdDr3e2z.js # public/pro/index.html
Three review findings from @koala73 on the sebuf-migration PR, all silent bugs that would have shipped to prod: ### 1. Sanctions rate-limit policy was dead code ENDPOINT_RATE_POLICIES keyed the 30/min budget under /api/sanctions/v1/lookup-entity, but the generated route (from the proto RPC LookupSanctionEntity) is /api/sanctions/v1/lookup-sanction-entity. hasEndpointRatePolicy / getEndpointRatelimit are exact-string pathname lookups, so the mismatch meant the endpoint fell through to the generic 600/min global limiter instead of the advertised 30/min. Net effect: the live OpenSanctions proxy endpoint (unauthenticated, external upstream) had 20x the intended rate budget. Fixed by renaming the policy key to match the generated route. ### 2. Lost stale-seed fallback on military-flights Legacy api/military-flights.js cascaded military:flights:v1 → military:flights:stale:v1 before returning empty. The new proto handler went straight to live OpenSky/relay and returned null on miss. Relay or OpenSky hiccup used to serve stale seeded data (24h TTL); under the new handler it showed an empty map. Both keys are still written by scripts/seed-military-flights.mjs on every run — fix just reads the stale key when the live fetch returns null, converts the seed's app-shape flights (flat lat/lon, lowercase enums, lastSeenMs) to the proto shape (nested GeoCoordinates, enum strings, lastSeenAt), and filters to the request bbox. Read via getRawJson (unprefixed) to match the seed cron's writes, which bypass the env-prefix system. ### 3. Hex-code casing mismatch broke getFlightByHex The seed cron writes hexCode: icao24.toUpperCase() (uppercase); src/services/military-flights.ts:getFlightByHex uppercases the lookup input: f.hexCode === hexCode.toUpperCase(). The new proto handler preserved OpenSky's lowercase icao24, and mapProtoFlight is a pass-through. getFlightByHex was silently returning undefined for every call after the migration. Fix: uppercase in the proto handler (live + stale paths), and document the invariant in a comment on MilitaryFlight.hex_code in military_flight.proto so future handlers don't re-break it. ### Verified - typecheck + typecheck:api clean - lint:api-contract (56 entries) / lint:boundaries clean - tests/edge-functions.test.mjs 130 pass - make generate zero-diff (openapi spec regenerated for proto comment) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
@koala73 HIGH #1-3 addressed in 1. Sanctions rate-limit key typoRenamed 2. military-flights stale fallback restoredWhen Shape conversion helper ( 3. Hex casing canonicalized
// ICAO 24-bit hex address. Canonical form is UPPERCASE — seeders and
// handlers must uppercase before writing so hex-based lookups
// (src/services/military-flights.ts:getFlightByHex) match regardless of
// upstream source casing.OpenAPI spec regenerated via Also fixed: Verified
Next upCommit B — HIGH #4 (desktop 2/hr cap). Going with your Option 1 (second Upstash limiter keyed Commit C after that — MEDIUM #5-6 (turnstile policy default, maritime enum fallbacks), LOW #copy-drift (435+ → 500+ sources), plus the suggested CI check that every |
|
@koala73 deferred items filed as follow-ups for after this merges:
#3279 is the one I'd prioritize — it mechanically prevents HIGH(new) #1 class and would have saved you a full-depth trace on this review. |
Merge blockers vs. follow-upsLanding the split here so we have one source of truth. I'm grouping the outstanding items by "needs to happen before this merges" vs "fine to do after", since there's a live follow-up queue on your side already. Must fix before merge1. Restore v1 path aliases on scenario + supply-chain (wire-contract break) Commit 6 and commit 7 renamed documented v1 URLs:
The legacy edge-function files are deleted. Commit 8 (shipping/v2) already proved the pattern for preserving partner URLs byte-for-byte ("Partner URLs Fix options, either works:
Prefer the alias files — co-located with the old shape, easy to grep, trivially deleted at v2. 2. Webhook tenant-isolation gap in commit 8
The sibling Simplest fix: reinstate Accept + document (not blocking)3. Scenario Commit Can defer (your follow-ups + a couple more)
Rationale for the splitBlockers are hard-to-reverse wire-contract breaks or security issues that external callers / other tenants hit on the first request after merge. Aliasing is trivial and keeps v1 a real v1. The tenant-isolation gap is low blast radius today but zero cost to close in the same PR. Everything in "defer" is either an already-accepted trade-off (202→200), a hardening follow-up that doesn't guard against a currently-live bug (#3279), or a minor drift that doesn't affect the migration's core claim. Once blockers (1) and (2) land, this is good to ship. |
…v2 (#3207) Koala flagged this as a merge blocker in PR #3242 review. server/worldmonitor/shipping/v2/{register-webhook,list-webhooks}.ts migrated without reinstating validateApiKey(req, { forceKey: true }), diverging from both the sibling api/v2/shipping/webhooks/[subscriberId] routes and the documented "X-WorldMonitor-Key required" contract in docs/api-shipping-v2.mdx. Attack surface: the gateway accepts Clerk bearer auth as a pro signal. A Clerk-authenticated pro user with no X-WorldMonitor-Key reaches the handler, callerFingerprint() falls back to 'anon', and every such caller collapses into a shared webhook:owner:anon:v1 bucket. The defense-in-depth ownerTag !== ownerHash check in list-webhooks.ts doesn't catch it because both sides equal 'anon' — every Clerk-session holder could enumerate / overwrite every other Clerk-session pro tenant's registered webhook URLs. Fix: reinstate validateApiKey(ctx.request, { forceKey: true }) at the top of each handler, throwing ApiError(401) when absent. Matches the sibling routes exactly and the published partner contract. Tests: - tests/shipping-v2-handler.test.mjs: two existing "non-PRO → 403" tests for register/list were using makeCtx() with no key, which now fails at the 401 layer first. Renamed to "no API key → 401 (tenant-isolation gate)" with a comment explaining the failure mode being tested. 18/18 pass. Verified: typecheck:api, lint:api-contract (no change), lint:boundaries, lint:rate-limit-policies, test:data (6005/6005). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ain (#3207) Koala flagged this as a merge blocker in PR #3242 review. Commits 6 + 7 of #3207 renamed five documented v1 URLs to the sebuf method-derived paths and deleted the legacy edge-function files: POST /api/scenario/v1/run → run-scenario GET /api/scenario/v1/status → get-scenario-status GET /api/scenario/v1/templates → list-scenario-templates GET /api/supply-chain/v1/country-products → get-country-products GET /api/supply-chain/v1/multi-sector-cost-shock → get-multi-sector-cost-shock server/router.ts is an exact static-match table (Map keyed on `METHOD PATH`), so any external caller — docs, partner scripts, grep-the- internet — hitting the old documented URL would 404 on first request after merge. Commit 8 (shipping/v2) preserved partner URLs byte-for- byte; the scenario + supply-chain renames missed that discipline. Fix: add five thin alias edge functions that rewrite the pathname to the canonical sebuf path and delegate to the domain [rpc].ts gateway via a new server/alias-rewrite.ts helper. Premium gating, rate limits, entitlement checks, and cache-tier lookups all fire on the canonical path — aliases are pure URL rewrites, not a duplicate handler pipeline. api/scenario/v1/{run,status,templates}.ts api/supply-chain/v1/{country-products,multi-sector-cost-shock}.ts Vite dev parity: file-based routing at api/ is a Vercel concern, so the dev middleware (vite.config.ts) gets a matching V1_ALIASES rewrite map before the router dispatch. Manifest: 5 new entries under `deferred` with removal_issue=#3282 (tracking their retirement at the next v1→v2 break). lint:api-contract stays green (89 files checked, 55 manifest entries validated). Docs: - docs/api-scenarios.mdx: migration callout at the top with the full old→new URL table and a link to the retirement issue. - CHANGELOG.md + docs/changelog.mdx: Changed entry documenting the rename + alias compat + the 202→200 shift (from commit 23c821a). Verified: typecheck:api, lint:api-contract, lint:rate-limit-policies, lint:boundaries, test:data (6005/6005). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
@koala73 — thanks for the split-triage, both blockers addressed. Blockers1. v1 path aliases on scenario + supply-chain — Took the alias-files route per your preference. Five thin wrappers:
Each rewrites the pathname to the canonical sebuf path and delegates to the domain Vite dev parity: Manifest: 5 new 2. Webhook tenant-isolation gap — Took your simplest-fix option: reinstated Tests: the two existing Docs
Deferred queue (your call on ordering)All acknowledged — my planned order once this lands:
Verified locally before push: Ready for another look when you get a chance 🙏 |
|
Both blockers verified green — ship it.
Next-PR checklist (priority-ordered)
Optional: a post-merge smoke on prod for the five alias URLs (old → 200 via rewrite, not 404) so we confirm Vercel's file-based routing picks up the new files on the first deploy — aliases that exist in the repo but aren't deployed would silently 404 exactly the way the old code did. Thanks for turning these fast — good pass on the fix commits. |
Adds scripts/enforce-premium-fetch.mjs — AST-walks src/, finds every
`new <ServiceClient>(...)` (variable decl OR `this.foo =` assignment),
tracks which methods each instance actually calls, and fails if any
called method targets a path in src/shared/premium-paths.ts
PREMIUM_RPC_PATHS without `{ fetch: premiumFetch }` on the constructor.
Per-call-site analysis (not class-level) keeps the trade/index.ts pattern
clean — publicClient with globalThis.fetch + premiumClient with
premiumFetch on the same TradeServiceClient class — since publicClient
never calls a premium method.
Wired into:
- npm run lint:premium-fetch
- .husky/pre-push (right after lint:rate-limit-policies)
- .github/workflows/lint-code.yml (right after lint:api-contract)
Found and fixed three latent instances of the HIGH(new) #1 class from
#3242 review (silent 401 → empty fallback for signed-in browser pros):
- src/services/correlation-engine/engine.ts — IntelligenceServiceClient
built with no fetch option called deductSituation. LLM-assessment overlay
on convergence cards never landed for browser pros without a WM key.
- src/services/economic/index.ts — EconomicServiceClient with
globalThis.fetch called getNationalDebt. National-debt panel rendered
empty for browser pros.
- src/services/sanctions-pressure.ts — SanctionsServiceClient with
globalThis.fetch called listSanctionsPressure. Sanctions-pressure panel
rendered empty for browser pros.
All three swap to premiumFetch (single shared client, mirrors the
supply-chain/index.ts justification — premiumFetch no-ops safely on
public methods, so the public methods on those clients keep working).
Verification:
- lint:premium-fetch clean (34 ServiceClient classes, 28 premium paths,
466 src/ files analyzed)
- Negative test: revert any of the three to globalThis.fetch → exit 1
with file:line and called-premium-method names
- typecheck + typecheck:api clean
- lint:api-contract / lint:rate-limit-policies / lint:boundaries clean
- tests/sanctions-pressure.test.mjs + premium-fetch.test.mts: 16/16 pass
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ranch (#3242 followup) Before: alert_threshold was a plain int32. proto3 scalar default is 0, so the handler couldn't distinguish "partner explicitly sent 0 (deliver every disruption)" from "partner omitted the field (apply legacy default 50)" — both arrived as 0 and got coerced to 50 by `> 0 ? : 50`. Silent intent-drop for any partner who wanted every alert. The subsequent `alertThreshold < 0` branch was also unreachable after that coercion. After: - Proto field is `optional int32 alert_threshold` — TS type becomes `alertThreshold?: number`, so omitted = undefined and explicit 0 stays 0. - Handler uses `req.alertThreshold ?? 50` — undefined → 50, any number passes through unchanged. - Dead `< 0 || > 100` runtime check removed; buf.validate `int32.gte = 0, int32.lte = 100` already enforces the range at the wire layer. Partner wire contract: identical for the omit-field and 1..100 cases. Only behavioural change is explicit 0 — previously impossible to request, now honored per proto3 optional semantics. Scoped `buf generate --path worldmonitor/shipping/v2` to avoid the full- regen `@ts-nocheck` drift Seb documented in the #3242 PR comments. Re-applied `@ts-nocheck` on the two regenerated files manually. Tests: - `alertThreshold 0 coerces to 50` flipped to `alertThreshold 0 preserved`. - New test: `alertThreshold omitted (undefined) applies legacy default 50`. - `rejects > 100` test removed — proto/wire validation handles it; direct handler calls intentionally bypass wire and the handler no longer carries a redundant runtime range check. Verified: 18/18 shipping-v2-handler tests pass, typecheck + typecheck:api clean, all 4 custom lints clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…inding contract (#3242 followup) #3242 followup checklist item 6 from @koala73 — sanity-check that the delivery worker honors the re-resolve-and-re-check contract that isBlockedCallbackUrl explicitly delegates to it. Audit finding: no delivery worker for shipping/v2 webhooks exists in this repo. Grep across the entire tree (excluding generated/dist) shows the only readers of webhook:sub:* records are the registration / inspection / rotate-secret handlers themselves. No code reads them and POSTs to the stored callbackUrl. The delivery worker is presumed to live in Railway (separate repo) or hasn't been built yet — neither is auditable from this repo. Refreshes the comment block at the top of webhook-shared.ts to: - explicitly state DNS rebinding is NOT mitigated at registration - spell out the four-step contract the delivery worker MUST follow (re-validate URL, dns.lookup, re-check resolved IP against patterns, fetch with resolved IP + Host header preserved) - flag the in-repo gap so anyone landing delivery code can't miss it Tracking the gap as #3288 — acceptance there is "delivery worker imports the patterns + helpers from webhook-shared.ts and applies the four steps before each send." Action moves to wherever the delivery worker actually lives (Railway likely). No code change. Tests + lints unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…nal-helper justification Carried in from #3248 merge as a band-aid (called out in #3242 review followup checklist item 7). The endpoint genuinely belongs in internal-helper — RELAY_SHARED_SECRET-bearer auth, cron-only caller, never reached by dashboards or partners. Same shape constraint as api/notify.ts. Replaces the apologetic "filed here to keep the lint green" framing with a proper structural justification: modeling it as a generated service would publish internal cron plumbing as user-facing API surface. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds scripts/enforce-premium-fetch.mjs — AST-walks src/, finds every
`new <ServiceClient>(...)` (variable decl OR `this.foo =` assignment),
tracks which methods each instance actually calls, and fails if any
called method targets a path in src/shared/premium-paths.ts
PREMIUM_RPC_PATHS without `{ fetch: premiumFetch }` on the constructor.
Per-call-site analysis (not class-level) keeps the trade/index.ts pattern
clean — publicClient with globalThis.fetch + premiumClient with
premiumFetch on the same TradeServiceClient class — since publicClient
never calls a premium method.
Wired into:
- npm run lint:premium-fetch
- .husky/pre-push (right after lint:rate-limit-policies)
- .github/workflows/lint-code.yml (right after lint:api-contract)
Found and fixed three latent instances of the HIGH(new) #1 class from
#3242 review (silent 401 → empty fallback for signed-in browser pros):
- src/services/correlation-engine/engine.ts — IntelligenceServiceClient
built with no fetch option called deductSituation. LLM-assessment overlay
on convergence cards never landed for browser pros without a WM key.
- src/services/economic/index.ts — EconomicServiceClient with
globalThis.fetch called getNationalDebt. National-debt panel rendered
empty for browser pros.
- src/services/sanctions-pressure.ts — SanctionsServiceClient with
globalThis.fetch called listSanctionsPressure. Sanctions-pressure panel
rendered empty for browser pros.
All three swap to premiumFetch (single shared client, mirrors the
supply-chain/index.ts justification — premiumFetch no-ops safely on
public methods, so the public methods on those clients keep working).
Verification:
- lint:premium-fetch clean (34 ServiceClient classes, 28 premium paths,
466 src/ files analyzed)
- Negative test: revert any of the three to globalThis.fetch → exit 1
with file:line and called-premium-method names
- typecheck + typecheck:api clean
- lint:api-contract / lint:rate-limit-policies / lint:boundaries clean
- tests/sanctions-pressure.test.mjs + premium-fetch.test.mts: 16/16 pass
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ranch (#3242 followup) Before: alert_threshold was a plain int32. proto3 scalar default is 0, so the handler couldn't distinguish "partner explicitly sent 0 (deliver every disruption)" from "partner omitted the field (apply legacy default 50)" — both arrived as 0 and got coerced to 50 by `> 0 ? : 50`. Silent intent-drop for any partner who wanted every alert. The subsequent `alertThreshold < 0` branch was also unreachable after that coercion. After: - Proto field is `optional int32 alert_threshold` — TS type becomes `alertThreshold?: number`, so omitted = undefined and explicit 0 stays 0. - Handler uses `req.alertThreshold ?? 50` — undefined → 50, any number passes through unchanged. - Dead `< 0 || > 100` runtime check removed; buf.validate `int32.gte = 0, int32.lte = 100` already enforces the range at the wire layer. Partner wire contract: identical for the omit-field and 1..100 cases. Only behavioural change is explicit 0 — previously impossible to request, now honored per proto3 optional semantics. Scoped `buf generate --path worldmonitor/shipping/v2` to avoid the full- regen `@ts-nocheck` drift Seb documented in the #3242 PR comments. Re-applied `@ts-nocheck` on the two regenerated files manually. Tests: - `alertThreshold 0 coerces to 50` flipped to `alertThreshold 0 preserved`. - New test: `alertThreshold omitted (undefined) applies legacy default 50`. - `rejects > 100` test removed — proto/wire validation handles it; direct handler calls intentionally bypass wire and the handler no longer carries a redundant runtime range check. Verified: 18/18 shipping-v2-handler tests pass, typecheck + typecheck:api clean, all 4 custom lints clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…inding contract (#3242 followup) #3242 followup checklist item 6 from @koala73 — sanity-check that the delivery worker honors the re-resolve-and-re-check contract that isBlockedCallbackUrl explicitly delegates to it. Audit finding: no delivery worker for shipping/v2 webhooks exists in this repo. Grep across the entire tree (excluding generated/dist) shows the only readers of webhook:sub:* records are the registration / inspection / rotate-secret handlers themselves. No code reads them and POSTs to the stored callbackUrl. The delivery worker is presumed to live in Railway (separate repo) or hasn't been built yet — neither is auditable from this repo. Refreshes the comment block at the top of webhook-shared.ts to: - explicitly state DNS rebinding is NOT mitigated at registration - spell out the four-step contract the delivery worker MUST follow (re-validate URL, dns.lookup, re-check resolved IP against patterns, fetch with resolved IP + Host header preserved) - flag the in-repo gap so anyone landing delivery code can't miss it Tracking the gap as #3288 — acceptance there is "delivery worker imports the patterns + helpers from webhook-shared.ts and applies the four steps before each send." Action moves to wherever the delivery worker actually lives (Railway likely). No code change. Tests + lints unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…nal-helper justification Carried in from #3248 merge as a band-aid (called out in #3242 review followup checklist item 7). The endpoint genuinely belongs in internal-helper — RELAY_SHARED_SECRET-bearer auth, cron-only caller, never reached by dashboards or partners. Same shape constraint as api/notify.ts. Replaces the apologetic "filed here to keep the lint green" framing with a proper structural justification: modeling it as a generated service would publish internal cron plumbing as user-facing API surface. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds scripts/enforce-premium-fetch.mjs — AST-walks src/, finds every
`new <ServiceClient>(...)` (variable decl OR `this.foo =` assignment),
tracks which methods each instance actually calls, and fails if any
called method targets a path in src/shared/premium-paths.ts
PREMIUM_RPC_PATHS without `{ fetch: premiumFetch }` on the constructor.
Per-call-site analysis (not class-level) keeps the trade/index.ts pattern
clean — publicClient with globalThis.fetch + premiumClient with
premiumFetch on the same TradeServiceClient class — since publicClient
never calls a premium method.
Wired into:
- npm run lint:premium-fetch
- .husky/pre-push (right after lint:rate-limit-policies)
- .github/workflows/lint-code.yml (right after lint:api-contract)
Found and fixed three latent instances of the HIGH(new) #1 class from
#3242 review (silent 401 → empty fallback for signed-in browser pros):
- src/services/correlation-engine/engine.ts — IntelligenceServiceClient
built with no fetch option called deductSituation. LLM-assessment overlay
on convergence cards never landed for browser pros without a WM key.
- src/services/economic/index.ts — EconomicServiceClient with
globalThis.fetch called getNationalDebt. National-debt panel rendered
empty for browser pros.
- src/services/sanctions-pressure.ts — SanctionsServiceClient with
globalThis.fetch called listSanctionsPressure. Sanctions-pressure panel
rendered empty for browser pros.
All three swap to premiumFetch (single shared client, mirrors the
supply-chain/index.ts justification — premiumFetch no-ops safely on
public methods, so the public methods on those clients keep working).
Verification:
- lint:premium-fetch clean (34 ServiceClient classes, 28 premium paths,
466 src/ files analyzed)
- Negative test: revert any of the three to globalThis.fetch → exit 1
with file:line and called-premium-method names
- typecheck + typecheck:api clean
- lint:api-contract / lint:rate-limit-policies / lint:boundaries clean
- tests/sanctions-pressure.test.mjs + premium-fetch.test.mts: 16/16 pass
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ranch (#3242 followup) Before: alert_threshold was a plain int32. proto3 scalar default is 0, so the handler couldn't distinguish "partner explicitly sent 0 (deliver every disruption)" from "partner omitted the field (apply legacy default 50)" — both arrived as 0 and got coerced to 50 by `> 0 ? : 50`. Silent intent-drop for any partner who wanted every alert. The subsequent `alertThreshold < 0` branch was also unreachable after that coercion. After: - Proto field is `optional int32 alert_threshold` — TS type becomes `alertThreshold?: number`, so omitted = undefined and explicit 0 stays 0. - Handler uses `req.alertThreshold ?? 50` — undefined → 50, any number passes through unchanged. - Dead `< 0 || > 100` runtime check removed; buf.validate `int32.gte = 0, int32.lte = 100` already enforces the range at the wire layer. Partner wire contract: identical for the omit-field and 1..100 cases. Only behavioural change is explicit 0 — previously impossible to request, now honored per proto3 optional semantics. Scoped `buf generate --path worldmonitor/shipping/v2` to avoid the full- regen `@ts-nocheck` drift Seb documented in the #3242 PR comments. Re-applied `@ts-nocheck` on the two regenerated files manually. Tests: - `alertThreshold 0 coerces to 50` flipped to `alertThreshold 0 preserved`. - New test: `alertThreshold omitted (undefined) applies legacy default 50`. - `rejects > 100` test removed — proto/wire validation handles it; direct handler calls intentionally bypass wire and the handler no longer carries a redundant runtime range check. Verified: 18/18 shipping-v2-handler tests pass, typecheck + typecheck:api clean, all 4 custom lints clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…inding contract (#3242 followup) #3242 followup checklist item 6 from @koala73 — sanity-check that the delivery worker honors the re-resolve-and-re-check contract that isBlockedCallbackUrl explicitly delegates to it. Audit finding: no delivery worker for shipping/v2 webhooks exists in this repo. Grep across the entire tree (excluding generated/dist) shows the only readers of webhook:sub:* records are the registration / inspection / rotate-secret handlers themselves. No code reads them and POSTs to the stored callbackUrl. The delivery worker is presumed to live in Railway (separate repo) or hasn't been built yet — neither is auditable from this repo. Refreshes the comment block at the top of webhook-shared.ts to: - explicitly state DNS rebinding is NOT mitigated at registration - spell out the four-step contract the delivery worker MUST follow (re-validate URL, dns.lookup, re-check resolved IP against patterns, fetch with resolved IP + Host header preserved) - flag the in-repo gap so anyone landing delivery code can't miss it Tracking the gap as #3288 — acceptance there is "delivery worker imports the patterns + helpers from webhook-shared.ts and applies the four steps before each send." Action moves to wherever the delivery worker actually lives (Railway likely). No code change. Tests + lints unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* chore(api-manifest): rewrite brief-why-matters reason as proper internal-helper justification Carried in from #3248 merge as a band-aid (called out in #3242 review followup checklist item 7). The endpoint genuinely belongs in internal-helper — RELAY_SHARED_SECRET-bearer auth, cron-only caller, never reached by dashboards or partners. Same shape constraint as api/notify.ts. Replaces the apologetic "filed here to keep the lint green" framing with a proper structural justification: modeling it as a generated service would publish internal cron plumbing as user-facing API surface. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(lint): premium-fetch parity check for ServiceClients (closes #3279) Adds scripts/enforce-premium-fetch.mjs — AST-walks src/, finds every `new <ServiceClient>(...)` (variable decl OR `this.foo =` assignment), tracks which methods each instance actually calls, and fails if any called method targets a path in src/shared/premium-paths.ts PREMIUM_RPC_PATHS without `{ fetch: premiumFetch }` on the constructor. Per-call-site analysis (not class-level) keeps the trade/index.ts pattern clean — publicClient with globalThis.fetch + premiumClient with premiumFetch on the same TradeServiceClient class — since publicClient never calls a premium method. Wired into: - npm run lint:premium-fetch - .husky/pre-push (right after lint:rate-limit-policies) - .github/workflows/lint-code.yml (right after lint:api-contract) Found and fixed three latent instances of the HIGH(new) #1 class from #3242 review (silent 401 → empty fallback for signed-in browser pros): - src/services/correlation-engine/engine.ts — IntelligenceServiceClient built with no fetch option called deductSituation. LLM-assessment overlay on convergence cards never landed for browser pros without a WM key. - src/services/economic/index.ts — EconomicServiceClient with globalThis.fetch called getNationalDebt. National-debt panel rendered empty for browser pros. - src/services/sanctions-pressure.ts — SanctionsServiceClient with globalThis.fetch called listSanctionsPressure. Sanctions-pressure panel rendered empty for browser pros. All three swap to premiumFetch (single shared client, mirrors the supply-chain/index.ts justification — premiumFetch no-ops safely on public methods, so the public methods on those clients keep working). Verification: - lint:premium-fetch clean (34 ServiceClient classes, 28 premium paths, 466 src/ files analyzed) - Negative test: revert any of the three to globalThis.fetch → exit 1 with file:line and called-premium-method names - typecheck + typecheck:api clean - lint:api-contract / lint:rate-limit-policies / lint:boundaries clean - tests/sanctions-pressure.test.mjs + premium-fetch.test.mts: 16/16 pass Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(military): fetchStaleFallback NEG_TTL=30s parity (closes #3277) The legacy /api/military-flights handler had NEG_TTL = 30_000ms — a short suppression window after a failed live + stale read so we don't Redis-hammer the stale key during sustained relay+seed outages. Carried into the sebuf list-military-flights handler: - Module-scoped `staleNegUntil` timestamp (per-isolate on Vercel Edge, which is fine — each warm isolate gets its own 30s suppression window). - Set whenever fetchStaleFallback returns null (key missing, parse fail, empty array after staleToProto filter, or thrown error). - Checked at the entry of fetchStaleFallback before doing the Redis read. - Test seam `_resetStaleNegativeCacheForTests()` exposed for unit tests. Test pinned in tests/redis-caching.test.mjs: drives a stale-empty cycle three times — first read hits Redis, second within window doesn't, after test-only reset it does again. Verified: 18/18 redis-caching tests pass, typecheck:api clean, lint:premium-fetch clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(lint): rate-limit-policies regex → import() (closes #3278) The previous lint regex-parsed ENDPOINT_RATE_POLICIES from the source file. That worked because the literal happens to fit a single line per key today, but a future reformat (multi-line key wrap, formatter swap, etc.) would silently break the lint without breaking the build — exactly the failure mode that's worse than no lint at all. Fix: - Export ENDPOINT_RATE_POLICIES from server/_shared/rate-limit.ts. - Convert scripts/enforce-rate-limit-policies.mjs to async + dynamic import() of the policy object directly. Same TS module that the gateway uses at runtime → no source-of-truth drift possible. - Run via tsx (already a dev dep, used by test:data) so the .mjs shebang can resolve a .ts import. - npm script swapped to `tsx scripts/...`. .husky/pre-push uses `npm run lint:rate-limit-policies` so no hook change needed. Verified: - Clean: 6 policies / 182 gateway routes. - Negative test (rename a key to the original sanctions typo /api/sanctions/v1/lookup-entity): exit 1 with the same incident- attributed remedy message as before. - Reformat test (split a single-line entry across multiple lines): still passes — the property is what's read, not the source layout. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(shipping/v2): alertThreshold: 0 preserved; drop dead validation branch (#3242 followup) Before: alert_threshold was a plain int32. proto3 scalar default is 0, so the handler couldn't distinguish "partner explicitly sent 0 (deliver every disruption)" from "partner omitted the field (apply legacy default 50)" — both arrived as 0 and got coerced to 50 by `> 0 ? : 50`. Silent intent-drop for any partner who wanted every alert. The subsequent `alertThreshold < 0` branch was also unreachable after that coercion. After: - Proto field is `optional int32 alert_threshold` — TS type becomes `alertThreshold?: number`, so omitted = undefined and explicit 0 stays 0. - Handler uses `req.alertThreshold ?? 50` — undefined → 50, any number passes through unchanged. - Dead `< 0 || > 100` runtime check removed; buf.validate `int32.gte = 0, int32.lte = 100` already enforces the range at the wire layer. Partner wire contract: identical for the omit-field and 1..100 cases. Only behavioural change is explicit 0 — previously impossible to request, now honored per proto3 optional semantics. Scoped `buf generate --path worldmonitor/shipping/v2` to avoid the full- regen `@ts-nocheck` drift Seb documented in the #3242 PR comments. Re-applied `@ts-nocheck` on the two regenerated files manually. Tests: - `alertThreshold 0 coerces to 50` flipped to `alertThreshold 0 preserved`. - New test: `alertThreshold omitted (undefined) applies legacy default 50`. - `rejects > 100` test removed — proto/wire validation handles it; direct handler calls intentionally bypass wire and the handler no longer carries a redundant runtime range check. Verified: 18/18 shipping-v2-handler tests pass, typecheck + typecheck:api clean, all 4 custom lints clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs(shipping/v2): document missing webhook delivery worker + DNS-rebinding contract (#3242 followup) #3242 followup checklist item 6 from @koala73 — sanity-check that the delivery worker honors the re-resolve-and-re-check contract that isBlockedCallbackUrl explicitly delegates to it. Audit finding: no delivery worker for shipping/v2 webhooks exists in this repo. Grep across the entire tree (excluding generated/dist) shows the only readers of webhook:sub:* records are the registration / inspection / rotate-secret handlers themselves. No code reads them and POSTs to the stored callbackUrl. The delivery worker is presumed to live in Railway (separate repo) or hasn't been built yet — neither is auditable from this repo. Refreshes the comment block at the top of webhook-shared.ts to: - explicitly state DNS rebinding is NOT mitigated at registration - spell out the four-step contract the delivery worker MUST follow (re-validate URL, dns.lookup, re-check resolved IP against patterns, fetch with resolved IP + Host header preserved) - flag the in-repo gap so anyone landing delivery code can't miss it Tracking the gap as #3288 — acceptance there is "delivery worker imports the patterns + helpers from webhook-shared.ts and applies the four steps before each send." Action moves to wherever the delivery worker actually lives (Railway likely). No code change. Tests + lints unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * ci(lint): add rate-limit-policies step (greptile P1 #3287) Pre-push hook ran lint:rate-limit-policies but the CI workflow did not, so fork PRs and --no-verify pushes bypassed the exact drift check the lint was added to enforce (closes #3278). Adding it right after lint:api-contract so it runs in the same context the lint was designed for. * refactor(lint): premium-fetch regex → import() + loop classRe (greptile P2 #3287) Two fragilities greptile flagged on enforce-premium-fetch.mjs: 1. loadPremiumPaths regex-parsed src/shared/premium-paths.ts with /'(\/api\/[^']+)'/g — same class of silent drift we just removed from enforce-rate-limit-policies in #3278. Reformatting the source Set (double quotes, spread, helper-computed entries) would drop paths from the lint while leaving the runtime untouched. Fix: flip the shebang to `#!/usr/bin/env -S npx tsx` and dynamic-import PREMIUM_RPC_PATHS directly, mirroring the rate-limit pattern. package.json lint:premium-fetch now invokes via tsx too so the npm-script path matches direct execution. 2. loadClientClassMap ran classRe.exec once, silently dropping every ServiceClient after the first if a file ever contained more than one. Current codegen emits one class per file so this was latent, but a template change would ship un-linted classes. Fix: collect every class-open match with matchAll, slice each class body with the next class's start as the boundary, and scan methods per-body so method-to-class binding stays correct even with multiple classes per file. Verification: - lint:premium-fetch clean (34 classes / 28 premium paths / 466 files — identical counts to pre-refactor, so no coverage regression). - Negative test: revert src/services/economic/index.ts to globalThis.fetch → exit 1 with file:line, bound var name, and premium method list (getNationalDebt). Restore → clean. - lint:rate-limit-policies still clean. * fix(shipping/v2): re-add alertThreshold handler range guard (greptile nit 1 #3287) Wire-layer buf.validate enforces 0..100, but direct handler invocation (internal jobs, test harnesses, future transports) bypasses it. Cheap invariant-at-the-boundary — rejects < 0 or > 100 with ValidationError before the record is stored. Tests: restored the rejects-out-of-range cases that were dropped when the branch was (correctly) deleted as dead code on the previous commit. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(lint): premium-fetch method-regex → TS AST (greptile nits 2+5 #3287) loadClientClassMap: The method regex `async (\w+)\s*\([^)]*\)\s*:\s*Promise<[^>]+>\s*\{\s*let path = "..."` assumed (a) no nested `)` in arg types, (b) no nested `>` in the return type, (c) `let path = "..."` as the literal first statement. Any codegen template shift would silently drop methods with the lint still passing clean — the same silent-drift class #3287 just closed on the premium-paths side. Now walks the service_client.ts AST, matches `export class *ServiceClient`, iterates `MethodDeclaration` members, and reads the first `let path: string = '...'` variable statement as a StringLiteral. Tolerant to any reformatting of arg/return types or method shape. findCalls scope-blindness: Added limitation comment — the walker matches `<varName>.<method>()` anywhere in the file without respecting scope. Two constructions in different function scopes sharing a var name merge their called-method sets. No current src/ file hits this; the lint errs cautiously (flags both instances). Keeping the walker simple until scope-aware binding is needed. webhook-shared.ts: Inlined issue reference (#3288) so the breadcrumb resolves without bouncing through an MDX that isn't in the diff. Verification: - lint:premium-fetch clean — 34 classes / 28 premium paths / 489 files. Pre-refactor: 34 / 28 / 466. Class + path counts identical; file bump is from the main-branch rebase, not the refactor. - Negative test: revert src/services/economic/index.ts premiumFetch → globalThis.fetch. Lint exits 1 at `src/services/economic/index.ts:64:7` with `premium method(s) called: getNationalDebt`. Restore → clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(lint): rate-limit OpenAPI regex → yaml parser (greptile nit 3 #3287) Input side (ENDPOINT_RATE_POLICIES) was flipped to live `import()` in 4e79d02. Output side (OpenAPI routes) still regex-scraped top-level `paths:` keys with `/^\s{4}(\/api\/[^\s:]+):/gm` — hard-coded 4-space indent. Any YAML formatter change (2-space indent, flow style, line folding) would silently drop routes and let policy-drift slip through — same silent-drift class the input-side fix closed. Now uses the `yaml` package (already a dep) to parse each .openapi.yaml and reads `doc.paths` directly. Verification: - Clean: 6 policies / 189 routes (was 182 — yaml parser picks up a handful the regex missed, closing a silent coverage gap). - Negative test: rename policy key back to /api/sanctions/v1/lookup-entity → exits 1 with the same incident-attributed remedy. Restore → clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore(codegen): regenerate unified OpenAPI bundle for alert_threshold proto change The shipping/v2 webhook alert_threshold field was flipped from `int32` to `optional int32` with an expanded doc comment in f333946. That comment now surfaces in the unified docs/api/worldmonitor.openapi.yaml bundle (introduced by #3341). Regenerated with sebuf v0.11.1 to pick it up. No behaviour change — bundle-only documentation drift. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Closes #3207.
Multi-commit PR — cc @koala73. Opening as draft so you can see the guardrail land on its own before the migration commits start flowing in.
Commit 1 (landed) — Guardrail scaffolding
api/api-route-exceptions.json— single source of truth for the 63 non-proto endpoints currently underapi/. Every entry carriescategory/reason/owner/removal_issue.scripts/enforce-sebuf-api-contract.mjs— walksapi/recursively (skips gitignored build artifacts likeapi/[domain]/v1/[rpc].js), classifies each file as either a sebuf gateway or a manifest entry, and checks both directions: orphaned gateways fail (proto was deleted), and generated services without a gateway fail (proto added but not wired).npm run lint:api-contractwired into.github/workflows/lint-code.ymlright afterlint:boundaries..github/CODEOWNERS(new) — pins the manifest and the enforcement script to @SebastienMelki so new exceptions don't slip through review..js-only legacy allowlist intests/edge-functions.test.mjs(the blind spot that letapi/supply-chain/v1/country-products.tsand friends drift under proto domain URL prefixes unchallenged).docs/adding-endpoints.mdx— documents the manifest and the CI gate.Commits 2–11 (queued, per
.planning/CHECKPOINT-sebuf-3207.md)The manifest only shrinks from here. Each subsequent commit removes its
migration-pendingentries:military-flights.js,sanctions-entity-search.js,ais-snapshot.js(proto + handler already exist).enrichment/*→enrichment/v1.contact.js+register-interest.js→leads/v1.eia/*+satellites.js→economic/v1+natural/v1.supply-chain/v1/country-products.ts+multi-sector-cost-shock.tsinto proto.scenario/v1/{run,status,templates}.ts→ sebuf gateway pattern.v2/shipping/*→shipping/v2.chat-analyst.ts+widget-agent.ts+skills/fetch-agentskills.ts→analyst/v1(blocked on sebuf#150 for SSE codegen).scripts/lint-boundaries.mjswith asrc/caller check.WM_API_FAIL_CLOSEDenv var).Test plan
npm run lint:api-contractclean (94 files checked, 63 manifest entries)npm run lint:boundariescleannpm run lint(biome) clean — warnings only, zero errorsnpm run lint:mdcleannpm run lint:unicodecleannpm run typecheckcleannpm run version:checkcleantests/edge-functions.test.mjsstill passes (145 tests)api/test-drift.js→ lint fails with remedy