diff --git a/Packages/macOS/CmuxTerminal/Sources/CmuxTerminal/Services/Pasteboard/TerminalPasteboardService.swift b/Packages/macOS/CmuxTerminal/Sources/CmuxTerminal/Services/Pasteboard/TerminalPasteboardService.swift index 44fe95ae6b31..3165172c9450 100644 --- a/Packages/macOS/CmuxTerminal/Sources/CmuxTerminal/Services/Pasteboard/TerminalPasteboardService.swift +++ b/Packages/macOS/CmuxTerminal/Sources/CmuxTerminal/Services/Pasteboard/TerminalPasteboardService.swift @@ -1,6 +1,7 @@ public import AppKit public import CmuxTerminalCore public import GhosttyKit +internal import os /// The terminal's pasteboard capability: clipboard reads and writes for the /// ghostty runtime, plus materialization of pasteboard images into owned @@ -23,13 +24,23 @@ public import GhosttyKit /// temp-file set and the one-shot write capture), the sanctioned shape for /// state shared with synchronous callbacks. public final class TerminalPasteboardService: Sendable { - /// One-shot interception slot for ``captureNextStandardClipboardWrite(_:)``. + /// One-shot interception slot for ``captureNextStandardClipboardWrite(matching:_:)``. final class ClipboardWriteCapture: Sendable { private let lock = NSLock() // SAFETY: guarded by `lock`; written by the runtime's write-clipboard // callback thread and read by the capturing caller. nonisolated(unsafe) private var capturedValue: String? + /// Predicate deciding whether a standard-clipboard write belongs to + /// this capture. Writes it rejects pass through to the real + /// pasteboard and leave the capture armed, so an unrelated write + /// (e.g. a user copy racing a VT export) is not swallowed. + let accepts: @Sendable (String) -> Bool + + init(accepts: @escaping @Sendable (String) -> Bool) { + self.accepts = accepts + } + /// Stores the diverted clipboard string. func capture(_ value: String) { lock.lock() @@ -57,6 +68,16 @@ public final class TerminalPasteboardService: Sendable { // ghostty runtime threads. nonisolated(unsafe) private let selectionPasteboard: NSPasteboard + // SAFETY: immutable reference; same argument as `selectionPasteboard`. + // Injectable so tests can exercise standard-location writes without + // touching the real general pasteboard. + nonisolated(unsafe) private let standardPasteboard: NSPasteboard + + private static let logger = Logger( + subsystem: "com.cmuxterm.app", + category: "terminal.pasteboard" + ) + /// The directory that owned temporary image files are written into. let temporaryDirectory: URL @@ -74,11 +95,19 @@ public final class TerminalPasteboardService: Sendable { /// Creates the process's pasteboard service. /// - /// - Parameter temporaryDirectory: Destination for owned temporary image - /// files. Tests inject a scratch directory; the app uses the user's - /// temporary directory. - public init(temporaryDirectory: URL = FileManager.default.temporaryDirectory) { + /// - Parameters: + /// - temporaryDirectory: Destination for owned temporary image files. + /// Tests inject a scratch directory; the app uses the user's + /// temporary directory. + /// - standardPasteboard: The pasteboard backing the standard clipboard + /// location. Tests inject a scratch pasteboard; the app uses + /// `NSPasteboard.general`. + public init( + temporaryDirectory: URL = FileManager.default.temporaryDirectory, + standardPasteboard: NSPasteboard = .general + ) { self.temporaryDirectory = temporaryDirectory + self.standardPasteboard = standardPasteboard self.selectionPasteboard = NSPasteboard( name: NSPasteboard.Name("com.mitchellh.ghostty.selection") ) @@ -88,43 +117,90 @@ public final class TerminalPasteboardService: Sendable { extension TerminalPasteboardService: TerminalClipboardWriting { /// Publishes all textual representations as one pasteboard item, honoring /// an armed one-shot capture for the standard location. + /// + /// An armed capture only consumes writes its predicate accepts (matched + /// against the preferred plain-text representation); any other + /// standard-location write (e.g. a user copy racing a VT export) passes + /// through to the real pasteboard with the capture left armed. public func writeRepresentations( _ representations: [TerminalClipboardRepresentation], to location: ghostty_clipboard_e ) { guard !representations.isEmpty else { return } + let preferredValue = representations.first(where: { + normalizedTerminalClipboardMIMEType($0.mimeType) == "text/plain" + })?.string ?? representations[0].string if location == GHOSTTY_CLIPBOARD_STANDARD { - var capture: ClipboardWriteCapture? standardClipboardWriteCaptureLock.lock() - capture = standardClipboardWriteCapture - if capture != nil { - standardClipboardWriteCapture = nil - } + let armed = standardClipboardWriteCapture standardClipboardWriteCaptureLock.unlock() - if let capture { - let value = representations.first(where: { - normalizedTerminalClipboardMIMEType($0.mimeType) == "text/plain" - })?.string ?? representations[0].string - capture.capture(value) - return + if let armed { + if armed.accepts(preferredValue) { + // Claim the one-shot slot atomically: only the write that + // actually clears it may capture. A concurrent matching + // write that loses this race falls through to the real + // pasteboard instead of overwriting the captured value + // and being swallowed. + standardClipboardWriteCaptureLock.lock() + let claimed = standardClipboardWriteCapture === armed + if claimed { + standardClipboardWriteCapture = nil + } + standardClipboardWriteCaptureLock.unlock() + if claimed { + armed.capture(preferredValue) + return + } + } else { + Self.logger.info( + "standard write passed through armed capture (length \(preferredValue.count, privacy: .public))" + ) + } } } + // An empty payload (e.g. copy-on-select firing after a TUI redraw + // already invalidated the selection) must not clear the clipboard: + // clearContents-then-write-nothing silently destroys whatever the + // user last copied. + guard !preferredValue.isEmpty else { + Self.logger.info("ignored empty clipboard write") + return + } + guard let pasteboard = pasteboard(for: location) else { return } - let item = NSPasteboardItem() - var writtenTypes = Set() - for representation in representations { - let type = terminalPasteboardType(forMIMEType: representation.mimeType) - guard writtenTypes.insert(type).inserted else { continue } - _ = item.setString( - representation.string, - forType: type + // A pasteboard item can only be attached to one pasteboard, so the + // retry path below needs a freshly built copy. + let makeItem: () -> NSPasteboardItem = { + let item = NSPasteboardItem() + var writtenTypes = Set() + for representation in representations { + let type = terminalPasteboardType(forMIMEType: representation.mimeType) + guard writtenTypes.insert(type).inserted else { continue } + _ = item.setString( + representation.string, + forType: type + ) + } + return item + } + let clearedChangeCount = pasteboard.clearContents() + if !pasteboard.writeObjects([makeItem()]) { + // A contended pasteboard can reject the write after clearContents, + // leaving the clipboard empty. Retry once so the failure is not + // silent data loss — but only while nothing else has written in + // the meantime, so the retry never clobbers a newer value. + var retried = false + if pasteboard.changeCount == clearedChangeCount { + pasteboard.clearContents() + retried = pasteboard.writeObjects([makeItem()]) + } + Self.logger.error( + "pasteboard write failed (length \(preferredValue.count, privacy: .public)), retry \(retried ? "succeeded" : "skipped-or-failed", privacy: .public)" ) } - pasteboard.clearContents() - _ = pasteboard.writeObjects([item]) } /// Writes a string to the given ghostty clipboard location, honoring an @@ -136,14 +212,39 @@ extension TerminalPasteboardService: TerminalClipboardWriting { ) } - /// Arms a one-shot diversion of the next standard-clipboard write that - /// happens while `action` runs, returning the diverted string. + /// Arms a one-shot diversion of the next matching standard-clipboard + /// write that happens while `action` runs, returning the diverted string. + /// + /// - Parameters: + /// - predicate: Decides whether a given standard-clipboard write is the + /// one this capture is waiting for. Non-matching writes reach the + /// real pasteboard and leave the capture armed, so unrelated writers + /// (a user copy, another surface) are not swallowed. Pass a predicate + /// as narrow as the expected payload allows — e.g. "an existing file + /// under the temporary directory" for a VT screen export. + /// - action: The operation expected to trigger the write. + /// + /// Returns `nil` without running a capture when another capture is + /// already in flight: replacing the armed slot would let one operation's + /// write satisfy the other's capture (both predicates accept export + /// paths), handing the wrong content to the wrong caller. Callers + /// already treat `nil` as "fall back to a non-capture read". @discardableResult - public func captureNextStandardClipboardWrite(_ action: () -> Bool) -> String? { - let capture = ClipboardWriteCapture() + public func captureNextStandardClipboardWrite( + matching predicate: @escaping @Sendable (String) -> Bool = { _ in true }, + _ action: () -> Bool + ) -> String? { + let capture = ClipboardWriteCapture(accepts: predicate) standardClipboardWriteCaptureLock.lock() - standardClipboardWriteCapture = capture + let alreadyArmed = standardClipboardWriteCapture != nil + if !alreadyArmed { + standardClipboardWriteCapture = capture + } standardClipboardWriteCaptureLock.unlock() + guard !alreadyArmed else { + Self.logger.info("clipboard capture rejected: another capture is in flight") + return nil + } defer { standardClipboardWriteCaptureLock.lock() @@ -186,7 +287,7 @@ extension TerminalPasteboardService { public func pasteboard(for location: ghostty_clipboard_e) -> NSPasteboard? { switch location { case GHOSTTY_CLIPBOARD_STANDARD: - return .general + return standardPasteboard case GHOSTTY_CLIPBOARD_SELECTION: return selectionPasteboard default: diff --git a/Packages/macOS/CmuxTerminal/Tests/CmuxTerminalTests/Services/TerminalPasteboardServiceTests.swift b/Packages/macOS/CmuxTerminal/Tests/CmuxTerminalTests/Services/TerminalPasteboardServiceTests.swift index d64d5030d952..488377728ddf 100644 --- a/Packages/macOS/CmuxTerminal/Tests/CmuxTerminalTests/Services/TerminalPasteboardServiceTests.swift +++ b/Packages/macOS/CmuxTerminal/Tests/CmuxTerminalTests/Services/TerminalPasteboardServiceTests.swift @@ -139,6 +139,67 @@ struct ClipboardWriteCaptureTests { #expect(service.hasString(for: GHOSTTY_CLIPBOARD_SELECTION)) } + @Test func nonMatchingWritePassesThroughAndLeavesCaptureArmed() throws { + let scratchDir = try makeScratchDirectory() + defer { try? FileManager.default.removeItem(at: scratchDir) } + let scratch = ScratchPasteboard() + let service = TerminalPasteboardService(standardPasteboard: scratch.pasteboard) + let exportFile = scratchDir.appendingPathComponent("export.vt") + try Data("screen".utf8).write(to: exportFile) + let userCopy = "user copied text \(UUID().uuidString)" + + let captured = service.captureNextStandardClipboardWrite( + matching: { $0 == exportFile.path } + ) { + // A user copy racing the export must reach the pasteboard... + service.writeString(userCopy, to: GHOSTTY_CLIPBOARD_STANDARD) + // ...and the capture must stay armed for the export's own write. + service.writeString(exportFile.path, to: GHOSTTY_CLIPBOARD_STANDARD) + return true + } + + #expect(captured == exportFile.path) + #expect(scratch.pasteboard.string(forType: .string) == userCopy) + } + + @Test func overlappingCaptureIsRejectedAndDoesNotStealTheFirstCapturesWrite() { + let service = TerminalPasteboardService() + let outer = service.captureNextStandardClipboardWrite { () -> Bool in + // A second capture while one is in flight must be rejected... + let inner = service.captureNextStandardClipboardWrite { true } + #expect(inner == nil) + // ...and the original capture still owns the next matching write. + service.writeString("outer-value", to: GHOSTTY_CLIPBOARD_STANDARD) + return true + } + #expect(outer == "outer-value") + } + + @Test func emptyStandardWriteDoesNotClearExistingClipboardContent() { + let scratch = ScratchPasteboard() + let service = TerminalPasteboardService(standardPasteboard: scratch.pasteboard) + let marker = "existing-content-\(UUID().uuidString)" + service.writeString(marker, to: GHOSTTY_CLIPBOARD_STANDARD) + + // An empty payload (e.g. copy-on-select firing after the selection + // was already invalidated) must be a no-op, not a destructive clear. + service.writeString("", to: GHOSTTY_CLIPBOARD_STANDARD) + + #expect(scratch.pasteboard.string(forType: .string) == marker) + } + + @Test func standardWritesLandOnInjectedPasteboardAfterCaptureDisarms() { + let scratch = ScratchPasteboard() + let service = TerminalPasteboardService(standardPasteboard: scratch.pasteboard) + service.captureNextStandardClipboardWrite { + service.writeString("diverted", to: GHOSTTY_CLIPBOARD_STANDARD) + return true + } + let marker = "after-capture-\(UUID().uuidString)" + service.writeString(marker, to: GHOSTTY_CLIPBOARD_STANDARD) + #expect(scratch.pasteboard.string(forType: .string) == marker) + } + @Test func mixedRepresentationsShareOnePasteboardItem() { let service = TerminalPasteboardService() let marker = "mixed-\(UUID().uuidString)" diff --git a/Packages/macOS/CmuxTerminalCore/Sources/CmuxTerminalCore/Clipboard/TerminalClipboardWriting.swift b/Packages/macOS/CmuxTerminalCore/Sources/CmuxTerminalCore/Clipboard/TerminalClipboardWriting.swift index 754af9796d74..65b1385a0230 100644 --- a/Packages/macOS/CmuxTerminalCore/Sources/CmuxTerminalCore/Clipboard/TerminalClipboardWriting.swift +++ b/Packages/macOS/CmuxTerminalCore/Sources/CmuxTerminalCore/Clipboard/TerminalClipboardWriting.swift @@ -23,14 +23,36 @@ public protocol TerminalClipboardWriting: AnyObject, Sendable { /// Writes a string to the given ghostty clipboard location. /// /// When a one-shot capture is armed via - /// ``captureNextStandardClipboardWrite(_:)``, a standard-location write is - /// diverted into the capture instead of the system pasteboard. + /// ``captureNextStandardClipboardWrite(matching:_:)``, a standard-location + /// write the capture's predicate accepts is diverted into the capture + /// instead of the system pasteboard; a write it rejects reaches the + /// pasteboard and leaves the capture armed. func writeString(_ string: String, to location: ghostty_clipboard_e) - /// Arms a one-shot diversion of the next standard-clipboard write that - /// happens while `action` runs, returning the diverted string. + /// Arms a one-shot diversion of the next matching standard-clipboard + /// write that happens while `action` runs, returning the diverted string. /// - /// Returns `nil` when `action` reports failure or no write occurred. + /// `predicate` decides whether a given write is the one this capture is + /// waiting for; non-matching writes (e.g. a concurrent user copy) pass + /// through to the real pasteboard un-swallowed. + /// + /// Returns `nil` when `action` reports failure, no matching write + /// occurred, or another capture is already in flight (overlapping + /// captures are rejected rather than allowed to steal each other's + /// writes; callers treat `nil` as "fall back to a non-capture read"). + @discardableResult + func captureNextStandardClipboardWrite( + matching predicate: @escaping @Sendable (String) -> Bool, + _ action: () -> Bool + ) -> String? +} + +extension TerminalClipboardWriting { + /// Arms a one-shot diversion of the next standard-clipboard write, + /// accepting any payload. Prefer the `matching:` variant with the + /// narrowest predicate the expected payload allows. @discardableResult - func captureNextStandardClipboardWrite(_ action: () -> Bool) -> String? + public func captureNextStandardClipboardWrite(_ action: () -> Bool) -> String? { + captureNextStandardClipboardWrite(matching: { _ in true }, action) + } } diff --git a/Sources/GhosttyTerminalView.swift b/Sources/GhosttyTerminalView.swift index 62ae1a0abb8c..38d6754f42b3 100644 --- a/Sources/GhosttyTerminalView.swift +++ b/Sources/GhosttyTerminalView.swift @@ -877,7 +877,15 @@ class GhosttyApp { } } runtimeConfig.write_clipboard_cb = { _, location, content, len, _ in - guard let content = content, len > 0 else { return } + guard let content = content, len > 0 else { + #if DEBUG + cmuxDebugLog("terminal.clipboard.write EMPTY payload location=\(location.rawValue) len=\(len)") + #endif + return + } + #if DEBUG + cmuxDebugLog("terminal.clipboard.write location=\(location.rawValue) items=\(len)") + #endif let buffer = UnsafeBufferPointer(start: content, count: Int(len)) let decoder = TerminalClipboardRepresentationDecoder() @@ -4643,8 +4651,13 @@ class GhosttyNSView: NSView, NSUserInterfaceValidations { func validateUserInterfaceItem(_ item: NSValidatedUserInterfaceItem) -> Bool { switch item.action { case #selector(copy(_:)): - guard let surface = surface else { return false } - return hasCopyableTerminalSelection(surface: surface) + // Enabled whenever a surface exists, not gated on + // ghostty_surface_has_selection: that flag can report false while + // the runtime still holds a live selection (e.g. under constant + // TUI redraw), and a disabled menu item swallows Cmd+C before the + // runtime's own copy binding can handle it. Copy with no + // selection is a harmless no-op. + return surface != nil case #selector(paste(_:)): return GhosttyApp.terminalPasteboard.hasString(for: GHOSTTY_CLIPBOARD_STANDARD) case #selector(pasteAsPlainText(_:)): diff --git a/Sources/TerminalController.swift b/Sources/TerminalController.swift index a2ce3afdfbc5..5b44d054550e 100644 --- a/Sources/TerminalController.swift +++ b/Sources/TerminalController.swift @@ -722,6 +722,36 @@ class TerminalController { return trimmed.hasPrefix("/") ? trimmed : nil } + /// Whether a standard-clipboard write plausibly is the file path a + /// `write_screen_file:copy` / `write_active_file:copy` binding action + /// produces: a single absolute path (or file URL) under the process + /// temporary directory, where ghostty writes screen exports. Gates the + /// VT-export clipboard capture so it cannot swallow an unrelated write + /// (e.g. a user copying some other absolute path) that races the export. + /// + /// Deliberately a pure string check: it runs inside the ghostty + /// write-clipboard runtime callback, where filesystem I/O (a `fileExists` + /// on a slow or network volume) could stall terminal processing. + nonisolated static func isPlausibleExportedScreenPath( + _ raw: String, + temporaryDirectory: URL = FileManager.default.temporaryDirectory + ) -> Bool { + guard let path = normalizedExportedScreenPath(raw), + !path.contains("\n") else { + return false + } + // TMPDIR is /var/... which is a symlink to /private/var/...; accept + // either spelling without resolving symlinks on the hot path. + let temporary = temporaryDirectory.standardizedFileURL.path + var prefixes = [temporary] + if temporary.hasPrefix("/var/") { + prefixes.append("/private" + temporary) + } else if temporary.hasPrefix("/private/var/") { + prefixes.append(String(temporary.dropFirst("/private".count))) + } + return prefixes.contains { path.hasPrefix($0.hasSuffix("/") ? $0 : $0 + "/") } + } + nonisolated static func shouldRemoveExportedScreenFile( fileURL: URL, temporaryDirectory: URL = FileManager.default.temporaryDirectory @@ -5464,7 +5494,9 @@ class TerminalController { normalizeLineEndings: Bool = true ) -> String? { var actionSucceeded = false - let exportedPath = GhosttyApp.terminalPasteboard.captureNextStandardClipboardWrite { + let exportedPath = GhosttyApp.terminalPasteboard.captureNextStandardClipboardWrite( + matching: { Self.isPlausibleExportedScreenPath($0) } + ) { let ok = terminalPanel.performInternalBindingAction(bindingAction) actionSucceeded = ok return ok diff --git a/cmuxTests/SessionPersistenceTests.swift b/cmuxTests/SessionPersistenceTests.swift index f2c026225d5a..c7f3f5f2ea25 100644 --- a/cmuxTests/SessionPersistenceTests.swift +++ b/cmuxTests/SessionPersistenceTests.swift @@ -708,6 +708,25 @@ final class SessionPersistenceTests: XCTestCase { XCTAssertNil(TerminalController.normalizedExportedScreenPath(nil)) } + func testIsPlausibleExportedScreenPathRequiresTemporaryDirectoryPrefix() { + let temporary = FileManager.default.temporaryDirectory + let exportPath = temporary.appendingPathComponent("cmux-screen-\(UUID().uuidString).vt").path + + XCTAssertTrue(TerminalController.isPlausibleExportedScreenPath(exportPath)) + XCTAssertTrue(TerminalController.isPlausibleExportedScreenPath("file://\(exportPath)")) + // Both spellings of the /var -> /private/var symlink are accepted. + if exportPath.hasPrefix("/var/") { + XCTAssertTrue(TerminalController.isPlausibleExportedScreenPath("/private" + exportPath)) + } + // Typical user copies must never be mistaken for an export path. + XCTAssertFalse(TerminalController.isPlausibleExportedScreenPath("user copied text")) + // Existing absolute paths outside the temporary directory (e.g. a + // copied /etc/hosts) are not export payloads. + XCTAssertFalse(TerminalController.isPlausibleExportedScreenPath("/etc/hosts")) + // Interior newlines mean a multi-line user copy, not a path. + XCTAssertFalse(TerminalController.isPlausibleExportedScreenPath(exportPath + "\n/second/line")) + } + func testNormalizedMobileVTExportTextSplitsGhosttyCRLFRows() { let normalized = TerminalController.normalizedMobileVTExportText("first\r\nsecond\r\nthird") let rows = normalized.split(separator: "\n", omittingEmptySubsequences: false).map(String.init)