4th-review remediation (FR4-01..15): settlement-ordering billing fixes + isolation + stricter controls#460
Merged
Merged
Conversation
…solation + byte caps
ROOT CAUSE the 4th auditor correctly identified: the installed @x402/express
v2.16 runs the handler FIRST and settles AFTER, cancelling settlement for any
handler status >=400. So a 4xx/5xx (incl. a capacity 503) is NEVER charged, and a
200 is charged only if settlement then succeeds. Prior code/docs (and the 3rd
audit) wrongly assumed "settles before the handler". Corrected in CLAUDE.md.
HIGH:
- FR4-02: render-credit (F13) was a LIVE free-render bypass — it minted a bearer
credit on `paid && 503`, but a 503 is never charged. Disabled the prod flag
immediately, then REMOVED the feature entirely (module + test + middleware +
gate bypass): its premise (charged-503) cannot occur under this middleware.
- FR4-01: idempotency cached the 200 at res.json (pre-settlement) → a
settlement-failure 402 left a replayable cached 200. Now SETTLEMENT-AWARE:
capture at res.json, COMMIT on res.on("finish") only when final status is 200.
test-idempotency-settlement.js proves a non-200 is not cached.
- FR4-03: skill-pack inline `render` ran Chromium in the main process — now
routes through runOnWorker() like /api/render.
- FR4-04: tollbooth enforcing mode FAILS CLOSED without an atomic (Durable
Object) replay store, unless TOLLBOOTH_ALLOW_NON_ATOMIC_REPLAY=true.
MEDIUM:
- FR4-05: worker guard also blocks URL/DSN/DB/LEDGER/redis/mongo names (Railway
metadata URLs exempt).
- FR4-06: workerEnabled() requires BOTH url+token; assertWorkerConfig() fails
boot on a partial config; bounded worker response read (502 past cap).
- FR4-07: httpHeadersHandler cancels the body instead of draining it.
- FR4-08: x402 quote/audit/RPC reads are byte-capped (boundedText).
- FR4-09: code-run error name+value go through the aggregate output budget.
- FR4-10: tollbooth README verifier example honors the AbortSignal.
- FR4-12: worker isMain uses a resolved-path compare (Windows-safe).
FR4-11 (platform sandbox/egress) residual; FR4-13 CSP accepted; FR4-14 owner-only.
Full ledger: docs/audit-closeout-f01-f25.md §4th-review. Offline suites green;
full test-all clean (214 strict + 284 lenient). NB: a same-class prompt/embeddings
cache pre-settlement write (llm-gateway-kit) is a follow-up.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…he is settlement-aware
Same class as FR4-01, which the 4th review did not flag: the prompt cache
(cache:true) and default-on embeddings cache wrote to the store INSIDE the
handler — before @x402/express settles. A settlement-failure 402 would leave the
200 cached, and a byte-identical repeat would be served free (X-Cache: hit,
pre-paywall). Fix: the handlers stash the write on req.__deferredCache and the
route binder commits it (plus the generic route cacheSet) on res.on("finish")
only when the FINAL status is 200. Non-HTTP callers (no settlement) fall back to
a direct write. Also corrects the stale "payment settles BEFORE this handler"
comments in the gateway.
test-idempotency-settlement.js locks the wiring; full test-all clean.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…lowlist, worker-required, verifier helper)
Per owner request, tighten the three items I'd flagged as judgment-calls:
1. Worker env guard → STRICT ALLOWLIST (P1.2). Secret-shaped names
(KEY|SECRET|TOKEN|MNEMONIC|PASSWORD|CREDENTIAL|DSN|LEDGER|DATABASE|REDIS|
POSTGRES|MONGO|MYSQL|CONN_STRING) are blocked in EVERY environment. The
deny-anything-not-allowlisted strictness is enforced in the controlled
runtime only — on Railway (RAILWAY_ENVIRONMENT present) or WORKER_STRICT_ENV
— where the container env is minimal; in dev/CI (env full of editor/OS noise:
a real run surfaced 25+ benign vars) it applies the shape denylist only, so
the worker still boots locally. Railway health-gates the deploy, so a strict
false positive fails the NEW deploy (old worker keeps serving), not prod.
(Dropped bare "PRIVATE"/"CONNECTION" — false-positived on RAILWAY_PRIVATE_DOMAIN
and MCP_CONNECTION_NONBLOCKING.)
2. RENDER_WORKER_REQUIRED (P1.1): when set, assertWorkerConfig() hard-fails boot
if the worker isn't fully configured (URL+token), so prod can DEMAND isolation
instead of silently running browser/media in-process.
3. First-party verifier helper (P1.5): x402VerifierFromExpress(paymentMiddleware,
{timeoutMs}) OWNS timeout + cancellation — honors the gate's AbortSignal
(opts.signal, the correct 2nd-arg location; the old README example used a wrong
3rd-arg signal), backstops with its own timeout, and a settlement that
completes AFTER the abort cannot flip the result to granted (no charged-then-
denied). README updated to use it; documents the @x402/express no-cancel-hook
limitation.
Tests: worker-isolation 48 (strict+dev modes, RENDER_WORKER_REQUIRED),
test-tollbooth-verifier 9 (grant/deny/abort/timeout/late-settle/throw). tollbooth
own suites + full test-all clean.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Remediates the 4th independent security review (A402-FR4-01..15).
Root cause the review correctly identified
The installed
@x402/expressv2.16 runs the handler FIRST, then settles, and cancels settlement for any handlerstatusCode >= 400. So a 4xx/5xx (incl. a capacity 503) is never charged, and a 200 is charged only if settlement then succeeds. Prior code/docs (and the 3rd audit) wrongly assumed "settles before the handler". Corrected inCLAUDE.md.High
res.json(pre-settlement); a settlement-failure 402 left a replayable cached 200. Now settlement-aware: capture atres.json, commit onres.on("finish")only when the final status is 200.renderran Chromium in the main process; now routes through the secretless worker.TOLLBOOTH_ALLOW_NON_ATOMIC_REPLAY=true).Medium
FR4-05 worker guard blocks URL/DSN/DB/ledger names; FR4-06 worker config is atomic (URL+token) + bounded response; FR4-07
httpHeadersHandlercancels the body; FR4-08 x402 quote/audit/RPC byte-capped; FR4-09 code-run error value budgeted; FR4-10 tollbooth verifier honors the AbortSignal; FR4-12 workerisMainis Windows-safe.Self-found (same class)
Stricter-form tightening (owner-requested)
RENDER_WORKER_REQUIRED— hard-fails boot if isolation is expected but the worker isn't configured.x402VerifierFromExpress— first-party verifier that owns timeout/cancellation; a late settle after abort can't grant.Tests
New/updated offline suites wired into CI:
test-idempotency-settlement.js,test-worker-isolation.js(48),test-tollbooth-verifier.js(9),test-code-run-caps.js. Fulltest-allclean (214 strict + 284 lenient). Worker + main redeployed and verified; F13 removed and off; prod healthy.Residual / owner-only
FR4-11 platform sandbox/egress (documented), FR4-13 CSP (accepted), FR4-14 GitHub ruleset / env / DNS (owner). Ledger:
docs/audit-closeout-f01-f25.md§4th-review.