Skip to content

Commit c2317eb

Browse files
committed
fix(interfaces): re-assert same-workspace on the authenticated form submit
The public submit route re-checks that the resolved workflow still belongs to the interface's workspace, with a comment explaining why it is not redundant: validateLayout grandfathers references that were already stored, so a stored workflowId is only proven in-workspace at the moment it was introduced. The authenticated route skipped that check and executed against workflowRecord.workspaceId — the workflow's workspace, not the interface's. It also grants at level 'read', so it was the lower-privilege of the two paths reaching the same executor. Adds the guard, releasing the reserved billing slot on the early exit exactly as the sibling branch above it does. The test is mutation-verified: without the guard it returns 200 instead of 404.
1 parent 3266185 commit c2317eb

2 files changed

Lines changed: 46 additions & 0 deletions

File tree

apps/sim/app/api/interfaces/[interfaceId]/modules/[moduleId]/submit/route.test.ts

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -211,6 +211,35 @@ describe('POST /api/interfaces/[interfaceId]/modules/[moduleId]/submit', () => {
211211
expect(executionPreprocessingMockFns.mockPreprocessExecution).not.toHaveBeenCalled()
212212
})
213213

214+
it("refuses a workflow that has left the interface's workspace, and frees the billing slot", async () => {
215+
/**
216+
* `validateLayout` grandfathers references already stored, so a stored
217+
* workflowId is only proven in-workspace at the moment it was introduced.
218+
* The public twin re-asserts this; so must the authenticated route, which
219+
* grants at `level: 'read'` and is therefore the lower-privilege of the two
220+
* paths reaching the same executor.
221+
*/
222+
executionPreprocessingMockFns.mockPreprocessExecution.mockResolvedValue({
223+
success: true,
224+
actorUserId: 'actor-1',
225+
billingAttribution: BILLING_ATTRIBUTION,
226+
workflowRecord: {
227+
id: 'wf-1',
228+
userId: 'owner-1',
229+
workspaceId: 'ws-somewhere-else',
230+
isDeployed: true,
231+
variables: {},
232+
},
233+
})
234+
235+
const response = await callPost(validBody)
236+
237+
expect(response.status).toBe(404)
238+
expect(mockExecuteWorkflow).not.toHaveBeenCalled()
239+
// The reserved slot must be released, or the workspace leaks concurrency.
240+
expect(mockReleaseExecutionSlot).toHaveBeenCalledTimes(1)
241+
})
242+
214243
it('returns 404 when the interface belongs to another workspace', async () => {
215244
mockGetInterfaceById.mockResolvedValue({
216245
...buildDefinition(),

apps/sim/app/api/interfaces/[interfaceId]/modules/[moduleId]/submit/route.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,23 @@ export const POST = withRouteHandler(
114114
return NextResponse.json({ error: 'Workflow has no associated workspace' }, { status: 500 })
115115
}
116116

117+
/**
118+
* Re-assert same-workspace on the resolved workflow, exactly as the public
119+
* twin does. `validateLayout` grandfathers references that were already
120+
* stored, so a stored reference is only proven in-workspace at the moment
121+
* it was introduced — and this route grants at `level: 'read'`, so it is
122+
* the lower-privilege of the two paths that reach the same executor.
123+
*
124+
* Same slot accounting as the branch above: release the reserved billing
125+
* slot on this early exit, since no LoggingSession will finalize to free
126+
* it.
127+
*/
128+
if (workflowRecord.workspaceId !== access.definition.workspaceId) {
129+
logger.warn(`[${requestId}] Interface form workflow left the workspace`, { moduleId })
130+
await releaseExecutionSlot(executionId)
131+
return NextResponse.json({ error: 'Module is not available' }, { status: 404 })
132+
}
133+
117134
const result = await executeWorkflow(
118135
{
119136
id: workflowRecord.id,

0 commit comments

Comments
 (0)