test: improve test quality

[arkavo-com]

Summary

Improves test quality by re-enabling a previously disabled test, adding negative parser coverage, cleaning up debug prints, moving shared test helpers, and adding an offline mock-KAS rewrap test.

Changes

• Re-enabled testVerifyPolicyBinding in KASServiceTests.swift and fixed it to pass the serialized embedded policy body.
• Added BinaryParserTests.swift with negative tests for invalid magic numbers, unsupported versions, truncated headers, and malformed resource locators.
• Replaced debug print calls in unit tests with meaningful assertions in:
  • NanoTDFTests.swift
  • NanoTDFCreationTests.swift
  • InitializationTests.swift
  • KeyStoreTests.swift
• Moved CryptoHelper test helpers from CryptoHelperTests.swift to a new Helpers/CryptoHelper+TestHelpers.swift file.
• Added offline mock-KAS rewrap test (testRewrapKeyMockResponse) using MockURLProtocol.
• Fixed a GMAC binding bug discovered while re-enabling the policy-binding test: createGMACBinding and verifyPolicyBinding now use a deterministic zero nonce so bindings can be reproduced and verified.
• Fixed KASServiceBenchmarkTests to use the same deterministic binding for the policy-binding benchmark and assert validity before measuring.

Verification

• swift test: 227 tests, 0 failures
• swiftformat --swiftversion 6.2 .: clean

Related

Part of the test-quality quick-wins identified in recent review feedback.

[gitar-bot]

Code Review ✅ Approved 2 resolved / 2 findings

Enhances test suite reliability by re-enabling policy binding tests, improving negative parser coverage, and replacing debug statements with assertions. The previously identified tautological assertions and GMAC nonce issues have been resolved.

✅ 2 resolved

✅ Quality: Tautological assertion compares header.toData() to itself

📄 OpenTDFKitTests/NanoTDFCreationTests.swift:35
In NanoTDFCreationTests.swift:35, XCTAssertEqual(header.toData(), header.toData(), "Header should serialize consistently") compares a value against itself, so it can never fail and verifies nothing. The apparent intent was to confirm the parsed header re-serializes to the same bytes as the original. Capture the original serialized header before parsing and compare against that, or rely on the existing full round-trip assertion at line 38 and remove this line.

✅ Security: Fixed zero nonce in createGMACBinding relies on per-message key uniqueness

📄 OpenTDFKit/CryptoHelper.swift:220-227
createGMACBinding (CryptoHelper.swift:223-224) now seals with a deterministic all-zero AES-GCM nonce, and the same symmetric key is also used to encrypt the payload via encryptPayload with a separately generated nonce (confirmed in NanoTDFCollectionBuilder). This change is necessary for reproducible/verifiable bindings and is safe as long as each NanoTDF derives a unique symmetric key (from a fresh ephemeral ECDH key) and the random payload nonce never collides with the zero nonce under the same key. AES-GCM nonce reuse under a shared key is catastrophic (authentication-key recovery / forgery), so this invariant must be preserved. Recommend adding a code comment documenting this assumption and ensuring ephemeral key pairs are never reused across NanoTDFs, so a future refactor (e.g., key caching) cannot silently reuse a key with both the zero GMAC nonce and a colliding payload nonce.

_{Was this helpful? React with 👍 / 👎 | Gitar}