From 4ab3e1a0e206c3c7c7e4b6b0115428f9c333c4ed Mon Sep 17 00:00:00 2001 From: ecgang Date: Sun, 26 Jul 2026 11:46:29 -0700 Subject: [PATCH] Courier vectors: fail the length checks before indexing the bytes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cross-model review (agy) on the previous commit. The length assertions were #expect, so a frame of the wrong size reported the mismatch and then kept going into `unpadded[2]` and the flags-offset reads. Data subscripting past the end traps, so the process would die and take the rest of the run's results with it — the failure mode where you learn least at the moment you need to learn most. They are #require now. Also marks the ciphertextHash vector as the weak one it is. Its derivation has no consumer on main: the function that reads it arrives with the courier-spray work, so the truncation is spelled out in the test rather than called, and a future helper that truncates differently would not fail here. Said plainly in the test rather than left for the next reader to discover. Verified by mutation: declaring the wrong unsignedUnpaddedLength now fails both signing tests with a reported expectation instead of a crash. Co-Authored-By: Claude Opus 5 (1M context) --- .../BitFoundationTests/CourierVectorTests.swift | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/localPackages/BitFoundation/Tests/BitFoundationTests/CourierVectorTests.swift b/localPackages/BitFoundation/Tests/BitFoundationTests/CourierVectorTests.swift index 435a4a6d..eff7db94 100644 --- a/localPackages/BitFoundation/Tests/BitFoundationTests/CourierVectorTests.swift +++ b/localPackages/BitFoundation/Tests/BitFoundationTests/CourierVectorTests.swift @@ -228,6 +228,14 @@ struct CourierVectorTests { } /// The 16-byte envelope identity a spray receipt carries. + /// + /// Weaker than the other vectors, and deliberately so: this derivation has + /// no consumer on main yet — the function that reads it arrives with the + /// courier-spray work. What production surface exists is asserted (the + /// `sha256Hash()` implementation and the `tagLength` constant), but the + /// truncation is spelled out here rather than called, so a future + /// `ciphertextHash` helper that truncates differently would not fail this. + /// When that helper lands it should assert against this same vector. @Test func ciphertextHashIsSHA256TruncatedTo16() throws { let v = try Self.loadVectors() let hash = Data(try Self.hex(v.inputs.ciphertext) @@ -261,11 +269,14 @@ struct CourierVectorTests { let packet = try Self.envelopePacket(from: v) let unpadded = try #require(BinaryProtocol.encode(packet, padding: false)) - #expect(unpadded.count == signing.unsignedUnpaddedLength) + // #require, not #expect: every assertion below subscripts these bytes, + // and `Data` subscripting past the end traps. A short frame has to fail + // the test, not crash the process and take the rest of the run with it. + try #require(unpadded.count == signing.unsignedUnpaddedLength) #expect(unpadded.hexEncodedString() == signing.unsignedUnpadded) let preimage = try #require(packet.toBinaryDataForSigning()) - #expect(preimage.count == signing.signingPreimageLength) + try #require(preimage.count == signing.signingPreimageLength) #expect(preimage.hexEncodedString() == signing.signingPreimage) // The pad byte equals the shortfall to the block boundary, and every @@ -323,7 +334,7 @@ struct CourierVectorTests { var signed = packet signed.signature = Data(a) let wire = try #require(BinaryProtocol.encode(signed, padding: false)) - #expect(wire.count == v.packetSigning.signedWireLength) + try #require(wire.count == v.packetSigning.signedWireLength) #expect(wire.count == v.packetSigning.unsignedUnpaddedLength + v.signature.signatureLength) #expect(wire[BinaryProtocol.Offsets.flags] == (try Self.flagByte(v.packetSigning.flags.signedWirePacket)))