Merge pull request #599 from ObolNetwork/fix/integration-593plus-review

review fixes for #598: Scalar SRI pin, buy-x402 guard order, 402 token sanitization

Author: bussyjd

internal/embed/skills/buy-x402/scripts/buy.py

@@ -1632,6 +1632,16 @@ def _set_agent_default_model(model_id, auto_refill):
             file=sys.stderr,
         )
         return False
+    # Existence guard first: if we're going to refuse anyway, don't emit the
+    # auto-refill warning below — it describes a primary-model failure mode
+    # that can't happen when the default was never switched.
+    if not _litellm_has_model(alias):
+        print(
+            f"  Refusing --set-default: {alias!r} is not selectable in LiteLLM; "
+            f"leaving the agent default unchanged.",
+            file=sys.stderr,
+        )
+        return False
     # Safety: a paid primary model bricks chat once the pre-signed pool empties.
     if not (auto_refill and auto_refill.get("enabled")):
         print(
@@ -1646,14 +1656,6 @@ def _set_agent_default_model(model_id, auto_refill):
             "           Re-run with --auto-refill, or run 'process --all' on a schedule.",
             file=sys.stderr,
         )
-    # Existence guard: never point the agent at an unpublished model.
-    if not _litellm_has_model(alias):
-        print(
-            f"  Refusing --set-default: {alias!r} is not selectable in LiteLLM; "
-            f"leaving the agent default unchanged.",
-            file=sys.stderr,
-        )
-        return False
     # Primary path: native Hermes writer (atomic; per-request re-read, no restart).
     hermes_bin = _find_hermes_bin()
     if hermes_bin:

internal/serviceoffercontroller/scalar_html.go

@@ -6,14 +6,18 @@ package serviceoffercontroller
 const scalarBundleVersion = "1.34.0"
 
 // scalarBundleSRI is the Subresource Integrity hash for the pinned bundle.
-// Left empty in phase 1 — the bundle still loads, browsers just skip the
-// integrity check. Populate by running:
+// The /api page is served over the public tunnel, so the third-party Scalar
+// JS it pulls from jsdelivr must be integrity-checked: without this the
+// browser executes whatever the CDN returns, unverified. Re-derive on every
+// version bump (Renovate touches scalarBundleVersion above) by running:
 //
 //	curl -sL https://cdn.jsdelivr.net/npm/@scalar/api-reference@<version> \
 //	  | openssl dgst -sha384 -binary | base64
 //
-// and prefixing the result with `sha384-`. Re-derive on every version bump.
-const scalarBundleSRI = ""
+// and prefixing the result with `sha384-`. The hash is taken over the exact
+// (jsdelivr-minified) bytes that the pinned URL serves; it must be refreshed
+// in lockstep with scalarBundleVersion or the browser will block the script.
+const scalarBundleSRI = "sha384-tNJHhVh8smfB4VJcBxQf3Q0Soj15UqqyVJ6Q6OTwqGVEyxy57gfDLo7DGcSclH7I"
 
 // scalarHTML returns the static HTML shell served at /api. It loads the
 // pinned @scalar/api-reference bundle from jsdelivr, points it at the

internal/x402/paymentrequired.go

@@ -8,11 +8,34 @@ import (
 	"html/template"
 	"math/big"
 	"net/http"
+	"regexp"
 	"strings"
 
 	x402types "github.com/coinbase/x402/go/types"
 )
 
+// displayTokenRe is the allowed charset for ServiceOffer-sourced strings
+// (offer name, model id) that get interpolated into copy-pasteable CLI
+// commands and prompts on the 402 page. It permits the model-id / k8s-name
+// vocabulary — alphanumerics plus `. _ : / -` — and nothing else, so shell
+// metacharacters can never reach a command a reader might paste.
+var displayTokenRe = regexp.MustCompile(`^[A-Za-z0-9._:/-]+$`)
+
+// sanitizeDisplayToken guards a CR-sourced string before it lands in a
+// copy-pasteable command on the public 402 page. A ServiceOffer is
+// operator-authored, but the page is served over the public tunnel, so a
+// hostile or fat-fingered spec.model.name / metadata.name must not smuggle
+// shell metacharacters into the rendered command. Anything that isn't a clean
+// model-id/k8s-name token (including empty/whitespace) collapses to the
+// caller's placeholder.
+func sanitizeDisplayToken(s, placeholder string) string {
+	s = strings.TrimSpace(s)
+	if s == "" || !displayTokenRe.MatchString(s) {
+		return placeholder
+	}
+	return s
+}
+
 //go:embed templates/payment_required.html
 var paymentRequiredHTMLSrc string
 
