tests: sharpen the cargo-fuzz oracles across the workspace

ci/release-qualification/pipeline.template.yml

@@ -278,12 +278,9 @@ steps:
           composition: cargo-fuzz
           ci_builder: nightly
           args:
-            - --profile=fruitful
+            - --profile=all
             - --max-seconds=86400
             - --wall-budget=84600
-            # Step hard-times out at 1440min (86400s). --wall-budget ends fuzzing
-            # at 84600s, leaving 1800s; cap minimize at 1200s so the corpus
-            # upload has ~600s of headroom before the kill.
             - --minimize-timeout=1200
             - --corpus-sync
     artifact_paths:

misc/python/materialize/cli/fmt.py

@@ -18,6 +18,12 @@
 from materialize import MZ_ROOT
 from materialize.parallel_task import TaskSpec, run_parallel
 
+# Rust sources live in more than one workspace, and `cargo metadata` only ever
+# reports the one it is pointed at. The `src/*/fuzz` cargo-fuzz crates attach to
+# the `test/cargo-fuzz` workspace, which the root workspace does not include, so
+# without a second invocation here they go unformatted entirely.
+RUST_MANIFESTS = ["Cargo.toml", "test/cargo-fuzz/Cargo.toml"]
+
 
 def main() -> int:
     parser = argparse.ArgumentParser(prog="fmt")
@@ -45,40 +51,55 @@ def _rustfmt_fn(*, check: bool):
     def run() -> tuple[bool, str]:
         ncpus = os.cpu_count() or 8
 
-        result = subprocess.run(
-            ["cargo", "metadata", "--no-deps", "--format-version=1"],
-            capture_output=True,
-            text=True,
-        )
-        if result.returncode != 0:
-            return False, result.stderr.strip()
-
-        meta = json.loads(result.stdout)
         kinds = {"lib", "bin", "bench", "test", "example", "proc-macro", "custom-build"}
-        paths = [
-            t["src_path"]
-            for pkg in meta["packages"]
-            for t in pkg["targets"]
-            if kinds & set(t["kind"])
-        ]
-        if not paths:
+        # Keyed by edition: `gen` is an identifier in 2021 but a reserved keyword
+        # in 2024, so rustfmt cannot even parse a file at the wrong edition.
+        paths_by_edition: dict[str, list[str]] = {}
+        for manifest in RUST_MANIFESTS:
+            result = subprocess.run(
+                [
+                    "cargo",
+                    "metadata",
+                    "--no-deps",
+                    "--format-version=1",
+                    f"--manifest-path={manifest}",
+                ],
+                capture_output=True,
+                text=True,
+            )
+            if result.returncode != 0:
+                return False, result.stderr.strip()
+
+            meta = json.loads(result.stdout)
+            for pkg in meta["packages"]:
+                for t in pkg["targets"]:
+                    if kinds & set(t["kind"]):
+                        paths_by_edition.setdefault(pkg["edition"], []).append(
+                            t["src_path"]
+                        )
+        if not paths_by_edition:
             return True, ""
 
         # Split into batches and run rustfmt in parallel.
-        batch_size = math.ceil(len(paths) / ncpus)
-        batches = [paths[i : i + batch_size] for i in range(0, len(paths), batch_size)]
+        batches = []
+        for edition, paths in paths_by_edition.items():
+            batch_size = math.ceil(len(paths) / ncpus)
+            batches += [
+                (edition, paths[i : i + batch_size])
+                for i in range(0, len(paths), batch_size)
+            ]
 
         cmd_base = ["rustfmt", "--config", "error_on_line_overflow=true"]
         if check:
             cmd_base.append("--check")
 
         procs = [
             subprocess.Popen(
-                cmd_base + batch,
+                cmd_base + [f"--edition={edition}"] + batch,
                 stdout=subprocess.PIPE,
                 stderr=subprocess.STDOUT,
             )
-            for batch in batches
+            for edition, batch in batches
         ]
 
         all_output = []

rustfmt.toml

@@ -1,3 +1,8 @@
 edition = "2024"
+# Formatting style is repo-wide, independent of the edition a crate parses as.
+# The `src/*/fuzz` crates are still edition 2021, and `bin/fmt` overrides
+# `edition` per package so they parse at all. Without this pin they would also
+# fall back to the 2021 *style*, which sorts imports differently.
+style_edition = "2024"
 tab_spaces = 4
 merge_derives = false

src/avro/fuzz/Cargo.toml

@@ -20,6 +20,10 @@ cargo-fuzz = true
 [dependencies]
 libfuzzer-sys = "0.4"
 mz-avro = { path = ".." }
