From 61e6a0e2b91c59c15a4cafb88012712b6eff2677 Mon Sep 17 00:00:00 2001 From: Austin Wang Date: Wed, 25 Mar 2026 19:12:24 -0700 Subject: [PATCH] Fix Release-only Cmd+N workspace snapshot UAF (#2181) * test: reproduce Cmd+N snapshot workspace lifetime race * fix: retain snapshot workspaces through Cmd+N creation * fix: repair workspace lifetime regression test --- Sources/TabManager.swift | 176 ++++++++++++++++------------- cmuxTests/WorkspaceUnitTests.swift | 47 ++++++++ 2 files changed, 144 insertions(+), 79 deletions(-) diff --git a/Sources/TabManager.swift b/Sources/TabManager.swift index 7d59d43d..52f69d18 100644 --- a/Sources/TabManager.swift +++ b/Sources/TabManager.swift @@ -1207,87 +1207,97 @@ class TabManager: ObservableObject { placementOverride: NewWorkspacePlacement? = nil, autoWelcomeIfNeeded: Bool = true ) -> Workspace { - // Snapshot current published state once so workspace creation doesn't repeatedly - // bounce through Combine-backed accessors while we're preparing the new workspace. - let snapshot = workspaceCreationSnapshot() - didCaptureWorkspaceCreationSnapshot() -#if DEBUG - maybeMutateSelectionDuringWorkspaceCreationForDev(snapshot: snapshot) -#endif - let nextTabCount = snapshot.tabs.count + 1 - sentryBreadcrumb("workspace.create", data: ["tabCount": nextTabCount]) - let explicitWorkingDirectory = normalizedWorkingDirectory(overrideWorkingDirectory) - let workingDirectory = explicitWorkingDirectory ?? snapshot.preferredWorkingDirectory - let inheritedConfig = workspaceCreationConfigTemplate( - inheritedTerminalFontPoints: snapshot.inheritedTerminalFontPoints - ) - // Resolve placement against the pre-creation snapshot before Workspace init - // boots terminal state. The ssh/new-workspace path can otherwise crash while - // reading @Published placement state from existing workspaces mid-creation. - let insertIndex = newTabInsertIndex(snapshot: snapshot, placementOverride: placementOverride) - let ordinal = Self.nextPortOrdinal - Self.nextPortOrdinal += 1 - let newWorkspace = makeWorkspaceForCreation( - title: "Terminal \(nextTabCount)", - workingDirectory: workingDirectory, - portOrdinal: ordinal, - configTemplate: inheritedConfig, - initialTerminalCommand: initialTerminalCommand, - initialTerminalEnvironment: initialTerminalEnvironment - ) - newWorkspace.owningTabManager = self - wireClosedBrowserTracking(for: newWorkspace) - if eagerLoadTerminal && !select { - requestBackgroundWorkspaceLoad(for: newWorkspace.id) - } - // Apply insertion to the current live array so post-snapshot closes/reorders - // are preserved instead of reintroducing stale workspace instances. - var updatedTabs = tabs - if insertIndex >= 0 && insertIndex <= updatedTabs.count { - updatedTabs.insert(newWorkspace, at: insertIndex) - } else { - updatedTabs.append(newWorkspace) - } - tabs = updatedTabs - if let explicitWorkingDirectory, - let terminalPanel = newWorkspace.focusedTerminalPanel { - scheduleInitialWorkspaceGitMetadataRefresh( - workspaceId: newWorkspace.id, - panelId: terminalPanel.id, - directory: explicitWorkingDirectory + let capturedTabs = tabs + let capturedSelectedTabId = selectedTabId + // Keep the pre-creation workspace array alive for the full Cmd+N path. Release ARC + // can otherwise drop intermediate retains before we re-read `tabs` for insertion, + // which turns mid-creation closes into use-after-free crashes in `swift_retain`. + return withExtendedLifetime(capturedTabs) { + // Snapshot current published state once so workspace creation doesn't repeatedly + // bounce through Combine-backed accessors while we're preparing the new workspace. + let snapshot = workspaceCreationSnapshot( + currentTabs: capturedTabs, + currentSelectedTabId: capturedSelectedTabId ) - } - if eagerLoadTerminal { - if select { - newWorkspace.focusedTerminalPanel?.surface.requestBackgroundSurfaceStartIfNeeded() + didCaptureWorkspaceCreationSnapshot() +#if DEBUG + maybeMutateSelectionDuringWorkspaceCreationForDev(snapshot: snapshot) +#endif + let nextTabCount = snapshot.tabs.count + 1 + sentryBreadcrumb("workspace.create", data: ["tabCount": nextTabCount]) + let explicitWorkingDirectory = normalizedWorkingDirectory(overrideWorkingDirectory) + let workingDirectory = explicitWorkingDirectory ?? snapshot.preferredWorkingDirectory + let inheritedConfig = workspaceCreationConfigTemplate( + inheritedTerminalFontPoints: snapshot.inheritedTerminalFontPoints + ) + // Resolve placement against the pre-creation snapshot before Workspace init + // boots terminal state. The ssh/new-workspace path can otherwise crash while + // reading @Published placement state from existing workspaces mid-creation. + let insertIndex = newTabInsertIndex(snapshot: snapshot, placementOverride: placementOverride) + let ordinal = Self.nextPortOrdinal + Self.nextPortOrdinal += 1 + let newWorkspace = makeWorkspaceForCreation( + title: "Terminal \(nextTabCount)", + workingDirectory: workingDirectory, + portOrdinal: ordinal, + configTemplate: inheritedConfig, + initialTerminalCommand: initialTerminalCommand, + initialTerminalEnvironment: initialTerminalEnvironment + ) + newWorkspace.owningTabManager = self + wireClosedBrowserTracking(for: newWorkspace) + if eagerLoadTerminal && !select { + requestBackgroundWorkspaceLoad(for: newWorkspace.id) } - } - if select { -#if DEBUG - debugPrimeWorkspaceSwitchTrigger("create", to: newWorkspace.id) -#endif - selectedTabId = newWorkspace.id - NotificationCenter.default.post( - name: .ghosttyDidFocusTab, - object: nil, - userInfo: [GhosttyNotificationKey.tabId: newWorkspace.id] - ) - } -#if DEBUG - UITestRecorder.incrementInt("addTabInvocations") - UITestRecorder.record([ - "tabCount": String(updatedTabs.count), - "selectedTabId": select ? newWorkspace.id.uuidString : (snapshot.selectedTabId?.uuidString ?? "") - ]) -#endif - if autoWelcomeIfNeeded && select && !UserDefaults.standard.bool(forKey: WelcomeSettings.shownKey) { - if let appDelegate = AppDelegate.shared { - appDelegate.sendWelcomeCommandWhenReady(to: newWorkspace, markShownOnSend: true) + // Apply insertion to the current live array so post-snapshot closes/reorders + // are preserved instead of reintroducing stale workspace instances. + var updatedTabs = tabs + if insertIndex >= 0 && insertIndex <= updatedTabs.count { + updatedTabs.insert(newWorkspace, at: insertIndex) } else { - sendWelcomeWhenReady(to: newWorkspace) + updatedTabs.append(newWorkspace) } + tabs = updatedTabs + if let explicitWorkingDirectory, + let terminalPanel = newWorkspace.focusedTerminalPanel { + scheduleInitialWorkspaceGitMetadataRefresh( + workspaceId: newWorkspace.id, + panelId: terminalPanel.id, + directory: explicitWorkingDirectory + ) + } + if eagerLoadTerminal { + if select { + newWorkspace.focusedTerminalPanel?.surface.requestBackgroundSurfaceStartIfNeeded() + } + } + if select { +#if DEBUG + debugPrimeWorkspaceSwitchTrigger("create", to: newWorkspace.id) +#endif + selectedTabId = newWorkspace.id + NotificationCenter.default.post( + name: .ghosttyDidFocusTab, + object: nil, + userInfo: [GhosttyNotificationKey.tabId: newWorkspace.id] + ) + } +#if DEBUG + UITestRecorder.incrementInt("addTabInvocations") + UITestRecorder.record([ + "tabCount": String(updatedTabs.count), + "selectedTabId": select ? newWorkspace.id.uuidString : (snapshot.selectedTabId?.uuidString ?? "") + ]) +#endif + if autoWelcomeIfNeeded && select && !UserDefaults.standard.bool(forKey: WelcomeSettings.shownKey) { + if let appDelegate = AppDelegate.shared { + appDelegate.sendWelcomeCommandWhenReady(to: newWorkspace, markShownOnSend: true) + } else { + sendWelcomeWhenReady(to: newWorkspace) + } + } + return newWorkspace } - return newWorkspace } @MainActor @@ -2177,9 +2187,10 @@ class TabManager: ObservableObject { terminalPanelForWorkspaceConfigInheritanceSource(workspace: selectedWorkspace) } - private func workspaceCreationSnapshot() -> WorkspaceCreationSnapshot { - let currentTabs = tabs - let currentSelectedTabId = selectedTabId + private func workspaceCreationSnapshot( + currentTabs: [Workspace], + currentSelectedTabId: UUID? + ) -> WorkspaceCreationSnapshot { let tabSnapshots = currentTabs.map { WorkspaceCreationTabSnapshot(workspace: $0) } let selectedTabSnapshot = currentSelectedTabId.flatMap { selectedTabId in tabSnapshots.first(where: { $0.id == selectedTabId }) @@ -2197,6 +2208,13 @@ class TabManager: ObservableObject { ) } + private func workspaceCreationSnapshot() -> WorkspaceCreationSnapshot { + workspaceCreationSnapshot( + currentTabs: tabs, + currentSelectedTabId: selectedTabId + ) + } + private func orderedLiveWorkspaceCreationTabs( from snapshot: WorkspaceCreationSnapshot ) -> [WorkspaceCreationTabSnapshot]? { diff --git a/cmuxTests/WorkspaceUnitTests.swift b/cmuxTests/WorkspaceUnitTests.swift index c0090586..87d825d7 100644 --- a/cmuxTests/WorkspaceUnitTests.swift +++ b/cmuxTests/WorkspaceUnitTests.swift @@ -448,6 +448,53 @@ final class WorkspaceCreationPlacementTests: XCTestCase { XCTAssertEqual(manager.selectedTabId, inserted.id) } + func testAddWorkspaceKeepsCapturedWorkspaceAliveUntilCreationFinishes() { + let manager = SnapshotMutatingTabManager() + guard let first = manager.tabs.first else { + XCTFail("Expected initial workspace") + return + } + + var closingWorkspace: Workspace? = manager.addWorkspace() + let third = manager.addWorkspace() + manager.selectWorkspace(third) + + guard let capturedClosingWorkspace = closingWorkspace else { + XCTFail("Expected secondary workspace") + return + } + + let closingWorkspaceId = capturedClosingWorkspace.id + weak var weakClosingWorkspace = capturedClosingWorkspace + XCTAssertEqual(manager.tabs.map(\.id), [first.id, closingWorkspaceId, third.id]) + closingWorkspace = nil + + manager.afterCaptureWorkspaceCreationSnapshot = { + guard let liveWorkspace = manager.tabs.first(where: { $0.id == closingWorkspaceId }) else { + XCTFail("Expected captured workspace to still be present when closing after snapshot") + return + } + manager.closeWorkspace(liveWorkspace) + } + + var didReachBeforeCreateWorkspace = false + manager.beforeCreateWorkspace = { + didReachBeforeCreateWorkspace = true + XCTAssertNotNil( + weakClosingWorkspace, + "Expected the workspace captured before Cmd+N to stay alive until creation finishes" + ) + } + + let inserted = manager.addWorkspace(placementOverride: .afterCurrent) + + XCTAssertTrue(didReachBeforeCreateWorkspace) + XCTAssertFalse(manager.tabs.contains(where: { $0.id == closingWorkspaceId })) + XCTAssertEqual(manager.tabs.map(\.id), [first.id, third.id, inserted.id]) + XCTAssertEqual(manager.selectedTabId, inserted.id) + XCTAssertNil(weakClosingWorkspace) + } + func testAddWorkspaceAfterCurrentUsesSnapshotPinnedStateWhenPinningMutatesAfterSnapshot() { let manager = SnapshotMutatingTabManager() guard let first = manager.tabs.first else {