Skip to content

Commit 5f67bf1

Browse files
committed
fix(workflows): lint only the JSON fields the operation sends, and lint deploys as the acting user
1 parent 0e06cd9 commit 5f67bf1

4 files changed

Lines changed: 44 additions & 14 deletions

File tree

‎apps/sim/lib/workflows/application/deployments.ts‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
import { AuditAction, AuditResourceType } from '@sim/audit'
2-
import { resolvePrincipalAttribution, toPrincipalActor } from '@sim/auth/principal'
2+
import {
3+
resolvePrincipalAttribution,
4+
resolvePrincipalSubjectUserId,
5+
toPrincipalActor,
6+
} from '@sim/auth/principal'
37
import { createLogger } from '@sim/logger'
48
import { assertWorkflowMutable, WorkflowLockedError } from '@sim/platform-authz/workflow'
59
import { getErrorMessage } from '@sim/utils/errors'
@@ -108,7 +112,7 @@ async function requireMutableWorkflow(workflowId: string): Promise<void> {
108112
async function deployedVersionLintWarning(
109113
context: ActiveWorkflowApplicationContext,
110114
deploymentVersionId: string | undefined,
111-
subjectUserId: string
115+
subjectUserId: string | null
112116
): Promise<string | undefined> {
113117
if (!deploymentVersionId) return undefined
114118
try {
@@ -156,7 +160,7 @@ export const deployWorkflow = defineAuthorizedWorkflowUseCase({
156160
const lintWarning = await deployedVersionLintWarning(
157161
context,
158162
result.deploymentVersionId,
159-
attribution.attributedUserId
163+
resolvePrincipalSubjectUserId(principal) ?? null
160164
)
161165
return {
162166
...result,

‎apps/sim/lib/workflows/application/workflow-deployments.test.ts‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -341,7 +341,7 @@ describe('workflow deployment application use cases', () => {
341341
],
342342
}
343343

344-
it('reports them as a deploy warning, linted as the deploying user', async () => {
344+
it('reports them as a deploy warning, linted as the acting user', async () => {
345345
mockDeploy.mockResolvedValueOnce({
346346
success: true,
347347
version: 4,
@@ -350,17 +350,16 @@ describe('workflow deployment application use cases', () => {
350350
'Deployment activation completed, and post-activation notifications are queued.',
351351
],
352352
})
353-
mocks.buildWorkflowLintReport.mockResolvedValueOnce(unquotedRowJson)
353+
mocks.buildWorkflowLintReport.mockImplementation(
354+
async (_graph: unknown, scope: { subjectUserId: string | null }) =>
355+
scope.subjectUserId === 'session-user' ? unquotedRowJson : cleanLint
356+
)
354357

355358
const result = await deployWorkflow.execute({
356359
principal: createSessionPrincipal({ userId: 'session-user' }),
357360
input: { workflowId: 'workflow-1', requestId: 'request-10' },
358361
})
359362

360-
expect(mocks.buildWorkflowLintReport).toHaveBeenCalledWith(
361-
{ blocks: {}, edges: [] },
362-
expect.objectContaining({ workflowId: 'workflow-1', subjectUserId: 'session-user' })
363-
)
364363
expect(result.warnings).toHaveLength(2)
365364
expect(result.warnings?.[1]).toContain('"Insert Order".data <start.order_id>')
366365
})

‎apps/sim/lib/workflows/editing/json-literal-refs.test.ts‎

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,12 @@ const MOCK_BLOCKS = vi.hoisted(
3131
category: 'blocks',
3232
subBlocks: [
3333
{ id: 'operation', type: 'dropdown' },
34-
{ id: 'data', type: 'code', language: 'json' },
34+
{
35+
id: 'data',
36+
type: 'code',
37+
language: 'json',
38+
condition: { field: 'operation', value: ['insert_row', 'update_row'] },
39+
},
3540
{ id: 'rows', type: 'code', language: 'json' },
3641
],
3742
outputs: {},
@@ -87,8 +92,12 @@ function graph(
8792
return { blocks } as Parameters<typeof collectUnquotedJsonStringReferences>[0]
8893
}
8994

90-
function insertRow(data: unknown) {
91-
return { type: 'table_v2', name: 'Insert Order', subBlocks: { data: { value: data } } }
95+
function insertRow(data: unknown, operation = 'insert_row') {
96+
return {
97+
type: 'table_v2',
98+
name: 'Insert Order',
99+
subBlocks: { operation: { value: operation }, data: { value: data } },
100+
}
92101
}
93102

94103
describe('collectUnquotedJsonStringReferences', () => {
@@ -196,4 +205,14 @@ describe('collectUnquotedJsonStringReferences', () => {
196205
)
197206
expect(findings).toHaveLength(0)
198207
})
208+
209+
it('ignores a JSON field the selected operation does not send', () => {
210+
const findings = collectUnquotedJsonStringReferences(
211+
graph({
212+
start: START,
213+
insert: insertRow('{"order_id": <start.order_id>}', 'get_schema'),
214+
})
215+
)
216+
expect(findings).toHaveLength(0)
217+
})
199218
})

‎apps/sim/lib/workflows/editing/lint.ts‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -798,8 +798,16 @@ export function collectUnquotedJsonStringReferences(
798798
const jsonFields = (block.type ? getBlock(block.type)?.subBlocks : undefined)?.filter(
799799
(subBlock) => subBlock.type === 'code' && subBlock.language === 'json'
800800
)
801-
for (const { id: field } of jsonFields ?? []) {
802-
const json = block.subBlocks?.[field]?.value
801+
if (!jsonFields?.length) continue
802+
/** Only what the serializer sends: a field the selected operation drops never runs. */
803+
let params: Record<string, unknown>
804+
try {
805+
params = extractBlockParams(block as Parameters<typeof extractBlockParams>[0])
806+
} catch {
807+
continue
808+
}
809+
for (const field of new Set(jsonFields.map((subBlock) => subBlock.id))) {
810+
const json = params[field]
803811
if (typeof json !== 'string') continue
804812
const unquoted = new Set<string>()
805813
for (const token of unquotedJsonReferenceTokens(json)) {

0 commit comments

Comments
 (0)