@@ -229,7 +252,7 @@ func sendPaymentRequiredHTML(w http.ResponseWriter, r *http.Request, requirement
 		NetworkLabel:        networkLabel,
 		PriceDisplay:        priceDisplay,
 		PayToDisplay:        payToDisplay,
-		PayToFull:            payToFull,
+		PayToFull:           payToFull,
 		ExplorerURL:         display.ExplorerURL,
 		OfferDescription:    display.OfferDescription,
 		Skills:              display.AgentSkills,
@@ -347,14 +370,8 @@ func normalizeOfferType(t string) string {
 // raw-JSON paths, but reframed so users understand they're buying remote
 // model time, not an agent with tools/memory.
 func inferenceCopy(url string, d PaymentDisplay) typeCopy {
-	model := strings.TrimSpace(d.Model)
-	if model == "" {
-		model = "<model-id>"
-	}
-	name := strings.TrimSpace(d.OfferName)
-	if name == "" {
-		name = "remote-inference"
-	}
+	model := sanitizeDisplayToken(d.Model, "<model-id>")
+	name := sanitizeDisplayToken(d.OfferName, "remote-inference")
 
 	cmd := fmt.Sprintf(
 		"obol buy inference %s \\\n  --seller %s \\\n  --model %s \\\n  --budget 1 \\\n  --no-verify-identity",
@@ -397,7 +414,7 @@ func inferenceCopy(url string, d PaymentDisplay) typeCopy {
 // example sits next to the raw x402 JSON in the Pay-manually card to
 // make the wire shape obvious to readers walking the spec by hand.
 func agentCopy(url string, d PaymentDisplay) typeCopy {
-	model := strings.TrimSpace(d.Model)
+	model := sanitizeDisplayToken(d.Model, "")
 	modelClause := ""
 	modelLine := ""
 	if model != "" {

internal/x402/paymentrequired_test.go

@@ -173,7 +173,7 @@ func TestHTMLAware_DegradeWithoutDisplay(t *testing.T) {
 	}
 	body := w.Body.String()
 	mustContain(t, body, "Payment required")
-	mustContain(t, body, "/anything") // endpoint falls back to URL.Path
+	mustContain(t, body, "/anything")           // endpoint falls back to URL.Path
 	mustContain(t, body, "1000 (atomic units)") // price falls back to atomic units
 }
 
@@ -367,3 +367,55 @@ func mustContain(t *testing.T, haystack, needle string) {
 		t.Errorf("body does not contain %q", needle)
 	}
 }
+
+// sanitizeDisplayToken must pass real model ids / offer names through
+// untouched while collapsing anything carrying shell metacharacters to the
+// placeholder — the values land in copy-pasteable commands on the public
+// 402 page.
+func TestSanitizeDisplayToken(t *testing.T) {
+	const ph = "<model-id>"
+	cases := []struct {
+		in   string
+		want string
+	}{
+		{"qwen3.5:9b", "qwen3.5:9b"},
+		{"AEON-7", "AEON-7"},
+		{"anthropic/claude-3-5-sonnet-latest", "anthropic/claude-3-5-sonnet-latest"},
+		{"gpt-5.4", "gpt-5.4"},
+		{"  qwen3.5:9b  ", "qwen3.5:9b"}, // trimmed
+		{"", ph},
+		{"   ", ph},
+		{"x; rm -rf ~", ph},
+		{"$(whoami)", ph},
+		{"a`id`b", ph},
+		{"a && b", ph},
+		{"a|b", ph},
+		{"a$b", ph},
+		{"a\nb", ph},
+		{`a"b`, ph},
+	}
+	for _, c := range cases {
+		if got := sanitizeDisplayToken(c.in, ph); got != c.want {
+			t.Errorf("sanitizeDisplayToken(%q) = %q, want %q", c.in, got, c.want)
+		}
+	}
+}
+
+// A hostile ServiceOffer must never get its raw spec.model.name /
+// metadata.name reflected into the rendered copy-paste command.
+func TestInferenceCopy_StripsShellMetacharsFromCommand(t *testing.T) {
+	d := PaymentDisplay{
+		OfferType: "inference",
+		Model:     "x; rm -rf ~",
+		OfferName: "a && curl evil",
+	}
+	c := inferenceCopy("https://agent.example.tunnel.dev/services/x", d)
+	for _, bad := range []string{"rm -rf", "&&", "curl evil", ";"} {
+		if strings.Contains(c.PrimaryPayload, bad) {
+			t.Fatalf("hostile token leaked into command payload %q (contains %q)", c.PrimaryPayload, bad)
+		}
+	}
+	// Falls back to the safe placeholders instead.
+	mustContain(t, c.PrimaryPayload, "--model <model-id>")
+	mustContain(t, c.PrimaryPayload, "obol buy inference remote-inference")
+}