+# Version must match the root `Cargo.toml`'s. `avro_schema_parse` needs a
+# `serde_json::Value` to reach `Schema::parse`, and that value has to be the same
+# type `mz-avro` was compiled against.
+serde_json = "1.0.150"
 
 [[bin]]
 name = "reader_decode"

src/avro/fuzz/corpus.dict

@@ -1,10 +1,16 @@
-# libFuzzer dictionary for the Avro reader_decode target.
+# libFuzzer dictionary for the Avro fuzz targets, resolved crate-wide by
+# `dict_for` in test/cargo-fuzz/mzcompose.py.
 #
 # An Avro object-container file starts with the 4-byte magic "Obj\x01" followed
 # by a map of metadata (notably "avro.schema" and "avro.codec") and a 16-byte
-# sync marker. Without the magic the decoder bails immediately, so the most
-# valuable token by far is the magic itself; the rest let the mutator build
-# plausible headers and schemas.
+# sync marker. Without the magic the decoder bails immediately, so the magic and
+# the schema JSON tokens let the mutator build plausible headers and schemas.
+#
+# These tokens pay off for the targets that consume their input as *bytes*:
+# `avro_schema_parse`, and `reader_decode`'s raw-bytes branch. `reader_decode`'s
+# container-building branch and `schema_resolve` instead read their input as an
+# `arbitrary::Unstructured` recipe of generator choices, where a token is just
+# more choice bytes and carries no structural signal.
 
 # Object-container magic.
 "Obj\x01"

src/avro/fuzz/fuzz_targets/avro_schema_parse.rs

@@ -16,20 +16,39 @@
 //!
 //! `schema_resolve` only parses the narrow set of schemas it generates (no
 //! logical types, no named back-references, shallow), and the decode targets
-//! cap nesting low. This one stresses the parser itself: it generates schema
-//! JSON that can nest *past* the depth limit (so the guard must fire cleanly
-//! rather than overflow the stack) and that re-references already-defined names
-//! (recursive definitions, a distinct resolution path).
+//! cap nesting low. This one stresses the parser itself: schemas that
+//! re-reference already-defined names (recursive definitions, a distinct
+//! resolution path), and nesting that straddles `MAX_SCHEMA_DEPTH`.
+//!
+//! Reaching that guard takes the *other* entry point. `Schema::from_str` runs
+//! `serde_json::from_str` first, and serde_json's own recursion limit is also
+//! 128 containers, so it rejects the schema one level before the avro guard can
+//! fire: the deepest JSON text that survives is 127 wrappers, which is avro
+//! depth 128, exactly at the limit and not over it. So the deep case goes
+//! through `Schema::parse`, which takes an already-parsed `serde_json::Value`,
+//! and the text case is bounded to nesting serde_json will accept rather than
+//! generating JSON no avro code will ever see.
 //!
 //! It also drives the parser's *naming* and *validation* paths:
 //!   * `namespace` fields and dotted/`a.b.C` names. The `FullName::from_parts`
 //!     split logic, including the documented edge case where a name has dots
 //!     *and* a `namespace` is also given (they may disagree).
-//!   * `aliases` arrays on named types, sometimes deliberately colliding with
-//!     another defined name or with the type's own name.
-//!   * structurally-valid-but-semantically-invalid schemas the parser is meant
-//!     to *reject* (not panic on): duplicate enum symbols, duplicate record
-//!     field names, decimal `scale > precision`, and `fixed` `size: 0`.
+//!   * a name that collides with an already-defined type's fullname, which
+//!     `alloc_name` must reject.
+//!   * `aliases` arrays on named types, sometimes colliding with another defined
+//!     name or with the type's own name. NOTE: parsing only *collects* aliases
+//!     into the `Name`, so a collision is indistinguishable from any other alias
+//!     here. Aliases only acquire meaning in `resolve_schemas`, which this target
+//!     does not call. They are generated to keep the attribute shape varied, not
+//!     as a validation path.
+//!   * semantically-invalid schemas, which must produce an error rather than a
+//!     panic: duplicate enum symbols, an enum `default` outside its symbols, and
+//!     `fixed` `size: 0` are all rejected. NOTE: two further shapes generated
+//!     here are *accepted* today, and are generated because the parser has to
+//!     survive them, not because it rejects them. `parse_record` has no
+//!     duplicate-field-name check, so a duplicate silently collapses in `lookup`
+//!     while both entries stay in `fields`; and `parse_bytes` catches a decimal
+//!     `scale > precision` error, logs it, and falls back to plain `bytes`.
 //! Parsing must never panic. It returns `Ok`/`Err`.
 
 #![no_main]
