mirror of
https://github.com/garrytan/gbrain.git
synced 2026-08-14 00:48:18 +00:00
fix(test): enforce the DATABASE_URL guard process-wide (#3485)
The guard was never wrong — it was unreachable. `assertSafeE2eDatabaseUrl`
could only be hit via `setupDB()` in `test/e2e/helpers.ts`, but 17 test files
build a `PostgresEngine` straight from `process.env.DATABASE_URL` and never
call it, so for those files the name check simply never ran. Several then
issue genuinely destructive SQL against whatever answered:
test/e2e/postgres-bootstrap.test.ts TRUNCATE ... CASCADE,
DROP TABLE sources CASCADE,
ALTER TABLE pages DROP COLUMN
test/phantom-redirect-engine-parity.test.ts TRUNCATE facts, DELETE FROM pages
test/e2e/multimodal-postgres.test.ts unscoped DELETE from 3 tables
test/e2e/embedding-column-postgres.test.ts unscoped DELETE FROM content_chunks
plus every file that calls initSchema() and so applies migrations to a brain
that never asked for them.
Two things make this sharp rather than theoretical. `gbrain init` writes
DATABASE_URL into `~/.gbrain/.env`, so a developer's real brain is a plausible
value. And `test/phantom-redirect-engine-parity.test.ts` lives in `test/`, not
`test/e2e/` — `scripts/test-shard.sh` excludes `test/e2e/*` and
`*.serial.test.ts` from unit collection but not top-level `test/*`, so a plain
`bun test` picks it up with none of the E2E ritual.
Fix: enforce it as a process-wide precondition via `bunfig.toml`'s preload
rather than a per-file call. The per-file call is exactly the ritual that was
already being skipped, and a preload covers all 17 files, needs no edit to any
of them, and automatically covers the next such file someone writes. This is
the same mechanism #2823 used to stop tests leaking into the operator's real
`~/.gbrain/audit/`.
The guard moves to `test/helpers/db-guard.ts` so files that never import the
E2E helpers can reach it; `test/e2e/helpers.ts` re-exports it for existing
callers. Net -13 lines in the touched files.
Verified: with a production-shaped DATABASE_URL the process aborts (exit 1)
before any test file loads; with CI's `gbrain_test` it passes; a no-DB run is
unaffected. `bun run verify` 32/32 and `bun run typecheck` clean on both sides.
`test/helpers/db-guard-preload.test.ts` pins the wiring, not the logic —
nothing imports a preload, so it is exactly the kind of thing a refactor drops
silently.
This commit is contained in:
Garry Tan
+12
-1
@@ -18,4 +18,15 @@ timeout = 60_000
|
||||
# runs, so audit-emitting code paths (content-sanity, shell-audit, etc.)
|
||||
# can't leak fixture events into the operator's real ~/.gbrain/audit/. See
|
||||
# test/helpers/audit-dir-preload.ts for the full rationale.
|
||||
preload = ["./test/helpers/legacy-embedding-preload.ts", "./test/helpers/audit-dir-preload.ts"]
|
||||
#
|
||||
# #3485: vet DATABASE_URL before any test file loads. The guard used to be
|
||||
# reachable only from setupDB(), so the 17 files that build a PostgresEngine
|
||||
# straight from process.env.DATABASE_URL never ran it — several of which
|
||||
# TRUNCATE/DROP/ALTER whatever answers. A process-wide precondition covers
|
||||
# them all, and covers the next such file automatically. See
|
||||
# test/helpers/db-guard-preload.ts.
|
||||
preload = [
|
||||
"./test/helpers/legacy-embedding-preload.ts",
|
||||
"./test/helpers/audit-dir-preload.ts",
|
||||
"./test/helpers/db-guard-preload.ts",
|
||||
]
|
||||
|
||||
+7
-31
@@ -67,38 +67,14 @@ export function hasDatabase(): boolean {
|
||||
}
|
||||
|
||||
/**
|
||||
* Production guard: setupDB() TRUNCATEs every data table on whatever
|
||||
* DATABASE_URL points at, and run-e2e.sh deliberately preserves an exported
|
||||
* DATABASE_URL — so a developer with a production URL in their environment
|
||||
* would wipe their real brain by running the suite. Refuse unless the
|
||||
* database name identifies itself as a test database ("test" as a word
|
||||
* segment, e.g. gbrain_test — the CI/.env.testing.example convention), or
|
||||
* the operator explicitly opts the exact name in via GBRAIN_E2E_ALLOW_DB.
|
||||
*
|
||||
* Exported for unit testing; pure — no connection is made.
|
||||
* The DATABASE_URL production guard moved to `test/helpers/db-guard.ts` in
|
||||
* #3485 so that files which never import these helpers can reach it — and so
|
||||
* `bunfig.toml`'s preload can enforce it process-wide instead of relying on
|
||||
* each file to remember. Re-exported here for the existing callers and for
|
||||
* `test/e2e/db-guard.test.ts`.
|
||||
*/
|
||||
export function assertSafeE2eDatabaseUrl(
|
||||
url: string,
|
||||
env: Record<string, string | undefined> = process.env,
|
||||
): void {
|
||||
let dbName: string;
|
||||
try {
|
||||
dbName = decodeURIComponent(new URL(url).pathname.replace(/^\//, ''));
|
||||
} catch {
|
||||
throw new Error(`E2E guard: DATABASE_URL is not a parseable URL; refusing to run destructive setup.`);
|
||||
}
|
||||
if (!dbName) {
|
||||
throw new Error(`E2E guard: DATABASE_URL has no database name; refusing to run destructive setup.`);
|
||||
}
|
||||
if (/(^|[_-])test([_-]|$)/i.test(dbName)) return;
|
||||
if (env.GBRAIN_E2E_ALLOW_DB && env.GBRAIN_E2E_ALLOW_DB === dbName) return;
|
||||
throw new Error(
|
||||
`E2E guard: database "${dbName}" does not look like a test database ` +
|
||||
`(expected "test" as a name segment, e.g. gbrain_test). setupDB() would ` +
|
||||
`TRUNCATE every data table in it. If this is intentional, set ` +
|
||||
`GBRAIN_E2E_ALLOW_DB=${dbName} to opt in explicitly.`,
|
||||
);
|
||||
}
|
||||
import { assertSafeE2eDatabaseUrl } from '../helpers/db-guard.ts';
|
||||
export { assertSafeE2eDatabaseUrl };
|
||||
|
||||
/**
|
||||
* Connect to DB, run schema init, truncate all tables.
|
||||
|
||||
@@ -0,0 +1,57 @@
|
||||
/**
|
||||
* Pins the #3485 fix in place.
|
||||
*
|
||||
* The vulnerability was never that the guard logic was wrong — it was that
|
||||
* the guard was only *reachable* from `setupDB()`, so the 17 test files that
|
||||
* build a PostgresEngine straight from `process.env.DATABASE_URL` never ran
|
||||
* it. The fix is a `bunfig.toml` preload, and a preload is exactly the kind
|
||||
* of thing a future refactor drops without noticing, because nothing imports
|
||||
* it. Hence this test: it asserts the wiring, not the logic.
|
||||
*
|
||||
* `test/e2e/db-guard.test.ts` covers the guard's behavior.
|
||||
*/
|
||||
import { describe, test, expect } from 'bun:test';
|
||||
import { readFileSync } from 'node:fs';
|
||||
import { resolve } from 'node:path';
|
||||
import { assertSafeE2eDatabaseUrl } from './db-guard.ts';
|
||||
|
||||
const REPO_ROOT = resolve(import.meta.dir, '../..');
|
||||
|
||||
describe('#3485: DATABASE_URL guard is enforced process-wide', () => {
|
||||
test('bunfig.toml preloads the db guard', () => {
|
||||
const bunfig = readFileSync(resolve(REPO_ROOT, 'bunfig.toml'), 'utf-8');
|
||||
// Tolerant of formatting (single-line or multi-line array), strict about presence.
|
||||
expect(bunfig).toContain('./test/helpers/db-guard-preload.ts');
|
||||
});
|
||||
|
||||
test('the preload actually calls the guard', () => {
|
||||
const preload = readFileSync(resolve(REPO_ROOT, 'test/helpers/db-guard-preload.ts'), 'utf-8');
|
||||
expect(preload).toContain('assertSafeE2eDatabaseUrl');
|
||||
// Must be conditional on DATABASE_URL — an unconditional call would break
|
||||
// every no-DB run.
|
||||
expect(preload).toContain('process.env.DATABASE_URL');
|
||||
});
|
||||
|
||||
test('the guard still rejects a production-shaped database name', () => {
|
||||
// Sanity: if this ever stops throwing, the preload is a no-op and the
|
||||
// wiring assertions above are worthless.
|
||||
expect(() =>
|
||||
assertSafeE2eDatabaseUrl('postgresql://u:p@db.example.com:5432/gbrain', {}),
|
||||
).toThrow(/does not look like a test database/);
|
||||
});
|
||||
|
||||
test('and accepts the CI/.env.testing convention', () => {
|
||||
expect(() =>
|
||||
assertSafeE2eDatabaseUrl('postgresql://postgres:postgres@localhost:5433/gbrain_test', {}),
|
||||
).not.toThrow();
|
||||
});
|
||||
|
||||
test('this very process passed the guard', () => {
|
||||
// If DATABASE_URL is set at all, the preload ran before this file loaded
|
||||
// and did not throw — otherwise the process would have aborted. Assert the
|
||||
// invariant explicitly so the guarantee is visible in test output.
|
||||
if (process.env.DATABASE_URL) {
|
||||
expect(() => assertSafeE2eDatabaseUrl(process.env.DATABASE_URL!, process.env)).not.toThrow();
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,55 @@
|
||||
/**
|
||||
* Pre-test setup: vet `DATABASE_URL` ONCE, before any test file loads, and
|
||||
* refuse to run the suite at all when it points at something that isn't
|
||||
* name-shaped like a test database.
|
||||
*
|
||||
* Why this exists (#3485): `assertSafeE2eDatabaseUrl` was reachable only
|
||||
* from `setupDB()` in `test/e2e/helpers.ts`. 17 test files build a
|
||||
* `PostgresEngine` directly from `process.env.DATABASE_URL` and never call
|
||||
* it, so for those files the check simply never executed. Several then run
|
||||
* genuinely destructive SQL against whatever answered:
|
||||
*
|
||||
* test/e2e/postgres-bootstrap.test.ts TRUNCATE ... CASCADE,
|
||||
* DROP TABLE sources CASCADE,
|
||||
* ALTER TABLE pages DROP COLUMN
|
||||
* test/phantom-redirect-engine-parity.test.ts TRUNCATE facts, DELETE FROM pages
|
||||
* test/e2e/multimodal-postgres.test.ts unscoped DELETE from 3 tables
|
||||
* test/e2e/embedding-column-postgres.test.ts unscoped DELETE FROM content_chunks
|
||||
* ... plus the eval-capture / eval-contradictions / facts-fence /
|
||||
* mcp-budget files, and every file that calls initSchema() and so
|
||||
* applies migrations to a brain that never asked for them.
|
||||
*
|
||||
* Two things made this sharper than a latent hazard. `gbrain init` writes
|
||||
* DATABASE_URL into `~/.gbrain/.env`, so a developer's real brain is a
|
||||
* plausible value. And `test/phantom-redirect-engine-parity.test.ts` sits in
|
||||
* `test/`, not `test/e2e/` — `scripts/test-shard.sh`'s collection excludes
|
||||
* `test/e2e/*` and `*.serial.test.ts` but not top-level `test/*`, so a plain
|
||||
* `bun test` picks it up with none of the E2E ritual.
|
||||
*
|
||||
* Fix shape: a process-wide precondition instead of a per-file call. This
|
||||
* covers all 17 files, needs no edit to any of them, and — the reason it is
|
||||
* the right seam — automatically covers the next file someone writes. A
|
||||
* per-file `assertSafeE2eDatabaseUrl()` line is exactly the ritual that was
|
||||
* already being skipped. Same mechanism #2823 used for `GBRAIN_AUDIT_DIR`.
|
||||
*
|
||||
* No DATABASE_URL means no risk and no-op: the DB-backed suites all skip
|
||||
* themselves in that case.
|
||||
*
|
||||
* Imported by `bunfig.toml` via
|
||||
* `preload = [..., "./test/helpers/db-guard-preload.ts"]`.
|
||||
*/
|
||||
import { assertSafeE2eDatabaseUrl } from './db-guard.ts';
|
||||
|
||||
if (process.env.DATABASE_URL) {
|
||||
// Throwing from a preload aborts the whole `bun test` process, which is the
|
||||
// intent — a wrong DATABASE_URL is not something to warn about and proceed
|
||||
// past, because the first beforeAll that runs may already have destroyed data.
|
||||
assertSafeE2eDatabaseUrl(
|
||||
process.env.DATABASE_URL,
|
||||
process.env,
|
||||
'the DB-backed test files TRUNCATE, DELETE, DROP TABLE and apply migrations against it',
|
||||
);
|
||||
if (process.env.GBRAIN_DEBUG_PRELOAD === '1') {
|
||||
console.error('[db-guard-preload] DATABASE_URL accepted');
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,54 @@
|
||||
/**
|
||||
* The DATABASE_URL production guard — single source of truth.
|
||||
*
|
||||
* Lives here rather than in `test/e2e/helpers.ts` (#3485) because the guard
|
||||
* is needed by files that never import those helpers. `assertSafeE2eDatabaseUrl`
|
||||
* used to be reachable only from `setupDB()`, so the 17 test files that build
|
||||
* a `PostgresEngine` straight from `process.env.DATABASE_URL` never ran it —
|
||||
* and several of them issue `TRUNCATE ... CASCADE`, unscoped `DELETE FROM
|
||||
* pages`, `DROP TABLE`, and `ALTER TABLE ... DROP COLUMN`. `gbrain init`
|
||||
* writes DATABASE_URL into `~/.gbrain/.env`, so a developer with a real brain
|
||||
* in their environment was one `bun test` away from losing it.
|
||||
*
|
||||
* Enforcement is `test/helpers/db-guard-preload.ts`, wired into
|
||||
* `bunfig.toml`'s `preload`, which calls this once before any test file
|
||||
* loads. That is deliberately the same mechanism #2823 used to stop tests
|
||||
* leaking into the operator's real `~/.gbrain/audit/`: a process-wide
|
||||
* precondition beats a per-file ritual nobody remembers to perform.
|
||||
*
|
||||
* Pure — makes no connection — so it is cheap to call redundantly and safe
|
||||
* to unit test.
|
||||
*/
|
||||
|
||||
/**
|
||||
* Refuse to proceed unless the database name identifies itself as a test
|
||||
* database ("test" as a word segment, e.g. `gbrain_test` — the
|
||||
* CI/.env.testing.example convention), or the operator explicitly opts the
|
||||
* exact name in via `GBRAIN_E2E_ALLOW_DB`.
|
||||
*
|
||||
* @param url the DATABASE_URL to vet
|
||||
* @param env env source, injectable for tests
|
||||
* @param what what would happen if we proceeded, for the error message
|
||||
*/
|
||||
export function assertSafeE2eDatabaseUrl(
|
||||
url: string,
|
||||
env: Record<string, string | undefined> = process.env,
|
||||
what = 'setupDB() would TRUNCATE every data table in it',
|
||||
): void {
|
||||
let dbName: string;
|
||||
try {
|
||||
dbName = decodeURIComponent(new URL(url).pathname.replace(/^\//, ''));
|
||||
} catch {
|
||||
throw new Error(`E2E guard: DATABASE_URL is not a parseable URL; refusing to run destructive setup.`);
|
||||
}
|
||||
if (!dbName) {
|
||||
throw new Error(`E2E guard: DATABASE_URL has no database name; refusing to run destructive setup.`);
|
||||
}
|
||||
if (/(^|[_-])test([_-]|$)/i.test(dbName)) return;
|
||||
if (env.GBRAIN_E2E_ALLOW_DB && env.GBRAIN_E2E_ALLOW_DB === dbName) return;
|
||||
throw new Error(
|
||||
`E2E guard: database "${dbName}" does not look like a test database ` +
|
||||
`(expected "test" as a name segment, e.g. gbrain_test). ${what}. ` +
|
||||
`If this is intentional, set GBRAIN_E2E_ALLOW_DB=${dbName} to opt in explicitly.`,
|
||||
);
|
||||
}
|
||||
Reference in New Issue
Block a user