[kixelated]

Summary

Rust (moq-auth) and JS (@moq/auth) now replay one shared corpus, rs/moq-auth/tests/corpus.json, of tokens, JWKs, key sets and grants. Each case is either accepted, with the claims it verifies to, or refused. rs/moq-auth/src/corpus.rs and js/auth/src/corpus.test.ts run it, so a difference between the two parsers fails a test in both languages. The corpus replaces the hand-copied fixtures in js/auth/src/interop.test.ts, which was JS reading Rust-signed tokens, and in the key.rs tests, which was Rust reading JS-signed tokens. Both directions are now corpus cases. The layout follows the existing shared vectors: the Rust crate owns the file under tests/, Rust loads it with include_str!, and JS reads it by relative URL.

The corpus pins the fixes from the audit:

• Times: exp, iat, nbf and the Grant wire expires now refuse null. They also refuse anything that is not a whole number of seconds from 0 to 18446744073, the last second Rust's clock holds. Before this, Rust read "exp": null as never expiring, and it silently clamped negative or too-large times into range.
• Duplicate kid: a key set with two keys sharing a kid is refused in both languages. Before, the first match won.
• Scope root: a JWK's scope root must pass path::validate on import in both languages. In Rust the check moved into Scope::validate, so the separate checks in with_scope and the JS CLI were removed.
• Token detection in the relay: the relay's public rules now form-decode the query the way moq auth serve does, so ?%6Awt=<token> is refused instead of falling through to the public grant.
• Lease::new: it now runs Grant::validate. An expired grant maps to Refused with reason expired; it used to map to Unavailable, which tells a client to retry.
• Key caching in moq auth serve: key files are parsed once and cached by path. Each token costs a stat; the file is re-read and re-parsed only when its modification time or size changes, so rotating a key still needs no restart. A key file that disappears is dropped from the cache.

Impact

• moq-auth (wire, breaking): strict times on claims and grants; KeySet refuses duplicate kids; JWK import refuses a scope root that path::validate refuses. Migration notes are in rs/moq-auth/MIGRATE.md.
• moq-auth (wire): times are written as whole seconds, rounded down instead of to the nearest second. This keeps the clock's last second readable on the way back in, and an expiry can only move earlier.
• moq-auth::serve::Keys: the doc now says a key file is re-read when it changes rather than per request. No API change.
• @moq/auth (breaking): ClaimsSchema, GrantSchema, KeySetSchema and ScopeSchema have the same refusals. Migration notes are in js/auth/MIGRATE.md.
• moq-relay (breaking): Lease::new refuses a grant that Grant::validate refuses. Migration notes are in rs/moq-relay/MIGRATE.md.

Choices made without asking

• Key cache checks the file on each token. Loading keys only when the policy reloads would mean rotating a key file needs a config reload, and keys given on the command line would never reload. It would also touch the moq-cli watcher that moq-dev/moq#8205 is changing.
• Lease::new refuses an already-expired grant rather than returning a lease that ends immediately. This matches how Auth::admit treats an answered grant.
• Times are serialized rounded down. Without this, test certificates that expire past the year 2554 produced a grant expires of 18446744074, which the strict reader refuses. The mTLS integration tests caught this.
• Scope kept tight on nulls: Key::from_file_async is left in place (public API, now unused in this repo). Grant fields other than expires (root, revalidate, tier, label) still read null as absent in Rust, while JS refuses it. A null in any of those means the same as leaving the field out, so neither language reads a wider grant.

Overlap check

moq-dev/moq#8205 changes moq auth serve defaults for exp and sign. It doesn't touch exp parsing; this PR only changes which exp values parse. The two PRs may have a textual conflict in js/auth/src/cli.ts and serve.rs. moq-dev/moq#8088 narrows the moq-auth API surface and builds no corpus.

Caused by

No single PR. The Rust and JS parsers were written separately and only checked against a few fixtures; the cause was found by the 2026-11-07 invariant audit.

Checks

quest gate nix develop --command just ci passes on d72729e, after merging main. Main's moq-dev/moq#8088 made Key::to_encoding_key private, which broke the new corpus test in CI. It is pub(crate) again.

Suggested follow-ups

• Fix the test-order dependency in js/net: a connection samples the transport only while a reservation reads the estimate (js/net/src/connection/bandwidth.test.ts) fails when bun's directory-listing order runs auth.test.ts, integration.test.ts and socket.test.ts before it. That is the order in one local checkout. A fresh checkout of the same commit, and current main, pass. This PR doesn't touch js/net.

• Make Rust refuse null on every Grant wire field, matching JS. Recommendation: drop, because a null there means the same as an absent field.

Models

Size: S
Implementer: Claude Code, claude-opus-5-5, medium effort
Implementer: Claude Code, claude-opus-5-5, medium effort (iterate, XS)

(Written by Claude Opus 5.5 with medium effort)

Review

Rewritten by quest review after each round.

• Reviewer: Codex, gpt-6.1-sol, medium effort
• Session: 01a1b9ae-ce40-7341-8705-7926e7ed8c9e, round 1
• Head: d72729e
• Base: origin/main at 4b83dcd
• Score: 9/10, No actionable regressions found; parsing, caching, and admission changes appear consistent, but builds and tests were not run as instructed.
• Findings: none

