fix(route-surface): security + correctness + x402 hardening for #717 (F1,F1-HMAC,F3,F3+,F4,F5,F6,F7,F8,P1b) - #718
Closed
bussyjd wants to merge 9 commits into
Closed
Conversation
bussyjd
changed the title
…(F4, F5) - Require Content-Type: application/json and reject cross-site Sec-Fetch-Site on /auth/verify so a cross-origin form cannot mint an attacker-wallet session in a victim browser (login-CSRF / session fixation). - Set the session cookie SameSite=Strict (was Lax); the post-sign-in redirect is same-origin so Strict never blocks the real flow. - Reject a backslash in sanitizeNextPath: Go url.Parse keeps '\' as a path byte but browsers normalize it to '/', so /\evil.com was an open redirect.
An offer whose non-free routes are all exact registered no paid prefix, so an undeclared sibling under the offer base 200-free-passed in HandleVerify — contradicting the route-table contract that undeclared paths are refused once any route is declared. Register the offer base (StripPrefix) as a tracked paid prefix whenever the offer has a non-free rule. Declared routes still return from matchRoute before the prefix check, so only genuinely-undeclared siblings fail closed. Adds CSRF-guard + exact-only-fail-closed tests.
…y, not declaration order (F6) (cherry picked from commit 36dfe7960de5d1d5923e9e1f0dd354c5653012f7)
…ffixes (F8) ReservedPathConflict was only ever checked against an offer's root EffectivePath(), in both the serviceoffer-controller reconcile and the CLI preflight. Individual spec.routes[].path entries went unchecked, so a route could be declared at a reserved verifier surface — most notably "/auth" or "/auth/verify", the SIWX sign-in endpoints the verifier serves for gate:auth offers — and silently shadow it. Add ReservedRoutePathConflict (types.go), which reuses the shared reservedPathRoots denylist plus "/auth", without the root check's "/" and "/services" special-casing (a route's "/" is the offer's own root route, not the storefront root). Wire it into the controller reconcile (new ReservedPath-reason branch before findHostnameConflict, mirroring the root-path branch) and into the CLI preflight (reservedRoutePathCollision, handling both the []map[string]any shape from --route flags and the []any/json.Unmarshal shape from --from-json). (cherry picked from commit 01f1f26dba191fadedba88e3c4922effff1b4cd3)
…don't steamroll bound origins (P1b) offerBoundHostnames() collapsed two different situations into the same nil return: "queried fine, zero offers are hostname-bound" and "the kubectl query itself failed" (cluster down, CRD missing). CreateStorefront treated both identically — skip the bound-hostname filter — so a transient kubectl error made the catch-all storefront route silently reclaim every tracked hostname, including ones already bound to a live ServiceOffer's dedicated origin. offerBoundHostnames now returns (map[string]bool, error), with a query failure surfaced as a real error instead of nil. CreateStorefront checks that error first and, on failure, logs a warning and returns without touching the storefront route at all — leaving whatever hostname set is already published in place — rather than proceeding as if nothing were bound. (cherry picked from commit cbeb2b76feb453e83916f81c441a6e63970f3fb9)
job-broker had no NetworkPolicy: any in-cluster pod could POST directly to job-broker.x402.svc with forged X-Obol-Upstream-* headers, since the broker trusts those headers to know where to replay a request and cannot itself re-verify payment (SSRF / free-compute risk). Add a NetworkPolicy that selects the job-broker pods and restricts Ingress to only the x402-verifier pods (the sole intended caller) on port 8090. Egress is left unrestricted by design — the broker must reach arbitrary seller upstreams. No HMAC (separate follow-up) and no image digest changes. (cherry picked from commit a96333e24c932d9ff29a347a52f265302e6fcf10)
Additively accept the spec transport (docs.x402.org/extensions/sign-in-with-x): a base64-JSON payload in the SIGN-IN-WITH-X header, alongside the existing Authorization: SIWX and Bearer/cookie forms. Uses the x402 Go SDK's signinwithx.ParseHeader + CreateMessage to rebuild the canonical EIP-4361 message, then runs it through the same VerifyMessage path (domain binding, freshness window, nonce-replay cache, EIP-191 EOA recovery) as the native form — so a stock x402 client interoperates without a bespoke credential shape. EVM/EOA only, matching the rest of the SIWx surface; Solana/EIP-1271 payloads get a clear error. Adds a signed round-trip + wrong-domain test.
bussyjd
force-pushed
the
fix/route-surface-hardening
branch
from
July 8, 2026 08:59
a0b5539 to
99c6a7e
Compare
bussyjd
changed the title
…depth) The NetworkPolicy (shipped) closes the network path to the job-broker; this adds the app-layer guarantee the review asked for. When JOB_BROKER_HMAC_SECRET is set, the verifier signs the security-critical contract fields (upstreamURL, offer, upstreamAuth) into X-Obol-Broker-Sig and the broker rejects any submit whose signature doesn't match — so a pod that slips past the network layer still can't forge an arbitrary-URL, attacker-credentialed job. Empty secret = disabled on both sides (NetworkPolicy-only), so installs without the key are unaffected. Secret + optional env wired into both containers; forged/missing/ valid-sig submits covered by a broker test.
…nge (F3 follow-up) The machine (JSON) 401 challenge now carries the x402 extensions[sign-in-with-x] block — message info with a freshly-minted server nonce + issuedAt, the supported EVM chains (eip191/EOA), and the SDK's canonical schema — so a cold stock x402 client can construct the credential without prior knowledge of obol. The nonce is stateless (obol accepts client nonces, so advertising a fresh one is additive, not a tightening). Completes the F3 intake shipped earlier: obol now both accepts the SIGN-IN-WITH-X transport and tells clients how to build it.
bussyjd
force-pushed
the
fix/route-surface-hardening
branch
from
July 8, 2026 10:12
5de1836 to
ae7dbcb
Compare
This was referenced Jul 8, 2026
Contributor
Author
|
Superseded — all commits from this branch (F1/F1-HMAC/F3/F3+/F4/F5/F6/F7/F8/P1b hardening, including #717 which it was stacked on) landed on |
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.
Hardening fixes for the findings in #717's review, based on
feat/route-surface-gates. Six commits, grouped by category — all touch disjoint files and are independently revertable, so this can merge whole or be split into per-category PRs on request. Every finding was verified against the x402 spec (sign-in-with-x,offer-receipt, core v2) and re-checked by a second model.Build + test:
go build ./...clean;go testgreen acrossinternal/x402,internal/serviceoffercontroller,internal/monetizeapi,internal/tunnel,internal/embed. New tests fail-before / pass-after each fix.Security (the two merge-blockers + one more)
NetworkPolicypinningjob-brokeringress to thex402-verifierpod (port 8090). The broker only checked header presence, and any in-cluster pod could POST forgedX-Obol-Upstream-*for a free SSRF/arbitrary-fetch relay. (HMAC on the verifier→broker hop is the recommended defense-in-depth follow-up — left out of this security fix as it touches both sides + a Secret; happy to add.)/auth/verify. RequireContent-Type: application/json, reject cross-siteSec-Fetch-Site, and set the session cookieSameSite=Strict. Blocks thetext/plain-form session-fixation vector; the browser sign-in page already sends JSON, so nothing legitimate breaks. Adds CSRF-guard tests.sanitizeNextPathnow rejects\(Go'surl.Parsekeeps it as a path byte; browsers normalize it to/, so/\evil.comescaped off-origin).Route-surface correctness
StripPrefix) as a tracked paid prefix whenever the offer has a non-free rule, so undeclared siblings 403 instead of 200 in ForwardAuth. Declared routes still resolve first, so no behavior change for real routes.spec.routes[].path(not just the offer root) and add the verifier-owned/authsuffixes, refusing shadowing offers at admission with a clear condition/CLI error.x402 spec-fidelity
sign-in-with-xspec transport. Additively accept the standardSIGN-IN-WITH-Xbase64-JSON header (CAIP-122 / EIP-4361) alongside the existingAuthorization: SIWXand Bearer/cookie forms, so a stock x402 client interoperates. Uses the x402 Go SDK'ssigninwithx.ParseHeader+CreateMessageto rebuild the canonical EIP-4361 message, then runs it through the sameVerifyMessagepath (domain binding, freshness window, nonce-replay cache, EIP-191 EOA recovery) — no bespoke crypto. EVM/EOA only, matching the rest of the surface. Adds a signed round-trip test. (Remaining for full interop: advertise thesign-in-with-xextension block in the 402 challenge — a larger change touching the challenge/nonce model, noted for a follow-up.)Reconcile safety
offerBoundHostnamesfails safe. It returnednilon any kubectl error, so a transient failure duringstack upre-seized bound per-offer origins under the storefront catch-all. Now distinguishes "query failed" from "zero bound" and leaves existing routes untouched on failure.Follow-ups now folded in
JOB_BROKER_HMAC_SECRETis set, the verifier signs the security-critical contract fields (upstreamURL,offer,upstreamAuth) intoX-Obol-Broker-Sigand the broker rejects any mismatched submit — so a pod that slips past the network layer still can't forge an arbitrary-URL, attacker-credentialed job. Empty secret = disabled on both sides (NetworkPolicy-only), so installs without the key are unaffected. Secret +optionalenv wired into both containers; forged/missing/valid-sig submits tested.sign-in-with-xin the challenge. The machine 401 challenge now carries theextensions[sign-in-with-x]block (fresh server nonce, supported EVM chains, SDK canonical schema), so a cold stock client can build the credential without prior knowledge of obol. Completes the F3 intake — obol now both accepts the spec transport and tells clients how to construct it. Stateless (obol accepts client nonces), so additive.Not in this PR (genuine blockers / decisions for the author)
callerMayRead:X-Verified-Wallet == payer), so that half is done. The remaining piece — anoffer-receiptsigned receipt on successful delivery — needs a receipt-signer-key decision (payTo signs / a stack service key / verifier-via-remote-signer) plus hand-rolled EIP-712 (no SDK package). Surfaced as a design decision rather than a guessed implementation.job-brokerimage isn't published to ghcr yet (no remote manifest), so there is no digest to pin. This is inherently a release-pipeline step, not a code change — pin when the image is first built and pushed.https://claude.ai/code/session_01VquWN9UMaSHH7MHGcG8bw1