fix(api): whitelist deployment endpoints from the general API rate limits #2774
Conversation
…mits Deployments are affected by general API rate limits, this is just a quick fix by whitelisting the deployment related endpoints. In a follow up PR we'll add a separate rate limiter for this group of endpoints.
|
WalkthroughThe change adds a new regex pattern entry ( Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/webapp/app/services/apiRateLimit.server.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects instead
Files:
apps/webapp/app/services/apiRateLimit.server.ts
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use zod for validation in packages/core and apps/webapp
Files:
apps/webapp/app/services/apiRateLimit.server.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Files:
apps/webapp/app/services/apiRateLimit.server.ts
apps/webapp/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Access all environment variables through the
envexport ofenv.server.tsinstead of directly accessingprocess.envin the Trigger.dev webapp
Files:
apps/webapp/app/services/apiRateLimit.server.ts
apps/webapp/app/services/**/*.server.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Separate testable services from configuration files; follow the pattern of
realtimeClient.server.ts(testable service) andrealtimeClientGlobal.server.ts(configuration) in the webapp
Files:
apps/webapp/app/services/apiRateLimit.server.ts
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
apps/webapp/**/*.{ts,tsx}: When importing from@trigger.dev/corein the webapp, use subpath exports from the package.json instead of importing from the root path
Follow the Remix 2.1.0 and Express server conventions when updating the main trigger.dev webapp
Files:
apps/webapp/app/services/apiRateLimit.server.ts
**/*.{js,ts,jsx,tsx,json,md,css,scss}
📄 CodeRabbit inference engine (AGENTS.md)
Format code using Prettier
Files:
apps/webapp/app/services/apiRateLimit.server.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (19)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: typecheck / typecheck
- GitHub Check: Analyze (javascript-typescript)
| "/api/v1/auth/jwt/claims", | ||
| /^\/api\/v1\/runs\/[^\/]+\/attempts$/, // /api/v1/runs/$runFriendlyId/attempts | ||
| /^\/api\/v1\/waitpoints\/tokens\/[^\/]+\/callback\/[^\/]+$/, // /api/v1/waitpoints/tokens/$waitpointFriendlyId/callback/$hash | ||
| /^\/api\/v1\/deployments/, // /api/v1/deployments/* |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the full apiRateLimit.server.ts file to understand context
cat -n apps/webapp/app/services/apiRateLimit.server.tsRepository: triggerdotdev/trigger.dev
Length of output: 3569
🏁 Script executed:
# Read the full apiRateLimit.server.ts file
wc -l apps/webapp/app/services/apiRateLimit.server.tsRepository: triggerdotdev/trigger.dev
Length of output: 119
🏁 Script executed:
# Get file size and content
cat -n apps/webapp/app/services/apiRateLimit.server.ts | head -100Repository: triggerdotdev/trigger.dev
Length of output: 3569
🏁 Script executed:
# Search for /api/v1/deployments route definitions
rg -n "api/v1/deployments" --type=ts --type=tsx -g '!node_modules' -g '!*.test.*' -A 5Repository: triggerdotdev/trigger.dev
Length of output: 96
🏁 Script executed:
# Find route files and deployment-related endpoints
find apps/webapp/app -name "*deploy*" -o -name "*route*" | head -20Repository: triggerdotdev/trigger.dev
Length of output: 1788
🏁 Script executed:
# Search for deployment route handlers in a different way
rg "deployments" apps/webapp/app --type=ts --type=tsx -l | grep -E "(route|api)" | head -10Repository: triggerdotdev/trigger.dev
Length of output: 96
🏁 Script executed:
# Look for routes directory structure
find apps/webapp/app -type d -name "routes" -o -name "api"Repository: triggerdotdev/trigger.dev
Length of output: 111
🏁 Script executed:
# Search for files containing /deployments in api context
rg "deployments" apps/webapp/app --type=ts --type=tsx -B 2 -A 2 | head -100Repository: triggerdotdev/trigger.dev
Length of output: 96
🏁 Script executed:
# List API directory structure
find apps/webapp/app/api -type f -name "*deploy*" | head -20Repository: triggerdotdev/trigger.dev
Length of output: 51
🏁 Script executed:
# Search for deployments in the api directory
rg "deployments" apps/webapp/app/api -t js -t ts -B 2 -A 5 2>/dev/null | head -150Repository: triggerdotdev/trigger.dev
Length of output: 51
🏁 Script executed:
# List the api directory structure
ls -la apps/webapp/app/api/ | head -30Repository: triggerdotdev/trigger.dev
Length of output: 243
🏁 Script executed:
# Find all TypeScript files in api directory
find apps/webapp/app/api -name "*.ts" -o -name "*.js" | grep -i deployRepository: triggerdotdev/trigger.dev
Length of output: 51
🏁 Script executed:
# Look at the structure of api directory
find apps/webapp/app/api -type d | head -20Repository: triggerdotdev/trigger.dev
Length of output: 88
🏁 Script executed:
# Search all app files for /api/v1/deployments references
rg "/api/v1/deployments" apps/webapp/app -t js -t ts 2>/dev/nullRepository: triggerdotdev/trigger.dev
Length of output: 172
🏁 Script executed:
# Check if deployments routes exist in routes directory
find apps/webapp/app/routes -name "*deploy*"Repository: triggerdotdev/trigger.dev
Length of output: 1697
🏁 Script executed:
# List what's in the api folder
ls -1 apps/webapp/app/api/ 2>/dev/null | head -50Repository: triggerdotdev/trigger.dev
Length of output: 80
🏁 Script executed:
# Examine the main deployments endpoint
cat -n apps/webapp/app/routes/api.v1.deployments.tsRepository: triggerdotdev/trigger.dev
Length of output: 7155
🏁 Script executed:
# Check the cancel endpoint to see what operations are being whitelisted
head -50 apps/webapp/app/routes/api.v1.deployments.$deploymentId.cancel.tsRepository: triggerdotdev/trigger.dev
Length of output: 180
🏁 Script executed:
# Check the finalize endpoint
head -50 apps/webapp/app/routes/api.v1.deployments.$deploymentId.finalize.tsRepository: triggerdotdev/trigger.dev
Length of output: 182
🏁 Script executed:
# List the exact deployment endpoint files
ls -1 apps/webapp/app/routes/api.v1.deployments*Repository: triggerdotdev/trigger.dev
Length of output: 729
🏁 Script executed:
# Check one of the state-changing endpoints
cat apps/webapp/app/routes/'api.v1.deployments.$deploymentId.cancel.ts' | head -40Repository: triggerdotdev/trigger.dev
Length of output: 1488
🏁 Script executed:
# Check how many deployment endpoints there are and their purpose
for file in apps/webapp/app/routes/api.v1.deployments*.ts; do
echo "=== $(basename "$file") ==="
head -20 "$file" | grep -E "(export|function|route)" | head -3
doneRepository: triggerdotdev/trigger.dev
Length of output: 1374
Verify if all /api/v1/deployments/* endpoints should be excluded from rate limiting.
The pathWhiteList excludes these endpoints from rate limiting entirely. Unlike other whitelist entries (which are webhook receivers like task callbacks and waitpoint callbacks), deployment endpoints are client-facing API operations that perform state changes: creating deployments, canceling them, finalizing them, and promoting versions.
Line 64's pattern /^\/api\/v1\/deployments/ is intentionally broad and will match all deployment sub-endpoints. These endpoints require API authentication but will no longer have rate limit protection, which differs from other whitelist entries that receive external webhooks and cannot control incoming request rates.
Consider whether this broad exclusion is appropriate, or if only specific callback-like sub-paths (if any exist) should be whitelisted instead. The temporary nature mentioned in the PR does not address the current unprotected state of these resource-creation operations.
🤖 Prompt for AI Agents
In apps/webapp/app/services/apiRateLimit.server.ts around line 64, the current
whitelist entry /^\/api\/v1\/deployments/ is too broad and removes rate-limiting
from all client-facing deployment endpoints; narrow the whitelist to only
specific webhook/callback subpaths that truly require exemption (or remove the
whitelist entry entirely) and ensure all state-changing routes (create, cancel,
finalize, promote) remain behind rate limiting and authentication; update the
pattern(s) to target only those exact callback paths (e.g., explicit /callbacks
or /webhook suffixes) or delete the entry and document the temporary exception
if it's required for a short window.
Deployments are affected by general API rate limits, this is just a quick fix by whitelisting the deployment related endpoints. In a follow up PR we'll add a separate rate limiter for this group of endpoints.