fix(sql-editor): critical data-migrate correctness (keys, identity, auth)

apps/web/src/backend/api/routes.ts

@@ -14,6 +14,7 @@ import {
   dialectSupportsIndexFragmentation,
   buildIndexFragmentationCustomTemplate,
   sqlStatementCategories,
+  statementVerb,
   type MigrationStep,
   type ConnectionOptions,
   type DbObjectType,
@@ -521,11 +522,22 @@ export function createApiRoutes(connectionModule: ConnectionModule, connectionSt
     }
     // Grid CRUD also needs the matching Data grid permission so Access control
     // can allow SQL DML without exposing Add/Edit/Delete on Peek / results.
+    // Require the SQL verb to match the claimed action so a client cannot label
+    // datagridAction=insert while sending UPDATE/DELETE (or DDL).
     if (datagridAction !== undefined) {
       if (!isDatagridAction(datagridAction)) {
         res.status(400).json({ error: 'datagridAction must be insert, update, or delete.' });
         return;
       }
+      for (const sql of statements as string[]) {
+        const verb = statementVerb(sql);
+        if (verb !== datagridAction) {
+          res.status(400).json({
+            error: `datagridAction (${datagridAction}) must match SQL verb (${verb ?? 'unknown'}).`,
+          });
+          return;
+        }
+      }
       needed.add(DATAGRID_ACTION_PERMISSION[datagridAction]);
     }
     if (needed.size > 0 && denyUnless(authed, res, ...needed)) return;
@@ -826,10 +838,38 @@ export function createApiRoutes(connectionModule: ConnectionModule, connectionSt
           res.status(400).json({ error: 'Each op needs key and sql.' });
           return;
         }
+        if (o.sql.length > MAX_STATEMENT_LENGTH) {
+          res.status(400).json({ error: `Each op.sql must be under ${MAX_STATEMENT_LENGTH} characters.` });
+          return;
+        }
         if (o.params !== undefined && !Array.isArray(o.params)) {
           res.status(400).json({ error: 'op.params must be an array when set.' });
           return;
         }
+        // Fail-closed like /sql/execute: classify the SQL itself so a client cannot
+        // label op=insert while sending DELETE/DDL/GRANT and bypass finer permissions.
+        const categories = sqlStatementCategories(o.sql);
+        if (categories.length === 0) {
+          res.status(400).json({ error: 'Could not classify op.sql.' });
+          return;
+        }
+        for (const category of categories) {
+          const permission = CATEGORY_PERMISSION[category];
+          if (permission) needed.add(permission);
+          if (category !== 'dml') {
+            res.status(400).json({
+              error: `Data migrate op.sql must be DML (got ${category}).`,
+            });
+            return;
+          }
+        }
+        const verb = statementVerb(o.sql);
+        if (verb !== o.op) {
+          res.status(400).json({
+            error: `op.sql verb (${verb ?? 'unknown'}) must match op (${o.op}).`,
+          });
+          return;
+        }
         needed.add(DATAGRID_ACTION_PERMISSION[o.op]);
         ops.push({
           op: o.op,

apps/web/src/frontend/components/sql-editor/DataMigrateBar.tsx

@@ -6,7 +6,7 @@
  * Side-by-side data migrate: key-based insert/update/delete onto a destination
  * grid (≤500 ops). Larger sets toast with Server Beam instructions.
  */
-import React, { useEffect, useMemo, useState } from 'react';
+import React, { useMemo, useState } from 'react';
 import { createPortal } from 'react-dom';
 import { ArrowRightLeft, History, Loader2, X } from 'lucide-react';
 import {
@@ -23,6 +23,7 @@ import { buildDataMigratePlans, buildDestSnapshotJson } from '../../lib/dataMigr
 import {
   classifyRowsByKey,
   DATA_MIGRATE_ROW_CAP,
+  migrateGridsAreComplete,
   selectMigrateOps,
   type ClassifiedRowDiff,
 } from '../../lib/resultRowDiff';
@@ -42,6 +43,10 @@ export interface DataMigrateGrid {
   columns: string[];
   rows: unknown[][];
   statementSql?: string;
+  /** 0-based page currently shown in the result grid. */
+  pageIndex?: number;
+  /** True when more rows exist beyond this page (hasNext / truncated). */
+  hasMore?: boolean;
 }
 
 type ProgressItem = {
@@ -91,19 +96,16 @@ export const DataMigrateBar: React.FC<Props> = ({
   const table: TableSchema | undefined = editTarget.ok ? editTarget.table : undefined;
   const tableName = table?.name ?? '';
 
-  const defaultKeys = useMemo(
-    () => resolvePeekKeyColumns(table, source.columns).map((k) => k.name),
+  // Only PK / non-partial unique keys that appear in the result — never fall back
+  // to "first column" (non-unique WHERE would UPDATE/DELETE multiple rows).
+  const keyNames = useMemo(
+    () =>
+      resolvePeekKeyColumns(table, source.columns)
+        .filter((k) => k.resultIndex >= 0)
+        .map((k) => k.name),
     [table, source.columns]
   );
 
-  const [keyNamesLocal, setKeyNamesLocal] = useState<string[]>([]);
-  const keyNames = keyNamesProp ?? keyNamesLocal;
-  const setKeyNames = onKeyNamesChange ?? setKeyNamesLocal;
-  useEffect(() => {
-    if (keyNamesProp) return;
-    setKeyNamesLocal(defaultKeys.length ? defaultKeys : source.columns.slice(0, 1));
-  }, [defaultKeys.join('\0'), source.columns.join('\0'), keyNamesProp]);
-
   /** User opts into each op — nothing selected until they choose. */
   const [doInsert, setDoInsert] = useState(false);
   const [doUpdate, setDoUpdate] = useState(false);
@@ -146,13 +148,6 @@ export const DataMigrateBar: React.FC<Props> = ({
     [classification, doInsert, doUpdate, doDelete]
   );
 
-  const toggleKey = (name: string) => {
-    const next = keyNames.some((k) => k.toLowerCase() === name.toLowerCase())
-      ? keyNames.filter((k) => k.toLowerCase() !== name.toLowerCase())
-      : [...keyNames, name];
-    setKeyNames(next);
-  };
-
   const openHistory = async () => {
     setHistoryOpen(true);
     try {
@@ -182,14 +177,49 @@ export const DataMigrateBar: React.FC<Props> = ({
       });
       return;
     }
-    if (keyNames.length === 0) {
-      toast({ tone: 'warning', title: 'Select at least one key column' });
+    if (!editability.editable || keyNames.length === 0) {
+      toast({
+        tone: 'warning',
+        title: 'Data migrate needs a unique key in the result',
+        body:
+          editability.reason ||
+          'Include the primary key (or a non-partial unique index) columns in the SELECT.',
+      });
+      return;
+    }
+    if (
+      !migrateGridsAreComplete({
+        sourcePageIndex: source.pageIndex ?? 0,
+        destPageIndex: dest.pageIndex ?? 0,
+        sourceHasMore: Boolean(source.hasMore),
+        destHasMore: Boolean(dest.hasMore),
+      })
+    ) {
+      toast({
+        tone: 'warning',
+        title: 'Migrate needs the full result on page 1',
+        body:
+          'Add / Edit / Delete classify only the rows currently loaded. ' +
+          'Page both grids to page 1 with no “next page”, or tighten the SELECT ' +
+          `(LIMIT ≤ page size). Otherwise Delete can remove destination rows that ` +
+          'still exist later in the source.',
+      });
       return;
     }
     if (selected.uncappedCount === 0) {
       toast({ tone: 'info', title: 'Nothing to migrate', body: 'Grids match for the selected ops.' });
       return;
     }
+    if (classification.duplicateKeys > 0) {
+      toast({
+        tone: 'warning',
+        title: 'Duplicate keys in result grids',
+        body:
+          `${classification.duplicateKeys} duplicate key value(s) — migrate refuses to guess which row to write. ` +
+          'Tighten the SELECT (DISTINCT / better keys) or pick a unique key set.',
+      });
+      return;
+    }
     if (selected.uncappedCount > DATA_MIGRATE_ROW_CAP) {
       toast({
         tone: 'warning',
@@ -397,17 +427,21 @@ export const DataMigrateBar: React.FC<Props> = ({
 
       <div className="flex flex-wrap items-center gap-x-3 gap-y-1 text-[10px] text-slate-400">
         <span className="text-slate-500">Keys</span>
-        {source.columns.map((c) => (
-          <label key={c} className="inline-flex items-center gap-1 cursor-pointer select-none">
-            <input
-              type="checkbox"
-              checked={keyNames.some((k) => k.toLowerCase() === c.toLowerCase())}
-              onChange={() => toggleKey(c)}
-              className="rounded border-slate-600"
-            />
-            <span className="font-mono">{c}</span>
-          </label>
-        ))}
+        {keyNames.length > 0 ? (
+          keyNames.map((c) => (
+            <span
+              key={c}
+              className="font-mono text-slate-300 rounded border border-slate-700 bg-slate-900/80 px-1.5 py-0.5"
+              title="Primary key / unique index columns from the schema (required for safe row matching)."
+            >
+              {c}
+            </span>
+          ))
+        ) : (
+          <span className="text-amber-400/90">
+            {editability.reason || 'No PK/unique key in this result — migrate disabled.'}
+          </span>
+        )}
       </div>
 
       <div className="flex flex-wrap items-center gap-x-3 gap-y-1 text-[10px] text-slate-400">
@@ -499,6 +533,7 @@ export const DataMigrateBar: React.FC<Props> = ({
             applying ||
             !canDml ||
             selected.uncappedCount === 0 ||
+            classification.duplicateKeys > 0 ||
             selected.uncappedCount > DATA_MIGRATE_ROW_CAP
           }
           onClick={() => void apply()}

apps/web/src/frontend/components/sql-editor/ResultsPanel.tsx

@@ -1057,6 +1057,11 @@ const SideBySideStatementSection: React.FC<{
             columns: sourceGrid.result.columns,
             rows: sourceGrid.result.rows,
             statementSql: sourceGrid.statementSql,
+            pageIndex:
+              pageState?.[`${sourceGrid.connectionId}:${statementIndex}`]?.pageIndex ?? 0,
+            hasMore: Boolean(
+              sourceGrid.result.hasNext || sourceGrid.result.truncated
+            ),
           }}
           dest={{
             connectionId: destGrid.connectionId,
@@ -1065,6 +1070,9 @@ const SideBySideStatementSection: React.FC<{
             columns: destGrid.result.columns,
             rows: destGrid.result.rows,
             statementSql: destGrid.statementSql,
+            pageIndex:
+              pageState?.[`${destGrid.connectionId}:${statementIndex}`]?.pageIndex ?? 0,
+            hasMore: Boolean(destGrid.result.hasNext || destGrid.result.truncated),
           }}
           ignoreColumns={triggerIgnoreColumns}
           keyNames={effectiveKeys}
@@ -1088,10 +1096,9 @@ const SideBySideStatementSection: React.FC<{
       />
       {compareActive && (
         <p className="text-[10px] text-slate-500 px-0.5" data-testid={`sql-result-compare-hint-${statementIndex}`}>
-          {keyAligned
-            ? 'Rows line up by Keys (same as Data migrate). Matching keys share a row; only-here rows show as missing/extra. '
-            : 'No usable key yet — cells align by row index. Pick Keys in Data migrate for friendlier matching. '}
-          Choose Add / Edit / Delete yourself; Transaction and Stop / Continue are safety assists.
+          Cell colors align by row index. Migrate matches rows by PK/unique keys and only runs when
+          both grids show the full result on page 1. Choose Add / Edit / Delete yourself; Transaction
+          and Stop / Continue are safety assists. Skip trigger cols ignores createdAt / updatedBy.
           Cap: 500 ops — larger sets use Server Beam.
         </p>
       )}

apps/web/src/frontend/lib/dataMigratePlans.test.ts

@@ -47,6 +47,27 @@ describe('buildDataMigratePlans', () => {
     expect(parsed.rows).toHaveLength(2);
   });
 
+  it('omits identity columns from INSERT when includeIdentity is false', () => {
+    const { plans, errors } = buildDataMigratePlans({
+      tableName: 'customers',
+      dialect: 'sqlite',
+      sourceColumns: cols,
+      destColumns: cols,
+      keyNames: ['id'],
+      ops: [{ op: 'insert', keyLabel: 'id=3', sourceRow: [3, 'New'] }],
+      includeIdentity: false,
+      identityColumns: new Set(['id']),
+    });
+    expect(errors).toEqual([]);
+    expect(plans).toHaveLength(1);
+    const sql = plans[0]!.plan.sql.toLowerCase();
+    expect(sql).toContain('insert');
+    expect(sql).toContain('name');
+    // Must not bind/preserve the source id when Include identity is off.
+    expect(sql).not.toMatch(/\bid\b/);
+    expect(plans[0]!.plan.params).toEqual(['New']);
+  });
+
   it('omits ignored trigger columns from INSERT and UPDATE SQL', () => {
     const auditCols = ['id', 'name', 'createdAt', 'updatedBy'];
     const auditOps: ClassifiedRowDiff[] = [

apps/web/src/frontend/lib/dataMigratePlans.ts

@@ -97,19 +97,28 @@ export function buildDataMigratePlans(opts: {
   const plans: DataMigratePlanItem[] = [];
   const errors: string[] = [];
 
+  // When Include identity is off, omit identity/autoincrement columns entirely
+  // so the destination generates them. buildPeekInsert only skips *empty*
+  // identity values (Peek UX fills those blank) — source migrate rows always
+  // carry real IDs, so we must strip here or IDs are preserved contrary to UI.
+  const insertIgnore = new Set(ignoreLower);
+  if (!includeIdentity) {
+    for (const name of identityColumns) insertIgnore.add(name.toLowerCase());
+  }
+
   for (const op of ops) {
     if (op.op === 'insert') {
       if (!op.sourceRow) {
         errors.push(`insert ${op.keyLabel}: missing source row`);
         continue;
       }
-      const stripped = stripIgnoredColumns(sourceColumns, op.sourceRow, ignoreLower);
+      const stripped = stripIgnoredColumns(sourceColumns, op.sourceRow, insertIgnore);
       const built = buildPeekInsert({
         tableName,
         dialect,
         values: rowToValues(stripped.columns, stripped.row),
-        // Empty skip-set when includeIdentity — keep source ID values.
-        identityColumns: includeIdentity ? undefined : identityColumns,
+        // Identity already stripped when !includeIdentity; when on, keep values.
+        identityColumns: undefined,
       });
       if ('error' in built) {
         errors.push(`insert ${op.keyLabel}: ${built.error}`);