feat(auth): enforce domain and account bans on sign-in and workflow executions#4948
feat(auth): enforce domain and account bans on sign-in and workflow executions#4948TheodoreSpeaks wants to merge 4 commits into
Conversation
|
@greptile review |
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryHigh Risk Overview A new
Mothership inbox tasks fail without running the agent or sending mail when the sender email or resolved user is blocked/banned. Minor: Reviewed by Cursor Bugbot for commit b41c0fe. Bugbot is set up for automated code reviews on this repo. Configure here. |
Greptile SummaryThis PR adds domain- and account-ban enforcement at every system entry point: sign-in (email/password and OAuth/SSO via a
Confidence Score: 5/5Safe to merge — all execution and sign-in entry points now enforce the domain and account ban consistently, fail-closed behaviour is applied on every error path, and the inbox executor correctly prevents email responses to suspended accounts. Every new gate follows the same fail-closed pattern established for the inbox executor fix; the ban helper is well-isolated from better-auth init, deduplication is handled at both the call site and inside the helper, and the test suite covers normal, expired-ban, domain-subdomain, DB-failure, and sentinel-userId scenarios. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[User Action / Trigger] --> B{Entry Point}
B -->|Email/Password Sign-in| C[beforeAction hook]
B -->|OAuth / SSO Sign-in| D[session.create.before hook]
B -->|Workflow Execution\nmanual / API / webhook / schedule / chat / HITL| E[preprocessExecution Step 3.5]
B -->|Mothership Inbox Email| F[executeInboxTask ban check]
C --> C1{isEmailInDenylist\nrequest body email}
C1 -->|Blocked domain| DENY1[throw APIError FORBIDDEN]
C1 -->|OK| ALLOW1[Continue sign-in]
D --> D1{isEmailInDenylist\nuser.email from DB}
D1 -->|Blocked domain| DENY2[throw APIError FORBIDDEN]
D1 -->|DB error| DENY2
D1 -->|OK| ALLOW2[Session created]
E --> E1[Resolve billing actorUserId\nStep 3]
E1 --> E2[getActivelyBannedUserIds\nactorUserId + userId]
E2 -->|DB ban or blocked domain| DENY3[403 Account suspended]
E2 -->|DB error| DENY4[500 fail-closed]
E2 -->|Clean| ALLOW3[Continue to Step 4]
F --> F1[Promise.all\nisEmailBlocked sender email\ngetActivelyBannedUserIds resolvedUserId]
F1 -->|Sender blocked OR user banned| DENY5[markTaskFailed\nno email sent]
F1 -->|Lookup error| DENY5
F1 -->|Clean| ALLOW4[Run agent]
subgraph ban.ts
BAN1[isBanActive\nbanned flag + banExpires]
BAN2[isEmailBlocked\ndomain list + account ban by email]
BAN3[getActivelyBannedUserIds\nbatch by user ID]
end
E2 -.->|uses| BAN3
F1 -.->|uses| BAN2
F1 -.->|uses| BAN3
BAN3 -.->|uses| BAN1
BAN2 -.->|uses| BAN1
Reviews (3): Last reviewed commit: "fix(mothership): also block inbox sender..." | Re-trigger Greptile |
Greptile SummaryThis PR enforces domain bans and account bans across all sign-in paths and workflow execution entry points. A new
Confidence Score: 3/5The auth and preprocessing layers are solid, but the inbox executor has a defect where a DB failure during the ban check causes the outer catch to send an error email to a sender who may be banned — directly contradicting the stated design intent. The core ban logic in ban.ts, the preprocessing gate, and the auth-layer hooks are well-designed and correctly fail-closed. The issue is in executor.ts: the ban check lives inside the outer try block, so any DB error during that check propagates to the catch handler which then calls sendInboxResponse if no response has been sent yet. The code comment explicitly calls out that no email should ever be sent to a suspended account, but a transient DB outage breaks that guarantee silently. apps/sim/lib/mothership/inbox/executor.ts needs the most attention — the ban-check placement inside the try block allows the catch handler to email potentially-banned senders on DB failure. Important Files Changed
Sequence DiagramsequenceDiagram
participant C as Client / Email Sender
participant Auth as Auth Layer (auth.ts)
participant Pre as preprocessExecution
participant Inbox as InboxExecutor
participant Ban as getActivelyBannedUserIds
participant DB as Database
Note over Auth: Email/password sign-in
C->>Auth: POST /sign-in/email
Auth->>Auth: isEmailInDenylist(requestEmail)
alt domain blocked
Auth-->>C: 403 FORBIDDEN
end
Note over Auth: OAuth/SSO sign-in
C->>Auth: OAuth callback
Auth->>Auth: session.create.before hook
Auth->>DB: "SELECT email WHERE userId = session.userId"
Auth->>Auth: isEmailInDenylist(email, blockedDomains)
alt domain blocked
Auth-->>C: 403 FORBIDDEN
end
Note over Pre: Any workflow execution
C->>Pre: preprocessExecution(options)
Pre->>Pre: Step 3 resolve actorUserId
Pre->>Ban: getActivelyBannedUserIds([actorUserId, userId])
Ban->>DB: SELECT id,email,banned,banExpires
alt banned
Pre-->>C: success false statusCode 403
else DB error
Pre-->>C: success false statusCode 500
end
Note over Inbox: Inbox task execution
C->>Inbox: executeInboxTask(taskId)
Inbox->>Inbox: claim task + resolveUserId
Inbox->>Ban: getActivelyBannedUserIds([userId])
alt banned
Inbox->>DB: markTaskFailed no email sent
else DB error catch block
Inbox->>C: error email sent unintended
end
Reviews (2): Last reviewed commit: "feat(auth): enforce domain and account b..." | Re-trigger Greptile |
|
@greptile review |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 590653e. Configure here.

Summary
blockedSignupDomainslist now block existing accounts too, not just signups: sign-in is rejected across all providers (email/password, OAuth, SSO via asession.create.beforegate) and every workflow execution is blockedlib/auth/ban.tshelper resolves effective ban status (active DB ban honoringbanExpires, or blocked email domain) in one querypreprocessExecutiongains a Step 3.5 ban gate covering all execution entry points (manual, API, webhooks, schedules, deployed chats, resume/HITL, table columns, async jobs); checks both the billing actor and the caller-provided user, fails closed on lookup errorsisEmailInDenylisttoaccess-control.tsso the ban helper doesn't pull the better-auth init into background workersuse-inline-rename.tsthat was failinglint:checkNote: every domain already in the prod blocked-domains list starts hard-blocking its existing users on deploy. Already-signed-in users keep UI access for up to ~24h (session cookie cache) but executions/API keys/billing are cut immediately.
Type of Change
Testing
New unit tests for the ban helper and the preprocessing gate (403, fail-closed 500, id dedup); 167 tests passing across touched dirs.
bun run lint:check,tsc --noEmit, andcheck:api-validation:strictall green.Checklist