Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
2 Skipped Deployments
|
|
|
Caution Review failedThe pull request is closed. WalkthroughWalkthroughThe changes involve the introduction of a new optional environment variable, Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Server
participant RateLimiter
Client->>Server: Request to create resource
Server->>RateLimiter: Check rate limit for user
RateLimiter-->>Server: Rate limit status
alt Within limit
Server->>Client: Process request and return response
else Exceeds limit
Server->>Client: Return error "TOO_MANY_REQUESTS"
end
sequenceDiagram
participant Client
participant Server
participant RateLimiter
Client->>Server: Request to delete resource
Server->>RateLimiter: Check rate limit for user
RateLimiter-->>Server: Rate limit status
alt Within limit
Server->>Client: Process request and return response
else Exceeds limit
Server->>Client: Return error "TOO_MANY_REQUESTS"
end
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
Thank you for following the naming conventions for pull request titles! 🙏 |
MichaelUnkey
requested review from
chronark,
mcstepp and
perkinsjr
as code owners
![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
Outside diff range, codebase verification and nitpick comments (1)
apps/dashboard/lib/env.ts (1)
35-35: LGTM! Update the documentation.The code change is approved.
Please ensure that the documentation is updated to reflect this new required environment variable.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (53)
- apps/dashboard/lib/env.ts (1 hunks)
- apps/dashboard/lib/ratelimitValues.ts (1 hunks)
- apps/dashboard/lib/trpc/routers/api/create.ts (1 hunks)
- apps/dashboard/lib/trpc/routers/api/delete.ts (1 hunks)
- apps/dashboard/lib/trpc/routers/api/updateDeleteProtection.ts (1 hunks)
- apps/dashboard/lib/trpc/routers/api/updateIpWhitelist.ts (1 hunks)
- apps/dashboard/lib/trpc/routers/api/updateName.ts (1 hunks)
- apps/dashboard/lib/trpc/routers/gateway/create.ts (1 hunks)
- apps/dashboard/lib/trpc/routers/key/create.ts (1 hunks)
- apps/dashboard/lib/trpc/routers/key/createRootKey.ts (1 hunks)
- apps/dashboard/lib/trpc/routers/key/delete.ts (1 hunks)
- apps/dashboard/lib/trpc/routers/key/deleteRootKey.ts (1 hunks)
- apps/dashboard/lib/trpc/routers/key/updateEnabled.ts (1 hunks)
- apps/dashboard/lib/trpc/routers/key/updateExpiration.ts (1 hunks)
- apps/dashboard/lib/trpc/routers/key/updateMetadata.ts (1 hunks)
- apps/dashboard/lib/trpc/routers/key/updateName.ts (1 hunks)
- apps/dashboard/lib/trpc/routers/key/updateOwnerId.ts (1 hunks)
- apps/dashboard/lib/trpc/routers/key/updateRatelimit.ts (1 hunks)
- apps/dashboard/lib/trpc/routers/key/updateRemaining.ts (1 hunks)
- apps/dashboard/lib/trpc/routers/llmGateway/create.ts (1 hunks)
- apps/dashboard/lib/trpc/routers/llmGateway/delete.ts (1 hunks)
- apps/dashboard/lib/trpc/routers/monitor/verification/create.ts (1 hunks)
- apps/dashboard/lib/trpc/routers/plain.ts (1 hunks)
- apps/dashboard/lib/trpc/routers/ratelimit/createNamespace.ts (1 hunks)
- apps/dashboard/lib/trpc/routers/ratelimit/createOverride.ts (1 hunks)
- apps/dashboard/lib/trpc/routers/ratelimit/deleteNamespace.ts (1 hunks)
- apps/dashboard/lib/trpc/routers/ratelimit/deleteOverride.ts (1 hunks)
- apps/dashboard/lib/trpc/routers/ratelimit/updateNamespaceName.ts (1 hunks)
- apps/dashboard/lib/trpc/routers/ratelimit/updateOverride.ts (1 hunks)
- apps/dashboard/lib/trpc/routers/rbac.ts (13 hunks)
- apps/dashboard/lib/trpc/routers/rbac/addPermissionToRootKey.ts (1 hunks)
- apps/dashboard/lib/trpc/routers/rbac/connectPermissionToRole.ts (1 hunks)
- apps/dashboard/lib/trpc/routers/rbac/connectRoleToKey.ts (1 hunks)
- apps/dashboard/lib/trpc/routers/rbac/createPermission.ts (2 hunks)
- apps/dashboard/lib/trpc/routers/rbac/createRole.ts (2 hunks)
- apps/dashboard/lib/trpc/routers/rbac/deletePermission.ts (1 hunks)
- apps/dashboard/lib/trpc/routers/rbac/deleteRole.ts (1 hunks)
- apps/dashboard/lib/trpc/routers/rbac/disconnectPermissionFromRole.ts (1 hunks)
- apps/dashboard/lib/trpc/routers/rbac/disconnectRoleFromKey.ts (1 hunks)
- apps/dashboard/lib/trpc/routers/rbac/removePermissionFromRootKey.ts (1 hunks)
- apps/dashboard/lib/trpc/routers/rbac/updatePermission.ts (2 hunks)
- apps/dashboard/lib/trpc/routers/rbac/updateRole.ts (2 hunks)
- apps/dashboard/lib/trpc/routers/secrets/create.ts (1 hunks)
- apps/dashboard/lib/trpc/routers/secrets/decrypt.ts (1 hunks)
- apps/dashboard/lib/trpc/routers/secrets/update.ts (1 hunks)
- apps/dashboard/lib/trpc/routers/webhook/create.ts (1 hunks)
- apps/dashboard/lib/trpc/routers/webhook/delete.ts (1 hunks)
- apps/dashboard/lib/trpc/routers/webhook/toggle.ts (1 hunks)
- apps/dashboard/lib/trpc/routers/workspace/changeName.ts (1 hunks)
- apps/dashboard/lib/trpc/routers/workspace/changePlan.ts (1 hunks)
- apps/dashboard/lib/trpc/routers/workspace/create.ts (1 hunks)
- apps/dashboard/lib/trpc/routers/workspace/optIntoBeta.ts (1 hunks)
- apps/dashboard/lib/trpc/trpc.ts (2 hunks)
Files skipped from review due to trivial changes (1)
- apps/dashboard/lib/ratelimitValues.ts
Additional comments not posted (112)
apps/dashboard/lib/trpc/trpc.ts (4)
4-4: LGTM!The import statement is approved.
5-5: LGTM!The import statement is approved.
23-23: LGTM!The
protectedProceduredeclaration is approved.
25-56: LGTM!The
rateLimitedProceduredeclaration is approved. It provides a reusable way to enforce rate limits on procedures and is correctly implemented.apps/dashboard/lib/trpc/routers/key/updateEnabled.ts (2)
2-2: LGTM!The import statement is approved.
8-11: LGTM!The changes to the
updateKeyEnabledprocedure are approved. Using therateLimitedProcedureinstead of theauthmiddleware is a good way to enforce rate limits on the procedure.apps/dashboard/lib/trpc/routers/llmGateway/delete.ts (2)
5-5: LGTM!The import statement is approved.
9-12: LGTM!The changes to the
deleteLlmGatewayprocedure are approved. Using therateLimitedProcedureinstead of theauthmiddleware is a good way to enforce rate limits on the procedure.apps/dashboard/lib/trpc/routers/key/updateName.ts (1)
2-2: LGTM! Verify the rate limit values.The code changes are approved.
The addition of rate limiting functionality using
rateLimitedProcedureis a critical enhancement that improves the robustness of the application.Please ensure that the
UPDATE_LIMITandUPDATE_LIMIT_DURATIONvalues are appropriate for the use case. Run the following script to verify the rate limit values:Also applies to: 7-12
Verification successful
Rate Limit Values Verified
The rate limit values for the
updateKeyNameprocedure have been verified:
UPDATE_LIMITis set to25.UPDATE_LIMIT_DURATIONis set to5.These constants are defined in
apps/dashboard/lib/ratelimitValues.ts. Please ensure these values align with your application's requirements.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the rate limit values for the `updateKeyName` procedure. # Test: Search for the rate limit constants. Expect: Appropriate values based on the use case. rg --type typescript -A 5 $'UPDATE_LIMIT' rg --type typescript -A 5 $'UPDATE_LIMIT_DURATION'Length of output: 160
Script:
#!/bin/bash # Use ast-grep to search for the declarations of UPDATE_LIMIT and UPDATE_LIMIT_DURATION in the codebase. ast-grep --lang typescript --pattern 'const UPDATE_LIMIT = $_' ast-grep --lang typescript --pattern 'const UPDATE_LIMIT_DURATION = $_'Length of output: 290
apps/dashboard/lib/trpc/routers/api/updateName.ts (1)
5-5: Rate limiting looks good, but verify the removal ofauthmiddleware.The addition of rate limiting functionality using
rateLimitedProcedureis a good enhancement.However, the removal of the
authmiddleware raises concerns about the procedure's security. Please ensure that the removal of authentication is intentional and does not introduce any security vulnerabilities.Run the following script to verify the usage of the
authmiddleware in other procedures:If the removal of
authis unintentional, consider applying this diff to reintroduce authentication:-export const updateApiName = rateLimitedProcedure({ - limit: UPDATE_LIMIT, - duration: UPDATE_LIMIT_DURATION, -}) +export const updateApiName = rateLimitedProcedure({ + limit: UPDATE_LIMIT, + duration: UPDATE_LIMIT_DURATION, +}).use(auth)Also applies to: 7-12
apps/dashboard/lib/trpc/routers/key/updateOwnerId.ts (3)
2-2: LGTM!The import statement is correct and the constants are used to configure the rate limiting.
7-7: LGTM!The import statement is correct and
rateLimitedProcedureis used to replace theauthmiddleware.
9-12: LGTM!The changes are correct and enhance the control over the procedure's usage by replacing the
authmiddleware withrateLimitedProcedure. The rate limiting parameters are correctly passed torateLimitedProcedure.apps/dashboard/lib/trpc/routers/secrets/decrypt.ts (3)
2-2: LGTM!The import statement is correct and the constants are used to configure the rate limiting.
6-6: LGTM!The import statement is correct and
rateLimitedProcedureis used to replace theauthmiddleware.
8-11: LGTM!The changes are correct and enhance the control over the procedure's usage by replacing the
authmiddleware withrateLimitedProcedure. The rate limiting parameters are correctly passed torateLimitedProcedure.apps/dashboard/lib/trpc/routers/webhook/delete.ts (3)
2-2: LGTM!The import statement is correct and the constants are used to configure the rate limiting.
6-6: LGTM!The import statement is correct and
rateLimitedProcedureis used to replace theauthmiddleware.
8-11: LGTM!The changes are correct and enhance the control over the procedure's usage by replacing the
authmiddleware withrateLimitedProcedure. The rate limiting parameters are correctly passed torateLimitedProcedure.apps/dashboard/lib/trpc/routers/api/updateDeleteProtection.ts (2)
5-7: LGTM!The code changes are approved.
9-12: LGTM!The code changes are approved.
Replacing the
authmiddleware withrateLimitedProcedureis a good enhancement. It helps prevent abuse by enforcing rate limits on the procedure using the configuredlimitanddurationoptions.apps/dashboard/lib/trpc/routers/rbac/disconnectRoleFromKey.ts (2)
2-2: LGTM!The code changes are approved.
6-11: LGTM!The code changes are approved.
Replacing the
authmiddleware withrateLimitedProcedureis a good enhancement. It helps prevent abuse by enforcing rate limits on the procedure using the configuredlimitanddurationoptions.apps/dashboard/lib/trpc/routers/rbac/deleteRole.ts (2)
2-2: LGTM!The code changes are approved.
6-11: LGTM!The code changes are approved.
Replacing the
authmiddleware withrateLimitedProcedureis a good enhancement. It helps prevent abuse by enforcing rate limits on the procedure using the configuredlimitanddurationoptions.apps/dashboard/lib/trpc/routers/ratelimit/createNamespace.ts (2)
5-5: LGTM!The code changes are approved.
9-14: LGTM, but verify the user experience.The code changes are approved.
However, please ensure that the rate limiting parameters are set appropriately to provide a good user experience and do not unnecessarily restrict legitimate usage of the namespace creation functionality.
To verify the user experience, consider performing the following tests:
- Create namespaces in quick succession and ensure that the rate limiting kicks in as expected without negatively impacting the user experience.
- Verify that the rate limiting parameters (
CREATE_LIMITandCREATE_LIMIT_DURATION) are set to reasonable values that align with the expected usage patterns of the namespace creation functionality.apps/dashboard/lib/trpc/routers/workspace/changeName.ts (2)
2-2: LGTM!The code changes are approved.
7-12: LGTM, but verify the user experience.The code changes are approved.
However, please ensure that the rate limiting parameters are set appropriately to provide a good user experience and do not unnecessarily restrict legitimate usage of the workspace name change functionality.
To verify the user experience, consider performing the following tests:
- Change the workspace name multiple times in quick succession and ensure that the rate limiting kicks in as expected without negatively impacting the user experience.
- Verify that the rate limiting parameters (
UPDATE_LIMITandUPDATE_LIMIT_DURATION) are set to reasonable values that align with the expected usage patterns of the workspace name change functionality.apps/dashboard/lib/trpc/routers/workspace/optIntoBeta.ts (2)
2-2: LGTM!The code changes are approved.
6-11: LGTM, but verify the user experience.The code changes are approved.
However, please ensure that the rate limiting parameters are set appropriately to provide a good user experience and do not unnecessarily restrict legitimate usage of the beta opt-in functionality.
To verify the user experience, consider performing the following tests:
- Opt into beta features multiple times in quick succession and ensure that the rate limiting kicks in as expected without negatively impacting the user experience.
- Verify that the rate limiting parameters (
CREATE_LIMITandCREATE_LIMIT_DURATION) are set to reasonable values that align with the expected usage patterns of the beta opt-in functionality.apps/dashboard/lib/trpc/routers/rbac/disconnectPermissionFromRole.ts (2)
2-2: LGTM!The changes to the import statements are approved.
Also applies to: 6-6
8-11: LGTM!The changes to introduce rate limiting to the
disconnectPermissionFromRoleprocedure are approved.apps/dashboard/lib/trpc/routers/webhook/toggle.ts (2)
2-2: LGTM!The changes to the import statements are approved.
Also applies to: 6-6
8-11: LGTM!The changes to introduce rate limiting to the
toggleWebhookprocedure are approved.apps/dashboard/lib/trpc/routers/key/delete.ts (2)
2-2: LGTM!The changes to the import statements are approved.
Also applies to: 6-6
8-11: LGTM!The changes to introduce rate limiting to the
deleteKeysprocedure are approved.apps/dashboard/lib/trpc/routers/rbac/createPermission.ts (3)
2-2: LGTM!The import statement is correctly importing the required rate limit constants.
7-7: LGTM!The import statement is correctly importing the
rateLimitedProcedure.
17-20: LGTM, but verify the rate limit values.The procedure changes are approved.
The
createPermissionprocedure is now using therateLimitedProcedurewith the imported rate limit constants.Please ensure that the
CREATE_LIMITandCREATE_LIMIT_DURATIONvalues are appropriate for this procedure.apps/dashboard/lib/trpc/routers/rbac/deletePermission.ts (3)
2-2: LGTM!The import statement is correctly importing the required rate limit constants.
6-6: LGTM!The import statement is correctly importing the
rateLimitedProcedure.
8-11: LGTM, but verify the rate limit values.The procedure changes are approved.
The
deletePermissionprocedure is now using therateLimitedProcedurewith the imported rate limit constants.Please ensure that the
DELETE_LIMITandDELETE_LIMIT_DURATIONvalues are appropriate for this procedure.apps/dashboard/lib/trpc/routers/ratelimit/deleteOverride.ts (3)
5-5: LGTM!The import statement is correctly importing the required rate limit constants.
7-7: LGTM!The import statement is correctly importing the
rateLimitedProcedure.
9-12: LGTM, but verify the rate limit values.The procedure changes are approved.
The
deleteOverrideprocedure is now using therateLimitedProcedurewith the imported rate limit constants.Please ensure that the
DELETE_LIMITandDELETE_LIMIT_DURATIONvalues are appropriate for this procedure.apps/dashboard/lib/trpc/routers/rbac/connectPermissionToRole.ts (2)
2-2: LGTM!The import statement is correct and necessary for the rate limiting functionality.
5-10: LGTM, but verify the rate limit values.The changes introduce rate limiting to the
connectPermissionToRoleprocedure. This is a good practice to prevent abuse and manage resource usage.Please ensure that the
UPDATE_LIMITandUPDATE_LIMIT_DURATIONvalues are appropriate for this procedure.apps/dashboard/lib/trpc/routers/llmGateway/create.ts (2)
2-2: LGTM!The import statement is correct and necessary for the rate limiting functionality.
8-13: LGTM, but verify the rate limit values.The changes introduce rate limiting to the
createLlmGatewayprocedure. This is a good practice to prevent abuse and manage resource usage.Please ensure that the
CREATE_LIMITandCREATE_LIMIT_DURATIONvalues are appropriate for this procedure.apps/dashboard/lib/trpc/routers/key/deleteRootKey.ts (3)
1-1: LGTM!The import statement is correct and necessary for the database operations.
3-3: LGTM!The import statement is correct and necessary for the rate limiting functionality.
7-12: LGTM, but verify the rate limit values.The changes introduce rate limiting to the
deleteRootKeysprocedure. This is a good practice to prevent abuse and manage resource usage.Please ensure that the
DELETE_LIMITandDELETE_LIMIT_DURATIONvalues are appropriate for this procedure.apps/dashboard/lib/trpc/routers/key/updateMetadata.ts (2)
2-2: LGTM!The import statement is correct.
8-11: LGTM, but verify the impact of rate limiting.The changes to introduce rate limiting to the
updateKeyMetadataprocedure are approved.However, please ensure that the chosen rate limiting parameters (
UPDATE_LIMITandUPDATE_LIMIT_DURATION) are appropriate for your use case and won't negatively impact the user experience.To verify the impact of rate limiting, consider load testing the API with a tool like Artillery or Locust to ensure that the rate limits are not too restrictive and that the API can handle the expected load.
apps/dashboard/lib/trpc/routers/rbac/updateRole.ts (2)
2-2: LGTM!The import statement is correct.
16-19: LGTM, but verify the impact of rate limiting.The changes to introduce rate limiting to the
updateRoleprocedure are approved.However, please ensure that the chosen rate limiting parameters (
UPDATE_LIMITandUPDATE_LIMIT_DURATION) are appropriate for your use case and won't negatively impact the user experience.To verify the impact of rate limiting, consider load testing the API with a tool like Artillery or Locust to ensure that the rate limits are not too restrictive and that the API can handle the expected load.
apps/dashboard/lib/trpc/routers/api/create.ts (2)
5-5: LGTM!The import statement is correct.
10-13: LGTM, but verify the impact of rate limiting.The changes to introduce rate limiting to the
createApiprocedure are approved.However, please ensure that the chosen rate limiting parameters (
CREATE_LIMITandCREATE_LIMIT_DURATION) are appropriate for your use case and won't negatively impact the user experience.To verify the impact of rate limiting, consider load testing the API with a tool like Artillery or Locust to ensure that the rate limits are not too restrictive and that the API can handle the expected load.
apps/dashboard/lib/trpc/routers/secrets/create.ts (2)
3-3: LGTM!The import statement for the rate limit constants is approved.
12-15: Approve the switch torateLimitedProcedure, but verify the rate limit values.The change from
authtorateLimitedProcedureis approved as it introduces necessary rate limiting to thecreateSecretprocedure.However, please ensure that the values of
CREATE_LIMITandCREATE_LIMIT_DURATIONare appropriate for your use case and won't negatively impact the user experience.apps/dashboard/lib/trpc/routers/api/updateIpWhitelist.ts (2)
5-5: LGTM!The import statement for the rate limit constants is approved.
9-12: Approve the switch torateLimitedProcedure, but verify the rate limit values.The change from
authtorateLimitedProcedureis approved as it introduces necessary rate limiting to theupdateApiIpWhitelistprocedure.However, please ensure that the values of
UPDATE_LIMITandUPDATE_LIMIT_DURATIONare appropriate for your use case and won't negatively impact the user experience.apps/dashboard/lib/trpc/routers/rbac/updatePermission.ts (2)
2-2: LGTM!The import statement for the rate limit constants is approved.
16-19: Approve the switch torateLimitedProcedure, but verify the rate limit values.The change from
authtorateLimitedProcedureis approved as it introduces necessary rate limiting to theupdatePermissionprocedure.However, please ensure that the values of
UPDATE_LIMITandUPDATE_LIMIT_DURATIONare appropriate for your use case and won't negatively impact the user experience.apps/dashboard/lib/trpc/routers/workspace/create.ts (2)
2-2: LGTM!The import statement is correct and necessary for the rate limiting functionality.
11-14: LGTM, but verify the rate limiting functionality.The changes to introduce rate limiting to the
createWorkspaceprocedure are approved.However, ensure that the rate limiting functionality is thoroughly tested to confirm that it behaves as expected and enforces the specified rate limits.
Run the following script to verify the rate limiting functionality:
apps/dashboard/lib/trpc/routers/ratelimit/updateNamespaceName.ts (2)
5-5: LGTM!The import statement is correct and necessary for the rate limiting functionality.
9-12: LGTM, but verify the rate limiting functionality.The changes to introduce rate limiting to the
updateNamespaceNameprocedure are approved.However, ensure that the rate limiting functionality is thoroughly tested to confirm that it behaves as expected and enforces the specified rate limits.
Run the following script to verify the rate limiting functionality:
apps/dashboard/lib/trpc/routers/ratelimit/updateOverride.ts (2)
5-5: LGTM!The import statement is correct and necessary for the rate limiting functionality.
9-12: LGTM, but verify the rate limiting functionality.The changes to introduce rate limiting to the
updateOverrideprocedure are approved.However, ensure that the rate limiting functionality is thoroughly tested to confirm that it behaves as expected and enforces the specified rate limits.
Run the following script to verify the rate limiting functionality:
Verification successful
Rate Limiting Functionality Verified
The rate limiting functionality for the
updateOverrideprocedure is correctly implemented withUPDATE_LIMITset to 25 andUPDATE_LIMIT_DURATIONset to 5. These values are appropriate for controlling the request rate. No further issues were found.
UPDATE_LIMITandUPDATE_LIMIT_DURATIONare defined inapps/dashboard/lib/ratelimitValues.ts.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the rate limiting functionality of the `updateOverride` procedure. # Test: Call the `updateOverride` procedure multiple times and expect it to be rate limited after exceeding the limit. rg --type typescript -A 5 $'updateOverride'Length of output: 77
Script:
#!/bin/bash # Use ast-grep to locate the `updateOverride` function and examine its implementation. ast-grep --lang typescript --pattern $'export const updateOverride = rateLimitedProcedure({ $$$ })'Length of output: 100
Script:
#!/bin/bash # Use rg to search for the `updateOverride` function without specifying a file type. rg 'export const updateOverride = rateLimitedProcedure' -A 5Length of output: 561
Script:
#!/bin/bash # Search for the definitions of `UPDATE_LIMIT` and `UPDATE_LIMIT_DURATION` to verify their values. rg 'const UPDATE_LIMIT' -A 2 rg 'const UPDATE_LIMIT_DURATION' -A 2Length of output: 512
apps/dashboard/lib/trpc/routers/key/updateExpiration.ts (1)
2-2: Rate limiting added to the procedure.The changes introduce rate limiting to the
updateKeyExpirationprocedure, which is a good practice to prevent abuse. Theauthmiddleware has been replaced withrateLimitedProcedure, and new imports for rate limit constants have been added.The code changes are straightforward and do not introduce any issues. The input validation remains unchanged, ensuring that the procedure still expects the same input as before.
Also applies to: 6-6, 8-11
apps/dashboard/lib/trpc/routers/rbac/connectRoleToKey.ts (1)
2-2: Rate limiting added to the procedure.The changes introduce rate limiting to the
connectRoleToKeyprocedure, which is a good practice to prevent abuse. Theauthmiddleware has been replaced withrateLimitedProcedure, and new imports for rate limit constants have been added.The code changes are straightforward and do not introduce any issues. The input validation remains unchanged, ensuring that the procedure still expects the same input as before.
Also applies to: 6-6, 8-11
apps/dashboard/lib/trpc/routers/key/updateRemaining.ts (1)
2-2: Rate limiting added to the procedure.The changes introduce rate limiting to the
updateKeyRemainingprocedure, which is a good practice to prevent abuse. Theauthmiddleware has been replaced withrateLimitedProcedure, and new imports for rate limit constants have been added.The code changes are straightforward and do not introduce any issues. The input validation remains unchanged, ensuring that the procedure still expects the same input as before.
Also applies to: 6-6, 8-11
apps/dashboard/lib/trpc/routers/monitor/verification/create.ts (1)
10-13: LGTM!The code changes are approved.
The
authmiddleware has been replaced withrateLimitedProcedureto introduce rate limiting to thecreateVerificationMonitorprocedure. The rate limiting parameters are imported from a centralized file, which is a good practice for managing rate limits across the application.apps/dashboard/lib/trpc/routers/secrets/update.ts (1)
9-12: LGTM!The code changes are approved.
The
authmiddleware has been replaced withrateLimitedProcedureto introduce rate limiting to theupdateSecretprocedure. The rate limiting parameters are imported from a centralized file, which is a good practice for managing rate limits across the application.apps/dashboard/lib/trpc/routers/rbac/removePermissionFromRootKey.ts (1)
8-11: LGTM!The code changes are approved.
The
authmiddleware has been replaced withrateLimitedProcedureto introduce rate limiting to theremovePermissionFromRootKeyprocedure. The rate limiting parameters are imported from a centralized file, which is a good practice for managing rate limits across the application.apps/dashboard/lib/trpc/routers/rbac/addPermissionToRootKey.ts (2)
2-2: LGTM!The import statement is correct and necessary for the rate limiting functionality.
10-13: LGTM, but verify the rate limit values.The changes are approved. The
authmiddleware has been correctly replaced withrateLimitedProcedureto introduce rate limiting functionality. The rate limiting parameters are correctly configured using the imported constants.However, ensure that the
UPDATE_LIMITandUPDATE_LIMIT_DURATIONvalues are appropriate for this procedure based on the expected usage patterns and performance requirements.apps/dashboard/lib/trpc/routers/rbac/createRole.ts (2)
2-2: LGTM!The import statement is correct and necessary for the rate limiting functionality.
17-20: LGTM, but verify the rate limit values.The changes are approved. The
authmiddleware has been correctly replaced withrateLimitedProcedureto introduce rate limiting functionality. The rate limiting parameters are correctly configured using the imported constants.However, ensure that the
CREATE_LIMITandCREATE_LIMIT_DURATIONvalues are appropriate for this procedure based on the expected usage patterns and performance requirements.apps/dashboard/lib/trpc/routers/plain.ts (2)
2-2: LGTM!The import statement is correct and necessary for the rate limiting functionality.
11-14: LGTM, but verify the rate limit values.The changes are approved. The
authmiddleware has been correctly replaced withrateLimitedProcedureto introduce rate limiting functionality. The rate limiting parameters are correctly configured using the imported constants.However, ensure that the
CREATE_LIMITandCREATE_LIMIT_DURATIONvalues are appropriate for this procedure based on the expected usage patterns and performance requirements.apps/dashboard/lib/trpc/routers/api/delete.ts (3)
5-5: LGTM!The code changes are approved.
7-7: LGTM!The code changes are approved.
9-12: LGTM, but verify the rate limiting.The code changes are approved.
However, ensure that the rate limiting is thoroughly tested to confirm that it behaves as expected and that the API is not accessible beyond the specified rate limits.
Run the following script to verify the rate limiting:
Verification successful
Rate limiting is correctly configured for
deleteApi.The
deleteApiprocedure uses therateLimitedProcedurewithDELETE_LIMITset to 5 andDELETE_LIMIT_DURATIONset to 3 seconds. This configuration ensures that the API is rate-limited as intended. The implementation is consistent across the codebase.
DELETE_LIMITandDELETE_LIMIT_DURATIONare defined inapps/dashboard/lib/ratelimitValues.ts.- These values are correctly used in
apps/dashboard/lib/trpc/routers/api/delete.ts.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the rate limiting for the `deleteApi` procedure. # Test: Call the `deleteApi` procedure multiple times and expect it to be rate limited. for i in {1..10}; do curl -X POST http://localhost:3000/api/trpc/deleteApi \ -H "Content-Type: application/json" \ -d '{"apiId": "test-api-id"}' \ -H "Authorization: Bearer test-token" doneLength of output: 5859
Script:
#!/bin/bash # Search for the implementation of `rateLimitedProcedure` to understand rate limiting configuration. ast-grep --lang typescript --pattern 'const rateLimitedProcedure = $_' # Search for the definitions of `DELETE_LIMIT` and `DELETE_LIMIT_DURATION`. rg 'DELETE_LIMIT|DELETE_LIMIT_DURATION' --type=typescriptLength of output: 2017
Script:
#!/bin/bash # Correct the file type and search for the definitions of `DELETE_LIMIT` and `DELETE_LIMIT_DURATION`. rg 'DELETE_LIMIT|DELETE_LIMIT_DURATION' --type=tsLength of output: 3237
apps/dashboard/lib/trpc/routers/ratelimit/createOverride.ts (3)
5-5: LGTM!The code changes are approved.
8-8: LGTM!The code changes are approved.
10-13: LGTM, but verify the rate limiting.The code changes are approved.
However, ensure that the rate limiting is thoroughly tested to confirm that it behaves as expected and that the API is not accessible beyond the specified rate limits.
Run the following script to verify the rate limiting:
Verification successful
Rate limiting logic is correctly implemented.
The
rateLimitedProcedureenforces rate limits as expected by using aRatelimitobject to track requests and throwing aTRPCErrorwhen limits are exceeded. This confirms that the rate limiting logic is properly integrated into thecreateOverrideprocedure.
- Implementation found in
apps/dashboard/lib/trpc/trpc.tslines 25-56.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the rate limiting for the `createOverride` procedure. # Test: Call the `createOverride` procedure multiple times and expect it to be rate limited. for i in {1..10}; do curl -X POST http://localhost:3000/api/trpc/createOverride \ -H "Content-Type: application/json" \ -d '{"namespaceId": "test-namespace-id", "identifier": "test-identifier", "limit": 10, "duration": 60}' \ -H "Authorization: Bearer test-token" doneLength of output: 6649
Script:
#!/bin/bash # Search for the implementation of `rateLimitedProcedure` to understand its rate limiting logic. ast-grep --lang typescript --pattern 'const rateLimitedProcedure = $_'Length of output: 1922
apps/dashboard/lib/trpc/routers/ratelimit/deleteNamespace.ts (3)
5-5: LGTM!The code changes are approved.
7-7: LGTM!The code changes are approved.
9-12: LGTM, but verify the rate limiting.The code changes are approved.
However, ensure that the rate limiting is thoroughly tested to confirm that it behaves as expected and that the API is not accessible beyond the specified rate limits.
Run the following script to verify the rate limiting:
Verification successful
Rate Limiting Verified for
deleteNamespaceProcedureThe rate limiting logic for the
deleteNamespaceprocedure is correctly implemented using therateLimitedProcedure. The procedure allows up to 5 requests every 3 seconds, as defined by the constantsDELETE_LIMITandDELETE_LIMIT_DURATION. The implementation ensures that requests exceeding this limit will result in a "TOO_MANY_REQUESTS" error.
rateLimitedProcedureimplementation:apps/dashboard/lib/trpc/trpc.ts- Constants definition:
apps/dashboard/lib/ratelimitValues.tsScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the rate limiting for the `deleteNamespace` procedure. # Test: Call the `deleteNamespace` procedure multiple times and expect it to be rate limited. for i in {1..10}; do curl -X POST http://localhost:3000/api/trpc/deleteNamespace \ -H "Content-Type: application/json" \ -d '{"namespaceId": "test-namespace-id"}' \ -H "Authorization: Bearer test-token" doneLength of output: 6118
Script:
#!/bin/bash # Search for the implementation of `rateLimitedProcedure` to understand its rate limiting logic. ast-grep --lang typescript --pattern 'const rateLimitedProcedure = $_' # Search for the definitions of `DELETE_LIMIT` and `DELETE_LIMIT_DURATION` to verify their values. rg 'const DELETE_LIMIT' -A 2 rg 'const DELETE_LIMIT_DURATION' -A 2Length of output: 2218
apps/dashboard/lib/trpc/routers/gateway/create.ts (1)
2-2: LGTM!The changes to introduce rate limiting to the
createGatewayprocedure using therateLimitedProcedureare approved.
- The rate limiting is configured using constants imported from a centralized location, which is a good practice for maintainability.
- The change does not introduce any new issues or break existing functionality.
Also applies to: 6-11
apps/dashboard/lib/trpc/routers/webhook/create.ts (1)
3-3: LGTM!The changes to introduce rate limiting to the
createWebhookprocedure using therateLimitedProcedureare approved.
- The rate limiting is configured using constants imported from a centralized location, which is a good practice for maintainability.
- The change does not introduce any new issues or break existing functionality.
Also applies to: 11-16
apps/dashboard/lib/trpc/routers/key/updateRatelimit.ts (1)
2-2: LGTM!The changes to introduce rate limiting to the
updateKeyRatelimitprocedure using therateLimitedProcedureare approved.
- The rate limiting is configured using constants imported from a centralized location, which is a good practice for maintainability.
- The change does not introduce any new issues or break existing functionality.
Also applies to: 6-11
apps/dashboard/lib/trpc/routers/key/create.ts (2)
2-2: LGTM!Externalizing the rate limit values is a good practice for maintainability and reusability.
8-13: LGTM, but verify the rate limit values.Introducing rate limiting to the
createKeyprocedure is a good practice to prevent abuse and protect against DoS attacks.However, ensure that the chosen rate limit values (
CREATE_LIMITandCREATE_LIMIT_DURATION) are appropriate for your use case and won't negatively impact the user experience.Run the following script to verify the rate limit values:
Verification successful
Rate limit values verified and appropriate
The rate limit values for the
createKeyprocedure have been verified.CREATE_LIMITis set to5, andCREATE_LIMIT_DURATIONis set to3. These values are reasonable for preventing abuse while allowing legitimate use. Ensure these values align with your application's specific requirements.
CREATE_LIMITis defined as5.CREATE_LIMIT_DURATIONis defined as3.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the rate limit values for the `createKey` procedure. # Test: Search for the rate limit values. Expect: Values to be defined and reasonable. rg --type typescript -A 5 $'CREATE_LIMIT|CREATE_LIMIT_DURATION'Length of output: 99
Script:
#!/bin/bash # Find all TypeScript files and search for the rate limit constants. fd --extension ts --exec rg 'CREATE_LIMIT|CREATE_LIMIT_DURATION' {}Length of output: 2400
apps/dashboard/lib/trpc/routers/key/createRootKey.ts (2)
3-3: LGTM!Externalizing the rate limit values is a good practice for maintainability and reusability.
10-16: LGTM, but verify the rate limit values.Introducing rate limiting to the
createRootKeyprocedure is a good practice to prevent abuse and protect against DoS attacks.However, ensure that the chosen rate limit values (
CREATE_LIMITandCREATE_LIMIT_DURATION) are appropriate for your use case and won't negatively impact the user experience.Run the following script to verify the rate limit values:
Verification successful
Rate limit values are defined and used consistently.
The
CREATE_LIMITis set to 5 andCREATE_LIMIT_DURATIONis set to 3, as defined inapps/dashboard/lib/ratelimitValues.ts. These values are used consistently across the codebase, including in thecreateRootKeyprocedure. Ensure these values are appropriate for your specific use case and expected traffic patterns. Adjust them if necessary to balance between security and user experience.
apps/dashboard/lib/ratelimitValues.ts: Definitions ofCREATE_LIMITandCREATE_LIMIT_DURATION.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the rate limit values for the `createRootKey` procedure. # Test: Search for the rate limit values. Expect: Values to be defined and reasonable. rg --type typescript -A 5 $'CREATE_LIMIT|CREATE_LIMIT_DURATION'Length of output: 99
Script:
#!/bin/bash # Description: Verify the rate limit values for the `createRootKey` procedure. # Test: Search for the rate limit values without specifying the file type. rg -A 5 'CREATE_LIMIT|CREATE_LIMIT_DURATION'Length of output: 18526
apps/dashboard/lib/trpc/routers/workspace/changePlan.ts (1)
3-3: LGTM!Externalizing the rate limit values is a good practice for maintainability and reusability.
apps/dashboard/lib/trpc/routers/rbac.ts (12)
27-30: Rate limiting looks good!The
addPermissionToRootKeyprocedure now usesrateLimitedProcedurewith appropriate rate limit values to prevent abuse.
88-91: Rate limiting looks good!The
removePermissionFromRootKeyprocedure now usesrateLimitedProcedurewith appropriate rate limit values to prevent abuse.
148-151: Rate limiting looks good!The
connectPermissionToRoleprocedure now usesrateLimitedProcedurewith appropriate rate limit values to prevent abuse.
204-207: Rate limiting looks good!The
disconnectPermissionToRoleprocedure now usesrateLimitedProcedurewith appropriate rate limit values to prevent abuse.
235-235: Rate limiting looks good!The
connectRoleToKeyprocedure now usesrateLimitedProcedurewith appropriate rate limit values to prevent abuse.
288-291: Rate limiting looks good!The
disconnectRoleFromKeyprocedure now usesrateLimitedProcedurewith appropriate rate limit values to prevent abuse.
319-319: Rate limiting looks good!The
createRoleprocedure now usesrateLimitedProcedurewith appropriate rate limit values to prevent abuse.
401-401: Rate limiting looks good!The
updateRoleprocedure now usesrateLimitedProcedurewith appropriate rate limit values to prevent abuse.
434-434: Rate limiting looks good!The
deleteRoleprocedure now usesrateLimitedProcedurewith appropriate rate limit values to prevent abuse.
467-467: Rate limiting looks good!The
createPermissionprocedure now usesrateLimitedProcedurewith appropriate rate limit values to prevent abuse.
516-516: Rate limiting looks good!The
updatePermissionprocedure now usesrateLimitedProcedurewith appropriate rate limit values to prevent abuse.
556-556: Rate limiting looks good!The
deletePermissionprocedure now usesrateLimitedProcedurewith appropriate rate limit values to prevent abuse.
|
Leaving this for @chronark to approve. |
…-add-ratelimiting-to-our-trpc-routes
…//github.com/unkeyed/unkey into eng-1314-add-ratelimiting-to-our-trpc-routes
4 participants
What does this PR do?
Fixes # (issue)
If there is not an issue for this, please create one first. This is used to tracking purposes and also helps use understand why this PR exists
Type of change
How should this be tested?
Checklist
Required
pnpm buildpnpm fmtconsole.logsgit pull origin mainAppreciated
Summary by CodeRabbit
Release Notes
New Features
UNKEY_ROOT_KEYfor enhanced configuration.Bug Fixes
Documentation
Refactor