mirror of https://github.com/garrytan/gstack.git
fix(gbrain): strip leaked db env for PGLite probes
PGLite configs may have no database_url to seed, so buildGbrainEnv used to pass through caller DATABASE_URL values loaded from a project .env. That made gbrain probes try an unrelated app DB and falsely classify healthy local brains as broken-db. When config indicates local PGLite and has no database_url, drop DATABASE_URL and GBRAIN_DATABASE_URL unless GSTACK_RESPECT_ENV_DATABASE_URL=1 is set. Add regression coverage at the env helper and local-status probe layers. Closes #1917
This commit is contained in:
parent
cab774cced
commit
6a70972900
|
|
@ -38,6 +38,8 @@ import { spawnSync, spawn, execFileSync, type SpawnSyncReturns, type ChildProces
|
|||
|
||||
interface GbrainConfig {
|
||||
database_url?: string;
|
||||
engine?: string;
|
||||
database_path?: string;
|
||||
}
|
||||
|
||||
export interface BuildGbrainEnvOptions {
|
||||
|
|
@ -80,9 +82,14 @@ export function isTransactionModePooler(url: string): boolean {
|
|||
* unchanged when:
|
||||
* - `GSTACK_RESPECT_ENV_DATABASE_URL=1` (intentional opt-out),
|
||||
* - the config file is missing or unparseable,
|
||||
* - the config has no `database_url`,
|
||||
* - the config has no `database_url` and is not a local PGLite config,
|
||||
* - the caller already set DATABASE_URL to the same value.
|
||||
*
|
||||
* For PGLite configs, there is no DATABASE_URL to seed. In that case we
|
||||
* strip caller-supplied database URLs so Bun's project `.env` autoload
|
||||
* cannot make gbrain probe an unrelated app database and classify a healthy
|
||||
* file-backed brain as `broken-db`.
|
||||
*
|
||||
* When the effective DATABASE_URL targets a PgBouncer transaction-mode
|
||||
* pooler (port 6543), sets `GBRAIN_PREPARE=true` so gbrain re-enables
|
||||
* prepared statements needed for search (#1435). Caller can override
|
||||
|
|
@ -108,7 +115,13 @@ export function buildGbrainEnv(opts: BuildGbrainEnvOptions = {}): NodeJS.Process
|
|||
} catch {
|
||||
return out;
|
||||
}
|
||||
if (!cfg.database_url) return out;
|
||||
if (!cfg.database_url) {
|
||||
if (cfg.engine === "pglite" || cfg.database_path) {
|
||||
delete out.DATABASE_URL;
|
||||
delete out.GBRAIN_DATABASE_URL;
|
||||
}
|
||||
return out;
|
||||
}
|
||||
|
||||
const hadCaller = baseEnv.DATABASE_URL !== undefined;
|
||||
const alreadyMatch = baseEnv.DATABASE_URL === cfg.database_url;
|
||||
|
|
|
|||
|
|
@ -70,13 +70,28 @@ describe("buildGbrainEnv", () => {
|
|||
expect(result.DATABASE_URL).toBe("postgresql://app/db");
|
||||
});
|
||||
|
||||
it("returns caller env unchanged when config has no database_url field", () => {
|
||||
writeFileSync(join(gbrainHome, "config.json"), JSON.stringify({ engine: "pglite" }));
|
||||
it("returns caller env unchanged when config has no database_url field and no local-engine signal", () => {
|
||||
writeFileSync(join(gbrainHome, "config.json"), JSON.stringify({ profile: "legacy" }));
|
||||
const baseEnv = { HOME: home, DATABASE_URL: "postgresql://app/db" };
|
||||
const result = buildGbrainEnv({ baseEnv });
|
||||
expect(result.DATABASE_URL).toBe("postgresql://app/db");
|
||||
});
|
||||
|
||||
it("strips caller database URLs for PGLite configs without a database_url", () => {
|
||||
writeFileSync(
|
||||
join(gbrainHome, "config.json"),
|
||||
JSON.stringify({ engine: "pglite", database_path: join(gbrainHome, "gbrain.db") }),
|
||||
);
|
||||
const baseEnv = {
|
||||
HOME: home,
|
||||
DATABASE_URL: "sqlite:///app.db",
|
||||
GBRAIN_DATABASE_URL: "postgresql://wrong/db",
|
||||
};
|
||||
const result = buildGbrainEnv({ baseEnv });
|
||||
expect(result.DATABASE_URL).toBeUndefined();
|
||||
expect(result.GBRAIN_DATABASE_URL).toBeUndefined();
|
||||
});
|
||||
|
||||
it("honors GBRAIN_HOME when set (config aligned with detectEngineTier)", () => {
|
||||
// Move the config to an alternate dir; set GBRAIN_HOME to point at it.
|
||||
const altGbrainHome = join(home, "alt-gbrain");
|
||||
|
|
|
|||
|
|
@ -55,8 +55,9 @@ interface FakeEnv {
|
|||
*/
|
||||
function makeEnv(opts: {
|
||||
withGbrain?: boolean;
|
||||
gbrainBehavior?: "ok" | "broken-db" | "broken-config" | "throws";
|
||||
gbrainBehavior?: "ok" | "broken-db" | "broken-config" | "throws" | "fail-on-database-url";
|
||||
withConfig?: boolean;
|
||||
configJson?: Record<string, unknown>;
|
||||
}): FakeEnv {
|
||||
const tmp = mkdtempSync(join(tmpdir(), "gbrain-local-status-test-"));
|
||||
const bindir = join(tmp, "bin");
|
||||
|
|
@ -73,7 +74,7 @@ function makeEnv(opts: {
|
|||
if (opts.withConfig) {
|
||||
writeFileSync(
|
||||
configPath,
|
||||
JSON.stringify({ engine: "pglite", database_url: "pglite:///fake" }),
|
||||
JSON.stringify(opts.configJson ?? { engine: "pglite", database_url: "pglite:///fake" }),
|
||||
);
|
||||
}
|
||||
|
||||
|
|
@ -96,7 +97,7 @@ function makeEnv(opts: {
|
|||
}
|
||||
|
||||
function makeFakeGbrainScript(
|
||||
behavior: "ok" | "broken-db" | "broken-config" | "throws",
|
||||
behavior: "ok" | "broken-db" | "broken-config" | "throws" | "fail-on-database-url",
|
||||
): string {
|
||||
const stderrLine =
|
||||
behavior === "broken-db"
|
||||
|
|
@ -113,6 +114,14 @@ if [ "$1" = "--version" ]; then
|
|||
exit 0
|
||||
fi
|
||||
if [ "$1 $2" = "sources list" ]; then
|
||||
if [ "${behavior}" = "fail-on-database-url" ]; then
|
||||
if [ -n "$DATABASE_URL" ] || [ -n "$GBRAIN_DATABASE_URL" ]; then
|
||||
echo "Cannot connect to database: leaked DATABASE_URL" >&2
|
||||
exit 1
|
||||
fi
|
||||
echo '{"sources":[]}'
|
||||
exit 0
|
||||
fi
|
||||
if [ ${exitCode} -eq 0 ]; then
|
||||
echo '{"sources":[]}'
|
||||
exit 0
|
||||
|
|
@ -136,6 +145,8 @@ function applyEnv(env: FakeEnv): () => void {
|
|||
HOME: process.env.HOME,
|
||||
PATH: process.env.PATH,
|
||||
GSTACK_HOME: process.env.GSTACK_HOME,
|
||||
DATABASE_URL: process.env.DATABASE_URL,
|
||||
GBRAIN_DATABASE_URL: process.env.GBRAIN_DATABASE_URL,
|
||||
};
|
||||
process.env.HOME = env.home;
|
||||
process.env.PATH = `${env.bindir}:/usr/bin:/bin`;
|
||||
|
|
@ -147,6 +158,10 @@ function applyEnv(env: FakeEnv): () => void {
|
|||
else process.env.PATH = prev.PATH;
|
||||
if (prev.GSTACK_HOME === undefined) delete process.env.GSTACK_HOME;
|
||||
else process.env.GSTACK_HOME = prev.GSTACK_HOME;
|
||||
if (prev.DATABASE_URL === undefined) delete process.env.DATABASE_URL;
|
||||
else process.env.DATABASE_URL = prev.DATABASE_URL;
|
||||
if (prev.GBRAIN_DATABASE_URL === undefined) delete process.env.GBRAIN_DATABASE_URL;
|
||||
else process.env.GBRAIN_DATABASE_URL = prev.GBRAIN_DATABASE_URL;
|
||||
};
|
||||
}
|
||||
|
||||
|
|
@ -206,6 +221,20 @@ describe("lib/gbrain-local-status — five status cases", () => {
|
|||
restoreEnv = applyEnv(env);
|
||||
expect(localEngineStatus({ noCache: true })).toBe("ok");
|
||||
});
|
||||
|
||||
it("does not misclassify a PGLite brain as broken-db when caller env leaks DATABASE_URL", () => {
|
||||
env = makeEnv({
|
||||
withGbrain: true,
|
||||
gbrainBehavior: "fail-on-database-url",
|
||||
withConfig: true,
|
||||
configJson: { engine: "pglite", database_path: "local/pglite.db" },
|
||||
});
|
||||
restoreEnv = applyEnv(env);
|
||||
process.env.DATABASE_URL = "sqlite:///app.db";
|
||||
process.env.GBRAIN_DATABASE_URL = "postgresql://wrong/db";
|
||||
|
||||
expect(localEngineStatus({ noCache: true })).toBe("ok");
|
||||
});
|
||||
});
|
||||
|
||||
describe("lib/gbrain-local-status — cache behavior", () => {
|
||||
|
|
|
|||
Loading…
Reference in New Issue