Repository navigation
Conversation
Quest-Claim: ee02af7b1680a382
Add moq-mux-fuzz (fmp4, mkv, ts, flv, cmaf, annexb, catalog) and moq-rtmp-fuzz (amf0). Each media target bounds heap traffic linearly in the input through a per-thread counting allocator; amf0 asserts the borrowed receive views allocate nothing. The bodies live in hidden `fuzz` modules so each crate's tests replay seeds and committed regressions on stable, and nightly.yml runs every target. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
# Conflicts: # sh/rs/select.test.py
kixelated
changed the title
|
Minimized reproduction inputs for the findings in the PR body, base64. Write one to a file and replay it with
|
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Findings
 |
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 73.91% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta… | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Title check The title clearly and concisely identifies the main change: adding fuzzing for media parsers reachable by untrusted publishers. Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Description check The pull request description directly explains the added media-parser fuzz targets, allocation bounds, nightly integration, regression tests, discovered findings, and related quest updates. It matches… Full details: Description check
-
Explanation
The pull request description directly explains the added media-parser fuzz targets, allocation bounds, nightly integration, regression tests, discovered findings, and related quest updates. It matches the changeset.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 73.91% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 23 functions across 26 files. (23 skipped: 23 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
Finishing Touches
-
Simplify code
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
kixelated
left a comment
kixelated
left a comment
There was a problem hiding this comment.
Automated review by review (OpenAI)
Reviewed commit: c2bfb3f5e6289905db1f4c9fb69a515d29d6b29d.
No new actionable bugs found in the changed code. Sharing target bodies between libFuzzer and stable regression tests is a sound direction; the thread-local allocation accounting and existing target-discovery workflow avoid unnecessary parallel harnesses.
The already documented parser failures and allocation amplification remain follow-up work, not new findings from this review (see the reproduction inputs). The acknowledged red nightly targets will stop at their first failure until those fixes land.
Verification: static inspection of target bodies, allocation accounting, seeds/replay, feature/workspace wiring and nightly integration. I did not run tests, builds, fuzzing or the nightly ASan path.
* fix: reject sgpd entry counts past the box A version 0 entry of an unknown grouping type reads to the end of the box, so every later entry decodes empty for free, and a 20-byte sgpd declaring 0xffff0000 entries grew a Vec past 3 GB. Every entry takes at least a byte, so a count above the remaining bytes is now OutOfBounds. Found by moq's media fuzz targets (moq-dev/moq#7313). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix: keep the sgpd entry capacity cap Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
iloc rejects zero-size extents after an item's first; webm-iterable and the SPS parsers are fixed in moq-mux and reported upstream; mp4-atom#264 merged, so the bump waits on a 0.17.x release containing it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Landing
(Written by Claude Opus 5.5) |
|
quest land removed this pull request from the queue: Conflicts with the base: .github/workflows/nightly.yml |
|
quest land removed this pull request from the queue: Conflicts with the base: quest/m1/security-audit/README.md, sh/coverage.test.py, sh/rs/coverage.sh, sh/rs/select.test.py |
|
quest land removed this pull request from the queue: Conflicts with the base: Cargo.toml |
|
codex / Codex land fixer: (written by codex / Codex) |
|
quest land removed this pull request from the queue: Conflicts with the base: Cargo.toml, sh/rs/select.test.py |
|
quest land removed this pull request from the queue: this maintainer pick already used its mechanical fix attempt. Run /quest-iterate to diagnose the repeated failure, then pick it again. |
Resolve the modify/delete conflict on quest/m1/security-audit/fmp4-stream-mdat.md by taking main's deletion (#7232 finished it; this branch only dropped its media-fuzz link). Align quests with main: - fuzz-findings/README.md: drop Related links to the security-audit MKV and fMP4 streaming quests, both finished on main. - mkv-block-header.md: #7234 replaced webm-iterable's block parsing with moq-mux's own header reader, so the 4-byte mkv reproducer now fails with InvalidSimpleBlock. Commit it under fuzz/regressions/mkv/ and delete the quest. - mkv-truncated-finish.md, import-metadata-policy.md, sps-fuzz-h264-reader.md: stop linking media-fuzz.md, which this PR finishes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
kixelated
left a comment
kixelated
left a comment
There was a problem hiding this comment.
Automated review by review (OpenAI)
Reviewed commit: c3eccc46032566cdf24bef9b3efe92f2e475b417, compared with my prior review at c2bfb3f5e6289905db1f4c9fb69a515d29d6b29d, accounting for the main merges.
No new actionable bugs found. The meaningful addition is rs/moq-mux/fuzz/regressions/mkv/short-block-header: replaying the formerly crashing input through the existing stable regression harness is the right direction, without a second test framework. The inherited MKV block-header fix bounds the header reads; the shared fuzz bodies and allocation accounting are unchanged. Workspace exclusions and the nightly matrix preserve the media targets alongside main's additions.
The earlier documented MKV panic is now covered by the new regression; the other documented parser/allocation issues remain tracked follow-up work, not new findings. The prior CodeRabbit documentation finding is addressed by the removal of its owning audit file (see discussion_r4431284114).
Verification: static comparison of PR-relative patches, regression discovery and MKV header handling. I did not run tests, builds or fuzzing. The GitHub connector refused the non-UTF-8 regression blob, so its exact bytes and runtime result were not independently verified.
Fuzz targets for the media parsers an untrusted publisher reaches, each asserting bounded heap work per input byte, wired into the nightly fuzz job, with committed regressions replayed in the merge gate. Finishes quest/m2/media-fuzz.md and deletes it.
Changes
rs/moq-mux/fuzz(moq-mux-fuzz):fmp4,mkv,ts,flv(container importers, fed in socket-sized writes chosen by a leading byte),annexb(H.264/H.265 Annex-B and AV1 OBU splitters throughTrackStream),cmaf(subscriber-sidefmp4::Wire::from_init+ fragment decode),catalog(hang and MSF catalog consumers read after each frame, then each rendition's container built).rs/moq-rtmp/fuzz(moq-rtmp-fuzz):amf0, which reads a message throughMessagePayload::amf0(), walks every value, and asserts zero allocations.moq_mux::fuzz::runcounts all heap traffic (cumulative bytes and allocation count, not just live memory) on the target's thread and fails an input above4 MiB + 1024 B/byteor4096 + 8 allocations/byte. Because it counts every allocation, it catches rebuffering or reparsing as well as holding memory. The constants come from what was measured: about 1.7 MB fixed setup (including the first compressed catalog and HLS groups), and about 4.6 KB per group opened by a 14-byte keyframe.fuzzmodules behind a newfuzzfeature, the same pattern as moq-net and moq-quic. Seeds come from the crates' existing fixtures.fuzz::tests::{seeds_are_accepted, regressions}replay them on stable undercfg(test), so they run in the merge gate.src/allocation.rsand gained cumulative counters. The import release tests and the fuzz targets now share it. moq-rtmp's existing AMF0 allocator is reused the same way.nightly.ymlfuzz matrix gained the 8 targets. Both crates were added to the workspace and to every--exclude *-fuzzlist (rs/justfile,sh/rs/select.sh,sh/rs/coverage.sh, and their tests).Public API: none (hidden
fuzzmodules behind an off-by-default feature). Wire: none.Findings from local fuzz runs
These come from short local runs (about 3 minutes per target, coverage-guided, stable toolchain, no ASan). Minimized inputs are in a PR comment. None are fixed here.
sgpddecodeilocdecodemeta/iref.rs:37blocks/block.rs:136,141rs/moq-mux/src/container/ts/step.rs:431(self.scan - self.at)scan < atsps.rs:186,197,204sps/mod.rs:303,sps/scaling_list.rs; scuffle-expgolomb 0.1.5amf0(60M runs) andcatalog(1M runs) found nothing.Decisions
Decided by the maintainer on 2026-10-30.
catalogandamf0in the nightly matrix, adding each other target once its finding is fixed.cache::Track) and billing it to the pool while it waits. That is a change to moq-net's memory accounting on every compressed track, not a small fix to bundle into a fuzz PR.sgpdfirst, upstream in kixelated/mp4-atom, without cutting a release, and pin moq to it once merged. Opened kixelated/mp4-atom#264. moq stays on 0.16.2 until Maybe close on disconnected element. #264 merges and a release is cut. A waiting quest tracks that. mp4-atom 0.17.0 already fixes theirefoverflow (Validate nestedirefreference sizes kixelated/mp4-atom#219).quest/m1/fuzz-findings, whose goal is a clean nightly run of every media target (see below).iloc(mp4-atom-iloc.md).mkv-block-header.md,sps-overflow.md).h264-reader, but moq-mux parses the H.264 SPS withh264-parser(rs/moq-mux/src/codec/h264/mod.rs:169) and usesh264-readeronly for TS video timing, so the quest says "the crates moq-mux uses now". If you meant a move toh264-reader, that needs a quest edit.sgpdcount bound).06a532aa. mp4-atom'srelease.ymlruns release-plz on push, butCargo.tomlis at the already-published 0.17.0, so nothing was published.mp4-atom-bump.mdnow waits on a 0.17.x release containing Maybe close on disconnected element. #264.Quests added
Under
quest/m1/fuzz-findings/README.md, listed in m1's Required. I used a new epic instead ofm1/security-audit, whose Goal is one external audit's findings; this epic's Goal is the nightly jobs going green.mp4-atom-bump.md: waiting on a 0.17.x release containing fix: reject sgpd entry counts past the box kixelated/mp4-atom#264 (merged), then the move off 0.16.2 and its breaking changes.mp4-atom-iloc.md: zero-sizeilocextents (31 bytes allocate 6 MB). Fixed in mp4-atom by rejecting zero-size extents after an item's first (Decision 4).mkv-block-header.mdts-step-scan.md:Ahead::scan'sscan < at.sps-overflow.md: the h264-parser and scuffle-h265/expgolomb overflows, validated in moq-mux before the crates run and reported upstream (Decision 5). Building the reproducers showed that the scuffle-h265 scaling-list underflow also panics in release builds, on the wrapped index.catalog-encoder.md: the DEFLATE encoder per update (Decision 2).Choices made without asking
moq-mux-fuzz,moq-rtmp-fuzz) to match the existingrs/<crate>/fuzzlayout thatsh/rs/fuzz.shresolves. hang catalog parsing is fuzzed from moq-mux, which driveshang::catalog::Consumer.rs/moq-mux/src/fuzz.rs. If a regression input trips the bound once,runmeasures it a second time, because lazy statics allocate once per process.h265::fixturesandcatalog::to_msfcrate-internally under thefuzzfeature, for seeds.regressions/, because each would fail the merge gate until its fix lands. They go in with their fixes. The one exception isregressions/mkv/short-block-header, whose fix landed on main first.Checks
quest gate just check origin/mainonc3eccc4603(main atd52d55ac69): passed, 10997 Rust tests, including both crates'fuzz::tests::{seeds_are_accepted,regressions}with the newregressions/mkv/short-block-header. The flake check was skipped because this host's Nix builder cannot create user namespaces.quest check: 618 documents ok.-Cflags (no ASan) and run for about 1+2 minutes each. Nightly CI'sjust rs fuzzpath (nightly toolchain plus ASan) has not run on this branch.Base merge (2026-11-01)
quest landejectedbb2bcbeb8cwith a conflict, and the optionalQuestcheck failed on links to quests main had finished. Mergedorigin/maintwice (206a5fccb9atd8288e6934, thenc3eccc4603atd52d55ac69):quest/m1/security-audit/fmp4-stream-mdat.md(moq-dev/moq#7232),quest/m1/mkv-truncated-finish.md(moq-dev/moq#7608), and thequest/m1/security-auditepic (moq-dev/moq#7611). This branch had only edited their links tomedia-fuzz.md.fuzz-findings/README.mddropped its Related links to the finished MKV and fMP4 streaming quests.block_header(rs/moq-mux/src/container/mkv/import.rs), so themkvreproducer (u6OBgA==) now fails withInvalidSimpleBlock. Committed it asrs/moq-mux/fuzz/regressions/mkv/short-block-headerand deletedmkv-block-header.mdwithquest rm.quest/m2/import-metadata-policy.mdandquest/m2/sps-fuzz-h264-reader.mdstopped linkingmedia-fuzz.md: the first names the fuzz targets in its Plan, and the second drops it from Required, since this PR finishes it.da4ac94df3: an earlier mechanical merge left a 153-character line insh/rs/select.test.py, which failedcheck python. It now builds the--exclude *-fuzzlist from aFUZZ_EXCLUDESconstant.quest/m1/security-audit/README.mdwas valid, ande29bc0d820restored the link. The second merge then took main's deletion of that whole epic, so the file no longer exists.Base merge (2026-10-31)
quest landejectedbd91a2d749with a conflict in.github/workflows/nightly.yml. Mergedorigin/main(a1169021b6) in65647df8b7:nightly.yml: main added moq-net'singestandrequesttargets to the fuzz matrix on the same line; resolved by hand to the union. Target names remain unique acrossrs/*/fuzz/fuzz_targets/.irefoverflow in the findings table.c03b3088fcrewritesmp4-atom-bump.mdto match: it still waits on a 0.17.x release with fix: reject sgpd entry counts past the box kixelated/mp4-atom#264 (crates.io's latest is 0.17.0), but no longer carries 0.17's breaking changes, so it is now[XS].quest/m2/wire-fuzz-coverage.md, linking themedia-fuzz.mdquest this PR deletes.6bcdc65970points that link at thequest/m1/fuzz-findingsepic.Review
quest review, gpt-6.1-sol, medium effort.--new), round 1 onda4ac94df3(first base merge, regression input, quest alignments, and the ruff fix; reviewed againstorigin/mainatd52d55ac69): 0 findings.c3eccc4603(second base merge, which only takes main's quest deletions; reviewed againstorigin/mainatfd0b0cb9e4): 0 findings.(Written by Claude Opus 5.5)
🤖 Generated with Claude Code