diff --git a/cmd/obol/sell.go b/cmd/obol/sell.go index de464453..ae6886ee 100644 --- a/cmd/obol/sell.go +++ b/cmd/obol/sell.go @@ -3855,6 +3855,10 @@ func resolvePriceTable(cmd *cli.Command, allowPerHour bool) (schemas.PriceTable, switch { case perRequest != "": + if err := validate.Price(perRequest); err != nil { + return schemas.PriceTable{}, fmt.Errorf("invalid --price/--per-request value %q: %w", perRequest, err) + } + return schemas.PriceTable{PerRequest: perRequest}, nil case perMTok != "": if _, err := schemas.ApproximateRequestPriceFromPerMTok(perMTok); err != nil { diff --git a/cmd/obol/sell_test.go b/cmd/obol/sell_test.go index af522b20..c3cb9c87 100644 --- a/cmd/obol/sell_test.go +++ b/cmd/obol/sell_test.go @@ -2623,3 +2623,66 @@ func TestOfferPathCollisionInList_Hostname(t *testing.T) { t.Fatalf("self-update must pass: %v", err) } } + +// TestResolvePriceTable_RejectsMalformedPrice is the regression guard for +// the Canary402 finding: an EU-style comma decimal ("0,01") or other +// malformed --price value used to sail through resolvePriceTable with zero +// validation, silently mispricing the offer at $0 (decimalToAtomic parses +// "0" and drops the rest) or panicking the verifier hot path on every +// request (nil *big.Float from a discarded Parse error). --per-mtok/--per-hour +// already validate via ApproximateRequestPriceFrom*; --price/--per-request +// must too. +func TestResolvePriceTable_RejectsMalformedPrice(t *testing.T) { + badPrices := []string{"0,01", "abc", "$0.01", "-1", ""} + + for _, price := range badPrices { + t.Run(price, func(t *testing.T) { + var resolveErr error + cmd := &cli.Command{ + Name: "x", + Flags: []cli.Flag{ + &cli.StringFlag{Name: "price"}, + &cli.StringFlag{Name: "per-request"}, + &cli.StringFlag{Name: "per-mtok"}, + &cli.StringFlag{Name: "per-hour"}, + }, + Action: func(_ context.Context, c *cli.Command) error { + _, resolveErr = resolvePriceTable(c, true) + return nil + }, + } + if err := cmd.Run(context.Background(), []string{"x", "--price", price}); err != nil { + t.Fatalf("cmd.Run: %v", err) + } + if resolveErr == nil { + t.Fatalf("resolvePriceTable(--price=%q) expected error, got nil", price) + } + }) + } + + // A well-formed price must still pass. + var resolveErr error + var got schemas.PriceTable + cmd := &cli.Command{ + Name: "x", + Flags: []cli.Flag{ + &cli.StringFlag{Name: "price"}, + &cli.StringFlag{Name: "per-request"}, + &cli.StringFlag{Name: "per-mtok"}, + &cli.StringFlag{Name: "per-hour"}, + }, + Action: func(_ context.Context, c *cli.Command) error { + got, resolveErr = resolvePriceTable(c, true) + return nil + }, + } + if err := cmd.Run(context.Background(), []string{"x", "--price", "0.01"}); err != nil { + t.Fatalf("cmd.Run: %v", err) + } + if resolveErr != nil { + t.Fatalf("resolvePriceTable(--price=0.01): unexpected error %v", resolveErr) + } + if got.PerRequest != "0.01" { + t.Fatalf("PerRequest = %q, want 0.01", got.PerRequest) + } +} diff --git a/internal/inference/gateway.go b/internal/inference/gateway.go index 8a917b7f..fcbe574b 100644 --- a/internal/inference/gateway.go +++ b/internal/inference/gateway.go @@ -177,7 +177,10 @@ func (g *Gateway) buildHandler(upstreamURL string) (http.Handler, error) { // Create x402 payment requirement. The standalone gateway has no // operator surface for MaxTimeoutSeconds yet, so pass 0 to fall back // to DefaultMaxTimeoutSeconds. - requirement := x402pkg.BuildV2Requirement(g.config.Chain, g.config.PricePerRequest, g.config.WalletAddress, 0) + requirement, err := x402pkg.BuildV2Requirement(g.config.Chain, g.config.PricePerRequest, g.config.WalletAddress, 0) + if err != nil { + return nil, fmt.Errorf("invalid price %q: %w", g.config.PricePerRequest, err) + } // Configure x402 ForwardAuth middleware. The bazaar discovery extension // advertises the chat-completions invocation shape on every 402 so diff --git a/internal/x402/chains.go b/internal/x402/chains.go index f12b1bc7..eb053e6a 100644 --- a/internal/x402/chains.go +++ b/internal/x402/chains.go @@ -219,8 +219,22 @@ func ResolveChainInfo(name string) (ChainInfo, error) { // decimalToAtomic converts a decimal token amount (e.g. "0.001") to atomic // units using big.Float with 128-bit precision to avoid floating-point // truncation (e.g. 0.001 * 1e6 must produce 1000, not 999). -func decimalToAtomic(amount string, decimals int) string { - amountFloat, _, _ := new(big.Float).SetPrec(128).Parse(amount, 10) +// +// Returns an error rather than silently mispricing or panicking on +// malformed input (e.g. "0,01" EU-style decimals, "abc", "", "$0.01") — +// amount ultimately comes from operator/CLI input and, via a ServiceOffer +// applied directly to the cluster, can bypass CLI validation entirely. +func decimalToAtomic(amount string, decimals int) (string, error) { + amountFloat, _, err := new(big.Float).SetPrec(128).Parse(amount, 10) + if err != nil { + return "", fmt.Errorf("invalid decimal amount %q: %w", amount, err) + } + if amountFloat == nil { + return "", fmt.Errorf("invalid decimal amount %q", amount) + } + if amountFloat.Sign() < 0 { + return "", fmt.Errorf("amount must be non-negative: %q", amount) + } multiplier := new(big.Float).SetPrec(128).SetInt( new(big.Int).Exp(big.NewInt(10), big.NewInt(int64(decimals)), nil), ) @@ -228,7 +242,7 @@ func decimalToAtomic(amount string, decimals int) string { // Add 0.5 before truncating to int so we round to nearest. atomicFloat.Add(atomicFloat, new(big.Float).SetPrec(128).SetFloat64(0.5)) atomicInt, _ := atomicFloat.Int(nil) - return atomicInt.String() + return atomicInt.String(), nil } // DefaultAsset returns the default settlement asset for a chain. @@ -368,18 +382,27 @@ func ClampMaxTimeoutSeconds(n int64) int64 { // BuildV2Requirement creates a v2 PaymentRequirements for USDC payment on the // given chain. amount is the decimal USDC amount (e.g. "0.001" = $0.001). -func BuildV2Requirement(chain ChainInfo, amount, recipientAddress string, maxTimeoutSeconds int64) x402types.PaymentRequirements { +// Returns an error — rather than a $0 or panicking requirement — if amount +// is not a valid non-negative decimal. +func BuildV2Requirement(chain ChainInfo, amount, recipientAddress string, maxTimeoutSeconds int64) (x402types.PaymentRequirements, error) { return BuildV2RequirementWithAsset(chain, chain.DefaultAsset(), amount, recipientAddress, maxTimeoutSeconds) } // BuildV2RequirementWithAsset creates a v2 PaymentRequirements for the given // chain and settlement asset. Pass maxTimeoutSeconds=0 to fall back to // DefaultMaxTimeoutSeconds; operator-set values are clamped to MaxMaxTimeoutSeconds. -func BuildV2RequirementWithAsset(chain ChainInfo, asset AssetInfo, amount, recipientAddress string, maxTimeoutSeconds int64) x402types.PaymentRequirements { +// 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. +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) + } return x402types.PaymentRequirements{ Scheme: "exact", Network: chain.CAIP2Network, - Amount: decimalToAtomic(amount, asset.Decimals), + Amount: atomicAmount, Asset: asset.Address, PayTo: recipientAddress, MaxTimeoutSeconds: int(ClampMaxTimeoutSeconds(maxTimeoutSeconds)), @@ -388,5 +411,5 @@ func BuildV2RequirementWithAsset(chain ChainInfo, asset AssetInfo, amount, recip "version": asset.EIP712Version, "assetTransferMethod": asset.TransferMethod, }, - } + }, nil } diff --git a/internal/x402/chains_test.go b/internal/x402/chains_test.go index 65835e2a..2aab1bb0 100644 --- a/internal/x402/chains_test.go +++ b/internal/x402/chains_test.go @@ -200,24 +200,33 @@ func TestBuildV2RequirementWithAsset_HonorsMaxTimeoutSeconds(t *testing.T) { EIP712Version: "2", } - got := BuildV2RequirementWithAsset(ChainBaseSepolia, asset, "0.001", "0xRecipient", 0) + got, err := BuildV2RequirementWithAsset(ChainBaseSepolia, asset, "0.001", "0xRecipient", 0) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } if got.MaxTimeoutSeconds != int(DefaultMaxTimeoutSeconds) { t.Errorf("zero spec value should map to default %d, got %d", DefaultMaxTimeoutSeconds, got.MaxTimeoutSeconds) } - got = BuildV2RequirementWithAsset(ChainBaseSepolia, asset, "0.001", "0xRecipient", 1800) + got, err = BuildV2RequirementWithAsset(ChainBaseSepolia, asset, "0.001", "0xRecipient", 1800) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } if got.MaxTimeoutSeconds != 1800 { t.Errorf("operator-set 1800 should reach the 402 verbatim, got %d", got.MaxTimeoutSeconds) } - got = BuildV2RequirementWithAsset(ChainBaseSepolia, asset, "0.001", "0xRecipient", MaxMaxTimeoutSeconds+1000) + got, err = BuildV2RequirementWithAsset(ChainBaseSepolia, asset, "0.001", "0xRecipient", MaxMaxTimeoutSeconds+1000) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } if got.MaxTimeoutSeconds != int(MaxMaxTimeoutSeconds) { t.Errorf("runaway value should clamp to cap %d, got %d", MaxMaxTimeoutSeconds, got.MaxTimeoutSeconds) } } func TestBuildV2RequirementWithAsset(t *testing.T) { - req := BuildV2RequirementWithAsset(ChainEthereumMainnet, AssetInfo{ + req, err := BuildV2RequirementWithAsset(ChainEthereumMainnet, AssetInfo{ Address: "0x0B010000b7624eb9B3DfBC279673C76E9D29D5F7", Symbol: "OBOL", Decimals: 18, @@ -225,6 +234,9 @@ func TestBuildV2RequirementWithAsset(t *testing.T) { EIP712Name: "Obol Network", EIP712Version: "1", }, "0.001", "0xRecipient", 0) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } if req.Amount != "1000000000000000" { t.Fatalf("Amount = %q, want 1000000000000000", req.Amount) @@ -239,3 +251,41 @@ func TestBuildV2RequirementWithAsset(t *testing.T) { t.Fatalf("name/version = %v/%v", req.Extra["name"], req.Extra["version"]) } } + +// TestDecimalToAtomic_RejectsMalformedInput is the regression guard for the +// Canary402 finding: decimalToAtomic used to discard the big.Float Parse +// error, so "0,01" (EU comma decimal) silently parsed as "0" (mispricing +// the route at $0) and "abc"/""/" " left amountFloat nil, panicking the +// next line's Mul(nil, ...) on every request in the verifier hot path. +func TestDecimalToAtomic_RejectsMalformedInput(t *testing.T) { + for _, amount := range []string{"abc", "", " ", "0,01", "$0.01", "-1"} { + t.Run(amount, func(t *testing.T) { + if _, err := decimalToAtomic(amount, 6); err == nil { + t.Fatalf("decimalToAtomic(%q) expected error, got nil", amount) + } + }) + } +} + +func TestDecimalToAtomic_ValidInput(t *testing.T) { + tests := []struct { + amount string + decimals int + want string + }{ + {"0.01", 6, "10000"}, + {"1.5", 6, "1500000"}, + {"0.001", 18, "1000000000000000"}, + } + for _, tc := range tests { + t.Run(tc.amount, func(t *testing.T) { + got, err := decimalToAtomic(tc.amount, tc.decimals) + if err != nil { + t.Fatalf("decimalToAtomic(%q, %d): unexpected error %v", tc.amount, tc.decimals, err) + } + if got != tc.want { + t.Fatalf("decimalToAtomic(%q, %d) = %q, want %q", tc.amount, tc.decimals, got, tc.want) + } + }) + } +} diff --git a/internal/x402/forwardauth_test.go b/internal/x402/forwardauth_test.go index be68d887..23a786d5 100644 --- a/internal/x402/forwardauth_test.go +++ b/internal/x402/forwardauth_test.go @@ -79,9 +79,11 @@ func validPaymentHeader() string { } func testRequirements() []x402types.PaymentRequirements { - return []x402types.PaymentRequirements{ - BuildV2Requirement(ChainBaseSepolia, "0.001", "0xWallet", 0), + req, err := BuildV2Requirement(ChainBaseSepolia, "0.001", "0xWallet", 0) + if err != nil { + panic(err) // hardcoded valid literal — an error here is a test bug } + return []x402types.PaymentRequirements{req} } func TestForwardAuth_NoPayment_Returns402_AdvertisesExtensions(t *testing.T) { diff --git a/internal/x402/verifier.go b/internal/x402/verifier.go index e4fc8c86..88c0c3a2 100644 --- a/internal/x402/verifier.go +++ b/internal/x402/verifier.go @@ -591,7 +591,17 @@ func (v *Verifier) resolvePaidRoute(cfg *PricingConfig, rule *RouteRule) (*match wallet = opt.PayTo } asset := ResolveAssetInfoForPayment(chain, opt) - req := BuildV2RequirementWithAsset(chain, asset, opt.Price, wallet, opt.MaxTimeoutSeconds) + req, err := BuildV2RequirementWithAsset(chain, asset, opt.Price, wallet, opt.MaxTimeoutSeconds) + if err != nil { + // Skip this option rather than serving it free or panicking — + // a malformed stored price (e.g. a ServiceOffer applied + // directly via kubectl, bypassing CLI validation) must never + // produce a payable route. Other options may still be payable; + // if none are, the len(reqs)==0 check below fails the route + // closed. + log.Printf("x402-verifier: invalid price %q for route %q option %d: %v", opt.Price, rule.Pattern, i, err) + continue + } mergeAgentExtras(&req, rule) reqs = append(reqs, req) optLabels = append(optLabels, labelsForPaymentOption(rule, opt)) diff --git a/internal/x402/verifier_test.go b/internal/x402/verifier_test.go index 3226df91..61ab40d2 100644 --- a/internal/x402/verifier_test.go +++ b/internal/x402/verifier_test.go @@ -180,6 +180,30 @@ func TestVerifier_PaidRoute_NoPayment_Returns402(t *testing.T) { } } +// TestVerifier_MalformedStoredPrice_FailsClosed is the resolvePaidRoute seam +// for the Canary402 finding: a ServiceOffer applied directly via kubectl +// bypasses CLI (`validate.Price`) entirely, so a malformed stored price +// (e.g. EU comma decimal "0,01", or "abc") must never reach the buyer as a +// free ($0) or crashing route. resolvePaidRoute must skip the unresolvable +// option; with no other options the route fails closed with 403, not a 402 +// advertising a free/mispriced requirement. +func TestVerifier_MalformedStoredPrice_FailsClosed(t *testing.T) { + fac := newMockFacilitator(t, mockFacilitatorOpts{}) + v := newTestVerifier(t, fac.URL, []RouteRule{ + {Pattern: "/rpc/*", Price: "0,01"}, + }) + + req := httptest.NewRequest(http.MethodPost, "/verify", nil) + req.Header.Set("X-Forwarded-Uri", "/rpc/mainnet") + req.Header.Set("X-Forwarded-Host", "obol.stack") + w := httptest.NewRecorder() + v.HandleVerify(w, req) + + if w.Code != http.StatusForbidden { + t.Errorf("expected 403 fail-closed for malformed stored price, got %d", w.Code) + } +} + func TestVerifier_PaidRoute_ValidPayment_Returns200(t *testing.T) { fac := newMockFacilitator(t, mockFacilitatorOpts{}) v := newTestVerifier(t, fac.URL, []RouteRule{