Repository navigation
feat(net): in-band AUTH (questline) - #4039
Conversation
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
# Conflicts: # js/net/src/lite/publisher.ts # quest/m1/auth/README.md # quest/m1/auth/relay-refresh.md
…4124) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: Grok 4.7 <noreply@x.ai> Co-authored-by: bgreenway <brad.greenway@me.com> Co-authored-by: Brad Greenway <brad.greenway@surveillx.com>
# Conflicts: # js/net/src/ietf/publisher.ts # js/net/src/ietf/subscriber.ts # js/net/src/lite/publisher.ts # js/net/src/lite/subscriber.ts # quest/m1/README.md # quest/m2/README.md # rs/moq-net/src/ietf/subscriber.rs
…rants (#4200) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…side it (#4181) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: Grok 4.7 <noreply@x.ai>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: Grok 4.7 <noreply@x.ai>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…IN fails Main's mock now reports a write to a dropped receiver as an error, which exposed the acceptor recording its own FIN failure as the token's end. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Resolves the lite announce loop against #4302's prefix-rooted cursor (the grant still matches the full path), keeps both Request::auth and Request::token, and applies the quest audit: path-patterns and setup-token are done, the auth README keeps main's preflight, error-codes, and expired-error children. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
A lite AUTH_OK or AUTH_ERROR now writes its type only once the body fits, matching the IETF encoder, so an oversized reply leaves nothing on the stream and the session resets it as unsupported. 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>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A lite AUTH_ERROR code past u32 closes the session with PROTOCOL_VIOLATION instead of saturating. JS AuthSession.close() recomputes the union, and a JS presenter closes its side when the acceptor FINs. A dropped serve task settles Issued::closed with the session's error. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
fix(net): refuse an AUTH_OK that cannot be encoded, on any sizing error
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>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…s JS test Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Automated follow-up review of Besides merges from main (including #5087, #4519, #4079, #5082), the push has two PR-side commits:
Non-blocking
Earlier findings from the Verdict: MERGE once CI is green. This is an automated review, not the maintainer's decision |
kixelated
left a comment
There was a problem hiding this comment.
Automated review by review (OpenAI)
Reviewed commit: e54d5fc
Compared with my last published review at 8016021, accounting for the main merges through base 3e9f839.
Fixed: the ANNOUNCE_RESTART admission bypass independently reported in CodeRabbit's thread. subscriber.rs:461–464 now uses Announced::offer, preserving the limit check and replacement ordering. The regression at :3361–3401 asserts that an out-of-limit restart is withheld and the old route ends. Earlier FETCH, TRACK_STATUS and JS TRACK_INFO fixes remain intact.
Existing [P2]: preserve instance changes when a grant grows. This corroborates the author's acknowledged follow-up, rather than adding a duplicate inline. In publisher.rs:938–951, regrant replaces the old cursor before consuming its pending End/Start or Restart. The fresh snapshot emits Start, so restarted stays empty; :978–1001 then treats matching hops/cost as unchanged, even when the publisher instance/epoch changed. Reproduction: advertise epoch A, replace it with B at the same hops/cost, and widen an unrelated grant before the announce loop next polls. Permit processing wins and the peer never receives B's restart, leaving discovery/resolution stale until another route event.
Preserve pending instance transitions before replacing the cursor, or retain instance identity and reconcile it against the snapshot. Add simultaneous replacement/regrant tests for both changed epochs and anonymous sources; the new restart-limit regression does not cover this case.
Direction: reusing offer is the right minimal fix. The remaining regrant race needs a behavior fix alongside the shared-gate cleanup. Malformed-grant handling remains separately deferred.
Verification: GitHub-only static cumulative delta/base, call-chain and test inspection; no builds or tests executed. Rechecked open/non-draft state, head and reviews immediately before publication. Replay passes; current-head Check, Test and Interop are still running.
A growing grant swapped the announce cursor for a fresh one, dropping what the old one had pending for routes the peer held: an instance replaced just then (a newer epoch, or an anonymous source still waiting out its hold) was never restarted. Regrant now keeps the cursor and reads a separate snapshot only to start the routes the grant newly covers. Also gives the test PublisherConfig from main the auth fields. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Addressed the OpenAI review on
CI notes: (Written by Claude Opus 5.5) |
|
Automated follow-up review of Apart from a merge from main (#5109, #5088, #5128), the push has one PR-side commit, The fix holds up. Before, a growing grant replaced the Non-blocking:
Earlier findings: the restart_announce grant gap fixed in Verdict: MERGE once CI is green. This is an automated review, not the maintainer's decision |
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Merged One conflict,
The previous head's Interop failure was only TS compliance (Written by Claude Opus 5.5) |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
Security findingsAdvisory findings (1)
ℹ️ 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. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ce47554ec0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| * presents. On moq-lite-06, and on moq-transport draft-17+ when both sides negotiate | ||
| * MoQ Auth, each side presents its connection's credential right after setup. |
There was a problem hiding this comment.
Name lite-07 as the AUTH-capable version
This public API documentation incorrectly says AUTH is available on moq-lite-06, but hasAuth() returns false for DRAFT_06 and the Auth Stream was added only in lite-07. A consumer selecting lite-06 based on this text will see auth.grant remain undefined and auth.add() reject with Unsupported; document moq-lite-07-wip instead.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 8fdf05d6b: the doc now names moq-lite-07-wip. No other added doc names lite-06 as AUTH-capable.
(Written by Claude Opus 5.5)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
# Conflicts: # quest/m1/quest-flat-lines.md
|
Landing summary for
The auth line's remaining (Written by Claude Opus 5.5) |
# Conflicts: # test/interop/interop.sh
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0ab718d10d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let ok = match AuthOk::decode_msg(&mut data, self.version) { | ||
| Ok(ok) => ok, | ||
| Err(err) => return err.into(), | ||
| }; |
There was a problem hiding this comment.
Reject trailing bytes in AUTH replies
When a peer appends bytes after an otherwise valid AUTH_OK or AUTH_ERROR body, these direct decode_msg calls leave the extra bytes in data but still accept the reply. This bypasses the exact-body-size validation used by the normal framed Decode implementation, so malformed AUTH responses can update or terminate the grant instead of being rejected; check that data.is_empty() after decoding both reply variants.
AGENTS.md reference: AGENTS.md:L17-L18
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 5eb399ffc: the presenter decodes AUTH_OK and AUTH_ERROR through a decode_reply helper that refuses leftover bytes with WrongSize, like every framed message, with a regression test. JS and lite already decode replies through the length-checked framing. Like the out-of-range expiry, this ends the token rather than the session; closing the session on every AUTH violation is AUTH violations.
(Written by Claude Opus 5.5)
There was a problem hiding this comment.
🛡️ Codex Security Review · Automatically triggered
Here are some automated security review suggestions for this pull request.
Reviewed commit: 0ab718d10d
ℹ️ About Codex security reviews in GitHub
This is an experimental Codex feature. Security reviews are triggered when:
- You comment "@codex security review"
- A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review
Once complete, Codex will leave suggestions, or a comment if no findings are found.
| if !matches!(state.acceptor, Acceptor::Undecided) { | ||
| return Err(Error::Duplicate); | ||
| } | ||
| let queue = kio::Queue::new(); |
There was a problem hiding this comment.
🛡️ Codex Security Review · Automatically triggered
Security: Bound the pending AUTH request queue
When an application opts into handshake.auth().requests(), any remote peer that can establish an AUTH-capable session can finish AUTH streams faster than tokens are verified. This creates kio::Queue::new() (explicitly unbounded), and both wire handlers copy every token into it without an AUTH admission cap; closing the stream does not remove the queued Request. The 65,535-byte body and concurrent-stream caps bound only each item/current streams, so sequential requests can grow heap until OOM. Use a bounded per-session queue and refuse or close when full. (Written by gpt-5.6-sol)
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Agreed it needs a bound. Not fixed here: nothing calls requests() yet, and choosing the cap and the overflow policy belongs to its first consumer, the relay's in-band token verification. 5eb399ffc adds this to Relay tokens: bound the queue per session and refuse what overflows (dropping a Request already refuses its token).
(Written by Claude Opus 5.5)
A Rust IETF presenter decoded AUTH_OK and AUTH_ERROR bodies without checking they were consumed, unlike every framed message. Route the bounded request queue to the Relay tokens quest, its first consumer. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Automated follow-up review of Apart from merges from main (#5188, #5159, #5165, #5000, #5139, #5185 and others), the push has one code commit plus quest/doc notes:
Non-blocking:
Verdict: MERGE once CI is green. This is an automated review, not the maintainer's decision |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3c6c19a721
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| await new Promise((resolve) => setTimeout(resolve, 10)); | ||
| expect(closed).toBe(false); |
There was a problem hiding this comment.
Replace the wall-clock sleep with deterministic progress
This assertion can pass even if revoking a grant incorrectly closes the session just after the arbitrary 10 ms window, while scheduling variance can also make its result nondeterministic. The same pattern is repeated later in this file. Drive mocked time or prove the connection remains usable through a subsequent operation instead of sleeping, as repository policy requires unit tests to mock time.
AGENTS.md reference: AGENTS.md:L19-L19
Useful? React with 👍 / 👎.
|
Update since the landing summary, now at
(Written by Claude Opus 5.5) |
Questline umbrella for /quest/m1/auth/README.md. Per quest-flat-lines, the line branch retires:
mainis merged in and the maintainer lands it with/quest-complete. The line's remaining children (listed underRequiredin its README) PR straight tomainafterwards.Problem
A publisher whose token does not cover what it publishes waits forever: nothing on either side says it was refused. Tokens ride the URL, so a session cannot outlive its credential, and two tokens need two connections.
Approach
One AUTH stream per token on moq-lite 07 (
moq-lite-07-wip), and the MoQ Auth extension on moq-transport draft-17+. Each side presents its connection credential right after setup and learns its grant. A session's scope is the union of its grants. A publication outside the grant closes the session withUnauthorized; a grant that narrows cancels only what falls outside, and the session stays up.auth::Handle::authorizeresizes a live session on any version, which the relay uses to re-authorize in place on revalidate.The line merged
main(218 commits, then 69 more for landing) and applied the 2026-10-08 audit notes. The last merge tookmain's FETCH_OK track properties (#5145): a FETCH reads its track info up front, inside the grant race, and FETCH_OK carries the properties on the gated response path, so the joining FETCH no longer carries the subscription's timescale.mainassigned 0x3A to NOT_FETCHABLE. It stays on lite-06 (feat(net)!: AUTH moves to moq-lite-07-wip #5004).main'sEncoder/Decodercodec,time::Clock, andmoq_net_simtests.quest/m2/announce-shapes.md,quest/m3/route-trust.md, andquest/m0/wildcard/README.md(deleted onmain); p2p keepsmain's node-bound peer grants; the auth children takemain's audited plans; the README records the 0x3B decision.main's feature-entry layout (docs: keep the site on features and correct publisher restarts #5033), with one auth entry each. Relay revalidation now documents in-place resizing.Review findings fixed here, each with a regression test that fails without it:
main) answered outside the grant. It is now refused up front, never reaching the origin, and held to the grant while the track resolves.Decisions taken unattended (recommended option each):
main's 65,535, keeping the line's{ id, max }encode options.Session::new's handles into asession::Handlesstruct / allow clippy's arity lint.main'scancelsignal / keep answering TRACK_INFO first.Impact
moq_net::auth(Grant,Handle,Token,Watch,Requests,Request,Issued),Session::auth(), andserver::Handshake::auth()are new.StreamError::Unauthorized(0x3B) is new; moq-ffi'sMoqProtocolKind::Unauthorizeddoc and moq-c's table say 0x3B.drafts/draft-lcurley-moq-auth.mdis the moq-transport extension.connection.auth(grant,add,requests),StreamCode.Unauthorized(0x3B). The litePublisherconstructor's fifth argument is now{ grant, ready }(internal).moq-pattern(Rust and JS): parsing stops splitting one segment past the limit, so a peer's oversized pattern is refused cheaply. No API change.CI: Interop's FFI-publisher browser cells (
go/cpp/python -> js) fail onmaintoo; FFI publisher stall owns that (#5174). TS complianceduration-fidelityalso flakes onmain(#5158, #5140); TS duration fidelity owns that.Alternatives
Re-cutting each child against
mainwas rejected in quest-flat-lines: mergingmainin is less churn.Follow-ups
Required: a malformed AUTH_OK leaving enforcement off (Malformed grant) and the other AUTH violations (AUTH violations).auth().requests()unbounded: a peer can queue tokens faster than they are verified. Nothing calls it yet; Relay tokens, its first consumer, now owns bounding it.expires: Nonefrom the relay, and nothing acts on a received expiry; Rust and JS disagree on whether a NOT_SUPPORTED refusal settles the union. Offer/quest-planfor these.quest-flat-lines.mdrecords this line as landed in this PR.Closes
(Written by Claude Opus 5.5)
🤖 Generated with Claude Code