feat(fuzz): fuzz storage services via nisshi-sans-io's new arbitrary feature

[shortishly]

Add an arbitrary cargo feature to nisshi-sans-io: build.rs now derives Arbitrary on every generated public message type, using #[arbitrary(with = ...)] for bytes fields (a foreign type we can't impl Arbitrary for directly) and #[arbitrary(default)] for records fields (whose crc/batch_length invariants arbitrary generation would violate).

Add 23 fuzz targets (fuzz/fuzz_targets/fuzz_*_storage.rs), one per non-transaction broker service, that build a fuzzed request directly via Arbitrary and run it through the real nisshi_storage::*Service against a fresh in-memory backend, failing on panics or any non-Error::Api result. The targets run in the nightly tier of fuzz/ci-targets.json (15 minutes each; see fuzz/README.md), and locally with just fuzz-ci nightly <target>.

Fix bugs the above found:

• list_offsets: invalid isolation_level/timestamp leaked Error::SansIo; now map to per-response/per-partition ErrorCode (InvalidRequest / InvalidTimestamp, scoped to how broadly each field applies). Rejecting a bad isolation_level with INVALID_REQUEST deliberately differs from Kafka, which answers UNKNOWN_SERVER_ERROR. Timestamps -3, -4 and -5 (ListOffsets v7-v9) are not supported yet and also answer INVALID_TIMESTAMP (follow-up: feat(list_offsets): support timestamps -3 (max), -4 (earliest local) and -5 (latest tiered) #952)
• fetch: invalid isolation_level/max_wait_ms/min_bytes/max_bytes leaked internal errors; they are now parsed once by ValidatedFetchRequest (TryFrom<FetchRequest>), which FetchService takes instead of a raw FetchRequest. FetchValidationService converts the request and maps a malformed one to the top-level ErrorCode::InvalidRequest, mirrored onto every partition for pre-v7 clients
• alter_user_scram_credentials: an unrecognized SASL mechanism leaked Error::SansIo; now maps to a per-item UNSUPPORTED_SASL_MECHANISM with Kafka's "Unknown SCRAM mechanism" message, for deletions and upsertions
• create_topics: a topic with manual assignments and an explicit num_partitions or replication_factor is now rejected with INVALID_REQUEST (replication factor checked first, with Kafka's messages), because storage creates num_partitions partitions regardless of the assignments, bypassing the partition cap. Both counts must be -1 beside an assignment
• create_acls: now answers one SECURITY_DISABLED result ("No Authorizer is configured.") per creation, in order, instead of an empty response, which desynced clients. DescribeAcls still succeeds with no ACLs, where Kafka returns SECURITY_DISABLED
• The CreateTopics and Fetch validation layers carry the request's Extensions to the wrapped service
• TopicId::from(&FetchTopic) / TopicId::from(&MetadataRequestTopic) panicked when given neither a name nor a topic id; now falls back to the NULL_TOPIC_ID sentinel already used elsewhere for "no such topic"

Known gap left unfixed: delete_records is an unconditional todo!() in nisshi-storage-dynostore and in the libSQL (nisshi-storage-sql/src/lite.rs) and turso (limbo.rs) backends, so a DeleteRecords request panics on memory://, s3:// and sqlite:// brokers, and its fuzz target crashes on the first execution. It is in no tier of fuzz/ci-targets.json, so CI never runs it.

Operator-visible changes: Fetch, ListOffsets and AlterUserScramCredentials answer invalid fields with an error code instead of closing the connection; CreateTopics rejects manual assignments with explicit counts; CreateAcls answers SECURITY_DISABLED per creation.

What does this change do, and why?

Makes the protocol layer fuzzable and fixes the error-mapping and panic bugs the fuzz targets found (see above).

How was this tested?

just ci then just test: 1109 nextest tests and all doc tests pass.

Checklist

• Commits are signed off (git commit -s) — see CONTRIBUTING.md
• just fmt, just clippy, and just test pass locally
• Tests added or updated for the behavior changed
• Docs updated if user-facing behavior changed

[sgamelin]

Approving. Deriving Arbitrary in build.rs is a neat way to cover every message type at once, and the fixes turn several client-triggered connection drops into proper error codes.

One thing to fix before merging: fetch.rs:452 sends a frame clients can't decode (details inline). It's a one-line change.

Worth adding here or in a follow-up: regression tests for the four fixes. just test-workspace excludes fuzz and no CI job runs the targets, so reverting any of the fixes today fails no test. nisshi-broker/tests/it/storage_list_offsets.rs and storage_fetch.rs already run against multiple backends and look like the natural home. For example:

• ListOffsets with one valid and one invalid timestamp in the same topic: the valid partition still resolves, the other gets INVALID_TIMESTAMP, and both sit under one topic entry.
• ListOffsets with isolation_level = 7: every partition gets INVALID_REQUEST.
• Fetch with a bad isolation_level or a negative min_bytes: top-level INVALID_REQUEST, and the response encodes and decodes.
• AlterUserScramCredentials with one bad and one good mechanism: the bad one gets UNSUPPORTED_SASL_MECHANISM and the good one is stored.

A smaller gap for later: Records is nullable, so #[arbitrary(default)] makes it None and fuzz_produce_storage never reaches the batch-handling code in ProduceService. Building batches with the existing encoder, which computes the CRC, would open that path up.

[shortishly]

806: Fuzzing support for the protocol layer

I've tried to cover most of the feedback above (thanks for al that!) - the main thing that is missing are error codes on earlier version FetchResponses: I'm wanting to try putting "validation" as a separate service/layer for each API rather than mixing it into the actual service.

I've also made (possibly) overlapping changes with #823, the fuzzing has surfaced several unbounded allocation issues in parts of the decoding.

I'm leaving this PR as draft for the moment.

Summary

Adds fuzz testing for nisshi-storage's per-API services and the
nisshi-sans-io wire codec, and fixes everything it found — including a
remote, pre-authentication memory-exhaustion DoS.

What's new

• arbitrary feature on nisshi-sans-io (build.rs): every generated
  public request/response type now derives Arbitrary, so a fuzzer can
  construct a well-typed message directly from raw bytes without going
  through the wire format first.
• 24 new fuzz targets (fuzz/fuzz_targets/fuzz_*_storage.rs), one per
  non-transaction broker service: each builds a fuzzed request via
  Arbitrary, round-trips it through nisshi-sans-io encode/decode, runs
  it through the real nisshi_storage::*Service against a fresh in-memory
  backend, and round-trips the response too.
• Shared harness (fuzz/src/lib.rs): check_request/check_response
  helpers used by every target (see "Harness fix" below for how their
  comparison strategy changed).
• just fuzz-all <seconds>: runs every fuzz target in turn for
  seconds each, stopping at the first crash.
• Known, deliberate gap: fuzz_delete_records_storage still crashes on
  first execution — delete_records is an unconditional todo!() in
  nisshi-storage-dynostore (a missing implementation, not an
  error-mapping bug), excluded from fuzz-all and left for follow-up work.

Bugs found and fixed

1. Error leaks on malformed input (list_offsets, fetch,
   alter_user_scram_credentials): an invalid isolation_level,
   timestamp, max_wait_ms/min_bytes/max_bytes, or unrecognized SASL
   mechanism leaked an internal Error::SansIo instead of the correct
   per-field/per-response Kafka ErrorCode. Also fixed a panic in
   TopicId::from when a FetchTopic/MetadataRequestTopic has neither a
   name nor a topic id.
2. CreateAcls/Fetch stub responses: CreateAcls returned an empty
   response instead of a per-creation SecurityDisabled result; Fetch
   returned responses: None instead of an empty list on an early
   InvalidRequest.
3. Decoder treated array elements as individually nullable
   (nisshi-sans-io/src/de.rs): a nullable []string field's
   element-decoding reused the array's own nullability metadata, so the
   first string in a non-empty, non-null array (e.g.
   DescribeConfigsRequest.ConfigurationKeys) failed to decode
   (StringWithoutLength). Fixed by gating on the existing
   in_seq_of_primitive flag.
4. Remote pre-auth OOM DoS (nisshi-sans-io/src/primitive/tagged.rs):
   a tagged field's byte-length varint — read straight off the wire, before
   any authentication — drove an unbounded Vec::with_capacity, so a
   single ~3.5GB length claim aborted the process. Fixed by capping the
   up-front allocation (4KB) rather than trusting the claimed length;
   legitimate larger payloads still decode correctly via incremental
   growth.
5. Mandatory sequence fields left at None encoded as nothing
   (nisshi-sans-io/src/ser.rs): every sequence field is generated as
   Option<Vec<T>> regardless of whether it's actually nullable on the
   wire, so a mandatory array left unset (e.g.
   DescribeUserScramCredentialsResponse::default()) wrote no length
   prefix at all, desyncing every field after it. Fixed by writing a
   zero-length array marker instead.
6. Nullable struct fields missing their presence marker
   (nisshi-sans-io/src/ser.rs): per
   KIP-893,
   a nullable struct must be preceded by a presence marker whether it's
   null or populated. The encoder only wrote the marker for the null
   case, so a populated Cursor/Assignment-shaped field (e.g.
   DescribeTopicPartitionsRequest.Cursor) desynced the stream. Currently
   latent against real traffic — NextCursor is hardcoded None and the
   KIP-848/KIP-932 heartbeat APIs aren't implemented yet — but fixed ahead
   of whichever of those lands next.
7. Fuzz harness false positives (fuzz/src/lib.rs): check_request/
   check_response asserted the first decode matched the pre-encode
   value, which is too strict whenever the original value isn't already in
   its wire-canonical shape — a service populating fields across several
   API-version eras at once (FindCoordinatorResponse's v0-3 scalars
   alongside its v4+ coordinators; MetadataResponse's
   cluster_authorized_operations), or bug 5's None-vs-Some(vec![]).
   Neither is a bug. Fixed by comparing a second round-trip against the
   first instead of against the original value — the actual round-trip
   property a sans-io codec has to satisfy.

Verification

• All 27 fuzz targets (excluding the known delete_records gap) run their
  full 10s budget clean — no panics, no libFuzzer errors, no OOMs.
• Every fix has a regression test (round-trip tests in nisshi-sans-io's
  tests/it/, confirmed to fail without the fix and pass with it).
• cargo fmt --all --check, cargo clippy --workspace --all-features --all-targets -- -D warnings, and the full nisshi-sans-io /
  nisshi-storage nextest suites are all clean.

[sgamelin]

I looked at the four commits from 5 Oct. The per-partition mirror matches Kafka's FetchRequest.getErrorResponse for pre-v13 clients, which fixes what I raised on the old line 452. The marker refactor is correct as well. KIP-893's presence marker is an int8 in every version, so that branch never did anything.

I left three comments inline. The first matters most, because it covers the layer pattern before it spreads to other APIs.

Also: main now has CHANGELOG.md. The decoder and error-code fixes here change what clients see, so they probably want entries.

[solace-aross]

Thanks for the iteration on the fetch validation layer. I read the sans-io changes (de.rs, ser.rs, tagged.rs) and the storage fixes; I did not run them locally, so treat the points below as read-only review.

What reads correct:
• is_nullable gated on in_seq_of_primitive: the sequence's nullability no longer leaks onto its elements.
• Tagged-field preallocation capped at 4096: the claimed length no longer drives with_capacity. The loop still stops at the end of the buffer ('byte' error), so allocation is bounded by input size. Removing the old length > 128 check also means tagged data can now be as large as the frame, so this relies on the pre-auth frame limit from #823 staying in place.
• Mandatory sequence at None encodes as an empty array, and the KIP-893 marker is written for a populated nullable struct.

Blocking merge, not the design:
• The branch conflicts with main in CHANGELOG.md and nisshi-storage/src/service/list_offsets.rs. #823 touches the same decode limits, so please re-check the tagged.rs change still makes sense on top of it after the rebase.
• zizmor is red on this head. Main has had a red push run recently as well, so it may not be yours; worth confirming after the rebase.

Sam has the layer-pattern question covered. I will re-review after the rebase.

[sgamelin]

Approving. Every thread from the earlier rounds is addressed, and ValidatedFetchRequest is the shape I was hoping for. I ran the sans-io and storage suites and the broker tests for the changed APIs locally, and they all pass.

None of the inline comments block the merge. Two things for the description, since it becomes the squash commit body:

• It doesn't mention the stricter CreateTopics check (manual assignments plus explicit counts) or the CreateAcls change.
• delete_records is a todo!() in the libSQL (lite.rs#L2517) and turso (limbo.rs#L1236) backends too, not only in dynostore.

#939 adds a CI workflow for the fuzz targets, and the two PRs overlap:

• Its fuzz/ci-targets.json lists only the five decode targets, so once this lands, fuzz-ci warns about the 23 new *_storage targets. fuzz_delete_records_storage needs to stay out of every tier until delete_records is implemented, because it panics on its first input.
• cargo fuzz build with no target builds every bin. #939 measured about 33 minutes for five targets against a 50 minute PR job timeout, so the storage targets need a tier decision rather than all going into pr.
• This PR's CLAUDE.md says the fuzzers are "not run in CI", and fuzz-all overlaps with #939's fuzz-ci. Whichever PR merges second will need to reconcile both.

[sgamelin]

Approving again. 6586c75 addresses everything from my last review:

• The Fetch and CreateTopics validation layers carry Extensions through, so the pattern is safe to copy to Produce.
• CreateTopics, AlterUserScramCredentials and CreateAcls send Kafka's messages. CreateTopics checks the replication factor first and logs the rejection.
• ListOffsets logs one warn! per request and says why INVALID_REQUEST differs from Kafka.
• New tests cover the SCRAM deletion branch, CreateAcls with 0, 1 and 3 creations, and the tagged-field preallocation cap.
• The description now covers the CreateTopics and CreateAcls changes and the libSQL and turso delete_records stubs.

Two smaller points are still open, and neither blocks: the harness suggestions (fuzzing across versions, comparing encodes byte for byte), and a note that rejecting a negative max_wait_ms (validation.rs#L106) is stricter than Kafka. Both are fine as follow-ups.

Before merging: #939 is on main now, and the branch conflicts with it in justfile. Resolving that is the reconciliation from my last review: give the 23 *_storage targets a tier in fuzz/ci-targets.json (or leave them out on purpose), keep fuzz_delete_records_storage out of every tier, and update the "not run in CI" line in CLAUDE.md#L150.

[solace-aross]

Approving at 1097ab1. Sam approved the substance at 6586c75; the one commit since moves the storage targets to the nightly tier and touches docs and a comment.

Checked at this head:

• Ran the sans-io and storage suites for the changed code: 177/177 (encode/decode, tagged fields, both validation layers, list_offsets, create_acls, scram). Memory backend only. Removing the mandatory-array-at-None branch in ser.rs makes both mandatory_sequence_encoding tests fail.
• fuzz-pr shows red on this head, but that run was cancelled by the concurrency cap. The other checks pass.
• git merge-tree against main is clean.

Two notes, neither blocking:

1. validate_topic now requires -1 counts beside a manual assignment and then substitutes the broker default of 3 partitions, and storage ignores the assignments. A client sending NewTopic(name, assignments) for six partitions gets three. This predates the PR, but the layer is now the place to set num_partitions from the assignments.
2. The nightly fuzz matrix grows from 5 to 28 legs, each with its own cargo fuzz build. Worth a look at the first few runs.

Sam's earlier thread on the tagged-field regression test (it still passes with the cap removed) is worth a follow-up.

[solace-aross]

Approving at c9d7411. This is a merge of main (#908) into the branch; the only conflict was fuzz/ci-targets.json, and the resolution keeps both sides: fuzz_batch_records from main plus the 27 storage targets in the nightly tier (28 entries, pr tier unchanged at 6). Nothing else differs from 1097ab1 apart from what #908 brought in. All 17 threads are resolved.

[solace-aross]

Approving at 0d18187. The delta since c9d7411 is a second merge of main and one test-only fix: the three broker tests from #805 now wrap CreateTopicsService in CreateTopicsValidationLayer, matching the ValidatedCreateTopicsRequest it takes on this branch. Production code is unchanged by that commit. All 17 threads are resolved.