Reject nul bearing input in str_to_bytes32

[thedavidmeister]

Closes #233.

The defect

str_to_bytes32 right pads with 0u8 and bytes32_to_str ends the string at
the first 0u8. The padding byte and the terminator are the same byte, so a nul
in the input is not representable — but nothing rejected it:

let b = str_to_bytes32("a\0b")?;        // Ok
assert_eq!(bytes32_to_str(&b)?, "a");   // "b" is gone
assert_eq!(str_to_bytes32("a\0")?, str_to_bytes32("a")?);  // collision

The adjudication

#233 offers two: reject embedded nuls, or document the truncation as intended.
Taken as reject, because the alternative is not writable as a true document.

bytes32_to_str cannot change: right padded, nul terminated bytes32 is the
on-chain word encoding these two functions exist to speak (AuthoringMetaV2Sol
carries bytes32 word and its assembly reader depends on that layout), and the
decode side has to read whatever the chain holds. So the only place the
ambiguity can be resolved is on the way in. Documenting it would have to say
that str_to_bytes32 returns a bytes32 that this crate's own decoder reads
back as a different string — which is a defect description, not a contract.

Rejecting is a tightening only. Every input that already round tripped
unchanged is nul free and still accepted; the inputs that now error are exactly
the ones whose result the crate could not read back. "" still maps to the zero
word and still round trips to "".

Neither validated path narrows, either: words go through REGEX_RAIN_SYMBOL
(^[a-z][0-9a-z-]*$) and abi_encode_validate, and no nul survives that. The
unvalidated AuthoringMetaItem::abi_encode is the path that could reach here,
and it is the one that now errors instead of silently truncating.

What changed

• str_to_bytes32 errors on any 0u8 in the input, after the existing length
  check, with a new Error::NulByteInInput ("unexpected nul byte in input") —
  a fixed string variant alongside BiggerThan32Bytes, the other rejection this
  same function makes.
• test_str_to_bytes32_rejects_nul covers the nul in every position (alone,
  leading, embedded, trailing, and trailing in a full 32 byte input), including
  the "a" / "a\0" pair the issue collides.
• test_str_to_bytes32_round_trip pins the property the issue is about:
  everything str_to_bytes32 accepts comes back out of bytes32_to_str
  unchanged, and no two accepted inputs share a bytes32.

Out of scope

bytes32_to_str still accepts a bytes32 carrying nonzero bytes after the
first nul and truncates there. That direction is many to one by construction —
bytes32 has more states than the strings it encodes — and it is the on-chain
data as written, which a decoder has to keep reading.
AuthoringMetaV2::abi_decode open codes the same truncation and is untouched.
#233's two claims are both about str_to_bytes32's domain and both close here.

Relation to the other open PRs on this file

The five open PRs touching crates/cli/src/meta/mod.rs are all in impl Store
or its tests: #239 (update, ~line 821), #254 (set_deployer, ~746), #256
(dotrain removal, ~789-896), #241 (search_deployer, ~680) and #286 (the
Deserialize visitor, ~360). This PR touches the two free functions at ~910-930
and adds tests at ~1290. No hunk overlaps, and Store is untouched here.

crates/cli/src/error/mod.rs is shared with #285 (CorruptRecord,
SubgraphError), #287 (AmbiguousSubject) and #288 (MetaNestingTooDeep),
which all add variants too. NulByteInInput is inserted after
BiggerThan32Bytes and its Display arm after that variant's arm, above where
those three insert, with unchanged lines between — so they append rather than
collide. Whichever lands second may still want a trivial rebase; the variants
themselves are independent.

#253 (#155) tightened REGEX_RAIN_STRING from ^[\s!-~]*$ to
^[\t\n\x0B\x0C\r !-~]*$. It does not reach this code: REGEX_RAIN_STRING
guards description, which is ABI string, not bytes32. It does confirm the
direction, though — nul is outside both the old and the new class, so nothing
that was ever a valid Rain string or symbol is refused by this change.

QA

