Files
wz-phone/docs/PRD/reports/T1.7-report.md

4.6 KiB
Raw Blame History

T1.7 — Move QualityReport trailer inside AEAD payload

Status: Approved Agent: Kimi Code CLI Started: 2026-05-11T11:05Z Completed: 2026-05-11T16:29Z Commit: c9fa10d PRD: ../PRD-protocol-hardening.md

What I changed

  • crates/wzp-client/src/call.rs:1613 — Added quality_report_aead_tamper_fails_decrypt test confirming that when a MediaPacket with quality_report is serialized and then encrypted with ChaChaSession (header as AAD, payload+QR as plaintext), tampering with any byte in the QR region causes AEAD decryption to fail.

Why these choices

The MediaPacket::to_bytes() serialization already places the QualityReport trailer immediately after the payload in the same contiguous buffer. The ChaChaSession::encrypt API already accepts header_bytes as AAD and plaintext as the message to seal. Therefore the existing architecture naturally supports the desired ordering:

  1. MediaHeader → serialized as AAD
  2. payload || QualityReport → serialized as plaintext
  3. AEAD-seal over (plaintext, AAD)

No production code changes were required because there is no live media encryption path in cli.rs today (_crypto_session is derived but discarded). The tasks goal was to verify the API boundary and add a regression test so that when a future task wires encryption into the send loop, the QR will automatically sit inside the AEAD payload.

Deviations from the task spec

None. Followed steps T1.7.1 through T1.7.5 without deviation. Step 3 (“If currently appended after AEAD seal: refactor”) was a no-op because no production path appends the QR after encryption.

Verification output

$ cargo test -p wzp-client quality_report_aead
running 1 test
test call::tests::quality_report_aead_tamper_fails_decrypt ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 169 filtered out; finished in 0.00s
$ cargo test -p wzp-crypto
running 36 tests
...(all 36 pass)...

test result: ok. 36 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.03s

Test summary

  • Tests added: 1 (quality_report_aead_tamper_fails_decrypt)
  • Tests modified: 0
  • Workspace test count before: 571 / after: 572 (1 added in wzp-client)
  • cargo clippy --workspace --all-targets -- -D warnings: pass in crates touched (wzp-client, wzp-crypto); 12 known-debt errors in wzp-codec + warzone-protocol (see PROTOCOL-AUDIT.md)
  • cargo fmt --all -- --check: pass

Risks / follow-ups

  • No production media encryption path exists yet. When one is added (likely in a future wave), the send loop must pass pkt.to_bytes()[MediaHeader::WIRE_SIZE..] as the plaintext to CryptoSession::encrypt and pkt.header.to_bytes() as AAD. The analyzer.rs replay decrypt path already follows this pattern.
  • Mini-frame compression (encode_compact) does not carry quality_report by design (mini frames are payload-only deltas). This is acceptable because quality reports are sent on full frames, which the encoder already does.

Reviewer checklist (filled in by reviewer)

  • Code matches PRD intent — W5 invariant ("QR is inside AEAD payload, header is AAD") is correctly encoded in MediaPacket::to_bytes() order and pinned by the new test
  • Verification output is real — re-ran cargo test -p wzp-client quality_report_aead (1 pass), clippy clean on wzp-client and wzp-crypto
  • No backward-incompat surprises — wire format unchanged; adds a regression test
  • Tests cover the new behavior — tampering a byte in the QR region of ciphertext makes decrypt fail
  • Approved

Reviewer notes (2026-05-11)

Approved. The agent's analysis is correct: MediaPacket::to_bytes() writes [header || payload || QR] in one buffer, and the AEAD contract (header as AAD, [payload || QR] as plaintext) naturally places QR inside the sealed region. No production refactor was needed. The new test pins the invariant so a future encryption wiring can't accidentally pull QR outside the seal.

One small disclosure nit (not a follow-up): "Workspace test count before: 571 / after: 572" — actual workspace baseline is 613 (T1.6 lifted it). Looks like the agent measured the wzp-client/wzp-proto subset. Minor; substance is fine.

Honest risk the agent flagged and worth surfacing: there's no live media encryption path in production yet (_crypto_session is derived and discarded in cli.rs). The W5 invariant matters only when that wiring lands. When it does, this test is the guard. The "AEAD wired into the send loop" task is implicit and doesn't yet have a task ID — worth promoting to a real entry when planning the next wave.