Skip to content

Commit 96505eb

Browse files
Bill LeoutsakosBill Leoutsakos
authored andcommitted
fix(selectors): restore migration parity and egress guarantees
1 parent 8bc3cb3 commit 96505eb

64 files changed

Lines changed: 2116 additions & 724 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

apps/sim/app/api/selectors/execute/route.test.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,9 @@ describe('POST /api/selectors/execute', () => {
110110
it.each([
111111
[new SelectorContextUnavailableError(), 400, 'Context unavailable'],
112112
[new SelectorConnectionUnavailableError(), 403, 'Connection unavailable'],
113+
[new SelectorConnectionUnavailableError(401), 401, 'Connection unavailable'],
113114
[new SelectorOptionsUnavailableError(), 502, 'Options unavailable'],
115+
[new SelectorOptionsUnavailableError(429), 429, 'Options temporarily unavailable'],
114116
])('preserves selector error projection for %s', (error, status, message) => {
115117
expect(project(error)).toEqual({
116118
status,

apps/sim/app/api/selectors/execute/route.ts

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,18 @@ const selectorOperationErrorPolicy = extendInternalErrorPolicy(
2828
return internalErrorResponse(400, { error: 'Context unavailable' }, PRIVATE_NO_STORE)
2929
}
3030
if (error instanceof SelectorConnectionUnavailableError) {
31-
return internalErrorResponse(403, { error: 'Connection unavailable' }, PRIVATE_NO_STORE)
31+
return internalErrorResponse(
32+
error.status,
33+
{ error: 'Connection unavailable' },
34+
PRIVATE_NO_STORE
35+
)
3236
}
3337
if (error instanceof SelectorOptionsUnavailableError) {
34-
return internalErrorResponse(502, { error: 'Options unavailable' }, PRIVATE_NO_STORE)
38+
return internalErrorResponse(
39+
error.status,
40+
{ error: error.status === 429 ? 'Options temporarily unavailable' : 'Options unavailable' },
41+
PRIVATE_NO_STORE
42+
)
3543
}
3644
return null
3745
}
@@ -67,7 +75,7 @@ export const POST = defineInternalJsonRoute({
6775
}),
6876
errorPolicy: selectorErrorPolicy,
6977
parseOptions: { maxBodyBytes: 256 * 1024 },
70-
mapInput: ({ body }, { request }) => ({ ...body, signal: request.signal }),
78+
mapInput: ({ body }, { request }) => ({ ...body, signal: request.signal, auditRequest: request }),
7179
useCase: executeSelector,
7280
staticResponseHeaders: PRIVATE_NO_STORE,
7381
})

apps/sim/app/api/tools/managed-agent/list/route.ts

Lines changed: 0 additions & 146 deletions
This file was deleted.

apps/sim/lib/api/contracts/index.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ export * from './execution-payloads'
2222
export * from './folders'
2323
export * from './hotspots'
2424
export * from './inbox'
25-
export * from './managed-agents'
2625
export * from './media'
2726
export * from './permission-groups'
2827
export * from './pinned-items'

apps/sim/lib/api/contracts/managed-agents.ts

Lines changed: 0 additions & 42 deletions
This file was deleted.

apps/sim/lib/auth/credential-access.test.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,25 @@ describe('authorizeCredentialUse', () => {
163163
expect(result.resolvedCredentialId).toBe(ACCOUNT_ID)
164164
})
165165

166+
it('pins a legacy account id to the explicitly authorized workspace', async () => {
167+
const targetWorkspace = 'ws-2'
168+
const targetRow = { id: 'cred-2', workspaceId: targetWorkspace, type: 'oauth' }
169+
queueTableRows(credential, [])
170+
queueTableRows(credential, [targetRow])
171+
queueActorContext(targetRow)
172+
queueTokenIdentity(null, OWNER)
173+
mockResolveWorkspaceAccess.mockResolvedValue(workspaceAdmin)
174+
175+
const result = await authorizeCredentialUseForAuth(
176+
{ success: true, userId: 'acting-user', authType: 'session' },
177+
{ credentialId: ACCOUNT_ID, workspaceId: targetWorkspace }
178+
)
179+
180+
expect(result.ok).toBe(true)
181+
expect(result.workspaceId).toBe(targetWorkspace)
182+
expect(result.resolvedCredentialId).toBe(ACCOUNT_ID)
183+
})
184+
166185
it('rejects when no workspace credential is reachable by the caller', async () => {
167186
queueTableRows(credential, [])
168187
queueTableRows(credential, [sharedRow])

apps/sim/lib/auth/credential-access.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ export async function authorizeCredentialUse(
5555
params: {
5656
credentialId: string
5757
workflowId?: string
58+
workspaceId?: string
5859
requireWorkflowIdForInternal?: boolean
5960
callerUserId?: string
6061
}
@@ -75,10 +76,11 @@ export async function authorizeCredentialUseForAuth(
7576
params: {
7677
credentialId: string
7778
workflowId?: string
79+
workspaceId?: string
7880
callerUserId?: string
7981
}
8082
): Promise<CredentialAccessResult> {
81-
const { credentialId, workflowId, callerUserId } = params
83+
const { credentialId, workflowId, workspaceId, callerUserId } = params
8284

8385
if (!auth.success || !auth.userId) {
8486
return { ok: false, error: auth.error || 'Authentication required' }
@@ -112,7 +114,11 @@ export async function authorizeCredentialUseForAuth(
112114
return { ok: false, error: 'Workflow not found' }
113115
}
114116

115-
const scopeWorkspaceId = workflowContext?.workspaceId ?? null
117+
if (workflowContext?.workspaceId && workspaceId && workflowContext.workspaceId !== workspaceId) {
118+
return { ok: false, error: 'Credential is not accessible from this workspace' }
119+
}
120+
121+
const scopeWorkspaceId = workflowContext?.workspaceId ?? workspaceId ?? null
116122
const platformCredential = platformAccess.credential
117123

118124
if (platformCredential) {

apps/sim/lib/core/security/input-validation.server.test.ts

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,14 @@
33
*/
44
import { beforeEach, describe, expect, it, vi } from 'vitest'
55

6-
const { mockResolve } = vi.hoisted(() => ({ mockResolve: vi.fn() }))
6+
const { mockResolve, mockWarn } = vi.hoisted(() => ({
7+
mockResolve: vi.fn(),
8+
mockWarn: vi.fn(),
9+
}))
10+
11+
vi.mock('@sim/logger', () => ({
12+
createLogger: () => ({ warn: mockWarn }),
13+
}))
714

815
vi.mock('@sim/security/dns', () => ({
916
resolveHostAddresses: mockResolve,
@@ -96,4 +103,15 @@ describe('validateUrlWithDNS address classification', () => {
96103

97104
expect((await validateUrlWithDNS('https://missing.example/api')).isValid).toBe(false)
98105
})
106+
107+
it('can conceal credential-derived host details in validation logs', async () => {
108+
mockResolve.mockRejectedValue(new Error('DNS failure with credential-host-canary'))
109+
110+
await validateUrlWithDNS('https://credential-host-canary.example/api', 'url', {
111+
logDetails: false,
112+
})
113+
114+
expect(mockWarn).toHaveBeenCalledWith('DNS lookup failed for URL', { paramName: 'url' })
115+
expect(JSON.stringify(mockWarn.mock.calls)).not.toContain('credential-host-canary')
116+
})
99117
})

apps/sim/lib/core/security/input-validation.server.ts

Lines changed: 21 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -49,9 +49,9 @@ export interface AsyncValidationResult extends ValidationResult {
4949
export async function validateUrlWithDNS(
5050
url: string | null | undefined,
5151
paramName = 'url',
52-
options: { allowHttp?: boolean } = {}
52+
options: { allowHttp?: boolean; logDetails?: boolean } = {}
5353
): Promise<AsyncValidationResult> {
54-
const basicValidation = validateExternalUrl(url, paramName, options)
54+
const basicValidation = validateExternalUrl(url, paramName, { allowHttp: options.allowHttp })
5555
if (!basicValidation.isValid) {
5656
return basicValidation
5757
}
@@ -77,11 +77,16 @@ export async function validateUrlWithDNS(
7777
)
7878

7979
if (usable.length === 0) {
80-
logger.warn('URL resolves to blocked IP address', {
81-
paramName,
82-
hostname,
83-
resolvedIP: addresses.find((address) => isPrivateIp(address)),
84-
})
80+
logger.warn(
81+
'URL resolves to blocked IP address',
82+
options.logDetails === false
83+
? { paramName }
84+
: {
85+
paramName,
86+
hostname,
87+
resolvedIP: addresses.find((address) => isPrivateIp(address)),
88+
}
89+
)
8590
return {
8691
isValid: false,
8792
error: `${paramName} resolves to a blocked IP address`,
@@ -96,11 +101,12 @@ export async function validateUrlWithDNS(
96101
originalHostname: hostname,
97102
}
98103
} catch (error) {
99-
logger.warn('DNS lookup failed for URL', {
100-
paramName,
101-
hostname,
102-
error: toError(error).message,
103-
})
104+
logger.warn(
105+
'DNS lookup failed for URL',
106+
options.logDetails === false
107+
? { paramName }
108+
: { paramName, hostname, error: toError(error).message }
109+
)
104110
return {
105111
isValid: false,
106112
error: `${paramName} hostname could not be resolved`,
@@ -383,6 +389,8 @@ export interface SecureFetchOptions {
383389
* bypassed (the proxy resolves the target).
384390
*/
385391
proxyUrl?: string
392+
/** Hide credential-derived URL details from validation logs. */
393+
logUrlValidationDetails?: boolean
386394
}
387395

388396
export class SecureFetchHeaders {
@@ -1317,6 +1325,7 @@ export async function secureFetchWithValidation(
13171325
): Promise<SecureFetchResponse> {
13181326
const validation = await validateUrlWithDNS(url, paramName, {
13191327
allowHttp: options.allowHttp,
1328+
logDetails: options.logUrlValidationDetails,
13201329
})
13211330
if (!validation.isValid) {
13221331
throw new Error(validation.error)

0 commit comments

Comments
 (0)