From 99af155aff033143dd19a83ee30e5483b177b3fe Mon Sep 17 00:00:00 2001 From: bussyjd Date: Thu, 16 Jul 2026 20:47:22 +0400 Subject: [PATCH] fix(chat-widget): harden /chat against clickjacking, stale-bundle drift, and withdraw race - Serve /chat with Content-Security-Policy: frame-ancestors 'self' (new addResponseHeader merges it into the existing ResponseHeaderModifier filter, since Gateway API disallows repeating a core filter type per rule). The page holds a hot session key and signs USDC transfers, so a cross-origin iframe could previously clickjack Connect/Fund/Withdraw/Send. - Add TestChatVendorVersionMatchesBundle: computes sha256 of the embedded chat-vendor.js and asserts chat.html's hardcoded ?v= prefix matches, so a vendor rebuild without a version bump now fails CI instead of silently serving returning visitors the old payment-signing bundle for up to a year (the bundle is cached 1-year immutable). - withdraw() now awaits the transfer receipt (like fund() already does) before declaring success and refreshing, and the withdraw/send guards are now mutual: withdraw() also checks `busy`, and the composer submit handler also checks `withdrawing`, closing the race where a withdrawal and a paid message could spend the same session balance concurrently. Claude-Session: https://claude.ai/code/session_014YjPMViNrZ7zBVgUQzwEKk --- .../serviceoffercontroller/assets/chat.html | 10 +++-- .../serviceoffercontroller/hostoffer_test.go | 37 +++++++++++++++++++ internal/serviceoffercontroller/render.go | 26 ++++++++++++- 3 files changed, 69 insertions(+), 4 deletions(-) diff --git a/internal/serviceoffercontroller/assets/chat.html b/internal/serviceoffercontroller/assets/chat.html index 3ce3ddbf..63f8284c 100644 --- a/internal/serviceoffercontroller/assets/chat.html +++ b/internal/serviceoffercontroller/assets/chat.html @@ -311,7 +311,9 @@

