Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: added Ratelimit to dashboard #2076

Merged
merged 18 commits into from
Sep 19, 2024

Conversation

MichaelUnkey
Copy link
Collaborator

@MichaelUnkey MichaelUnkey commented Sep 10, 2024

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

  • Bug fix (non-breaking change which fixes an issue)
  • Chore (refactoring code, technical debt, workflow improvements)
  • Enhancement (small improvements)
  • 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 should this be tested?

  • Test A
  • Test B

Checklist

Required

  • Filled out the "How to test" section in this PR
  • Read Contributing Guide
  • Self-reviewed my own code
  • Commented on my code in hard-to-understand areas
  • Ran pnpm build
  • Ran pnpm fmt
  • Checked for warnings, there are none
  • Removed all console.logs
  • Merged the latest changes from main onto my branch with git pull origin main
  • My changes don't cause any responsiveness issues

Appreciated

  • If a UI change was made: Added a screen recording or screenshots to this PR
  • Updated the Unkey Docs if changes were necessary

Summary by CodeRabbit

Release Notes

  • New Features

    • Introduced a new environment variable UNKEY_ROOT_KEY for enhanced configuration.
    • Added centralized rate limiting for create, update, and delete operations across multiple API procedures.
  • Bug Fixes

    • Improved robustness by replacing authentication middleware with rate limiting in various procedures.
  • Documentation

    • Updated documentation to reflect changes in rate limiting and environment variable requirements.
  • Refactor

    • Streamlined API procedures by integrating rate limiting, enhancing control over request frequency.

Copy link

linear bot commented Sep 10, 2024

Copy link

vercel bot commented Sep 10, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
dashboard ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 19, 2024 7:26am
play ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 19, 2024 7:26am
www ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 19, 2024 7:26am
2 Skipped Deployments
Name Status Preview Comments Updated (UTC)
planetfall ⬜️ Ignored (Inspect) Visit Preview Sep 19, 2024 7:26am
workflows ⬜️ Ignored (Inspect) Visit Preview Sep 19, 2024 7:26am

Copy link

changeset-bot bot commented Sep 10, 2024

⚠️ No Changeset found

Latest commit: c6e26bf

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link
Contributor

coderabbitai bot commented Sep 10, 2024

Caution

Review failed

The pull request is closed.

Walkthrough

Walkthrough

The changes involve the introduction of a new optional environment variable, UNKEY_ROOT_KEY, to enhance application configuration. Additionally, a new file implements rate limiting for TRPC procedures, replacing the existing auth middleware with a rateLimitedProcedure across various routers. This modification enforces specific request limits for operations related to creating, updating, and deleting resources, thereby improving control over user request rates.

Changes

File(s) Change Summary
apps/dashboard/lib/env.ts Added UNKEY_ROOT_KEY as an optional environment variable.
apps/dashboard/lib/trpc/ratelimitProcedure.ts Introduced rate limiting functionality with specific limits for create, update, and delete operations.
apps/dashboard/lib/trpc/routers/plain.ts, apps/dashboard/lib/trpc/routers/rbac.ts Replaced auth middleware with rateLimitedProcedure for various procedures, enforcing rate limits on requests.

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
Loading
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
Loading

Possibly related PRs

  • feat: added Ratelimit to dashboard #2076: This PR is directly related as it introduces the UNKEY_ROOT_KEY environment variable, which is utilized in the rate limiting functionality implemented in the same PR. The changes in both PRs involve modifications to the env.ts file and the introduction of rate limiting features that depend on this environment variable.

Suggested reviewers

  • mcstepp
  • chronark
  • perkinsjr

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?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    -- I pushed a fix in commit <commit_id>, please review it.
    -- Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    -- @coderabbitai generate unit testing code for this file.
    -- @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    -- @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    -- @coderabbitai read src/utils.ts and generate unit testing code.
    -- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    -- @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

github-actions bot commented Sep 10, 2024

Thank you for following the naming conventions for pull request titles! 🙏

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Commits

