diff --git a/.changeset/23622-read-only-arms-journal-note-remainder.md b/.changeset/23622-read-only-arms-journal-note-remainder.md new file mode 100644 index 00000000000..a04cf9591ee --- /dev/null +++ b/.changeset/23622-read-only-arms-journal-note-remainder.md @@ -0,0 +1,18 @@ +--- +'@objectstack/cli': patch +--- + +fix(cli): `os migrate security-catalog-overlays`, `recorded-by` and `resume`, `os secret orphans` and `rewrap`, and `os storage orphans` say they write no row or table, and that opening a SQLite file can still switch its journal mode, instead of saying they write nothing + +**What was wrong.** Each of these commands boots through the SQL driver before it prints, and the driver's connect switches a SQLite file to the journal mode it asks for (WAL, unless `OS_DATABASE_SQLITE_JOURNAL_MODE=delete` is set in the command's own environment or the project's `.env`). Measured on a file still on a rollback journal, every command's default mode moved the header's journal-mode bytes (18 and 19, from 1,1 to 2,2) and its change counter, and changed no row or table, while printing "writes nothing", "nothing was written" or "Nothing was changed". The same held for an `--apply`, `--run` or `--delete` declined at its prompt, whose plain boot can also create a table the database is missing. Nothing was lost. + +**What changed.** Each sentence below now claims no row or table (no row, after a declined prompt) and, where the command opened a SQLite file, is followed by the note the read-only `os migrate` arms already carry: "Opening a SQLite file can still switch its journal mode (to WAL by default), which changes its header only." On PostgreSQL or MySQL the sentence ends without it. + +- `os migrate security-catalog-overlays` (the preview), beside a process holding the file. FROM: "The preview below writes nothing." TO: "The preview below writes no row or table." and the note. +- `os migrate recorded-by` (the dry run). FROM: "Nothing was changed. Re-run with --apply to rewrite them to NULL." TO: "No row or table was changed. Re-run with --apply to rewrite them to NULL." Its `--apply` declined at the prompt. FROM: "Aborted — nothing changed." TO: "Aborted — no row was changed." +- `os migrate resume` (the list). FROM: `Nothing was changed. Re-run with --run to act on one.` TO: `No row or table was changed. Re-run with --run to act on one.` Its `--run` declined at the prompt. FROM: "Aborted — nothing changed." TO: "Aborted — no row was changed." +- `os secret orphans` (the report). FROM: "Report only — nothing was written or deleted." TO: "Report only — no row or table was written or deleted." The note sits before the elapsed time. Its `--delete` declined at the prompt. FROM: "Aborted — nothing was written or deleted." TO: "Aborted — no export was written and no row was deleted." Its help description. FROM: "… Report-only by default: it writes nothing and deletes nothing." TO: "… it writes no row or table and deletes nothing." Its `--delete` flag description. FROM: "(default is a report that writes nothing)" TO: "(default is a report that writes no row or table)". +- `os secret rewrap` (the dry run). FROM: "Dry run — nothing was written." TO: "Dry run — no row or table was written." The note sits before the elapsed time. Its `--apply` declined at the prompt. FROM: "Aborted — nothing was opened or written." TO: "Aborted — no row was opened or written." Its help description. FROM: "… A dry run by default: it writes nothing without --apply." TO: "… it writes no row or table without --apply." Its `--apply` flag description. FROM: "(default is a dry run that writes nothing)" TO: "(default is a dry run that writes no row or table)". +- `os storage orphans`, its help description. FROM: "… Writes nothing, tombstones nothing, deletes nothing." TO: "… Writes no row or table, tombstones nothing, deletes nothing." Help text carries no note. + +**Unchanged.** No exit code, `--json` document, flag, prompt or refusal changes; the note is printed in the text output only. What the driver does on connect is unchanged: once a file is in the mode the driver asks for, these runs leave it byte-identical. diff --git a/packages/cli/src/commands/migrate/data-commands.absent-database.integration.test.ts b/packages/cli/src/commands/migrate/data-commands.absent-database.integration.test.ts index 74323d67b29..0c8461638f7 100644 --- a/packages/cli/src/commands/migrate/data-commands.absent-database.integration.test.ts +++ b/packages/cli/src/commands/migrate/data-commands.absent-database.integration.test.ts @@ -503,7 +503,7 @@ const DOORS: readonly Door[] = [ expect(family.status).toBe('enumerated'); } }, - humanEmpty: /Report only — nothing was written or deleted/, + humanEmpty: /Report only — no row or table was written or deleted/, tables: ['sys_secret', 'sys_setting', 'sys_metadata'], work: (doc) => expect(doc.plan.counts.total).toBe(1), }, @@ -516,7 +516,7 @@ const DOORS: readonly Door[] = [ expect(family.status).toBe('enumerated'); } }, - humanEmpty: /Dry run — nothing was written/, + humanEmpty: /Dry run — no row or table was written/, tables: ['sys_secret', 'sys_setting', 'sys_metadata'], work: (doc) => expect(doc.report.counts.total).toBe(1), }, diff --git a/packages/cli/src/commands/migrate/plan.ts b/packages/cli/src/commands/migrate/plan.ts index 4b11b830714..663da86ac91 100644 --- a/packages/cli/src/commands/migrate/plan.ts +++ b/packages/cli/src/commands/migrate/plan.ts @@ -207,7 +207,7 @@ export default class MigratePlan extends Command { // are separate CUSTOMERS. The HARD gate runs at app install; this covers // the two populations it cannot reach (installs predating the gate, and // environments whose posture changed after install). Never fatal: a plan - // writes nothing, and this reports a metadata decision, not drift. + // writes no row or table, and this reports a metadata decision, not drift. const uniqueScopeAdvisory = postureGatesGlobalUniques(resolveTenancyPosture()) ? collectGlobalUniques(stack.allObjects()) : []; diff --git a/packages/cli/src/commands/migrate/recorded-by.test.ts b/packages/cli/src/commands/migrate/recorded-by.test.ts index ec41fbf49d7..81e87648d86 100644 --- a/packages/cli/src/commands/migrate/recorded-by.test.ts +++ b/packages/cli/src/commands/migrate/recorded-by.test.ts @@ -8,16 +8,43 @@ * What is pinned here is the thing a unit test of the plan cannot see: that * this subcommand honours #2186 — a bare `os migrate ` must never * mutate the database by surprise, so writing is opt-in behind `--apply`. + * + * And what its human face says about the database once the boot has run. + * Either mode's connect can switch a SQLite file's journal mode, which + * rewrites its header and no row, so neither the dry run nor a declined + * `--apply` says that nothing was changed; each names the switch on a SQLite + * target, and only there. The boot is replaced by a stack over a fake engine. */ -import { describe, it, expect } from 'vitest'; +import { afterEach, beforeAll, beforeEach, describe, it, expect, vi } from 'vitest'; +import { Config } from '@oclif/core'; +import { fileURLToPath } from 'node:url'; import MigrateRecordedBy from './recorded-by.js'; +import { JOURNAL_MODE_SWITCH_NOTE } from '../../utils/sqlite-occupancy.js'; import { RECORDED_BY_SENTINEL, RECORDED_BY_SENTINEL_PLAN_ID, createRecordedBySentinelPlan, } from '@objectstack/metadata-protocol'; +const boot = vi.hoisted(() => vi.fn()); +vi.mock('../../utils/schema-migrate.js', () => ({ bootSchemaStack: boot })); +// The plugins the real boot composes; the replaced boot never reads them. +vi.mock('../../utils/data-migration-plugins.js', () => ({ buildDataMigrationPlugins: async () => [] })); +// The `--apply` confirmation, answered without a terminal: `undefined` leaves +// readline as it is. +const prompt = vi.hoisted(() => ({ answer: undefined as string | undefined })); +vi.mock('node:readline', async (importOriginal) => { + const real = await importOriginal(); + return { + ...real, + createInterface: ((...args: Parameters) => (prompt.answer === undefined + ? real.createInterface(...args) + : { question: (_q: string, answer: (a: string) => void) => answer(prompt.answer as string), close: () => {} } + )) as typeof real.createInterface, + }; +}); + describe('os migrate recorded-by', () => { it('is a dry run by default — --apply is opt-in (#2186)', () => { expect(MigrateRecordedBy.flags.apply.default).toBe(false); @@ -41,3 +68,91 @@ describe('os migrate recorded-by', () => { expect(RECORDED_BY_SENTINEL).toBe('system'); }); }); + +const CLI_ROOT = fileURLToPath(new URL('../../..', import.meta.url)); + +// oclif reads the package's config (its whole command table) on a first run; +// it is paid here, outside the cases. +let oclifConfig: Config; +beforeAll(async () => { + oclifConfig = await Config.load({ root: CLI_ROOT }); +}, 60_000); + +const SQLITE_CONFIG = { client: 'better-sqlite3', connection: { filename: '/srv/app/.objectstack/data/objectstack.db' } }; +const POSTGRES_CONFIG = { client: 'pg', connection: 'postgres://db.example.com:5432/app' }; + +/** + * A booted stack over one history row holding the sentinel. The engine can + * only read: a write the command attempted would throw, and the run with it. + */ +function stackOver(config: Record): void { + const rows = [{ id: 'h_sentinel', recorded_by: RECORDED_BY_SENTINEL }]; + const engine = { + find: async (_object: string, query: { where?: Record; limit?: number }) => { + const matched = rows.filter((row) => + Object.entries(query.where ?? {}).every(([key, value]) => (row as Record)[key] === value)); + return typeof query?.limit === 'number' ? matched.slice(0, query.limit) : matched; + }, + }; + boot.mockResolvedValue({ + driver: { config }, + kernel: { getService: (name: string) => (name === 'objectql' ? engine : undefined) }, + tableAbsent: () => false, + shutdown: async () => {}, + }); +} + +async function runHuman(argv: string[]): Promise { + const log = vi.spyOn(console, 'log').mockImplementation(() => {}); + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + const error = vi.spyOn(console, 'error').mockImplementation(() => {}); + const savedExitCode = process.exitCode; + try { + await MigrateRecordedBy.run(argv, oclifConfig); + return log.mock.calls.map((c) => c.map(String).join(' ')); + } finally { + process.exitCode = savedExitCode; + for (const spy of [log, warn, error]) spy.mockRestore(); + } +} + +describe('os migrate recorded-by — the human face names no change it cannot vouch for', () => { + let savedIsTTY: PropertyDescriptor | undefined; + beforeEach(() => { + savedIsTTY = Object.getOwnPropertyDescriptor(process.stdin, 'isTTY'); + }); + afterEach(() => { + prompt.answer = undefined; + boot.mockReset(); + if (savedIsTTY) Object.defineProperty(process.stdin, 'isTTY', savedIsTTY); + else delete (process.stdin as { isTTY?: boolean }).isTTY; + }); + + it('the dry run claims no row or table, and names the journal-mode switch on a SQLite file', async () => { + stackOver(SQLITE_CONFIG); + const lines = await runHuman([]); + expect(boot).toHaveBeenCalledWith(expect.objectContaining({ deferSchemaDdl: true, readOnlyProbe: true })); + const closing = lines.find((line) => line.includes('Re-run with --apply')); + expect(closing, lines.join('\n')).toContain( + `No row or table was changed. Re-run with --apply to rewrite them to NULL. ${JOURNAL_MODE_SWITCH_NOTE}`, + ); + expect(lines.join('\n')).not.toContain('Nothing was changed'); + }, 60_000); + + it('on a target with no file header (PostgreSQL) the dry run claims no row or table and names no switch', async () => { + stackOver(POSTGRES_CONFIG); + const lines = await runHuman([]); + const closing = lines.find((line) => line.includes('Re-run with --apply')); + expect(closing, lines.join('\n')).toContain('No row or table was changed. Re-run with --apply to rewrite them to NULL.'); + expect(lines.join('\n')).not.toContain(JOURNAL_MODE_SWITCH_NOTE); + }, 60_000); + + it('an --apply declined at the prompt claims no row changed, and names the switch on a SQLite file', async () => { + Object.defineProperty(process.stdin, 'isTTY', { value: true, configurable: true }); + prompt.answer = 'n'; + stackOver(SQLITE_CONFIG); + const lines = await runHuman(['--apply']); + const aborted = lines.find((line) => line.includes('Aborted')); + expect(aborted, lines.join('\n')).toContain(`Aborted — no row was changed. ${JOURNAL_MODE_SWITCH_NOTE}`); + }, 60_000); +}); diff --git a/packages/cli/src/commands/migrate/recorded-by.ts b/packages/cli/src/commands/migrate/recorded-by.ts index 4fc3e783c48..df42f731ac1 100644 --- a/packages/cli/src/commands/migrate/recorded-by.ts +++ b/packages/cli/src/commands/migrate/recorded-by.ts @@ -30,6 +30,8 @@ import { } from '../../utils/format.js'; import { bootSchemaStack } from '../../utils/schema-migrate.js'; import { buildDataMigrationPlugins } from '../../utils/data-migration-plugins.js'; +import { describeDriverSqliteFile } from '../../utils/connection-display.js'; +import { JOURNAL_MODE_SWITCH_NOTE } from '../../utils/sqlite-occupancy.js'; async function confirm(question: string): Promise { if (!process.stdin.isTTY) return false; // non-interactive → require --yes @@ -116,6 +118,17 @@ export default class MigrateRecordedBy extends Command { } try { + // [#23622] Either mode's connect may have switched a SQLite file's + // journal mode (`SqlDriver.applySqliteJournalMode`, one rule for every + // connect), which rewrites the file header and no row. So no line below + // says that nothing changed: the read-only mode claims no row or table + // (its boot defers DDL), an abort after the plain boot claims no row + // (that boot may have created a missing table), and on a SQLite file + // each says what the connect may still have changed. A target that is + // no SQLite file (PostgreSQL, MySQL) has no header to switch. + const journalNote = describeDriverSqliteFile(stack.driver?.config) !== undefined + ? ` ${JOURNAL_MODE_SWITCH_NOTE}` + : ''; const engine: IObjectQLEngine = stack.kernel.getService('objectql'); if (typeof engine?.find !== 'function') { throw new Error('No ObjectQL engine on this stack — cannot read sys_metadata_history.'); @@ -155,7 +168,7 @@ export default class MigrateRecordedBy extends Command { return; } printWarning(`${pending.length} sys_metadata_history row(s) hold recorded_by = '${RECORDED_BY_SENTINEL}'.`); - printInfo("Nothing was changed. Re-run with --apply to rewrite them to NULL."); + printInfo(`No row or table was changed. Re-run with --apply to rewrite them to NULL.${journalNote}`); return; } @@ -176,7 +189,7 @@ export default class MigrateRecordedBy extends Command { return; } const ok = await confirm(chalk.bold(`\n${summary}? [y/N] `)); - if (!ok) { printInfo('Aborted — nothing changed.'); return; } + if (!ok) { printInfo(`Aborted — no row was changed.${journalNote}`); return; } } if (!flags.json) printStep('Converting…'); diff --git a/packages/cli/src/commands/migrate/resume.test.ts b/packages/cli/src/commands/migrate/resume.test.ts new file mode 100644 index 00000000000..7b6db981402 --- /dev/null +++ b/packages/cli/src/commands/migrate/resume.test.ts @@ -0,0 +1,142 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * `os migrate resume` — what its human face says about the database once the + * boot has run. + * + * Either mode's connect can switch a SQLite file's journal mode, which + * rewrites its header and no row (measured on a rollback-journal file: header + * bytes 18 and 19 from 1,1 to 2,2, the change counter up by one, schema and + * rows byte-identical). So neither the list nor a declined `--run` says that + * nothing was changed: the list claims no row or table, the abort claims no + * row, and each names the switch on a SQLite target, and only there. The list + * prints the same sentence on every dialect, so the base sentence is the + * corrected one on PostgreSQL too. + * + * The boot is replaced by a stack over a fake engine holding one interrupted + * run; the journal reads (`findInterruptedRuns`, `readRunJournal`) run for real + * against it. + */ + +import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'; +import { Config } from '@oclif/core'; +import { fileURLToPath } from 'node:url'; +import MigrateResume from './resume.js'; +import { JOURNAL_MODE_SWITCH_NOTE } from '../../utils/sqlite-occupancy.js'; + +const boot = vi.hoisted(() => vi.fn()); +vi.mock('../../utils/schema-migrate.js', () => ({ bootSchemaStack: boot })); +// The plugins the real boot composes; the replaced boot never reads them. +vi.mock('../../utils/data-migration-plugins.js', () => ({ buildDataMigrationPlugins: async () => [] })); +// The `--run` confirmation, answered without a terminal: `undefined` leaves +// readline as it is. +const prompt = vi.hoisted(() => ({ answer: undefined as string | undefined })); +vi.mock('node:readline', async (importOriginal) => { + const real = await importOriginal(); + return { + ...real, + createInterface: ((...args: Parameters) => (prompt.answer === undefined + ? real.createInterface(...args) + : { question: (_q: string, answer: (a: string) => void) => answer(prompt.answer as string), close: () => {} } + )) as typeof real.createInterface, + }; +}); + +const CLI_ROOT = fileURLToPath(new URL('../../..', import.meta.url)); + +// oclif reads the package's config (its whole command table) on a first run; +// it is paid here, outside the cases. +let oclifConfig: Config; +beforeAll(async () => { + oclifConfig = await Config.load({ root: CLI_ROOT }); +}, 60_000); + +const SQLITE_CONFIG = { client: 'better-sqlite3', connection: { filename: '/srv/app/.objectstack/data/objectstack.db' } }; +const POSTGRES_CONFIG = { client: 'pg', connection: 'postgres://db.example.com:5432/app' }; + +const RUN_ID = 'run_interrupted'; +const PLAN_ID = 'plan.fixture'; + +/** + * A booted stack whose journal holds one interrupted run of a registered plan. + * The engine can only read: a write the command attempted would throw, and the + * run with it. + */ +function stackOver(config: Record): void { + const journal = [ + { run_id: RUN_ID, seq: 0, kind: 'run_started', plan_hash: 'h', detail: JSON.stringify({ planId: PLAN_ID }) }, + { run_id: RUN_ID, seq: 1, kind: 'chunk_started', chunk_index: 0 }, + ]; + const engine = { + find: async (_object: string, query: { where?: Record; limit?: number }) => { + const matched = journal.filter((row) => + Object.entries(query.where ?? {}).every(([key, value]) => (row as Record)[key] === value)); + return typeof query?.limit === 'number' ? matched.slice(0, query.limit) : matched; + }, + }; + const plans = { get: (id: string) => (id === PLAN_ID ? { id: PLAN_ID, onCrash: 'resume' } : undefined) }; + boot.mockResolvedValue({ + driver: { config }, + kernel: { + getService: (name: string) => (name === 'objectql' ? engine : name === 'migration-plans' ? plans : undefined), + }, + tableAbsent: () => false, + shutdown: async () => {}, + }); +} + +async function runHuman(argv: string[]): Promise { + const log = vi.spyOn(console, 'log').mockImplementation(() => {}); + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + const error = vi.spyOn(console, 'error').mockImplementation(() => {}); + const savedExitCode = process.exitCode; + try { + await MigrateResume.run(argv, oclifConfig); + return log.mock.calls.map((c) => c.map(String).join(' ')); + } finally { + process.exitCode = savedExitCode; + for (const spy of [log, warn, error]) spy.mockRestore(); + } +} + +describe('os migrate resume — the human face names no change it cannot vouch for', () => { + let savedIsTTY: PropertyDescriptor | undefined; + beforeEach(() => { + savedIsTTY = Object.getOwnPropertyDescriptor(process.stdin, 'isTTY'); + }); + afterEach(() => { + prompt.answer = undefined; + boot.mockReset(); + if (savedIsTTY) Object.defineProperty(process.stdin, 'isTTY', savedIsTTY); + else delete (process.stdin as { isTTY?: boolean }).isTTY; + }); + + it('the list claims no row or table, and names the journal-mode switch on a SQLite file', async () => { + stackOver(SQLITE_CONFIG); + const lines = await runHuman([]); + expect(boot).toHaveBeenCalledWith(expect.objectContaining({ deferSchemaDdl: true, readOnlyProbe: true })); + expect(lines.join('\n')).toContain(RUN_ID); + const closing = lines.find((line) => line.includes('Re-run with --run')); + expect(closing, lines.join('\n')).toContain( + `No row or table was changed. Re-run with --run to act on one. ${JOURNAL_MODE_SWITCH_NOTE}`, + ); + expect(lines.join('\n')).not.toContain('Nothing was changed'); + }, 60_000); + + it('on a target with no file header (PostgreSQL) the list claims no row or table and names no switch', async () => { + stackOver(POSTGRES_CONFIG); + const lines = await runHuman([]); + const closing = lines.find((line) => line.includes('Re-run with --run')); + expect(closing, lines.join('\n')).toContain('No row or table was changed. Re-run with --run to act on one.'); + expect(lines.join('\n')).not.toContain(JOURNAL_MODE_SWITCH_NOTE); + }, 60_000); + + it('a --run declined at the prompt claims no row changed, and names the switch on a SQLite file', async () => { + Object.defineProperty(process.stdin, 'isTTY', { value: true, configurable: true }); + prompt.answer = 'n'; + stackOver(SQLITE_CONFIG); + const lines = await runHuman(['--run', RUN_ID]); + const aborted = lines.find((line) => line.includes('Aborted')); + expect(aborted, lines.join('\n')).toContain(`Aborted — no row was changed. ${JOURNAL_MODE_SWITCH_NOTE}`); + }, 60_000); +}); diff --git a/packages/cli/src/commands/migrate/resume.ts b/packages/cli/src/commands/migrate/resume.ts index 06a8af42843..ee02d6a341b 100644 --- a/packages/cli/src/commands/migrate/resume.ts +++ b/packages/cli/src/commands/migrate/resume.ts @@ -28,6 +28,8 @@ import { } from '../../utils/format.js'; import { bootSchemaStack } from '../../utils/schema-migrate.js'; import { buildDataMigrationPlugins } from '../../utils/data-migration-plugins.js'; +import { describeDriverSqliteFile } from '../../utils/connection-display.js'; +import { JOURNAL_MODE_SWITCH_NOTE } from '../../utils/sqlite-occupancy.js'; async function confirm(question: string): Promise { if (!process.stdin.isTTY) return false; // non-interactive → require --yes @@ -125,6 +127,18 @@ export default class MigrateResume extends Command { } try { + // [#23622] Either mode's connect may have switched a SQLite file's + // journal mode (`SqlDriver.applySqliteJournalMode`, one rule for every + // connect), which rewrites the file header and no row. So no line below + // says that nothing changed: the read-only mode claims no row or table + // (its boot defers DDL), an abort after the plain boot claims no row + // (that boot may have created a missing table), and on a SQLite file + // each says what the connect may still have changed. A target that is + // no SQLite file (PostgreSQL, MySQL) has no header to switch. + const journalNote = describeDriverSqliteFile(stack.driver?.config) !== undefined + ? ` ${JOURNAL_MODE_SWITCH_NOTE}` + : ''; + // Typed off the slot's contract, not erased to `any` (#4168/#4176/#4251): // the journal reads below are exactly the surface `IObjectQLEngine` // declares, so there is nothing here that needs the checking switched off. @@ -169,7 +183,7 @@ export default class MigrateResume extends Command { } printWarning(`${interrupted.length} interrupted migration run(s):`); for (const run of interrupted) this.log(` ${describeInterruptedRun(run, plans)}`); - printInfo('Nothing was changed. Re-run with --run to act on one.'); + printInfo(`No row or table was changed. Re-run with --run to act on one.${journalNote}`); return; } @@ -215,7 +229,7 @@ export default class MigrateResume extends Command { this.log(''); this.log(` ${describeInterruptedRun(target, plans)}`); const ok = await confirm(chalk.bold(`\n${summary}? [y/N] `)); - if (!ok) { printInfo('Aborted — nothing changed.'); return; } + if (!ok) { printInfo(`Aborted — no row was changed.${journalNote}`); return; } } if (!flags.json) printStep(policy === 'compensate' ? 'Unwinding…' : 'Resuming forward…'); diff --git a/packages/cli/src/commands/migrate/security-catalog-overlays.preview-note.test.ts b/packages/cli/src/commands/migrate/security-catalog-overlays.preview-note.test.ts new file mode 100644 index 00000000000..7b2950c7ad8 --- /dev/null +++ b/packages/cli/src/commands/migrate/security-catalog-overlays.preview-note.test.ts @@ -0,0 +1,95 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * `os migrate security-catalog-overlays` without `--apply`, beside a process + * holding the SQLite file: what the occupancy warning says the preview writes. + * + * The preview's boot is read-only (deferred DDL, no seed, no hydration), but + * its connect can switch a SQLite file's journal mode, which rewrites the + * file's header and no row (measured on a rollback-journal file beside an idle + * holder: header bytes 18 and 19 from 1,1 to 2,2, the change counter up by + * one, schema and rows byte-identical). So the warning claims no row or table + * and names the switch (`JOURNAL_MODE_SWITCH_NOTE`); it never says the preview + * writes nothing. The warning prints only on a busy SQLite file, so it always + * carries the note. + * + * The warning is printed before the boot, so the probe answers busy and the + * boot is replaced by one that refuses: the case reads what was printed first. + */ + +import { afterEach, beforeAll, describe, expect, it, vi } from 'vitest'; +import { Config } from '@oclif/core'; +import { fileURLToPath } from 'node:url'; +import { isExitSignal } from '../../utils/format.js'; +import { JOURNAL_MODE_SWITCH_NOTE } from '../../utils/sqlite-occupancy.js'; +import MigrateSecurityCatalogOverlays from './security-catalog-overlays.js'; + +const probe = vi.hoisted(() => vi.fn()); +vi.mock('../../utils/migrate-occupancy-gate.js', async (importOriginal) => ({ + ...(await importOriginal()), + probeMigrationTarget: probe, +})); +const boot = vi.hoisted(() => vi.fn()); +vi.mock('../../utils/schema-migrate.js', async (importOriginal) => ({ + ...(await importOriginal()), + bootSchemaStack: boot, +})); + +const CLI_ROOT = fileURLToPath(new URL('../../..', import.meta.url)); + +// oclif reads the package's config (its whole command table) on a first run; +// it is paid here, outside the cases. +let oclifConfig: Config; +beforeAll(async () => { + oclifConfig = await Config.load({ root: CLI_ROOT }); +}, 60_000); + +const FILE = '/srv/app/.objectstack/data/objectstack.db'; + +afterEach(() => { + probe.mockReset(); + boot.mockReset(); +}); + +/** Run the command with its output captured and its exit trapped. */ +async function run(argv: string[]): Promise<{ lines: string[]; exitCode: number | undefined }> { + const log = vi.spyOn(console, 'log').mockImplementation(() => {}); + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + const error = vi.spyOn(console, 'error').mockImplementation(() => {}); + const savedExitCode = process.exitCode; + let exitCode: number | undefined; + try { + try { + await MigrateSecurityCatalogOverlays.run(argv, oclifConfig); + } catch (thrown) { + if (!isExitSignal(thrown)) throw thrown; + exitCode = (thrown as { oclif?: { exit?: number } }).oclif?.exit; + } + return { lines: log.mock.calls.map((c) => c.map(String).join(' ')), exitCode }; + } finally { + process.exitCode = savedExitCode; + for (const spy of [log, warn, error]) spy.mockRestore(); + } +} + +describe('os migrate security-catalog-overlays (preview) beside a holder', () => { + it('warns that the preview writes no row or table, and names the journal-mode switch', async () => { + probe.mockResolvedValue({ + status: 'busy', + filename: FILE, + signal: 'file_open', + detail: 'it is open in pid 4242 (node)', + sidecars: [], + }); + boot.mockRejectedValue(new Error('boot refused by the test: the warning is printed before it')); + + const { lines, exitCode } = await run([]); + + expect(exitCode).toBe(1); + // The preview's boot is the read-only one. + expect(boot).toHaveBeenCalledWith(expect.objectContaining({ deferSchemaDdl: true, readOnlyProbe: true })); + const warning = lines.find((line) => line.includes(`${FILE} is in use`)); + expect(warning, lines.join('\n')).toContain(`The preview below writes no row or table. ${JOURNAL_MODE_SWITCH_NOTE}`); + expect(lines.join('\n')).not.toContain('writes nothing'); + }, 60_000); +}); diff --git a/packages/cli/src/commands/migrate/security-catalog-overlays.ts b/packages/cli/src/commands/migrate/security-catalog-overlays.ts index cb9e84efd4c..2c6f7f47b35 100644 --- a/packages/cli/src/commands/migrate/security-catalog-overlays.ts +++ b/packages/cli/src/commands/migrate/security-catalog-overlays.ts @@ -20,7 +20,7 @@ import { bootSchemaStack } from '../../utils/schema-migrate.js'; import { describeSecurityPluginComposition, describeUnloadableHostConfig } from '../../utils/schema-migration-plugins.js'; import { absentTableReads } from '../../utils/absent-table-reads.js'; import { OCCUPANCY_HINT, probeMigrationTarget } from '../../utils/migrate-occupancy-gate.js'; -import { describeOccupancy } from '../../utils/sqlite-occupancy.js'; +import { describeOccupancy, JOURNAL_MODE_SWITCH_NOTE } from '../../utils/sqlite-occupancy.js'; import { deleteSecurityCatalogOverlayRows, listSecurityCatalogOverlayRows, @@ -195,7 +195,7 @@ export default class MigrateSecurityCatalogOverlays extends Command { if (occupancy.status === 'busy' && !flags.json) { printWarning(apply ? `--force: ${describeOccupancy(occupancy)} Deleting anyway — the live process may write metadata mid-run.` - : `${describeOccupancy(occupancy)} The preview below writes nothing.`); + : `${describeOccupancy(occupancy)} The preview below writes no row or table. ${JOURNAL_MODE_SWITCH_NOTE}`); } if (occupancy.status === 'unknown' && !flags.json) { printWarning(`Could not check whether the database is in use — ${occupancy.detail}`); diff --git a/packages/cli/src/commands/secret/orphans.guards.test.ts b/packages/cli/src/commands/secret/orphans.guards.test.ts index d6669b3aeb5..57f4245cb38 100644 --- a/packages/cli/src/commands/secret/orphans.guards.test.ts +++ b/packages/cli/src/commands/secret/orphans.guards.test.ts @@ -24,9 +24,14 @@ * `mode: 0o600` that keeps cipher material owner-only is applied by the OS only * at CREATION, so a truncating write would have inherited the squatter's owner * and permissions and put cipher material inside them. + * + * The last is what the human face says about the database. Either mode's + * connect can switch a SQLite file's journal mode, which rewrites its header + * and no row, so neither the report nor an abort says that nothing was + * written; each names the switch on a SQLite target, and only there. */ -import { describe, it, expect, afterEach, vi } from 'vitest'; +import { describe, it, expect, afterEach, beforeEach, vi } from 'vitest'; import { existsSync, mkdtempSync, readFileSync, rmSync, statSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { dirname, join, resolve } from 'node:path'; @@ -35,6 +40,7 @@ import SecretOrphans, { asDeletingDriver, readDeclaredDatasources } from './orph import { bootSchemaStack } from '../../utils/schema-migrate.js'; import { collectSecretReferenceUnion } from '../../utils/secret-reference-union.js'; import type { SecretReferenceUnion } from '../../utils/secret-reference-union.js'; +import { JOURNAL_MODE_SWITCH_NOTE } from '../../utils/sqlite-occupancy.js'; // The command's own control flow is the subject here, so only the two seams // that would boot a database or walk a real engine are replaced. Everything @@ -46,6 +52,19 @@ vi.mock('../../utils/secret-reference-union.js', () => ({ collectSecretReference // Constructed and handed to the (mocked) boot, never used — a stub keeps the // platform-objects graph out of this file's import cost. vi.mock('@objectstack/platform-objects/plugin', () => ({ PlatformObjectsPlugin: class {} })); +// The `--delete` confirmation, answered without a terminal: `undefined` leaves +// readline as it is. +const prompt = vi.hoisted(() => ({ answer: undefined as string | undefined })); +vi.mock('node:readline', async (importOriginal) => { + const real = await importOriginal(); + return { + ...real, + createInterface: ((...args: Parameters) => (prompt.answer === undefined + ? real.createInterface(...args) + : { question: (_q: string, answer: (a: string) => void) => answer(prompt.answer as string), close: () => {} } + )) as typeof real.createInterface, + }; +}); const HERE = dirname(fileURLToPath(import.meta.url)); const CLI_ROOT = resolve(HERE, '..', '..', '..'); @@ -274,3 +293,100 @@ describe('the pre-delete export is created exclusively, never written onto', () expect(statSync(file).mode & 0o777).toBe(0o600); }, 60_000); }); + +// ─────────────────────────────────────────────────────────────────────────── +// The human face: what the report and an abort say about the database +// ─────────────────────────────────────────────────────────────────────────── + +/** The primary datasource's driver config, as the boot's `SchemaStack.driver` carries it. */ +const SQLITE_CONFIG = { client: 'better-sqlite3', connection: { filename: '/srv/app/.objectstack/data/objectstack.db' } }; +const POSTGRES_CONFIG = { client: 'pg', connection: 'postgres://db.example.com:5432/app' }; + +/** Drive one human-mode run over the deletable orphan, with the boot's driver on `config`. */ +async function runHuman( + argv: string[], + config: Record, +): Promise<{ lines: string[]; removed: string[] }> { + const removed: string[] = []; + vi.mocked(collectSecretReferenceUnion).mockResolvedValue(unreferencingCompleteUnion()); + vi.mocked(bootSchemaStack).mockImplementation(async () => { + const objectql = { + getConfigs: () => ({}), + getDriverForObject: (object: string) => + object === 'sys_secret' + ? { + async find() { return [{ ...ORPHAN_ROW }]; }, + async delete(_object: string, id: string) { removed.push(id); return true; }, + } + : { async find() { return []; } }, + }; + return { + driver: { config }, + kernel: { + getService: (name: string) => + name === 'objectql' ? objectql + : name === 'settings' ? { listManifests: () => SETTINGS_MANIFESTS } + : undefined, + }, + tableAbsent: () => false, + shutdown: async () => { /* nothing was booted */ }, + } as never; + }); + const log = vi.spyOn(console, 'log').mockImplementation(() => {}); + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + const error = vi.spyOn(console, 'error').mockImplementation(() => {}); + const savedExitCode = process.exitCode; + try { + await SecretOrphans.run(argv, { root: CLI_ROOT }); + return { lines: log.mock.calls.map((c) => c.map(String).join(' ')), removed }; + } finally { + process.exitCode = savedExitCode; + for (const spy of [log, warn, error]) spy.mockRestore(); + } +} + +describe('the human face names no write it cannot vouch for', () => { + let savedIsTTY: PropertyDescriptor | undefined; + beforeEach(() => { + savedIsTTY = Object.getOwnPropertyDescriptor(process.stdin, 'isTTY'); + }); + afterEach(() => { + prompt.answer = undefined; + if (savedIsTTY) Object.defineProperty(process.stdin, 'isTTY', savedIsTTY); + else delete (process.stdin as { isTTY?: boolean }).isTTY; + }); + + it('the report claims no row or table, and names the journal-mode switch on a SQLite file', async () => { + const { lines } = await runHuman(['--no-declared-datasources'], SQLITE_CONFIG); + const closing = lines.find((line) => line.includes('Report only')); + expect(closing, lines.join('\n')).toContain(`Report only — no row or table was written or deleted. ${JOURNAL_MODE_SWITCH_NOTE}`); + expect(lines.join('\n')).not.toContain('nothing was written'); + }, 60_000); + + it('on a target with no file header (PostgreSQL) the report claims no row or table and names no switch', async () => { + const { lines } = await runHuman(['--no-declared-datasources'], POSTGRES_CONFIG); + const closing = lines.find((line) => line.includes('Report only')); + expect(closing, lines.join('\n')).toContain('Report only — no row or table was written or deleted.'); + expect(lines.join('\n')).not.toContain(JOURNAL_MODE_SWITCH_NOTE); + }, 60_000); + + it('a --delete declined at the prompt claims no export and no row, and names the switch on a SQLite file', async () => { + Object.defineProperty(process.stdin, 'isTTY', { value: true, configurable: true }); + prompt.answer = 'n'; + const exportPath = join(tempDir(), 'export.json'); + const { lines, removed } = await runHuman( + ['--delete', '--no-declared-datasources', '--export', exportPath], + SQLITE_CONFIG, + ); + expect(removed).toEqual([]); + expect(existsSync(exportPath)).toBe(false); + const aborted = lines.find((line) => line.includes('Aborted')); + expect(aborted, lines.join('\n')).toContain(`Aborted — no export was written and no row was deleted. ${JOURNAL_MODE_SWITCH_NOTE}`); + }, 60_000); + + it('the help says the report writes no row or table, never that it writes nothing', () => { + expect(SecretOrphans.description).toContain('Report-only by default: it writes no row or table and deletes nothing.'); + expect(SecretOrphans.flags.delete.description).toContain('(default is a report that writes no row or table)'); + expect(`${SecretOrphans.description} ${SecretOrphans.flags.delete.description}`).not.toContain('writes nothing'); + }); +}); diff --git a/packages/cli/src/commands/secret/orphans.journal-mode.integration.test.ts b/packages/cli/src/commands/secret/orphans.journal-mode.integration.test.ts new file mode 100644 index 00000000000..e4a869b4d15 --- /dev/null +++ b/packages/cli/src/commands/secret/orphans.journal-mode.integration.test.ts @@ -0,0 +1,169 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * `os secret orphans` (the report) against a real SQLite file still on a + * rollback journal, through the real read-only boot: what the report says + * about the database stays true for that file. + * + * The boot is read-only (no DDL, no seed, reads only), but the SQL driver's + * connect switches a file still on a rollback journal to WAL — its rule for + * every connect, read-only boots included — which rewrites the file header and + * no row. So the report says that no row or table was written or deleted and + * names the journal-mode switch (`JOURNAL_MODE_SWITCH_NOTE`); it never says + * that nothing was written. + * + * One command stands for the family here because the family shares the + * mechanism and the test: `os migrate security-catalog-overlays`, `recorded-by` + * and `resume`, `os secret orphans` and `rewrap` and `os storage orphans` all + * boot through `bootSchemaStack` onto the same `SqlDriver` connect, and the + * four that print on any dialect ask the same question of the booted primary + * (`describeDriverSqliteFile(stack.driver?.config)`). This case is where that + * question meets a real driver's config; each command's own sentence is pinned + * beside it against a replaced boot. + * + * The header is read before and after, so the case cannot pass over a file + * that was never switched. If a read-only boot ever stops switching it, the + * header assertion goes red: the note is then no longer needed, and this pin + * goes with it. + */ + +import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'; +import { Config } from '@oclif/core'; +import Database from 'better-sqlite3'; +import { mkdtempSync, readFileSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { isExitSignal } from '../../utils/format.js'; +import { JOURNAL_MODE_SWITCH_NOTE } from '../../utils/sqlite-occupancy.js'; + +// Pay the first transform of the dist-resolved workspace deps the boot and the +// report reach through dynamic `import()`s at MODULE LOAD, not inside a case. +import '@objectstack/runtime'; +import '@objectstack/service-settings'; +import '@objectstack/platform-objects/plugin'; + +type Command = (typeof import('./orphans.js'))['default']; +let SecretOrphans: Command; +let oclifConfig: Config; + +const CASE_TIMEOUT_MS = 60_000; +const CLI_ROOT = fileURLToPath(new URL('../../..', import.meta.url)); + +// The command's module graph is large, and oclif reads the package's config +// (its whole command table) on a first run: both are paid here, outside the +// case, which then clocks the boot and the report alone. +beforeAll(async () => { + ({ default: SecretOrphans } = await import('./orphans.js')); + oclifConfig = await Config.load({ root: CLI_ROOT }); +}, 60_000); + +/** + * Env that would point the boot elsewhere, ask the driver for another journal + * mode, or hand the settings service a key from this runner's own key home. + * `OS_HOME` is pointed at the case directory instead: an empty key home. + */ +const OVERRIDING_ENV = [ + 'OS_DATABASE_URL', 'DATABASE_URL', 'TURSO_DATABASE_URL', 'OS_DATABASE_DRIVER', 'OS_DATABASE_SQLITE_JOURNAL_MODE', + 'OS_SECRET_KEY', 'OS_DEV_CRYPTO_KEY', 'OBJECTSTACK_DEV_CRYPTO_KEY', 'OS_HOME', 'OBJECTSTACK_HOME', 'OS_CRYPTO_AUTOKEY', +] as const; + +let dir: string; +let file: string; +let savedCwd: string; +const savedEnv: Record = {}; + +beforeEach(() => { + savedCwd = process.cwd(); + for (const key of OVERRIDING_ENV) { + savedEnv[key] = process.env[key]; + delete process.env[key]; + } + dir = mkdtempSync(join(tmpdir(), 'os-secret-orphans-journal-mode-')); + process.env.OS_HOME = dir; + file = join(dir, 'fixture.db'); + const seed = new Database(file); + seed.pragma('journal_mode = delete'); + seed.exec(` + CREATE TABLE fixture_note (id TEXT PRIMARY KEY, body TEXT); + INSERT INTO fixture_note VALUES ('n1', 'kept'); + `); + seed.close(); + // A project directory with no config and no `.env`: the boot reads the fixture alone. + process.chdir(dir); +}); + +afterEach(() => { + process.chdir(savedCwd); + for (const key of OVERRIDING_ENV) { + if (savedEnv[key] === undefined) delete process.env[key]; + else process.env[key] = savedEnv[key]; + } + rmSync(dir, { recursive: true, force: true }); +}); + +/** The file format's read and write version bytes: 1 and 1 on a rollback journal, 2 and 2 in WAL. */ +function journalBytes(): [number, number] { + const header = readFileSync(file).subarray(0, 100); + return [header[18], header[19]]; +} + +/** The schema and every row, read on a connection of our own. */ +function state(): unknown { + const db = new Database(file, { readonly: true }); + try { + return { + schema: db.prepare('SELECT type, name, sql FROM sqlite_master ORDER BY type, name').all(), + rows: db.prepare('SELECT * FROM fixture_note ORDER BY id').all(), + }; + } finally { + db.close(); + } +} + +/** Run the command with its output captured and its exit trapped. */ +async function run(argv: string[]): Promise<{ lines: string; exitCode: number }> { + const savedExit = process.exitCode; + vi.spyOn(process.stdout, 'write').mockImplementation(((_chunk: unknown, ...rest: unknown[]) => { + const cb = rest.find((a) => typeof a === 'function') as (() => void) | undefined; + if (cb) cb(); + return true; + }) as typeof process.stdout.write); + vi.spyOn(process.stderr, 'write').mockImplementation((() => true) as typeof process.stderr.write); + const log = vi.spyOn(console, 'log').mockImplementation(() => {}); + vi.spyOn(console, 'warn').mockImplementation(() => {}); + vi.spyOn(console, 'error').mockImplementation(() => {}); + let thrownExit: number | undefined; + try { + try { + await SecretOrphans.run(argv, oclifConfig); + } catch (error) { + if (!isExitSignal(error)) throw error; + thrownExit = (error as { oclif?: { exit?: number } }).oclif?.exit; + } + return { + lines: log.mock.calls.map((c) => c.map(String).join(' ')).join('\n'), + exitCode: thrownExit ?? (process.exitCode as number | undefined) ?? 0, + }; + } finally { + process.exitCode = savedExit; + vi.restoreAllMocks(); + } +} + +describe('os secret orphans (report) on a SQLite file still on a rollback journal', () => { + it('the boot switches the header to WAL and no row; the report says no row or table and names the switch', async () => { + expect(journalBytes(), 'the fixture is not on a rollback journal: nothing here could be switched').toEqual([1, 1]); + const before = state(); + + const { lines, exitCode } = await run(['--no-declared-datasources', '--database-url', `file:${file}`]); + + expect(exitCode, lines).toBe(0); + expect(journalBytes()).toEqual([2, 2]); + expect(state()).toEqual(before); + const closing = lines.split('\n').find((line) => line.includes('Report only')); + expect(closing, lines).toBeDefined(); + expect(closing).toContain(`Report only — no row or table was written or deleted. ${JOURNAL_MODE_SWITCH_NOTE}`); + expect(lines).not.toContain('nothing was written'); + }, CASE_TIMEOUT_MS); +}); diff --git a/packages/cli/src/commands/secret/orphans.ts b/packages/cli/src/commands/secret/orphans.ts index 1802837cf68..cc172174118 100644 --- a/packages/cli/src/commands/secret/orphans.ts +++ b/packages/cli/src/commands/secret/orphans.ts @@ -20,6 +20,8 @@ import { import { bootSchemaStack } from '../../utils/schema-migrate.js'; import { oneShotSettingsPlugin } from '../../utils/one-shot-settings.js'; import { absentTableReads, secretUnionReadView } from '../../utils/absent-table-reads.js'; +import { describeDriverSqliteFile } from '../../utils/connection-display.js'; +import { JOURNAL_MODE_SWITCH_NOTE } from '../../utils/sqlite-occupancy.js'; import type { DatasourceArtefactLike, SecretReferenceEngineLike, @@ -95,7 +97,7 @@ import type { export default class SecretOrphans extends Command { static override description = 'Report `sys_secret` rows no producer references any more, and (only with --delete) remove the ' - + 'ones attributable to settings. Report-only by default: it writes nothing and deletes nothing.'; + + 'ones attributable to settings. Report-only by default: it writes no row or table and deletes nothing.'; static override examples = [ '<%= config.bin %> secret orphans', @@ -112,7 +114,7 @@ export default class SecretOrphans extends Command { }), delete: Flags.boolean({ description: - 'Delete the deletable rows (default is a report that writes nothing). Requires --export, ' + 'Delete the deletable rows (default is a report that writes no row or table). Requires --export, ' + 'and refuses whenever the reference union is incomplete.', default: false, }), @@ -233,6 +235,17 @@ export default class SecretOrphans extends Command { } try { + // [#23622] Either mode's connect may have switched a SQLite file's + // journal mode (`SqlDriver.applySqliteJournalMode`, one rule for every + // connect), which rewrites the file header and no row. So no line below + // says that nothing changed: the read-only mode claims no row or table + // (its boot defers DDL), an abort after the plain boot claims no row + // (that boot may have created a missing table), and on a SQLite file + // each says what the connect may still have changed. A target that is + // no SQLite file (PostgreSQL, MySQL) has no header to switch. + const journalNote = describeDriverSqliteFile(stack.driver?.config) !== undefined + ? ` ${JOURNAL_MODE_SWITCH_NOTE}` + : ''; const engine = stack.kernel.getService('objectql') as SecretReferenceEngineLike | undefined; if (!engine) { const message = 'No ObjectQL engine on this runtime, so no producer family can be enumerated.'; @@ -329,7 +342,7 @@ export default class SecretOrphans extends Command { if (!flags.delete) { if (json) { await emitJson({ mode: 'report', plan }, 0, { compact: true }); return; } renderPlan(plan); - printInfo(`Report only — nothing was written or deleted. (${timer.display()})`); + printInfo(`Report only — no row or table was written or deleted.${journalNote} (${timer.display()})`); printInfo('Re-run with --delete --export to remove the deletable rows.'); return; } @@ -369,7 +382,7 @@ export default class SecretOrphans extends Command { const ok = await confirm( chalk.yellow(` Delete ${plan.deletable.length} row(s) after writing the export? [y/N] `), ); - if (!ok) { printInfo('Aborted — nothing was written or deleted.'); return; } + if (!ok) { printInfo(`Aborted — no export was written and no row was deleted.${journalNote}`); return; } } // ── The export, written and READ BACK before anything is deleted ────── diff --git a/packages/cli/src/commands/secret/rewrap.guards.test.ts b/packages/cli/src/commands/secret/rewrap.guards.test.ts index d5d05fc74c8..88ad108cd7b 100644 --- a/packages/cli/src/commands/secret/rewrap.guards.test.ts +++ b/packages/cli/src/commands/secret/rewrap.guards.test.ts @@ -18,6 +18,11 @@ * up by the next boot of this host; * - `--json --apply` without `--yes` is refused. * + * And what the human face says about the database: either mode's connect can + * switch a SQLite file's journal mode, which rewrites its header and no row, + * so neither the dry run nor a declined `--apply` says that nothing was + * written; each names the switch on a SQLite target, and only there. + * * Only the seams that would boot a database are replaced. The reference union, * the planner, the executor, `ciphertextDerivationStatus` and * `LocalCryptoProvider` all run for real. @@ -32,10 +37,24 @@ import { fileURLToPath } from 'node:url'; import { ciphertextDerivationStatus } from '@objectstack/service-settings'; import SecretRewrap from './rewrap.js'; import { bootSchemaStack } from '../../utils/schema-migrate.js'; +import { JOURNAL_MODE_SWITCH_NOTE } from '../../utils/sqlite-occupancy.js'; vi.mock('../../utils/schema-migrate.js', () => ({ bootSchemaStack: vi.fn() })); // Constructed and handed to the (mocked) boot, never used. vi.mock('@objectstack/platform-objects/plugin', () => ({ PlatformObjectsPlugin: class {} })); +// The `--apply` confirmation, answered without a terminal: `undefined` leaves +// readline as it is. +const prompt = vi.hoisted(() => ({ answer: undefined as string | undefined })); +vi.mock('node:readline', async (importOriginal) => { + const real = await importOriginal(); + return { + ...real, + createInterface: ((...args: Parameters) => (prompt.answer === undefined + ? real.createInterface(...args) + : { question: (_q: string, answer: (a: string) => void) => answer(prompt.answer as string), close: () => {} } + )) as typeof real.createInterface, + }; +}); const HERE = dirname(fileURLToPath(import.meta.url)); const CLI_ROOT = resolve(HERE, '..', '..', '..'); @@ -82,7 +101,7 @@ interface Harness { */ function wireBoot( rows: { secrets: Row[]; settings: Row[] }, - opts: { conditionalWrite?: boolean; absent?: readonly string[] } = {}, + opts: { conditionalWrite?: boolean; absent?: readonly string[]; driverConfig?: Record } = {}, ): Harness { const harness: Harness = { secrets: rows.secrets, writes: [], reads: [] }; const secretDriver: Record = { @@ -118,6 +137,8 @@ function wireBoot( }; const absent = new Set(opts.absent ?? []); vi.mocked(bootSchemaStack).mockResolvedValue({ + // The primary datasource's driver, as `SchemaStack.driver` carries it. + ...(opts.driverConfig ? { driver: { config: opts.driverConfig } } : {}), kernel: { getService: (name: string) => (name === 'objectql' ? engine : undefined) }, tableAbsent: (objectName: string) => absent.has(objectName), shutdown: async () => {}, @@ -287,3 +308,69 @@ describe('os secret rewrap — guards that stop a run before any row is opened o } }, 60_000); }); + +// ─────────────────────────────────────────────────────────────────────────── +// The human face: what the dry run and a declined --apply say about the database +// ─────────────────────────────────────────────────────────────────────────── + +const SQLITE_CONFIG = { client: 'better-sqlite3', connection: { filename: '/srv/app/.objectstack/data/objectstack.db' } }; +const POSTGRES_CONFIG = { client: 'pg', connection: 'postgres://db.example.com:5432/app' }; + +/** One human-mode run; every line `console.log` received. */ +async function runHuman(argv: string[]): Promise { + const log = vi.spyOn(console, 'log').mockImplementation(() => {}); + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + const error = vi.spyOn(console, 'error').mockImplementation(() => {}); + const savedExitCode = process.exitCode; + try { + await SecretRewrap.run(argv, { root: CLI_ROOT }); + return log.mock.calls.map((c) => c.map(String).join(' ')); + } finally { + process.exitCode = savedExitCode; + for (const spy of [log, warn, error]) spy.mockRestore(); + } +} + +describe('os secret rewrap — the human face names no write it cannot vouch for', () => { + let savedIsTTY: PropertyDescriptor | undefined; + beforeEach(() => { + savedIsTTY = Object.getOwnPropertyDescriptor(process.stdin, 'isTTY'); + }); + afterEach(() => { + prompt.answer = undefined; + if (savedIsTTY) Object.defineProperty(process.stdin, 'isTTY', savedIsTTY); + else delete (process.stdin as { isTTY?: boolean }).isTTY; + }); + + it('the dry run claims no row or table, and names the journal-mode switch on a SQLite file', async () => { + const h = wireBoot(freshRows(), { driverConfig: SQLITE_CONFIG }); + const lines = await runHuman(['--no-declared-datasources']); + expect(h.writes).toEqual([]); + const closing = lines.find((line) => line.includes('Dry run —')); + expect(closing, lines.join('\n')).toContain(`Dry run — no row or table was written. ${JOURNAL_MODE_SWITCH_NOTE}`); + }, 60_000); + + it('on a target with no file header (PostgreSQL) the dry run claims no row or table and names no switch', async () => { + wireBoot(freshRows(), { driverConfig: POSTGRES_CONFIG }); + const lines = await runHuman(['--no-declared-datasources']); + const closing = lines.find((line) => line.includes('Dry run —')); + expect(closing, lines.join('\n')).toContain('Dry run — no row or table was written.'); + expect(lines.join('\n')).not.toContain(JOURNAL_MODE_SWITCH_NOTE); + }, 60_000); + + it('an --apply declined at the prompt claims no row opened or written, and names the switch on a SQLite file', async () => { + Object.defineProperty(process.stdin, 'isTTY', { value: true, configurable: true }); + prompt.answer = 'n'; + const h = wireBoot(freshRows(), { driverConfig: SQLITE_CONFIG }); + const lines = await runHuman(['--apply', '--no-declared-datasources']); + expect(h.writes).toEqual([]); + const aborted = lines.find((line) => line.includes('Aborted')); + expect(aborted, lines.join('\n')).toContain(`Aborted — no row was opened or written. ${JOURNAL_MODE_SWITCH_NOTE}`); + }, 60_000); + + it('the help says the dry run writes no row or table, never that it writes nothing', () => { + expect(SecretRewrap.description).toContain('A dry run by default: it writes no row or table without --apply.'); + expect(SecretRewrap.flags.apply.description).toContain('(default is a dry run that writes no row or table)'); + expect(`${SecretRewrap.description} ${SecretRewrap.flags.apply.description}`).not.toContain('writes nothing'); + }); +}); diff --git a/packages/cli/src/commands/secret/rewrap.ts b/packages/cli/src/commands/secret/rewrap.ts index 94222576227..5a312851ef9 100644 --- a/packages/cli/src/commands/secret/rewrap.ts +++ b/packages/cli/src/commands/secret/rewrap.ts @@ -18,6 +18,8 @@ import { import { bootSchemaStack } from '../../utils/schema-migrate.js'; import { oneShotSettingsPlugin, resolveExistingDataKey } from '../../utils/one-shot-settings.js'; import { absentTableReads, secretUnionReadView } from '../../utils/absent-table-reads.js'; +import { describeDriverSqliteFile } from '../../utils/connection-display.js'; +import { JOURNAL_MODE_SWITCH_NOTE } from '../../utils/sqlite-occupancy.js'; import type { DatasourceArtefactLike, SecretReferenceEngineLike, @@ -66,7 +68,7 @@ import { readDeclaredDatasources } from './orphans.js'; export default class SecretRewrap extends Command { static override description = 'Re-wrap version-1 `sys_secret` ciphertext under the current AAD derivation, each row under the ' - + 'scope of the producer that holds it. A dry run by default: it writes nothing without --apply.'; + + 'scope of the producer that holds it. A dry run by default: it writes no row or table without --apply.'; static override examples = [ '<%= config.bin %> secret rewrap --no-declared-datasources', @@ -82,7 +84,7 @@ export default class SecretRewrap extends Command { }), apply: Flags.boolean({ description: - 'Write the re-wrapped rows (default is a dry run that writes nothing). Refuses whenever a ' + 'Write the re-wrapped rows (default is a dry run that writes no row or table). Refuses whenever a ' + 'holder family could not be enumerated.', default: false, }), @@ -184,6 +186,17 @@ export default class SecretRewrap extends Command { } try { + // [#23622] Either mode's connect may have switched a SQLite file's + // journal mode (`SqlDriver.applySqliteJournalMode`, one rule for every + // connect), which rewrites the file header and no row. So no line below + // says that nothing changed: the read-only mode claims no row or table + // (its boot defers DDL), an abort after the plain boot claims no row + // (that boot may have created a missing table), and on a SQLite file + // each says what the connect may still have changed. A target that is + // no SQLite file (PostgreSQL, MySQL) has no header to switch. + const journalNote = describeDriverSqliteFile(stack.driver?.config) !== undefined + ? ` ${JOURNAL_MODE_SWITCH_NOTE}` + : ''; const engine = stack.kernel.getService('objectql') as SecretReferenceEngineLike | undefined; if (!engine) { const message = 'No ObjectQL engine on this runtime, so no holder family can be enumerated.'; @@ -291,7 +304,7 @@ export default class SecretRewrap extends Command { const ok = await confirm( chalk.yellow(` Open, re-seal, verify and write up to ${plan.attempts} sys_secret row(s)? [y/N] `), ); - if (!ok) { printInfo('Aborted — nothing was opened or written.'); return; } + if (!ok) { printInfo(`Aborted — no row was opened or written.${journalNote}`); return; } } const result = await executeSysSecretRewrap({ @@ -313,7 +326,7 @@ export default class SecretRewrap extends Command { if (flags.apply) { printSuccess(`Re-wrapped ${report.counts.rewrap} sys_secret row(s) in ${timer.display()}.`); } else { - printInfo(`Dry run — nothing was written. (${timer.display()})`); + printInfo(`Dry run — no row or table was written.${journalNote} (${timer.display()})`); } if (exitCode !== 0) this.exit(exitCode); } catch (error) { diff --git a/packages/cli/src/commands/storage/orphans.test.ts b/packages/cli/src/commands/storage/orphans.test.ts new file mode 100644 index 00000000000..385eda73232 --- /dev/null +++ b/packages/cli/src/commands/storage/orphans.test.ts @@ -0,0 +1,23 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * `os storage orphans` — what its help says the inventory writes. + * + * The inventory boots read-only (deferred DDL, no seed, no file brought into + * existence), but the boot's connect can switch a SQLite file's journal mode, + * which rewrites the file's header and no row (measured on a rollback-journal + * file: header bytes 18 and 19 from 1,1 to 2,2, the change counter up by one, + * schema and rows byte-identical). So the help claims no row or table, never + * that the command writes nothing. The help names no target, so it carries no + * SQLite note. + */ + +import { describe, expect, it } from 'vitest'; +import StorageOrphans from './orphans.js'; + +describe('os storage orphans --help', () => { + it('says the inventory writes no row or table, tombstones nothing and deletes nothing', () => { + expect(StorageOrphans.description).toContain('Writes no row or table, tombstones nothing, deletes nothing.'); + expect(StorageOrphans.description).not.toContain('Writes nothing'); + }); +}); diff --git a/packages/cli/src/commands/storage/orphans.ts b/packages/cli/src/commands/storage/orphans.ts index 3c475a8d9da..53ba7f61ce1 100644 --- a/packages/cli/src/commands/storage/orphans.ts +++ b/packages/cli/src/commands/storage/orphans.ts @@ -58,7 +58,7 @@ export default class StorageOrphans extends Command { static override description = 'Report-only inventory of stranded sys_file orphans — attachments-scope files that no ' + 'sys_attachment join row and no ref_* owner holds, and that no lifecycle policy can ever ' + - 'nominate for reaping. Writes nothing, tombstones nothing, deletes nothing.'; + 'nominate for reaping. Writes no row or table, tombstones nothing, deletes nothing.'; static override examples = [ '$ os storage orphans',