reject duplicate cbor map keys on meta item decode

[thedavidmeister]

Closes #191.

The defect

RainMetaDocumentV1Item's Deserialize visitor assigned each recognised cbor
map key with a plain overwrite, so a map repeating a key decoded successfully
with last-key-wins. RFC 8949 §5.6 makes such a map invalid, and two
byte-distinct documents decoding to the same item matters wherever the keccak
hash of the encoded bytes is the identity (metaboard subjects) while consumers
act on the decoded item.

The issue's repro reproduced on main (45ca96c) exactly as filed:

RESULT: Ok([RainMetaDocumentV1Item { payload: [2], magic: DotrainV1, ... }])

Why a guard in the visitor was not the whole fix

cbor_decode did not decode into the item directly — it deserialized each
element into a serde_cbor::Value first and then ran from_value.
serde_cbor::Value::Map is a BTreeMap, so the duplicate was collapsed to
last-wins before the visitor ever ran. Probed on the repro bytes:

VALUE: Map({Integer(0): Bytes([2]), Integer(1): Integer(18436262373317404820)})

So a duplicate-key guard in the visitor with the Value hop still in place
leaves cbor_decode returning Ok — measured below, not assumed.

The change

• cbor_decode deserializes RainMetaDocumentV1Item straight off the
  serde_cbor::Deserializer stream instead of through Value + from_value.
  serde_cbor forwards deserialize_map to deserialize_any/parse_value, so
  this is the same parse path the Value hop used (transparent tag handling
  included) — it just no longer discards the duplicate before the visitor sees
  it, and it drops an intermediate allocation.
• The visitor assigns through a set_once helper that returns
  serde::de::Error::duplicate_field on a second sighting of any of keys 0–4
  or the OaSchema magic key.

The error surface is unchanged in kind: the rejection arrives as
Error::SerdeCborError, the same variant the missing-key and unknown-magic
rejections already use.

cargo test -p rain-metadata (lib 310, doc 2, integration 15) green, cargo fmt --all --check and cargo clippy -p rain-metadata --all-targets -D warnings
clean, pre-commit hooks pass on the changed file. The repo-wide suite was not
run locally — this machine is running many concurrent agents.

QA

• Discriminating tests: meta::tests::test_cbor_decode_duplicate_payload_key_errors,
  meta::tests::test_cbor_decode_duplicate_any_key_errors — each fails on base
  (the issue's repro bytes were run against unmodified main 45ca96c and printed
  RESULT: Ok([... payload: [2] ...]), so the unwrap_err() both tests start
  from panics there; confirmed again by mutation 2 below, which restores the base
  decode path byte for byte and fails both tests).
• Mutations applied:
  • crates/cli/src/meta/mod.rs set_once guard: if slot.is_some() →
    if false && slot.is_some() (duplicate key silently overwrites again) →
    killed by test_cbor_decode_duplicate_payload_key_errors AND
    test_cbor_decode_duplicate_any_key_errors (0 passed; 2 failed).
  • crates/cli/src/meta/mod.rs cbor_decode loop: Self::deserialize(&mut deserializer) → base's serde_cbor::Value::deserialize + from_value
    (set_once left intact, so the visitor guard is present but unreachable) →
    killed by the same two tests (0 passed; 2 failed). This is the mutation that
    proves the fix needs both halves; the other 50 meta::tests pass under it.
• Oracle: RFC 8949 §5.6 ("a map that has duplicate keys may be well-formed, but
  it is not valid") for the verdict, and the metadata-v1 header alias table for
  which keys are recognised (0–4 plus the OaSchema magic). Every test input is
  handwritten cbor — map header 0xa0 | n, byte string 0x41, text string
  0x60 | len, 0x1b + 8 big-endian magic bytes — assembled by
  handwritten_entries/handwritten_magic/handwritten_text and never by
  cbor_encode, which cannot emit a repeated key at all. Expected decode results
  are named literally (payload [0x01], DotrainV1, ContentType::Cbor,
  ContentEncoding::Identity, ContentLanguage::En, schema "hi").
• Category check: the issue asks one question — reject duplicate map keys, or
  accept last-wins? Answered as reject, on the RFC 8949 §5.6 authority the issue
  itself names as the intent oracle; the "flagged rather than adjudicated" triage
  note is therefore adjudicated here rather than deferred, and is the one call in
  this PR worth a reviewer's disagreement. Covered for every recognised key, not
  just the key 0 in the repro: each of 0, 1, 2, 3, 4 and the OaSchema
  magic key is repeated in turn, once with a differing value and once with an
  identical value (§5.6 does not care whether the values agree). The same map
  without any repeat is asserted to decode into the expected item first, so
  "everything errors now" cannot pass these tests.

Overlap with other in-flight work

#186, #188, #190 and #203 all propose changes to the same cbor_decode region,
so they will conflict textually with this one and want rebasing rather than
merging blind:

• cbor_decode rejects entire documents containing unknown-magic items; spec expects unsupported metas to be droppable/ignorable #186 (drop unsupported-magic items rather than rejecting the document) and
  Items missing mandatory cbor keys 0/1 abort the whole decode; spec says they MUST be treated as unexpected and dropped/ignored #188 (drop items missing keys 0/1) both change where a per-item error goes.
  With the Value hop removed, that decision now lives at the single
  Self::deserialize call site instead of being split across the Value parse
  and from_value, which should make either change simpler, not harder.
• cbor_decode corrupt-meta guard carries unreachable sub-conditions (eof offset equality, track/metas length) — equivalent-mutant evidence #190 (unreachable sub-conditions in the corrupt-meta guard) touches the
  if metas.is_empty() || ... block directly below the loop this PR edits. This
  PR changes neither that guard nor the reachability of its arms: track and
  metas are still pushed together on the same success path.
• cbor_decode accepts input without the rain meta document prefix; spec says tooling MUST discard data not beginning with the magic number #203 (require the rain meta document prefix) touches the prefix branch
  above the loop, untouched here.
• KnownMagic is missing Web data v1 (0xff5dcce9b571ba42) listed in the metadata-v1 spec magic number table #187 (missing Web data v1 magic) is magic.rs only — no overlap.

rainix-rs / static / rs-static fails repo-wide on an unrelated rainix hook bug.

[coderabbitai]

Warning

Review limit reached

Next included review available in 4 minutes.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: df423d18-0fbd-49ae-ad24-f1a02e723b5c

📥 Commits

Reviewing files that changed from the base of the PR and between 45ca96c and 70e313f.

📒 Files selected for processing (1)

• crates/cli/src/meta/mod.rs

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}