Security audit fixes: WALLET_BLOCKLIST, uptimeSeconds, @x402 bump, WCAG contrast, /contact, buyer trend - #799
Draft
MikeyPetrillo wants to merge 25 commits into
Draft
Security audit fixes: WALLET_BLOCKLIST, uptimeSeconds, @x402 bump, WCAG contrast, /contact, buyer trend#799MikeyPetrillo wants to merge 25 commits into
MikeyPetrillo wants to merge 25 commits into
Conversation
…orand
Audited today: WALLET_BLOCKLIST (the enforcement behind /terms' "we may
refuse service to any wallet") only ever matched EVM payers. Traced the
actual installed SDKs (@x402/svm, @x402/stellar, @x402/avm) to confirm
why - none of those exact schemes carry a payer field on the raw client
payload (SVM/Stellar ship {transaction}, AVM ships
{paymentGroup,paymentIndex}), and @x402/core's settlePayment() builds the
beforeSettle hook's context straight from the caller's paymentPayload
argument, never enriched with the verify() result. So a blocked wallet
could trivially evade the ban by paying on any of those three rails
instead of an EVM chain.
Fixed the only way @x402/core's hook API allows: onAfterVerify DOES
receive the verify() result (which does correctly carry payer for all
three schemes, confirmed directly in the installed SDK source), but can't
itself abort settlement - a throw there is caught and only logged. So it
stashes the verified payer onto the SAME paymentPayload object instance
beforeSettle will see moments later in the same request (verifyPayment
and settlePayment both build their context from the exact object
reference the caller passed in, never a clone, confirmed by reading
@x402/core's server/index.mjs directly - no external cache/keying
needed). Guarded to never touch an EVM payload, since authorization.from
is already signature-covered and shouldn't be traded for an
unauthenticated facilitator-reported value.
The existing unit test's non-EVM cases used a fabricated
{ payload: { payer } } shape that no real SDK ever produces - exactly
what let this ship unnoticed. Rewrote with real wire shapes plus direct
coverage of the new enrichment hook (stashes correctly, never touches
EVM, no-op when verify carries no payer, full end-to-end chain).
Mutation-tested twice (removing the new candidate field, loosening the
EVM guard) - both caught correctly by the rewritten suite, then restored.
Verified: local boot clean (no errors), test-supported-guard.js (16/16,
the closest adjacent suite touching the same server-setup flow) unaffected.
Audit finding: /api/reliability and /api/stats exposed uptimeSeconds (resets to 0 on every deploy) directly beside servingSince (a real, ~2-month figure). /api/reliability is explicitly framed as "every claim an agent might want before depending on this seller" - a naive agent parsing field names alone would read uptimeSeconds as service-availability uptime and derive ~0.02% against a real 99.8-100%. Renamed the field everywhere it's produced or consumed: both getStats() occurrences in src/stats.js (public /api/stats and the operator-only breakdown), src/discovery.js's /api/reliability projection, and src/operator.js's dashboard display (labelled "since process boot" there, so no ambiguity in that UI - renamed anyway for consistency across the codebase). Updated the three envelope tests that asserted the old key name (test-discovery.js, test-reliability-envelope.js, test-stats-envelope.js) and added an explicit negative assertion in the latter two locking the old name's absence, so a regression can't silently reintroduce it. Mutation-tested both negative assertions against the actual code path each test exercises (stats.js's public getStats() for the /api/stats lock; discovery.js's field-picking in reliabilityReport() for the /api/reliability lock, since that function explicitly picks fields rather than spreading its input - a stray key on the input object alone doesn't prove anything about its own output shape) - both caught correctly, then restored. Verified live against a fresh local boot: both /api/reliability and /api/stats now carry processUptimeSeconds and no longer carry uptimeSeconds at all.
… bypass Audit flagged @x402/express as 6 minor versions behind npm latest with no review of what changed. Read every CHANGELOG between 2.16 and 2.22 (express + core, the two packages with the actual settlement/routing logic) rather than just bumping blind. Found this is not just hygiene: @x402/core 2.21.0 (commit 5192e50) fixed a REAL, currently-exploitable payment bypass - the compiled wildcard-route regex used `.*?` without the dotAll flag, so a percent-encoded ECMAScript line terminator (U+2028, LF, CR) in the wildcard-matched segment would fail to match, making requiresPayment() return false and skipping payment verification and settlement entirely. This is NOT theoretical for us: server.js registers /api/convert/* and /api/convert-* as wildcard routes (~970 legacy pairwise-converter compatibility paths), and we were pinned at 2.16.0 - before the fix. (2.22.0 also fixed a second, unrelated paywall bypass via backslash in :param/[param] segments - checked, we register no such routes, so that one didn't apply to us.) Bumped the WHOLE @x402 family together (express/core/evm/fetch/svm/ stellar/avm, all now 2.22.0) rather than express alone - the existing baseline already carried some cross-package skew (stellar@2.21, avm@2.18 vs the rest@2.16), and bumping only express would have made that worse, not better. This is exactly the "coordinated bump, not piecemeal" a prior review already flagged for this same package family. Verified the core settlement-ordering safety property (handler runs FIRST, settle only after, a >=400 cancels settlement) is architecturally unchanged at 2.22.0 by reading @x402/express's actual request-lifecycle code directly - same buffered-response/cancel-on->=400 shape as before. A new `beforeHandlerSettlement` concept exists in 2.22.0 but is dead code for every scheme we register: read @x402/stellar's and @x402/avm's own scheme source directly and confirmed both declare `paymentFlows: { default: "authorization" }` only, matching the changelog's own claim that all shipped schemes currently declare authorization-only flow. New test-wildcard-route-bypass.js proves the fix against our REAL /api/convert-* route (not a synthetic example): an ordinary request still requires payment (402), and four percent-encoded line-terminator payloads (U+2028, LF, CR, U+2029) all still 402 rather than reaching the handler for free. Mutation-tested by directly patching the installed vendor regex flag back to the pre-fix shape in node_modules - all four payloads then either free-200'd or fell through to a 404 that skipped the payment gate (confirmed via the exactly-402 assertion, which is why the test checks for exactly 402, not merely "not 200" - this route's own strict downstream unit-pair parser incidentally also rejects the mangled path, which would have hidden a real regression behind a coincidental second line of defense if the assertion were weaker). Restored the vendor file after confirming the test passes clean again. Full regression sweep: test-idempotency-settlement.js (8/8), test-head-paywall.js (8/8), test-refund-ledger.js (70/70), test-settle-fallback.js (all), test-rail-selfheal.js (25/25), test-self-funding.js (26/26), test-price-premium.js (all), test-payment-identity-rails.js (24/24), test-rails.js (189/189, live local boot), test-x402-kit.js (all live checks pass), clean local server boot with no errors, npm audit: 0 vulnerabilities.
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
Audit finding: --faint (#6C6C68), used at 10-13px throughout shared nav/footer chrome (every page inherits ledger-chrome.js's :root) and in compare.js/sell.js/status.js via the same shared token, measured 3.15-3.66:1 against the dark surfaces it's actually composited on (--paper/--card/--card-zebra/--footer-bg) - normal text needs 4.5:1 under WCAG AA, and none of this text is large/bold enough to qualify for the relaxed 3:1 threshold. Raised to #8B8B87: clears 4.5:1 with margin (4.86-5.64:1) against every one of those four backgrounds, keeps the original color's subtle warm tint (R=G, B slightly lower), and stays visually distinct from --muted (the "one level up" token) rather than collapsing the two into the same shade. Single fix at the :root definition - grepped every other reference across the codebase and confirmed all of them use var(--faint), never a hardcoded hex, so this propagates everywhere with no other file needing changes. New test-faint-contrast.js computes real WCAG relative-luminance contrast directly from the token values in source (not a hardcoded string comparison), locking all four background pairings plus a sanity check that --faint stays distinct from --muted. Mutation-tested by reverting to the original color - correctly fails all 4 contrast assertions with the exact measured ratios, then restored and re-verified clean. Verified: test-theme.js (23/23) and test-market-pages.js (219/219) unaffected, live local boot confirms the new hex renders in the served page.
Audit finding: #ctSent (.ct-sent{display:none}) was fully unreachable
dead code - the form uses a native mailto:/enctype=text/plain submission,
which navigates to the visitor's mail client (or does nothing visible if
none is configured) rather than ever running JS to reveal the "sent"
div. No script anywhere on the page ever touched it.
Standing up a real backend submit handler (fetch to a new endpoint, or a
third-party form service) is a bigger decision - which service, where
submissions land, spam/abuse handling - that needs Mike's input rather
than a unilateral pick, so this ships the audit's other suggested
minimum: removed the dead markup entirely (leaving it in was actively
misleading - a future reader would assume there's a working success
flow), reworded the form's own copy to set the right expectation up
front ("Opens in your email app, pre-filled and ready to send"), and
added an explicit fallback line with a plain mailto: link for visitors on
a device with no configured mail client, so there's always a working
path to reach the address even when the native mailto handoff silently
does nothing.
Verified: grepped for any other reference to ctSent/ct-sent (none),
clean removal. Live local boot confirms the dead markup is gone and the
new fallback copy renders. test-link-integrity.js (6/6, 712 sitemap URLs)
and test-reveal-coverage.js (28/28, /contact's section count unaffected -
ctSent was an inner div, not a section) both clean.
…tric Audit finding: distinct daily buyers fell 45% over the last 60 days (13.8/day -> 7.6/day), independent of the known Solana whale going silent - a real, structural decline, but nothing on /revenue surfaced it directly. A viewer had to eyeball the chart and do the math themselves. Added buyersTrend(): a rolling 14-day recent-vs-prior comparison (not a lifetime first-half/second-half split, which would dilute toward flat as more historical days accumulate and stop being timely). Needs 28 days of real data before saying anything - thin history omits the line entirely rather than asserting a trend from a handful of points, matching this codebase's "unavailable, never fabricated" discipline. Shows up as an extra sentence on the existing Buyers-metric note: "Last 14 days averaged N distinct buyers/day, down/up X% from the 14 days before that." New test-buyers-trend.js extracts the actual function source from revenue-live.js and executes it for real against fixture data (not a regex/string check, since this is genuine arithmetic) - 14 assertions covering: insufficient history (<28 days, null/empty/missing buyers field, none of which throw), the exact 28-day boundary, a real decline matching the audit's own measured shape, growth, flat, a division-by-zero guard on an all-zero prior period, that rows are sorted by day rather than trusting input order, and that only the trailing 28 days feed the comparison regardless of how much older history exists. Mutation-tested twice (the length guard, the zero-guard) - both caught correctly, then restored. Live-verified in a real browser (Playwright, with the API response intercepted and replaced by a realistic 40-day declining series): clicked into the Buyers metric and confirmed the exact expected sentence renders with zero console/page errors: "Last 14 days averaged 7.0 distinct buyers/day, down 50% from the 14 days before that (14.0/day)." test-revenue-chart.js and test-revenue-buyers.js both unaffected.
MikeyPetrillo
changed the title
…syntax Regression in my own prior commit (d20bc4f, the --faint WCAG contrast fix, already deployed to production): the fix's explanatory comment used JS-style `//` line comments INSIDE a CSS :root {} block. `//` is not valid CSS comment syntax (CSS requires /* */) - the browser's parser silently dropped every declaration from that comment through to the next recovery point, taking --faint itself down with it, along with --green, --hairline, --dash, --on-dark, --on-dark2, --dk-muted/2/3, and --surface, all of which sit later in the same :root block. Caught while live-verifying the NEXT fix in this session (adding success/failure color branching to /sell's registration form, which uses --green) - the success color rendered as black instead of green in a real Playwright run, traced back through getComputedStyle showing --green resolving to an empty string sitewide despite the correct hex being right there in the served HTML's source text. This is exactly why "the string is in the HTML" is not proof a CSS declaration actually parsed - only a real browser evaluating real CSS can tell you that. Fixed by converting the comment to proper /* */ syntax. New test-css-tokens-resolve.js extracts every custom property name from the :root block and verifies each one resolves to a non-empty value via a REAL Playwright-driven browser (not static regex on the source text, which the existing test-faint-contrast.js already did and could never have caught this - the broken declaration's text is identical whether or not the parser actually accepted it). Mutation-tested against the exact bug shape (reinserted a `//` comment immediately before --faint) - caught correctly, then restored and re-verified clean. Confirmed the fix restores all previously-broken tokens across multiple pages (/, /sell, /marketplace, /tools, /status) and that --font-body/--font-mono (the LAST declarations in the block) resolve too, proving the parser now reaches the end of the block cleanly.
Audit finding: the seller-registration form (id="list-api", duplicated in both src/sell.js's /sell page and src/market-page.js's per-chain pages) had three gaps - the origin input had no accessible label (placeholder text only), the status line had no aria-live so screen readers never announced the probe/success/failure state, and success/failure rendered in visually identical muted grey text with no way to tell them apart at a glance. Fixed both copies identically: added a proper <label for="reg-origin">, role="status" aria-live="polite" on #reg-out so assistive tech announces state changes automatically, and outcome-based color branching (green on listed, accent-red on not-listed/failed) using the SAME --green/ --accent-lit tokens already used elsewhere on these pages. Also guarded against double-submission (the button now disables for the duration of the fetch and re-enables in a finally block, whether it succeeds, fails, or throws). Live-verified in a real browser (Playwright): label is correctly associated, aria-live is present, a mocked SUCCESS response renders in green (rgb(62,155,110), matching --green's hex) with the button re-enabled after, a mocked FAILURE response renders in a visibly different red, and both the /sell and per-chain (/base) copies of the markup carry the same fixes. test-market-pages.js (219/219) and test-sell-page.js (25/25) both unaffected.
Audit finding: /tools catalog search, homepage PoW demo input, and both
market-page.js search boxes (/marketplace and every /{chain} page)
stripped outline:none with no replacement - unlike 7 other inputs
already using an established border-color:var(--accent) focus pattern
elsewhere in this codebase (contact.js, playground.js, docs.js, etc.).
First attempt at the fix (:focus{border-color:var(--accent)} added as a
class-scoped stylesheet rule) silently did nothing: verified via a real
Playwright browser test, not assumed. Root cause: all four elements set
their border via an INLINE style="border:..." attribute, and inline
styles unconditionally win over any external/embedded stylesheet rule
for the same property, :focus or not - no specificity fight, the
external rule simply never gets a chance. The working examples elsewhere
in this codebase avoid this because THEIR border is already set via a
class rule, not inline. Fixed properly by moving each element's border
declaration out of its inline style and into the same CSS block as the
new :focus/:focus-within rule, so both the base and focused states now
come from the same cascade origin and the focus rule can actually win.
New test-focus-visible.js drives a real browser to each of the 4 pages,
reads the actual border color before and after focusing the input, and
asserts it changes to the specific accent color (not just "some change,"
which could mask an unrelated hover rule coincidentally firing). This
caught the inline-style bug on the first run (all 4 cases failed with
identical before/after colors) before it could ship - exactly the kind
of failure a static source-text check could never see, the same lesson
from this session's --faint/token-resolution incident a few commits
back. Mutation-tested by removing one rule - caught correctly, restored.
Full local verification: all 12 assertions pass clean on a fresh boot,
test-market-pages.js (219/219), test-index-page.js (39/39),
test-catalog-page.js (15/15), test-home-page.js (27/27) all unaffected -
the border-property refactor changes WHERE the base border is declared,
not its value, so the unfocused appearance is byte-identical.
Audit flagged /docs/:slug and /revenue for nesting a page-level <main> inside ledgerShell()'s own <main>...</main> (ledger-chrome.js:782, which wraps every page's body) - an invalid, duplicate landmark that confuses assistive tech (only one <main> per document should exist). Grepping the whole src/ tree for the same shape while fixing it found two more instances the audit's own spot-check missed: adapter-docs.js (/docs/adapters/:slug) and webhooks.js (/docs/webhooks), both rendering through the same ledgerShell(). Fixed all four identically - change the inner element from <main> to <div>, keep its class/style untouched so nothing visually changes. integrations.js also has its own <main>, but checked directly rather than assumed: it renders through the OLDER src/chrome.js shell, which never wraps in <main> at all, so that one isn't nested and is correctly left alone - included in the new test specifically to prove that negative case, not just skipped. New test-single-main-landmark.js checks 15 real pages (the 4 fixed pages plus a broad sanity sweep of other page types) for exactly one <main> element. Mutation-tested by reverting docs.js's fix - caught correctly (got 2), then restored and re-verified clean (15/15). test-docs-gitbook.js (27/27) and test-docs-truth.js (6/6, live catalog-vs-docs cross-check) both unaffected.
Audit finding: /compare's 4 tables were the only ones site-wide with no
horizontal-scroll wrapper - every other table (leaderboard, status,
revenue) already follows this codebase's own documented pattern
(<div class="X-scroll" style="overflow-x:auto"><table style=
"min-width:...">). Without it, a narrow viewport either clips the third
column or forces the whole page to scroll sideways instead of just the
table.
Wrapped all 4 (identical 3-column shape: Dimension / Agent402 /
comparison target) in a new .cmp-scroll div, gave .cmp-table a
min-width:520px so there's something real to scroll on mobile instead of
squishing three columns into unreadable widths.
First test draft was vacuous and didn't know it: checked
scrollWidth > clientWidth on the wrapper, which is true whether or not
overflow-x is actually enabled - it only proves the content is wider
than the box, not that scrolling works. Caught by mutation-testing (the
required discipline, not optional): removing the CSS rule left every
assertion green. Root cause was also a real, useful finding on its own -
ledger-chrome.js:59 has a SITE-WIDE `html { overflow-x: clip }` guard
that silently clips any unhandled overflow instead of showing a
scrollbar, which is exactly why a naive page-level "does the document
scroll sideways" check can never catch a missing per-table wrapper
regression - the global guard masks it. Fixed the test to check
getComputedStyle(wrapper).overflowX directly; the same mutation now
correctly fails all 4 wrapper checks, restored and re-verified clean
(13/13).
Live-verified with Playwright at a genuinely narrow viewport (390px):
all 4 wrappers report overflow-x:auto and a real scrollWidth/clientWidth
gap (520 vs 330), confirming actual scroll behavior, not just markup
presence.
…equests Audit finding: 408 of 531 POST-only catalog routes returned a bare, generic HTML 404 on a wrong-method GET - indistinguishable to a naive client from "this route doesn't exist at all", when the real answer is "you used the wrong HTTP method". Root cause: each tool is registered on exactly one Express verb (app[lowerMethod](path, ...) in the per-tool binding loop), so a mismatched method never matches any route and falls straight through to Express's default handler. Added a catch-all middleware, built from CATALOG (the same route strings /api/pricing and openapi.json already derive from, so it can never drift from what's actually registered): for any request whose PATH is a known catalog path but whose METHOD isn't among the ones registered for it, respond 405 with an Allow header naming the real method(s) plus a small JSON body - never falls through to a bare HTML page. A path that isn't in the catalog at all is untouched, still 404s exactly as before; a non-catalog page (marketing/HTML surfaces) is untouched entirely, since the map is scoped to CATALOG only. Verified against the FULL real catalog, not just one example: ran test-all.js (every one of 510 exercised endpoints, each hit with its own documented example and therefore its own correct method) - 211/211 strict pure-CPU passes, 297 lenient network passes, 0 server errors, proving the new middleware causes zero regressions across the real catalog by construction (a correct-method request can never reach the mismatch branch). test-head-paywall.js (8/8) also unaffected - the subtle HEAD-to-GET auto-mapping Express does internally still resolves before this middleware ever sees those requests. New test-wrong-method-405.js: GET on a POST-only route gets 405 + Allow: POST + JSON body naming the method; the real method (POST) is completely unaffected; a path outside the catalog entirely still plain-404s with no spurious Allow header; a non-catalog page (/marketplace) renders normally. Mutation-tested (forced the path lookup to always return null) - all 3 core assertions failed correctly, then restored and re-verified clean.
…ection Audit finding: the hero-flash fix only ever exempted the FIRST matched header/section. /pricing has a second section (the tier cards) that also sits reliably above the fold on an ordinary viewport, directly under a short hero - it still gets the same flash-of-blank-content the hero fix already solved for index 0. Deliberately NOT a generic "exempt the first N sections" heuristic - that would be wrong on every page where the second section really is below the fold (most pages, /marketplace checked directly as the counter- example). Added [data-reveal-eager] as an explicit, opt-in marker a page template sets only on a specific section it knows is always visible on load - same DOM-order safety property as the original hero fix (never a getBoundingClientRect read before webfonts/map settle), just marked by the page author instead of inferred by position. Applied it to /pricing's tier section specifically. New test-reveal-eager.js verifies via a real browser: the marked section on /pricing never gets the hiding class and renders at full opacity immediately; critically, an UNMARKED second section on /marketplace still gets the hiding class exactly as before, proving the opt-in doesn't leak to pages that never asked for it (the most likely regression this kind of fix could cause); the original hero exemption (index 0) is undisturbed on both pages. Mutation-tested by reverting to the pre-fix shape (dropping the .filter(...) call) - correctly failed the eager-specific assertions, then restored and re-verified clean (9/9). test-reveal-coverage.js (28/28) and test-reveal-no-hero-flash.js (5/5) both unaffected.
…nded
Audit finding: a402ToggleMenu() already correctly toggled aria-expanded
on open/close, but never touched aria-label, which stayed "Open menu"
permanently even while the menu was open. A screen reader user
activating the control got no confirmation it opened, and had no way to
know the same button now closes it.
One-line fix: toggle aria-label alongside the existing aria-expanded
toggle, same conditional.
New test-burger-aria-label.js drives a real browser at mobile width
(where the burger actually renders), clicks the real button, and checks
both attributes after each click - closed ("Open menu"/"false"), opened
("Close menu"/"true"), closed again (back to "Open menu"/"false").
Mutation-tested by reverting to the pre-fix single-attribute toggle -
correctly caught (aria-label stayed "Open menu" while expanded), then
restored and re-verified clean (7/7). test-theme.js (23/23) - which
already asserts the burger button stays wired to a402ToggleMenu at
all - unaffected.
serviceManifest() mapped Object.keys(catalog) straight to URLs with no dedup - any tool registered under 2 HTTP methods (e.g. GET+POST /api/memory) appeared twice in resources[] with identical URLs and no way for a consumer to tell why. x402scan's discovery format wants a flat URL list (openapi.json already carries the method-annotated view), so the fix dedupes by URL via a Set. New scripts/test-manifest-resources-dedup.js uses a fixture catalog with a deliberate GET+POST pair (a catalog with no duplicate paths would pass trivially and prove nothing) - 5 assertions. Mutation-tested: reverting the dedup back to a plain map correctly fails 3 of 5 assertions; restored and re-verified clean. Verified live against the real catalog (FREE_MODE boot): 529 resources in, 529 unique out, zero duplicates. Regression sweep clean: test-discovery.js, test-leaderboard-surface.js. Wired into deploy.yml next to the existing discovery-surface test step.
…penapi.json Both headers are real and documented in prose (/docs, /llms.txt, quickstart) but were invisible to any OpenAPI-driven client - a caller using Postman, codegen, or an agent framework that reads the machine spec instead of prose had no way to discover them. openapiSpec() now declares Idempotency-Key on every operation (optional) and X-Pow-Solution only on PoW-eligible (non-wallet-only) tools - advertising a free-tier option that doesn't exist on a wallet-only tool like memory would mislead a codegen client, so isComputePayable(tool) gates it. New scripts/test-openapi-header-params.js (7 assertions) boots FREE_MODE and checks the live spec: Idempotency-Key present + optional on a 30-tool sample, X-Pow-Solution present on hash (PoW-eligible) and absent on memory (wallet-only). Mutation-tested: reverting headerParams to an empty array correctly fails the Idempotency-Key assertion; restored and re-verified clean. Regression sweep clean: test-openapi-coverage.js (10/10, path-count delta and MPP offer checks unaffected), test-openapi-fallback.js (6/6), and the full 510-tool test-all.js catalog sweep (0 failed, 0 server errors). Wired into deploy.yml next to the existing openapi coverage test.
…enue view Two related operator-dashboard gaps from the 2026-08-16 audit: 1. Ops-cost-vs-revenue visibility for zero-revenue rails. viaUSDCByNetwork only ever carries a key for a rail that has settled at least once, so a configured-but-unused rail (facilitator config, canary legs, and test maintenance all still paid for) was invisible by omission rather than flagged - an operator had to already know to go looking. getOperatorBreakdown() takes a new offeredNetworks list (server.js passes enabledNetworks(NETWORK)) and returns railBreakdown: one row per offered rail with its lifetime settled call count, explicit 0 and all. The dashboard now renders this as a table with a ZERO REVENUE badge on any rail that's never settled. 2. Top tools by revenue (call-volume != revenue proxy). The per-tool table already carried both calls and revenueUsd but defaulted to sorting by calls, so the revenue leaderboard was one click away instead of visible. Added a standalone top-5-by-revenue list next to top-5-by-calls so the divergence (e.g. route-execute-pro: 1 call, $3.30, vs a $0.001 tool called 500x) is visible without interaction. Also fixed a latent bug found while testing: operatorPage()'s estimated- revenue stat used a broken ternary (`(x ?? 0).toFixed ? x.toFixed(4) : x`) that checks the fallback's .toFixed but calls .toFixed on the possibly- undefined original - harmless today because getOperatorBreakdown() always supplies a number, but a real crash risk for any future caller that doesn't. Simplified to `(x ?? 0).toFixed(4)`. New scripts/test-operator-revenue-visibility.js (12 assertions, offline - calls getOperatorBreakdown/operatorPage directly): railBreakdown shape, robinhood's plain PAYMENT_NETWORKS name vs its "(USDG)" display bucket, ZERO REVENUE badge presence/absence, both leaderboards render real data, and cold-boot (no calls, no networks) renders friendly fallbacks instead of crashing - which is what caught the ternary bug. Mutation-tested twice (railBreakdown computation, badge logic) - both correctly fail 1-3 assertions when reverted; restored and re-verified clean. Regression sweep clean: test-operator-auth.js (34/34), test-operator-wishes.js (16/16), full test-all.js catalog sweep (0 failed). Live-verified against a real FREE_MODE boot with PAYMENT_NETWORKS=base,polygon,solana,stellar - all four rails listed with real accumulated per-tool revenue/call data.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| const memoryUrl = "https://agent402.tools/api/memory"; | ||
| const memoryCount = manifest.resources.filter((r) => r === memoryUrl).length; | ||
| ok(memoryCount === 1, `/api/memory (registered under 2 methods in the fixture) appears exactly once in resources[] (got ${memoryCount})`); | ||
| ok(manifest.resources.includes("https://agent402.tools/api/hash"), "a normal, single-method tool is still present"); |
…urn tracking Self-serve seller signup (POST /api/index/register) had no durable signal at all beyond a bare Set<origin> (submittedSeeds), persisted with no timestamps - no way to answer "of everyone who registered via /sell, how many are still live, and how many ever actually settled a payment" without hand-diffing crawl-cache JSON snapshots. New stats.js table seller_registrations(origin, first_seen, last_routable_seen, last_settled_seen): first_seen is stamped once at registration (immutable). last_routable_seen advances every crawl cycle the origin's x402 surface answers - the churn signal, since it simply stops advancing the moment a seller goes dark, never gets reset to a false "still fine". last_settled_seen advances only when that cycle's leaderboard snapshot shows the origin (matched by canonical host against leaderboard rows' `origins[]`, no new payTo-matching plumbing needed) with callsSettled > 0 - the conversion signal, sticky (a later non-settled observation never erases a prior real settlement). Wired into x402-index.js at both points a self-serve origin gets touched: registerOrigin()'s success paths (new registration AND the cache-hit re-registration path, both gated on submittedSeeds.has() so ecosystem sellers discovered via Bazaar/registry crawling never get misrecorded as our own signups) and runCrawl()'s post-cycle pass over submittedSeeds only. New operator-only GET /__operator/seller-registrations.json (same auth pattern as the existing refunds.json). New scripts/test-seller-registrations.js (15 assertions): first_seen immutability, last_routable_seen advancing, last_settled_seen stickiness, both directly against stats.js and through registerOrigin() with an injected crawler. Mutation-tested twice (the sticky-settlement SQL, the submittedSeeds self-serve gate) - both correctly fail assertions when reverted; restored and re-verified clean. Regression sweep clean: test-index-register.js (20/20), test-crawl-backoff.js (22/22), full test-all.js catalog sweep (0 failed). Live-verified against a real FREE_MODE boot: the operator endpoint lists real accumulated registrations with correct everSettled/daysSinceLastSeen derived fields.
An external client implementer (payagents.io) hard-failed with
no_supported_rail against a live paid call: their router only
recognized the MPP WWW-Authenticate: Payment scheme and canonical
L402, and bailed on ours instead of also checking for the real x402
PAYMENT-REQUIRED header present on the exact same response. Mike
diagnosed and replied in the issue thread, but that guidance only
existed as a GitHub comment - the next client author hitting the same
wall would find nothing in the machine-readable docs.
Added a new paragraph to llmsTxt() (src/seo.js) right after the
existing MPP dual-stack description: states plainly that both headers
are always present, WWW-Authenticate is additive and never replaces
PAYMENT-REQUIRED, names the exact failure mode ("no supported rail"
on a payable 402) and cites issue #794 as a real precedent, not a
hypothetical.
New scripts/test-llms-mpp-header-note.js (6 assertions, offline):
section present, both header names cited, issue number cited, the
"additive never a replacement" framing present, and the note lands
after the paragraph it explains. Mutation-tested: renaming the
heading correctly fails 2 of 6 assertions; restored and re-verified
clean. Regression clean: test-llms-txt.js (18/18 unchanged) and the
full test-all.js catalog sweep (0 failed).
…x registry
DEFAULT_CHALLENGE_CHAIN_IDS ({8453, 42220}, Base + Celo mainnets) rested
on an in-code claim about what a stock mppx client can natively sign,
never actually checked against the installed package. Read mppx@0.8.17's
real source: evm/Chains.ts defines exactly four chain ids (base,
baseSepolia, celo, celoSepolia), and evm/Assets.ts's known-USDC/USDT
registry covers only those same four; a stock client's Charge.ts
resolves an accepted currency via Assets.matches() against ONLY that
registry, so a challenge for any other chain has nothing for a stock
client to auto-sign. Confirms the existing default is exactly right -
the two mainnets, no more, no fewer.
Exported challengeEnabledForChain() (was module-private) so a test can
exercise the REAL production function rather than duplicating the
default as a fixture, which would only prove the fixture equals
itself. New scripts/test-mpp-shim-mppx-registry.js (5 assertions):
every mppx mainnet chain id is enabled by the real default, a chain
mppx doesn't know (Polygon, 137) is NOT enabled, and each mppx mainnet
has a known USDC asset. This is a drift guard, not a one-time check -
a future mppx bump that adds a new mainnet to its registry will fail
this test, forcing a deliberate review of whether the default should
grow with it, instead of silently leaving that chain's challenge
unoffered forever.
Mutation-tested: shrinking the default to {8453} correctly fails 1
assertion; restored and re-verified clean. Regression clean:
test-mpp-shim.js (36/36 unchanged - the exported function didn't touch
the middleware's behavior) and the full test-all.js catalog sweep
(0 failed).
…d CI state
Broke CI (run 31948489148): the test asserted "base"/"polygon"/"solana"
all report settledCalls:0, but those are REAL production counter keys
read from the shared, persistent stats.js SQLite DB - other test steps
running earlier in the same CI job legitimately settle real USDC on
"base" before this step runs, so asserting exactly 0 for it is a false
assumption about process-shared state, not a fixture. Passed locally
because my local /tmp DB happened to be clean at the time; CI's shared
job DB was not.
Fixed by using a synthetic network name
("__test_fake_network_zzz__") that can never collide with a real
CAIP2_NAMES value for the "reports exactly 0" assertion, and relaxing
the cross-network check to "every row's settledCalls is a
non-negative integer, never absent/undefined/NaN" - still a real
shape assertion, just one that doesn't assume a network nobody else
touched. Verified the fix holds under the exact CI failure condition:
manually bumped the real "base" counter locally (recordServedCall)
before rerunning - all 13 assertions still pass.
The "Proportional tiers" sentence in llmsTxt() was a hand-typed list of
3 tiers (execute $0.01, execute-plus $0.05, execute-max $0.55) - it
never mentioned route-execute-pro ($3.30, covering underlying tools up
to $3.00), added to route-execute.js on 2026-08-04. Nobody remembered
to touch this unrelated prose file when that tier shipped, so agents
reading llms.txt saw a pricing summary that undersold what the router
could actually do.
Fixed by deriving the sentence from EXEC_TIERS directly (now exported
from route-execute.js's own list) instead of hand-typing it, so a
future 5th tier can't repeat the same silent omission.
Caught a real bug in my own first draft while building this: the URL
was built as `/api/route/${slug}` (slug is "route-execute-plus"),
producing the doubled path /api/route/route-execute-plus. The real
registered route (buildRouteExecuteTool in route-execute.js) strips
the "route-execute" prefix and prepends "execute", giving the correct
/api/route/execute-plus. Fixed to use the identical derivation.
New scripts/test-llms-route-execute-tiers.js (9 assertions): every
EXEC_TIERS price is mentioned, route-execute-pro specifically is
present (the tier that was missing), no doubled route-execute path,
and every non-base tier's URL is its real route. Mutation-tested
twice - reverting to the raw-slug URL and truncating to 3 tiers each
correctly fail 2-3 assertions; restored and re-verified clean.
Regression clean: test-llms-txt.js (18/18) and
test-llms-mpp-header-note.js (6/6), both unchanged.
…lish
Live-verified against the real registry (2026-08-16): 24 of our 25
published server.json versions were still status "active" and only
one is isLatest - nothing had ever marked a superseded version
deprecated. The registry's own PATCH /v0/servers/{name}/versions/{ver}/status
endpoint (exposed via `mcp-publisher status`) exists exactly for this,
but the publish job never called it.
Publish job now: before publishing, captures whichever version is
currently isLatest (the one about to become stale) via a new small
helper, scripts/mcp-find-latest-version.js, piped the registry search
response already being fetched for the "already published" check. After
a successful publish, if that captured version differs from the new
one, calls `mcp-publisher status --status deprecated` on it. Scoped
narrowly on purpose: this only ever touches the ONE immediately-previous
version per publish, never a bulk rewrite of history, and never fails
the job on a registry hiccup (warning only) - a missed deprecation is
housekeeping debt, not a reason to fail a publish that already
succeeded.
New scripts/test-mcp-find-latest-version.js (5 assertions) drives the
real CLI script via child_process/stdin (the script IS the interface
deploy.yml pipes into, so this isn't a re-import of internal logic).
Mutation-tested: replacing the isLatest predicate with `() => true`
correctly fails 2 assertions; restored and re-verified clean.
NOT done here, and deliberately left for Mike: backfilling deprecation
status on the 24 ALREADY-stale historical versions. That's a one-time,
visible, bulk edit to already-published public registry entries -
different in kind from this ongoing per-publish housekeeping, and
worth a deliberate go-ahead rather than a unilateral cleanup pass.
`mcp-publisher status --status deprecated --all-versions` (excluding
the current latest, run per-version) is the tool to do it whenever
that's wanted.
Prod runs multiple replicas (RATE_LIMIT_REPLICAS is set), and this guard was per-process only - a concurrent replay of the same x402 payment authorization landing on TWO DIFFERENT replicas within the same request window was invisible to it (each replica's inFlight Set only sees its own traffic). The chain's own EIP-3009 nonce single-use property still prevented a double-CHARGE, but the HANDLER ran twice - for a tool that makes a real paid upstream call (Blockscout, LLM gateway, image-gen), that's a real cost duplication bounded only by how fast a caller can fire one authorization at two replicas. Discussed with Mike; go-ahead given. begin()/settle()/release() are now async and, when REDIS_URL is configured and reachable, use the SAME Redis connection shared-limit.js already maintains (via new export getSharedRedisClient() - one connection per process, not a second one to the same server). The concurrent claim uses SET NX (atomic across replicas, same property inFlight.add() gave for free within one process); the in-flight key carries a 120s TTL so a replica that crashes mid-request self-heals instead of leaving that nonce stuck. FAILS OPEN to the original per-process Map/Set on any Redis absence or error - the opposite fail-direction from shared-limit.js's rate limiter, deliberately: that limiter protects a metered free-tier budget (failing open = unmetered free access, a direct loss). This guard is documented defense-in-depth on top of a chain-enforced guarantee that holds with or without it, so degrading to exactly today's per-process guarantee during a Redis blip is strictly better than refusing every paid call over an optimization layer going dark. server.js's call site now awaits begin() (already inside an async middleware) and fires settle()/release() from the finish/close listeners with a defensive .catch() (they already swallow their own Redis errors internally; this only backstops a synchronous throw). Tests: scripts/test-replay-guard.js rewritten async (all 25 prior assertions preserved, now exercising the local-fallback path since no REDIS_URL is set in that step). New scripts/test-replay-guard-redis.js (12 assertions) drives the REAL redis client against a REAL server (installed locally via `brew install redis` for verification, matching the CI job's redis:7-alpine service) - not a stub, which would only prove "the code calls set/get/del" (the same shallow coverage this repo has been burned by before): two independent createReplayGuard() instances stand in for two replicas and prove claim/settle/release are actually visible to each other, the in-flight/consumed TTLs are correct, a real paymentReplayKey()-derived key round-trips, and a broken Redis client (injected via shared-limit.js's own __setTestClient seam) proves the fail-OPEN path resolves cleanly rather than throwing/hanging. Mutation-tested three ways: removing the atomic NX check (2 assertions fail), stripping the try/catch fallback (process crashes with an uncaught rejection - an even stronger kill), and breaking the local state machine directly (9 assertions fail). All restored and re-verified clean. Regression sweep clean: full test-all.js catalog sweep (0 failed, run three times), test-mpp-shim.js (36/36, real paid-call path including this exact middleware), test-idempotency.js (12/12), test-idempotency-settlement.js (8/8), test-wallet-e2e.js (2/2), test-redis-integration.js (11/11, unaffected by the new export).
…a fast scroll Reported live: /marketplace's "Markets by chain" section (and everything below it) appeared missing. Reproduced with Playwright: a single large scroll jump - a trackpad flick, the End key, or clicking the scrollbar track, all ordinary user actions - can move a short <section> from below the viewport to above it within one rendered frame. The IntersectionObserver driving reveal-on-scroll never sees such a section cross its 8% threshold, so it never gets .ml-reveal-in and stays at opacity:0. Worse, it's unrecoverable: a revealed element is unobserve()'d, but a never-intersected one stays observed with no further callback ever firing for it, so subsequent normal scrolling back through the same spot does not self-heal it either - confirmed with a second Playwright pass. Since every <section> on every ledgerShell page opts into this behavior, the bug was reachable anywhere, not just /marketplace. Fixed with a safety net alongside the existing IntersectionObserver (kept for its efficiency on the common case): a debounced scroll listener plus a one-time delayed check directly measure each not-yet-revealed section's bounding rect and reveal it if its top edge has reached or passed the viewport bottom - i.e. it's visible now OR already scrolled past. A section still below the viewport is left untouched, so the intended scroll-in animation for ordinary scrolling is unaffected. New scripts/test-reveal-scroll-skip.js (4 assertions, real browser): reproduces the exact single-jump scenario on /marketplace and asserts zero sections remain stuck, and separately asserts a fresh page load with NO scrolling still leaves below-the-fold sections hidden (proving the fix didn't degenerate into "reveal everything immediately"). Also spot-checked live on /compare, /community, /skills, and / - all clean. Mutation-tested by reverting to the pre-fix script: the new test correctly fails (4/7 sections stuck); restored and re-verified clean. Regression sweep clean: all 4 existing reveal-related suites unchanged (test-reveal-eager.js 9/9, test-reveal-no-hero-flash.js 5/5, test-reveal-coverage.js 28/28, test-reveal-on-scroll.js 8/8), test-theme.js (23/23), and the full test-all.js catalog sweep (0 failed, run twice).
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
2 participants
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.
Summary
WALLET_BLOCKLISTonly evermatched EVM payers. Traced the actual installed SDKs to confirm why -
SVM/Stellar/AVM's exact schemes never carry a
payerfield on the rawclient payload; it's only derivable from the verify() result, which
@x402/core'ssettlePayment()never threads into thebeforeSettlehook's context. A blocked wallet could trivially evade the ban by paying
on any of those three rails.
onAfterVerify(which DOES receive the verify result) stashingthe verified payer onto the same
paymentPayloadobject instancebeforeSettlesees later in the same request - confirmed by reading@x402/core's source directly that both hooks share the same objectreference, never a clone. Never touches an EVM payload (its
authorization.fromis already signature-covered).payload shape no real SDK produces - exactly what let this ship
unnoticed. New tests use real wire shapes plus direct coverage of the
enrichment hook itself.
Test plan
test-wallet-blocklist.jsEVM guard) - both caught correctly, then restored
test-supported-guard.js(16/16, closest adjacent suite touchingthe same server-setup flow) unaffected