From 46f70ab20035affdbf3898c3039d9815250c1de9 Mon Sep 17 00:00:00 2001 From: Guilherme Souza Date: Wed, 9 Sep 2026 20:57:51 -0300 Subject: [PATCH] feat!: stop trapping on dynamic values in library code MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every `fatalError`/`preconditionFailure`/`precondition`/`try!` reached from a value that varies at runtime is gone. A per-call redirect URL, a WebSocket close code, or a payload built from a server-issued token used to take the host app down. The dividing line is when the value is fixed, not who supplied it: - Fixed once, at construction — an initializer argument or a configuration field. These still trap. The value cannot change afterwards, so a bad one is a programmer error, and `precondition` reports it at the exact point it was introduced instead of burying it behind an unrelated failure later. `SupabaseClient.init` still traps on a `supabaseURL` with no host, `StorageApi` on a URL it cannot decompose, `RetryRequestInterceptor` on a backoff base below 2. - Varies at runtime — a per-call parameter, a system callback, a value derived from a server response. These never trap. They throw where the context already throws, and otherwise report and fall back. `AGENTS.md` gains a "When trapping is allowed" section so this is not re-litigated per site. ## Changes - **Auth** — a missing OAuth redirect scheme throws the new `AuthError.oauthFlowFailed(message:)`. `redirectTo` is a per-call parameter and the enclosing method already throws, so the error costs nothing. Same for an `ASWebAuthenticationSession` callback carrying neither a URL nor an error. PKCE's `data(using: .utf8)` becomes the non-failable `Data(_:)`. The two flow-type `precondition`s are deleted: the `switch` in `session(from:)` is their only caller and already proves them. - **RealtimeV2** — a non-`ws` scheme throws `WebSocketError.connection` rather than trapping, in a function that already throws. `close(code:reason:)` takes both values per call, so an out-of-range code or an overlong reason is reported and clamped, truncating on whole characters so the frame never carries a split UTF-8 scalar. The `try! JSONObject` on the join payload (which embeds a server-issued access token) reports and skips. `RealtimeClientV2.apikey` was assigned once and never read, so the `precondition` guarding it and its force unwrap both delete — the rest of the file already treats the apikey as optional. - **Storage** — the `try! NSRegularExpression` becomes three `hasSuffix` checks, which also stops `supabaseXco` matching as a platform host (`.` in the old pattern matched any character). - **Supabase** — the storage-key derivation moves into `defaultStorageKey(for:)`, fixing an index-out-of-range the old `host.split(separator: ".")[0]` hit on an empty host, where `split` returns an empty array. The construction-time check itself still traps. ## Out of scope - `Dependencies.swift:30` — a lifetime error, not a value at all, and the subscript has no value to return after reporting. - `PostgrestUpdate.swift:67,86` — `@available(*, unavailable)` getters, so calling one is already a compile error. BREAKING CHANGE: `AuthError` gains `oauthFlowFailed(message:)`, which breaks exhaustive switches over `AuthError`. See V3_MIGRATION.md. Fixes SDK-1793 --- AGENTS.md | 19 ++++ Sources/Auth/AuthClient.swift | 27 +++-- Sources/Auth/AuthError.swift | 8 +- Sources/Auth/Internal/PKCE.swift | 7 +- .../HTTP/RetryRequestInterceptor.swift | 2 + Sources/RealtimeV2/CallbackManager.swift | 14 +-- Sources/RealtimeV2/RealtimeChannelV2.swift | 7 +- Sources/RealtimeV2/RealtimeClientV2.swift | 4 - .../WebSocket/URLSessionWebSocket.swift | 75 ++++++++++--- Sources/Storage/StorageApi.swift | 36 ++++-- Sources/Supabase/SupabaseClient.swift | 25 ++++- .../xcshareddata/swiftpm/Package.resolved | 14 +-- Tests/AuthTests/AuthErrorTests.swift | 5 + .../RealtimeClientOptionsTests.swift | 12 ++ ...SessionWebSocketCloseValidationTests.swift | 103 ++++++++++++++++++ .../StorageTests/StorageBucketAPITests.swift | 20 ++++ .../SupabaseClientStorageKeyTests.swift | 65 +++++++++++ V3_MIGRATION.md | 58 +++++++++- sdk-compliance.yaml | 1 + 19 files changed, 440 insertions(+), 62 deletions(-) create mode 100644 Tests/RealtimeTests/URLSessionWebSocketCloseValidationTests.swift create mode 100644 Tests/SupabaseTests/SupabaseClientStorageKeyTests.swift diff --git a/AGENTS.md b/AGENTS.md index 8d9a328a0..7f454991f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -180,6 +180,25 @@ Use standard file headers with copyright: - Use `async throws` for async error handling - Report issues using `IssueReporting` from xctest-dynamic-overlay +#### When trapping is allowed + +The dividing line is *when the value is fixed*, not who supplied it. + +- **Fixed once, at construction** — an initializer argument, a configuration field, a `package` + tuning constant. `precondition`/`preconditionFailure` is the right tool: the value cannot change + afterwards, so a bad one is a programmer error, and trapping reports it at the exact point it + was introduced. `SupabaseClient.init` traps on a `supabaseURL` with no host; `StorageApi` traps + on a URL it cannot decompose; `RetryRequestInterceptor` traps on a backoff base below 2. + Degrading instead would bury the mistake behind an unrelated failure much later. +- **Varies at runtime, or comes from the server** — a per-call parameter, a response header, a + decoded payload, a WebSocket close code. Never trap. Throw if the context already throws; + otherwise `reportIssue` and fall back. `HTTPFields.init(_:)` drops invalid field names rather + than trapping precisely because `HTTPResponse.init` builds it from `response.allHeaderFields`, + which a proxy or a hostile server controls. + +A value being "user input" is not on its own a reason to avoid trapping — `supabaseURL` is user +input and traps. A value being *dynamic* is. See SDK-1793. + ### Testing Conventions This project uses the [Swift Testing](https://developer.apple.com/documentation/testing) framework, and only Swift Testing — do not use XCTest or `XCTestCase`. diff --git a/Sources/Auth/AuthClient.swift b/Sources/Auth/AuthClient.swift index a6a172505..9412a458a 100644 --- a/Sources/Auth/AuthClient.swift +++ b/Sources/Auth/AuthClient.swift @@ -856,9 +856,15 @@ public actor AuthClient { ) { @MainActor url in try await withCheckedThrowingContinuation { [configuration] continuation in guard let callbackScheme = (configuration.redirectToURL ?? redirectTo)?.scheme else { - preconditionFailure( - "Please, provide a valid redirect URL, either through `redirectTo` param, or globally through `AuthClient.Configuration.redirectToURL`." + continuation.resume( + throwing: AuthError.oauthFlowFailed( + message: """ + Provide a redirect URL with a scheme, either through the `redirectTo` parameter \ + or globally through `AuthClient.Configuration.redirectToURL`. + """ + ) ) + return } #if !os(tvOS) && !os(watchOS) @@ -874,7 +880,14 @@ public actor AuthClient { } else if let url { continuation.resume(returning: url) } else { - fatalError("Expected url or error, but got none.") + // `ASWebAuthenticationSession` always reports a URL or an error. Surface a broken + // contract as a thrown error rather than taking the host app down with it. + reportIssue("ASWebAuthenticationSession returned neither a URL nor an error.") + continuation.resume( + throwing: AuthError.oauthFlowFailed( + message: "ASWebAuthenticationSession returned neither a URL nor an error." + ) + ) } #if !os(tvOS) && !os(watchOS) @@ -991,9 +1004,9 @@ public actor AuthClient { } } + /// Only reached from ``session(from:)``, under `case .implicit` of its switch on + /// `configuration.flowType`, which is what guarantees the flow type here. private func handleImplicitGrantFlow(params: [String: String]) async throws -> Session { - precondition(configuration.flowType == .implicit, "Method only allowed for implicit flow.") - if let errorMessage = params["error_description"] ?? params["error"] { throw AuthError.implicitGrantRedirect(message: errorMessage) } @@ -1040,9 +1053,9 @@ public actor AuthClient { return session } + /// Only reached from ``session(from:)``, under `case .pkce` of its switch on + /// `configuration.flowType`, which is what guarantees the flow type here. private func handlePKCEFlow(params: [String: String], flowId: String?) async throws -> Session { - precondition(configuration.flowType == .pkce, "Method only allowed for PKCE flow.") - if params["error"] != nil || params["error_description"] != nil || params["error_code"] != nil { throw AuthError.pkceGrantCodeExchange( message: params["error_description"] diff --git a/Sources/Auth/AuthError.swift b/Sources/Auth/AuthError.swift index 811ad4db0..7907cbd71 100644 --- a/Sources/Auth/AuthError.swift +++ b/Sources/Auth/AuthError.swift @@ -230,6 +230,7 @@ extension ErrorCode { /// ### OAuth flow errors /// - ``pkceGrantCodeExchange(message:error:code:)`` /// - ``implicitGrantRedirect(message:)`` +/// - ``oauthFlowFailed(message:)`` /// /// ### JWT errors /// - ``jwtVerificationFailed(message:)`` @@ -255,6 +256,10 @@ public enum AuthError: LocalizedError, Equatable { /// Error thrown when an error happens during implicit grant flow. case implicitGrantRedirect(message: String) + /// Error thrown when an OAuth flow cannot start or finish on the client, before any request + /// reaches the server — most often because no redirect URL with a scheme is configured. + case oauthFlowFailed(message: String) + /// Error thrown when JWT verification fails. case jwtVerificationFailed(message: String) @@ -265,6 +270,7 @@ public enum AuthError: LocalizedError, Equatable { .api(let message, _, _, _), .pkceGrantCodeExchange(let message, _, _), .implicitGrantRedirect(let message), + .oauthFlowFailed(let message), .jwtVerificationFailed(let message): message } @@ -275,7 +281,7 @@ public enum AuthError: LocalizedError, Equatable { case .sessionMissing: .sessionNotFound case .weakPassword: .weakPassword case .api(_, let errorCode, _, _): errorCode - case .pkceGrantCodeExchange, .implicitGrantRedirect: .unknown + case .pkceGrantCodeExchange, .implicitGrantRedirect, .oauthFlowFailed: .unknown case .jwtVerificationFailed: .invalidJWT } } diff --git a/Sources/Auth/Internal/PKCE.swift b/Sources/Auth/Internal/PKCE.swift index 01d7fcfb5..a97d075f3 100644 --- a/Sources/Auth/Internal/PKCE.swift +++ b/Sources/Auth/Internal/PKCE.swift @@ -13,12 +13,9 @@ extension PKCE { return Data(buffer).pkceBase64EncodedString() }, generateCodeChallenge: { codeVerifier in - guard let data = codeVerifier.data(using: .utf8) else { - preconditionFailure("provided string should be utf8 encoded.") - } - + // `Data(_:)` over a String's UTF-8 view cannot fail, unlike `data(using: .utf8)`. var hasher = SHA256() - hasher.update(data: data) + hasher.update(data: Data(codeVerifier.utf8)) let hashed = hasher.finalize() return Data(hashed).pkceBase64EncodedString() } diff --git a/Sources/Helpers/HTTP/RetryRequestInterceptor.swift b/Sources/Helpers/HTTP/RetryRequestInterceptor.swift index e99b476ed..b7cc393b7 100644 --- a/Sources/Helpers/HTTP/RetryRequestInterceptor.swift +++ b/Sources/Helpers/HTTP/RetryRequestInterceptor.swift @@ -86,6 +86,8 @@ package actor RetryRequestInterceptor: HTTPClientInterceptor { retryableErrorCodes: Set = RetryRequestInterceptor.defaultRetryableURLErrorCodes, clock: any Clock = ContinuousClock() ) { + // A base below 2 makes each wait shorter than the last instead of longer. The value is fixed + // at construction, so this is a programmer error, not a runtime condition. precondition( exponentialBackoffBase >= 2, "The `exponentialBackoffBase` must be a minimum of 2." diff --git a/Sources/RealtimeV2/CallbackManager.swift b/Sources/RealtimeV2/CallbackManager.swift index c1c76ca83..d23e21fca 100644 --- a/Sources/RealtimeV2/CallbackManager.swift +++ b/Sources/RealtimeV2/CallbackManager.swift @@ -151,15 +151,11 @@ final class CallbackManager: Sendable { } func triggerBroadcastData(event: String, data: Data) { - let callbacks = mutableState.callbacks.filter { - isBroadcastDataCallback(callback: $0, for: event) - } - .map { callback -> BroadcastDataCallback in - if case .broadcastData(let callback) = callback { - return callback - } else { - fatalError("Expected broadcast data callback") - } + let callbacks = mutableState.callbacks.compactMap { callback -> BroadcastDataCallback? in + guard isBroadcastDataCallback(callback: callback, for: event), + case .broadcastData(let broadcastData) = callback + else { return nil } + return broadcastData } callbacks.forEach { $0.callback(data) } } diff --git a/Sources/RealtimeV2/RealtimeChannelV2.swift b/Sources/RealtimeV2/RealtimeChannelV2.swift index 74fdb8e77..dfa307398 100644 --- a/Sources/RealtimeV2/RealtimeChannelV2.swift +++ b/Sources/RealtimeV2/RealtimeChannelV2.swift @@ -257,10 +257,15 @@ public final class RealtimeChannelV2: Sendable, RealtimeChannelProtocol { version: socket.options.headers[.xClientInfo] ) + guard let encodedPayload = try? JSONObject(payload) else { + reportIssue("Failed to encode the phx_join payload for channel '\(topic)'. Skipping join.") + return + } + await push( ChannelEvent.join, ref: ref, - payload: try! JSONObject(payload) + payload: encodedPayload ) } diff --git a/Sources/RealtimeV2/RealtimeClientV2.swift b/Sources/RealtimeV2/RealtimeClientV2.swift index a542e5a99..3feea9400 100644 --- a/Sources/RealtimeV2/RealtimeClientV2.swift +++ b/Sources/RealtimeV2/RealtimeClientV2.swift @@ -124,7 +124,6 @@ public final class RealtimeClientV2: Sendable, RealtimeClientProtocol { let wsTransport: WebSocketTransport let mutableState = LockIsolated(MutableState()) let http: any HTTPClientType - let apikey: String let serializer = RealtimeSerializer() let clock: any Clock @@ -193,9 +192,6 @@ public final class RealtimeClientV2: Sendable, RealtimeClientProtocol { self.http = http self.clock = clock - precondition(options.apikey != nil, "API key is required to connect to Realtime") - apikey = options.apikey! - mutableState.withValue { [options] in if let accessToken = options.headers[.authorization]?.split(separator: " ").last { $0.accessToken = String(accessToken) diff --git a/Sources/RealtimeV2/WebSocket/URLSessionWebSocket.swift b/Sources/RealtimeV2/WebSocket/URLSessionWebSocket.swift index 38a3b174b..1d001da8b 100644 --- a/Sources/RealtimeV2/WebSocket/URLSessionWebSocket.swift +++ b/Sources/RealtimeV2/WebSocket/URLSessionWebSocket.swift @@ -1,5 +1,6 @@ import ConcurrencyExtras import Foundation +import IssueReporting #if canImport(FoundationNetworking) import FoundationNetworking @@ -70,7 +71,10 @@ final class URLSessionWebSocket: WebSocket { session: URLSession? = nil ) async throws -> URLSessionWebSocket { guard url.scheme == "ws" || url.scheme == "wss" else { - preconditionFailure("only ws: and wss: schemes are supported") + throw WebSocketError.connection( + message: "only ws: and wss: schemes are supported, got \(url.scheme ?? "no scheme").", + error: URLError(.unsupportedURL) + ) } struct MutableState { @@ -406,33 +410,76 @@ final class URLSessionWebSocket: WebSocket { return } - // Validate close code per RFC 6455 - if let code = code, code != 1000, !(code >= 3000 && code <= 4999) { - preconditionFailure( - "Invalid close code: \(code). Must be 1000 or in range 3000-4999" + let validatedCode = Self.validatedCloseCode(code) + if let code, validatedCode == nil { + reportIssue( + "Invalid close code \(code). Must be 1000 or in 3000...4999. Closing without a code." ) } - // Validate reason length per RFC 6455 - if let reason = reason, reason.utf8.count > 123 { - preconditionFailure("Close reason must be ≤ 123 bytes when UTF-8 encoded") + let validatedReason = Self.validatedCloseReason(reason) + if let reason, validatedReason != reason { + reportIssue( + "Close reason is \(reason.utf8.count) bytes, over the 123-byte limit. Truncating it." + ) + } + + // The two platforms disagree about what `CloseCode` accepts. On Darwin it is imported from + // Objective-C as a non-exhaustive `NS_ENUM`, so any `Int` round-trips and an application code + // like 4001 goes out as 4001. In swift-corelibs-foundation it is a plain Swift enum holding + // only the named cases, so everything in 3000...4999 converts to `nil` and cannot be sent. + // Close without a status there rather than substituting a different code — reporting 1000 + // ("normal closure") for what was meant to be an application error would misinform the peer. + let closeCode = validatedCode.flatMap(URLSessionWebSocketTask.CloseCode.init(rawValue:)) + if let validatedCode, closeCode == nil { + reportIssue( + """ + Close code \(validatedCode) is not representable by \ + `URLSessionWebSocketTask.CloseCode` on this platform. Closing without a code. + """ + ) } mutableState.withValue { guard !$0.isClosed else { return } - if let code = code { - let closeReason = reason ?? "" - _task.cancel( - with: URLSessionWebSocketTask.CloseCode(rawValue: code)!, - reason: Data(closeReason.utf8) - ) + if let closeCode { + _task.cancel(with: closeCode, reason: Data((validatedReason ?? "").utf8)) } else { _task.cancel() } } } + /// Returns `code` if RFC 6455 §7.4 allows an endpoint to send it, otherwise `nil`. + /// + /// Only 1000 and the application-defined range 3000...4999 may be sent. Anything else closes + /// without a code (the peer sees 1005) rather than trapping — ``close(code:reason:)`` is called + /// from user code and cannot throw. + /// + /// Pure by design: the caller reports the rejection. Driving `reportIssue` from a `@Test` + /// function segfaults under `xcodebuild test` (SDK-435), so keeping it out of here is what lets + /// this be tested directly on both runners. + static func validatedCloseCode(_ code: Int?) -> Int? { + guard let code else { return nil } + return code == 1000 || (3000...4999).contains(code) ? code : nil + } + + /// Returns `reason` truncated to the 123-byte close-frame payload limit of RFC 6455 §5.5. + /// + /// Truncation happens on whole characters, so the frame never carries a split UTF-8 scalar. + /// Pure for the same reason as ``validatedCloseCode(_:)``. + static func validatedCloseReason(_ reason: String?) -> String? { + guard let reason, reason.utf8.count > 123 else { return reason } + + var truncated = "" + for character in reason { + guard truncated.utf8.count + character.utf8.count <= 123 else { break } + truncated.append(character) + } + return truncated + } + /// The WebSocket subprotocol negotiated with the peer. /// /// Returns an empty string if no subprotocol was negotiated during the handshake. diff --git a/Sources/Storage/StorageApi.swift b/Sources/Storage/StorageApi.swift index d1cf2609c..11d85a374 100644 --- a/Sources/Storage/StorageApi.swift +++ b/Sources/Storage/StorageApi.swift @@ -12,6 +12,22 @@ import HTTPTypes /// requests. Each of the types above holds a ``StorageApi`` value and delegates to it rather than /// inheriting from it. struct StorageApi: Sendable { + /// The apex domains the storage hostname rewrite applies to, each carrying a leading dot so the + /// match has to land on a hostname-label boundary. + /// + /// Without the dot, any host merely *ending* in the apex matches: a caller-owned domain like + /// `not-supabase.co` would be rewritten to `not-storage.supabase.co`, pointing requests at a + /// domain the caller does not control. The bare apex `supabase.co` is excluded for the same + /// reason — it is not a project host. + private static let legacySupabaseHostSuffixes = [".supabase.co", ".supabase.in", ".supabase.red"] + + /// Reports whether `host` is a Supabase project host that has not already been pointed at + /// storage. + private static func isLegacySupabaseHost(_ host: String) -> Bool { + !host.contains("storage.supabase.") + && legacySupabaseHostSuffixes.contains(where: host.hasSuffix) + } + /// The configuration used to initialize this client instance. let configuration: StorageClientConfiguration @@ -30,23 +46,27 @@ struct StorageApi: Sendable { // if legacy uri is used, replace with new storage host (disables request buffering to allow > 50GB uploads) // "project-ref.supabase.co" becomes "project-ref.storage.supabase.co" if configuration.useNewHostname == true { + // `configuration.url` is supplied once, at construction, so a URL that cannot be decomposed + // into host components is a programmer error, not a runtime condition. Trap here, where the + // offending value is, rather than letting it fail later as an opaque `URLError`. guard var components = URLComponents(url: configuration.url, resolvingAgainstBaseURL: false), let host = components.host else { - fatalError("Client initialized with invalid URL: \(configuration.url)") + preconditionFailure("Storage client initialized with an invalid URL: \(configuration.url)") } - let regex = try! NSRegularExpression(pattern: "supabase.(co|in|red)$") + if Self.isLegacySupabaseHost(host) { + // Substitute on the same label boundary the check used, so a host that happens to start + // with `supabase.` is not rewritten at that leading position too. + components.host = host.replacingOccurrences(of: ".supabase.", with: ".storage.supabase.") - let isSupabaseHost = - regex.firstMatch(in: host, range: NSRange(location: 0, length: host.utf16.count)) != nil + guard let rewritten = components.url else { + preconditionFailure("Rewriting the storage host produced an invalid URL: \(components)") + } - if isSupabaseHost, !host.contains("storage.supabase.") { - components.host = host.replacingOccurrences(of: "supabase.", with: "storage.supabase.") + configuration.url = rewritten } - - configuration.url = components.url! } self.configuration = configuration diff --git a/Sources/Supabase/SupabaseClient.swift b/Sources/Supabase/SupabaseClient.swift index 099e0b600..833966fc4 100644 --- a/Sources/Supabase/SupabaseClient.swift +++ b/Sources/Supabase/SupabaseClient.swift @@ -67,6 +67,17 @@ import Logging /// span is active (via `opentelemetry-swift`) when a request is made gets propagated; with no /// active span, or with the trait disabled, requests go out unchanged. public final class SupabaseClient: Sendable { + /// Derives the default auth storage key from the project ref in `url`'s host, so two projects + /// in the same app do not share a stored session. + /// + /// - Returns: `nil` when `url` has no host, and so no project ref to namespace by. An empty + /// host counts as none: `"".split(separator: ".")` is empty. + static func defaultStorageKey(for url: URL) -> String? { + url.host(percentEncoded: false)? + .split(separator: ".").first + .map { "sb-\($0)-auth-token" } + } + let options: SupabaseClientOptions let supabaseURL: URL let supabaseKey: String @@ -234,11 +245,17 @@ public final class SupabaseClient: Sendable { ) .merging(with: HTTPFields(options.global.headers)) - // default storage key uses the supabase project ref as a namespace - guard let host = supabaseURL.host(percentEncoded: false) else { - preconditionFailure("supabaseURL must have a valid host.") + // The default storage key namespaces the stored session by project ref, taken from the URL's + // host. `supabaseURL` is supplied once, at construction, so a URL without a host is a + // programmer error rather than a runtime condition — trap on it, where the offending value is, + // instead of degrading into a shared storage key that silently collides across projects. + guard let defaultStorageKey = Self.defaultStorageKey(for: supabaseURL) else { + preconditionFailure( + """ + supabaseURL must have a host to derive the auth storage key from, got \(supabaseURL). + """ + ) } - let defaultStorageKey = "sb-\(host.split(separator: ".")[0])-auth-token" _auth = AuthClient( url: supabaseURL.appendingPathComponent("/auth/v1"), diff --git a/Supabase.xcworkspace/xcshareddata/swiftpm/Package.resolved b/Supabase.xcworkspace/xcshareddata/swiftpm/Package.resolved index 1ecda4ba4..74bd3a46b 100644 --- a/Supabase.xcworkspace/xcshareddata/swiftpm/Package.resolved +++ b/Supabase.xcworkspace/xcshareddata/swiftpm/Package.resolved @@ -1,5 +1,5 @@ { - "originHash" : "9d9f6b0fc1bc72b8bfc309542e450e769d50124b550f98600e5d16cb59f23a43", + "originHash" : "0861f6f9e0bb77dce0f0f7938312eeb8924f1ee11be72f64e0e8508438888c07", "pins" : [ { "identity" : "appauth-ios", @@ -150,8 +150,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/pointfreeco/swift-case-paths", "state" : { - "revision" : "19b7263bacb9751f151ec0c93ec816fe1ef67c7b", - "version" : "1.6.1" + "revision" : "1866d1046fd4e54d590af6be5b23de6ca3af2bf9", + "version" : "1.9.2" } }, { @@ -249,8 +249,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/pointfreeco/swift-snapshot-testing", "state" : { - "revision" : "b2d4cb30735f4fbc3a01963a9c658336dd21e9ba", - "version" : "1.18.1" + "revision" : "59a99c458de4d2dee580529b61b4f78dca7b7fa6", + "version" : "1.19.4" } }, { @@ -258,8 +258,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/swiftlang/swift-syntax", "state" : { - "revision" : "0687f71944021d616d34d922343dcef086855920", - "version" : "600.0.1" + "revision" : "79e4b74a295b6eb74a8b585e3a39d29e70c1dbd1", + "version" : "603.0.2" } }, { diff --git a/Tests/AuthTests/AuthErrorTests.swift b/Tests/AuthTests/AuthErrorTests.swift index 7456bc783..56dc93740 100644 --- a/Tests/AuthTests/AuthErrorTests.swift +++ b/Tests/AuthTests/AuthErrorTests.swift @@ -44,6 +44,11 @@ struct AuthErrorTests { let implicitGrantRedirect = AuthError.implicitGrantRedirect(message: "Implicit grant failure") #expect(implicitGrantRedirect.errorCode == .unknown) #expect(implicitGrantRedirect.message == "Implicit grant failure") + + let oauthFlowFailed = AuthError.oauthFlowFailed(message: "No redirect URL configured") + #expect(oauthFlowFailed.errorCode == .unknown) + #expect(oauthFlowFailed.message == "No redirect URL configured") + #expect(oauthFlowFailed.errorDescription == "No redirect URL configured") } @Test diff --git a/Tests/RealtimeTests/RealtimeClientOptionsTests.swift b/Tests/RealtimeTests/RealtimeClientOptionsTests.swift index 44ec67484..919b1c2ab 100644 --- a/Tests/RealtimeTests/RealtimeClientOptionsTests.swift +++ b/Tests/RealtimeTests/RealtimeClientOptionsTests.swift @@ -10,6 +10,18 @@ import Testing @Suite struct RealtimeClientOptionsTests { + @Test + func initializesWithoutAnAPIKeyInsteadOfTrapping() { + // The apikey rides along as a query item when present, and is simply absent when it is not. + // Constructing the client used to trap instead, taking the host app down over a header. + let client = RealtimeClientV2( + url: URL(string: "https://project-ref.supabase.co/realtime/v1")!, + options: RealtimeClientOptions(headers: [:]) + ) + + #expect(client.options.apikey == nil) + } + @Test func sessionDefaultsToNil() { let options = RealtimeClientOptions(headers: ["apikey": "test-key"]) diff --git a/Tests/RealtimeTests/URLSessionWebSocketCloseValidationTests.swift b/Tests/RealtimeTests/URLSessionWebSocketCloseValidationTests.swift new file mode 100644 index 000000000..e339b17d5 --- /dev/null +++ b/Tests/RealtimeTests/URLSessionWebSocketCloseValidationTests.swift @@ -0,0 +1,103 @@ +// +// URLSessionWebSocketCloseValidationTests.swift +// Supabase +// +// Created by Guilherme Souza on 09/09/26. +// + +import Foundation +import Testing + +@testable import RealtimeV2 + +#if canImport(FoundationNetworking) + // `URLSessionWebSocketTask` lives in `FoundationNetworking` on Linux, matching the guard in + // `URLSessionWebSocket.swift`. + import FoundationNetworking +#endif + +/// `close(code:reason:)` takes both values straight from user code, so neither may trap. +/// +/// The validators are pure — `close` reports the rejection — so these tests never drive +/// `reportIssue`, which segfaults under `xcodebuild test` (SDK-435). +@Suite +struct URLSessionWebSocketCloseValidationTests { + @Test(arguments: [1000, 3000, 4000, 4999]) + func acceptsCloseCodesRFC6455Allows(code: Int) { + #expect(URLSessionWebSocket.validatedCloseCode(code) == code) + } + + @Test + func passesNoCodeThrough() { + #expect(URLSessionWebSocket.validatedCloseCode(nil) == nil) + } + + @Test(arguments: [1001, 1005, 1006, 2999, 5000, 0, -1]) + func dropsCloseCodesRFC6455Forbids(code: Int) { + // Dropping the code closes without one (the peer sees 1005) rather than trapping. + #expect(URLSessionWebSocket.validatedCloseCode(code) == nil) + } + + @Test + func passesReasonWithinTheByteLimitThrough() { + let reason = String(repeating: "a", count: 123) + #expect(URLSessionWebSocket.validatedCloseReason(reason) == reason) + #expect(URLSessionWebSocket.validatedCloseReason(nil) == nil) + } + + @Test + func truncatesAnOverlongReasonToAPrefix() throws { + let reason = String(repeating: "a", count: 200) + let truncated = URLSessionWebSocket.validatedCloseReason(reason) + + #expect(truncated == String(repeating: "a", count: 123)) + #expect(reason.hasPrefix(try #require(truncated))) + } + + @Test + func truncatesOnWholeCharactersSoTheFrameNeverSplitsAScalar() throws { + // Each rocket is 4 UTF-8 bytes, so 30 fit within the 123-byte limit (120) and the 31st + // would overshoot it at 124. A byte-wise cut would split that 31st rocket, leaving a + // replacement character — the result would be neither this value nor a prefix of the original. + let reason = String(repeating: "🚀", count: 40) + let truncated = URLSessionWebSocket.validatedCloseReason(reason) + + #expect(truncated == String(repeating: "🚀", count: 30)) + #expect(try #require(truncated).utf8.count <= 123) + #expect(reason.hasPrefix(try #require(truncated))) + } + + /// 1000 is a named case everywhere, so it converts on every platform. + @Test + func normalClosureConvertsOnEveryPlatform() throws { + let converted = try #require(URLSessionWebSocketTask.CloseCode(rawValue: 1000)) + #expect(converted.rawValue == 1000) + } + + /// The application range RFC 6455 §7.4 permits is only actually *sendable* where + /// `URLSessionWebSocketTask.CloseCode` can represent it, and the platforms disagree. + /// + /// On Darwin `CloseCode` comes from Objective-C as a non-exhaustive `NS_ENUM`, so any `Int` + /// round-trips. In swift-corelibs-foundation it is a plain Swift enum with only the named + /// cases, so 3000...4999 convert to `nil`. ``URLSessionWebSocket/close(code:reason:)`` closes + /// without a status in that case instead of substituting a different code. + /// + /// Pinned because assuming the Darwin result held everywhere was wrong. + @Test(arguments: [3000, 4000, 4001, 4999]) + func applicationCloseCodesConvertOnlyWhereThePlatformRepresentsThem(code: Int) { + let converted = URLSessionWebSocketTask.CloseCode(rawValue: code) + + #if canImport(FoundationNetworking) + #expect(converted == nil) + #else + #expect(converted?.rawValue == code) + #endif + } + + @Test + func rejectsANonWebSocketSchemeInsteadOfTrapping() async { + await #expect(throws: WebSocketError.self) { + _ = try await URLSessionWebSocket.connect(to: URL(string: "https://example.com")!) + } + } +} diff --git a/Tests/StorageTests/StorageBucketAPITests.swift b/Tests/StorageTests/StorageBucketAPITests.swift index c0b918965..0abadffaf 100644 --- a/Tests/StorageTests/StorageBucketAPITests.swift +++ b/Tests/StorageTests/StorageBucketAPITests.swift @@ -82,6 +82,26 @@ extension StorageMockerTests { "http://localhost:1234/storage/v1", "support local host with port without modification" ), + ( + "https://blah.supabasexco/storage/v1", + "https://blah.supabasexco/storage/v1", + "not treat a non-dot separator as a platform host" + ), + ( + "https://supabase.co/storage/v1", + "https://supabase.co/storage/v1", + "not rewrite the bare apex domain, which is not a project host" + ), + ( + "https://not-supabase.co/storage/v1", + "https://not-supabase.co/storage/v1", + "not rewrite a host that only ends with the apex, without a label boundary" + ), + ( + "https://mysupabase.in/storage/v1", + "https://mysupabase.in/storage/v1", + "not rewrite a caller-owned domain ending in the apex" + ), ] ) func urlConstructionWithNewHostname(input: String, expected: String, description: String) { diff --git a/Tests/SupabaseTests/SupabaseClientStorageKeyTests.swift b/Tests/SupabaseTests/SupabaseClientStorageKeyTests.swift new file mode 100644 index 000000000..22de8920e --- /dev/null +++ b/Tests/SupabaseTests/SupabaseClientStorageKeyTests.swift @@ -0,0 +1,65 @@ +// +// SupabaseClientStorageKeyTests.swift +// Supabase +// +// Created by Guilherme Souza on 09/09/26. +// + +import Foundation +import Testing + +@testable import Auth +@testable import Supabase + +/// The default auth storage key namespaces the stored session by project ref, so two projects in +/// the same app do not share a session. +/// +/// A `supabaseURL` with no host is a construction-time programmer error and traps, so that case is +/// covered here at the derivation helper rather than by constructing a client. +@Suite +struct SupabaseClientStorageKeyTests { + @Test( + arguments: [ + ("https://project-ref.supabase.co", "sb-project-ref-auth-token"), + ("https://project-ref.supabase.co/rest/v1", "sb-project-ref-auth-token"), + ("http://localhost:54321", "sb-localhost-auth-token"), + ] + ) + func derivesTheStorageKeyFromTheProjectRef(url: String, expected: String) throws { + #expect(SupabaseClient.defaultStorageKey(for: try #require(URL(string: url))) == expected) + } + + // `"".split(separator: ".")` is empty, so an empty host yields `nil` rather than reading past + // the end of the array — which is what the old `host.split(separator: ".")[0]` did. + @Test(arguments: ["https:///rest/v1", "mailto:someone@example.com"]) + func derivesNoStorageKeyWhenTheURLHasNoHost(url: String) throws { + #expect(SupabaseClient.defaultStorageKey(for: try #require(URL(string: url))) == nil) + } + + // `storage` is passed explicitly, and `options` is never omitted: on Linux and Android + // `AuthOptions.init` has no default storage, and the two-argument `SupabaseClient.init` does + // not exist there at all. + @Test + func honorsAnExplicitStorageKeyOverTheProjectRef() { + let client = SupabaseClient( + supabaseURL: URL(string: "https://project-ref.supabase.co")!, + supabaseKey: "test-key", + options: SupabaseClientOptions( + auth: .init(storage: AuthLocalStorageMock(), storageKey: "my-key") + ) + ) + + #expect(client.auth.configuration.storageKey == "my-key") + } + + @Test + func usesTheDerivedKeyWhenNoneIsGiven() { + let client = SupabaseClient( + supabaseURL: URL(string: "https://project-ref.supabase.co")!, + supabaseKey: "test-key", + options: SupabaseClientOptions(auth: .init(storage: AuthLocalStorageMock())) + ) + + #expect(client.auth.configuration.storageKey == "sb-project-ref-auth-token") + } +} diff --git a/V3_MIGRATION.md b/V3_MIGRATION.md index 2cb6b6582..6b099523b 100644 --- a/V3_MIGRATION.md +++ b/V3_MIGRATION.md @@ -176,8 +176,11 @@ message) is now mandatory. This is a compile error everywhere: the old symbols n | `UserAttributes.emailChangeToken` | *(removed, no replacement — was unused by GoTrue)* | Also removed, with no replacement, because they no longer represent something GoTrue can throw: -`AuthError.missingExpClaim`, `AuthError.malformedJWT`, `AuthError.missingURL`, -`AuthError.invalidRedirectScheme`. +`AuthError.missingExpClaim`, `AuthError.malformedJWT`, `AuthError.missingURL`. + +`AuthError.invalidRedirectScheme` was removed on the same grounds, but has since come back as +`AuthError.oauthFlowFailed(message:)` — the condition is client-side, not something GoTrue throws. +See "`AuthError` gains `oauthFlowFailed(message:)`" below. `UserCredentials` was deprecated ("access will be removed on the next major release") and is now internal — it was only ever used by `AuthClient` itself to encode the request body for @@ -1742,3 +1745,54 @@ Read a `!` on this change as "the guarantee moved", not "your build breaks". Nothing is withdrawn today, so there is no escape hatch to reach for. When the deprecation warnings do arrive, the replacement is the typed API — `client.schema("public").from(Todo.self)` and the `@Table` macro — not a different spelling of the same builder. + +## `AuthError` gains `oauthFlowFailed(message:)`; client-side OAuth failures throw instead of trapping + +`AuthError` has a new case: + +```swift +case oauthFlowFailed(message: String) +``` + +It covers OAuth failures that happen entirely on the client, before any request reaches GoTrue. +Two call sites in `signInWithOAuth(provider:redirectTo:scopes:queryParams:configure:)` (the +`ASWebAuthenticationSession` overload) used to end the process instead of throwing: + +| Condition | Before | After | +| --- | --- | --- | +| No redirect URL with a scheme, from either `redirectTo` or `AuthClient.Configuration.redirectToURL` | `preconditionFailure` | `throws AuthError.oauthFlowFailed(message:)` | +| `ASWebAuthenticationSession` reports neither a URL nor an error | `fatalError` | `reportIssue`, then `throws AuthError.oauthFlowFailed(message:)` | + +The redirect URL is read per call: `redirectTo` is a parameter of the sign-in method, falling back +to `AuthClient.Configuration.redirectToURL`. A value that varies per call is not something to trap +on, and the enclosing method already throws, so an error costs nothing. (A value fixed once at +construction is different — those still trap. See "When trapping is allowed" in `AGENTS.md`.) + +This also restores something v3 dropped. `AuthError.invalidRedirectScheme` existed in v2 and was +listed above as removed with no replacement, on the grounds that it no longer represented anything +GoTrue could throw. That was right about the server and wrong about the client: the condition is +still real, it just belongs to the SDK rather than the API. Read that row as: + +| Before | After | +| --- | --- | +| `AuthError.invalidRedirectScheme` | `AuthError.oauthFlowFailed(message:)` | + +```swift +// Before — no way to handle this; the app died on the missing redirect URL +let session = try await supabase.auth.signInWithOAuth(provider: .github) + +// After +do { + let session = try await supabase.auth.signInWithOAuth(provider: .github) +} catch let AuthError.oauthFlowFailed(message) { + presentSetupError(message) +} +``` + +**This is a compile error only if you switch exhaustively over `AuthError`.** `AuthError` is a +public non-frozen enum, so a `switch` without a `default` stops compiling until you add the new +case; the compiler lists every such site. Code that catches with `catch let error as AuthError` or +reads `error.message` / `error.errorCode` is unaffected. + +`errorCode` for the new case is `.unknown`, matching the other client-side cases +(`pkceGrantCodeExchange`, `implicitGrantRedirect`). diff --git a/sdk-compliance.yaml b/sdk-compliance.yaml index 348254a5c..7f1273495 100644 --- a/sdk-compliance.yaml +++ b/sdk-compliance.yaml @@ -273,6 +273,7 @@ features: - Provider.workos - Provider.zoom - Provider.fly + - AuthError.oauthFlowFailed auth.sign_in.sign_in_with_otp: status: implemented symbols: