diff --git a/internal/x402/chains.go b/internal/x402/chains.go index 10b1c5b0..cf910aff 100644 --- a/internal/x402/chains.go +++ b/internal/x402/chains.go @@ -425,13 +425,17 @@ func BuildV2Requirement(chain ChainInfo, amount, recipientAddress string, maxTim // chain and settlement asset. Pass maxTimeoutSeconds=0 to fall back to // DefaultMaxTimeoutSeconds; operator-set values are clamped to MaxMaxTimeoutSeconds. // Returns an error — rather than a $0 or panicking requirement — if amount -// is not a valid non-negative decimal; callers must fail closed on error, -// never serve the route for free. +// is not a valid non-negative decimal, or if it rounds to 0 atomic units +// (e.g. "0", or a sub-atomic price like "0.0000001" at 6 decimals); callers +// must fail closed on error, never serve the route for free. func BuildV2RequirementWithAsset(chain ChainInfo, asset AssetInfo, amount, recipientAddress string, maxTimeoutSeconds int64) (x402types.PaymentRequirements, error) { atomicAmount, err := decimalToAtomic(amount, asset.Decimals) if err != nil { return x402types.PaymentRequirements{}, fmt.Errorf("invalid price %q: %w", amount, err) } + if atomicAmount == "0" { + return x402types.PaymentRequirements{}, fmt.Errorf("price %q rounds to 0 atomic units at %d decimals: refusing to advertise a $0 paid route", amount, asset.Decimals) + } return x402types.PaymentRequirements{ Scheme: "exact", Network: chain.CAIP2Network, diff --git a/internal/x402/chains_test.go b/internal/x402/chains_test.go index e33dc36c..aa44818d 100644 --- a/internal/x402/chains_test.go +++ b/internal/x402/chains_test.go @@ -293,6 +293,31 @@ func TestDecimalToAtomic_RejectsMalformedInput(t *testing.T) { } } +// TestBuildV2RequirementWithAsset_RejectsZeroAtomicAmount pins the +// Canary402 zero-price finding: decimalToAtomic only rejected parse errors +// and negative amounts, so a "0" price — or a sub-atomic price like +// "0.0000001" at 6 decimals, which rounds to 0 — sailed through and +// BuildV2RequirementWithAsset returned a payment requirement with +// Amount "0", contradicting its own doc promise to never do that. +// resolvePaidRoute must fail this option closed instead of advertising a +// free accepts[] entry. +func TestBuildV2RequirementWithAsset_RejectsZeroAtomicAmount(t *testing.T) { + asset := AssetInfo{ + Address: ChainBaseSepolia.USDCAddress, + Symbol: "USDC", + Decimals: 6, + TransferMethod: "eip3009", + } + for _, amount := range []string{"0", "0.0000001", "0.00000049"} { + t.Run(amount, func(t *testing.T) { + req, err := BuildV2RequirementWithAsset(ChainBaseSepolia, asset, amount, "0xRecipient", 0) + if err == nil { + t.Fatalf("BuildV2RequirementWithAsset(%q) = %+v, want error (0 atomic units)", amount, req) + } + }) + } +} + func TestDecimalToAtomic_ValidInput(t *testing.T) { tests := []struct { amount string diff --git a/internal/x402/verifier.go b/internal/x402/verifier.go index 88c0c3a2..7873837e 100644 --- a/internal/x402/verifier.go +++ b/internal/x402/verifier.go @@ -818,18 +818,24 @@ func buildUpstreamProxy(rule *RouteRule) (http.Handler, error) { pr.Out.URL.Path = singleJoiningSlash(target.Path, strippedPath) pr.Out.URL.RawQuery = pr.In.URL.RawQuery pr.Out.Host = target.Host - // The verifier is the browser-facing boundary: it authenticates - // the request (payment / SIWX) and re-issues it upstream under - // its own authority. Browser fetch-context headers must not leak - // through — an upstream with its own origin allowlist (hermes's - // API server 403s any Origin it has not allowlisted, and - // browsers attach Origin to every POST) would otherwise reject - // paid browser requests after the payment already verified. - pr.Out.Header.Del("Origin") - pr.Out.Header.Del("Sec-Fetch-Site") - pr.Out.Header.Del("Sec-Fetch-Mode") - pr.Out.Header.Del("Sec-Fetch-Dest") - pr.Out.Header.Del("Sec-Fetch-User") + // The verifier is the browser-facing boundary ONLY on routes it + // actually gates (payment / SIWX): it authenticates the request + // and re-issues it upstream under its own authority, so browser + // fetch-context headers must not leak through — an upstream with + // its own origin allowlist (hermes's API server 403s any Origin + // it has not allowlisted, and browsers attach Origin to every + // POST) would otherwise reject paid browser requests after the + // payment already verified. gate:free routes are the opposite: + // the verifier performs no auth and doesn't own CSRF for them, + // so stripping Origin here would instead break the upstream's + // own Origin-based CSRF defense. + if !rule.IsFree() { + pr.Out.Header.Del("Origin") + pr.Out.Header.Del("Sec-Fetch-Site") + pr.Out.Header.Del("Sec-Fetch-Mode") + pr.Out.Header.Del("Sec-Fetch-Dest") + pr.Out.Header.Del("Sec-Fetch-User") + } if rule.Async && rule.BrokerURL != "" { pr.Out.Header.Set(headerBrokerUpstreamURL, rule.UpstreamURL) pr.Out.Header.Set(headerBrokerOffer, rule.OfferNamespace+"/"+rule.OfferName) diff --git a/internal/x402/verifier_test.go b/internal/x402/verifier_test.go index 61ab40d2..4984fbaf 100644 --- a/internal/x402/verifier_test.go +++ b/internal/x402/verifier_test.go @@ -1865,3 +1865,103 @@ func TestVerifier_HandleProxy_FreeGateRule_ProxiesWithoutPayment(t *testing.T) { t.Errorf("expected 402 for paid sibling, got %d", w.Code) } } + +// TestVerifier_ResolvePaidRoute_ZeroPriceFailsClosed pins the Canary402 +// zero-price finding end to end: a paid route stored (or applied directly +// via kubectl, bypassing CLI validation) with a "0" or sub-atomic price +// must never resolve to a payable requirement. resolvePaidRoute must skip +// the option and, with no other options, fail the whole route closed (403) +// rather than advertise a $0 accepts[] entry. +func TestVerifier_ResolvePaidRoute_ZeroPriceFailsClosed(t *testing.T) { + fac := newMockFacilitator(t, mockFacilitatorOpts{}) + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + t.Fatal("upstream should not be called for an unresolvable-priced route") + })) + defer upstream.Close() + + for _, price := range []string{"0", "0.0000001"} { + t.Run(price, func(t *testing.T) { + v := newTestVerifier(t, fac.URL, []RouteRule{{ + Pattern: "/services/free-ish/*", + Price: price, + UpstreamURL: upstream.URL, + StripPrefix: "/services/free-ish", + }}) + + req := httptest.NewRequest(http.MethodGet, "/services/free-ish/data", nil) + w := httptest.NewRecorder() + v.HandleProxy(w, req) + + if w.Code != http.StatusForbidden { + t.Fatalf("price %q: status = %d, want 403 (fail closed, no payable option)", price, w.Code) + } + }) + } +} + +// TestVerifier_BuildUpstreamProxy_OriginStripping pins the F-finding that +// buildUpstreamProxy unconditionally stripped Origin/Sec-Fetch-* on every +// route class. The verifier is the auth boundary on a paid route (it +// authenticates via x402 and re-issues upstream under its own authority), +// so stripping there is correct — but a gate:free route has no verifier +// auth to protect and must let the upstream's own Origin-based CSRF +// defense see the real browser headers. +func TestVerifier_BuildUpstreamProxy_OriginStripping(t *testing.T) { + fac := newMockFacilitator(t, mockFacilitatorOpts{}) + var lastHeaders http.Header + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + lastHeaders = r.Header.Clone() + w.WriteHeader(http.StatusOK) + })) + defer upstream.Close() + + v := newTestVerifier(t, fac.URL, []RouteRule{ + { + Pattern: "/services/mix/free/*", + Gate: "free", + UpstreamURL: upstream.URL, + StripPrefix: "/services/mix", + }, + { + Pattern: "/services/mix/paid/*", + Price: "0.0001", + UpstreamURL: upstream.URL, + StripPrefix: "/services/mix", + }, + }) + + // Free route: Origin/Sec-Fetch-* must pass through untouched so the + // upstream's own CSRF check still sees them. + req := httptest.NewRequest(http.MethodPost, "/services/mix/free/submit", strings.NewReader(`{}`)) + req.Header.Set("Origin", "https://evil.example") + req.Header.Set("Sec-Fetch-Site", "cross-site") + w := httptest.NewRecorder() + v.HandleProxy(w, req) + if w.Code != http.StatusOK { + t.Fatalf("free route status = %d, want 200", w.Code) + } + if got := lastHeaders.Get("Origin"); got != "https://evil.example" { + t.Errorf("free route Origin = %q, want preserved", got) + } + if got := lastHeaders.Get("Sec-Fetch-Site"); got != "cross-site" { + t.Errorf("free route Sec-Fetch-Site = %q, want preserved", got) + } + + // Paid route: the verifier IS the auth boundary here, so browser + // fetch-context headers must be stripped before reaching upstream. + req = httptest.NewRequest(http.MethodPost, "/services/mix/paid/submit", strings.NewReader(`{}`)) + req.Header.Set("Origin", "https://evil.example") + req.Header.Set("Sec-Fetch-Site", "cross-site") + req.Header.Set("X-PAYMENT", testPaymentHeader(t)) + w = httptest.NewRecorder() + v.HandleProxy(w, req) + if w.Code != http.StatusOK { + t.Fatalf("paid route status = %d, want 200 (body %s)", w.Code, w.Body.String()) + } + if got := lastHeaders.Get("Origin"); got != "" { + t.Errorf("paid route Origin = %q, want stripped", got) + } + if got := lastHeaders.Get("Sec-Fetch-Site"); got != "" { + t.Errorf("paid route Sec-Fetch-Site = %q, want stripped", got) + } +}