chore(guard): move token routing out off inner router for path-based routing

[NathanFlurry]

No description provided.

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Preview	Comments	Updated (UTC)
rivetkit-serverless	Error			Oct 31, 2025 1:23am

3 Skipped Deployments

Project	Deployment	Preview	Comments	Updated (UTC)
rivet-cloud	Ignored			Oct 31, 2025 1:23am
rivet-inspector	Ignored	Preview		Oct 31, 2025 1:23am
rivet-site	Ignored	Preview		Oct 31, 2025 1:23am

[NathanFlurry]

• chore(guard): move token routing out off inner router for path-based routing #3279 👈 (View in Graphite)
• chore(api-public): return build metadata #3278
• chore(scripts): add ping actor script #3277
• main

---

How to use the Graphite Merge Queue

Add the label merge-queue to this PR to add it to the merge queue.

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

[pkg-pr-new]

More templates

• example-ai-agent
• example-background-jobs
• example-better-auth-external-db
• example-bots
• chat-room
• example-cloudflare-workers
• example-cloudflare-workers-hono
• example-counter
• example-counter-serverless
• example-crdt
• example-cursors
• example-cursors-raw-websocket
• example-database
• example-deno
• example-sqlite
• example-elysia
• example-express
• example-freestyle
• example-game
• example-hono
• example-hono-bun
• example-hono-react
• example-kitchen-sink
• example-next-js
• example-rate
• example-raw-fetch-handler
• example-raw-websocket-handler
• example-raw-websocket-handler-proxy
• example-react
• example-smoke-test
• starter
• example-stream
• example-sync
• example-tenant
• example-trpc
• example-workflows

@rivetkit/actor

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/actor@3279

@rivetkit/cloudflare-workers

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/cloudflare-workers@3279

@rivetkit/core

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/core@3279

@rivetkit/db

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/db@3279

@rivetkit/framework-base

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/framework-base@3279

@rivetkit/next-js

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/next-js@3279

@rivetkit/react

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/react@3279

rivetkit

pnpm add https://pkg.pr.new/rivet-dev/rivet/rivetkit@3279

@rivetkit/sql-loader

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/sql-loader@3279

@rivetkit/engine-runner

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/engine-runner@3279

@rivetkit/engine-runner-protocol

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/engine-runner-protocol@3279

commit: e7c0c67

[claude]

Code Review

Overview

This PR refactors token routing in the guard package by moving token extraction logic out of the inner routing function to enable proper path-based routing. The changes improve the architecture by handling tokens at the routing layer where they belong.

Positive Aspects

1. Better Separation of Concerns: Moving token extraction to the outer routing functions is architecturally cleaner and allows path-based routing to work correctly.

2. Consistent Token Handling: The PR now consistently extracts tokens from both WebSocket protocols (rivet_token.*) and HTTP headers (x-rivet-token) in both routing paths.

3. Code Organization: Moving the WS_PROTOCOL_ACTOR and WS_PROTOCOL_TOKEN constants to the parent module makes them more accessible and reduces duplication.

4. Proper Error Handling: The WebSocket protocol parsing now properly validates the presence of required headers before attempting to parse them.

Issues and Concerns

1. Token Parameter Unused

The _token parameter is prefixed with underscore indicating it is intentionally unused. While the comment indicates validation happens in the EE edition, ensure EE implementation properly validates tokens to prevent unauthorized access.

2. Typo in Header Constant Name

X_RIVET_AMESPACE should be X_RIVET_NAMESPACE (missing N). This constant exists in the current code but should be fixed in a follow-up PR.

3. Error Message Could Be More Accurate

Line 102 uses error message invalid x-rivet-actor header but in the route_request_path_based code path, the actor ID comes from the URL path. Consider using a more generic message like invalid actor id format.

4. Typo in Log Message

Line 195: beocme should be become. This exists in the current code but noticed during review.

5. Missing Test Coverage

No unit tests found for the routing logic. Add tests for token extraction, error cases, and path-based routing scenarios.

6. Potential Edge Case: Empty Token

If a client sends rivet_token. with empty token after the dot, this will successfully extract an empty string. Consider validating that the token is not empty when present.

Performance Considerations

The changes have minimal performance impact. The protocols.split operation creates a small temporary allocation, but this is acceptable given WebSocket upgrade requests are infrequent.

Security Considerations

The code properly uses to_str() which will fail on invalid UTF-8, preventing header injection attacks. Ensure EE implementation validates tokens properly.

Code Quality

Follows repository conventions with proper error handling, structured logging, and consistent code style.

Recommendations Summary

Should Fix in Follow-up PRs:

1. Fix typo: X_RIVET_AMESPACE to X_RIVET_NAMESPACE
2. Fix typo: beocme to become
3. Make error message more generic (line 102)
4. Add validation for empty tokens
5. Add comprehensive test coverage

Conclusion

This is a solid refactoring that improves the architecture. The code is well-structured and follows the repository conventions. The main concerns are around test coverage and a few minor issues that can be addressed in follow-up PRs. Approved with minor suggestions.

[claude]

PR Review: Move token routing out of inner router for path-based routing

Summary

This PR refactors token extraction logic to support both header-based and WebSocket protocol-based routing for actor requests. The main change moves token parsing to the outer routing layer (route_request and route_request_path_based) and passes it down to route_request_inner, which currently doesn't use it (marked for EE implementation).

Code Quality & Best Practices

Positive Changes

✅ Consistent token extraction: Token is now extracted consistently in both path-based and header-based routing flows
✅ WebSocket support: Proper handling of rivet_token.* in WebSocket protocol headers
✅ Code organization: Constants (WS_PROTOCOL_ACTOR, WS_PROTOCOL_TOKEN) are now properly shared in mod.rs
✅ Error handling: Appropriate use of custom error types following the project's error handling patterns

Issues & Concerns

1. Premature allocation in WebSocket protocol parsing (pegboard_gateway.rs:60)

let protocols: Vec<&str> = protocols_header.split(',').map(|p| p.trim()).collect();

This allocates a Vec unnecessarily. Consider using an iterator directly:

let mut protocols = protocols_header.split(',').map(|p| p.trim());
let actor_id = protocols
    .find_map(|p| p.strip_prefix(WS_PROTOCOL_ACTOR))
    .ok_or_else(|| ...)?;
let token = protocols_header
    .split(',')
    .map(|p| p.trim())
    .find_map(|p| p.strip_prefix(WS_PROTOCOL_TOKEN));

Or if you need to iterate multiple times, reuse the split operation.

2. Inconsistent error handling for token parsing

For actor ID extraction, you return detailed errors when the header is invalid (lines 83, 94), but for token extraction, you only propagate the context without a custom error. Consider consistency:

• Either both should use custom MissingHeader errors
• Or both should use generic context

Given that token is optional, the current approach might be correct, but it's worth clarifying the intent.

3. Dead code / unused parameter (pegboard_gateway.rs:111)

The _token parameter in route_request_inner is unused with a comment "Token validation implemented in EE". Consider:

• Adding a #[allow(unused_variables)] attribute if this is intentional
• Or documenting why it's passed but not used (e.g., "prepared for EE extension")

Security Concerns

Medium Priority

⚠️ Token exposed in path parameters: The path-based routing supports tokens in URLs like /gateway/actors/{actor_id}/tokens/{token}/route/.... This is a security concern because:

• Tokens in URLs are logged by proxies, load balancers, and web servers
• They appear in browser history
• They may be cached or shared inadvertently

Recommendation:

• Document that path-based token routing is intended for specific use cases only
• Consider deprecating path-based token routing in favor of header-based or WebSocket protocol-based approaches
• Add security warnings in documentation if this pattern must remain

⚠️ No token validation: While marked as "implemented in EE", the lack of any validation in the OSS version means tokens are accepted but never checked. Consider:

• Adding a clear log statement when tokens are received but not validated
• Documenting the security implications for users of the OSS version

Performance Considerations

✅ No significant performance impact: The changes are minimal and don't introduce performance regressions
✅ Efficient string operations: Using strip_prefix and iterator methods appropriately

Test Coverage

⚠️ No tests found: I couldn't locate any tests for the routing logic. Given the complexity of this code (especially the path parsing in parse_actor_path), this is concerning.

Recommendations:

1. Add unit tests for parse_actor_path with various inputs:

   • Valid paths with and without tokens
   • Invalid paths (missing segments, double slashes, empty actor_id/token)
   • Edge cases (trailing slashes, query strings, fragments)

2. Add integration tests for token extraction:

   • WebSocket protocol parsing with multiple protocols
   • HTTP header extraction
   • Missing token scenarios

3. Test error conditions:

   • Missing headers
   • Invalid header values
   • Actor not found scenarios

Additional Observations

1. Logging: Good use of structured logging with tracing, following the project conventions (lowercase messages, structured fields)

2. Error context: Appropriate use of .context() for error propagation following the project's anyhow-based error handling

3. Code style: Follows the project's conventions (hard tabs, import organization)

Recommendations Summary

Must address:

• Consider the security implications of path-based token routing
• Add test coverage for the routing logic

Should address:

• Optimize WebSocket protocol parsing to avoid unnecessary allocation
• Document the unused _token parameter intent
• Add clear logging when tokens are received but not validated

Nice to have:

