Skip to content

Commit bad68ac

Browse files
committed
fix(network-policy): enforce on v1 API path, isTruthy break-glass, mapped-CIDR matching, realtime fail-open+audit consistency, drop dead exports
1 parent b50f238 commit bad68ac

5 files changed

Lines changed: 108 additions & 20 deletions

File tree

apps/realtime/src/middleware/network-policy.ts

Lines changed: 52 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { getIp } from '@better-auth/core/utils/ip'
2+
import { AuditAction, AuditResourceType, recordAudit } from '@sim/audit'
23
import { db } from '@sim/db'
34
import { member, organization } from '@sim/db/schema'
45
import { createLogger } from '@sim/logger'
@@ -72,23 +73,45 @@ async function getOrgAllowlist(organizationId: string): Promise<CompiledAllowlis
7273

7374
interface HandshakeLike {
7475
headers: Record<string, string | string[] | undefined>
75-
address: string
76+
}
77+
78+
/**
79+
* Per-member throttle for socket-denial audit events (this is a separate
80+
* process from the app, so it keeps its own map) — one event per member per
81+
* window is enough to show who is blocked without a reconnect loop flooding
82+
* the log.
83+
*/
84+
const DENIAL_AUDIT_WINDOW_MS = 5 * 60 * 1000
85+
const lastDenialAuditAt = new Map<string, number>()
86+
87+
function recordSocketDenial(userId: string, organizationId: string, clientIp: string | null): void {
88+
const last = lastDenialAuditAt.get(userId)
89+
if (last && Date.now() - last < DENIAL_AUDIT_WINDOW_MS) return
90+
lastDenialAuditAt.set(userId, Date.now())
91+
recordAudit({
92+
workspaceId: null,
93+
actorId: userId,
94+
action: AuditAction.ORG_IP_ACCESS_DENIED,
95+
resourceType: AuditResourceType.ORGANIZATION,
96+
resourceId: organizationId,
97+
description: clientIp
98+
? `Denied realtime connection from ${clientIp} by the IP allowlist`
99+
: 'Denied realtime connection by the IP allowlist (client IP unresolvable)',
100+
metadata: { clientIp, surface: 'realtime' },
101+
})
76102
}
77103

78104
/**
79105
* Socket-handshake counterpart of the app's org IP-allowlist enforcement.
80106
* Resolves the client IP with Better Auth's trusted-proxy resolver from the
81-
* handshake headers (falling back to the socket peer address when no
82-
* forwarding headers are present), then checks the member org's allowlist.
83-
* Fail-closed like the app side: an active policy with an unresolvable
84-
* client IP denies the connection.
107+
* handshake headers, then checks the member org's allowlist. Shares the
108+
* app's posture: fail-closed on an unresolvable client IP (active policy +
109+
* no derivable trusted IP → deny), fail-open on an unexpected/DB error.
85110
*
86111
* Plan gating is intentionally absent here — `apps/realtime` cannot import
87-
* billing code, and over-denial is the safe direction for a security
88-
* control (the app-side check makes the opposite, fail-open call on DB
89-
* errors because a blip must not lock an org out of the product). A
90-
* downgraded org's stored-but-enabled policy therefore keeps gating sockets
91-
* until the org disables it; the app is the sole writer of the settings.
112+
* billing code. A downgraded org's stored-but-enabled policy therefore keeps
113+
* gating sockets until the org disables it (the app is the sole writer of
114+
* the settings), which is the safe direction for a security control.
92115
*/
93116
export async function isSocketAllowedByNetworkPolicy(
94117
userId: string,
@@ -109,20 +132,33 @@ export async function isSocketAllowedByNetworkPolicy(
109132
if (typeof value === 'string') headers.set(key, value)
110133
else if (Array.isArray(value)) headers.set(key, value.join(', '))
111134
}
112-
const clientIp =
113-
getIp(new Request('http://socket.internal/', { headers }), IP_RESOLUTION_OPTIONS) ??
114-
(handshake.address || null)
135+
// Trust only the trusted-proxy-resolved address — the same resolution the
136+
// app uses. No fallback to the raw socket peer: matching the app's
137+
// fail-closed-on-unresolvable-IP behavior is more important than salvaging
138+
// a direct-connect address the app-side check would never have accepted.
139+
const clientIp = getIp(
140+
new Request('http://socket.internal/', { headers }),
141+
IP_RESOLUTION_OPTIONS
142+
)
115143

116144
if (!clientIp) {
117145
logger.warn('Denying socket: network policy active but client IP unresolvable', {
118146
userId,
119147
organizationId,
120148
})
149+
recordSocketDenial(userId, organizationId, null)
150+
return false
151+
}
152+
if (!isAddressAllowed(clientIp, allowlist)) {
153+
recordSocketDenial(userId, organizationId, clientIp)
121154
return false
122155
}
123-
return isAddressAllowed(clientIp, allowlist)
156+
return true
124157
} catch (error) {
125-
logger.error('Network policy check failed; denying socket', { userId, error })
126-
return false
158+
// Fail OPEN on an unexpected/DB error, matching the app-side network-policy
159+
// loader — a transient blip must not lock members out of collaboration.
160+
// The primary boundary (policy loaded, IP not allowed) stays fail-closed.
161+
logger.error('Network policy check failed; allowing socket', { userId, error })
162+
return true
127163
}
128164
}

