fix(v3/assetserver): give webview requests a loopback RemoteAddr - #6047
fix(v3/assetserver): give webview requests a loopback RemoteAddr#6047midagedev wants to merge 2 commits into
Conversation
The asset server filled a synthesized webview request's RemoteAddr with the RFC 5737 TEST-NET placeholder 192.0.2.1:1234. A webview request originates on the local machine, so this made the most-local caller look like a routable remote peer. Use 127.0.0.1:0 so a Handler that gates on request origin (net.ParseIP(host).IsLoopback()) sees it as loopback. Wails' own MCP loopback gate (internal/commands/mcp.go) classifies 192.0.2.1:1234 as non-loopback (mcp_test.go:111, want:false), so a handler reusing that pattern in front of the asset server rejected its own webview. No Wails code reads RemoteAddr; the change affects only downstream handlers that gate on it. Adds a test asserting the webview peer is loopback.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. WalkthroughWebView requests without a ChangesWebView loopback peer address
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: 🔵 Low · up to WebView requests without a peer address will now appear to come from loopback, which may cause application handlers that use loopback as an authorization signal to allow requests they previously rejected. The change is mergeable with explicit owner awareness that this synthetic address should not independently prove caller identity. Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 2 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
normalizeWebviewPeer works around the wails asset server stamping a non-loopback TEST-NET peer on webview requests. The upstream fix is now proposed at wailsapp/wails#6047; note it so the shim's clear can be dropped once a pinned wails version carries a loopback peer. Comment-only. GDK-918
What
The asset server synthesizes an
http.Requestfor every webview request and,when the platform gives no peer address, fills
RemoteAddrwith the RFC 5737TEST-NET placeholder
192.0.2.1:1234. A webview request originates from theembedded webview on the local machine, so this makes the most local possible
caller look like a routable remote peer. This changes it to a loopback address,
127.0.0.1:0.Why it matters
Nothing in the Wails tree reads
RemoteAddr— the only assignment is this oneline, and no production code compares or parses it (verified by grepping the
whole
v3/tree). So this changes no Wails-internal behaviour. The impact is ona user
Handlerwired into the asset server that gates on request origin — thestandard "only accept loopback callers" check:
Today such a handler rejects the webview, because
192.0.2.1is not loopback.Wails' own code already treats that exact address as the canonical non-loopback
peer.
v3/internal/commands/mcp.gogates MCP requests withmcpHostIsLoopback(net.ParseIP(host).IsLoopback()), and its test pins theclassification:
So a Wails app that reuses Wails' own loopback-gating pattern in front of its
asset server would reject its own webview. The synthesized address should agree
with how the framework classifies addresses elsewhere: a webview is loopback.
Sibling precedent
127.0.0.1is what Wails already uses as its canonical loopback in the same MCPgate's tests (
mcp_test.go:108,want: true). This change makes theasset-server placeholder consistent with that.
Test
Added
TestWebViewRequestPeerIsLoopbackinv3/internal/assetserver/assetserver_webview_test.go, reusing the existingcontextWebViewRequest/contextWebViewResponsefakes. It drivesprocessWebViewRequestand asserts the handler observes a loopbackRemoteAddr.FAIL-first checked: with the old
192.0.2.1:1234, the test fails withRemoteAddr = "192.0.2.1:1234", want a loopback peer; with the fix it passes.go test ./internal/assetserver/,go vet, andgofmt -lare clean.Scope / provenance
v3/UNRELEASED_CHANGELOG.md).public-API change —
RemoteAddrwas never a documented contract and stays anon-routable synthetic address either way. If maintainers consider the
observable value part of public behaviour and would prefer a WEP, I'm happy to
open one.
Summary by CodeRabbit
Bug Fixes
Documentation