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
This commit is contained in:
Austin Wang
GitHub
parent
18531fd78e
commit
61e6a0e2b9
2 changed files with 144 additions and 79 deletions
|
|
@ -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]? {
|
||||
|
|
|
|||
|
|
@ -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 {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue