[jeremy]

Summary

The native redirect-verification fetch in HttpRepository attaches the WebView's cookies to the request as a static Cookie header and then issues it with a client that follows redirects. When the requested URL responds with a cross-origin redirect, OkHttp forwards that manually-set Cookie header to the redirect destination — delivering first-party session cookies to a different origin. (OkHttp drops the Authorization header when a redirect changes host, but it does not drop a caller-set Cookie header.)

This fetch only needs to detect whether the response is a cross-origin redirect; it never needs to follow that redirect with credentials. This change stops following redirects on the verification fetch and inspects the Location header directly, so no credential-bearing request is ever sent to a redirect destination.

Changes

• Don't follow redirects on the verification fetch. Its client is now derived from the shared client (keeping its cache, timeouts, and interceptors) with followRedirects(false) / followSslRedirects(false).
• Resolve the destination from Location. The redirect target is computed by resolving the Location header against the request URL, so relative, protocol-relative, and absolute locations are all handled correctly.
• Compare full origin, not just host. A redirect is treated as cross-origin when scheme, host, or port differ — so an HTTPS→HTTP downgrade or a port change is correctly detected as cross-origin.
• Caller. Session.visitRequestFailedWithNonHttpStatusCode previously gated on response.isSuccessful, which only held when redirects were followed through to a 2xx. Since the verification fetch no longer follows redirects, the response is the unfollowed 3xx; detecting a cross-origin redirect is now sufficient to propose the cross-origin redirect visit.

Only the first redirect hop is inspected — sufficient to detect a direct cross-origin redirect. Deeper same-origin chains that would eventually cross origin now fail closed (the visit fails) rather than being followed with credentials.

Testing

Adds HttpRepositoryTest (MockWebServer):

• A cross-origin redirect is detected and its destination receives no request at all — so no Cookie or other credential can reach it — while the first-party request still carries its cookies.
• A same-origin redirect is detected and correctly not flagged cross-origin.
• A relative Location resolves against the request origin (same-origin).
• A direct 2xx response reports no redirect.

Scope run locally: ./gradlew :core:testDebugUnitTest (core module unit tests).

[rosa]

🤖 Review pass on this branch, run by an agent on Rosa's account. Two commits pushed on top of yours; everything else below is left for you to decide.

Base branch

host-scoped-webview-cookies still exists, hasn't moved since Aug 20, and is still the head of open #211. main hasn't moved since Jul 27. This branch was already sitting exactly on its base (0 behind), so no rebase was needed and I left the base alone.

Retargeting to main would be wrong today. This diff is written against the post-#211 HttpRepository (no manually-set Cookie header), and both HttpRepositoryTest and BaseRepositoryTest depend on WebViewCookieJar. #211 is also the broader fix — it covers OfflineHttpRepository and OfflinePreCacheRequest, which this PR doesn't touch. turbo-android already merged its twin of #211 (#356, Aug 19), so the sequence worth agreeing on is #211, then this, then #215.

Credentials on a redirect, every hop

With followRedirects(false) there is exactly one hop by construction, so "every hop" holds trivially — the real question is whether anything else can still egress to the destination. Traced against okhttp 5.3.2 and I couldn't find a path:

• RetryAndFollowUpInterceptor.buildRedirectRequest returns null on its first statement when followRedirects is false, and all six 3xx codes go through it.
• authenticator / proxyAuthenticator are Authenticator.NONE — never set in HotwireHttpClient, and newBuilder() inherits them unchanged.
• 408 / 421 / 503 follow-ups re-issue the same request URL.
• BridgeInterceptor sets Cookie from cookieJar.loadForRequest(url) for the one URL actually being sent.
• The cache and the BASIC logging interceptor produce no egress.

newBuilder() sharing is clean: same connection pool, dispatcher, cache and interceptors, and the shared client isn't mutated, so PathConfigurationRepository and OfflineHttpRepository keep following redirects exactly as before.

Hostile Location values are safe too. HttpUrl.resolve returns null for javascript:, intent:, data:, file:, market: and for a tab-injected URL, so none of them can reach navigator.route() — redirectFrom returns null and the visit just fails. //evil.com, ///evil.com, /\evil.com and https://app.example.com@evil.com all resolve the way Chromium's parser resolves them and come out cross-origin.

Pushed

• Trim the comments to what a reader of the current code needs.
• Add an off-host redirect test. Worth knowing why: both MockWebServer instances bind the same loopback host, so does not forward credentials to a cross-origin redirect destination differs from the first-party origin only by port. It passes solely because of the port term this PR adds, and there was no cross-host case anywhere in the suite — the shape actually reported. The new case uses an absolute off-host Location, which needs no second server precisely because the redirect isn't followed.

./gradlew :core:testDebugUnitTest — 23 classes, 155 tests, green.

Worth deciding before merge

1. Multi-hop is the regression to weigh. redirectFrom reads the first Location only, so first-party → same-origin → cross-origin reports isCrossOrigin = false and Session falls through to visitRequestFailedWithStatusCode(WebError.Unknown), an error screen. The description calls this failing closed, which is accurate, but it's worth being explicit that this is the SSO shape (/dashboard → /session/refresh → IdP) and that the browser reaches this bridge call because fetch followed the same-origin hops itself and only tripped CORS at the cross-origin one. A bounded loop that follows same-origin hops and stops before sending on the first cross-origin Location would keep both the property and the feature — the cookie jar already keys per hop URL, so a same-origin hop carries nothing a same-origin request wouldn't.

2. HttpRequestResult.response no longer has a production reader. Session reads only result.redirect. The response is never closed, and against Rails (redirect_to always emits a body) an unread 302 body holds a pooled connection until finalization, plus an OkHttp leaked-connection log line. It leaks identically on main today so it isn't a regression, but this PR removes the last reason to hold it: execute().use { redirectFrom(it) } and dropping response from the result would close it.

3. The origin comparison widens behaviour as well as tightening it. https://app → http://app and https://app → https://app:8443 used to read same-origin (error screen) and now read cross-origin, which means pop() + route(location). Correct by the web's definition of origin, but the downgrade row means the library now proposes a cleartext navigation where it previously refused. Worth a line in the description.

4. Nits. An empty Location header is non-null and resolve("") returns the request URL, so you get HttpRedirect(location = <the original URL>, isCrossOrigin = false) — inert today because only isCrossOrigin is read, but .takeIf { it.isNotEmpty() } would make it honest. And followSslRedirects(false) is dead code: buildRedirectRequest returns on !followRedirects before it's ever consulted.

5. The description and commit message describe the pre-#211 code ("attached the WebView's cookies as a static Cookie header and issued the request with a redirect-following client"). On this base that isn't what the fetch does any more. I left your commit message alone.

6. #215 overlaps. It adds dev.hotwire.core.security.Origins.hasSameOriginAs — a duplicate of the private isSameOriginAs here — and converts visitRequestFailedWithNonHttpStatusCode from @JavascriptInterface to an internal fun dispatched from a WebMessageListener, which is a textual conflict with this diff in the same method.

One nice side effect nobody claimed: SessionTest (130-150) used to reach the live internet — the old code followed the enqueued 301 → https://example.com/ off the mock server out to the real host, and the assertion held only because example.com answers 2xx. Reading Location directly makes it hermetic. It passes unchanged.

[mbarta]

Thanks, looks good but I'll re-build the changes on #215 when it's merged.