diff --git a/Sources/Panels/BrowserWebAuthnFallbackPresentationWindow.swift b/Sources/Panels/BrowserWebAuthnFallbackPresentationWindow.swift new file mode 100644 index 000000000000..5d219823cb92 --- /dev/null +++ b/Sources/Panels/BrowserWebAuthnFallbackPresentationWindow.swift @@ -0,0 +1,34 @@ +import AppKit + +@MainActor +final class BrowserWebAuthnFallbackPresentationWindow: NSPanel { + override var canBecomeKey: Bool { false } + override var canBecomeMain: Bool { false } + + init() { + super.init( + contentRect: NSRect(x: -10_000, y: -10_000, width: 1, height: 1), + styleMask: [.borderless, .nonactivatingPanel], + backing: .buffered, + defer: false + ) + self.identifier = NSUserInterfaceItemIdentifier("cmux.browserWebAuthnFallbackPresentation") + isReleasedWhenClosed = false + isRestorable = false + isExcludedFromWindowsMenu = true + collectionBehavior = [.transient, .ignoresCycle, .stationary] + level = .normal + isOpaque = false + backgroundColor = .clear + alphaValue = 0 + hasShadow = false + ignoresMouseEvents = true + animationBehavior = .none + orderOut(nil) + } + + @available(*, unavailable) + required init?(coder: NSCoder) { + fatalError("init(coder:) has not been implemented") + } +} diff --git a/Sources/Panels/BrowserWebAuthnSupport.swift b/Sources/Panels/BrowserWebAuthnSupport.swift index 19370dd060b1..d1127c4ac6a4 100644 --- a/Sources/Panels/BrowserWebAuthnSupport.swift +++ b/Sources/Panels/BrowserWebAuthnSupport.swift @@ -1006,14 +1006,33 @@ private final class BrowserPasskeyAuthorizationGate { @MainActor final class BrowserWebAuthnCoordinator: NSObject, WKScriptMessageHandlerWithReply { private weak var installedWebView: WKWebView? + private let existingPresentationWindowProvider: () -> NSWindow? private var activeAuthorizationController: ASAuthorizationController? private var activeAuthorizationContinuation: CheckedContinuation<[String: Any], Error>? private var activePresentationWindow: NSWindow? + private var fallbackPresentationWindow: BrowserWebAuthnFallbackPresentationWindow? override init() { + existingPresentationWindowProvider = { + NSApp.keyWindow ?? NSApp.mainWindow + } super.init() } + init(existingPresentationWindowProvider: @escaping () -> NSWindow?) { + self.existingPresentationWindowProvider = existingPresentationWindowProvider + super.init() + } + + deinit { + let window = fallbackPresentationWindow + // Swift 5 deinitializers are nonisolated; keep AppKit cleanup on its owner. + Task { @MainActor in + window?.orderOut(nil) + window?.close() + } + } + func install(on webView: WKWebView) { let controller = webView.configuration.userContentController controller.removeScriptMessageHandler( @@ -1053,6 +1072,7 @@ final class BrowserWebAuthnCoordinator: NSObject, WKScriptMessageHandlerWithRepl activePresentationWindow = nil controller?.delegate = nil controller?.presentationContextProvider = nil + retireFallbackPresentationWindow() if #available(macOS 13.0, *) { controller?.cancel() } @@ -1170,7 +1190,9 @@ extension BrowserWebAuthnCoordinator: ASAuthorizationControllerDelegate, ASAutho } func presentationAnchor(for controller: ASAuthorizationController) -> ASPresentationAnchor { - let anchor = activePresentationWindow ?? NSApp.keyWindow ?? NSApp.mainWindow ?? NSWindow() + let anchor = activePresentationWindow + ?? existingPresentationWindowProvider() + ?? reusableFallbackPresentationWindow() #if DEBUG cmuxDebugLog("webauthn.asAuth.presentationAnchor hasTitle=\(anchor.title.isEmpty ? 0 : 1) isVisible=\(anchor.isVisible) isKey=\(anchor.isKeyWindow)") #endif @@ -1387,10 +1409,14 @@ private extension BrowserWebAuthnCoordinator { } func finishAuthorization(with result: Result<[String: Any], Error>) { + let controller = activeAuthorizationController let continuation = activeAuthorizationContinuation activeAuthorizationController = nil activeAuthorizationContinuation = nil activePresentationWindow = nil + controller?.delegate = nil + controller?.presentationContextProvider = nil + retireFallbackPresentationWindow() switch result { case .success(let reply): @@ -1400,6 +1426,23 @@ private extension BrowserWebAuthnCoordinator { } } + func reusableFallbackPresentationWindow() -> NSWindow { + if let fallbackPresentationWindow { + return fallbackPresentationWindow + } + + let window = BrowserWebAuthnFallbackPresentationWindow() + fallbackPresentationWindow = window + return window + } + + func retireFallbackPresentationWindow() { + guard let window = fallbackPresentationWindow else { return } + fallbackPresentationWindow = nil + window.orderOut(nil) + window.close() + } + func buildCreationPlan( _ request: BrowserWebAuthnCreationRequest, clientDataContext: BrowserWebAuthnClientDataContext diff --git a/cmux.xcodeproj/project.pbxproj b/cmux.xcodeproj/project.pbxproj index 510fbedf74bd..ee9189bce5a7 100644 --- a/cmux.xcodeproj/project.pbxproj +++ b/cmux.xcodeproj/project.pbxproj @@ -473,6 +473,7 @@ C0DE71B10000000000000001 /* AppDelegate+AgentChatNotifications.swift in Sources A50100000000000000000016 /* BrowserWebAuthnCreationRequest.swift in Sources */ = {isa = PBXBuildFile; fileRef = A50100000000000000000015 /* BrowserWebAuthnCreationRequest.swift */; }; A50100000000000000000010 /* BrowserWebAuthnCredentialDescriptor.swift in Sources */ = {isa = PBXBuildFile; fileRef = A5010000000000000000000F /* BrowserWebAuthnCredentialDescriptor.swift */; }; A50100000000000000000012 /* BrowserWebAuthnCredentialParameter.swift in Sources */ = {isa = PBXBuildFile; fileRef = A50100000000000000000011 /* BrowserWebAuthnCredentialParameter.swift */; }; + A75030010000000000000001 /* BrowserWebAuthnFallbackPresentationWindow.swift in Sources */ = {isa = PBXBuildFile; fileRef = A75030010000000000000002 /* BrowserWebAuthnFallbackPresentationWindow.swift */; }; A50100000000000000000018 /* BrowserWebAuthnMessageEnvelope.swift in Sources */ = {isa = PBXBuildFile; fileRef = A50100000000000000000017 /* BrowserWebAuthnMessageEnvelope.swift */; }; A5010000000000000000001A /* BrowserWebAuthnRelyingPartyDescriptor.swift in Sources */ = {isa = PBXBuildFile; fileRef = A50100000000000000000019 /* BrowserWebAuthnRelyingPartyDescriptor.swift */; }; A5007426 /* BrowserWebAuthnRequestParser.swift in Sources */ = {isa = PBXBuildFile; fileRef = A5007427 /* BrowserWebAuthnRequestParser.swift */; }; @@ -3150,6 +3151,7 @@ C0DE71B10000000000000002 /* AppDelegate+AgentChatNotifications.swift */ = {isa = A50100000000000000000015 /* BrowserWebAuthnCreationRequest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Panels/BrowserWebAuthnCreationRequest.swift; sourceTree = ""; }; A5010000000000000000000F /* BrowserWebAuthnCredentialDescriptor.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Panels/BrowserWebAuthnCredentialDescriptor.swift; sourceTree = ""; }; A50100000000000000000011 /* BrowserWebAuthnCredentialParameter.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Panels/BrowserWebAuthnCredentialParameter.swift; sourceTree = ""; }; + A75030010000000000000002 /* BrowserWebAuthnFallbackPresentationWindow.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Panels/BrowserWebAuthnFallbackPresentationWindow.swift; sourceTree = ""; }; A50100000000000000000017 /* BrowserWebAuthnMessageEnvelope.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Panels/BrowserWebAuthnMessageEnvelope.swift; sourceTree = ""; }; A50100000000000000000019 /* BrowserWebAuthnRelyingPartyDescriptor.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Panels/BrowserWebAuthnRelyingPartyDescriptor.swift; sourceTree = ""; }; A5007427 /* BrowserWebAuthnRequestParser.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Panels/BrowserWebAuthnRequestParser.swift; sourceTree = ""; }; @@ -6733,6 +6735,7 @@ C0DE71B10000000000000002 /* AppDelegate+AgentChatNotifications.swift */ = {isa = A5007425 /* BrowserWebAuthnSecurityOrigin.swift */, A5007427 /* BrowserWebAuthnRequestParser.swift */, A5010000000000000000001B /* BrowserWebAuthnStringValidation.swift */, + A75030010000000000000002 /* BrowserWebAuthnFallbackPresentationWindow.swift */, A5007423 /* BrowserWebAuthnSupport.swift */, A5010000000000000000001D /* BrowserWebAuthnTransport.swift */, A5010000000000000000001F /* BrowserWebAuthnTransportSummary.swift */, @@ -8758,6 +8761,7 @@ C0DE71B10000000000000002 /* AppDelegate+AgentChatNotifications.swift */ = {isa = A50100000000000000000016 /* BrowserWebAuthnCreationRequest.swift in Sources */, A50100000000000000000010 /* BrowserWebAuthnCredentialDescriptor.swift in Sources */, A50100000000000000000012 /* BrowserWebAuthnCredentialParameter.swift in Sources */, + A75030010000000000000001 /* BrowserWebAuthnFallbackPresentationWindow.swift in Sources */, A50100000000000000000018 /* BrowserWebAuthnMessageEnvelope.swift in Sources */, A5010000000000000000001A /* BrowserWebAuthnRelyingPartyDescriptor.swift in Sources */, A5007426 /* BrowserWebAuthnRequestParser.swift in Sources */, diff --git a/cmuxTests/BrowserWebContentProcessTests.swift b/cmuxTests/BrowserWebContentProcessTests.swift index bad8e3066cc5..cf8e2394b999 100644 --- a/cmuxTests/BrowserWebContentProcessTests.swift +++ b/cmuxTests/BrowserWebContentProcessTests.swift @@ -1,3 +1,5 @@ +import AppKit +import AuthenticationServices import CMUXAuthCore import CmuxAuthRuntime import CmuxBrowser @@ -955,6 +957,73 @@ struct BrowserWebContentProcessTests { #expect(result?["handler"] == false) } + @Test + func webAuthnPresentationAnchorDoesNotAccumulateFallbackWindows() async { + weak var retiredCoordinator: BrowserWebAuthnCoordinator? + weak var delegateRetiredAnchor: NSWindow? + weak var teardownRetiredAnchor: NSWindow? + weak var deinitRetiredAnchor: NSWindow? + + autoreleasepool { + let coordinator = BrowserWebAuthnCoordinator( + existingPresentationWindowProvider: { nil } + ) + retiredCoordinator = coordinator + let authorizationRequest = ASAuthorizationAppleIDProvider().createRequest() + let controller = ASAuthorizationController(authorizationRequests: [authorizationRequest]) + + autoreleasepool { + let anchors = (0..<3).map { _ in + coordinator.presentationAnchor(for: controller) + } + #expect(Set(anchors.map(ObjectIdentifier.init)).count == 1) + #expect(anchors.first?.isVisible == false) + #expect(anchors.first?.canBecomeKey == false) + #expect(anchors.first?.ignoresMouseEvents == true) + #expect(anchors.first?.isExcludedFromWindowsMenu == true) + + delegateRetiredAnchor = anchors.first + } + + #expect(delegateRetiredAnchor != nil) + autoreleasepool { + coordinator.authorizationController( + controller: controller, + didCompleteWithError: NSError( + domain: "BrowserWebContentProcessTests", + code: 1 + ) + ) + } + #expect(delegateRetiredAnchor == nil) + + let webView = WKWebView(frame: .zero, configuration: WKWebViewConfiguration()) + coordinator.install(on: webView) + autoreleasepool { + let anchor = coordinator.presentationAnchor(for: controller) + teardownRetiredAnchor = anchor + } + + #expect(teardownRetiredAnchor != nil) + autoreleasepool { + coordinator.tearDown(from: webView) + } + #expect(teardownRetiredAnchor == nil) + + autoreleasepool { + let anchor = coordinator.presentationAnchor(for: controller) + deinitRetiredAnchor = anchor + } + #expect(deinitRetiredAnchor != nil) + } + + #expect(retiredCoordinator == nil) + for _ in 0..<10 where deinitRetiredAnchor != nil { + await Task.yield() + } + #expect(deinitRetiredAnchor == nil) + } + @Test func webAuthnPageBridgeRelaysCredentialGetThroughContentWorldHandler() async throws { let configuration = WKWebViewConfiguration() diff --git a/scripts/lint_auxiliary_window_close_shortcuts.py b/scripts/lint_auxiliary_window_close_shortcuts.py index 63341df69f39..b3a533dccfb2 100755 --- a/scripts/lint_auxiliary_window_close_shortcuts.py +++ b/scripts/lint_auxiliary_window_close_shortcuts.py @@ -17,6 +17,9 @@ # main window. Add to this set only when a window is intentionally not user # closable. IGNORED_IDENTIFIERS = { + # Hidden AuthenticationServices presentation fallback; it never becomes + # key/main and must not take Cmd+W away from the active user window. + "cmux.browserWebAuthnFallbackPresentation", # Hidden WebKit preload host; it is not user closable and must not own Cmd+W. "cmux.browserBackgroundPreload", # Hidden WebKit hover-prewarm host; it is not user closable and must not own Cmd+W. diff --git a/tests/test_ci_auxiliary_window_close_shortcuts.sh b/tests/test_ci_auxiliary_window_close_shortcuts.sh index 4aacd128977e..2a95e84e20e7 100755 --- a/tests/test_ci_auxiliary_window_close_shortcuts.sh +++ b/tests/test_ci_auxiliary_window_close_shortcuts.sh @@ -97,6 +97,33 @@ SWIFT python3 scripts/lint_auxiliary_window_close_shortcuts.py --repo-root "$TMP_DIR" +# A bare identifier assignment is ambiguous without Swift type information. +# In particular, NSView also inherits an identifier property, so the lint must +# ignore the bare form instead of treating every view identifier as a window. +cat > "$TMP_DIR/Sources/cmuxApp.swift" <<'SWIFT' +private let cmuxAuxiliaryWindowIdentifiers: Set = [ + "cmux.settings", +] +SWIFT + +cat > "$TMP_DIR/Sources/NewWindow.swift" <<'SWIFT' +import AppKit + +final class IdentifiedView: NSView { + init() { + super.init(frame: .zero) + identifier = NSUserInterfaceItemIdentifier("cmux.viewIdentifier") + } + + @available(*, unavailable) + required init?(coder: NSCoder) { + fatalError("init(coder:) has not been implemented") + } +} +SWIFT + +python3 scripts/lint_auxiliary_window_close_shortcuts.py --repo-root "$TMP_DIR" + # Identifier assigned through a named constant (the MobilePairingWindowController # pattern) must be resolved and enforced, not silently skipped. cat > "$TMP_DIR/Sources/cmuxApp.swift" <<'SWIFT' diff --git a/verification.html b/verification.html new file mode 100644 index 000000000000..804e64d71137 --- /dev/null +++ b/verification.html @@ -0,0 +1,54 @@ + + + + +Issue 7503 verification + +
+

Human verification for #7503

+

WebAuthn now reuses one hidden, noninteractive, non-restorable fallback panel instead of returning a fresh default window for every presentation callback. The fallback is retired when authorization finishes, the browser tears down, or its coordinator is destroyed.

+

Unverified claim: after hours or days of real use—including sleep/wake and external-display reconnects—the tagged build no longer accumulates the reporter’s untitled WindowServer entries or level-20 click-intercepting dead zones.

+
    +
  1. + Build and launch the isolated app; verify its socket. + 
    if [[ -x ./scripts/reload-cloud.sh ]]; then
+  ./scripts/reload-cloud.sh --tag sym7503
+else
+  ./scripts/reload.sh --tag sym7503
+fi
+open -b com.cmuxterm.app.debug.sym7503
+CMUX_TAG=sym7503 scripts/cmux-debug-cli.sh list-workspaces
    + Correct: the command lists the tagged app’s workspaces. Bug/blocker: the socket is absent or the command targets a different build. +
    2. +
  2. + Capture a quiet WindowServer baseline and save it. + 
    CMUX_PID="$(pgrep -n -f 'DerivedData/cmux-sym7503.*cmux DEV sym7503.app')"
+xcrun swift -e 'import CoreGraphics; import Foundation; let p=Int32(CommandLine.arguments[1])!; let a=CGWindowListCopyWindowInfo([.optionAll],kCGNullWindowID) as! [[String:Any]]; for w in a where (w[kCGWindowOwnerPID as String] as? NSNumber)?.int32Value == p { print("id=\(w[kCGWindowNumber as String] ?? -1) layer=\(w[kCGWindowLayer as String] ?? -1) alpha=\(w[kCGWindowAlpha as String] ?? -1) onscreen=\(w[kCGWindowIsOnscreen as String] ?? false) title=\(w[kCGWindowName as String] ?? "") bounds=\(w[kCGWindowBounds as String] ?? [:])") }' "$CMUX_PID" | tee /tmp/sym7503-windows-before.txt
    + Correct: only intentional windows appear; record the count. The old bug included hidden untitled 800×600 windows and opaque on-screen untitled level-20 rectangles. +
    3. +
  3. + Run real WebAuthn ceremonies before and between every lifecycle cycle. + In a cmux browser pane, open https://webauthn.io, enter a unique username, click Register, and complete the macOS passkey sheet. Click Authenticate ten times, completing some sheets and canceling others. Repeat those authentications before and after each of these cycles: create/close many browser and terminal panes while hovering sidebar controls, sleep/wake the Mac, and disconnect/reconnect an external display. Close and reopen the WebAuthn browser pane between some runs so browser teardown is exercised. After each quiet period, re-run the step-2 dump to /tmp/sym7503-windows-after-N.txt. + Correct: every WebAuthn sheet completes or cancels normally, transient entries disappear, and the quiet count returns to the intentional-window baseline. Bug: a ceremony hangs or untitled entries grow monotonically, especially level 20 at full alpha or hidden 800×600/display-strip windows. +
    4. +
  4. + Check click routing, then clean up. + Put another app beneath every suspicious rectangle from the dump and click it. + Correct: the underlying app receives the click; cmux does not activate. Bug: a visually empty rectangle intercepts the click and activates cmux. + 
    pkill -f 'DerivedData/cmux-sym7503'
    +
    5. +
+

The fix is disproven by: any repeatable growth in quiet untitled-window count, any surviving opaque on-screen level-20 rectangle without visible cmux UI, or any such rectangle stealing a click from another app. Attach the before/after dumps and the exact hover/display cycle.

+
+