mirror of
https://github.com/n8n-io/n8n.git
synced 2026-08-29 01:39:24 +08:00
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 <rmthamir@gmail.com>
This commit is contained in:
committed by
GitHub
parent
69140c0610
commit
a29cabc294
@@ -286,6 +286,7 @@ export {
|
||||
|
||||
export {
|
||||
buildRunWorkflowSessionGrantKey,
|
||||
buildUpdateWorkflowSessionGrantKey,
|
||||
buildDataTablesSessionGrantKey,
|
||||
buildFetchUrlGrantKey,
|
||||
FETCH_URL_ALLOW_ALL_GRANT_KEY,
|
||||
|
||||
@@ -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');
|
||||
|
||||
@@ -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)'),
|
||||
|
||||
@@ -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({
|
||||
|
||||
@@ -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:<id>`).
|
||||
const resumeSchema = setupResumeSchema.extend({
|
||||
scope: z.enum(['once', 'session']).optional(),
|
||||
});
|
||||
|
||||
interface WorkflowToolContext {
|
||||
resumeData: z.infer<typeof resumeSchema> | 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,
|
||||
|
||||
@@ -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<BuildToolOutput>(
|
||||
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<BuildToolOutput>(
|
||||
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<BuildToolOutput>(
|
||||
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',
|
||||
|
||||
@@ -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<string>()).add(created.id);
|
||||
await recordSessionOwnedWorkflow(context, created.id);
|
||||
return await createSuccessResponse(created, 'create');
|
||||
} catch (error) {
|
||||
const message = error instanceof Error ? error.message : 'Unknown error';
|
||||
|
||||
@@ -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:<id>` 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<void> {
|
||||
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<void> {
|
||||
(context.aiCreatedWorkflowIds ??= new Set<string>()).add(workflowId);
|
||||
await grantSessionWorkflowUpdate(context, workflowId);
|
||||
}
|
||||
|
||||
export async function resolveWorkflowName(
|
||||
context: InstanceAiContext,
|
||||
workflowId: string,
|
||||
|
||||
@@ -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:<id>` session grant
|
||||
* written at create time — this in-memory set alone does not survive a new run.
|
||||
* Lazily initialized on first create.
|
||||
*/
|
||||
aiCreatedWorkflowIds?: Set<string>;
|
||||
/**
|
||||
|
||||
+79
-3
@@ -101,10 +101,11 @@ const renderComponent = createThreadComponentRenderer(InstanceAiConfirmationPane
|
||||
function makeToolCall(
|
||||
confirmation: InstanceAiConfirmation,
|
||||
args: Record<string, unknown> = {},
|
||||
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<string, unknown> = {},
|
||||
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',
|
||||
|
||||
+82
@@ -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' });
|
||||
|
||||
+14
-6
@@ -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,
|
||||
[
|
||||
|
||||
+62
-10
@@ -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:<id>`, `workflows:update:<id>`, `data-tables:<action>`.
|
||||
// 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<Set<string>>(new Set());
|
||||
|
||||
function buildAlwaysAllowKey(toolName: string, args: Record<string, unknown>): string {
|
||||
function resolveAlwaysAllowWorkflowId(
|
||||
args: Record<string, unknown>,
|
||||
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<string, unknown>,
|
||||
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:<id>` 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<string, unknown>): void {
|
||||
function addAlwaysAllowKey(
|
||||
toolName: string,
|
||||
args: Record<string, unknown>,
|
||||
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<string, unknown>,
|
||||
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,
|
||||
|
||||
Reference in New Issue
Block a user