Repository navigation
feat(auth): add JWT signing prerequisites for actor streams - #5862
NathanFlurry wants to merge 1 commit into
Conversation
|
Stack for rivet-dev/rivet
Get stack: change xsszlsvl |
|
🚅 Deployed to the rivet-pr-5862 environment in rivet-frontend
4 services not affected by this PR
|
|
Review of PR 5862 (partial). I read the external-issuer verification path in auth-jwt/src/key_ring_cache/external.rs closely and did not review the rest line by line. Good: unverified iss and rivet_ns are only lookup hints, signature then issuer, audience, expiry and grant ceiling are all checked with no ACL fallback; the header is strictly parsed with pinned alg and bounded sizes; JWKS fetch disables redirects, pins DNS and uses an IP allowlist; cold-issuer admission is bounded. Issues: (1) external_cache is Mutex, which violates the CLAUDE.md rule against Mutex; use scc::HashMap or moka::Cache. (2) trim_verified_cache evicts the first verified key in key order, not LRU, so the same issuers are evicted under churn; moka would fix this too. (3) verify_external holds the per-issuer state mutex across the JWKS refresh (up to 5s), so verifications with already-cached keys queue behind it after TTL expiry; consider serving stale keys during a single-flight refresh. (4) tokio::time::sleep(5ms) in the webhook rotation retry path is an unexplained wall-clock delay; use an event-driven wait or document and back off. (5) public_ip is hand-rolled; add table-driven boundary tests (192.0.0.0/24, 192.0.2.0/24, 198.51.100.0/24, 255.255.255.255, IPv6 transition ranges). (6) I did not check the new logs for lowercase messages and structured fields. (7) The PR description is empty, and please confirm pnpm docs:gen-api was run for the generated artifacts. (8) The PR is about 18k lines; splitting it (external issuers, webhook signing/keys, application tokens/quota) would ease review. Security: no exploitable problem found in the paths I read; webhook key rotation/recovery and signing quota/audit were not reviewed in depth and deserve a second reviewer. |
No description provided.