Repository navigation
feat(net): refuse inbound IETF track listings - #7236
Conversation
Quest-Claim: 7809020f8f09e540
Co-Authored-By: GPT-6.1 Sol <noreply@openai.com>
Our moq-transport publishers refuse a peer's SUBSCRIBE_TRACKS with NOT_SUPPORTED again, on its own stream, and the session stays up. Rust still sends SUBSCRIBE_TRACKS upstream for a TRACKS listing. Deletes the Rust and JS answerers, their tests, and the subscribe-tracks JS bench, trims the SUBSCRIBE_TRACKS codec to what our own request carries, and covers outbound listing coalescing with a unit test on its state. Finishes quest/m1/ietf-tracks-refuse.md. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…listings # Conflicts: # doc/concept/standard.md # js/net/src/ietf/parameters.ts # js/net/src/ietf/publisher.ts # js/net/src/ietf/subscribe_tracks.test.ts # rs/moq-net/src/ietf/publisher/tracks.rs # rs/moq-net/src/ietf/request_stream.rs # rs/moq-net/src/ietf/subscribe_tracks.rs
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 15 minutes. View limit details
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. |
kixelated
left a comment
There was a problem hiding this comment.
Automated review by review (OpenAI)
Reviewed commit: 83e5f24
No actionable bugs found in the changed code. The direction is coherent: refuse inbound SUBSCRIBE_TRACKS on its own stream, retain decoding/validation and reserved-namespace handling, and preserve Rust's outbound discovery. Removing the answerer and PUBLISH-only forwarding branches is simpler than maintaining an unused inbound implementation. The new refusal/session-survival coverage and overlapping outbound-listing test match that boundary.
Verification limits: static review of the changed Rust/JavaScript paths, related request-stream handling, tests, and documentation; I did not run tests or interop. Successful outbound discovery now relies more heavily on scripted-peer coverage after removal of the self-peer integration cases.
|
Landing via
(Written by Claude Opus 5.5) |
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
- Rust publisher refuses inbound
SUBSCRIBE_TRACKS(rs/moq-net/src/ietf/publisher.rs): still decodes the message, then rejects on its own stream —DOES_NOT_EXISTfor a reserved prefix,NOT_SUPPORTEDotherwise. Deletesietf/publisher/tracks.rs(2,565 lines), the publisher'slistingsstate, and the draft-18DUPLICATE_SUBSCRIPTIONguard;pause_joinmoves intopublisher.rs. - Codec narrows (
ietf/subscribe_tracks.rs,ietf/request_stream.rs,ietf/error.rs):SubscribeTrackskeeps only what outbound carries; the unread fields are still decoded for validation then dropped and no longer encoded.NamespaceUpdateloses itsTRACKSconst generic;PREFIX_OVERLAPis removed. - JS returns to a refusal (
js/net/src/ietf/publisher.ts):runSubscribeTracksrefuses (refuseReserved, elsenot_supported); the answerer,PublishSkipped,#listings,#trackClaims, thetracks-update/publish-okparameter kinds, and thesubscribe-tracksbench are deleted. - Tests/docs/quests: refusal-per-request across drafts 18/20/22,
a_listing_of_our_own_peer_fails_alone, newoverlapping_listings_share_one_request;doc/concept/standard.mdandstats.mdupdated; the finished quest is deleted and its links repointed.
I verified the removal leaves no dangling references (PublishSkipped, PREFIX_OVERLAP, tracks-update, publish-ok, the removed SubscribeTracks fields, NamespaceUpdate::<TRACKS>), that pause_join is still called at rs/moq-net/src/ietf/publisher.rs:1477, that no ietf::Publish sender remains (so the docs' "We never send PUBLISH" holds), and that the JS bench list and sh/gh/nightly.test.py stay consistent. mod ietf is private (rs/moq-net/src/lib.rs:112) and the JS symbols are absent from js/net/src/ietf/index.ts, so the "no public API" claim checks out and no MIGRATE entry is needed.
Noted, not blocking: end-to-end coverage of Rust's outbound listing (track info, joining FETCH head, announces cap) drops to scripted-peer unit tests, which the author records as a follow-up to port.
deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏
|
quest land removed this pull request from the queue: the batch check failed (exit 1). Checking ashpd v0.13.13
Checking http-cache v1.0.0-alpha.8
Compiling proc-macro2-diagnostics v0.10.1
Checking moq-sock v0.2.0 (/work/rs/moq-sock)
Checking smithay-clipboard v0.7.3
Checking fs-err v3.3.1
Checking p384 v0.14.0
Checking p256 v0.14.0
Checking arboard v3.7.0
Checking reqwest-middleware v0.5.2
Checking web-transport-trait v0.5.2
Checking axum v0.8.9
Checking egui-winit v0.36.2
Checking axum-server v0.8.0
Checking moq-auth v0.2.1 (/work/rs/moq-auth)
Checking egui-wgpu v0.36.2
Checking http-cache-reqwest v1.0.0-alpha.9
Compiling duplicate v2.0.1
Checking eframe v0.36.2
Checking egui_dock v0.21.1
Checking moq-json v0.5.8 (/work/rs/moq-json)
Checking moq-loc v0.2.19 (/work/rs/moq-loc)
Checking qmux v0.7.0 (/work/rs/qmux)
Checking hang v0.21.11 (/work/rs/hang)
Checking moq-stats v0.2.12 (/work/rs/moq-stats)
Checking moq-tokio v0.19.23 (/work/rs/moq-tokio)
Checking moq-mux v0.10.11 (/work/rs/moq-mux)
Checking moq-relay v0.17.2 (/work/rs/moq-relay)
Checking moq-audio v0.1.10 (/work/rs/moq-audio)
Checking moq-scene v0.1.0 (/work/rs/moq-scene)
Checking moq-studio v0.1.0 (/work/rs/moq-studio)
Finished `dev` profile [unoptimized + debuginfo] target(s) in 26.34s
info: running `cargo clippy --locked --tests --all-features -- -D warnings` on moq-wasm (51/52)
Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.21s
info: running `cargo clippy --locked --tests --all-features -- -D warnings` on uniffi-bindgen (52/52)
Compiling getrandom v0.4.3
Compiling winnow v1.0.4
Compiling uniffi_internal_macros v0.32.2 (https://github.com/kixelated/uniffi-rs?rev=88d835d#88d835d6)
Compiling serde v1.0.229
Compiling libc v0.2.191
Checking indexmap v2.14.2
Compiling fs-err v3.3.1
Checking nom v7.1.3
Compiling scroll_derive v0.12.1
Compiling basic-toml v0.1.10
Compiling askama_parser v0.16.1
Checking smawk v0.3.3
Compiling toml_parser v1.1.3+spec-1.1.0
Checking weedle2 v5.0.0 (https://github.com/kixelated/uniffi-rs?rev=88d835d#88d835d6)
Checking serde_json v1.0.154
Checking textwrap v0.16.4
Checking scroll v0.12.0
Compiling askama_derive v0.16.1
Checking tempfile v3.27.0
Compiling toml_datetime v1.1.1+spec-1.1.0
Checking plain v0.2.3
Checking clap_builder v4.6.7
Checking goblin v0.8.2
Compiling toml v1.1.6+spec-1.1.0
Checking cargo_metadata v0.23.1
Checking uniffi_pipeline v0.32.2 (https://github.com/kixelated/uniffi-rs?rev=88d835d#88d835d6)
Compiling askama_macros v0.16.1
Checking uniffi_core v0.32.2 (https://github.com/kixelated/uniffi-rs?rev=88d835d#88d835d6)
Checking clap v4.6.7
Checking uniffi_meta v0.32.2 (https://github.com/kixelated/uniffi-rs?rev=88d835d#88d835d6)
Checking uniffi_udl v0.32.2 (https://github.com/kixelated/uniffi-rs?rev=88d835d#88d835d6)
Checking askama v0.16.1
Checking uniffi_bindgen v0.32.2 (https://github.com/kixelated/uniffi-rs?rev=88d835d#88d835d6)
Compiling uniffi_macros v0.32.2 (https://github.com/kixelated/uniffi-rs?rev=88d835d#88d835d6)
Checking uniffi v0.32.2 (https://github.com/kixelated/uniffi-rs?rev=88d835d#88d835d6)
Checking uniffi-bindgen v0.1.0 (/work/rs/uniffi-bindgen)
Finished `dev` profile [unoptimized + debuginfo] target(s) in 2.52s
mbx[cache]: object cache: 1246 hits, 114 misses, 50 not looked up; 0 B downloaded, 0 B uploaded, 398.8 MiB stored locally
mbx[cache]: 8m 10s session; 21m 20s estimated compiler time avoided (summed across compilations)
mbx[cache]: 50 compilations had no usable prior inputs or matching prediction for a lookup
mbx[cache]: Cargo artifact reuse and CI cache archive transfers are not included above
mbx[savings]: 2.4 TiB in every checkout, 2.4 TiB on disk. arithmetic declined to comment.
ci: check flake failed
ci: seconds per module
617 check rs
272 test rs
1136 check rs_shapes
307 check js
37 test js
12 check markdown
2 check gh
42 test gh
62 check flake (failed)
0 check drill
4 check quest
5 check ts_paths
24 check shell
1 check large_files
4 check toml
3 check nix
0 check justfile
2026 wall time
error: recipe `ci` failed on line 106 with exit code 1
. |
|
Codex land fixer: moq-doc:check | .vitepress/api/index.test.ts:
moq-doc:check | (pass) a rendered workspace > each entrypoint gets a page named by its import specifier [4.97ms]
moq-doc:check | (pass) a rendered workspace > a re-export from another workspace package links to its owner [2.95ms]
moq-doc:check | (pass) a rendered workspace > an external re-export is documented under each public name [9.88ms]
moq-doc:check | (pass) a rendered workspace > a namespace export keeps its public name [2.69ms]
moq-doc:check | (pass) a rendered workspace > a hidden declaration is documented under the alias that re-exports it [1.25ms]
moq-doc:check | �[93m[warning]�[0m Value, defined in @test/hidden/src/access.ts, is referenced by read but not included in the documentation
moq-doc:check | �[93m[warning]�[0m Value, defined in @test/hidden/src/access.ts, is referenced by read but not included in the documentation
moq-doc:check | (pass) an unexported declaration that a public one names fails the run [279.41ms]
moq-doc:check | �[93m[warning]�[0m Failed to resolve link to "Missing" in comment for value
moq-doc:check | (pass) a link that does not resolve fails the run [474.38ms]
moq-doc:check |
moq-doc:check | concept/audio-jitter/corpus.test.ts:
moq-doc:check | (pass) audio jitter corpus > paced > matches the checked-in file [0.11ms]
moq-doc:check | (pass) audio jitter corpus > paced > is a trace of equal-length clocks [0.03ms]
moq-doc:check | (pass) audio jitter corpus > paced > arrives in a monotonic order [0.58ms]
moq-doc:check | (pass) audio jitter corpus > paced > names a target for the first frame [0.02ms]
moq-doc:check | (pass) audio jitter corpus > paced > reports every target as a change [0.06ms]
moq-doc:check | (pass) audio jitter corpus > flush > matches the checked-in file [0.03ms]
moq-doc:check | (pass) audio jitter corpus > flush > is a trace of equal-length clocks
moq-doc:check | (pass) audio jitter corpus > flush > arrives in a monotonic order [0.15ms]
moq-doc:check | (pass) audio jitter corpus > flush > names a target for the first frame
moq-doc:check | (pass) audio jitter corpus > flush > reports every target as a change
moq-doc:check | (pass) audio jitter corpus > advertised > matches the checked-in file [0.03ms]
moq-doc:check | (pass) audio jitter corpus > advertised > is a trace of equal-length clocks
moq-doc:check | (pass) audio jitter corpus > advertised > arrives in a monotonic order [0.17ms]
moq-doc:check | (pass) audio jitter corpus > advertised > names a target for the first frame
moq-doc:check | (pass) audio jitter corpus > advertised > reports every target as a change [0.01ms]
moq-doc:check | (pass) audio jitter corpus > buildup > matches the checked-in file [0.04ms]
moq-doc:check | (pass) audio jitter corpus > buildup > is a trace of equal-length clocks
moq-doc:check | (pass) audio jitter corpus > buildup > arrives in a monotonic order [0.23ms]
moq-doc:check | (pass) audio jitter corpus > buildup > names a target for the first frame
moq-doc:check | (pass) audio jitter corpus > buildup > reports every target as a change
moq-doc:check | (pass) audio jitter corpus > reorder > matches the checked-in file [0.03ms]
moq-doc:check | (pass) audio jitter corpus > reorder > is a trace of equal-length clocks
moq-doc:check | (pass) audio jitter corpus > reorder > arrives in a monotonic order [0.14ms]
moq-doc:check | (pass) audio jitter corpus > reorder > names a target for the first frame
moq-doc:check | (pass) audio jitter corpus > reorder > reports every target as a change
moq-doc:check | (pass) audio jitter corpus > backlog > matches the checked-in file [0.04ms]
moq-doc:check | (pass) audio jitter corpus > backlog > is a trace of equal-length clocks
moq-doc:check | (pass) audio jitter corpus > backlog > arrives in a monotonic order [0.26ms]
moq-doc:check | (pass) audio jitter corpus > backlog > names a target for the first frame
moq-doc:check | (pass) audio jitter corpus > backlog > reports every target as a change
moq-doc:check | (pass) audio jitter corpus > idle > matches the checked-in file [0.03ms]
moq-doc:check | (pass) audio jitter corpus > idle > is a trace of equal-length clocks
moq-doc:check | (pass) audio jitter corpus > idle > arrives in a monotonic order [0.13ms]
moq-doc:check | (pass) audio jitter corpus > idle > names a target for the first frame
moq-doc:check | (pass) audio jitter corpus > idle > reports every target as a change
moq-doc:check | (pass) audio jitter corpus > tunein > matches the checked-in file [0.04ms]
moq-doc:check | (pass) audio jitter corpus > tunein > is a trace of equal-length clocks
moq-doc:check | (pass) audio jitter corpus > tunein > arrives in a monotonic order [0.19ms]
moq-doc:check | (pass) audio jitter corpus > tunein > names a target for the first frame
moq-doc:check | (pass) audio jitter corpus > tunein > reports every target as a change
moq-doc:check | (pass) audio jitter corpus > spike > matches the checked-in file [0.05ms]
moq-doc:check | (pass) audio jitter corpus > spike > is a trace of equal-length clocks
moq-doc:check | (pass) audio jitter corpus > spike > arrives in a monotonic order [0.26ms]
moq-doc:check | (pass) audio jitter corpus > spike > names a target for the first frame
moq-doc:check | (pass) audio jitter corpus > spike > reports every target as a change
moq-doc:check | (pass) audio jitter corpus > straggler > matches the checked-in file [0.03ms]
moq-doc:check | (pass) audio jitter corpus > straggler > is a trace of equal-length clocks
moq-doc:check | (pass) audio jitter corpus > straggler > arrives in a monotonic order [0.23ms]
moq-doc:check | (pass) audio jitter corpus > straggler > names a target for the first frame
moq-doc:check | (pass) audio jitter corpus > straggler > reports every target as a change
moq-doc:check | (pass) audio jitter corpus > fixed > matches the checked-in file [0.02ms]
moq-doc:check | (pass) audio jitter corpus > fixed > is a trace of equal-length clocks
moq-doc:check | (pass) audio jitter corpus > fixed > arrives in a monotonic order [0.10ms]
moq-doc:check | (pass) audio jitter corpus > fixed > names a target for the first frame
moq-doc:check | (pass) audio jitter corpus > fixed > reports every target as a change
moq-doc:check |
moq-doc:check | 134 pass
moq-doc:check | 0 fail
moq-doc:check | 15964 expect() calls
moq-doc:check | Ran 134 tests across 3 files. [7.35s]
moq-doc:check | API reference: 17 packages in 46.5s
moq-doc:check |
moq-doc:check | �[32m�[1mvitepress�[22m v1.6.4�[39m
moq-doc:check |
moq-doc:check | - building client + server bundles...
moq-doc:check | �[34mllmstxt�[39m�[2m » �[22m �[1mvitepress-plugin-llms�[22m initialized �[2m(client build)�[22m with workDir: �[36m/work/doc�[39m
moq-doc:check | �[34mllmstxt�[39m�[2m » �[22m Build started, file collection cleared
moq-doc:check | �[34mllmstxt�[39m�[2m » �[22m Processing �[1m124�[22m markdown files from �[36m/work/doc�[39m
moq-doc:check | �[34mllmstxt�[39m�[2m » �[22m Generating �[36mllms.txt�[39m...
moq-doc:check | �[34mllmstxt�[39m�[2m » �[22m Generating full documentation bundle (�[36mllms-full.txt�[39m)...
moq-doc:check
thread 'codex-main' (86) panicked at /rustc/48a229ceaefd4985c50990b14116b6d856af0985/library/std/src/io/stdio.rs:1166:9:
failed printing to stderr: Resource temporarily unavailable (os error 11)
stack backtrace:
0: __rustc::rust_begin_unwind
1: core::panicking::panic_fmt
2: std::io::stdio::_eprint
3: <codex_exec::event_processor_with_human_output::EventProcessorWithHumanOutput as codex_exec::event_processor::EventProcessor>::process_server_notification
4: codex_exec::run_exec_session::{closure#0}
5: <tracing::instrument::Instrumented<codex_exec::run_exec_session::{closure#0}> as core::future::future::Future>::poll
6: codex_exec::run_main::{closure#0}
7: codex::cli_main::{closure#0}
8: <tokio::runtime::park::CachedParkThread>::block_on::<codex_arg0::run_main_with_arg0_guard<codex::main::{closure#0}, codex::main::{closure#0}::{closure#0}>::{closure#0}>
9: tokio::runtime::context::runtime::enter_runtime::<<tokio::runtime::scheduler::multi_thread::MultiThread>::block_on<codex_arg0::run_main_with_arg0_guard<codex::main::{closure#0}, codex::main::{closure#0}::{closure#0}>::{closure#0}>::{closure#0}, core::result::Result<(), anyhow::Error>>
10: <tokio::runtime::runtime::Runtime>::block_on::<codex_arg0::run_main_with_arg0_guard<codex::main::{closure#0}, codex::main::{closure#0}::{closure#0}>::{closure#0}>
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
quest: fixer agent exited 101
(written by Codex) |
Main's #7236 deletes the inbound SUBSCRIBE_TRACKS handler this branch token-gated, so accept the deletion and drop the RequestGrant observer machinery only those listed PUBLISHes used. Move the new relay test to moq_clock::timeout for main's clock lint. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Problem
Our moq-transport publishers answered a peer's SUBSCRIBE_TRACKS with paused PUBLISHes, PUBLISH_SKIPPED, resumes through REQUEST_UPDATE, PREFIX_OVERLAP, and credit-gated opens. That is about 3,000 lines across Rust and JS for a request no known third party sends; our own peers list tracks over moq-lite. The quest (quest/m1/ietf-tracks-refuse.md) decided on 2026-10-28 to cut the inbound half and keep Rust's outbound discovery.
Approach
Publisher::handle_streamstill decodes SUBSCRIBE_TRACKS, so the refusal admits its Request ID. It then refuses on the request's own stream:DOES_NOT_EXISTfor a reserved namespace,NOT_SUPPORTEDotherwise. Deletesietf/publisher/tracks.rs, the publisher'slistingsstate, and the draft-18DUPLICATE_SUBSCRIPTIONcheck that only guarded tracks we had PUBLISHed.pause_joinmoves intopublisher.rs.SubscribeTrackskeeps only what our outbound request carries (forward,group_order,properties_wanted, token). Every other parameter is still read, so validation is unchanged, then dropped. Also removesNamespaceUpdate<const TRACKS>, whosetruevariant only decoded updates on an inbound listing, and the unusedPREFIX_OVERLAPconstant.runSubscribeTracksreturns to its refusal from before moq-dev/moq#6336. Deletes the answerer and its helpers (#listings,#trackClaims, the PUBLISH-only branches of#forward),PublishSkipped, thetracks-updateandpublish-okparameter kinds,subscribe_tracks.test.ts, and thesubscribe-tracksbench (also removed fromjs/justfileandsh/gh/nightly.sh).rs/moq-net/tests/integration/tracks_ietf.rsdrove Rust's outbound listing against our own answerer, so it now checks one thing end to end: a Rust listing of our own peer fails on its own while its SUBSCRIBE keeps delivering. Its other cases are covered byietf::subscriber::tracksunit tests against a scripted peer. The newoverlapping_listings_share_one_requesttest covers the nested and broader listing coalescing, which previously had only end-to-end coverage.SUBSCRIBE_TRACKSbullets indoc/concept/standard.mdand the listing paragraph indoc/concept/stats.md.quest/m1/README.md. The archive README's decision link now points here.Choices made without asking, for review:
test/interop/tracks.tsis moq-lite's TRACKS stream, not IETF, so the quest's step to assert the refusal there doesn't apply. No interop case drove our IETF answerer.test/interop/compat), so no planned break was added.rs/moq-net/benches/tracks.rsmeasures only the model's listing, not the IETF answerer, so it stays.Publishcodec keeps itsproperties/parametersfields, which round-trip a decoded PUBLISH.Impact
NOT_SUPPORTED(reserved namespaces getDOES_NOT_EXIST), and the session stays up. A Rust TRACKS listing of one of our own moq-transport peers now fails instead of listing; Rust still sends SUBSCRIBE_TRACKS upstream to third parties.Alternatives
Keeping inbound listing support was rejected in the quest's maintainer decision. So was cutting the outbound half, since the archive learns an IETF peer's tracks only through it.
Validation
quest gate just check origin/mainpasses on head83e5f24acd: Rust 9086 tests, JS, markdown, quest, shell, and the clippy/shape gates.de6634606d:just test interop --versions ietf --publishers rust,js --subscribers rust,js,js-native-node,js-native-bun. Every cell passes on drafts 18 and 22 (relay sessions, CLI FETCH, late join, tails, datagrams, status, goaway, track status) except the browserjs -> jscell on draft 22. That cell times out waiting for playback to resume after a pause. It fails the same way on main at1d8bf24998, so it predates this PR (see Follow-ups). Interop was not rerun after the draft-20 merge.Review
Codex review via
quest review --deep(codex, gpt-6-astra, medium effort), session01a1909b:de6634606d: no findings.83e5f24acd(after merging main with hand-resolved conflicts): no findings.The GPT-6.1 Sol checkpoint commit (
060229d0dc, the JS refusal test) is covered by these rounds; its wrong error code (0x9) and ALPN pairing were fixed here.Follow-ups
js -> jsinterop cell fails on draft 22 on main too: after a pause/resume the browser publisher ends the resumed subscriptions, and the relay forwardsPUBLISH_DONEINTERNAL_ERROR. Worth a quest via/quest-plan.tracks_ietf.rsend-to-end outbound cases (track info, joining FETCH head, announces cap) to the scripted-peer harness inietf::subscriber::tracks.(Written by Claude Opus 5.5)
🤖 Generated with Claude Code