Files that changed from the base of the PR and between 7820c2a and d07a80a.

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 protectedProcedure declaration is approved.


25-56: LGTM!

The rateLimitedProcedure declaration 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 updateKeyEnabled procedure are approved. Using the rateLimitedProcedure instead of the auth middleware 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 deleteLlmGateway procedure are approved. Using the rateLimitedProcedure instead of the auth middleware 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 rateLimitedProcedure is a critical enhancement that improves the robustness of the application.

Please ensure that the UPDATE_LIMIT and UPDATE_LIMIT_DURATION values 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 updateKeyName procedure have been verified:

  • UPDATE_LIMIT is set to 25.
  • UPDATE_LIMIT_DURATION is set to 5.

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 of auth middleware.

The addition of rate limiting functionality using rateLimitedProcedure is a good enhancement.

However, the removal of the auth middleware 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 auth middleware in other procedures:

If the removal of auth is 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 rateLimitedProcedure is used to replace the auth middleware.


9-12: LGTM!

The changes are correct and enhance the control over the procedure's usage by replacing the auth middleware with rateLimitedProcedure. The rate limiting parameters are correctly passed to rateLimitedProcedure.

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 rateLimitedProcedure is used to replace the auth middleware.


8-11: LGTM!

The changes are correct and enhance the control over the procedure's usage by replacing the auth middleware with rateLimitedProcedure. The rate limiting parameters are correctly passed to rateLimitedProcedure.

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 rateLimitedProcedure is used to replace the auth middleware.


8-11: LGTM!