apps/sim/app/api/v1/auth.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { createLogger } from '@sim/logger'
22
import type { NextRequest } from 'next/server'
33
import { authenticateApiKeyFromHeader, updateApiKeyLastUsed } from '@/lib/api-key/service'
44
import { ANONYMOUS_USER_ID } from '@/lib/auth/constants'
5+
import { enforceOrgNetworkPolicy, getTrustedClientIp } from '@/lib/auth/network-policy'
56
import { isAuthDisabled } from '@/lib/core/config/env-flags'
67

78
const logger = createLogger('V1Auth')
@@ -43,6 +44,17 @@ export async function authenticateV1Request(request: NextRequest): Promise<AuthR
4344
}
4445
}
4546

47+
// Org IP allowlist applies to API-key auth on the public v1 surface too,
48+
// mirroring the legacy hybrid-auth path — otherwise a member under an IP
49+
// restriction could bypass it by calling v1 instead.
50+
const network = await enforceOrgNetworkPolicy(result.userId, () => getTrustedClientIp(request))
51+
if (!network.allowed) {
52+
return {
53+
authenticated: false,
54+
error: network.reason,
55+
}
56+
}
57+
4658
await updateApiKeyLastUsed(result.keyId!)
4759

4860
return {

apps/sim/lib/auth/network-policy.ts

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,13 +14,13 @@ import {
1414
import { eq } from 'drizzle-orm'
1515
import { getMemberOrganizationId } from '@/lib/auth/security-policy'
1616
import { isOrganizationOnEnterprisePlan } from '@/lib/billing/core/subscription'
17-
import { env } from '@/lib/core/config/env'
17+
import { env, isTruthy } from '@/lib/core/config/env'
1818
import { isBillingEnabled } from '@/lib/core/config/env-flags'
1919

2020
const logger = createLogger('NetworkPolicy')
2121

2222
/** How long a compiled org network policy is served from process memory. */
23-
export const NETWORK_POLICY_CACHE_TTL_MS = 60 * 1000
23+
const NETWORK_POLICY_CACHE_TTL_MS = 60 * 1000
2424

2525
interface ResolvedNetworkPolicy {
2626
/** Compiled allowlist, or null when no restriction applies. */
@@ -59,7 +59,7 @@ export function getTrustedClientIp(request: Request): string | null {
5959
* lock an entire org out of the product; the realtime handshake makes the
6060
* opposite call because denying one socket is cheap.
6161
*/
62-
export async function getNetworkPolicy(
62+
async function getNetworkPolicy(
6363
organizationId: string | null | undefined
6464
): Promise<ResolvedNetworkPolicy> {
6565
if (!organizationId) return NO_POLICY
@@ -119,11 +119,24 @@ const DENIED: NetworkPolicyDecision = {
119119
* enough for a security team to see who is being blocked and from where.
120120
*/
121121
const DENIAL_AUDIT_WINDOW_MS = 5 * 60 * 1000
122+
const DENIAL_AUDIT_MAX_TRACKED = 10_000
122123
const lastDenialAuditAt = new Map<string, number>()
123124

125+
/** Test-only: clears the denial-audit throttle so suites don't cross-pollute. */
126+
export function __resetDenialAuditThrottle(): void {
127+
lastDenialAuditAt.clear()
128+
}
129+
124130
function recordDenialAudit(userId: string, organizationId: string, clientIp: string | null): void {
125131
const last = lastDenialAuditAt.get(userId)
126132
if (last && Date.now() - last < DENIAL_AUDIT_WINDOW_MS) return
133+
// Bound the throttle map: on overflow, drop the oldest-inserted key (Map
134+
// preserves insertion order). Denied users are all authenticated, so this
135+
// is a slow ceiling, not an attacker-inflatable one.
136+
if (lastDenialAuditAt.size >= DENIAL_AUDIT_MAX_TRACKED) {
137+
const oldest = lastDenialAuditAt.keys().next().value
138+
if (oldest !== undefined) lastDenialAuditAt.delete(oldest)
139+
}
127140
lastDenialAuditAt.set(userId, Date.now())
128141
recordAudit({
129142
workspaceId: null,
@@ -153,7 +166,7 @@ export async function enforceOrgNetworkPolicy(
153166
userId: string | null | undefined,
154167
resolveClientIp: () => string | null | undefined
155168
): Promise<NetworkPolicyDecision> {
156-
if (env.DISABLE_ORG_IP_ALLOWLIST) return ALLOWED
169+
if (isTruthy(env.DISABLE_ORG_IP_ALLOWLIST)) return ALLOWED
157170

158171
const organizationId = await getMemberOrganizationId(userId)
159172
if (!organizationId) return ALLOWED

packages/platform-authz/src/network.test.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ describe('isValidCidrEntry', () => {
1515
' 10.0.0.0/24 ',
1616
'10.0.0.0/16 # Frankfurt VPN',
1717
'203.0.113.7 #office',
18+
'::ffff:10.0.0.0/120',
1819
])('accepts %s', (entry) => {
1920
expect(isValidCidrEntry(entry)).toBe(true)
2021
})
@@ -34,6 +35,7 @@ describe('isValidCidrEntry', () => {
3435
'fe80::%eth0',
3536
'# label only',
3637
'banana # labelled garbage',
38+
'::ffff:10.0.0.0/24',
3739
])('rejects %s', (entry) => {
3840
expect(isValidCidrEntry(entry)).toBe(false)
3941
})
@@ -81,6 +83,15 @@ describe('isAddressAllowed', () => {
8183
expect(isAddressAllowed('203.0.113.7', compiled)).toBe(false)
8284
})
8385

86+
it('matches an IPv4-mapped-IPv6 CIDR entry against demoted mapped clients', () => {
87+
const mapped = compileAllowlist(['::ffff:10.0.0.0/120'])
88+
expect(mapped.v4).toHaveLength(1)
89+
expect(mapped.v6).toHaveLength(0)
90+
expect(isAddressAllowed('::ffff:10.0.0.5', mapped)).toBe(true)
91+
expect(isAddressAllowed('10.0.0.5', mapped)).toBe(true)
92+
expect(isAddressAllowed('::ffff:10.0.1.5', mapped)).toBe(false)
93+
})
94+
8495
it('labels never affect matching', () => {
8596
const labelled = compileAllowlist(['10.0.0.0/16 # Frankfurt VPN'])
8697
expect(isAddressAllowed('10.0.5.5', labelled)).toBe(true)

packages/platform-authz/src/network.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,22 @@ function parseCidr(entry: string): ParsedCidr | null {
9090
const [host, prefixPart, ...rest] = stripEntryLabel(entry).split('/')
9191
if (rest.length > 0) return null
9292

93+
// An IPv4-mapped IPv6 host (`::ffff:a.b.c.d`) is canonicalized to IPv4 so it
94+
// lands in the same family bucket as the client addresses isAddressAllowed
95+
// demotes — otherwise a mapped-form entry compiles into the v6 list and a
96+
// mapped client, demoted to v4, would never match it. A prefix on the
97+
// mapped form addresses the full 128-bit space, so its meaningful range is
98+
// the low 32 bits (>= /96); demote it by 96.
99+
const mappedV4 = host.match(/^::ffff:(\d{1,3}(?:\.\d{1,3}){3})$/i)
100+
if (mappedV4) {
101+
const v4Mapped = parseIpv4(mappedV4[1])
102+
if (v4Mapped === null) return null
103+
if (prefixPart === undefined) return { kind: 'v4', value: v4Mapped, prefix: 32 }
104+
const mappedPrefix = Number(prefixPart)
105+
if (!Number.isInteger(mappedPrefix) || mappedPrefix < 96 || mappedPrefix > 128) return null
106+
return { kind: 'v4', value: v4Mapped, prefix: mappedPrefix - 96 }
107+
}
108+
93109
const v4 = parseIpv4(host)
94110
if (v4 !== null) {
95111
const prefix = prefixPart === undefined ? 32 : Number(prefixPart)

0 commit comments

Comments
 (0)