From 0ea3d0b1dd8a0a2d55266b43fceabc8e2eaec50d Mon Sep 17 00:00:00 2001 From: holistis Date: Sun, 18 Oct 2026 10:10:35 +0200 Subject: [PATCH 1/2] The facilitator is not a witness we can skip checking Six holes on the live paid route, found by an adversarial review of our own server. Every one is measured below, and two of them turned out different from what the review assumed, which is written down rather than smoothed over. 1. The published output contract promised a field the route never returned. `required: ["method","token","location","verifiedAt"]`, and the real result is { ok, method, location, verifiedAt } or { ok, method, location, reason }. No token, ever. Our own delivery checker reads exactly that field, so it could not process a single real answer from its own target service. The example also named `_tokenizen.example.com` while the code queries `_endpoint-attest`, so an agent copying it published in the wrong place and could never pass. Fixed on both sides. The route now returns the token it compared against, which is safe because that value is public by construction: the owner has to publish it or the check cannot pass. Returning it is what lets a third party re-read the answer instead of taking our word. The declaration now describes ok, token, location, verifiedAt and reason, with the real label in the example, and a test validates a REAL result from the service against the declared schema in both the pass and the fail case. 2. X402_FACILITATOR_URL had no validation at all, three lines below where X402_RESOURCE_BASE is validated strictly. It is the more dangerous of the two: every payment this server verifies and settles goes to whatever is in it, and the answer that comes back is what our key signs a receipt about. Now https only, no credentials, no query or fragment, and a usable url. 3. Our key signed the facilitator's word about the network. `network` came straight out of the settle answer and nothing compared it to what this route offered, so a facilitator naming another chain got our signature on a statement about a payment we never advertised. We now refuse to sign rather than fall back, because falling back to the version 1 receipt would sign the same wrong network without the commitment. Measured against our three live receipts first: all three carry eip155:84532, so this refuses nothing that works today. Proven both ways: with the gate disabled, both new tests fail. 4. The paid route shared its rate limiter with the free one, so anyone could spend a paying customer's budget with free calls. It has its own bucket now, falling back to the old numbers when unset. Measured decisively: with the free bucket exhausted (429), the paid route still runs. 5. NOT REPRODUCED, and the honest version is different. The review believed a broken facilitator could shut the whole process down, free routes included. The payment middleware is built lazily and only reaches out on the first request, so the app comes up fine. The try/catch around building it stays as a cheap guard against a future eager client, but it fixed nothing measurable and is not claimed to. What a dead facilitator DID do: the paid route answered a bare 500 {"error":"Internal Server Error"}. For a buyer that is indistinguishable from "this product is broken", while the true answer is that we cannot settle right now and nothing was charged. It is also not catchable by an express error handler, because the middleware answers rather than throws, which is why the fix sits in front of it and rewrites that one status. 6. Our own payTo address was the example ownerAddress in the public declaration, so our receiving address sat in a catalogue as an example of "the party claiming to control this endpoint". Replaced with an unmistakable example. Tests: self-check-server 131 (was 113), endpoint-attest 73, trust-attest-server and website typecheck clean. --- packages/endpoint-attest/src/ownership.ts | 36 ++- packages/self-check-server/src/app.ts | 109 ++++++- .../src/x402-keuring-18-okt.test.ts | 272 ++++++++++++++++++ .../src/x402-seller-v2.test.ts | 34 +++ packages/self-check-server/src/x402-seller.ts | 91 +++++- 5 files changed, 521 insertions(+), 21 deletions(-) create mode 100644 packages/self-check-server/src/x402-keuring-18-okt.test.ts diff --git a/packages/endpoint-attest/src/ownership.ts b/packages/endpoint-attest/src/ownership.ts index 586f239..b2d417e 100644 --- a/packages/endpoint-attest/src/ownership.ts +++ b/packages/endpoint-attest/src/ownership.ts @@ -193,9 +193,25 @@ export interface OwnershipCheckDeps { maxFileBytes?: number; } +/** + * WAAROM `token` HIERIN ZIT, en dat is geen uitbreiding maar een reparatie. + * + * Het gepubliceerde 402-contract van de betaalde route eist `token` als verplicht + * uitvoerveld, en dit type gaf het nooit terug. Gemeten op 2026-10-18: ons eigen + * leveringscontroleur leest precies dat veld, dus hij kon geen enkel echt antwoord + * van onze eigen dienst verwerken. Een belofte in een catalogus die de route niet + * nakomt is erger dan een ontbrekende belofte. + * + * EN HET IS VEILIG, want dit is per constructie een publieke waarde: de eigenaar + * MOET hem zelf publiceren in DNS of in een bestand, anders kan de controle nooit + * slagen. Hem teruggeven verklapt dus niets wat niet al openbaar hoort te staan, + * en het is precies wat een derde partij nodig heeft om het antwoord zelf na te + * lezen in plaats van ons op ons woord te geloven. Hij gaat ook mee bij een + * MISLUKTE controle, want dan is hij het antwoord op "wat had er moeten staan". + */ export type OwnershipResult = - | { ok: true; method: OwnershipMethod; location: string; verifiedAt: string } - | { ok: false; method: OwnershipMethod; location: string; reason: string }; + | { ok: true; method: OwnershipMethod; token: string; location: string; verifiedAt: string } + | { ok: false; method: OwnershipMethod; token: string; location: string; reason: string }; /** * Verify control of `endpoint` by the holder of `ownerAddress`, using one @@ -260,16 +276,16 @@ async function verifyTokenPublished( try { records = await resolver(location); } catch (e) { - return { ok: false, method, location, reason: `dns lookup failed: ${(e as Error).message}` }; + return { ok: false, method, token: expected, location, reason: `dns lookup failed: ${(e as Error).message}` }; } // A TXT record can be split into multiple strings by the resolver; the // published value is their concatenation, which is how every DNS client // reassembles long records. const values = records.map((chunks) => chunks.join("").trim()); if (!values.includes(expected)) { - return { ok: false, method, location, reason: "no TXT record at that name matched the expected token" }; + return { ok: false, method, token: expected, location, reason: "no TXT record at that name matched the expected token" }; } - return { ok: true, method, location, verifiedAt: now() }; + return { ok: true, method, token: expected, location, verifiedAt: now() }; } const fetchImpl = deps.fetchImpl; @@ -285,25 +301,25 @@ async function verifyTokenPublished( // merely has an open redirect at /.well-known/endpoint-attest.txt. response = await fetchImpl(location, { signal: AbortSignal.timeout(timeoutMs), redirect: "error" }); } catch (e) { - return { ok: false, method, location, reason: `fetch failed: ${(e as Error).message}` }; + return { ok: false, method, token: expected, location, reason: `fetch failed: ${(e as Error).message}` }; } if (!response.ok) { - return { ok: false, method, location, reason: `responded ${response.status} ${response.statusText}` }; + return { ok: false, method, token: expected, location, reason: `responded ${response.status} ${response.statusText}` }; } let body: string; try { body = await readBounded(response, maxFileBytes); } catch (e) { - return { ok: false, method, location, reason: (e as Error).message }; + return { ok: false, method, token: expected, location, reason: (e as Error).message }; } // Exact match on the trimmed content, not a substring search: a token // buried in an otherwise unrelated page does not demonstrate that the // operator deliberately published it. if (body.trim() !== expected) { - return { ok: false, method, location, reason: "file content did not exactly match the expected token" }; + return { ok: false, method, token: expected, location, reason: "file content did not exactly match the expected token" }; } - return { ok: true, method, location, verifiedAt: now() }; + return { ok: true, method, token: expected, location, verifiedAt: now() }; } /** Read a response body up to maxBytes, refusing the moment the limit is crossed instead of buffering the rest. */ diff --git a/packages/self-check-server/src/app.ts b/packages/self-check-server/src/app.ts index d5f7b2c..6885688 100644 --- a/packages/self-check-server/src/app.ts +++ b/packages/self-check-server/src/app.ts @@ -105,6 +105,14 @@ export interface CreateAppOptions { trustProxy?: boolean | number | string; /** Rate limit for the (real network I/O) verify-ownership route. Defaults to 10 requests / 5 minutes per IP. */ verifyRateLimit?: { windowMs: number; max: number }; + /** + * Rate limit for the PAID verify-ownership route, with its OWN bucket. + * + * It shared the free route's bucket, which let anyone spend a paying customer's + * budget with free calls. Falls back to verifyRateLimit when unset, so nothing + * changes for an existing caller except that the two no longer drain each other. + */ + paidRateLimit?: { windowMs: number; max: number }; /** Rate limit for the (no network I/O) challenge route. Defaults to 30 requests / 5 minutes per IP — looser, since it is pure local HMAC, same posture trust-attest-server's own comments describe for get_ownership_challenge. */ challengeRateLimit?: { windowMs: number; max: number }; } @@ -194,6 +202,16 @@ export function createApp(options: CreateAppOptions = {}): Express { windowMs: options.verifyRateLimit?.windowMs ?? 5 * 60 * 1000, max: options.verifyRateLimit?.max ?? 10, }); + // EEN EIGEN EMMER VOOR DE BETAALDE ROUTE. Hij deelde die met de gratis route, en + // dat is tegen de regel die rate-limit.ts zelf opschrijft: een limiet PER AANROEPER + // op alles wat echt werk doet. De betaalde route doet MEER werk dan de gratis + // (hij praat ook met een facilitator), en een gedeelde emmer betekent dat iemand + // met gratis verzoeken het budget van de betalende klant kan opmaken. Dat is geen + // bescherming, dat is een dicht te drukken verdienroute. + const paidLimiter = createRateLimiter({ + windowMs: options.paidRateLimit?.windowMs ?? options.verifyRateLimit?.windowMs ?? 5 * 60 * 1000, + max: options.paidRateLimit?.max ?? options.verifyRateLimit?.max ?? 10, + }); const app = express(); app.disable("x-powered-by"); @@ -283,18 +301,103 @@ export function createApp(options: CreateAppOptions = {}): Express { // the buyer's half of a delivery record has nothing to name while no seller // emits receipts. const seller = options.seller === undefined ? readSellerConfig(process.env) : options.seller; + // A THIRD PARTY MUST NOT BE ABLE TO CLOSE OUR FREE DOOR. Building the payment + // middleware constructs a facilitator client, so anything wrong out there (an + // unusable url, a client that reaches for /supported while being constructed) + // threw right here, at app construction, and took down every free route with it: + // health, challenge, verify-ownership, the badge and the conformance endpoint. + // The paid route is one route; the free ones are the reason this server exists. + // + // What we do NOT do is mount the paid route anyway. Advertising a price we cannot + // settle is a false promise, which is the same reason an unconfigured seller has + // no paid route at all. So: loud on the way out, and the paid door stays shut. + let betaalmiddleware: ReturnType | null = null; if (seller) { + try { + betaalmiddleware = buildPaymentMiddleware(seller); + } catch (error) { + console.error( + "[app] the paid route could not be built, so it is NOT mounted. Every free route stays up:", + error, + ); + } + } + + /** + * Een onbereikbare facilitator is onze storing, niet die van de koper, en hij + * hoort dat te horen in plaats van een kale 500 te krijgen. + * + * GEMETEN, en anders dan de keuring vermoedde. Die dacht dat een kapotte + * facilitator het hele proces kon afsluiten. Dat reproduceert NIET: de + * betaalmiddleware wordt lui opgebouwd en reikt pas bij het eerste verzoek naar + * buiten. Wat er wel gebeurde, met een adres dat niet oplost: de betaalde route + * antwoordde `500 Internal Server Error` en niets meer. Voor een koper is dat niet + * te onderscheiden van "dit product is stuk", terwijl het ware antwoord is "wij + * kunnen nu niet afrekenen, er is niets betaald". 503 met een korte, ware regel. + * De details blijven in ons eigen log, niet in het antwoord: daarin staat de naam + * van onze facilitator en een stapeldump. + */ + /** + * Zet een storing in de afrekening om in een eerlijk antwoord, VOOR de + * betaalmiddleware aan het woord komt. + * + * WAAROM DIT GEEN FOUTOPVANGER IS, en dat is gemeten in plaats van aangenomen. Een + * express-foutopvanger kan hier niets, want de betaalmiddleware GOOIT niet: hij + * antwoordt zelf, letterlijk `500 {"error":"Internal Server Error"}`, met een + * onbereikbare facilitator. Voor een koper is dat niet te onderscheiden van "dit + * product is stuk", terwijl het ware antwoord is dat wij nu niet kunnen afrekenen en + * dat er niets is afgeschreven. Daarom wordt hier alleen dat ENE geval herschreven: + * status 500 op de betaalde route. Al het andere gaat ongemoeid door, want een + * middleware die meer herschrijft dan hij begrijpt verbergt de volgende storing. + */ + const betaalstoringNaar503 = (_req: Request, res: Response, next: NextFunction): void => { + const origineel = res.json.bind(res); + res.json = ((lichaam: unknown) => { + if (res.statusCode === 500) { + console.error("[app] the paid route answered 500, reported to the buyer as 503:", lichaam); + res.status(503); + return origineel({ + ok: false, + error: "settlement is temporarily unavailable, so no payment was taken and no receipt was issued", + }); + } + return origineel(lichaam as never); + }) as typeof res.json; + next(); + }; + + // Tweede net, voor het geval de middleware ooit wel GOOIT in plaats van zelf te + // antwoorden. Kost niets en vangt een vorm die de bovenstaande niet ziet. + const betaalfoutmelder = ( + error: unknown, + _req: Request, + res: Response, + next: NextFunction, + ): void => { + if (res.headersSent) { + next(error); + return; + } + console.error("[app] the paid route could not settle:", error); + res.status(503).json({ + ok: false, + error: "settlement is temporarily unavailable, so no payment was taken and no receipt was issued", + }); + }; + if (seller && betaalmiddleware) { // The payment middleware talks to a facilitator on every request it sees, // so it is mounted ON the paid path and BEHIND the limiter, never globally // with app.use. Mounted globally it would let an anonymous caller make this // server call out repeatedly, which is someone else's rate limit to pay. app.post( "/api/paid/verify-ownership", - verifyLimiter, - buildPaymentMiddleware(seller), + paidLimiter, + betaalstoringNaar503, + betaalmiddleware, async (req: Request, res: Response) => { await handleVerifyOwnership(req, res); }, + betaalfoutmelder, ); // A GET on the paid path used to fall through to Express's default handler, @@ -308,7 +411,7 @@ export function createApp(options: CreateAppOptions = {}): Express { // 405 is what the method actually is, and the body says where to read the // terms. Naming the wrong status to look more x402-shaped would be the kind // of small untruth this whole project exists to avoid. - app.get("/api/paid/verify-ownership", verifyLimiter, (_req: Request, res: Response) => { + app.get("/api/paid/verify-ownership", paidLimiter, (_req: Request, res: Response) => { res .status(405) .set("Allow", "POST") diff --git a/packages/self-check-server/src/x402-keuring-18-okt.test.ts b/packages/self-check-server/src/x402-keuring-18-okt.test.ts new file mode 100644 index 0000000..ef32ea6 --- /dev/null +++ b/packages/self-check-server/src/x402-keuring-18-okt.test.ts @@ -0,0 +1,272 @@ +// x402-keuring-18-okt.test.ts — de gaten die onze eigen keuring op 18 oktober vond, +// elk met een test die rood zou zijn geweest. +// +// WAAROM DIT EEN APART BESTAAND IS EN NIET VERSPREID OVER DE BESTAANDE SUITES. Deze +// gevallen horen bij elkaar omdat ze allemaal dezelfde vorm hebben: iets buiten ons +// (een facilitator, een verkoper, een vreemde) bepaalde iets waar wij met onze eigen +// naam of onze eigen sleutel voor instonden. Bij elkaar gezet is dat een leesbaar +// verhaal; verspreid over vijf bestanden is het vijf losse rariteiten. +// +// ALLE VIJF ZATEN IN LIVE CODE. Geen van hen is theoretisch, en bij elk staat +// hieronder wat er gemeten is in plaats van wat er bedacht is. + +import { describe, it, expect } from "vitest"; +import Ajv from "ajv"; +import { DNS_LABEL, WELL_KNOWN_PATH, verifyOwnership } from "endpoint-attest"; +import { declareBazaarDiscovery, readSellerConfig, FACILITATOR_URL } from "./x402-seller.js"; +import { createApp } from "./app.js"; + +/** Lang genoeg voor hun eigen poort: een kort geheim maakt elk token te raden. */ +const GEHEIM = "een-wegwerp-attestergeheim-van-ruim-genoeg-tekens"; + +const TEST_ENV = { + X402_RECEIPT_SIGNING_KEY: "0x" + "11".repeat(32), + X402_PAY_TO: "0x2888F36548a900B9b08d584524e1f73BC563eC26", +}; + +describe("de facilitator-url krijgt dezelfde strengheid als de resource-base", () => { + // GEMETEN: X402_RESOURCE_BASE werd in drie regels streng gecontroleerd, en + // X402_FACILITATOR_URL ging er in dezelfde functie volstrekt ongecontroleerd in. + // Dat is de gevaarlijkste van de twee: elke betaling die deze server verifieert en + // afrekent gaat naar wat daarin staat, en het antwoord dat terugkomt is waar onze + // sleutel een bonnetje over ondertekent. + + it("weigert http, want hier reist een betaalpayload over", () => { + expect(() => readSellerConfig({ ...TEST_ENV, X402_FACILITATOR_URL: "http://facilitator.example" })).toThrow( + /must be https/, + ); + }); + + it("weigert inloggegevens in de url", () => { + expect(() => + readSellerConfig({ ...TEST_ENV, X402_FACILITATOR_URL: "https://naam:geheim@facilitator.example" }), + ).toThrow(/credentials/); + }); + + it("weigert een query of fragment, want dat is geen facilitator-adres", () => { + expect(() => + readSellerConfig({ ...TEST_ENV, X402_FACILITATOR_URL: "https://facilitator.example?doorsturen=elders" }), + ).toThrow(/query or fragment/); + expect(() => + readSellerConfig({ ...TEST_ENV, X402_FACILITATOR_URL: "https://facilitator.example#elders" }), + ).toThrow(/query or fragment/); + }); + + it("weigert iets wat geen url is", () => { + expect(() => readSellerConfig({ ...TEST_ENV, X402_FACILITATOR_URL: "dit is geen url" })).toThrow( + /not a usable url/, + ); + }); + + it("accepteert de twee die wij echt gebruiken, en knipt een naslepende schuine streep", () => { + expect(readSellerConfig(TEST_ENV)?.facilitatorUrl).toBe(FACILITATOR_URL); + expect( + readSellerConfig({ ...TEST_ENV, X402_FACILITATOR_URL: "https://facilitator.payai.network/" })?.facilitatorUrl, + ).toBe("https://facilitator.payai.network"); + }); +}); + +describe("het gepubliceerde uitvoercontract beschrijft wat de route echt teruggeeft", () => { + // GEMETEN OP 18 OKTOBER, en dit is het ergste van de vijf omdat het al in een + // publieke catalogus staat: de verklaring eiste `token` als verplicht uitvoerveld + // en de route gaf dat veld nooit terug. Onze eigen leveringscontroleur leest + // precies dat veld, dus hij kon geen enkel echt antwoord van zijn eigen doeldienst + // verwerken. De verklaring noemde ook een locatievorm die de code niet gebruikt. + + // GEMETEN, NIET AANGENOMEN, en mijn eerste poging had dit mis. Hun bouwer legt het + // uitvoerVOORBEELD onder `info.output.example` en het uitvoerSCHEMA onder + // `schema.properties.output.properties.example`. Een facilitator valideert die twee + // tegen elkaar, dus dat is het paar dat moet kloppen. + const voorbeeld = () => (declareBazaarDiscovery().bazaar as any).info.output.example; + const uitvoerSchema = () => + (declareBazaarDiscovery().bazaar as any).schema.properties.output.properties.example; + const invoerVoorbeeld = () => (declareBazaarDiscovery().bazaar as any).info.input.body; + + it("het voorbeeld voldoet aan het schema dat ernaast staat", () => { + const ajv = new Ajv({ strict: false }); + const valideer = ajv.compile(uitvoerSchema()); + expect(valideer(voorbeeld())).toBe(true); + }); + + it("een ECHT antwoord van de route voldoet aan het verklaarde schema", async () => { + // Dit is de controle die de vorige editie niet had: niet of het voorbeeld bij het + // schema past, maar of de WERKELIJKE uitvoer van de dienst erbij past. + const ajv = new Ajv({ strict: false }); + const valideer = ajv.compile(uitvoerSchema()); + + const token = "endpoint-attest-" + "a".repeat(20); + const geslaagd = await verifyOwnership("dns-txt", "https://klant.example/pad", TEST_ENV.X402_PAY_TO, GEHEIM, { + resolveTxtRecords: async () => [[token]], + fetchImpl: async () => { + throw new Error("niet gebruikt in deze tak"); + }, + } as never); + // De token die de dienst teruggeeft is de token die hij verwachtte, niet de onze, + // dus het geslaagde geval wordt hier met de echte verwachte waarde nagespeeld. + const echteToken = (geslaagd as any).token; + expect(typeof echteToken).toBe("string"); + + const opnieuw = await verifyOwnership("dns-txt", "https://klant.example/pad", TEST_ENV.X402_PAY_TO, GEHEIM, { + resolveTxtRecords: async () => [[echteToken]], + fetchImpl: async () => { + throw new Error("niet gebruikt in deze tak"); + }, + } as never); + expect(opnieuw.ok).toBe(true); + expect(valideer(opnieuw)).toBe(true); + }); + + it("en een MISLUKT antwoord voldoet er ook aan, want dat is een antwoord en geen storing", async () => { + const ajv = new Ajv({ strict: false }); + const valideer = ajv.compile(uitvoerSchema()); + const mislukt = await verifyOwnership("dns-txt", "https://klant.example/pad", TEST_ENV.X402_PAY_TO, GEHEIM, { + resolveTxtRecords: async () => [["iets volstrekt anders"]], + fetchImpl: async () => { + throw new Error("niet gebruikt in deze tak"); + }, + } as never); + expect(mislukt.ok).toBe(false); + expect(valideer(mislukt)).toBe(true); + }); + + it("het token zit in de uitvoer, want zonder dat kan een derde het antwoord niet nalezen", () => { + expect(Object.keys(uitvoerSchema().properties)).toContain("token"); + expect(uitvoerSchema().required).toContain("token"); + expect(voorbeeld().token).toBeTypeOf("string"); + }); + + it("de voorbeeldlocatie gebruikt het label dat de code echt opvraagt", () => { + // De oude verklaring zei `_tokenizen.example.com`, en het echte label is + // `_endpoint-attest`. Een agent die het voorbeeld kopieert publiceerde dus op de + // verkeerde plek en kwam er nooit door. + expect(voorbeeld().location).toContain(DNS_LABEL); + expect(voorbeeld().location).not.toContain("_tokenizen"); + expect(WELL_KNOWN_PATH).toContain("endpoint-attest"); + }); + + it("het voorbeeld-ownerAddress is niet ons eigen ontvangstadres", () => { + // Ons payTo stond hier als voorbeeld van "de partij die beweert dit eindpunt te + // beheren". Dat is onze operator-identiteit in een veld waar hij niet hoort, in + // een catalogus die iedereen leest. + const invoer = invoerVoorbeeld(); + expect(String(invoer.ownerAddress).toLowerCase()).not.toBe(TEST_ENV.X402_PAY_TO.toLowerCase()); + }); +}); + +/** Een verkoper die wel te bouwen is, met een facilitator die nergens naartoe gaat. */ +const VERKOPER_MET_DODE_FACILITATOR = { + kid: "did:pkh:eip155:1:0x0000000000000000000000000000000000000002", + signerAddress: "0x0000000000000000000000000000000000000002", + payTo: TEST_ENV.X402_PAY_TO, + price: "$0.001", + resourceBase: "https://api.tokenizen.nl", + privateKey: "0x" + "11".repeat(32), + receiptV2: false, + bazaar: false, + facilitatorUrl: "https://nergens.invalid", +}; + +const LOOKUP_PUBLIEK = { lookup: async () => [{ address: "93.184.216.34", family: 4 }] }; + +async function metServer(opties: Record, werk: (basis: string) => Promise) { + const app = createApp({ routableHostDeps: LOOKUP_PUBLIEK, ...opties } as never); + const server = app.listen(0); + try { + await werk(`http://127.0.0.1:${(server.address() as { port: number }).port}`); + } finally { + server.close(); + } +} + +const LICHAAM = { + endpoint: "https://voorbeeld.example/pad", + ownerAddress: TEST_ENV.X402_PAY_TO, + method: "dns-txt", +}; + +const post = (basis: string, pad: string) => + fetch(basis + pad, { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify(LICHAAM), + }); + +describe("de betaalde route heeft zijn eigen emmer", () => { + // GEMETEN: de betaalde en de gratis route deelden een limiter-instantie, dus een + // vreemde kon met gratis verzoeken het budget van een betalende klant opmaken. Dat + // is geen bescherming maar een dicht te drukken verdienroute. De regel staat in + // rate-limit.ts zelf: een limiet PER AANROEPER op alles wat echt werk doet. + // + // WAAROM DEZE TEST ER ZO UITZIET, en dat is een eigen correctie. Mijn eerste versie + // draaide zonder verkoper, dus zonder betaalde route, en keek of die geen 429 gaf. + // Dat was VALS GROEN: zonder route loopt er geen limiter, dus hij gaf ook voor de + // reparatie geen 429. Nu staat er een echte verkoper, en het verschil is beslissend: + // met een gedeelde emmer is de derde regel 429, met een eigen emmer niet. + + it("een lege gratis emmer laat de betaalde route gewoon door", async () => { + await metServer( + { + seller: VERKOPER_MET_DODE_FACILITATOR, + verifyRateLimit: { windowMs: 60_000, max: 1 }, + paidRateLimit: { windowMs: 60_000, max: 3 }, + }, + async (basis) => { + await post(basis, "/api/verify-ownership"); + const tweede = await post(basis, "/api/verify-ownership"); + expect(tweede.status).toBe(429); + + const betaald = await post(basis, "/api/paid/verify-ownership"); + expect(betaald.status).not.toBe(429); + }, + ); + }); + + it("en de betaalde emmer loopt zelf ook echt leeg", async () => { + await metServer( + { + seller: VERKOPER_MET_DODE_FACILITATOR, + verifyRateLimit: { windowMs: 60_000, max: 9 }, + paidRateLimit: { windowMs: 60_000, max: 1 }, + }, + async (basis) => { + await post(basis, "/api/paid/verify-ownership"); + const tweede = await post(basis, "/api/paid/verify-ownership"); + expect(tweede.status).toBe(429); + // En de gratis route is daardoor NIET dichtgegaan, de andere richting. + const gratis = await post(basis, "/api/verify-ownership"); + expect(gratis.status).not.toBe(429); + }, + ); + }); +}); + +describe("een facilitator die niet antwoordt is onze storing, niet die van de koper", () => { + // GEMETEN, EN ANDERS DAN DE KEURING VERMOEDDE. Die dacht dat een kapotte facilitator + // het hele proces kon afsluiten. Dat reproduceert niet: de betaalmiddleware wordt lui + // opgebouwd en reikt pas bij het eerste verzoek naar buiten, dus de app komt gewoon + // overeind. Wat er wel gebeurde met een adres dat niet oplost: de betaalde route gaf + // een kale `500 Internal Server Error`. Voor een koper is dat niet te onderscheiden + // van "dit product is stuk". Dat is het gat dat hier dicht gaat. + + it("de gratis routes blijven antwoorden", async () => { + await metServer({ seller: VERKOPER_MET_DODE_FACILITATOR }, async (basis) => { + const gezond = await fetch(`${basis}/api/health`); + expect(gezond.status).toBe(200); + expect(await gezond.json()).toEqual({ ok: true }); + }); + }); + + it("de betaalde route zegt 503 met een ware regel, geen kale 500", async () => { + await metServer({ seller: VERKOPER_MET_DODE_FACILITATOR }, async (basis) => { + const r = await post(basis, "/api/paid/verify-ownership"); + expect(r.status).toBe(503); + const body = (await r.json()) as { ok: boolean; error: string }; + expect(body.ok).toBe(false); + // De koper hoort te weten dat er NIETS is afgeschreven. Dat is het enige wat + // hij op dit moment echt moet weten. + expect(body.error).toMatch(/no payment was taken/); + // En onze eigen facilitator-naam hoort NIET in het antwoord te staan. + expect(JSON.stringify(body)).not.toContain("nergens.invalid"); + }); + }); +}); diff --git a/packages/self-check-server/src/x402-seller-v2.test.ts b/packages/self-check-server/src/x402-seller-v2.test.ts index 08c9f1e..7289b65 100644 --- a/packages/self-check-server/src/x402-seller-v2.test.ts +++ b/packages/self-check-server/src/x402-seller-v2.test.ts @@ -163,4 +163,38 @@ describe("the seller with version 2 switched on", () => { it("emits nothing at all for a failed settlement", async () => { expect(await settle(true, { result: { success: false } })).toBeUndefined(); }); + + // ONZE SLEUTEL TEKENT NIET OP HET WOORD VAN EEN DERDE. `network` komt rechtstreeks + // uit het settle-antwoord van de facilitator, en niets vergeleek het met het netwerk + // dat deze route zelf aanbood. Een facilitator die een andere keten noemt, kreeg + // daarmee onze handtekening op een uitspraak over een betaling die wij nooit hebben + // geadverteerd, en een lezer van dat bonnetje kan dat niet zien. + // + // WEIGEREN, NIET TERUGVALLEN. Terugvallen op het bonnetje van versie 1 zou hetzelfde + // verkeerde netwerk ondertekenen, alleen zonder commitment. Geen bonnetje is een + // eerlijke uitkomst; een ondertekend onwaar bonnetje niet. + // + // GEMETEN VOOR DIT ERIN GING: onze drie echte bonnetjes dragen alle drie + // eip155:84532, precies SELLER_NETWORK, dus dit weigert niets wat vandaag werkt. + it("weigert te tekenen als de facilitator een ander netwerk meldt dan wij aanboden", async () => { + const uit = await settle(true, { + result: { success: true, payer: BUYER, network: "eip155:1", transaction: "0x" + "ee".repeat(32) }, + }); + expect(uit).toBeUndefined(); + }); + + it("en ook als de route een netwerk EIST dat de facilitator niet gebruikte", async () => { + const uit = await settle(true, { + requirements: { asset: ASSET, network: "eip155:8453" }, + }); + expect(uit).toBeUndefined(); + }); + + it("maar tekent gewoon als het netwerk klopt met wat de route eist", async () => { + const uit = await settle(true, { + requirements: { asset: ASSET, network: SELLER_NETWORK }, + }); + expect(uit?.info?.receipt).toBeTruthy(); + expect(uit?.info?.authorizationSalt).toMatch(/^0x[0-9a-f]{64}$/); + }); }); diff --git a/packages/self-check-server/src/x402-seller.ts b/packages/self-check-server/src/x402-seller.ts index 7f8ef16..c81b243 100644 --- a/packages/self-check-server/src/x402-seller.ts +++ b/packages/self-check-server/src/x402-seller.ts @@ -36,7 +36,7 @@ import { declareOfferReceiptExtension, } from "@x402/extensions/offer-receipt"; import { bazaarResourceServerExtension, declareDiscoveryExtension } from "@x402/extensions/bazaar"; -import { OWNERSHIP_METHODS } from "endpoint-attest"; +import { DNS_LABEL, OWNERSHIP_METHODS } from "endpoint-attest"; import { HTTPFacilitatorClient, x402ResourceServer } from "@x402/core/server"; import { ExactEvmScheme } from "@x402/evm/exact/server"; import { paymentMiddleware } from "@x402/express"; @@ -143,6 +143,31 @@ export function readSellerConfig(env: SellerEnv): SellerConfig | null { throw new Error("X402_RESOURCE_BASE must be an https origin with no path, e.g. https://api.tokenizen.nl"); } + // THE FACILITATOR URL GETS THE SAME SCRUTINY AS THE RESOURCE BASE, and it did + // not before. Three lines above validate X402_RESOURCE_BASE strictly; this one + // went in unchecked, and it is the more dangerous of the two: every payment this + // server verifies and settles goes to whatever is in it, and the answer that + // comes back is what our key then signs a receipt about. A typo in an env file, + // or anything that can set an env var, silently redirects that. + const facilitatorUrl = (env.X402_FACILITATOR_URL?.trim() || FACILITATOR_URL).replace(/\/+$/, ""); + const checkFacilitator = (() => { + let u: URL; + try { + u = new URL(facilitatorUrl); + } catch { + return "X402_FACILITATOR_URL is not a usable url"; + } + // http is refused even for a local facilitator: the payment payload and the + // settlement answer both travel over this, and a receipt our key signs depends + // on the answer. Someone who needs plain http in development can say so by + // changing this line on purpose, which is the point. + if (u.protocol !== "https:") return `X402_FACILITATOR_URL must be https, got ${u.protocol}`; + if (u.username || u.password) return "X402_FACILITATOR_URL must not carry credentials"; + if (u.search || u.hash) return "X402_FACILITATOR_URL must not carry a query or fragment"; + return null; + })(); + if (checkFacilitator) throw new Error(checkFacilitator); + return { // Matches the controller published at _controllers. exactly. A // verifier compares these two strings, so they must not drift. @@ -154,7 +179,7 @@ export function readSellerConfig(env: SellerEnv): SellerConfig | null { privateKey: key, receiptV2: env.X402_RECEIPT_V2?.trim() === "1", bazaar: env.X402_BAZAAR?.trim() === "1", - facilitatorUrl: env.X402_FACILITATOR_URL?.trim() || FACILITATOR_URL, + facilitatorUrl, }; } @@ -269,6 +294,27 @@ export function buildOfferReceiptExtension(config: SellerConfig): OfferReceiptEx const resourceUrl = context.transportContext?.request?.adapter?.getUrl?.(); if (!payer || !network || !resourceUrl) return upstreamSettle(declaration, context); + // THE FACILITATOR IS NOT A WITNESS WE CAN SKIP CHECKING. `network` comes + // straight out of its settle answer, and our key is about to sign a receipt + // that names it. Nothing compared it to the network we actually offered. A + // facilitator that answers with another chain would get our signature on a + // statement about a payment we never advertised, and a reader of that receipt + // has no way to know. + // + // We refuse to sign rather than fall back, and that is deliberate. Falling + // back to the upstream version 1 receipt would sign the same wrong network, + // just without the commitment. No receipt is an honest outcome; a signed + // false one is not. Measured against our three live receipts first: all three + // carry eip155:84532, exactly SELLER_NETWORK, so this refuses nothing that + // works today. + const verwachtNetwerk = context?.requirements?.network ?? SELLER_NETWORK; + if (String(network) !== String(verwachtNetwerk)) { + console.error( + `[x402-seller] refusing to sign: the facilitator settled on ${String(network)} while this route offered ${String(verwachtNetwerk)}`, + ); + return undefined; + } + const auth = readSettlementAuthorization(context, payer); if (!auth) return upstreamSettle(declaration, context); @@ -372,7 +418,11 @@ export function declareBazaarDiscovery(): Record { bodyType: "json", input: { endpoint: "https://example.com/api/health", - ownerAddress: "0x2888F36548a900B9b08d584524e1f73BC563eC26", + // NIET ons payTo-adres. Dat stond hier, en dan staat ons eigen ontvangstadres + // in een publieke catalogus als voorbeeld van "de partij die beweert dit + // eindpunt te beheren". Dit is een adres uit de documentatie van ethers zelf, + // onmiskenbaar een voorbeeld. + ownerAddress: "0x0000000000000000000000000000000000000001", method: "dns-txt", }, inputSchema: { @@ -401,27 +451,52 @@ export function declareBazaarDiscovery(): Record { required: ["endpoint", "ownerAddress", "method"], additionalProperties: false, }, + // THE OUTPUT CONTRACT, AND IT IS READ BY MACHINES. Measured on 2026-10-18: + // this block required a `token` field that the route never returned, named a + // location format the code does not use (the real DNS label is DNS_LABEL, + // `_endpoint-attest`), and left out `ok` and `reason`, which the route does + // return. A failed check carries no `verifiedAt` at all, so the old `required` + // list could not hold even in principle. Our own delivery checker reads the + // `token` field, so it could not process a single real answer from its own + // target service. A promise in a catalogue that the route does not keep is + // worse than no promise: the facilitator validates `info` against `schema`, + // and an agent decides whether to call us from exactly this. output: { example: { + ok: true, method: "dns-txt", - token: "tokenizen-ownership-0000000000000000", - location: "_tokenizen.example.com", + token: "endpoint-attest-0000000000000000", + location: `${DNS_LABEL}.example.com`, verifiedAt: "2026-10-18T00:00:00Z", }, schema: { type: "object", properties: { + ok: { type: "boolean", description: "Whether control was proven. False is an answer, not an error." }, method: { type: "string", enum: [...OWNERSHIP_METHODS] }, - token: { type: "string", minLength: 16, maxLength: 512 }, + token: { + type: "string", + minLength: 16, + maxLength: 512, + description: + "The exact token the check compared against. Public by construction: the owner has to publish it, so returning it is what lets a third party re-read the answer without trusting us.", + }, location: { type: "string", minLength: 1, maxLength: 2048, description: "The DNS name queried or the URL fetched, so a reader can re-check it without us.", }, - verifiedAt: { type: "string", description: "ISO timestamp of when control was proven." }, + verifiedAt: { + type: "string", + description: "ISO timestamp of when control was proven. Present only when ok is true.", + }, + reason: { + type: "string", + description: "Why control was not proven. Present only when ok is false.", + }, }, - required: ["method", "token", "location", "verifiedAt"], + required: ["ok", "method", "token", "location"], }, }, } as never) as Record; From 35e4e03c0b4645ff5a40b3b5da6c9cec80d81af4 Mon Sep 17 00:00:00 2001 From: holistis Date: Sun, 18 Oct 2026 11:20:00 +0200 Subject: [PATCH 2/2] Rewrite less than you understand, and justify it with something that is true Two findings from the second review of this branch, plus two things that did not reproduce and are written down as such rather than repaired. 1. The 503 wrapper rewrote more than it understood and less than it had to. Measured with real express: facilitator dead via res.json -> 503 (intended) facilitator dead via res.send -> 500 (missed, and @x402/express USES res.send) facilitator dead via res.sendStatus -> 500 (missed) OUR OWN 500, nothing to do with paying -> 503 (a false excuse for a real fault) The last one is the worst of the four: a buyer is told nothing was charged while the fault was somewhere else entirely, and a real error disappears behind it. All three exits are covered now, and the rewrite only applies while the payment middleware owns the response. "Which code produced this 500" is invisible from outside; "who held the response at that moment" is not, and that is the boundary drawn here. A headersSent check was missing and is in. Both middlewares moved to module scope and are exported, because inside createApp they could not be tested: the review had to copy the wrapper verbatim to measure it, and a test on a copy measures the copy. One of the new tests deliberately leaves the restore step out and asserts the hole comes back, so nobody removes it later for looking decorative. 2. "Public by construction" was false for a FAILED check, and that wording had reached the public bazaar schema. Failed means precisely that the owner did NOT publish it. The real reason returning the token discloses nothing is different and measurable: /api/challenge already hands out the same token for any endpoint and owner pair, with no proof, a looser limit (30 per 5 minutes against 10) and a weaker host check. Measured live: that route returns the ownership token AND the active-consent token, for both methods, for an arbitrary pair. So the safety rests on a coupling that was written down nowhere. It is in a test now, with the reason beside it: if someone later puts /api/challenge behind a login, verifyOwnership quietly becomes the cheaper oracle and that test fails, which is when the decision has to be made again instead of inherited. NOT REPRODUCED, and the honest version is in the notes rather than in a patch: - The review reported a facilitator timeout of 90 seconds that had not been lowered. There is no 90 anywhere in this tree. The proof fetch uses 10 seconds and maxTimeoutSeconds follows the library default, so there was nothing to lower. - Refusing to sign a receipt on a network mismatch happens AFTER settlement, so the buyer pays, gets the answer, and gets no receipt, with only a console line on our side. The refusal is right and the silence is a real gap, but the response shape has no slot for "no receipt, and here is why", and inventing one for a case that needs a facilitator settling on a chain we never advertised is not proportionate. Recorded as a named limit instead of papered over. And a proxy measurement replaced by a real one. The bazaar test asserted the whole declaration, stringified, did not contain certain words, which also searched every description. A description explaining where a reader can fetch that token for free made it fail while no field had been added. It now checks the actual field names on both the schema and the example, plus that additionalProperties is false, so the list is a boundary and not a wish. Tests: self-check-server 141 pass (was 131), endpoint-attest 73, both typecheck clean. --- packages/endpoint-attest/src/ownership.ts | 29 +- packages/self-check-server/src/app.ts | 84 +++++- .../self-check-server/src/x402-bazaar.test.ts | 20 +- .../src/x402-keuring-18-okt.test.ts | 269 +++++++++++++++++- packages/self-check-server/src/x402-seller.ts | 2 +- 5 files changed, 374 insertions(+), 30 deletions(-) diff --git a/packages/endpoint-attest/src/ownership.ts b/packages/endpoint-attest/src/ownership.ts index b2d417e..a70b21f 100644 --- a/packages/endpoint-attest/src/ownership.ts +++ b/packages/endpoint-attest/src/ownership.ts @@ -202,12 +202,29 @@ export interface OwnershipCheckDeps { * van onze eigen dienst verwerken. Een belofte in een catalogus die de route niet * nakomt is erger dan een ontbrekende belofte. * - * EN HET IS VEILIG, want dit is per constructie een publieke waarde: de eigenaar - * MOET hem zelf publiceren in DNS of in een bestand, anders kan de controle nooit - * slagen. Hem teruggeven verklapt dus niets wat niet al openbaar hoort te staan, - * en het is precies wat een derde partij nodig heeft om het antwoord zelf na te - * lezen in plaats van ons op ons woord te geloven. Hij gaat ook mee bij een - * MISLUKTE controle, want dan is hij het antwoord op "wat had er moeten staan". + * WAAROM DIT VEILIG IS, EN DE EERSTE VERSIE VAN DEZE ONDERBOUWING WAS ONJUIST. Er stond + * dat het token "per constructie een publieke waarde" is omdat de eigenaar hem zelf moet + * publiceren. Dat klopt bij een GESLAAGDE controle en is bij een MISLUKTE per definitie + * onwaar: mislukt betekent juist dat hij hem NIET gepubliceerd heeft. Onze eigen tweede + * keuring wees dat aan, en de verkeerde formulering stond toen ook al in het publieke + * Bazaar-schema van de betaalde route. + * + * De echte reden dat dit niets weggeeft is gemeten en luidt anders. `/api/challenge` + * geeft precies hetzelfde token al terug voor een WILLEKEURIG paar van eindpunt en + * eigenaar, zonder enig bewijs, met een RUIMERE limiet (30 per 5 minuten tegen 10) en een + * LOSSERE gastheercontrole. Wie het token wil hebben, heeft deze route dus niet nodig en + * neemt de goedkopere. Drie scenario's zijn nagelopen (een overgenomen subdomein, een + * zwevend dns-record, een plek waar een ander een bestand kan zetten) en geen van de + * drie geeft een aanvaller iets extra: het token bindt aan het VOLLEDIGE eindpunt, dus + * schema plus host plus poort plus pad, en publiceren kan alleen wie die plek beheert. + * + * DE KOPPELING DIE HIERUIT VOLGT, en die is nergens anders vastgelegd dan hier en in de + * test ernaast: zet iemand `/api/challenge` later achter een inlog, dan wordt deze + * functie stil het goedkoopste orakel voor hetzelfde token. Dan hoort deze afweging + * opnieuw gemaakt te worden, en niet overgenomen. + * + * Het token gaat ook mee bij een MISLUKTE controle, want dan is hij het antwoord op "wat + * had er moeten staan", en dat is wat een eigenaar nodig heeft om het te repareren. */ export type OwnershipResult = | { ok: true; method: OwnershipMethod; token: string; location: string; verifiedAt: string } diff --git a/packages/self-check-server/src/app.ts b/packages/self-check-server/src/app.ts index 6885688..60c9df5 100644 --- a/packages/self-check-server/src/app.ts +++ b/packages/self-check-server/src/app.ts @@ -185,6 +185,73 @@ function guardEndpointLiteralOnly(endpoint: string, res: Response): boolean { return true; } +/** + * Zet een storing in de afrekening om in een eerlijk antwoord, en zet de res daarna terug. + * + * OP MODULEBEREIK EN GEEXPORTEERD, met opzet. Deze twee sluiten over niets uit createApp + * heen, en binnen createApp waren ze niet te toetsen: onze eigen keuring moest de wrapper + * letterlijk kopieren om hem te kunnen meten, en een test op een kopie meet de kopie. Nu + * meet de test het echte ding. + */ +export const BETAAL_STORING = { + ok: false, + error: "settlement is temporarily unavailable, so no payment was taken and no receipt was issued", +}; + +export const betaalstoringNaar503 = (_req: Request, res: Response, next: NextFunction): void => { + const origJson = res.json.bind(res); + const origSend = res.send.bind(res); + const origSendStatus = res.sendStatus.bind(res); + + // ALLE DRIE DE UITGANGEN, niet alleen res.json. Gemeten: @x402/express antwoordt met + // res.send voor een html-lichaam, dus die weg liep gewoon om deze omzetting heen en + // leverde de koper een kale 500. res.sendStatus idem. + const naar503 = (watErGebeurde: unknown): boolean => { + if (res.headersSent) return false; + if (res.statusCode !== 500) return false; + console.error("[app] the paid route answered 500, reported to the buyer as 503:", watErGebeurde); + res.status(503); + origJson(BETAAL_STORING); + return true; + }; + + res.json = ((lichaam: unknown) => (naar503(lichaam) ? res : origJson(lichaam as never))) as typeof res.json; + res.send = ((lichaam: unknown) => (naar503(lichaam) ? res : origSend(lichaam as never))) as typeof res.send; + res.sendStatus = ((code: number) => { + if (code === 500 && !res.headersSent) { + console.error("[app] the paid route answered sendStatus 500, reported to the buyer as 503"); + res.status(503); + origJson(BETAAL_STORING); + return res; + } + return origSendStatus(code); + }) as typeof res.sendStatus; + + // En de terugzetter hangt aan dit verzoek, zodat de volgende middleware hem kan + // aanroepen zonder de sluitingen hierboven nog eens op te bouwen. + (res as unknown as { __herstelAntwoord?: () => void }).__herstelAntwoord = () => { + res.json = origJson as typeof res.json; + res.send = origSend as typeof res.send; + res.sendStatus = origSendStatus as typeof res.sendStatus; + }; + next(); +}; + +/** + * Zet de originele res-uitgangen terug zodra de betaalmiddleware heeft doorgegeven. + * + * DIT IS DE HELE REPARATIE, en zonder deze stap herschreef de wrapper ook een 500 uit + * ONZE EIGEN handler, die niets met afrekenen te maken heeft. Een echte fout kreeg dan + * een vals excuus mee, en dat is erger dan een kale 500: de koper krijgt te horen dat + * er niets is afgeschreven, terwijl de storing ergens anders zat. Van buitenaf is niet + * te zien welke code een 500 gaf, maar wel WIE het antwoord op dat moment in handen + * had, en dat is de grens die hier getrokken wordt. + */ +export const herstelAntwoord = (_req: Request, res: Response, next: NextFunction): void => { + (res as unknown as { __herstelAntwoord?: () => void }).__herstelAntwoord?.(); + next(); +}; + export function createApp(options: CreateAppOptions = {}): Express { const identity = options.identity ?? loadOwnershipSecret(); const allowedOrigins = options.allowedOrigins ?? DEFAULT_ALLOWED_ORIGINS; @@ -350,21 +417,7 @@ export function createApp(options: CreateAppOptions = {}): Express { * status 500 op de betaalde route. Al het andere gaat ongemoeid door, want een * middleware die meer herschrijft dan hij begrijpt verbergt de volgende storing. */ - const betaalstoringNaar503 = (_req: Request, res: Response, next: NextFunction): void => { - const origineel = res.json.bind(res); - res.json = ((lichaam: unknown) => { - if (res.statusCode === 500) { - console.error("[app] the paid route answered 500, reported to the buyer as 503:", lichaam); - res.status(503); - return origineel({ - ok: false, - error: "settlement is temporarily unavailable, so no payment was taken and no receipt was issued", - }); - } - return origineel(lichaam as never); - }) as typeof res.json; - next(); - }; + // Tweede net, voor het geval de middleware ooit wel GOOIT in plaats van zelf te // antwoorden. Kost niets en vangt een vorm die de bovenstaande niet ziet. @@ -394,6 +447,7 @@ export function createApp(options: CreateAppOptions = {}): Express { paidLimiter, betaalstoringNaar503, betaalmiddleware, + herstelAntwoord, async (req: Request, res: Response) => { await handleVerifyOwnership(req, res); }, diff --git a/packages/self-check-server/src/x402-bazaar.test.ts b/packages/self-check-server/src/x402-bazaar.test.ts index f2d54a8..0da5bcc 100644 --- a/packages/self-check-server/src/x402-bazaar.test.ts +++ b/packages/self-check-server/src/x402-bazaar.test.ts @@ -142,12 +142,20 @@ describe("waarheid tegenover de echte route", () => { }); it("de drie velden die de route echt vereist staan erin, en geen verzonnen vierde", () => { - const platteTekst = JSON.stringify(declareBazaarDiscovery()); - for (const veld of ["endpoint", "ownerAddress", "method"]) { - expect(platteTekst).toContain(veld); - } - expect(platteTekst).not.toContain("apiKey"); - expect(platteTekst).not.toContain("challenge"); + // GEMETEN OP DE VELDNAMEN, NIET OP DE HELE JSON, en dat is een correctie. Dit zocht + // eerder de losse woorden in JSON.stringify van de hele verklaring, dus ook in elke + // BESCHRIJVING. Een beschrijving die uitlegt waar een lezer datzelfde token gratis + // kan ophalen liet de test daardoor vallen, terwijl er geen enkel veld bij was + // gekomen. Dat is een proxy-meting: hij meet iets dat met de vraag samenhangt in + // plaats van de vraag zelf, en valt dan om op tekst die niets met velden te maken + // heeft. + const b = declareBazaarDiscovery().bazaar as any; + const schemaVelden = Object.keys(b.schema.properties.input.properties.body.properties); + expect(schemaVelden.sort()).toEqual(["endpoint", "method", "ownerAddress"]); + expect(Object.keys(b.info.input.body).sort()).toEqual(["endpoint", "method", "ownerAddress"]); + // En het schema mag geen extra velden toestaan, anders is de lijst hierboven een + // wens en geen grens. + expect(b.schema.properties.input.properties.body.additionalProperties).toBe(false); }); }); diff --git a/packages/self-check-server/src/x402-keuring-18-okt.test.ts b/packages/self-check-server/src/x402-keuring-18-okt.test.ts index ef32ea6..3347196 100644 --- a/packages/self-check-server/src/x402-keuring-18-okt.test.ts +++ b/packages/self-check-server/src/x402-keuring-18-okt.test.ts @@ -12,9 +12,11 @@ import { describe, it, expect } from "vitest"; import Ajv from "ajv"; -import { DNS_LABEL, WELL_KNOWN_PATH, verifyOwnership } from "endpoint-attest"; +import { DNS_LABEL, WELL_KNOWN_PATH, challengeToken, verifyOwnership } from "endpoint-attest"; import { declareBazaarDiscovery, readSellerConfig, FACILITATOR_URL } from "./x402-seller.js"; -import { createApp } from "./app.js"; +import express from "express"; +import type { NextFunction, Request, Response } from "express"; +import { createApp, betaalstoringNaar503, herstelAntwoord } from "./app.js"; /** Lang genoeg voor hun eigen poort: een kort geheim maakt elk token te raden. */ const GEHEIM = "een-wegwerp-attestergeheim-van-ruim-genoeg-tekens"; @@ -270,3 +272,266 @@ describe("een facilitator die niet antwoordt is onze storing, niet die van de ko }); }); }); + +describe("de 503-omzetting dekt alle uitgangen en niets meer dan dat", () => { + // GEMETEN DOOR ONZE TWEEDE KEURING, met echte express. De eerste versie van deze + // wrapper verving alleen res.json, en liet twee dingen mis gaan tegelijk: + // + // facilitator stuk via res.json -> 503 (de bedoeling) + // facilitator stuk via res.send -> 500 (gemist, en @x402/express GEBRUIKT res.send) + // facilitator stuk via res.sendStatus -> 500 (gemist) + // ONZE EIGEN 500, niets met afrekenen -> 503 (een vals excuus voor een echte fout) + // + // Dat laatste is het ergste van de vier: een koper krijgt te horen dat er niets is + // afgeschreven, terwijl de storing ergens anders zat. De grens die nu getrokken wordt + // is niet "welke code gaf deze 500" (van buitenaf onzichtbaar) maar "wie had het + // antwoord op dat moment in handen", en dat is wel te zien. + // + // Deze tests draaien tegen de ECHTE, geexporteerde middlewares. De keuring moest de + // wrapper letterlijk kopieren om hem te kunnen meten, en een test op een kopie meet de + // kopie. + + const metKeten = async ( + nepMiddleware: (req: Request, res: Response, next: NextFunction) => void, + onzeHandler: (req: Request, res: Response) => void, + werk: (basis: string) => Promise, + ) => { + const app = express(); + app.post("/betaald", betaalstoringNaar503, nepMiddleware, herstelAntwoord, onzeHandler); + const server = app.listen(0); + try { + await werk(`http://127.0.0.1:${(server.address() as { port: number }).port}`); + } finally { + server.close(); + } + }; + + const post = (basis: string) => fetch(basis + "/betaald", { method: "POST" }); + const nooitBereikt = (_req: Request, res: Response) => { + res.status(200).json({ ok: true, let: "de middleware hoorde dit niet te bereiken" }); + }; + + it("res.json met 500 wordt 503 met de ware regel", async () => { + await metKeten( + (_req, res) => { + res.status(500).json({ error: "Internal Server Error" }); + }, + nooitBereikt, + async (basis) => { + const r = await post(basis); + expect(r.status).toBe(503); + const body = (await r.json()) as { ok: boolean; error: string }; + expect(body.ok).toBe(false); + expect(body.error).toMatch(/no payment was taken/); + }, + ); + }); + + it("res.send met 500 wordt OOK 503, en dat is de weg die @x402/express neemt", async () => { + await metKeten( + (_req, res) => { + res.status(500).send("Internal Server Error"); + }, + nooitBereikt, + async (basis) => { + const r = await post(basis); + expect(r.status).toBe(503); + const body = (await r.json()) as { ok: boolean; error: string }; + expect(body.error).toMatch(/no payment was taken/); + // En de html van de ander komt niet mee naar de koper. + expect(JSON.stringify(body)).not.toContain(""); + }, + ); + }); + + it("res.sendStatus(500) wordt ook 503", async () => { + await metKeten( + (_req, res) => { + res.sendStatus(500); + }, + nooitBereikt, + async (basis) => { + const r = await post(basis); + expect(r.status).toBe(503); + expect(((await r.json()) as { error: string }).error).toMatch(/no payment was taken/); + }, + ); + }); + + it("maar een 402 of 200 van de middleware gaat ONGEMOEID door", async () => { + await metKeten( + (_req, res) => { + res.status(402).json({ x402Version: 2, accepts: [] }); + }, + nooitBereikt, + async (basis) => { + const r = await post(basis); + expect(r.status).toBe(402); + expect((await r.json()) as Record).toHaveProperty("x402Version", 2); + }, + ); + }); + + it("en ONZE EIGEN 500, na de middleware, blijft 500 in plaats van een vals excuus", async () => { + // Dit is de reparatie. De betaling is hier GELUKT (de middleware geeft door), en pas + // daarna loopt onze eigen handler stuk. "er is niets afgeschreven" zou dan een + // onwaarheid zijn, en een echte fout zou erachter verdwijnen. + await metKeten( + (_req, _res, next) => { + next(); + }, + (_req, res) => { + res.status(500).json({ error: "onze eigen fout, niets met afrekenen te maken" }); + }, + async (basis) => { + const r = await post(basis); + expect(r.status).toBe(500); + const body = (await r.json()) as { error: string }; + expect(body.error).toMatch(/onze eigen fout/); + expect(JSON.stringify(body)).not.toContain("no payment was taken"); + }, + ); + }); + + it("en dat geldt ook voor res.send en res.sendStatus na de middleware", async () => { + await metKeten( + (_req, _res, next) => { + next(); + }, + (_req, res) => { + res.status(500).send("onze eigen kale fout"); + }, + async (basis) => { + const r = await post(basis); + expect(r.status).toBe(500); + expect(await r.text()).toContain("onze eigen kale fout"); + }, + ); + await metKeten( + (_req, _res, next) => { + next(); + }, + (_req, res) => { + res.sendStatus(500); + }, + async (basis) => { + const r = await post(basis); + expect(r.status).toBe(500); + }, + ); + }); + + it("en als de kop al verstuurd is wordt er niets meer herschreven", async () => { + await metKeten( + (_req, res) => { + res.status(500); + res.write("eerste stuk"); + res.json({ error: "te laat" }); + }, + nooitBereikt, + async (basis) => { + const r = await post(basis); + // De kop stond al vast op 500, dus die blijft. Het punt is dat er geen tweede + // antwoord bovenop wordt geschreven, want dat gooit in express. + expect(r.status).toBe(500); + }, + ); + }); +}); + +describe("de terugzetter is dragend, niet decoratief", () => { + // Een test die bewijst DAT de reparatie het verschil maakt. Zonder deze toets zou + // iemand herstelAntwoord later kunnen weghalen omdat hij er overbodig uitziet, en dan + // is het gat stil terug: een eigen 500 die aan de koper gemeld wordt als + // afrekeningsstoring. Dezelfde keten, EEN middleware minder. + it("laat je de terugzetter weg, dan krijgt een eigen 500 een vals excuus", async () => { + const app = express(); + app.post( + "/betaald", + betaalstoringNaar503, + (_req: Request, _res: Response, next: NextFunction) => next(), + // GEEN herstelAntwoord hier, met opzet. + (_req: Request, res: Response) => { + res.status(500).json({ error: "onze eigen fout, niets met afrekenen te maken" }); + }, + ); + const server = app.listen(0); + try { + const poort = (server.address() as { port: number }).port; + const r = await fetch(`http://127.0.0.1:${poort}/betaald`, { method: "POST" }); + expect(r.status).toBe(503); + expect(JSON.stringify(await r.json())).toContain("no payment was taken"); + } finally { + server.close(); + } + }); +}); + +describe("waarom het token teruggeven niets weggeeft, en waar die reden aan hangt", () => { + // GEMETEN DOOR ONZE TWEEDE KEURING. De eerste onderbouwing zei dat het token "per + // constructie publiek" is omdat de eigenaar hem moet publiceren. Dat klopt bij een + // GESLAAGDE controle en is bij een MISLUKTE per definitie onwaar, want mislukt betekent + // juist dat hij hem niet gepubliceerd heeft. De echte reden is anders en meetbaar: + // /api/challenge geeft precies hetzelfde token al gratis voor een willekeurig paar. + // + // DEZE TEST IS DE PLEK WAAR DIE KOPPELING VASTLIGT. Zet iemand /api/challenge later + // achter een inlog, dan valt deze test en wordt de afweging opnieuw gemaakt in plaats + // van stilzwijgend overgenomen. + + const challengeTokenVan = (endpoint: string) => challengeToken(endpoint, TEST_ENV.X402_PAY_TO, GEHEIM); + + it("de gratis challenge-route geeft hetzelfde token als een mislukte verificatie", async () => { + const endpoint = "https://klant.example/mcp"; + const eigenaar = TEST_ENV.X402_PAY_TO; + + const viaChallenge = challengeToken(endpoint, eigenaar, GEHEIM); + const mislukt = await verifyOwnership("dns-txt", endpoint, eigenaar, GEHEIM, { + resolveTxtRecords: async () => [["iets volstrekt anders"]], + fetchImpl: async () => { + throw new Error("niet gebruikt in deze tak"); + }, + } as never); + + expect(mislukt.ok).toBe(false); + expect((mislukt as { token: string }).token).toBe(viaChallenge); + }); + + it("en die route zit niet achter een inlog, wat de hele reden is dat dit mag", async () => { + // Faalt deze, dan is /api/challenge dichtgezet en is verifyOwnership het goedkoopste + // orakel geworden. Dan hoort de onderbouwing in ownership.ts opnieuw bekeken te + // worden, en daarom staat die verwijzing hier letterlijk in de test. + const app = createApp({ + seller: null, + routableHostDeps: { lookup: async () => [{ address: "93.184.216.34", family: 4 }] } as never, + } as never); + const server = app.listen(0); + try { + const poort = (server.address() as { port: number }).port; + const r = await fetch(`http://127.0.0.1:${poort}/api/challenge`, { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ endpoint: "https://klant.example/mcp", ownerAddress: TEST_ENV.X402_PAY_TO }), + }); + expect(r.status).toBe(200); + // GEMETEN, niet aangenomen: de route geeft een blok per methode terug, met per + // blok de plek EN het token. Hij geeft bovendien ook het consent-token weg, dus de + // route is nog ruimer dan de onderbouwing nodig heeft. + const body = (await r.json()) as { + ok: boolean; + dnsTxt: { location: string; token: string }; + wellKnownFile: { location: string; token: string }; + }; + expect(body.ok).toBe(true); + expect(typeof body.dnsTxt.token).toBe("string"); + expect(body.dnsTxt.token).toBe(body.wellKnownFile.token); + // NIET vergelijken met een token uit ons eigen geheim: deze server maakt in een + // test zijn eigen willekeurige geheim, dus dat kan per definitie niet kloppen. Wat + // hier telt is dat de route OPEN staat en een token teruggeeft zonder enig bewijs, + // en dat beide methoden hetzelfde token dragen, wat laat zien dat het aan het PAAR + // hangt en niet aan de methode. + expect(body.dnsTxt.token.startsWith("tok_")).toBe(true); + } finally { + server.close(); + } + }); +}); diff --git a/packages/self-check-server/src/x402-seller.ts b/packages/self-check-server/src/x402-seller.ts index c81b243..832bef5 100644 --- a/packages/self-check-server/src/x402-seller.ts +++ b/packages/self-check-server/src/x402-seller.ts @@ -479,7 +479,7 @@ export function declareBazaarDiscovery(): Record { minLength: 16, maxLength: 512, description: - "The exact token the check compared against. Public by construction: the owner has to publish it, so returning it is what lets a third party re-read the answer without trusting us.", + "The exact token the check compared against, so a third party can re-read the answer instead of trusting us. Returning it discloses nothing new: the same token is already available from /api/challenge for any endpoint and owner, with no proof required.", }, location: { type: "string",