Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 6 additions & 5 deletions apps/sim/executor/handlers/agent/agent-handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1173,11 +1173,12 @@ export class AgentBlockHandler implements BlockHandler {
let finalApiKey: string | undefined = providerRequest.apiKey

if (providerId === 'vertex' && providerRequest.vertexCredential) {
finalApiKey = await resolveVertexCredential(
providerRequest.vertexCredential,
ctx.userId,
'vertex-agent'
)
finalApiKey = await resolveVertexCredential({
credentialId: providerRequest.vertexCredential,
actingUserId: ctx.userId,
workspaceId: ctx.workspaceId,
callerLabel: 'vertex-agent',
})
}

const { blockData, blockNameMapping } = collectBlockData(ctx)
Expand Down
11 changes: 6 additions & 5 deletions apps/sim/executor/handlers/evaluator/evaluator-handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -135,11 +135,12 @@ export class EvaluatorBlockHandler implements BlockHandler {

let finalApiKey: string | undefined = evaluatorConfig.apiKey
if (providerId === 'vertex' && evaluatorConfig.vertexCredential) {
finalApiKey = await resolveVertexCredential(
evaluatorConfig.vertexCredential,
ctx.userId,
'vertex-evaluator'
)
finalApiKey = await resolveVertexCredential({
credentialId: evaluatorConfig.vertexCredential,
actingUserId: ctx.userId,
workspaceId: ctx.workspaceId,
callerLabel: 'vertex-evaluator',
})
}

try {
Expand Down
22 changes: 12 additions & 10 deletions apps/sim/executor/handlers/router/router-handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,11 +99,12 @@ export class RouterBlockHandler implements BlockHandler {

let finalApiKey: string | undefined = routerConfig.apiKey
if (providerId === 'vertex' && routerConfig.vertexCredential) {
finalApiKey = await resolveVertexCredential(
routerConfig.vertexCredential,
ctx.userId,
'vertex-router'
)
finalApiKey = await resolveVertexCredential({
credentialId: routerConfig.vertexCredential,
actingUserId: ctx.userId,
workspaceId: ctx.workspaceId,
callerLabel: 'vertex-router',
})
}

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

let finalApiKey: string | undefined = routerConfig.apiKey
if (providerId === 'vertex' && routerConfig.vertexCredential) {
finalApiKey = await resolveVertexCredential(
routerConfig.vertexCredential,
ctx.userId,
'vertex-router'
)
finalApiKey = await resolveVertexCredential({
credentialId: routerConfig.vertexCredential,
actingUserId: ctx.userId,
workspaceId: ctx.workspaceId,
callerLabel: 'vertex-router',
})
}

const providerRequest: Record<string, any> = {
Expand Down
95 changes: 95 additions & 0 deletions apps/sim/executor/utils/vertex-credential.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
/**
* @vitest-environment node
*/
import { beforeEach, describe, expect, it, vi } from 'vitest'

const { mockGetCredentialActorContext, mockGetServiceAccountToken, mockRefreshTokenIfNeeded } =
vi.hoisted(() => ({
mockGetCredentialActorContext: vi.fn(),
mockGetServiceAccountToken: vi.fn(),
mockRefreshTokenIfNeeded: vi.fn(),
}))

vi.mock('@/lib/credentials/access', () => ({
getCredentialActorContext: mockGetCredentialActorContext,
}))
vi.mock('@/app/api/auth/oauth/utils', () => ({
getServiceAccountToken: mockGetServiceAccountToken,
refreshTokenIfNeeded: mockRefreshTokenIfNeeded,
}))

import { resolveVertexCredential } from '@/executor/utils/vertex-credential'

function actorContext(workspaceId: string) {
return {
credential: {
id: 'cred-b',
workspaceId,
type: 'service_account',
accountId: null,
},
member: { id: 'member-1' },
hasWorkspaceAccess: true,
canWriteWorkspace: true,
isAdmin: false,
}
}

describe('resolveVertexCredential workspace binding', () => {
beforeEach(() => {
vi.clearAllMocks()
mockGetServiceAccountToken.mockResolvedValue('gcp-access-token')
})

it('rejects a credential owned by a different workspace than the executing workflow', async () => {
mockGetCredentialActorContext.mockResolvedValue(actorContext('workspace-b'))

await expect(
resolveVertexCredential({
credentialId: 'cred-b',
actingUserId: 'user-1',
workspaceId: 'workspace-a',
})
).rejects.toThrow('Credential is not accessible from this workflow workspace')

expect(mockGetServiceAccountToken).not.toHaveBeenCalled()
})

it('resolves a credential owned by the executing workspace', async () => {
mockGetCredentialActorContext.mockResolvedValue(actorContext('workspace-a'))

await expect(
resolveVertexCredential({
credentialId: 'cred-b',
actingUserId: 'user-1',
workspaceId: 'workspace-a',
})
).resolves.toBe('gcp-access-token')
})

it('still enforces the user-to-credential check within the same workspace', async () => {
mockGetCredentialActorContext.mockResolvedValue({
...actorContext('workspace-a'),
member: null,
isAdmin: false,
})

await expect(
resolveVertexCredential({
credentialId: 'cred-b',
actingUserId: 'user-1',
workspaceId: 'workspace-a',
})
).rejects.toThrow('Not authorized to use this Vertex AI credential')
})

it('requires an authenticated acting user', async () => {
await expect(
resolveVertexCredential({
credentialId: 'cred-b',
actingUserId: undefined,
workspaceId: 'workspace-a',
})
).rejects.toThrow('requires an authenticated user')
})
})
33 changes: 25 additions & 8 deletions apps/sim/executor/utils/vertex-credential.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,17 +7,27 @@ import { getServiceAccountToken, refreshTokenIfNeeded } from '@/app/api/auth/oau

const logger = createLogger('VertexCredential')

export interface ResolveVertexCredentialParams {
credentialId: string
actingUserId: string | undefined
/** Workspace of the executing workflow. The credential must belong to it. */
workspaceId: string | null | undefined
callerLabel?: string
}

/**
* Resolves a Vertex AI OAuth credential to an access token.
* Shared across agent, evaluator, and router handlers. Authorizes the executing
* user against the credential first — workspace credentials are usable by their
* members and by derived workspace admins, matching `authorizeCredentialUse`.
* Shared across agent, evaluator, and router handlers. Enforces the same two
* predicates as `authorizeCredentialUse`: the executing user must be a member
* (or derived workspace admin) of the credential, and the credential must belong
* to the workspace the workflow is executing in.
*/
export async function resolveVertexCredential(
credentialId: string,
actingUserId: string | undefined,
callerLabel = 'vertex'
): Promise<string> {
export async function resolveVertexCredential({
credentialId,
actingUserId,
workspaceId,
callerLabel = 'vertex',
}: ResolveVertexCredentialParams): Promise<string> {
const requestId = `${callerLabel}-${Date.now()}`

logger.info(`[${requestId}] Resolving Vertex AI credential: ${credentialId}`)
Expand All @@ -31,6 +41,13 @@ export async function resolveVertexCredential(
if (!cred) {
throw new Error(`Vertex AI credential not found: ${credentialId}`)
}
if (workspaceId && cred.workspaceId !== workspaceId) {
logger.warn(`[${requestId}] Vertex AI credential belongs to a different workspace`, {
credentialId,
executingWorkspaceId: workspaceId,
})
throw new Error('Credential is not accessible from this workflow workspace')
}
if (!access.hasWorkspaceAccess || (!access.member && !access.isAdmin)) {
throw new Error('Not authorized to use this Vertex AI credential')
}
Expand Down
100 changes: 100 additions & 0 deletions apps/sim/lib/core/security/input-validation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import {
validateExternalUrl,
validateFileExtension,
validateGoogleCalendarId,
validateGoogleCloudLocation,
validateGoogleCloudProject,
validateHostname,
validateImageUrl,
validateInteger,
Expand Down Expand Up @@ -1458,6 +1460,19 @@ describe('validateAwsRegion', () => {
})
})

describe('valid European Sovereign Cloud regions', () => {
it.concurrent('should accept eusc-de-east-1', () => {
const result = validateAwsRegion('eusc-de-east-1')
expect(result.isValid).toBe(true)
expect(result.sanitized).toBe('eusc-de-east-1')
})

it.concurrent('should reject a malformed eusc region', () => {
expect(validateAwsRegion('eusc-de-east').isValid).toBe(false)
expect(validateAwsRegion('eusc-deu-east-1').isValid).toBe(false)
})
})

describe('valid China regions', () => {
it.concurrent('should accept cn-north-1', () => {
const result = validateAwsRegion('cn-north-1')
Expand Down Expand Up @@ -1561,6 +1576,91 @@ describe('validateAwsRegion', () => {
})
})

describe('validateGoogleCloudLocation', () => {
describe('valid locations', () => {
it.concurrent.each([
'us-central1',
'us-east5',
'europe-west4',
'northamerica-northeast1',
'southamerica-east1',
'asia-northeast3',
'australia-southeast2',
'africa-south1',
'me-central2',
'global',
])('should accept %s', (location) => {
const result = validateGoogleCloudLocation(location)
expect(result.isValid).toBe(true)
expect(result.sanitized).toBe(location)
})
})

describe('hostname injection', () => {
it.concurrent.each([
'attacker.example.com/x',
'us-central1/../attacker.tld',
'us-central1:8080',
'user@attacker.tld',
'us-central1?a=b',
'us-central1#frag',
'us central1',
'us-central1\n',
'US-CENTRAL1',
'../us-central1',
])('should reject %j', (location) => {
const result = validateGoogleCloudLocation(location)
expect(result.isValid).toBe(false)
})
})

it.concurrent('should reject empty and missing values', () => {
expect(validateGoogleCloudLocation('').isValid).toBe(false)
expect(validateGoogleCloudLocation(null).isValid).toBe(false)
expect(validateGoogleCloudLocation(undefined).isValid).toBe(false)
})

it.concurrent('should name the parameter in the error', () => {
const result = validateGoogleCloudLocation('bad host', 'vertexLocation')
expect(result.error).toContain('vertexLocation')
})
})

describe('validateGoogleCloudProject', () => {
describe('valid projects', () => {
it.concurrent.each(['my-project', 'sim-prod-1', 'abcdef', '123456789012'])(
'should accept %s',
(project) => {
const result = validateGoogleCloudProject(project)
expect(result.isValid).toBe(true)
expect(result.sanitized).toBe(project)
}
)
})

describe('path injection and malformed ids', () => {
it.concurrent.each([
'my-project/../../other',
'my-project:alias',
'my project',
'My-Project',
'1project',
'my-project-',
'abc',
'a'.repeat(31),
])('should reject %j', (project) => {
const result = validateGoogleCloudProject(project)
expect(result.isValid).toBe(false)
})
})

it.concurrent('should reject empty and missing values', () => {
expect(validateGoogleCloudProject('').isValid).toBe(false)
expect(validateGoogleCloudProject(null).isValid).toBe(false)
expect(validateGoogleCloudProject(undefined).isValid).toBe(false)
})
})

describe('validateS3BucketName', () => {
describe('valid bucket names', () => {
it.concurrent('should accept simple bucket name', () => {
Expand Down
Loading
Loading