Skip to content

Commit 590653e

Browse files
fix(mothership): also block inbox senders whose own account is banned
1 parent 3188ee9 commit 590653e

3 files changed

Lines changed: 35 additions & 20 deletions

File tree

apps/sim/lib/auth/ban.test.ts

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ vi.mock('@sim/db', () => ({
1212
db: { select: vi.fn(() => ({ from: vi.fn(() => ({ where: mockWhere })) })) },
1313
user: { id: 'id', email: 'email', banned: 'banned', banExpires: 'banExpires' },
1414
}))
15-
vi.mock('drizzle-orm', () => ({ inArray: vi.fn() }))
15+
vi.mock('drizzle-orm', () => ({ inArray: vi.fn(), sql: vi.fn() }))
1616
vi.mock('@/lib/core/config/appconfig', () => ({ fetchAppConfigProfile: vi.fn() }))
1717
vi.mock('@/lib/core/config/env', () => ({
1818
get env() {
@@ -21,7 +21,7 @@ vi.mock('@/lib/core/config/env', () => ({
2121
}))
2222
vi.mock('@/lib/core/config/feature-flags', () => ({ isAppConfigEnabled: false }))
2323

24-
import { getActivelyBannedUserIds, isBanActive, isEmailDomainBlocked } from '@/lib/auth/ban'
24+
import { getActivelyBannedUserIds, isBanActive, isEmailBlocked } from '@/lib/auth/ban'
2525

2626
describe('isBanActive', () => {
2727
it('returns true for a permanent ban', () => {
@@ -42,20 +42,28 @@ describe('isBanActive', () => {
4242
})
4343
})
4444

45-
describe('isEmailDomainBlocked', () => {
45+
describe('isEmailBlocked', () => {
4646
beforeEach(() => {
47+
vi.clearAllMocks()
4748
envRef.BLOCKED_SIGNUP_DOMAINS = 'bad.com'
49+
mockWhere.mockResolvedValue([])
50+
})
51+
52+
it('returns true for blocked domains and subdomains without querying users', async () => {
53+
expect(await isEmailBlocked('a@bad.com')).toBe(true)
54+
expect(await isEmailBlocked('a@mail.bad.com')).toBe(true)
55+
expect(mockWhere).not.toHaveBeenCalled()
4856
})
4957

50-
it('returns true for blocked domains and subdomains', async () => {
51-
expect(await isEmailDomainBlocked('a@bad.com')).toBe(true)
52-
expect(await isEmailDomainBlocked('a@mail.bad.com')).toBe(true)
58+
it('returns true when the email belongs to an actively banned account', async () => {
59+
mockWhere.mockResolvedValue([{ banned: true, banExpires: null }])
60+
expect(await isEmailBlocked('a@good.com')).toBe(true)
5361
})
5462

55-
it('returns false for clean domains and missing emails', async () => {
56-
expect(await isEmailDomainBlocked('a@good.com')).toBe(false)
57-
expect(await isEmailDomainBlocked(null)).toBe(false)
58-
expect(await isEmailDomainBlocked(undefined)).toBe(false)
63+
it('returns false for clean accounts and missing emails', async () => {
64+
expect(await isEmailBlocked('a@good.com')).toBe(false)
65+
expect(await isEmailBlocked(null)).toBe(false)
66+
expect(await isEmailBlocked(undefined)).toBe(false)
5967
})
6068
})
6169

apps/sim/lib/auth/ban.ts

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { db, user } from '@sim/db'
2-
import { inArray } from 'drizzle-orm'
2+
import { inArray, sql } from 'drizzle-orm'
33
import { getAccessControlConfig, isEmailInDenylist } from '@/lib/auth/access-control'
44

55
/**
@@ -13,13 +13,19 @@ export function isBanActive(row: { banned: boolean | null; banExpires: Date | nu
1313
}
1414

1515
/**
16-
* True when the email's domain is in the appconfig blocked-domains list.
17-
* For gating raw emails (e.g. inbound senders) that have no user row.
16+
* True when a raw email (e.g. an inbound sender) is blocked: its domain is in
17+
* the appconfig blocked-domains list, or it belongs to an account with an
18+
* active ban. Covers senders that don't resolve to a known user id.
1819
*/
19-
export async function isEmailDomainBlocked(email: string | null | undefined): Promise<boolean> {
20+
export async function isEmailBlocked(email: string | null | undefined): Promise<boolean> {
2021
if (!email) return false
2122
const accessControl = await getAccessControlConfig()
22-
return isEmailInDenylist(email, accessControl.blockedSignupDomains)
23+
if (isEmailInDenylist(email, accessControl.blockedSignupDomains)) return true
24+
const rows = await db
25+
.select({ banned: user.banned, banExpires: user.banExpires })
26+
.from(user)
27+
.where(sql`lower(${user.email}) = ${email.toLowerCase()}`)
28+
return rows.some(isBanActive)
2329
}
2430

2531
/**

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

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { createLogger } from '@sim/logger'
33
import { getErrorMessage } from '@sim/utils/errors'
44
import { generateId } from '@sim/utils/id'
55
import { and, eq, sql } from 'drizzle-orm'
6-
import { getActivelyBannedUserIds, isEmailDomainBlocked } from '@/lib/auth/ban'
6+
import { getActivelyBannedUserIds, isEmailBlocked } from '@/lib/auth/ban'
77
import { resolveOrCreateChat } from '@/lib/copilot/chat/lifecycle'
88
import { appendCopilotChatMessages } from '@/lib/copilot/chat/messages-store'
99
import { buildIntegrationToolSchemas } from '@/lib/copilot/chat/payload'
@@ -95,13 +95,14 @@ export async function executeInboxTask(taskId: string): Promise<void> {
9595
}
9696

9797
// Blocked senders and banned accounts must not drive the agent; the sender
98-
// email is checked directly because non-members resolve to the workspace
99-
// owner. Fails closed on lookup errors. No email response in any of these
100-
// paths — never mail a suspended account.
98+
// email is checked directly (domain list + the sender's own account ban)
99+
// because non-members resolve to the workspace owner. Fails closed on
100+
// lookup errors. No email response in any of these paths — never mail a
101+
// suspended account.
101102
let blockReason: string | null = null
102103
try {
103104
const [senderBlocked, [bannedUserId]] = await Promise.all([
104-
isEmailDomainBlocked(inboxTask.fromEmail),
105+
isEmailBlocked(inboxTask.fromEmail),
105106
getActivelyBannedUserIds([userId]),
106107
])
107108
if (senderBlocked || bannedUserId) {

0 commit comments

Comments
 (0)