mirror of
https://github.com/n8n-io/n8n.git
synced 2026-08-28 17:22:01 +08:00
fix(core): Accept Always allow scope on data-tables resume (#36070)
Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
@@ -287,6 +287,7 @@ export {
|
||||
|
||||
export {
|
||||
buildRunWorkflowSessionGrantKey,
|
||||
buildDataTablesSessionGrantKey,
|
||||
buildFetchUrlGrantKey,
|
||||
FETCH_URL_ALLOW_ALL_GRANT_KEY,
|
||||
WEB_SEARCH_GRANT_KEY,
|
||||
|
||||
@@ -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');
|
||||
|
||||
@@ -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:<id>` mirrors the executions `run` action, so a persisted grant row
|
||||
|
||||
@@ -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: {} });
|
||||
|
||||
|
||||
@@ -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<typeof confirmationResumeSchema>;
|
||||
@@ -58,6 +63,20 @@ interface ConfirmationToolContext {
|
||||
suspend: (payload: z.infer<typeof confirmationSuspendSchema>) => Promise<never>;
|
||||
}
|
||||
|
||||
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<void> {
|
||||
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,
|
||||
|
||||
@@ -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}`;
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user