From 25720ec6e77884c0d7ed7ac9f3da9a42b5b43622 Mon Sep 17 00:00:00 2001 From: David Calhoun Date: Fri, 28 Aug 2026 14:37:28 -0400 Subject: [PATCH 01/31] fix(ios): correct which requests the local HTTP server admits MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two defects in how the local server decides what reaches its handler. A relayed `OPTIONS` and a browser preflight arrive at the same target, so the permissive CORS policy answered both with 204 and the handler never ran. `canUser` issues `OPTIONS /wp/v2/{resource}` and reads the `Allow` response header, so the editor silently reported that the user could not create pages, update settings, upload media, or edit global styles — with no error surfaced, because the request "succeeded". Discriminate on `Access-Control-Request-Method`: a preflight always carries it, a deliberate `OPTIONS` never does. The authentication exemption narrows to the same condition, so a deliberate `OPTIONS` is authenticated like any other request rather than riding in on the preflight exemption. The server also gains an opt-in requirement for `Origin` or `Sec-Fetch-Site`, headers WebKit sets on every editor `fetch()` and a raw socket opened by another process on the device does not. The bearer token remains the control; this is defense in depth, and cheap. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_014A21xYJuymCU7My8yyS8Bv --- .../Sources/Media/MediaUploadServer.swift | 3 + ios/Sources/GutenbergKitHTTP/HTTPServer.swift | 83 ++++++++++++++++--- .../GutenbergKitHTTP/HTTPServerError.swift | 7 +- .../HTTPServerAuthenticationTests.swift | 66 ++++++++++++++- .../HTTPServerTimeoutTests.swift | 20 ++--- .../Media/MediaUploadServerTests.swift | 37 +++++++++ 6 files changed, 190 insertions(+), 26 deletions(-) diff --git a/ios/Sources/GutenbergKit/Sources/Media/MediaUploadServer.swift b/ios/Sources/GutenbergKit/Sources/Media/MediaUploadServer.swift index 69428bea8..bcc6976fa 100644 --- a/ios/Sources/GutenbergKit/Sources/Media/MediaUploadServer.swift +++ b/ios/Sources/GutenbergKit/Sources/Media/MediaUploadServer.swift @@ -59,6 +59,9 @@ final class MediaUploadServer: Sendable { let server = try await HTTPServer.start( name: "media-upload", requiresAuthentication: true, + // The editor web view is this server's only legitimate client, and + // every request it makes carries these headers. + requiresBrowserOrigin: true, maxRequestBodySize: maxRequestBodySize, bodyReadTimeout: bodyReadTimeout, cors: .permissive, diff --git a/ios/Sources/GutenbergKitHTTP/HTTPServer.swift b/ios/Sources/GutenbergKitHTTP/HTTPServer.swift index ac05fbb01..0debe4c1b 100644 --- a/ios/Sources/GutenbergKitHTTP/HTTPServer.swift +++ b/ios/Sources/GutenbergKitHTTP/HTTPServer.swift @@ -37,10 +37,14 @@ import OSLog /// /// ## CORS /// -/// When `requiresAuthentication` is enabled, `OPTIONS` requests are exempt -/// from authentication because CORS preflight requests never include -/// credentials (Fetch spec §3.3.5). However, the server does not generate -/// CORS response headers — this is the handler's responsibility. +/// When `requiresAuthentication` is enabled, CORS preflight requests are exempt +/// from authentication because a preflight never includes credentials (Fetch +/// spec §3.3.5). The exemption is scoped to genuine preflights — an `OPTIONS` +/// without `Access-Control-Request-Method` is a request the client made on its +/// own behalf and is authenticated, and dispatched to the handler, like any +/// other. Under ``CORSPolicy/permissive`` the server answers preflights itself; +/// otherwise it does not generate CORS response headers and that is the +/// handler's responsibility. /// /// When proxying to a remote server, the upstream response will typically /// include the correct CORS headers already — pass it through unaltered. @@ -141,6 +145,11 @@ public final class HTTPServer: Sendable { /// to choose a descriptive, collision-free identifier (e.g. `"media-proxy"`, /// `"editor-assets"`). /// - port: The port to listen on. Pass `nil` or omit to let the system assign an available port. + /// - requiresBrowserOrigin: When enabled, requests must carry `Origin` or + /// `Sec-Fetch-Site` — headers a web view's `fetch()` always sets and a raw + /// socket does not — and receive a 403 otherwise. Defense in depth behind the + /// bearer token, for servers whose only legitimate client is a web view; both + /// headers are trivially forged by a process that cares to. Defaults to off. /// - maxRequestBodySize: The maximum allowed request body size in bytes. /// Requests exceeding this limit receive a 413 response. Defaults to 4 GB. /// - maxConnections: The maximum number of concurrent connections. New connections @@ -169,6 +178,7 @@ public final class HTTPServer: Sendable { port: UInt16? = nil, listenOnAllInterfaces: Bool = false, requiresAuthentication: Bool = true, + requiresBrowserOrigin: Bool = false, maxRequestBodySize: Int64 = HTTPRequestParser.defaultMaxBodySize, maxConnections: Int = HTTPServer.defaultMaxConnections, readTimeout: Duration = HTTPServer.defaultReadTimeout, @@ -210,6 +220,7 @@ public final class HTTPServer: Sendable { let queue = DispatchQueue(label: "com.gutenbergkit.http-server.\(safeName)") let requiresAuth = requiresAuthentication + let requiresOrigin = requiresBrowserOrigin // Falls back to `readTimeout` so consumers that don't distinguish the two // keep the prior whole-request behavior. let resolvedBodyReadTimeout = bodyReadTimeout ?? readTimeout @@ -222,6 +233,7 @@ public final class HTTPServer: Sendable { handleConnection( connection, queue: queue, token: token, requiresAuthentication: requiresAuth, + requiresBrowserOrigin: requiresOrigin, maxRequestBodySize: maxRequestBodySize, readTimeout: readTimeout, bodyReadTimeout: resolvedBodyReadTimeout, idleTimeout: idleTimeout, cors: cors, tempDirectory: tempDirectory, @@ -331,6 +343,7 @@ public final class HTTPServer: Sendable { queue: DispatchQueue, token: String, requiresAuthentication: Bool, + requiresBrowserOrigin: Bool, maxRequestBodySize: Int64, readTimeout: Duration, bodyReadTimeout: Duration, @@ -370,20 +383,27 @@ public final class HTTPServer: Sendable { // Check auth on headers alone, before draining or consuming any // body bytes — an unauthenticated client must not be able to make // the server read (and discard) an arbitrarily large body, and the - // handler must never see an unauthenticated request. OPTIONS is - // exempt because CORS preflight requests never include credentials - // (Fetch spec §3.3.5). - if requiresAuthentication && partial.method.uppercased() != "OPTIONS" { + // handler must never see an unauthenticated request. A CORS + // preflight is exempt because preflights never include credentials + // (Fetch spec §3.3.5); an `OPTIONS` the client sent deliberately is + // not a preflight and is authenticated like any other request. + if requiresAuthentication && !isPreflight(partial) { guard authenticate(partial, token: token) else { throw HTTPServerError.authenticationFailed } } - // Reject auth-exempt OPTIONS that carry a body. Real CORS preflight + if requiresBrowserOrigin { + guard hasBrowserOrigin(partial) else { + throw HTTPServerError.forbiddenOrigin + } + } + + // Reject preflights that carry a body. Real CORS preflight // requests are bodyless; a body on the auth-exempt path would // otherwise be read/drained without authentication — and the // accepted-body read below is bounded only by the idle timeout. - if partial.method.uppercased() == "OPTIONS", (parser.expectedBodyLength ?? 0) > 0 { + if isPreflight(partial), (parser.expectedBodyLength ?? 0) > 0 { throw HTTPServerError.unexpectedBody } @@ -439,7 +459,7 @@ public final class HTTPServer: Sendable { if let parseError = parser.parseError { response = delegate?.response(forRecoverableParseError: parseError) ?? Self.defaultErrorResponse(for: parseError) - } else if cors == .permissive, request.method.uppercased() == "OPTIONS" { + } else if cors == .permissive, isPreflight(request) { // Under a permissive CORS policy the library answers the OPTIONS // preflight itself; the send layer stamps the CORS headers. response = HTTPResponse(status: 204) @@ -480,6 +500,9 @@ public final class HTTPServer: Sendable { Logger.httpServer.debug("\(request.method) \(request.target) → \(response.status) (\(String(format: "%.1f", ms))ms)") } catch HTTPServerError.authenticationFailed { await send(HTTPResponse(status: 407, headers: [("Content-Type", "text/plain"), ("Proxy-Authenticate", "Bearer")]), on: connection, cors: cors) + } catch HTTPServerError.forbiddenOrigin { + Logger.httpServer.warning("Rejected a request that did not originate from a web view") + await send(HTTPResponse(status: 403, statusText: "Forbidden", body: Data("Forbidden".utf8)), on: connection, cors: cors) } catch HTTPServerError.lengthRequired { await send(HTTPResponse(status: 411, statusText: "Length Required", body: Data("Length Required".utf8)), on: connection, cors: cors) } catch HTTPServerError.unexpectedBody { @@ -772,6 +795,44 @@ public final class HTTPServer: Sendable { } } + // MARK: - CORS + + /// Whether a request is a CORS preflight rather than an `OPTIONS` the + /// client sent on its own behalf. + /// + /// Both arrive as `OPTIONS` at the same target, so the two are only + /// separable by `Access-Control-Request-Method`: a preflight always carries + /// it (Fetch spec §4.8), and a deliberate `OPTIONS` — WordPress's + /// `canUser`, which reads the `Allow` response header — never does. + /// Answering the latter with the library's 204 would swallow it before the + /// handler ran, reporting no capabilities at all and surfacing no error, + /// because the request "succeeded". + /// + /// A preflight also only *announces* custom headers via + /// `Access-Control-Request-Headers`; it never sends them. So a client + /// cannot use this to skip authentication: dropping the bearer token to + /// look like a preflight means adding `Access-Control-Request-Method`, + /// which routes the request to the 204 answer instead of the handler. + private static func isPreflight(_ request: ParsedHTTPRequest) -> Bool { + request.method.uppercased() == "OPTIONS" + && request.header("Access-Control-Request-Method") != nil + } + + /// Whether a request looks like it came from a web view's `fetch()`. + /// + /// WebKit sets `Origin` on every cross-origin fetch and `Sec-Fetch-Site` on + /// every fetch, so the editor's requests always carry at least one. Either + /// is accepted rather than a specific value: the editor's origin is + /// `file://` in a release build but the dev server's `http://localhost:…` + /// when `GUTENBERG_EDITOR_URL` is set, and neither is worth pinning. + /// + /// This is a speed bump, not the control — the per-session bearer token is. + /// Another process on the device sends neither header by default, but can + /// forge both the moment it cares to. + private static func hasBrowserOrigin(_ request: ParsedHTTPRequest) -> Bool { + request.header("Origin") != nil || request.header("Sec-Fetch-Site") != nil + } + // MARK: - Authentication /// Validates the proxy bearer token from the request. diff --git a/ios/Sources/GutenbergKitHTTP/HTTPServerError.swift b/ios/Sources/GutenbergKitHTTP/HTTPServerError.swift index 1b24009a5..55a1309d5 100644 --- a/ios/Sources/GutenbergKitHTTP/HTTPServerError.swift +++ b/ios/Sources/GutenbergKitHTTP/HTTPServerError.swift @@ -17,9 +17,13 @@ public enum HTTPServerError: Error, LocalizedError, Sendable { case readTimeout /// The request failed authentication (checked after headers, before body). case authenticationFailed + /// The request carried neither `Origin` nor `Sec-Fetch-Site`, so it did not + /// come from a web view's `fetch()`. Only raised when the server is started + /// with `requiresBrowserOrigin`. + case forbiddenOrigin /// The request method requires a Content-Length header but none was provided. case lengthRequired - /// An auth-exempt request (OPTIONS) carried a body. CORS preflights are + /// An auth-exempt request (a CORS preflight) carried a body. Preflights are /// bodyless, so a body on the auth-exempt path is rejected rather than read. case unexpectedBody /// A network-level error occurred on the connection. @@ -32,6 +36,7 @@ public enum HTTPServerError: Error, LocalizedError, Sendable { case .connectionClosed: "Connection closed before request was complete" case .readTimeout: "Read timeout expired before request was complete" case .authenticationFailed: "Request failed authentication" + case .forbiddenOrigin: "Request did not originate from a web view" case .lengthRequired: "Content-Length header is required for this method" case .unexpectedBody: "Request method must not carry a body" case .networkError(let error): "Network error: \(error.localizedDescription)" diff --git a/ios/Tests/GutenbergKitHTTPTests/HTTPServerAuthenticationTests.swift b/ios/Tests/GutenbergKitHTTPTests/HTTPServerAuthenticationTests.swift index 427526eaa..fdd8ab4b9 100644 --- a/ios/Tests/GutenbergKitHTTPTests/HTTPServerAuthenticationTests.swift +++ b/ios/Tests/GutenbergKitHTTPTests/HTTPServerAuthenticationTests.swift @@ -241,10 +241,29 @@ struct HTTPServerAuthenticationTests { #expect(http.value(forHTTPHeaderField: "X-Received-Auth") == "Basic dXNlcjpwYXNz") } - // MARK: - CORS Preflight (OPTIONS) Auth Exemption + // MARK: - CORS Preflight Auth Exemption - @Test("OPTIONS without token returns 200 (CORS preflight exempt from auth)") - func optionsWithoutTokenReturns200() async throws { + @Test("preflight without token returns 200 (CORS preflight exempt from auth)") + func preflightWithoutTokenReturns200() async throws { + let server = try await HTTPServer.start( + name: "auth-test", + requiresAuthentication: true + ) { _ in + HTTPResponse(status: 200, body: Data("OK\n".utf8)) + } + defer { server.stop() } + + let raw = "OPTIONS /test HTTP/1.1\r\nHost: 127.0.0.1\r\nAccess-Control-Request-Method: GET\r\n\r\n" + let response = try await sendRaw(raw, toPort: server.port) + #expect(response.hasPrefix("HTTP/1.1 200")) + } + + @Test("OPTIONS without Access-Control-Request-Method is not a preflight and returns 407") + func nonPreflightOptionsWithoutTokenReturns407() async throws { + // `canUser` issues a deliberate `OPTIONS` to read the `Allow` header. It + // is a request the client made on its own behalf, so it carries the + // token and must be authenticated like any other — the exemption covers + // preflights, which cannot carry credentials, and nothing else. let server = try await HTTPServer.start( name: "auth-test", requiresAuthentication: true @@ -255,10 +274,49 @@ struct HTTPServerAuthenticationTests { let raw = "OPTIONS /test HTTP/1.1\r\nHost: 127.0.0.1\r\n\r\n" let response = try await sendRaw(raw, toPort: server.port) + #expect(response.hasPrefix("HTTP/1.1 407")) + } + + @Test("authenticated OPTIONS without Access-Control-Request-Method reaches the handler") + func nonPreflightOptionsWithTokenReachesHandler() async throws { + let server = try await HTTPServer.start( + name: "auth-test", + requiresAuthentication: true + ) { request in + HTTPResponse(status: 200, headers: [("Allow", request.parsed.method)], body: Data()) + } + defer { server.stop() } + + let raw = "OPTIONS /test HTTP/1.1\r\nHost: 127.0.0.1\r\nRelay-Authorization: Bearer \(server.token)\r\n\r\n" + let response = try await sendRaw(raw, toPort: server.port) + #expect(response.hasPrefix("HTTP/1.1 200")) + #expect(response.contains("Allow: OPTIONS")) + } + + @Test("permissive CORS answers a preflight itself but forwards a deliberate OPTIONS") + func permissiveCORSDistinguishesPreflightFromOptions() async throws { + let server = try await HTTPServer.start( + name: "auth-test", + requiresAuthentication: true, + cors: .permissive + ) { _ in + HTTPResponse(status: 200, headers: [("Allow", "GET, POST")], body: Data()) + } + defer { server.stop() } + + let preflight = "OPTIONS /test HTTP/1.1\r\nHost: 127.0.0.1\r\nAccess-Control-Request-Method: POST\r\n\r\n" + #expect(try await sendRaw(preflight, toPort: server.port).hasPrefix("HTTP/1.1 204")) + + // Without the preflight header the request belongs to the handler; the + // library answering it with its own 204 would swallow the `Allow` + // header `canUser` exists to read. + let deliberate = "OPTIONS /test HTTP/1.1\r\nHost: 127.0.0.1\r\nRelay-Authorization: Bearer \(server.token)\r\n\r\n" + let response = try await sendRaw(deliberate, toPort: server.port) #expect(response.hasPrefix("HTTP/1.1 200")) + #expect(response.contains("Allow: GET, POST")) } - @Test("GET without token still returns 407 (only OPTIONS is exempt)") + @Test("GET without token still returns 407 (only a preflight is exempt)") func getWithoutTokenStillReturns407() async throws { let server = try await HTTPServer.start( name: "auth-test", diff --git a/ios/Tests/GutenbergKitHTTPTests/HTTPServerTimeoutTests.swift b/ios/Tests/GutenbergKitHTTPTests/HTTPServerTimeoutTests.swift index 4fc444480..7000e581f 100644 --- a/ios/Tests/GutenbergKitHTTPTests/HTTPServerTimeoutTests.swift +++ b/ios/Tests/GutenbergKitHTTPTests/HTTPServerTimeoutTests.swift @@ -8,7 +8,7 @@ import Testing /// Covers the split read-timeout model: the pre-body phase (headers + drain) is /// bounded by `readTimeout`, while an accepted body is bounded by the generous /// `bodyReadTimeout` plus the per-read `idleTimeout`. Also covers rejecting an -/// auth-exempt `OPTIONS` request that carries a body. +/// auth-exempt CORS preflight that carries a body. @Suite("HTTPServer Timeouts") struct HTTPServerTimeoutTests { @@ -72,8 +72,8 @@ struct HTTPServerTimeoutTests { #expect(elapsed < .seconds(3)) // reaped by the 500ms idle timeout, not the 10s ceiling } - @Test("auth-exempt OPTIONS carrying a body is rejected with 400") - func optionsWithBodyReturns400() async throws { + @Test("auth-exempt preflight carrying a body is rejected with 400") + func preflightWithBodyReturns400() async throws { let server = try await HTTPServer.start( name: "options-with-body", requiresAuthentication: true @@ -82,15 +82,15 @@ struct HTTPServerTimeoutTests { } defer { server.stop() } - // A real CORS preflight is bodyless; an OPTIONS with a body must not be + // A real CORS preflight is bodyless; one with a body must not be // read/drained on the auth-exempt path. - let raw = "OPTIONS /test HTTP/1.1\r\nHost: 127.0.0.1\r\nContent-Length: 5\r\n\r\nhello" + let raw = "OPTIONS /test HTTP/1.1\r\nHost: 127.0.0.1\r\nAccess-Control-Request-Method: POST\r\nContent-Length: 5\r\n\r\nhello" let response = try await sendRaw(raw, toPort: server.port) #expect(response.hasPrefix("HTTP/1.1 400")) } - @Test("auth-exempt OPTIONS with an oversized body is rejected with 400, not drained") - func optionsWithOversizedBodyReturns400() async throws { + @Test("auth-exempt preflight with an oversized body is rejected with 400, not drained") + func preflightWithOversizedBodyReturns400() async throws { let server = try await HTTPServer.start( name: "options-oversized-body", requiresAuthentication: true, @@ -101,8 +101,8 @@ struct HTTPServerTimeoutTests { defer { server.stop() } // Content-Length exceeds the max body size, so the parser would otherwise - // enter the drain path — the OPTIONS-with-body guard must reject it first. - let raw = "OPTIONS /test HTTP/1.1\r\nHost: 127.0.0.1\r\nContent-Length: 1000\r\n\r\n" + // enter the drain path — the preflight-with-body guard must reject it first. + let raw = "OPTIONS /test HTTP/1.1\r\nHost: 127.0.0.1\r\nAccess-Control-Request-Method: POST\r\nContent-Length: 1000\r\n\r\n" let response = try await sendRaw(raw, toPort: server.port) #expect(response.hasPrefix("HTTP/1.1 400")) } @@ -117,7 +117,7 @@ struct HTTPServerTimeoutTests { } defer { server.stop() } - let raw = "OPTIONS /test HTTP/1.1\r\nHost: 127.0.0.1\r\n\r\n" + let raw = "OPTIONS /test HTTP/1.1\r\nHost: 127.0.0.1\r\nAccess-Control-Request-Method: GET\r\n\r\n" let response = try await sendRaw(raw, toPort: server.port) #expect(response.hasPrefix("HTTP/1.1 200")) } diff --git a/ios/Tests/GutenbergKitTests/Media/MediaUploadServerTests.swift b/ios/Tests/GutenbergKitTests/Media/MediaUploadServerTests.swift index 97a300733..8a62c240c 100644 --- a/ios/Tests/GutenbergKitTests/Media/MediaUploadServerTests.swift +++ b/ios/Tests/GutenbergKitTests/Media/MediaUploadServerTests.swift @@ -77,6 +77,8 @@ struct MediaUploadServerTests { let url = URL(string: "http://127.0.0.1:\(server.port)/upload")! var request = URLRequest(url: url) request.httpMethod = "OPTIONS" + request.setValue("POST", forHTTPHeaderField: "Access-Control-Request-Method") + request.setBrowserOrigin() let (_, response) = try await URLSession.shared.data(for: request) let httpResponse = try #require(response as? HTTPURLResponse) @@ -85,6 +87,23 @@ struct MediaUploadServerTests { #expect(httpResponse.value(forHTTPHeaderField: "Access-Control-Allow-Methods")?.contains("POST") == true) } + @Test("rejects a request that did not come from the web view") + func rejectsNonBrowserRequest() async throws { + let server = try await MediaUploadServer.start() + defer { server.stop() } + + // A correct token but none of the headers WebKit sets on a `fetch()`: the + // shape another process on the device would produce over a raw socket. + let url = URL(string: "http://127.0.0.1:\(server.port)/upload")! + var request = URLRequest(url: url) + request.httpMethod = "POST" + request.setValue("Bearer \(server.token)", forHTTPHeaderField: "Relay-Authorization") + + let (_, response) = try await URLSession.shared.data(for: request) + let httpResponse = try #require(response as? HTTPURLResponse) + #expect(httpResponse.statusCode == 403) + } + @Test("returns 404 for unknown paths") func unknownPath() async throws { let server = try await MediaUploadServer.start() @@ -94,6 +113,7 @@ struct MediaUploadServerTests { var request = URLRequest(url: url) request.httpMethod = "POST" request.setValue("Bearer \(server.token)", forHTTPHeaderField: "Relay-Authorization") + request.setBrowserOrigin() let (_, response) = try await URLSession.shared.data(for: request) let httpResponse = try #require(response as? HTTPURLResponse) @@ -117,6 +137,7 @@ struct MediaUploadServerTests { var request = URLRequest(url: url) request.httpMethod = "POST" request.setValue("Bearer \(server.token)", forHTTPHeaderField: "Relay-Authorization") + request.setBrowserOrigin() request.setValue("multipart/form-data; boundary=\(boundary)", forHTTPHeaderField: "Content-Type") request.httpBody = body @@ -145,6 +166,7 @@ struct MediaUploadServerTests { var request = URLRequest(url: url) request.httpMethod = "POST" request.setValue("Bearer \(server.token)", forHTTPHeaderField: "Relay-Authorization") + request.setBrowserOrigin() request.setValue("multipart/form-data; boundary=\(boundary)", forHTTPHeaderField: "Content-Type") request.httpBody = body @@ -180,6 +202,7 @@ struct MediaUploadServerTests { var request = URLRequest(url: url) request.httpMethod = "POST" request.setValue("Bearer \(server.token)", forHTTPHeaderField: "Relay-Authorization") + request.setBrowserOrigin() request.setValue("multipart/form-data; boundary=\(boundary)", forHTTPHeaderField: "Content-Type") request.httpBody = body @@ -212,6 +235,7 @@ struct MediaUploadServerTests { var request = URLRequest(url: url) request.httpMethod = "POST" request.setValue("Bearer \(server.token)", forHTTPHeaderField: "Relay-Authorization") + request.setBrowserOrigin() request.setValue("multipart/form-data; boundary=\(boundary)", forHTTPHeaderField: "Content-Type") request.httpBody = body @@ -240,6 +264,7 @@ struct MediaUploadServerTests { var request = URLRequest(url: url) request.httpMethod = "POST" request.setValue("Bearer \(server.token)", forHTTPHeaderField: "Relay-Authorization") + request.setBrowserOrigin() request.setValue("multipart/form-data; boundary=\(boundary)", forHTTPHeaderField: "Content-Type") request.httpBody = body @@ -266,6 +291,7 @@ struct MediaUploadServerTests { var request = URLRequest(url: url) request.httpMethod = "POST" request.setValue("Bearer \(server.token)", forHTTPHeaderField: "Relay-Authorization") + request.setBrowserOrigin() request.setValue("multipart/form-data; boundary=\(boundary)", forHTTPHeaderField: "Content-Type") request.httpBody = body @@ -291,6 +317,7 @@ struct MediaUploadServerTests { var request = URLRequest(url: url) request.httpMethod = "POST" request.setValue("Bearer \(server.token)", forHTTPHeaderField: "Relay-Authorization") + request.setBrowserOrigin() request.setValue("multipart/form-data; boundary=\(boundary)", forHTTPHeaderField: "Content-Type") request.httpBody = body @@ -827,6 +854,16 @@ private struct MockHTTPClient: EditorHTTPClientProtocol { } } +private extension URLRequest { + /// Adds the header WebKit sets on every cross-origin `fetch()` the editor + /// makes. The server rejects requests carrying neither `Origin` nor + /// `Sec-Fetch-Site` (see `requiresBrowserOrigin`), so a test standing in for + /// the web view has to look like one. + mutating func setBrowserOrigin() { + setValue("file://", forHTTPHeaderField: "Origin") + } +} + private extension Data { mutating func append(_ string: String) { append(string.data(using: .utf8)!) From 1a85ad78f4086c5ce1497a4b139fee520c1a0e9d Mon Sep 17 00:00:00 2001 From: David Calhoun Date: Fri, 28 Aug 2026 14:39:46 -0400 Subject: [PATCH 02/31] fix(ios): relay REST requests by path, contained to the site API root MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The relay has never served a request. `upstreamURL(from:)` assigned `parsed.query` to `percentEncodedQuery`, but `query` includes the leading `?`, so the first query item was named `?url`, the lookup for `url` returned nil, and every request 400d. Rather than fix the parse, drop the caller-supplied URL. The upstream path now rides in the request path — `/proxy/wp/v2/posts?_locale=user` — and resolves natively against the configured site API root, so there is no URL to contain in the first place. The old `hasPrefix` guard ran on an absolute URL without normalizing `..` segments; those are now refused outright, literal or percent-encoded, and the resolved URL is re-checked against the root. Each request also identifies itself in a network log instead of every row reading `/proxy`. Resolution appends to the root rather than resolving relative to it, mirroring `createRootURLMiddleware`: a site on plain permalinks has `https://example.com/?rest_route=/`, where relative resolution would discard the query and the path has to merge into it. Three further defects, all in what comes back: - `Allow` was not exposed, so `canUser` read null even once its `OPTIONS` reached the handler. - Error bodies were `text/plain`, reaching JavaScript as an unparseable `invalid_json` with the real reason lost. They are now WordPress-shaped `{code, message}`, as `MediaUploadServer.errorResponse` already was. - `URLSession` followed 3xx responses with no task delegate, so the containment check only ever applied to the first hop and a redirect carried the site credential to another host. Cross-root redirects are now refused and the 3xx handed back instead. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_014A21xYJuymCU7My8yyS8Bv --- .../Sources/Media/MediaUploadServer.swift | 8 +- .../Sources/Media/RestRelay.swift | 198 ++++++++++++++---- .../Media/RestRelayTests.swift | 147 +++++++++++++ 3 files changed, 306 insertions(+), 47 deletions(-) create mode 100644 ios/Tests/GutenbergKitTests/Media/RestRelayTests.swift diff --git a/ios/Sources/GutenbergKit/Sources/Media/MediaUploadServer.swift b/ios/Sources/GutenbergKit/Sources/Media/MediaUploadServer.swift index bcc6976fa..4233bb47d 100644 --- a/ios/Sources/GutenbergKit/Sources/Media/MediaUploadServer.swift +++ b/ios/Sources/GutenbergKit/Sources/Media/MediaUploadServer.swift @@ -91,10 +91,10 @@ final class MediaUploadServer: Sendable { private static func handleRequest(_ request: HTTPServer.Request, context: UploadContext) async -> HTTPResponse { let parsed = request.parsed - // REST relay route: `/proxy` requests are forwarded to the site's REST - // API (Lockdown Mode support). The upstream URL rides in the query - // string, so the library's permissive CORS policy covers the preflight. - if let restRelay = context.restRelay, parsed.path == "/proxy" { + // REST relay route: `/proxy/…` requests are forwarded to the site's REST + // API (Lockdown Mode support), the path after the route resolving + // against the site API root. + if let restRelay = context.restRelay, RestRelay.handles(parsed) { return await restRelay.handle(request) } diff --git a/ios/Sources/GutenbergKit/Sources/Media/RestRelay.swift b/ios/Sources/GutenbergKit/Sources/Media/RestRelay.swift index f909c6d92..53ad846e4 100644 --- a/ios/Sources/GutenbergKit/Sources/Media/RestRelay.swift +++ b/ios/Sources/GutenbergKit/Sources/Media/RestRelay.swift @@ -25,21 +25,34 @@ import GutenbergKitHTTP /// /// - Requests reach the relay only through the local server's loopback /// listener and per-session bearer token. -/// - Forwarding is restricted to URLs under the configured site API root, -/// so the relay cannot be used to reach arbitrary hosts. +/// - The caller supplies a **path**, not a URL: everything after `/proxy/` is +/// resolved natively against the configured site API root, so the relay +/// cannot be pointed at another host by construction rather than by string +/// matching. The resolved URL is re-checked against the root, and redirects +/// away from it are refused. /// - The upstream `Authorization` header is injected natively from the editor /// configuration; any client-supplied value is discarded. struct RestRelay: Sendable { - /// Query parameter carrying the absolute upstream URL to forward to. + /// The local server route the relay answers. Everything after it is the + /// upstream path, relative to the site API root — `/proxy/wp/v2/posts?…` + /// relays to `wp/v2/posts?…`. /// - /// The URL rides in the query string rather than a custom header so the - /// HTTP library's permissive CORS policy (which enumerates allowed - /// headers) covers the preflight without additions. - static let upstreamURLQueryItem = "url" + /// A path rather than an absolute URL in a query parameter: there is no + /// caller-supplied URL to contain in the first place, and each request + /// identifies itself in a network log instead of every row reading + /// `/proxy`. + static let route = "/proxy" - /// The URL prefix (the site's API root) that forwarded requests must match. - private let allowedPrefix: String + /// The site's API root, slash-terminated. Upstream paths are appended to + /// it, and every resulting URL — including redirect targets — must still + /// start with it. + /// + /// Held as a string rather than a `URL` because the root is not always + /// directory-shaped: a site on plain permalinks has + /// `https://example.com/?rest_route=/`, where relative URL resolution would + /// discard the query. + private let apiRoot: String /// The authorization header injected into upstream requests. private let authHeader: String @@ -47,11 +60,11 @@ struct RestRelay: Sendable { private let session: URLSession init(configuration: EditorConfiguration) { - var prefix = configuration.siteApiRoot.absoluteString - if !prefix.hasSuffix("/") { - prefix += "/" + var root = configuration.siteApiRoot.absoluteString + if !root.hasSuffix("/") { + root += "/" } - self.allowedPrefix = prefix + self.apiRoot = root self.authHeader = configuration.authHeader let sessionConfiguration = URLSessionConfiguration.ephemeral @@ -60,19 +73,23 @@ struct RestRelay: Sendable { self.session = URLSession(configuration: sessionConfiguration) } + /// Whether a request targets the relay. + static func handles(_ request: ParsedHTTPRequest) -> Bool { + request.path == route || request.path.hasPrefix("\(route)/") + } + /// Forwards a relayed request to the site's REST API and returns the /// upstream response with permissive CORS headers. func handle(_ request: HTTPServer.Request) async -> HTTPResponse { let parsed = request.parsed - guard let upstreamURL = Self.upstreamURL(from: parsed.query) else { - return Self.errorResponse(status: 400, body: "Missing or invalid `\(Self.upstreamURLQueryItem)` query parameter") - } - - // SSRF guard: only forward to the configured site API root. - guard upstreamURL.absoluteString.hasPrefix(allowedPrefix) else { - Logger.restRelay.error("Refusing to relay request outside the site API root") - return Self.errorResponse(status: 403, body: "Upstream URL is outside the allowed API root") + guard let upstreamURL = upstreamURL(for: parsed) else { + Logger.restRelay.error("Refusing to relay a request outside the site API root") + return Self.errorResponse( + status: 403, + code: "relay_forbidden_path", + message: "The requested path is outside the site API root." + ) } var upstreamRequest = URLRequest(url: upstreamURL) @@ -96,13 +113,18 @@ struct RestRelay: Sendable { upstreamRequest.setValue("\(body.count)", forHTTPHeaderField: "Content-Length") } catch { Logger.restRelay.error("Failed to open request body stream: \(error)") - return Self.errorResponse(status: 500, body: "Failed to read request body") + return Self.errorResponse(status: 500, code: "relay_body_unreadable", message: "Failed to read the request body.") } } } do { - let upstream = HTTPResponse(try await session.data(for: upstreamRequest)) + // The redirect guard is a per-task delegate: `URLSession` follows + // 3xx responses on its own, which would carry the site credential + // to whatever host the `Location` header names and relay that + // response back. See ``RedirectGuard``. + let redirectGuard = RedirectGuard(allowedPrefix: apiRoot) + let upstream = HTTPResponse(try await session.data(for: upstreamRequest, delegate: redirectGuard)) return HTTPResponse( status: upstream.status, statusText: upstream.statusText, @@ -111,7 +133,92 @@ struct RestRelay: Sendable { ) } catch { Logger.restRelay.error("Upstream request failed: \(error.localizedDescription)") - return Self.errorResponse(status: 502, body: "Upstream request failed: \(error.localizedDescription)") + return Self.errorResponse(status: 502, code: "relay_upstream_failed", message: error.localizedDescription) + } + } + + // MARK: - Upstream URL + + /// Builds the upstream URL for a relayed request, or `nil` if the result + /// would address anything outside the site API root. + /// + /// Everything after the ``route`` prefix is treated as a path relative to + /// the API root and appended to it. Appending rather than resolving is what + /// `createRootURLMiddleware` does on the JavaScript side, and it is the only + /// approach that works for both root shapes WordPress produces: pretty + /// permalinks give `https://example.com/wp-json/`, plain permalinks give + /// `https://example.com/?rest_route=/`, where the path has to merge into an + /// existing query string. + /// + /// Dot segments — literal or percent-encoded — are refused rather than + /// normalized. A REST path never contains one, `URLSession` resolves them + /// before sending, and a normalized `..` is the one thing that could walk + /// out of the API root and reach the rest of the site with the credential + /// attached. + func upstreamURL(for request: ParsedHTTPRequest) -> URL? { + let path = request.path + guard path == Self.route || path.hasPrefix("\(Self.route)/") else { return nil } + + // Strip the route and any leading slashes, so the remainder appends to + // the API root rather than resolving against the site root. + let relativePath = path.dropFirst(Self.route.count).drop(while: { $0 == "/" }) + guard !Self.containsDotSegment(relativePath) else { return nil } + + var suffix = String(relativePath) + request.query + // A root that already carries a query (plain permalinks) continues it + // rather than starting a second one — mirroring `createRootURLMiddleware`. + if apiRoot.contains("?"), let separator = suffix.firstIndex(of: "?") { + suffix.replaceSubrange(separator...separator, with: "&") + } + + guard let url = URL(string: apiRoot + suffix), + url.absoluteString.hasPrefix(apiRoot) else { + return nil + } + return url + } + + /// Whether `path` contains a `.` or `..` segment, including the + /// percent-encoded spellings a server may decode before resolving it. + private static func containsDotSegment(_ path: some StringProtocol) -> Bool { + let lowercased = path.lowercased() + guard lowercased.contains(".") || lowercased.contains("%2e") else { return false } + return lowercased.split(separator: "/", omittingEmptySubsequences: false).contains { + let segment = $0.replacingOccurrences(of: "%2e", with: ".") + return segment == "." || segment == ".." + } + } + + /// Refuses redirects that leave the site API root. + /// + /// `URLSession` follows 3xx responses automatically, so without this the + /// containment check would only ever apply to the first hop: a site that + /// redirected `/wp-json/wp/v2/posts` elsewhere would have the request — + /// carrying the site credential — followed to that host, and its response + /// relayed back to the editor. Refusing hands the 3xx itself back instead. + /// + /// `@unchecked Sendable`: `allowedPrefix` is a `let` set at init and only + /// read afterwards. + private final class RedirectGuard: NSObject, URLSessionTaskDelegate, @unchecked Sendable { + private let allowedPrefix: String + + init(allowedPrefix: String) { + self.allowedPrefix = allowedPrefix + } + + func urlSession( + _ session: URLSession, + task: URLSessionTask, + willPerformHTTPRedirection response: HTTPURLResponse, + newRequest request: URLRequest, + completionHandler: @escaping (URLRequest?) -> Void + ) { + guard let url = request.url, url.absoluteString.hasPrefix(allowedPrefix) else { + Logger.restRelay.error("Refusing to follow a relay redirect outside the site API root") + completionHandler(nil) + return + } + completionHandler(request) } } @@ -121,19 +228,28 @@ struct RestRelay: Sendable { /// permissive CORS policy stamps `Access-Control-Allow-Origin` and friends; /// the exposed headers keep paginated REST responses readable to /// `api-fetch` callers. + /// + /// `Allow` is what `canUser` reads off an `OPTIONS` response to decide + /// whether the user may create a page, update settings, upload media, or + /// edit global styles; without it every such capability reads as false with + /// no error surfaced. `Link` backs `fetchAllMiddleware`'s pagination and + /// `X-WP-Total`/`X-WP-TotalPages` back list counts. Nothing else in the + /// editor reads a response header. private static let corsHeaders: [(String, String)] = [ - ("Access-Control-Expose-Headers", "X-WP-Total, X-WP-TotalPages, Link"), + ("Access-Control-Expose-Headers", "Allow, Link, X-WP-Total, X-WP-TotalPages"), ] /// Request headers that must not be forwarded upstream. /// /// `host`/`content-length`/`accept-encoding` are recalculated by URLSession; - /// `origin` and `referer` would leak the local page context to the server - /// (and WordPress rejects `file://` origins — the exact problem the relay - /// exists to solve); the rest are relay-internal. + /// `origin`, `referer`, and `sec-fetch-*` describe the web view's fetch + /// context and would leak the local page to the server (and WordPress + /// rejects `file://` origins — the exact problem the relay exists to + /// solve); the rest are relay-internal. private static let requestHeadersToStrip: Set = [ "host", "content-length", "accept-encoding", "connection", "origin", "referer", + "sec-fetch-site", "sec-fetch-mode", "sec-fetch-dest", "sec-fetch-user", "authorization", "relay-authorization", "proxy-authorization", ] @@ -165,22 +281,18 @@ struct RestRelay: Sendable { upstream.filter { !Self.responseHeadersToStrip.contains($0.0.lowercased()) } + cors } - /// Extracts the upstream URL from the relay request's query string. - private static func upstreamURL(from query: String) -> URL? { - var components = URLComponents() - components.percentEncodedQuery = query - guard let value = components.queryItems?.first(where: { $0.name == upstreamURLQueryItem })?.value, - let url = URL(string: value) else { - return nil - } - return url - } - - private static func errorResponse(status: Int, body: String) -> HTTPResponse { - HTTPResponse( + /// Emits a WordPress-REST-style error object rather than plain text, so the + /// editor decodes a relay failure the same way it decodes WordPress's own — + /// a `text/plain` body reaches JavaScript as an unparseable `invalid_json` + /// with the real reason lost. + static func errorResponse(status: Int, code: String, message: String) -> HTTPResponse { + let payload = ["code": code, "message": message] + let body = (try? JSONSerialization.data(withJSONObject: payload)) + ?? Data(#"{"code":"relay_error","message":"The editor could not reach the site."}"#.utf8) + return HTTPResponse( status: status, - headers: corsHeaders + [("Content-Type", "text/plain")], - body: Data(body.utf8) + headers: corsHeaders + [("Content-Type", "application/json")], + body: body ) } } diff --git a/ios/Tests/GutenbergKitTests/Media/RestRelayTests.swift b/ios/Tests/GutenbergKitTests/Media/RestRelayTests.swift new file mode 100644 index 000000000..b24edac16 --- /dev/null +++ b/ios/Tests/GutenbergKitTests/Media/RestRelayTests.swift @@ -0,0 +1,147 @@ +#if canImport(Network) + +import Foundation +import GutenbergKitHTTP +import Testing +@testable import GutenbergKit + +/// Covers how a relayed request's path becomes an upstream URL. This is the +/// relay's containment boundary: the web view supplies a path, never a URL, and +/// nothing it can put in that path may address anything outside the site API +/// root. +@Suite("RestRelay upstream URL") +struct RestRelayTests { + + /// A site on pretty permalinks. + private static let prettyRoot = URL(string: "https://example.com/wp-json/")! + + /// A site on plain permalinks, where the API root carries a query and the + /// path has to merge into it rather than start a second one. + private static let plainRoot = URL(string: "https://example.com/?rest_route=/")! + + // MARK: - Path resolution + + @Test("appends the path and query to the API root") + func appendsPathAndQuery() { + let relay = makeRelay(apiRoot: Self.prettyRoot) + #expect( + relay.upstreamURL(for: request("/proxy/wp/v2/posts?_locale=user"))?.absoluteString + == "https://example.com/wp-json/wp/v2/posts?_locale=user" + ) + } + + @Test("resolves the API root itself for a bare route") + func bareRouteResolvesToAPIRoot() { + let relay = makeRelay(apiRoot: Self.prettyRoot) + #expect(relay.upstreamURL(for: request("/proxy"))?.absoluteString == "https://example.com/wp-json/") + #expect(relay.upstreamURL(for: request("/proxy/"))?.absoluteString == "https://example.com/wp-json/") + } + + @Test("adds a trailing slash to an API root configured without one") + func normalizesAPIRootWithoutTrailingSlash() { + let relay = makeRelay(apiRoot: URL(string: "https://example.com/wp-json")!) + #expect( + relay.upstreamURL(for: request("/proxy/wp/v2/posts"))?.absoluteString + == "https://example.com/wp-json/wp/v2/posts" + ) + } + + @Test("continues the query of an API root that already carries one") + func mergesIntoAQueryCarryingAPIRoot() { + // Plain permalinks: `https://example.com/?rest_route=/` + `wp/v2/posts` + // has to produce one query string, not two — mirroring what + // `createRootURLMiddleware` does on the JavaScript side. + let relay = makeRelay(apiRoot: Self.plainRoot) + #expect( + relay.upstreamURL(for: request("/proxy/wp/v2/posts?_locale=user"))?.absoluteString + == "https://example.com/?rest_route=/wp/v2/posts&_locale=user" + ) + #expect( + relay.upstreamURL(for: request("/proxy/wp/v2/posts?a=1&b=2"))?.absoluteString + == "https://example.com/?rest_route=/wp/v2/posts&a=1&b=2" + ) + } + + @Test("preserves percent-encoding in the query") + func preservesPercentEncoding() { + let relay = makeRelay(apiRoot: Self.prettyRoot) + #expect( + relay.upstreamURL(for: request("/proxy/wp/v2/search?search=caf%C3%A9&per_page=100"))?.absoluteString + == "https://example.com/wp-json/wp/v2/search?search=caf%C3%A9&per_page=100" + ) + } + + // MARK: - Containment + + @Test("refuses a path that walks out of the API root") + func refusesDotSegments() { + let relay = makeRelay(apiRoot: Self.prettyRoot) + #expect(relay.upstreamURL(for: request("/proxy/../wp-admin/admin-ajax.php")) == nil) + #expect(relay.upstreamURL(for: request("/proxy/wp/v2/../../../wp-admin/")) == nil) + #expect(relay.upstreamURL(for: request("/proxy/wp/v2/./posts")) == nil) + } + + @Test("refuses percent-encoded dot segments") + func refusesEncodedDotSegments() { + // `URLSession` leaves these encoded, but the receiving server may decode + // before resolving, so they are refused here rather than forwarded. + let relay = makeRelay(apiRoot: Self.prettyRoot) + #expect(relay.upstreamURL(for: request("/proxy/%2e%2e/wp-admin/")) == nil) + #expect(relay.upstreamURL(for: request("/proxy/wp/%2E%2E/%2e%2e/")) == nil) + } + + @Test("a dot inside a path segment is not a dot segment") + func allowsDotsWithinSegments() { + let relay = makeRelay(apiRoot: Self.prettyRoot) + #expect( + relay.upstreamURL(for: request("/proxy/oembed/1.0/embed?url=https%3A%2F%2Fexample.com"))?.absoluteString + == "https://example.com/wp-json/oembed/1.0/embed?url=https%3A%2F%2Fexample.com" + ) + } + + @Test("an absolute URL in the path stays under the API root") + func absoluteURLInPathStaysContained() { + // There is no URL to resolve, so a smuggled one becomes an ordinary + // (404ing) path segment rather than another host. + let relay = makeRelay(apiRoot: Self.prettyRoot) + let resolved = relay.upstreamURL(for: request("/proxy/https://elsewhere.example/x")) + #expect(resolved?.absoluteString.hasPrefix("https://example.com/wp-json/") == true) + #expect(relay.upstreamURL(for: request("/proxy//elsewhere.example/x"))?.host() == "example.com") + } + + @Test("refuses a request outside the relay route") + func refusesForeignRoute() { + let relay = makeRelay(apiRoot: Self.prettyRoot) + #expect(relay.upstreamURL(for: request("/upload")) == nil) + #expect(relay.upstreamURL(for: request("/proxying/wp/v2/posts")) == nil) + } + + // MARK: - Routing + + @Test("claims its own route and nothing else") + func routeMatching() { + #expect(RestRelay.handles(request("/proxy"))) + #expect(RestRelay.handles(request("/proxy/wp/v2/posts?_locale=user"))) + #expect(!RestRelay.handles(request("/upload"))) + #expect(!RestRelay.handles(request("/proxying"))) + } + + // MARK: - Helpers + + private func makeRelay(apiRoot: URL) -> RestRelay { + RestRelay( + configuration: EditorConfigurationBuilder( + postType: .post, + siteURL: URL(string: "https://example.com")!, + siteApiRoot: apiRoot, + authHeader: "Bearer test-token" + ).build() + ) + } + + private func request(_ target: String, method: String = "GET") -> ParsedHTTPRequest { + .complete(method: method, target: target, httpVersion: "HTTP/1.1", headers: [:], body: nil) + } +} + +#endif // canImport(Network) From b68400c8002d7c773f328d7be1bd04ccf23d6457 Mon Sep 17 00:00:00 2001 From: David Calhoun Date: Fri, 28 Aug 2026 14:39:53 -0400 Subject: [PATCH 03/31] fix(ios): raise the local HTTP server's connection limit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `MediaUploadServer.start` omitted `maxConnections`, so the server ran on the library default of 5. That suits a server receiving one upload at a time, but this one also carries every editor REST request under Lockdown Mode, and editor boot fans out well past five. Each connection serves exactly one request, and one past the limit is closed immediately — surfacing in JavaScript as an unretried `fetch_error`. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_014A21xYJuymCU7My8yyS8Bv --- .../Sources/Media/MediaUploadServer.swift | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/ios/Sources/GutenbergKit/Sources/Media/MediaUploadServer.swift b/ios/Sources/GutenbergKit/Sources/Media/MediaUploadServer.swift index 4233bb47d..01cf4812a 100644 --- a/ios/Sources/GutenbergKit/Sources/Media/MediaUploadServer.swift +++ b/ios/Sources/GutenbergKit/Sources/Media/MediaUploadServer.swift @@ -26,6 +26,18 @@ final class MediaUploadServer: Sendable { /// Exposed so tests can await completion. (Mirrors Android's `cleanupJob`.) let cleanupTask: Task + /// The concurrent connection ceiling for the local server. + /// + /// The library's default of 5 suits a server that only ever receives one + /// upload at a time. This one also carries every REST request the editor + /// makes under Lockdown Mode (see ``RestRelay``), and editor boot fans out + /// well past five: each connection serves exactly one request + /// (`Connection: close`), and a connection past the limit is closed + /// immediately, surfacing in JavaScript as an unretried `fetch_error`. + /// WebKit caps its own concurrency per host well below this, so the ceiling + /// exists to bound a runaway, not to schedule normal traffic. + static let maxConnections = 32 + /// Creates and starts a new upload server. /// /// - Parameters: @@ -63,6 +75,7 @@ final class MediaUploadServer: Sendable { // every request it makes carries these headers. requiresBrowserOrigin: true, maxRequestBodySize: maxRequestBodySize, + maxConnections: maxConnections, bodyReadTimeout: bodyReadTimeout, cors: .permissive, delegate: ServerDelegate(), From 71d3ab617d0acd614728ad1905527217a04d2d0c Mon Sep 17 00:00:00 2001 From: David Calhoun Date: Fri, 28 Aug 2026 14:43:12 -0400 Subject: [PATCH 04/31] fix: route editor REST requests through the relay as the fetch handler MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The relay was a middleware that short-circuited past `next()`, but `apiFetch.use()` unshifts and api-fetch applies its middleware with `reduceRight`, so a registered middleware runs *outside* the four built-in ones and `defaultFetchHandler`, not inside them. Every relayed request therefore lost what those do: `options.data` never became a body (so every save sent `Content-Length: 0`, which WordPress accepts as a no-op — silent data loss), the `Accept` header WordPress uses to recognize a REST request was dropped, the HTTP v1 method override was lost, and `signal` never reached `fetch`, so cancellation did not propagate. Installing the relay as the fetch handler puts it where the old comment said it already was. Everything above is fixed at once, and `_locale=user` and the `per_page=-1` expansion now hold because their middleware runs, rather than by the accident of a failed direct attempt leaving its mutations behind. Which transport to use is now read from configuration. The relay is only advertised when the host knows direct requests cannot work, so a direct attempt first is a guaranteed-doomed round trip per request; the previous module-global flag inferred the answer from an observed success, which one misleading response could latch on for the rest of the session. The upstream path travels in the request path, so `Access-Control-Allow- Headers` no longer needs a relay header, and `PATCH` joins the allowed methods — it was blocked outright. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_014A21xYJuymCU7My8yyS8Bv --- ios/Sources/GutenbergKitHTTP/CORSPolicy.swift | 7 +- src/utils/api-fetch-relay.test.js | 298 ++++++++++++++++++ src/utils/api-fetch.js | 280 ++++++++++------ 3 files changed, 480 insertions(+), 105 deletions(-) create mode 100644 src/utils/api-fetch-relay.test.js diff --git a/ios/Sources/GutenbergKitHTTP/CORSPolicy.swift b/ios/Sources/GutenbergKitHTTP/CORSPolicy.swift index 26524d11e..e3aee1f30 100644 --- a/ios/Sources/GutenbergKitHTTP/CORSPolicy.swift +++ b/ios/Sources/GutenbergKitHTTP/CORSPolicy.swift @@ -31,8 +31,11 @@ public enum CORSPolicy: Sendable { // anyway: the editor loads from `file://` (Origin `null`), which // can't be cleanly allowlisted. ("Access-Control-Allow-Origin", "*"), - ("Access-Control-Allow-Methods", "GET, POST, PUT, DELETE, OPTIONS"), - ("Access-Control-Allow-Headers", "Authorization, Relay-Authorization, Content-Type"), + ("Access-Control-Allow-Methods", "GET, POST, PUT, PATCH, DELETE, OPTIONS"), + // `Accept` is listed because api-fetch's default value + // (`application/json, */*;q=0.1`) contains CORS-unsafe bytes, + // so it is not safelisted and does reach the preflight. + ("Access-Control-Allow-Headers", "Accept, Authorization, Content-Type, Relay-Authorization, X-HTTP-Method-Override"), ("Access-Control-Max-Age", "86400"), ] } diff --git a/src/utils/api-fetch-relay.test.js b/src/utils/api-fetch-relay.test.js new file mode 100644 index 000000000..657cc7564 --- /dev/null +++ b/src/utils/api-fetch-relay.test.js @@ -0,0 +1,298 @@ +/** + * External dependencies + */ +import { + describe, + it, + expect, + beforeAll, + beforeEach, + afterEach, + vi, +} from 'vitest'; + +/** + * WordPress dependencies + */ +import apiFetch from '@wordpress/api-fetch'; + +/** + * Internal dependencies + */ +import { configureApiFetch } from './api-fetch'; +import * as bridge from './bridge'; + +vi.mock( './bridge', async ( importOriginal ) => { + const actual = await importOriginal(); + return { + ...actual, + getGBKit: vi.fn(), + }; +} ); + +vi.mock( './logger', () => ( { + debug: vi.fn(), + info: vi.fn(), + warn: vi.fn(), + error: vi.fn(), +} ) ); + +const API_ROOT = 'https://example.com/wp-json/'; +const RELAY_ROOT = 'http://127.0.0.1:5555/proxy/'; + +const GBKIT = { + siteApiRoot: API_ROOT, + siteApiNamespace: [ 'wp/v2' ], + namespaceExcludedPaths: [], + authHeader: 'Bearer site-token', + networkProxy: { port: 5555, token: 'relay-token' }, +}; + +/** + * A minimal stand-in for the `Response` the relay returns, carrying only what + * api-fetch and its middleware read. + * + * @param {Object} options Response shape. + * @param {number} options.status HTTP status. + * @param {any} options.body The decoded JSON body, or a rejecting decoder. + * @param {Object} options.headers Response headers, lowercase-keyed. + * @return {Object} The response stand-in. + */ +function makeResponse( { status = 200, body = {}, headers = {} } = {} ) { + return { + ok: status >= 200 && status < 300, + status, + json: () => + body instanceof Error + ? Promise.reject( body ) + : Promise.resolve( body ), + headers: { get: ( name ) => headers[ name.toLowerCase() ] ?? null }, + }; +} + +/** + * The arguments of the nth `fetch` call, as `{ url, init }`. + * + * @param {number} index Call index. + * @return {{url: string, init: Object}} The call. + */ +function fetchCall( index = 0 ) { + const [ url, init ] = global.fetch.mock.calls[ index ]; + return { url, init }; +} + +describe( 'REST relay transport', () => { + beforeAll( () => { + bridge.getGBKit.mockReturnValue( GBKIT ); + configureApiFetch(); + } ); + + beforeEach( () => { + bridge.getGBKit.mockReturnValue( GBKIT ); + global.fetch = vi.fn( () => Promise.resolve( makeResponse() ) ); + } ); + + afterEach( () => { + vi.clearAllMocks(); + } ); + + describe( 'request', () => { + it( 'sends the upstream path below the API root to the relay route', async () => { + await apiFetch( { path: '/wp/v2/posts' } ); + + const { url } = fetchCall(); + expect( url ).toBe( `${ RELAY_ROOT }wp/v2/posts?_locale=user` ); + } ); + + it( 'authenticates to the relay without sending the site credential', async () => { + await apiFetch( { path: '/wp/v2/posts' } ); + + const { init } = fetchCall(); + expect( init.headers[ 'Relay-Authorization' ] ).toBe( + 'Bearer relay-token' + ); + // The relay injects the site credential natively, so it must not + // travel over loopback. + expect( init.headers.Authorization ).toBeUndefined(); + } ); + + it( 'sends the Accept header WordPress uses to recognize a REST request', async () => { + await apiFetch( { path: '/wp/v2/posts' } ); + + expect( fetchCall().init.headers.Accept ).toBe( + 'application/json, */*;q=0.1' + ); + } ); + + it( 'serializes `data` into a JSON body', async () => { + await apiFetch( { + path: '/wp/v2/posts/1', + method: 'POST', + data: { title: 'Hello' }, + } ); + + const { init } = fetchCall(); + expect( init.body ).toBe( JSON.stringify( { title: 'Hello' } ) ); + expect( init.headers[ 'Content-Type' ] ).toBe( 'application/json' ); + } ); + + it( 'applies the HTTP v1 method override', async () => { + await apiFetch( { + path: '/wp/v2/posts/1', + method: 'PUT', + data: { title: 'Hello' }, + } ); + + const { init } = fetchCall(); + expect( init.method ).toBe( 'POST' ); + expect( init.headers[ 'X-HTTP-Method-Override' ] ).toBe( 'PUT' ); + } ); + + it( 'forwards an abort signal', async () => { + const controller = new AbortController(); + await apiFetch( { + path: '/wp/v2/posts', + signal: controller.signal, + } ); + + expect( fetchCall().init.signal ).toBe( controller.signal ); + } ); + + it( 'does not send credentials the relay would reject', async () => { + // The relay answers `Access-Control-Allow-Origin: *`, which a + // browser refuses to pair with a credentialed request — and + // api-fetch defaults `credentials` to `include`. + await apiFetch( { path: '/wp/v2/posts' } ); + + expect( fetchCall().init.credentials ).toBeUndefined(); + } ); + + it( 'relays the absolute URL of a paginated next page', async () => { + // `fetchAllMiddleware` follows the absolute URL WordPress puts in + // the `Link` header, so the transport has to recognize the site's + // own API root in it. + global.fetch = vi + .fn() + .mockResolvedValueOnce( + makeResponse( { + body: [ { id: 1 } ], + headers: { + link: `<${ API_ROOT }wp/v2/posts?page=2>; rel="next"`, + }, + } ) + ) + .mockResolvedValueOnce( + makeResponse( { body: [ { id: 2 } ] } ) + ); + + const result = await apiFetch( { + path: '/wp/v2/posts?per_page=-1', + } ); + + // `per_page=-1` is expanded by api-fetch's own middleware rather + // than forwarded verbatim, which WordPress would reject. + expect( fetchCall( 0 ).url ).toContain( 'per_page=100' ); + expect( fetchCall( 0 ).url ).not.toContain( 'per_page=-1' ); + expect( fetchCall( 1 ).url ).toBe( + `${ RELAY_ROOT }wp/v2/posts?page=2&_locale=user` + ); + expect( result ).toEqual( [ { id: 1 }, { id: 2 } ] ); + } ); + + it( 'refuses a request for somewhere other than the site API', async () => { + await expect( + apiFetch( { url: 'https://elsewhere.example/x' } ) + ).rejects.toMatchObject( { code: 'fetch_error' } ); + + expect( global.fetch ).not.toHaveBeenCalled(); + } ); + } ); + + describe( 'response', () => { + it( 'resolves the decoded body', async () => { + global.fetch = vi.fn( () => + Promise.resolve( makeResponse( { body: { id: 7 } } ) ) + ); + + await expect( + apiFetch( { path: '/wp/v2/posts/7' } ) + ).resolves.toEqual( { id: 7 } ); + } ); + + it( 'resolves a 204 to null', async () => { + global.fetch = vi.fn( () => + Promise.resolve( makeResponse( { status: 204 } ) ) + ); + + await expect( + apiFetch( { path: '/wp/v2/posts/7' } ) + ).resolves.toBeNull(); + } ); + + it( 'returns the raw response when parsing is declined', async () => { + const response = makeResponse( { + headers: { allow: 'GET, POST' }, + } ); + global.fetch = vi.fn( () => Promise.resolve( response ) ); + + // `canUser` reads the `Allow` header off an unparsed response. + const result = await apiFetch( { + path: '/wp/v2/posts', + method: 'OPTIONS', + parse: false, + } ); + + expect( result ).toBe( response ); + expect( result.headers.get( 'allow' ) ).toBe( 'GET, POST' ); + } ); + + it( 'throws the decoded error body of a failed request', async () => { + global.fetch = vi.fn( () => + Promise.resolve( + makeResponse( { + status: 403, + body: { + code: 'rest_cannot_edit', + message: 'Sorry, you are not allowed to do that.', + }, + } ) + ) + ); + + await expect( + apiFetch( { path: '/wp/v2/posts/7' } ) + ).rejects.toMatchObject( { code: 'rest_cannot_edit' } ); + } ); + + it( 'normalizes an undecodable body', async () => { + global.fetch = vi.fn( () => + Promise.resolve( + makeResponse( { body: new Error( 'not json' ) } ) + ) + ); + + await expect( + apiFetch( { path: '/wp/v2/posts/7' } ) + ).rejects.toMatchObject( { code: 'invalid_json' } ); + } ); + + it( 'normalizes a transport failure', async () => { + global.fetch = vi.fn( () => + Promise.reject( new TypeError( 'Load failed' ) ) + ); + + await expect( + apiFetch( { path: '/wp/v2/posts/7' } ) + ).rejects.toMatchObject( { code: 'fetch_error' } ); + } ); + + it( 're-throws an abort for the caller to handle', async () => { + const abortError = new DOMException( 'Aborted', 'AbortError' ); + global.fetch = vi.fn( () => Promise.reject( abortError ) ); + + await expect( apiFetch( { path: '/wp/v2/posts/7' } ) ).rejects.toBe( + abortError + ); + } ); + } ); +} ); diff --git a/src/utils/api-fetch.js b/src/utils/api-fetch.js index 8fdf82801..8b3d206d8 100644 --- a/src/utils/api-fetch.js +++ b/src/utils/api-fetch.js @@ -9,7 +9,7 @@ import { __ } from '@wordpress/i18n'; * Internal dependencies */ import { getGBKit, POST_FALLBACKS } from './bridge'; -import { debug, info, error as logError } from './logger'; +import { info, error as logError } from './logger'; /** * @typedef {import('@wordpress/api-fetch').APIFetchMiddleware} APIFetchMiddleware @@ -24,11 +24,24 @@ const MEDIA_UPLOAD_PATH = /^\/wp\/v2\/media(\?|$)/; * @return {void} */ export function configureApiFetch() { - const { siteApiRoot = '', preloadData = null } = getGBKit(); + const { + siteApiRoot = '', + preloadData = null, + networkProxy = null, + } = getGBKit(); + + // The relay replaces the transport rather than intercepting requests, + // because `apiFetch.use()` unshifts and api-fetch applies its middleware + // with `reduceRight`: a registered middleware can never run innermost, so + // interception there would short-circuit past api-fetch's own middleware — + // losing `_locale=user`, the `per_page=-1` expansion, and the HTTP v1 + // method override — and past everything `defaultFetchHandler` does. + if ( networkProxy ) { + apiFetch.setFetchHandler( + createRelayFetchHandler( networkProxy, siteApiRoot ) + ); + } - // Registered first so it runs innermost (after all option transforms), - // where it can retry the fully-built request through the native proxy. - apiFetch.use( networkProxyFallbackMiddleware ); apiFetch.use( apiFetch.createRootURLMiddleware( siteApiRoot ) ); apiFetch.use( corsMiddleware ); apiFetch.use( apiPathModifierMiddleware ); @@ -43,116 +56,171 @@ export function configureApiFetch() { } /** - * Tracks whether the native network proxy successfully served a request. - * Once it has, subsequent requests go straight to the proxy instead of - * paying for a doomed direct attempt first. + * Header values api-fetch's own handler defaults, which this transport + * replaces. WordPress treats `Accept` as a condition for handling a request as + * a REST request (core trac 44534), so losing it changes what the site returns. */ -let isNetworkProxyPreferred = false; +const DEFAULT_RELAY_HEADERS = { + Accept: 'application/json, */*;q=0.1', +}; /** - * Middleware that retries failed requests through the native loopback proxy. - * - * Under iOS Lockdown Mode the editor's `file://` page loses its CORS - * exemption and WordPress sanitizes its `Origin: file://` into an empty - * `Access-Control-Allow-Origin`, so every REST request rejects with - * api-fetch's generic `fetch_error`. When the native host provides a - * loopback proxy (`GBKit.networkProxy`), such failures are retried through - * it: the proxy forwards the request to the site's REST API natively and - * responds with CORS headers the web view accepts. - * - * This middleware must run innermost so `options` carries the final - * request (absolute `url`, headers, body) built by the other middleware. - * - * @type {APIFetchMiddleware} + * Builds the api-fetch transport that routes every request through the native + * loopback relay. + * + * Under iOS Lockdown Mode the editor's `file://` page loses its CORS exemption + * and WordPress sanitizes its `Origin: file://` into an empty + * `Access-Control-Allow-Origin`, so every direct REST request rejects. The + * native host answers by running a loopback server and advertising it as + * `GBKit.networkProxy`; it forwards each request to the site's REST API + * natively and responds with CORS headers the web view accepts. + * + * Whether to use it is decided by that configuration alone. The relay is only + * advertised when the host knows direct requests cannot work, so attempting one + * first would be a guaranteed-doomed round trip per request — and inferring the + * answer from a response, as this once did, means one misleading response can + * latch the editor onto the wrong path for the rest of the session. + * + * Installed as the fetch handler rather than a middleware, so api-fetch's own + * middleware has already run and `options` carries the finished request. + * `defaultFetchHandler` is not exported, so what it does — `data` to a JSON + * body, default headers, response parsing and error normalization — is + * reproduced here. + * + * @param {Object} networkProxy Relay connection details. + * @param {number} networkProxy.port Loopback port. + * @param {string} networkProxy.token Per-session bearer token. + * @param {string} siteApiRoot The site's REST API root. + * @return {(options: Object) => Promise} The fetch handler. */ -function networkProxyFallbackMiddleware( options, next ) { - const { networkProxy } = getGBKit(); - - if ( ! networkProxy ) { - return next( options ); - } +function createRelayFetchHandler( networkProxy, siteApiRoot ) { + const apiRoot = siteApiRoot.endsWith( '/' ) + ? siteApiRoot + : `${ siteApiRoot }/`; + // The upstream path follows the route; the relay resolves it natively + // against the site API root, so no absolute URL is ever sent. + const relayRoot = `http://127.0.0.1:${ networkProxy.port }/proxy/`; + + return async ( options ) => { + const { data, parse = true, method = 'GET', signal } = options; + + const upstreamPath = relayUpstreamPath( + options.url ?? options.path ?? '', + apiRoot + ); + if ( upstreamPath === null ) { + logError( + 'api-fetch: refusing to relay a request outside the site API root', + options.url ?? options.path + ); + throw { + code: 'fetch_error', + message: __( + 'Could not get a valid response from the server.' + ), + }; + } - if ( isNetworkProxyPreferred ) { - return proxyFetch( options, networkProxy ); - } + const headers = { ...DEFAULT_RELAY_HEADERS, ...options.headers }; + // The upstream `Authorization` header is injected natively, so the + // site credential never travels over loopback. The relay's own + // per-session token rides in `Relay-Authorization` because `fetch()` + // silently strips `Proxy-*` headers. + delete headers.Authorization; + headers[ 'Relay-Authorization' ] = `Bearer ${ networkProxy.token }`; + + // `data` is api-fetch's shorthand for a JSON body, serialized by the + // handler rather than by a middleware. Missing it meant every relayed + // write sent `Content-Length: 0`, which WordPress accepts as a no-op: + // silent data loss on every save. + let { body } = options; + if ( data ) { + body = JSON.stringify( data ); + headers[ 'Content-Type' ] = 'application/json'; + } - return next( options ).catch( ( fetchError ) => { - if ( fetchError?.code !== 'fetch_error' ) { - throw fetchError; + let response; + try { + // Only the fields the transport needs, rather than spreading the + // remaining options: api-fetch defaults `credentials` to + // `include`, which a browser refuses against the relay's + // `Access-Control-Allow-Origin: *`. + response = await globalThis.fetch( relayRoot + upstreamPath, { + method, + headers, + body, + signal, + } ); + } catch ( relayError ) { + // Mirror `defaultFetchHandler`'s rejection handling, including + // re-throwing an abort for the caller to handle itself. + if ( relayError?.name === 'AbortError' ) { + throw relayError; + } + logError( 'api-fetch: relayed request failed', relayError ); + if ( ! globalThis.navigator.onLine ) { + throw { + code: 'offline_error', + message: __( + 'Unable to connect. Please check your Internet connection.' + ), + }; + } + throw { + code: 'fetch_error', + message: __( + 'Could not get a valid response from the server.' + ), + }; } - debug( - 'api-fetch: direct request failed, retrying through the native network proxy' - ); - return proxyFetch( options, networkProxy ).then( ( result ) => { - isNetworkProxyPreferred = true; - return result; - } ); - } ); + return parseRelayResponse( response, parse ); + }; } /** - * Performs a request through the native loopback proxy. - * - * The absolute upstream URL travels in the `url` query parameter (a query - * parameter rather than a custom header, so the local server's stock CORS - * policy covers the preflight) and the per-session proxy token in - * `Relay-Authorization` (`Proxy-*` headers are stripped by `fetch()`). The - * upstream `Authorization` header is injected natively, so any value present - * here is dropped. - * - * @param {Object} options Fully-transformed api-fetch options. - * @param {Object} networkProxy Proxy connection details. - * @param {number} networkProxy.port Loopback port. - * @param {string} networkProxy.token Per-session bearer token. - * @return {Promise} The parsed response, mirroring api-fetch semantics. + * The upstream path for a relayed request: the part of its target below the + * site API root, without a leading slash. + * + * Returns `null` for a target the relay cannot serve — an absolute URL for + * somewhere other than the site's API. Paginated requests arrive this way: + * `fetchAllMiddleware` follows the absolute URL WordPress puts in the `Link` + * header, so the relay has to recognize the site's own API root in it. + * + * @param {string} target The request's `url`, or its `path` when no root URL + * middleware has run. + * @param {string} apiRoot The site's REST API root, slash-terminated. + * @return {string|null} The upstream path, or `null` when it cannot be relayed. */ -async function proxyFetch( options, networkProxy ) { - const upstreamUrl = options.url ?? options.path; - const headers = { ...( options.headers || {} ) }; - delete headers.Authorization; - headers[ 'Relay-Authorization' ] = `Bearer ${ networkProxy.token }`; - - let response; - try { - response = await window.fetch( - `http://127.0.0.1:${ - networkProxy.port - }/proxy?url=${ encodeURIComponent( upstreamUrl ) }`, - { - method: options.method || 'GET', - headers, - body: options.body, - } - ); - } catch ( proxyError ) { - logError( - 'api-fetch: native network proxy request failed', - proxyError - ); - throw { - code: 'fetch_error', - message: 'Could not get a valid response from the server.', - }; +function relayUpstreamPath( target, apiRoot ) { + if ( target.startsWith( apiRoot ) ) { + return target.slice( apiRoot.length ); } - - return parseProxyResponse( response, options.parse ?? true ); + // Anything that is not an absolute URL is a path relative to the API root. + if ( ! /^([a-z][a-z0-9+.-]*:|\/\/)/i.test( target ) ) { + return target.replace( /^\/+/, '' ); + } + return null; } /** - * Parses a proxied response, mirroring api-fetch's default handler: - * unparsed requests get the raw `Response`, 204s resolve to `null`, - * error statuses throw the decoded JSON body. + * Parses a relayed response the way api-fetch's own handler does: unparsed + * requests get the raw `Response`, 204s resolve to `null`, and error statuses + * throw the decoded JSON body. * - * @param {Response} response The proxy response. + * @param {Response} response The relay response. * @param {boolean} shouldParse Whether the caller requested parsing. - * @return {Promise} The parsed body or raw response. + * @return {Promise} The parsed body or the raw response. */ -async function parseProxyResponse( response, shouldParse ) { - if ( ! shouldParse ) { - if ( ! response.ok ) { +async function parseRelayResponse( response, shouldParse ) { + if ( ! response.ok ) { + if ( ! shouldParse ) { throw response; } + throw await parseRelayJSON( response ); + } + + if ( ! shouldParse ) { return response; } @@ -160,21 +228,27 @@ async function parseProxyResponse( response, shouldParse ) { return null; } - let json; + return parseRelayJSON( response ); +} + +/** + * Decodes a relayed response body, normalizing a decode failure into the same + * error api-fetch raises. The relay reports its own failures as WordPress-shaped + * `{ code, message }` JSON, so a real error reaches the caller intact rather + * than as an opaque parse failure. + * + * @param {Response} response The relay response. + * @return {Promise} The decoded body. + */ +async function parseRelayJSON( response ) { try { - json = await response.json(); + return await response.json(); } catch { throw { code: 'invalid_json', - message: 'The response is not a valid JSON response.', + message: __( 'The response is not a valid JSON response.' ), }; } - - if ( ! response.ok ) { - throw json; - } - - return json; } /** From 051d50f35559a191268add2bbee18f443020bc8e Mon Sep 17 00:00:00 2001 From: David Calhoun Date: Fri, 28 Aug 2026 14:43:54 -0400 Subject: [PATCH 05/31] chore(ios): drop the demo app's baked-in wp-env credentials `LocalWordPressCredentials.load()` fell back to a compiled-in application password and a hardcoded LAN IP when `WP_ENV_CREDENTIALS_PATH` was unset. Throwaway local dev credentials rather than production ones, but they should not ship, and the silent fallback turned a misconfigured environment into a confusing failure against someone else's machine. `SitePreparationView` already explains what to run when `load()` returns nil. Keeping the screen awake is now opt-in behind `GUTENBERG_DISABLE_IDLE_ TIMER` rather than unconditional; it exists for debugging workflows that break on auto-lock. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_014A21xYJuymCU7My8yyS8Bv --- ios/Demo-iOS/Sources/ConfigurationItem.swift | 27 +++++++------------- ios/Demo-iOS/Sources/GutenbergApp.swift | 11 +++++--- 2 files changed, 16 insertions(+), 22 deletions(-) diff --git a/ios/Demo-iOS/Sources/ConfigurationItem.swift b/ios/Demo-iOS/Sources/ConfigurationItem.swift index 5abec3156..282245d4d 100644 --- a/ios/Demo-iOS/Sources/ConfigurationItem.swift +++ b/ios/Demo-iOS/Sources/ConfigurationItem.swift @@ -68,27 +68,18 @@ struct LocalWordPressCredentials: Codable { let authHeader: String /// Loads credentials from the file path specified in the `WP_ENV_CREDENTIALS_PATH` environment variable. + /// + /// Returns `nil` when the variable is unset or the file cannot be read, so + /// a misconfigured environment surfaces as the "not configured" message + /// rather than as a confusing failure against some other site. static func load() -> LocalWordPressCredentials? { - if let path = ProcessInfo.processInfo.environment["WP_ENV_CREDENTIALS_PATH"], - let data = FileManager.default.contents(atPath: path), - let credentials = try? JSONDecoder().decode(LocalWordPressCredentials.self, from: data) { - return credentials + guard let path = ProcessInfo.processInfo.environment["WP_ENV_CREDENTIALS_PATH"], + let data = FileManager.default.contents(atPath: path), + let credentials = try? JSONDecoder().decode(LocalWordPressCredentials.self, from: data) else { + return nil } - - return .bakedIn + return credentials } - - /// Debug automation: physical devices can't read the wp-env credentials - /// file from the Mac's filesystem, so fall back to compiled-in wp-env - /// credentials that point at the Mac's LAN IP. These are throwaway local - /// dev credentials generated by `make wp-env-start`. - static let bakedIn = LocalWordPressCredentials( - siteUrl: "http://192.168.0.57:8888", - siteApiRoot: "http://192.168.0.57:8888/wp-json/", - username: "admin", - appPassword: "lsei gHof sVsj ITvL pMuC qB5U", - authHeader: "Basic YWRtaW46bHNlaSBnSG9mIHNWc2ogSVR2TCBwTXVDIHFCNVU=" - ) } // MARK: - Account Helpers diff --git a/ios/Demo-iOS/Sources/GutenbergApp.swift b/ios/Demo-iOS/Sources/GutenbergApp.swift index 5df6f83ac..9764562e4 100644 --- a/ios/Demo-iOS/Sources/GutenbergApp.swift +++ b/ios/Demo-iOS/Sources/GutenbergApp.swift @@ -45,10 +45,13 @@ struct GutenbergApp: App { EditorLogger.shared = OSLogEditorLogger() EditorLogger.logLevel = .debug - // Keep the device awake while the demo app is foregrounded — the - // debugging workflows here (probes, Web Inspector, devicectl console) - // break when the device auto-locks. - UIApplication.shared.isIdleTimerDisabled = true + // Opt-in: keep the device awake while the demo app is foregrounded. + // The debugging workflows here (probes, Web Inspector, devicectl + // console) break when the device auto-locks, but a demo app that never + // lets the screen sleep is its own surprise. + if ProcessInfo.processInfo.environment["GUTENBERG_DISABLE_IDLE_TIMER"] == "1" { + UIApplication.shared.isIdleTimerDisabled = true + } OriginProbeRunner.runIfRequested() } From f03fe6db43eb7f8369b4a4be22cd0539cda9ae10 Mon Sep 17 00:00:00 2001 From: David Calhoun Date: Fri, 28 Aug 2026 14:51:33 -0400 Subject: [PATCH 06/31] test(ios): cover the REST relay against a live WordPress site MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The relay's defects were invisible to unit tests: each one needed a real request to cross the loopback server, be forwarded natively, and come back. These tests do that against the `make wp-env-start` environment, and are skipped unless `WP_ENV_CREDENTIALS_PATH` is set, so a normal run and CI never need a site. The write test reads the record back rather than trusting the create response, because the defect it covers sent an empty body — which WordPress accepts as a no-op while still answering 2xx. `canUser` is not verifiable here: the Playground runtime's web server answers every `OPTIONS` itself with a bodiless 204 before WordPress is reached, so there is no `Allow` header to relay. The test asserts the relay forwards the `OPTIONS` upstream instead, which is the half of that chain this code owns. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_014A21xYJuymCU7My8yyS8Bv --- .../Media/RestRelayIntegrationTests.swift | 238 ++++++++++++++++++ 1 file changed, 238 insertions(+) create mode 100644 ios/Tests/GutenbergKitTests/Media/RestRelayIntegrationTests.swift diff --git a/ios/Tests/GutenbergKitTests/Media/RestRelayIntegrationTests.swift b/ios/Tests/GutenbergKitTests/Media/RestRelayIntegrationTests.swift new file mode 100644 index 000000000..dac62bb5a --- /dev/null +++ b/ios/Tests/GutenbergKitTests/Media/RestRelayIntegrationTests.swift @@ -0,0 +1,238 @@ +#if canImport(Network) + +import Foundation +import GutenbergKitHTTP +import Testing +@testable import GutenbergKit + +/// End-to-end coverage of the relay against a real WordPress site: the request +/// crosses the loopback server, is forwarded natively, and the site's response +/// comes back to the caller. +/// +/// Skipped unless `WP_ENV_CREDENTIALS_PATH` points at the credentials file +/// `make wp-env-start` writes, so a normal test run — and CI — never needs a +/// site: +/// +/// ```sh +/// make wp-env-start +/// WP_ENV_CREDENTIALS_PATH="$PWD/.wp-env.credentials.json" swift test +/// ``` +/// +/// **`canUser` is not verifiable here.** The Playground runtime's web server +/// answers every `OPTIONS` itself with a bodiless 204 and permissive CORS +/// headers, before WordPress is reached, so no `Allow` header exists to relay. +/// What is checked instead is that the relay *forwards* an `OPTIONS` rather +/// than answering it locally — the defect that made `canUser` report no +/// capabilities at all. +@Suite("RestRelay against a live site", .enabled(if: WPEnvSite.current != nil), .serialized) +struct RestRelayIntegrationTests { + + // MARK: - Reads + + @Test("relays a GET and returns the site's response") + func relaysGet() async throws { + try await withRelay { server, site in + let (data, response) = try await site.relayed("wp/v2/posts?per_page=1&_locale=user", on: server) + + #expect(response.statusCode == 200) + #expect((try? JSONSerialization.jsonObject(with: data)) as? [Any] != nil) + } + } + + @Test("exposes the response headers the editor reads") + func exposesResponseHeaders() async throws { + try await withRelay { server, site in + let (_, response) = try await site.relayed("wp/v2/posts?per_page=1", on: server) + + let exposed = try #require(response.value(forHTTPHeaderField: "Access-Control-Expose-Headers")) + // `Allow` backs `canUser`, `Link` backs pagination, and the totals + // back list counts. Unexposed, each reads as null in JavaScript. + for header in ["Allow", "Link", "X-WP-Total", "X-WP-TotalPages"] { + #expect(exposed.contains(header)) + } + } + } + + @Test("forwards an OPTIONS request upstream instead of answering it locally") + func forwardsOptions() async throws { + try await withRelay { server, site in + let (_, response) = try await site.relayed("wp/v2/pages", method: "OPTIONS", on: server) + + // The local server would answer with a bare 204 carrying only its + // own CORS headers. A response bearing the site's server headers + // can only have come from the site. + #expect(response.value(forHTTPHeaderField: "X-Powered-By") != nil) + } + } + + @Test("serves concurrent requests past the library's default connection cap") + func servesConcurrentRequests() async throws { + try await withRelay { server, site in + // Editor boot fans out well past the default of 5, and a connection + // over the limit is closed rather than queued. + let statuses = try await withThrowingTaskGroup(of: Int.self) { group in + for page in 1...10 { + group.addTask { + let (_, response) = try await site.relayed( + "wp/v2/types?_locale=user&page=\(page)", on: server + ) + return response.statusCode + } + } + return try await group.reduce(into: [Int]()) { $0.append($1) } + } + + #expect(statuses.count == 10) + #expect(statuses.allSatisfy { $0 == 200 }) + } + } + + // MARK: - Writes + + @Test("relays a write body, and the site actually stores it") + func relaysWriteBody() async throws { + try await withRelay { server, site in + let title = "Relay integration \(UUID().uuidString.prefix(8))" + + let (created, createResponse) = try await site.relayed( + "wp/v2/posts", + method: "POST", + body: ["title": title, "status": "draft"], + on: server + ) + #expect(createResponse.statusCode == 201) + + let post = try #require((try? JSONSerialization.jsonObject(with: created)) as? [String: Any]) + let id = try #require(post["id"] as? Int) + + // Read it back rather than trusting the create response: the defect + // this covers sent an empty body, which WordPress accepts as a + // no-op while still answering 2xx. + let (fetched, _) = try await site.relayed("wp/v2/posts/\(id)?context=edit", on: server) + let stored = try #require((try? JSONSerialization.jsonObject(with: fetched)) as? [String: Any]) + let storedTitle = (stored["title"] as? [String: Any])?["raw"] as? String + #expect(storedTitle == title) + + _ = try await site.relayed( + "wp/v2/posts/\(id)?force=true", + method: "POST", + headers: ["X-HTTP-Method-Override": "DELETE"], + on: server + ) + } + } + + // MARK: - Refusals + + @Test("refuses a path that walks out of the API root, in a shape the editor can read") + func refusesEscapingPath() async throws { + try await withRelay { server, site in + let (data, response) = try await site.relayed("../wp-admin/admin-ajax.php", on: server) + + #expect(response.statusCode == 403) + #expect(response.value(forHTTPHeaderField: "Content-Type") == "application/json") + // A `text/plain` body reaches JavaScript as an unparseable + // `invalid_json` with the real reason lost. + let error = try #require((try? JSONSerialization.jsonObject(with: data)) as? [String: Any]) + #expect(error["code"] as? String == "relay_forbidden_path") + #expect(error["message"] is String) + } + } + + @Test("refuses a request without the relay token") + func refusesUnauthenticated() async throws { + try await withRelay { server, _ in + var request = URLRequest(url: URL(string: "http://127.0.0.1:\(server.port)/proxy/wp/v2/posts")!) + request.setValue("file://", forHTTPHeaderField: "Origin") + + let (_, response) = try await URLSession.shared.data(for: request) + #expect((response as? HTTPURLResponse)?.statusCode == 407) + } + } + + // MARK: - Helpers + + /// Runs `body` against a local server hosting a relay for the wp-env site, + /// stopping it afterwards. + private func withRelay( + _ body: (MediaUploadServer, WPEnvSite) async throws -> Void + ) async throws { + let site = try #require(WPEnvSite.current) + let server = try await MediaUploadServer.start(restRelay: RestRelay(configuration: site.configuration)) + defer { server.stop() } + try await body(server, site) + } +} + +/// The local WordPress environment `make wp-env-start` provisions, as described +/// by the credentials file it writes. +struct WPEnvSite { + let apiRoot: URL + let authHeader: String + + /// The site described by `WP_ENV_CREDENTIALS_PATH`, or `nil` when the + /// variable is unset or the file cannot be read. + static let current: WPEnvSite? = { + struct Credentials: Decodable { + let siteApiRoot: String + let authHeader: String + } + guard let path = ProcessInfo.processInfo.environment["WP_ENV_CREDENTIALS_PATH"], + let data = FileManager.default.contents(atPath: path), + let credentials = try? JSONDecoder().decode(Credentials.self, from: data), + let apiRoot = URL(string: credentials.siteApiRoot) else { + return nil + } + return WPEnvSite(apiRoot: apiRoot, authHeader: credentials.authHeader) + }() + + /// A session that will actually open ten connections at once. The shared + /// session caps concurrency per host at six, which would leave the + /// connection-limit test unable to fail. + private static let session: URLSession = { + let configuration = URLSessionConfiguration.ephemeral + configuration.httpMaximumConnectionsPerHost = 10 + return URLSession(configuration: configuration) + }() + + var configuration: EditorConfiguration { + EditorConfigurationBuilder( + postType: .post, + siteURL: apiRoot, + siteApiRoot: apiRoot, + authHeader: authHeader + ).build() + } + + /// Issues a request through the relay the way the editor's `fetch()` does: + /// the upstream path below the route, the relay's own bearer token, and the + /// headers WebKit sets on every cross-origin fetch. + func relayed( + _ path: String, + method: String = "GET", + body: [String: Any]? = nil, + headers: [String: String] = [:], + on server: MediaUploadServer + ) async throws -> (Data, HTTPURLResponse) { + var request = URLRequest(url: URL(string: "http://127.0.0.1:\(server.port)/proxy/\(path)")!) + request.httpMethod = method + request.setValue("Bearer \(server.token)", forHTTPHeaderField: "Relay-Authorization") + request.setValue("file://", forHTTPHeaderField: "Origin") + request.setValue("application/json, */*;q=0.1", forHTTPHeaderField: "Accept") + for (name, value) in headers { + request.setValue(value, forHTTPHeaderField: name) + } + if let body { + request.httpBody = try JSONSerialization.data(withJSONObject: body) + request.setValue("application/json", forHTTPHeaderField: "Content-Type") + } + + let (data, response) = try await Self.session.data(for: request) + guard let http = response as? HTTPURLResponse else { + throw URLError(.badServerResponse) + } + return (data, http) + } +} + +#endif // canImport(Network) From ef857d07b7b22f0d2a7613641bda43d67f54c6e7 Mon Sep 17 00:00:00 2001 From: David Calhoun Date: Fri, 28 Aug 2026 18:07:32 -0400 Subject: [PATCH 07/31] fix: never take the relay's port and token from persisted config MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `getGBKit()` falls back to the copy of the config in `localStorage`, which iOS writes on every load and never clears. A relay's port and per-session token belong to the server that issued them, so a persisted copy points at a listener that has been stopped — or at a port something else now owns. That was survivable while the relay was a fallback for requests that had already failed. It is not now that it is the transport: every REST request in the session would go to the stale port. Read the relay's details from the injected global only, and fall back to no relay, which is what requests did before one existed. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_014A21xYJuymCU7My8yyS8Bv --- src/utils/api-fetch-relay.test.js | 3 +++ src/utils/api-fetch.js | 9 +++---- src/utils/bridge.js | 20 ++++++++++++++++ src/utils/bridge.test.js | 39 ++++++++++++++++++++++++++++++- 4 files changed, 64 insertions(+), 7 deletions(-) diff --git a/src/utils/api-fetch-relay.test.js b/src/utils/api-fetch-relay.test.js index 657cc7564..63a07e97e 100644 --- a/src/utils/api-fetch-relay.test.js +++ b/src/utils/api-fetch-relay.test.js @@ -83,6 +83,9 @@ function fetchCall( index = 0 ) { describe( 'REST relay transport', () => { beforeAll( () => { + // The relay's connection details are read from the injected global + // rather than through `getGBKit`, so they have to be there. + window.GBKit = GBKIT; bridge.getGBKit.mockReturnValue( GBKIT ); configureApiFetch(); } ); diff --git a/src/utils/api-fetch.js b/src/utils/api-fetch.js index 8b3d206d8..0ea40b57a 100644 --- a/src/utils/api-fetch.js +++ b/src/utils/api-fetch.js @@ -8,7 +8,7 @@ import { __ } from '@wordpress/i18n'; /** * Internal dependencies */ -import { getGBKit, POST_FALLBACKS } from './bridge'; +import { getGBKit, getNetworkProxy, POST_FALLBACKS } from './bridge'; import { info, error as logError } from './logger'; /** @@ -24,11 +24,8 @@ const MEDIA_UPLOAD_PATH = /^\/wp\/v2\/media(\?|$)/; * @return {void} */ export function configureApiFetch() { - const { - siteApiRoot = '', - preloadData = null, - networkProxy = null, - } = getGBKit(); + const { siteApiRoot = '', preloadData = null } = getGBKit(); + const networkProxy = getNetworkProxy(); // The relay replaces the transport rather than intercepting requests, // because `apiFetch.use()` unshifts and api-fetch applies its middleware diff --git a/src/utils/bridge.js b/src/utils/bridge.js index f04b50830..a84994588 100644 --- a/src/utils/bridge.js +++ b/src/utils/bridge.js @@ -268,6 +268,26 @@ export function getGBKit() { } } +/** + * The native loopback relay's connection details, or `null` when the host is + * not running one. + * + * Read from the injected global only, never through ``getGBKit``: its + * `localStorage` fallback returns whatever the last editor session wrote, and + * iOS does not clear it. A relay's port and per-session token are valid only + * for the server that issued them, so a persisted copy points at a listener + * that has been stopped — or, worse, at a port something else now owns. Every + * REST request would go there. + * + * Falling back to no relay is the safe direction to be wrong in: requests take + * the direct path, which is what they did before a relay existed. + * + * @return {{port: number, token: string}|null} The relay details. + */ +export function getNetworkProxy() { + return window.GBKit?.networkProxy ?? null; +} + /** * @typedef {Object} Post * @property {string} [title] The title of the post. diff --git a/src/utils/bridge.test.js b/src/utils/bridge.test.js index 62178fb1a..36effe142 100644 --- a/src/utils/bridge.test.js +++ b/src/utils/bridge.test.js @@ -6,7 +6,12 @@ import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; /** * Internal dependencies */ -import { requestLatestContent, getPost, showBlockInserter } from './bridge'; +import { + requestLatestContent, + getPost, + getNetworkProxy, + showBlockInserter, +} from './bridge'; vi.mock( './logger.js', () => ( { error: vi.fn(), @@ -545,3 +550,35 @@ describe( 'showBlockInserter', () => { expect( postMessage ).toHaveBeenCalledTimes( 1 ); } ); } ); + +describe( 'getNetworkProxy', () => { + afterEach( () => { + delete window.GBKit; + localStorage.clear(); + } ); + + it( 'returns the injected relay details', () => { + window.GBKit = { networkProxy: { port: 5555, token: 'fresh' } }; + + expect( getNetworkProxy() ).toEqual( { port: 5555, token: 'fresh' } ); + } ); + + it( 'returns null when the host is not running a relay', () => { + window.GBKit = { siteApiRoot: 'https://example.com/wp-json/' }; + + expect( getNetworkProxy() ).toBeNull(); + } ); + + it( 'ignores a persisted relay from an earlier session', () => { + // A relay's port and token belong to the server that issued them, and + // iOS never clears `GBKit` from localStorage. Trusting a persisted copy + // would point every REST request at a stopped listener — or at a port + // something else now owns. + localStorage.setItem( + 'GBKit', + JSON.stringify( { networkProxy: { port: 4444, token: 'stale' } } ) + ); + + expect( getNetworkProxy() ).toBeNull(); + } ); +} ); From 659273f5b73af5018ded837e4bdad80677e8c71b Mon Sep 17 00:00:00 2001 From: David Calhoun Date: Mon, 31 Aug 2026 09:41:22 -0400 Subject: [PATCH 08/31] fix: relay site URLs that arrive on a host alias MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit WordPress builds `Link` headers and `_links` hrefs from `home_url()`, which need not be the host the app was configured with — `www.` versus bare, a mapped or reverse-proxied domain, an `http` `siteurl` behind `https`. wp-env is the everyday case: its credentials report `localhost` while WordPress reports `127.0.0.1`. Matching the target against the configured root as a plain string left those unrecognized, so `fetchAllMiddleware` — which follows the absolute URL from the `Link` header — had page 2 of every `per_page=-1` collection refused with a bare `fetch_error`. The existing pagination test could not catch it: it mocks the `Link` header with the configured root verbatim. Move the target onto the root's origin before comparing, and parse both sides so they normalize identically. Path differences stay unmatched deliberately — in a subdirectory multisite two roots that differ only by path are separate sites, and matching across them would route one site's request into the other's API root. The root is also parsed slash-terminated, so a sibling can no longer match it as a prefix: `https://site/wp-json` matched `https://site/wp-jsonx/…`, and a host-supplied root without a trailing slash is not hypothetical. Found by the session working the scheme-handler branch, which hit it first. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_014A21xYJuymCU7My8yyS8Bv --- src/utils/api-fetch-relay.test.js | 52 +++++++++++++++++++++++++++++++ src/utils/api-fetch.js | 47 ++++++++++++++++++++++------ 2 files changed, 89 insertions(+), 10 deletions(-) diff --git a/src/utils/api-fetch-relay.test.js b/src/utils/api-fetch-relay.test.js index 63a07e97e..70b92294e 100644 --- a/src/utils/api-fetch-relay.test.js +++ b/src/utils/api-fetch-relay.test.js @@ -202,6 +202,58 @@ describe( 'REST relay transport', () => { expect( result ).toEqual( [ { id: 1 }, { id: 2 } ] ); } ); + it( 'relays a next page that arrives on a host alias', async () => { + // WordPress builds `Link` from `home_url()`, which need not be the + // host the app was configured with — `www.` versus bare, a mapped + // domain, `http` behind `https`. wp-env is the everyday case: its + // credentials report `localhost` while WordPress reports + // `127.0.0.1`. + global.fetch = vi + .fn() + .mockResolvedValueOnce( + makeResponse( { + body: [ { id: 1 } ], + headers: { + link: '; rel="next"', + }, + } ) + ) + .mockResolvedValueOnce( + makeResponse( { body: [ { id: 2 } ] } ) + ); + + const result = await apiFetch( { + path: '/wp/v2/posts?per_page=-1', + } ); + + expect( fetchCall( 1 ).url ).toBe( + `${ RELAY_ROOT }wp/v2/posts?page=2&_locale=user` + ); + expect( result ).toEqual( [ { id: 1 }, { id: 2 } ] ); + } ); + + it( 'preserves a percent-encoded value in a relayed next page', async () => { + // Matching moves the target onto the API root's origin, which + // re-parses it. An encoded value has to survive that unchanged. + global.fetch = vi + .fn() + .mockResolvedValueOnce( + makeResponse( { + body: [ { id: 1 } ], + headers: { + link: `<${ API_ROOT }wp/v2/posts?search=caf%C3%A9&page=2>; rel="next"`, + }, + } ) + ) + .mockResolvedValueOnce( + makeResponse( { body: [ { id: 2 } ] } ) + ); + + await apiFetch( { path: '/wp/v2/posts?per_page=-1' } ); + + expect( fetchCall( 1 ).url ).toContain( 'search=caf%C3%A9' ); + } ); + it( 'refuses a request for somewhere other than the site API', async () => { await expect( apiFetch( { url: 'https://elsewhere.example/x' } ) diff --git a/src/utils/api-fetch.js b/src/utils/api-fetch.js index 0ea40b57a..f75966a9c 100644 --- a/src/utils/api-fetch.js +++ b/src/utils/api-fetch.js @@ -91,9 +91,13 @@ const DEFAULT_RELAY_HEADERS = { * @return {(options: Object) => Promise} The fetch handler. */ function createRelayFetchHandler( networkProxy, siteApiRoot ) { - const apiRoot = siteApiRoot.endsWith( '/' ) - ? siteApiRoot - : `${ siteApiRoot }/`; + // Slash-terminated so a sibling cannot match the root as a prefix + // (`https://site/wp-json` would otherwise match `https://site/wp-jsonx/…`), + // and parsed so both sides of the comparison are normalized the same way — + // default ports collapsed, host lowercased. + const apiRoot = new URL( + siteApiRoot.endsWith( '/' ) ? siteApiRoot : `${ siteApiRoot }/` + ); // The upstream path follows the route; the relay resolves it natively // against the site API root, so no absolute URL is ever sent. const relayRoot = `http://127.0.0.1:${ networkProxy.port }/proxy/`; @@ -184,20 +188,43 @@ function createRelayFetchHandler( networkProxy, siteApiRoot ) { * `fetchAllMiddleware` follows the absolute URL WordPress puts in the `Link` * header, so the relay has to recognize the site's own API root in it. * + * **Host aliases are tolerated; path differences are not.** WordPress builds + * `Link` headers and `_links` hrefs from `home_url()`, which need not be the + * host the app was configured with — `www.` versus bare, a mapped or + * reverse-proxied domain, an `http` `siteurl` behind `https`. wp-env is the + * everyday case: its credentials report `localhost` while WordPress reports + * `127.0.0.1`. Those are the same resource under another name, so the target is + * moved onto the root's origin before comparing. A *path* difference is a + * different resource: in a subdirectory multisite `https://site/a/wp-json/` and + * `https://site/b/wp-json/` are separate sites, and matching across them would + * route one site's request into the other's API root. + * * @param {string} target The request's `url`, or its `path` when no root URL * middleware has run. - * @param {string} apiRoot The site's REST API root, slash-terminated. + * @param {URL} apiRoot The site's REST API root, normalized and + * slash-terminated. * @return {string|null} The upstream path, or `null` when it cannot be relayed. */ function relayUpstreamPath( target, apiRoot ) { - if ( target.startsWith( apiRoot ) ) { - return target.slice( apiRoot.length ); - } - // Anything that is not an absolute URL is a path relative to the API root. - if ( ! /^([a-z][a-z0-9+.-]*:|\/\/)/i.test( target ) ) { + let url; + try { + url = new URL( target ); + } catch { + // Not an absolute URL, so it is a path relative to the API root. return target.replace( /^\/+/, '' ); } - return null; + + // Assign the origin's parts separately: the `host` setter leaves an + // existing port in place when the value carries none, so `host` alone turns + // `http://127.0.0.1:8888/…` into `https://example.com:8888/…`. + url.protocol = apiRoot.protocol; + url.hostname = apiRoot.hostname; + url.port = apiRoot.port; + + if ( ! url.href.startsWith( apiRoot.href ) ) { + return null; + } + return url.href.slice( apiRoot.href.length ); } /** From f6254fb3e153213bca6007855e6e612d95ecb90b Mon Sep 17 00:00:00 2001 From: David Calhoun Date: Mon, 31 Aug 2026 09:57:11 -0400 Subject: [PATCH 09/31] refactor: relay REST requests by wrapping fetch, not api-fetch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `setFetchHandler` was the only api-fetch hook that runs after every middleware, so it was the only way to intercept without short-circuiting past `_locale=user`, the `per_page=-1` expansion, and the HTTP v1 method override. But it replaces the response half too, so `data` serialization, the default `Accept` header, `parse: false`, the 204 case, parse-and-throw on non-2xx, and `offline_error`/`fetch_error` normalization all had to be reimplemented here and kept in step with a package we do not control. Wrapping `fetch` sits below all of it. api-fetch builds the request, hands it over, and parses whatever comes back; this layer only changes where the request goes. That deletes the reimplementation — about 130 lines — and `configureApiFetch` goes back to nothing but middleware registration. The predicate is not a new skip list. It is the same "is this a site API request?" test, minus its relative-path branch, which was needed only because `setFetchHandler` receives options where `path` may be set without `url`. `blob:`, `data:`, `gbk-media-file:` and relative URLs all fail an absolute-URL-under-the-API-root test on their own. The one addition is a guard for the relay's own origin, which is load-bearing because matching deliberately ignores the origin to tolerate host aliases — the upload route shares that server, and a site configured with a bare root would otherwise match it on path. Ordering is explicit in the bootstrap: the relay installs before the network log, so the log records the request the editor made rather than the loopback rewrite of it. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_014A21xYJuymCU7My8yyS8Bv --- src/utils/api-fetch-relay.test.js | 102 +++++++------ src/utils/api-fetch.js | 238 +----------------------------- src/utils/editor-environment.js | 21 ++- src/utils/fetch-relay.js | 160 ++++++++++++++++++++ src/utils/fetch-relay.test.js | 159 ++++++++++++++++++++ 5 files changed, 398 insertions(+), 282 deletions(-) create mode 100644 src/utils/fetch-relay.js create mode 100644 src/utils/fetch-relay.test.js diff --git a/src/utils/api-fetch-relay.test.js b/src/utils/api-fetch-relay.test.js index 70b92294e..40ffda59e 100644 --- a/src/utils/api-fetch-relay.test.js +++ b/src/utils/api-fetch-relay.test.js @@ -20,6 +20,7 @@ import apiFetch from '@wordpress/api-fetch'; * Internal dependencies */ import { configureApiFetch } from './api-fetch'; +import { installRelayFetch } from './fetch-relay'; import * as bridge from './bridge'; vi.mock( './bridge', async ( importOriginal ) => { @@ -48,6 +49,9 @@ const GBKIT = { networkProxy: { port: 5555, token: 'relay-token' }, }; +/** The fetch the relay wrapper delegates to; replaced per test. */ +let transport; + /** * A minimal stand-in for the `Response` the relay returns, carrying only what * api-fetch and its middleware read. @@ -71,28 +75,46 @@ function makeResponse( { status = 200, body = {}, headers = {} } = {} ) { } /** - * The arguments of the nth `fetch` call, as `{ url, init }`. + * The arguments the transport received on its nth call. * * @param {number} index Call index. * @return {{url: string, init: Object}} The call. */ -function fetchCall( index = 0 ) { - const [ url, init ] = global.fetch.mock.calls[ index ]; +function transportCall( index = 0 ) { + const [ url, init ] = transport.mock.calls[ index ]; return { url, init }; } +/** + * A header from an intercepted call, case-insensitively. + * + * @param {Object} init The `fetch` init the transport received. + * @param {string} name The header name. + * @return {string|null} The header value. + */ +function header( init, name ) { + return new Headers( init.headers ).get( name ); +} + describe( 'REST relay transport', () => { beforeAll( () => { // The relay's connection details are read from the injected global // rather than through `getGBKit`, so they have to be there. window.GBKit = GBKIT; bridge.getGBKit.mockReturnValue( GBKIT ); + + // A stable indirection so each test can swap the fetch the relay + // delegates to; the wrapper captures whatever `window.fetch` is at + // install time. + window.fetch = ( ...args ) => transport( ...args ); + installRelayFetch(); + configureApiFetch(); } ); beforeEach( () => { bridge.getGBKit.mockReturnValue( GBKIT ); - global.fetch = vi.fn( () => Promise.resolve( makeResponse() ) ); + transport = vi.fn( () => Promise.resolve( makeResponse() ) ); } ); afterEach( () => { @@ -103,26 +125,27 @@ describe( 'REST relay transport', () => { it( 'sends the upstream path below the API root to the relay route', async () => { await apiFetch( { path: '/wp/v2/posts' } ); - const { url } = fetchCall(); - expect( url ).toBe( `${ RELAY_ROOT }wp/v2/posts?_locale=user` ); + expect( transportCall().url ).toBe( + `${ RELAY_ROOT }wp/v2/posts?_locale=user` + ); } ); it( 'authenticates to the relay without sending the site credential', async () => { await apiFetch( { path: '/wp/v2/posts' } ); - const { init } = fetchCall(); - expect( init.headers[ 'Relay-Authorization' ] ).toBe( + const { init } = transportCall(); + expect( header( init, 'Relay-Authorization' ) ).toBe( 'Bearer relay-token' ); // The relay injects the site credential natively, so it must not // travel over loopback. - expect( init.headers.Authorization ).toBeUndefined(); + expect( header( init, 'Authorization' ) ).toBeNull(); } ); it( 'sends the Accept header WordPress uses to recognize a REST request', async () => { await apiFetch( { path: '/wp/v2/posts' } ); - expect( fetchCall().init.headers.Accept ).toBe( + expect( header( transportCall().init, 'Accept' ) ).toBe( 'application/json, */*;q=0.1' ); } ); @@ -134,9 +157,9 @@ describe( 'REST relay transport', () => { data: { title: 'Hello' }, } ); - const { init } = fetchCall(); + const { init } = transportCall(); expect( init.body ).toBe( JSON.stringify( { title: 'Hello' } ) ); - expect( init.headers[ 'Content-Type' ] ).toBe( 'application/json' ); + expect( header( init, 'Content-Type' ) ).toBe( 'application/json' ); } ); it( 'applies the HTTP v1 method override', async () => { @@ -146,9 +169,9 @@ describe( 'REST relay transport', () => { data: { title: 'Hello' }, } ); - const { init } = fetchCall(); + const { init } = transportCall(); expect( init.method ).toBe( 'POST' ); - expect( init.headers[ 'X-HTTP-Method-Override' ] ).toBe( 'PUT' ); + expect( header( init, 'X-HTTP-Method-Override' ) ).toBe( 'PUT' ); } ); it( 'forwards an abort signal', async () => { @@ -158,7 +181,7 @@ describe( 'REST relay transport', () => { signal: controller.signal, } ); - expect( fetchCall().init.signal ).toBe( controller.signal ); + expect( transportCall().init.signal ).toBe( controller.signal ); } ); it( 'does not send credentials the relay would reject', async () => { @@ -167,14 +190,14 @@ describe( 'REST relay transport', () => { // api-fetch defaults `credentials` to `include`. await apiFetch( { path: '/wp/v2/posts' } ); - expect( fetchCall().init.credentials ).toBeUndefined(); + expect( transportCall().init.credentials ).toBe( 'omit' ); } ); it( 'relays the absolute URL of a paginated next page', async () => { // `fetchAllMiddleware` follows the absolute URL WordPress puts in // the `Link` header, so the transport has to recognize the site's // own API root in it. - global.fetch = vi + transport = vi .fn() .mockResolvedValueOnce( makeResponse( { @@ -194,9 +217,9 @@ describe( 'REST relay transport', () => { // `per_page=-1` is expanded by api-fetch's own middleware rather // than forwarded verbatim, which WordPress would reject. - expect( fetchCall( 0 ).url ).toContain( 'per_page=100' ); - expect( fetchCall( 0 ).url ).not.toContain( 'per_page=-1' ); - expect( fetchCall( 1 ).url ).toBe( + expect( transportCall( 0 ).url ).toContain( 'per_page=100' ); + expect( transportCall( 0 ).url ).not.toContain( 'per_page=-1' ); + expect( transportCall( 1 ).url ).toBe( `${ RELAY_ROOT }wp/v2/posts?page=2&_locale=user` ); expect( result ).toEqual( [ { id: 1 }, { id: 2 } ] ); @@ -208,7 +231,7 @@ describe( 'REST relay transport', () => { // domain, `http` behind `https`. wp-env is the everyday case: its // credentials report `localhost` while WordPress reports // `127.0.0.1`. - global.fetch = vi + transport = vi .fn() .mockResolvedValueOnce( makeResponse( { @@ -226,7 +249,7 @@ describe( 'REST relay transport', () => { path: '/wp/v2/posts?per_page=-1', } ); - expect( fetchCall( 1 ).url ).toBe( + expect( transportCall( 1 ).url ).toBe( `${ RELAY_ROOT }wp/v2/posts?page=2&_locale=user` ); expect( result ).toEqual( [ { id: 1 }, { id: 2 } ] ); @@ -235,7 +258,7 @@ describe( 'REST relay transport', () => { it( 'preserves a percent-encoded value in a relayed next page', async () => { // Matching moves the target onto the API root's origin, which // re-parses it. An encoded value has to survive that unchanged. - global.fetch = vi + transport = vi .fn() .mockResolvedValueOnce( makeResponse( { @@ -251,21 +274,21 @@ describe( 'REST relay transport', () => { await apiFetch( { path: '/wp/v2/posts?per_page=-1' } ); - expect( fetchCall( 1 ).url ).toContain( 'search=caf%C3%A9' ); + expect( transportCall( 1 ).url ).toContain( 'search=caf%C3%A9' ); } ); - it( 'refuses a request for somewhere other than the site API', async () => { - await expect( - apiFetch( { url: 'https://elsewhere.example/x' } ) - ).rejects.toMatchObject( { code: 'fetch_error' } ); + it( 'leaves a request for somewhere other than the site API alone', async () => { + await apiFetch( { url: 'https://elsewhere.example/x' } ); - expect( global.fetch ).not.toHaveBeenCalled(); + const { url, init } = transportCall(); + expect( url ).toBe( 'https://elsewhere.example/x?_locale=user' ); + expect( header( init, 'Relay-Authorization' ) ).toBeNull(); } ); } ); describe( 'response', () => { it( 'resolves the decoded body', async () => { - global.fetch = vi.fn( () => + transport = vi.fn( () => Promise.resolve( makeResponse( { body: { id: 7 } } ) ) ); @@ -275,7 +298,7 @@ describe( 'REST relay transport', () => { } ); it( 'resolves a 204 to null', async () => { - global.fetch = vi.fn( () => + transport = vi.fn( () => Promise.resolve( makeResponse( { status: 204 } ) ) ); @@ -288,7 +311,7 @@ describe( 'REST relay transport', () => { const response = makeResponse( { headers: { allow: 'GET, POST' }, } ); - global.fetch = vi.fn( () => Promise.resolve( response ) ); + transport = vi.fn( () => Promise.resolve( response ) ); // `canUser` reads the `Allow` header off an unparsed response. const result = await apiFetch( { @@ -302,7 +325,7 @@ describe( 'REST relay transport', () => { } ); it( 'throws the decoded error body of a failed request', async () => { - global.fetch = vi.fn( () => + transport = vi.fn( () => Promise.resolve( makeResponse( { status: 403, @@ -320,7 +343,7 @@ describe( 'REST relay transport', () => { } ); it( 'normalizes an undecodable body', async () => { - global.fetch = vi.fn( () => + transport = vi.fn( () => Promise.resolve( makeResponse( { body: new Error( 'not json' ) } ) ) @@ -332,7 +355,7 @@ describe( 'REST relay transport', () => { } ); it( 'normalizes a transport failure', async () => { - global.fetch = vi.fn( () => + transport = vi.fn( () => Promise.reject( new TypeError( 'Load failed' ) ) ); @@ -340,14 +363,5 @@ describe( 'REST relay transport', () => { apiFetch( { path: '/wp/v2/posts/7' } ) ).rejects.toMatchObject( { code: 'fetch_error' } ); } ); - - it( 're-throws an abort for the caller to handle', async () => { - const abortError = new DOMException( 'Aborted', 'AbortError' ); - global.fetch = vi.fn( () => Promise.reject( abortError ) ); - - await expect( apiFetch( { path: '/wp/v2/posts/7' } ) ).rejects.toBe( - abortError - ); - } ); } ); } ); diff --git a/src/utils/api-fetch.js b/src/utils/api-fetch.js index f75966a9c..5f8885e4d 100644 --- a/src/utils/api-fetch.js +++ b/src/utils/api-fetch.js @@ -8,7 +8,7 @@ import { __ } from '@wordpress/i18n'; /** * Internal dependencies */ -import { getGBKit, getNetworkProxy, POST_FALLBACKS } from './bridge'; +import { getGBKit, POST_FALLBACKS } from './bridge'; import { info, error as logError } from './logger'; /** @@ -25,19 +25,6 @@ const MEDIA_UPLOAD_PATH = /^\/wp\/v2\/media(\?|$)/; */ export function configureApiFetch() { const { siteApiRoot = '', preloadData = null } = getGBKit(); - const networkProxy = getNetworkProxy(); - - // The relay replaces the transport rather than intercepting requests, - // because `apiFetch.use()` unshifts and api-fetch applies its middleware - // with `reduceRight`: a registered middleware can never run innermost, so - // interception there would short-circuit past api-fetch's own middleware — - // losing `_locale=user`, the `per_page=-1` expansion, and the HTTP v1 - // method override — and past everything `defaultFetchHandler` does. - if ( networkProxy ) { - apiFetch.setFetchHandler( - createRelayFetchHandler( networkProxy, siteApiRoot ) - ); - } apiFetch.use( apiFetch.createRootURLMiddleware( siteApiRoot ) ); apiFetch.use( corsMiddleware ); @@ -52,229 +39,6 @@ export function configureApiFetch() { ); } -/** - * Header values api-fetch's own handler defaults, which this transport - * replaces. WordPress treats `Accept` as a condition for handling a request as - * a REST request (core trac 44534), so losing it changes what the site returns. - */ -const DEFAULT_RELAY_HEADERS = { - Accept: 'application/json, */*;q=0.1', -}; - -/** - * Builds the api-fetch transport that routes every request through the native - * loopback relay. - * - * Under iOS Lockdown Mode the editor's `file://` page loses its CORS exemption - * and WordPress sanitizes its `Origin: file://` into an empty - * `Access-Control-Allow-Origin`, so every direct REST request rejects. The - * native host answers by running a loopback server and advertising it as - * `GBKit.networkProxy`; it forwards each request to the site's REST API - * natively and responds with CORS headers the web view accepts. - * - * Whether to use it is decided by that configuration alone. The relay is only - * advertised when the host knows direct requests cannot work, so attempting one - * first would be a guaranteed-doomed round trip per request — and inferring the - * answer from a response, as this once did, means one misleading response can - * latch the editor onto the wrong path for the rest of the session. - * - * Installed as the fetch handler rather than a middleware, so api-fetch's own - * middleware has already run and `options` carries the finished request. - * `defaultFetchHandler` is not exported, so what it does — `data` to a JSON - * body, default headers, response parsing and error normalization — is - * reproduced here. - * - * @param {Object} networkProxy Relay connection details. - * @param {number} networkProxy.port Loopback port. - * @param {string} networkProxy.token Per-session bearer token. - * @param {string} siteApiRoot The site's REST API root. - * @return {(options: Object) => Promise} The fetch handler. - */ -function createRelayFetchHandler( networkProxy, siteApiRoot ) { - // Slash-terminated so a sibling cannot match the root as a prefix - // (`https://site/wp-json` would otherwise match `https://site/wp-jsonx/…`), - // and parsed so both sides of the comparison are normalized the same way — - // default ports collapsed, host lowercased. - const apiRoot = new URL( - siteApiRoot.endsWith( '/' ) ? siteApiRoot : `${ siteApiRoot }/` - ); - // The upstream path follows the route; the relay resolves it natively - // against the site API root, so no absolute URL is ever sent. - const relayRoot = `http://127.0.0.1:${ networkProxy.port }/proxy/`; - - return async ( options ) => { - const { data, parse = true, method = 'GET', signal } = options; - - const upstreamPath = relayUpstreamPath( - options.url ?? options.path ?? '', - apiRoot - ); - if ( upstreamPath === null ) { - logError( - 'api-fetch: refusing to relay a request outside the site API root', - options.url ?? options.path - ); - throw { - code: 'fetch_error', - message: __( - 'Could not get a valid response from the server.' - ), - }; - } - - const headers = { ...DEFAULT_RELAY_HEADERS, ...options.headers }; - // The upstream `Authorization` header is injected natively, so the - // site credential never travels over loopback. The relay's own - // per-session token rides in `Relay-Authorization` because `fetch()` - // silently strips `Proxy-*` headers. - delete headers.Authorization; - headers[ 'Relay-Authorization' ] = `Bearer ${ networkProxy.token }`; - - // `data` is api-fetch's shorthand for a JSON body, serialized by the - // handler rather than by a middleware. Missing it meant every relayed - // write sent `Content-Length: 0`, which WordPress accepts as a no-op: - // silent data loss on every save. - let { body } = options; - if ( data ) { - body = JSON.stringify( data ); - headers[ 'Content-Type' ] = 'application/json'; - } - - let response; - try { - // Only the fields the transport needs, rather than spreading the - // remaining options: api-fetch defaults `credentials` to - // `include`, which a browser refuses against the relay's - // `Access-Control-Allow-Origin: *`. - response = await globalThis.fetch( relayRoot + upstreamPath, { - method, - headers, - body, - signal, - } ); - } catch ( relayError ) { - // Mirror `defaultFetchHandler`'s rejection handling, including - // re-throwing an abort for the caller to handle itself. - if ( relayError?.name === 'AbortError' ) { - throw relayError; - } - logError( 'api-fetch: relayed request failed', relayError ); - if ( ! globalThis.navigator.onLine ) { - throw { - code: 'offline_error', - message: __( - 'Unable to connect. Please check your Internet connection.' - ), - }; - } - throw { - code: 'fetch_error', - message: __( - 'Could not get a valid response from the server.' - ), - }; - } - - return parseRelayResponse( response, parse ); - }; -} - -/** - * The upstream path for a relayed request: the part of its target below the - * site API root, without a leading slash. - * - * Returns `null` for a target the relay cannot serve — an absolute URL for - * somewhere other than the site's API. Paginated requests arrive this way: - * `fetchAllMiddleware` follows the absolute URL WordPress puts in the `Link` - * header, so the relay has to recognize the site's own API root in it. - * - * **Host aliases are tolerated; path differences are not.** WordPress builds - * `Link` headers and `_links` hrefs from `home_url()`, which need not be the - * host the app was configured with — `www.` versus bare, a mapped or - * reverse-proxied domain, an `http` `siteurl` behind `https`. wp-env is the - * everyday case: its credentials report `localhost` while WordPress reports - * `127.0.0.1`. Those are the same resource under another name, so the target is - * moved onto the root's origin before comparing. A *path* difference is a - * different resource: in a subdirectory multisite `https://site/a/wp-json/` and - * `https://site/b/wp-json/` are separate sites, and matching across them would - * route one site's request into the other's API root. - * - * @param {string} target The request's `url`, or its `path` when no root URL - * middleware has run. - * @param {URL} apiRoot The site's REST API root, normalized and - * slash-terminated. - * @return {string|null} The upstream path, or `null` when it cannot be relayed. - */ -function relayUpstreamPath( target, apiRoot ) { - let url; - try { - url = new URL( target ); - } catch { - // Not an absolute URL, so it is a path relative to the API root. - return target.replace( /^\/+/, '' ); - } - - // Assign the origin's parts separately: the `host` setter leaves an - // existing port in place when the value carries none, so `host` alone turns - // `http://127.0.0.1:8888/…` into `https://example.com:8888/…`. - url.protocol = apiRoot.protocol; - url.hostname = apiRoot.hostname; - url.port = apiRoot.port; - - if ( ! url.href.startsWith( apiRoot.href ) ) { - return null; - } - return url.href.slice( apiRoot.href.length ); -} - -/** - * Parses a relayed response the way api-fetch's own handler does: unparsed - * requests get the raw `Response`, 204s resolve to `null`, and error statuses - * throw the decoded JSON body. - * - * @param {Response} response The relay response. - * @param {boolean} shouldParse Whether the caller requested parsing. - * @return {Promise} The parsed body or the raw response. - */ -async function parseRelayResponse( response, shouldParse ) { - if ( ! response.ok ) { - if ( ! shouldParse ) { - throw response; - } - throw await parseRelayJSON( response ); - } - - if ( ! shouldParse ) { - return response; - } - - if ( response.status === 204 ) { - return null; - } - - return parseRelayJSON( response ); -} - -/** - * Decodes a relayed response body, normalizing a decode failure into the same - * error api-fetch raises. The relay reports its own failures as WordPress-shaped - * `{ code, message }` JSON, so a real error reaches the caller intact rather - * than as an opaque parse failure. - * - * @param {Response} response The relay response. - * @return {Promise} The decoded body. - */ -async function parseRelayJSON( response ) { - try { - return await response.json(); - } catch { - throw { - code: 'invalid_json', - message: __( 'The response is not a valid JSON response.' ), - }; - } -} - /** * Middleware setting the CORS mode and remove a specific header causing CORS errors. * diff --git a/src/utils/editor-environment.js b/src/utils/editor-environment.js index 92737ffde..b316b2078 100644 --- a/src/utils/editor-environment.js +++ b/src/utils/editor-environment.js @@ -12,6 +12,7 @@ import { loadEditorAssets } from './editor-loader'; import { configureAjax } from './ajax'; import { initializeVideoPressAjaxBridge } from './videopress-bridge'; import { initializeFetchInterceptor } from './fetch-interceptor'; +import { installRelayFetch } from './fetch-relay'; import EditorLoadError from '../components/editor-load-error'; import { setLogLevel, error } from './logger'; import { setUpGlobalErrorHandlers } from './global-error-handler'; @@ -30,7 +31,7 @@ export async function setUpEditorEnvironment() { setBodyClasses(); await awaitGBKitGlobal(); setLogLevelFromGBKit(); - initializeFetchInterceptor(); + installFetchWrappers(); const isRTL = await configureLocale(); injectEditorStyles( isRTL ); await initializeWordPressGlobals(); @@ -42,6 +43,24 @@ export async function setUpEditorEnvironment() { } } +/** + * Wraps the global `fetch`, innermost wrapper first. + * + * Each call wraps whatever is already installed, so the **last** one installed + * runs **first**. The relay goes on before the network log, so the log records + * the request the editor made rather than the loopback rewrite of it — the + * native HTTP server already logs that hop. + * + * Both are no-ops unless their feature is configured: the relay needs + * `GBKit.networkProxy` (iOS Lockdown Mode), the log needs `enableNetworkLogging`. + * + * @return {void} + */ +function installFetchWrappers() { + installRelayFetch(); + initializeFetchInterceptor(); +} + /** * Adds conditional CSS classes to `document.body`. * diff --git a/src/utils/fetch-relay.js b/src/utils/fetch-relay.js new file mode 100644 index 000000000..2cca1b3c5 --- /dev/null +++ b/src/utils/fetch-relay.js @@ -0,0 +1,160 @@ +/** + * Internal dependencies + */ +import { getGBKit, getNetworkProxy } from './bridge'; +import { debug } from './logger'; + +/** + * Wraps `fetch` so requests for the site's REST API go through the native + * loopback relay. + * + * Under iOS Lockdown Mode the editor's `file://` page loses its CORS exemption + * and WordPress sanitizes its `Origin: file://` into an empty + * `Access-Control-Allow-Origin`, so every direct REST request rejects. The + * native host answers by running a loopback server and advertising it as + * `GBKit.networkProxy`; it forwards each request to the site's REST API + * natively and responds with CORS headers the web view accepts. + * + * ## Why the transport, and not `apiFetch` + * + * `apiFetch.use()` unshifts and api-fetch composes with `reduceRight`, so a + * registered middleware always runs *outside* api-fetch's own middleware and + * outside `defaultFetchHandler` — it would short-circuit past the `data`-to-body + * serialization, the default `Accept` header, the HTTP v1 method override, and + * every response-parsing rule, all of which would then have to be reimplemented + * and kept in step with a package we do not control. `setFetchHandler` runs late + * enough for the request half but still replaces the response half. + * + * Wrapping `fetch` puts the relay below all of it: api-fetch builds the whole + * request, hands it to `fetch`, and parses whatever comes back. This layer only + * changes where the request goes. + * + * @param {typeof fetch} next The fetch to delegate to. + * @param {Object} config Relay configuration. + * @param {{port: number, token: string}} config.networkProxy Relay connection details. + * @param {string} config.siteApiRoot The site's REST API root. + * @return {typeof fetch} The wrapped fetch. + */ +export function createRelayFetch( next, { networkProxy, siteApiRoot } ) { + // Slash-terminated so a sibling cannot match the root as a prefix + // (`https://site/wp-json` would otherwise match `https://site/wp-jsonx/…`), + // and parsed so both sides of every comparison normalize the same way — + // default ports collapsed, host lowercased. + const apiRoot = new URL( + siteApiRoot.endsWith( '/' ) ? siteApiRoot : `${ siteApiRoot }/` + ); + const relayRoot = `http://127.0.0.1:${ networkProxy.port }/proxy/`; + const relayOrigin = new URL( relayRoot ).origin; + + return ( input, init ) => { + const target = requestURL( input ); + const upstreamPath = + target && target.origin !== relayOrigin + ? relayUpstreamPath( target, apiRoot ) + : null; + + // Not a site REST request: media uploads to the loopback server, + // `blob:`/`data:`/`gbk-media-file:` reads, a third party's own API. + // Those keep the network path they had before a relay existed. + if ( upstreamPath === null ) { + return next( input, init ); + } + + const headers = new Headers( init?.headers ); + // The site credential is injected natively, so it never travels over + // loopback. The relay's own per-session token rides in + // `Relay-Authorization` because `fetch()` silently strips `Proxy-*`. + headers.delete( 'Authorization' ); + headers.set( 'Relay-Authorization', `Bearer ${ networkProxy.token }` ); + + return next( relayRoot + upstreamPath, { + ...init, + headers, + // The relay answers `Access-Control-Allow-Origin: *`, which a + // browser refuses to pair with a credentialed request — and + // api-fetch defaults `credentials` to `include`. Cookies are not + // how the loopback server authenticates anyway; the bearer token is. + credentials: 'omit', + } ); + }; +} + +/** + * Installs ``createRelayFetch`` over the global `fetch`, if the native host is + * running a relay. + * + * @return {void} + */ +export function installRelayFetch() { + const networkProxy = getNetworkProxy(); + const { siteApiRoot } = getGBKit(); + + if ( ! networkProxy || ! siteApiRoot ) { + return; + } + + debug( `Relaying site REST requests through port ${ networkProxy.port }` ); + window.fetch = createRelayFetch( window.fetch.bind( window ), { + networkProxy, + siteApiRoot, + } ); +} + +/** + * The URL a `fetch` call addresses, or `null` when it cannot be determined. + * + * A `Request` object returns `null` rather than being relayed: rewriting one + * means rebuilding it, and nothing in the editor's REST path constructs one — + * api-fetch always calls `fetch( url, init )`. Such a request keeps the direct + * path it had before a relay existed. + * + * @param {string|URL|Request} input The first argument to `fetch`. + * @return {URL|null} The target URL. + */ +function requestURL( input ) { + if ( typeof input !== 'string' && ! ( input instanceof URL ) ) { + return null; + } + try { + // Resolved against the document so a relative URL becomes a `file://` + // (or dev-server) URL, which cannot match the API root and so is never + // mistaken for a site request. + return new URL( input, document.baseURI ); + } catch { + return null; + } +} + +/** + * The upstream path for a relayed request: the part of its target below the + * site API root, without a leading slash. `null` for anything else. + * + * **Host aliases are tolerated; path differences are not.** WordPress builds + * `Link` headers and `_links` hrefs from `home_url()`, which need not be the + * host the app was configured with — `www.` versus bare, a mapped or + * reverse-proxied domain, an `http` `siteurl` behind `https`. wp-env is the + * everyday case: its credentials report `localhost` while WordPress reports + * `127.0.0.1`. Those are the same resource under another name, so the target is + * moved onto the root's origin before comparing. A *path* difference is a + * different resource: in a subdirectory multisite `https://site/a/wp-json/` and + * `https://site/b/wp-json/` are separate sites, and matching across them would + * route one site's request into the other's API root. + * + * @param {URL} target The request's target. + * @param {URL} apiRoot The site's REST API root, normalized and slash-terminated. + * @return {string|null} The upstream path, or `null` when it is not a site request. + */ +function relayUpstreamPath( target, apiRoot ) { + const aliased = new URL( target ); + // Assign the origin's parts separately: the `host` setter leaves an + // existing port in place when the value carries none, so `host` alone turns + // `http://127.0.0.1:8888/…` into `https://example.com:8888/…`. + aliased.protocol = apiRoot.protocol; + aliased.hostname = apiRoot.hostname; + aliased.port = apiRoot.port; + + if ( ! aliased.href.startsWith( apiRoot.href ) ) { + return null; + } + return aliased.href.slice( apiRoot.href.length ); +} diff --git a/src/utils/fetch-relay.test.js b/src/utils/fetch-relay.test.js new file mode 100644 index 000000000..950d1fe58 --- /dev/null +++ b/src/utils/fetch-relay.test.js @@ -0,0 +1,159 @@ +/** + * External dependencies + */ +import { describe, it, expect, beforeEach, vi } from 'vitest'; + +/** + * Internal dependencies + */ +import { createRelayFetch } from './fetch-relay'; + +vi.mock( './logger', () => ( { + debug: vi.fn(), + info: vi.fn(), + warn: vi.fn(), + error: vi.fn(), +} ) ); + +const NETWORK_PROXY = { port: 5555, token: 'relay-token' }; +const RELAY_ROOT = 'http://127.0.0.1:5555/proxy/'; + +describe( 'createRelayFetch', () => { + let next; + + beforeEach( () => { + next = vi.fn( () => Promise.resolve( 'response' ) ); + } ); + + /** + * The wrapper under test, over the shared `next` spy. + * + * @param {string} siteApiRoot The site's REST API root. + * @return {typeof fetch} The wrapped fetch. + */ + function relayFetch( siteApiRoot = 'https://example.com/wp-json/' ) { + return createRelayFetch( next, { + networkProxy: NETWORK_PROXY, + siteApiRoot, + } ); + } + + /** The URL `next` was called with. */ + function calledURL() { + return next.mock.calls[ 0 ][ 0 ]; + } + + describe( 'requests it relays', () => { + it( 'rewrites a site API request onto the relay route', async () => { + await relayFetch()( 'https://example.com/wp-json/wp/v2/posts?x=1' ); + + expect( calledURL() ).toBe( `${ RELAY_ROOT }wp/v2/posts?x=1` ); + } ); + + it( 'swaps the site credential for the relay token', async () => { + await relayFetch()( 'https://example.com/wp-json/wp/v2/posts', { + headers: { Authorization: 'Bearer site-token' }, + } ); + + const headers = new Headers( next.mock.calls[ 0 ][ 1 ].headers ); + expect( headers.get( 'Relay-Authorization' ) ).toBe( + 'Bearer relay-token' + ); + expect( headers.get( 'Authorization' ) ).toBeNull(); + } ); + + it( 'tolerates a host alias', async () => { + // The same resource under another name: `www.` versus bare, a + // mapped domain, wp-env's `127.0.0.1` versus `localhost`. + await relayFetch()( + 'https://www.example.com/wp-json/wp/v2/posts?page=2' + ); + + expect( calledURL() ).toBe( `${ RELAY_ROOT }wp/v2/posts?page=2` ); + } ); + + it( 'tolerates a scheme and default port that differ from the root', async () => { + await relayFetch()( 'http://example.com:80/wp-json/wp/v2/posts' ); + + expect( calledURL() ).toBe( `${ RELAY_ROOT }wp/v2/posts` ); + } ); + + it( 'accepts a root configured without a trailing slash', async () => { + await relayFetch( 'https://example.com/wp-json' )( + 'https://example.com/wp-json/wp/v2/posts' + ); + + expect( calledURL() ).toBe( `${ RELAY_ROOT }wp/v2/posts` ); + } ); + + it( 'merges into a root that already carries a query', async () => { + // Plain permalinks: `https://site/?rest_route=/`. + await relayFetch( 'https://example.com/?rest_route=/' )( + 'https://example.com/?rest_route=/wp/v2/posts&x=1' + ); + + expect( calledURL() ).toBe( `${ RELAY_ROOT }wp/v2/posts&x=1` ); + } ); + } ); + + describe( 'requests it leaves alone', () => { + /** + * Asserts the wrapper passed the call through untouched. + * + * @param {any} input The `fetch` input. + */ + async function expectPassthrough( input ) { + await relayFetch()( input ); + expect( next ).toHaveBeenCalledWith( input, undefined ); + } + + it( 'a request to the relay server itself', async () => { + // The upload route shares the relay's server. Matching ignores the + // origin, so without this the guard would come down to the path — + // which a root configured as a bare `https://site/` would not + // distinguish. + await expectPassthrough( + 'http://127.0.0.1:5555/upload?_embed=wp:featuredmedia' + ); + await expectPassthrough( `${ RELAY_ROOT }wp/v2/posts` ); + } ); + + it( 'a blob, data, or custom-scheme read', async () => { + await expectPassthrough( 'blob:https://example.com/abc-123' ); + await expectPassthrough( 'data:text/plain,hello' ); + await expectPassthrough( 'gbk-media-file:///photo.jpg' ); + } ); + + it( 'another origin entirely', async () => { + await expectPassthrough( + 'https://public-api.wordpress.com/wpcom/v2/jetpack-ai-query' + ); + } ); + + it( 'a sibling of the API root', async () => { + // `https://site/wp-json` must not match `https://site/wp-jsonx/…`. + await expectPassthrough( 'https://example.com/wp-jsonx/secrets' ); + } ); + + it( 'a different site in a subdirectory multisite', async () => { + // A path difference is a different site, not an alias. Matching + // across them would route site B through site A's API root. + await relayFetch( 'https://example.com/a/wp-json/' )( + 'https://example.com/b/wp-json/wp/v2/posts' + ); + + expect( calledURL() ).toBe( + 'https://example.com/b/wp-json/wp/v2/posts' + ); + } ); + + it( 'a Request object, which would have to be rebuilt', async () => { + const request = new Request( + 'https://example.com/wp-json/wp/v2/posts' + ); + await relayFetch()( request ); + + expect( next ).toHaveBeenCalledWith( request, undefined ); + } ); + } ); +} ); From 9fe0c2fafdf313b4dafa45dd27f3d2e1da33d37e Mon Sep 17 00:00:00 2001 From: David Calhoun Date: Mon, 31 Aug 2026 10:14:30 -0400 Subject: [PATCH 10/31] refactor: compose the fetch wrappers through a chain MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two wrappers now sit on the global `fetch` — the network log and the Lockdown Mode relay — and their order is behavior, not preference: a wrapper that rewrites the request changes what every wrapper inside it observes, so the log has to sit outside the relay to record the request the editor made rather than the loopback rewrite of it. Encoding that in the order two modules happen to patch `window.fetch` leaves nothing to read and nothing to test. `installFetchWrappers` takes the chain outermost-first and composes it, so the ordering and its rationale live in one place. Each wrapper is a `( next ) => fetch` transform — the same shape as an `apiFetch` middleware, one layer down — which also makes each testable against a stub `next` rather than against globals. A wrapper reports itself inapplicable by returning `null`, so a disabled feature installs no pass-through layer. `fetch-interceptor` is renamed to `fetch-logging`: it was "the" interceptor when it was the only one, and is now one wrapper among several. Its behavior is unchanged and its tests carry over as they were, against a one-line helper standing in for the chain. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_014A21xYJuymCU7My8yyS8Bv --- src/utils/api-fetch-relay.test.js | 7 +- src/utils/editor-environment.js | 29 +-- src/utils/editor-environment.test.js | 14 +- src/utils/fetch-chain.js | 44 ++++ src/utils/fetch-chain.test.js | 90 ++++++++ ...{fetch-interceptor.js => fetch-logging.js} | 212 +++++++++--------- ...erceptor.test.js => fetch-logging.test.js} | 65 +++--- src/utils/fetch-relay.js | 15 +- 8 files changed, 301 insertions(+), 175 deletions(-) create mode 100644 src/utils/fetch-chain.js create mode 100644 src/utils/fetch-chain.test.js rename src/utils/{fetch-interceptor.js => fetch-logging.js} (64%) rename src/utils/{fetch-interceptor.test.js => fetch-logging.test.js} (90%) diff --git a/src/utils/api-fetch-relay.test.js b/src/utils/api-fetch-relay.test.js index 40ffda59e..12e3eaf5f 100644 --- a/src/utils/api-fetch-relay.test.js +++ b/src/utils/api-fetch-relay.test.js @@ -20,7 +20,8 @@ import apiFetch from '@wordpress/api-fetch'; * Internal dependencies */ import { configureApiFetch } from './api-fetch'; -import { installRelayFetch } from './fetch-relay'; +import { installFetchWrappers } from './fetch-chain'; +import { createRelayFetchWrapper } from './fetch-relay'; import * as bridge from './bridge'; vi.mock( './bridge', async ( importOriginal ) => { @@ -104,10 +105,10 @@ describe( 'REST relay transport', () => { bridge.getGBKit.mockReturnValue( GBKIT ); // A stable indirection so each test can swap the fetch the relay - // delegates to; the wrapper captures whatever `window.fetch` is at + // delegates to; the chain captures whatever `window.fetch` is at // install time. window.fetch = ( ...args ) => transport( ...args ); - installRelayFetch(); + installFetchWrappers( [ createRelayFetchWrapper() ] ); configureApiFetch(); } ); diff --git a/src/utils/editor-environment.js b/src/utils/editor-environment.js index b316b2078..3ff386154 100644 --- a/src/utils/editor-environment.js +++ b/src/utils/editor-environment.js @@ -11,8 +11,9 @@ import { configureLocale } from './localization'; import { loadEditorAssets } from './editor-loader'; import { configureAjax } from './ajax'; import { initializeVideoPressAjaxBridge } from './videopress-bridge'; -import { initializeFetchInterceptor } from './fetch-interceptor'; -import { installRelayFetch } from './fetch-relay'; +import { installFetchWrappers } from './fetch-chain'; +import { createLoggingFetchWrapper } from './fetch-logging'; +import { createRelayFetchWrapper } from './fetch-relay'; import EditorLoadError from '../components/editor-load-error'; import { setLogLevel, error } from './logger'; import { setUpGlobalErrorHandlers } from './global-error-handler'; @@ -31,7 +32,7 @@ export async function setUpEditorEnvironment() { setBodyClasses(); await awaitGBKitGlobal(); setLogLevelFromGBKit(); - installFetchWrappers(); + installEditorFetchWrappers(); const isRTL = await configureLocale(); injectEditorStyles( isRTL ); await initializeWordPressGlobals(); @@ -44,21 +45,21 @@ export async function setUpEditorEnvironment() { } /** - * Wraps the global `fetch`, innermost wrapper first. + * Wraps the global `fetch`, outermost wrapper first. * - * Each call wraps whatever is already installed, so the **last** one installed - * runs **first**. The relay goes on before the network log, so the log records - * the request the editor made rather than the loopback rewrite of it — the - * native HTTP server already logs that hop. - * - * Both are no-ops unless their feature is configured: the relay needs - * `GBKit.networkProxy` (iOS Lockdown Mode), the log needs `enableNetworkLogging`. + * The network log sits outside the relay so it records the request the editor + * made rather than the loopback rewrite of it — the native HTTP server already + * logs that hop. Each wrapper reports itself inapplicable by returning `null`: + * the log needs `enableNetworkLogging`, the relay needs `GBKit.networkProxy` + * (iOS Lockdown Mode). * * @return {void} */ -function installFetchWrappers() { - installRelayFetch(); - initializeFetchInterceptor(); +function installEditorFetchWrappers() { + installFetchWrappers( [ + createLoggingFetchWrapper(), + createRelayFetchWrapper(), + ] ); } /** diff --git a/src/utils/editor-environment.test.js b/src/utils/editor-environment.test.js index fc37543ef..f58285650 100644 --- a/src/utils/editor-environment.test.js +++ b/src/utils/editor-environment.test.js @@ -22,11 +22,13 @@ import { initializeWordPressGlobals } from './wordpress-globals.js'; import { configureLocale } from './localization.js'; import { configureApiFetch } from './api-fetch.js'; import { initializeEditor } from './editor.jsx'; -import { initializeFetchInterceptor } from './fetch-interceptor.js'; +import { installFetchWrappers } from './fetch-chain.js'; import { injectEditorStyles } from './editor-styles.js'; vi.mock( './bridge.js' ); -vi.mock( './fetch-interceptor.js' ); +vi.mock( './fetch-chain.js' ); +vi.mock( './fetch-logging.js' ); +vi.mock( './fetch-relay.js' ); vi.mock( './logger.js' ); vi.mock( './editor-styles.js' ); vi.mock( './ajax.js' ); @@ -65,7 +67,7 @@ describe( 'setUpEditorEnvironment', () => { configureLocale.mockResolvedValue( false ); initializeWordPressGlobals.mockImplementation( () => {} ); configureApiFetch.mockImplementation( () => {} ); - initializeFetchInterceptor.mockImplementation( () => {} ); + installFetchWrappers.mockImplementation( () => {} ); configureAjax.mockImplementation( () => {} ); initializeVideoPressAjaxBridge.mockImplementation( () => {} ); initializeEditor.mockImplementation( () => {} ); @@ -83,8 +85,8 @@ describe( 'setUpEditorEnvironment', () => { return Promise.resolve(); } ); - initializeFetchInterceptor.mockImplementation( () => { - callOrder.push( 'initializeFetchInterceptor' ); + installFetchWrappers.mockImplementation( () => { + callOrder.push( 'installFetchWrappers' ); } ); configureLocale.mockImplementation( () => { @@ -120,7 +122,7 @@ describe( 'setUpEditorEnvironment', () => { expect( callOrder ).toEqual( [ 'awaitGBKitGlobal', - 'initializeFetchInterceptor', + 'installFetchWrappers', 'configureLocale', 'injectEditorStyles', 'loadRemainingGlobals', diff --git a/src/utils/fetch-chain.js b/src/utils/fetch-chain.js new file mode 100644 index 000000000..b68c6200b --- /dev/null +++ b/src/utils/fetch-chain.js @@ -0,0 +1,44 @@ +/** + * Internal dependencies + */ +import { debug } from './logger'; + +/** + * A transform on `fetch`: given the fetch to delegate to, returns a fetch. + * + * The same shape as an `apiFetch` middleware, one layer down. A wrapper may + * change where the request goes, observe it, or decline to touch it and hand it + * straight to `next`. + * + * @typedef {(next: typeof fetch) => typeof fetch} FetchWrapper + */ + +/** + * Wraps the global `fetch` with a chain of wrappers, outermost first. + * + * `[ a, b ]` produces `a( b( fetch ) )`, so `a` sees the request first and the + * response last. Order is behavior, not preference: a wrapper that rewrites the + * request changes what every wrapper inside it observes, so the network log has + * to sit outside the relay to record the request the editor made rather than + * the loopback rewrite of it. + * + * Entries that are `null` are skipped, so a wrapper module can report "not + * applicable" — network logging switched off, no relay configured — by + * returning nothing rather than by installing a pass-through. + * + * @param {Array} wrappers The chain, outermost first. + * @return {void} + */ +export function installFetchWrappers( wrappers ) { + const active = wrappers.filter( Boolean ); + + if ( ! active.length ) { + return; + } + + window.fetch = active.reduceRight( + ( next, wrap ) => wrap( next ), + window.fetch.bind( window ) + ); + debug( `Installed ${ active.length } fetch wrapper(s)` ); +} diff --git a/src/utils/fetch-chain.test.js b/src/utils/fetch-chain.test.js new file mode 100644 index 000000000..1af03d70c --- /dev/null +++ b/src/utils/fetch-chain.test.js @@ -0,0 +1,90 @@ +/** + * External dependencies + */ +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; + +/** + * Internal dependencies + */ +import { installFetchWrappers } from './fetch-chain'; + +vi.mock( './logger', () => ( { + debug: vi.fn(), + info: vi.fn(), + warn: vi.fn(), + error: vi.fn(), +} ) ); + +describe( 'installFetchWrappers', () => { + let originalFetch; + let calls; + + beforeEach( () => { + originalFetch = window.fetch; + calls = []; + window.fetch = vi.fn( () => { + calls.push( 'fetch' ); + return Promise.resolve( 'response' ); + } ); + } ); + + afterEach( () => { + window.fetch = originalFetch; + } ); + + /** + * A wrapper that records when it runs, relative to the others. + * + * @param {string} name Recorded on the way in and out. + * @return {import('./fetch-chain').FetchWrapper} The wrapper. + */ + function recorder( name ) { + return ( next ) => async ( input, init ) => { + calls.push( `${ name }:in` ); + const response = await next( input, init ); + calls.push( `${ name }:out` ); + return response; + }; + } + + it( 'runs wrappers outermost first and unwinds in reverse', async () => { + installFetchWrappers( [ recorder( 'a' ), recorder( 'b' ) ] ); + + await window.fetch( 'https://example.com/' ); + + expect( calls ).toEqual( [ + 'a:in', + 'b:in', + 'fetch', + 'b:out', + 'a:out', + ] ); + } ); + + it( 'passes the request through the chain to the underlying fetch', async () => { + const rewrite = ( next ) => ( input, init ) => + next( `${ input }rewritten`, init ); + installFetchWrappers( [ recorder( 'a' ), rewrite ] ); + + await window.fetch( 'https://example.com/', { method: 'POST' } ); + + expect( window.fetch ).not.toBe( originalFetch ); + expect( calls ).toEqual( [ 'a:in', 'fetch', 'a:out' ] ); + } ); + + it( 'skips entries that reported themselves inapplicable', async () => { + installFetchWrappers( [ null, recorder( 'a' ), null ] ); + + await window.fetch( 'https://example.com/' ); + + expect( calls ).toEqual( [ 'a:in', 'fetch', 'a:out' ] ); + } ); + + it( 'leaves fetch untouched when nothing applies', () => { + const before = window.fetch; + + installFetchWrappers( [ null, null ] ); + + expect( window.fetch ).toBe( before ); + } ); +} ); diff --git a/src/utils/fetch-interceptor.js b/src/utils/fetch-logging.js similarity index 64% rename from src/utils/fetch-interceptor.js rename to src/utils/fetch-logging.js index 5144cf187..53bc12463 100644 --- a/src/utils/fetch-interceptor.js +++ b/src/utils/fetch-logging.js @@ -5,130 +5,120 @@ import { onNetworkRequest, getGBKit } from './bridge'; import { debug } from './logger'; /** - * Initializes the global fetch interceptor. - * Wraps window.fetch to log all network requests and responses. - * Only overrides window.fetch if network logging is enabled in config. + * A `fetch` wrapper that reports every request and response to the native host, + * or `null` when network logging is not enabled. * - * @return {void} + * Reporting is fire-and-forget: the response is returned as soon as it arrives + * and its body is serialized afterwards from a clone, so logging never delays + * or locks the response the caller sees. + * + * @return {import('./fetch-chain').FetchWrapper|null} The wrapper. */ -export function initializeFetchInterceptor() { - // Don't initialize if already done - if ( window.__fetchInterceptorInitialized ) { - return; +export function createLoggingFetchWrapper() { + if ( ! getGBKit().enableNetworkLogging ) { + debug( 'Network logging disabled' ); + return null; } - const config = getGBKit(); - - // Only override window.fetch if network logging is enabled - if ( ! config.enableNetworkLogging ) { - debug( 'Network logging disabled, fetch interceptor not initialized' ); - return; - } - - const originalFetch = window.fetch; - - window.fetch = async function ( input, init ) { - const startTime = performance.now(); - const requestDetails = extractRequestDetails( input, init ); + return ( next ) => + async function ( input, init ) { + const startTime = performance.now(); + const requestDetails = extractRequestDetails( input, init ); - let requestBody = null; - let clonedRequest = null; + let requestBody = null; + let clonedRequest = null; - // Try to read request body if present - try { - if ( init?.body ) { - // Body is provided in init options - if ( typeof init.body === 'string' ) { - requestBody = init.body; - } else { - requestBody = serializeRequestBody( init.body ); + // Try to read request body if present + try { + if ( init?.body ) { + // Body is provided in init options + if ( typeof init.body === 'string' ) { + requestBody = init.body; + } else { + requestBody = serializeRequestBody( init.body ); + } + } else if ( input instanceof Request ) { + // Body might be in Request object - clone to read it + clonedRequest = input.clone(); + requestBody = await serializeBody( clonedRequest ); } - } else if ( input instanceof Request ) { - // Body might be in Request object - clone to read it - clonedRequest = input.clone(); - requestBody = await serializeBody( clonedRequest ); + } catch ( error ) { + debug( `Error reading request body: ${ error.message }` ); + requestBody = `[Error reading request body: ${ error.message }]`; } - } catch ( error ) { - debug( `Error reading request body: ${ error.message }` ); - requestBody = `[Error reading request body: ${ error.message }]`; - } - - let response; - let responseStatus; - let responseHeaders = {}; - try { - // Call original fetch - response = await originalFetch( input, init ); + let response; + let responseStatus; + let responseHeaders = {}; - // Capture response metadata immediately - const responseClone = response.clone(); - responseStatus = response.status; - const responseStatusText = - response.statusText || getStatusText( response.status ); - responseHeaders = serializeHeaders( response.headers ); - const duration = Math.round( performance.now() - startTime ); - - // Log asynchronously without blocking the response return - // This prevents Android WebView Response locking issues - serializeBody( responseClone ) - .then( ( body ) => { - onNetworkRequest( { - url: requestDetails.url, - method: requestDetails.method, - requestHeaders: serializeHeaders( - requestDetails.headers - ), - requestBody, - status: responseStatus, - statusText: responseStatusText, - responseHeaders, - responseBody: body, - duration, - } ); - } ) - .catch( ( error ) => { - // Log without body if reading fails - onNetworkRequest( { - url: requestDetails.url, - method: requestDetails.method, - requestHeaders: serializeHeaders( - requestDetails.headers - ), - requestBody, - status: responseStatus, - statusText: responseStatusText, - responseHeaders, - responseBody: `[Error reading body: ${ error.message }]`, - duration, + try { + response = await next( input, init ); + + // Capture response metadata immediately + const responseClone = response.clone(); + responseStatus = response.status; + const responseStatusText = + response.statusText || getStatusText( response.status ); + responseHeaders = serializeHeaders( response.headers ); + const duration = Math.round( performance.now() - startTime ); + + // Log asynchronously without blocking the response return + // This prevents Android WebView Response locking issues + serializeBody( responseClone ) + .then( ( body ) => { + onNetworkRequest( { + url: requestDetails.url, + method: requestDetails.method, + requestHeaders: serializeHeaders( + requestDetails.headers + ), + requestBody, + status: responseStatus, + statusText: responseStatusText, + responseHeaders, + responseBody: body, + duration, + } ); + } ) + .catch( ( error ) => { + // Log without body if reading fails + onNetworkRequest( { + url: requestDetails.url, + method: requestDetails.method, + requestHeaders: serializeHeaders( + requestDetails.headers + ), + requestBody, + status: responseStatus, + statusText: responseStatusText, + responseHeaders, + responseBody: `[Error reading body: ${ error.message }]`, + duration, + } ); } ); - } ); - - // Return response immediately - don't wait for body serialization - return response; - } catch ( error ) { - // Log failed request - const duration = Math.round( performance.now() - startTime ); - - onNetworkRequest( { - url: requestDetails.url, - method: requestDetails.method, - requestHeaders: serializeHeaders( requestDetails.headers ), - requestBody, - status: 0, - statusText: '', - responseHeaders: {}, - responseBody: `[Network error: ${ error.message }]`, - duration, - } ); - // Re-throw the error - throw error; - } - }; + // Return response immediately - don't wait for body serialization + return response; + } catch ( error ) { + // Log failed request + const duration = Math.round( performance.now() - startTime ); + + onNetworkRequest( { + url: requestDetails.url, + method: requestDetails.method, + requestHeaders: serializeHeaders( requestDetails.headers ), + requestBody, + status: 0, + statusText: '', + responseHeaders: {}, + responseBody: `[Network error: ${ error.message }]`, + duration, + } ); - window.__fetchInterceptorInitialized = true; - debug( 'Fetch interceptor initialized' ); + // Re-throw the error + throw error; + } + }; } /** diff --git a/src/utils/fetch-interceptor.test.js b/src/utils/fetch-logging.test.js similarity index 90% rename from src/utils/fetch-interceptor.test.js rename to src/utils/fetch-logging.test.js index b431e09e6..c794d8f95 100644 --- a/src/utils/fetch-interceptor.test.js +++ b/src/utils/fetch-logging.test.js @@ -6,23 +6,30 @@ import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; /** * Internal dependencies */ -import { initializeFetchInterceptor } from './fetch-interceptor'; +import { createLoggingFetchWrapper } from './fetch-logging'; import * as bridge from './bridge'; vi.mock( './bridge' ); // Helper to await the nested, non-blocking async logging that occurs within the -// fetch interceptor. +// wrapper. const waitForAsyncLogging = () => new Promise( ( resolve ) => setTimeout( resolve, 10 ) ); -describe( 'initializeFetchInterceptor', () => { +/** + * Wraps the current `window.fetch` with the logging wrapper, standing in for + * what `installFetchWrappers` does at runtime. + * + * @return {void} + */ +const installLogging = () => { + window.fetch = createLoggingFetchWrapper()( window.fetch.bind( window ) ); +}; + +describe( 'createLoggingFetchWrapper', () => { let originalFetch; beforeEach( () => { - // Reset window state - delete window.__fetchInterceptorInitialized; - // Store original fetch originalFetch = global.fetch; @@ -61,20 +68,14 @@ describe( 'initializeFetchInterceptor', () => { vi.clearAllMocks(); } ); - it( 'should not initialize when network logging is disabled', () => { - // Store the current fetch (which is the mock from beforeEach) - const currentFetch = window.fetch; - + it( 'should produce no wrapper when network logging is disabled', () => { bridge.getGBKit.mockReturnValue( { enableNetworkLogging: false, } ); - initializeFetchInterceptor(); - - // Should not have initialized - expect( window.__fetchInterceptorInitialized ).toBeUndefined(); - // Fetch should not have been wrapped (should still be the same mock) - expect( window.fetch ).toBe( currentFetch ); + // `null` rather than a pass-through, so the chain leaves `fetch` + // untouched instead of installing a layer that does nothing. + expect( createLoggingFetchWrapper() ).toBeNull(); } ); it( 'should derive statusText from status code when empty (HTTP/2)', async () => { @@ -102,7 +103,7 @@ describe( 'initializeFetchInterceptor', () => { } ) ); - initializeFetchInterceptor(); + installLogging(); await window.fetch( 'https://example.com/api', { method: 'POST' } ); @@ -118,7 +119,7 @@ describe( 'initializeFetchInterceptor', () => { describe( 'request header capture', () => { it( 'should capture headers from plain object with string URL', async () => { - initializeFetchInterceptor(); + installLogging(); await window.fetch( 'https://example.com/api', { method: 'POST', @@ -144,7 +145,7 @@ describe( 'initializeFetchInterceptor', () => { } ); it( 'should capture headers from Request object', async () => { - initializeFetchInterceptor(); + installLogging(); const request = new Request( 'https://example.com/api', { method: 'GET', @@ -169,7 +170,7 @@ describe( 'initializeFetchInterceptor', () => { } ); it( 'should merge Request headers with init override', async () => { - initializeFetchInterceptor(); + installLogging(); const request = new Request( 'https://example.com/api', { headers: { @@ -199,7 +200,7 @@ describe( 'initializeFetchInterceptor', () => { } ); it( 'should handle empty headers', async () => { - initializeFetchInterceptor(); + installLogging(); await window.fetch( 'https://example.com/api' ); @@ -213,7 +214,7 @@ describe( 'initializeFetchInterceptor', () => { } ); it( 'should handle Headers instance', async () => { - initializeFetchInterceptor(); + installLogging(); const headers = new Headers(); headers.append( 'Authorization', 'Bearer token123' ); @@ -239,7 +240,7 @@ describe( 'initializeFetchInterceptor', () => { describe( 'request body serialization', () => { it( 'should serialize FormData with files correctly', async () => { - initializeFetchInterceptor(); + installLogging(); // Create a FormData with a file const formData = new FormData(); @@ -276,7 +277,7 @@ describe( 'initializeFetchInterceptor', () => { } ); it( 'should serialize Blob bodies correctly', async () => { - initializeFetchInterceptor(); + installLogging(); const blob = new Blob( [ 'binary content' ], { type: 'image/png', @@ -299,7 +300,7 @@ describe( 'initializeFetchInterceptor', () => { } ); it( 'should serialize File bodies correctly', async () => { - initializeFetchInterceptor(); + installLogging(); const file = new File( [ 'file content' ], 'document.pdf', { type: 'application/pdf', @@ -322,7 +323,7 @@ describe( 'initializeFetchInterceptor', () => { } ); it( 'should serialize ArrayBuffer bodies correctly', async () => { - initializeFetchInterceptor(); + installLogging(); const buffer = new ArrayBuffer( 1024 ); @@ -341,7 +342,7 @@ describe( 'initializeFetchInterceptor', () => { } ); it( 'should serialize URLSearchParams bodies correctly', async () => { - initializeFetchInterceptor(); + installLogging(); const params = new URLSearchParams(); params.append( 'key1', 'value1' ); @@ -362,7 +363,7 @@ describe( 'initializeFetchInterceptor', () => { } ); it( 'should handle string bodies correctly', async () => { - initializeFetchInterceptor(); + installLogging(); const jsonString = JSON.stringify( { test: 'data' } ); @@ -381,7 +382,7 @@ describe( 'initializeFetchInterceptor', () => { } ); it( 'should handle FormData with mixed content types', async () => { - initializeFetchInterceptor(); + installLogging(); const formData = new FormData(); formData.append( 'text', 'simple text value' ); @@ -446,7 +447,7 @@ describe( 'initializeFetchInterceptor', () => { } ); it( 'should truncate long string values in FormData', async () => { - initializeFetchInterceptor(); + installLogging(); const formData = new FormData(); const longString = 'a'.repeat( 100 ); @@ -479,7 +480,7 @@ describe( 'initializeFetchInterceptor', () => { } ); it( 'should handle ReadableStream bodies', async () => { - initializeFetchInterceptor(); + installLogging(); const stream = new ReadableStream( { start( controller ) { @@ -504,7 +505,7 @@ describe( 'initializeFetchInterceptor', () => { } ); it( 'should handle missing body gracefully', async () => { - initializeFetchInterceptor(); + installLogging(); await window.fetch( 'https://example.com/api' ); diff --git a/src/utils/fetch-relay.js b/src/utils/fetch-relay.js index 2cca1b3c5..78a4fba0b 100644 --- a/src/utils/fetch-relay.js +++ b/src/utils/fetch-relay.js @@ -80,24 +80,21 @@ export function createRelayFetch( next, { networkProxy, siteApiRoot } ) { } /** - * Installs ``createRelayFetch`` over the global `fetch`, if the native host is - * running a relay. + * The relay wrapper for the host's configuration, or `null` when the host is + * not running a relay. * - * @return {void} + * @return {import('./fetch-chain').FetchWrapper|null} The wrapper. */ -export function installRelayFetch() { +export function createRelayFetchWrapper() { const networkProxy = getNetworkProxy(); const { siteApiRoot } = getGBKit(); if ( ! networkProxy || ! siteApiRoot ) { - return; + return null; } debug( `Relaying site REST requests through port ${ networkProxy.port }` ); - window.fetch = createRelayFetch( window.fetch.bind( window ), { - networkProxy, - siteApiRoot, - } ); + return ( next ) => createRelayFetch( next, { networkProxy, siteApiRoot } ); } /** From 39a43eda20a38f596d991fa7a1ac9a53d049c337 Mon Sep 17 00:00:00 2001 From: David Calhoun Date: Mon, 31 Aug 2026 13:56:06 -0400 Subject: [PATCH 11/31] fix(wp-env): stop swallowing OPTIONS requests that are not preflights MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The CORS shim short-circuited every `OPTIONS` request site-wide with a bodiless 204, without checking whether it was a CORS preflight. A preflight always carries `Access-Control-Request-Method`; an `OPTIONS` a client sent on its own behalf never does. `canUser` issues the latter and reads `Allow` to decide whether the user may create a page, update settings, upload media, or edit global styles — so against the local environment every one of those read as false, and read as *success*, so nothing surfaced. Discriminating on that header lets core answer the deliberate ones, where `rest_handle_options_request` builds the response and `rest_send_allow_header` fills in `Allow`. The site-wide hook stays: the editor also sends authenticated requests to `admin-ajax.php`, which core does not answer preflights for. `Allow` is also now added to the exposed CORS headers, through core's `rest_exposed_cors_headers` filter rather than by sending the header directly — core sends its own `Access-Control-Expose-Headers`, so a second `header()` call would replace that value instead of extending it. Without this the header is on the wire and invisible to JavaScript cross-origin, which is the same failure by a different route. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_014A21xYJuymCU7My8yyS8Bv --- wp-env/mu-plugins/gutenbergkit-cors.php | 53 ++++++++++++++++++++++--- 1 file changed, 47 insertions(+), 6 deletions(-) diff --git a/wp-env/mu-plugins/gutenbergkit-cors.php b/wp-env/mu-plugins/gutenbergkit-cors.php index 666b745f2..72d76ca25 100644 --- a/wp-env/mu-plugins/gutenbergkit-cors.php +++ b/wp-env/mu-plugins/gutenbergkit-cors.php @@ -36,6 +36,27 @@ function gutenbergkit_cors_send_origin_headers( $origin ) { header( 'Access-Control-Allow-Headers: Authorization, Content-Type, X-WP-Nonce' ); } +/** + * Exposes the `Allow` header to cross-origin callers. + * + * `canUser` issues `OPTIONS /wp/v2/{resource}` and reads `Allow` to decide + * whether the user may create a page, update settings, upload media, or edit + * global styles. Core sets that header from the matched route's permission + * callbacks, but as of 6.8.3 exposes only `X-WP-Total`, `X-WP-TotalPages` and + * `Link` — so cross-origin the header is on the wire and invisible to + * JavaScript, and every capability reads as false with no error surfaced. + * + * Added through core's filter rather than by sending the header directly: + * core sends its own `Access-Control-Expose-Headers`, so a second `header()` + * call would replace that value rather than extend it. + * + * @see https://github.com/WordPress/wordpress-develop/blob/6.8.3/src/wp-includes/rest-api/class-wp-rest-server.php#L395-L408 + */ +add_filter( 'rest_exposed_cors_headers', function ( $headers ) { + $headers[] = 'Allow'; + return $headers; +} ); + add_action( 'rest_api_init', function () { // Remove default WordPress CORS headers to avoid duplicates. remove_filter( 'rest_pre_serve_request', 'rest_send_cors_headers' ); @@ -46,12 +67,32 @@ function gutenbergkit_cors_send_origin_headers( $origin ) { }); }, 15 ); -// Handle preflight OPTIONS requests early. +// Answer CORS preflights early, before WordPress routes the request. +// +// This runs site-wide rather than on `rest_api_init` because the editor also +// sends an authenticated request to `admin-ajax.php`, which core does not +// answer preflights for. +// +// Only a genuine preflight is answered here. A preflight always carries +// `Access-Control-Request-Method`; an `OPTIONS` a client sent on its own behalf +// never does, and core answers those itself — `rest_handle_options_request` +// builds the response and `rest_send_allow_header` sets `Allow` from the +// matched route's permission callbacks. Short-circuiting both made every +// `canUser` check report that the user could do nothing, with no error +// surfaced, because the request "succeeded". +// +// @see https://github.com/WordPress/wordpress-develop/blob/6.8.3/src/wp-includes/rest-api.php#L252-L256 add_action( 'init', function () { - if ( isset( $_SERVER['REQUEST_METHOD'] ) && 'OPTIONS' === $_SERVER['REQUEST_METHOD'] ) { - gutenbergkit_cors_send_origin_headers( get_http_origin() ); - header( 'Access-Control-Max-Age: 86400' ); - status_header( 204 ); - exit; + if ( ! isset( $_SERVER['REQUEST_METHOD'] ) || 'OPTIONS' !== $_SERVER['REQUEST_METHOD'] ) { + return; } + + if ( ! isset( $_SERVER['HTTP_ACCESS_CONTROL_REQUEST_METHOD'] ) ) { + return; + } + + gutenbergkit_cors_send_origin_headers( get_http_origin() ); + header( 'Access-Control-Max-Age: 86400' ); + status_header( 204 ); + exit; }); From 7a3f0acacf338e6bfc8bc9465fa0f8d114a872de Mon Sep 17 00:00:00 2001 From: David Calhoun Date: Mon, 31 Aug 2026 13:56:13 -0400 Subject: [PATCH 12/31] test(ios): assert the relay carries the Allow header back `canUser` issues a deliberate `OPTIONS` and reads `Allow` to decide what the user may do. Asserting the header arrives covers the whole chain in one check: the request reaches WordPress rather than being answered locally as a preflight, WordPress computes the header from the matched route's permission callbacks, and the relay passes it through and exposes it. Replaces a weaker assertion that the response merely carried the site's server headers. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_014A21xYJuymCU7My8yyS8Bv --- .../Media/RestRelayIntegrationTests.swift | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/ios/Tests/GutenbergKitTests/Media/RestRelayIntegrationTests.swift b/ios/Tests/GutenbergKitTests/Media/RestRelayIntegrationTests.swift index dac62bb5a..dc888fa28 100644 --- a/ios/Tests/GutenbergKitTests/Media/RestRelayIntegrationTests.swift +++ b/ios/Tests/GutenbergKitTests/Media/RestRelayIntegrationTests.swift @@ -18,12 +18,9 @@ import Testing /// WP_ENV_CREDENTIALS_PATH="$PWD/.wp-env.credentials.json" swift test /// ``` /// -/// **`canUser` is not verifiable here.** The Playground runtime's web server -/// answers every `OPTIONS` itself with a bodiless 204 and permissive CORS -/// headers, before WordPress is reached, so no `Allow` header exists to relay. -/// What is checked instead is that the relay *forwards* an `OPTIONS` rather -/// than answering it locally — the defect that made `canUser` report no -/// capabilities at all. +/// Includes the `canUser` chain: a deliberate `OPTIONS` has to reach the site +/// and its `Allow` header has to survive back to the caller, or the editor +/// reports that the user can do nothing at all. @Suite("RestRelay against a live site", .enabled(if: WPEnvSite.current != nil), .serialized) struct RestRelayIntegrationTests { @@ -53,15 +50,18 @@ struct RestRelayIntegrationTests { } } - @Test("forwards an OPTIONS request upstream instead of answering it locally") - func forwardsOptions() async throws { + @Test("relays the Allow header that reports what the user may do") + func relaysAllowHeader() async throws { try await withRelay { server, site in + // `canUser` issues a deliberate `OPTIONS` — no + // `Access-Control-Request-Method` — and reads `Allow` off the + // response. Answering it locally as a CORS preflight instead of + // forwarding it reports no capabilities at all, and reports it as + // success, so nothing surfaces to the user. let (_, response) = try await site.relayed("wp/v2/pages", method: "OPTIONS", on: server) - // The local server would answer with a bare 204 carrying only its - // own CORS headers. A response bearing the site's server headers - // can only have come from the site. - #expect(response.value(forHTTPHeaderField: "X-Powered-By") != nil) + let allow = try #require(response.value(forHTTPHeaderField: "Allow")) + #expect(allow.contains("GET")) } } From 6279f3764e5398fd9a24faec74b224b6c85b000c Mon Sep 17 00:00:00 2001 From: David Calhoun Date: Mon, 31 Aug 2026 14:01:24 -0400 Subject: [PATCH 13/31] docs: record why host aliases are tolerated in one place and not the other The alias divergence is a documented property of this environment, not a one-off: `e2e/wp-env-fixtures.js` already works around the Playground runtime resolving `localhost` to `127.0.0.1` in `WP_SITEURL` by matching uploads on path rather than hostname. Citing it justifies the tolerance better than a single observation could. The relay's redirect guard stays origin-exact, and now says why. The two comparisons answer different questions: recognizing an alias decides where a request is sent, while the redirect guard decides whether the site credential follows a redirect somewhere we did not choose. `isSameOrigin` in `ajax.js` already draws that line for the same reason. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_014A21xYJuymCU7My8yyS8Bv --- .../GutenbergKit/Sources/Media/RestRelay.swift | 7 +++++++ src/utils/fetch-relay.js | 15 +++++++++------ 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/ios/Sources/GutenbergKit/Sources/Media/RestRelay.swift b/ios/Sources/GutenbergKit/Sources/Media/RestRelay.swift index 53ad846e4..f6f75597e 100644 --- a/ios/Sources/GutenbergKit/Sources/Media/RestRelay.swift +++ b/ios/Sources/GutenbergKit/Sources/Media/RestRelay.swift @@ -197,6 +197,13 @@ struct RestRelay: Sendable { /// carrying the site credential — followed to that host, and its response /// relayed back to the editor. Refusing hands the 3xx itself back instead. /// + /// The comparison is origin-exact, unlike the JavaScript side's, which + /// tolerates host aliases when recognizing a site URL. That asymmetry is + /// deliberate: recognizing an alias decides where a request is *sent*, + /// while this decides whether the site credential *follows* a redirect + /// somewhere we did not choose. A canonical redirect onto an alias host is + /// refused here rather than followed. + /// /// `@unchecked Sendable`: `allowedPrefix` is a `let` set at init and only /// read afterwards. private final class RedirectGuard: NSObject, URLSessionTaskDelegate, @unchecked Sendable { diff --git a/src/utils/fetch-relay.js b/src/utils/fetch-relay.js index 78a4fba0b..f2d861db9 100644 --- a/src/utils/fetch-relay.js +++ b/src/utils/fetch-relay.js @@ -130,12 +130,15 @@ function requestURL( input ) { * `Link` headers and `_links` hrefs from `home_url()`, which need not be the * host the app was configured with — `www.` versus bare, a mapped or * reverse-proxied domain, an `http` `siteurl` behind `https`. wp-env is the - * everyday case: its credentials report `localhost` while WordPress reports - * `127.0.0.1`. Those are the same resource under another name, so the target is - * moved onto the root's origin before comparing. A *path* difference is a - * different resource: in a subdirectory multisite `https://site/a/wp-json/` and - * `https://site/b/wp-json/` are separate sites, and matching across them would - * route one site's request into the other's API root. + * everyday case, and a documented one: the Playground runtime resolves + * `localhost` to `127.0.0.1` in `WP_SITEURL`, which the e2e fixtures already + * work around by matching uploads on path rather than hostname (see + * `e2e/wp-env-fixtures.js`). Those are the same resource under another name, so + * the target is moved onto the root's origin before comparing. A *path* + * difference is a different resource: in a subdirectory multisite + * `https://site/a/wp-json/` and `https://site/b/wp-json/` are separate sites, + * and matching across them would route one site's request into the other's API + * root. * * @param {URL} target The request's target. * @param {URL} apiRoot The site's REST API root, normalized and slash-terminated. From 9970d0c45d2a0160080f0f478d7e8a44d756cbb3 Mon Sep 17 00:00:00 2001 From: David Calhoun Date: Mon, 31 Aug 2026 14:05:49 -0400 Subject: [PATCH 14/31] fix(ios): answer a refused relay redirect instead of relaying the 3xx MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Refusing the redirect left `URLSession` holding the 3xx, and the relay passed it straight through — `Location` header included. `fetch` follows redirects by default, so the web view then chased the redirect to the very host the guard had just declined, and the request failed there as an opaque CORS error. The refusal was undone by the layer above it, and the reason was nowhere in the result. The relay now answers with a WordPress-shaped 502 naming the target it declined, so whoever hits this can see that the site redirected out of its own API root rather than going hunting in the relay. Also records the trade the prefix check makes, which is not obvious from the code: matching the whole URL rather than the host refuses a redirect to another path on the same site, and refuses a scheme downgrade without a rule of its own — but it also refuses a legitimate permalink-structure redirect. Refusing is the right default, because it cannot hand the site credential somewhere unverified. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_014A21xYJuymCU7My8yyS8Bv --- .../Sources/Media/RestRelay.swift | 44 +++++++++++-- .../Media/RestRelayTests.swift | 64 +++++++++++++++++++ 2 files changed, 104 insertions(+), 4 deletions(-) diff --git a/ios/Sources/GutenbergKit/Sources/Media/RestRelay.swift b/ios/Sources/GutenbergKit/Sources/Media/RestRelay.swift index f6f75597e..35c44c6fd 100644 --- a/ios/Sources/GutenbergKit/Sources/Media/RestRelay.swift +++ b/ios/Sources/GutenbergKit/Sources/Media/RestRelay.swift @@ -125,6 +125,22 @@ struct RestRelay: Sendable { // response back. See ``RedirectGuard``. let redirectGuard = RedirectGuard(allowedPrefix: apiRoot) let upstream = HTTPResponse(try await session.data(for: upstreamRequest, delegate: redirectGuard)) + + // A refused redirect leaves `URLSession` holding the 3xx itself. + // Relaying that would undo the refusal: the response carries the + // `Location` the guard just declined, and `fetch` follows redirects + // by default, so the web view would chase it to the very host the + // guard exists to keep the request away from — arriving as an + // opaque CORS failure rather than as this. + if let refused = redirectGuard.refusedTarget { + Logger.restRelay.error("Refused a relay redirect outside the site API root") + return Self.errorResponse( + status: 502, + code: "relay_redirect_refused", + message: "The site redirected this request to \(refused), which is outside its configured REST API root. The editor did not follow it." + ) + } + return HTTPResponse( status: upstream.status, statusText: upstream.statusText, @@ -197,6 +213,19 @@ struct RestRelay: Sendable { /// carrying the site credential — followed to that host, and its response /// relayed back to the editor. Refusing hands the 3xx itself back instead. /// + /// The comparison is a prefix match on the whole URL, not just its host, + /// so a redirect to another path on the same site — `/wp-login.php` for a + /// request to `/wp-json/…` — is refused too. The site credential should + /// follow the request only to the API it was configured for. It also means + /// a scheme downgrade fails without a rule of its own, since `http://…` + /// cannot prefix-match an `https://` root. + /// + /// The known cost: a redirect that changes permalink structure — pretty + /// REST URLs to `?rest_route=`, or the reverse after a permalink change — + /// is a legitimate redirect that this refuses. Refusing is the right + /// default because it cannot hand the credential somewhere unverified, and + /// the response says so specifically enough to find. + /// /// The comparison is origin-exact, unlike the JavaScript side's, which /// tolerates host aliases when recognizing a site URL. That asymmetry is /// deliberate: recognizing an alias decides where a request is *sent*, @@ -204,10 +233,17 @@ struct RestRelay: Sendable { /// somewhere we did not choose. A canonical redirect onto an alias host is /// refused here rather than followed. /// - /// `@unchecked Sendable`: `allowedPrefix` is a `let` set at init and only - /// read afterwards. - private final class RedirectGuard: NSObject, URLSessionTaskDelegate, @unchecked Sendable { + /// `@unchecked Sendable`: `allowedPrefix` is a `let` set at init; the + /// refusal is recorded under a lock. + final class RedirectGuard: NSObject, URLSessionTaskDelegate, @unchecked Sendable { private let allowedPrefix: String + private let lock = NSLock() + private var _refusedTarget: String? + + /// The redirect target that was refused, or `nil` if none was. + var refusedTarget: String? { + lock.withLock { _refusedTarget } + } init(allowedPrefix: String) { self.allowedPrefix = allowedPrefix @@ -221,7 +257,7 @@ struct RestRelay: Sendable { completionHandler: @escaping (URLRequest?) -> Void ) { guard let url = request.url, url.absoluteString.hasPrefix(allowedPrefix) else { - Logger.restRelay.error("Refusing to follow a relay redirect outside the site API root") + lock.withLock { _refusedTarget = request.url?.absoluteString ?? "an unreadable URL" } completionHandler(nil) return } diff --git a/ios/Tests/GutenbergKitTests/Media/RestRelayTests.swift b/ios/Tests/GutenbergKitTests/Media/RestRelayTests.swift index b24edac16..dca2da674 100644 --- a/ios/Tests/GutenbergKitTests/Media/RestRelayTests.swift +++ b/ios/Tests/GutenbergKitTests/Media/RestRelayTests.swift @@ -116,6 +116,42 @@ struct RestRelayTests { #expect(relay.upstreamURL(for: request("/proxying/wp/v2/posts")) == nil) } + // MARK: - Redirects + + @Test("follows a redirect that stays under the API root") + func followsContainedRedirect() { + let followed = redirectDecision(to: "https://example.com/wp-json/wp/v2/posts/1") + + #expect(followed?.url?.absoluteString == "https://example.com/wp-json/wp/v2/posts/1") + } + + @Test("refuses a redirect that leaves the API root") + func refusesEscapingRedirect() { + // Another host, another path on the same site, and a scheme downgrade. + // The credential should follow the request only to the API it was + // configured for. + for target in [ + "https://elsewhere.example/wp-json/wp/v2/posts", + "https://example.com/wp-login.php", + "http://example.com/wp-json/wp/v2/posts", + ] { + #expect(redirectDecision(to: target) == nil, "should refuse \(target)") + } + } + + @Test("reports the refused target so the editor can say what happened") + func recordsRefusedTarget() { + // Without this the relay would hand back the 3xx itself, and `fetch` + // — which follows redirects by default — would chase it to the host + // the guard just declined. + let redirectGuard = RestRelay.RedirectGuard(allowedPrefix: "https://example.com/wp-json/") + #expect(redirectGuard.refusedTarget == nil) + + _ = decide(redirectGuard, target: "https://elsewhere.example/x") + + #expect(redirectGuard.refusedTarget == "https://elsewhere.example/x") + } + // MARK: - Routing @Test("claims its own route and nothing else") @@ -142,6 +178,34 @@ struct RestRelayTests { private func request(_ target: String, method: String = "GET") -> ParsedHTTPRequest { .complete(method: method, target: target, httpVersion: "HTTP/1.1", headers: [:], body: nil) } + + /// The request a fresh guard would follow for a redirect to `target`, or + /// `nil` if it refuses. + private func redirectDecision(to target: String) -> URLRequest? { + decide( + RestRelay.RedirectGuard(allowedPrefix: "https://example.com/wp-json/"), + target: target + ) + } + + /// Asks `redirectGuard` whether to follow a redirect to `target`. + private func decide(_ redirectGuard: RestRelay.RedirectGuard, target: String) -> URLRequest? { + let url = URL(string: target)! + var followed: URLRequest? + redirectGuard.urlSession( + .shared, + task: URLSession.shared.dataTask(with: url), + willPerformHTTPRedirection: HTTPURLResponse( + url: URL(string: "https://example.com/wp-json/wp/v2/posts")!, + statusCode: 301, + httpVersion: "HTTP/1.1", + headerFields: ["Location": target] + )!, + newRequest: URLRequest(url: url), + completionHandler: { followed = $0 } + ) + return followed + } } #endif // canImport(Network) From d7979bc68865123a4b7a70f6fd208ebc85c7f8fd Mon Sep 17 00:00:00 2001 From: David Calhoun Date: Mon, 31 Aug 2026 14:41:29 -0400 Subject: [PATCH 15/31] chore(ios): remove the origin probe MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Scratch tooling from the Lockdown Mode investigation, committed alongside it. It builds bare web views to compare page-origin variants, which is not something the relay depends on, and seven of its eleven measurements point at a CORS-instrumented echo server that only ever ran on one developer's machine — as its own comment says. It cannot run for anyone, and making it run would mean building that server for a question this work does not ask. Takes the last hardcoded LAN address in the demo app with it. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_014A21xYJuymCU7My8yyS8Bv --- ios/Demo-iOS/Sources/GutenbergApp.swift | 140 +----------------------- 1 file changed, 3 insertions(+), 137 deletions(-) diff --git a/ios/Demo-iOS/Sources/GutenbergApp.swift b/ios/Demo-iOS/Sources/GutenbergApp.swift index 9764562e4..4c600a3a4 100644 --- a/ios/Demo-iOS/Sources/GutenbergApp.swift +++ b/ios/Demo-iOS/Sources/GutenbergApp.swift @@ -1,6 +1,5 @@ import SwiftUI import OSLog -import WebKit import GutenbergKit final class Navigation: ObservableObject { @@ -46,14 +45,12 @@ struct GutenbergApp: App { EditorLogger.logLevel = .debug // Opt-in: keep the device awake while the demo app is foregrounded. - // The debugging workflows here (probes, Web Inspector, devicectl - // console) break when the device auto-locks, but a demo app that never - // lets the screen sleep is its own surprise. + // The debugging workflows here (the upload probe, Web Inspector, + // devicectl console) break when the device auto-locks, but a demo app + // that never lets the screen sleep is its own surprise. if ProcessInfo.processInfo.environment["GUTENBERG_DISABLE_IDLE_TIMER"] == "1" { UIApplication.shared.isIdleTimerDisabled = true } - - OriginProbeRunner.runIfRequested() } var body: some Scene { @@ -82,137 +79,6 @@ struct GutenbergApp: App { } } -/// Serves a trivial HTML page for the custom-scheme origin probe variant. -final class ProbeSchemeHandler: NSObject, WKURLSchemeHandler { - func webView(_ webView: WKWebView, start urlSchemeTask: WKURLSchemeTask) { - guard let url = urlSchemeTask.request.url else { return } - let html = Data("probe".utf8) - let response = URLResponse(url: url, mimeType: "text/html", expectedContentLength: html.count, textEncodingName: "utf-8") - urlSchemeTask.didReceive(response) - urlSchemeTask.didReceive(html) - urlSchemeTask.didFinish() - } - - func webView(_ webView: WKWebView, stop urlSchemeTask: WKURLSchemeTask) {} -} - -/// Debug automation: probes network capabilities from bare web views with -/// different page origins to characterize Lockdown Mode restrictions. -/// Enabled with GUTENBERG_ORIGIN_PROBE=1; results print to stdout. -@MainActor -final class OriginProbeRunner: NSObject, WKNavigationDelegate { - static let shared = OriginProbeRunner() - - /// The CORS-instrumented echo server run on the Mac during investigation. - private let echoBase = "http://192.168.0.57:8890" - - private var webViews: [WKWebView] = [] - private var loadContinuations: [ObjectIdentifier: CheckedContinuation] = [:] - - static func runIfRequested() { - guard ProcessInfo.processInfo.environment["GUTENBERG_ORIGIN_PROBE"] == "1" else { return } - Task { @MainActor in - await shared.run() - } - } - - private enum LoadMode { - case file - case htmlString(base: URL?) - case customScheme - } - - private func run() async { - print("ORIGIN_PROBE_START") - await runVariant(name: "custom_scheme", universalPrefs: false, load: .customScheme) - await runVariant(name: "file_with_universal_prefs", universalPrefs: true, load: .file) - print("ORIGIN_PROBE_DONE") - webViews.removeAll() - } - - private func runVariant(name: String, universalPrefs: Bool, load: LoadMode) async { - let config = WKWebViewConfiguration() - if universalPrefs { - config.preferences.setValue(true, forKey: "allowFileAccessFromFileURLs") - config.setValue(true, forKey: "allowUniversalAccessFromFileURLs") - } - if case .customScheme = load { - config.setURLSchemeHandler(ProbeSchemeHandler(), forURLScheme: "gbk-probe") - } - - let webView = WKWebView(frame: .zero, configuration: config) - webView.isInspectable = true - webView.navigationDelegate = self - webViews.append(webView) - - let lockdown = config.defaultWebpagePreferences.isLockdownModeEnabled - print("ORIGIN_PROBE_VARIANT name=\(name) lockdown=\(lockdown)") - - await withCheckedContinuation { (continuation: CheckedContinuation) in - loadContinuations[ObjectIdentifier(webView)] = continuation - switch load { - case .file: - let dir = FileManager.default.temporaryDirectory.appendingPathComponent("origin-probe", isDirectory: true) - let file = dir.appendingPathComponent("probe.html") - try? FileManager.default.createDirectory(at: dir, withIntermediateDirectories: true) - try? "probe".write(to: file, atomically: true, encoding: .utf8) - webView.loadFileURL(file, allowingReadAccessTo: dir) - case .htmlString(let base): - webView.loadHTMLString("probe", baseURL: base) - case .customScheme: - webView.load(URLRequest(url: URL(string: "gbk-probe://probe-host/probe.html")!)) - } - } - - do { - let result = try await webView.callAsyncJavaScript( - Self.probeJS, - arguments: ["echoBase": echoBase], - contentWorld: .page - ) - print("ORIGIN_PROBE_RESULT name=\(name) \(result ?? "nil")") - } catch { - print("ORIGIN_PROBE_ERROR name=\(name) \(error)") - } - } - - nonisolated func webView(_ webView: WKWebView, didFinish navigation: WKNavigation!) { - let id = ObjectIdentifier(webView) - Task { @MainActor in - loadContinuations.removeValue(forKey: id)?.resume() - } - } - - nonisolated func webView(_ webView: WKWebView, didFail navigation: WKNavigation!, withError error: Error) { - let id = ObjectIdentifier(webView) - Task { @MainActor in - loadContinuations.removeValue(forKey: id)?.resume() - } - } - - private static let probeJS = """ - const out = {}; - const S = e => (e && e.name ? e.name + ': ' + e.message : String(e)); - const T = () => AbortSignal.timeout(8000); - const j = async (p) => { try { const r = await p; return r.status; } catch (e) { return 'REJECT ' + S(e); } }; - out.origin = String(location.origin); - out.href = location.href.split('?')[0].slice(0, 90); - out.star_get = await j(fetch(echoBase + '/star/get', {signal: T()})); - out.star_post_text = await j(fetch(echoBase + '/star/post', {method: 'POST', body: 'x', signal: T()})); - const fd = new FormData(); - fd.append('probe', 'x'); - out.star_post_formdata = await j(fetch(echoBase + '/star/fd', {method: 'POST', body: fd, signal: T()})); - out.star_post_preflight = await j(fetch(echoBase + '/star/pf', {method: 'POST', headers: {'X-Probe': '1'}, body: 'x', signal: T()})); - out.star_put = await j(fetch(echoBase + '/star/put', {method: 'PUT', body: 'x', signal: T()})); - out.echo_post_text = await j(fetch(echoBase + '/echo/post', {method: 'POST', body: 'x', signal: T()})); - try { const r = await fetch(echoBase + '/star/nc', {method: 'POST', mode: 'no-cors', body: 'x', signal: T()}); out.nocors_post = 'ok type=' + r.type + ' status=' + r.status; } catch (e) { out.nocors_post = 'REJECT ' + S(e); } - out.https_get = await j(fetch('https://public-api.wordpress.com/rest/v1.1/sites/en.blog.wordpress.com', {signal: T()})); - out.https_post = await j(fetch('https://public-api.wordpress.com/rest/v1.1/sites/en.blog.wordpress.com/posts/new', {method: 'POST', body: 'x', signal: T()})); - try { const b = new Blob(['xy']); out.blob_arrayBuffer = 'ok len=' + (await b.arrayBuffer()).byteLength; } catch (e) { out.blob_arrayBuffer = 'FAIL ' + S(e); } - return JSON.stringify(out, null, 1); - """ -} - struct OSLogEditorLogger: GutenbergKit.EditorLogging { private let logger: Logger From c2ebc0415017930098e7fcb874798fe503d69eff Mon Sep 17 00:00:00 2001 From: David Calhoun Date: Mon, 31 Aug 2026 14:41:39 -0400 Subject: [PATCH 16/31] fix(ios): point the upload probe only at the site MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two of its measurements went to a CORS-instrumented echo server that only ever ran on one developer's machine, so they reported connection failures for everyone else. What they covered — a cross-origin GET and a FormData POST — the `site_get_direct` and `site_post_media_direct` cases already cover against a real WordPress. The probe now needs nothing but a reachable site. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_014A21xYJuymCU7My8yyS8Bv --- ios/Demo-iOS/Sources/Views/EditorView.swift | 3 --- 1 file changed, 3 deletions(-) diff --git a/ios/Demo-iOS/Sources/Views/EditorView.swift b/ios/Demo-iOS/Sources/Views/EditorView.swift index 2edb3cfae..3cda9a92d 100644 --- a/ios/Demo-iOS/Sources/Views/EditorView.swift +++ b/ios/Demo-iOS/Sources/Views/EditorView.swift @@ -241,9 +241,6 @@ private struct _EditorView: UIViewControllerRepresentable { out.typeof_apiFetch = typeof (window.wp && window.wp.apiFetch); out.gbk_nativeUploadPort = !!(window.GBKit && window.GBKit.nativeUploadPort); out.gbk_networkProxy = !!(window.GBKit && window.GBKit.networkProxy); - const ECHO = 'http://192.168.0.57:8890'; - try { const r = await fetch(ECHO + '/star/get', {signal: T()}); out.echo_star_get = r.status; } catch (e) { out.echo_star_get = 'REJECT ' + S(e); } - try { const fdE = new FormData(); fdE.append('probe', 'x'); const r = await fetch(ECHO + '/star/fd', {method: 'POST', body: fdE, signal: T()}); out.echo_star_post_formdata = r.status; } catch (e) { out.echo_star_post_formdata = 'REJECT ' + S(e); } try { const r = await fetch(apiRoot, {method: 'GET', signal: T()}); out.site_get_direct = r.status; } catch (e) { out.site_get_direct = 'REJECT ' + S(e); } try { const fd1 = new FormData(); From 4afaa1d314cf52ded6174af001e30760c2d4102c Mon Sep 17 00:00:00 2001 From: David Calhoun Date: Mon, 31 Aug 2026 15:45:09 -0400 Subject: [PATCH 17/31] fix(ios): refuse dot segments whose separators are encoded `containsDotSegment` split on literal `/` only, so a traversal spelled `%2e%2e%2f%2e%2e%2fwp-admin` arrived as one segment and passed the guard. A server that decodes the separator before normalizing then resolves it outside the API root, with the site credential attached. Decode the separators alongside the dots so the guard holds the boundary its documentation promises. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_014tQvFHoZpEeD6obF2YCoor --- .../Sources/Media/RestRelay.swift | 19 +++++++++++++----- .../Media/RestRelayTests.swift | 20 +++++++++++++++++++ 2 files changed, 34 insertions(+), 5 deletions(-) diff --git a/ios/Sources/GutenbergKit/Sources/Media/RestRelay.swift b/ios/Sources/GutenbergKit/Sources/Media/RestRelay.swift index 35c44c6fd..11e9e6c8e 100644 --- a/ios/Sources/GutenbergKit/Sources/Media/RestRelay.swift +++ b/ios/Sources/GutenbergKit/Sources/Media/RestRelay.swift @@ -196,12 +196,21 @@ struct RestRelay: Sendable { /// Whether `path` contains a `.` or `..` segment, including the /// percent-encoded spellings a server may decode before resolving it. + /// + /// The separators are decoded alongside the dots. A server that decodes + /// `%2f` before normalizing — nginx normalizes the request URI ahead of + /// location matching — reads `%2e%2e%2fwp-admin` as `../wp-admin`, which + /// splitting on literal slashes alone would pass through. `%5c` is decoded + /// too because Windows-hosted servers treat a backslash as a separator. private static func containsDotSegment(_ path: some StringProtocol) -> Bool { - let lowercased = path.lowercased() - guard lowercased.contains(".") || lowercased.contains("%2e") else { return false } - return lowercased.split(separator: "/", omittingEmptySubsequences: false).contains { - let segment = $0.replacingOccurrences(of: "%2e", with: ".") - return segment == "." || segment == ".." + let decoded = path.lowercased() + .replacingOccurrences(of: "%2e", with: ".") + .replacingOccurrences(of: "%2f", with: "/") + .replacingOccurrences(of: "%5c", with: "/") + .replacingOccurrences(of: "\\", with: "/") + guard decoded.contains(".") else { return false } + return decoded.split(separator: "/", omittingEmptySubsequences: false).contains { + $0 == "." || $0 == ".." } } diff --git a/ios/Tests/GutenbergKitTests/Media/RestRelayTests.swift b/ios/Tests/GutenbergKitTests/Media/RestRelayTests.swift index dca2da674..aa23eac9b 100644 --- a/ios/Tests/GutenbergKitTests/Media/RestRelayTests.swift +++ b/ios/Tests/GutenbergKitTests/Media/RestRelayTests.swift @@ -90,6 +90,26 @@ struct RestRelayTests { #expect(relay.upstreamURL(for: request("/proxy/wp/%2E%2E/%2e%2e/")) == nil) } + @Test("refuses dot segments whose separators are encoded too") + func refusesDotSegmentsWithEncodedSeparators() { + // The whole traversal is one literal segment, so it is only a dot + // segment to a server that decodes the separator before normalizing. + let relay = makeRelay(apiRoot: Self.prettyRoot) + #expect(relay.upstreamURL(for: request("/proxy/%2e%2e%2f%2e%2e%2fwp-admin/admin-ajax.php")) == nil) + #expect(relay.upstreamURL(for: request("/proxy/wp%5C..%5Cwp-admin/")) == nil) + #expect(relay.upstreamURL(for: request("/proxy/wp\\..\\wp-admin/")) == nil) + } + + @Test("an encoded slash within a segment is not a dot segment") + func allowsEncodedSlashesWithinSegments() { + // A template ID is `theme//slug`, encoded into a single path segment. + let relay = makeRelay(apiRoot: Self.prettyRoot) + #expect( + relay.upstreamURL(for: request("/proxy/wp/v2/templates/twentytwentyfour%2F%2Fsingle"))?.absoluteString + == "https://example.com/wp-json/wp/v2/templates/twentytwentyfour%2F%2Fsingle" + ) + } + @Test("a dot inside a path segment is not a dot segment") func allowsDotsWithinSegments() { let relay = makeRelay(apiRoot: Self.prettyRoot) From 00524e61a83898bc28de85ae6f2530e190d7b6c5 Mon Sep 17 00:00:00 2001 From: David Calhoun Date: Mon, 31 Aug 2026 15:46:52 -0400 Subject: [PATCH 18/31] fix: relay only the configured site's hosts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `relayUpstreamPath` replaced the target's whole origin before comparing, so any host whose path started with the API root's path was captured and rewritten onto the configured site — a third party's request sent to the user's own site with the site credential attached. Compare the hosts first, tolerating only the spellings that name the same host: a `www.` prefix and the loopback addresses. Anything else keeps the direct path it had before a relay existed. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_014tQvFHoZpEeD6obF2YCoor --- src/utils/fetch-relay.js | 48 +++++++++++++++++++++++++---------- src/utils/fetch-relay.test.js | 22 ++++++++++++++-- 2 files changed, 55 insertions(+), 15 deletions(-) diff --git a/src/utils/fetch-relay.js b/src/utils/fetch-relay.js index f2d861db9..19ee77ad8 100644 --- a/src/utils/fetch-relay.js +++ b/src/utils/fetch-relay.js @@ -4,6 +4,9 @@ import { getGBKit, getNetworkProxy } from './bridge'; import { debug } from './logger'; +/** Hostnames that name the loopback interface. */ +const LOOPBACK_HOSTNAMES = new Set( [ 'localhost', '127.0.0.1', '[::1]' ] ); + /** * Wraps `fetch` so requests for the site's REST API go through the native * loopback relay. @@ -126,25 +129,30 @@ function requestURL( input ) { * The upstream path for a relayed request: the part of its target below the * site API root, without a leading slash. `null` for anything else. * - * **Host aliases are tolerated; path differences are not.** WordPress builds - * `Link` headers and `_links` hrefs from `home_url()`, which need not be the - * host the app was configured with — `www.` versus bare, a mapped or - * reverse-proxied domain, an `http` `siteurl` behind `https`. wp-env is the - * everyday case, and a documented one: the Playground runtime resolves - * `localhost` to `127.0.0.1` in `WP_SITEURL`, which the e2e fixtures already - * work around by matching uploads on path rather than hostname (see - * `e2e/wp-env-fixtures.js`). Those are the same resource under another name, so - * the target is moved onto the root's origin before comparing. A *path* - * difference is a different resource: in a subdirectory multisite - * `https://site/a/wp-json/` and `https://site/b/wp-json/` are separate sites, - * and matching across them would route one site's request into the other's API - * root. + * **Host aliases are tolerated; other hosts and path differences are not.** + * WordPress builds `Link` headers and `_links` hrefs from `home_url()`, which + * need not spell the host the app was configured with — `www.` versus bare, or + * wp-env's `localhost` versus the `127.0.0.1` its runtime writes into + * `WP_SITEURL` (the e2e fixtures work around the same thing by matching uploads + * on path rather than hostname, see `e2e/wp-env-fixtures.js`). Those spellings + * name the configured host, so the target is moved onto the root's origin — + * scheme and port included — before comparing. Any other host keeps the direct + * path it had before a relay existed: rewriting it would send a third party's + * request to the user's own site, with the site credential attached. A *path* + * difference is likewise a different resource: in a subdirectory multisite + * `https://site/a/wp-json/` and `https://site/b/wp-json/` are separate sites. * * @param {URL} target The request's target. * @param {URL} apiRoot The site's REST API root, normalized and slash-terminated. * @return {string|null} The upstream path, or `null` when it is not a site request. */ function relayUpstreamPath( target, apiRoot ) { + if ( + canonicalHost( target.hostname ) !== canonicalHost( apiRoot.hostname ) + ) { + return null; + } + const aliased = new URL( target ); // Assign the origin's parts separately: the `host` setter leaves an // existing port in place when the value carries none, so `host` alone turns @@ -158,3 +166,17 @@ function relayUpstreamPath( target, apiRoot ) { } return aliased.href.slice( apiRoot.href.length ); } + +/** + * A hostname reduced to the form its aliases share: every loopback spelling + * collapses to one, and a `www.` prefix is dropped. + * + * @param {string} hostname A URL hostname. + * @return {string} The canonical form. + */ +function canonicalHost( hostname ) { + if ( LOOPBACK_HOSTNAMES.has( hostname ) ) { + return 'localhost'; + } + return hostname.replace( /^www\./, '' ); +} diff --git a/src/utils/fetch-relay.test.js b/src/utils/fetch-relay.test.js index 950d1fe58..7b7a1c8e4 100644 --- a/src/utils/fetch-relay.test.js +++ b/src/utils/fetch-relay.test.js @@ -63,8 +63,7 @@ describe( 'createRelayFetch', () => { } ); it( 'tolerates a host alias', async () => { - // The same resource under another name: `www.` versus bare, a - // mapped domain, wp-env's `127.0.0.1` versus `localhost`. + // The same host under another spelling: `www.` versus bare. await relayFetch()( 'https://www.example.com/wp-json/wp/v2/posts?page=2' ); @@ -72,6 +71,16 @@ describe( 'createRelayFetch', () => { expect( calledURL() ).toBe( `${ RELAY_ROOT }wp/v2/posts?page=2` ); } ); + it( 'tolerates a loopback host alias', async () => { + // wp-env: the site is configured as `localhost`, and WordPress + // writes `127.0.0.1` into the URLs it emits. + await relayFetch( 'http://localhost:8888/wp-json/' )( + 'http://127.0.0.1:8888/wp-json/wp/v2/posts' + ); + + expect( calledURL() ).toBe( `${ RELAY_ROOT }wp/v2/posts` ); + } ); + it( 'tolerates a scheme and default port that differ from the root', async () => { await relayFetch()( 'http://example.com:80/wp-json/wp/v2/posts' ); @@ -130,6 +139,15 @@ describe( 'createRelayFetch', () => { ); } ); + it( 'another WordPress site whose path matches the root', async () => { + // A different host is a different server, however its paths are + // shaped. Rewriting one onto the configured site would send its + // request to the user's own site with the site credential. + await expectPassthrough( + 'https://other-wp.example/wp-json/wp/v2/posts' + ); + } ); + it( 'a sibling of the API root', async () => { // `https://site/wp-json` must not match `https://site/wp-jsonx/…`. await expectPassthrough( 'https://example.com/wp-jsonx/secrets' ); From 7c0e1189cb7f8c70559bfd285df8c0a7bbd42017 Mon Sep 17 00:00:00 2001 From: David Calhoun Date: Mon, 31 Aug 2026 15:47:42 -0400 Subject: [PATCH 19/31] fix: pass local server requests through by port, not origin The passthrough guard compared the target's origin against the relay's `127.0.0.1` spelling, but the native upload request it exists to exempt is issued to `localhost` (the name Android hosts permit cleartext to). That request fell through to the site match, where only the path stood between a multipart upload and being relayed to the REST API. Match the local server by port instead, accepting any loopback spelling. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_014tQvFHoZpEeD6obF2YCoor --- src/utils/fetch-relay.js | 22 ++++++++++++++++++++-- src/utils/fetch-relay.test.js | 11 +++++++---- 2 files changed, 27 insertions(+), 6 deletions(-) diff --git a/src/utils/fetch-relay.js b/src/utils/fetch-relay.js index 19ee77ad8..9ff83b8fc 100644 --- a/src/utils/fetch-relay.js +++ b/src/utils/fetch-relay.js @@ -47,12 +47,12 @@ export function createRelayFetch( next, { networkProxy, siteApiRoot } ) { siteApiRoot.endsWith( '/' ) ? siteApiRoot : `${ siteApiRoot }/` ); const relayRoot = `http://127.0.0.1:${ networkProxy.port }/proxy/`; - const relayOrigin = new URL( relayRoot ).origin; + const localServerPort = String( networkProxy.port ); return ( input, init ) => { const target = requestURL( input ); const upstreamPath = - target && target.origin !== relayOrigin + target && ! addressesLocalServer( target, localServerPort ) ? relayUpstreamPath( target, apiRoot ) : null; @@ -125,6 +125,24 @@ function requestURL( input ) { } } +/** + * Whether a target addresses the native local server, in any spelling of + * loopback. + * + * The relay and the media upload route share one server, and they are addressed + * by different names: the relay route by address, the upload route by hostname + * (`localhost` is what Android hosts permit cleartext to). Neither is a site + * request, so both keep the path they had before a relay existed — the port + * identifies the server whichever name reached it. + * + * @param {URL} target The request's target. + * @param {string} port The local server's port. + * @return {boolean} Whether the target is the local server. + */ +function addressesLocalServer( target, port ) { + return target.port === port && LOOPBACK_HOSTNAMES.has( target.hostname ); +} + /** * The upstream path for a relayed request: the part of its target below the * site API root, without a leading slash. `null` for anything else. diff --git a/src/utils/fetch-relay.test.js b/src/utils/fetch-relay.test.js index 7b7a1c8e4..9ded8014f 100644 --- a/src/utils/fetch-relay.test.js +++ b/src/utils/fetch-relay.test.js @@ -117,10 +117,13 @@ describe( 'createRelayFetch', () => { } it( 'a request to the relay server itself', async () => { - // The upload route shares the relay's server. Matching ignores the - // origin, so without this the guard would come down to the path — - // which a root configured as a bare `https://site/` would not - // distinguish. + // The upload route shares the relay's server and is addressed as + // `localhost` rather than by address. Without matching the port, + // the guard would come down to the path — which a root configured + // as a bare `https://site/` would not distinguish. + await expectPassthrough( + 'http://localhost:5555/upload?_embed=wp:featuredmedia' + ); await expectPassthrough( 'http://127.0.0.1:5555/upload?_embed=wp:featuredmedia' ); From 4243493c30b135e0b642f33552ee3271d4f0b770 Mon Sep 17 00:00:00 2001 From: David Calhoun Date: Mon, 31 Aug 2026 15:49:15 -0400 Subject: [PATCH 20/31] fix(ios): advertise the upload port only when an uploader is behind it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `buildEditorConfiguration` decided on the delegate alone while `startUploadServer` also required a site credential, so a host with a delegate but no auth header — with the relay starting the server anyway — advertised a port whose `/upload` route had no uploader. Every media upload then failed with a 500 instead of falling back to the WebView path. Record the decision once, where it is made, and read it when the configuration is built. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_014tQvFHoZpEeD6obF2YCoor --- .../Sources/EditorViewController.swift | 24 ++++++++++++------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/ios/Sources/GutenbergKit/Sources/EditorViewController.swift b/ios/Sources/GutenbergKit/Sources/EditorViewController.swift index ca2beab15..2cccea1b7 100644 --- a/ios/Sources/GutenbergKit/Sources/EditorViewController.swift +++ b/ios/Sources/GutenbergKit/Sources/EditorViewController.swift @@ -160,6 +160,11 @@ public final class EditorViewController: UIViewController, GutenbergEditorContro private let lockdownModeMonitor: LockdownModeMonitor private var uploadServer: MediaUploadServer? + /// Whether `uploadServer` was started with a media upload pipeline behind + /// it, and so whether its port and token may be advertised to JavaScript. + /// See `startUploadServer()`. + private var isUploadPipelineEnabled = false + /// Whether `uploadServer` also hosts the Lockdown Mode REST relay. /// See `RestRelay` and `startUploadServer()`. private var isRestRelayEnabled = false @@ -442,18 +447,18 @@ public final class EditorViewController: UIViewController, GutenbergEditorContro /// private func buildEditorConfiguration(dependencies: EditorDependencies) throws -> WKUserScript { // The upload pipeline and the REST relay share one local server, but - // each is advertised to JavaScript only when its feature is active: - // `nativeUploadPort` requires a delegate to process uploads, and - // `networkProxy` is only useful under Lockdown Mode. - let hasUploadPipeline = mediaUploadDelegate != nil + // each is advertised to JavaScript only when `startUploadServer()` + // actually enabled it: routing uploads to a server started without an + // uploader behind it would fail every one of them, and `networkProxy` + // is only useful under Lockdown Mode. let networkProxyGlobal = isRestRelayEnabled ? uploadServer.map { GBKitGlobal.NetworkProxy(port: Int($0.port), token: $0.token) } : nil let gbkitGlobal = try GBKitGlobal( configuration: self.configuration, dependencies: dependencies, - nativeUploadPort: hasUploadPipeline ? uploadServer.map { Int($0.port) } : nil, - nativeUploadToken: hasUploadPipeline ? uploadServer?.token : nil, + nativeUploadPort: isUploadPipelineEnabled ? uploadServer.map { Int($0.port) } : nil, + nativeUploadToken: isUploadPipelineEnabled ? uploadServer?.token : nil, networkProxy: networkProxyGlobal ) let stringValue = try gbkitGlobal.toString() @@ -493,15 +498,15 @@ public final class EditorViewController: UIViewController, GutenbergEditorContro // it because the WebView has no auth cookies). Without both there is nothing // to upload through, so leave the upload pipeline down and let uploads fall // to the default WebView path rather than start a pipeline that could only fail. - let needsUploadPipeline = mediaUploadDelegate != nil && !configuration.authHeader.isEmpty + isUploadPipelineEnabled = mediaUploadDelegate != nil && !configuration.authHeader.isEmpty isRestRelayEnabled = webView.configuration.defaultWebpagePreferences.isLockdownModeEnabled && !configuration.isOfflineModeEnabled - guard needsUploadPipeline || isRestRelayEnabled else { + guard isUploadPipelineEnabled || isRestRelayEnabled else { return } - let defaultUploader = needsUploadPipeline ? DefaultMediaUploader( + let defaultUploader = isUploadPipelineEnabled ? DefaultMediaUploader( httpClient: httpClient.uploadClient(), siteApiRoot: configuration.siteApiRoot, siteApiNamespace: configuration.siteApiNamespace @@ -514,6 +519,7 @@ public final class EditorViewController: UIViewController, GutenbergEditorContro restRelay: isRestRelayEnabled ? RestRelay(configuration: configuration) : nil ) } catch { + isUploadPipelineEnabled = false isRestRelayEnabled = false Logger.uploadServer.error("Failed to start upload server: \(error). Falling back to default upload behavior.") } From c69054dd540ee1572bf50d08f6412a5b4d6068fc Mon Sep 17 00:00:00 2001 From: David Calhoun Date: Mon, 31 Aug 2026 15:56:03 -0400 Subject: [PATCH 21/31] fix(ios): stop persisting the editor configuration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `GBKit` carries the site credential and the local server's port and tokens, all valid only for the load that injected them, and iOS mirrored it into `localStorage` where it outlived the session. Anything reading through the `getGBKit` fallback — the media upload port and token — could pick up a previous session's values. Remove the key as the configuration is injected, matching Android, which clears the WebView's web storage before each load. With no stale copy to guard against, the relay details read through `getGBKit` like every other field and `getNetworkProxy` goes away. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_014tQvFHoZpEeD6obF2YCoor --- .../Sources/EditorViewController.swift | 22 ++++++--- .../EditorConfigurationScriptTests.swift | 26 ++++++++++ src/utils/api-fetch-relay.test.js | 2 - src/utils/bridge.js | 48 +++++++------------ src/utils/bridge.test.js | 39 +-------------- src/utils/fetch-relay.js | 5 +- 6 files changed, 63 insertions(+), 79 deletions(-) create mode 100644 ios/Tests/GutenbergKitTests/EditorConfigurationScriptTests.swift diff --git a/ios/Sources/GutenbergKit/Sources/EditorViewController.swift b/ios/Sources/GutenbergKit/Sources/EditorViewController.swift index 2cccea1b7..1375869cb 100644 --- a/ios/Sources/GutenbergKit/Sources/EditorViewController.swift +++ b/ios/Sources/GutenbergKit/Sources/EditorViewController.swift @@ -461,15 +461,25 @@ public final class EditorViewController: UIViewController, GutenbergEditorContro nativeUploadToken: isUploadPipelineEnabled ? uploadServer?.token : nil, networkProxy: networkProxyGlobal ) - let stringValue = try gbkitGlobal.toString() + return WKUserScript( + source: Self.configurationScript(gbkitGlobal: try gbkitGlobal.toString()), + injectionTime: .atDocumentStart, + forMainFrameOnly: true + ) + } - let jsCode = """ - window.GBKit = \(stringValue); - localStorage.setItem('GBKit', JSON.stringify(window.GBKit)); + /// The document-start script that installs `window.GBKit`. + /// + /// The configuration is session-scoped — it carries the site credential and + /// the local server's port and tokens — so no copy of it outlives the load + /// that injected it. (Android clears the WebView's web storage before each + /// load for the same reason.) + static func configurationScript(gbkitGlobal: String) -> String { + """ + window.GBKit = \(gbkitGlobal); + localStorage.removeItem('GBKit'); "done"; """ - - return WKUserScript(source: jsCode, injectionTime: .atDocumentStart, forMainFrameOnly: true) } /// Starts the local HTTP server for routing file uploads through native processing. diff --git a/ios/Tests/GutenbergKitTests/EditorConfigurationScriptTests.swift b/ios/Tests/GutenbergKitTests/EditorConfigurationScriptTests.swift new file mode 100644 index 000000000..1115dd931 --- /dev/null +++ b/ios/Tests/GutenbergKitTests/EditorConfigurationScriptTests.swift @@ -0,0 +1,26 @@ +import Foundation +import Testing + +@testable import GutenbergKit + +#if canImport(UIKit) + +@Suite("Editor configuration script") +struct EditorConfigurationScriptTests { + + @MainActor + @Test("injects the configuration without persisting it") + func doesNotPersistTheConfiguration() { + // The injected configuration carries the site credential and the local + // server's port and tokens, all of them valid only for this session. + let script = EditorViewController.configurationScript( + gbkitGlobal: #"{"authHeader":"Bearer secret"}"# + ) + + #expect(script.contains(#"window.GBKit = {"authHeader":"Bearer secret"};"#)) + #expect(script.contains("localStorage.removeItem('GBKit')")) + #expect(!script.contains("localStorage.setItem")) + } +} + +#endif diff --git a/src/utils/api-fetch-relay.test.js b/src/utils/api-fetch-relay.test.js index 12e3eaf5f..d8fe3be91 100644 --- a/src/utils/api-fetch-relay.test.js +++ b/src/utils/api-fetch-relay.test.js @@ -99,8 +99,6 @@ function header( init, name ) { describe( 'REST relay transport', () => { beforeAll( () => { - // The relay's connection details are read from the injected global - // rather than through `getGBKit`, so they have to be there. window.GBKit = GBKIT; bridge.getGBKit.mockReturnValue( GBKIT ); diff --git a/src/utils/bridge.js b/src/utils/bridge.js index a84994588..4406514df 100644 --- a/src/utils/bridge.js +++ b/src/utils/bridge.js @@ -221,19 +221,27 @@ export function onNetworkRequest( requestData ) { } } +/** + * @typedef {Object} NetworkProxy + * + * @property {number} port The port the loopback REST relay is listening on. + * @property {string} token Per-session auth token for requests to the relay. + */ + /** * @typedef GBKitConfig * - * @property {boolean} [themeStyles] Controls if theme styles are applied to the editor. - * @property {string} [siteApiRoot] The root URL of the site's API. - * @property {string[]} [siteApiNamespace] The namespace of the site's API; if multiple namespaces are provided, the first one is used as the default. - * @property {string[]} [namespaceExcludedPaths] The paths that should not be namespaced. - * @property {string} [authHeader] The authentication header. - * @property {string} [hideTitle] Whether to hide the title. - * @property {Post} [post] The post data. - * @property {boolean} [enableNetworkLogging] Enables logging of all network requests/responses to the native host via onNetworkRequest bridge method. - * @property {number} [nativeUploadPort] Port the local HTTP server is listening on. If absent, the native upload override is not activated. - * @property {string} [nativeUploadToken] Per-session auth token for requests to the local upload server. + * @property {boolean} [themeStyles] Controls if theme styles are applied to the editor. + * @property {string} [siteApiRoot] The root URL of the site's API. + * @property {string[]} [siteApiNamespace] The namespace of the site's API; if multiple namespaces are provided, the first one is used as the default. + * @property {string[]} [namespaceExcludedPaths] The paths that should not be namespaced. + * @property {string} [authHeader] The authentication header. + * @property {string} [hideTitle] Whether to hide the title. + * @property {Post} [post] The post data. + * @property {boolean} [enableNetworkLogging] Enables logging of all network requests/responses to the native host via onNetworkRequest bridge method. + * @property {number} [nativeUploadPort] Port the local HTTP server is listening on. If absent, the native upload override is not activated. + * @property {string} [nativeUploadToken] Per-session auth token for requests to the local upload server. + * @property {NetworkProxy} [networkProxy] The loopback REST relay's connection details. If absent, the host is not running a relay. */ /** @@ -268,26 +276,6 @@ export function getGBKit() { } } -/** - * The native loopback relay's connection details, or `null` when the host is - * not running one. - * - * Read from the injected global only, never through ``getGBKit``: its - * `localStorage` fallback returns whatever the last editor session wrote, and - * iOS does not clear it. A relay's port and per-session token are valid only - * for the server that issued them, so a persisted copy points at a listener - * that has been stopped — or, worse, at a port something else now owns. Every - * REST request would go there. - * - * Falling back to no relay is the safe direction to be wrong in: requests take - * the direct path, which is what they did before a relay existed. - * - * @return {{port: number, token: string}|null} The relay details. - */ -export function getNetworkProxy() { - return window.GBKit?.networkProxy ?? null; -} - /** * @typedef {Object} Post * @property {string} [title] The title of the post. diff --git a/src/utils/bridge.test.js b/src/utils/bridge.test.js index 36effe142..62178fb1a 100644 --- a/src/utils/bridge.test.js +++ b/src/utils/bridge.test.js @@ -6,12 +6,7 @@ import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; /** * Internal dependencies */ -import { - requestLatestContent, - getPost, - getNetworkProxy, - showBlockInserter, -} from './bridge'; +import { requestLatestContent, getPost, showBlockInserter } from './bridge'; vi.mock( './logger.js', () => ( { error: vi.fn(), @@ -550,35 +545,3 @@ describe( 'showBlockInserter', () => { expect( postMessage ).toHaveBeenCalledTimes( 1 ); } ); } ); - -describe( 'getNetworkProxy', () => { - afterEach( () => { - delete window.GBKit; - localStorage.clear(); - } ); - - it( 'returns the injected relay details', () => { - window.GBKit = { networkProxy: { port: 5555, token: 'fresh' } }; - - expect( getNetworkProxy() ).toEqual( { port: 5555, token: 'fresh' } ); - } ); - - it( 'returns null when the host is not running a relay', () => { - window.GBKit = { siteApiRoot: 'https://example.com/wp-json/' }; - - expect( getNetworkProxy() ).toBeNull(); - } ); - - it( 'ignores a persisted relay from an earlier session', () => { - // A relay's port and token belong to the server that issued them, and - // iOS never clears `GBKit` from localStorage. Trusting a persisted copy - // would point every REST request at a stopped listener — or at a port - // something else now owns. - localStorage.setItem( - 'GBKit', - JSON.stringify( { networkProxy: { port: 4444, token: 'stale' } } ) - ); - - expect( getNetworkProxy() ).toBeNull(); - } ); -} ); diff --git a/src/utils/fetch-relay.js b/src/utils/fetch-relay.js index 9ff83b8fc..013948f07 100644 --- a/src/utils/fetch-relay.js +++ b/src/utils/fetch-relay.js @@ -1,7 +1,7 @@ /** * Internal dependencies */ -import { getGBKit, getNetworkProxy } from './bridge'; +import { getGBKit } from './bridge'; import { debug } from './logger'; /** Hostnames that name the loopback interface. */ @@ -89,8 +89,7 @@ export function createRelayFetch( next, { networkProxy, siteApiRoot } ) { * @return {import('./fetch-chain').FetchWrapper|null} The wrapper. */ export function createRelayFetchWrapper() { - const networkProxy = getNetworkProxy(); - const { siteApiRoot } = getGBKit(); + const { networkProxy, siteApiRoot } = getGBKit(); if ( ! networkProxy || ! siteApiRoot ) { return null; From 4c7330e4459f1c3d8aba0b6ba0092d4851d50d03 Mon Sep 17 00:00:00 2001 From: David Calhoun Date: Mon, 31 Aug 2026 15:57:12 -0400 Subject: [PATCH 22/31] fix: install the fetch wrappers only once The chain replaced the interceptor's idempotency guard with nothing, so a retried boot or a re-injected bundle wrapped the already-wrapped `fetch`: every request logged to the native host twice and relayed through two layers. Mark the wrapped `fetch` and skip a second install. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_014tQvFHoZpEeD6obF2YCoor --- src/utils/fetch-chain.js | 21 ++++++++++++++++++++- src/utils/fetch-chain.test.js | 9 +++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/src/utils/fetch-chain.js b/src/utils/fetch-chain.js index b68c6200b..87a95a457 100644 --- a/src/utils/fetch-chain.js +++ b/src/utils/fetch-chain.js @@ -3,6 +3,13 @@ */ import { debug } from './logger'; +/** + * Marks the wrapped `fetch` as ours. Registered globally so a second copy of + * this module — a re-injected bundle — recognizes the mark rather than wrapping + * an already-wrapped `fetch`. + */ +const WRAPPED = Symbol.for( 'gutenbergkit.fetchWrappers' ); + /** * A transform on `fetch`: given the fetch to delegate to, returns a fetch. * @@ -26,6 +33,11 @@ import { debug } from './logger'; * applicable" — network logging switched off, no relay configured — by * returning nothing rather than by installing a pass-through. * + * Installing twice is a no-op. A retried boot or a re-injected bundle would + * otherwise wrap the wrapped `fetch`, logging every request to the native host + * twice and sending it through two relay layers. A page load resets this along + * with `window.fetch` itself. + * * @param {Array} wrappers The chain, outermost first. * @return {void} */ @@ -36,9 +48,16 @@ export function installFetchWrappers( wrappers ) { return; } - window.fetch = active.reduceRight( + if ( window.fetch[ WRAPPED ] ) { + debug( 'Fetch wrappers are already installed' ); + return; + } + + const wrapped = active.reduceRight( ( next, wrap ) => wrap( next ), window.fetch.bind( window ) ); + wrapped[ WRAPPED ] = true; + window.fetch = wrapped; debug( `Installed ${ active.length } fetch wrapper(s)` ); } diff --git a/src/utils/fetch-chain.test.js b/src/utils/fetch-chain.test.js index 1af03d70c..e8389dbac 100644 --- a/src/utils/fetch-chain.test.js +++ b/src/utils/fetch-chain.test.js @@ -80,6 +80,15 @@ describe( 'installFetchWrappers', () => { expect( calls ).toEqual( [ 'a:in', 'fetch', 'a:out' ] ); } ); + it( 'installs once, however many times it is called', async () => { + installFetchWrappers( [ recorder( 'a' ) ] ); + installFetchWrappers( [ recorder( 'b' ) ] ); + + await window.fetch( 'https://example.com/' ); + + expect( calls ).toEqual( [ 'a:in', 'fetch', 'a:out' ] ); + } ); + it( 'leaves fetch untouched when nothing applies', () => { const before = window.fetch; From 4685df0cf46990422cc613ad1450d163e4907868 Mon Sep 17 00:00:00 2001 From: David Calhoun Date: Mon, 31 Aug 2026 15:59:00 -0400 Subject: [PATCH 23/31] fix(ios): share the relay's URL session and guard an unmeasurable body MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two changes to the relay's upstream request: A `URLSession` was built per relay and never invalidated, so one accumulated per editor load under Lockdown Mode. Nothing about it is per-relay, so share one for the process. `RequestBody.count` is a file-size lookup that reports zero when it fails, and the streamed branch sent that as the `Content-Length` — uploading nothing, which WordPress accepts as a no-op and answers 2xx. The missing file that is the likeliest cause already throws in `makeInputStream()`, which the existing catch turns into a 500, so this closes the remaining window rather than a live bug. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_014tQvFHoZpEeD6obF2YCoor --- .../Sources/Media/RestRelay.swift | 33 ++++++++++++++----- 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/ios/Sources/GutenbergKit/Sources/Media/RestRelay.swift b/ios/Sources/GutenbergKit/Sources/Media/RestRelay.swift index 11e9e6c8e..a384ab515 100644 --- a/ios/Sources/GutenbergKit/Sources/Media/RestRelay.swift +++ b/ios/Sources/GutenbergKit/Sources/Media/RestRelay.swift @@ -57,7 +57,19 @@ struct RestRelay: Sendable { /// The authorization header injected into upstream requests. private let authHeader: String - private let session: URLSession + /// The session every relay shares. + /// + /// A `URLSession` holds its resources until it is invalidated, and a relay + /// is built on every editor load under Lockdown Mode, so one session each + /// would accumulate for the life of the process. Nothing about the session + /// is per-relay, and a relayed request is cancelled through its own task + /// rather than by tearing the session down. + private static let session: URLSession = { + let configuration = URLSessionConfiguration.ephemeral + configuration.timeoutIntervalForRequest = 120 + configuration.httpCookieStorage = nil + return URLSession(configuration: configuration) + }() init(configuration: EditorConfiguration) { var root = configuration.siteApiRoot.absoluteString @@ -66,11 +78,6 @@ struct RestRelay: Sendable { } self.apiRoot = root self.authHeader = configuration.authHeader - - let sessionConfiguration = URLSessionConfiguration.ephemeral - sessionConfiguration.timeoutIntervalForRequest = 120 - sessionConfiguration.httpCookieStorage = nil - self.session = URLSession(configuration: sessionConfiguration) } /// Whether a request targets the relay. @@ -108,9 +115,19 @@ struct RestRelay: Sendable { } else { // Large bodies are buffered to disk by the request parser; // stream them to avoid loading uploads fully into memory. + // + // `count` is a file-size lookup, and reports zero when it + // fails. Sending that as the `Content-Length` of a body the + // parser says exists would upload nothing, and WordPress + // answers a no-op with a 2xx the editor would take for success. do { + let length = body.count + guard length > 0 else { + Logger.restRelay.error("Refusing to relay a request body whose length could not be read") + return Self.errorResponse(status: 500, code: "relay_body_unreadable", message: "Failed to read the request body.") + } upstreamRequest.httpBodyStream = try body.makeInputStream() - upstreamRequest.setValue("\(body.count)", forHTTPHeaderField: "Content-Length") + upstreamRequest.setValue("\(length)", forHTTPHeaderField: "Content-Length") } catch { Logger.restRelay.error("Failed to open request body stream: \(error)") return Self.errorResponse(status: 500, code: "relay_body_unreadable", message: "Failed to read the request body.") @@ -124,7 +141,7 @@ struct RestRelay: Sendable { // to whatever host the `Location` header names and relay that // response back. See ``RedirectGuard``. let redirectGuard = RedirectGuard(allowedPrefix: apiRoot) - let upstream = HTTPResponse(try await session.data(for: upstreamRequest, delegate: redirectGuard)) + let upstream = HTTPResponse(try await Self.session.data(for: upstreamRequest, delegate: redirectGuard)) // A refused redirect leaves `URLSession` holding the 3xx itself. // Relaying that would undo the refusal: the response carries the From 5833b726393ef859709e950e83c0f5765095a969 Mon Sep 17 00:00:00 2001 From: David Calhoun Date: Mon, 31 Aug 2026 16:01:36 -0400 Subject: [PATCH 24/31] fix(ios): scope the preflight auth exemption to the policy that answers it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The exemption rests on the library answering a preflight with its own 204 before the handler runs, which only happens under `CORSPolicy.permissive`. Under any other policy an unauthenticated `OPTIONS` carrying `Access-Control-Request-Method` reached the handler — an unauthenticated way into a server that requires authentication. Exempt a preflight only when the permissive policy will answer it. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_014tQvFHoZpEeD6obF2YCoor --- ios/Sources/GutenbergKitHTTP/HTTPServer.swift | 14 +++++++--- .../HTTPServerAuthenticationTests.swift | 27 ++++++++++++++++--- .../HTTPServerTimeoutTests.swift | 13 +++++---- 3 files changed, 42 insertions(+), 12 deletions(-) diff --git a/ios/Sources/GutenbergKitHTTP/HTTPServer.swift b/ios/Sources/GutenbergKitHTTP/HTTPServer.swift index 0debe4c1b..6359578f5 100644 --- a/ios/Sources/GutenbergKitHTTP/HTTPServer.swift +++ b/ios/Sources/GutenbergKitHTTP/HTTPServer.swift @@ -385,9 +385,12 @@ public final class HTTPServer: Sendable { // the server read (and discard) an arbitrarily large body, and the // handler must never see an unauthenticated request. A CORS // preflight is exempt because preflights never include credentials - // (Fetch spec §3.3.5); an `OPTIONS` the client sent deliberately is - // not a preflight and is authenticated like any other request. - if requiresAuthentication && !isPreflight(partial) { + // (Fetch spec §3.3.5), and only under the policy that answers one + // below without reaching the handler; an `OPTIONS` the client sent + // deliberately is not a preflight and is authenticated like any + // other request. + let isExemptPreflight = cors == .permissive && isPreflight(partial) + if requiresAuthentication && !isExemptPreflight { guard authenticate(partial, token: token) else { throw HTTPServerError.authenticationFailed } @@ -403,7 +406,7 @@ public final class HTTPServer: Sendable { // requests are bodyless; a body on the auth-exempt path would // otherwise be read/drained without authentication — and the // accepted-body read below is bounded only by the idle timeout. - if isPreflight(partial), (parser.expectedBodyLength ?? 0) > 0 { + if isExemptPreflight, (parser.expectedBodyLength ?? 0) > 0 { throw HTTPServerError.unexpectedBody } @@ -813,6 +816,9 @@ public final class HTTPServer: Sendable { /// cannot use this to skip authentication: dropping the bearer token to /// look like a preflight means adding `Access-Control-Request-Method`, /// which routes the request to the 204 answer instead of the handler. + /// That holds only under ``CORSPolicy/permissive``, which is why the + /// authentication exemption is scoped to it — under any other policy a + /// preflight reaches the handler, so it is authenticated like anything else. private static func isPreflight(_ request: ParsedHTTPRequest) -> Bool { request.method.uppercased() == "OPTIONS" && request.header("Access-Control-Request-Method") != nil diff --git a/ios/Tests/GutenbergKitHTTPTests/HTTPServerAuthenticationTests.swift b/ios/Tests/GutenbergKitHTTPTests/HTTPServerAuthenticationTests.swift index fdd8ab4b9..c8066dc09 100644 --- a/ios/Tests/GutenbergKitHTTPTests/HTTPServerAuthenticationTests.swift +++ b/ios/Tests/GutenbergKitHTTPTests/HTTPServerAuthenticationTests.swift @@ -243,8 +243,29 @@ struct HTTPServerAuthenticationTests { // MARK: - CORS Preflight Auth Exemption - @Test("preflight without token returns 200 (CORS preflight exempt from auth)") - func preflightWithoutTokenReturns200() async throws { + @Test("preflight without token is answered under permissive CORS") + func preflightWithoutTokenIsAnsweredUnderPermissiveCORS() async throws { + // A preflight cannot carry credentials, so it is exempt from + // authentication — and the library answers it itself, so the exemption + // never reaches the handler. + let server = try await HTTPServer.start( + name: "auth-test", + requiresAuthentication: true, + cors: .permissive + ) { _ in + HTTPResponse(status: 200, body: Data("OK\n".utf8)) + } + defer { server.stop() } + + let raw = "OPTIONS /test HTTP/1.1\r\nHost: 127.0.0.1\r\nAccess-Control-Request-Method: GET\r\n\r\n" + let response = try await sendRaw(raw, toPort: server.port) + #expect(response.hasPrefix("HTTP/1.1 204")) + } + + @Test("preflight without token is authenticated without a CORS policy") + func preflightWithoutTokenRequiresAuthWithoutCORS() async throws { + // Without a policy to answer it, a preflight would reach the handler, + // so the exemption would be an unauthenticated way in. let server = try await HTTPServer.start( name: "auth-test", requiresAuthentication: true @@ -255,7 +276,7 @@ struct HTTPServerAuthenticationTests { let raw = "OPTIONS /test HTTP/1.1\r\nHost: 127.0.0.1\r\nAccess-Control-Request-Method: GET\r\n\r\n" let response = try await sendRaw(raw, toPort: server.port) - #expect(response.hasPrefix("HTTP/1.1 200")) + #expect(response.hasPrefix("HTTP/1.1 407")) } @Test("OPTIONS without Access-Control-Request-Method is not a preflight and returns 407") diff --git a/ios/Tests/GutenbergKitHTTPTests/HTTPServerTimeoutTests.swift b/ios/Tests/GutenbergKitHTTPTests/HTTPServerTimeoutTests.swift index 7000e581f..3281e69ec 100644 --- a/ios/Tests/GutenbergKitHTTPTests/HTTPServerTimeoutTests.swift +++ b/ios/Tests/GutenbergKitHTTPTests/HTTPServerTimeoutTests.swift @@ -76,7 +76,8 @@ struct HTTPServerTimeoutTests { func preflightWithBodyReturns400() async throws { let server = try await HTTPServer.start( name: "options-with-body", - requiresAuthentication: true + requiresAuthentication: true, + cors: .permissive ) { _ in HTTPResponse(status: 200, body: Data("OK\n".utf8)) } @@ -94,7 +95,8 @@ struct HTTPServerTimeoutTests { let server = try await HTTPServer.start( name: "options-oversized-body", requiresAuthentication: true, - maxRequestBodySize: 16 + maxRequestBodySize: 16, + cors: .permissive ) { _ in HTTPResponse(status: 200, body: Data("OK\n".utf8)) } @@ -107,11 +109,12 @@ struct HTTPServerTimeoutTests { #expect(response.hasPrefix("HTTP/1.1 400")) } - @Test("bodyless OPTIONS preflight still succeeds") + @Test("bodyless OPTIONS preflight is still answered") func bodylessOptionsSucceeds() async throws { let server = try await HTTPServer.start( name: "options-bodyless", - requiresAuthentication: true + requiresAuthentication: true, + cors: .permissive ) { _ in HTTPResponse(status: 200, body: Data("OK\n".utf8)) } @@ -119,7 +122,7 @@ struct HTTPServerTimeoutTests { let raw = "OPTIONS /test HTTP/1.1\r\nHost: 127.0.0.1\r\nAccess-Control-Request-Method: GET\r\n\r\n" let response = try await sendRaw(raw, toPort: server.port) - #expect(response.hasPrefix("HTTP/1.1 200")) + #expect(response.hasPrefix("HTTP/1.1 204")) } // MARK: - Start timeout From 57523687629c7d288175ce5544ba968768b578f3 Mon Sep 17 00:00:00 2001 From: David Calhoun Date: Mon, 31 Aug 2026 16:02:59 -0400 Subject: [PATCH 25/31] fix(ios): make NetworkProxy constructible outside the module The struct is public and names a parameter of a public initializer, but its memberwise initializer is internal, so the only value a host could pass was nil. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_014tQvFHoZpEeD6obF2YCoor --- ios/Sources/GutenbergKit/Sources/Model/GBKitGlobal.swift | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/ios/Sources/GutenbergKit/Sources/Model/GBKitGlobal.swift b/ios/Sources/GutenbergKit/Sources/Model/GBKitGlobal.swift index b77d9c237..ce8ca3914 100644 --- a/ios/Sources/GutenbergKit/Sources/Model/GBKitGlobal.swift +++ b/ios/Sources/GutenbergKit/Sources/Model/GBKitGlobal.swift @@ -101,6 +101,14 @@ public struct GBKitGlobal: Sendable, Codable { public struct NetworkProxy: Sendable, Codable { let port: Int let token: String + + /// The synthesized memberwise initializer is internal, which would + /// leave the `networkProxy` parameter of this type's public initializer + /// with no value a host could pass it. + public init(port: Int, token: String) { + self.port = port + self.token = token + } } let networkProxy: NetworkProxy? From c5265976036b68248344799902b481ea603676c1 Mon Sep 17 00:00:00 2001 From: David Calhoun Date: Mon, 31 Aug 2026 16:03:04 -0400 Subject: [PATCH 26/31] docs(ios): repair two comments the relay work left behind MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A stray fragment above `loadEditorWithoutDependencies` merged into that method's documentation, and the symbol link to `HTTPServer.start(…)` was not updated for the `requiresBrowserOrigin` parameter, so it no longer resolved. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_014tQvFHoZpEeD6obF2YCoor --- ios/Sources/GutenbergKit/Sources/EditorViewController.swift | 1 - ios/Sources/GutenbergKitHTTP/HTTPServerDelegate.swift | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/ios/Sources/GutenbergKit/Sources/EditorViewController.swift b/ios/Sources/GutenbergKit/Sources/EditorViewController.swift index 1375869cb..bb07e6dc2 100644 --- a/ios/Sources/GutenbergKit/Sources/EditorViewController.swift +++ b/ios/Sources/GutenbergKit/Sources/EditorViewController.swift @@ -426,7 +426,6 @@ public final class EditorViewController: UIViewController, GutenbergEditorContro } } - /// Starts the loopback network proxy when the web view is subject to /// Loads the editor HTML without any dependencies (warmup mode only). /// /// This method is used exclusively by the warmup mechanism to preload editor resources diff --git a/ios/Sources/GutenbergKitHTTP/HTTPServerDelegate.swift b/ios/Sources/GutenbergKitHTTP/HTTPServerDelegate.swift index 281533746..719b77b20 100644 --- a/ios/Sources/GutenbergKitHTTP/HTTPServerDelegate.swift +++ b/ios/Sources/GutenbergKitHTTP/HTTPServerDelegate.swift @@ -8,7 +8,7 @@ import Foundation /// behavior it wants to change. A server started without a delegate — or whose /// delegate leaves a method defaulted — uses the library's built-in behavior. /// New customization points are added here as new defaulted methods, so -/// ``HTTPServer/start(name:port:listenOnAllInterfaces:requiresAuthentication:maxRequestBodySize:maxConnections:readTimeout:bodyReadTimeout:idleTimeout:startTimeout:cors:delegate:handler:)`` +/// ``HTTPServer/start(name:port:listenOnAllInterfaces:requiresAuthentication:requiresBrowserOrigin:maxRequestBodySize:maxConnections:readTimeout:bodyReadTimeout:idleTimeout:startTimeout:cors:delegate:handler:)`` /// never grows another parameter for them. /// /// The server **retains** its delegate for its lifetime. Because the delegate is From d52fc12ed7dcde98178b0d7d7d63084b45c7f367 Mon Sep 17 00:00:00 2001 From: David Calhoun Date: Mon, 31 Aug 2026 16:04:33 -0400 Subject: [PATCH 27/31] fix(wp-env): allow the method-override header through CORS MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit api-fetch turns every PUT/PATCH/DELETE into a POST carrying `X-HTTP-Method-Override`. The header is not CORS-safelisted, so the browser announces it in the preflight and wp-env's allow-list — which replaces core's — rejected every such request from the dev server. The iOS policy gained the same header in this branch. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_014tQvFHoZpEeD6obF2YCoor --- wp-env/mu-plugins/gutenbergkit-cors.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/wp-env/mu-plugins/gutenbergkit-cors.php b/wp-env/mu-plugins/gutenbergkit-cors.php index 72d76ca25..2059fae5d 100644 --- a/wp-env/mu-plugins/gutenbergkit-cors.php +++ b/wp-env/mu-plugins/gutenbergkit-cors.php @@ -33,7 +33,10 @@ function gutenbergkit_cors_send_origin_headers( $origin ) { } header( 'Access-Control-Allow-Methods: GET, POST, PUT, PATCH, DELETE, OPTIONS' ); - header( 'Access-Control-Allow-Headers: Authorization, Content-Type, X-WP-Nonce' ); + // api-fetch turns every PUT/PATCH/DELETE into a POST carrying + // `X-HTTP-Method-Override`, which is not CORS-safelisted, so the browser + // announces it in the preflight and blocks the request without it here. + header( 'Access-Control-Allow-Headers: Authorization, Content-Type, X-HTTP-Method-Override, X-WP-Nonce' ); } /** From cd550c1b32974295821ff140c37151aab660fd60 Mon Sep 17 00:00:00 2001 From: David Calhoun Date: Mon, 31 Aug 2026 16:06:17 -0400 Subject: [PATCH 28/31] docs: trim the relay's commentary to what the code needs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `RedirectGuard` and `createRelayFetch` carried several paragraphs of rationale each; keep the reason and the cost, drop the retelling. Also remove the claim that api-fetch's default `Accept` value reaches a preflight — none of its bytes are CORS-unsafe, so the header is safelisted. It stays in the allow-list for values that are not. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_014tQvFHoZpEeD6obF2YCoor --- .../Sources/Media/RestRelay.swift | 25 +++++------------- ios/Sources/GutenbergKitHTTP/CORSPolicy.swift | 3 --- src/utils/fetch-relay.js | 26 +++++++------------ 3 files changed, 15 insertions(+), 39 deletions(-) diff --git a/ios/Sources/GutenbergKit/Sources/Media/RestRelay.swift b/ios/Sources/GutenbergKit/Sources/Media/RestRelay.swift index a384ab515..7674a7ca5 100644 --- a/ios/Sources/GutenbergKit/Sources/Media/RestRelay.swift +++ b/ios/Sources/GutenbergKit/Sources/Media/RestRelay.swift @@ -239,25 +239,12 @@ struct RestRelay: Sendable { /// carrying the site credential — followed to that host, and its response /// relayed back to the editor. Refusing hands the 3xx itself back instead. /// - /// The comparison is a prefix match on the whole URL, not just its host, - /// so a redirect to another path on the same site — `/wp-login.php` for a - /// request to `/wp-json/…` — is refused too. The site credential should - /// follow the request only to the API it was configured for. It also means - /// a scheme downgrade fails without a rule of its own, since `http://…` - /// cannot prefix-match an `https://` root. - /// - /// The known cost: a redirect that changes permalink structure — pretty - /// REST URLs to `?rest_route=`, or the reverse after a permalink change — - /// is a legitimate redirect that this refuses. Refusing is the right - /// default because it cannot hand the credential somewhere unverified, and - /// the response says so specifically enough to find. - /// - /// The comparison is origin-exact, unlike the JavaScript side's, which - /// tolerates host aliases when recognizing a site URL. That asymmetry is - /// deliberate: recognizing an alias decides where a request is *sent*, - /// while this decides whether the site credential *follows* a redirect - /// somewhere we did not choose. A canonical redirect onto an alias host is - /// refused here rather than followed. + /// The comparison is a prefix match on the whole URL, so another path on + /// the same site (`/wp-login.php`), a scheme downgrade, and an alias of the + /// configured host are all refused: the site credential follows the request + /// only to the API it was configured for. The cost is that a legitimate + /// permalink-structure redirect is refused too, which the response says + /// specifically enough to diagnose. /// /// `@unchecked Sendable`: `allowedPrefix` is a `let` set at init; the /// refusal is recorded under a lock. diff --git a/ios/Sources/GutenbergKitHTTP/CORSPolicy.swift b/ios/Sources/GutenbergKitHTTP/CORSPolicy.swift index e3aee1f30..36a13ece9 100644 --- a/ios/Sources/GutenbergKitHTTP/CORSPolicy.swift +++ b/ios/Sources/GutenbergKitHTTP/CORSPolicy.swift @@ -32,9 +32,6 @@ public enum CORSPolicy: Sendable { // can't be cleanly allowlisted. ("Access-Control-Allow-Origin", "*"), ("Access-Control-Allow-Methods", "GET, POST, PUT, PATCH, DELETE, OPTIONS"), - // `Accept` is listed because api-fetch's default value - // (`application/json, */*;q=0.1`) contains CORS-unsafe bytes, - // so it is not safelisted and does reach the preflight. ("Access-Control-Allow-Headers", "Accept, Authorization, Content-Type, Relay-Authorization, X-HTTP-Method-Override"), ("Access-Control-Max-Age", "86400"), ] diff --git a/src/utils/fetch-relay.js b/src/utils/fetch-relay.js index 013948f07..7f61dd7ac 100644 --- a/src/utils/fetch-relay.js +++ b/src/utils/fetch-relay.js @@ -18,24 +18,16 @@ const LOOPBACK_HOSTNAMES = new Set( [ 'localhost', '127.0.0.1', '[::1]' ] ); * `GBKit.networkProxy`; it forwards each request to the site's REST API * natively and responds with CORS headers the web view accepts. * - * ## Why the transport, and not `apiFetch` + * The relay wraps `fetch` rather than registering an api-fetch middleware or + * fetch handler, either of which would replace api-fetch's own request building + * and response parsing — the `data`-to-body serialization, the method override, + * every parsing rule — and leave us reimplementing a package we do not control. + * Below `fetch`, this layer only changes where the request goes. * - * `apiFetch.use()` unshifts and api-fetch composes with `reduceRight`, so a - * registered middleware always runs *outside* api-fetch's own middleware and - * outside `defaultFetchHandler` — it would short-circuit past the `data`-to-body - * serialization, the default `Accept` header, the HTTP v1 method override, and - * every response-parsing rule, all of which would then have to be reimplemented - * and kept in step with a package we do not control. `setFetchHandler` runs late - * enough for the request half but still replaces the response half. - * - * Wrapping `fetch` puts the relay below all of it: api-fetch builds the whole - * request, hands it to `fetch`, and parses whatever comes back. This layer only - * changes where the request goes. - * - * @param {typeof fetch} next The fetch to delegate to. - * @param {Object} config Relay configuration. - * @param {{port: number, token: string}} config.networkProxy Relay connection details. - * @param {string} config.siteApiRoot The site's REST API root. + * @param {typeof fetch} next The fetch to delegate to. + * @param {Object} config Relay configuration. + * @param {import('./bridge').NetworkProxy} config.networkProxy Relay connection details. + * @param {string} config.siteApiRoot The site's REST API root. * @return {typeof fetch} The wrapped fetch. */ export function createRelayFetch( next, { networkProxy, siteApiRoot } ) { From 1494ca63811323a68a60973b836981a5f3ebe22c Mon Sep 17 00:00:00 2001 From: David Calhoun Date: Tue, 1 Sep 2026 07:34:43 -0400 Subject: [PATCH 29/31] revert(ios): keep persisting the editor configuration Restores the `localStorage` copy of `GBKit`, keeping iOS in step with Android until the persistence is removed from both. The relay details still read through `getGBKit` like every other field. The fallback that copy feeds cannot reach the relay in a production build: boot aborts on a missing `window.GBKit` before the fetch wrappers are installed, except under `?dev_mode`. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_014tQvFHoZpEeD6obF2YCoor --- .../Sources/EditorViewController.swift | 22 +++++----------- .../EditorConfigurationScriptTests.swift | 26 ------------------- 2 files changed, 6 insertions(+), 42 deletions(-) delete mode 100644 ios/Tests/GutenbergKitTests/EditorConfigurationScriptTests.swift diff --git a/ios/Sources/GutenbergKit/Sources/EditorViewController.swift b/ios/Sources/GutenbergKit/Sources/EditorViewController.swift index bb07e6dc2..8c56ac1d9 100644 --- a/ios/Sources/GutenbergKit/Sources/EditorViewController.swift +++ b/ios/Sources/GutenbergKit/Sources/EditorViewController.swift @@ -460,25 +460,15 @@ public final class EditorViewController: UIViewController, GutenbergEditorContro nativeUploadToken: isUploadPipelineEnabled ? uploadServer?.token : nil, networkProxy: networkProxyGlobal ) - return WKUserScript( - source: Self.configurationScript(gbkitGlobal: try gbkitGlobal.toString()), - injectionTime: .atDocumentStart, - forMainFrameOnly: true - ) - } + let stringValue = try gbkitGlobal.toString() - /// The document-start script that installs `window.GBKit`. - /// - /// The configuration is session-scoped — it carries the site credential and - /// the local server's port and tokens — so no copy of it outlives the load - /// that injected it. (Android clears the WebView's web storage before each - /// load for the same reason.) - static func configurationScript(gbkitGlobal: String) -> String { - """ - window.GBKit = \(gbkitGlobal); - localStorage.removeItem('GBKit'); + let jsCode = """ + window.GBKit = \(stringValue); + localStorage.setItem('GBKit', JSON.stringify(window.GBKit)); "done"; """ + + return WKUserScript(source: jsCode, injectionTime: .atDocumentStart, forMainFrameOnly: true) } /// Starts the local HTTP server for routing file uploads through native processing. diff --git a/ios/Tests/GutenbergKitTests/EditorConfigurationScriptTests.swift b/ios/Tests/GutenbergKitTests/EditorConfigurationScriptTests.swift deleted file mode 100644 index 1115dd931..000000000 --- a/ios/Tests/GutenbergKitTests/EditorConfigurationScriptTests.swift +++ /dev/null @@ -1,26 +0,0 @@ -import Foundation -import Testing - -@testable import GutenbergKit - -#if canImport(UIKit) - -@Suite("Editor configuration script") -struct EditorConfigurationScriptTests { - - @MainActor - @Test("injects the configuration without persisting it") - func doesNotPersistTheConfiguration() { - // The injected configuration carries the site credential and the local - // server's port and tokens, all of them valid only for this session. - let script = EditorViewController.configurationScript( - gbkitGlobal: #"{"authHeader":"Bearer secret"}"# - ) - - #expect(script.contains(#"window.GBKit = {"authHeader":"Bearer secret"};"#)) - #expect(script.contains("localStorage.removeItem('GBKit')")) - #expect(!script.contains("localStorage.setItem")) - } -} - -#endif From 7a6ac1f49971c0308dbac8b456075b5a3151209a Mon Sep 17 00:00:00 2001 From: David Calhoun Date: Tue, 1 Sep 2026 07:42:55 -0400 Subject: [PATCH 30/31] fix: require the site's port to match before relaying MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The origin rewrite replaced the target's port instead of comparing it, so a request to another service on the site's host — `example.com:8443` for a site on 443 — matched the API root and was answered by the site's REST API instead. Scheme replacement has a reason (an `http` `siteurl` behind a TLS-terminating proxy); the port never did. Also record the alias shapes this cannot recognize, and where that belongs instead: a site reached by LAN IP emits `localhost` URLs, whose paginated `Link` targets go direct and fail under Lockdown Mode. Growing the list of spellings cannot close that class — the relay can, because it knows where the response came from. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_014tQvFHoZpEeD6obF2YCoor --- src/utils/fetch-relay.js | 33 +++++++++++++++++++++------------ src/utils/fetch-relay.test.js | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 12 deletions(-) diff --git a/src/utils/fetch-relay.js b/src/utils/fetch-relay.js index 7f61dd7ac..3357d4bb7 100644 --- a/src/utils/fetch-relay.js +++ b/src/utils/fetch-relay.js @@ -143,13 +143,22 @@ function addressesLocalServer( target, port ) { * need not spell the host the app was configured with — `www.` versus bare, or * wp-env's `localhost` versus the `127.0.0.1` its runtime writes into * `WP_SITEURL` (the e2e fixtures work around the same thing by matching uploads - * on path rather than hostname, see `e2e/wp-env-fixtures.js`). Those spellings - * name the configured host, so the target is moved onto the root's origin — - * scheme and port included — before comparing. Any other host keeps the direct - * path it had before a relay existed: rewriting it would send a third party's - * request to the user's own site, with the site credential attached. A *path* - * difference is likewise a different resource: in a subdirectory multisite - * `https://site/a/wp-json/` and `https://site/b/wp-json/` are separate sites. + * on path rather than hostname, see `e2e/wp-env-fixtures.js`). Only the scheme + * may differ beyond that — an `http` `siteurl` behind a TLS-terminating proxy. + * The port has to match: another port on the same host is another server, and + * answering its request with the site's response is a different bug from the + * one this tolerance exists to fix. A *path* difference is likewise a different + * resource: in a subdirectory multisite `https://site/a/wp-json/` and + * `https://site/b/wp-json/` are separate sites. + * + * Anything else keeps the direct path it had before a relay existed, because + * rewriting it would send a third party's request to the user's own site with + * the site credential attached. The cost is an alias this cannot recognize — + * a site reached by LAN IP whose `home_url()` says `localhost`, a mapped + * domain, a migration — where a paginated `Link` target goes direct and fails + * under Lockdown Mode. Growing the list of spellings cannot close that: the + * fix is for the relay to canonicalize the URLs the site emits, since it knows + * the response came from the configured root and this layer can only guess. * * @param {URL} target The request's target. * @param {URL} apiRoot The site's REST API root, normalized and slash-terminated. @@ -157,18 +166,18 @@ function addressesLocalServer( target, port ) { */ function relayUpstreamPath( target, apiRoot ) { if ( - canonicalHost( target.hostname ) !== canonicalHost( apiRoot.hostname ) + canonicalHost( target.hostname ) !== + canonicalHost( apiRoot.hostname ) || + target.port !== apiRoot.port ) { return null; } const aliased = new URL( target ); - // Assign the origin's parts separately: the `host` setter leaves an - // existing port in place when the value carries none, so `host` alone turns - // `http://127.0.0.1:8888/…` into `https://example.com:8888/…`. + // `hostname` rather than `host`: the `host` setter would drop the port when + // the value carries none, and the port is part of the match. aliased.protocol = apiRoot.protocol; aliased.hostname = apiRoot.hostname; - aliased.port = apiRoot.port; if ( ! aliased.href.startsWith( apiRoot.href ) ) { return null; diff --git a/src/utils/fetch-relay.test.js b/src/utils/fetch-relay.test.js index 9ded8014f..6421ec556 100644 --- a/src/utils/fetch-relay.test.js +++ b/src/utils/fetch-relay.test.js @@ -142,6 +142,38 @@ describe( 'createRelayFetch', () => { ); } ); + it( 'another service on the site host', async () => { + // A different port is a different server, however its paths are + // shaped. Relaying it would answer one service's request with + // another's response. + await expectPassthrough( + 'https://example.com:8443/wp-json/wp/v2/posts' + ); + await relayFetch( 'http://localhost:8888/wp-json/' )( + 'http://127.0.0.1:3000/wp-json/wp/v2/posts' + ); + expect( next ).toHaveBeenCalledWith( + 'http://127.0.0.1:3000/wp-json/wp/v2/posts', + undefined + ); + } ); + + it( 'a host alias the canonical form does not cover', async () => { + // A documented limitation, not a decision: a site reached by LAN IP + // emits `localhost` URLs, which are not recognized as the same host, + // so paginated `Link` targets take the direct path — and fail under + // Lockdown Mode. Provenance belongs at the relay, which knows the + // response came from the configured site; see `relayUpstreamPath`. + await relayFetch( 'http://192.168.1.50:8888/wp-json/' )( + 'http://localhost:8888/wp-json/wp/v2/posts?page=2' + ); + + expect( next ).toHaveBeenCalledWith( + 'http://localhost:8888/wp-json/wp/v2/posts?page=2', + undefined + ); + } ); + it( 'another WordPress site whose path matches the root', async () => { // A different host is a different server, however its paths are // shaped. Rewriting one onto the configured site would send its From 64e59190ef229de4d4d06c22eb3df3604cc37757 Mon Sep 17 00:00:00 2001 From: David Calhoun Date: Tue, 1 Sep 2026 07:52:46 -0400 Subject: [PATCH 31/31] docs: correct the reason for assigning hostname over host MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `host` setter retains an existing port when the value carries none — it does not drop it, as the comment claimed. That retention is what made `host` wrong for the old code, which assigned the port separately afterwards; now that the port is compared before substitution, either setter would do and the honest reason is simply that nothing but the scheme and the host spelling is left to reconcile. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_014tQvFHoZpEeD6obF2YCoor --- src/utils/fetch-relay.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/utils/fetch-relay.js b/src/utils/fetch-relay.js index 3357d4bb7..521dd8a75 100644 --- a/src/utils/fetch-relay.js +++ b/src/utils/fetch-relay.js @@ -174,8 +174,8 @@ function relayUpstreamPath( target, apiRoot ) { } const aliased = new URL( target ); - // `hostname` rather than `host`: the `host` setter would drop the port when - // the value carries none, and the port is part of the match. + // Only the scheme and the host spelling are left to reconcile; the port + // matched above, and assigning `hostname` leaves it in place. aliased.protocol = apiRoot.protocol; aliased.hostname = apiRoot.hostname;