Skip to content

feat(auth): enforce domain and account bans on sign-in and workflow executions#4948

Open
TheodoreSpeaks wants to merge 4 commits into
stagingfrom
feat/ban-blocks-account
Open

feat(auth): enforce domain and account bans on sign-in and workflow executions#4948
TheodoreSpeaks wants to merge 4 commits into
stagingfrom
feat/ban-blocks-account

Conversation

@TheodoreSpeaks

Copy link
Copy Markdown
Collaborator

Summary

  • domains in the appconfig blockedSignupDomains list now block existing accounts too, not just signups: sign-in is rejected across all providers (email/password, OAuth, SSO via a session.create.before gate) and every workflow execution is blocked
  • new lib/auth/ban.ts helper resolves effective ban status (active DB ban honoring banExpires, or blocked email domain) in one query
  • preprocessExecution gains 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 errors
  • Mothership inbox tasks for banned/blocked users are marked failed without running (and without emailing the suspended account)
  • moved isEmailInDenylist to access-control.ts so the ban helper doesn't pull the better-auth init into background workers
  • drive-by: fixed a pre-existing biome error in use-inline-rename.ts that was failing lint:check

Note: 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

  • New feature

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, and check:api-validation:strict all green.

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@TheodoreSpeaks

Copy link
Copy Markdown
Collaborator Author

@greptile review

@vercel

vercel Bot commented Jun 10, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped Jun 10, 2026 8:39pm

Request Review

@cursor

cursor Bot commented Jun 10, 2026

Copy link
Copy Markdown

PR Summary

High Risk
Changes authentication session creation and broad execution gating; deploy immediately blocks existing users on prod blocked-domain lists (UI may linger until session cache expires).

Overview
Blocked signup domains now apply to existing accounts, not only new signups. Email/password sign-in is rejected with an admin-contact message; OAuth/SSO is covered by a session.create.before hook that loads the user email and forbids new sessions for denylisted domains.

A new lib/auth/ban module centralizes enforcement: active DB bans (respecting banExpires), plus blocked domains via isEmailInDenylist moved to access-control so workers avoid pulling in the full better-auth stack.

preprocessExecution adds a Step 3.5 gate (before subscription/usage) that batches getActivelyBannedUserIds for the billing actor, caller userId (skipping unknown), and workflow owner—403 on hit, 500 fail-closed on lookup errors.

Mothership inbox tasks fail without running the agent or sending mail when the sender email or resolved user is blocked/banned.

Minor: use-inline-rename onSave typing tweak for lint.

Reviewed by Cursor Bugbot for commit b41c0fe. Bugbot is set up for automated code reviews on this repo. Configure here.

@greptile-apps

greptile-apps Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds domain- and account-ban enforcement at every system entry point: sign-in (email/password and OAuth/SSO via a session.create.before gate), all workflow execution paths (preprocessExecution Step 3.5), and Mothership inbox tasks. A new lib/auth/ban.ts module centralises the ban-resolution logic, and isEmailInDenylist is moved from auth.ts to access-control.ts so it can be imported by background workers without pulling in the better-auth init.

  • Session blocking is layered: the existing beforeAction hook catches email/password sign-in using the request body; the new session.create.before hook catches OAuth/SSO sign-in by looking up the user's email after authentication. Both throw APIError('FORBIDDEN') deliberately outside inner try-catches so errors are not swallowed.
  • Execution blocking (preprocessExecution Step 3.5) checks both the billing actor and the caller-provided userId in a single getActivelyBannedUserIds call, with deduplication and the 'unknown' sentinel excluded. Fail-closed on DB errors (500 with retryable hint).
  • Inbox executor adds parallel isEmailBlocked + getActivelyBannedUserIds checks with a self-contained try-catch that sets responseSent = true before markTaskFailed, ensuring no email is ever sent to a suspended account on any path (including lookup failures).

Confidence Score: 5/5

Safe 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

