fix(access): custom SQL cannot write; batches stop at the first failure

apps/web/src/frontend/features/utilities/components/CloneTableModal.tsx

@@ -238,11 +238,14 @@ export const CloneTableModal: React.FC<Props> = ({
       schema: conn.schema?.trim() || undefined,
     };
     try {
-      // Stop on first failure; chunk to backend MAX_STATEMENTS.
+      // Stop on first failure; chunk to backend MAX_STATEMENTS. The server
+      // stops too: without it, a failed index rename on the archive still
+      // ran the CREATE of the new live table after it, and "rename the
+      // archive back" then failed because the live name was taken.
       let ran = 0;
       for (let i = 0; i < executable.length; i += EXECUTE_BATCH_SIZE) {
         const batch = executable.slice(i, i + EXECUTE_BATCH_SIZE);
-        const result = await executeSql(ref, batch);
+        const result = await executeSql(ref, batch, undefined, undefined, undefined, { stopOnError: true });
         for (let j = 0; j < result.results.length; j++) {
           const r = result.results[j]!;
           if (r.ok) {

apps/web/src/frontend/shared/api/accessSql.test.ts

@@ -51,13 +51,36 @@ describe('runAccessSql', () => {
     expect(sent()).toEqual(['GRANT SELECT ON t TO r']);
   });
 
-  it('joins the errors of the statements that failed', async () => {
+  it('asks the server to stop at the first failure', async () => {
+    // A script's later steps assume its earlier ones; running them anyway
+    // half-applies access the reader was never shown as a plan.
+    await runAccessSql(ref, 'GRANT SELECT ON t TO r;\nGRANT INSERT ON t TO r;');
+    expect(executeSql.mock.calls[0]![5]).toEqual({ stopOnError: true });
+  });
+
+  it('says which statement failed, what was applied before it and what did not run', async () => {
+    // The server stops at the failure, so the results end there.
     executeSql.mockResolvedValue({
       results: [{ ok: true }, { ok: false, error: 'role "r" does not exist' }],
     });
+    await expect(
+      runAccessSql(ref, 'GRANT USAGE ON SCHEMA s TO r;\nGRANT SELECT ON t TO r;\nGRANT INSERT ON t TO r;')
+    ).resolves.toEqual({
+      ok: false,
+      error: 'Statement 2 of 3 failed: role "r" does not exist. The 1 before it was applied. The 1 after it did not run.',
+    });
+  });
+
+  it('reports a lone statement’s error as it is', async () => {
+    executeSql.mockResolvedValue({ results: [{ ok: false, error: 'permission denied' }] });
+    await expect(runAccessSql(ref, 'GRANT SELECT ON t TO r;')).resolves.toEqual({ ok: false, error: 'permission denied' });
+  });
+
+  it('does not report ok when fewer results came back than statements were sent', async () => {
+    executeSql.mockResolvedValue({ results: [{ ok: true }] });
     await expect(runAccessSql(ref, 'GRANT SELECT ON t TO r;\nGRANT INSERT ON t TO r;')).resolves.toEqual({
       ok: false,
-      error: 'role "r" does not exist',
+      error: 'Only 1 of 2 statements ran.',
     });
   });
 

apps/web/src/frontend/shared/api/accessSql.ts

@@ -20,29 +20,40 @@ import { splitSqlStatements } from '@foxschema/ui-shared';
 import type { ConnectionRef } from './schemaApi';
 import { executeSql } from './sqlApi';
 
-/** Every statement succeeded, or the joined errors of those that did not. */
+/** Every statement succeeded, or which one failed and what ran around it. */
 export type AccessSqlOutcome = { ok: true } | { ok: false; error: string };
 
 /**
  * Execute a grant/revoke script statement by statement.
  *
- * Statements run in order and are not wrapped in a transaction: most engines
- * auto-commit DDL anyway, so a mid-script failure leaves the earlier grants
- * applied. The caller reports the failure and re-reads the catalog rather than
- * claiming the script was undone.
+ * Statements run in order and stop at the first failure: a script's later
+ * steps assume its earlier ones (USAGE ON SCHEMA before ON ALL TABLES, CREATE
+ * USER before its quota), so running them against a state the script did not
+ * reach only adds errors or half-applied access. Nothing is wrapped in a
+ * transaction — most engines auto-commit DDL anyway — so the statements before
+ * the failure stay applied. The error says so, and the caller re-reads the
+ * catalog rather than claiming the script was undone.
  */
 export async function runAccessSql(
   ref: ConnectionRef,
   sql: string
 ): Promise<AccessSqlOutcome> {
-  const statements = splitSqlStatements(sql)
+  const split = splitSqlStatements(sql)
     .map((s) => s.text.trim())
     .filter(Boolean);
-  const { results } = await executeSql(ref, statements.length ? statements : [sql]);
-  const failed = results.filter((r) => !r.ok);
-  if (failed.length === 0) return { ok: true };
-  return {
-    ok: false,
-    error: failed.map((r) => ('error' in r ? r.error : 'failed')).join(' · '),
-  };
+  const statements = split.length ? split : [sql];
+  const { results } = await executeSql(ref, statements, undefined, undefined, undefined, { stopOnError: true });
+  const failedAt = results.findIndex((r) => !r.ok);
+  if (failedAt === -1 && results.length === statements.length) return { ok: true };
+  if (failedAt === -1) {
+    return { ok: false, error: `Only ${results.length} of ${statements.length} statements ran.` };
+  }
+  const failure = results[failedAt]!;
+  const error = 'error' in failure ? failure.error : 'failed';
+  if (statements.length === 1) return { ok: false, error };
+  const notRun = statements.length - failedAt - 1;
+  const before = failedAt === 0 ? '' : ` The ${failedAt} before it ${failedAt === 1 ? 'was' : 'were'} applied.`;
+  const after = notRun === 0 ? '' : ` The ${notRun} after it did not run.`;
+  const reason = error.trim().replace(/\.$/, '');
+  return { ok: false, error: `Statement ${failedAt + 1} of ${statements.length} failed: ${reason}.${before}${after}` };
 }

apps/web/src/frontend/shared/api/sqlApi.ts

@@ -35,6 +35,8 @@ export async function executeSql(
   opts?: {
     datagridAction?: 'insert' | 'update' | 'delete';
     seek?: { columns: string[]; values: unknown[]; descending?: boolean | boolean[] };
+    /** Leave the statements after a failure unrun; `results` then ends at it. */
+    stopOnError?: boolean;
   }
 ): Promise<{ results: SqlStatementResult[] }> {
   const data = await api.post<{ results?: SqlStatementResult[]; error?: string }>(`/sql/execute`, {
@@ -45,6 +47,7 @@ export async function executeSql(
       params,
       ...(opts?.datagridAction ? { datagridAction: opts.datagridAction } : {}),
       ...(opts?.seek ? { seek: opts.seek } : {}),
+      ...(opts?.stopOnError ? { stopOnError: true } : {}),
     });
   if (!data.results) throw new Error(data.error || 'Query failed: no results returned.');
   return { results: data.results };

packages/server/src/features/access/access.routes.test.ts

@@ -40,34 +40,6 @@ async function serve(permissions: Permission[]) {
   return { server: app, resolveRef };
 }
 
-describe('table insight permissions', () => {
-  it('allows viewers who can run Data Peek queries', async () => {
-    const { server, resolveRef } = await serve(['editor.run']);
-    const res = await server.inject({
-      method: 'POST',
-      url: '/schema/table-insight',
-      payload: { connectionId: 'c1', table: 'orders' },
-    });
-
-    expect(res.statusCode).toBe(400);
-    expect(res.json().code).not.toBe('forbidden');
-    expect(resolveRef).toHaveBeenCalledOnce();
-  });
-
-  it('does not grant SQL Editor insight through Utilities alone', async () => {
-    const { server, resolveRef } = await serve(['utility.access']);
-    const res = await server.inject({
-      method: 'POST',
-      url: '/schema/table-insight',
-      payload: { connectionId: 'c1', table: 'orders' },
-    });
-
-    expect(res.statusCode).toBe(403);
-    expect(res.json()).toMatchObject({ code: 'forbidden', missing: ['editor.run'] });
-    expect(resolveRef).not.toHaveBeenCalled();
-  });
-});
-
 describe('db-access catalog permissions', () => {
   it('allows Access workspace viewers with access.access (not only utility.access)', async () => {
     const { server, resolveRef } = await serve(['access.access']);

packages/server/src/features/access/access.routes.ts

@@ -17,7 +17,6 @@ import type { AuthedRequest } from '../../platform/http/types';
 import type { ConnectionRef } from '../../platform/connections/resolve';
 import { probeTableFragmentation, mapPool } from './index-fragmentation.service';
 import { probeDbaUtility } from './dba-utilities.service';
-import { probeTableInsight } from '../schema';
 import type { DbaUtilityKind } from '@foxschema/db';
 import { probeDbAccess } from './db-access.service';
 import {
@@ -194,42 +193,6 @@ export function createAccessRoutes(deps: AccessRouteDeps): Router {
     }
   });
 
-  router.post(
-    '/schema/table-insight',
-    dbaUtilityLimiter,
-    // This powers Data Peek's Insight tab in the SQL Editor. Requiring
-    // utility.access locked the whole tab out for the built-in viewer role,
-    // even though viewers can open Data Peek and run its read-only queries.
-    requirePermissions('editor.run'),
-    async (req: AppRequest, res: FastifyReply) => {
-      const body = req.body as ConnectionRef & { schema?: unknown; table?: unknown };
-      const table = typeof body.table === 'string' ? body.table.trim() : '';
-      if (!table) {
-        sendError(res, 'invalid_input', 'table is required.');
-        return;
-      }
-      try {
-        const resolved = await deps.resolveRef(scopeOf(req as AuthedRequest), body);
-        const schema =
-          (typeof body.schema === 'string' && body.schema.trim()) || resolved.schema || '';
-        const probed = await probeTableInsight({
-          dialect: resolved.dialect,
-          option: resolved.option,
-          schema,
-          table,
-        });
-        if (!probed.ok) {
-          const { status, ...rest } = probed.failure;
-          res.status(status).send(rest);
-          return;
-        }
-        res.send({ ...probed.value, dialect: resolved.dialect });
-      } catch (error: unknown) {
-        sendThrown(res, error, 'Failed to load table insight');
-      }
-    }
-  );
-
   router.post(
     '/schema/db-access',
     dbAccessLimiter,

packages/server/src/features/access/index-fragmentation.service.test.ts

@@ -162,6 +162,24 @@ describe('probeTableFragmentation against a real SQLite file', () => {
     expect(probed.failure.error).toMatch(/SELECT/);
   });
 
+  it('refuses a SELECT … INTO hidden behind a comment marker in a string, before running it', async () => {
+    // On Postgres or SQL Server this creates a table. SQLite has no
+    // SELECT … INTO, so had it reached the database the failure would be its
+    // syntax error as a 500; refused first, it is a 400 naming the write.
+    const probed = await probeTableFragmentation({
+      dialect: 'sqlite',
+      option,
+      schema: '',
+      table: 'orders',
+      preferCustom: true,
+      customSql: `SELECT '/*', 1 INTO fox_probe FROM orders -- */`,
+    });
+    expect(probed.ok).toBe(false);
+    if (probed.ok) return;
+    expect(probed.failure.status).toBe(400);
+    expect(probed.failure.error).toMatch(/read-only/);
+  });
+
   it('rejects multi-statement custom SQL', async () => {
     const probed = await probeTableFragmentation({
       dialect: 'sqlite',

packages/server/src/features/access/index-fragmentation.service.ts

@@ -206,7 +206,7 @@ export async function probeTableFragmentation(opts: {
         },
       };
     }
-    const safe = isSafeIndexFragmentationCustomSql(customSql);
+    const safe = isSafeIndexFragmentationCustomSql(customSql, dialect);
     if (safe !== true) {
       return {
         ok: false,
@@ -248,7 +248,7 @@ export async function probeTableFragmentation(opts: {
   const built = buildIndexFragmentationQuery({ dialect, schema, table });
   if ('error' in built) {
     if (customSql) {
-      const safe = isSafeIndexFragmentationCustomSql(customSql);
+      const safe = isSafeIndexFragmentationCustomSql(customSql, dialect);
       if (safe !== true) {
         return {
           ok: false,
@@ -355,7 +355,7 @@ export async function probeTableFragmentation(opts: {
         },
       };
     }
-    const safe = isSafeIndexFragmentationCustomSql(customSql);
+    const safe = isSafeIndexFragmentationCustomSql(customSql, dialect);
     if (safe !== true) {
       return {
         ok: false,

packages/server/src/features/schema/index.ts

@@ -7,7 +7,6 @@
 import type { ServerFeatureModule } from '../../platform/feature-module';
 import { createSchemaRoutes } from './schema.routes';
 
-export { probeTableInsight } from './table-insight.service';
 
 export const schemaFeature: ServerFeatureModule = {
   id: 'schema',

packages/server/src/features/schema/schema.routes.ts

@@ -3,7 +3,7 @@
  * Copyright 2024-2026 Huy Phan <huyplb@gmail.com>
  * SPDX-License-Identifier: Apache-2.0
  *
- * Schema browse routes.
+ * Schema browse routes, and the table insight Data Peek shows.
  */
 import { scopeOf } from '../../platform/http/scope';
 import type { ConnectionResolver } from '../../platform/connections/resolve';
@@ -15,6 +15,8 @@ import type { AuthedRequest } from '../../platform/http/types';
 import type { ConnectionRef } from '../../platform/connections/resolve';
 import { getProviderSettings, type DbObjectType } from '@foxschema/db';
 import { sendError, sendThrown } from '../../platform/http/respond';
+import { rateLimit } from '../../platform/guards/rate-limit';
+import { probeTableInsight } from './table-insight.service';
 
 export interface SchemaRouteDeps {
   resolveRef: ConnectionResolver['resolveRef'];
@@ -54,5 +56,43 @@ export function createSchemaRoutes(deps: SchemaRouteDeps): Router {
     }
   });
 
+  const tableInsightLimiter = rateLimit({ windowMs: 60 * 1000, max: 30 });
+  router.post(
+    '/schema/table-insight',
+    tableInsightLimiter,
+    // This powers Data Peek's Insight tab in the SQL Editor. Requiring
+    // utility.access locked the whole tab out for the built-in viewer role,
+    // even though viewers can open Data Peek and run its read-only queries.
+    requirePermissions('editor.run'),
+    async (req: AppRequest, res: FastifyReply) => {
+      const body = req.body as ConnectionRef & { schema?: unknown; table?: unknown };
+      const table = typeof body.table === 'string' ? body.table.trim() : '';
+      if (!table) {
+        sendError(res, 'invalid_input', 'table is required.');
+        return;
+      }
+      try {
+        const resolved = await deps.resolveRef(scopeOf(req as AuthedRequest), body);
+        const schema =
+          (typeof body.schema === 'string' && body.schema.trim()) || resolved.schema || '';
+        const probed = await probeTableInsight({
+          dialect: resolved.dialect,
+          option: resolved.option,
+          schema,
+          table,
+        });
+        if (!probed.ok) {
+          const { status, ...rest } = probed.failure;
+          res.status(status).send(rest);
+          return;
+        }
+        res.send({ ...probed.value, dialect: resolved.dialect });
+      } catch (error: unknown) {
+        sendThrown(res, error, 'Failed to load table insight');
+      }
+    }
+  );
+
+
   return router;
 }