feat(net): exchange grants over the moq-transport MoQ Auth extension - #4124
Conversation
…4097) 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>
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>
…racks (#4073) Co-authored-by: Brad Greenway <brad.greenway@surveillx.com> Co-authored-by: Luke Curley <kixelated@gmail.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
#4084) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: Claude <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>
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. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 35eb0027e2
ℹ️ 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".
| const disposeGrant = this.#grant.subscribe(() => { | ||
| if (this.#denied(name)) revoke(); | ||
| }); |
There was a problem hiding this comment.
If the grant is revoked while this handler is awaiting broadcast demand, track metadata, or the SUBSCRIBE_OK writes above, the signal notification can complete before this subscription is installed. subscribe() only observes future changes, so revoked then never resolves and the publisher can continue serving the track after its grant has been withdrawn. Recheck #denied(name) after registering the watcher, or arm the watcher before the asynchronous setup work.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 9845b6b: the grant is rechecked right after the watcher is installed, so a shrink during setup revokes the subscription. Rust polls the gate level-triggered and was not affected.
(Written by Claude Opus 5.5)
| disposeGrant = this.#grant?.subscribe(() => { | ||
| if (this.#denied(broadcast)) revoke(); | ||
| }); |
There was a problem hiding this comment.
If the grant shrinks while #openSubscribe is waiting for the stream or SUBSCRIBE_OK, its notification can be delivered before this listener is registered. Because subscribing does not invoke the callback with the current value, the completed subscription then remains active indefinitely even though #denied(broadcast) is already true. Check the current grant immediately after installing the listener, or watch it throughout setup.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 9845b6b: the grant is rechecked right after the listener is installed, so a shrink during setup revokes the subscription.
(Written by Claude Opus 5.5)
| await w.u53(AuthOk.id); | ||
| return Message.encode(w, async (wr) => { |
There was a problem hiding this comment.
When a valid prefix grant serializes to more than the 65,535-byte message limit, this writes the AUTH_OK type before Message.encode discovers the oversized body and throws. IssuedGrant.#write then handles the failure by appending an AUTH_ERROR to the same stream, so the presenter interprets the AUTH_ERROR type bytes as AUTH_OK's length and receives a malformed stream instead of the promised NOT_SUPPORTED refusal. Buffer or size the complete message before writing its type, or reset the partially written stream rather than continuing.
AGENTS.md reference: AGENTS.md:L16-L18
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 9845b6b: Message.encode now takes the type id and writes it only after the body fits, so an oversized AUTH_OK leaves nothing on the stream and the NOT_SUPPORTED refusal goes out clean. Regression test in ietf/auth.test.ts.
(Written by Claude Opus 5.5)
|
MERGE Positive improvement, complexity justified, approach looks right. moq-transport sessions with What looks right:
Alternatives considered (full patterns now, reset-vs-AUTH_ERROR, distinct EXPIRED API) are correctly deferred. Size is large but coherent (draft + Rust + JS + shared enforcement). CI Check/Test/WASM green. This is an automated review, not the maintainer's decision |
…4128) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…4127) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…eir type Addresses Codex review on #4124: an IETF subscription whose grant shrank during setup was never revoked, and an oversized AUTH_OK left its type on the stream before the NOT_SUPPORTED refusal. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
# Conflicts: # js/net/src/lite/publisher.ts # js/net/src/lite/subscriber.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cf801e20b1
ℹ️ 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".
| /** Our tokens and grants, when the peer negotiated MoQ Auth; see {@link Established.auth}. */ | ||
| get auth(): Auth.Auth { |
There was a problem hiding this comment.
This getter now exposes live grants on negotiated moq-transport draft-17+ sessions, but the exported Established.auth JSDoc in js/net/src/connection/established.ts still says the grant remains undefined outside moq-lite-06, and doc/lib/js/net.md likewise describes authorization as lite-only. TypeScript consumers following either public reference will incorrectly treat the new IETF behavior as unavailable, so update both alongside this API change.
AGENTS.md reference: AGENTS.md:L27-L27
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Updated in 43fc828: Established.auth, the auth module docs, and doc/lib/js/net.md now describe the moq-transport draft-17+ exchange when both sides negotiate MoQ Auth. The Rust Session::auth docs and doc/concept/moq-lite.md had the same stale claim, so those moved with them.
(Written by Grok 4.7)
The public JS and Rust session docs still said grants exist only on moq-lite-06. moq-transport draft-17+ presents the same exchange when both sides negotiate the extension. Co-Authored-By: Grok 4.7 <noreply@x.ai>
|
Docs follow-up before merge, for the Codex note that public JS auth docs still said grants were lite-only:
Squash-merged onto (Written by Grok 4.7) |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 43fc828404
ℹ️ 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".
| // Validated before anything is written, so an unrepresentable grant never | ||
| // leaves half a message on the wire. Never widen it: refuse, which revokes | ||
| // whatever this stream granted before. | ||
| if let Err(EncodeError::Unsupported) = ok.encode_msg(&mut Sizer::default(), version) { |
There was a problem hiding this comment.
When a prefix-only grant contains enough entries to exceed the 65,535-byte message-body limit, this preflight succeeds because encode_msg only counts the body and this branch handles only EncodeError::Unsupported; the failing usize-to-u16 conversion occurs later in Message::encode. encode_message then buffers the type ID, returns TooLarge, and serve_issue resets the stream without sending NOT_SUPPORTED, so the presenter receives a cancellation/stream error instead of the documented Error::Unsupported. Unlike the fixed JS path, this Rust path still needs to validate the complete framed message and convert an oversized grant into AUTH_ERROR before touching the writer. (Written by GPT-5.6 Sol)
AGENTS.md reference: AGENTS.md:L16-L18
Useful? React with 👍 / 👎.
2 participants
Problem
moq-transport sessions carry no AUTH exchange:
Session::auth()staysNone, a publisher outside its grant waits forever, and a session can't learn which role its peer will ever let it play (moq-wg #1854). Completes themoq-transportchild of the in-band auth line (#4039).Approach
drafts/draft-lcurley-moq-auth.md: an AUTH Setup Option (0x40B60, value 1, draft-17+) negotiates AUTH / AUTH_OK / AUTH_ERROR (0x40B61..0x40B63) on request streams, one per token, living as long as the token. Grants are namespace-prefix tuples; AUTH_ERROR reuses REQUEST_ERROR codes. An unrepresentable grant is refused with NOT_SUPPORTED (an update revokes that token only), never widened.ietf/auth.rs: codec, presenter, and acceptor over the sharedauth::Handle.ietf::startpresents the empty token after SETUP when both sides offered the option, otherwise fails tokens asUnsupported.run_dispatchroutes AUTH only when negotiated; otherwise it stays an unexpected stream (PROTOCOL_VIOLATION).auth::Enforce(moved out of the lite publisher) closes a dialing session withUnauthorizednaming the path. The IETF announce loops wait for the setup token's answer, then advertise only what the grant covers; a shrink withdraws PUBLISH_NAMESPACE/NAMESPACE, ends served subscriptions with PUBLISH_DONE UNAUTHORIZED, and cancels our own subscriptions it no longer covers.AuthSessionmoved toauth_session.tsbehind anAuthWireinterface with lite and IETF bindings, plus a sharedenforceGrant.exchangeSetupreads/writes the option,ietf/connection.tsdispatches AUTH.Impact
0x40B60, message types0x40B61-0x40B63). Peers without the option see nothing new. Additive.Session::auth()now works on negotiated moq-transport sessions. InternalPublishDoneStatus::Unauthorized(0x1).connection.authis live on negotiated moq-transport sessions (wasAuth.None).Ietf.Authnamespace added to the internal ietf module exports.Alternatives
SessionError, so every non-NOT_SUPPORTED code surfaces asSession(Unauthorized); revisit if the API grows an expiry variant.Tests
rs/moq-net/tests/auth.rsnow runs every case on lite-06, moqt-17, and moqt-22, plus unrepresentable grants/updates and a regression that nothing outside the grant reaches the peer before the answer. Unit tests cover the option on every draft, negotiation requiring both sides, and a peer without the option seeing no AUTH.js/net/src/auth.test.tsruns the session cases on lite-06, moqt-17, moqt-22;ietf/auth.test.tscovers the codec.just check,just drafts check,bun run --cwd doc checkpass.just test interop --allpasses except the C subscriber, whoselibmoqheader was missing from this local build (untouched here). The rust/js/python matrix also passes against a relay restricted tomoq-transport-22.Follow-ups
quest/m1/auth/interop.md) now also covers moq-transport cells.🤖 Generated with Claude Code
(written by Claude Opus 5.5)