From 703efd8bd333bec93f45a14e763361721b0e164a Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 12 Sep 2026 17:47:27 -0700 Subject: [PATCH] fix: validate remote observations before publishing files --- CHANGELOG.md | 2 + .../UI/UIAutomationService+Operations.swift | 59 +------- .../RemoteDesktopObservationService.swift | 92 ++++++++---- .../RemoteCaptureGateOwnershipTests.swift | 140 +++++++++++++++++- docs/commands/see.md | 4 + 5 files changed, 209 insertions(+), 88 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 15e640aae..dc7e6e790 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ ## Unreleased +- Keep caller screenshot destinations intact when remote evidence is rejected or raw output was not requested, staging ordinary captures before file publication as well as ROI captures. #710. + - Fix host-routed screen observations with Accessibility elements by validating their semantic owner separately from the screen raster target. #715, #710. ## 4.3.4 - 2026-09-11 diff --git a/Core/PeekabooAutomationKit/Sources/PeekabooAutomationKit/Services/UI/UIAutomationService+Operations.swift b/Core/PeekabooAutomationKit/Sources/PeekabooAutomationKit/Services/UI/UIAutomationService+Operations.swift index a3627e4c2..b75d186fc 100644 --- a/Core/PeekabooAutomationKit/Sources/PeekabooAutomationKit/Services/UI/UIAutomationService+Operations.swift +++ b/Core/PeekabooAutomationKit/Sources/PeekabooAutomationKit/Services/UI/UIAutomationService+Operations.swift @@ -5,64 +5,7 @@ import PeekabooFoundation extension UIAutomationService { // MARK: - Element Detection - /** - * Detect and analyze UI elements in a captured screen image using AI-powered recognition. - * - * This method uses advanced computer vision and AI models to identify interactive UI elements - * in screenshots. Elements are classified by type (buttons, text fields, etc.) and assigned - * unique identifiers for subsequent automation operations. - * - * - Parameters: - * - imageData: PNG or JPEG image data containing the screen capture - * - snapshotId: Optional snapshot identifier for element caching and state management - * - windowContext: Optional context about the captured window for improved accuracy - * - Returns: `ElementDetectionResult` containing detected elements and metadata - * - Throws: `PeekabooError` if detection fails or image data is invalid - * - * ## Detection Process - * 1. **Image Analysis**: AI model analyzes the screenshot for UI patterns - * 2. **Element Classification**: Elements are categorized (button, textField, image, etc.) - * 3. **Coordinate Mapping**: Screen coordinates are calculated for each element - * 4. **Accessibility Correlation**: Elements are matched with accessibility tree data - * 5. **Session Caching**: Results are stored for quick access in subsequent operations - * - * ## Element Types - * Detected elements include: - * - `button`: Clickable buttons and controls - * - `textField`: Text input fields and text areas - * - `image`: Images and icons - * - `staticText`: Labels and static text content - * - `other`: Other interactive elements - * - * ## Performance - * - **Typical Duration**: 200-800ms depending on screen complexity - * - **Caching**: Results are cached per snapshot to avoid re-detection - * - **Batch Processing**: Multiple elements detected in single pass - * - * ## Example - * ```swift - * let captureResult = try await screenCapture.captureScreen() - * let windowContext = WindowContext( - * applicationName: "Safari", - * windowTitle: "Apple", - * windowBounds: CGRect(x: 0, y: 0, width: 1920, height: 1080) - * ) - * - * let elements = try await automation.detectElements( - * in: captureResult.imageData, - * snapshotId: "ps1_0123456789abcdef0123456789abcdef", - * windowContext: windowContext - * ) - * - * print("Detected \(elements.elements.all.count) elements") - * for element in elements.elements.buttons { - * print("Button: \(element.label ?? "Unlabeled") at \(element.bounds)") - * } - * ``` - * - * - Important: Requires Screen Recording permission for screen capture - * - Note: Detection accuracy improves with window context information - */ + /// Reads the target's Accessibility element map. An explicit snapshot ID also stores the result. public func detectElements( in imageData: Data, snapshotId: String?, diff --git a/Core/PeekabooCore/Sources/PeekabooCore/Support/RemoteDesktopObservationService.swift b/Core/PeekabooCore/Sources/PeekabooCore/Support/RemoteDesktopObservationService.swift index ef6af1445..dced35d9f 100644 --- a/Core/PeekabooCore/Sources/PeekabooCore/Support/RemoteDesktopObservationService.swift +++ b/Core/PeekabooCore/Sources/PeekabooCore/Support/RemoteDesktopObservationService.swift @@ -90,7 +90,10 @@ public final class RemoteDesktopObservationService: DesktopObservationActionResu else { throw RemoteDesktopObservationCapabilityPolicy.captureEnginePreferenceUnavailableError() } - guard request.capture.roi != nil else { + let isROI = request.capture.roi != nil + let writesArtifacts = request.output.saveRawScreenshot || request.output.saveAnnotatedScreenshot || + request.output.saveSnapshot + guard isROI || writesArtifacts else { let actionResult = try await self.client.desktopObservationWithOutcome(request) do { // The Bridge client verifies every returned artifact under the negotiated content @@ -107,7 +110,7 @@ public final class RemoteDesktopObservationService: DesktopObservationActionResu throw Self.failurePreservingOutcome(error, from: actionResult) } } - guard self.supportsExactWindowROIObservation else { + guard !isROI || self.supportsExactWindowROIObservation else { throw PeekabooBridgeErrorEnvelope( code: .operationNotSupported, message: "Bridge host lacks protocol 1.21 exact-window ROI observation support") @@ -117,8 +120,9 @@ public final class RemoteDesktopObservationService: DesktopObservationActionResu try Self.checkPostProcessingAllowance(deadline: deadline, timeout: overallTimeout) let directory = FileManager.default.temporaryDirectory - .appendingPathComponent("peekaboo-remote-roi-\(UUID().uuidString)", isDirectory: true) - try FileManager.default.createDirectory(at: directory, withIntermediateDirectories: false) + .appendingPathComponent("peekaboo-remote-observation-\(UUID().uuidString)", isDirectory: true) + try FileManager.default.createDirectory( + at: directory, withIntermediateDirectories: false, attributes: [.posixPermissions: 0o700]) defer { try? FileManager.default.removeItem(at: directory) } let quarantinePath = directory @@ -126,10 +130,12 @@ public final class RemoteDesktopObservationService: DesktopObservationActionResu .path var remoteRequest = request remoteRequest.output.path = quarantinePath - // The client owns ROI validation and publication. Force one quarantined raster for proof, - // and defer snapshot publication until the receipt and every requested artifact pass. + // Caller-visible files stay private until response validation. Ordinary observations retain + // host-owned snapshot publication; ROI keeps its existing deferred snapshot transaction. remoteRequest.output.saveRawScreenshot = true - remoteRequest.output.saveSnapshot = false + if isROI { + remoteRequest.output.saveSnapshot = false + } let remoteResult: UIAutomationActionResult do { @@ -150,18 +156,38 @@ public final class RemoteDesktopObservationService: DesktopObservationActionResu target: result.target, capture: result.capture, request: request) - try DesktopObservationROIProcessor.validateApplied( - request.capture.roi, - requestTarget: request.target, - resolvedTarget: result.target, - capture: result.capture) + if isROI { + try DesktopObservationROIProcessor.validateApplied( + request.capture.roi, + requestTarget: request.target, + resolvedTarget: result.target, + capture: result.capture) + } try Self.checkPostProcessingAllowance(deadline: deadline, timeout: overallTimeout) - let prepared = try self.prepareROIResult( + let prepared = try self.prepareObservationResult( result, request: request, quarantinePath: quarantinePath, deadline: deadline, timeout: overallTimeout) + if !isROI { + try self.artifactInstallationPreflight() + for artifact in prepared.artifacts { + try Self.checkPostProcessingAllowance(deadline: deadline, timeout: overallTimeout) + let destination = URL(fileURLWithPath: artifact.path) + try FileManager.default.createDirectory( + at: destination.deletingLastPathComponent(), withIntermediateDirectories: true) + try artifact.data.write(to: destination, options: .atomic) + } + if let evidenceError { + throw evidenceError + } + return UIAutomationActionResult( + payload: prepared.result, + outcome: remoteResult.outcome, + targetIdentity: remoteResult.targetIdentity, + selectedLeafEvidence: remoteResult.selectedLeafEvidence) + } let stagedArtifacts = try Self.stageArtifacts( prepared.artifacts, deadline: deadline, @@ -198,8 +224,14 @@ public final class RemoteDesktopObservationService: DesktopObservationActionResu return try await UIAutomationActionResult( payload: commitTask.value, outcome: remoteResult.outcome, - targetIdentity: remoteResult.targetIdentity) + targetIdentity: remoteResult.targetIdentity, + selectedLeafEvidence: remoteResult.selectedLeafEvidence) } catch { + if !isROI, error is CaptureROIError { + throw Self.failurePreservingOutcome( + PeekabooError.captureFailed("Remote observation returned invalid screenshot artifacts"), + from: remoteResult) + } throw Self.failurePreservingOutcome(error, from: remoteResult) } } @@ -215,20 +247,22 @@ public final class RemoteDesktopObservationService: DesktopObservationActionResu operation: "remote desktop observation post-processing") } - private struct PreparedROIResult { + private struct PreparedObservationResult { let result: DesktopObservationResult let artifacts: [(data: Data, path: String)] let quarantineRawPath: String let quarantineAnnotatedPath: String? } - private func prepareROIResult( + private func prepareObservationResult( _ result: DesktopObservationResult, request: DesktopObservationRequest, quarantinePath: String, deadline: ContinuousClock.Instant?, - timeout: TimeInterval?) throws -> PreparedROIResult + timeout: TimeInterval?) throws -> PreparedObservationResult { + let isROI = request.capture.roi != nil + let defaultFilePrefix = isROI ? "peekaboo-roi" : "peekaboo" try Self.checkPostProcessingAllowance(deadline: deadline, timeout: timeout) guard Self.sameFile(result.files.rawScreenshotPath, quarantinePath) else { throw CaptureROIError.hostDidNotApplyROI @@ -245,7 +279,7 @@ public final class RemoteDesktopObservationService: DesktopObservationActionResu ? ObservationOutputPathResolver.resolve( path: request.output.path, format: request.output.format, - defaultFileName: "peekaboo-roi-\(UUID().uuidString).\(request.output.format.rawValue)") + defaultFileName: "\(defaultFilePrefix)-\(UUID().uuidString).\(request.output.format.rawValue)") .standardizedFileURL .path : nil @@ -273,14 +307,15 @@ public final class RemoteDesktopObservationService: DesktopObservationActionResu } var artifacts: [(data: Data, path: String)] = [] - if request.output.saveRawScreenshot, let rawPath { + if let rawPath { artifacts.append((rawData, rawPath)) } if let annotatedPath, let annotatedData { artifacts.append((annotatedData, annotatedPath)) } + let includesImageData = isROI || rawPath == nil let capture = CaptureResult( - imageData: rawData, + imageData: includesImageData ? rawData : result.capture.imageData, savedPath: rawPath, metadata: result.capture.metadata, warning: result.capture.warning) @@ -298,21 +333,22 @@ public final class RemoteDesktopObservationService: DesktopObservationActionResu ocr: result.ocr, files: DesktopObservationFiles( rawScreenshotPath: rawPath, - annotatedScreenshotPath: annotatedPath), + annotatedScreenshotPath: annotatedPath, + publishedSnapshotID: result.files.publishedSnapshotID), timings: result.timings, - diagnostics: result.diagnostics) - .withCaptureContentDigest( - rawScreenshotData: rawData, - annotatedScreenshotData: annotatedData) - return PreparedROIResult( - result: preparedResult, + diagnostics: result.diagnostics, + captureContentDigest: result.captureContentDigest) + return PreparedObservationResult( + result: includesImageData ? preparedResult.withCaptureContentDigest( + rawScreenshotData: rawPath == nil ? nil : rawData, + annotatedScreenshotData: annotatedData) : preparedResult, artifacts: artifacts, quarantineRawPath: quarantinePath, quarantineAnnotatedPath: quarantineAnnotatedPath) } private func storeSnapshotIfNeeded( - _ prepared: PreparedROIResult, + _ prepared: PreparedObservationResult, request: DesktopObservationRequest, deadline: ContinuousClock.Instant?, timeout: TimeInterval?) async throws diff --git a/Core/PeekabooCore/Tests/PeekabooTests/RemoteCaptureGateOwnershipTests.swift b/Core/PeekabooCore/Tests/PeekabooTests/RemoteCaptureGateOwnershipTests.swift index 3935840de..e5b1d29d8 100644 --- a/Core/PeekabooCore/Tests/PeekabooTests/RemoteCaptureGateOwnershipTests.swift +++ b/Core/PeekabooCore/Tests/PeekabooTests/RemoteCaptureGateOwnershipTests.swift @@ -8,6 +8,7 @@ import UniformTypeIdentifiers @testable import PeekabooBridge @testable import PeekabooCore +@Suite(.serialized) @MainActor struct RemoteCaptureGateOwnershipTests { private static let roiFixtureBounds = CGRect(x: 100, y: 200, width: 100, height: 80) @@ -324,6 +325,110 @@ extension RemoteCaptureGateOwnershipTests { #expect(seconds == 0) } + @Test(arguments: [false, true]) + func `rejected ordinary observation preserves caller screenshot destinations`( + existingDestination: Bool) async throws + { + let root = URL(fileURLWithPath: "/tmp/pb-publication-\(UUID().uuidString.prefix(8))", isDirectory: true) + try FileManager.default.createDirectory(at: root, withIntermediateDirectories: false) + defer { try? FileManager.default.removeItem(at: root) } + let output = root.appendingPathComponent("capture.png") + let sentinel = Data("existing caller content".utf8) + if existingDestination { + try sentinel.write(to: output) + } + let observation = NonROIFileObservationService(captureMode: .window) + let host = PeekabooBridgeHost( + socketPath: root.appendingPathComponent("bridge.sock").path, + server: self.makeROIServer(services: StubServices(desktopObservation: observation)), + allowedTeamIDs: [], + requestTimeoutSec: 2) + try await host.startChecked() + defer { Task { await host.stop() } } + let remote = try await RemoteDesktopObservationService( + client: self.makeNegotiatedClient( + socketPath: root.appendingPathComponent("bridge.sock").path, + requestTimeoutSec: 2)) + + await #expect(throws: (any Error).self) { + _ = try await remote.observe(DesktopObservationRequest( + target: .screen(index: 0), + detection: .init(mode: .none), + output: .init(path: output.path, saveRawScreenshot: true))) + } + + if existingDestination { + #expect(try Data(contentsOf: output) == sentinel) + } else { + #expect(!FileManager.default.fileExists(atPath: output.path)) + } + let hostPath = try #require(observation.lastPath) + #expect(hostPath != output.path) + #expect(!FileManager.default.fileExists(atPath: hostPath)) + await host.stop() + } + + @Test(arguments: [false, true]) + func `ordinary annotation and snapshot only requests never replace the raw destination`( + savesSnapshot: Bool) async throws + { + let root = URL(fileURLWithPath: "/tmp/pb-no-raw-\(UUID().uuidString.prefix(8))", isDirectory: true) + try FileManager.default.createDirectory(at: root, withIntermediateDirectories: false) + defer { try? FileManager.default.removeItem(at: root) } + let output = root.appendingPathComponent("capture.png") + let sentinel = Data("existing caller content".utf8) + try sentinel.write(to: output) + let snapshots = InMemorySnapshotManager(options: .init(copyArtifactsOnStore: true)) + let snapshotID: String? = if savesSnapshot { + try await snapshots.createSnapshot() + } else { + nil + } + defer { + if let snapshotID { + Task { try? await snapshots.cleanSnapshot(snapshotId: snapshotID) } + } + } + let observation = NonROIFileObservationService(snapshots: snapshots) + let host = PeekabooBridgeHost( + socketPath: root.appendingPathComponent("bridge.sock").path, + server: self.makeROIServer(services: StubServices(snapshots: snapshots, desktopObservation: observation)), + allowedTeamIDs: [], + requestTimeoutSec: 2) + try await host.startChecked() + defer { Task { await host.stop() } } + let remote = try await RemoteDesktopObservationService( + client: self.makeNegotiatedClient( + socketPath: root.appendingPathComponent("bridge.sock").path, + requestTimeoutSec: 2)) + let result = try await remote.observe(DesktopObservationRequest( + target: .screen(index: 0), + detection: .init(mode: .none), + output: .init( + path: output.path, + saveRawScreenshot: false, + saveAnnotatedScreenshot: !savesSnapshot, + saveSnapshot: savesSnapshot, + snapshotID: snapshotID))) + + #expect(try Data(contentsOf: output) == sentinel) + #expect(result.files.rawScreenshotPath == nil) + #expect(result.capture.savedPath == nil) + #expect(result.captureContentDigest?.rawScreenshotSHA256 == nil) + #expect(try result.verifiedCaptureImageData(requirement: .requireDigest) == observation.imageData) + if let snapshotID { + #expect(result.files.publishedSnapshotID == snapshotID) + let snapshot = try #require(try await snapshots.getUIAutomationSnapshot(snapshotId: snapshotID)) + let storedPath = try #require(snapshot.screenshotPath) + #expect(try Data(contentsOf: URL(fileURLWithPath: storedPath)) == observation.imageData) + try await snapshots.cleanSnapshot(snapshotId: snapshotID) + } else { + let annotation = try #require(result.files.annotatedScreenshotPath) + #expect(try Data(contentsOf: URL(fileURLWithPath: annotation)) == observation.imageData) + } + await host.stop() + } + @Test func `legacy remote observation delegates capture transaction gating to host`() { let client = PeekabooBridgeClient( @@ -1011,10 +1116,38 @@ private final class PathlessTrackingObservationService: DesktopObservationServic @MainActor private final class NonROIFileObservationService: DesktopObservationServiceProtocol { let imageData = makeROITestImageData(width: 1, height: 1, red: 0.2, green: 0.4, blue: 0.8) + private let captureMode: CaptureMode + private let snapshots: (any SnapshotManagerProtocol)? + private(set) var lastPath: String? + + init(captureMode: CaptureMode = .screen, snapshots: (any SnapshotManagerProtocol)? = nil) { + self.captureMode = captureMode + self.snapshots = snapshots + } func observe(_ request: DesktopObservationRequest) async throws -> DesktopObservationResult { let path = try #require(request.output.path) + self.lastPath = path try self.imageData.write(to: URL(fileURLWithPath: path), options: .atomic) + let annotatedPath = request.output.saveAnnotatedScreenshot + ? ObservationOutputWriter.annotatedScreenshotPath(forRawScreenshotPath: path) : nil + if let annotatedPath { + try self.imageData.write(to: URL(fileURLWithPath: annotatedPath), options: .atomic) + } + var publishedSnapshotID: String? + if request.output.saveSnapshot { + let snapshots = try #require(self.snapshots) + let snapshotID = try #require(request.output.snapshotID) + try await snapshots.storeScreenshot(.init( + snapshotId: snapshotID, + screenshotPath: path, + applicationBundleId: nil, + applicationProcessId: nil, + applicationName: nil, + windowTitle: nil, + windowBounds: nil)) + publishedSnapshotID = snapshotID + } let size = CGSize(width: 1, height: 1) return DesktopObservationResult( target: ResolvedObservationTarget(kind: .screen(index: 0)), @@ -1023,7 +1156,7 @@ private final class NonROIFileObservationService: DesktopObservationServiceProto savedPath: path, metadata: CaptureMetadata( size: size, - mode: .screen, + mode: self.captureMode, displayInfo: DisplayInfo( index: 0, name: "Fixture", @@ -1037,7 +1170,10 @@ private final class NonROIFileObservationService: DesktopObservationServiceProto finalPixelSize: size, engine: "ScreenCaptureKit"))), elements: nil, - files: DesktopObservationFiles(rawScreenshotPath: path)) + files: DesktopObservationFiles( + rawScreenshotPath: path, + annotatedScreenshotPath: annotatedPath, + publishedSnapshotID: publishedSnapshotID)) } } diff --git a/docs/commands/see.md b/docs/commands/see.md index 4345cb88b..7b186b9d6 100644 --- a/docs/commands/see.md +++ b/docs/commands/see.md @@ -169,6 +169,10 @@ Observation verification and MCP image reloads limit each artifact to 256 MiB. R descriptor and reject files that grow or change during the read. Ordinary symlink paths remain supported; oversized or replaced artifacts fail before their content is published. +Remote observations stage screenshot files privately until the response identity, signed content, and raster +dimensions validate. Rejected evidence leaves an existing caller destination intact and does not create a new +caller-visible image. Valid-raster `ACCESSIBILITY_INCOMPLETE` failures retain their documented screenshot behavior. + - If the CLI reports **blind typing**, pass an explicit `--app`, `--pid`, `--window-id`, or fresh `--snapshot` so `type` can resolve a background target process, or add `--foreground` when the target app requires focused keyboard input. - If a concrete snapshot reports no unique live host affinity, keep the producing host running or capture again. Do not rewrite the reference, switch to a legacy timestamp ID, or force a different Bridge socket: explicit sockets