mirror of
https://github.com/simstudioai/sim.git
synced 2026-09-24 15:45:35 +08:00
fix(security): enforce workspace scope on workflow middleware and validate shopify shop domain (#4535)
* fix(security): enforce workspace scope on workflow middleware and validate shopify shop domain
- validateWorkflowAccess now rejects workspace-scoped API keys whose
workspaceId doesn't match the workflow's workspace, closing a boundary
leak across /api/workflows/[id]/{log,paused,status} and
/api/resume/[workflowId]/[executionId]/[contextId]
- shopify authorize route now validates the resolved shop domain against
shopifyShopDomainSchema before proceeding
- adds middleware tests covering workspace/personal/session auth paths
* fix(shopify): disallow trailing hyphen in shop subdomain regex
* fix(shopify): align shop domain regex with shopify handle rules (3-60 lowercase, no edge hyphens)
* fix(security): widen shopify subdomain regex to allow up to 63 chars
Shopify and RFC 1123 allow labels up to 63 chars; the previous
{1,58} quantifier capped the subdomain at 60 chars and rejected
valid 61–63 char shops with a 400.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.7
parent
773720ec7f
commit
4cc00b2ef9
@@ -1,7 +1,10 @@
|
||||
import { createLogger } from '@sim/logger'
|
||||
import { generateId } from '@sim/utils/id'
|
||||
import { type NextRequest, NextResponse } from 'next/server'
|
||||
import { shopifyAuthorizeQuerySchema } from '@/lib/api/contracts/oauth-connections'
|
||||
import {
|
||||
shopifyAuthorizeQuerySchema,
|
||||
shopifyShopDomainSchema,
|
||||
} from '@/lib/api/contracts/oauth-connections'
|
||||
import { getSession } from '@/lib/auth'
|
||||
import { env } from '@/lib/core/config/env'
|
||||
import { getBaseUrl } from '@/lib/core/utils/urls'
|
||||
@@ -161,6 +164,11 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
|
||||
cleanShop = `${cleanShop.replace('.myshopify.com', '')}.myshopify.com`
|
||||
}
|
||||
|
||||
if (!shopifyShopDomainSchema.safeParse(cleanShop).success) {
|
||||
logger.warn('Rejected invalid Shopify shop domain', { shop: shopDomain })
|
||||
return NextResponse.json({ error: 'Invalid Shopify shop domain' }, { status: 400 })
|
||||
}
|
||||
|
||||
const baseUrl = getBaseUrl()
|
||||
const redirectUri = `${baseUrl}/api/auth/oauth2/callback/shopify`
|
||||
|
||||
|
||||
@@ -0,0 +1,130 @@
|
||||
/**
|
||||
* Tests for workflow access middleware — focused on the workspace-scoped
|
||||
* API key boundary check in the `requireDeployment=false` branch.
|
||||
*
|
||||
* @vitest-environment node
|
||||
*/
|
||||
|
||||
import {
|
||||
hybridAuthMockFns,
|
||||
workflowAuthzMock,
|
||||
workflowAuthzMockFns,
|
||||
workflowsUtilsMock,
|
||||
workflowsUtilsMockFns,
|
||||
} from '@sim/testing'
|
||||
import { NextRequest } from 'next/server'
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
vi.mock('@/lib/workflows/utils', () => workflowsUtilsMock)
|
||||
vi.mock('@sim/workflow-authz', () => workflowAuthzMock)
|
||||
vi.mock('@/lib/api-key/service', () => ({
|
||||
authenticateApiKeyFromHeader: vi.fn(),
|
||||
updateApiKeyLastUsed: vi.fn(),
|
||||
}))
|
||||
|
||||
import { validateWorkflowAccess } from '@/app/api/workflows/middleware'
|
||||
|
||||
function makeRequest() {
|
||||
return new NextRequest(new URL('https://example.com/api/workflows/wf-1/log'))
|
||||
}
|
||||
|
||||
describe('validateWorkflowAccess (requireDeployment=false)', () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
workflowsUtilsMockFns.mockGetWorkflowById.mockResolvedValue({
|
||||
id: 'wf-1',
|
||||
workspaceId: 'ws-A',
|
||||
isDeployed: true,
|
||||
})
|
||||
workflowAuthzMockFns.mockAuthorizeWorkflowByWorkspacePermission.mockResolvedValue({
|
||||
allowed: true,
|
||||
status: 200,
|
||||
workflow: { id: 'wf-1', workspaceId: 'ws-A' },
|
||||
})
|
||||
})
|
||||
|
||||
it('rejects a workspace-scoped API key issued for a different workspace', async () => {
|
||||
hybridAuthMockFns.mockCheckHybridAuth.mockResolvedValueOnce({
|
||||
success: true,
|
||||
userId: 'user-1',
|
||||
authType: 'api_key',
|
||||
apiKeyType: 'workspace',
|
||||
workspaceId: 'ws-B',
|
||||
})
|
||||
|
||||
const result = await validateWorkflowAccess(makeRequest(), 'wf-1', false)
|
||||
|
||||
expect(result.error).toEqual({
|
||||
message: 'API key is not authorized for this workspace',
|
||||
status: 403,
|
||||
})
|
||||
expect(workflowAuthzMockFns.mockAuthorizeWorkflowByWorkspacePermission).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('allows a workspace-scoped API key issued for the matching workspace', async () => {
|
||||
hybridAuthMockFns.mockCheckHybridAuth.mockResolvedValueOnce({
|
||||
success: true,
|
||||
userId: 'user-1',
|
||||
authType: 'api_key',
|
||||
apiKeyType: 'workspace',
|
||||
workspaceId: 'ws-A',
|
||||
})
|
||||
|
||||
const result = await validateWorkflowAccess(makeRequest(), 'wf-1', false)
|
||||
|
||||
expect(result.error).toBeUndefined()
|
||||
expect(result.workflow).toBeDefined()
|
||||
expect(result.auth?.workspaceId).toBe('ws-A')
|
||||
expect(workflowAuthzMockFns.mockAuthorizeWorkflowByWorkspacePermission).toHaveBeenCalledWith({
|
||||
workflowId: 'wf-1',
|
||||
userId: 'user-1',
|
||||
action: 'read',
|
||||
})
|
||||
})
|
||||
|
||||
it('allows a personal API key regardless of workspaceId on the auth result', async () => {
|
||||
hybridAuthMockFns.mockCheckHybridAuth.mockResolvedValueOnce({
|
||||
success: true,
|
||||
userId: 'user-1',
|
||||
authType: 'api_key',
|
||||
apiKeyType: 'personal',
|
||||
workspaceId: 'ws-B',
|
||||
})
|
||||
|
||||
const result = await validateWorkflowAccess(makeRequest(), 'wf-1', false)
|
||||
|
||||
expect(result.error).toBeUndefined()
|
||||
expect(result.workflow).toBeDefined()
|
||||
})
|
||||
|
||||
it('allows session auth (no apiKeyType) when workspace permission grants access', async () => {
|
||||
hybridAuthMockFns.mockCheckHybridAuth.mockResolvedValueOnce({
|
||||
success: true,
|
||||
userId: 'user-1',
|
||||
authType: 'session',
|
||||
})
|
||||
|
||||
const result = await validateWorkflowAccess(makeRequest(), 'wf-1', false)
|
||||
|
||||
expect(result.error).toBeUndefined()
|
||||
expect(result.workflow).toBeDefined()
|
||||
})
|
||||
|
||||
it('still enforces workspace-permission rejection for personal keys', async () => {
|
||||
hybridAuthMockFns.mockCheckHybridAuth.mockResolvedValueOnce({
|
||||
success: true,
|
||||
userId: 'user-1',
|
||||
authType: 'api_key',
|
||||
apiKeyType: 'personal',
|
||||
})
|
||||
workflowAuthzMockFns.mockAuthorizeWorkflowByWorkspacePermission.mockResolvedValueOnce({
|
||||
allowed: false,
|
||||
status: 403,
|
||||
message: 'Access denied',
|
||||
})
|
||||
|
||||
const result = await validateWorkflowAccess(makeRequest(), 'wf-1', false)
|
||||
|
||||
expect(result.error).toEqual({ message: 'Access denied', status: 403 })
|
||||
})
|
||||
})
|
||||
@@ -54,6 +54,15 @@ export async function validateWorkflowAccess(
|
||||
}
|
||||
}
|
||||
|
||||
if (auth.apiKeyType === 'workspace' && auth.workspaceId !== workflow.workspaceId) {
|
||||
return {
|
||||
error: {
|
||||
message: 'API key is not authorized for this workspace',
|
||||
status: 403,
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
const authorization = await authorizeWorkflowByWorkspacePermission({
|
||||
workflowId,
|
||||
userId: auth.userId,
|
||||
|
||||
@@ -143,7 +143,7 @@ export const oauthAuthorizeParamsResponseSchema = z.object({
|
||||
response_type: z.literal('code'),
|
||||
})
|
||||
|
||||
const SHOPIFY_SHOP_DOMAIN_REGEX = /^[a-zA-Z0-9][a-zA-Z0-9-]*\.myshopify\.com$/
|
||||
const SHOPIFY_SHOP_DOMAIN_REGEX = /^[a-z0-9][a-z0-9-]{1,61}[a-z0-9]\.myshopify\.com$/
|
||||
export const shopifyShopDomainSchema = z.string().regex(SHOPIFY_SHOP_DOMAIN_REGEX)
|
||||
|
||||
export const listOAuthConnectionsContract = defineRouteContract({
|
||||
|
||||
Reference in New Issue
Block a user