garrytan / garrytan/gstack

buildGbrainEnv leaves PGLite users exposed to .env poisoning → broken-db false-positive

Open
#1,917 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
TypeScript
Stars
133k
Forks
19.9k
Avg merge
18h 46m
Merged PRs (30d)
26

Description

# `buildGbrainEnv` leaves PGLite users exposed to `.env` poisoning → `broken-db` false-positive

## Summary

PR #1583 (CHANGELOG line 1359, 1410) fixed `lib/gbrain-local-status.ts:freshClassify` so the probe's env is routed through `buildGbrainEnv`. The fix is **complete for Postgres/Supabase brains** but **incomplete for PGLite brains**, because `buildGbrainEnv` works by *seeding* `DATABASE_URL` from `~/.gbrain/config.json` — and a PGLite config has no `database_url` field.

For PGLite users, line 111 of `lib/gbrain-exec.ts` fires `if (!cfg.database_url) return out;` and returns the caller env **unmodified**. Bun's auto-load of the cwd's `.env` then flows straight through to the `gbrain sources list --json` probe, gbrain treats the leaked `DATABASE_URL` as a Postgres override, connection fails, classifier reads `"Cannot connect to database"` from stderr, and returns `broken-db`. The 60s cache then propagates that poisoned negative to clean directories.

Net effect: every PGLite user with a `DATABASE_URL` in any project's `.env` (sqlite, mysql, app DB on a different port, etc.) sees `broken-db` from gstack — and brain-aware blocks get silently suppressed in their planning-skill `SKILL.md` files.

## Reproduction

Setup: gbrain 0.42.x with `engine: "pglite"` (e.g. `~/.gbrain/config.json` lacks `database_url`).

```bash
# Working: $HOME, no .env in scope, no DATABASE_URL leaked
rm -f ~/.gstack/.gbrain-local-status-cache.json
( cd "$HOME" && env -u DATABASE_URL -u GBRAIN_DATABASE_URL \
~/.claude/skills/gstack/bin/gstack-gbrain-detect ) | jq .gbrain_local_status
# → "ok"

# Broken: any project repo with DATABASE_URL in .env
cd /path/to/some-project # contains .env with DATABASE_URL=sqlite:///app.db
rm -f ~/.gstack/.gbrain-local-status-cache.json
~/.claude/skills/gstack/bin/gstack-gbrain-detect | jq .gbrain_local_status
# → "broken-db"

# Direct gbrain works fine — proving the brain isn't actually broken:
gbrain sources list --json # exit 0, valid JSON, empty stderr
```

The `broken-db` classification then gets cached for 60s and propagates to subsequent probes even from clean directories.

## Why PR #1583 didn't cover this

`buildGbrainEnv` in `lib/gbrain-exec.ts:95-127` is built around *seeding* `DATABASE_URL`:

```ts
export function buildGbrainEnv(opts: BuildGbrainEnvOptions = {}): NodeJS.ProcessEnv {
const baseEnv = opts.baseEnv || process.env;
const out: NodeJS.ProcessEnv = { ...baseEnv };
if (baseEnv.GSTACK_RESPECT_ENV_DATABASE_URL === "1") return out;
...
if (!cfg.database_url) return out; // ← PGLite escape hatch
...
out.DATABASE_URL = cfg.database_url;
}
```

For PGLite (file-engine, no URL), `cfg.database_url` is undefined, line 111 returns `out` (which is `{ ...baseEnv }`), and the `.env`-poisoned `DATABASE_URL` is preserved unchanged. The probe sees it, gbrain dutifully tries to connect to Postgres, fails, and the classifier locks in `broken-db`.

## Suggested fix

When the resolved config engine is PGLite, *strip* `DATABASE_URL` / `GBRAIN_DATABASE_URL` from the returned env instead of passing through. The config schema already carries enough signal — either `engine === "pglite"` or `database_path` present with no `database_url`. Sketch:

```ts
// lib/gbrain-exec.ts
if (!cfg.database_url) {
// PGLite engine has no DATABASE_URL to seed. Strip any caller-supplied
// value (e.g. Bun-autoloaded project .env) so the file-backed brain
// isn't probed as Postgres → "Cannot connect to database" → broken-db.
if (cfg.engine === "pglite" || cfg.database_path) {
delete out.DATABASE_URL;
delete out.GBRAIN_DATABASE_URL;
}
return out;
}
```

`GSTACK_RESPECT_ENV_DATABASE_URL=1` keeps its escape hatch (handled before this branch).

Same one-liner pattern should probably be mirrored in `lib/gbrain-local-status.ts:223-265 freshClassify` if `buildGbrainEnv` shouldn't take on the engine-aware behavior directly.

## Test coverage to add

A `freshClassify` test case with:
- `~/.gbrain/config.json` set to `{"engine":"pglite","database_path":"..."}` (no `database_url`)
- caller env supplies `DATABASE_URL=sqlite:///x.db` (simulating the Bun `.env` autoload)
- stub `gbrain` exits 0 with valid JSON when called with no `DATABASE_URL` env
- assert `localEngineStatus()` returns `"ok"`, not `"broken-db"`

This would have caught the regression and would prevent it from re-emerging.

## Environment

- `gstack`: 1.57.6.0 (current `main` tip, commit `9cc41b7`)
- `gbrain`: 0.42.29.0
- Engine: PGLite (`~/.gbrain/config.json` has `engine: "pglite"`, `database_path: ...`, no `database_url`)
- Host: macOS / zsh
- Triggering `.env` (in an unrelated project repo, cwd at probe time): `DATABASE_URL=sqlite:///node0.db`

## References

- PR #1583 — original `freshClassify` fix (Postgres/Supabase complete, PGLite incomplete)
- CHANGELOG.md lines 1359, 1410 — describes the symptom this issue extends
- `lib/gbrain-exec.ts:95-127` — `buildGbrainEnv` definition
- `lib/gbrain-local-status.ts:223-265` — `freshClassify` (the probe call site)

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.