diff --git a/reference-implementation/scripts/check-test-backends.test.ts b/reference-implementation/scripts/check-test-backends.test.ts new file mode 100644 index 00000000..460fb7c1 --- /dev/null +++ b/reference-implementation/scripts/check-test-backends.test.ts @@ -0,0 +1,375 @@ +// Copyright The PDP-Connect Contributors +// SPDX-License-Identifier: Apache-2.0 + +// Fixture-driven checks for the backend classifier. Each case builds the +// exact disagreement it is about -- a manifest that omits a file, one that +// names a file that no longer exists, one that declares a database-importing +// file as needing no database -- and asserts the checker rejects it. The +// positive controls matter as much as the negative ones: a checker that +// rejects everything is as useless as one that rejects nothing. + +import { strict as assert } from "node:assert/strict"; +import { spawnSync } from "node:child_process"; +import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { dirname, join } from "node:path"; +import test from "node:test"; +import { fileURLToPath } from "node:url"; + +import { + type Backend, + checkBackendManifest, + enumerateTestEntries, + importedSpecifiers, + specifierNamesPackage, + specifierNamesStorageModule, + storageImports, +} from "./check-test-backends.ts"; +import { trackedFiles } from "./test-accounting/inventory.ts"; + +const NO_SOURCE = () => ""; +const NOT_ONE_OF_RE = /not one of/; +const STORAGE_MODULE_RE = /storage module/; +const MISSING_OR_STALE_RE = /missing-entry|stale-entry/; +const USAGE_RE = /usage/; + +const SCRIPT = join(dirname(fileURLToPath(import.meta.url)), "check-test-backends.ts"); +const RI_ROOT = join(dirname(fileURLToPath(import.meta.url)), ".."); + +/** + * Run the checker as a real command and return its actual exit code. + * + * The exit code is the entire contract of a CLI gate, and an in-process + * assertion cannot observe it: before the entry guard existed, calling the + * exported `main` rejected a bad manifest while running the file as a command + * exited 0 and printed nothing. + */ +function runCli(args: readonly string[]) { + const { NODE_TEST_CONTEXT: _parentTestContext, ...env } = process.env; + const result = spawnSync(process.execPath, ["--import", "tsx", SCRIPT, ...args], { + cwd: RI_ROOT, + encoding: "utf8", + env, + timeout: 120_000, + }); + return { ...result, output: `${result.stdout ?? ""}${result.stderr ?? ""}` }; +} + +/** Write a manifest to a scratch file and hand its path to the CLI. */ +function runCliWithManifest(manifest: unknown) { + const dir = mkdtempSync(join(tmpdir(), "pdpp-backend-cli-")); + try { + const file = join(dir, "manifest.json"); + writeFileSync(file, JSON.stringify(manifest)); + return runCli([file]); + } finally { + rmSync(dir, { force: true, recursive: true }); + } +} + +function sourcesFor(sources: Record) { + return (path: string) => sources[path] ?? ""; +} + +test("a manifest that matches the enumerated entries exactly is accepted", () => { + const enumerated = ["reference-implementation/test/a.test.ts", "reference-implementation/test/b.test.ts"]; + const violations = checkBackendManifest( + { + entries: [ + { backend: "none", path: "reference-implementation/test/a.test.ts" }, + { backend: "postgres", path: "reference-implementation/test/b.test.ts" }, + ], + }, + enumerated, + NO_SOURCE + ); + + assert.deepEqual(violations, []); +}); + +test("an enumerated entry absent from the manifest is rejected as missing", () => { + const violations = checkBackendManifest( + { entries: [{ backend: "none", path: "reference-implementation/test/a.test.ts" }] }, + ["reference-implementation/test/a.test.ts", "reference-implementation/test/unclassified.test.ts"], + NO_SOURCE + ); + + assert.deepEqual( + violations.map((violation) => [violation.kind, violation.path]), + [["missing-entry", "reference-implementation/test/unclassified.test.ts"]] + ); +}); + +test("a manifest entry with no enumerated test entry is rejected as stale", () => { + const violations = checkBackendManifest( + { + entries: [ + { backend: "none", path: "reference-implementation/test/a.test.ts" }, + { backend: "sqlite", path: "reference-implementation/test/deleted.test.ts" }, + ], + }, + ["reference-implementation/test/a.test.ts"], + NO_SOURCE + ); + + assert.deepEqual( + violations.map((violation) => [violation.kind, violation.path]), + [["stale-entry", "reference-implementation/test/deleted.test.ts"]] + ); +}); + +test("a path declared twice is rejected as a duplicate before the set conversion hides it", () => { + // The two entries disagree about the backend. A map-keyed manifest would + // keep only the last one and report a clean run, which is the bug this + // array-plus-duplicate-check shape exists to prevent. + const violations = checkBackendManifest( + { + entries: [ + { backend: "postgres", path: "reference-implementation/test/a.test.ts" }, + { backend: "none", path: "reference-implementation/test/a.test.ts" }, + ], + }, + ["reference-implementation/test/a.test.ts"], + NO_SOURCE + ); + + assert.deepEqual( + violations.map((violation) => violation.kind), + ["duplicate-entry"] + ); +}); + +test("a backend outside the declared set is rejected", () => { + const violations = checkBackendManifest( + { + entries: [{ backend: "mixed" as unknown as Backend, path: "reference-implementation/test/a.test.ts" }], + }, + ["reference-implementation/test/a.test.ts"], + NO_SOURCE + ); + + assert.deepEqual( + violations.map((violation) => violation.kind), + ["unknown-backend"] + ); + assert.match(violations[0]?.detail ?? "", NOT_ONE_OF_RE); +}); + +test("a file importing a database cannot be declared as needing none", () => { + const violations = checkBackendManifest( + { entries: [{ backend: "none", path: "reference-implementation/test/a.test.ts" }] }, + ["reference-implementation/test/a.test.ts"], + sourcesFor({ + "reference-implementation/test/a.test.ts": 'import { getDb } from "../server/db.ts";\n', + }) + ); + + assert.deepEqual( + violations.map((violation) => violation.kind), + ["storage-import-in-none"] + ); + assert.match(violations[0]?.detail ?? "", STORAGE_MODULE_RE); +}); + +test("the same file declared with a real backend is accepted", () => { + // The import check adds obligations; it never invents them. Declaring the + // backend the code actually needs must pass, or the checker would just be + // banning database tests. + const violations = checkBackendManifest( + { entries: [{ backend: "sqlite", path: "reference-implementation/test/a.test.ts" }] }, + ["reference-implementation/test/a.test.ts"], + sourcesFor({ + "reference-implementation/test/a.test.ts": 'import { getDb } from "../server/db.ts";\n', + }) + ); + + assert.deepEqual(violations, []); +}); + +// Per-specifier-form coverage. A deny rule that matches a subpath but not the +// bare specifier reports zero violations on a file that genuinely imports +// Postgres -- a silent false pass. Every form is asserted separately so no +// single spelling can regress unnoticed. +for (const [form, specifier, pkg] of [ + ["bare package", "pg", "pg"], + ["package subpath", "pg/lib/client", "pg"], + ["node: builtin", "node:sqlite", "node:sqlite"], + ["bare builtin", "sqlite", "node:sqlite"], + ["native package", "better-sqlite3", "better-sqlite3"], +] as const) { + test(`a denied ${form} specifier is detected`, () => { + assert.equal(specifierNamesPackage(specifier, pkg), true); + }); +} + +test("a package whose name merely starts with a denied name is not detected", () => { + // "pgvector" is not "pg". Without a separator boundary this check would + // deny unrelated packages, and a checker with false positives gets disabled. + assert.equal(specifierNamesPackage("pgvector", "pg"), false); + assert.equal(specifierNamesPackage("pg-boss", "pg"), false); +}); + +// Loader paths. A storage import counts however it is written, because the +// choice between static import, dynamic import and require is not a +// meaningful difference in backend obligation. +for (const [loader, source] of [ + ["static import", 'import { getDb } from "../server/db.ts";'], + ["bare side-effect import", 'import "../server/db.ts";'], + ["literal dynamic import", 'const db = await import("../server/db.ts");'], + ["require", 'const db = require("../server/db.ts");'], + ["createRequire", 'const db = createRequire(import.meta.url)("better-sqlite3");'], + // Resolving a driver and importing the resulting path is a load whose own + // import carries no package name. Naming the driver in `resolve` is the last + // point at which source reading can see it, so it counts here. + ["resolve then import", 'const url = pathToFileURL(req.resolve("pg")).href;\nconst pg = await import(url);'], + ["resolve then require", 'const pg = req(req.resolve("pg"));'], + // A builtin fetched off the process object involves no import or require at + // all, so this route has to be read separately. + ["getBuiltinModule", 'const { DatabaseSync } = process.getBuiltinModule("node:sqlite");'], +] as const) { + test(`a storage dependency loaded by ${loader} is detected`, () => { + assert.notDeepEqual(storageImports(source), []); + }); +} + +test("fetching an allowed builtin is not a storage dependency", () => { + // Every test file reaches for node:path and node:assert this way. + for (const source of [ + 'const p = process.getBuiltinModule("node:path");', + 'const a = process.getBuiltinModule("node:assert");', + ]) { + assert.deepEqual(storageImports(source), []); + } +}); + +test("a computed builtin name is not recoverable by reading source", () => { + // Recorded deliberately, like the computed-specifier case: this is the bound + // on source reading, and the reason the runtime guard wraps the function + // itself rather than scanning for names. + assert.deepEqual(storageImports('const n = "node:" + "sqlite";\nprocess.getBuiltinModule(n);'), []); +}); + +test("resolving an unrelated package or path is not a storage dependency", () => { + // `resolve` is a general-purpose call. Only a denied driver name makes it + // interesting, or every file that uses path.resolve would be flagged. + for (const source of [ + 'const v = req.resolve("pgvector");', + 'const p = path.resolve("a", "b");', + 'const p = resolve(dir, "fixture.json");', + ]) { + assert.deepEqual(storageImports(source), []); + } +}); + +test("a computed specifier yields no literal to detect, which bounds this check", () => { + // Recorded deliberately: source reading cannot recover a computed + // specifier, so this checker cannot be the only control. The runtime guard + // in scripts/test-unit-preload.mjs covers the executed case. + assert.deepEqual(storageImports('const db = await import(base + "/db.ts");'), []); +}); + +test("ordinary non-storage imports are not flagged", () => { + assert.deepEqual(storageImports('import { join } from "node:path";\nimport test from "node:test";'), []); +}); + +test("importedSpecifiers recovers each literal specifier once", () => { + assert.deepEqual(importedSpecifiers('import a from "node:path";\nimport a2 from "node:path";\nrequire("pg");'), [ + "node:path", + "pg", + ]); +}); + +test("a storage module is recognised through any relative spelling", () => { + assert.equal(specifierNamesStorageModule("../server/db.ts"), "server/db.ts"); + assert.equal(specifierNamesStorageModule("../../reference-implementation/server/db.ts"), "server/db.ts"); + assert.equal(specifierNamesStorageModule("./helpers/fixture.ts"), undefined); +}); + +test("enumeration selects tracked RI test entries and nothing else", () => { + const entries = enumerateTestEntries([ + "reference-implementation/test/a.test.ts", + "reference-implementation/runtime/b.test.ts", + "reference-implementation/scripts/c.test.mjs", + "reference-implementation/server/streaming/d.test.ts", + // Not test entries: a helper, a product source file, and a test that + // belongs to a different package. + "reference-implementation/test/helpers/fixture.ts", + "reference-implementation/server/db.ts", + "packages/polyfill-connectors/src/x.test.ts", + ]); + + assert.deepEqual(entries, [ + "reference-implementation/runtime/b.test.ts", + "reference-implementation/scripts/c.test.mjs", + "reference-implementation/server/streaming/d.test.ts", + "reference-implementation/test/a.test.ts", + ]); +}); + +test("every violation kind reports the path it concerns", () => { + const violations = checkBackendManifest( + { + entries: [ + { backend: "none", path: "reference-implementation/test/dup.test.ts" }, + { backend: "none", path: "reference-implementation/test/dup.test.ts" }, + { backend: "none", path: "reference-implementation/test/stale.test.ts" }, + ], + }, + ["reference-implementation/test/dup.test.ts", "reference-implementation/test/absent.test.ts"], + NO_SOURCE + ); + + assert.deepEqual( + violations.map((violation) => [violation.path, violation.kind]), + [ + ["reference-implementation/test/absent.test.ts", "missing-entry"], + ["reference-implementation/test/dup.test.ts", "duplicate-entry"], + ["reference-implementation/test/stale.test.ts", "stale-entry"], + ] + ); + for (const violation of violations) { + assert.notEqual(violation.detail, ""); + } +}); + +// The command surface. `main` is exported and unit-testable, but a checker is +// only a gate if running it as a command actually fails the caller, so each +// case below asserts on a real process exit code. +test("the command rejects a manifest with a violation", () => { + const result = runCliWithManifest({ + entries: [{ backend: "none", path: "reference-implementation/test/does-not-exist.test.ts" }], + }); + + assert.equal(result.status, 1, result.output); + assert.match(result.output, MISSING_OR_STALE_RE); +}); + +test("the command reports usage and fails when given no manifest", () => { + // A missing argument must not be a silent success. + const result = runCli([]); + + assert.equal(result.status, 2, result.output); + assert.match(result.output, USAGE_RE); +}); + +test("the command fails rather than passing when the manifest is unreadable", () => { + const result = runCli([join(tmpdir(), "pdpp-no-such-manifest-6f2a.json")]); + + assert.notEqual(result.status, 0, result.output); +}); + +test("the command accepts a manifest that classifies every tracked test entry", () => { + // The positive control: without it, a checker that rejected everything would + // pass every test above. The backend value is uniform because this asserts + // the manifest/tree agreement, not per-file obligations. + const entries = enumerateTestEntries(trackedFiles(join(RI_ROOT, ".."))).map((path) => ({ + backend: "sqlite" as const, + path, + })); + assert.ok(entries.length > 0, "expected the tracked tree to contain test entries"); + + const result = runCliWithManifest({ entries }); + + assert.equal(result.status, 0, result.output); + assert.match(result.output, new RegExp(`${entries.length} entries classified`)); +}); diff --git a/reference-implementation/scripts/check-test-backends.ts b/reference-implementation/scripts/check-test-backends.ts new file mode 100644 index 00000000..3273381f --- /dev/null +++ b/reference-implementation/scripts/check-test-backends.ts @@ -0,0 +1,308 @@ +// Copyright The PDP-Connect Contributors +// SPDX-License-Identifier: Apache-2.0 + +/** + * Backend classifier for reference-implementation test entries. + * + * Every RI test entry has a backend obligation -- it needs SQLite, it needs + * Postgres, it needs both, or it needs no database at all -- and today that + * obligation is implicit. It lives in whichever fixtures a file happens to + * import, so the only way to learn it is to run the file and watch what it + * connects to. That makes the obligation invisible to scheduling: a case that + * requires Postgres and a case that requires nothing look identical to the + * runner, so the runner cannot allocate a database to the first without + * allocating one to the second, and cannot tell a Postgres case that silently + * skipped from one that actually ran. + * + * This module makes the obligation explicit and checkable. A manifest + * declares one entry per test file with a backend; this checker independently + * enumerates the tracked test files and rejects any disagreement. + * + * Two rules carry the weight: + * + * Exact set equality. The manifest must name every enumerated entry and no + * others. A missing entry (a new test file nobody classified), a stale + * entry (a classification for a deleted file), a duplicate entry and an + * unknown backend value all fail. Set equality is what makes the manifest + * trustworthy as a scheduling input -- a manifest that merely permits + * unlisted files tells the runner nothing about the files it omits. + * + * Declaration is not proof. An entry claiming `none` -- no database -- is + * checked against its actual imports, and an entry whose import closure + * reaches a storage module or SQL driver is rejected however it is + * labelled. There is no override. A label can only ever add an obligation, + * never remove one the code demonstrably has. + * + * The import scan here is deliberately shallow: it reads the entry file's own + * static and literal-dynamic imports, and it does not follow the graph + * transitively or resolve aliases, computed specifiers or generated code. + * That bounds what it can prove. It catches a file that imports a database + * directly, which is the common mislabelling; it cannot catch one that + * reaches storage three modules deep. scripts/test-unit-preload.mjs is the + * runtime counterpart that closes that gap for executed edges, and the two + * are meant to be read together -- neither alone is sufficient. + * + * Scope note: this checker ships with a fixture-driven test suite and no + * production manifest. Classifying the full RI inventory is separate work; + * shipping a partial manifest would be worse than shipping none, because set + * equality against a partial list is not a meaningful check. + */ + +import { readFileSync } from "node:fs"; +import { join } from "node:path"; +import { pathToFileURL } from "node:url"; + +import { compareStrings, EXECUTABLE_TEST_SUFFIX, normalizePath, trackedFiles } from "./test-accounting/inventory.ts"; + +/** Backends an entry may declare. */ +export const BACKENDS = ["none", "sqlite", "postgres", "sqlite+postgres"] as const; +export type Backend = (typeof BACKENDS)[number]; + +/** One declared test entry. */ +export interface BackendEntry { + /** The backend this entry requires. */ + backend: Backend; + /** Repository-relative path to the test file. */ + path: string; +} + +/** + * The manifest is an ARRAY, not a map keyed by path. A map cannot represent a + * duplicate -- a second entry for the same path silently overwrites the first + * during parsing, so the check would never see it. Duplicates are rejected + * below, before any conversion to a set. + */ +export interface BackendManifest { + entries: BackendEntry[]; +} + +/** Storage modules whose import proves a database obligation. */ +export const STORAGE_MODULES = ["server/db.ts", "lib/db.ts", "server/postgres-storage.ts"]; + +/** SQL driver packages whose import proves a database obligation. */ +export const SQL_DRIVERS = ["pg", "better-sqlite3", "node:sqlite"]; + +/** Directories enumerated for RI test entries, relative to the RI root. */ +export const TEST_ROOTS = ["test", "runtime", "scripts", join("server", "streaming")]; + +export interface Violation { + detail: string; + kind: "missing-entry" | "stale-entry" | "duplicate-entry" | "unknown-backend" | "storage-import-in-none"; + path: string; +} + +/** + * Enumerate tracked RI test entries independently of the manifest. + * + * Enumeration reads the git index rather than walking the filesystem, so an + * untracked scratch file cannot enter the inventory and a tracked file cannot + * hide from it by being absent from a directory listing. + */ +export function enumerateTestEntries(trackedPaths: readonly string[]): string[] { + const prefix = "reference-implementation/"; + const roots = TEST_ROOTS.map((root) => `${prefix}${normalizePath(root)}/`); + return trackedPaths + .map((path) => normalizePath(path)) + .filter((path) => EXECUTABLE_TEST_SUFFIX.test(path) && roots.some((root) => path.startsWith(root))) + .sort(compareStrings); +} + +const STATIC_IMPORT_RE = /^\s*import\s[^;]*?from\s*["']([^"']+)["']/gm; +const BARE_IMPORT_RE = /^\s*import\s*["']([^"']+)["']/gm; +const DYNAMIC_IMPORT_RE = /\bimport\s*\(\s*["']([^"']+)["']\s*\)/g; +const REQUIRE_RE = /\brequire\s*\(\s*["']([^"']+)["']\s*\)/g; +// `createRequire(import.meta.url)("better-sqlite3")` loads a module without +// the specifier ever appearing inside a call spelled `require(...)`: the +// specifier sits in a call on the RESULT of createRequire. server/db.ts uses +// exactly this shape to reach better-sqlite3, so a scan that only looked for +// `require(` would miss the repository's own primary SQLite entry point. +const CREATE_REQUIRE_CALL_RE = /\bcreateRequire\s*\([^)]*\)\s*\(\s*["']([^"']+)["']\s*\)/g; +// `req.resolve("pg")` turns a package name into a path, which can then be +// imported as a file URL or absolute path. The load that follows carries no +// package name at all, so a scan that ignored `resolve` would report a file +// reaching Postgres as having no storage dependency. Naming a denied driver +// here is treated as reaching it: the only reason to resolve a driver is to +// load it. +const RESOLVE_CALL_RE = /\.resolve\s*\(\s*["']([^"']+)["']\s*\)/g; +// `process.getBuiltinModule("node:sqlite")` returns a builtin without any +// import or require, so it appears in none of the patterns above. The runtime +// guard covers it by wrapping the function; source reading covers the literal +// form here so a mislabelled file is caught before it is ever run. +const GET_BUILTIN_MODULE_RE = /\bgetBuiltinModule\s*\(\s*["']([^"']+)["']\s*\)/g; + +/** + * Literal module specifiers this source uses to reach a module, by any of the + * loader routes above -- import, require, createRequire, resolve, or + * process.getBuiltinModule. + * + * Only literal specifiers are recoverable by reading source. A computed + * specifier -- `import(base + name)` -- yields no string here, which is + * precisely why this checker cannot be the only control. + */ +export function importedSpecifiers(source: string): string[] { + const found = new Set(); + for (const pattern of [ + STATIC_IMPORT_RE, + BARE_IMPORT_RE, + DYNAMIC_IMPORT_RE, + REQUIRE_RE, + CREATE_REQUIRE_CALL_RE, + RESOLVE_CALL_RE, + GET_BUILTIN_MODULE_RE, + ]) { + pattern.lastIndex = 0; + for (const [, specifier] of source.matchAll(pattern)) { + if (specifier) { + found.add(specifier); + } + } + } + return [...found].sort(compareStrings); +} + +/** + * Does `specifier` name `pkg`, in any of the forms a specifier can take? + * + * Matching only the subpath form ("pg/lib/client") while missing the bare + * form ("pg") reports zero violations on a file that plainly imports + * Postgres. That is a silent false pass, so every form is matched: bare + * package, subpath, and both spellings of a builtin. The boundary after the + * package name must be `/` or end-of-string, or "pg" would also match the + * unrelated "pgvector". + */ +export function specifierNamesPackage(specifier: string, pkg: string): boolean { + const bare = pkg.startsWith("node:") ? pkg.slice("node:".length) : pkg; + const forms = pkg.startsWith("node:") ? [pkg, bare] : [pkg, `node:${pkg}`]; + return forms.some((form) => specifier === form || specifier.startsWith(`${form}/`)); +} + +/** Does `specifier` point at a known storage module, however it is spelled? */ +export function specifierNamesStorageModule(specifier: string): string | undefined { + const normalized = specifier.replaceAll("\\", "/"); + return STORAGE_MODULES.find((module) => normalized === module || normalized.endsWith(`/${module}`)); +} + +/** + * Storage dependencies this source demonstrably has, as human-readable + * reasons. Empty means "no storage import found by this scan" -- which is a + * weaker statement than "this file touches no database". + */ +export function storageImports(source: string): string[] { + const reasons: string[] = []; + for (const specifier of importedSpecifiers(source)) { + const driver = SQL_DRIVERS.find((pkg) => specifierNamesPackage(specifier, pkg)); + if (driver) { + reasons.push(`reaches SQL driver "${specifier}"`); + continue; + } + const module = specifierNamesStorageModule(specifier); + if (module) { + reasons.push(`imports storage module "${specifier}"`); + } + } + return reasons; +} + +/** + * Check a manifest against the enumerated entries and the files themselves. + * + * Returns every violation found rather than throwing on the first, so one run + * reports the whole gap instead of revealing it one commit at a time. + */ +export function checkBackendManifest( + manifest: BackendManifest, + enumerated: readonly string[], + readSource: (path: string) => string +): Violation[] { + const violations: Violation[] = []; + const entries = manifest.entries ?? []; + + // Duplicates first: this must precede any set conversion, because the + // conversion is exactly what makes a duplicate invisible. + const seen = new Set(); + for (const entry of entries) { + const path = normalizePath(entry.path); + if (seen.has(path)) { + violations.push({ detail: "declared more than once", kind: "duplicate-entry", path }); + } + seen.add(path); + } + + for (const entry of entries) { + if (!BACKENDS.includes(entry.backend)) { + violations.push({ + detail: `backend "${entry.backend}" is not one of ${BACKENDS.join(", ")}`, + kind: "unknown-backend", + path: normalizePath(entry.path), + }); + } + } + + // Exact set equality in both directions. + const enumeratedSet = new Set(enumerated.map((path) => normalizePath(path))); + for (const path of enumeratedSet) { + if (!seen.has(path)) { + violations.push({ detail: "test entry is not classified in the manifest", kind: "missing-entry", path }); + } + } + for (const path of seen) { + if (!enumeratedSet.has(path)) { + violations.push({ detail: "manifest classifies a path that is not a test entry", kind: "stale-entry", path }); + } + } + + // Declaration is not proof: a `none` entry must survive its own imports. + for (const entry of entries) { + if (entry.backend !== "none" || !enumeratedSet.has(normalizePath(entry.path))) { + continue; + } + const path = normalizePath(entry.path); + for (const reason of storageImports(readSource(path))) { + violations.push({ + detail: `declared backend "none" but ${reason}`, + kind: "storage-import-in-none", + path, + }); + } + } + + return violations.sort( + (a, b) => compareStrings(a.path, b.path) || compareStrings(a.kind, b.kind) || compareStrings(a.detail, b.detail) + ); +} + +export function formatViolations(violations: readonly Violation[]): string { + return violations.map((violation) => `${violation.path}: ${violation.kind}: ${violation.detail}`).join("\n"); +} + +/** + * CLI: check a manifest file against the current tracked tree. + * + * Returns the intended exit code rather than calling `process.exit`, so the + * tests can drive it directly. + */ +export function main(manifestPath: string, repoRoot: string): number { + const manifest = JSON.parse(readFileSync(manifestPath, "utf8")) as BackendManifest; + const enumerated = enumerateTestEntries(trackedFiles(repoRoot)); + const violations = checkBackendManifest(manifest, enumerated, (path) => readFileSync(join(repoRoot, path), "utf8")); + if (violations.length > 0) { + process.stderr.write(`${formatViolations(violations)}\n`); + return 1; + } + process.stdout.write(`test backend manifest: ${enumerated.length} entries classified\n`); + return 0; +} + +// Executed directly, this file is a command and must behave like one. Without +// this guard `node scripts/check-test-backends.ts ` exited 0 and +// printed nothing -- a checker that passes silently when it was asked to +// check, which is the one failure mode a checker must not have. A missing +// argument is also a failure, not a no-op. +if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) { + const [, , manifestPath] = process.argv; + if (!manifestPath) { + process.stderr.write("usage: check-test-backends \n"); + process.exit(2); + } + process.exit(main(manifestPath, join(import.meta.dirname, "..", ".."))); +} diff --git a/reference-implementation/scripts/postgres-template-eligibility.ts b/reference-implementation/scripts/postgres-template-eligibility.ts index 2c6943f5..b7226e2e 100644 --- a/reference-implementation/scripts/postgres-template-eligibility.ts +++ b/reference-implementation/scripts/postgres-template-eligibility.ts @@ -160,7 +160,7 @@ export const POSTGRES_TEMPLATE_ELIGIBLE_FILES: readonly string[] = [ */ export const POSTGRES_TEMPLATE_COLD_REQUIRED_FILES: readonly string[] = [ "test/absent-only-grant-expiry-postgres.test.ts", - "test/backup-table-inventory.test.ts", + "test/backup-table-inventory-postgres.test.ts", "test/browser-surface-lease-store.test.ts", "test/connector-detail-gap-store.test.ts", "test/connector-instance-store.test.ts", diff --git a/reference-implementation/scripts/test-unit-preload.mjs b/reference-implementation/scripts/test-unit-preload.mjs new file mode 100644 index 00000000..fcab72e3 --- /dev/null +++ b/reference-implementation/scripts/test-unit-preload.mjs @@ -0,0 +1,408 @@ +// Copyright The PDP-Connect Contributors +// SPDX-License-Identifier: Apache-2.0 + +// Resolution-only storage guard for test entries classified as `unit`. +// +// A test file can be labelled `unit` in scripts/test-backends.json and still +// pull in a real database: the label is a declaration, not a proof. The +// backend classifier (scripts/check-test-backends.ts) derives the same +// obligation statically from the import graph, but static analysis cannot see +// every edge -- computed specifiers, code generation and native loading all +// resolve at runtime. This preload is the runtime half of that pair: it +// watches what the process actually resolves and records a violation when a +// unit-classified test reaches a storage module or a SQL driver. +// +// Wiring mirrors scripts/hermetic/preload.ts: +// +// node --import tsx --import +// +// tsx registers first so the hook sees the specifiers the test source really +// wrote, before type stripping rewrites anything. This file is inert unless +// PDPP_TEST_UNIT_GUARD === "1", so having it on disk -- or an accidental +// --import of it -- can never deny storage to a real operator or product run. +// +// Two properties matter and are tested in test-unit-preload.test.ts: +// +// 1. Resolution only. The hook rejects at `resolve`, before any module +// evaluates. It never loads, patches or mocks a database module, so it +// cannot change what a passing test observes -- an admissible unit test +// runs byte-identically with the guard on or off. +// +// 2. Violations are recorded outside test assertions. The hook throws at the +// denied import so the offending load cannot silently proceed, but it +// also records the violation in module state and forces a non-zero exit +// code from a `process.on("exit")` handler. A test that wraps its own +// import in try/catch, or asserts that the import throws, therefore still +// fails the run. Catching the guard cannot turn a mislabelled file green. +// +// Two chokepoints, because resolution is not the only way in. Module +// resolution covers everything loaded by specifier, but a builtin can be +// fetched straight off the process object: +// +// const { DatabaseSync } = process.getBuiltinModule("node:sqlite"); +// new DatabaseSync(":memory:").prepare("select 42").get(); +// +// That executes real SQL and never resolves anything, so a resolve hook cannot +// see it at all. `process.getBuiltinModule` is therefore wrapped as well (the +// CJS `require` path for builtins needs no separate wrap -- `registerHooks` +// already intercepts CJS resolution, see installBuiltinGuard's own doc +// comment). Wrapping the one function that returns builtins is what makes a +// COMPUTED name -- `"node:" + "sqlite"` -- as covered as a literal one: the +// check runs on the runtime value, after any computation. +// +// Denial is per specifier FORM, not per file. A pattern that matches +// "pg/lib/client" but not the bare specifier "pg" reports zero violations on a +// file that genuinely imports Postgres, which is a silent false pass -- the +// worst failure mode available to a guard like this. Each denied package is +// therefore matched as: the bare specifier, any subpath under it, and (for +// builtins) the `node:` prefixed form. Relative and absolute specifiers are +// matched on the resolved path instead, so `./db.ts`, `../server/db.ts` and a +// file URL all reach the same rule. +// +// Matching the raw specifier is NOT sufficient on its own, and a guard that +// stops there has an executed hole. A caller can resolve the driver itself and +// import the resulting location, at which point no denied package name is ever +// spelled as a specifier: +// +// const url = pathToFileURL(createRequire(import.meta.url).resolve("pg")).href; +// await import(url); // specifier is "file:///.../node_modules/pg/lib/index.js" +// +// The lesson of that hole is worth stating, because enumerating spellings is a +// losing game: there is always one more way to name the same file. So the rule +// is not a list of forms but an IDENTITY. A denied target is denied by WHAT IT +// IS, whatever specifier reached it: +// +// on disk the resolved real path (symlinks resolved) of the driver's own +// installed package root, or of a denied storage module +// builtin the canonical `node:` name, normalised from the runtime value +// +// The specifier rules below are kept as a cheap first check and as the only +// thing available when resolution itself fails, but they are no longer what +// makes the guard sound. Identity is. `pgvector`, `pg-boss` and `pgtools` +// resolve to their own package roots and so are outside the denied roots -- +// a real directory boundary, not a string coincidence. +// +// What this boundary does and does not cover, stated precisely because a +// guard whose promise is vaguer than its rule invites the next surprise: +// +// COVERED any load of the installed driver package or a denied storage +// module, by any specifier -- bare, subpath, relative, absolute, +// file URL, dynamic, require, createRequire, pre-resolved path -- +// and any access to a denied builtin through import, require or +// process.getBuiltinModule, by literal or computed name. +// +// NOT a COPY of a driver's source at a different real path. That is a +// COVERED different file on disk, and identifying it as the same driver +// would need content fingerprinting, which this guard does not do. +// A test that vendors its own copy of pg is not what this guard is +// for; the classifier's source scan is what would notice that. +// Also not covered: storage reached over a socket by hand-rolled +// protocol code, or an unexecuted branch (both halves only see +// what actually runs). + +import { realpathSync } from "node:fs"; +import { createRequire, registerHooks } from "node:module"; +import { dirname, sep } from "node:path"; +import { fileURLToPath } from "node:url"; + +/** + * Storage modules denied to unit-classified tests, as RI-relative paths. + * Matched against the RESOLVED path, so every spelling of the same file -- + * relative, absolute, or file URL -- is covered by one entry. + */ +export const DENIED_STORAGE_MODULES = ["server/db.ts", "lib/db.ts", "server/postgres-storage.ts"]; + +/** + * SQL drivers denied to unit-classified tests, as package/builtin specifiers. + * Matched per specifier form: bare, subpath, and the `node:` builtin form. + */ +export const DENIED_SQL_DRIVERS = ["pg", "better-sqlite3", "node:sqlite"]; + +/** Normalize a specifier or resolved URL to a POSIX-ish path for matching. */ +function normalizePath(value) { + if (!value) { + return ""; + } + let path = value; + if (path.startsWith("file:")) { + try { + path = fileURLToPath(path); + } catch { + return ""; + } + } + return sep === "/" ? path : path.split(sep).join("/"); +} + +/** + * Does `specifier` name `pkg` in any of its denied forms? + * + * Covers the four forms d3 names, so no single spelling slips through: + * bare package "pg" -> denied + * subpath "pg/lib/client" -> denied + * node: builtin "node:sqlite" -> denied + * bare builtin "sqlite" -> denied when pkg is "node:sqlite" + * + * A prefix test alone would accept "pg" but also wrongly deny "pgvector", so + * the boundary after the package name must be a path separator or nothing. + */ +export function matchesDeniedSpecifier(specifier, pkg) { + if (typeof specifier !== "string" || specifier === "") { + return false; + } + const bare = pkg.startsWith("node:") ? pkg.slice("node:".length) : pkg; + const candidates = pkg.startsWith("node:") ? [pkg, bare] : [pkg, `node:${pkg}`]; + for (const candidate of candidates) { + if (specifier === candidate) { + return true; + } + if (specifier.startsWith(`${candidate}/`)) { + return true; + } + } + return false; +} + +/** + * Real installed root of each denied driver package, resolved once at install + * time, keyed by package name. A driver that is not installed is absent. + * + * This is the driver's IDENTITY: the directory its own package.json sits in, + * with symlinks resolved. Resolution and `realpathSync` are what establish it, + * so a pnpm store path, a workspace symlink and a hoisted install all reduce + * to the same answer without this file knowing anything about install layout. + */ +const deniedDriverRoots = new Map(); + +function driverRoots() { + if (deniedDriverRoots.size > 0) { + return deniedDriverRoots; + } + const require = createRequire(import.meta.url); + for (const pkg of DENIED_SQL_DRIVERS) { + if (pkg.startsWith("node:")) { + continue; + } + for (const target of [`${pkg}/package.json`, pkg]) { + try { + const resolved = require.resolve(target); + const root = target.endsWith("package.json") ? dirname(resolved) : resolved; + deniedDriverRoots.set(pkg, normalizePath(realpathSync(root))); + break; + } catch { + // Not installed, or `exports` hides package.json: try the entry point, + // and if that also fails leave the package out. An absent driver + // cannot be loaded, so it needs no rule. + } + } + } + return deniedDriverRoots; +} + +/** + * Does `resolved` land inside the real installed root of driver `pkg`? + * + * Matched on the resolved REAL path, not on a `node_modules//` substring. + * That substring was an installation-layout heuristic: it happened to hold for + * a hoisted npm tree and said nothing about identity, so it both missed a + * driver installed somewhere else and would have caught an unrelated file that + * merely sat under such a directory. Comparing against the root that + * resolution itself reports removes the guesswork -- and the prefix boundary + * that keeps `pgvector` and `pg-boss` out is now a real directory boundary + * rather than a string coincidence. + * + * Builtins have no directory on disk; `matchesDeniedBuiltin` covers them. + */ +export function matchesDeniedDriverPath(resolved, pkg) { + if (pkg.startsWith("node:")) { + return false; + } + const root = driverRoots().get(pkg); + if (!root) { + return false; + } + const path = normalizePath(resolved); + if (path === "") { + return false; + } + let real = path; + try { + real = normalizePath(realpathSync(path)); + } catch { + // Not a path that exists (a builtin, or a URL scheme we do not handle). + // Fall through and compare the normalized form as given. + } + return real === root || real.startsWith(`${root}/`); +} + +/** + * Canonical `node:` name for a builtin request, or undefined if `name` does + * not identify a builtin this guard denies. + * + * Normalisation is done on the runtime VALUE, which is what makes this immune + * to how the name was written. `process.getBuiltinModule("node:" + "sqlite")` + * and a literal `"node:sqlite"` arrive here as the same string, so a computed + * name cannot evade the check the way it evades a source-text scan. + */ +export function matchesDeniedBuiltin(name) { + if (typeof name !== "string" || name === "") { + return; + } + const canonical = name.startsWith("node:") ? name : `node:${name}`; + return DENIED_SQL_DRIVERS.find((pkg) => pkg === canonical); +} + +/** Does a resolved URL/path point at one of the denied storage modules? */ +export function matchesDeniedModule(resolved) { + const path = normalizePath(resolved); + if (path === "") { + return; + } + return DENIED_STORAGE_MODULES.find((module) => path.endsWith(`/${module}`) || path === module); +} + +/** + * Classify one resolution. Returns the denied rule, or undefined when the + * load is admissible. Both the raw specifier and the resolved location are + * inspected, and for drivers BOTH directions are needed: the specifier rule + * catches a driver named directly even when resolution fails, while the + * resolved-path rule catches a driver reached through a pre-resolved file URL + * or absolute path that never spells the package name. A storage module is + * likewise caught by resolved path even when it is reached through an alias or + * a relative specifier that names none of the denied strings. + */ +export function classifyResolution(specifier, resolvedUrl) { + for (const driver of DENIED_SQL_DRIVERS) { + if (matchesDeniedSpecifier(specifier, driver) || matchesDeniedDriverPath(resolvedUrl, driver)) { + return { kind: "sql-driver", rule: driver }; + } + } + const module = matchesDeniedModule(resolvedUrl) ?? matchesDeniedModule(specifier); + if (module) { + return { kind: "storage-module", rule: module }; + } +} + +const violations = []; + +/** Violations recorded so far, in resolution order. */ +export function recordedViolations() { + return [...violations]; +} + +function describe(violation) { + return `${violation.kind} "${violation.rule}" via specifier "${violation.specifier}"${ + violation.parent ? ` from ${violation.parent}` : "" + }`; +} + +/** + * Install the guard. Idempotent per process: a second call is a no-op, so an + * accidental double --import cannot double-count a violation. + */ +let installed = false; +export function installUnitStorageGuard() { + if (installed) { + return; + } + installed = true; + + registerHooks({ + resolve(specifier, context, nextResolve) { + // Resolve first so aliases, package exports and extensionless + // specifiers are matched on where they actually land, not on how they + // were spelled. A specifier that fails to resolve is left to Node's own + // error, except when its raw form already names a denied driver. + let resolution; + try { + resolution = nextResolve(specifier, context); + } catch (error) { + const bySpecifier = classifyResolution(specifier, undefined); + if (bySpecifier) { + throw deny(bySpecifier, specifier, context); + } + throw error; + } + const denied = classifyResolution(specifier, resolution?.url); + if (denied) { + throw deny(denied, specifier, context); + } + return resolution; + }, + }); + + installBuiltinGuard(); + + // Recording the violation outside the thrown error is what makes this + // guard uncatchable by the code under test: even if every denied import is + // wrapped in try/catch, or asserted to throw, this handler still fails the + // process. + process.on("exit", (code) => { + if (violations.length === 0) { + return; + } + const lines = violations.map((violation) => ` - ${describe(violation)}`).join("\n"); + process.stderr.write( + `\nunit storage guard: ${violations.length} denied load(s) in a test classified as unit:\n${lines}\n` + + "Reclassify the entry in scripts/test-backends.json, or remove the storage dependency.\n" + ); + if (code === 0) { + process.exitCode = 1; + } + }); +} + +/** + * Close the route that does not resolve anything. + * + * `process.getBuiltinModule(name)` hands back a builtin directly, without + * consulting module resolution, so the resolve hook cannot see it. Wrapping + * that one function closes the route -- and because the check runs on the + * argument's runtime value, a COMPUTED name is covered exactly as a literal + * one is. That is the point of guarding the function rather than scanning for + * spellings of the name. + * + * The CJS `require("node:sqlite")` path needs nothing here: `registerHooks` + * intercepts CJS resolution as well, so the resolve hook above already denies + * it. Verified by disabling this function and re-probing that route -- it + * still fails. A second wrap of `Module._load` would be dead code around a + * Node internal. + */ +function installBuiltinGuard() { + const originalGetBuiltinModule = process.getBuiltinModule; + if (typeof originalGetBuiltinModule === "function") { + process.getBuiltinModule = function getBuiltinModule(name) { + const denied = matchesDeniedBuiltin(name); + if (denied) { + throw deny({ kind: "sql-driver", rule: denied }, String(name), undefined); + } + return originalGetBuiltinModule.call(this, name); + }; + } +} + +/** + * Record a violation and build the error to throw. + * + * `parent` is a parentURL when the resolve hook calls this and a filename when + * the builtin guard does; both are only ever used for the diagnostic, so + * either is accepted as-is. + */ +function deny(denied, specifier, parent) { + const violation = { + kind: denied.kind, + parent: typeof parent === "string" ? parent : parent?.parentURL, + rule: denied.rule, + specifier, + }; + violations.push(violation); + const error = new Error( + `unit storage guard denied ${describe(violation)}. A test classified as unit must not reach a database.` + ); + error.code = "ERR_PDPP_UNIT_STORAGE_DENIED"; + return error; +} + +if (process.env.PDPP_TEST_UNIT_GUARD === "1") { + installUnitStorageGuard(); +} diff --git a/reference-implementation/scripts/test-unit-preload.test.ts b/reference-implementation/scripts/test-unit-preload.test.ts new file mode 100644 index 00000000..4e17060a --- /dev/null +++ b/reference-implementation/scripts/test-unit-preload.test.ts @@ -0,0 +1,538 @@ +// Copyright The PDP-Connect Contributors +// SPDX-License-Identifier: Apache-2.0 + +// Checks for the unit storage guard. The matching rules are checked in +// process; the two properties that only hold end to end -- that the guard +// denies a real load through the real tsx/module chain, and that catching the +// denial still fails the run -- are checked by spawning actual child +// processes, because an in-process assertion about an exit code proves +// nothing about the exit code. + +import { strict as assert } from "node:assert/strict"; +import { spawnSync } from "node:child_process"; +import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { createRequire } from "node:module"; +import { tmpdir } from "node:os"; +import { dirname, join } from "node:path"; +import test from "node:test"; +import { fileURLToPath, pathToFileURL } from "node:url"; + +import { + classifyResolution, + DENIED_SQL_DRIVERS, + DENIED_STORAGE_MODULES, + matchesDeniedBuiltin, + matchesDeniedDriverPath, + matchesDeniedSpecifier, +} from "./test-unit-preload.mjs"; + +const __dirname = dirname(fileURLToPath(import.meta.url)); +const PRELOAD = join(__dirname, "test-unit-preload.mjs"); +const RI_ROOT = join(__dirname, ".."); +const GUARD_MESSAGE_RE = /unit storage guard/; +const ONE_PASS_RE = /pass 1/; +const LOADED_DRIVER_RE = /LOADED DRIVER/; +const ALLOWED_OK_RE = /ALLOWED OK/; +const SQL_RESULT_RE = /SQL RESULT/; +const PAST_CATCH_RE = /PAST CATCH/; + +/** + * Run `source` as a test file under the guard, through the same + * `--import tsx --import ` chain the runner uses. Returns the child's + * status and output so a test can assert on the real exit code. + */ +function runGuarded(source: string, { guard = "1" }: { guard?: string } = {}) { + const dir = mkdtempSync(join(tmpdir(), "pdpp-unit-guard-")); + const file = join(dir, "subject.test.mjs"); + try { + writeFileSync(file, source); + // NODE_TEST_CONTEXT is set in this process because these checks + // themselves run under `node --test`. Inherited, it makes the child + // believe it is already inside a test run and skip its own files, so it + // must be dropped for the child to actually execute anything. + const { NODE_TEST_CONTEXT: _parentTestContext, ...env } = process.env; + const result = spawnSync(process.execPath, ["--import", "tsx", "--import", PRELOAD, "--test", file], { + cwd: RI_ROOT, + encoding: "utf8", + env: { ...env, PDPP_TEST_UNIT_GUARD: guard }, + timeout: 120_000, + }); + return { ...result, output: `${result.stdout ?? ""}${result.stderr ?? ""}` }; + } finally { + rmSync(dir, { force: true, recursive: true }); + } +} + +/** + * Like `runGuarded`, but the subject file is written INSIDE the RI tree. + * + * Node resolves bare specifiers from the importing file's location, so a + * subject in the OS temp directory cannot reach the repo's `node_modules` and + * `require.resolve("pg")` throws MODULE_NOT_FOUND there. The pre-resolved-path + * routes below must resolve the genuine installed driver to be worth anything, + * so they need a subject that sits where a real test file sits. + */ +function runGuardedInTree(source: string, { guard = "1" }: { guard?: string } = {}) { + const dir = mkdtempSync(join(RI_ROOT, ".pdpp-unit-guard-")); + const file = join(dir, "subject.test.mjs"); + try { + writeFileSync(file, source); + const { NODE_TEST_CONTEXT: _parentTestContext, ...env } = process.env; + const result = spawnSync(process.execPath, ["--import", "tsx", "--import", PRELOAD, "--test", file], { + cwd: RI_ROOT, + encoding: "utf8", + env: { ...env, PDPP_TEST_UNIT_GUARD: guard }, + timeout: 120_000, + }); + return { ...result, output: `${result.stdout ?? ""}${result.stderr ?? ""}` }; + } finally { + rmSync(dir, { force: true, recursive: true }); + } +} + +test("the guard is inert unless explicitly enabled", () => { + // Having the preload on disk, or importing it by accident, must never deny + // storage to a real run. Activation is opt-in, exactly as the hermetic + // network guard is. + const result = runGuarded('import test from "node:test";\nimport "node:sqlite";\ntest("loads sqlite", () => {});\n', { + guard: "0", + }); + + assert.equal(result.status, 0, result.output); +}); + +// Per specifier form. A rule that matches "pg/lib/client" but not "pg" +// reports zero violations on a file that plainly imports Postgres -- a silent +// false pass, and the worst thing this guard could do. Each form is asserted +// on its own so no single spelling can regress unnoticed. +for (const [form, specifier, pkg] of [ + ["bare package", "pg", "pg"], + ["package subpath", "pg/lib/client", "pg"], + ["deep package subpath", "pg/lib/connection-parameters", "pg"], + ["native package", "better-sqlite3", "better-sqlite3"], + ["node: builtin", "node:sqlite", "node:sqlite"], + ["bare builtin", "sqlite", "node:sqlite"], +] as const) { + test(`a denied ${form} specifier is matched`, () => { + assert.equal(matchesDeniedSpecifier(specifier, pkg), true); + assert.equal(classifyResolution(specifier, undefined)?.kind, "sql-driver"); + }); +} + +test("a package whose name merely begins with a denied name is allowed", () => { + // pgvector and pg-boss are not pg. A guard with false positives is a guard + // somebody switches off. + for (const allowed of ["pgvector", "pg-boss", "pgtools/index.js"]) { + assert.equal(matchesDeniedSpecifier(allowed, "pg"), false); + assert.equal(classifyResolution(allowed, undefined), undefined); + } +}); + +test("a storage module is matched on its resolved path, not its spelling", () => { + // The specifier "../server/db.ts" names none of the denied strings on its + // own; the resolved path is what identifies it. + for (const module of DENIED_STORAGE_MODULES) { + const resolved = `file:///repo/reference-implementation/${module}`; + assert.equal(classifyResolution("../whatever.ts", resolved)?.kind, "storage-module"); + } +}); + +test("ordinary modules resolve untouched", () => { + assert.equal(classifyResolution("node:path", "node:path"), undefined); + assert.equal(classifyResolution("./helpers/fixture.ts", "file:///repo/test/helpers/fixture.ts"), undefined); +}); + +test("every denied driver is classified as a driver, not silently ignored", () => { + for (const driver of DENIED_SQL_DRIVERS) { + assert.equal(classifyResolution(driver, undefined)?.rule, driver); + } +}); + +test("a test that loads a denied builtin fails the run", () => { + const result = runGuarded('import test from "node:test";\nimport "node:sqlite";\ntest("loads sqlite", () => {});\n'); + + assert.notEqual(result.status, 0, result.output); + assert.match(result.output, GUARD_MESSAGE_RE); +}); + +test("a test that loads a real storage module fails the run", () => { + // The genuine article: the RI's own SQLite entry point, resolved through + // tsx exactly as a real test file would reach it. + const result = runGuarded( + `import test from "node:test";\nimport "${join(RI_ROOT, "server", "db.ts").replaceAll("\\", "/")}";\ntest("loads db", () => {});\n` + ); + + assert.notEqual(result.status, 0, result.output); + assert.match(result.output, GUARD_MESSAGE_RE); +}); + +test("catching the denial does not turn the run green", () => { + // This is the property that makes the guard worth having. A mislabelled + // file that wraps its own database import in try/catch, or asserts that the + // import throws, would defeat a guard that only threw. The violation is + // recorded outside the assertion and forced onto the exit code, so the run + // still fails while every individual test reports as passing. + const result = runGuarded( + [ + 'import test from "node:test";', + 'test("swallows the guard", async () => {', + " try {", + ' await import("node:sqlite");', + " } catch {", + " // deliberately ignored", + " }", + "});", + ].join("\n") + ); + + assert.match(result.output, ONE_PASS_RE, result.output); + assert.notEqual(result.status, 0, "a swallowed violation must still fail the run"); + assert.match(result.output, GUARD_MESSAGE_RE); +}); + +// A driver reached through its already-resolved location never spells the +// package name as a specifier, so the specifier rule alone cannot see it. These +// use the REAL installed drivers rather than synthetic path strings, because +// the rule is now the driver's real resolved root: a made-up path under a +// directory named `node_modules/pg` is not the driver and must not be treated +// as proof that it is. +const requireHere = createRequire(import.meta.url); + +for (const [form, target, pkg] of [ + ["entry point", "pg", "pg"], + ["package subpath file", "pg/lib/client.js", "pg"], + ["native package entry", "better-sqlite3", "better-sqlite3"], +] as const) { + test(`the real installed driver reached by ${form} is matched on its identity`, () => { + const resolved = requireHere.resolve(target); + const asFileUrl = pathToFileURL(resolved).href; + + assert.equal(matchesDeniedDriverPath(resolved, pkg), true); + assert.equal(matchesDeniedDriverPath(asFileUrl, pkg), true); + // Neither spelling names the package, so identity is the only thing that + // can catch them. + assert.equal(matchesDeniedSpecifier(resolved, pkg), false); + assert.equal(classifyResolution(asFileUrl, asFileUrl)?.kind, "sql-driver"); + }); +} + +test("a path that merely contains a denied package name is not the driver", () => { + // This is what replaced the old `node_modules//` substring rule. That + // rule was an installation-layout guess: it would have called every path + // below a directory of that name a driver, and missed a driver installed + // anywhere else. Identity is the resolved real root, so these are not + // matched -- they do not resolve inside it. + for (const notTheDriver of [ + "/nowhere/node_modules/pg/lib/index.js", + "file:///nowhere/node_modules/better-sqlite3/lib/index.js", + "/tmp/vendor/pg/lib/client.js", + ]) { + assert.equal(matchesDeniedDriverPath(notTheDriver, "pg"), false); + assert.equal(matchesDeniedDriverPath(notTheDriver, "better-sqlite3"), false); + } +}); + +test("a genuinely installed lookalike package is outside the denied roots", () => { + // sqlite-vec is really installed, so this compares real root against real + // root rather than trusting a string boundary. + const lookalike = requireHere.resolve("sqlite-vec"); + + assert.equal(matchesDeniedDriverPath(lookalike, "pg"), false); + assert.equal(matchesDeniedDriverPath(lookalike, "better-sqlite3"), false); + assert.equal(classifyResolution(lookalike, pathToFileURL(lookalike).href), undefined); +}); + +test("a builtin has no package directory and is covered by the builtin rule", () => { + // `node:sqlite` cannot be reached through a file path, so a path rule for it + // would be dead weight. + assert.equal(matchesDeniedDriverPath("/repo/node_modules/node:sqlite/index.js", "node:sqlite"), false); + assert.equal(classifyResolution("node:sqlite", "node:sqlite")?.kind, "sql-driver"); + assert.equal(matchesDeniedBuiltin("node:sqlite"), "node:sqlite"); +}); + +// The executed half of the resolved-identity rule. These spawn real child +// processes and assert on the real exit code, because the bypass they close +// was an exit-0 run that loaded the genuine driver: an in-process assertion +// about matching would not have caught it. +test("a driver imported by pre-resolved file URL fails the run", () => { + const result = runGuardedInTree( + [ + 'import test from "node:test";', + 'import { createRequire } from "node:module";', + 'import { pathToFileURL } from "node:url";', + 'test("reaches pg through a computed file URL", async () => {', + " const req = createRequire(import.meta.url);", + ' const loaded = await import(pathToFileURL(req.resolve("pg")).href);', + ' console.log("LOADED DRIVER", typeof loaded.default.Client);', + "});", + ].join("\n") + ); + + assert.notEqual(result.status, 0, result.output); + assert.match(result.output, GUARD_MESSAGE_RE); + // The driver must not have evaluated. Resolution-only denial means the + // import never returns, so the marker can never be printed. + assert.doesNotMatch(result.output, LOADED_DRIVER_RE); +}); + +test("a driver imported by pre-resolved absolute path fails the run", () => { + const result = runGuardedInTree( + [ + 'import test from "node:test";', + 'import { createRequire } from "node:module";', + 'test("reaches better-sqlite3 through its absolute path", async () => {', + " const req = createRequire(import.meta.url);", + ' const loaded = await import(req.resolve("better-sqlite3"));', + ' console.log("LOADED DRIVER", typeof loaded.default);', + "});", + ].join("\n") + ); + + assert.notEqual(result.status, 0, result.output); + assert.match(result.output, GUARD_MESSAGE_RE); + assert.doesNotMatch(result.output, LOADED_DRIVER_RE); +}); + +test("a driver reached through createRequire fails the run", () => { + // `createRequire(import.meta.url)("better-sqlite3")` is how the RI's own + // server/db.ts reaches SQLite, so this form must never be admissible. + const result = runGuardedInTree( + [ + 'import test from "node:test";', + 'import { createRequire } from "node:module";', + 'test("reaches better-sqlite3 through createRequire", () => {', + " const req = createRequire(import.meta.url);", + ' const loaded = req("better-sqlite3");', + ' console.log("LOADED DRIVER", typeof loaded);', + "});", + ].join("\n") + ); + + assert.notEqual(result.status, 0, result.output); + assert.match(result.output, GUARD_MESSAGE_RE); + assert.doesNotMatch(result.output, LOADED_DRIVER_RE); +}); + +test("a driver required by its own resolved path fails the run", () => { + // createRequire combined with the resolved-path route: neither the call nor + // the specifier names the package. + const result = runGuardedInTree( + [ + 'import test from "node:test";', + 'import { createRequire } from "node:module";', + 'test("requires pg by resolved path", () => {', + " const req = createRequire(import.meta.url);", + ' const loaded = req(req.resolve("pg"));', + ' console.log("LOADED DRIVER", typeof loaded.Client);', + "});", + ].join("\n") + ); + + assert.notEqual(result.status, 0, result.output); + assert.match(result.output, GUARD_MESSAGE_RE); + assert.doesNotMatch(result.output, LOADED_DRIVER_RE); +}); + +test("catching a pre-resolved driver denial does not turn the run green", () => { + // The combination that section-level review is most concerned with: a + // computed file URL AND a swallowed error. Every assertion passes and the + // body completes, yet the run still fails on the recorded violation. + const result = runGuardedInTree( + [ + 'import test from "node:test";', + 'import { createRequire } from "node:module";', + 'import { pathToFileURL } from "node:url";', + 'test("swallows a computed-URL denial", async () => {', + " const req = createRequire(import.meta.url);", + " try {", + ' await import(pathToFileURL(req.resolve("pg")).href);', + " } catch {", + " // deliberately ignored", + " }", + "});", + ].join("\n") + ); + + assert.match(result.output, ONE_PASS_RE, result.output); + assert.notEqual(result.status, 0, "a swallowed computed-URL violation must still fail the run"); + assert.match(result.output, GUARD_MESSAGE_RE); +}); + +test("a real installed lookalike package still loads under the guard", () => { + // sqlite-vec is genuinely installed and resolves through + // node_modules/sqlite-vec/, so this is the false-positive control against + // real resolution rather than a synthetic path string. + const result = runGuardedInTree( + [ + 'import test from "node:test";', + 'import { createRequire } from "node:module";', + 'test("loads sqlite-vec", async () => {', + " const req = createRequire(import.meta.url);", + ' const loaded = await import(req.resolve("sqlite-vec"));', + ' console.log("ALLOWED OK", typeof loaded);', + "});", + ].join("\n") + ); + + assert.equal(result.status, 0, result.output); + assert.doesNotMatch(result.output, GUARD_MESSAGE_RE); + assert.match(result.output, ALLOWED_OK_RE); +}); + +test("an admissible unit test passes unchanged under the guard", () => { + // Resolution-only means an allowed test observes nothing different. If this + // ever fails, the guard has started changing behaviour rather than just + // watching it. + const source = [ + 'import { strict as assert } from "node:assert/strict";', + 'import { join } from "node:path";', + 'import test from "node:test";', + 'test("does arithmetic and path work", () => {', + ' assert.equal(join("a", "b"), "a/b");', + " assert.equal(2 + 2, 4);", + "});", + ].join("\n"); + + const guarded = runGuarded(source); + const unguarded = runGuarded(source, { guard: "0" }); + + assert.equal(guarded.status, 0, guarded.output); + assert.equal(unguarded.status, 0, unguarded.output); + assert.match(guarded.output, ONE_PASS_RE); + assert.doesNotMatch(guarded.output, GUARD_MESSAGE_RE); +}); + +// `process.getBuiltinModule` returns a builtin without resolving anything, so +// the resolve hook never sees it. Before the builtin guard existed, the first +// case below printed a real query result and exited 0. Each route is asserted +// separately, and the computed-name case matters most: it is the one a scan for +// literal names could never cover, and it is covered here because the check +// runs on the argument's runtime value. +test("a builtin fetched through getBuiltinModule fails the run", () => { + const result = runGuardedInTree( + [ + 'import test from "node:test";', + 'test("executes real SQL through getBuiltinModule", () => {', + ' const { DatabaseSync } = process.getBuiltinModule("node:sqlite");', + ' const db = new DatabaseSync(":memory:");', + ' console.log("SQL RESULT", db.prepare("select 42 AS answer").get().answer);', + " db.close();", + "});", + ].join("\n") + ); + + assert.notEqual(result.status, 0, result.output); + assert.match(result.output, GUARD_MESSAGE_RE); + // No query may have run: denial happens before the module is handed over. + assert.doesNotMatch(result.output, SQL_RESULT_RE); +}); + +test("a builtin fetched by computed name fails the run", () => { + const result = runGuardedInTree( + [ + 'import test from "node:test";', + 'test("computes the builtin name", () => {', + ' const name = "node:" + "sqlite";', + " const { DatabaseSync } = process.getBuiltinModule(name);", + ' const db = new DatabaseSync(":memory:");', + ' console.log("SQL RESULT", db.prepare("select 7 AS answer").get().answer);', + " db.close();", + "});", + ].join("\n") + ); + + assert.notEqual(result.status, 0, result.output); + assert.match(result.output, GUARD_MESSAGE_RE); + assert.doesNotMatch(result.output, SQL_RESULT_RE); +}); + +test("catching a getBuiltinModule denial does not turn the run green", () => { + const result = runGuardedInTree( + [ + 'import test from "node:test";', + 'test("swallows the builtin denial", () => {', + " try {", + ' process.getBuiltinModule("node:sqlite");', + " } catch {", + " // deliberately ignored", + " }", + ' console.log("PAST CATCH");', + "});", + ].join("\n") + ); + + // The body completes and its assertions pass, and the run still fails. + assert.match(result.output, PAST_CATCH_RE, result.output); + assert.match(result.output, ONE_PASS_RE); + assert.notEqual(result.status, 0, "a swallowed builtin violation must still fail the run"); +}); + +test("a builtin required through createRequire fails the run", () => { + // require() of a builtin is served from the builtin table without consulting + // the resolve hook, so this needs the CJS side of the builtin guard. + const result = runGuardedInTree( + [ + 'import test from "node:test";', + 'import { createRequire } from "node:module";', + 'test("requires the builtin", () => {', + " const req = createRequire(import.meta.url);", + ' const loaded = req("node:sqlite");', + ' console.log("LOADED DRIVER", typeof loaded.DatabaseSync);', + "});", + ].join("\n") + ); + + assert.notEqual(result.status, 0, result.output); + assert.match(result.output, GUARD_MESSAGE_RE); + assert.doesNotMatch(result.output, LOADED_DRIVER_RE); +}); + +// The false-positive side of the builtin rule. Every test file in the +// repository reaches for node:path and node:fs, so a rule that caught them +// would break the whole lane rather than guard it. +for (const [form, source] of [ + [ + "getBuiltinModule", + 'const p = process.getBuiltinModule("node:path");\nconsole.log("ALLOWED OK", p.join("a", "b"));', + ], + [ + "createRequire", + 'import { createRequire } from "node:module";\nconst p = createRequire(import.meta.url)("node:path");\nconsole.log("ALLOWED OK", p.join("a", "b"));', + ], +] as const) { + test(`an allowed builtin reached through ${form} still loads`, () => { + const result = runGuardedInTree(`import test from "node:test";\n${source}\ntest("uses path", () => {});\n`); + + assert.equal(result.status, 0, result.output); + assert.doesNotMatch(result.output, GUARD_MESSAGE_RE); + assert.match(result.output, ALLOWED_OK_RE); + }); +} + +// The name rule itself, on runtime values rather than source text. +test("a denied builtin is recognised by canonical name, however it is written", () => { + assert.equal(matchesDeniedBuiltin("node:sqlite"), "node:sqlite"); + // The bare name canonicalises to the same builtin. + assert.equal(matchesDeniedBuiltin("sqlite"), "node:sqlite"); + // A computed value is just a string by the time it arrives here. + assert.equal(matchesDeniedBuiltin(`node:${"sqlite"}`), "node:sqlite"); +}); + +test("allowed builtins and non-strings are not denied", () => { + for (const allowed of ["node:path", "path", "node:fs", "node:assert", "sqlite-vec", ""]) { + assert.equal(matchesDeniedBuiltin(allowed), undefined); + } + assert.equal(matchesDeniedBuiltin(undefined), undefined); + assert.equal(matchesDeniedBuiltin(null), undefined); +}); + +test("a driver is denied by its real installed root, not by a path substring", () => { + // The rule is identity: the driver's own resolved package root. A file that + // merely sits under some directory named like the package is not the driver. + const realDriver = createRequire(import.meta.url).resolve("pg"); + + assert.equal(classifyResolution(realDriver, realDriver)?.kind, "sql-driver"); + // A path that contains the package name as a plain substring is not matched + // on that basis -- it does not resolve inside the real root. + assert.equal(classifyResolution("/somewhere/pg/lib/index.js", "/somewhere/pg/lib/index.js"), undefined); +}); diff --git a/reference-implementation/test/backup-table-inventory-postgres.test.ts b/reference-implementation/test/backup-table-inventory-postgres.test.ts new file mode 100644 index 00000000..ae61919b --- /dev/null +++ b/reference-implementation/test/backup-table-inventory-postgres.test.ts @@ -0,0 +1,304 @@ +// Copyright The PDP-Connect Contributors +// SPDX-License-Identifier: Apache-2.0 + +// Postgres-backend half of the backup table inventory suite. Split out of +// backup-table-inventory.test.ts so each entry declares exactly one backend: +// these three cases require a real Postgres server (PDPP_TEST_POSTGRES_URL) +// and drive pg_dump/psql against it. The no-DB cases stay in +// backup-table-inventory.test.ts and the SQLite cases live in +// backup-table-inventory-sqlite.test.ts. Case names and assertions are +// unchanged by the split, including the existing URL-absent skip guards -- +// replacing those skips with failure where Postgres is required is a +// scheduling change, not part of this split. + +import { strict as assert } from "node:assert/strict"; +import { execFileSync } from "node:child_process"; +import { randomBytes } from "node:crypto"; +import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import test from "node:test"; + +import { + BACKUP_TABLE_INVENTORY, + POSTGRES_LAZY_STORAGE_TABLES, + POSTGRES_SQLITE_ONLY_STORAGE_TABLES, + POSTGRES_STORAGE_TABLES, +} from "../server/backup-table-policy.ts"; +import { + closePostgresStorage, + initPostgresStorage, + withPostgresReadOnlyTransaction, +} from "../server/postgres-storage.ts"; +import { provisionTestDatabase, TEST_DATABASE_SENTINEL_SCHEMA } from "../server/postgres-test-database-guard.ts"; +import { withTemporaryPostgresDatabase } from "./helpers/postgres-temp-database.ts"; + +const POSTGRES_VERSION_RE = /PostgreSQL\)\s+(\d+)\./; + +function sorted(values: Iterable): string[] { + return [...values].sort((a, b) => a.localeCompare(b)); +} + +function missingRequiredTables(restoredTables: Set, lazyTables: ReadonlySet): string[] { + return Object.entries(BACKUP_TABLE_INVENTORY) + .filter(([, entry]) => entry.classification === "backup_required") + .map(([table]) => table) + .filter((table) => !(restoredTables.has(table) || lazyTables.has(table))); +} + +function postgresTool(tool: "pg_dump" | "psql", args: string[]): void { + const image = process.env.PDPP_TEST_POSTGRES_CLIENT_IMAGE; + if (image) { + execFileSync("docker", ["run", "--rm", "--network", "host", image, tool, ...args], { stdio: "inherit" }); + return; + } + execFileSync(tool, args, { stdio: "inherit" }); +} + +function postgresToolOutput(tool: "pg_dump" | "psql", args: string[]): string { + const image = process.env.PDPP_TEST_POSTGRES_CLIENT_IMAGE; + if (image) { + return execFileSync("docker", ["run", "--rm", "--network", "host", image, tool, ...args], { encoding: "utf8" }); + } + return execFileSync(tool, args, { encoding: "utf8" }); +} + +function postgresToolWithInput(tool: "psql", args: string[], input: string): void { + const image = process.env.PDPP_TEST_POSTGRES_CLIENT_IMAGE; + if (image) { + execFileSync("docker", ["run", "--rm", "--interactive", "--network", "host", image, tool, ...args], { + input, + stdio: ["pipe", "inherit", "inherit"], + }); + return; + } + execFileSync(tool, args, { input, stdio: ["pipe", "inherit", "inherit"] }); +} + +function postgresClientMajor(tool: "pg_dump" | "psql"): number { + const output = postgresToolOutput(tool, ["--version"]); + const match = POSTGRES_VERSION_RE.exec(output); + assert(match, `could not parse ${tool} version from ${output}`); + return Number(match[1]); +} + +function postgresServerMajor(url: string): number { + const version = postgresToolOutput("psql", [url, "-At", "-c", "SHOW server_version_num;"]).trim(); + return Math.floor(Number(version) / 10_000); +} + +function assertPostgresDumpClientCompatible(url: string): void { + const serverMajor = postgresServerMajor(url); + const dumpMajor = postgresClientMajor("pg_dump"); + const psqlMajor = postgresClientMajor("psql"); + + assert.equal( + dumpMajor, + serverMajor, + `pg_dump major ${dumpMajor} must match PostgreSQL server major ${serverMajor}; set PDPP_TEST_POSTGRES_CLIENT_IMAGE=postgres:${serverMajor}-alpine or equivalent` + ); + assert.equal( + psqlMajor, + serverMajor, + `psql major ${psqlMajor} must match PostgreSQL server major ${serverMajor}; set PDPP_TEST_POSTGRES_CLIENT_IMAGE=postgres:${serverMajor}-alpine or equivalent` + ); +} + +test("backup inventory matches a bootstrapped Postgres catalog when configured", async (t) => { + const url = process.env.PDPP_TEST_POSTGRES_URL; + if (!url) { + t.skip("PDPP_TEST_POSTGRES_URL is not set"); + return; + } + await initPostgresStorage({ backend: "postgres", databaseUrl: url }); + try { + const actualTables = await withPostgresReadOnlyTransaction(async (client) => { + const result = await client.query<{ table_name: string }>( + `SELECT table_name + FROM information_schema.tables + WHERE table_schema = current_schema() + AND table_type = 'BASE TABLE' + ORDER BY table_name` + ); + return result.rows.map((row) => row.table_name); + }); + const actualAndLazyTables = new Set([...actualTables, ...POSTGRES_LAZY_STORAGE_TABLES]); + assert.deepEqual(sorted(actualAndLazyTables), sorted(POSTGRES_STORAGE_TABLES)); + } finally { + await closePostgresStorage(); + } +}); + +test("Postgres dump/restore preserves every required durable table when configured", async (t) => { + const sourceUrl = process.env.PDPP_TEST_POSTGRES_URL; + const restoreUrl = process.env.PDPP_TEST_POSTGRES_RESTORE_URL; + if (!(sourceUrl && restoreUrl)) { + t.skip("PDPP_TEST_POSTGRES_URL and PDPP_TEST_POSTGRES_RESTORE_URL are not both set"); + return; + } + + const dir = mkdtempSync(join(tmpdir(), "pdpp-postgres-backup-oracle-")); + const dumpPath = join(dir, "backup.sql"); + try { + assertPostgresDumpClientCompatible(sourceUrl); + assertPostgresDumpClientCompatible(restoreUrl); + + postgresTool("psql", [ + sourceUrl, + "-v", + "ON_ERROR_STOP=1", + "-c", + "DROP SCHEMA IF EXISTS public CASCADE; CREATE SCHEMA public;", + ]); + await initPostgresStorage({ backend: "postgres", databaseUrl: sourceUrl }); + await closePostgresStorage(); + + // Exclude the test-guard schema from the dump: pg_dump with no --schema + // filter captures every schema in the source database, including + // pdpp_test_guard (stamped by provisionTestDatabase so this run's own + // sentinel survives a `DROP SCHEMA public CASCADE`). The dump's bare + // `CREATE SCHEMA pdpp_test_guard;` (no IF NOT EXISTS) then collides with + // the restore target's own sentinel, which must already exist there for + // the restore database to be admissible in the first place. The guard + // schema is test-harness bookkeeping, not durable product data, so + // dropping it from the dump changes nothing this test verifies -- the + // restore target's sentinel is independently proven by its own + // provisioning, never by anything this dump carries. + const dumpSql = postgresToolOutput("pg_dump", [ + "--no-owner", + "--no-privileges", + `--exclude-schema=${TEST_DATABASE_SENTINEL_SCHEMA}`, + sourceUrl, + ]); + writeFileSync(dumpPath, dumpSql); + postgresTool("psql", [ + restoreUrl, + "-v", + "ON_ERROR_STOP=1", + "-c", + "DROP SCHEMA IF EXISTS public CASCADE; CREATE SCHEMA public;", + ]); + postgresToolWithInput("psql", [restoreUrl, "-v", "ON_ERROR_STOP=1"], dumpSql); + + await initPostgresStorage({ backend: "postgres", databaseUrl: restoreUrl }); + try { + const restoredTables = await withPostgresReadOnlyTransaction(async (client) => { + const result = await client.query<{ table_name: string }>( + `SELECT table_name + FROM information_schema.tables + WHERE table_schema = current_schema() + AND table_type = 'BASE TABLE' + ORDER BY table_name` + ); + return new Set(result.rows.map((row) => row.table_name)); + }); + const missingTables = missingRequiredTables( + restoredTables, + new Set([...POSTGRES_LAZY_STORAGE_TABLES, ...POSTGRES_SQLITE_ONLY_STORAGE_TABLES]) + ); + + assert.deepEqual(sorted(missingTables), [], "Postgres dump/restore must contain every non-lazy required table"); + } finally { + await closePostgresStorage(); + } + } finally { + rmSync(dir, { force: true, recursive: true }); + } +}); + +test("Postgres dump/restore succeeds against a restore target that already carries its own test-guard sentinel", async (t) => { + const sourceUrl = process.env.PDPP_TEST_POSTGRES_URL; + if (!sourceUrl) { + t.skip("PDPP_TEST_POSTGRES_URL is not set"); + return; + } + + // Regression oracle for the incident this fix addresses: a real gate run + // reuses a persistent PDPP_TEST_POSTGRES_RESTORE_URL across invocations, so + // the restore target already carries its own pdpp_test_guard sentinel + // (stamped by an earlier run or by operator setup -- it must, or + // initPostgresStorage would refuse it as unprovisioned). This test builds + // that exact precondition -- a disposable database pre-stamped with the + // sentinel via provisionTestDatabase, standing in for the persistent + // restore target -- and proves the dump/restore no longer collides on + // `CREATE SCHEMA pdpp_test_guard`, without ever dropping or bypassing the + // restore target's own sentinel (assertTestDatabase below re-verifies it + // survived, independent of anything the dump carried). + const dir = mkdtempSync(join(tmpdir(), "pdpp-postgres-backup-guard-collision-")); + const dumpPath = join(dir, "backup.sql"); + try { + await withTemporaryPostgresDatabase( + { + connectionString: sourceUrl, + databaseName: `pdpp_backup_guard_collision_src_${randomBytes(6).toString("hex")}`, + }, + async (freshSourceUrl) => { + await withTemporaryPostgresDatabase( + { + connectionString: sourceUrl, + databaseName: `pdpp_backup_guard_collision_dst_${randomBytes(6).toString("hex")}`, + }, + async (preStampedRestoreUrl) => { + // withTemporaryPostgresDatabase already provisions freshSourceUrl + // with the sentinel; provision the restore target too so it + // independently carries its own sentinel (already true here, but + // explicit provisioning models "a persistent restore DB that was + // stamped in a prior run" rather than "this run's own callback + // provisioning", matching the real gate's actual precondition). + await provisionTestDatabase(preStampedRestoreUrl); + + await initPostgresStorage({ backend: "postgres", databaseUrl: freshSourceUrl }); + await closePostgresStorage(); + + const dumpSql = postgresToolOutput("pg_dump", [ + "--no-owner", + "--no-privileges", + `--exclude-schema=${TEST_DATABASE_SENTINEL_SCHEMA}`, + freshSourceUrl, + ]); + writeFileSync(dumpPath, dumpSql); + + postgresTool("psql", [ + preStampedRestoreUrl, + "-v", + "ON_ERROR_STOP=1", + "-c", + "DROP SCHEMA IF EXISTS public CASCADE; CREATE SCHEMA public;", + ]); + // This is the exact statement that failed before the fix: replaying + // a dump onto a restore target whose pdpp_test_guard schema already + // exists. Success here is the regression proof. + postgresToolWithInput("psql", [preStampedRestoreUrl, "-v", "ON_ERROR_STOP=1"], dumpSql); + + await initPostgresStorage({ backend: "postgres", databaseUrl: preStampedRestoreUrl }); + try { + const restoredTables = await withPostgresReadOnlyTransaction(async (client) => { + const result = await client.query<{ table_name: string }>( + `SELECT table_name + FROM information_schema.tables + WHERE table_schema = current_schema() + AND table_type = 'BASE TABLE' + ORDER BY table_name` + ); + return new Set(result.rows.map((row) => row.table_name)); + }); + const missingTables = missingRequiredTables( + restoredTables, + new Set([...POSTGRES_LAZY_STORAGE_TABLES, ...POSTGRES_SQLITE_ONLY_STORAGE_TABLES]) + ); + assert.deepEqual( + sorted(missingTables), + [], + "restore onto a pre-guarded target must still contain every non-lazy required table" + ); + } finally { + await closePostgresStorage(); + } + } + ); + } + ); + } finally { + rmSync(dir, { force: true, recursive: true }); + } +}); diff --git a/reference-implementation/test/backup-table-inventory-sqlite.test.ts b/reference-implementation/test/backup-table-inventory-sqlite.test.ts new file mode 100644 index 00000000..d9f05ac6 --- /dev/null +++ b/reference-implementation/test/backup-table-inventory-sqlite.test.ts @@ -0,0 +1,198 @@ +// Copyright The PDP-Connect Contributors +// SPDX-License-Identifier: Apache-2.0 + +// SQLite-backend half of the backup table inventory suite. Split out of +// backup-table-inventory.test.ts so each entry declares exactly one backend: +// these three cases bootstrap a real SQLite database (initDb/getDb) or read a +// SQLite backup artifact with the sqlite3 CLI. The no-DB cases stay in +// backup-table-inventory.test.ts and the Postgres cases live in +// backup-table-inventory-postgres.test.ts. Case names and assertions are +// unchanged by the split. + +import { strict as assert } from "node:assert/strict"; +import { execFileSync } from "node:child_process"; +import { mkdtempSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import test from "node:test"; + +import { + BACKUP_TABLE_INVENTORY, + isInternalBackupCatalogTable, + POSTGRES_STORAGE_TABLES, + SQLITE_LAZY_STORAGE_TABLES, + SQLITE_POSTGRES_ONLY_STORAGE_TABLES, +} from "../server/backup-table-policy.ts"; +import { closeDb, getDb, initDb } from "../server/db.ts"; + +function sorted(values: Iterable): string[] { + return [...values].sort((a, b) => a.localeCompare(b)); +} + +function bootstrappedSqliteTables(): string[] { + const dir = mkdtempSync(join(tmpdir(), "pdpp-backup-inventory-")); + try { + initDb(join(dir, "pdpp.sqlite")); + const rows = getDb() + .prepare( + `SELECT name + FROM sqlite_schema + WHERE type IN ('table', 'virtual table') + AND name NOT LIKE 'sqlite_%' + ORDER BY name` + ) + .all<{ name: string }>(); + return rows.map((row) => row.name).filter((name) => !isInternalBackupCatalogTable(name)); + } finally { + closeDb(); + rmSync(dir, { force: true, recursive: true }); + } +} + +function sqliteQueryRows(path: string, sql: string): string[] { + return execFileSync("sqlite3", ["-batch", "-noheader", path, sql], { encoding: "utf8" }) + .split("\n") + .map((row) => row.trim()) + .filter(Boolean); +} + +function sqliteCatalogTables(path: string): string[] { + return sqliteQueryRows( + path, + `SELECT name + FROM sqlite_schema + WHERE type IN ('table', 'virtual table') + AND name NOT LIKE 'sqlite_%' + ORDER BY name` + ).filter((name) => !isInternalBackupCatalogTable(name)); +} + +function missingRequiredTables(restoredTables: Set, lazyTables: ReadonlySet): string[] { + return Object.entries(BACKUP_TABLE_INVENTORY) + .filter(([, entry]) => entry.classification === "backup_required") + .map(([table]) => table) + .filter((table) => !(restoredTables.has(table) || lazyTables.has(table))); +} + +test("backup inventory classifies every bootstrapped SQLite catalog table", () => { + const liveTables = new Set(bootstrappedSqliteTables()); + const classifiedTables = new Set(Object.keys(BACKUP_TABLE_INVENTORY)); + + assert.deepEqual( + sorted([...liveTables].filter((table) => !classifiedTables.has(table))), + [], + "every live table must be classified as backup_required, derived_rebuildable, or ephemeral_crash_reconciled" + ); +}); + +test("backup inventory has deterministic SQLite/Postgres table parity", () => { + const sqliteTables = new Set([...bootstrappedSqliteTables(), ...SQLITE_LAZY_STORAGE_TABLES]); + const postgresTables = new Set(POSTGRES_STORAGE_TABLES); + + assert.deepEqual( + sorted([...sqliteTables].filter((table) => !postgresTables.has(table))), + ["semantic_search_rowid"], + "SQLite-only semantic rowid state must be the only static storage parity exception" + ); + assert.deepEqual( + sorted([...postgresTables].filter((table) => !sqliteTables.has(table))), + sorted(SQLITE_POSTGRES_ONLY_STORAGE_TABLES), + "Postgres storage table seam must not contain tables absent from bootstrapped SQLite beyond the declared Postgres-only exceptions" + ); +}); + +test("SQLite stopped backup preserves every required durable table", () => { + const dir = mkdtempSync(join(tmpdir(), "pdpp-sqlite-backup-oracle-")); + const sourcePath = join(dir, "source.sqlite"); + const backupPath = join(dir, "backup.sqlite"); + try { + initDb(sourcePath); + const source = getDb(); + source.prepare("INSERT INTO connectors(connector_id, manifest) VALUES (?, ?)").run("connector_backup", "{}"); + source + .prepare( + `INSERT INTO connector_instances( + connector_instance_id, owner_subject_id, connector_id, display_name, + source_kind, source_binding_key, created_at, updated_at + ) VALUES (?, ?, ?, ?, ?, ?, ?, ?)` + ) + .run( + "cin_backup", + "owner_backup", + "connector_backup", + "Backup", + "account", + "account_backup", + "2026-08-12T00:00:00.000Z", + "2026-08-12T00:00:00.000Z" + ); + source + .prepare( + `INSERT INTO source_webhook_run_receipts( + source_id, event_id, body_hash, connector_id, connector_instance_id, + owner_subject_id, action, run_id, trace_id, started_at + ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?)` + ) + .run( + "source_backup", + "evt_backup", + "sha256:body", + "connector_backup", + "cin_backup", + "owner_backup", + "schedule_run", + "run_backup", + "trace_backup", + "2026-08-12T00:00:00.000Z" + ); + source + .prepare( + `INSERT INTO record_rejection_quota(owner_subject_id, pending_payload_bytes, pending_receipt_count) + VALUES (?, ?, ?)` + ) + .run("owner_backup", 7, 1); + source + .prepare( + `INSERT INTO record_rejections( + receipt_id, owner_subject_id, connector_instance_id, stream, + connector_id, run_id, first_input_index, latest_input_index, reason_code, + payload, payload_sha256, payload_bytes, replay_key, rejection_generation, + created_at, last_seen_at + ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)` + ) + .run( + "rr_backup", + "owner_backup", + "cin_backup", + "messages", + "connector_backup", + "run_backup", + 0, + 0, + "validation_error", + Buffer.from("payload"), + "sha256:fixture", + 7, + "record-rejection-v2:fixture", + "record-rejection-v2", + "2026-08-12T00:00:00.000Z", + "2026-08-12T00:00:00.000Z" + ); + source.prepare("VACUUM INTO ?").run(backupPath); + closeDb(); + + const restoredTables = new Set(sqliteCatalogTables(backupPath)); + const missingTables = missingRequiredTables( + restoredTables, + new Set([...SQLITE_LAZY_STORAGE_TABLES, ...SQLITE_POSTGRES_ONLY_STORAGE_TABLES]) + ); + + assert.deepEqual(sorted(missingTables), [], "SQLite backup artifact must contain every non-lazy required table"); + assert.equal(sqliteQueryRows(backupPath, "SELECT COUNT(*) FROM source_webhook_run_receipts")[0], "1"); + assert.equal(sqliteQueryRows(backupPath, "SELECT COUNT(*) FROM record_rejections")[0], "1"); + assert.equal(sqliteQueryRows(backupPath, "SELECT pending_payload_bytes FROM record_rejection_quota")[0], "7"); + } finally { + closeDb(); + rmSync(dir, { force: true, recursive: true }); + } +}); diff --git a/reference-implementation/test/backup-table-inventory.test.ts b/reference-implementation/test/backup-table-inventory.test.ts index 4ef63ff4..bc08ca68 100644 --- a/reference-implementation/test/backup-table-inventory.test.ts +++ b/reference-implementation/test/backup-table-inventory.test.ts @@ -1,33 +1,23 @@ // Copyright The PDP-Connect Contributors // SPDX-License-Identifier: Apache-2.0 +// Backend-independent half of the backup table inventory suite. These five +// cases read source files, docs and the static backup/migration policy +// exports; none of them opens a database, so this entry has no backend and +// runs once. The SQLite cases live in backup-table-inventory-sqlite.test.ts +// and the Postgres cases in backup-table-inventory-postgres.test.ts. Case +// names and assertions are unchanged by the split; the database imports and +// fixtures the moved cases used were removed with them, which is what lets +// this file run without any database. + import { strict as assert } from "node:assert/strict"; -import { execFileSync } from "node:child_process"; -import { randomBytes } from "node:crypto"; -import { mkdtempSync, readdirSync, readFileSync, rmSync, writeFileSync } from "node:fs"; -import { tmpdir } from "node:os"; +import { readdirSync, readFileSync } from "node:fs"; import { dirname, join } from "node:path"; import test from "node:test"; import { fileURLToPath } from "node:url"; import { DERIVED_TABLES, SKIP_TABLES, TABLES } from "../scripts/migrate-storage/schema.ts"; -import { - BACKUP_TABLE_INVENTORY, - isInternalBackupCatalogTable, - POSTGRES_LAZY_STORAGE_TABLES, - POSTGRES_SQLITE_ONLY_STORAGE_TABLES, - POSTGRES_STORAGE_TABLES, - SQLITE_LAZY_STORAGE_TABLES, - SQLITE_POSTGRES_ONLY_STORAGE_TABLES, -} from "../server/backup-table-policy.ts"; -import { closeDb, getDb, initDb } from "../server/db.ts"; -import { - closePostgresStorage, - initPostgresStorage, - withPostgresReadOnlyTransaction, -} from "../server/postgres-storage.ts"; -import { provisionTestDatabase, TEST_DATABASE_SENTINEL_SCHEMA } from "../server/postgres-test-database-guard.ts"; -import { withTemporaryPostgresDatabase } from "./helpers/postgres-temp-database.ts"; +import { BACKUP_TABLE_INVENTORY, isInternalBackupCatalogTable } from "../server/backup-table-policy.ts"; const __dirname = dirname(fileURLToPath(import.meta.url)); const repoRoot = join(__dirname, "..", ".."); @@ -35,7 +25,6 @@ const SERVER_SOURCE_FILE_RE = /\.(?:js|sql|ts)$/; const CREATE_TABLE_NAME_RE = /CREATE\s+(?:VIRTUAL\s+)?TABLE\s+(?:IF\s+NOT\s+EXISTS\s+)?([a-z][a-z0-9_]*)\s*[(]/gi; const BACKUP_POLICY_PATH_RE = /server\/backup-table-policy\.ts/; const LOGICAL_MIGRATION_SUBSET_RE = /logical migration subset/i; -const POSTGRES_VERSION_RE = /PostgreSQL\)\s+(\d+)\./; function sorted(values: Iterable): string[] { return [...values].sort((a, b) => a.localeCompare(b)); @@ -51,135 +40,6 @@ function walkFiles(dir: string): string[] { }); } -function bootstrappedSqliteTables(): string[] { - const dir = mkdtempSync(join(tmpdir(), "pdpp-backup-inventory-")); - try { - initDb(join(dir, "pdpp.sqlite")); - const rows = getDb() - .prepare( - `SELECT name - FROM sqlite_schema - WHERE type IN ('table', 'virtual table') - AND name NOT LIKE 'sqlite_%' - ORDER BY name` - ) - .all<{ name: string }>(); - return rows.map((row) => row.name).filter((name) => !isInternalBackupCatalogTable(name)); - } finally { - closeDb(); - rmSync(dir, { force: true, recursive: true }); - } -} - -function sqliteQueryRows(path: string, sql: string): string[] { - return execFileSync("sqlite3", ["-batch", "-noheader", path, sql], { encoding: "utf8" }) - .split("\n") - .map((row) => row.trim()) - .filter(Boolean); -} - -function sqliteCatalogTables(path: string): string[] { - return sqliteQueryRows( - path, - `SELECT name - FROM sqlite_schema - WHERE type IN ('table', 'virtual table') - AND name NOT LIKE 'sqlite_%' - ORDER BY name` - ).filter((name) => !isInternalBackupCatalogTable(name)); -} - -function missingRequiredTables(restoredTables: Set, lazyTables: ReadonlySet): string[] { - return Object.entries(BACKUP_TABLE_INVENTORY) - .filter(([, entry]) => entry.classification === "backup_required") - .map(([table]) => table) - .filter((table) => !(restoredTables.has(table) || lazyTables.has(table))); -} - -function postgresTool(tool: "pg_dump" | "psql", args: string[]): void { - const image = process.env.PDPP_TEST_POSTGRES_CLIENT_IMAGE; - if (image) { - execFileSync("docker", ["run", "--rm", "--network", "host", image, tool, ...args], { stdio: "inherit" }); - return; - } - execFileSync(tool, args, { stdio: "inherit" }); -} - -function postgresToolOutput(tool: "pg_dump" | "psql", args: string[]): string { - const image = process.env.PDPP_TEST_POSTGRES_CLIENT_IMAGE; - if (image) { - return execFileSync("docker", ["run", "--rm", "--network", "host", image, tool, ...args], { encoding: "utf8" }); - } - return execFileSync(tool, args, { encoding: "utf8" }); -} - -function postgresToolWithInput(tool: "psql", args: string[], input: string): void { - const image = process.env.PDPP_TEST_POSTGRES_CLIENT_IMAGE; - if (image) { - execFileSync("docker", ["run", "--rm", "--interactive", "--network", "host", image, tool, ...args], { - input, - stdio: ["pipe", "inherit", "inherit"], - }); - return; - } - execFileSync(tool, args, { input, stdio: ["pipe", "inherit", "inherit"] }); -} - -function postgresClientMajor(tool: "pg_dump" | "psql"): number { - const output = postgresToolOutput(tool, ["--version"]); - const match = POSTGRES_VERSION_RE.exec(output); - assert(match, `could not parse ${tool} version from ${output}`); - return Number(match[1]); -} - -function postgresServerMajor(url: string): number { - const version = postgresToolOutput("psql", [url, "-At", "-c", "SHOW server_version_num;"]).trim(); - return Math.floor(Number(version) / 10_000); -} - -function assertPostgresDumpClientCompatible(url: string): void { - const serverMajor = postgresServerMajor(url); - const dumpMajor = postgresClientMajor("pg_dump"); - const psqlMajor = postgresClientMajor("psql"); - - assert.equal( - dumpMajor, - serverMajor, - `pg_dump major ${dumpMajor} must match PostgreSQL server major ${serverMajor}; set PDPP_TEST_POSTGRES_CLIENT_IMAGE=postgres:${serverMajor}-alpine or equivalent` - ); - assert.equal( - psqlMajor, - serverMajor, - `psql major ${psqlMajor} must match PostgreSQL server major ${serverMajor}; set PDPP_TEST_POSTGRES_CLIENT_IMAGE=postgres:${serverMajor}-alpine or equivalent` - ); -} -test("backup inventory classifies every bootstrapped SQLite catalog table", () => { - const liveTables = new Set(bootstrappedSqliteTables()); - const classifiedTables = new Set(Object.keys(BACKUP_TABLE_INVENTORY)); - - assert.deepEqual( - sorted([...liveTables].filter((table) => !classifiedTables.has(table))), - [], - "every live table must be classified as backup_required, derived_rebuildable, or ephemeral_crash_reconciled" - ); -}); - -test("backup inventory has deterministic SQLite/Postgres table parity", () => { - const sqliteTables = new Set([...bootstrappedSqliteTables(), ...SQLITE_LAZY_STORAGE_TABLES]); - const postgresTables = new Set(POSTGRES_STORAGE_TABLES); - - assert.deepEqual( - sorted([...sqliteTables].filter((table) => !postgresTables.has(table))), - ["semantic_search_rowid"], - "SQLite-only semantic rowid state must be the only static storage parity exception" - ); - assert.deepEqual( - sorted([...postgresTables].filter((table) => !sqliteTables.has(table))), - sorted(SQLITE_POSTGRES_ONLY_STORAGE_TABLES), - "Postgres storage table seam must not contain tables absent from bootstrapped SQLite beyond the declared Postgres-only exceptions" - ); -}); - test("backup inventory accounts for store-created table DDL outside bootstrap", () => { const classifiedTables = new Set(Object.keys(BACKUP_TABLE_INVENTORY)); const createdTables = new Set(); @@ -211,101 +71,6 @@ test("non-required backup classifications require executable proof", () => { ); }); -test("SQLite stopped backup preserves every required durable table", () => { - const dir = mkdtempSync(join(tmpdir(), "pdpp-sqlite-backup-oracle-")); - const sourcePath = join(dir, "source.sqlite"); - const backupPath = join(dir, "backup.sqlite"); - try { - initDb(sourcePath); - const source = getDb(); - source.prepare("INSERT INTO connectors(connector_id, manifest) VALUES (?, ?)").run("connector_backup", "{}"); - source - .prepare( - `INSERT INTO connector_instances( - connector_instance_id, owner_subject_id, connector_id, display_name, - source_kind, source_binding_key, created_at, updated_at - ) VALUES (?, ?, ?, ?, ?, ?, ?, ?)` - ) - .run( - "cin_backup", - "owner_backup", - "connector_backup", - "Backup", - "account", - "account_backup", - "2026-08-12T00:00:00.000Z", - "2026-08-12T00:00:00.000Z" - ); - source - .prepare( - `INSERT INTO source_webhook_run_receipts( - source_id, event_id, body_hash, connector_id, connector_instance_id, - owner_subject_id, action, run_id, trace_id, started_at - ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?)` - ) - .run( - "source_backup", - "evt_backup", - "sha256:body", - "connector_backup", - "cin_backup", - "owner_backup", - "schedule_run", - "run_backup", - "trace_backup", - "2026-08-12T00:00:00.000Z" - ); - source - .prepare( - `INSERT INTO record_rejection_quota(owner_subject_id, pending_payload_bytes, pending_receipt_count) - VALUES (?, ?, ?)` - ) - .run("owner_backup", 7, 1); - source - .prepare( - `INSERT INTO record_rejections( - receipt_id, owner_subject_id, connector_instance_id, stream, - connector_id, run_id, first_input_index, latest_input_index, reason_code, - payload, payload_sha256, payload_bytes, replay_key, rejection_generation, - created_at, last_seen_at - ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)` - ) - .run( - "rr_backup", - "owner_backup", - "cin_backup", - "messages", - "connector_backup", - "run_backup", - 0, - 0, - "validation_error", - Buffer.from("payload"), - "sha256:fixture", - 7, - "record-rejection-v2:fixture", - "record-rejection-v2", - "2026-08-12T00:00:00.000Z", - "2026-08-12T00:00:00.000Z" - ); - source.prepare("VACUUM INTO ?").run(backupPath); - closeDb(); - - const restoredTables = new Set(sqliteCatalogTables(backupPath)); - const missingTables = missingRequiredTables( - restoredTables, - new Set([...SQLITE_LAZY_STORAGE_TABLES, ...SQLITE_POSTGRES_ONLY_STORAGE_TABLES]) - ); - - assert.deepEqual(sorted(missingTables), [], "SQLite backup artifact must contain every non-lazy required table"); - assert.equal(sqliteQueryRows(backupPath, "SELECT COUNT(*) FROM source_webhook_run_receipts")[0], "1"); - assert.equal(sqliteQueryRows(backupPath, "SELECT COUNT(*) FROM record_rejections")[0], "1"); - assert.equal(sqliteQueryRows(backupPath, "SELECT pending_payload_bytes FROM record_rejection_quota")[0], "7"); - } finally { - closeDb(); - rmSync(dir, { force: true, recursive: true }); - } -}); test("migration schema exports load and preserve the logical migration subset", () => { const tableNames = TABLES.map((table) => table.name); @@ -319,205 +84,6 @@ test("migration schema exports load and preserve the logical migration subset", ); }); -test("backup inventory matches a bootstrapped Postgres catalog when configured", async (t) => { - const url = process.env.PDPP_TEST_POSTGRES_URL; - if (!url) { - t.skip("PDPP_TEST_POSTGRES_URL is not set"); - return; - } - await initPostgresStorage({ backend: "postgres", databaseUrl: url }); - try { - const actualTables = await withPostgresReadOnlyTransaction(async (client) => { - const result = await client.query<{ table_name: string }>( - `SELECT table_name - FROM information_schema.tables - WHERE table_schema = current_schema() - AND table_type = 'BASE TABLE' - ORDER BY table_name` - ); - return result.rows.map((row) => row.table_name); - }); - const actualAndLazyTables = new Set([...actualTables, ...POSTGRES_LAZY_STORAGE_TABLES]); - assert.deepEqual(sorted(actualAndLazyTables), sorted(POSTGRES_STORAGE_TABLES)); - } finally { - await closePostgresStorage(); - } -}); - -test("Postgres dump/restore preserves every required durable table when configured", async (t) => { - const sourceUrl = process.env.PDPP_TEST_POSTGRES_URL; - const restoreUrl = process.env.PDPP_TEST_POSTGRES_RESTORE_URL; - if (!(sourceUrl && restoreUrl)) { - t.skip("PDPP_TEST_POSTGRES_URL and PDPP_TEST_POSTGRES_RESTORE_URL are not both set"); - return; - } - - const dir = mkdtempSync(join(tmpdir(), "pdpp-postgres-backup-oracle-")); - const dumpPath = join(dir, "backup.sql"); - try { - assertPostgresDumpClientCompatible(sourceUrl); - assertPostgresDumpClientCompatible(restoreUrl); - - postgresTool("psql", [ - sourceUrl, - "-v", - "ON_ERROR_STOP=1", - "-c", - "DROP SCHEMA IF EXISTS public CASCADE; CREATE SCHEMA public;", - ]); - await initPostgresStorage({ backend: "postgres", databaseUrl: sourceUrl }); - await closePostgresStorage(); - - // Exclude the test-guard schema from the dump: pg_dump with no --schema - // filter captures every schema in the source database, including - // pdpp_test_guard (stamped by provisionTestDatabase so this run's own - // sentinel survives a `DROP SCHEMA public CASCADE`). The dump's bare - // `CREATE SCHEMA pdpp_test_guard;` (no IF NOT EXISTS) then collides with - // the restore target's own sentinel, which must already exist there for - // the restore database to be admissible in the first place. The guard - // schema is test-harness bookkeeping, not durable product data, so - // dropping it from the dump changes nothing this test verifies -- the - // restore target's sentinel is independently proven by its own - // provisioning, never by anything this dump carries. - const dumpSql = postgresToolOutput("pg_dump", [ - "--no-owner", - "--no-privileges", - `--exclude-schema=${TEST_DATABASE_SENTINEL_SCHEMA}`, - sourceUrl, - ]); - writeFileSync(dumpPath, dumpSql); - postgresTool("psql", [ - restoreUrl, - "-v", - "ON_ERROR_STOP=1", - "-c", - "DROP SCHEMA IF EXISTS public CASCADE; CREATE SCHEMA public;", - ]); - postgresToolWithInput("psql", [restoreUrl, "-v", "ON_ERROR_STOP=1"], dumpSql); - - await initPostgresStorage({ backend: "postgres", databaseUrl: restoreUrl }); - try { - const restoredTables = await withPostgresReadOnlyTransaction(async (client) => { - const result = await client.query<{ table_name: string }>( - `SELECT table_name - FROM information_schema.tables - WHERE table_schema = current_schema() - AND table_type = 'BASE TABLE' - ORDER BY table_name` - ); - return new Set(result.rows.map((row) => row.table_name)); - }); - const missingTables = missingRequiredTables( - restoredTables, - new Set([...POSTGRES_LAZY_STORAGE_TABLES, ...POSTGRES_SQLITE_ONLY_STORAGE_TABLES]) - ); - - assert.deepEqual(sorted(missingTables), [], "Postgres dump/restore must contain every non-lazy required table"); - } finally { - await closePostgresStorage(); - } - } finally { - rmSync(dir, { force: true, recursive: true }); - } -}); - -test("Postgres dump/restore succeeds against a restore target that already carries its own test-guard sentinel", async (t) => { - const sourceUrl = process.env.PDPP_TEST_POSTGRES_URL; - if (!sourceUrl) { - t.skip("PDPP_TEST_POSTGRES_URL is not set"); - return; - } - - // Regression oracle for the incident this fix addresses: a real gate run - // reuses a persistent PDPP_TEST_POSTGRES_RESTORE_URL across invocations, so - // the restore target already carries its own pdpp_test_guard sentinel - // (stamped by an earlier run or by operator setup -- it must, or - // initPostgresStorage would refuse it as unprovisioned). This test builds - // that exact precondition -- a disposable database pre-stamped with the - // sentinel via provisionTestDatabase, standing in for the persistent - // restore target -- and proves the dump/restore no longer collides on - // `CREATE SCHEMA pdpp_test_guard`, without ever dropping or bypassing the - // restore target's own sentinel (assertTestDatabase below re-verifies it - // survived, independent of anything the dump carried). - const dir = mkdtempSync(join(tmpdir(), "pdpp-postgres-backup-guard-collision-")); - const dumpPath = join(dir, "backup.sql"); - try { - await withTemporaryPostgresDatabase( - { - connectionString: sourceUrl, - databaseName: `pdpp_backup_guard_collision_src_${randomBytes(6).toString("hex")}`, - }, - async (freshSourceUrl) => { - await withTemporaryPostgresDatabase( - { - connectionString: sourceUrl, - databaseName: `pdpp_backup_guard_collision_dst_${randomBytes(6).toString("hex")}`, - }, - async (preStampedRestoreUrl) => { - // withTemporaryPostgresDatabase already provisions freshSourceUrl - // with the sentinel; provision the restore target too so it - // independently carries its own sentinel (already true here, but - // explicit provisioning models "a persistent restore DB that was - // stamped in a prior run" rather than "this run's own callback - // provisioning", matching the real gate's actual precondition). - await provisionTestDatabase(preStampedRestoreUrl); - - await initPostgresStorage({ backend: "postgres", databaseUrl: freshSourceUrl }); - await closePostgresStorage(); - - const dumpSql = postgresToolOutput("pg_dump", [ - "--no-owner", - "--no-privileges", - `--exclude-schema=${TEST_DATABASE_SENTINEL_SCHEMA}`, - freshSourceUrl, - ]); - writeFileSync(dumpPath, dumpSql); - - postgresTool("psql", [ - preStampedRestoreUrl, - "-v", - "ON_ERROR_STOP=1", - "-c", - "DROP SCHEMA IF EXISTS public CASCADE; CREATE SCHEMA public;", - ]); - // This is the exact statement that failed before the fix: replaying - // a dump onto a restore target whose pdpp_test_guard schema already - // exists. Success here is the regression proof. - postgresToolWithInput("psql", [preStampedRestoreUrl, "-v", "ON_ERROR_STOP=1"], dumpSql); - - await initPostgresStorage({ backend: "postgres", databaseUrl: preStampedRestoreUrl }); - try { - const restoredTables = await withPostgresReadOnlyTransaction(async (client) => { - const result = await client.query<{ table_name: string }>( - `SELECT table_name - FROM information_schema.tables - WHERE table_schema = current_schema() - AND table_type = 'BASE TABLE' - ORDER BY table_name` - ); - return new Set(result.rows.map((row) => row.table_name)); - }); - const missingTables = missingRequiredTables( - restoredTables, - new Set([...POSTGRES_LAZY_STORAGE_TABLES, ...POSTGRES_SQLITE_ONLY_STORAGE_TABLES]) - ); - assert.deepEqual( - sorted(missingTables), - [], - "restore onto a pre-guarded target must still contain every non-lazy required table" - ); - } finally { - await closePostgresStorage(); - } - } - ); - } - ); - } finally { - rmSync(dir, { force: true, recursive: true }); - } -}); - test("storage migration inventory does not imply complete backup coverage", () => { const migratedTables = new Set(TABLES.filter((table) => !table.skipMigration).map((table) => table.name)); const backupRequiredTables = Object.entries(BACKUP_TABLE_INVENTORY) diff --git a/reference-implementation/test/helpers/ri-zero-connector-knowledge-data-load-scan.ts b/reference-implementation/test/helpers/ri-zero-connector-knowledge-data-load-scan.ts index 6a22245c..53f9bae9 100644 --- a/reference-implementation/test/helpers/ri-zero-connector-knowledge-data-load-scan.ts +++ b/reference-implementation/test/helpers/ri-zero-connector-knowledge-data-load-scan.ts @@ -313,6 +313,17 @@ const SANCTIONED_GENERIC_DATA_READ_CALL_SITES: ReadonlySet = new Set([ // the scanner (no rule here folds an `execFileSync` call result as a path // anchor, matching this scanner's stated bounded-resolver scope). "reference-implementation/scripts/test-accounting/with-local-full-suite-lock.mjs:44", + // main(manifestPath, repoRoot) in check-test-backends.ts: readFileSync(manifestPath, + // "utf8") where `manifestPath` is the operator's own CLI positional + // argument (process.argv[2]), same class as deploy-canary.ts:606 and + // cli/lib/common.ts:35 above. The file it names is a TEST BACKEND manifest + // (one `Backend` value -- "none"/"sqlite"/"postgres"/"sqlite+postgres" -- + // per tracked test-file path); checkBackendManifest validates its shape + // and rejects any entry that disagrees with the file's own imports, so it + // can carry no connector/provider identity the harness would act on. + // Line-pinned by design (see this array's own doc comment above); it must + // be re-derived if an edit above the call site moves it. + "reference-implementation/scripts/check-test-backends.ts:285", ]); /** Directory segments, relative to a production scan root (e.g. `server/`),