Repository navigation
The facilitator is not a witness we can skip checking - #80
Merged
Merged
Conversation
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.
…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.
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
tokenizen | 35e4e03 | Commit Preview URL Branch Preview URL |
Oct 18 2026, 09:22 AM |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
tokenizen-website | 35e4e03 | Commit Preview URL Branch Preview URL |
Oct 18 2026, 09:22 AM |
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
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.
Eight findings on the live paid route, from two rounds of adversarial review of our
own server. Every one is measured; two turned out different from what the review
assumed and that is written down rather than smoothed over.
Round one
1. The published output contract promised a field the route never returned.
required: ["method","token","location","verifiedAt"], while the real result is{ ok, method, location, verifiedAt }or{ ok, method, location, reason }. Notoken, ever. Measured live at 05:19 that morning. 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.comwhile the code queries_endpoint-attest, so an agent copying it published in the wrong place.Fixed on both sides. The route returns the token it compared against, 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_URLhad no validation at all, three lines below whereX402_RESOURCE_BASEis 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.
3. Our key signed the facilitator's word about the network. 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 refuse to sign rather than
fall back, because falling back 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 thegate 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. Measured decisively: with the free
bucket exhausted (429), the paid route still runs.
5. Our own payTo address was the example
ownerAddressin the publicdeclaration, so our receiving address sat in a catalogue as an example of "the party
claiming to control this endpoint".
Round two, on the repairs
6. The 503 wrapper rewrote more than it understood and less than it had to.
Measured with real express:
res.jsonres.send@x402/expressusesres.send)res.sendStatusThe last is the worst: a buyer is told nothing was charged while the fault was
elsewhere, and a real error hides behind it. All three exits are covered now, and the
rewrite applies only while the payment middleware owns the response. Both middlewares
are exported, because inside
createAppthey could not be tested: the review had tocopy the wrapper verbatim to measure it. One new test deliberately omits the restore
step and asserts the hole comes back.
7. "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 measurable:
/api/challengealready hands out the same token for any pair, with no proof, alooser limit (30 per 5 min against 10) and a weaker host check. That coupling is in a
test now, so if someone later puts that route behind a login, the test fails and the
decision gets made again instead of inherited.
Not reproduced
uses 10 seconds and
maxTimeoutSecondsfollows the library default.built lazily and only reaches out on the first request, so the app comes up fine.
The try/catch stays as a cheap guard but is not claimed as a fix for anything
measured. What a dead facilitator did do was answer a bare
500 Internal Server Error, which is finding 6.Also
A proxy measurement replaced by a real one: the bazaar test asserted the stringified
declaration did not contain certain words, which also searched every description. It
now checks the actual field names on schema and example, plus
additionalProperties: false.