Filename Overview
apps/sim/lib/auth/ban.ts New module exposing isBanActive, isEmailBlocked, and getActivelyBannedUserIds; correct fail-closed contract, proper deduplication, and clean separation from better-auth init.
apps/sim/lib/auth/auth.ts Adds session.create.before domain-block gate (outside inner try so errors propagate) and extends beforeAction to block sign-in (not just sign-up) for blocked-domain emails.
apps/sim/lib/execution/preprocessing.ts Inserts Step 3.5 ban gate between billing-actor resolution and subscription lookup; correctly passes deduped [actorUserId, userId] and handles both 403 and fail-closed 500 paths.
apps/sim/lib/mothership/inbox/executor.ts Adds parallel isEmailBlocked + getActivelyBannedUserIds check in its own try-catch; sets responseSent before markTaskFailed on all blocked/error paths, eliminating the email-to-suspended-account contract violation.
apps/sim/lib/auth/access-control.ts isEmailInDenylist moved here from auth.ts; function is unchanged, enabling import by ban.ts without pulling in better-auth init.
apps/sim/lib/auth/ban.test.ts New test file; covers isBanActive semantics (permanent, temporary, expired), isEmailBlocked (domain list, account ban, null inputs), and getActivelyBannedUserIds (dedup, empty, domain block, DB failure propagation).
apps/sim/lib/execution/preprocessing.test.ts Added ban gate tests: 403 on banned actor (skips billing/usage), single-call dedup, 'unknown' sentinel exclusion, and fail-closed 500 on DB error.
apps/sim/lib/auth/access-control.test.ts isEmailInDenylist tests moved from auth.test.ts; exact domain, subdomain, look-alike, and multi-entry cases all covered.
apps/sim/lib/auth/auth.test.ts Deleted — tests migrated to access-control.test.ts following the function move.
apps/sim/hooks/use-inline-rename.ts Drive-by biome fix: onSave return type simplified from void

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
Loading

Reviews (3): Last reviewed commit: "fix(mothership): also block inbox sender..." | Re-trigger Greptile

Comment thread apps/sim/lib/mothership/inbox/executor.ts Outdated
@greptile-apps

greptile-apps Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR enforces domain bans and account bans across all sign-in paths and workflow execution entry points. A new ban.ts helper centralises ban resolution (active DB ban + blocked email domain) in a single cacheable query, and a session.create.before hook extends domain blocking from signup-only to all providers including OAuth/SSO.

  • Auth layer: session.create.before now blocks domain-banned accounts from creating sessions (OAuth/SSO path); the existing before hook is extended to reject sign-in (not just sign-up) for email/password with a blocked domain.
  • Execution layer: preprocessExecution gains a Step 3.5 ban gate that checks both the billing actor and the caller-provided userId before any billing or rate-limit work, failing closed with 500 on DB errors.
  • Inbox executor: Inbox tasks for banned users are marked failed without running, and no response email is sent — with one edge case noted below.

Confidence Score: 3/5

The 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

Filename Overview
apps/sim/lib/auth/ban.ts New helper: isBanActive (expiry-aware) + getActivelyBannedUserIds (single DB query, fails closed). Logic is correct and well-tested.
apps/sim/lib/auth/auth.ts Adds domain-block check to session.create.before (OAuth/SSO path) and extends the before-hook to block sign-in (not just sign-up) for email/password. The deliberate placement outside the inner try is correct and well-commented.
apps/sim/lib/execution/preprocessing.ts Adds Step 3.5 ban gate after billing actor is resolved; checks both billing actor and caller-provided userId, deduplicates, skips 'unknown' sentinel, and fails closed with 500 on DB error. Coverage and fail-closed semantics look correct.
apps/sim/lib/mothership/inbox/executor.ts Ban check added after task is claimed. Two issues: (1) ban-check DB failure causes the outer catch to send an error email to the sender, violating 'never mail a suspended account'; (2) non-member external senders with banned accounts slip through because resolveUserId falls back to ws.ownerId.
apps/sim/lib/auth/access-control.ts isEmailInDenylist moved here from auth.ts to decouple background workers from better-auth init. Logic unchanged and well-tested.

Sequence Diagram

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

Reviews (2): Last reviewed commit: "feat(auth): enforce domain and account b..." | Re-trigger Greptile

Comment thread apps/sim/lib/mothership/inbox/executor.ts Outdated
Comment thread apps/sim/lib/mothership/inbox/executor.ts Outdated
@TheodoreSpeaks

Copy link
Copy Markdown
Collaborator Author

@greptile review

Comment thread apps/sim/lib/mothership/inbox/executor.ts

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ 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.

Comment thread apps/sim/lib/execution/preprocessing.ts
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.

1 participant