Skip to content

Commit be8a93d

Browse files
authored
fix(security): validate cloud region/project inputs and bind vertex credentials to the executing workspace (#6167)
* fix(security): validate cloud region/project inputs and bind vertex credentials to the executing workspace Two credential-exposure bugs on the Vertex AI path. 1. `vertexLocation` reached `new GoogleGenAI({ location })` unvalidated. The SDK interpolates that value straight into the API hostname (`https://${location}-aiplatform.googleapis.com/`), so a value like `attacker.tld/x` terminates the authority component and relocates the request — with the workspace's GCP bearer token attached by the auth client — to an arbitrary host. Reachable from agent/router/evaluator blocks and from `POST /api/guardrails/validate` with only a session cookie. `vertexProject` (interpolated into the URL path) and the Bedrock region (interpolated into the endpoint hostname) had the same shape of problem. Adds `validateGoogleCloudLocation` / `validateGoogleCloudProject` to the shared input-validation module and applies them, plus the existing `validateAwsRegion`, at the provider chokepoints. Every path to the SDK goes through `executeRequest`, so no caller can bypass them. `azureEndpoint` already routes through the DNS-pinning SSRF guard. 2. `resolveVertexCredential` enforced only the user↔credential predicate and never the workflow-workspace↔credential predicate that `authorizeCredentialUse` applies on the HTTP path. A credential held in workspace B could be pasted into a workflow in workspace A and consumed by workspace-A principals with no access to B — including on deployed runs, where `enforceCredentialAccess` is false and `ctx.userId` is the workflow owner rather than the trigger caller. Service-account credentials mint a `cloud-platform`-scoped token, so this handed workspace A the use of workspace B's GCP identity. The resolver now takes the executing `workspaceId` and rejects a credential belonging to a different workspace, mirroring `credential-access.ts`. All four executor call sites pass `ctx.workspaceId`. * fix(providers): normalize vertex location case before validating Hostnames are case-insensitive, so a mixed-case location like US-Central1 reaches Google today. Lowercase it before the region check rather than rejecting an input that currently works. * fix(security): accept AWS European Sovereign Cloud regions in validateAwsRegion eusc-de-east-1 is a real Bedrock-enabled region that the existing pattern did not cover, so applying the validator to bedrockRegion would have rejected a config that worked before. Adds the eusc-<country> partition.
1 parent cd50a44 commit be8a93d

10 files changed

Lines changed: 466 additions & 30 deletions

File tree

apps/sim/executor/handlers/agent/agent-handler.ts

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1173,11 +1173,12 @@ export class AgentBlockHandler implements BlockHandler {
11731173
let finalApiKey: string | undefined = providerRequest.apiKey
11741174

11751175
if (providerId === 'vertex' && providerRequest.vertexCredential) {
1176-
finalApiKey = await resolveVertexCredential(
1177-
providerRequest.vertexCredential,
1178-
ctx.userId,
1179-
'vertex-agent'
1180-
)
1176+
finalApiKey = await resolveVertexCredential({
1177+
credentialId: providerRequest.vertexCredential,
1178+
actingUserId: ctx.userId,
1179+
workspaceId: ctx.workspaceId,
1180+
callerLabel: 'vertex-agent',
1181+
})
11811182
}
11821183

11831184
const { blockData, blockNameMapping } = collectBlockData(ctx)

apps/sim/executor/handlers/evaluator/evaluator-handler.ts

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -135,11 +135,12 @@ export class EvaluatorBlockHandler implements BlockHandler {
135135

136136
let finalApiKey: string | undefined = evaluatorConfig.apiKey
137137
if (providerId === 'vertex' && evaluatorConfig.vertexCredential) {
138-
finalApiKey = await resolveVertexCredential(
139-
evaluatorConfig.vertexCredential,
140-
ctx.userId,
141-
'vertex-evaluator'
142-
)
138+
finalApiKey = await resolveVertexCredential({
139+
credentialId: evaluatorConfig.vertexCredential,
140+
actingUserId: ctx.userId,
141+
workspaceId: ctx.workspaceId,
142+
callerLabel: 'vertex-evaluator',
143+
})
143144
}
144145

145146
try {

apps/sim/executor/handlers/router/router-handler.ts

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -99,11 +99,12 @@ export class RouterBlockHandler implements BlockHandler {
9999

100100
let finalApiKey: string | undefined = routerConfig.apiKey
101101
if (providerId === 'vertex' && routerConfig.vertexCredential) {
102-
finalApiKey = await resolveVertexCredential(
103-
routerConfig.vertexCredential,
104-
ctx.userId,
105-
'vertex-router'
106-
)
102+
finalApiKey = await resolveVertexCredential({
103+
credentialId: routerConfig.vertexCredential,
104+
actingUserId: ctx.userId,
105+
workspaceId: ctx.workspaceId,
106+
callerLabel: 'vertex-router',
107+
})
107108
}
108109

109110
const providerRequest: Record<string, any> = {
@@ -239,11 +240,12 @@ export class RouterBlockHandler implements BlockHandler {
239240

240241
let finalApiKey: string | undefined = routerConfig.apiKey
241242
if (providerId === 'vertex' && routerConfig.vertexCredential) {
242-
finalApiKey = await resolveVertexCredential(
243-
routerConfig.vertexCredential,
244-
ctx.userId,
245-
'vertex-router'
246-
)
243+
finalApiKey = await resolveVertexCredential({
244+
credentialId: routerConfig.vertexCredential,
245+
actingUserId: ctx.userId,
246+
workspaceId: ctx.workspaceId,
247+
callerLabel: 'vertex-router',
248+
})
247249
}
248250

249251
const providerRequest: Record<string, any> = {
Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import { beforeEach, describe, expect, it, vi } from 'vitest'
5+
6+
const { mockGetCredentialActorContext, mockGetServiceAccountToken, mockRefreshTokenIfNeeded } =
7+
vi.hoisted(() => ({
8+
mockGetCredentialActorContext: vi.fn(),
9+
mockGetServiceAccountToken: vi.fn(),
10+
mockRefreshTokenIfNeeded: vi.fn(),
11+
}))
12+
13+
vi.mock('@/lib/credentials/access', () => ({
14+
getCredentialActorContext: mockGetCredentialActorContext,
15+
}))
16+
vi.mock('@/app/api/auth/oauth/utils', () => ({
17+
getServiceAccountToken: mockGetServiceAccountToken,
18+
refreshTokenIfNeeded: mockRefreshTokenIfNeeded,
19+
}))
20+
21+
import { resolveVertexCredential } from '@/executor/utils/vertex-credential'
22+
23+
function actorContext(workspaceId: string) {
24+
return {
25+
credential: {
26+
id: 'cred-b',
27+
workspaceId,
28+
type: 'service_account',
29+
accountId: null,
30+
},
31+
member: { id: 'member-1' },
32+
hasWorkspaceAccess: true,
33+
canWriteWorkspace: true,
34+
isAdmin: false,
35+
}
36+
}
37+
38+
describe('resolveVertexCredential workspace binding', () => {
39+
beforeEach(() => {
40+
vi.clearAllMocks()
41+
mockGetServiceAccountToken.mockResolvedValue('gcp-access-token')
42+
})
43+
44+
it('rejects a credential owned by a different workspace than the executing workflow', async () => {
45+
mockGetCredentialActorContext.mockResolvedValue(actorContext('workspace-b'))
46+
47+
await expect(
48+
resolveVertexCredential({
49+
credentialId: 'cred-b',
50+
actingUserId: 'user-1',
51+
workspaceId: 'workspace-a',
52+
})
53+
).rejects.toThrow('Credential is not accessible from this workflow workspace')
54+
55+
expect(mockGetServiceAccountToken).not.toHaveBeenCalled()
56+
})
57+
58+
it('resolves a credential owned by the executing workspace', async () => {
59+
mockGetCredentialActorContext.mockResolvedValue(actorContext('workspace-a'))
60+
61+
await expect(
62+
resolveVertexCredential({
63+
credentialId: 'cred-b',
64+
actingUserId: 'user-1',
65+
workspaceId: 'workspace-a',
66+
})
67+
).resolves.toBe('gcp-access-token')
68+
})
69+
70+
it('still enforces the user-to-credential check within the same workspace', async () => {
71+
mockGetCredentialActorContext.mockResolvedValue({
72+
...actorContext('workspace-a'),
73+
member: null,
74+
isAdmin: false,
75+
})
76+
77+
await expect(
78+
resolveVertexCredential({
79+
credentialId: 'cred-b',
80+
actingUserId: 'user-1',
81+
workspaceId: 'workspace-a',
82+
})
83+
).rejects.toThrow('Not authorized to use this Vertex AI credential')
84+
})
85+
86+
it('requires an authenticated acting user', async () => {
87+
await expect(
88+
resolveVertexCredential({
89+
credentialId: 'cred-b',
90+
actingUserId: undefined,
91+
workspaceId: 'workspace-a',
92+
})
93+
).rejects.toThrow('requires an authenticated user')
94+
})
95+
})

apps/sim/executor/utils/vertex-credential.ts

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7,17 +7,27 @@ import { getServiceAccountToken, refreshTokenIfNeeded } from '@/app/api/auth/oau
77

88
const logger = createLogger('VertexCredential')
99

10+
export interface ResolveVertexCredentialParams {
11+
credentialId: string
12+
actingUserId: string | undefined
13+
/** Workspace of the executing workflow. The credential must belong to it. */
14+
workspaceId: string | null | undefined
15+
callerLabel?: string
16+
}
17+
1018
/**
1119
* Resolves a Vertex AI OAuth credential to an access token.
12-
* Shared across agent, evaluator, and router handlers. Authorizes the executing
13-
* user against the credential first — workspace credentials are usable by their
14-
* members and by derived workspace admins, matching `authorizeCredentialUse`.
20+
* Shared across agent, evaluator, and router handlers. Enforces the same two
21+
* predicates as `authorizeCredentialUse`: the executing user must be a member
22+
* (or derived workspace admin) of the credential, and the credential must belong
23+
* to the workspace the workflow is executing in.
1524
*/
16-
export async function resolveVertexCredential(
17-
credentialId: string,
18-
actingUserId: string | undefined,
19-
callerLabel = 'vertex'
20-
): Promise<string> {
25+
export async function resolveVertexCredential({
26+
credentialId,
27+
actingUserId,
28+
workspaceId,
29+
callerLabel = 'vertex',
30+
}: ResolveVertexCredentialParams): Promise<string> {
2131
const requestId = `${callerLabel}-${Date.now()}`
2232

2333
logger.info(`[${requestId}] Resolving Vertex AI credential: ${credentialId}`)
@@ -31,6 +41,13 @@ export async function resolveVertexCredential(
3141
if (!cred) {
3242
throw new Error(`Vertex AI credential not found: ${credentialId}`)
3343
}
44+
if (workspaceId && cred.workspaceId !== workspaceId) {
45+
logger.warn(`[${requestId}] Vertex AI credential belongs to a different workspace`, {
46+
credentialId,
47+
executingWorkspaceId: workspaceId,
48+
})
49+
throw new Error('Credential is not accessible from this workflow workspace')
50+
}
3451
if (!access.hasWorkspaceAccess || (!access.member && !access.isAdmin)) {
3552
throw new Error('Not authorized to use this Vertex AI credential')
3653
}

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

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ import {
99
validateExternalUrl,
1010
validateFileExtension,
1111
validateGoogleCalendarId,
12+
validateGoogleCloudLocation,
13+
validateGoogleCloudProject,
1214
validateHostname,
1315
validateImageUrl,
1416
validateInteger,
@@ -1458,6 +1460,19 @@ describe('validateAwsRegion', () => {
14581460
})
14591461
})
14601462

1463+
describe('valid European Sovereign Cloud regions', () => {
1464+
it.concurrent('should accept eusc-de-east-1', () => {
1465+
const result = validateAwsRegion('eusc-de-east-1')
1466+
expect(result.isValid).toBe(true)
1467+
expect(result.sanitized).toBe('eusc-de-east-1')
1468+
})
1469+
1470+
it.concurrent('should reject a malformed eusc region', () => {
1471+
expect(validateAwsRegion('eusc-de-east').isValid).toBe(false)
1472+
expect(validateAwsRegion('eusc-deu-east-1').isValid).toBe(false)
1473+
})
1474+
})
1475+
14611476
describe('valid China regions', () => {
14621477
it.concurrent('should accept cn-north-1', () => {
14631478
const result = validateAwsRegion('cn-north-1')
@@ -1561,6 +1576,91 @@ describe('validateAwsRegion', () => {
15611576
})
15621577
})
15631578

1579+
describe('validateGoogleCloudLocation', () => {
1580+
describe('valid locations', () => {
1581+
it.concurrent.each([
1582+
'us-central1',
1583+
'us-east5',
1584+
'europe-west4',
1585+
'northamerica-northeast1',
1586+
'southamerica-east1',
1587+
'asia-northeast3',
1588+
'australia-southeast2',
1589+
'africa-south1',
1590+
'me-central2',
1591+
'global',
1592+
])('should accept %s', (location) => {
1593+
const result = validateGoogleCloudLocation(location)
1594+
expect(result.isValid).toBe(true)
1595+
expect(result.sanitized).toBe(location)
1596+
})
1597+
})
1598+
1599+
describe('hostname injection', () => {
1600+
it.concurrent.each([
1601+
'attacker.example.com/x',
1602+
'us-central1/../attacker.tld',
1603+
'us-central1:8080',
1604+
'user@attacker.tld',
1605+
'us-central1?a=b',
1606+
'us-central1#frag',
1607+
'us central1',
1608+
'us-central1\n',
1609+
'US-CENTRAL1',
1610+
'../us-central1',
1611+
])('should reject %j', (location) => {
1612+
const result = validateGoogleCloudLocation(location)
1613+
expect(result.isValid).toBe(false)
1614+
})
1615+
})
1616+
1617+
it.concurrent('should reject empty and missing values', () => {
1618+
expect(validateGoogleCloudLocation('').isValid).toBe(false)
1619+
expect(validateGoogleCloudLocation(null).isValid).toBe(false)
1620+
expect(validateGoogleCloudLocation(undefined).isValid).toBe(false)
1621+
})
1622+
1623+
it.concurrent('should name the parameter in the error', () => {
1624+
const result = validateGoogleCloudLocation('bad host', 'vertexLocation')
1625+
expect(result.error).toContain('vertexLocation')
1626+
})
1627+
})
1628+
1629+
describe('validateGoogleCloudProject', () => {
1630+
describe('valid projects', () => {
1631+
it.concurrent.each(['my-project', 'sim-prod-1', 'abcdef', '123456789012'])(
1632+
'should accept %s',
1633+
(project) => {
1634+
const result = validateGoogleCloudProject(project)
1635+
expect(result.isValid).toBe(true)
1636+
expect(result.sanitized).toBe(project)
1637+
}
1638+
)
1639+
})
1640+
1641+
describe('path injection and malformed ids', () => {
1642+
it.concurrent.each([
1643+
'my-project/../../other',
1644+
'my-project:alias',
1645+
'my project',
1646+
'My-Project',
1647+
'1project',
1648+
'my-project-',
1649+
'abc',
1650+
'a'.repeat(31),
1651+
])('should reject %j', (project) => {
1652+
const result = validateGoogleCloudProject(project)
1653+
expect(result.isValid).toBe(false)
1654+
})
1655+
})
1656+
1657+
it.concurrent('should reject empty and missing values', () => {
1658+
expect(validateGoogleCloudProject('').isValid).toBe(false)
1659+
expect(validateGoogleCloudProject(null).isValid).toBe(false)
1660+
expect(validateGoogleCloudProject(undefined).isValid).toBe(false)
1661+
})
1662+
})
1663+
15641664
describe('validateS3BucketName', () => {
15651665
describe('valid bucket names', () => {
15661666
it.concurrent('should accept simple bucket name', () => {

0 commit comments

Comments
 (0)