Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 34 additions & 0 deletions Sources/Panels/BrowserWebAuthnFallbackPresentationWindow.swift
Original file line number Diff line number Diff line change
@@ -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")
}
}
45 changes: 44 additions & 1 deletion Sources/Panels/BrowserWebAuthnSupport.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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()
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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):
Expand All @@ -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
Expand Down
4 changes: 4 additions & 0 deletions cmux.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -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 */; };
Expand Down Expand Up @@ -3150,6 +3151,7 @@ C0DE71B10000000000000002 /* AppDelegate+AgentChatNotifications.swift */ = {isa =
A50100000000000000000015 /* BrowserWebAuthnCreationRequest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Panels/BrowserWebAuthnCreationRequest.swift; sourceTree = "<group>"; };
A5010000000000000000000F /* BrowserWebAuthnCredentialDescriptor.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Panels/BrowserWebAuthnCredentialDescriptor.swift; sourceTree = "<group>"; };
A50100000000000000000011 /* BrowserWebAuthnCredentialParameter.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Panels/BrowserWebAuthnCredentialParameter.swift; sourceTree = "<group>"; };
A75030010000000000000002 /* BrowserWebAuthnFallbackPresentationWindow.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Panels/BrowserWebAuthnFallbackPresentationWindow.swift; sourceTree = "<group>"; };
A50100000000000000000017 /* BrowserWebAuthnMessageEnvelope.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Panels/BrowserWebAuthnMessageEnvelope.swift; sourceTree = "<group>"; };
A50100000000000000000019 /* BrowserWebAuthnRelyingPartyDescriptor.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Panels/BrowserWebAuthnRelyingPartyDescriptor.swift; sourceTree = "<group>"; };
A5007427 /* BrowserWebAuthnRequestParser.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Panels/BrowserWebAuthnRequestParser.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -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 */,
Expand Down Expand Up @@ -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 */,
Expand Down
69 changes: 69 additions & 0 deletions cmuxTests/BrowserWebContentProcessTests.swift
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import AppKit
import AuthenticationServices
import CMUXAuthCore
import CmuxAuthRuntime
import CmuxBrowser
Expand Down Expand Up @@ -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()
Expand Down
3 changes: 3 additions & 0 deletions scripts/lint_auxiliary_window_close_shortcuts.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
27 changes: 27 additions & 0 deletions tests/test_ci_auxiliary_window_close_shortcuts.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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<String> = [
"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'
Expand Down
54 changes: 54 additions & 0 deletions verification.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
<!doctype html>
<html lang="en">
<meta charset="utf-8">
<meta name="viewport" content="width=device-width, initial-scale=1">
<title>Issue 7503 verification</title>
<style>
:root { color-scheme: light dark; font: 15px/1.38 system-ui, sans-serif; }
body { margin: 0; background: Canvas; color: CanvasText; }
main { max-width: 1050px; margin: 0 auto; padding: 24px; }
h1 { margin: 0 0 10px; font-size: 24px; }
p { margin: 7px 0; }
.claim { padding: 10px 12px; border-left: 4px solid #e58b20; background: color-mix(in srgb, #e58b20 12%, Canvas); }
ol { margin: 14px 0 0; padding-left: 24px; display: grid; gap: 10px; }
li > strong { display: block; }
pre { margin: 5px 0; padding: 8px 10px; overflow-x: auto; border-radius: 6px; background: color-mix(in srgb, CanvasText 8%, Canvas); font: 12px/1.35 ui-monospace, monospace; }
.verdict { margin-top: 14px; padding-top: 10px; border-top: 1px solid color-mix(in srgb, CanvasText 20%, Canvas); }
</style>
<main>
<h1>Human verification for #7503</h1>
<p>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.</p>
<p class="claim"><strong>Unverified claim:</strong> 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.</p>
<ol>
<li>
<strong>Build and launch the isolated app; verify its socket.</strong>
<pre>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</pre>
Correct: the command lists the tagged app’s workspaces. Bug/blocker: the socket is absent or the command targets a different build.
</li>
<li>
<strong>Capture a quiet WindowServer baseline and save it.</strong>
<pre>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</pre>
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.
</li>
<li>
<strong>Run real WebAuthn ceremonies before and between every lifecycle cycle.</strong>
In a cmux browser pane, open <code>https://webauthn.io</code>, enter a unique username, click <strong>Register</strong>, and complete the macOS passkey sheet. Click <strong>Authenticate</strong> 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 <code>/tmp/sym7503-windows-after-N.txt</code>.
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.
</li>
<li>
<strong>Check click routing, then clean up.</strong>
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.
<pre>pkill -f 'DerivedData/cmux-sym7503'</pre>
</li>
</ol>
<p class="verdict"><strong>The fix is disproven by:</strong> 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.</p>
</main>
</html>