Skip to content

Security - add throttle in message resend#16070

Merged
etiennejouan merged 6 commits intomainfrom
ej/message-bombing
Nov 28, 2025
Merged

Security - add throttle in message resend#16070
etiennejouan merged 6 commits intomainfrom
ej/message-bombing

Conversation

@etiennejouan
Copy link
Copy Markdown
Contributor

@etiennejouan etiennejouan changed the title Securityt - add throttle in message resend Security - add throttle in message resend Nov 25, 2025
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Nov 25, 2025

Greptile Overview

Greptile Summary

Added rate limiting to workspace invitation resending to prevent abuse, but critically missed adding the same throttling to the main sendInvitations method, leaving a significant security vulnerability.

  • Added throttle configuration variables (WORKSPACE_INVITATION_THROTTLE_LIMIT: 100 invites per hour)
  • Implemented throttleInvitationSending() private method using token bucket algorithm
  • Applied throttling only to resendWorkspaceInvitation()
  • Security gap: sendInvitations() remains unprotected and can be exploited to spam invitations
  • Cleaned up unused FileService injection from resolver
  • Left debug comment //here at line 70 in resolver

Confidence Score: 1/5

  • This PR has a critical security flaw that defeats its main purpose
  • The PR aims to add rate limiting to prevent invitation spam, but only protects resendWorkspaceInvitation while leaving the main sendInvitations endpoint completely unprotected. Attackers can bypass the rate limiting entirely by using sendInvitations instead of resendWorkspaceInvitation, making this security fix ineffective
  • workspace-invitation.service.ts requires immediate attention - the sendInvitations method at line 255 must have throttling added

Important Files Changed

File Analysis

Filename Score Overview
packages/twenty-server/src/engine/core-modules/workspace-invitation/workspace-invitation.resolver.ts 3/5 Removed unused FileService injection, but left debug comment
packages/twenty-server/src/engine/core-modules/workspace-invitation/services/workspace-invitation.service.ts 1/5 Added throttling to resendWorkspaceInvitation but critically missed sendInvitations, leaving a security gap

Sequence Diagram

sequenceDiagram
    participant Client
    participant Resolver as WorkspaceInvitationResolver
    participant Service as WorkspaceInvitationService
    participant Throttler as ThrottlerService
    participant Cache as CacheStorage
    participant DB as AppTokenRepository
    participant Email as EmailService

    Note over Client,Email: resendWorkspaceInvitation Flow (WITH throttling)
    Client->>Resolver: resendWorkspaceInvitation(appTokenId)
    Resolver->>Service: resendWorkspaceInvitation(appTokenId, workspace, sender)
    Service->>Throttler: throttleInvitationSending(workspaceId)
    Throttler->>Cache: get token bucket state
    Cache-->>Throttler: {tokens, lastRefillAt}
    alt Rate limit exceeded
        Throttler-->>Service: throw ThrottlerException
        Service-->>Resolver: throw WorkspaceInvitationException
        Resolver-->>Client: Error: Too many invitations sent
    else Rate limit ok
        Throttler->>Cache: update token bucket
        Service->>DB: findOne(appTokenId)
        DB-->>Service: appToken
        Service->>DB: delete(appToken.id)
        Service->>Service: sendInvitations([email], workspace, sender)
        Service->>Email: send invitation email
        Email-->>Service: email sent
        Service-->>Resolver: SendInvitationsOutput
        Resolver-->>Client: success
    end

    Note over Client,Email: sendInvitations Flow (MISSING throttling)
    Client->>Resolver: sendInvitations(emails[])
    Resolver->>Service: sendInvitations(emails, workspace, sender)
    Note right of Service: ⚠️ No throttle check here!
    Service->>DB: createWorkspaceInvitation(email)
    Service->>Email: send invitation emails
    Service-->>Resolver: SendInvitationsOutput
    Resolver-->>Client: success
Loading

Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

Additional Comments (1)

  1. packages/twenty-server/src/engine/core-modules/workspace-invitation/services/workspace-invitation.service.ts, line 255-260 (link)

    logic: throttling is missing here. the sendInvitations method is the main entry point for sending invitations but doesn't call throttleInvitationSending, while resendWorkspaceInvitation does. this creates a security gap where attackers can spam invitations through sendInvitations while only resendWorkspaceInvitation is rate-limited

    add throttling at the start of this method:

4 files reviewed, 2 comments

Edit Code Review Agent Settings | Greptile

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Nov 25, 2025

🚀 Preview Environment Ready!

Your preview environment is available at: http://bore.pub:6314

This environment will automatically shut down when the PR is closed or after 5 hours.

Copy link
Copy Markdown
Contributor

@ijreilly ijreilly left a comment

Choose a reason for hiding this comment

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

i think i approved to quickly, following greptile comment, why do we add the throttle to resendWorkspaceInvitation and not sendWorkspaceInvitation?

@etiennejouan
Copy link
Copy Markdown
Contributor Author

etiennejouan commented Nov 26, 2025

@ijreilly At first, i protect the sendInvitation method but you can't invite twice the same user.

@etiennejouan
Copy link
Copy Markdown
Contributor Author

At first I protected sending invitations (not just resending) but while testing I figured that you couldn't "sendInvitations" to the same user.
But, thinking about it, you can do a "sendInvitations" then "deleteWorkspaceInvitation" then "sendInvitations" again and again. And then spam a user, so I'm going to put the throttle back in the sendInvitations method.

@etiennejouan etiennejouan enabled auto-merge (squash) November 28, 2025 17:23
@etiennejouan etiennejouan merged commit 63afed6 into main Nov 28, 2025
53 checks passed
@etiennejouan etiennejouan deleted the ej/message-bombing branch November 28, 2025 17:33
@twenty-eng-sync
Copy link
Copy Markdown

Hey @etiennejouan! After you've done the QA of your Pull Request, you can mark it as done here. Thank you!

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants