Cosmos: Implement Binary Round-Trip Fuzzer - #4898
Cosmos: Implement Binary Round-Trip Fuzzer#4898Debdatta Kunda (kundadebdatta) wants to merge 26 commits into
Conversation
…fuzzer implementation Adds BINARY_ENCODING_RFC.md, a normative, self-contained wire-format specification modeled on RFC 8949 (CBOR) and the protobuf encoding guide. Covers the preamble/auto-detection, the full marker taxonomy, scalars (ints/floats/strings incl. system/base64/compressed/GUID forms), containers, uniform number arrays, reference strings, canonical encoding rules, decoder conformance + security requirements, plus golden-vector and worked-example appendices. Wire constants are transcribed from the .NET reference and cross-checked against the Rust codec; details not confirmable from Rust alone are tagged [CROSS-VERIFY: .NET/C++] for a follow-up cross-language pass. Adds CBOR/Signedness to the cspell dictionary. cosmos(perf): add canonicalize+hash binary-encoding round-trip fuzzer Adds a design doc + runnable harness that generates random JSON with a seeded PRNG and validates it survives a live Cosmos round-trip across binary-encoding configs (text control, binary, binary+text-response), comparing a Cosmos-compatible canonical form of sent vs returned. Enables validating millions of distinct JSON structures over a multi-day soak; every run prints its seed for exact reproduction. Canonicalization normalizes numbers to a backend-compatible form (integral floats -> integer, shortest round-trippable decimal otherwise) and computes the sent canonical form from a normalized (one serialize->parse) copy so sent and backend-round-tripped docs are on equal footing - the number-normalization is the documented tuning/calibration surface. Ships 8 offline unit tests (canonicalizer + generator determinism + normalize idempotence) that run without a live account; the live fuzzer is gated behind test_category=binary_encoding. Adds canonicalizer/reparses/trippable to the cspell dictionary. cosmos(driver): add encode-direction conformance tests from binary-encoding RFC Implements the encoder conformance requirements of BINARY_ENCODING_RFC.md (section 7 canonical encoding + Appendix A golden vectors), filling the previously-missing encode side (decoder conformance per section 8 is already covered in reader.rs/de.rs). Adds binary_json/conformance.rs with four tests: (1) every golden-corpus value re-encodes to a buffer that decodes back to itself; (2) the encoder's exact canonical bytes for representative values are pinned as regression snapshots; (3) where the corpus stores a compact form the encoder does not emit (system strings, Arr0/Arr1, NumberUInt8, uniform arrays), the encoder's verbose output differs byte-wise but still round-trips - pinning the intentional valid-subset asymmetry; (4) the encoder always emits the preamble. 4 new tests, binary_json suite now 102 passing. cosmos(driver): cross-link the binary-encoding RFC and round-trip fuzzer docs Adds RFC section 1.4 'Relationship to the other binary-encoding artifacts' with a Mermaid diagram showing the RFC as source of truth and the golden corpus / conformance tests / in-tree fuzz / round-trip fuzzer as derived validators, plus a table mapping each artifact to the RFC sections it enforces. Updates the round-trip fuzzer doc's section 8 to point at the RFC as the normative spec and note that its number-canonicalization calibration feeds the backend rewrite rules back into RFC section 7. cosmos(perf): implement calibration mode for the round-trip fuzzer Adds AZURE_COSMOS_FUZZ_CALIBRATE mode to the binary round-trip fuzzer. It stores a fixed spread of numeric edge cases (NUMBER_PROBES: integral floats, repeating/high-precision floats, large/small exponents, integers near 2^63/2^64, negative zero, trailing zeros) through the binary path, reads them back, and prints a table comparing how canonicalize_number renders each value against the backend's actual returned form. DIFF rows flag number forms the canonicalizer does not yet model, so canonicalize_number can be tuned to the real backend rewrite - closing the design-doc section 3.1 calibration gap that a trustworthy soak depends on. Calibration is a diagnostic (prints the table, does not assert) since a DIFF on the first run is the expected signal to tune. Adds an offline unit test asserting every probe literal parses as a unique JSON number (9 offline tests now). Updates the design doc section 3.1/section 6 to document the implemented mode. cosmos(perf): calibrate canonicalize_number for integers above i64::MAX First live calibration run (18 probes) showed 16/18 number forms already match the backend: integral floats and integral exponents collapse to integers, -0 -> 0, trailing zeros drop, high-precision/0.1+0.2 round-trip exactly, and large/small exponents reparse to the same f64. The two DIFFs were integers above i64::MAX: the backend stores them as IEEE-754 doubles (lossy) and returns scientific notation (18446744073709551614 -> 1.8446744073709552e+19, 2^63 -> 9.223372036854776e+18). Routes u64-above-i64::MAX through f64 in canonicalize_number so a sent u64 and its returned double canonicalize identically; values up to i64::MAX keep exact integer form. Updates the offline test to assert the sent u64 matches the backend double form (and i64::MAX stays exact), and records the calibration findings in the design doc section 3.1. Re-running calibration after this yields all MATCH. cosmos(driver): document how the round-trip fuzzer detects codec gaps Adds section 2.1 'How this detects codec gaps' to the fuzzer design doc: a table mapping the failing config (text vs binary vs binary+text-response) to the broken layer (encoder / decoder / driver transcode / backend-rewrite calibration), the specific gap classes it surfaces that curated tests miss (encoder<->decoder disagreement on backend-only wire forms, number precision edges, unicode/escaping, container framing, the transcode path), the seed-based debugging loop, and its calibration/generator-range limitations. cosmos(perf): give each fuzzer config a distinct item id to avoid 409 conflicts The fuzzer stored one document under all three configs (text-control, binary, binary+text-response) using a single id generated per iteration, so the second config's create_item collided on the (pk, id) key and failed with 409 Conflict (surfacing as 'iter=0 config=binary: create failed: 409'). This was a harness bug, not a codec issue. Generates the document body once per iteration (so all three configs still test the same value three ways) but assigns a distinct id per config, recomputing the sent canonical form per config. Document content stays deterministic from the seed; only the ids differ (they were always random uuids). test(cosmos): add cargo-fuzz codec crate and enrich round-trip generator Phase 6: add an offline, coverage-guided cargo-fuzz crate (azure_data_cosmos_driver/fuzz) that fuzzes the binary-JSON protocol itself with four targets (decode, from_slice, transcode_to_text, decode_reencode_roundtrip) feeding arbitrary/mutated bytes into the codec. It closes the gap the value-space round-trip fuzzer can't: mis-encoded frames the encoder never produces. Isolated from the stable workspace via its own empty [workspace]; nightly/libFuzzer only. Enrich the live round-trip generator so each document is a really complex JSON: a guaranteed _sampler subtree covering every value category (integer, float, alphabetic, alphanumeric, free text, non-ASCII/emoji, boolean, null, number array, nested), plus a new AZURE_COSMOS_FUZZ_BREADTH branching knob, deeper default depth, and a bigger arbitrary-json budget. Add a coverage assertion test, an offline print_sample_documents test, and an AZURE_COSMOS_FUZZ_PRINT knob to pretty-print generated docs. Update the design doc (new section 9.8) and cspell dictionary. test(cosmos-perf): hybrid depth-controlled generator for deep/complex JSON The arbitrary-json-only generator was near-flat (avg nesting depth ~1.3 regardless of max_depth) because arbitrary_iter stops recursing almost immediately. Replace it with a hybrid: an explicit depth-controlled skeleton builds a nested object/array spine to a target depth in [1, max_depth], while every leaf and filler branch is irregular arbitrary-json (random keys/strings/numbers, mixed and uniform-number arrays, nested arrays-of-objects). Numbers/strings still pass through bound_value for the calibrated envelope. Measured avg depth now scales with the knob (~3.9 at 3, ~8.5 at 12; deepest 11-17 levels), exercising the codec container framing and the decoder MAX_DEPTH guard. Adds a generator_depth_scales_with_max_depth regression test; updates design doc 4. 13 offline tests pass, clippy clean. docs(cosmos): record 12,000-round-trip 4-op soak (create/read/replace/upsert) Add the all-four-point-ops soak result: 1000 docs x 3 configs x 4 ops = 12,000 round-trips all canonical-equal (seed 1784944014111583800), confirming replace/upsert round-trip identically to create/read. Keeps the earlier create+read soak for history. test(cosmos-perf): exercise all four binary point ops in the fuzzer (create/read/replace/upsert) The harness previously only drove create + read. Binary encoding is honored for the four body-carrying point ops -- create, read, replace, upsert -- so each config now round-trips all four against the same generated document, widening coverage of the request-encode + response-decode paths for replace and upsert. Progress/DONE counters and the design doc (2, 2.2 loop diagram) updated accordingly. delete (no body) and patch/batch/bulk (deferred) stay excluded. docs(cosmos): add visualized 'how it works' section to the round-trip fuzzer design Adds section 2.2 with four Mermaid diagrams -- the per-document canonicalize/store/compare loop, the three-config layer-localization, the reproduce-reduce-classify debugging loop, and the calibration safety valve -- so readers can grasp how the live fuzzer catches and localizes codec issues at a glance. docs(cosmos): record Phase 5 live validation (18/18 calibration MATCH, 1500 round-trips clean) Calibration against a real account: all 18 number probes MATCH, including every JCS-unsafe edge routed through the string-token workaround. Soak: 500 docs x 3 configs = 1500 round-trips all canonical-equal (seed 1784934026943565900). Marks the first three 9.6 acceptance items done and flips the Phase 5 status to landed. docs(cosmos): record fuzzer crate-refactor status + json-canon safe-integer finding (Phase 5) Add 9.7 implementation-status table (Phases 1-4 landed, Phase 5 live re-calibration manual, Phase 6 deferred), document the json-canon >= 2^53 integer rejection and the string-token workaround, and update 2's hash description from SipHash to SHA-256. test(cosmos-perf): generate documents via arbitrary-json (fuzzer Phase 4) Replace the hand-rolled recursive generator (gen_value/gen_array/gen_string/gen_number/gen_key) with an arbitrary-json-backed generator. The document is produced by feeding a byte buffer -- derived deterministically from the SplitMix64 seed stream -- into arbitrary_json::ArbitraryObject, so the same AZURE_COSMOS_FUZZ_SEED still reproduces the same document. max_depth scales the byte budget (arbitrary stops nesting when the bytes run out). A post-generation bound_value pass preserves the env-knob contract: by default numbers are clamped into the calibrated-safe envelope (design doc 3.2) and strings to ASCII; wide_numbers / unicode widen those. Removed the now-unused SplitMix64 chance/f64_unit helpers. Offline determinism + idempotence tests still pass (12). test(cosmos-perf): canonicalize via json-canon (RFC 8785) + SHA-256 (fuzzer Phase 3) Replace the hand-rolled structural canonicalizer with json_canon (RFC 8785 / JCS) fed by the calibrated normalize_numbers, and switch the differential hash from 64-bit SipHash to SHA-256 (stable across runs/platforms for a durable corpus). Finding: json-canon refuses to emit integers >= 2^53 (u64 must be less than JSON max safe integer). Cosmos preserves i64 integers exactly and stores u64 above i64::MAX as lossy doubles, so normalize_number now maps JCS-unsafe integers (and integral floats >= 2^53) to stable decimal/double string tokens that are only compared for equality, keeping sent and round-tripped values comparable without tripping the JCS safe-integer guard. Offline tests updated; 12 passed. test(cosmos-perf): extract Cosmos-calibrated normalize_numbers (fuzzer Phase 2) Isolate the one number-specific canonicalization step (the calibrated backend number-rewrite rules, design doc 3.1) into a standalone normalize_number / normalize_numbers Value-to-Value transform. This is the piece that must stay under our control because RFC 8785 (JCS) number formatting is not the backend's store-time rewrite; the surrounding structural canonicalization can later be delegated to a JCS serializer. canonicalize_number now delegates to normalize_number and renders the result, keeping the canonical output byte-identical (verified by the existing tests plus a new normalize_number_matches_canonicalize_number probe check). Adds recursive-rewrite and idempotence tests. No live-behavior change. test(cosmos-perf): add fuzzer dev-deps (arbitrary, arbitrary-json, json-canon, sha2) Phase 1 of evolving the binary round-trip fuzzer to use arbitrary-json for generation, json-canon (RFC 8785) for structural canonicalization, and sha2 for durable hashing. Adds the workspace deps and wires them as dev-dependencies of azure_data_cosmos_perf (test-only; crate is publish=false). No behavior change yet.
|
Azure Pipelines: Successfully started running 1 pipeline(s). 2 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
…d-field guard to round-trip fuzzer Add ~24 corpus-shape samplers (SHAPE_SAMPLERS) that reproduce the structure of the local testdata/*.json families — GeoJSON features, embedding vectors, user/telemetry/log records, Cosmos-run metadata, nutrition/food docs, error buckets, legislator/committee records, and more — populated with randomized, seed-reproducible data spanning every datatype (ints, floats, high-precision doubles, alphabetic/alphanumeric/free-text/non-ASCII+emoji strings, booleans, nulls, ISO datetimes, hex hashes, UUIDs). Only shapes are reproduced; no corpus bytes are embedded. Add AZURE_COSMOS_FUZZ_SHAPE_RATIO (default 50) to control the fraction of corpus-shaped vs free-form documents, and AZURE_COSMOS_FUZZ_SIZE_SCALE (default 1) to multiply per-item collection sizes toward corpus-scale payloads. Every shaped document still carries the all-category _sampler subtree. Fix a false round-trip mismatch: strip Cosmos-reserved system properties (_rid/_self/_etag/_ts/_attachments) from every generated document before send (the service owns them), and stop shape_cosmos_run from authoring _self. Add offline tests: every_corpus_shape_produces_a_valid_object, shaped_documents_are_emitted_when_ratio_is_full, no_shape_emits_reserved_system_fields, generated_documents_cover_all_value_categories. Update design doc and cspell dictionary.
…htly leg Add a coverage-guided cargo-fuzz leg to the existing cosmos ci.yml via AdditionalMatrixConfigs (sdk/cosmos/fuzz-matrix.json): one Linux + nightly job, ContinueOnError=true so a discovered crash reports 'succeeded with issues' instead of blocking merge. cargo-fuzz/libFuzzer is Linux-only, so it cannot ride the cross-platform matrix. The job's test-setup hook (Invoke-CosmosTestSetup.ps1, gated on AZURE_COSMOS_FUZZ=1) calls the new Run-BinaryJsonFuzz.ps1, which installs nightly + cargo-fuzz, seeds each target's corpus from the golden vectors, and runs all four binary_json fuzz targets under a wall-clock budget that scales by build reason (~120s/target on PR/CI, ~1800s on the weekly schedule). Crash inputs are published as the fuzz-crashes artifact. Because it rides the already-registered cosmos - ci pipeline, the fuzz job shows up automatically on every cosmos PR. Also document the send-side reserved-field strip in the round-trip fuzzer design doc, and add cspell terms.
|
/azp run rust - cosmos - weekly |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
…the… Add section 8.1 to the round-trip fuzzer design doc: a pipeline diagram plus a side-by-side table distinguishing the two fuzzers (byte-space protocol fuzzer vs. value-space round-trip fuzzer), why both are needed, and a which-to-use-when decision guide. fix(cosmos): remove empty line splitting gen_filler_value doc comment clippy::empty_line_after_doc_comments (Rust 1.95, -D warnings) flagged a stray blank line between two lines of the gen_filler_value doc comment, left by an earlier edit. Join the comment; no behavior change. ci(cosmos): trim weekly fuzz budget to fit the 90-min cap and rename the fuzz leg The first weekly run showed 4 targets x 1800s/target (~120 min) overruns the archetype's 90-min TestTimeoutInMinutes. Drop the weekly per-target budget to 1000s (4 x 1000 + ~5min compile ~= 73 min, comfortably under 90). PR/CI smoke stays 120s/target. Rename the matrix leg display name to binary_json_fuzz_nightly by mapping the AZURE_COSMOS_FUZZ=1 and ContinueOnError=true matrix values to empty display-name segments (was binary_json_fuzz_nightly_1_true). Document a thorough manual Linux-VM fuzzing workflow in the fuzz README.
|
/azp run rust - cosmos - weekly |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
The link-verification CI check does not resolve relative (../) or bare sibling markdown links; the repo convention is absolute https://github.com/Azure/azure-sdk-for-rust/blob|tree/main/... URLs. Convert the 10 relative links across the fuzz README, the round-trip fuzzer doc, and the RFC to that form. Shell/code-block path references are left untouched.
…e markers Audited the .NET reference (Microsoft.Azure.Cosmos/src/Json: TypeMarker.cs, SystemStrings.cs, Numbers.cs, UniformArrays.cs, MultiByteTypeMarker.cs, NodeTypes.cs) against the corpus and added 27 vectors for every decodable wire form that was previously unpinned: wide string/binary/array/object length framings (StrL4, Base64UrlL2, Binary4, ArrL2/L4/LC2/LC4, ObjL2/L4/LC2/LC4), reference strings (StrR1-R4), nested uniform arrays (ArrArrNumC2C2), system-string range boundaries and interior entries, and the remaining uniform number-array item types (Int8, Int64, UInt32, Float64). Excludes intentionally-undecodable forms (Float16, user strings, reserved slots). Validated by decodes_golden_corpus, encode_round_trips_golden_corpus, and corpus_vectors_deserialize_natively (108 passed).
…; make Build-stage fuzz a fast golden-vector check Two Build-stage fixes: 1) account_metadata_503_surfaces_as_status_error was gated only by #[cfg(feature=fault_injection)] and lacked the runtime #[cfg_attr(not(test_category=emulator|emulator_vnext), ignore)] its sibling emulator tests carry. On the non-emulator Build legs it ran unconditionally and panicked with 'AZURE_COSMOS_CONNECTION_STRING is not set but test mode is required'. Add the missing category gate. 2) The cargo-fuzz Build leg ran a multi-minute coverage-guided libFuzzer soak (weekly budget 4x1000s) that timed out the leg. Byte-space value fuzzing belongs on the live/perf path; on Build we only need to prove the committed golden vectors still decode. Add a -ValidateOnly (-runs=0 corpus replay) mode to Run-BinaryJsonFuzz.ps1 and switch the setup hook to it, and strip any test_category cfg (e.g. emulator) injected via COSMOS_RUSTFLAGS on the fuzz leg so the archetype's later cargo test compiles/runs only offline unit tests.
… it runs in Live tests The round-trip fuzzer lived in azure_data_cosmos_perf (publish=false), which is not in the pipeline's tested-crate set (not listed in ci.yml Artifacts / PackageInfo), so it could never run in the Build or LiveTest stages. Move it to azure_data_cosmos/tests/binary_roundtrip_fuzzer.rs -- a crate the LiveTest Public stage already tests, alongside the existing binary_encoding live tests under the same test_category="binary_encoding" gate. Changes: git-mv the test file (history preserved); add a test target (required-features key_auth,fault_injection) and relocate the 4 test-only dev-deps (arbitrary, arbitrary-json, json-canon, sha2) from azure_data_cosmos_perf to azure_data_cosmos; update doc/run-command references. The live test stays #[ignore] unless test_category=binary_encoding + a connection string are provided. Validated: relocated target compiles; 17 offline unit tests pass (2 ignored); perf crate still builds after dev-dep removal.
…g leg Add a 'Session SingleWrite BinaryEncoding' live matrix leg (testCategory='binary_encoding') to live-platform-matrix.json. The bicep emits --cfg=test_category=binary_encoding into RUSTFLAGS and supplies the live connection string, so the relocated binary_encoding_roundtrip_fuzz test (and the sibling binary_encoding item tests) stop being ignored and actually run against a real account. Previously no live leg set this category, so those tests never ran in CI. Set AZURE_COSMOS_FUZZ_ITERATIONS=200 in ci.yml EnvVars to make the live per-run budget explicit and tunable (200 iters x 3 encoding configs x 4 point ops = ~2400 round-trips, bounded to fit the live-test time cap; only consumed on this leg). Document the CI wiring in the round-trip fuzzer design doc. Live tests run on the weekly schedule or when queued with Run live tests.
The Cosmos_binary_json_fuzz leg lived in AdditionalMatrixConfigs, which is part of the Build stage that runs on every PR -- so the cargo-fuzz golden-vector validation ran on the default PR gate. Wrap it in a compile-time conditional (Build.Reason == Schedule OR DefinitionName endsWith '- weekly'), the same condition the archetype uses for RunLiveTests. Net effect: both binary-fuzz surfaces now belong to the weekly pipeline, not the PR gate -- the byte-level cargo-fuzz golden-vector replay (Build stage, weekly) and the live round-trip fuzzer (LiveTest Public, weekly/opt-in). The golden vectors remain covered on every PR by the always-on offline decodes_golden_corpus / encode_round_trips_golden_corpus unit tests, so PR coverage is unchanged.
|
/azp run rust - cosmos - weekly |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
Both binary-fuzz surfaces were easy to confuse: the Build cargo-fuzz leg showed as 'binary_json_fuzz_nightly', which did not convey that it only replays the golden vectors, and it read similarly to the live round-trip fuzzer. Rename the Build (byte-level codec) leg: fuzz-matrix.json displayName + Agent key -> 'binary_codec_golden_vector_validation' (job: binary_codec_golden_vector_validation_nightly); ci.yml matrix Name -> Cosmos_binary_codec_golden_vector_validation. Rename the live leg account setting -> 'Session SingleWrite BinaryEncodingRoundtripFuzz' so the LiveTest job name reads as the round-trip fuzzer. Update the doc reference to the live leg.
… Live Test The archetype appends AdditionalMatrixConfigs to BOTH the Build stage (jobs/ci.yml) AND the Live Test stage (generate-job-matrix.yml), so the cargo-fuzz golden-vector leg was showing up in Live Test Public alongside the round-trip fuzzer. Move it from AdditionalMatrixConfigs to MatrixConfigs, which the archetype consumes only in the Build stage, so it runs Build-only. Keep the weekly conditional. Net separation now matches intent: weekly Build stage = byte-level codec golden-vector validation (fuzz-matrix.json); weekly Live Test stage = value-space round-trip fuzzer (live-platform-matrix.json binary_encoding leg). The vnext-emulator leg is left in AdditionalMatrixConfigs (unchanged; not in scope).
Bump DEFAULT_SHAPE_RATIO 50 -> 85 so ~85% of generated documents follow a real corpus shape (SHAPE_SAMPLERS) and only ~15% are free-form hybrids, biasing the live fuzz toward realistic wire shapes. Pin shape_ratio=0 in the generator_depth_scales_with_max_depth unit test: it asserts the free-form hybrid-skeleton generator's depth scales with max_depth, but corpus shape samplers have fixed sampler-defined depth, so at the new 85%% default they diluted the signal and the avg8 > avg3 + 1.0 assertion failed. Forcing the free-form path makes the test measure exactly what it intends, independent of the production default. 17 offline tests pass.
|
/azp run rust - cosmos - weekly |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
|
Azure Pipelines: Successfully started running 1 pipeline(s). 2 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
There was a problem hiding this comment.
🟡 Not ready to approve
The new harness has feature-gating, reproducibility, retry-safety, configuration, and specification consistency issues.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Pull request overview
Adds Cosmos binary-JSON validation infrastructure without changing production paths.
Changes:
- Adds live seeded round-trip and offline libFuzzer harnesses.
- Expands golden vectors and encoder conformance tests.
- Adds weekly CI wiring and design documentation.
File summaries
| File | Description |
|---|---|
sdk/cosmos/live-platform-matrix.json |
Adds the binary-encoding live leg. |
sdk/cosmos/fuzz-matrix.json |
Defines the nightly fuzz validation job. |
sdk/cosmos/eng/scripts/Run-BinaryJsonFuzz.ps1 |
Runs and seeds fuzz targets. |
sdk/cosmos/eng/scripts/Invoke-CosmosTestSetup.ps1 |
Hooks fuzzing into test setup. |
sdk/cosmos/ci.yml |
Wires weekly fuzz and live runs. |
sdk/cosmos/azure_data_cosmos/tests/binary_roundtrip_fuzzer.rs |
Implements the live value fuzzer. |
sdk/cosmos/azure_data_cosmos/tests/binary_encoding_tests/cosmos_binary_encoding.rs |
Adjusts live numeric coverage. |
sdk/cosmos/azure_data_cosmos/Cargo.toml |
Registers fuzzer dependencies and target. |
sdk/cosmos/azure_data_cosmos_driver/tests/emulator_tests/driver_account_metadata_failover.rs |
Gates an emulator-only test. |
sdk/cosmos/azure_data_cosmos_driver/testdata/binary_json_vectors.json |
Expands binary golden vectors. |
sdk/cosmos/azure_data_cosmos_driver/src/binary_json/mod.rs |
Registers conformance tests. |
sdk/cosmos/azure_data_cosmos_driver/src/binary_json/conformance.rs |
Adds encoder conformance coverage. |
sdk/cosmos/azure_data_cosmos_driver/fuzz/README.md |
Documents byte-level fuzzing. |
sdk/cosmos/azure_data_cosmos_driver/fuzz/fuzz_targets/transcode_to_text.rs |
Fuzzes response transcoding. |
sdk/cosmos/azure_data_cosmos_driver/fuzz/fuzz_targets/from_slice.rs |
Fuzzes streaming deserialization. |
sdk/cosmos/azure_data_cosmos_driver/fuzz/fuzz_targets/decode.rs |
Fuzzes raw decoding. |
sdk/cosmos/azure_data_cosmos_driver/fuzz/fuzz_targets/decode_reencode_roundtrip.rs |
Adds differential fuzzing. |
sdk/cosmos/azure_data_cosmos_driver/fuzz/Cargo.toml |
Defines the isolated fuzz crate. |
sdk/cosmos/azure_data_cosmos_driver/fuzz/.gitignore |
Ignores generated fuzz data. |
sdk/cosmos/azure_data_cosmos_driver/docs/BINARY_ENCODING_ROUNDTRIP_FUZZER.md |
Documents fuzzer design. |
sdk/cosmos/azure_data_cosmos_driver/docs/BINARY_ENCODING_ROUND_TRIP_FINDINGS.md |
Records numeric findings. |
sdk/cosmos/azure_data_cosmos_driver/docs/BINARY_ENCODING_RFC.md |
Adds the draft wire specification. |
sdk/cosmos/.cspell.json |
Adds fuzzing terminology. |
Cargo.toml |
Adds workspace test dependencies. |
Cargo.lock |
Locks added dependencies. |
Review details
- Files reviewed: 23/25 changed files
- Comments generated: 21
- Review effort level: Balanced
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
… fuzzer - require control_plane feature for the fuzzer test target and docs - honor AZURE_COSMOS_FUZZ_UNICODE=false for edge-case strings - handle ambiguous create retry via read-on-conflict - fix duplicated section 3.2, stale CI/crate-name docs - add arbitrary/json-canon/libfuzzer-sys to crates dictionary
…, and breadth clamp - derive item id deterministically from (seed, iter, config) so a seed reproduces exactly (was Uuid::new_v4) - strip non-ASCII from object keys (not just values) when UNICODE=false - clamp breadth into [1, u32::MAX] before the u32 cast to avoid rng.below(0) panic - add regression tests for the ASCII-key and breadth-clamp fixes
…/RFC - RFC section 7: encoder is deterministic-but-verbose, not narrowest (matches conformance.rs) - RFC intro: fix missing noun in the canonical-encoding bullet - roundtrip doc: integers beyond 2^53 (i64 and u64) are lossy doubles, not exact - correct SHAPE_RATIO (85) and MAX_DEPTH (6) defaults - drop unimplemented second-account claim; note it as a planned extension - RFC 8785 orders keys by UTF-16 code units - rename canonicalize_number to normalize_number throughout
There was a problem hiding this comment.
🟡 Not ready to approve
Retry and canonicalization behavior can mask codec failures, and seed-based reproduction currently fails on existing items.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
Comments suppressed due to low confidence (4)
sdk/cosmos/azure_data_cosmos_driver/fuzz/README.md:130
- Like the PowerShell version above, this shell snippet mixes driver-root-relative output (
fuzz/corpus) with fuzz-directory-relative input (../testdata), so it fails or writes to the wrong directory from every possible working directory. Make both paths relative tofuzz/(the directory established by the README).
mkdir -p fuzz/corpus/decode
jq -r '.[] | "\(.name) \(.binary)"' ../testdata/binary_json_vectors.json |
while read -r name hex; do
echo "$hex" | tr -d ' ' | xxd -r -p > "fuzz/corpus/decode/$name"
sdk/cosmos/azure_data_cosmos_driver/fuzz/README.md:121
- This PowerShell snippet has no working directory from which both paths are correct: from
fuzz/,../testdataworks but$dircreatesfuzz/fuzz/corpus; from the driver root,$dirworks but../testdatadoes not. Use paths consistently relative to one documented directory.
This issue also appears on line 127 of the same file.
$dir = "fuzz/corpus/decode"; New-Item -ItemType Directory -Force $dir | Out-Null
(Get-Content ../testdata/binary_json_vectors.json | ConvertFrom-Json) | ForEach-Object {
$bytes = $_.binary -split '\s+' | ForEach-Object { [Convert]::ToByte($_, 16) }
[IO.File]::WriteAllBytes("$dir/$($_.name)", [byte[]]$bytes)
sdk/cosmos/azure_data_cosmos_driver/docs/BINARY_ENCODING_ROUNDTRIP_FUZZER.md:254
- The harness has no
--wide-numberscommand-line option; it only readsAZURE_COSMOS_FUZZ_WIDE_NUMBERS. Following this guidance passes an inert test-harness argument instead of widening generation, so document the actual environment variable.
### 3.2 Generator stays inside the calibrated envelope
To avoid false positives from *un-calibrated* number forms, the generator emits
numbers in **backend-safe ranges by default** (bounded integers, bounded-precision
floats). A `--wide-numbers` flag widens the range once the canonicalizer is
calibrated for those forms — this is how you progressively expand coverage.
sdk/cosmos/azure_data_cosmos_driver/docs/BINARY_ENCODING_RFC.md:502
- This encoder requirement contradicts §7 and the new conformance tests: §7 explicitly permits any deterministic valid form, and
encoder_emits_valid_subset_for_compact_corpus_formsasserts that Rust does not reproduce compact golden bytes. Calling byte reproduction a MUST would incorrectly reject the documented Rust encoder; limit Appendix A's cross-SDK requirement to decoder value parity and implementation-specific snapshots.
The normative, cross-SDK test corpus lives in machine-readable form at
`azure_data_cosmos_driver/testdata/binary_json_vectors.json`. Each entry pairs a
`name`, a spaced-hex `binary` buffer (including the `0x80` preamble), and the
`json` value it decodes to. A conforming decoder MUST reproduce every `json` from
its `binary`; a conforming encoder MUST reproduce the canonical `binary` for
every `json` that is in canonical form (§7).
- Files reviewed: 24/26 changed files
- Comments generated: 2
- Review effort level: Balanced
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
There was a problem hiding this comment.
🟡 Not ready to approve
Seed replay, text-control isolation, comparison completeness, and live-job behavior contain unresolved correctness issues.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
Comments suppressed due to low confidence (4)
sdk/cosmos/azure_data_cosmos_driver/fuzz/README.md:130
- As in the PowerShell snippet above, the output path is relative to the driver root but the input path is relative to
fuzz/, so there is no working directory from which this command seeds the intended corpus. Use the driver-root-relative testdata path consistently.
mkdir -p fuzz/corpus/decode
jq -r '.[] | "\(.name) \(.binary)"' ../testdata/binary_json_vectors.json |
while read -r name hex; do
echo "$hex" | tr -d ' ' | xxd -r -p > "fuzz/corpus/decode/$name"
sdk/cosmos/live-platform-matrix.json:68
- This live matrix entry is not non-blocking as described: it does not set
ContinueOnError, andeng/pipelines/templates/jobs/live.tests.ymlhas nocontinueOnErrorbinding for matrix variables. A fuzzer mismatch will therefore fail the live job/stage rather than report “succeeded with issues.” Wire the live-job template and this entry consistently if non-blocking behavior is required.
"RustToolchainName": ["stable"],
"Account Settings": {
"Session SingleWrite BinaryEncodingRoundtripFuzz": {
"ArmTemplateParameters": "@{ defaultConsistencyLevel = 'Session'; enableAutomaticFailover = $false; testCategory = 'binary_encoding' }"
sdk/cosmos/azure_data_cosmos_driver/docs/BINARY_ENCODING_RFC.md:7
- The “self-contained” implementation claim contradicts this draft’s unresolved content: Appendix D omits 30 system-string entries, and the base64/compressed-string rules remain marked for cross-verification (see open items at lines 593-600). An implementer cannot build a conforming decoder from this document alone yet, so qualify this claim until those normative tables and algorithms are complete.
> This document is a **normative, self-contained** description of the Cosmos
> Binary JSON wire format. A conforming encoder/decoder can be implemented from
> this document alone, without reference to any SDK source. It is modeled on the
sdk/cosmos/azure_data_cosmos_driver/fuzz/README.md:121
- These paths assume different working directories:
fuzz/corpus/decodeis correct from the driver root, while../testdata/...is correct from thefuzz/directory. As written, the snippet fails to read the vectors from the driver root and writes tofuzz/fuzz/corpusfromfuzz/. Keep both paths relative to the driver root, matching the earlier walkthrough.
This issue also appears on line 127 of the same file.
$dir = "fuzz/corpus/decode"; New-Item -ItemType Directory -Force $dir | Out-Null
(Get-Content ../testdata/binary_json_vectors.json | ConvertFrom-Json) | ForEach-Object {
$bytes = $_.binary -split '\s+' | ForEach-Object { [Convert]::ToByte($_, 16) }
[IO.File]::WriteAllBytes("$dir/$($_.name)", [byte[]]$bytes)
- Files reviewed: 24/26 changed files
- Comments generated: 1
- Review effort level: Balanced
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
- is_transient: exclude 500/SERIALIZATION_RESPONSE_BODY_INVALID so a decode
corruption is never retried into a masking 409
- normalize wide numbers to a tagged value ({$__cosmos_wide_number__: token})
instead of a bare string, so a number-to-string codec bug can't pass silently
- recover from 409 Conflict on any create attempt (deterministic ids collide
with prior-run items on seed replay); safe now that decode failures are
non-transient
- add regression test wide_number_token_cannot_collide_with_a_plain_string
|
/azp run rust - cosmos - weekly |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
There was a problem hiding this comment.
🟡 Not ready to approve
The fuzz CI setup can run account-backed tests without credentials, and the text control can unintentionally inherit binary encoding.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
Suppressed comments (4)
sdk/cosmos/azure_data_cosmos_driver/fuzz/README.md:131
- The bash example has the same mixed working-directory paths:
../testdata/...assumes the current directory isfuzz/, whilefuzz/corpus/decodeassumes the driver root. As written it either reads a nonexistent vector file or writes the corpus underfuzz/fuzz, so cargo-fuzz will not use the seeded files.
mkdir -p fuzz/corpus/decode
jq -r '.[] | "\(.name) \(.binary)"' ../testdata/binary_json_vectors.json |
while read -r name hex; do
echo "$hex" | tr -d ' ' | xxd -r -p > "fuzz/corpus/decode/$name"
done
sdk/cosmos/live-platform-matrix.json:69
- The PR states that the live fuzzer leg is non-blocking, but this live matrix entry does not set
ContinueOnError, andeng/pipelines/templates/jobs/live.tests.ymldoes not consume that matrix variable at job level. A fuzzer mismatch will therefore fail the weekly live stage rather than report “succeeded with issues” as described.
"RustToolchainName": ["stable"],
"Account Settings": {
"Session SingleWrite BinaryEncodingRoundtripFuzz": {
"ArmTemplateParameters": "@{ defaultConsistencyLevel = 'Session'; enableAutomaticFailover = $false; testCategory = 'binary_encoding' }"
}
sdk/cosmos/azure_data_cosmos_driver/docs/BINARY_ENCODING_RFC.md:29
- This introductory requirement says all SDKs must agree byte-for-byte, but §7 now explicitly permits different deterministic narrowing choices and warns that cross-SDK byte equality is not generally valid. The normative introduction should require interoperable interpretation rather than identical emitted bytes.
parse than UTF-8 JSON text. The service and every language SDK MUST agree on
this format byte-for-byte.
sdk/cosmos/azure_data_cosmos_driver/fuzz/README.md:121
- These paths cannot be correct from one working directory: from
fuzz/,../testdataresolves correctly but$dir = "fuzz/corpus/decode"creates a nestedfuzz/fuzz/...; from the driver root, the corpus path works but../testdatadoes not. Usecorpus/decodewith../testdatafor the documentedfuzz/working directory.
This issue also appears on line 127 of the same file.
$dir = "fuzz/corpus/decode"; New-Item -ItemType Directory -Force $dir | Out-Null
(Get-Content ../testdata/binary_json_vectors.json | ConvertFrom-Json) | ForEach-Object {
$bytes = $_.binary -split '\s+' | ForEach-Object { [Convert]::ToByte($_, 16) }
[IO.File]::WriteAllBytes("$dir/$($_.name)", [byte[]]$bytes)
- Files reviewed: 24/26 changed files
- Comments generated: 1
- Review effort level: Balanced
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
| $env:RUSTFLAGS = ($env:RUSTFLAGS -replace '--cfg[= ]test_category="[^"]*"', '' -replace '\s+', ' ').Trim() | ||
| Write-Host "Stripped test_category from RUSTFLAGS on fuzz leg: '$env:RUSTFLAGS'" | ||
| } | ||
| return |
| # Test-only deps for the binary round-trip fuzzer | ||
| # (tests/binary_roundtrip_fuzzer.rs): it generates arbitrary JSON, canonicalizes | ||
| # it (RFC 8785), and hashes the canonical form for differential comparison. |
There was a problem hiding this comment.
Unnecessary comment IMO
There was a problem hiding this comment.
Can you separate this into a different PR? I'd like to develop a spec like this, but we should get the backend team engaged in reviewing it to make sure it's correct and not just hallucinated by Copilot :D
There was a problem hiding this comment.
Before GA, I'd like us to run this corpus through a known-good implementation, like the .NET SDK (or even better if we could find a way to share it with the backend team and run it against whatever code they use as a codec). Because the reality is we can only hand-validate it so far :D
| version = "1.0.0" | ||
|
|
||
| [workspace.dependencies] | ||
| arbitrary = { version = "1.4", features = ["derive"] } |
There was a problem hiding this comment.
Could you add a blurb to /CONTRIBUTING.md about fuzz testing and using arbitrary - even just a "Fuzzing" subheading under "Testing" and recommending use of arbitrary with a link to https://docs.rs/arbitrary would be fine. I want to avoid a situation where people keep adding similar but disparate dependencies for what should be shared. Or if you have more that you think other should use, just a brief write-up. Doesn't have to be extensive.
|
This PR is split into two:
Hence closing the original PR. |
Summary
Adds test/validation infrastructure around the Cosmos binary-JSON codec (shipped in #4671). No production code paths change. Two complementary fuzzers plus golden-vector expansion, conformance tests, design docs, and CI wiring.
What's included
1. Live value-space round-trip fuzzer —
azure_data_cosmos/tests/binary_roundtrip_fuzzer.rsAZURE_COSMOS_FUZZ_SEED=<seed>.binary_encodingleg, pinned to ubuntu).2. Byte-level codec fuzzer (cargo-fuzz) —
azure_data_cosmos_driver/fuzz/decode/from_slice/transcode_to_text) plus a decode→encode→decode differential — no network, so coverage guidance applies.fuzz_tests.rs; hardens the protocol itself (mis-encoded frames the encoder never produces).-runs=0golden-vector replay (Linux + nightly, non-blocking); coverage-guided soaks are manual/local.Round-trip fuzzer mechanism
sequenceDiagram participant H as Harness participant Enc as Binary codec participant Svc as Cosmos service H->>H: gen random JSON doc (seed) H->>H: sent_hash = canonical(normalize(doc)) loop create / read / replace / upsert H->>Enc: encode(doc) Enc->>Svc: binary body (+ negotiation header) Svc-->>Enc: binary echo body Enc->>H: decode -> returned doc H->>H: assert canonical(returned) == sent_hash end Note over H: mismatch -> dump seed + both canonical formsTesting
ContinueOnError).