• Consistent error handling between actor_id and token extraction

Verdict

The code changes are functionally sound and follow project conventions, but would benefit from addressing the security concerns around path-based tokens and adding comprehensive test coverage before merging.

[claude]

PR Review: Move token routing out of inner router for path-based routing

Summary

This PR refactors token handling in the guard routing logic, moving token extraction to the outer routing layer to support path-based routing patterns. The changes enable tokens to be passed through both WebSocket protocols and HTTP headers to the inner routing logic.

Code Quality ✅

Positive aspects:

• Clean refactoring that consolidates WebSocket protocol constants (WS_PROTOCOL_ACTOR, WS_PROTOCOL_TOKEN) in a central location (routing/mod.rs)
• Consistent token extraction pattern across both WebSocket and HTTP paths
• Proper error handling using the custom RivetError system
• Good separation of concerns between path-based and header-based routing

Areas for improvement:

1. Duplicate constant definition (minor):

   • WS_PROTOCOL_TOKEN is now defined in both routing/mod.rs:20 and routing/runner.rs:7
   • The one in runner.rs:7 should be removed since it's now imported from the parent module
   • This could lead to maintenance issues if they diverge

2. Inconsistent error messages (minor):

   • Line 101 uses context: "invalid x-rivet-actor header" but the actor_id actually comes from actor_id_str which could be from either headers or WebSocket protocol
   • Consider a more generic message like "invalid actor id" since this is after the header/protocol extraction

Potential Bugs 🟡

WebSocket protocol parsing:

• The code in pegboard_gateway.rs:60 collects all protocols into a Vec<&str> and then searches for the token
• This is fine, but worth noting that if a client sends duplicate rivet_token.* protocols, only the first one will be used (due to find_map)
• Consider if this is the desired behavior or if duplicate tokens should be an error

Token parameter now always passed but unused:

• Line 111 shows _token: Option<&str> is unused with the comment "NOTE: Token validation implemented in EE"
• While this is intentional, it means the open-source version now extracts and passes tokens without validating them
• This is likely acceptable for the enterprise edition integration, but ensure the EE implementation properly overrides this function

Performance Considerations ✅

No significant performance concerns:

• The additional protocol parsing (collecting into a Vec) is minimal overhead
• Token extraction happens once per request
• The refactoring doesn't introduce any new database queries or network calls

Security Concerns 🟡

1. Token validation deferred to EE:

   • Line 113: "NOTE: Token validation implemented in EE" means tokens are extracted but not validated in the open-source version
   • Ensure that:
     • The EE version properly validates tokens before allowing access
     • There's no security regression where tokens could bypass validation
     • Path-based token routing has the same security guarantees as header-based routing

2. Token exposure in logs:

   • Verify that tokens from WS_PROTOCOL_TOKEN and X_RIVET_TOKEN are not being logged
   • The current #[tracing::instrument(skip_all)] should prevent this, which is good

3. Consistent token handling:

   • Both WebSocket and HTTP paths now extract tokens consistently
   • Good: Tokens are optional (Option<&str>), matching the path-based routing signature

Test Coverage ✅

Excellent test coverage for path parsing:

• The tests/parse_actor_path.rs file has comprehensive tests covering:
  • Valid paths with and without tokens
  • Query parameters and fragments
  • Edge cases (empty paths, trailing slashes, special characters)
  • Invalid inputs (malformed paths, wrong segments, case sensitivity)

Missing test coverage:

• No tests specifically for the WebSocket token extraction logic in pegboard_gateway.rs:72-74
• No tests for HTTP header token extraction in pegboard_gateway.rs:91-95
• Consider adding integration tests that verify:
  • WebSocket requests with rivet_token.* protocol headers
  • HTTP requests with x-rivet-token headers
  • Both path-based and header-based routing with tokens

Style & Conventions ✅

The code follows the repository's conventions well:

• Uses anyhow::* for error handling
• Structured logging with tracing (using ? for debug formatting)
• Lowercase log messages: "routing using path-based actor routing"
• Proper use of workspace dependencies
• Hard tabs for formatting (as per rustfmt.toml)

Recommendations

1. High priority: Remove the duplicate WS_PROTOCOL_TOKEN constant from runner.rs:7
2. Medium priority: Add integration tests for WebSocket and HTTP token extraction
3. Low priority: Update error message at line 101 to be more generic
4. Security: Verify EE implementation validates tokens properly for path-based routing

Overall Assessment

This is a solid refactoring that improves code organization and enables path-based token routing. The main concern is the duplicate constant definition, which should be cleaned up. The security implications depend on the EE implementation, which should be verified separately.

Recommendation: Approve with minor cleanup (remove duplicate constant)

🤖 Generated with Claude Code