fix(#496): reject oversized variable option probes

CHANGELOG.md

@@ -10,6 +10,12 @@ auto-generated per-PR notes; this file is the curated, human-readable history.
 ## [Unreleased]
 
 ### Fixed
+- **Dashboard variable option SQL can no longer pass Run with more than 1000
+  rows** (#496). The existing bounded probe still fetches one sentinel row, but
+  Run now rejects that 1001-row response after column-shape validation and
+  before History or detached-result source capture. Zero through 1000 raw rows
+  remain valid, authored `LIMIT` text is not parsed, and transport errors or
+  cancellation keep their original outcome.
 - **In-place authentication recovery no longer compresses the mounted
   workspace** (#512). The preserved editor or Dashboard now stays behind a
   blocking viewport overlay, rather than being pushed below a second,

docs/ADR-0003-dashboard-viewing.md

@@ -897,6 +897,29 @@ batch-level failure.
   Explain button itself stays visible, same as the multi-statement case) and
   is checked first, since option SQL is always one statement.
 
+## Addendum (#496, 2026-07-29): Run consumes the probe's overflow sentinel
+
+The #465 probe already wrapped authored option SQL with
+`LIMIT VARIABLE_OPTION_CAP + 1` and requested the same 1001-row client bound,
+but Run validated only response columns. A query returning the sentinel row
+could therefore be recorded as a successful run even though the Dashboard
+option batch supports at most 1000 options.
+
+- **Raw row count follows successful column-shape validation.**
+  `validateOptionRowCount` is a pure companion to `validateOptionColumns`; the
+  Workbench applies it to `result.rows.length` only when the response completed
+  without a server error or cancellation. Column shape wins when both contracts
+  are invalid, and counting happens before any future filtering or
+  de-duplication.
+- **The authored SQL text is irrelevant to the verdict.** Zero through 1000
+  rows pass whether or not the query contains its own `LIMIT`; the bounded
+  probe's 1001st row fails with the specific maximum-option diagnostic. The
+  batch compiler, its independent per-variable truncation handling, transport
+  bounds, and read-only byte cap are unchanged.
+- **Overflow uses the existing failure gate.** The diagnostic is assigned
+  before the success bookkeeping in `runVariableSql`, so an over-cap response
+  creates neither History nor a detached-result source.
+
 ## Alternatives considered
 
 - **Durable detached snapshots:** rejected because they silently diverge from

src/core/variable-options.ts

@@ -25,9 +25,11 @@
 // metadata describes that query alone — which is what a variable's own
 // main-editor tab and the ordinary Run action are for (#457/#465):
 // `compileOptionProbe` embeds the SQL exactly as a batch branch would (so Run
-// cannot pass what the batch would reject) but drops the branch tag, and
+// cannot pass what the batch would reject) but drops the branch tag;
 // `validateOptionColumns` reads that probe's own, unmerged response metadata —
-// the one place the "exactly two String columns" rule is checkable at all.
+// the one place the "exactly two String columns" rule is checkable at all — and
+// `validateOptionRowCount` rejects the +1 sentinel row before Run can record the
+// response as a success.
 //
 // Deliberately NOT here: cascading/dependent option queries. Option SQL may not
 // reference `{name:Type}` parameters at all in this issue, which is what keeps
@@ -376,6 +378,19 @@ export function validateOptionColumns(
   return null;
 }
 
+/**
+ * Validate the raw row count of a SINGLE variable's successful option-query
+ * probe (#496). The probe is bounded at `VARIABLE_OPTION_CAP + 1`, so the extra
+ * row is a sentinel proving that the authored query exceeded the supported
+ * option count. Call this only after column validation and before any future
+ * filtering or de-duplication.
+ */
+export function validateOptionRowCount(rowCount: number): VariableOptionDiagnostic | null {
+  if (rowCount <= VARIABLE_OPTION_CAP) return null;
+  return diagnostic('variable-option-row-count',
+    `Dashboard variable option SQL may return at most ${VARIABLE_OPTION_CAP} rows.`);
+}
+
 /**
  * Turn one option-batch response into per-variable option lists.
  *

src/ui/workbench/workbench-session.ts

@@ -34,7 +34,7 @@ import { newResult } from '../../core/stream.js';
 import { buildResultSource } from '../../core/query-source.js';
 import {
   VARIABLE_OPTION_BYTE_CAP, VARIABLE_OPTION_CAP,
-  compileOptionProbe, optionSqlDiagnostics, validateOptionColumns,
+  compileOptionProbe, optionSqlDiagnostics, validateOptionColumns, validateOptionRowCount,
 } from '../../core/variable-options.js';
 import type { QueryResult, ScriptResult, ScriptEntry } from '../results.js';
 import type { QueryExecutionService } from '../../application/query-execution-service.js';
@@ -564,11 +564,14 @@ export function createWorkbenchSession(deps: WorkbenchSessionDeps): WorkbenchSes
       });
       // Only the probe's own transport succeeding makes its response metadata
       // meaningful — a transport error or a cancellation already has its own
-      // story and must not be overwritten by a shape verdict about a response
-      // that never fully arrived.
+      // story and must not be overwritten by a verdict about a response that
+      // never fully arrived. Shape is checked before raw row count so a malformed
+      // response retains the more fundamental column diagnostic; the count is
+      // read before any future filtering or de-duplication can hide the sentinel.
       if (isCurrent(registration) && !result.error && !result.cancelled) {
-        const shape = validateOptionColumns(result.columns);
-        if (shape) result.error = shape.message;
+        const invalid = validateOptionColumns(result.columns)
+          ?? validateOptionRowCount(result.rows.length);
+        if (invalid) result.error = invalid.message;
       }
     } finally {
       if (!isCurrent(registration)) {

tests/unit/variable-options.test.ts

@@ -11,6 +11,7 @@ import {
   VARIABLE_OPTION_BYTE_CAP, VARIABLE_OPTION_CAP,
   compileOptionProbe, compileVariableOptionBatch, isOptionColumnType, normalizeOptionSql,
   optionBatchVariables, optionSqlDiagnostics, readVariableOptionBatch, validateOptionColumns,
+  validateOptionRowCount,
 } from '../../src/core/variable-options.js';
 import type { DashboardVariable } from '../../src/core/dashboard-variables.js';
 
@@ -355,6 +356,21 @@ describe('validateOptionColumns', () => {
   });
 });
 
+describe('validateOptionRowCount', () => {
+  it('accepts every supported boundary count', () => {
+    expect(validateOptionRowCount(0)).toBeNull();
+    expect(validateOptionRowCount(VARIABLE_OPTION_CAP)).toBeNull();
+  });
+
+  it('rejects the probe sentinel and any defensive overage with a specific diagnostic', () => {
+    for (const count of [VARIABLE_OPTION_CAP + 1, VARIABLE_OPTION_CAP + 50]) {
+      const found = validateOptionRowCount(count)!;
+      expect(found.code).toBe('variable-option-row-count');
+      expect(found.message).toBe('Dashboard variable option SQL may return at most 1000 rows.');
+    }
+  });
+});
+
 describe('readVariableOptionBatch', () => {
   const cols = [
     { name: '__variable_name', type: 'String' },

tests/unit/workbench-session.test.ts

@@ -906,6 +906,81 @@ describe('createWorkbenchSession: dashboard-variable Run (#465)', () => {
     expect(result?.rows).toEqual([]);
   });
 
+  it('exactly 1000 raw rows pass without requiring an authored LIMIT', async () => {
+    const h = makeHarness({ tab: variableTab({ sqlDraft: 'SELECT a, b FROM t' }) });
+    const rows = Array.from({ length: VARIABLE_OPTION_CAP }, (_, i) => [`v${i}`, `l${i}`]);
+    h.execFakes.executeRead.mockImplementation(async (result: StreamResult) => {
+      Object.assign(result, {
+        columns: [{ name: 'a', type: 'String' }, { name: 'b', type: 'String' }],
+        rows,
+      });
+      return result;
+    });
+    const session = createWorkbenchSession(h.deps);
+    await session.run();
+    const result = h.tab.result as { error: string | null; source?: unknown } | null;
+    expect(result?.error).toBeNull();
+    expect(result?.source).toBeDefined();
+    expect(h.hooks.recordHistory).toHaveBeenCalledWith(h.tab, 'SELECT a, b FROM t');
+  });
+
+  it('an authored LIMIT passes when the actual returned row count is within the cap', async () => {
+    const sql = 'SELECT a, b FROM t LIMIT 5';
+    const h = makeHarness({ tab: variableTab({ sqlDraft: sql }) });
+    h.execFakes.executeRead.mockImplementation(async (result: StreamResult) => {
+      Object.assign(result, {
+        columns: [{ name: 'a', type: 'String' }, { name: 'b', type: 'String' }],
+        rows: [['v', 'l']],
+      });
+      return result;
+    });
+    const session = createWorkbenchSession(h.deps);
+    await session.run();
+    const result = h.tab.result as { error: string | null; source?: unknown } | null;
+    expect(result?.error).toBeNull();
+    expect(result?.source).toBeDefined();
+    expect(h.hooks.recordHistory).toHaveBeenCalledWith(h.tab, sql);
+  });
+
+  it('rejects the 1001-row sentinel by actual count even when the authored SQL has a LIMIT', async () => {
+    const sql = 'SELECT a, b FROM t LIMIT 5000';
+    const h = makeHarness({ tab: variableTab({ sqlDraft: sql }) });
+    const rows = Array.from({ length: VARIABLE_OPTION_CAP + 1 }, (_, i) => [`v${i}`, `l${i}`]);
+    h.execFakes.executeRead.mockImplementation(async (result: StreamResult) => {
+      Object.assign(result, {
+        columns: [{ name: 'a', type: 'String' }, { name: 'b', type: 'String' }],
+        rows,
+      });
+      return result;
+    });
+    const session = createWorkbenchSession(h.deps);
+    await session.run();
+    const req = h.execFakes.executeRead.mock.calls[0][1] as ExecuteReadRequest;
+    expect(req.sql).toContain(sql);
+    const result = h.tab.result as { error: string | null; source?: unknown } | null;
+    expect(result?.error).toBe('Dashboard variable option SQL may return at most 1000 rows.');
+    expect(result?.source).toBeUndefined();
+    expect(h.hooks.recordHistory).not.toHaveBeenCalled();
+    expect(h.hooks.recordBoundParams).not.toHaveBeenCalled();
+    expect(h.tab.lastSuccessfulResultColumns).toEqual([]);
+  });
+
+  it('checks column shape before the raw row count', async () => {
+    const h = makeHarness({ tab: variableTab({ sqlDraft: 'SELECT a FROM t' }) });
+    h.execFakes.executeRead.mockImplementation(async (result: StreamResult) => {
+      Object.assign(result, {
+        columns: [{ name: 'a', type: 'String' }],
+        rows: Array.from({ length: VARIABLE_OPTION_CAP + 1 }, () => ['v']),
+      });
+      return result;
+    });
+    const session = createWorkbenchSession(h.deps);
+    await session.run();
+    const result = h.tab.result as { error: string | null } | null;
+    expect(result?.error).toMatch(/exactly two columns/);
+    expect(result?.error).not.toMatch(/at most 1000 rows/);
+  });
+
   it('a one-column result reports the actual column count', async () => {
     const h = makeHarness({ tab: variableTab({ sqlDraft: 'SELECT a FROM t' }) });
     h.execFakes.executeRead.mockImplementation(async (result: StreamResult) => {
@@ -951,7 +1026,10 @@ describe('createWorkbenchSession: dashboard-variable Run (#465)', () => {
   it('preserves a transport error, never overwritten by shape validation', async () => {
     const h = makeHarness({ tab: variableTab({ sqlDraft: 'SELECT a, b FROM t' }) });
     h.execFakes.executeRead.mockImplementation(async (result: StreamResult) => {
-      result.error = 'Some server error';
+      Object.assign(result, {
+        error: 'Some server error',
+        rows: Array.from({ length: VARIABLE_OPTION_CAP + 1 }, () => ['v', 'l']),
+      });
       return result;
     });
     const session = createWorkbenchSession(h.deps);
@@ -963,7 +1041,10 @@ describe('createWorkbenchSession: dashboard-variable Run (#465)', () => {
   it('preserves cancellation, never overwritten by shape validation', async () => {
     const h = makeHarness({ tab: variableTab({ sqlDraft: 'SELECT a, b FROM t' }) });
     h.execFakes.executeRead.mockImplementation(async (result: StreamResult) => {
-      result.cancelled = true;
+      Object.assign(result, {
+        cancelled: true,
+        rows: Array.from({ length: VARIABLE_OPTION_CAP + 1 }, () => ['v', 'l']),
+      });
       return result;
     });
     const session = createWorkbenchSession(h.deps);