[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

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 53e537dc-7f1c-4eb4-bb56-bcf653f771dc

Commits

Reviewing files that changed from the base of the PR and between be1c32f and d72729e.

Files selected for processing: 24

• doc/bin/relay/auth.md
• js/auth/MIGRATE.md
• js/auth/src/claims.ts
• js/auth/src/cli.ts
• js/auth/src/contract.ts
• js/auth/src/corpus.test.ts
• js/auth/src/interop.test.ts
• js/auth/src/set.ts
• js/auth/src/wire.ts
• quest/m1/README.md
• quest/m1/token-parse-strict.md
• rs/moq-auth/MIGRATE.md
• rs/moq-auth/src/claims.rs
• rs/moq-auth/src/corpus.rs
• rs/moq-auth/src/grant.rs
• rs/moq-auth/src/key.rs
• rs/moq-auth/src/lib.rs
• rs/moq-auth/src/serve.rs
• rs/moq-auth/src/set.rs
• rs/moq-auth/src/wire.rs
• rs/moq-auth/tests/corpus.json
• rs/moq-relay/MIGRATE.md
• rs/moq-relay/src/auth.rs
• rs/moq-relay/src/cluster.rs

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

Share

• X
• Mastodon
• Reddit
• LinkedIn

[kixelated]

Automated review by review (OpenAI)

Reviewed head af9ce72.

[P3] Keep migration-guide updates in the release cut — js/auth/MIGRATE.md:7–12 (also rs/moq-auth/MIGRATE.md:7–14 and rs/moq-relay/MIGRATE.md:8–13). Root AGENTS.md reserves MIGRATE.md edits for release cuts and requires breaking PRs to put migration instructions in their Impact section. These additions publish unreleased behavior into the package guides. Move the instructions into this PR's Impact section and leave the guide updates to the release cut.

Direction: the shared corpus and stricter validation are coherent. The current source rounds nbf upward and keys the cache by both path and format, addressing the two earlier issues described in the PR. No additional actionable runtime regression found in this static review.

Verification limits: GitHub-only review of the diff, relevant Rust/JS parsing, grant validation, key-cache code and fixtures. I did not run builds, tests or live rotation/integration checks.

[pullfrog]

ℹ️ One minor inconsistency between the PR description and the shipped public API.

Reviewed changes

• Shared corpus: rs/moq-auth/tests/corpus.json is replayed by rs/moq-auth/src/corpus.rs and js/auth/src/corpus.test.ts, replacing the hand-copied Rust-to-JS and JS-to-Rust interop fixtures; the relay contract tests stay in interop.test.ts.
• Strict times: token exp/iat/nbf and grant expires refuse null, negatives, fractions, and values past 18446744073 in both languages, with exp/iat written truncated and nbf written rounded up.
• Duplicate kid: KeySet parsing refuses two keys sharing a kid in Rust and JS.
• Scope roots: a JWK scope root is validated on import in both languages; the Rust check moved into Scope::validate and the duplicate checks in with_scope and the JS CLI were dropped.
• Relay token detection and lease validation: presents_token form-decodes the query, and Lease::new runs Grant::validate with GrantExpired mapping to Refused/expired.
• Key-file cache: serve::Policy caches parsed key files by path and format, re-reading only when the modification time or size changes and dropping a file that disappears.

Verified this run: cargo test -p moq-auth --lib (259 passed), cargo test -p moq-relay --lib auth::tests (34 passed), cd js/auth && bun test (166 passed, 55 corpus cases), plus tsc --noEmit and biome check on the changed JS.

  ^{｜ Fix all ➔ ｜ Fix 👍s ➔ ｜ View workflow run ｜ Using deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) ｜ 𝕏}

[pullfrog]

ℹ️ No new issues in the delta. The earlier note on js/auth/src/wire.ts:37 remains open and these commits do not touch it.

Reviewed changes

• Base-branch merges: two merges of origin/main bring #8088's API narrowing (private Key helpers, peer.rs removed) into the branch without changing the PR's own hunks.
• Corpus signing helper: Key::to_encoding_key is restored to pub(crate) because corpus.rs signs raw payloads with it; it stays crate-internal, so there is no public API change.

Verified this run: cd js/auth && bun test (166 passed, 0 failed, 55 corpus cases). The Rust suite was not re-run here because this checkout has no debug artifacts and a cold --release build exceeds the sandbox cap; CI covers it.

  ^{｜ Fix it ➔ ｜ View workflow run ｜ Using deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) ｜ 𝕏}

[pullfrog]

✅ No new issues found.

Reviewed changes

No commits landed since the prior Pullfrog review: the synchronize event re-ran at the same head d72729e, and git diff d72729e..HEAD is empty. The only change is the author's edit to the PR description.

• Impact wording corrected: the "New export: UnixSecondsSchema" line was dropped from the Impact section, matching the code. UnixSecondsSchema remains an internal helper in js/auth/src/wire.ts, imported by claims.ts and contract.ts; only PatternListSchema is re-exported via claims.ts, so the public API surface is unchanged.
• Prior thread resolved: the inline note on js/auth/src/wire.ts:37 is resolved and the substantive concern no longer holds.

  ^{｜ View workflow run ｜ Using deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) ｜ 𝕏}