Parse Casework review request bodies with parse_json_strict

Open
#1,209 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
3/5
Estimated time
1-2 days
Newbie friendliness
72/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Active
Tech stack
rust

Research direction

Start with the create_review_request and decide handlers in crates/registry-casework/src/http.rs, then read parse_json_strict in crates/registry-platform-canonical-json/src/lib.rs and the accountability digest path in review.rs. Compare the response-side parsing in crates/registry-review-client/src/client.rs and add an endpoint regression test covering the effective depth guard. Done means both request paths use strict parsing and the test pins the intended behavior.

Written by the indexing model from the issue text.

Description

area:casework bug rust

Casework's review HTTP handlers parse request bodies with plain axum Json extractors instead of the workspace's strict parser. As of PR #1189, create_review_request (crates/registry-casework/src/http.rs:307-310) and decide (crates/registry-casework/src/http.rs:940-944) both use Json<T> directly; registry_platform_canonical_json::parse_json_strict appears nowhere in registry-casework's http.rs or review.rs, even though the decide path canonicalizes and SHA-256s the caller's result into an accountability digest (crates/registry-casework/src/review.rs:3858-3863). The review client already uses the strict parser on the response side (crates/registry-review-client/src/client.rs:340).

Failure scenario: a request body with a duplicate JSON key (e.g. two "result" members) is accepted by serde_json's default last-key-wins behavior, so the value that gets validated against the outcome schema is not necessarily the same value a naive downstream consumer or log line would show for the first occurrence, silently diverging from what an auditor might expect the recorded decision to be.

Also relevant: parse_json_strict (crates/registry-platform-canonical-json/src/lib.rs:33-36) documents that "the caller remains responsible for bounding bytes before parsing" and has no depth limit of its own, relying on serde_json's built-in default recursion limit (128). And the 16 KiB / 64 KiB canonical-byte bounds enforced in crates/registry-casework-core/src/review.rs (e.g. MAXIMUM_REVIEW_RESULT_BYTES, MAXIMUM_REVIEW_SCHEMA_BYTES) are measured on canonicalized bytes, so a raw wire payload using \uXXXX escapes can be meaningfully larger than the bound while still passing it after canonicalization; it's only bounded overall by the outer request body limit.

Suggested fix:

  • Route review create/decide bodies through parse_json_strict before deserializing into the closed request types, matching the response-side client.
  • Add a regression test that pins the effective parse-time depth guard for these endpoints.

Filed from the review of #1189; not merge-blocking.

Dominant language
Rust
Stars
2
Forks
0
Avg merge
3h 45m
Merged PRs (30d)
134

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from registrystack/registry-stack

All issues in registrystack/registry-stack

Similar issues

More Rust issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.