@@ -100,11 +119,22 @@ fn gen_named_attrs(
 /// error rather than panic.
 fn gen_invalid(u: &mut Unstructured, counter: &mut u32) -> arbitrary::Result<String> {
     *counter += 1;
-    Ok(match u.int_in_range(0u8..=4)? {
-        // Enum with duplicate symbols.
-        0 => format!(
-            "{{\"type\":\"enum\",\"name\":\"E{counter}\",\"symbols\":[\"A\",\"B\",\"A\"]}}"
+    Ok(match u.int_in_range(0u8..=5)? {
+        // Two named types sharing a fullname. The names generated elsewhere carry
+        // a strictly increasing counter and so can never collide, which leaves
+        // `alloc_name`'s duplicate-fullname rejection unreachable; this is the one
+        // shape that reaches it. Both twins are declared bare, with no
+        // `namespace`, so the collision does not depend on how the enclosing type
+        // was named.
+        5 => format!(
+            "{{\"type\":\"record\",\"name\":\"Outer{counter}\",\"fields\":[\
+             {{\"name\":\"a\",\"type\":{{\"type\":\"record\",\"name\":\"Twin{counter}\",\"fields\":[]}}}},\
+             {{\"name\":\"b\",\"type\":{{\"type\":\"record\",\"name\":\"Twin{counter}\",\"fields\":[]}}}}]}}"
         ),
+        // Enum with duplicate symbols.
+        0 => {
+            format!("{{\"type\":\"enum\",\"name\":\"E{counter}\",\"symbols\":[\"A\",\"B\",\"A\"]}}")
+        }
         // Record with duplicate field names.
         1 => format!(
             "{{\"type\":\"record\",\"name\":\"R{counter}\",\"fields\":[\
@@ -215,14 +245,60 @@ fn gen_type(
     })
 }
 
+/// Enough array layers to straddle `MAX_SCHEMA_DEPTH` (128) from both sides.
+const MAX_WRAPPERS: u32 = 300;
+
 fn run(mut u: Unstructured) -> arbitrary::Result<()> {
     let mut counter = 0u32;
     let mut defined = Vec::new();
-    // Start deeper than MAX_SCHEMA_DEPTH (128) so the fuzzer can drive nesting
-    // past the limit and exercise the depth guard, not just shallow schemas.
-    let depth = u.int_in_range(1u32..=200)?;
+    // Bounded so the generated text always survives serde_json, which refuses
+    // the 128th nested container. `depth` counts generator levels, not
+    // containers, and a level spends up to three of them (a record is
+    // `{record}` → `[fields]` → `{field}` before recursing), with `gen_invalid`
+    // spending five at the leaf. So a schema nests at most `3 * depth + 2`
+    // containers, and 40 keeps the worst case at 122. Deep nesting is the
+    // wrapped path's job below, which is not subject to this limit at all.
+    let depth = u.int_in_range(1u32..=40)?;
     let schema = gen_type(&mut u, &mut counter, &mut defined, depth)?;
-    let _ = Schema::from_str(&schema);
+
+    // Half the inputs go through the text entry point unchanged. The other half
+    // get wrapped to straddle `MAX_SCHEMA_DEPTH`, which is only reachable behind
+    // `Schema::parse`. Splitting rather than doing both keeps the cost at one JSON
+    // parse plus one schema parse per input, the same as parsing the text alone.
+    let wrappers = if u.int_in_range(0u8..=1)? == 0 {
+        0
+    } else {
+        u.int_in_range(1u32..=MAX_WRAPPERS)?
+    };
+    if wrappers == 0 {
+        let _ = Schema::from_str(&schema);
+        return Ok(());
+    }
+
+    // Every generated schema is well-formed JSON by construction (all
+    // interpolated names are `R<n>`/`E<n>`/`F<n>`/`AnAlias`/`Twin<n>`), so a
+    // parse failure here is a generator bug, and swallowing it would silently
+    // skip the depth-guard half of this target.
+    let mut value: serde_json::Value = serde_json::from_str(&schema)
+        .unwrap_or_else(|e| panic!("generated schema {schema} must be valid JSON: {e}"));
+
+    // `Schema::parse` takes an already-parsed value and so is not subject to
+    // serde_json's limit, which is what makes the guard reachable at all. The
+    // wrapper count falls on either side of it, exercising the accepted depths,
+    // the rejection, and the `self.depth` decrement on the guard's error path.
+    // Built by moving `value` into each new layer. `serde_json::json!` would
+    // deep-clone it instead (its leaf case goes through `to_value`), making the
+    // wrapping quadratic in the layer count.
+    for _ in 0..wrappers {
+        let mut layer = serde_json::Map::new();
+        layer.insert(
+            "type".to_string(),
+            serde_json::Value::String("array".to_string()),
+        );
+        layer.insert("items".to_string(), value);
+        value = serde_json::Value::Object(layer);
+    }
+    let _ = Schema::parse(&value);
     Ok(())
 }