Skip to content

chore: disable auth on serverless#4721

Closed
NathanFlurry wants to merge 1 commit into04-23-chore_fix_cors_for_envoysfrom
04-23-chore_disable_auth_on_serverless
Closed

chore: disable auth on serverless#4721
NathanFlurry wants to merge 1 commit into04-23-chore_fix_cors_for_envoysfrom
04-23-chore_disable_auth_on_serverless

Conversation

@NathanFlurry
Copy link
Copy Markdown
Member

Description

Please include a summary of the changes and the related issue. Please also include relevant motivation and context.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes.

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

Copy link
Copy Markdown
Member Author

NathanFlurry commented Apr 24, 2026

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

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

@claude
Copy link
Copy Markdown

claude Bot commented Apr 24, 2026

PR #4721 Review: chore: disable auth on serverless

Overview

This PR temporarily disables token authentication on the serverless /start endpoint in rivetkit-core. The root cause is a token mismatch: the incoming x-rivet-token carries the user's API token, but configured_token is expected to be a shared pool secret — a distinction that doesn't yet exist in the envoy-era serverless pool. The PR also removes the subtle crate dep, Forbidden error struct, and constant_time_eq helper.


Security Concerns (Primary)

  • Unauthenticated endpoint: The /start endpoint is now fully open to any caller that can reach the serverless runtime. No validation that the caller is authorized to start an actor.
  • Endpoint + namespace validation remains: The validate_endpoint path still enforces endpoint and namespace matching, which provides some constraint — but this is significantly weaker than token auth.
  • No linked follow-up issue: The TODO comment explains the intent to re-enable auth, but there's no issue number referenced (e.g. // TODO(#XXXX):). This increases the risk that the temporary disable becomes permanent. Consider linking a tracking issue before this is merged.

Code Quality

Positives:

  • The tracing::warn! additions for endpoint/namespace rejection are well-formed: structured fields, lowercase messages, correct conventions per CLAUDE.md.
  • Proper cleanup of all now-unused code (constant_time_eq, Forbidden, subtle dep in both Cargo.toml and Cargo.lock).
  • The TODO comment clearly explains the why.

Suggestions:

  • The commented-out block in validate_start_headers is fairly long. Since git preserves the history, consider deleting it entirely and referencing the tracking issue instead. Commented-out code adds noise and can mislead future readers.
  • If the commented-out code must stay, add an issue number to the TODO: // TODO(#XXXX): re-enable once envoy pool carries a shared secret.

Summary

The change is a deliberate and documented security regression with a clear rationale. Draft status is appropriate. The main ask before merging is to ensure a tracking issue exists for re-enabling auth, so this doesn't slip through. Everything else (logging, dep cleanup, error removal) looks correct.

@github-actions
Copy link
Copy Markdown
Contributor

Preview packages published to npm

Install with:

npm install rivetkit@pr-4721

All packages published as 0.0.0-pr.4721.a4e11fe with tag pr-4721.

Engine binary is shipped via @rivetkit/engine-cli on linux-x64-musl, linux-arm64-musl, darwin-x64, and darwin-arm64. Windows users should use the release installer or set RIVET_ENGINE_BINARY.

Docker images:

docker pull rivetdev/engine:slim-a4e11fe
docker pull rivetdev/engine:full-a4e11fe
Individual packages
npm install rivetkit@pr-4721
npm install @rivetkit/react@pr-4721
npm install @rivetkit/rivetkit-napi@pr-4721
npm install @rivetkit/workflow-engine@pr-4721

@NathanFlurry NathanFlurry force-pushed the 04-23-chore_fix_cors_for_envoys branch from 3f52ae7 to d81be66 Compare April 24, 2026 09:52
@NathanFlurry NathanFlurry force-pushed the 04-23-chore_disable_auth_on_serverless branch from 12a86d5 to 54eeddd Compare April 24, 2026 09:52
@NathanFlurry NathanFlurry mentioned this pull request Apr 24, 2026
11 tasks
@NathanFlurry NathanFlurry force-pushed the 04-23-chore_disable_auth_on_serverless branch from 54eeddd to f700e8a Compare April 24, 2026 10:19
@NathanFlurry NathanFlurry force-pushed the 04-23-chore_disable_auth_on_serverless branch from f700e8a to c2c77e5 Compare April 24, 2026 10:32
@NathanFlurry NathanFlurry force-pushed the 04-23-chore_disable_auth_on_serverless branch from c2c77e5 to 8bc569f Compare April 24, 2026 11:48
@NathanFlurry NathanFlurry force-pushed the 04-23-chore_fix_cors_for_envoys branch from 1d1fa8a to 6b87b7e Compare April 24, 2026 12:14
@NathanFlurry NathanFlurry force-pushed the 04-23-chore_disable_auth_on_serverless branch from 8bc569f to 1eb664c Compare April 24, 2026 12:14
@NathanFlurry NathanFlurry force-pushed the 04-23-chore_fix_cors_for_envoys branch from 6b87b7e to df85d9b Compare April 24, 2026 12:32
@NathanFlurry NathanFlurry force-pushed the 04-23-chore_disable_auth_on_serverless branch from 1eb664c to 77e200d Compare April 24, 2026 12:32
@NathanFlurry
Copy link
Copy Markdown
Member Author

Landed in main via stack-merge fast-forward push. Commits are in main; closing to match.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant