fix(sender): fail fast on corrupt packet lengths and skip unknown packet types

.github/workflows/ci.yml

@@ -2,9 +2,9 @@ name: CI Build Verification
 
 on:
   push:
-    branches: [ main ]
+    branches: [ main, 3.1-maint, 3.2-dev ]
   pull_request:
-    branches: [ main ]
+    branches: [ main, 3.1-maint, 3.2-dev ]
 
 jobs:
   build-sender:
@@ -23,6 +23,11 @@ jobs:
           cd TargetBridge-Sender
           ./scripts/build_targetbridge_sender_app.sh
 
+      - name: Run Sender Unit Tests
+        run: |
+          cd TargetBridge-Sender
+          xcodebuild test -project TargetBridge.xcodeproj -scheme TBDisplaySender -destination 'platform=macOS' -derivedDataPath .build/DerivedData
+
   build-receiver:
     name: Build Receiver (${{ matrix.os }} - ${{ matrix.arch }})
     strategy:

TargetBridge-Sender/TBDisplaySender/TBDisplaySenderAutomation.swift

@@ -166,7 +166,8 @@ enum TBSenderAutomation {
     ///   - `nil` when the explicit session is invalid.
     ///   - `.some(nil)` when no session was requested and the caller should target all sessions.
     ///   - `.some(index)` with a zero-based index for a specific session.
-    private static func resolveSessionIndex(
+    /// - Note: `internal` (not `private`) so the unit-test bundle can exercise the tri-state logic.
+    static func resolveSessionIndex(
         _ raw: String?,
         sessionCount: Int,
         createDefaultIfNeeded: Bool
@@ -201,7 +202,9 @@ enum TBSenderAutomation {
         return service.discoveredReceivers.first
     }
 
-    private static func matches(_ value: String, _ receiver: TBDiscoveredReceiver) -> Bool {
+    // The pure parsing helpers below are `internal` (not `private`) so the
+    // unit-test bundle can exercise them directly.
+    static func matches(_ value: String, _ receiver: TBDiscoveredReceiver) -> Bool {
         let needle = value.lowercased()
         if receiver.id.lowercased() == needle { return true }
         if receiver.receiverName.lowercased() == needle { return true }
@@ -211,7 +214,7 @@ enum TBSenderAutomation {
             || receiver.networkIP.lowercased() == needle
     }
 
-    private static func parseTransport(_ value: String) -> TBTransportKind {
+    static func parseTransport(_ value: String) -> TBTransportKind {
         switch value.lowercased() {
         case "net", "network", "networklink", "link":
             return .networkLink
@@ -220,7 +223,7 @@ enum TBSenderAutomation {
         }
     }
 
-    private static func parseMode(_ value: String) -> TBDisplayCaptureSource? {
+    static func parseMode(_ value: String) -> TBDisplayCaptureSource? {
         switch value.lowercased() {
         case "extended", "extend", "extendeddesktop", "ext":
             return .extendedDesktop
@@ -231,7 +234,7 @@ enum TBSenderAutomation {
         }
     }
 
-    private static func parsePreset(_ value: String) -> TBDisplayCapturePreset? {
+    static func parsePreset(_ value: String) -> TBDisplayCapturePreset? {
         if let preset = TBDisplayCapturePreset(rawValue: value) { return preset }
         switch value.lowercased() {
         case "1440p", "1440", "standard": return .standard1440p

TargetBridge-Sender/TBDisplaySender/TBDisplaySenderService.swift

@@ -1584,7 +1584,20 @@ final class TBDisplaySenderSession: NSObject, ObservableObject, Identifiable, @u
     }
 
     private func drainPackets() {
-        while let (type, payload) = TBMonitorProtocol.drainPacket(from: &recvBuffer) {
+        do {
+            try drainPacketsOrThrow()
+        } catch {
+            // Corrupt length prefix: the framing is unrecoverable, so tear the
+            // connection down instead of buffering inbound data forever.
+            NSLog("[protocol] corrupt inbound stream (\(error)); closing connection")
+            recvBuffer.removeAll(keepingCapacity: false)
+            setStatus(.connectionClosed(String(describing: error)))
+            stop(resetStatusTo: nil)
+        }
+    }
+
+    private func drainPacketsOrThrow() throws {
+        while let (type, payload) = try TBMonitorProtocol.drainPacket(from: &recvBuffer) {
             switch type {
             case .displayProfile:
                 handleDisplayProfile(payload)

TargetBridge-Sender/TBDisplaySenderTests/TBMonitorProtocolTests.swift

@@ -0,0 +1,255 @@
+import XCTest
+@testable import TargetBridge
+
+/// Wire-protocol unit tests. These run with no network, no Thunderbolt hardware,
+/// and no receiver — they pin down the framing invariants both apps rely on:
+/// `[4B BE length][1B type][payload]` where length counts type + payload.
+final class TBMonitorProtocolTests: XCTestCase {
+
+    // MARK: - BE32 primitives
+
+    func testBE32RoundTrip() {
+        let values: [UInt32] = [0, 1, 0xFF, 0x1234_5678, 0x7FFF_FFFF, 0xFFFF_FFFF]
+        for value in values {
+            var data = Data()
+            TBMonitorProtocol.appendBE32(&data, value)
+            XCTAssertEqual(data.count, 4)
+            XCTAssertEqual(TBMonitorProtocol.readBE32(data, offset: 0), value, "round trip failed for \(value)")
+        }
+    }
+
+    func testAppendBE32IsBigEndian() {
+        var data = Data()
+        TBMonitorProtocol.appendBE32(&data, 0x0102_0304)
+        XCTAssertEqual([UInt8](data), [0x01, 0x02, 0x03, 0x04])
+    }
+
+    func testReadBE32HonorsOffset() {
+        var data = Data()
+        TBMonitorProtocol.appendBE32(&data, 0xAAAA_AAAA)
+        TBMonitorProtocol.appendBE32(&data, 0x0000_BEEF)
+        XCTAssertEqual(TBMonitorProtocol.readBE32(data, offset: 4), 0x0000_BEEF)
+    }
+
+    // MARK: - Packet framing
+
+    func testMakePacketLayout() {
+        let packet = TBMonitorProtocol.makePacket(type: .heartbeat, payload: Data([0xAA, 0xBB, 0xCC]))
+        // length = 1 (type byte) + 3 (payload) = 4
+        XCTAssertEqual([UInt8](packet), [0x00, 0x00, 0x00, 0x04, 0x30, 0xAA, 0xBB, 0xCC])
+    }
+
+    func testDrainPacketRoundTrip() throws {
+        let payload = Data("hello receiver".utf8)
+        var buffer = TBMonitorProtocol.makePacket(type: .helloReceiver, payload: payload)
+
+        let drained = try TBMonitorProtocol.drainPacket(from: &buffer)
+        XCTAssertNotNil(drained)
+        XCTAssertEqual(drained?.0, .helloReceiver)
+        XCTAssertEqual(drained?.1, payload)
+        XCTAssertTrue(buffer.isEmpty, "drain must consume the packet")
+    }
+
+    func testDrainPacketEmptyPayload() throws {
+        var buffer = TBMonitorProtocol.makePacket(type: .teardown, payload: Data())
+        let drained = try TBMonitorProtocol.drainPacket(from: &buffer)
+        XCTAssertEqual(drained?.0, .teardown)
+        XCTAssertEqual(drained?.1, Data())
+        XCTAssertTrue(buffer.isEmpty)
+    }
+
+    func testDrainPacketWaitsForCompleteHeader() {
+        var buffer = Data([0x00, 0x00, 0x00, 0x04]) // header missing its 5th byte
+        XCTAssertNil(try TBMonitorProtocol.drainPacket(from: &buffer))
+        XCTAssertEqual(buffer.count, 4, "incomplete data must not be consumed")
+    }
+
+    func testDrainPacketWaitsForCompletePayload() {
+        let full = TBMonitorProtocol.makePacket(type: .frame, payload: Data(repeating: 0x42, count: 100))
+        var buffer = full.prefix(50) as Data
+        XCTAssertNil(try TBMonitorProtocol.drainPacket(from: &buffer))
+        XCTAssertEqual(buffer.count, 50, "incomplete data must not be consumed")
+    }
+
+    func testDrainTwoContiguousPackets() throws {
+        var buffer = TBMonitorProtocol.makePacket(type: .cursor, payload: Data([0x01]))
+        buffer.append(TBMonitorProtocol.makePacket(type: .brightness, payload: Data([0x02, 0x03])))
+
+        let first = try TBMonitorProtocol.drainPacket(from: &buffer)
+        XCTAssertEqual(first?.0, .cursor)
+        XCTAssertEqual(first?.1, Data([0x01]))
+
+        let second = try TBMonitorProtocol.drainPacket(from: &buffer)
+        XCTAssertEqual(second?.0, .brightness)
+        XCTAssertEqual(second?.1, Data([0x02, 0x03]))
+
+        XCTAssertTrue(buffer.isEmpty)
+    }
+
+    /// Simulates TCP fragmentation: the packet arrives one byte at a time and
+    /// must only drain once the final byte lands.
+    func testDrainPacketAcrossSplitFeeds() throws {
+        let packet = TBMonitorProtocol.makePacket(type: .clipboard, payload: Data("copy me".utf8))
+        var buffer = Data()
+
+        for (index, byte) in packet.enumerated() {
+            buffer.append(byte)
+            let drained = try TBMonitorProtocol.drainPacket(from: &buffer)
+            if index < packet.count - 1 {
+                XCTAssertNil(drained, "must not drain before byte \(packet.count - 1), drained at \(index)")
+            } else {
+                XCTAssertEqual(drained?.0, .clipboard)
+                XCTAssertEqual(drained?.1, Data("copy me".utf8))
+            }
+        }
+    }
+
+    // MARK: - Corrupt and unknown framing
+
+    func testDrainPacketThrowsOnZeroLength() {
+        var buffer = Data([0x00, 0x00, 0x00, 0x00, 0x30])
+        XCTAssertThrowsError(try TBMonitorProtocol.drainPacket(from: &buffer)) { error in
+            XCTAssertEqual(error as? TBMonitorProtocolError, .invalidPacketLength(0))
+        }
+    }
+
+    func testDrainPacketThrowsOnOversizedLength() {
+        var buffer = Data()
+        TBMonitorProtocol.appendBE32(&buffer, TBMonitorProtocol.maxPacketLength + 1)
+        buffer.append(0x21)
+        XCTAssertThrowsError(try TBMonitorProtocol.drainPacket(from: &buffer)) { error in
+            XCTAssertEqual(error as? TBMonitorProtocolError, .invalidPacketLength(TBMonitorProtocol.maxPacketLength + 1))
+        }
+    }
+
+    /// A corrupted length like 0xFFFFFFFF must fail fast instead of making the
+    /// drain loop buffer inbound data forever for a packet that never completes.
+    func testDrainPacketThrowsOnAllOnesLength() {
+        var buffer = Data([0xFF, 0xFF, 0xFF, 0xFF, 0x21, 0x00])
+        XCTAssertThrowsError(try TBMonitorProtocol.drainPacket(from: &buffer)) { error in
+            XCTAssertEqual(error as? TBMonitorProtocolError, .invalidPacketLength(0xFFFF_FFFF))
+        }
+    }
+
+    func testDrainPacketAcceptsLengthAtCapWhileWaitingForPayload() {
+        var buffer = Data()
+        TBMonitorProtocol.appendBE32(&buffer, TBMonitorProtocol.maxPacketLength)
+        buffer.append(0x21)
+        // Length is legal but the payload has not arrived: need more data, no throw.
+        XCTAssertNil(try TBMonitorProtocol.drainPacket(from: &buffer))
+        XCTAssertEqual(buffer.count, 5)
+    }
+
+    func testDrainPacketThrowsOnCorruptLengthBehindValidPacket() throws {
+        var buffer = TBMonitorProtocol.makePacket(type: .heartbeat, payload: Data([0x01]))
+        buffer.append(Data([0xFF, 0xFF, 0xFF, 0xFF, 0x21]))
+
+        let first = try TBMonitorProtocol.drainPacket(from: &buffer)
+        XCTAssertEqual(first?.0, .heartbeat)
+
+        XCTAssertThrowsError(try TBMonitorProtocol.drainPacket(from: &buffer))
+    }
+
+    /// An unrecognized type byte (e.g. a packet from a newer peer) must be
+    /// skipped so it cannot stall valid packets queued behind it.
+    func testDrainPacketSkipsUnknownTypeAndReturnsNextPacket() throws {
+        var buffer = Data()
+        TBMonitorProtocol.appendBE32(&buffer, 3)
+        buffer.append(contentsOf: [0xEE, 0x00, 0x00]) // unknown type 0xEE + 2 payload bytes
+        buffer.append(TBMonitorProtocol.makePacket(type: .heartbeat, payload: Data([0x07])))
+
+        let drained = try TBMonitorProtocol.drainPacket(from: &buffer)
+        XCTAssertEqual(drained?.0, .heartbeat)
+        XCTAssertEqual(drained?.1, Data([0x07]))
+        XCTAssertTrue(buffer.isEmpty)
+    }
+
+    func testDrainPacketConsumesLoneUnknownType() throws {
+        var buffer = Data()
+        TBMonitorProtocol.appendBE32(&buffer, 1)
+        buffer.append(0xEE)
+
+        XCTAssertNil(try TBMonitorProtocol.drainPacket(from: &buffer))
+        XCTAssertTrue(buffer.isEmpty, "unknown packet must be consumed, not left to stall the stream")
+    }
+
+    // MARK: - JSON payloads
+
+    func testJSONPacketRoundTrip() throws {
+        let heartbeat = TBMonitorHeartbeat(sequence: 42)
+        guard var buffer = TBMonitorProtocol.makeJSONPacket(type: .heartbeat, value: heartbeat) else {
+            XCTFail("encode failed"); return
+        }
+        guard let (type, payload) = try TBMonitorProtocol.drainPacket(from: &buffer) else {
+            XCTFail("drain failed"); return
+        }
+        XCTAssertEqual(type, .heartbeat)
+        XCTAssertEqual(TBMonitorProtocol.decodeJSON(TBMonitorHeartbeat.self, from: payload)?.sequence, 42)
+    }
+
+    // MARK: - Hand-rolled input-event encoder parity
+    //
+    // `makeInputEventPacket` documents this invariant: "emits the same JSON shape
+    // `JSONDecoder` reconstructs into a `TBMonitorInputEvent` (omitted fields
+    // decode as nil)". These tests guard it, since the receiver's snprintf-based
+    // emitter mirrors the same shape.
+
+    private func makeEvent(
+        kind: String,
+        dx: Int? = nil,
+        dy: Int? = nil,
+        scrollX: Int? = nil,
+        scrollY: Int? = nil,
+        keyCode: UInt16? = nil
+    ) -> TBMonitorInputEvent {
+        TBMonitorInputEvent(kind: kind, dx: dx, dy: dy, scrollX: scrollX, scrollY: scrollY, keyCode: keyCode)
+    }
+
+    private func assertEncoderParity(
+        _ event: TBMonitorInputEvent,
+        file: StaticString = #filePath,
+        line: UInt = #line
+    ) {
+        var buffer = TBMonitorProtocol.makeInputEventPacket(event)
+        guard let (type, payload) = try? TBMonitorProtocol.drainPacket(from: &buffer) ?? nil else {
+            XCTFail("packet did not drain", file: file, line: line)
+            return
+        }
+        XCTAssertEqual(type, .inputEvent, file: file, line: line)
+        guard let decoded = TBMonitorProtocol.decodeJSON(TBMonitorInputEvent.self, from: payload) else {
+            XCTFail("payload did not decode as TBMonitorInputEvent: \(String(decoding: payload, as: UTF8.self))",
+                    file: file, line: line)
+            return
+        }
+        XCTAssertEqual(decoded.kind, event.kind, file: file, line: line)
+        XCTAssertEqual(decoded.dx, event.dx, file: file, line: line)
+        XCTAssertEqual(decoded.dy, event.dy, file: file, line: line)
+        XCTAssertEqual(decoded.scrollX, event.scrollX, file: file, line: line)
+        XCTAssertEqual(decoded.scrollY, event.scrollY, file: file, line: line)
+        XCTAssertEqual(decoded.keyCode, event.keyCode, file: file, line: line)
+    }
+
+    func testInputEventEncoderParityMouseMove() {
+        assertEncoderParity(makeEvent(kind: "move", dx: 5, dy: -3))
+    }
+
+    func testInputEventEncoderParityScroll() {
+        assertEncoderParity(makeEvent(kind: "scroll", scrollX: -120, scrollY: 42))
+    }
+
+    func testInputEventEncoderParityKeyDown() {
+        assertEncoderParity(makeEvent(kind: "keyDown", keyCode: 0x24))
+    }
+
+    func testInputEventEncoderParityAllFields() {
+        assertEncoderParity(makeEvent(kind: "drag", dx: 1, dy: 2, scrollX: 3, scrollY: 4, keyCode: UInt16.max))
+    }
+
+    func testInputEventEncoderParityNoOptionalFields() {
+        assertEncoderParity(makeEvent(kind: "leftUp"))
+    }
+
+    func testInputEventEncoderParityExtremeValues() {
+        assertEncoderParity(makeEvent(kind: "move", dx: Int.min, dy: Int.max))
+    }
+}