[kixelated]

Problem

#4277 specified that a malformed or non-canonical grant pattern is a PROTOCOL_VIOLATION (drafts/draft-lcurley-moq-lite.md, Path Pattern), but Rust and JS only end the token and keep the session up. Worse, when the bad AUTH_OK answers the session's own setup token, the grant union stays unknown, so the client's fail-loud publish check (Enforce in Rust, enforceGrant in JS) never runs. Maintainer decisions from the 09-28 merged-PR audit: close the session with PROTOCOL_VIOLATION in both languages to match the draft, with tests, and treat an out-of-range Expires the same way.

Approach

New quest Malformed grant, a Required child of the auth line on main (questline branches are being retired, so this targets main instead of quest/m1/auth/README).

Checked against the auth line's code (origin/quest/m1/auth/README), since the AUTH stream is not on main yet:

• Still true: a bad pattern decodes as DecodeError::InvalidValue and ends only the token (Rust PresentToken, JS #run), and an unanswered setup token leaves enforcement off in both languages.
• Corrected: Rust already closes the session when Expires overflows the local clock (since fix(net): make every AUTH stream ending exact on both sides #4550). What remains is that Rust decodes any u64 while JS refuses past 2^53 - 1, and the draft never defines the range.

Impact

• Quest files only. No API or wire change.

Follow-ups

• The line branch's quest/m1/auth/violations.md (quest(auth): AUTH protocol violations close the session everywhere #4554) is a wider sweep that lists this lite decode gap. When the line's quest tree reaches main, the second to land drops the overlap.

(Written by Claude Opus 5.5)

🤖 Generated with Claude Code

[coderabbitai]

Warning

Review limit reached

You'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 5 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 4 included reviews currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 7533c966-61ad-4b3e-a74b-b1b06b2383a0

📥 Commits

Reviewing files that changed from the base of the PR and between b1feff5 and f66c69a.

📒 Files selected for processing (2)

• quest/m1/auth/README.md
• quest/m1/auth/malformed-grant.md

• Autopilot · 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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[kixelated]

Automated review of f66c69ac (quest-only: quest/m1/auth/malformed-grant.md plus the Required entry in the auth README)

I checked the claims against the auth line's code (quest/m1/auth/README). They hold up:

• Rust decode_patterns (rs/moq-net/src/lite/auth.rs:60-74) refuses a bad or non-canonical pattern with DecodeError::InvalidValue. PresentToken::poll (rs/moq-net/src/lite/session.rs:514-541) only records an Error::ProtocolViolation as a session violation, so that decode error ends just the token.
• Rust already returns ProtocolViolation when now.checked_add(expires) overflows (session.rs:593-595). Its decoder takes any u64 (auth.rs:84-87), while JS AuthOk.decode reads r.u53() (js/net/src/lite/auth.ts:92), which throws past Number.MAX_SAFE_INTEGER.
• JS #run (js/net/src/auth_session.ts:339-389) catches the decode error and ends the token without setting #replied, so the union stays unknown and enforceGrant / Enforce skip the check.
• The Path Pattern text in the draft on the line branch does end with "A pattern that breaks any of these rules is a PROTOCOL_VIOLATION." (Note that this section isn't in the draft on main yet.)

No blocking issues.

Non-blocking

1. The "out of range" rule can be pinned down now. Both encoders already clamp to 2^53 - 1: Rust uses MAX_EXPIRES_MS = (1 << 53) - 1 (auth.rs:49, :105) and JS uses Number.MAX_SAFE_INTEGER (lite/auth.ts:85). So "Expires above 2^53 - 1 is a PROTOCOL_VIOLATION" already matches what both sides send. Stating that in the quest, instead of leaving the range open, removes a decision from the implementer, and Rust's checked_add guard can stay as a backstop.
2. Circular citation. "The maintainer decided this in [quest(auth): close the session on a malformed AUTH_OK grant #4380]" links to this PR, which has no maintainer comment. The PR body says the decision came from the 09-28 merged-PR audit, so cite that or drop the link.
3. Say which failures count as malformed. In JS, decodePatterns and u53 throw a plain Error, and #run handles every failure the same way. The implementation needs a way to tell a decode failure (close the session) apart from a transport or stream error (keep today's behavior). It's worth saying so in the Plan, so the fix doesn't turn every token-ending error into a session close.
4. Truncated replies (a hunch to verify). In Rust, Error::Decode(Short) before the first reply maps to Unsupported (session.rs:527). If the peer FINs partway through an AUTH_OK, the setup token is treated like a peer that predates AUTH, which also leaves enforcement off. Decide whether a partial AUTH_OK counts as malformed here or belongs to violations.md, and say which.

The overlap with the line branch's violations.md (#4554, which lists "an out-of-range AUTH_OK expiry, or an AUTH, AUTH_OK, or AUTH_ERROR that fails to decode, such as a bad pattern") is already called out. CI is still pending on this head.

Verdict: MERGE

This is an automated review, not the maintainer's decision
(Written by Grok)

[kixelated]

Landing summary:

• Retargeted to main per the maintainer's 2026-10-05 decision (questline branches are being retired) and rebuilt the branch on main, so the diff is only quest/m1/auth/malformed-grant.md plus its Required entry in quest/m1/auth/README.md, ranked after AUTH endings.
• Rechecked the claims against origin/quest/m1/auth/README: a bad pattern still ends only its token in Rust and JS, and on the setup token it leaves the union unknown, so client enforcement never runs. Rust already closes the session on an Expires that overflows the clock (fix(net): make every AUTH stream ending exact on both sides #4550); the quest now asks only for a draft-defined range that Rust and JS refuse alike.
• Overlap: the line branch's quest/m1/auth/violations.md lists the same lite decode gap. The second to reach main drops it.
• quest check and CI pass. No review arrived; the maintainer's decision covers merging.

(Written by Claude Opus 5.5)