{{.Title}}

], }]; async function withdraw() { - if (!burner || !wallet || balance <= 0n || withdrawing) return; + // busy = a paid message is mid-flight and may be about to spend this same + // balance via payFetch; withdrawing itself blocks a send below. + if (!burner || !wallet || balance <= 0n || withdrawing || busy) return; withdrawing = true; const amt = balance; try { @@ -339,11 +341,13 @@

{{.Title}}

const r = sig.slice(0, 66); const s = "0x" + sig.slice(66, 130); const v = parseInt(sig.slice(130, 132), 16); - await wallet.writeContract({ + const hash = await wallet.writeContract({ account: mainAddr, chain, address: asset, abi: TWA_ABI, functionName: "transferWithAuthorization", args: [burner.address, mainAddr, amt, 0n, validBefore, nonce, v, r, s], }); + status("withdrawal sent, waiting for confirmation…", true); + await pub.waitForTransactionReceipt({ hash }); say("sys", "Withdrew $" + formatUnits(amt, 6) + " to " + short(mainAddr)); await refreshBalance(); status(""); @@ -372,7 +376,7 @@

{{.Title}}

ev.preventDefault(); const ta = $("input"); const text = ta?.value.trim(); - if (!text || !payFetch || busy) return; + if (!text || !payFetch || busy || withdrawing) return; busy = true; ta.value = ""; ta.disabled = true; say("user", text); messages.push({ role: "user", content: text }); diff --git a/internal/serviceoffercontroller/hostoffer_test.go b/internal/serviceoffercontroller/hostoffer_test.go index d3164d1c..bb10080e 100644 --- a/internal/serviceoffercontroller/hostoffer_test.go +++ b/internal/serviceoffercontroller/hostoffer_test.go @@ -1,7 +1,9 @@ package serviceoffercontroller import ( + "crypto/sha256" "encoding/json" + "fmt" "strings" "testing" @@ -317,6 +319,27 @@ func TestBuildHostHTTPRoute_AgentChatWidget(t *testing.T) { } } + // /chat holds a hot session key and signs USDC transfers — it must carry + // frame-ancestors 'self' so it can't be clickjacked into a cross-origin + // iframe (the offer's own landing page still embeds it same-origin). + chatRule := rules[4].(map[string]any) + var sawCSP bool + for _, rawFilter := range chatRule["filters"].([]any) { + filter := rawFilter.(map[string]any) + if filter["type"] != "ResponseHeaderModifier" { + continue + } + for _, s := range filter["responseHeaderModifier"].(map[string]any)["set"].([]any) { + h := s.(map[string]any) + if h["name"] == "Content-Security-Policy" && h["value"] == "frame-ancestors 'self'" { + sawCSP = true + } + } + } + if !sawCSP { + t.Errorf("/chat rule missing Content-Security-Policy: frame-ancestors 'self'") + } + // Catch-all must still be last. last := rules[6].(map[string]any) match := last["matches"].([]any)[0].(map[string]any)["path"].(map[string]any) @@ -401,6 +424,20 @@ func TestStaticSiteServesChatWidget(t *testing.T) { } } +// TestChatVendorVersionMatchesBundle guards the ?v= cache-buster on +// chat.html's chat-vendor.js import against a forgotten bump: the bundle is +// served 1-year immutable (buildHostHTTPRoute's exactToShared), so a rebuild +// that forgets to bump ?v= would silently serve returning visitors the OLD +// payment-signing bundle for up to a year. This must fail CI whenever +// assets/chat-vendor.js and assets/chat.html's ?v= go out of sync. +func TestChatVendorVersionMatchesBundle(t *testing.T) { + sum := sha256.Sum256([]byte(chatWidgetVendorJS)) + want := "chat-vendor.js?v=" + fmt.Sprintf("%x", sum)[:8] + if !strings.Contains(chatWidgetTmplSrc, want) { + t.Fatalf("chat.html's chat-vendor.js ?v= does not match sha256(assets/chat-vendor.js); want %q (see assets/README.md rebuild steps)", want) + } +} + // TestHostRouteDiscoveryRulesAreGETOnly pins the method scoping: a // root-priced offer advertises POST / as its paid resource, so the // Exact "/" discovery rule must only capture GETs — POSTs fall through to diff --git a/internal/serviceoffercontroller/render.go b/internal/serviceoffercontroller/render.go index e8b810b5..cd05d294 100644 --- a/internal/serviceoffercontroller/render.go +++ b/internal/serviceoffercontroller/render.go @@ -940,8 +940,15 @@ func hostRouteRules(offer *monetizeapi.ServiceOffer, exactTo, exactToShared func exactTo("/.well-known/agent-registration.json", "agent-registration.json"), } if offer.IsAgent() { + // The chat page holds a hot session key and signs USDC transfers — + // frame-ancestors 'self' stops it being clickjacked into a + // cross-origin iframe (Connect/Fund/Withdraw/Send). The offer's own + // landing page embeds it same-origin (TestOfferLandingChatEmbed), so + // that legitimate embed still works. + chatRule := exactTo("/chat", "chat.html") + addResponseHeader(chatRule, "Content-Security-Policy", "frame-ancestors 'self'") rules = append(rules, - exactTo("/chat", "chat.html"), + chatRule, exactToShared("/chat-vendor.js", "chat-vendor.js"), ) } @@ -956,6 +963,23 @@ func hostRouteRules(offer *monetizeapi.ServiceOffer, exactTo, exactToShared func }) } +// addResponseHeader appends a Set entry to a rule's existing +// ResponseHeaderModifier filter (every exactTo rule has exactly one, from +// cacheFilter) — Gateway API's core filters are unspecified/implementation +// -defined if repeated within one rule, so extra headers merge into the +// existing filter instead of adding a second one. +func addResponseHeader(rule map[string]any, name, value string) { + for _, f := range rule["filters"].([]any) { + fm := f.(map[string]any) + if fm["type"] != "ResponseHeaderModifier" { + continue + } + mod := fm["responseHeaderModifier"].(map[string]any) + mod["set"] = append(mod["set"].([]any), map[string]any{"name": name, "value": value}) + return + } +} + // sharedOriginRule is the /services/ PathPrefix rule → verifier, // with the protection middleware attached when spec.limits is set. func sharedOriginRule(offer *monetizeapi.ServiceOffer) map[string]any {