• Discriminating tests: meta::tests::test_str_to_bytes32_rejects_nul and
  meta::tests::test_str_to_bytes32_round_trip — rejects_nul fails on base,
  verified by mutation 1 below, which restores the base str_to_bytes32 body
  byte for byte; round_trip fails on mutation 2.
• Mutations applied: two, both on crates/cli/src/meta/mod.rs str_to_bytes32,
  each run over cargo test -p rain-metadata --lib -- str_to_bytes32 bytes32_to_str authoring (42 cases).
  1. The new guard deleted, i.e. byte for byte the base function body → 41
     passed / 1 failed, killed only by test_str_to_bytes32_rejects_nul
     (nul bearing input "\0" accepted at mod.rs:1300). Every pre-existing test
     in that set survives it — that is the gap this PR closes.
  2. Guard inverted (if !bytes.contains(&0u8)) → 24 passed / 18 failed, killed
     by test_str_to_bytes32, test_str_to_bytes32_round_trip, the authoring
     v1 encode/decode tests and the get_authoring_meta query tests, so the
     rejection cannot degenerate into refusing valid words.
  • Restored: 42 passed / 0 failed.
• Oracle: the round trip identity the issue states, derived from the encoding
  rather than from the implementation — right padding with the same byte the
  decoder terminates on means the representable strings are exactly the nul free
  ones. Expected values in test_str_to_bytes32_round_trip are the input
  strings themselves, compared after a round trip; the collision check compares
  the produced words to each other, not to anything recomputed by the code under
  test.
• Category check: the issue asks for two things of one unit — no silent data
  loss on the round trip, and no collision between distinct inputs. Covered:
  both hold for every accepted input, and the rejected set is characterised by
  nul position rather than by one example. The issue's third sentence (document
  instead) is answered in "The adjudication" above.

Not run locally: the full test suite. Sibling agents are building in this
tree's neighbourhood, so verification was scoped to the filters above plus
error::tests (12 passed) and cargo fmt --all -- --check (clean); the rest is
left to CI.

🤖 Generated with Claude Code

[coderabbitai]

Warning

Review limit reached

Next included review available in 25 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: e0c38a5f-89ac-407d-ad00-ff1fcadb595b

📥 Commits

Reviewing files that changed from the base of the PR and between c5a1cb0 and 65fb6db.

📒 Files selected for processing (2)

• crates/cli/src/error/mod.rs
• 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.}

[github-actions]

@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:

• Simple bug fixes, typos, or minor refactoring
• Single-purpose changes affecting 1-2 files
• Documentation updates
• Configuration tweaks
• Changes that require minimal context to review

Review Effort: Would have taken 5-10 minutes

Examples:

• Fix typo in variable name
• Update README with new instructions
• Adjust configuration values
• Simple one-line bug fixes
• Import statement cleanup

Medium (M)

Characteristics:

• Feature additions or enhancements
• Refactoring that touches multiple files but maintains existing behavior
• Breaking changes with backward compatibility
• Changes requiring some domain knowledge to review

Review Effort: Would have taken 15-30 minutes

Examples:

• Add new feature or component
• Refactor common utility functions
• Update dependencies with minor breaking changes
• Add new component with tests
• Performance optimizations
• More complex bug fixes

Large (L)

Characteristics:

• Major feature implementations
• Breaking changes or API redesigns
• Complex refactoring across multiple modules
• New architectural patterns or significant design changes
• Changes requiring deep context and multiple review rounds

Review Effort: Would have taken 45+ minutes

Examples:

• Complete new feature with frontend/backend changes
• Protocol upgrades or breaking changes
• Major architectural refactoring
• Framework or technology upgrades

Additional Factors to Consider

When deciding between sizes, also consider:

• Test coverage impact: More comprehensive test changes lean toward larger classification
• Risk level: Changes to critical systems bump up a size category
• Team familiarity: Novel patterns or technologies increase complexity

Notes:

• the assessment must be for the totality of the PR, that means comparing the base branch to the last commit of the PR
• the assessment output must be exactly one of: S, M or L (single-line comment) in format of: SIZE={S/M/L}
• do not include any additional text, only the size classification
• your assessment comment must not include tips or additional sections
• do NOT tag me or anyone else on your comment