The changes are correct and enhance the control over the procedure's usage by replacing the auth middleware with rateLimitedProcedure. The rate limiting parameters are correctly passed to rateLimitedProcedure.

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 auth middleware with rateLimitedProcedure is a good enhancement. It helps prevent abuse by enforcing rate limits on the procedure using the configured limit and duration options.

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 auth middleware with rateLimitedProcedure is a good enhancement. It helps prevent abuse by enforcing rate limits on the procedure using the configured limit and duration options.

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 auth middleware with rateLimitedProcedure is a good enhancement. It helps prevent abuse by enforcing rate limits on the procedure using the configured limit and duration options.

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:

  1. Create namespaces in quick succession and ensure that the rate limiting kicks in as expected without negatively impacting the user experience.
  2. Verify that the rate limiting parameters (CREATE_LIMIT and CREATE_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:

  1. 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.
  2. Verify that the rate limiting parameters (UPDATE_LIMIT and UPDATE_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:

  1. 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.
  2. Verify that the rate limiting parameters (CREATE_LIMIT and CREATE_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 disconnectPermissionFromRole procedure 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 toggleWebhook procedure 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 deleteKeys procedure 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 createPermission procedure is now using the rateLimitedProcedure with the imported rate limit constants.

Please ensure that the CREATE_LIMIT and CREATE_LIMIT_DURATION values 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 deletePermission procedure is now using the rateLimitedProcedure with the imported rate limit constants.

Please ensure that the DELETE_LIMIT and DELETE_LIMIT_DURATION values 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 deleteOverride procedure is now using the rateLimitedProcedure with the imported rate limit constants.

Please ensure that the DELETE_LIMIT and DELETE_LIMIT_DURATION values 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 connectPermissionToRole procedure. This is a good practice to prevent abuse and manage resource usage.

Please ensure that the UPDATE_LIMIT and UPDATE_LIMIT_DURATION values 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 createLlmGateway procedure. This is a good practice to prevent abuse and manage resource usage.

Please ensure that the CREATE_LIMIT and CREATE_LIMIT_DURATION values 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 deleteRootKeys procedure. This is a good practice to prevent abuse and manage resource usage.

Please ensure that the DELETE_LIMIT and DELETE_LIMIT_DURATION values 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 updateKeyMetadata procedure are approved.

However, please ensure that the chosen rate limiting parameters (UPDATE_LIMIT and UPDATE_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 updateRole procedure are approved.

However, please ensure that the chosen rate limiting parameters (UPDATE_LIMIT and UPDATE_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 createApi procedure are approved.

However, please ensure that the chosen rate limiting parameters (CREATE_LIMIT and CREATE_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 to rateLimitedProcedure, but verify the rate limit values.

The change from auth to rateLimitedProcedure is approved as it introduces necessary rate limiting to the createSecret procedure.

However, please ensure that the values of CREATE_LIMIT and CREATE_LIMIT_DURATION are 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 to rateLimitedProcedure, but verify the rate limit values.

The change from auth to rateLimitedProcedure is approved as it introduces necessary rate limiting to the updateApiIpWhitelist procedure.

However, please ensure that the values of UPDATE_LIMIT and UPDATE_LIMIT_DURATION are 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 to rateLimitedProcedure, but verify the rate limit values.

The change from auth to rateLimitedProcedure is approved as it introduces necessary rate limiting to the updatePermission procedure.

However, please ensure that the values of UPDATE_LIMIT and UPDATE_LIMIT_DURATION are 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 createWorkspace procedure 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 updateNamespaceName procedure 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 updateOverride procedure 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 updateOverride procedure is correctly implemented with UPDATE_LIMIT set to 25 and UPDATE_LIMIT_DURATION set to 5. These values are appropriate for controlling the request rate. No further issues were found.

  • UPDATE_LIMIT and UPDATE_LIMIT_DURATION are defined in apps/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 5

Length 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 2

Length 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 updateKeyExpiration procedure, which is a good practice to prevent abuse. The auth middleware has been replaced with rateLimitedProcedure, 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 connectRoleToKey procedure, which is a good practice to prevent abuse. The auth middleware has been replaced with rateLimitedProcedure, 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 updateKeyRemaining procedure, which is a good practice to prevent abuse. The auth middleware has been replaced with rateLimitedProcedure, 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 auth middleware has been replaced with rateLimitedProcedure to introduce rate limiting to the createVerificationMonitor procedure. 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 auth middleware has been replaced with rateLimitedProcedure to introduce rate limiting to the updateSecret procedure. 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 auth middleware has been replaced with rateLimitedProcedure to introduce rate limiting to the removePermissionFromRootKey procedure. 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 auth middleware has been correctly replaced with rateLimitedProcedure to introduce rate limiting functionality. The rate limiting parameters are correctly configured using the imported constants.

However, ensure that the UPDATE_LIMIT and UPDATE_LIMIT_DURATION values 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 auth middleware has been correctly replaced with rateLimitedProcedure to introduce rate limiting functionality. The rate limiting parameters are correctly configured using the imported constants.

However, ensure that the CREATE_LIMIT and CREATE_LIMIT_DURATION values 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 auth middleware has been correctly replaced with rateLimitedProcedure to introduce rate limiting functionality. The rate limiting parameters are correctly configured using the imported constants.

However, ensure that the CREATE_LIMIT and CREATE_LIMIT_DURATION values 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 deleteApi procedure uses the rateLimitedProcedure with DELETE_LIMIT set to 5 and DELETE_LIMIT_DURATION set to 3 seconds. This configuration ensures that the API is rate-limited as intended. The implementation is consistent across the codebase.

  • DELETE_LIMIT and DELETE_LIMIT_DURATION are defined in apps/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"
done

Length 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=typescript

Length 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=ts

Length 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 rateLimitedProcedure enforces rate limits as expected by using a Ratelimit object to track requests and throwing a TRPCError when limits are exceeded. This confirms that the rate limiting logic is properly integrated into the createOverride procedure.

  • Implementation found in apps/dashboard/lib/trpc/trpc.ts lines 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"
done

Length 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 deleteNamespace Procedure

The rate limiting logic for the deleteNamespace procedure is correctly implemented using the rateLimitedProcedure. The procedure allows up to 5 requests every 3 seconds, as defined by the constants DELETE_LIMIT and DELETE_LIMIT_DURATION. The implementation ensures that requests exceeding this limit will result in a "TOO_MANY_REQUESTS" error.

  • rateLimitedProcedure implementation: apps/dashboard/lib/trpc/trpc.ts
  • Constants definition: apps/dashboard/lib/ratelimitValues.ts
Scripts 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"
done

Length 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 2

Length of output: 2218

apps/dashboard/lib/trpc/routers/gateway/create.ts (1)

2-2: LGTM!

The changes to introduce rate limiting to the createGateway procedure using the rateLimitedProcedure are 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 createWebhook procedure using the rateLimitedProcedure are 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 updateKeyRatelimit procedure using the rateLimitedProcedure are 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 createKey procedure is a good practice to prevent abuse and protect against DoS attacks.

However, ensure that the chosen rate limit values (CREATE_LIMIT and CREATE_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 createKey procedure have been verified. CREATE_LIMIT is set to 5, and CREATE_LIMIT_DURATION is set to 3. These values are reasonable for preventing abuse while allowing legitimate use. Ensure these values align with your application's specific requirements.

  • CREATE_LIMIT is defined as 5.
  • CREATE_LIMIT_DURATION is defined as 3.
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 createRootKey procedure is a good practice to prevent abuse and protect against DoS attacks.

However, ensure that the chosen rate limit values (CREATE_LIMIT and CREATE_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_LIMIT is set to 5 and CREATE_LIMIT_DURATION is set to 3, as defined in apps/dashboard/lib/ratelimitValues.ts. These values are used consistently across the codebase, including in the createRootKey procedure. 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 of CREATE_LIMIT and CREATE_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 addPermissionToRootKey procedure now uses rateLimitedProcedure with appropriate rate limit values to prevent abuse.


88-91: Rate limiting looks good!

The removePermissionFromRootKey procedure now uses rateLimitedProcedure with appropriate rate limit values to prevent abuse.


148-151: Rate limiting looks good!

The connectPermissionToRole procedure now uses rateLimitedProcedure with appropriate rate limit values to prevent abuse.


204-207: Rate limiting looks good!

The disconnectPermissionToRole procedure now uses rateLimitedProcedure with appropriate rate limit values to prevent abuse.


235-235: Rate limiting looks good!

The connectRoleToKey procedure now uses rateLimitedProcedure with appropriate rate limit values to prevent abuse.


288-291: Rate limiting looks good!

The disconnectRoleFromKey procedure now uses rateLimitedProcedure with appropriate rate limit values to prevent abuse.


319-319: Rate limiting looks good!

The createRole procedure now uses rateLimitedProcedure with appropriate rate limit values to prevent abuse.


401-401: Rate limiting looks good!

The updateRole procedure now uses rateLimitedProcedure with appropriate rate limit values to prevent abuse.


434-434: Rate limiting looks good!

The deleteRole procedure now uses rateLimitedProcedure with appropriate rate limit values to prevent abuse.


467-467: Rate limiting looks good!

The createPermission procedure now uses rateLimitedProcedure with appropriate rate limit values to prevent abuse.


516-516: Rate limiting looks good!

The updatePermission procedure now uses rateLimitedProcedure with appropriate rate limit values to prevent abuse.


556-556: Rate limiting looks good!

The deletePermission procedure now uses rateLimitedProcedure with appropriate rate limit values to prevent abuse.

apps/dashboard/lib/trpc/routers/workspace/changePlan.ts Outdated Show resolved Hide resolved
@perkinsjr
Copy link
Collaborator

Leaving this for @chronark to approve.

apps/dashboard/lib/env.ts Outdated Show resolved Hide resolved
apps/dashboard/lib/ratelimitValues.ts Outdated Show resolved Hide resolved
apps/dashboard/lib/trpc/trpc.ts Outdated Show resolved Hide resolved
apps/dashboard/lib/trpc/trpc.ts Outdated Show resolved Hide resolved
apps/dashboard/lib/trpc/trpc.ts Show resolved Hide resolved
apps/dashboard/lib/ratelimitValues.ts Outdated Show resolved Hide resolved
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.

4 participants