From 54bd6b4d3e33c855f77c4ab6af0378a8e0559459 Mon Sep 17 00:00:00 2001 From: Dan Draper Date: Tue, 18 Aug 2026 14:59:28 +1000 Subject: [PATCH 1/3] stash eql verify: assert the installed EQL surface is complete (#890) A partial EQL install - domains present, some of their comparison functions or operators absent - reported success at install time and failed at query time on a specific predicate (e.g. `weight >= x`). Nothing detected it: isInstalled() is a presence test and `eql validate` checks only the columns an application declared. `stash eql verify` compares what the database actually has against everything the pinned bundle installs - every domain, function overload, operator, cast, and the ORE operator class - via read-only catalog queries. The manifest is parsed out of the bundle itself, so a bundle bump updates the expectation automatically; the bundle's two DO-block conditionals (the ORE opclass and its poison fallback) are modelled explicitly instead. Expected absence reads as such: the ORE opclass skipped on managed Postgres with the loud-failure fallback in place is a supported configuration, not damage. Damage is grouped per-domain, exits 1, and `--json` emits the structured report for agents. A version mismatch with the pinned bundle skips the object diff (wrong manifest to compare against) and suggests `eql upgrade`. `stash eql install` now runs the same check before declaring success. Coverage: unit tests run the parser and differ against the real pinned bundle; a live-Postgres suite (gated on STASH_TEST_DATABASE_URL) installs the bundle, asserts the full surface reads complete with exact counts, then drops an operator and version() and asserts the damage is named and attributed. Claude-Session: https://claude.ai/code/session_01AwM5Cm5ddasXozb6stxPR1 --- .changeset/eql-verify-surface.md | 5 + packages/cli/src/bin/main.ts | 9 + packages/cli/src/cli/registry.ts | 34 + packages/cli/src/commands/db/install.ts | 33 + packages/cli/src/commands/eql/verify.ts | 190 +++++ .../installer/__tests__/verify.live.test.ts | 96 +++ .../src/installer/__tests__/verify.test.ts | 260 +++++++ packages/cli/src/installer/verify.ts | 699 ++++++++++++++++++ .../cli/tests/e2e/command-help.e2e.test.ts | 11 + packages/cli/tests/e2e/smoke.e2e.test.ts | 1 + skills/stash-cli/SKILL.md | 18 +- 11 files changed, 1355 insertions(+), 1 deletion(-) create mode 100644 .changeset/eql-verify-surface.md create mode 100644 packages/cli/src/commands/eql/verify.ts create mode 100644 packages/cli/src/installer/__tests__/verify.live.test.ts create mode 100644 packages/cli/src/installer/__tests__/verify.test.ts create mode 100644 packages/cli/src/installer/verify.ts diff --git a/.changeset/eql-verify-surface.md b/.changeset/eql-verify-surface.md new file mode 100644 index 000000000..330abf7d7 --- /dev/null +++ b/.changeset/eql-verify-surface.md @@ -0,0 +1,5 @@ +--- +'stash': minor +--- + +New `stash eql verify`: assert the installed EQL surface is complete and coherent, independent of any application schema. A partial install — domains present, some of their comparison functions or operators absent — used to report success at install time and fail at query time on a specific predicate (e.g. `weight >= x`); nothing detected it. `eql verify` compares the database against everything the pinned bundle installs (every domain, function overload, operator, cast, and the ORE operator class) via read-only catalog queries, reports damage grouped per domain, and distinguishes expected absence from damage: the ORE operator class being skipped on managed Postgres, with its loud-failure fallback in place, reads as the supported configuration it is rather than a failed install. Exits 1 on damage; `--json` emits the structured report for agents. `stash eql install` now runs the same check automatically before declaring success. diff --git a/packages/cli/src/bin/main.ts b/packages/cli/src/bin/main.ts index 3746c3ca8..d13de446a 100644 --- a/packages/cli/src/bin/main.ts +++ b/packages/cli/src/bin/main.ts @@ -111,6 +111,7 @@ Commands: eql preflight Report whether this database role can install EQL, before trying eql install Scaffold stash.config.ts (if missing) and install EQL extensions + eql verify Check the installed EQL surface is complete (catches partial installs) eql migration Generate an EQL v3 install migration (Drizzle, or supabase/migrations/) eql repair Repair migrations with an un-runnable ALTER COLUMN to an encrypted type eql upgrade Upgrade EQL extensions to the latest version @@ -272,6 +273,14 @@ async function runEqlCommand( case 'install': await runInstall(flags, values) break + case 'verify': { + const { verifyCommand } = await import('../commands/eql/verify.js') + await verifyCommand({ + databaseUrl: values['database-url'], + json: flags.json, + }) + break + } case 'migration': { const { eqlMigrationCommand } = await import( '../commands/eql/migration.js' diff --git a/packages/cli/src/cli/registry.ts b/packages/cli/src/cli/registry.ts index db05cffed..cc33fec57 100644 --- a/packages/cli/src/cli/registry.ts +++ b/packages/cli/src/cli/registry.ts @@ -344,6 +344,40 @@ export const registry: CommandGroup[] = [ DATABASE_URL_FLAG, ], }, + { + name: 'eql verify', + summary: + 'Check the installed EQL surface is complete, not just present', + long: [ + 'Compare what the database actually has against everything the pinned', + 'EQL v3 bundle installs — every domain, function overload, operator,', + 'cast, and the ORE operator class — via read-only catalog queries. A', + 'partial install (domains present, some comparison functions or', + 'operators absent) reports success at install time and fails at query', + 'time on a specific predicate; this is the check that catches it.', + '', + 'Expected absences read as such: on managed Postgres the bundle', + 'legitimately skips the ORE operator class (creating it requires', + 'superuser) and poisons the `_ord_ore` domains to fail loudly — that', + 'is a supported configuration, reported as info. Anything else', + 'missing is a broken install and exits 1.', + '', + 'When the installed EQL version differs from the pinned bundle, the', + 'object-level checks are skipped (the pinned bundle is the wrong', + 'manifest to diff against) and the command suggests `eql upgrade`.', + '', + 'Runs automatically at the end of `stash eql install`.', + ].join('\n'), + examples: ['eql verify', 'eql verify --json'], + flags: [ + { + name: '--json', + description: + 'Emit the machine-readable verification report instead of the table.', + }, + DATABASE_URL_FLAG, + ], + }, { name: 'eql migration', summary: diff --git a/packages/cli/src/commands/db/install.ts b/packages/cli/src/commands/db/install.ts index e9aafad6e..9c122dec7 100644 --- a/packages/cli/src/commands/db/install.ts +++ b/packages/cli/src/commands/db/install.ts @@ -4,6 +4,7 @@ import { resolveDatabaseUrl } from '@/config/database-url.js' import { findConfigFile, loadStashConfig } from '@/config/index.js' import { createPgClient } from '@/db/client.js' import { EQLInstaller } from '@/installer/index.js' +import { verifyEqlSurface } from '@/installer/verify.js' import { messages } from '@/messages.js' import { detectPackageManager, runnerCommand } from '../init/utils.js' import { ensureEncryptionClient } from './client-scaffold.js' @@ -185,6 +186,38 @@ export async function installCommand( s.stop('EQL extensions installed.') if (supabase) reportSupabaseGrantsOutcome(installResult) + // #890: an install can commit and still be incomplete at query time (the + // bundle's own conditional paths, platform quirks, concurrent DDL). Verify + // the surface against the pinned bundle before declaring success — the + // check is a handful of read-only catalog queries. + s.start('Verifying the installed EQL surface...') + try { + const report = await verifyEqlSurface(databaseUrl) + if (report.ok) { + s.stop( + report.ore?.state === 'fallback' + ? 'EQL surface verified (ORE operator class skipped — expected for this role; use the `_ord_ope` ordering domains).' + : 'EQL surface verified — install is complete.', + ) + } else { + s.stop('The install committed but its surface is incomplete.') + const { reportVerifyFindings } = await import('../eql/verify.js') + reportVerifyFindings(report) + p.log.error( + 'Queries against the missing objects will fail. Re-run `stash eql install --force`, and if this persists, please report it: https://github.com/cipherstash/stack/issues', + ) + p.outro('Installation incomplete.') + process.exit(1) + } + } catch (err) { + // A verification error is not an install failure — the install itself + // committed. Say so and keep going. + s.stop('Could not verify the installed EQL surface.') + p.log.warn( + `${err instanceof Error ? err.message : String(err)} — run \`stash eql verify\` to check the install later.`, + ) + } + s.start('Installing cs_migrations tracking schema...') const migrationsDb = createPgClient(databaseUrl) try { diff --git a/packages/cli/src/commands/eql/verify.ts b/packages/cli/src/commands/eql/verify.ts new file mode 100644 index 000000000..1eb2c39b3 --- /dev/null +++ b/packages/cli/src/commands/eql/verify.ts @@ -0,0 +1,190 @@ +import * as p from '@clack/prompts' +import { emitJsonError, emitJsonEvent } from '@/commands/auth/events.js' +import { detectPackageManager, runnerCommand } from '@/commands/init/utils.js' +import { resolveDatabaseUrl } from '@/config/database-url.js' +import { findConfigFile, loadStashConfig } from '@/config/index.js' +import type { SurfaceFinding, VerifyReport } from '@/installer/verify.js' +import { verifyEqlSurface } from '@/installer/verify.js' + +/** + * `stash eql verify` — assert the installed EQL surface is complete and + * coherent, independent of any application schema (#890). A partial install — + * domains present, some supporting functions or operators absent — reports + * success at install time and fails at query time on a specific predicate; + * this is the check that catches it early. + * + * Exit code: 1 when the install is damaged or absent (`status` of + * `incomplete` or `not-installed`), else 0. The ORE operator class being + * absent WITH its loud-failure fallback in place is a supported + * managed-Postgres configuration and reads as such, not as damage. + */ +export async function verifyCommand( + options: { databaseUrl?: string; json?: boolean } = {}, +): Promise { + if (options.json) { + const databaseUrl = await resolveVerifyDatabaseUrl( + options.databaseUrl, + true, + ) + let report: VerifyReport + try { + report = await verifyEqlSurface(databaseUrl) + } catch (error) { + const message = error instanceof Error ? error.message : String(error) + emitJsonError('verify_failed', message) + process.exit(1) + } + emitJsonEvent({ ...report }) + if (!report.ok) process.exit(1) + return + } + + p.intro(runnerCommand(detectPackageManager(), 'stash eql verify')) + + // Resolve the URL before any spinner exists: tier 4 of the resolver is an + // interactive prompt, and a live spinner would redraw over it. + const databaseUrl = await resolveVerifyDatabaseUrl(options.databaseUrl, false) + + const s = p.spinner() + s.start('Comparing the installed EQL surface with the pinned bundle...') + let report: VerifyReport + try { + report = await verifyEqlSurface(databaseUrl) + } catch (error) { + s.stop('Verification failed.') + p.log.error(error instanceof Error ? error.message : String(error)) + p.outro('Verification failed.') + process.exit(1) + } + s.stop('Surface compared.') + + reportVerifyFindings(report) + + switch (report.status) { + case 'complete': + p.outro(`EQL ${report.bundleVersion} install is complete.`) + return + case 'version-mismatch': + p.outro('Surface not verified — version mismatch.') + return + case 'not-installed': + p.outro('EQL is not installed.') + process.exit(1) + break + case 'incomplete': + p.outro('The EQL install is incomplete — see the damage above.') + process.exit(1) + } +} + +/** + * Like preflight, verify must work without a stash.config.ts — fall back to + * the plain DATABASE_URL resolution chain. In json mode the resolver keeps + * stdout parseable (quiet chrome, shared error envelope). + */ +async function resolveVerifyDatabaseUrl( + databaseUrlFlag: string | undefined, + json: boolean, +): Promise { + const configPath = findConfigFile(process.cwd()) + if (configPath) { + const config = await loadStashConfig( + { databaseUrlFlag, quiet: json, jsonErrors: json }, + configPath, + ) + if ( + databaseUrlFlag !== undefined && + config.databaseUrl !== databaseUrlFlag.trim() + ) { + const warning = `Ignoring --database-url: ${configPath} sets an explicit databaseUrl that takes precedence. Verifying the config's database.` + if (json) { + process.stderr.write(`${warning}\n`) + } else { + p.log.warn(warning) + } + } + return config.databaseUrl + } + return resolveDatabaseUrl({ + databaseUrlFlag, + quiet: json, + jsonErrors: json, + }) +} + +/** + * Render a report: the counts note, then damage grouped per-domain — the + * shape the failure arrives in ("`weight >= x` errored") is per-domain, so + * the diagnosis should be too. Exported for reuse by `eql install`'s + * post-install verification. + */ +export function reportVerifyFindings(report: VerifyReport): void { + if (report.counts) { + p.note(renderSurfaceCounts(report), 'EQL surface') + } + + const damage = report.findings.filter( + (finding) => finding.severity === 'damage', + ) + for (const finding of report.findings) { + if (finding.severity === 'warning') p.log.warn(finding.message) + if (finding.severity === 'expected') p.log.info(finding.message) + } + if (damage.length === 0) return + + for (const [domain, messages] of groupByDomain(damage)) { + const capped = messages.slice(0, 10) + const more = + messages.length > capped.length + ? [`… and ${messages.length - capped.length} more`] + : [] + p.log.error( + [domain === undefined ? 'install-wide:' : `${domain}:`] + .concat([...capped, ...more].map((message) => ` - ${message}`)) + .join('\n'), + ) + } +} + +function groupByDomain( + damage: SurfaceFinding[], +): Map { + const groups = new Map() + for (const finding of damage) { + const existing = groups.get(finding.domain) ?? [] + existing.push(finding.message) + groups.set(finding.domain, existing) + } + return groups +} + +/** The aligned counts rows. Exported for unit tests. */ +export function renderSurfaceCounts(report: VerifyReport): string { + const { counts, ore } = report + if (!counts || !ore) return '' + const rows: Array<[string, string]> = [ + ['installed version', report.installedVersion ?? 'missing'], + ['pinned bundle', report.bundleVersion], + ...(['domains', 'types', 'functions', 'operators', 'casts'] as const).map( + (kind): [string, string] => { + const { expected, present } = counts[kind] + return [ + kind, + `${present}/${expected}${present === expected ? '' : ' <- incomplete'}`, + ] + }, + ), + [ + 'ORE operator class', + ore.state === 'indexable' + ? 'present' + : ore.state === 'fallback' + ? 'skipped (expected on managed Postgres)' + : 'INCOHERENT', + ], + ] + const labelWidth = Math.max(...rows.map(([label]) => label.length)) + return rows + .map(([label, value]) => `${label.padEnd(labelWidth)} ${value}`) + .join('\n') +} diff --git a/packages/cli/src/installer/__tests__/verify.live.test.ts b/packages/cli/src/installer/__tests__/verify.live.test.ts new file mode 100644 index 000000000..1fdf579ed --- /dev/null +++ b/packages/cli/src/installer/__tests__/verify.live.test.ts @@ -0,0 +1,96 @@ +/** + * Live-Postgres coverage for `stash eql verify` (#890). + * + * The unit suite proves the parser and the differ against synthetic installed + * states — what it cannot prove is the seam between the two spellings of a + * type: the bundle writes `text[]` and `public.eql_v3_double_ord`, the + * catalog stores `_text` and typname rows, and the OPERATORS_SQL/format_type + * normalisation is what makes them meet. A wrong spelling on either side + * reports thousands of phantom missing operators (or none at all, ever) and + * no fake can catch it. So: install the real pinned bundle, verify it reads + * as complete, then break it surgically and check the damage is named. + * + * Gated on STASH_TEST_DATABASE_URL so the default `pnpm test` stays green + * without a database. Locally: + * + * docker compose -f local/docker-compose.postgres.yml up -d --wait + * export STASH_TEST_DATABASE_URL=postgres://cipherstash:password@localhost:55432/cipherstash + */ + +import { beforeAll, describe, expect, it } from 'vitest' +import { EQLInstaller } from '../index.js' +import { verifyEqlSurface } from '../verify.js' + +const DATABASE_URL = process.env.STASH_TEST_DATABASE_URL +const describeLive = DATABASE_URL ? describe : describe.skip + +async function query(sql: string): Promise { + const { default: pg } = await import('pg') + const client = new pg.Client({ connectionString: DATABASE_URL }) + await client.connect() + try { + await client.query(sql) + } finally { + await client.end().catch(() => undefined) + } +} + +describeLive('verifyEqlSurface — live Postgres', () => { + beforeAll(async () => { + // A real install of the pinned bundle. The docker role is a superuser, so + // the ORE operator class is created and the expected state is + // `indexable`. Re-running over a previous (possibly broken-by-this-suite) + // install is exactly what the bundle supports. + const url = DATABASE_URL ?? '' + await new EQLInstaller({ databaseUrl: url }).install() + }, 180_000) + + it('reads a fresh superuser install as complete', async () => { + const url = DATABASE_URL ?? '' + const report = await verifyEqlSurface(url) + expect(report.findings.filter((f) => f.severity === 'damage')).toEqual([]) + expect(report.status).toBe('complete') + expect(report.ore?.state).toBe('indexable') + // Full-count sanity: the catalog spelling met the bundle spelling for + // every single object, not just most of them. + expect(report.counts?.operators.present).toBe( + report.counts?.operators.expected, + ) + expect(report.counts?.functions.present).toBe( + report.counts?.functions.expected, + ) + expect(report.counts?.domains.present).toBe(report.counts?.domains.expected) + expect(report.counts?.casts.present).toBe(report.counts?.casts.expected) + }, 60_000) + + it('names a dropped comparison operator, attributed to its domain', async () => { + // The failure class from #890: the domain exists, its comparison surface + // does not, and `weight >= x` errors at query time. + await query( + 'DROP OPERATOR >= (public.eql_v3_double_ord, public.eql_v3_double_ord)', + ) + const url = DATABASE_URL ?? '' + const report = await verifyEqlSurface(url) + expect(report.status).toBe('incomplete') + expect(report.ok).toBe(false) + const finding = report.findings.find( + (f) => f.kind === 'operator' && f.severity === 'damage', + ) + expect(finding?.message).toContain( + '>= (public.eql_v3_double_ord, public.eql_v3_double_ord)', + ) + expect(finding?.domain).toBe('eql_v3_double_ord') + }, 60_000) + + it('treats a dropped version() as damage rather than "unknown version"', async () => { + await query('DROP FUNCTION eql_v3.version()') + const url = DATABASE_URL ?? '' + const report = await verifyEqlSurface(url) + expect(report.status).toBe('incomplete') + expect( + report.findings.some( + (f) => f.kind === 'version' && f.severity === 'damage', + ), + ).toBe(true) + }, 60_000) +}) diff --git a/packages/cli/src/installer/__tests__/verify.test.ts b/packages/cli/src/installer/__tests__/verify.test.ts new file mode 100644 index 000000000..08854b648 --- /dev/null +++ b/packages/cli/src/installer/__tests__/verify.test.ts @@ -0,0 +1,260 @@ +import { readInstallSql } from '@cipherstash/eql/sql' +import { describe, expect, it } from 'vitest' +import { + diffSurface, + type ExpectedSurface, + type InstalledSurface, + parseExpectedSurface, +} from '../verify.js' + +/** + * The parser side runs against the REAL pinned bundle, not a fixture: the + * whole point of #890 is that the expected surface tracks what the bundle + * actually installs, so the assertions here pin known members and internal + * consistency rather than a fixture that could drift from the dependency. + */ +const expected = parseExpectedSurface(readInstallSql()) + +describe('parseExpectedSurface (pinned bundle)', () => { + it('names the two EQL schemas', () => { + expect(expected.schemas).toEqual(['eql_v3', 'eql_v3_internal']) + }) + + it('finds the storage domains and their query-operand twins', () => { + // The issue's reported domain, both halves. + expect(expected.domains).toContain('public.eql_v3_double_ord') + expect(expected.domains).toContain('eql_v3.query_double_ord') + // Domains are created inside IF NOT EXISTS DO blocks — a parser that only + // reads column-0 statements finds none of them. + expect(expected.domains.length).toBeGreaterThan(90) + // Every queryable public domain (capability suffix) has a query twin; + // the storage-only base domains (`eql_v3_bigint`, …) do not. + for (const domain of expected.domains) { + const scalar = + /^public\.eql_v3_(.+_(?:eq|ord|ope|ore|match|search))$/.exec(domain) + // `eql_v3_json_search`'s query twin is `query_json` (reached via the + // bundle's cast), not `query_json_search` — skip it here. + if (scalar && scalar[1] !== 'json_search') { + expect(expected.domains).toContain(`eql_v3.query_${scalar[1]}`) + } + } + }) + + it('derives the ORE-carrying domains for the fallback model', () => { + expect(expected.oreDomains).toContain('public.eql_v3_double_ord_ore') + expect(expected.oreDomains).toContain('eql_v3.query_text_search_ore') + expect(expected.oreDomains.every((domain) => domain.endsWith('_ore'))).toBe( + true, + ) + // 9 scalar `_ord_ore` + `text_search_ore`, each with a query twin. + expect(expected.oreDomains).toHaveLength(20) + }) + + it('counts function overloads per name, including quoted names and aggregates', () => { + expect(expected.functions.get('eql_v3.version')).toBe(1) + // The term extractors — one overload per queryable domain. + expect(expected.functions.get('eql_v3.eq_term') ?? 0).toBeGreaterThan(5) + expect(expected.functions.get('eql_v3.ord_term') ?? 0).toBeGreaterThan(5) + // Quoted operator-implementation names are stored unquoted, as pg_proc + // spells them. + expect(expected.functions.get('eql_v3_internal.-') ?? 0).toBeGreaterThan(0) + // Aggregates share pg_proc with functions, so they share the map. + expect(expected.functions.get('eql_v3.min') ?? 0).toBeGreaterThan(0) + expect(expected.functions.get('eql_v3.max') ?? 0).toBeGreaterThan(0) + }) + + it('excludes the bundle-conditional objects (DO-block bodies)', () => { + // Created only when the ORE opclass could NOT be created — expecting it + // unconditionally would report damage on every superuser install. + expect( + expected.functions.has('eql_v3_internal.ore_domain_unavailable'), + ).toBe(false) + }) + + it('extracts operator identities by operand types', () => { + // The exact predicate from #890: `weight >= x` on a double_ord column. + expect(expected.operators).toContain( + '>= (public.eql_v3_double_ord, public.eql_v3_double_ord)', + ) + expect(expected.operators).toContain('>= (public.eql_v3_double_ord, jsonb)') + // text[] and text RHS variants must stay distinct operators. + expect(expected.operators).toContain('- (public.eql_v3_bigint, text)') + expect(expected.operators).toContain('- (public.eql_v3_bigint, text[])') + expect(expected.operators.length).toBeGreaterThan(2000) + }) + + it('extracts the cast', () => { + expect(expected.casts).toContain( + 'public.eql_v3_json_search AS eql_v3.query_json', + ) + }) +}) + +/** A database that has exactly what the bundle installs, superuser flavour. */ +function completeInstall( + surface: ExpectedSurface, + overrides: Partial = {}, +): InstalledSurface { + return { + eqlV3SchemaPresent: true, + eqlV3InternalSchemaPresent: true, + pgcryptoInstalled: true, + installedVersion: surface.eqlVersion, + presentTypes: new Set([...surface.domains, ...surface.types]), + functionCounts: new Map(surface.functions), + presentOperators: new Set(surface.operators), + presentCasts: new Set(surface.casts), + oreOpclassPresent: true, + poisonedDomains: 0, + ...overrides, + } +} + +describe('diffSurface', () => { + it('reports a complete superuser install as complete and ORE-indexable', () => { + const report = diffSurface(expected, completeInstall(expected)) + expect(report.status).toBe('complete') + expect(report.ok).toBe(true) + expect(report.ore?.state).toBe('indexable') + expect(report.findings.filter((f) => f.severity === 'damage')).toHaveLength( + 0, + ) + }) + + it('reads the managed-Postgres ORE skip as expected, not damage', () => { + const report = diffSurface( + expected, + completeInstall(expected, { + oreOpclassPresent: false, + poisonedDomains: expected.oreDomains.length, + }), + ) + expect(report.status).toBe('complete') + expect(report.ore?.state).toBe('fallback') + const skip = report.findings.find((f) => f.kind === 'opclass') + expect(skip?.severity).toBe('expected') + }) + + it('flags an absent opclass with an incomplete poison fallback as damage', () => { + const report = diffSurface( + expected, + completeInstall(expected, { + oreOpclassPresent: false, + poisonedDomains: 0, + }), + ) + expect(report.status).toBe('incomplete') + expect(report.ore?.state).toBe('incoherent-unpoisoned') + }) + + it('flags leftover poison constraints alongside a present opclass as damage', () => { + const report = diffSurface( + expected, + completeInstall(expected, { poisonedDomains: 3 }), + ) + expect(report.status).toBe('incomplete') + expect(report.ore?.state).toBe('incoherent-poisoned') + }) + + it('detects a missing operator and attributes it to its domain', () => { + const operators = new Set(expected.operators) + operators.delete('>= (public.eql_v3_double_ord, public.eql_v3_double_ord)') + const report = diffSurface( + expected, + completeInstall(expected, { presentOperators: operators }), + ) + expect(report.status).toBe('incomplete') + const finding = report.findings.find( + (f) => f.kind === 'operator' && f.severity === 'damage', + ) + expect(finding?.domain).toBe('eql_v3_double_ord') + expect(report.counts?.operators.present).toBe(expected.operators.length - 1) + }) + + it('detects a wholly missing function and a partially missing overload set', () => { + const counts = new Map(expected.functions) + counts.delete('eql_v3.eq_term') + const expectedOrd = expected.functions.get('eql_v3.ord_term') ?? 0 + counts.set('eql_v3.ord_term', expectedOrd - 1) + const report = diffSurface( + expected, + completeInstall(expected, { functionCounts: counts }), + ) + expect(report.status).toBe('incomplete') + const messages = report.findings + .filter((f) => f.kind === 'function') + .map((f) => f.message) + expect( + messages.some((m) => m.includes('`eql_v3.eq_term` is missing')), + ).toBe(true) + expect( + messages.some((m) => + m.includes( + `\`eql_v3.ord_term\` has ${expectedOrd - 1} of ${expectedOrd} expected overloads`, + ), + ), + ).toBe(true) + }) + + it('detects a missing domain', () => { + const types = new Set([...expected.domains, ...expected.types]) + types.delete('public.eql_v3_double_ord') + const report = diffSurface( + expected, + completeInstall(expected, { presentTypes: types }), + ) + expect(report.status).toBe('incomplete') + const finding = report.findings.find((f) => f.kind === 'domain') + expect(finding?.message).toContain('public.eql_v3_double_ord') + expect(finding?.domain).toBe('eql_v3_double_ord') + }) + + it('reports not-installed when the eql_v3 schema is absent', () => { + const report = diffSurface( + expected, + completeInstall(expected, { + eqlV3SchemaPresent: false, + installedVersion: null, + }), + ) + expect(report.status).toBe('not-installed') + expect(report.ok).toBe(false) + }) + + it('skips the object diff on a version mismatch instead of reporting noise', () => { + const report = diffSurface( + expected, + completeInstall(expected, { + installedVersion: '3.0.0', + // Even with everything missing, a different version must not produce + // object-level damage — the pinned bundle is the wrong manifest. + presentOperators: new Set(), + functionCounts: new Map(), + }), + ) + expect(report.status).toBe('version-mismatch') + expect(report.ok).toBe(true) + expect(report.counts).toBeNull() + expect(report.findings).toHaveLength(1) + expect(report.findings[0].message).toContain('stash eql upgrade') + }) + + it('treats a missing version() on a present schema as damage', () => { + const report = diffSurface( + expected, + completeInstall(expected, { installedVersion: null }), + ) + expect(report.status).toBe('incomplete') + const finding = report.findings.find((f) => f.kind === 'version') + expect(finding?.severity).toBe('damage') + }) + + it('treats missing pgcrypto as damage', () => { + const report = diffSurface( + expected, + completeInstall(expected, { pgcryptoInstalled: false }), + ) + expect(report.status).toBe('incomplete') + expect(report.findings.some((f) => f.kind === 'extension')).toBe(true) + }) +}) diff --git a/packages/cli/src/installer/verify.ts b/packages/cli/src/installer/verify.ts new file mode 100644 index 000000000..c87fde92f --- /dev/null +++ b/packages/cli/src/installer/verify.ts @@ -0,0 +1,699 @@ +import { readInstallSql, releaseManifest } from '@cipherstash/eql/sql' +import type pg from 'pg' +import { createPgClient, TlsVerificationError } from '@/db/client.js' +import { EQL_V3_INTERNAL_SCHEMA_NAME, EQL_V3_SCHEMA_NAME } from './grants.js' + +/** + * `stash eql verify` — assert that the installed EQL surface is complete and + * coherent, independent of any application schema (#890). + * + * `isInstalled()` is a presence test (do the two schemas exist) and + * `eql validate` checks the columns an application declared. Neither notices a + * partial install where the domains landed but some of their supporting + * functions or operators did not — that failure mode reports success at + * install time and errors at query time, on a specific predicate. + * + * The manifest of what a complete install looks like is not hand-maintained: + * it is parsed out of the pinned bundle itself ({@link parseExpectedSurface}), + * so a bundle upgrade updates the expectation automatically. The one seam this + * leaves is the bundle's own conditional objects — the ORE operator class the + * bundle skips for non-superusers, and the poison fallback it installs in its + * place — which the parser cannot see (they live inside DO blocks) and the + * differ therefore models explicitly ({@link OreSurfaceState}). + */ + +/** Everything the pinned bundle installs unconditionally. */ +export interface ExpectedSurface { + /** The bundle's own version string (from the release manifest). */ + eqlVersion: string + /** The two EQL schemas. */ + schemas: string[] + /** Qualified domain names, e.g. `public.eql_v3_double_ord`. */ + domains: string[] + /** Qualified composite type names (the ORE term/block types). */ + types: string[] + /** + * Distinct overload count per qualified routine name (functions and + * aggregates share `pg_proc`, so they share this map). Quoted names are + * stored unquoted: `eql_v3_internal.-`, matching `pg_proc.proname`. + */ + functions: Map + /** Operator identities: ` (, )`, lowercase. */ + operators: string[] + /** Casts as ` AS `. */ + casts: string[] + /** + * The ORE-carrying domains (`*_ord_ore` / `*_search_ore`). When the bundle + * cannot create the ORE operator class (non-superuser), it poisons exactly + * these with an always-raising `eql_ore_unavailable` CHECK. + */ + oreDomains: string[] +} + +/** + * How the ORE half of the install reads. Only the first two are healthy: + * the bundle either created the operator class (superuser install) or + * skipped it and poisoned every ORE domain so the gap fails loudly + * (managed-Postgres install — a supported configuration, not damage). + */ +export type OreSurfaceState = + | 'indexable' + | 'fallback' + | 'incoherent-unpoisoned' + | 'incoherent-poisoned' + +export interface SurfaceFinding { + severity: 'damage' | 'warning' | 'expected' + kind: + | 'schema' + | 'extension' + | 'version' + | 'domain' + | 'type' + | 'function' + | 'operator' + | 'cast' + | 'opclass' + /** + * The bare EQL domain this finding concerns (e.g. `eql_v3_double_ord`), + * when one can be attributed — lets the report group per-domain. + */ + domain?: string + message: string +} + +export interface SurfaceCounts { + domains: { expected: number; present: number } + types: { expected: number; present: number } + functions: { expected: number; present: number } + operators: { expected: number; present: number } + casts: { expected: number; present: number } +} + +export interface VerifyReport { + /** + * `complete` — every expected object is present (ORE either indexable or in + * its supported fallback). `incomplete` — damage findings exist. + * `not-installed` — the `eql_v3` schema is absent. `version-mismatch` — an + * older/newer EQL is installed, so the pinned bundle is the wrong manifest + * to diff against and the object-level checks were skipped. + */ + status: 'complete' | 'incomplete' | 'not-installed' | 'version-mismatch' + bundleVersion: string + installedVersion: string | null + counts: SurfaceCounts | null + ore: { + opclassPresent: boolean + poisonedDomains: number + expectedPoisoned: number + state: OreSurfaceState + } | null + findings: SurfaceFinding[] + /** True when no damage was found. */ + ok: boolean +} + +// --------------------------------------------------------------------------- +// Parsing the pinned bundle into an expected surface +// --------------------------------------------------------------------------- + +/** + * Blank out every dollar-quoted body (`$$...$$`, `$tag$...$tag$`) so the + * statement-level regexes below cannot match SQL inside function bodies or DO + * blocks. Newlines are preserved to keep the column-0 anchors meaningful. + * + * This is also what keeps the bundle's conditional objects out of the + * expected set: the ORE operator class/family and the poison fallback are + * created inside DO blocks, so they vanish here and are modelled explicitly + * by the differ instead. + */ +function stripDollarQuoted(sql: string): string { + return sql.replace(/\$([A-Za-z_]*)\$[\s\S]*?\$\1\$/g, (quoted) => + quoted.replace(/[^\n]/g, ' '), + ) +} + +/** `eql_v3_internal."-"` -> `eql_v3_internal.-` (matching pg_catalog names). */ +function unquoteName(raw: string): string { + return raw.replace(/"/g, '').toLowerCase() +} + +/** + * Reduce one routine argument to its type: the bundle names every argument + * (`a public.eql_v3_bigint`, `val jsonb`) and uses no DEFAULTs or arg modes, + * so dropping the first token of a multi-token entry leaves the type — + * including multi-word types, which keep their remaining tokens. A + * single-token entry (aggregate signatures are types-only) is already a type. + */ +function argType(entry: string): string { + const tokens = entry.trim().split(/\s+/) + return (tokens.length > 1 ? tokens.slice(1) : tokens).join(' ').toLowerCase() +} + +/** Parse the pinned install SQL into the surface it creates unconditionally. */ +export function parseExpectedSurface(sql: string): ExpectedSurface { + const stripped = stripDollarQuoted(sql) + + // Domains are created inside `IF NOT EXISTS ... CREATE DOMAIN` DO blocks, + // so they are read from the RAW text (any indentation) and deduped. They + // are unconditional in effect — the guard only makes reinstalls idempotent. + const domains = new Set() + for (const match of sql.matchAll(/^\s*CREATE DOMAIN\s+([\w.]+)\s+AS/gim)) { + domains.add(match[1].toLowerCase()) + } + + const types = new Set() + for (const match of stripped.matchAll(/^CREATE TYPE\s+([\w.]+)\s+AS/gim)) { + types.add(match[1].toLowerCase()) + } + + // Functions and aggregates: count DISTINCT type-signatures per name (the + // bundle re-CREATEs some signatures, which must not inflate the expectation) + // and fold both into one map — they share pg_proc on the observed side. + const signatures = new Map>() + const routinePattern = + /^CREATE (?:OR REPLACE )?(?:FUNCTION|AGGREGATE)\s+((?:[\w]+\.)?(?:"[^"]+"|[\w]+))\s*\(([^)]*)\)/gim + for (const match of stripped.matchAll(routinePattern)) { + const name = unquoteName(match[1]) + const args = match[2].trim() + const signature = args === '' ? '' : args.split(',').map(argType).join(', ') + const existing = signatures.get(name) ?? new Set() + existing.add(signature) + signatures.set(name, existing) + } + const functions = new Map() + for (const [name, sigs] of signatures) functions.set(name, sigs.size) + + // Operators: identity is (name, leftarg, rightarg). Names are created + // unqualified (or explicitly `public.`) — either way they land in `public`, + // so the schema is dropped from the key. + const operators = new Set() + const operatorPattern = /^CREATE OPERATOR\s+([^\s(]+)\s*\(([\s\S]*?)\);/gim + for (const match of stripped.matchAll(operatorPattern)) { + const name = match[1].toLowerCase().replace(/^public\./, '') + const left = /LEFTARG\s*=\s*([\w.[\]]+)/i.exec(match[2])?.[1] ?? 'none' + const right = /RIGHTARG\s*=\s*([\w.[\]]+)/i.exec(match[2])?.[1] ?? 'none' + operators.add(`${name} (${left.toLowerCase()}, ${right.toLowerCase()})`) + } + + const casts = new Set() + for (const match of stripped.matchAll( + /^CREATE CAST\s*\(\s*([\w.[\]]+)\s+AS\s+([\w.[\]]+)\s*\)/gim, + )) { + casts.add(`${match[1].toLowerCase()} AS ${match[2].toLowerCase()}`) + } + + const sortedDomains = [...domains].sort() + return { + eqlVersion: releaseManifest.eqlVersion, + schemas: [EQL_V3_SCHEMA_NAME, EQL_V3_INTERNAL_SCHEMA_NAME], + domains: sortedDomains, + types: [...types].sort(), + functions, + operators: [...operators].sort(), + casts: [...casts].sort(), + oreDomains: sortedDomains.filter((domain) => domain.endsWith('_ore')), + } +} + +/** The expected surface of the pinned bundle this CLI installs. */ +export function bundledExpectedSurface(): ExpectedSurface { + try { + return parseExpectedSurface(readInstallSql()) + } catch (error) { + throw new Error( + 'Failed to read the EQL v3 install SQL from `@cipherstash/eql`. Reinstall dependencies (the package ships the bundle in `dist/sql/`).', + { cause: error }, + ) + } +} + +// --------------------------------------------------------------------------- +// Reading the installed surface +// --------------------------------------------------------------------------- + +export interface InstalledSurface { + eqlV3SchemaPresent: boolean + eqlV3InternalSchemaPresent: boolean + pgcryptoInstalled: boolean + installedVersion: string | null + presentTypes: Set + functionCounts: Map + presentOperators: Set + presentCasts: Set + oreOpclassPresent: boolean + poisonedDomains: number +} + +const SCHEMAS_SQL = ` + SELECT + EXISTS (SELECT 1 FROM pg_catalog.pg_namespace WHERE nspname = '${EQL_V3_SCHEMA_NAME}') AS eql_v3_present, + EXISTS (SELECT 1 FROM pg_catalog.pg_namespace WHERE nspname = '${EQL_V3_INTERNAL_SCHEMA_NAME}') AS eql_v3_internal_present, + EXISTS (SELECT 1 FROM pg_catalog.pg_extension WHERE extname = 'pgcrypto') AS pgcrypto_installed +` + +/** Domains and composite types by qualified name, in one probe. */ +const TYPES_SQL = ` + SELECT n.nspname || '.' || t.typname AS name + FROM pg_catalog.pg_type t + JOIN pg_catalog.pg_namespace n ON n.oid = t.typnamespace + WHERE n.nspname || '.' || t.typname = ANY($1::text[]) +` + +const FUNCTION_COUNTS_SQL = ` + SELECT n.nspname || '.' || p.proname AS name, count(*)::int AS overloads + FROM pg_catalog.pg_proc p + JOIN pg_catalog.pg_namespace n ON n.oid = p.pronamespace + WHERE n.nspname || '.' || p.proname = ANY($1::text[]) + GROUP BY 1 +` + +/** + * Every operator with an EQL operand, in the same ` (, )` + * spelling the bundle parser produces: catalog types via `format_type` (so + * `integer`, `text[]`), everything else as `schema.typname`. Extra operators + * (a user's own) are harmless — the differ only looks for absences. + */ +const OPERATORS_SQL = ` + SELECT o.oprname AS name, + CASE WHEN o.oprleft = 0 THEN 'none' + WHEN ln.nspname = 'pg_catalog' THEN pg_catalog.format_type(o.oprleft, NULL) + ELSE ln.nspname || '.' || lt.typname END AS leftarg, + CASE WHEN o.oprright = 0 THEN 'none' + WHEN rn.nspname = 'pg_catalog' THEN pg_catalog.format_type(o.oprright, NULL) + ELSE rn.nspname || '.' || rt.typname END AS rightarg + FROM pg_catalog.pg_operator o + LEFT JOIN pg_catalog.pg_type lt ON lt.oid = o.oprleft + LEFT JOIN pg_catalog.pg_namespace ln ON ln.oid = lt.typnamespace + LEFT JOIN pg_catalog.pg_type rt ON rt.oid = o.oprright + LEFT JOIN pg_catalog.pg_namespace rn ON rn.oid = rt.typnamespace + WHERE ln.nspname IN ('${EQL_V3_SCHEMA_NAME}', '${EQL_V3_INTERNAL_SCHEMA_NAME}') + OR rn.nspname IN ('${EQL_V3_SCHEMA_NAME}', '${EQL_V3_INTERNAL_SCHEMA_NAME}') + OR lt.typname LIKE 'eql\\_v3\\_%' + OR rt.typname LIKE 'eql\\_v3\\_%' +` + +const CASTS_SQL = ` + SELECT sn.nspname || '.' || st.typname AS source, + tn.nspname || '.' || tt.typname AS target + FROM pg_catalog.pg_cast c + JOIN pg_catalog.pg_type st ON st.oid = c.castsource + JOIN pg_catalog.pg_namespace sn ON sn.oid = st.typnamespace + JOIN pg_catalog.pg_type tt ON tt.oid = c.casttarget + JOIN pg_catalog.pg_namespace tn ON tn.oid = tt.typnamespace + WHERE sn.nspname IN ('public', '${EQL_V3_SCHEMA_NAME}', '${EQL_V3_INTERNAL_SCHEMA_NAME}') + AND tn.nspname IN ('public', '${EQL_V3_SCHEMA_NAME}', '${EQL_V3_INTERNAL_SCHEMA_NAME}') +` + +/** + * The two halves of the bundle's conditional ORE story: the default btree + * operator class over `ore_block_256` (mirrors `eql validate`'s probe — see + * `ORE_AVAILABLE_SQL` there for why `to_regtype`, not a `::regtype` cast), + * and how many domains carry the `eql_ore_unavailable` poison CHECK the + * bundle installs when the class could not be created. + */ +const ORE_STATE_SQL = ` + SELECT + EXISTS ( + SELECT 1 + FROM pg_catalog.pg_opclass c + JOIN pg_catalog.pg_am am ON am.oid = c.opcmethod + WHERE am.amname = 'btree' + AND c.opcdefault + AND c.opcintype = to_regtype('${EQL_V3_INTERNAL_SCHEMA_NAME}.ore_block_256') + ) AS ore_opclass_present, + ( + SELECT count(*)::int + FROM pg_catalog.pg_constraint + WHERE conname = 'eql_ore_unavailable' AND contypid <> 0 + ) AS poisoned_domains +` + +async function readInstalledSurface( + client: pg.ClientBase, + expected: ExpectedSurface, +): Promise { + // Sequential on purpose: a single pg.Client serialises concurrent query() + // calls anyway (and deprecates them); these are six fast catalogue reads. + const schemas = await client.query<{ + eql_v3_present: boolean + eql_v3_internal_present: boolean + pgcrypto_installed: boolean + }>(SCHEMAS_SQL) + const types = await client.query<{ name: string }>(TYPES_SQL, [ + [...expected.domains, ...expected.types], + ]) + const functions = await client.query<{ name: string; overloads: number }>( + FUNCTION_COUNTS_SQL, + [[...expected.functions.keys()]], + ) + const operators = await client.query<{ + name: string + leftarg: string + rightarg: string + }>(OPERATORS_SQL) + const casts = await client.query<{ source: string; target: string }>( + CASTS_SQL, + ) + const ore = await client.query<{ + ore_opclass_present: boolean + poisoned_domains: number + }>(ORE_STATE_SQL) + + const eqlV3SchemaPresent = schemas.rows[0]?.eql_v3_present === true + let installedVersion: string | null = null + if (eqlV3SchemaPresent) { + try { + const version = await client.query<{ version: string }>( + `SELECT ${EQL_V3_SCHEMA_NAME}.version() AS version`, + ) + installedVersion = version.rows[0]?.version ?? null + } catch { + // A missing version() on a present schema is itself reported by the + // differ — the function is part of the expected surface. + } + } + + return { + eqlV3SchemaPresent, + eqlV3InternalSchemaPresent: + schemas.rows[0]?.eql_v3_internal_present === true, + pgcryptoInstalled: schemas.rows[0]?.pgcrypto_installed === true, + installedVersion, + presentTypes: new Set(types.rows.map((row) => row.name.toLowerCase())), + functionCounts: new Map( + functions.rows.map((row) => [row.name.toLowerCase(), row.overloads]), + ), + presentOperators: new Set( + operators.rows.map( + (row) => + `${row.name.toLowerCase()} (${row.leftarg.toLowerCase()}, ${row.rightarg.toLowerCase()})`, + ), + ), + presentCasts: new Set( + casts.rows.map( + (row) => `${row.source.toLowerCase()} AS ${row.target.toLowerCase()}`, + ), + ), + oreOpclassPresent: ore.rows[0]?.ore_opclass_present === true, + poisonedDomains: ore.rows[0]?.poisoned_domains ?? 0, + } +} + +// --------------------------------------------------------------------------- +// Diffing +// --------------------------------------------------------------------------- + +/** + * An attributor from any object identity (qualified name, operator key, + * signature text) to the bare EQL domain it concerns, so the report can group + * per-domain. Longest name wins (`eql_v3_double_ord_ope` before + * `eql_v3_double_ord`), and a domain's query-operand twin + * (`eql_v3.query_double_ord`) attributes to the same bare name. + */ +function domainAttributor( + expected: ExpectedSurface, +): (text: string) => string | undefined { + const bareNames = [ + ...new Set( + expected.domains.map((domain) => + domain + .replace(/^public\./, '') + .replace(/^eql_v3\.query_/, 'eql_v3_') + .replace(/^eql_v3\./, 'eql_v3_'), + ), + ), + ].sort((a, b) => b.length - a.length) + return (text) => { + for (const bare of bareNames) { + if ( + text.includes(bare) || + text.includes(`query_${bare.slice('eql_v3_'.length)}`) + ) { + return bare + } + } + return undefined + } +} + +/** Compare the expected surface with what the database has. Pure. */ +export function diffSurface( + expected: ExpectedSurface, + installed: InstalledSurface, +): VerifyReport { + const findings: SurfaceFinding[] = [] + const domainMentioned = domainAttributor(expected) + + if (!installed.eqlV3SchemaPresent) { + return { + status: 'not-installed', + bundleVersion: expected.eqlVersion, + installedVersion: null, + counts: null, + ore: null, + findings: [ + { + severity: 'damage', + kind: 'schema', + message: `The \`${EQL_V3_SCHEMA_NAME}\` schema does not exist — EQL is not installed. Run \`stash eql install\`.`, + }, + ], + ok: false, + } + } + + if ( + installed.installedVersion !== null && + installed.installedVersion !== expected.eqlVersion + ) { + return { + status: 'version-mismatch', + bundleVersion: expected.eqlVersion, + installedVersion: installed.installedVersion, + counts: null, + ore: null, + findings: [ + { + severity: 'warning', + kind: 'version', + message: `EQL ${installed.installedVersion} is installed, but this CLI pins EQL ${expected.eqlVersion} — the object-level surface checks only know the pinned bundle, so they were skipped. Run \`stash eql upgrade\`, then verify again.`, + }, + ], + ok: true, + } + } + + if (!installed.eqlV3InternalSchemaPresent) { + findings.push({ + severity: 'damage', + kind: 'schema', + message: `The \`${EQL_V3_INTERNAL_SCHEMA_NAME}\` schema is missing.`, + }) + } + if (!installed.pgcryptoInstalled) { + findings.push({ + severity: 'damage', + kind: 'extension', + message: + 'The pgcrypto extension is not installed — every EQL hashing function fails without it.', + }) + } + if (installed.installedVersion === null) { + findings.push({ + severity: 'damage', + kind: 'version', + message: `\`${EQL_V3_SCHEMA_NAME}.version()\` is missing or failed — the bundle always installs it.`, + }) + } + + for (const domain of expected.domains) { + if (!installed.presentTypes.has(domain)) { + findings.push({ + severity: 'damage', + kind: 'domain', + domain: domainMentioned(domain), + message: `Domain \`${domain}\` is missing.`, + }) + } + } + for (const type of expected.types) { + if (!installed.presentTypes.has(type)) { + findings.push({ + severity: 'damage', + kind: 'type', + message: `Type \`${type}\` is missing.`, + }) + } + } + + let functionsPresent = 0 + for (const [name, expectedOverloads] of expected.functions) { + const present = installed.functionCounts.get(name) ?? 0 + functionsPresent += Math.min(present, expectedOverloads) + if (present === 0) { + findings.push({ + severity: 'damage', + kind: 'function', + domain: domainMentioned(name), + message: `Function \`${name}\` is missing (expected ${expectedOverloads} overload${expectedOverloads === 1 ? '' : 's'}).`, + }) + } else if (present < expectedOverloads) { + findings.push({ + severity: 'damage', + kind: 'function', + domain: domainMentioned(name), + message: `Function \`${name}\` has ${present} of ${expectedOverloads} expected overloads.`, + }) + } + } + + let operatorsPresent = 0 + for (const operator of expected.operators) { + if (installed.presentOperators.has(operator)) { + operatorsPresent += 1 + } else { + findings.push({ + severity: 'damage', + kind: 'operator', + domain: domainMentioned(operator), + message: `Operator \`${operator}\` is missing.`, + }) + } + } + + let castsPresent = 0 + for (const cast of expected.casts) { + // The parser spells cast types as the bundle wrote them; the catalog read + // spells them `schema.typname`. Both are qualified, so compare directly. + if (installed.presentCasts.has(cast)) { + castsPresent += 1 + } else { + findings.push({ + severity: 'damage', + kind: 'cast', + message: `Cast \`${cast}\` is missing.`, + }) + } + } + + // The ORE conditional: exactly one of the two halves must be present, in + // full. Anything else is a half-working state the bundle never produces. + const expectedPoisoned = expected.oreDomains.length + let oreState: OreSurfaceState + if (installed.oreOpclassPresent) { + oreState = + installed.poisonedDomains === 0 ? 'indexable' : 'incoherent-poisoned' + } else { + oreState = + installed.poisonedDomains === expectedPoisoned + ? 'fallback' + : 'incoherent-unpoisoned' + } + switch (oreState) { + case 'indexable': + findings.push({ + severity: 'expected', + kind: 'opclass', + message: + 'ORE operator class present — ORE ordered indexes are available (superuser install).', + }) + break + case 'fallback': + findings.push({ + severity: 'expected', + kind: 'opclass', + message: + 'ORE operator class absent, and every ORE domain carries the loud-failure fallback. This is the supported managed-Postgres configuration (creating the class requires superuser), not a failed install — use the `_ord_ope` ordering domains.', + }) + break + case 'incoherent-poisoned': + findings.push({ + severity: 'damage', + kind: 'opclass', + message: `The ORE operator class exists, but ${installed.poisonedDomains} domain${installed.poisonedDomains === 1 ? ' still carries' : 's still carry'} the \`eql_ore_unavailable\` poison CHECK — writes to those domains fail although ORE works. Reinstall with \`stash eql install --force\`.`, + }) + break + case 'incoherent-unpoisoned': + findings.push({ + severity: 'damage', + kind: 'opclass', + message: `The ORE operator class is absent, but only ${installed.poisonedDomains} of ${expectedPoisoned} ORE domains carry the loud-failure fallback — the rest would fail at index/ORDER BY time with opaque errors instead. Reinstall with \`stash eql install --force\`.`, + }) + break + } + + const damaged = findings.some((finding) => finding.severity === 'damage') + return { + status: damaged ? 'incomplete' : 'complete', + bundleVersion: expected.eqlVersion, + installedVersion: installed.installedVersion, + counts: { + domains: { + expected: expected.domains.length, + present: expected.domains.filter((domain) => + installed.presentTypes.has(domain), + ).length, + }, + types: { + expected: expected.types.length, + present: expected.types.filter((type) => + installed.presentTypes.has(type), + ).length, + }, + functions: { + expected: [...expected.functions.values()].reduce( + (sum, count) => sum + count, + 0, + ), + present: functionsPresent, + }, + operators: { + expected: expected.operators.length, + present: operatorsPresent, + }, + casts: { expected: expected.casts.length, present: castsPresent }, + }, + ore: { + opclassPresent: installed.oreOpclassPresent, + poisonedDomains: installed.poisonedDomains, + expectedPoisoned, + state: oreState, + }, + findings, + ok: !damaged, + } +} + +// --------------------------------------------------------------------------- +// Orchestration +// --------------------------------------------------------------------------- + +/** Connect, read, and diff — the whole check, in a handful of catalog reads. */ +export async function verifyEqlSurface( + databaseUrl: string, +): Promise { + const expected = bundledExpectedSurface() + const client = createPgClient(databaseUrl) + try { + await client.connect() + } catch (error) { + await client.end().catch(() => {}) + if (error instanceof TlsVerificationError) throw error + const detail = error instanceof Error ? error.message : String(error) + throw new Error(`Failed to connect to database: ${detail}`, { + cause: error, + }) + } + try { + const installed = await readInstalledSurface(client, expected) + return diffSurface(expected, installed) + } catch (error) { + const detail = error instanceof Error ? error.message : String(error) + throw new Error(`EQL surface verification failed: ${detail}`, { + cause: error, + }) + } finally { + await client.end() + } +} diff --git a/packages/cli/tests/e2e/command-help.e2e.test.ts b/packages/cli/tests/e2e/command-help.e2e.test.ts index ef4b62c85..182082e7d 100644 --- a/packages/cli/tests/e2e/command-help.e2e.test.ts +++ b/packages/cli/tests/e2e/command-help.e2e.test.ts @@ -18,6 +18,7 @@ describe('per-command --help', () => { expect(r.output).toContain('Usage: npx stash eql [options]') expect(r.output).toContain('eql preflight') expect(r.output).toContain('eql install') + expect(r.output).toContain('eql verify') expect(r.output).toContain('eql migration') expect(r.output).toContain('eql repair') expect(r.output).toContain('eql upgrade') @@ -50,6 +51,16 @@ describe('per-command --help', () => { expect(r.output).toContain('--force') }) + it('renders full command help for `eql verify --help`', async () => { + const r = await run(['eql', 'verify', '--help'], { + env: { npm_config_user_agent: '' }, + }) + expect(r.exitCode).toBe(0) + expect(r.output).toContain('Usage: npx stash eql verify [options]') + expect(r.output).toContain('--json') + expect(r.output).toContain('--database-url') + }) + it('renders full command help for `eql preflight --help`', async () => { const r = await run(['eql', 'preflight', '--help'], { env: { npm_config_user_agent: '' }, diff --git a/packages/cli/tests/e2e/smoke.e2e.test.ts b/packages/cli/tests/e2e/smoke.e2e.test.ts index 034658253..2ada4e998 100644 --- a/packages/cli/tests/e2e/smoke.e2e.test.ts +++ b/packages/cli/tests/e2e/smoke.e2e.test.ts @@ -24,6 +24,7 @@ describe('stash CLI — non-interactive smoke', () => { expect(r.output).toContain('init') expect(r.output).toContain('eql preflight') expect(r.output).toContain('eql install') + expect(r.output).toContain('eql verify') expect(r.output).toContain('eql migration') expect(r.output).toContain('eql repair') expect(r.output).toContain('eql upgrade') diff --git a/skills/stash-cli/SKILL.md b/skills/stash-cli/SKILL.md index 8963eee08..3892919aa 100644 --- a/skills/stash-cli/SKILL.md +++ b/skills/stash-cli/SKILL.md @@ -1,6 +1,6 @@ --- name: stash-cli -description: Drive CipherStash setup and encryption migrations through the `stash` CLI — `init`, `plan`, `impl`, `status`, `auth login`, `eql preflight/install/migration/repair/upgrade/status/validate`, `encrypt backfill/drop`, `schema build`, and `manifest --json`. Covers the agent / non-interactive interface, credential rules, and the staged EQL v3 rollout lifecycle. +description: Drive CipherStash setup and encryption migrations through the `stash` CLI — `init`, `plan`, `impl`, `status`, `auth login`, `eql preflight/install/verify/migration/repair/upgrade/status/validate`, `encrypt backfill/drop`, `schema build`, and `manifest --json`. Covers the agent / non-interactive interface, credential rules, and the staged EQL v3 rollout lifecycle. --- # CipherStash CLI (`stash`) @@ -351,6 +351,7 @@ Flags below are the decision-relevant ones. Run `stash --help` for the ```bash stash eql preflight stash eql install +stash eql verify stash eql migration --drizzle stash eql migration --supabase stash eql repair --drizzle @@ -390,6 +391,21 @@ The removed `--eql-version`, `--latest`, `--drizzle`, `--migration`, `--direct`, **`--database-url` is a one-shot.** It installs against that database and leaves the project untouched — no config is loaded, and none is scaffolded, nor is an encryption client. This lets `npx --package=stash@1.0.0 stash eql install --database-url 'postgres://...'` run in a bare project with no CipherStash dependencies while pinning the CLI to this skill's release. It also means the flag always wins: loading a config could pick up a parent-directory `databaseUrl` literal and install against the wrong database. +**The install verifies itself.** `eql install` ends by running the same surface check as `eql verify` (below) and exits 1 if the committed install is incomplete — "install succeeded" now means the full query-time surface is present, not just that the SQL ran. + +#### `eql verify` + +Read-only check that the **installed EQL surface is complete**, independent of any application schema. It compares what the database actually has against everything the pinned bundle installs — every domain, function overload, operator, cast, and the ORE operator class — via catalog queries, and reports damage grouped per domain. This catches the failure `eql validate` cannot: a partial install where the domains exist but some comparison functions or operators do not, so `weight >= x` errors at query time long after "install succeeded". + +Expected absences read as info, not damage: on managed Postgres the bundle legitimately skips the ORE operator class (creating one requires superuser) and poisons the `_ord_ore` domains to fail loudly — `eql verify` reports that as the supported configuration it is. Exits 1 only for genuine damage (missing objects, or an incoherent ORE state) or when EQL is not installed at all. When the installed EQL version differs from the pinned bundle, the object-level diff is skipped (the pinned bundle is the wrong manifest to compare against) and the command suggests `eql upgrade`. + +Run it whenever query-time behaviour looks inconsistent with a "successful" install — e.g. an `operator does not exist` or `function ... does not exist` error naming an `eql_v3` object. + +| Flag | Description | +|---|---| +| `--json` | Machine-readable report. `status` is the discriminator: `complete`, `incomplete` (exit 1), `not-installed` (exit 1), or `version-mismatch`; `findings[]` carries per-object damage with a `domain` attribution | +| `--database-url ` | Verify that database (no config needed). A hand-set literal `databaseUrl` in stash.config.ts still wins, with a warning (stderr in `--json` mode) | + #### `eql migration` Generates an **EQL v3 install migration**, instead of running SQL directly against the database (`eql install`). Migration-first is the preferred path: the install lands in your migration history and ships to every environment through the same migrate step as the rest of your schema. On Supabase it is the *only* durable path — `supabase db reset` replays the migrations directory, so a direct install is wiped by the next reset. v3 only — there is no `--eql-version` here. From b6c0da0f1e496c7240963d7490c163207bd58cb7 Mon Sep 17 00:00:00 2001 From: Dan Draper Date: Tue, 18 Aug 2026 15:49:00 +1000 Subject: [PATCH 2/3] eql verify: address review findings from the two agent passes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Semantics: - A version mismatch now reports ok:false and exits 1 — "could not verify" must never read as "verified", or a `stash eql verify || fail` CI gate passes on a damaged older install. Exit 0 now means exactly one thing: checked and complete. One exit predicate serves both output modes. - `eql install` verifies on the already-installed early exit too, so a plain re-run over a damaged database fails instead of printing "Nothing to do." (isInstalled() is only a presence test). Precision: - Function checks compare type-only signatures, not per-name counts, so a stale same-name function cannot mask a genuinely missing overload. Catalogue spelling comes from format_type() under a pinned empty search_path, which qualifies every non-catalogue type deterministically (and spells composite arrays `ore_block_256_term[]`, not `_ore_block_256_term`). - The ORE poison-constraint count is scoped to the expected ORE domains (constraint names are not globally unique). - pgcrypto is checked for a supported schema, not bare presence, matching the install preflight. - The version() probe distinguishes 42883 (missing — damage) from other errors (EXECUTE denied, timeout — verification failure), instead of a bare catch that produced a phantom full diff. - Operator identity deliberately stays schema-agnostic: the reviewer's suggested public-only scope phantom-fails a healthy install on any database with a "$user" schema (unqualified CREATE OPERATOR follows the install-time search_path) — the live suite runs against exactly such a database and caught it. Robustness / coverage: - The live suite now runs in CI: tests.yml's run-tests job already has a Postgres service, so STASH_TEST_DATABASE_URL points the CLI's .live.test.ts suites at it. The parser<->catalogue spelling seam is no longer guarded only on developer machines. - `--database-url` with a missing value is rejected up front on verify/preflight instead of silently resolving a different database. - `eql verify --database-url` is a one-shot like `eql install`'s: it bypasses config loading, so the database named is the database judged. - The preflight/verify URL resolver is one shared function; a TYPE_ALIASES map absorbs non-canonical spellings a future bundle might use. Docs: stash-cli skill updated for the new exit semantics and one-shot flag; stash-indexing now points its hand-run ORE-state SQL walkthrough at `stash eql verify` and lists the command. Claude-Session: https://claude.ai/code/session_01AwM5Cm5ddasXozb6stxPR1 --- .changeset/eql-verify-surface.md | 2 +- .github/workflows/tests.yml | 15 ++ packages/cli/src/bin/main.ts | 25 ++ packages/cli/src/cli/registry.ts | 22 +- packages/cli/src/commands/db/install.ts | 80 ++++-- packages/cli/src/commands/db/preflight.ts | 44 +-- .../src/commands/db/resolve-diagnostic-url.ts | 62 +++++ packages/cli/src/commands/eql/verify.ts | 132 ++++----- .../src/installer/__tests__/verify.test.ts | 102 +++++-- packages/cli/src/installer/index.ts | 2 +- packages/cli/src/installer/verify.ts | 255 ++++++++++++++---- skills/stash-cli/SKILL.md | 8 +- skills/stash-indexing/SKILL.md | 4 +- 13 files changed, 508 insertions(+), 245 deletions(-) create mode 100644 packages/cli/src/commands/db/resolve-diagnostic-url.ts diff --git a/.changeset/eql-verify-surface.md b/.changeset/eql-verify-surface.md index 330abf7d7..98b61ca87 100644 --- a/.changeset/eql-verify-surface.md +++ b/.changeset/eql-verify-surface.md @@ -2,4 +2,4 @@ 'stash': minor --- -New `stash eql verify`: assert the installed EQL surface is complete and coherent, independent of any application schema. A partial install — domains present, some of their comparison functions or operators absent — used to report success at install time and fail at query time on a specific predicate (e.g. `weight >= x`); nothing detected it. `eql verify` compares the database against everything the pinned bundle installs (every domain, function overload, operator, cast, and the ORE operator class) via read-only catalog queries, reports damage grouped per domain, and distinguishes expected absence from damage: the ORE operator class being skipped on managed Postgres, with its loud-failure fallback in place, reads as the supported configuration it is rather than a failed install. Exits 1 on damage; `--json` emits the structured report for agents. `stash eql install` now runs the same check automatically before declaring success. +New `stash eql verify`: assert the installed EQL surface is complete and coherent, independent of any application schema. A partial install — domains present, some of their comparison functions or operators absent — used to report success at install time and fail at query time on a specific predicate (e.g. `weight >= x`); nothing detected it. `eql verify` compares the database against everything the pinned bundle installs (every domain, function overload, operator, cast, and the ORE operator class) via read-only catalog queries, reports damage grouped per domain, and distinguishes expected absence from damage: the ORE operator class being skipped on managed Postgres, with its loud-failure fallback in place, reads as the supported configuration it is rather than a failed install. Exit 0 means exactly one thing — the surface was checked and found complete; damage, EQL absent, and a version mismatch with the pinned bundle (nothing verifiable) all exit 1. `--json` emits the structured report for agents. `stash eql install` now runs the same check automatically before declaring success, on the fresh-install path and the already-installed early exit alike. diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 5d94ad044..0144def77 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -259,8 +259,23 @@ jobs: echo "DATABASE_URL=postgres://cipherstash:password@localhost:5432/cipherstash" >> ./packages/stack/.env # Run TurboRepo tests + # + # STASH_TEST_DATABASE_URL points the CLI's live-Postgres suites + # (`packages/cli/src/**/__tests__/**.live.test.ts`) at this job's + # service container — without it they self-skip, and the only guard on + # the `stash eql verify` parser↔catalog spelling seam + # (`installer/__tests__/verify.live.test.ts`) would run in no CI + # workflow at all: a routine `@cipherstash/eql` bump could then make + # every `stash eql install` fail with phantom damage, on green CI. + # These suites need Postgres only, no CipherStash credentials; the + # verify suite installs EQL v3 into its own schemas, which coexists + # with the image's pre-installed EQL v2 that the stack tests use. + # (`supabase-push.live.test.ts` gates on different env vars and still + # skips here — it needs the Supabase CLI binary.) - name: Run tests run: pnpm run test + env: + STASH_TEST_DATABASE_URL: postgres://cipherstash:password@localhost:5432/cipherstash # CLI E2E tests drive the built `dist/bin/stash.js` through a real # pseudo-terminal via node-pty. Run via turbo so the `^build` + `build` diff --git a/packages/cli/src/bin/main.ts b/packages/cli/src/bin/main.ts index d13de446a..de5125112 100644 --- a/packages/cli/src/bin/main.ts +++ b/packages/cli/src/bin/main.ts @@ -258,6 +258,29 @@ function rejectRetiredEqlFlags( } } +/** + * `parseArgs` booleanises a `--database-url` whose value is missing (next + * token starts with `-`), so a typo'd `--database-url --json` would silently + * fall back to env/config resolution — and the read-only diagnostics would + * judge a different database than the user targeted. Reject it up front, + * keeping stdout parseable in `--json` mode (same pattern as `stash env`'s + * `nameMissingValue`). + */ +async function rejectMissingDatabaseUrlValue( + flags: Record, +): Promise { + if (flags['database-url'] !== true) return + const message = + '`--database-url` needs a value (e.g. --database-url postgres://...). Without one the command would silently resolve a different database from DATABASE_URL or stash.config.ts.' + if (flags.json) { + const { emitJsonError } = await import('../commands/auth/events.js') + emitJsonError('missing_flag_value', message) + } else { + p.log.error(message) + } + throw new CliExit(1) +} + async function runEqlCommand( sub: string | undefined, flags: Record, @@ -265,6 +288,7 @@ async function runEqlCommand( ) { switch (sub) { case 'preflight': + await rejectMissingDatabaseUrlValue(flags) await preflightCommand({ databaseUrl: values['database-url'], json: flags.json, @@ -274,6 +298,7 @@ async function runEqlCommand( await runInstall(flags, values) break case 'verify': { + await rejectMissingDatabaseUrlValue(flags) const { verifyCommand } = await import('../commands/eql/verify.js') await verifyCommand({ databaseUrl: values['database-url'], diff --git a/packages/cli/src/cli/registry.ts b/packages/cli/src/cli/registry.ts index cc33fec57..da132f4cf 100644 --- a/packages/cli/src/cli/registry.ts +++ b/packages/cli/src/cli/registry.ts @@ -359,14 +359,16 @@ export const registry: CommandGroup[] = [ 'Expected absences read as such: on managed Postgres the bundle', 'legitimately skips the ORE operator class (creating it requires', 'superuser) and poisons the `_ord_ore` domains to fail loudly — that', - 'is a supported configuration, reported as info. Anything else', - 'missing is a broken install and exits 1.', + 'is a supported configuration, reported as info.', '', - 'When the installed EQL version differs from the pinned bundle, the', - 'object-level checks are skipped (the pinned bundle is the wrong', - 'manifest to diff against) and the command suggests `eql upgrade`.', + 'Exit 0 means exactly one thing: the surface was checked and found', + 'complete. Damage, EQL not installed, and a version mismatch with', + 'the pinned bundle all exit 1 — on a mismatch the object-level diff', + 'is skipped (the pinned bundle is the wrong manifest to compare', + 'against) and the command suggests `eql upgrade`.', '', - 'Runs automatically at the end of `stash eql install`.', + 'Runs automatically at the end of `stash eql install`, on the', + 'fresh-install path and the already-installed early exit alike.', ].join('\n'), examples: ['eql verify', 'eql verify --json'], flags: [ @@ -375,7 +377,13 @@ export const registry: CommandGroup[] = [ description: 'Emit the machine-readable verification report instead of the table.', }, - DATABASE_URL_FLAG, + { + name: '--database-url', + value: '', + description: + "One-shot, like `eql install`'s: bypasses config loading entirely, so the database you name is the database that gets judged. Also settable via DATABASE_URL.", + env: 'DATABASE_URL', + }, ], }, { diff --git a/packages/cli/src/commands/db/install.ts b/packages/cli/src/commands/db/install.ts index 9c122dec7..7135c44e5 100644 --- a/packages/cli/src/commands/db/install.ts +++ b/packages/cli/src/commands/db/install.ts @@ -4,7 +4,7 @@ import { resolveDatabaseUrl } from '@/config/database-url.js' import { findConfigFile, loadStashConfig } from '@/config/index.js' import { createPgClient } from '@/db/client.js' import { EQLInstaller } from '@/installer/index.js' -import { verifyEqlSurface } from '@/installer/verify.js' +import { type VerifyReport, verifyEqlSurface } from '@/installer/verify.js' import { messages } from '@/messages.js' import { detectPackageManager, runnerCommand } from '../init/utils.js' import { ensureEncryptionClient } from './client-scaffold.js' @@ -175,6 +175,15 @@ export async function installCommand( s.stop('Supabase role grants applied.') reportSupabaseGrantsOutcome(grantsResult) } + // isInstalled() is a schemas-exist presence test, so this path is + // exactly where a partial install would otherwise slip through: install + // commits, verify fails, and the natural retry — a plain re-run without + // --force — used to print "Nothing to do." and exit 0 on the very + // database it just called damaged. Verify here too. + await verifySurfaceOrExit(databaseUrl, s, { + remedy: + 'The existing install is incomplete — queries against the missing objects will fail. Re-run with `stash eql install --force` to reinstall the bundle.', + }) p.log.info('Use --force to re-run the install script.') p.outro('Nothing to do.') return 'already-installed' @@ -190,33 +199,10 @@ export async function installCommand( // bundle's own conditional paths, platform quirks, concurrent DDL). Verify // the surface against the pinned bundle before declaring success — the // check is a handful of read-only catalog queries. - s.start('Verifying the installed EQL surface...') - try { - const report = await verifyEqlSurface(databaseUrl) - if (report.ok) { - s.stop( - report.ore?.state === 'fallback' - ? 'EQL surface verified (ORE operator class skipped — expected for this role; use the `_ord_ope` ordering domains).' - : 'EQL surface verified — install is complete.', - ) - } else { - s.stop('The install committed but its surface is incomplete.') - const { reportVerifyFindings } = await import('../eql/verify.js') - reportVerifyFindings(report) - p.log.error( - 'Queries against the missing objects will fail. Re-run `stash eql install --force`, and if this persists, please report it: https://github.com/cipherstash/stack/issues', - ) - p.outro('Installation incomplete.') - process.exit(1) - } - } catch (err) { - // A verification error is not an install failure — the install itself - // committed. Say so and keep going. - s.stop('Could not verify the installed EQL surface.') - p.log.warn( - `${err instanceof Error ? err.message : String(err)} — run \`stash eql verify\` to check the install later.`, - ) - } + await verifySurfaceOrExit(databaseUrl, s, { + remedy: + 'Queries against the missing objects will fail. Re-run `stash eql install --force`, and if this persists, please report it: https://github.com/cipherstash/stack/issues', + }) s.start('Installing cs_migrations tracking schema...') const migrationsDb = createPgClient(databaseUrl) @@ -240,6 +226,44 @@ export async function installCommand( return 'installed' } +/** + * The #890 surface check, shared by the fresh-install tail and the + * already-installed path. Exits 1 on damage; a verification ERROR (the + * database dropped the connection, a timeout) is a warning, not a failure — + * the install itself is committed, and `stash eql verify` can re-check later. + */ +async function verifySurfaceOrExit( + databaseUrl: string, + s: ReturnType, + options: { remedy: string }, +): Promise { + s.start('Verifying the installed EQL surface...') + let report: VerifyReport + try { + report = await verifyEqlSurface(databaseUrl) + } catch (err) { + s.stop('Could not verify the installed EQL surface.') + p.log.warn( + `${err instanceof Error ? err.message : String(err)} — run \`stash eql verify\` to check the install later.`, + ) + return + } + if (report.ok) { + s.stop( + report.ore?.state === 'fallback' + ? 'EQL surface verified (ORE operator class skipped — expected for this role; use the `_ord_ope` ordering domains).' + : 'EQL surface verified — install is complete.', + ) + return + } + s.stop('The installed EQL surface is incomplete.') + const { reportVerifyFindings } = await import('../eql/verify.js') + reportVerifyFindings(report) + p.log.error(options.remedy) + p.outro('Installation incomplete.') + process.exit(1) +} + export function prismaNextInstallGuard( cwd: string, options: Pick, diff --git a/packages/cli/src/commands/db/preflight.ts b/packages/cli/src/commands/db/preflight.ts index a98fa9a79..d5b8543bd 100644 --- a/packages/cli/src/commands/db/preflight.ts +++ b/packages/cli/src/commands/db/preflight.ts @@ -1,49 +1,19 @@ import * as p from '@clack/prompts' import { emitJsonError, emitJsonEvent } from '@/commands/auth/events.js' import { detectPackageManager, runnerCommand } from '@/commands/init/utils.js' -import { resolveDatabaseUrl } from '@/config/database-url.js' -import { findConfigFile, loadStashConfig } from '@/config/index.js' import { EQLInstaller, type PreflightResult } from '@/installer/index.js' +import { resolveDiagnosticDatabaseUrl } from './resolve-diagnostic-url.js' -/** - * Preflight runs BEFORE anything is set up, so a missing stash.config.ts must - * not fail it — fall back to the plain DATABASE_URL resolution chain the - * installer itself uses when no config exists yet. In `json` mode the - * resolver keeps stdout parseable: informational chrome and the interactive - * prompt are suppressed (`quiet`) and failures come out as the shared - * `{ status: 'error', code, message }` envelope (`jsonErrors`). - * - * Mirrors `installCommand`'s precedence caveat: a hand-set literal - * `databaseUrl` in stash.config.ts beats `--database-url`. That is surprising - * enough to say out loud — to stderr in json mode, so stdout stays JSON. - */ -async function resolvePreflightDatabaseUrl( +/** See {@link resolveDiagnosticDatabaseUrl} — preflight keeps config-wins. */ +function resolvePreflightDatabaseUrl( databaseUrlFlag: string | undefined, json: boolean, ): Promise { - const configPath = findConfigFile(process.cwd()) - if (configPath) { - const config = await loadStashConfig( - { databaseUrlFlag, quiet: json, jsonErrors: json }, - configPath, - ) - if ( - databaseUrlFlag !== undefined && - config.databaseUrl !== databaseUrlFlag.trim() - ) { - const warning = `Ignoring --database-url: ${configPath} sets an explicit databaseUrl that takes precedence. Probing the config's database.` - if (json) { - process.stderr.write(`${warning}\n`) - } else { - p.log.warn(warning) - } - } - return config.databaseUrl - } - return resolveDatabaseUrl({ + return resolveDiagnosticDatabaseUrl({ databaseUrlFlag, - quiet: json, - jsonErrors: json, + json, + flagWins: false, + verb: 'Probing', }) } diff --git a/packages/cli/src/commands/db/resolve-diagnostic-url.ts b/packages/cli/src/commands/db/resolve-diagnostic-url.ts new file mode 100644 index 000000000..e1629d07e --- /dev/null +++ b/packages/cli/src/commands/db/resolve-diagnostic-url.ts @@ -0,0 +1,62 @@ +import * as p from '@clack/prompts' +import { resolveDatabaseUrl } from '@/config/database-url.js' +import { findConfigFile, loadStashConfig } from '@/config/index.js' + +/** + * The database-URL resolution shared by the read-only diagnostic commands + * (`eql preflight`, `eql verify`). Both run BEFORE anything is set up, so a + * missing stash.config.ts must not fail them — fall back to the plain + * DATABASE_URL resolution chain the installer itself uses when no config + * exists yet. In `json` mode the resolver keeps stdout parseable: + * informational chrome and the interactive prompt are suppressed (`quiet`) + * and failures come out as the shared `{ status: 'error', code, message }` + * envelope (`jsonErrors`). + * + * The two commands differ on one deliberate point, `flagWins`: + * + * - `false` (preflight): mirrors `installCommand`'s config-loading path — a + * hand-set literal `databaseUrl` in stash.config.ts beats `--database-url`. + * Surprising enough to say out loud, to stderr in json mode. + * - `true` (verify): mirrors `installCommand`'s one-shot path — an explicit + * `--database-url` bypasses config loading entirely, so the database the + * user named is the database that gets judged. Verify pairs with one-shot + * installs (`stash eql install --database-url …`), and a config literal + * found up the directory tree silently redirecting the verdict to a + * different database would defeat the flag's whole purpose. + */ +export async function resolveDiagnosticDatabaseUrl(options: { + databaseUrlFlag: string | undefined + json: boolean + flagWins: boolean + /** Present-participle for the config-precedence warning, e.g. `Probing`. */ + verb: string +}): Promise { + const { databaseUrlFlag, json, flagWins, verb } = options + if (flagWins && databaseUrlFlag !== undefined) { + return resolveDatabaseUrl({ + databaseUrlFlag, + quiet: json, + jsonErrors: json, + }) + } + const configPath = findConfigFile(process.cwd()) + if (configPath) { + const config = await loadStashConfig( + { databaseUrlFlag, quiet: json, jsonErrors: json }, + configPath, + ) + if ( + databaseUrlFlag !== undefined && + config.databaseUrl !== databaseUrlFlag.trim() + ) { + const warning = `Ignoring --database-url: ${configPath} sets an explicit databaseUrl that takes precedence. ${verb} the config's database.` + if (json) { + process.stderr.write(`${warning}\n`) + } else { + p.log.warn(warning) + } + } + return config.databaseUrl + } + return resolveDatabaseUrl({ databaseUrlFlag, quiet: json, jsonErrors: json }) +} diff --git a/packages/cli/src/commands/eql/verify.ts b/packages/cli/src/commands/eql/verify.ts index 1eb2c39b3..d323bd75f 100644 --- a/packages/cli/src/commands/eql/verify.ts +++ b/packages/cli/src/commands/eql/verify.ts @@ -1,8 +1,7 @@ import * as p from '@clack/prompts' import { emitJsonError, emitJsonEvent } from '@/commands/auth/events.js' +import { resolveDiagnosticDatabaseUrl } from '@/commands/db/resolve-diagnostic-url.js' import { detectPackageManager, runnerCommand } from '@/commands/init/utils.js' -import { resolveDatabaseUrl } from '@/config/database-url.js' -import { findConfigFile, loadStashConfig } from '@/config/index.js' import type { SurfaceFinding, VerifyReport } from '@/installer/verify.js' import { verifyEqlSurface } from '@/installer/verify.js' @@ -13,103 +12,74 @@ import { verifyEqlSurface } from '@/installer/verify.js' * success at install time and fails at query time on a specific predicate; * this is the check that catches it early. * - * Exit code: 1 when the install is damaged or absent (`status` of - * `incomplete` or `not-installed`), else 0. The ORE operator class being - * absent WITH its loud-failure fallback in place is a supported - * managed-Postgres configuration and reads as such, not as damage. + * ONE exit predicate for both output modes: `report.ok`, true only when the + * surface was checked and found complete. `not-installed`, `incomplete`, and + * `version-mismatch` (checked nothing — "could not verify" must never read as + * "verified") all exit 1. The ORE operator class being absent WITH its + * loud-failure fallback in place is a supported managed-Postgres + * configuration and reads as such, not as damage. + * + * `--database-url` is a one-shot, like `eql install`'s: it bypasses config + * loading, so the database the user named is the database that gets judged + * (see {@link resolveDiagnosticDatabaseUrl}). */ export async function verifyCommand( options: { databaseUrl?: string; json?: boolean } = {}, ): Promise { - if (options.json) { - const databaseUrl = await resolveVerifyDatabaseUrl( - options.databaseUrl, - true, - ) - let report: VerifyReport - try { - report = await verifyEqlSurface(databaseUrl) - } catch (error) { - const message = error instanceof Error ? error.message : String(error) - emitJsonError('verify_failed', message) - process.exit(1) - } - emitJsonEvent({ ...report }) - if (!report.ok) process.exit(1) - return - } + const json = options.json === true - p.intro(runnerCommand(detectPackageManager(), 'stash eql verify')) + if (!json) { + p.intro(runnerCommand(detectPackageManager(), 'stash eql verify')) + } // Resolve the URL before any spinner exists: tier 4 of the resolver is an // interactive prompt, and a live spinner would redraw over it. - const databaseUrl = await resolveVerifyDatabaseUrl(options.databaseUrl, false) + const databaseUrl = await resolveDiagnosticDatabaseUrl({ + databaseUrlFlag: options.databaseUrl, + json, + flagWins: true, + verb: 'Verifying', + }) - const s = p.spinner() - s.start('Comparing the installed EQL surface with the pinned bundle...') + const s = json ? null : p.spinner() + s?.start('Comparing the installed EQL surface with the pinned bundle...') let report: VerifyReport try { report = await verifyEqlSurface(databaseUrl) } catch (error) { - s.stop('Verification failed.') - p.log.error(error instanceof Error ? error.message : String(error)) - p.outro('Verification failed.') + const message = error instanceof Error ? error.message : String(error) + if (json) { + emitJsonError('verify_failed', message) + } else { + s?.stop('Verification failed.') + p.log.error(message) + p.outro('Verification failed.') + } process.exit(1) } - s.stop('Surface compared.') + s?.stop('Surface compared.') - reportVerifyFindings(report) - - switch (report.status) { - case 'complete': - p.outro(`EQL ${report.bundleVersion} install is complete.`) - return - case 'version-mismatch': - p.outro('Surface not verified — version mismatch.') - return - case 'not-installed': - p.outro('EQL is not installed.') - process.exit(1) - break - case 'incomplete': - p.outro('The EQL install is incomplete — see the damage above.') - process.exit(1) - } -} - -/** - * Like preflight, verify must work without a stash.config.ts — fall back to - * the plain DATABASE_URL resolution chain. In json mode the resolver keeps - * stdout parseable (quiet chrome, shared error envelope). - */ -async function resolveVerifyDatabaseUrl( - databaseUrlFlag: string | undefined, - json: boolean, -): Promise { - const configPath = findConfigFile(process.cwd()) - if (configPath) { - const config = await loadStashConfig( - { databaseUrlFlag, quiet: json, jsonErrors: json }, - configPath, - ) - if ( - databaseUrlFlag !== undefined && - config.databaseUrl !== databaseUrlFlag.trim() - ) { - const warning = `Ignoring --database-url: ${configPath} sets an explicit databaseUrl that takes precedence. Verifying the config's database.` - if (json) { - process.stderr.write(`${warning}\n`) - } else { - p.log.warn(warning) - } + if (json) { + emitJsonEvent({ ...report }) + } else { + reportVerifyFindings(report) + switch (report.status) { + case 'complete': + p.outro(`EQL ${report.bundleVersion} install is complete.`) + break + case 'version-mismatch': + p.outro('Surface not verified — version mismatch.') + break + case 'not-installed': + p.outro('EQL is not installed.') + break + case 'incomplete': + p.outro('The EQL install is incomplete — see the damage above.') + break } - return config.databaseUrl } - return resolveDatabaseUrl({ - databaseUrlFlag, - quiet: json, - jsonErrors: json, - }) + + if (!report.ok) process.exit(1) } /** diff --git a/packages/cli/src/installer/__tests__/verify.test.ts b/packages/cli/src/installer/__tests__/verify.test.ts index 08854b648..4087bf765 100644 --- a/packages/cli/src/installer/__tests__/verify.test.ts +++ b/packages/cli/src/installer/__tests__/verify.test.ts @@ -50,17 +50,27 @@ describe('parseExpectedSurface (pinned bundle)', () => { expect(expected.oreDomains).toHaveLength(20) }) - it('counts function overloads per name, including quoted names and aggregates', () => { - expect(expected.functions.get('eql_v3.version')).toBe(1) - // The term extractors — one overload per queryable domain. - expect(expected.functions.get('eql_v3.eq_term') ?? 0).toBeGreaterThan(5) - expect(expected.functions.get('eql_v3.ord_term') ?? 0).toBeGreaterThan(5) + it('collects type-only signatures per name, including quoted names and aggregates', () => { + // Zero-arg signature is the empty string. + expect(expected.functions.get('eql_v3.version')).toEqual(['']) + // The term extractors — one overload per queryable domain, each keyed by + // its argument type so a stale same-name function cannot stand in for it. + const eqTerm = expected.functions.get('eql_v3.eq_term') ?? [] + expect(eqTerm.length).toBeGreaterThan(5) + expect(eqTerm).toContain('public.eql_v3_text_eq') // Quoted operator-implementation names are stored unquoted, as pg_proc // spells them. - expect(expected.functions.get('eql_v3_internal.-') ?? 0).toBeGreaterThan(0) - // Aggregates share pg_proc with functions, so they share the map. - expect(expected.functions.get('eql_v3.min') ?? 0).toBeGreaterThan(0) - expect(expected.functions.get('eql_v3.max') ?? 0).toBeGreaterThan(0) + expect( + (expected.functions.get('eql_v3_internal.-') ?? []).length, + ).toBeGreaterThan(0) + // Aggregates share pg_proc with functions, so they share the map — and + // their argument lists are types-only already. + expect(expected.functions.get('eql_v3.min') ?? []).toContain( + 'public.eql_v3_json_entry', + ) + expect((expected.functions.get('eql_v3.max') ?? []).length).toBeGreaterThan( + 0, + ) }) it('excludes the bundle-conditional objects (DO-block bodies)', () => { @@ -99,9 +109,15 @@ function completeInstall( eqlV3SchemaPresent: true, eqlV3InternalSchemaPresent: true, pgcryptoInstalled: true, + pgcryptoSchema: 'extensions', installedVersion: surface.eqlVersion, presentTypes: new Set([...surface.domains, ...surface.types]), - functionCounts: new Map(surface.functions), + functionSignatures: new Map( + [...surface.functions].map(([name, signatures]) => [ + name, + new Set(signatures), + ]), + ), presentOperators: new Set(surface.operators), presentCasts: new Set(surface.casts), oreOpclassPresent: true, @@ -171,15 +187,13 @@ describe('diffSurface', () => { expect(report.counts?.operators.present).toBe(expected.operators.length - 1) }) - it('detects a wholly missing function and a partially missing overload set', () => { - const counts = new Map(expected.functions) - counts.delete('eql_v3.eq_term') - const expectedOrd = expected.functions.get('eql_v3.ord_term') ?? 0 - counts.set('eql_v3.ord_term', expectedOrd - 1) - const report = diffSurface( - expected, - completeInstall(expected, { functionCounts: counts }), - ) + it('detects a wholly missing function and names a missing overload by signature', () => { + const installed = completeInstall(expected) + installed.functionSignatures.delete('eql_v3.eq_term') + const ordSignatures = installed.functionSignatures.get('eql_v3.ord_term') + expect(ordSignatures?.has('public.eql_v3_text_ord')).toBe(true) + ordSignatures?.delete('public.eql_v3_text_ord') + const report = diffSurface(expected, installed) expect(report.status).toBe('incomplete') const messages = report.findings .filter((f) => f.kind === 'function') @@ -187,10 +201,26 @@ describe('diffSurface', () => { expect( messages.some((m) => m.includes('`eql_v3.eq_term` is missing')), ).toBe(true) + // The missing overload is named by its argument types, not a count. + expect(messages).toContain( + 'Function `eql_v3.ord_term(public.eql_v3_text_ord)` is missing.', + ) + }) + + it('is not fooled by a stale same-name function standing in for a missing overload', () => { + // The count-level trap: 1 current overload removed, 1 impostor added — + // the per-name COUNT is unchanged, but the signature diff still names + // the missing one. This is the #890 false-negative class. + const installed = completeInstall(expected) + const ordSignatures = installed.functionSignatures.get('eql_v3.ord_term') + ordSignatures?.delete('public.eql_v3_text_ord') + ordSignatures?.add('public.some_hand_rolled_type') + const report = diffSurface(expected, installed) + expect(report.status).toBe('incomplete') expect( - messages.some((m) => - m.includes( - `\`eql_v3.ord_term\` has ${expectedOrd - 1} of ${expectedOrd} expected overloads`, + report.findings.some((f) => + f.message.includes( + 'Function `eql_v3.ord_term(public.eql_v3_text_ord)` is missing.', ), ), ).toBe(true) @@ -221,7 +251,7 @@ describe('diffSurface', () => { expect(report.ok).toBe(false) }) - it('skips the object diff on a version mismatch instead of reporting noise', () => { + it('skips the object diff on a version mismatch, and does NOT report ok', () => { const report = diffSurface( expected, completeInstall(expected, { @@ -229,11 +259,13 @@ describe('diffSurface', () => { // Even with everything missing, a different version must not produce // object-level damage — the pinned bundle is the wrong manifest. presentOperators: new Set(), - functionCounts: new Map(), + functionSignatures: new Map(), }), ) expect(report.status).toBe('version-mismatch') - expect(report.ok).toBe(true) + // Nothing was verified, so `ok` must be false — a `verify || fail` gate + // must not pass on an install the command could not actually check. + expect(report.ok).toBe(false) expect(report.counts).toBeNull() expect(report.findings).toHaveLength(1) expect(report.findings[0].message).toContain('stash eql upgrade') @@ -252,9 +284,27 @@ describe('diffSurface', () => { it('treats missing pgcrypto as damage', () => { const report = diffSurface( expected, - completeInstall(expected, { pgcryptoInstalled: false }), + completeInstall(expected, { + pgcryptoInstalled: false, + pgcryptoSchema: null, + }), ) expect(report.status).toBe('incomplete') expect(report.findings.some((f) => f.kind === 'extension')).toBe(true) }) + + it('treats pgcrypto relocated off the EQL search_path as damage', () => { + // Presence alone is not enough — the EQL functions pin + // `search_path = pg_catalog, extensions, public`, so a pgcrypto in any + // other schema fails at runtime (same rule as the install preflight). + const report = diffSurface( + expected, + completeInstall(expected, { pgcryptoSchema: 'crypto_tools' }), + ) + expect(report.status).toBe('incomplete') + const finding = report.findings.find((f) => f.kind === 'extension') + expect(finding?.severity).toBe('damage') + expect(finding?.message).toContain('crypto_tools') + expect(finding?.message).toContain('ALTER EXTENSION pgcrypto SET SCHEMA') + }) }) diff --git a/packages/cli/src/installer/index.ts b/packages/cli/src/installer/index.ts index c8deec5b2..7f09c0f76 100644 --- a/packages/cli/src/installer/index.ts +++ b/packages/cli/src/installer/index.ts @@ -145,7 +145,7 @@ const PREFLIGHT_SQL = ` ` /** The schemas the pinned bundle accepts `pgcrypto` in (its search_path). */ -const SUPPORTED_PGCRYPTO_SCHEMAS = ['extensions', 'public'] +export const SUPPORTED_PGCRYPTO_SCHEMAS = ['extensions', 'public'] export class EQLInstaller { private readonly databaseUrl: string diff --git a/packages/cli/src/installer/verify.ts b/packages/cli/src/installer/verify.ts index c87fde92f..7bb4bf45e 100644 --- a/packages/cli/src/installer/verify.ts +++ b/packages/cli/src/installer/verify.ts @@ -2,6 +2,7 @@ import { readInstallSql, releaseManifest } from '@cipherstash/eql/sql' import type pg from 'pg' import { createPgClient, TlsVerificationError } from '@/db/client.js' import { EQL_V3_INTERNAL_SCHEMA_NAME, EQL_V3_SCHEMA_NAME } from './grants.js' +import { SUPPORTED_PGCRYPTO_SCHEMAS } from './index.js' /** * `stash eql verify` — assert that the installed EQL surface is complete and @@ -33,11 +34,15 @@ export interface ExpectedSurface { /** Qualified composite type names (the ORE term/block types). */ types: string[] /** - * Distinct overload count per qualified routine name (functions and + * Distinct type-only signatures per qualified routine name (functions and * aggregates share `pg_proc`, so they share this map). Quoted names are - * stored unquoted: `eql_v3_internal.-`, matching `pg_proc.proname`. + * stored unquoted: `eql_v3_internal.-`, matching `pg_proc.proname`. Each + * signature is the comma-joined argument type list (`''` for zero-arg), in + * the same spelling {@link InstalledSurface.functionSignatures} produces — + * signatures, not counts, so a stale same-name function cannot mask a + * missing overload. */ - functions: Map + functions: Map /** Operator identities: ` (, )`, lowercase. */ operators: string[] /** Casts as ` AS `. */ @@ -109,7 +114,11 @@ export interface VerifyReport { state: OreSurfaceState } | null findings: SurfaceFinding[] - /** True when no damage was found. */ + /** + * True ONLY when the surface was actually checked and found complete + * (`status: 'complete'`). A `version-mismatch` — where the checks were + * skipped — is NOT ok: "could not verify" must never read as "verified". + */ ok: boolean } @@ -138,6 +147,35 @@ function unquoteName(raw: string): string { return raw.replace(/"/g, '').toLowerCase() } +/** + * SQL type-name aliases mapped to the canonical spelling `format_type()` + * emits, so a future bundle writing `int8` or `timestamptz` still meets the + * catalogue read. The current bundle only uses spellings that are already + * canonical (`integer`, `text[]`, `jsonb`, …) — the live suite is what proves + * that on every run, this map is the cheap safety margin for a bump. + */ +const TYPE_ALIASES: Record = { + int: 'integer', + int2: 'smallint', + int4: 'integer', + int8: 'bigint', + bool: 'boolean', + float4: 'real', + float8: 'double precision', + decimal: 'numeric', + timestamptz: 'timestamp with time zone', + varchar: 'character varying', + char: 'character', +} + +function canonicalType(raw: string): string { + const lower = raw.toLowerCase() + const array = lower.endsWith('[]') + const base = array ? lower.slice(0, -2) : lower + const canonical = TYPE_ALIASES[base] ?? base + return array ? `${canonical}[]` : canonical +} + /** * Reduce one routine argument to its type: the bundle names every argument * (`a public.eql_v3_bigint`, `val jsonb`) and uses no DEFAULTs or arg modes, @@ -147,7 +185,7 @@ function unquoteName(raw: string): string { */ function argType(entry: string): string { const tokens = entry.trim().split(/\s+/) - return (tokens.length > 1 ? tokens.slice(1) : tokens).join(' ').toLowerCase() + return canonicalType((tokens.length > 1 ? tokens.slice(1) : tokens).join(' ')) } /** Parse the pinned install SQL into the surface it creates unconditionally. */ @@ -167,9 +205,10 @@ export function parseExpectedSurface(sql: string): ExpectedSurface { types.add(match[1].toLowerCase()) } - // Functions and aggregates: count DISTINCT type-signatures per name (the - // bundle re-CREATEs some signatures, which must not inflate the expectation) - // and fold both into one map — they share pg_proc on the observed side. + // Functions and aggregates: DISTINCT type-only signatures per name (the + // bundle re-CREATEs some signatures, which must not inflate the + // expectation), folded into one map — they share pg_proc on the observed + // side. const signatures = new Map>() const routinePattern = /^CREATE (?:OR REPLACE )?(?:FUNCTION|AGGREGATE)\s+((?:[\w]+\.)?(?:"[^"]+"|[\w]+))\s*\(([^)]*)\)/gim @@ -181,26 +220,32 @@ export function parseExpectedSurface(sql: string): ExpectedSurface { existing.add(signature) signatures.set(name, existing) } - const functions = new Map() - for (const [name, sigs] of signatures) functions.set(name, sigs.size) - - // Operators: identity is (name, leftarg, rightarg). Names are created - // unqualified (or explicitly `public.`) — either way they land in `public`, - // so the schema is dropped from the key. + const functions = new Map() + for (const [name, sigs] of signatures) functions.set(name, [...sigs].sort()) + + // Operators: identity is (name, leftarg, rightarg) — deliberately WITHOUT + // the operator's schema. Most of the bundle's operators are created + // unqualified, so they land in the first existing schema of the + // install-time search_path: usually `public`, but a "$user" schema named + // after the installing role (common on provisioned databases) legitimately + // captures them instead. Only the six ore_block_256 comparison operators + // are explicitly `public.`-qualified (the opclass block references them by + // that name). Scoping the check to one schema would therefore phantom-fail + // healthy installs — the live suite runs against exactly such a database. const operators = new Set() const operatorPattern = /^CREATE OPERATOR\s+([^\s(]+)\s*\(([\s\S]*?)\);/gim for (const match of stripped.matchAll(operatorPattern)) { const name = match[1].toLowerCase().replace(/^public\./, '') const left = /LEFTARG\s*=\s*([\w.[\]]+)/i.exec(match[2])?.[1] ?? 'none' const right = /RIGHTARG\s*=\s*([\w.[\]]+)/i.exec(match[2])?.[1] ?? 'none' - operators.add(`${name} (${left.toLowerCase()}, ${right.toLowerCase()})`) + operators.add(`${name} (${canonicalType(left)}, ${canonicalType(right)})`) } const casts = new Set() for (const match of stripped.matchAll( /^CREATE CAST\s*\(\s*([\w.[\]]+)\s+AS\s+([\w.[\]]+)\s*\)/gim, )) { - casts.add(`${match[1].toLowerCase()} AS ${match[2].toLowerCase()}`) + casts.add(`${canonicalType(match[1])} AS ${canonicalType(match[2])}`) } const sortedDomains = [...domains].sort() @@ -236,9 +281,12 @@ export interface InstalledSurface { eqlV3SchemaPresent: boolean eqlV3InternalSchemaPresent: boolean pgcryptoInstalled: boolean + /** The schema pgcrypto lives in, `null` when not installed. */ + pgcryptoSchema: string | null installedVersion: string | null presentTypes: Set - functionCounts: Map + /** Type-only argument signatures per qualified routine name. */ + functionSignatures: Map> presentOperators: Set presentCasts: Set oreOpclassPresent: boolean @@ -249,7 +297,10 @@ const SCHEMAS_SQL = ` SELECT EXISTS (SELECT 1 FROM pg_catalog.pg_namespace WHERE nspname = '${EQL_V3_SCHEMA_NAME}') AS eql_v3_present, EXISTS (SELECT 1 FROM pg_catalog.pg_namespace WHERE nspname = '${EQL_V3_INTERNAL_SCHEMA_NAME}') AS eql_v3_internal_present, - EXISTS (SELECT 1 FROM pg_catalog.pg_extension WHERE extname = 'pgcrypto') AS pgcrypto_installed + EXISTS (SELECT 1 FROM pg_catalog.pg_extension WHERE extname = 'pgcrypto') AS pgcrypto_installed, + (SELECT n.nspname FROM pg_catalog.pg_extension e + JOIN pg_catalog.pg_namespace n ON n.oid = e.extnamespace + WHERE e.extname = 'pgcrypto') AS pgcrypto_schema ` /** Domains and composite types by qualified name, in one probe. */ @@ -260,28 +311,44 @@ const TYPES_SQL = ` WHERE n.nspname || '.' || t.typname = ANY($1::text[]) ` -const FUNCTION_COUNTS_SQL = ` - SELECT n.nspname || '.' || p.proname AS name, count(*)::int AS overloads +/** + * One row per overload of an expected routine name, with its input argument + * types rendered by `format_type` — which, under the empty `search_path` the + * surrounding transaction pins (see {@link readInstalledSurface}), emits the + * same spelling the parser produces: catalogue types unqualified (`integer`, + * `text[]`), everything else schema-qualified, arrays with a `[]` suffix + * (`eql_v3_internal.ore_block_256_term[]`, where the raw catalogue row would + * say `_ore_block_256_term`). Signatures rather than counts, so a stale + * same-name function cannot mask a genuinely missing overload. `proargtypes` + * is input arguments only, which is exactly what the parsed + * `CREATE FUNCTION`/`AGGREGATE` argument lists carry. + */ +const FUNCTION_SIGNATURES_SQL = ` + SELECT n.nspname || '.' || p.proname AS name, + COALESCE(( + SELECT string_agg(pg_catalog.format_type(a.oid, NULL), ', ' ORDER BY a.ordinality) + FROM unnest(p.proargtypes) WITH ORDINALITY AS a(oid, ordinality) + ), '') AS signature FROM pg_catalog.pg_proc p JOIN pg_catalog.pg_namespace n ON n.oid = p.pronamespace WHERE n.nspname || '.' || p.proname = ANY($1::text[]) - GROUP BY 1 ` /** - * Every operator with an EQL operand, in the same ` (, )` - * spelling the bundle parser produces: catalog types via `format_type` (so - * `integer`, `text[]`), everything else as `schema.typname`. Extra operators - * (a user's own) are harmless — the differ only looks for absences. + * Every operator with an EQL operand — in ANY schema, for the reason the + * parser comment gives: unqualified `CREATE OPERATOR` follows the + * install-time search_path, so a "$user" schema legitimately holds them. + * Operand types via `format_type` under the pinned empty search_path, so the + * spelling matches the parser's (`integer`, `text[]`, `public.eql_v3_bigint`). + * Extra operators (a user's own) are harmless — the differ only looks for + * absences. */ const OPERATORS_SQL = ` SELECT o.oprname AS name, CASE WHEN o.oprleft = 0 THEN 'none' - WHEN ln.nspname = 'pg_catalog' THEN pg_catalog.format_type(o.oprleft, NULL) - ELSE ln.nspname || '.' || lt.typname END AS leftarg, + ELSE pg_catalog.format_type(o.oprleft, NULL) END AS leftarg, CASE WHEN o.oprright = 0 THEN 'none' - WHEN rn.nspname = 'pg_catalog' THEN pg_catalog.format_type(o.oprright, NULL) - ELSE rn.nspname || '.' || rt.typname END AS rightarg + ELSE pg_catalog.format_type(o.oprright, NULL) END AS rightarg FROM pg_catalog.pg_operator o LEFT JOIN pg_catalog.pg_type lt ON lt.oid = o.oprleft LEFT JOIN pg_catalog.pg_namespace ln ON ln.oid = lt.typnamespace @@ -294,8 +361,8 @@ const OPERATORS_SQL = ` ` const CASTS_SQL = ` - SELECT sn.nspname || '.' || st.typname AS source, - tn.nspname || '.' || tt.typname AS target + SELECT pg_catalog.format_type(c.castsource, NULL) AS source, + pg_catalog.format_type(c.casttarget, NULL) AS target FROM pg_catalog.pg_cast c JOIN pg_catalog.pg_type st ON st.oid = c.castsource JOIN pg_catalog.pg_namespace sn ON sn.oid = st.typnamespace @@ -309,8 +376,12 @@ const CASTS_SQL = ` * The two halves of the bundle's conditional ORE story: the default btree * operator class over `ore_block_256` (mirrors `eql validate`'s probe — see * `ORE_AVAILABLE_SQL` there for why `to_regtype`, not a `::regtype` cast), - * and how many domains carry the `eql_ore_unavailable` poison CHECK the - * bundle installs when the class could not be created. + * and how many of the EXPECTED ORE domains ($1) carry the + * `eql_ore_unavailable` poison CHECK the bundle installs when the class could + * not be created. Scoped to those domains rather than counting by constraint + * name alone — constraint names are not globally unique, so a same-named + * CHECK on an unrelated domain must not flip a healthy install to + * incoherent (or mask a missing poison on a fallback install). */ const ORE_STATE_SQL = ` SELECT @@ -324,8 +395,11 @@ const ORE_STATE_SQL = ` ) AS ore_opclass_present, ( SELECT count(*)::int - FROM pg_catalog.pg_constraint - WHERE conname = 'eql_ore_unavailable' AND contypid <> 0 + FROM pg_catalog.pg_constraint c + JOIN pg_catalog.pg_type t ON t.oid = c.contypid + JOIN pg_catalog.pg_namespace tn ON tn.oid = t.typnamespace + WHERE c.conname = 'eql_ore_unavailable' + AND tn.nspname || '.' || t.typname = ANY($1::text[]) ) AS poisoned_domains ` @@ -335,16 +409,32 @@ async function readInstalledSurface( ): Promise { // Sequential on purpose: a single pg.Client serialises concurrent query() // calls anyway (and deprecates them); these are six fast catalogue reads. + // + // The read-only transaction exists for `SET LOCAL search_path = ''`: + // `format_type` qualifies a name exactly when the type is not visible on + // the search_path, so pinning it empty makes every non-catalogue type come + // out fully qualified (`public.eql_v3_bigint`, + // `eql_v3_internal.ore_block_256_term[]`) while `pg_catalog` — always + // implicitly searched — keeps its canonical unqualified spellings + // (`integer`, `text[]`). That is precisely the spelling the bundle parser + // produces; without the pin the output would vary with the connection's + // search_path. SET LOCAL dies with the transaction, so the caller's + // session is untouched (the version() probe below runs after COMMIT and + // needs the default path restored — `eql_v3.version` is qualified, but its + // body's search_path is its own SET clause either way). + await client.query('BEGIN READ ONLY') + await client.query(`SET LOCAL search_path = ''`) const schemas = await client.query<{ eql_v3_present: boolean eql_v3_internal_present: boolean pgcrypto_installed: boolean + pgcrypto_schema: string | null }>(SCHEMAS_SQL) const types = await client.query<{ name: string }>(TYPES_SQL, [ [...expected.domains, ...expected.types], ]) - const functions = await client.query<{ name: string; overloads: number }>( - FUNCTION_COUNTS_SQL, + const functions = await client.query<{ name: string; signature: string }>( + FUNCTION_SIGNATURES_SQL, [[...expected.functions.keys()]], ) const operators = await client.query<{ @@ -358,7 +448,10 @@ async function readInstalledSurface( const ore = await client.query<{ ore_opclass_present: boolean poisoned_domains: number - }>(ORE_STATE_SQL) + }>(ORE_STATE_SQL, [expected.oreDomains]) + // Ends the SET LOCAL scope. On a mid-transaction error the caller's + // client.end() discards the aborted transaction with the connection. + await client.query('COMMIT') const eqlV3SchemaPresent = schemas.rows[0]?.eql_v3_present === true let installedVersion: string | null = null @@ -368,22 +461,46 @@ async function readInstalledSurface( `SELECT ${EQL_V3_SCHEMA_NAME}.version() AS version`, ) installedVersion = version.rows[0]?.version ?? null - } catch { - // A missing version() on a present schema is itself reported by the - // differ — the function is part of the expected surface. + } catch (error) { + // Only a genuinely absent function reads as "version missing" (the + // differ reports that as damage — the bundle always installs it). + // Anything else — EXECUTE denied, statement_timeout — is a failure to + // verify, not evidence of damage; letting it fall through here would + // produce a full phantom object diff against a healthy install. + const code = + error !== null && typeof error === 'object' && 'code' in error + ? (error as { code?: string }).code + : undefined + if (code !== '42883') { + const detail = error instanceof Error ? error.message : String(error) + throw new Error( + `Could not read ${EQL_V3_SCHEMA_NAME}.version(): ${detail}`, + { cause: error }, + ) + } } } + const functionSignatures = new Map>() + for (const row of functions.rows) { + const name = row.name.toLowerCase() + const existing = functionSignatures.get(name) ?? new Set() + existing.add(row.signature.toLowerCase()) + functionSignatures.set(name, existing) + } + return { eqlV3SchemaPresent, eqlV3InternalSchemaPresent: schemas.rows[0]?.eql_v3_internal_present === true, pgcryptoInstalled: schemas.rows[0]?.pgcrypto_installed === true, + pgcryptoSchema: + typeof schemas.rows[0]?.pgcrypto_schema === 'string' + ? schemas.rows[0].pgcrypto_schema + : null, installedVersion, presentTypes: new Set(types.rows.map((row) => row.name.toLowerCase())), - functionCounts: new Map( - functions.rows.map((row) => [row.name.toLowerCase(), row.overloads]), - ), + functionSignatures, presentOperators: new Set( operators.rows.map( (row) => @@ -480,7 +597,10 @@ export function diffSurface( message: `EQL ${installed.installedVersion} is installed, but this CLI pins EQL ${expected.eqlVersion} — the object-level surface checks only know the pinned bundle, so they were skipped. Run \`stash eql upgrade\`, then verify again.`, }, ], - ok: true, + // NOT ok: nothing was verified. `ok` must mean "checked and complete" — + // an exit-0 here would let `stash eql verify || fail` pass on a damaged + // older install, the command's headline scenario. + ok: false, } } @@ -498,6 +618,18 @@ export function diffSurface( message: 'The pgcrypto extension is not installed — every EQL hashing function fails without it.', }) + } else if ( + installed.pgcryptoSchema !== null && + !SUPPORTED_PGCRYPTO_SCHEMAS.includes(installed.pgcryptoSchema) + ) { + // Same rule as the install preflight: the EQL functions' pinned + // search_path only resolves pgcrypto from these schemas, so presence + // alone is not enough — a relocated extension fails at runtime. + findings.push({ + severity: 'damage', + kind: 'extension', + message: `pgcrypto is installed in schema "${installed.pgcryptoSchema}", which is not on the EQL search_path — every EQL hashing function fails at runtime. Fix with: ALTER EXTENSION pgcrypto SET SCHEMA extensions`, + }) } if (installed.installedVersion === null) { findings.push({ @@ -527,24 +659,31 @@ export function diffSurface( } } + // Signature-level, not count-level: a stale or hand-created same-name + // function must not mask a genuinely missing overload. let functionsPresent = 0 - for (const [name, expectedOverloads] of expected.functions) { - const present = installed.functionCounts.get(name) ?? 0 - functionsPresent += Math.min(present, expectedOverloads) - if (present === 0) { - findings.push({ - severity: 'damage', - kind: 'function', - domain: domainMentioned(name), - message: `Function \`${name}\` is missing (expected ${expectedOverloads} overload${expectedOverloads === 1 ? '' : 's'}).`, - }) - } else if (present < expectedOverloads) { + for (const [name, expectedSignatures] of expected.functions) { + const present = installed.functionSignatures.get(name) ?? new Set() + const missing = expectedSignatures.filter( + (signature) => !present.has(signature), + ) + functionsPresent += expectedSignatures.length - missing.length + if (missing.length === expectedSignatures.length) { findings.push({ severity: 'damage', kind: 'function', domain: domainMentioned(name), - message: `Function \`${name}\` has ${present} of ${expectedOverloads} expected overloads.`, + message: `Function \`${name}\` is missing (expected ${expectedSignatures.length} overload${expectedSignatures.length === 1 ? '' : 's'}).`, }) + } else { + for (const signature of missing) { + findings.push({ + severity: 'damage', + kind: 'function', + domain: domainMentioned(`${name}(${signature})`), + message: `Function \`${name}(${signature})\` is missing.`, + }) + } } } @@ -643,7 +782,7 @@ export function diffSurface( }, functions: { expected: [...expected.functions.values()].reduce( - (sum, count) => sum + count, + (sum, signatures) => sum + signatures.length, 0, ), present: functionsPresent, diff --git a/skills/stash-cli/SKILL.md b/skills/stash-cli/SKILL.md index 3892919aa..c2fe5f68f 100644 --- a/skills/stash-cli/SKILL.md +++ b/skills/stash-cli/SKILL.md @@ -391,20 +391,20 @@ The removed `--eql-version`, `--latest`, `--drizzle`, `--migration`, `--direct`, **`--database-url` is a one-shot.** It installs against that database and leaves the project untouched — no config is loaded, and none is scaffolded, nor is an encryption client. This lets `npx --package=stash@1.0.0 stash eql install --database-url 'postgres://...'` run in a bare project with no CipherStash dependencies while pinning the CLI to this skill's release. It also means the flag always wins: loading a config could pick up a parent-directory `databaseUrl` literal and install against the wrong database. -**The install verifies itself.** `eql install` ends by running the same surface check as `eql verify` (below) and exits 1 if the committed install is incomplete — "install succeeded" now means the full query-time surface is present, not just that the SQL ran. +**The install verifies itself.** `eql install` ends by running the same surface check as `eql verify` (below) — on the fresh-install path *and* on the already-installed early exit, so a plain re-run over a damaged database fails rather than printing "Nothing to do." It exits 1 if the surface is incomplete; if the check itself cannot run (connection dropped mid-verify), it warns and points at `stash eql verify` instead of failing the committed install. #### `eql verify` Read-only check that the **installed EQL surface is complete**, independent of any application schema. It compares what the database actually has against everything the pinned bundle installs — every domain, function overload, operator, cast, and the ORE operator class — via catalog queries, and reports damage grouped per domain. This catches the failure `eql validate` cannot: a partial install where the domains exist but some comparison functions or operators do not, so `weight >= x` errors at query time long after "install succeeded". -Expected absences read as info, not damage: on managed Postgres the bundle legitimately skips the ORE operator class (creating one requires superuser) and poisons the `_ord_ore` domains to fail loudly — `eql verify` reports that as the supported configuration it is. Exits 1 only for genuine damage (missing objects, or an incoherent ORE state) or when EQL is not installed at all. When the installed EQL version differs from the pinned bundle, the object-level diff is skipped (the pinned bundle is the wrong manifest to compare against) and the command suggests `eql upgrade`. +Expected absences read as info, not damage: on managed Postgres the bundle legitimately skips the ORE operator class (creating one requires superuser) and poisons the `_ord_ore` domains to fail loudly — `eql verify` reports that as the supported configuration it is. Exit 0 means exactly one thing: the surface was checked and found complete. Genuine damage, EQL not installed, and a version mismatch with the pinned bundle all exit 1 — on a mismatch the object-level diff is skipped (the pinned bundle is the wrong manifest to compare against) and the command suggests `eql upgrade`; "could not verify" never reads as "verified". Run it whenever query-time behaviour looks inconsistent with a "successful" install — e.g. an `operator does not exist` or `function ... does not exist` error naming an `eql_v3` object. | Flag | Description | |---|---| -| `--json` | Machine-readable report. `status` is the discriminator: `complete`, `incomplete` (exit 1), `not-installed` (exit 1), or `version-mismatch`; `findings[]` carries per-object damage with a `domain` attribution | -| `--database-url ` | Verify that database (no config needed). A hand-set literal `databaseUrl` in stash.config.ts still wins, with a warning (stderr in `--json` mode) | +| `--json` | Machine-readable report. `status` is the discriminator: `complete` (the only exit-0 status), `incomplete`, `not-installed`, or `version-mismatch`; `findings[]` carries per-object damage with a `domain` attribution | +| `--database-url ` | One-shot, like `eql install`'s: bypasses config loading entirely, so the database you name is the database that gets judged | #### `eql migration` diff --git a/skills/stash-indexing/SKILL.md b/skills/stash-indexing/SKILL.md index e64782418..62a8c5063 100644 --- a/skills/stash-indexing/SKILL.md +++ b/skills/stash-indexing/SKILL.md @@ -130,7 +130,7 @@ SELECT * FROM orders ORDER BY eql_v3.ord_term(data_encrypted -> ''::te The `_ord_ore` restriction, precisely: its btree ordering depends on a hand-written operator class created by the EQL installer, and `CREATE OPERATOR CLASS` is a superuser-gated command in stock PostgreSQL. Whether that blocks ORE is per-platform, not a blanket managed-Postgres rule: **AWS RDS and Aurora fully support it** (their master role can create operator classes), while **cloud-hosted Supabase is the one confirmed platform that refuses it**. Where the install role can't create the opclass, the installer detects this and **disables the `_ord_ore` domains** — using one raises `feature_not_supported` with a hint naming the alternatives. -**The silent-failure mode to check for:** if an `_ord_ore` column somehow exists without the opclass, `CREATE INDEX … USING btree (eql_v3.ord_term_ore(col))` does **not** fail — PostgreSQL binds the generic `record_ops` instead. The index builds, occupies space, and never engages. Verify which opclass an ORE index actually bound: +**The silent-failure mode to check for:** if an `_ord_ore` column somehow exists without the opclass, `CREATE INDEX … USING btree (eql_v3.ord_term_ore(col))` does **not** fail — PostgreSQL binds the generic `record_ops` instead. The index builds, occupies space, and never engages. Run `stash eql verify` first: it reads the ORE state directly and distinguishes the two healthy configurations (opclass present, or opclass skipped with every `_ord_ore` domain disabled) from the incoherent half-working state that makes this trap possible — and `stash eql install` runs the same check automatically. What `verify` does not tell you is which opclass an *existing index* bound at build time; for that, check the index itself: ```sql SELECT i.relname, oc.opcname @@ -269,7 +269,7 @@ Index not being used: ## Reference - `stash-encryption` — the `types.*` domain catalog, wire-format operators and ordering, and the staged rollout lifecycle. -- `stash-cli` — `stash eql install`, `stash eql validate` (its "No functional index over `eql_v3.…`" Info finding is resolved by this skill), and `stash encrypt backfill` / `drop`. +- `stash-cli` — `stash eql install`, `stash eql verify` (is the installed operator/opclass surface complete and the ORE state coherent), `stash eql validate` (its "No functional index over `eql_v3.…`" Info finding is resolved by this skill), and `stash encrypt backfill` / `drop`. - `stash-drizzle`, `stash-supabase`, `stash-prisma` — per-integration query patterns; index DDL placement per the section above. - `stash-postgres` — the hand-written predicate forms these indexes serve (`pg` / `postgres-js`, no ORM). - `stash-edge` — the WASM entry, for apps whose queries run on Deno / Workers / Supabase Edge Functions. From 4ae44cb00cd9e8171d54b123b1f187d2169ccd4b Mon Sep 17 00:00:00 2001 From: Dan Draper Date: Tue, 18 Aug 2026 18:20:02 +1000 Subject: [PATCH 3/3] Address tobyhede's review: install stays idempotent on version skew, live suites serialised, --database-url guard global MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - `stash eql install`'s surface check no longer exits 1 on a version mismatch: `ok: false` there means "nothing was checked", and the pre-verification behaviour of a no-op re-run over an older EQL was exit 0 — idempotent provisioning scripts and `stash init`'s direct-install route depend on that. Damage still fails the install; `stash eql verify` keeps its strict gate. The mismatch finding now also names `eql install --force --database-url ...` as the remedy that works without a stash.config.ts (`eql upgrade` requires one). New unit suite covers all four verifySurfaceOrExit outcomes. - The CLI vitest config now splits into `unit` and `live` projects, with `fileParallelism: false` on `live` only: four live suites share one database, and verify.live's bundle install opens with DROP SCHEMA ... CASCADE, which raced destructively under guarded-grants.live in parallel forks. The ~1300 unit tests keep their parallelism. verify.live also gained an afterAll reinstall so its surgical damage does not outlive the file. - The valueless `--database-url` rejection moved from the two diagnostic commands to the top of dispatch(), covering every subcommand — most importantly `eql install --force`, where the silent fallback to DATABASE_URL meant dropping and reinstalling the EQL schemas on a database the command never named. E2E-pinned. Claude-Session: https://claude.ai/code/session_01AwM5Cm5ddasXozb6stxPR1 --- .changeset/eql-verify-surface.md | 2 +- .github/workflows/tests.yml | 4 + packages/cli/src/bin/main.ts | 16 +- packages/cli/src/cli/registry.ts | 7 +- .../db/__tests__/install-verify-gate.test.ts | 155 ++++++++++++++++++ packages/cli/src/commands/db/install.ts | 18 +- .../installer/__tests__/verify.live.test.ts | 11 +- packages/cli/src/installer/verify.ts | 5 +- packages/cli/tests/e2e/smoke.e2e.test.ts | 15 ++ packages/cli/vitest.config.ts | 32 ++++ skills/stash-cli/SKILL.md | 4 +- 11 files changed, 255 insertions(+), 14 deletions(-) create mode 100644 packages/cli/src/commands/db/__tests__/install-verify-gate.test.ts diff --git a/.changeset/eql-verify-surface.md b/.changeset/eql-verify-surface.md index 98b61ca87..608669ac2 100644 --- a/.changeset/eql-verify-surface.md +++ b/.changeset/eql-verify-surface.md @@ -2,4 +2,4 @@ 'stash': minor --- -New `stash eql verify`: assert the installed EQL surface is complete and coherent, independent of any application schema. A partial install — domains present, some of their comparison functions or operators absent — used to report success at install time and fail at query time on a specific predicate (e.g. `weight >= x`); nothing detected it. `eql verify` compares the database against everything the pinned bundle installs (every domain, function overload, operator, cast, and the ORE operator class) via read-only catalog queries, reports damage grouped per domain, and distinguishes expected absence from damage: the ORE operator class being skipped on managed Postgres, with its loud-failure fallback in place, reads as the supported configuration it is rather than a failed install. Exit 0 means exactly one thing — the surface was checked and found complete; damage, EQL absent, and a version mismatch with the pinned bundle (nothing verifiable) all exit 1. `--json` emits the structured report for agents. `stash eql install` now runs the same check automatically before declaring success, on the fresh-install path and the already-installed early exit alike. +New `stash eql verify`: assert the installed EQL surface is complete and coherent, independent of any application schema. A partial install — domains present, some of their comparison functions or operators absent — used to report success at install time and fail at query time on a specific predicate (e.g. `weight >= x`); nothing detected it. `eql verify` compares the database against everything the pinned bundle installs (every domain, function overload, operator, cast, and the ORE operator class) via read-only catalog queries, reports damage grouped per domain, and distinguishes expected absence from damage: the ORE operator class being skipped on managed Postgres, with its loud-failure fallback in place, reads as the supported configuration it is rather than a failed install. Exit 0 means exactly one thing — the surface was checked and found complete; damage, EQL absent, and a version mismatch with the pinned bundle (nothing verifiable) all exit 1. `--json` emits the structured report for agents. `stash eql install` now runs the same check automatically before declaring success, on the fresh-install path and the already-installed early exit alike — there, only damage fails the install: a version mismatch warns and continues, so a no-op re-run over an older EQL stays exit 0 for idempotent provisioning scripts. A valueless `--database-url` (booleanised by the parser when the next token is another flag) is now rejected up front on every command instead of silently falling back to `DATABASE_URL` — previously `eql install --database-url --force` could drop and reinstall the EQL schemas on a database the command never named. diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 0144def77..252e63c46 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -270,6 +270,10 @@ jobs: # These suites need Postgres only, no CipherStash credentials; the # verify suite installs EQL v3 into its own schemas, which coexists # with the image's pre-installed EQL v2 that the stack tests use. + # They share that one database, so the CLI vitest config runs them + # serially (the `live` project sets `fileParallelism: false` — + # verify.live's bundle install opens with DROP SCHEMA … CASCADE, which + # races destructively under the other suites in parallel forks). # (`supabase-push.live.test.ts` gates on different env vars and still # skips here — it needs the Supabase CLI binary.) - name: Run tests diff --git a/packages/cli/src/bin/main.ts b/packages/cli/src/bin/main.ts index de5125112..af6a90de1 100644 --- a/packages/cli/src/bin/main.ts +++ b/packages/cli/src/bin/main.ts @@ -260,11 +260,14 @@ function rejectRetiredEqlFlags( /** * `parseArgs` booleanises a `--database-url` whose value is missing (next - * token starts with `-`), so a typo'd `--database-url --json` would silently - * fall back to env/config resolution — and the read-only diagnostics would - * judge a different database than the user targeted. Reject it up front, - * keeping stdout parseable in `--json` mode (same pattern as `stash env`'s - * `nameMissingValue`). + * token starts with `-`, or the flag is last), so a typo'd `--database-url + * --force` would silently fall back to env/config resolution — and the + * command would act on a different database than the user targeted, without + * ever naming it. Harmless for the read-only diagnostics, catastrophic for + * `eql install --force` (DROP SCHEMA … CASCADE, no confirmation). A valueless + * `--database-url` is always a typo, so `dispatch()` rejects it for EVERY + * command rather than per-case, keeping stdout parseable in `--json` mode + * (same pattern as `stash env`'s `nameMissingValue`). */ async function rejectMissingDatabaseUrlValue( flags: Record, @@ -288,7 +291,6 @@ async function runEqlCommand( ) { switch (sub) { case 'preflight': - await rejectMissingDatabaseUrlValue(flags) await preflightCommand({ databaseUrl: values['database-url'], json: flags.json, @@ -298,7 +300,6 @@ async function runEqlCommand( await runInstall(flags, values) break case 'verify': { - await rejectMissingDatabaseUrlValue(flags) const { verifyCommand } = await import('../commands/eql/verify.js') await verifyCommand({ databaseUrl: values['database-url'], @@ -578,6 +579,7 @@ async function dispatch( flags: Record, values: Record, ) { + await rejectMissingDatabaseUrlValue(flags) switch (command) { case 'init': await initCommand(flags, values) diff --git a/packages/cli/src/cli/registry.ts b/packages/cli/src/cli/registry.ts index da132f4cf..2770c3a8a 100644 --- a/packages/cli/src/cli/registry.ts +++ b/packages/cli/src/cli/registry.ts @@ -365,10 +365,15 @@ export const registry: CommandGroup[] = [ 'complete. Damage, EQL not installed, and a version mismatch with', 'the pinned bundle all exit 1 — on a mismatch the object-level diff', 'is skipped (the pinned bundle is the wrong manifest to compare', - 'against) and the command suggests `eql upgrade`.', + 'against) and the command suggests `eql upgrade` (or a one-shot', + '`eql install --force --database-url ...` where no stash.config.ts', + 'exists — `eql upgrade` requires one).', '', 'Runs automatically at the end of `stash eql install`, on the', 'fresh-install path and the already-installed early exit alike.', + 'There, only damage fails the install — a version mismatch warns', + 'and continues, keeping a no-op re-run over an older EQL exit 0', + 'for idempotent provisioning scripts.', ].join('\n'), examples: ['eql verify', 'eql verify --json'], flags: [ diff --git a/packages/cli/src/commands/db/__tests__/install-verify-gate.test.ts b/packages/cli/src/commands/db/__tests__/install-verify-gate.test.ts new file mode 100644 index 000000000..c2871f80b --- /dev/null +++ b/packages/cli/src/commands/db/__tests__/install-verify-gate.test.ts @@ -0,0 +1,155 @@ +/** + * `verifySurfaceOrExit` — the #890 gate `installCommand` runs on both the + * fresh-install tail and the already-installed early exit. The differ's own + * behaviour is covered in `installer/__tests__/verify.test.ts`; what lives + * here is the install-side POLICY layered on top of the report: + * + * - damage exits 1 (a committed-but-incomplete install must not read as + * success); + * - a version mismatch does NOT — `ok: false` there means "nothing was + * checked", and a no-op `eql install` re-run over an older EQL was exit 0 + * before verification existed. Idempotent provisioning scripts (and + * `stash init`, whose direct-install route calls `installCommand` inside a + * try/catch that `process.exit` escapes) depend on that. `stash eql verify` + * keeps the strict gate; the installer must not inherit it. + * - a verification error (connection dropped) warns and continues — the + * install itself is committed. + */ + +import { beforeEach, describe, expect, it, vi } from 'vitest' +import type { VerifyReport } from '@/installer/verify.js' +import { verifySurfaceOrExit } from '../install.js' + +const clack = vi.hoisted(() => ({ + spinnerInstance: { start: vi.fn(), stop: vi.fn() }, + log: { + info: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + success: vi.fn(), + step: vi.fn(), + }, + intro: vi.fn(), + note: vi.fn(), + outro: vi.fn(), +})) +vi.mock('@clack/prompts', () => ({ + spinner: vi.fn(() => clack.spinnerInstance), + log: clack.log, + intro: clack.intro, + note: clack.note, + outro: clack.outro, +})) + +const verifier = vi.hoisted(() => ({ verifyEqlSurface: vi.fn() })) +vi.mock('@/installer/verify.js', () => ({ + verifyEqlSurface: verifier.verifyEqlSurface, +})) + +// Imported dynamically by the damage path for its findings renderer. +const findingsReporter = vi.hoisted(() => ({ reportVerifyFindings: vi.fn() })) +vi.mock('../../eql/verify.js', () => ({ + reportVerifyFindings: findingsReporter.reportVerifyFindings, +})) + +function report(overrides: Partial): VerifyReport { + return { + status: 'complete', + bundleVersion: '3.0.4', + installedVersion: '3.0.4', + counts: null, + ore: null, + findings: [], + ok: true, + ...overrides, + } +} + +function spinner() { + return clack.spinnerInstance as unknown as ReturnType< + typeof import('@clack/prompts').spinner + > +} + +describe('verifySurfaceOrExit', () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + it('returns without exiting on a complete surface', async () => { + verifier.verifyEqlSurface.mockResolvedValueOnce(report({})) + await expect( + verifySurfaceOrExit('postgres://db', spinner(), { remedy: 'r' }), + ).resolves.toBeUndefined() + }) + + it('exits 1 on damage, after reporting the findings and the remedy', async () => { + verifier.verifyEqlSurface.mockResolvedValueOnce( + report({ + status: 'incomplete', + ok: false, + findings: [ + { severity: 'damage', kind: 'operator', message: 'op missing' }, + ], + }), + ) + const exit = vi + .spyOn(process, 'exit') + .mockImplementation((code?: string | number | null | undefined) => { + throw new Error(`exit ${code}`) + }) + try { + await expect( + verifySurfaceOrExit('postgres://db', spinner(), { + remedy: 'use --force', + }), + ).rejects.toThrow('exit 1') + expect(findingsReporter.reportVerifyFindings).toHaveBeenCalled() + expect(clack.log.error).toHaveBeenCalledWith('use --force') + } finally { + exit.mockRestore() + } + }) + + it('does NOT exit on a version mismatch — warns with the skew instead', async () => { + const mismatch = report({ + status: 'version-mismatch', + installedVersion: '3.0.2', + ok: false, + findings: [ + { + severity: 'warning', + kind: 'version', + message: + 'EQL 3.0.2 installed, CLI pins 3.0.4 — run stash eql upgrade', + }, + ], + }) + verifier.verifyEqlSurface.mockResolvedValueOnce(mismatch) + const exit = vi.spyOn(process, 'exit').mockImplementation(() => { + throw new Error('exit called') + }) + try { + await expect( + verifySurfaceOrExit('postgres://db', spinner(), { remedy: 'r' }), + ).resolves.toBeUndefined() + expect(clack.log.warn).toHaveBeenCalledWith(mismatch.findings[0].message) + expect(clack.log.error).not.toHaveBeenCalled() + expect(exit).not.toHaveBeenCalled() + } finally { + exit.mockRestore() + } + }) + + it('warns and continues when verification itself errors', async () => { + verifier.verifyEqlSurface.mockRejectedValueOnce( + new Error('connection terminated'), + ) + await expect( + verifySurfaceOrExit('postgres://db', spinner(), { remedy: 'r' }), + ).resolves.toBeUndefined() + expect(clack.log.warn).toHaveBeenCalledWith( + expect.stringContaining('connection terminated'), + ) + }) +}) diff --git a/packages/cli/src/commands/db/install.ts b/packages/cli/src/commands/db/install.ts index 7135c44e5..5b401cb6a 100644 --- a/packages/cli/src/commands/db/install.ts +++ b/packages/cli/src/commands/db/install.ts @@ -231,8 +231,10 @@ export async function installCommand( * already-installed path. Exits 1 on damage; a verification ERROR (the * database dropped the connection, a timeout) is a warning, not a failure — * the install itself is committed, and `stash eql verify` can re-check later. + * + * Exported for the unit suite only — the command surface is `installCommand`. */ -async function verifySurfaceOrExit( +export async function verifySurfaceOrExit( databaseUrl: string, s: ReturnType, options: { remedy: string }, @@ -248,6 +250,20 @@ async function verifySurfaceOrExit( ) return } + if (report.status === 'version-mismatch') { + // `ok: false`, but NOT damage: the pinned bundle is the wrong manifest to + // diff against, so nothing was actually checked. `stash eql verify` stays + // strict about that (exit 1 — could-not-verify must never gate as + // verified), but a no-op `eql install` re-run over an older EQL was exit 0 + // before verification existed, and idempotent provisioning scripts depend + // on that. Warn with the skew and the remedy, and carry on. + s.stop( + 'Installed EQL version differs from the pinned bundle — surface not verified.', + ) + const mismatch = report.findings[0]?.message + if (mismatch) p.log.warn(mismatch) + return + } if (report.ok) { s.stop( report.ore?.state === 'fallback' diff --git a/packages/cli/src/installer/__tests__/verify.live.test.ts b/packages/cli/src/installer/__tests__/verify.live.test.ts index 1fdf579ed..8fed58e84 100644 --- a/packages/cli/src/installer/__tests__/verify.live.test.ts +++ b/packages/cli/src/installer/__tests__/verify.live.test.ts @@ -17,7 +17,7 @@ * export STASH_TEST_DATABASE_URL=postgres://cipherstash:password@localhost:55432/cipherstash */ -import { beforeAll, describe, expect, it } from 'vitest' +import { afterAll, beforeAll, describe, expect, it } from 'vitest' import { EQLInstaller } from '../index.js' import { verifyEqlSurface } from '../verify.js' @@ -45,6 +45,15 @@ describeLive('verifyEqlSurface — live Postgres', () => { await new EQLInstaller({ databaseUrl: url }).install() }, 180_000) + afterAll(async () => { + // The tests below drop an operator and eql_v3.version() — reinstall so + // the damage does not outlive this file into whatever runs against the + // database next (the live suites run serially in the `live` project, so + // "next" is a real thing, not a race). + const url = DATABASE_URL ?? '' + await new EQLInstaller({ databaseUrl: url }).install() + }, 180_000) + it('reads a fresh superuser install as complete', async () => { const url = DATABASE_URL ?? '' const report = await verifyEqlSurface(url) diff --git a/packages/cli/src/installer/verify.ts b/packages/cli/src/installer/verify.ts index 7bb4bf45e..c1d2f67cb 100644 --- a/packages/cli/src/installer/verify.ts +++ b/packages/cli/src/installer/verify.ts @@ -594,7 +594,10 @@ export function diffSurface( { severity: 'warning', kind: 'version', - message: `EQL ${installed.installedVersion} is installed, but this CLI pins EQL ${expected.eqlVersion} — the object-level surface checks only know the pinned bundle, so they were skipped. Run \`stash eql upgrade\`, then verify again.`, + // Both remedies run the same pinned-bundle DDL, but `eql upgrade` + // requires a stash.config.ts — on a one-shot `--database-url` + // database, `install --force` is the one that actually works. + message: `EQL ${installed.installedVersion} is installed, but this CLI pins EQL ${expected.eqlVersion} — the object-level surface checks only know the pinned bundle, so they were skipped. Run \`stash eql upgrade\` (or \`stash eql install --force --database-url ...\` for a database without a stash.config.ts), then verify again.`, }, ], // NOT ok: nothing was verified. `ok` must mean "checked and complete" — diff --git a/packages/cli/tests/e2e/smoke.e2e.test.ts b/packages/cli/tests/e2e/smoke.e2e.test.ts index 2ada4e998..c3a3b2871 100644 --- a/packages/cli/tests/e2e/smoke.e2e.test.ts +++ b/packages/cli/tests/e2e/smoke.e2e.test.ts @@ -195,6 +195,21 @@ describe('stash CLI — non-interactive smoke', () => { expect(unwrapped(r.output)).toContain('`--drizzle` emits a Drizzle') }) + // parseArgs booleanises a `--database-url` with no value (next token is a + // flag), which used to silently fall back to DATABASE_URL/config — with + // `--force` that meant DROP SCHEMA … CASCADE against a database the command + // never named. dispatch() now rejects it for every command, before any I/O, + // so this needs no database to observe. + it('a valueless --database-url exits 1 before any command runs', async () => { + const r = render(['eql', 'install', '--database-url', '--force']) + const { exitCode } = await r.exit + expect(exitCode).toBe(1) + expect(unwrapped(r.output)).toContain('`--database-url` needs a value') + // The install command itself never started (no permission-check spinner, + // no clack intro banner). + expect(r.output).not.toContain('Checking database permissions') + }) + it('db migrate is a stub that exits 0 with a "not yet implemented" warning', async () => { const r = render(['db', 'migrate']) const { exitCode } = await r.exit diff --git a/packages/cli/vitest.config.ts b/packages/cli/vitest.config.ts index 76b3903f4..c65ccc12f 100644 --- a/packages/cli/vitest.config.ts +++ b/packages/cli/vitest.config.ts @@ -5,6 +5,38 @@ export default defineConfig({ test: { globals: true, exclude: ['**/node_modules/**', '**/dist/**', 'tests/e2e/**'], + // Two projects so ONLY the live suites are serialised. Four of them gate + // on STASH_TEST_DATABASE_URL and share one database and one + // eql_v3/eql_v3_internal pair — and verify.live's beforeAll installs the + // full bundle, which opens with `DROP SCHEMA … CASCADE`, destroying the + // schemas (and their ACLs/OIDs) under a concurrently running + // guarded-grants.live. Run in parallel forks they race; run serially each + // suite sees the database state its comments already assume. The unit + // project keeps default file parallelism — serialising all ~1300 tests + // for the sake of four files is the `packages/migrate` fix at the wrong + // scale. + projects: [ + { + extends: true, + test: { + name: 'unit', + exclude: [ + '**/node_modules/**', + '**/dist/**', + 'tests/e2e/**', + '**/*.live.test.ts', + ], + }, + }, + { + extends: true, + test: { + name: 'live', + include: ['src/**/*.live.test.ts'], + fileParallelism: false, + }, + }, + ], }, resolve: { alias: { diff --git a/skills/stash-cli/SKILL.md b/skills/stash-cli/SKILL.md index c2fe5f68f..9973c7e62 100644 --- a/skills/stash-cli/SKILL.md +++ b/skills/stash-cli/SKILL.md @@ -391,13 +391,13 @@ The removed `--eql-version`, `--latest`, `--drizzle`, `--migration`, `--direct`, **`--database-url` is a one-shot.** It installs against that database and leaves the project untouched — no config is loaded, and none is scaffolded, nor is an encryption client. This lets `npx --package=stash@1.0.0 stash eql install --database-url 'postgres://...'` run in a bare project with no CipherStash dependencies while pinning the CLI to this skill's release. It also means the flag always wins: loading a config could pick up a parent-directory `databaseUrl` literal and install against the wrong database. -**The install verifies itself.** `eql install` ends by running the same surface check as `eql verify` (below) — on the fresh-install path *and* on the already-installed early exit, so a plain re-run over a damaged database fails rather than printing "Nothing to do." It exits 1 if the surface is incomplete; if the check itself cannot run (connection dropped mid-verify), it warns and points at `stash eql verify` instead of failing the committed install. +**The install verifies itself.** `eql install` ends by running the same surface check as `eql verify` (below) — on the fresh-install path *and* on the already-installed early exit, so a plain re-run over a damaged database fails rather than printing "Nothing to do." It exits 1 if the surface is incomplete; if the check itself cannot run (connection dropped mid-verify), it warns and points at `stash eql verify` instead of failing the committed install. A version mismatch with the pinned bundle also warns rather than fails there — nothing was actually checked, and a no-op re-run over an older EQL must stay exit 0 for idempotent provisioning scripts. (`eql verify` itself stays strict and exits 1 on a mismatch.) #### `eql verify` Read-only check that the **installed EQL surface is complete**, independent of any application schema. It compares what the database actually has against everything the pinned bundle installs — every domain, function overload, operator, cast, and the ORE operator class — via catalog queries, and reports damage grouped per domain. This catches the failure `eql validate` cannot: a partial install where the domains exist but some comparison functions or operators do not, so `weight >= x` errors at query time long after "install succeeded". -Expected absences read as info, not damage: on managed Postgres the bundle legitimately skips the ORE operator class (creating one requires superuser) and poisons the `_ord_ore` domains to fail loudly — `eql verify` reports that as the supported configuration it is. Exit 0 means exactly one thing: the surface was checked and found complete. Genuine damage, EQL not installed, and a version mismatch with the pinned bundle all exit 1 — on a mismatch the object-level diff is skipped (the pinned bundle is the wrong manifest to compare against) and the command suggests `eql upgrade`; "could not verify" never reads as "verified". +Expected absences read as info, not damage: on managed Postgres the bundle legitimately skips the ORE operator class (creating one requires superuser) and poisons the `_ord_ore` domains to fail loudly — `eql verify` reports that as the supported configuration it is. Exit 0 means exactly one thing: the surface was checked and found complete. Genuine damage, EQL not installed, and a version mismatch with the pinned bundle all exit 1 — on a mismatch the object-level diff is skipped (the pinned bundle is the wrong manifest to compare against) and the command suggests `eql upgrade` (or a one-shot `eql install --force --database-url ...` for a database without a `stash.config.ts` — `eql upgrade` needs one); "could not verify" never reads as "verified". Run it whenever query-time behaviour looks inconsistent with a "successful" install — e.g. an `operator does not exist` or `function ... does not exist` error naming an `eql_v3` object.