From a29cabc2946fcf758b1cc5c2e143bd3d779c620c Mon Sep 17 00:00:00 2001 From: "n8n-assistant[bot]" <100856346+n8n-assistant[bot]@users.noreply.github.com> Date: Fri, 14 Aug 2026 09:09:26 +0000 Subject: [PATCH] feat(core): Skip update approval for workflows created in the same Instance AI session (backport to release-candidate/2.35.x) (#36106) Co-authored-by: Riqwan Thamir --- packages/@n8n/api-types/src/index.ts | 1 + .../__tests__/instance-ai.schema.test.ts | 7 + .../src/schemas/instance-ai.schema.ts | 19 ++- .../tools/__tests__/workflows.tool.test.ts | 138 ++++++++++++++++++ .../instance-ai/src/tools/workflows.tool.ts | 37 ++++- .../__tests__/build-workflow.tool.test.ts | 75 ++++++++++ .../tools/workflows/build-workflow.tool.ts | 21 ++- .../tools/workflows/workflow-build-context.ts | 38 +++++ packages/@n8n/instance-ai/src/types.ts | 14 +- .../InstanceAiConfirmationPanel.test.ts | 82 ++++++++++- .../instanceAi.threadRuntime.test.ts | 82 +++++++++++ .../InstanceAiConfirmationPanel.vue | 20 ++- .../ai/instanceAi/instanceAi.threadRuntime.ts | 72 +++++++-- 13 files changed, 565 insertions(+), 41 deletions(-) diff --git a/packages/@n8n/api-types/src/index.ts b/packages/@n8n/api-types/src/index.ts index f01ee51de3d..9e13b7812f0 100644 --- a/packages/@n8n/api-types/src/index.ts +++ b/packages/@n8n/api-types/src/index.ts @@ -286,6 +286,7 @@ export { export { buildRunWorkflowSessionGrantKey, + buildUpdateWorkflowSessionGrantKey, buildDataTablesSessionGrantKey, buildFetchUrlGrantKey, FETCH_URL_ALLOW_ALL_GRANT_KEY, diff --git a/packages/@n8n/api-types/src/schemas/__tests__/instance-ai.schema.test.ts b/packages/@n8n/api-types/src/schemas/__tests__/instance-ai.schema.test.ts index eab9e735f0c..2818dd58ecb 100644 --- a/packages/@n8n/api-types/src/schemas/__tests__/instance-ai.schema.test.ts +++ b/packages/@n8n/api-types/src/schemas/__tests__/instance-ai.schema.test.ts @@ -2,6 +2,7 @@ import { AI_GATEWAY_MANAGED_TAG, applyBranchReadOnlyOverrides, buildDataTablesSessionGrantKey, + buildUpdateWorkflowSessionGrantKey, buildFetchUrlGrantKey, DEFAULT_INSTANCE_AI_PERMISSIONS, errorPayloadSchema, @@ -439,6 +440,12 @@ describe('data-tables session grant keys', () => { }); }); +describe('workflow update session grant keys', () => { + it('builds per-workflow keys matching the frontend always-allow format', () => { + expect(buildUpdateWorkflowSessionGrantKey('wf-1')).toBe('workflows:update:wf-1'); + }); +}); + describe('domain-access grant keys', () => { it('builds and parses per-host grant keys round-trip', () => { const key = buildFetchUrlGrantKey('example.com'); diff --git a/packages/@n8n/api-types/src/schemas/instance-ai.schema.ts b/packages/@n8n/api-types/src/schemas/instance-ai.schema.ts index 6ea583cf755..16e798d9c89 100644 --- a/packages/@n8n/api-types/src/schemas/instance-ai.schema.ts +++ b/packages/@n8n/api-types/src/schemas/instance-ai.schema.ts @@ -52,6 +52,18 @@ export function buildRunWorkflowSessionGrantKey(workflowId: string): string { return `executions:run:${workflowId}`; } +/** + * Builds the thread-level grant key for updating a specific workflow without HITL. + * + * Written automatically when the agent creates a workflow in this thread, so follow-up + * edits to that same artifact (same run or later runs in the session) skip the update + * approval prompt. Foreign workflows still require approval unless the admin policy is + * `always_allow`. + */ +export function buildUpdateWorkflowSessionGrantKey(workflowId: string): string { + return `workflows:update:${workflowId}`; +} + /** * Builds the thread-level "always allow" grant key for a data-tables action * (e.g. `create`, `insert-rows`). Must match the frontend key @@ -608,7 +620,12 @@ export const confirmationRequestPayloadSchema = z.object({ .array(workflowSetupNodeSchema) .optional() .describe('Per-node setup cards for workflow credential/parameter configuration'), - workflowId: z.string().optional().describe('Workflow ID for setup-workflow tool'), + workflowId: z + .string() + .optional() + .describe( + 'Workflow ID for setup cards and per-workflow edit approvals (build-workflow / workflows update)', + ), resourceDecision: gatewayConfirmationRequiredPayloadSchema .optional() .describe('Gateway resource-access decision data (inputType=resource-decision)'), diff --git a/packages/@n8n/instance-ai/src/tools/__tests__/workflows.tool.test.ts b/packages/@n8n/instance-ai/src/tools/__tests__/workflows.tool.test.ts index 58b92c94c61..5f014b6b0b9 100644 --- a/packages/@n8n/instance-ai/src/tools/__tests__/workflows.tool.test.ts +++ b/packages/@n8n/instance-ai/src/tools/__tests__/workflows.tool.test.ts @@ -760,6 +760,144 @@ describe('workflows tool', () => { }); }); + describe('update action', () => { + const workflowPayload = { name: 'Updated WF', nodes: [], connections: {} }; + + it('should suspend for approval before updating a foreign workflow', async () => { + const context = createMockContext({ + permissions: { updateWorkflow: 'require_approval' }, + }); + (context.workflowService.get as Mock).mockResolvedValue({ + id: 'wf1', + name: 'Foreign WF', + }); + const suspend = vi.fn(); + + await executeTool( + createWorkflowsTool(context, 'full'), + { action: 'update', workflowId: 'wf1', workflow: workflowPayload }, + { suspend } as never, + ); + + expect(suspend).toHaveBeenCalledWith( + expect.objectContaining({ + message: 'Update workflow "Foreign WF" (ID: wf1)?', + severity: 'warning', + workflowId: 'wf1', + }), + ); + expect(context.workflowService.updateFromWorkflowJSON).not.toHaveBeenCalled(); + }); + + it('should update without approval when the workflow was created in this run', async () => { + const context = createMockContext({ + permissions: { updateWorkflow: 'require_approval' }, + aiCreatedWorkflowIds: new Set(['wf1']), + }); + (context.workflowService.updateFromWorkflowJSON as Mock).mockResolvedValue({ + id: 'wf1', + versionId: 'v2', + }); + const suspend = vi.fn(); + + const result = await executeTool( + createWorkflowsTool(context, 'full'), + { action: 'update', workflowId: 'wf1', workflow: workflowPayload }, + { suspend } as never, + ); + + expect(result).toEqual({ success: true, workflowId: 'wf1' }); + expect(suspend).not.toHaveBeenCalled(); + expect(context.workflowService.updateFromWorkflowJSON).toHaveBeenCalledWith( + 'wf1', + workflowPayload, + ); + }); + + it('should update without approval when the workflow has a session ownership grant', async () => { + const context = createMockContext({ + permissions: { updateWorkflow: 'require_approval' }, + sessionApprovedToolKeys: new Set(['workflows:update:wf1']), + }); + (context.workflowService.updateFromWorkflowJSON as Mock).mockResolvedValue({ + id: 'wf1', + versionId: 'v2', + }); + const suspend = vi.fn(); + + const result = await executeTool( + createWorkflowsTool(context, 'full'), + { action: 'update', workflowId: 'wf1', workflow: workflowPayload }, + { suspend } as never, + ); + + expect(result).toEqual({ success: true, workflowId: 'wf1' }); + expect(suspend).not.toHaveBeenCalled(); + expect(context.workflowService.updateFromWorkflowJSON).toHaveBeenCalled(); + }); + + it('should still block updates when admin policy denies them for owned workflows', async () => { + const context = createMockContext({ + permissions: { updateWorkflow: 'blocked' }, + aiCreatedWorkflowIds: new Set(['wf1']), + }); + + const result = await executeTool( + createWorkflowsTool(context, 'full'), + { action: 'update', workflowId: 'wf1', workflow: workflowPayload }, + {} as never, + ); + + expect(result).toEqual({ + success: false, + denied: true, + reason: 'Action blocked by admin', + }); + expect(context.workflowService.updateFromWorkflowJSON).not.toHaveBeenCalled(); + }); + + it('should persist a session update grant when resumed with scope=session', async () => { + const grantSessionToolApproval = vi.fn().mockResolvedValue(undefined); + const context = createMockContext({ + permissions: { updateWorkflow: 'require_approval' }, + grantSessionToolApproval, + }); + (context.workflowService.updateFromWorkflowJSON as Mock).mockResolvedValue({ + id: 'wf1', + versionId: 'v2', + }); + + const result = await executeTool( + createWorkflowsTool(context, 'full'), + { action: 'update', workflowId: 'wf1', workflow: workflowPayload }, + { resumeData: { approved: true, scope: 'session' } } as never, + ); + + expect(result).toEqual({ success: true, workflowId: 'wf1' }); + expect(grantSessionToolApproval).toHaveBeenCalledWith('workflows:update:wf1'); + }); + + it('should not persist a grant for a one-time update approval', async () => { + const grantSessionToolApproval = vi.fn().mockResolvedValue(undefined); + const context = createMockContext({ + permissions: { updateWorkflow: 'require_approval' }, + grantSessionToolApproval, + }); + (context.workflowService.updateFromWorkflowJSON as Mock).mockResolvedValue({ + id: 'wf1', + versionId: 'v2', + }); + + await executeTool( + createWorkflowsTool(context, 'full'), + { action: 'update', workflowId: 'wf1', workflow: workflowPayload }, + { resumeData: { approved: true } } as never, + ); + + expect(grantSessionToolApproval).not.toHaveBeenCalled(); + }); + }); + describe('delete action', () => { it('should return denied when permission is blocked', async () => { const context = createMockContext({ diff --git a/packages/@n8n/instance-ai/src/tools/workflows.tool.ts b/packages/@n8n/instance-ai/src/tools/workflows.tool.ts index 4659289118c..97eb588a208 100644 --- a/packages/@n8n/instance-ai/src/tools/workflows.tool.ts +++ b/packages/@n8n/instance-ai/src/tools/workflows.tool.ts @@ -31,6 +31,10 @@ import { summarizeWorkflowStructure, } from './workflows/summarize-workflow'; import { validateWorkflowConfig } from './workflows/validate-workflow.service'; +import { + grantSessionWorkflowUpdate, + canSkipWorkflowUpdateHitl, +} from './workflows/workflow-build-context'; import { refreshWorkflowSourceFileBindingFromWorkflow } from './workflows/workflow-file-bindings'; import { getReferencedWorkflowIds } from './workflows/workflow-json-utils'; @@ -205,16 +209,22 @@ const updateVersionAction = z.object({ // ── Suspend / resume schemas ──────────────────────────────────────────────── -const confirmationSuspendSchema = setupSuspendSchema.pick({ - requestId: true, - message: true, - severity: true, -}); +const confirmationSuspendSchema = setupSuspendSchema + .pick({ + requestId: true, + message: true, + severity: true, + workflowId: true, + }) + .partial({ workflowId: true }); const suspendSchema = z.union([setupSuspendSchema, confirmationSuspendSchema]); -// Resume: union of standard confirmation (approved) and setup-specific fields. -const resumeSchema = setupResumeSchema; +// Resume: setup-specific fields plus optional session scope for generic approvals +// (e.g. update "always allow" → persist `workflows:update:`). +const resumeSchema = setupResumeSchema.extend({ + scope: z.enum(['once', 'session']).optional(), +}); interface WorkflowToolContext { resumeData: z.infer | undefined; @@ -893,7 +903,10 @@ async function handleUpdate( return { success: false, denied: true, reason: 'Action blocked by admin' }; } - const needsApproval = context.permissions?.updateWorkflow !== 'always_allow'; + // Skip HITL for session-created or always-allowed workflows; others still need approval. + const needsApproval = + context.permissions?.updateWorkflow !== 'always_allow' && + !canSkipWorkflowUpdateHitl(context, input.workflowId); if (needsApproval && (resumeData === undefined || resumeData === null)) { const workflowName = await resolveWorkflowName(context, input.workflowId); @@ -901,6 +914,9 @@ async function handleUpdate( requestId: nanoid(), message: `Update workflow "${workflowName}" (ID: ${input.workflowId})?`, severity: 'warning' as const, + // Carried on the confirmation so the UI can scope "always allow" per workflow + // even if tool-call args are incomplete on resume. + workflowId: input.workflowId, }); } @@ -908,6 +924,11 @@ async function handleUpdate( return { success: false, denied: true, reason: 'User denied the action' }; } + // "Always allow" — persist so later edits of this workflow skip HITL. + if (resumeData?.approved && resumeData.scope === 'session') { + await grantSessionWorkflowUpdate(context, input.workflowId); + } + if (!isWorkflowJson(input.workflow)) { return { success: false, diff --git a/packages/@n8n/instance-ai/src/tools/workflows/__tests__/build-workflow.tool.test.ts b/packages/@n8n/instance-ai/src/tools/workflows/__tests__/build-workflow.tool.test.ts index 1731e13c086..df06cecd67d 100644 --- a/packages/@n8n/instance-ai/src/tools/workflows/__tests__/build-workflow.tool.test.ts +++ b/packages/@n8n/instance-ai/src/tools/workflows/__tests__/build-workflow.tool.test.ts @@ -488,9 +488,11 @@ describe('createBuildWorkflowTool', () => { }); it('updates a workflow created earlier in the run without requesting approval', async () => { + const grantSessionToolApproval = vi.fn().mockResolvedValue(undefined); const { context, filePath } = makeContext({ source: 'workflow source', overrides: { + grantSessionToolApproval, permissions: { createWorkflow: 'always_allow', updateWorkflow: 'require_approval', @@ -507,6 +509,31 @@ describe('createBuildWorkflowTool', () => { expect(created).toMatchObject({ success: true, workflowId: 'wf-1' }); expect(updated).toMatchObject({ success: true, workflowId: 'wf-1' }); expect(context.aiCreatedWorkflowIds).toEqual(new Set(['wf-1'])); + expect(grantSessionToolApproval).toHaveBeenCalledWith('workflows:update:wf-1'); + expect(suspend).not.toHaveBeenCalled(); + expect(context.workflowService.updateFromWorkflowJSON).toHaveBeenCalledTimes(1); + }); + + it('updates a workflow with a session ownership grant without requesting approval', async () => { + const { context, filePath } = makeContext({ + source: 'workflow source', + overrides: { + sessionApprovedToolKeys: new Set(['workflows:update:wf-session']), + permissions: { + createWorkflow: 'always_allow', + updateWorkflow: 'require_approval', + } as InstanceAiContext['permissions'], + }, + }); + const suspend = vi.fn(); + + const result = await executeTool( + createBuildWorkflowTool(context), + { filePath, workflowId: 'wf-session' }, + { suspend }, + ); + + expect(result).toMatchObject({ success: true, workflowId: 'wf-session' }); expect(suspend).not.toHaveBeenCalled(); expect(context.workflowService.updateFromWorkflowJSON).toHaveBeenCalledTimes(1); }); @@ -533,12 +560,60 @@ describe('createBuildWorkflowTool', () => { expect.objectContaining({ message: 'Edit Target workflow (ID: wf-existing)?', severity: 'warning', + workflowId: 'wf-existing', }), ); expect(compileWorkflowSource).not.toHaveBeenCalled(); expect(context.workflowService.updateFromWorkflowJSON).not.toHaveBeenCalled(); }); + it('persists a session update grant when edit approval resumes with scope=session', async () => { + const grantSessionToolApproval = vi.fn().mockResolvedValue(undefined); + const { context, filePath } = makeContext({ + source: 'workflow source', + overrides: { + grantSessionToolApproval, + permissions: { + createWorkflow: 'always_allow', + updateWorkflow: 'require_approval', + } as InstanceAiContext['permissions'], + }, + }); + + const result = await executeTool( + createBuildWorkflowTool(context), + { filePath, workflowId: 'wf-existing' }, + { resumeData: { approved: true, scope: 'session' } }, + ); + + expect(result).toMatchObject({ success: true, workflowId: 'wf-existing' }); + expect(grantSessionToolApproval).toHaveBeenCalledWith('workflows:update:wf-existing'); + expect(context.workflowService.updateFromWorkflowJSON).toHaveBeenCalledTimes(1); + }); + + it('does not persist a grant for a one-time edit approval', async () => { + const grantSessionToolApproval = vi.fn().mockResolvedValue(undefined); + const { context, filePath } = makeContext({ + source: 'workflow source', + overrides: { + grantSessionToolApproval, + permissions: { + createWorkflow: 'always_allow', + updateWorkflow: 'require_approval', + } as InstanceAiContext['permissions'], + }, + }); + + await executeTool( + createBuildWorkflowTool(context), + { filePath, workflowId: 'wf-existing' }, + { resumeData: { approved: true } }, + ); + + expect(grantSessionToolApproval).not.toHaveBeenCalled(); + expect(context.workflowService.updateFromWorkflowJSON).toHaveBeenCalledTimes(1); + }); + it('blocks updates to workflows created earlier in the run when admin policy denies them', async () => { const { context, filePath } = makeContext({ source: 'workflow source', diff --git a/packages/@n8n/instance-ai/src/tools/workflows/build-workflow.tool.ts b/packages/@n8n/instance-ai/src/tools/workflows/build-workflow.tool.ts index 2cc7f38e4c0..5c58fa45e47 100644 --- a/packages/@n8n/instance-ai/src/tools/workflows/build-workflow.tool.ts +++ b/packages/@n8n/instance-ai/src/tools/workflows/build-workflow.tool.ts @@ -20,8 +20,11 @@ import { combineWarnings, formatWarning, getBuildFailureTrackingKey, + grantSessionWorkflowUpdate, isApprovedBuildContext, + canSkipWorkflowUpdateHitl, markSourceBuildFailed, + recordSessionOwnedWorkflow, resolveBuildIdentifiers, resolveWorkflowName, sourceResponseBase, @@ -77,10 +80,14 @@ const confirmationSuspendSchema = z.object({ requestId: z.string(), message: z.string(), severity: instanceAiConfirmationSeveritySchema, + /** Resolved target workflow — used by the UI for per-workflow always-allow keys. */ + workflowId: z.string(), }); const confirmationResumeSchema = z.object({ approved: z.boolean(), + /** `'session'` — user chose "always allow"; persist a per-workflow update grant. */ + scope: z.enum(['once', 'session']).optional(), }); interface BuildCtx { @@ -421,13 +428,12 @@ export function createBuildWorkflowTool(context: InstanceAiContext) { }; } - const isOwnInFlightWorkflow = - targetWorkflowId !== undefined && - (context.aiCreatedWorkflowIds?.has(targetWorkflowId) ?? false); + const canSkipUpdateHitl = + targetWorkflowId !== undefined && canSkipWorkflowUpdateHitl(context, targetWorkflowId); if ( targetWorkflowId && - !isOwnInFlightWorkflow && + !canSkipUpdateHitl && !isApprovedBuildContext(context) && context.permissions?.updateWorkflow !== 'always_allow' ) { @@ -494,8 +500,13 @@ export function createBuildWorkflowTool(context: InstanceAiContext) { requestId: nanoid(), message: `Edit ${workflowName} (ID: ${targetWorkflowId})?`, severity: 'warning', + workflowId: targetWorkflowId, }); } + // "Always allow" — persist so later edits of this workflow skip HITL. + if (ctx.resumeData.approved && ctx.resumeData.scope === 'session') { + await grantSessionWorkflowUpdate(context, targetWorkflowId); + } } // Persist inline source first so the workspace file stays canonical for later repairs. @@ -968,7 +979,7 @@ export function createBuildWorkflowTool(context: InstanceAiContext) { ...(projectId ? { projectId } : {}), markAsAiTemporary: true, }); - (context.aiCreatedWorkflowIds ??= new Set()).add(created.id); + await recordSessionOwnedWorkflow(context, created.id); return await createSuccessResponse(created, 'create'); } catch (error) { const message = error instanceof Error ? error.message : 'Unknown error'; diff --git a/packages/@n8n/instance-ai/src/tools/workflows/workflow-build-context.ts b/packages/@n8n/instance-ai/src/tools/workflows/workflow-build-context.ts index 5b2ed2c075b..2ea35415ff5 100644 --- a/packages/@n8n/instance-ai/src/tools/workflows/workflow-build-context.ts +++ b/packages/@n8n/instance-ai/src/tools/workflows/workflow-build-context.ts @@ -1,3 +1,4 @@ +import { buildUpdateWorkflowSessionGrantKey } from '@n8n/api-types'; import { nanoid } from 'nanoid'; import { @@ -12,6 +13,43 @@ export function isApprovedBuildContext(context: InstanceAiContext): boolean { return Boolean(buildContext?.plannedTaskService ?? buildContext?.allowPostPlanWorkflowCreate); } +/** + * True when update HITL can be skipped for this workflow in this session: + * created earlier in the current run (`aiCreatedWorkflowIds`), or covered by a + * `workflows:update:` thread grant (written on create, or when the user + * chose "always allow" for an edit — including foreign workflows). Untrusted + * workflows without a grant still require approval. + */ +export function canSkipWorkflowUpdateHitl(context: InstanceAiContext, workflowId: string): boolean { + if (context.aiCreatedWorkflowIds?.has(workflowId) === true) return true; + const grantKey = buildUpdateWorkflowSessionGrantKey(workflowId); + return context.sessionApprovedToolKeys?.has(grantKey) === true; +} + +/** + * Persist a thread grant so later update HITL for this workflow is skipped + * (same run and later runs in the thread). Used when the user chooses + * "always allow" on an edit confirmation. + */ +export async function grantSessionWorkflowUpdate( + context: InstanceAiContext, + workflowId: string, +): Promise { + await context.grantSessionToolApproval?.(buildUpdateWorkflowSessionGrantKey(workflowId)); +} + +/** + * Mark a newly created workflow as owned by this session: in-memory for the + * current run, and as a persisted thread grant so later runs skip update HITL. + */ +export async function recordSessionOwnedWorkflow( + context: InstanceAiContext, + workflowId: string, +): Promise { + (context.aiCreatedWorkflowIds ??= new Set()).add(workflowId); + await grantSessionWorkflowUpdate(context, workflowId); +} + export async function resolveWorkflowName( context: InstanceAiContext, workflowId: string, diff --git a/packages/@n8n/instance-ai/src/types.ts b/packages/@n8n/instance-ai/src/types.ts index f8a2dce030b..9f3ed079ff4 100644 --- a/packages/@n8n/instance-ai/src/types.ts +++ b/packages/@n8n/instance-ai/src/types.ts @@ -1111,14 +1111,12 @@ export interface InstanceAiContext { /** Records workflow code snapshots for the run debug buffer (dev tooling). */ recordWorkflowCodeSnapshot?: (snapshot: WorkflowCodeSnapshotInput) => void; /** - * IDs of workflows the agent created during the **currently active plan - * cycle**. Populated by build-workflow on every successful create, and - * hydrated at run start from the persisted plan graph when — and only when — - * the plan is still `active` or - * `awaiting_replan`, so replan follow-up runs keep the bypass active but - * the window closes as soon as the plan settles. Consumed by the delete - * handler to skip the confirmation gate when the agent cleans up its own - * in-flight artifacts. Lazily initialized on first create. + * IDs of workflows the agent created during the **current run**. Populated by + * build-workflow on every successful create (via `recordSessionOwnedWorkflow`). + * Same-run update HITL bypasses consult this set. Cross-run bypass for + * the same thread uses the persisted `workflows:update:` session grant + * written at create time — this in-memory set alone does not survive a new run. + * Lazily initialized on first create. */ aiCreatedWorkflowIds?: Set; /** diff --git a/packages/frontend/editor-ui/src/features/ai/instanceAi/__tests__/InstanceAiConfirmationPanel.test.ts b/packages/frontend/editor-ui/src/features/ai/instanceAi/__tests__/InstanceAiConfirmationPanel.test.ts index 63c8b67b4c6..3cd1d7d4623 100644 --- a/packages/frontend/editor-ui/src/features/ai/instanceAi/__tests__/InstanceAiConfirmationPanel.test.ts +++ b/packages/frontend/editor-ui/src/features/ai/instanceAi/__tests__/InstanceAiConfirmationPanel.test.ts @@ -101,10 +101,11 @@ const renderComponent = createThreadComponentRenderer(InstanceAiConfirmationPane function makeToolCall( confirmation: InstanceAiConfirmation, args: Record = {}, + toolName = 'test-tool', ): InstanceAiToolCallState & { confirmation: InstanceAiConfirmation } { return { toolCallId: `tc-${confirmation.requestId}`, - toolName: 'test-tool', + toolName, args, isLoading: true, confirmation, @@ -129,8 +130,9 @@ function injectPendingConfirmation( thread: ThreadRuntime, confirmation: InstanceAiConfirmation, args: Record = {}, + toolName = 'test-tool', ) { - const tc = makeToolCall(confirmation, args); + const tc = makeToolCall(confirmation, args, toolName); const agentNode = makeAgentNode([tc]); thread.messages.push({ id: 'msg-1', @@ -256,7 +258,7 @@ describe('InstanceAiConfirmationPanel telemetry', () => { approved: true, scope: 'session', }); - expect(addKeySpy).toHaveBeenCalledWith('test-tool', { action: 'run' }); + expect(addKeySpy).toHaveBeenCalledWith('test-tool', { action: 'run' }, undefined); expect(mockTelemetryTrack).toHaveBeenCalledWith( 'User finished providing input', expect.objectContaining({ @@ -294,6 +296,80 @@ describe('InstanceAiConfirmationPanel telemetry', () => { expect(mockTelemetryTrack).not.toHaveBeenCalled(); }); + it('scopes always-allow for build-workflow to confirmation.workflowId', async () => { + injectPendingConfirmation( + thread, + { + requestId: 'req-build-always', + severity: 'warning', + message: 'Edit Target workflow (ID: wf-1)?', + workflowId: 'wf-1', + }, + { filePath: 'src/workflows/main.workflow.ts' }, + 'build-workflow', + ); + const confirmSpy = vi.spyOn(thread, 'confirmAction').mockResolvedValue(true); + const addKeySpy = vi.spyOn(thread, 'addAlwaysAllowKey'); + + const { getByTestId } = renderComponent({ props: { kind: 'floating' } }); + await userEvent.click(getByTestId('instance-ai-panel-confirm-always-allow')); + + expect(confirmSpy).toHaveBeenCalledWith('req-build-always', { + kind: 'approval', + approved: true, + scope: 'session', + }); + expect(addKeySpy).toHaveBeenCalledWith( + 'build-workflow', + { filePath: 'src/workflows/main.workflow.ts' }, + 'wf-1', + ); + }); + + it('hides always-allow for unscoped build-workflow edits', () => { + injectPendingConfirmation( + thread, + { + requestId: 'req-build-unscoped', + severity: 'warning', + message: 'Edit workflow?', + }, + { filePath: 'src/workflows/main.workflow.ts' }, + 'build-workflow', + ); + + const { getByTestId, queryByTestId } = renderComponent({ props: { kind: 'floating' } }); + + expect(queryByTestId('instance-ai-panel-confirm-always-allow')).toBeNull(); + expect(getByTestId('instance-ai-panel-confirm-approve')).toBeVisible(); + expect(getByTestId('instance-ai-panel-confirm-deny')).toBeVisible(); + }); + + it('does not record a session key on allow-once for workflow edits', async () => { + injectPendingConfirmation( + thread, + { + requestId: 'req-build-once', + severity: 'warning', + message: 'Edit Target workflow (ID: wf-1)?', + workflowId: 'wf-1', + }, + { filePath: 'src/workflows/main.workflow.ts' }, + 'build-workflow', + ); + const confirmSpy = vi.spyOn(thread, 'confirmAction').mockResolvedValue(true); + const addKeySpy = vi.spyOn(thread, 'addAlwaysAllowKey'); + + const { getByTestId } = renderComponent({ props: { kind: 'floating' } }); + await userEvent.click(getByTestId('instance-ai-panel-confirm-approve')); + + expect(confirmSpy).toHaveBeenCalledWith('req-build-once', { + kind: 'approval', + approved: true, + }); + expect(addKeySpy).not.toHaveBeenCalled(); + }); + it('does not resolve the confirmation when an approve POST fails', async () => { injectPendingConfirmation(thread, { requestId: 'req-approve-fail', diff --git a/packages/frontend/editor-ui/src/features/ai/instanceAi/__tests__/instanceAi.threadRuntime.test.ts b/packages/frontend/editor-ui/src/features/ai/instanceAi/__tests__/instanceAi.threadRuntime.test.ts index e2e908d476c..0323583c422 100644 --- a/packages/frontend/editor-ui/src/features/ai/instanceAi/__tests__/instanceAi.threadRuntime.test.ts +++ b/packages/frontend/editor-ui/src/features/ai/instanceAi/__tests__/instanceAi.threadRuntime.test.ts @@ -1669,6 +1669,7 @@ describe('createThreadRuntime - session always-allow', () => { severity?: 'info' | 'warning' | 'destructive'; channelConfig?: { integrationType: string; agentId: string }; targetApproval?: InstanceAiTargetApproval; + workflowId?: string; }, ): void { runtime.messages.push({ @@ -1699,6 +1700,7 @@ describe('createThreadRuntime - session always-allow', () => { message: 'Approve?', ...(opts.channelConfig ? { channelConfig: opts.channelConfig } : {}), ...(opts.targetApproval ? { targetApproval: opts.targetApproval } : {}), + ...(opts.workflowId ? { workflowId: opts.workflowId } : {}), }, }, ], @@ -1803,6 +1805,86 @@ describe('createThreadRuntime - session always-allow', () => { expect(runtime.resolvedConfirmationIds.has('req-update')).toBe(false); }); + it('scopes workflow update grants per workflow', async () => { + const runtime = registry.getOrCreateRuntime(activeThreadId); + runtime.addAlwaysAllowKey('workflows', { action: 'update', workflowId: 'wf-1' }); + runtime.addAlwaysAllowKey('build-workflow', { workflowId: 'wf-1' }); + + pushPendingApproval(runtime, { + messageId: 'msg-update-1', + requestId: 'req-update-1', + toolName: 'workflows', + args: { action: 'update', workflowId: 'wf-1' }, + }); + await vi.waitFor(() => { + expect(runtime.resolvedConfirmationIds.has('req-update-1')).toBe(true); + }); + + pushPendingApproval(runtime, { + messageId: 'msg-build-1', + requestId: 'req-build-1', + toolName: 'build-workflow', + args: { workflowId: 'wf-1' }, + }); + await vi.waitFor(() => { + expect(runtime.resolvedConfirmationIds.has('req-build-1')).toBe(true); + }); + + pushPendingApproval(runtime, { + messageId: 'msg-update-2', + requestId: 'req-update-2', + toolName: 'workflows', + args: { action: 'update', workflowId: 'wf-2' }, + }); + await new Promise((resolve) => setTimeout(resolve, 10)); + expect(runtime.resolvedConfirmationIds.has('req-update-2')).toBe(false); + }); + + it('scopes bound build-workflow grants from confirmation.workflowId when args omit it', async () => { + const runtime = registry.getOrCreateRuntime(activeThreadId); + // Bound saves often omit args.workflowId; the suspend payload carries it. + runtime.addAlwaysAllowKey('build-workflow', {}, 'wf-1'); + + pushPendingApproval(runtime, { + messageId: 'msg-bound-1', + requestId: 'req-bound-1', + toolName: 'build-workflow', + args: { filePath: 'src/workflows/main.workflow.ts' }, + workflowId: 'wf-1', + }); + await vi.waitFor(() => { + expect(runtime.resolvedConfirmationIds.has('req-bound-1')).toBe(true); + }); + + pushPendingApproval(runtime, { + messageId: 'msg-bound-2', + requestId: 'req-bound-2', + toolName: 'build-workflow', + args: { filePath: 'src/workflows/other.workflow.ts' }, + workflowId: 'wf-2', + }); + await new Promise((resolve) => setTimeout(resolve, 10)); + expect(runtime.resolvedConfirmationIds.has('req-bound-2')).toBe(false); + }); + + it('does not store a blanket build-workflow always-allow key without a workflow id', () => { + const runtime = registry.getOrCreateRuntime(activeThreadId); + runtime.addAlwaysAllowKey('build-workflow', { filePath: 'src/workflows/main.workflow.ts' }); + expect(runtime.sessionAlwaysAllowKeys.size).toBe(0); + }); + + it('reports canAlwaysAllow false for unscoped workflow edits', () => { + const runtime = registry.getOrCreateRuntime(activeThreadId); + expect( + runtime.canAlwaysAllow('build-workflow', { filePath: 'src/workflows/main.workflow.ts' }), + ).toBe(false); + expect(runtime.canAlwaysAllow('workflows', { action: 'update' })).toBe(false); + expect(runtime.canAlwaysAllow('build-workflow', {}, 'wf-1')).toBe(true); + expect(runtime.canAlwaysAllow('workflows', { action: 'update', workflowId: 'wf-1' })).toBe( + true, + ); + }); + it('scopes executions run grants per workflow', async () => { const runtime = registry.getOrCreateRuntime(activeThreadId); runtime.addAlwaysAllowKey('executions', { action: 'run', workflowId: 'wf-1' }); diff --git a/packages/frontend/editor-ui/src/features/ai/instanceAi/components/InstanceAiConfirmationPanel.vue b/packages/frontend/editor-ui/src/features/ai/instanceAi/components/InstanceAiConfirmationPanel.vue index 292aa505113..2c6955d7fde 100644 --- a/packages/frontend/editor-ui/src/features/ai/instanceAi/components/InstanceAiConfirmationPanel.vue +++ b/packages/frontend/editor-ui/src/features/ai/instanceAi/components/InstanceAiConfirmationPanel.vue @@ -175,8 +175,15 @@ function buildApprovalSubtitle(item: PendingConfirmationItem): string { */ function buildApprovalOptions(item: PendingConfirmationItem): ApprovalOption[] { const destructive = isDestructive(item); + const conf = item.toolCall.confirmation; + // Workflow edits must be scoped to a workflow ID — never offer a session grant + // that would collapse to a blanket tool key. + const alwaysAllowAvailable = + !destructive && + !conf.targetApproval && + thread.canAlwaysAllow(item.toolCall.toolName, item.toolCall.args ?? {}, conf.workflowId); const options: ApprovalOption[] = []; - if (!destructive && !item.toolCall.confirmation.targetApproval) { + if (alwaysAllowAvailable) { options.push({ key: 'always-allow', icon: 'check-check', @@ -244,10 +251,11 @@ async function handleConfirm(item: PendingConfirmationItem, approved: boolean) { // behaviour. `confirmAction` already surfaces a toast on failure. const ok = await thread.confirmAction(conf.requestId, { kind: 'approval', approved }); if (!ok) return; - // "Always allow" is offered alongside Approve/Deny for non-destructive - // generic approvals; include it in the option set so telemetry reflects - // what the user actually chose between. - const alwaysAllowAvailable = !isDestructive(item) && !conf.targetApproval; + // Match the options actually shown in `buildApprovalOptions`. + const alwaysAllowAvailable = + !isDestructive(item) && + !conf.targetApproval && + thread.canAlwaysAllow(item.toolCall.toolName, item.toolCall.args ?? {}, conf.workflowId); trackInputCompleted( conf, [ @@ -283,7 +291,7 @@ async function handleAlwaysAllow(item: PendingConfirmationItem) { scope: 'session', }); if (!ok) return; - thread.addAlwaysAllowKey(item.toolCall.toolName, item.toolCall.args ?? {}); + thread.addAlwaysAllowKey(item.toolCall.toolName, item.toolCall.args ?? {}, conf.workflowId); trackInputCompleted( conf, [ diff --git a/packages/frontend/editor-ui/src/features/ai/instanceAi/instanceAi.threadRuntime.ts b/packages/frontend/editor-ui/src/features/ai/instanceAi/instanceAi.threadRuntime.ts index a2dec79b33d..9df22ed8db5 100644 --- a/packages/frontend/editor-ui/src/features/ai/instanceAi/instanceAi.threadRuntime.ts +++ b/packages/frontend/editor-ui/src/features/ai/instanceAi/instanceAi.threadRuntime.ts @@ -4,6 +4,7 @@ import { ResponseError } from '@n8n/rest-api-client'; import { buildDataTablesSessionGrantKey, buildRunWorkflowSessionGrantKey, + buildUpdateWorkflowSessionGrantKey, INSTANCE_AI_EPHEMERAL_EVENT_TYPES, INSTANCE_AI_THREAD_SOURCE_FALLBACK, instanceAiEventSchema, @@ -565,36 +566,82 @@ export function createThreadRuntime( // --- Session "Always allow" --- // Thread-scoped: cleared by `resetState()` so grants don't leak when the - // runtime is disposed and recreated. Key: `${toolName}:${args.action ?? ''}` - // for most tools; `submit-workflow` is keyed on `workflowId` presence so a - // create grant doesn't silently auto-approve later updates (the backend - // distinguishes createWorkflow vs updateWorkflow by that field). + // runtime is disposed and recreated. Prefer shared builders from + // `@n8n/api-types` so UI keys match persisted thread grants: + // `executions:run:`, `workflows:update:`, `data-tables:`. + // Fallback for other tools: `${toolName}:${args.action ?? ''}`. + // `submit-workflow` is keyed on `workflowId` presence so a create grant + // doesn't silently auto-approve later updates. const sessionAlwaysAllowKeys = ref>(new Set()); - function buildAlwaysAllowKey(toolName: string, args: Record): string { + function resolveAlwaysAllowWorkflowId( + args: Record, + confirmationWorkflowId?: string, + ): string { + if (typeof args.workflowId === 'string' && args.workflowId.length > 0) { + return args.workflowId; + } + if (typeof confirmationWorkflowId === 'string' && confirmationWorkflowId.length > 0) { + return confirmationWorkflowId; + } + return ''; + } + + /** + * Returns null when an edit grant cannot be scoped to a workflow ID — storing a + * generic `build-workflow:` key would auto-approve later foreign edits. + */ + function buildAlwaysAllowKey( + toolName: string, + args: Record, + confirmationWorkflowId?: string, + ): string | null { if (toolName === 'submit-workflow') { const isUpdate = typeof args.workflowId === 'string' && args.workflowId.length > 0; return `submit-workflow:${isUpdate ? 'update' : 'create'}`; } const action = typeof args.action === 'string' ? args.action : ''; + const workflowId = resolveAlwaysAllowWorkflowId(args, confirmationWorkflowId); // Running a workflow grants "always allow" per workflow, so the grant applies only to the // workflow the user approved. if (toolName === 'executions' && action === 'run') { - const workflowId = typeof args.workflowId === 'string' ? args.workflowId : ''; return buildRunWorkflowSessionGrantKey(workflowId); } + // Editing a workflow (build-workflow save or workflows update) is also per-workflow, + // matching the backend `workflows:update:` thread grant. Bound build-workflow + // saves often omit args.workflowId — use confirmation.workflowId from the suspend + // payload instead. Without either ID, refuse to store a key (fail closed). + if ((toolName === 'workflows' && action === 'update') || toolName === 'build-workflow') { + if (!workflowId) return null; + return buildUpdateWorkflowSessionGrantKey(workflowId); + } if (toolName === 'data-tables') { return buildDataTablesSessionGrantKey(action); } return `${toolName}:${action}`; } - function addAlwaysAllowKey(toolName: string, args: Record): void { + function addAlwaysAllowKey( + toolName: string, + args: Record, + confirmationWorkflowId?: string, + ): void { + const key = buildAlwaysAllowKey(toolName, args, confirmationWorkflowId); + if (key === null) return; const next = new Set(sessionAlwaysAllowKeys.value); - next.add(buildAlwaysAllowKey(toolName, args)); + next.add(key); sessionAlwaysAllowKeys.value = next; } + /** False when Always allow cannot be scoped (e.g. workflow edit with no workflow ID). */ + function canAlwaysAllow( + toolName: string, + args: Record, + confirmationWorkflowId?: string, + ): boolean { + return buildAlwaysAllowKey(toolName, args, confirmationWorkflowId) !== null; + } + function isGenericApprovalEligible(item: PendingConfirmationItem): boolean { const conf = item.toolCall.confirmation; if (conf.targetApproval) return false; @@ -623,8 +670,12 @@ export function createThreadRuntime( if (resolvedConfirmationIds.has(conf.requestId)) continue; if (autoApproveInFlight.has(conf.requestId)) continue; if (!isGenericApprovalEligible(item)) continue; - const key = buildAlwaysAllowKey(item.toolCall.toolName, item.toolCall.args ?? {}); - if (!sessionAlwaysAllowKeys.value.has(key)) continue; + const key = buildAlwaysAllowKey( + item.toolCall.toolName, + item.toolCall.args ?? {}, + conf.workflowId, + ); + if (key === null || !sessionAlwaysAllowKeys.value.has(key)) continue; autoApproveInFlight.add(conf.requestId); try { @@ -1320,6 +1371,7 @@ export function createThreadRuntime( confirmResourceDecision, resolveConfirmation, addAlwaysAllowKey, + canAlwaysAllow, findToolCallByRequestId, copyFullTrace, submitFeedback,