From 560455faf1089808a4ec4064a95f09a77041148a Mon Sep 17 00:00:00 2001 From: Riqwan Thamir Date: Tue, 11 Aug 2026 20:11:46 +0200 Subject: [PATCH] fix(core): Accept Always allow scope on data-tables resume (#36070) Co-authored-by: Cursor --- packages/@n8n/api-types/src/index.ts | 1 + .../__tests__/instance-ai.schema.test.ts | 8 +++ .../src/schemas/instance-ai.schema.ts | 9 +++ .../tools/__tests__/data-tables.tool.test.ts | 62 ++++++++++++++++- .../instance-ai/src/tools/data-tables.tool.ts | 67 ++++++++++++++++--- .../ai/instanceAi/instanceAi.threadRuntime.ts | 4 ++ 6 files changed, 140 insertions(+), 11 deletions(-) diff --git a/packages/@n8n/api-types/src/index.ts b/packages/@n8n/api-types/src/index.ts index ab0eb40dc1d..ee705d09db7 100644 --- a/packages/@n8n/api-types/src/index.ts +++ b/packages/@n8n/api-types/src/index.ts @@ -287,6 +287,7 @@ export { export { buildRunWorkflowSessionGrantKey, + buildDataTablesSessionGrantKey, buildFetchUrlGrantKey, FETCH_URL_ALLOW_ALL_GRANT_KEY, WEB_SEARCH_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 89793f5de67..eab9e735f0c 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 @@ -1,6 +1,7 @@ import { AI_GATEWAY_MANAGED_TAG, applyBranchReadOnlyOverrides, + buildDataTablesSessionGrantKey, buildFetchUrlGrantKey, DEFAULT_INSTANCE_AI_PERMISSIONS, errorPayloadSchema, @@ -431,6 +432,13 @@ describe('instance-ai launch schema', () => { }); }); +describe('data-tables session grant keys', () => { + it('builds action-scoped keys matching the frontend always-allow format', () => { + expect(buildDataTablesSessionGrantKey('create')).toBe('data-tables:create'); + expect(buildDataTablesSessionGrantKey('insert-rows')).toBe('data-tables:insert-rows'); + }); +}); + 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 7301f9dac56..f1d22fa4c73 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,15 @@ export function buildRunWorkflowSessionGrantKey(workflowId: string): string { return `executions:run:${workflowId}`; } +/** + * Builds the thread-level "always allow" grant key for a data-tables action + * (e.g. `create`, `insert-rows`). Must match the frontend key + * `${toolName}:${action}` so UI auto-approve and persisted grants stay aligned. + */ +export function buildDataTablesSessionGrantKey(action: string): string { + return `data-tables:${action}`; +} + // --- Domain-access grants ("always allow" for web access) --- // These keys mirror the research tool's action names (`fetch-url`, `web-search`) the same // way `executions:run:` mirrors the executions `run` action, so a persisted grant row diff --git a/packages/@n8n/instance-ai/src/tools/__tests__/data-tables.tool.test.ts b/packages/@n8n/instance-ai/src/tools/__tests__/data-tables.tool.test.ts index f3f0365045e..37249ff07f6 100644 --- a/packages/@n8n/instance-ai/src/tools/__tests__/data-tables.tool.test.ts +++ b/packages/@n8n/instance-ai/src/tools/__tests__/data-tables.tool.test.ts @@ -1,5 +1,6 @@ import type { InstanceAiPermissions } from '@n8n/api-types'; import type { Mock } from 'vitest'; +import type { z } from 'zod'; import { executeTool } from '../../__tests__/tool-test-utils'; import type { InstanceAiContext } from '../../types'; @@ -40,8 +41,8 @@ function suspendCtx(suspendFn: Mock) { return { resumeData: undefined, suspend: suspendFn } as never; } -function resumeCtx(approved: boolean) { - return { resumeData: { approved } } as never; +function resumeCtx(approved: boolean, scope?: 'once' | 'session') { + return { resumeData: { approved, ...(scope ? { scope } : {}) } } as never; } function noSuspendCtx() { @@ -63,6 +64,21 @@ describe('data-tables tool', () => { expect(tool.description).toContain('load_skill'); expect(tool.description).toContain('what data tables do I have?'); }); + + // The SDK validates resume payloads against this schema (converted to JSON + // schema with additionalProperties: false) and replaces resume data with the + // parse result — an undeclared `scope` is rejected or silently stripped, so + // the "Always allow" grant would never reach the handler. + it('resume schema declares scope so the SDK preserves it on resume', () => { + const context = createMockContext(); + const tool = createDataTablesTool(context); + + const parsed: unknown = (tool.resumeSchema as z.ZodTypeAny).parse({ + approved: true, + scope: 'session', + }); + expect(parsed).toEqual({ approved: true, scope: 'session' }); + }); }); // ── list ──────────────────────────────────────────────────────────────── @@ -372,6 +388,48 @@ describe('data-tables tool', () => { expect(result).toEqual({ table }); }); + it('should accept scope=session on resume and persist a session grant', async () => { + const table = { id: 'dt-new', name: 'Contacts' }; + const grantSessionToolApproval = vi.fn().mockResolvedValue(undefined); + const context = createMockContext({ permissions: {}, grantSessionToolApproval }); + (context.dataTableService.create as Mock).mockResolvedValue(table); + + const tool = createDataTablesTool(context); + const result = await executeTool(tool, createInput as never, resumeCtx(true, 'session')); + + expect(result).toEqual({ table }); + expect(grantSessionToolApproval).toHaveBeenCalledWith('data-tables:create'); + }); + + it('should not persist a session grant when resume has no scope', async () => { + const table = { id: 'dt-new', name: 'Contacts' }; + const grantSessionToolApproval = vi.fn().mockResolvedValue(undefined); + const context = createMockContext({ permissions: {}, grantSessionToolApproval }); + (context.dataTableService.create as Mock).mockResolvedValue(table); + + const tool = createDataTablesTool(context); + await executeTool(tool, createInput as never, resumeCtx(true)); + + expect(grantSessionToolApproval).not.toHaveBeenCalled(); + }); + + it('should skip HITL when a session grant already exists', async () => { + const table = { id: 'dt-new', name: 'Contacts' }; + const context = createMockContext({ + permissions: {}, + sessionApprovedToolKeys: new Set(['data-tables:create']), + }); + (context.dataTableService.create as Mock).mockResolvedValue(table); + const suspendFn = vi.fn(); + + const tool = createDataTablesTool(context); + const result = await executeTool(tool, createInput as never, suspendCtx(suspendFn)); + + expect(suspendFn).not.toHaveBeenCalled(); + expect(context.dataTableService.create).toHaveBeenCalled(); + expect(result).toEqual({ table }); + }); + it('should return denied when user denies on resume', async () => { const context = createMockContext({ permissions: {} }); diff --git a/packages/@n8n/instance-ai/src/tools/data-tables.tool.ts b/packages/@n8n/instance-ai/src/tools/data-tables.tool.ts index 79f2ba2c851..4a115feb29a 100644 --- a/packages/@n8n/instance-ai/src/tools/data-tables.tool.ts +++ b/packages/@n8n/instance-ai/src/tools/data-tables.tool.ts @@ -3,7 +3,10 @@ * add-column, delete-column, rename-column, insert-rows, update-rows, delete-rows. */ import { Tool } from '@n8n/agents'; -import { instanceAiConfirmationSeveritySchema } from '@n8n/api-types'; +import { + buildDataTablesSessionGrantKey, + instanceAiConfirmationSeveritySchema, +} from '@n8n/api-types'; import { nanoid } from 'nanoid'; import { z } from 'zod'; @@ -49,6 +52,8 @@ const confirmationSuspendSchema = z.object({ const confirmationResumeSchema = z.object({ approved: z.boolean(), + /** `'session'` — user chose "always allow"; persist a thread-level grant. */ + scope: z.enum(['once', 'session']).optional(), }); type ResumeData = z.infer; @@ -58,6 +63,20 @@ interface ConfirmationToolContext { suspend: (payload: z.infer) => Promise; } +function hasSessionGrant(context: InstanceAiContext, action: string): boolean { + return context.sessionApprovedToolKeys?.has(buildDataTablesSessionGrantKey(action)) === true; +} + +async function persistSessionGrantIfRequested( + context: InstanceAiContext, + action: string, + resumeData: ResumeData | undefined, +): Promise { + if (resumeData?.approved && resumeData.scope === 'session') { + await context.grantSessionToolApproval?.(buildDataTablesSessionGrantKey(action)); + } +} + /** * Check if an error (or its cause chain) is a DataTableNameConflictError. * The error class lives in packages/cli so we can't import it directly — @@ -337,7 +356,8 @@ async function handleCreate( return { denied: true, reason: 'Action blocked by admin' }; } - const needsApproval = context.permissions?.createDataTable !== 'always_allow'; + const needsApproval = + context.permissions?.createDataTable !== 'always_allow' && !hasSessionGrant(context, 'create'); // State 1: First call — suspend for confirmation (unless always_allow) if (needsApproval && (resumeData === undefined || resumeData === null)) { @@ -359,6 +379,8 @@ async function handleCreate( return { denied: true, reason: 'User denied the action' }; } + await persistSessionGrantIfRequested(context, 'create', resumeData); + // State 3: Approved or always_allow — execute try { const table = await context.dataTableService.create(input.name, input.columns, { @@ -389,7 +411,8 @@ async function handleDelete( return { success: false, denied: true, reason: 'Action blocked by admin' }; } - const needsApproval = context.permissions?.deleteDataTable !== 'always_allow'; + const needsApproval = + context.permissions?.deleteDataTable !== 'always_allow' && !hasSessionGrant(context, 'delete'); // State 1: First call — suspend for confirmation (unless always_allow) if (needsApproval && (resumeData === undefined || resumeData === null)) { @@ -405,6 +428,8 @@ async function handleDelete( return { success: false, denied: true, reason: 'User denied the action' }; } + await persistSessionGrantIfRequested(context, 'delete', resumeData); + // State 3: Approved or always_allow — execute await context.dataTableService.delete(input.dataTableId, { projectId: input.projectId }); return { success: true }; @@ -421,7 +446,9 @@ async function handleAddColumn( return { denied: true, reason: 'Action blocked by admin' }; } - const needsApproval = context.permissions?.mutateDataTableSchema !== 'always_allow'; + const needsApproval = + context.permissions?.mutateDataTableSchema !== 'always_allow' && + !hasSessionGrant(context, 'add-column'); // State 1: First call — suspend for confirmation (unless always_allow) if (needsApproval && (resumeData === undefined || resumeData === null)) { @@ -437,6 +464,8 @@ async function handleAddColumn( return { denied: true, reason: 'User denied the action' }; } + await persistSessionGrantIfRequested(context, 'add-column', resumeData); + // State 3: Approved or always_allow — execute const column = await context.dataTableService.addColumn( input.dataTableId, @@ -457,7 +486,9 @@ async function handleDeleteColumn( return { success: false, denied: true, reason: 'Action blocked by admin' }; } - const needsApproval = context.permissions?.mutateDataTableSchema !== 'always_allow'; + const needsApproval = + context.permissions?.mutateDataTableSchema !== 'always_allow' && + !hasSessionGrant(context, 'delete-column'); // State 1: First call — suspend for confirmation (unless always_allow) if (needsApproval && (resumeData === undefined || resumeData === null)) { @@ -473,6 +504,8 @@ async function handleDeleteColumn( return { success: false, denied: true, reason: 'User denied the action' }; } + await persistSessionGrantIfRequested(context, 'delete-column', resumeData); + // State 3: Approved or always_allow — execute await context.dataTableService.deleteColumn(input.dataTableId, input.columnId, { projectId: input.projectId, @@ -491,7 +524,9 @@ async function handleRenameColumn( return { success: false, denied: true, reason: 'Action blocked by admin' }; } - const needsApproval = context.permissions?.mutateDataTableSchema !== 'always_allow'; + const needsApproval = + context.permissions?.mutateDataTableSchema !== 'always_allow' && + !hasSessionGrant(context, 'rename-column'); // State 1: First call — suspend for confirmation (unless always_allow) if (needsApproval && (resumeData === undefined || resumeData === null)) { @@ -507,6 +542,8 @@ async function handleRenameColumn( return { success: false, denied: true, reason: 'User denied the action' }; } + await persistSessionGrantIfRequested(context, 'rename-column', resumeData); + // State 3: Approved or always_allow — execute await context.dataTableService.renameColumn(input.dataTableId, input.columnId, input.newName, { projectId: input.projectId, @@ -525,7 +562,9 @@ async function handleInsertRows( return { denied: true, reason: 'Action blocked by admin' }; } - const needsApproval = context.permissions?.mutateDataTableRows !== 'always_allow'; + const needsApproval = + context.permissions?.mutateDataTableRows !== 'always_allow' && + !hasSessionGrant(context, 'insert-rows'); // State 1: First call — suspend for confirmation (unless always_allow) if (needsApproval && (resumeData === undefined || resumeData === null)) { @@ -541,6 +580,8 @@ async function handleInsertRows( return { denied: true, reason: 'User denied the action' }; } + await persistSessionGrantIfRequested(context, 'insert-rows', resumeData); + // State 3: Approved or always_allow — execute return await context.dataTableService.insertRows(input.dataTableId, input.rows, { projectId: input.projectId, @@ -558,7 +599,9 @@ async function handleUpdateRows( return { denied: true, reason: 'Action blocked by admin' }; } - const needsApproval = context.permissions?.mutateDataTableRows !== 'always_allow'; + const needsApproval = + context.permissions?.mutateDataTableRows !== 'always_allow' && + !hasSessionGrant(context, 'update-rows'); // State 1: First call — suspend for confirmation (unless always_allow) if (needsApproval && (resumeData === undefined || resumeData === null)) { @@ -574,6 +617,8 @@ async function handleUpdateRows( return { denied: true, reason: 'User denied the action' }; } + await persistSessionGrantIfRequested(context, 'update-rows', resumeData); + // State 3: Approved or always_allow — execute return await context.dataTableService.updateRows(input.dataTableId, input.filter, input.data, { projectId: input.projectId, @@ -591,7 +636,9 @@ async function handleDeleteRows( return { success: false, denied: true, reason: 'Action blocked by admin' }; } - const needsApproval = context.permissions?.mutateDataTableRows !== 'always_allow'; + const needsApproval = + context.permissions?.mutateDataTableRows !== 'always_allow' && + !hasSessionGrant(context, 'delete-rows'); // State 1: First call — suspend for confirmation (unless always_allow) if (needsApproval && (resumeData === undefined || resumeData === null)) { @@ -616,6 +663,8 @@ async function handleDeleteRows( return { success: false, denied: true, reason: 'User denied the action' }; } + await persistSessionGrantIfRequested(context, 'delete-rows', resumeData); + // State 3: Approved or always_allow — execute const result = await context.dataTableService.deleteRows(input.dataTableId, input.filter, { projectId: input.projectId, 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 5a897a6a824..a2dec79b33d 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 @@ -2,6 +2,7 @@ import { computed, reactive, ref, triggerRef, watch } from 'vue'; import { v4 as uuidv4 } from 'uuid'; import { ResponseError } from '@n8n/rest-api-client'; import { + buildDataTablesSessionGrantKey, buildRunWorkflowSessionGrantKey, INSTANCE_AI_EPHEMERAL_EVENT_TYPES, INSTANCE_AI_THREAD_SOURCE_FALLBACK, @@ -582,6 +583,9 @@ export function createThreadRuntime( const workflowId = typeof args.workflowId === 'string' ? args.workflowId : ''; return buildRunWorkflowSessionGrantKey(workflowId); } + if (toolName === 'data-tables') { + return buildDataTablesSessionGrantKey(action); + } return `${toolName}:${action}`; }