gate OpMeta and InterpreterCallerMeta item conversions on magic - #242
Conversation
Closes #165 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. WalkthroughTyped metadata conversions now validate ChangesMetadata magic validation
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to The PR rejects metadata items whose declared type does not match the requested conversion, preventing cross-type decoding without changing privileges, persistence, deployment, or dependencies. No actionable merge-blocking risk remains beyond normal checks and review. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Full details: Linked Issues checkExplanation The changes satisfy issue Full details: Out of Scope Changes checkExplanation The changes to
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Warning Some tools did not complete. Review the errors below. 🔧 Clippy (1.97.1)Clippy execution failed 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. Comment |
This branch gated `OpMeta` and `InterpreterCallerMeta`. Three of the eight `TryFrom<RainMetaDocumentV1Item>` impls were still ungated, which would have left the crate half checking and half not, with no way for a reader to tell which was deliberate. `AuthoringMeta`, `SolidityAbiMeta` and `JsonAbi` now check too. That is all eight. The magic is not decoration. metadata-v1 specifies it as "a signal of intent for the payload _without_ needing to first parse (and possibly decompress) the entire payload", and cas.md names the alternative as the thing not to do: "it isn't really good enough to assume some data is JSON or otherwise and try to read clues from JSON keys, because by the time the tool has reached that point it has already dedicated resources to parsing". An ungated conversion did exactly that. It read `value.payload` whatever the item said it was, so the only evidence it held an authoring meta was that decoding had not failed - which is evidence of nothing. A metaboard takes any payload under any magic from anyone, so an emitter could pick which type a reader believed it was holding by choosing bytes that parse two ways. Each check runs before the payload is touched, so rejection costs one comparison rather than a parse. The tests use an empty and a trivial payload precisely to pin that ordering: they would not reject on parse failure, only on the label. Deleting the three guards leaves 337 passing and fails exactly `authoring::v1::tests::test_try_from_item_rejects_wrong_magic` and `solidity_abi::v2::tests::test_try_from_item_rejects_wrong_magic`. cargo test --workspace --lib 351 passed 0 failed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014RoHV66knQy6ya7NNkFLpK
|
@coderabbitai assess this PR size classification for the totality of the PR with the following criterias and report it in your comment: S/M/L PR Classification Guidelines:This guide helps classify merged pull requests by effort and complexity rather than just line count. The goal is to assess the difficulty and scope of changes after they have been completed. Small (S)Characteristics:
Review Effort: Would have taken 5-10 minutes Examples:
Medium (M)Characteristics:
Review Effort: Would have taken 15-30 minutes Examples:
Large (L)Characteristics:
Review Effort: Would have taken 45+ minutes Examples:
Additional Factors to ConsiderWhen deciding between sizes, also consider:
Notes:
|
1 participant
Closes #165.
OpMeta::try_from(RainMetaDocumentV1Item)andInterpreterCallerMeta::try_from(RainMetaDocumentV1Item)wereSelf::try_from(value.unpack()?)with no magic check, so an item of any magicwhose payload happened to parse as the target JSON was accepted — including the
two types accepting each other's items, and
unpack_into::<OpMeta>accepting aSolidityAbiV2-magic item.Both now reject a mismatched magic with
Error::InvalidMetaMagic(expected, actual), the same gateDotrainSourceV1andOrderBuilderStateV1alreadyapply, in the same argument order.
QA
meta::types::op::v1::tests::test_opmeta_try_from_item_rejects_wrong_magic,meta::types::interpreter_caller::v1::tests::test_try_from_meta_item_rejects_wrong_magic— each fails on base (verified by reverting its own guard to base behaviour,if false {, and re-running: the matching test FAILED, the other still passed; see mutations M1/M2 below)op/v1.rs:146if value.magic != KnownMagic::OpMetaV1→if false→ killed bytest_opmeta_try_from_item_rejects_wrong_magic;interpreter_caller/v1.rs:69if value.magic != KnownMagic::InterpreterCallerMetaV1→if false→ killed bytest_try_from_meta_item_rejects_wrong_magic;op/v1.rs:147InvalidMetaMagic(KnownMagic::OpMetaV1, value.magic)→ args swapped → killed bytest_opmeta_try_from_item_rejects_wrong_magic(the test asserts expected/actual separately, not justis_err)KnownMagicdoc), and the four sibling typed metas (DotrainSourceV1,OrderBuilderStateV1,RaindexSignedContextOracleV1,AuthoringMetaV2) all gate item conversion on it. Expected error type and argument order come fromDotrainSourceV1/OrderBuilderStateV1and fromerror/mod.rs'sInvalidMetaMagic(expected, actual)rendering test, not from the code under test.OpMetaand onInterpreterCallerMeta, and names the cross-type andSolidityAbiV2repros; both types are gated and both tests coverSolidityAbiV2, the opposite type's magic, andRainMetaDocumentV1.Also run locally (
nix develop -c cargo test -p rain-metadata --lib <filter>):meta::types::op::13 passed,meta::types::interpreter_caller::18 passed,test_unpack_into_whitelist1 passed,meta::normalize10 passed.cargo fmt --checkclean on both files. The full suite was not run locally by request.🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Tests