From e2279650249beffc1310a9f204f833fb595915ec Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Tue, 19 May 2026 13:12:40 -0700 Subject: [PATCH] v0.36.5.0 feat: secure DATABASE_URL access for shell jobs (inherit: ["database_url"]) (#1192) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * v0.36.5.0 feat: secure DATABASE_URL access for shell jobs (inherit: ["database_url"]) Replaces PR #1137's plaintext-config / plaintext-env workarounds with code. Shell-job params gain `inherit: ["database_url"]`, validated pre-enqueue in both the CLI (`gbrain jobs submit`) and `submit_job` MCP op handler. Worker resolves the value from its own loadConfig() at child-spawn time; the persisted `minion_jobs.data` row stores only the name. Plain `env: { GBRAIN_DATABASE_URL: ... }` / `env: { DATABASE_URL: ... }` / `env: { GBRAIN_DIRECT_DATABASE_URL: ... }` are rejected pre-enqueue with a paste-ready hint pointing at `inherit:`. Codex pre-landing review caught two bypasses + one missing shadow name: - H1: cmd/argv inline-secret regex scan (cmd:"GBRAIN_DATABASE_URL=... gbrain sync" was a clean bypass — fixed) - H3: GBRAIN_DIRECT_DATABASE_URL added to shadowKeys - H2: honest docs about output-side leakage (stdout_tail/stderr_tail can still carry the value if the script prints it; that's the script author's responsibility, not gbrain's) Also: gbrain doctor learns home_dir_in_worktree (warns when ~/.gbrain lives inside a git worktree); ~/.gbrain/.gitignore retroactive via saveConfig + post-upgrade. New canonical guide: docs/guides/agent-to-gbrain.md (two-domain framing for downstream agent authors: MCP ops via OAuth vs localOnly admin ops via shell-job inherit:). Closes #1137. Tests: +53 new (21 validator + 12 inherit-record + 6 ensureGitignore + 5 doctor + 2 PGLite E2E + 7 codex-driven H1/H3 cases). Credit: @wintermute filed PR #1137 which made the env-stripping gap visible enough to fix in code. Thank you. Co-Authored-By: Claude Opus 4.7 * v0.36.5.0 redesign: free-form inherit:, drop closed enum User feedback: "agent spawning minions should have agency to do what it wants with secrets and pass only the ones that it needs. don't be a security nazi please." Replaces the closed INHERITABLE enum (database_url only) with three small helpers in shell-inherit.ts: - INHERIT_NAME_RE: snake_case shape guard. Rejects __proto__, leading underscore, uppercase, path-traversal. Prototype-pollution defense. - deriveEnvKey(name): config-key → child-env-key. Uppercase by default with one override: database_url → GBRAIN_DATABASE_URL. - resolveInheritValue(cfg, name): value lookup with Object.hasOwn. inherit: now accepts any snake_case config-key the worker has. Agent picks what it needs per-job (database_url, anthropic_api_key, voyage_api_key, or any custom field). Validator does NOT police WHICH keys — single-uid trust model treats agent as peer of worker. Drops the v0.36.5.0-RC rules that were paternalistic for the actual threat model: - closed-enum check - env-shadow rejection - cmd/argv inline-secret scan Keeps the parts that defend real problems: - pre-enqueue validation (closes the persistence-before-throw window) - snake_case regex (prototype-pollution + audit-log readability) - fail-fast on missing config value (UX guardrail, not security) Tests: shell-validate (existing rules + new free-form + prototype-pollution defense + T1 regression guard) and shell-inherit (regex matrix, deriveEnvKey per-name, resolveInheritValue with hasOwn defense). E2E case now exercises inherit:["anthropic_api_key"] to prove genuinely free-form. Docs and CHANGELOG rewritten to reflect the open design + the design-arc story (closed → cut → free-form). Migration file too. 7653 unit tests green. Co-Authored-By: Claude Opus 4.7 * v0.36.5.0 add: redact_secrets opt-in for stdout/stderr scrubbing Honest defense for the documented output-side leakage. When a script prints an inherited secret, the value lands plaintext in result.stdout_tail / result.stderr_tail / error_text. v0.36.5.0 adds: - `redact_secrets: true` ShellJobParams field - `--redact-secrets` CLI convenience flag on `gbrain jobs submit shell` - shell-redact.ts: pure `redactSecretsInText(text, secrets)` helper (string-mode replaceAll; regex metachars in values stay literal) - Handler post-processes both tails before throw/return, so the persisted row carries `` tokens instead of values Only inherit-resolved values are scrubbed. env: values are not (those are the agent's "fine in the row" channel by design). Heuristic — defeats accidental `echo "$GBRAIN_DATABASE_URL"`, not adversarial encode-then-print. Default false for back-compat. Tests: - test/minions-shell-redact.test.ts (9 cases): pure-function behavior, regex-metachar safety, multi-secret independent redaction, substring overlap, empty-input/map edge cases - test/minions-shell-validate.test.ts: +4 cases for redact_secrets shape - test/e2e/minions-shell-pglite.test.ts: +2 cases proving redact_secrets: true scrubs persisted row AND redact_secrets:false preserves plaintext (back-compat regression guard) Docs + CHANGELOG + migration file + CLAUDE.md updated. 7667 unit tests green. Co-Authored-By: Claude Opus 4.7 --------- Co-authored-by: Claude Opus 4.7 --- CHANGELOG.md | 132 +++++++++- CLAUDE.md | 7 +- VERSION | 2 +- docs/UPGRADING_DOWNSTREAM_AGENTS.md | 72 +++++ docs/guides/agent-to-gbrain.md | 199 ++++++++++++++ docs/guides/minions-shell-jobs.md | 125 ++++++++- llms-full.txt | 78 +++++- package.json | 2 +- skills/migrations/v0.36.5.0.md | 115 ++++++++ src/commands/doctor.ts | 64 +++++ src/commands/jobs.ts | 29 ++ src/commands/upgrade.ts | 10 + src/core/config.ts | 54 ++++ src/core/minions/handlers/shell-audit.ts | 3 + src/core/minions/handlers/shell-inherit.ts | 69 +++++ src/core/minions/handlers/shell-redact.ts | 51 ++++ src/core/minions/handlers/shell-validate.ts | 181 +++++++++++++ src/core/minions/handlers/shell.ts | 163 +++++++----- src/core/operations.ts | 46 +++- test/config-ensure-gitignore.test.ts | 93 +++++++ test/doctor-home-dir-in-worktree.test.ts | 156 +++++++++++ test/e2e/minions-shell-pglite.test.ts | 277 ++++++++++++++++++++ test/minions-shell-inherit.test.ts | 120 +++++++++ test/minions-shell-redact.test.ts | 112 ++++++++ test/minions-shell-validate.test.ts | 247 +++++++++++++++++ 25 files changed, 2320 insertions(+), 87 deletions(-) create mode 100644 docs/guides/agent-to-gbrain.md create mode 100644 skills/migrations/v0.36.5.0.md create mode 100644 src/core/minions/handlers/shell-inherit.ts create mode 100644 src/core/minions/handlers/shell-redact.ts create mode 100644 src/core/minions/handlers/shell-validate.ts create mode 100644 test/config-ensure-gitignore.test.ts create mode 100644 test/doctor-home-dir-in-worktree.test.ts create mode 100644 test/minions-shell-inherit.test.ts create mode 100644 test/minions-shell-redact.test.ts create mode 100644 test/minions-shell-validate.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index a92b198b6..8a522e5d0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,116 @@ All notable changes to GBrain will be documented in this file. +## [0.36.5.0] - 2026-05-18 + +**Your agent picks which secrets to pass to a shell job by name. The worker resolves the values; names land in the row, values don't.** + +If you submit a `gbrain` CLI command as a shell job, you have hit the env-stripping wall and probably worked around it the way PR #1137 documented: write secrets into `~/.gbrain/config.json` plaintext, or pass `env: { GBRAIN_DATABASE_URL: "..." }` per job. Both work. The per-job pattern in particular lands the URL in `minion_jobs.data` (a real DB row) and in the shell-audit JSONL — so if your brain ever travels (shared via mounts, dumped, backed up), the URL travels with it. + +v0.36.5.0 adds a new shell-job field: `inherit: ["database_url", "anthropic_api_key", ...]`. Pass any snake_case config-key name on it. The worker resolves the value from its `loadConfig()` at child-spawn time and injects it into the child env (`database_url` → `GBRAIN_DATABASE_URL`; `anthropic_api_key` → `ANTHROPIC_API_KEY`; convention is uppercase). Names persist to the row + audit; values resolve fresh on every spawn and never persist from `inherit:` itself. Validation runs **pre-enqueue** in both submit surfaces — `gbrain jobs submit shell` (CLI) and the `submit_job` MCP op — so a malformed payload never lands in `minion_jobs.data`. + +The validator does NOT police WHICH config keys you inherit. The agent submitting the minion is in the same uid as the worker; it's the agent's call. If you want a value in the row (non-secret correlation token, OR a secret you've decided is fine to persist), keep using `env:` — that path is unchanged. Use `inherit:` when you want the value OUT of the row. + +This wave started as a 4-layer cathedral (encrypted vault + Unix-socket broker + SO_PEERCRED + per-call tokens). The /cso audit cut it as security theater for the single-uid topology that's actually deployed — gbrain runs on your machine, hermes and openclaw run as your uid, defending against same-uid attackers is moot. Codex's outside-voice pass then caught the load-bearing correctness bug in the "minimum-scope" plan: validation in the worker handler runs AFTER `queue.add()` has already persisted the row, so the headline "input-secrets never persist" claim was technically false. Fixed by lifting validation to a shared module called pre-enqueue from both submit paths. + +### What you can now do + +**Submit shell jobs that pass any subset of secrets the agent needs.** + +```bash +gbrain jobs submit shell --params '{ + "cmd": "gbrain sync --skip-failed && gbrain embed --stale", + "cwd": "/data/gbrain", + "inherit": ["database_url", "anthropic_api_key", "voyage_api_key"] +}' +``` + +The worker resolves each name from its config and injects values under the derived env-key names. The `minion_jobs.data` row carries `inherit: ["database_url", "anthropic_api_key", "voyage_api_key"]` — names only. The shell-audit JSONL records the same. Pre-enqueue validation rejects the submission if the worker can't resolve a requested name, with a paste-ready `gbrain config set ` hint. + +**Inherit any custom config-key.** If you stuff `my_custom_token` into `~/.gbrain/config.json`, `inherit: ["my_custom_token"]` works — child env gets `MY_CUSTOM_TOKEN`. No allowlist. The validator only enforces snake_case shape (prototype-pollution defense for the `__proto__` / `constructor` family). + +**Use `env:` when you want the value in the row.** `env: { CORRELATION_TOKEN: "audit-trail-uuid-abc" }` works the same as it always has. The validator doesn't second-guess. + +**Surface `~/.gbrain` worktree risk via `gbrain doctor`.** New check `home_dir_in_worktree` walks up from `gbrainPath()` (honoring `GBRAIN_HOME`) looking for a `.git` directory OR file — both shapes — and warns if found. Handles linked worktrees correctly (the Conductor + git-worktrees topology this branch was developed in literally hits this path). Walk terminates at `$HOME` so a `.git` above your home doesn't false-positive. + +**Get `~/.gbrain/.gitignore` retroactively.** Every `saveConfig()` call AND every `gbrain post-upgrade` now lays down a single-line `*` gitignore in `~/.gbrain/`. Idempotent — never clobbers user customization. Honest scope: this blocks casual `git add ~/.gbrain` from inside an enclosing worktree, but does NOT cover already-tracked files, screenshots, backup tools (Time Machine / iCloud / Dropbox), or `git add -f`. The doctor check surfaces what the `.gitignore` can't. + +**Read `docs/guides/agent-to-gbrain.md`** for the canonical two-domain framing for downstream agent authors: MCP ops via thin-client OAuth for everything with an MCP equivalent, shell job + `inherit:` for the `localOnly` admin ops (`sync`, `embed`, `dream`, `doctor`, `autopilot`). Not a fallback hierarchy — pick by op. + +### Itemized changes + +- **`src/core/minions/handlers/shell-inherit.ts`** (NEW) — three small helpers. `INHERIT_NAME_RE` (snake_case regex), `deriveEnvKey(name)` (config-key → child-env-key with `database_url` → `GBRAIN_DATABASE_URL` override), `resolveInheritValue(cfg, name)` (uses `Object.hasOwn` to defeat prototype-pollution lookups). +- **`src/core/minions/handlers/shell-validate.ts`** (NEW) — `validateShellJobParams(data, opts?)` shared validator called pre-enqueue from both submit surfaces. Existing cmd/argv/cwd/env shape checks + new `inherit` shape check (array of snake_case strings) + fail-fast on missing config value. The test seam `opts.config` lets unit tests drive the validator hermetically. +- **`src/commands/jobs.ts`** — `gbrain jobs submit shell` calls `validateShellJobParams(data)` before `queue.add()`. Audit-log call extends to record `inherit: [...]` names. +- **`src/core/operations.ts`** — `submit_job` op for `name === 'shell'` runs the same pre-enqueue validator AND lifts the shell-audit JSONL write so the MCP-op path also produces audit lines (codex F-CDX-4). +- **`src/core/minions/handlers/shell.ts`** — `ShellJobParams.inherit?: string[]` + `ShellJobParams.redact_secrets?: boolean`. `buildChildEnv` resolves names via `resolveInheritValue` and injects under `deriveEnvKey(name)`. When `redact_secrets` is true, the handler builds a redaction map (inherit-name → value) and post-processes `stdout_tail` / `stderr_tail` via `redactSecretsInText` before throw/return. Handler-entry re-validation as defense-in-depth catches pre-existing rows. +- **`src/core/minions/handlers/shell-redact.ts`** (NEW) — pure `redactSecretsInText(text, secrets)` helper. String-mode `replaceAll` so regex metacharacters in values are treated as literal (safety property). Empty values in the map are skipped. +- **`src/commands/jobs.ts`** — CLI flag `--redact-secrets` merges `redact_secrets: true` into params before validation. Convenience for `gbrain jobs submit shell --redact-secrets`. +- **`src/core/minions/handlers/shell-audit.ts`** — `ShellAuditEvent.inherit?: string[]` (names only). +- **`src/core/config.ts`** — `ensureGitignore()` helper. Idempotent; never clobbers user content. Called from `saveConfig()`. +- **`src/commands/upgrade.ts:runPostUpgrade`** — calls `ensureGitignore()` once per upgrade. +- **`src/commands/doctor.ts`** — new `home_dir_in_worktree` filesystem check. +- **`docs/guides/minions-shell-jobs.md`** — new `#secrets` section. Documents free-form `inherit:`, output-side leakage caveat, error catalog. +- **`docs/guides/agent-to-gbrain.md`** (NEW) — single-page guide for downstream agent authors. Two-domain framing (MCP ops via thin-client OAuth vs `localOnly` admin ops via shell-job `inherit:`), decision table, migration recipe. +- **`docs/UPGRADING_DOWNSTREAM_AGENTS.md`** — v0.36.5.0 section with before/after recipe and error-handling table. +- **New tests** (50+ cases across 5 files): + - `test/minions-shell-validate.test.ts` — every validation path (shape, snake_case regex, fail-fast, prototype-pollution defense, the deliberate non-rules) + T1 regression guard pinning the load-bearing invariant. + - `test/minions-shell-inherit.test.ts` — `INHERIT_NAME_RE` matrix (10 accepts, 10 rejects), `deriveEnvKey` per-name behavior, `resolveInheritValue` round-trip including `__proto__` / `constructor` / `toString` prototype-pollution cases. + - `test/config-ensure-gitignore.test.ts` (6 cases) — idempotency, no-clobber, GBRAIN_HOME honored. + - `test/doctor-home-dir-in-worktree.test.ts` (5 cases) — walks dir-style + file-style `.git`, walk termination at $HOME, GBRAIN_HOME override. + - `test/e2e/minions-shell-pglite.test.ts` (+2 cases) — full PGLite round-trip: `inherit: ["database_url"]` resolves into child env (negative-shape JSON assertion that value is NOT in row), AND a separate `inherit: ["anthropic_api_key"]` test proving the mechanism is genuinely free-form. + +### Output-side redaction (opt-in) + +**`redact_secrets: true`** (or `--redact-secrets` on the CLI) opts into +output-side scrubbing. When set, the worker resolves each `inherit:` name +to a value, runs the child, then literal-string-replaces every occurrence +of those values in `stdout_tail` / `stderr_tail` (and in the `error_text` +derived from `stderr_tail` on non-zero exit) with `` before +persistence. Only `inherit:`-resolved values are scrubbed; caller-supplied +`env:` values are not (those are the "I'm fine with this in the row" channel +by design). + +```bash +gbrain jobs submit shell --params '{ + "cmd": "gbrain sync --skip-failed", + "cwd": "/data/gbrain", + "inherit": ["database_url"], + "redact_secrets": true +}' +``` + +**Heuristic, not perfect.** Literal string-replace. A script that +base64-encodes the secret before printing, or emits it one character at a +time, bypasses the scrub. Those are adversarial shapes; the agent and the +script are in the same trust domain, so this layer defends against +accidental echo (the common case), not deliberate exfiltration. Default is +`false` for back-compat — no behavior change for callers who don't opt in. + +### For contributors + +The wave's bug-class lesson is worth recording: **in queue-based job systems, validation must run before `queue.add()` if the row contents are part of the security boundary.** Validation in the worker handler runs AFTER persistence; rejecting there leaves the bad payload in the DB row. Codex caught this in plan review; the in-Claude eng review missed it on the first pass. The shared `validateShellJobParams` module exists specifically to make pre-enqueue validation the only path on both CLI and op-handler surfaces; the handler-side call is defense-in-depth only. + +Design note on the closed-vs-free-form arc: an early draft of v0.36.5.0 used a closed `INHERITABLE` enum (one record per allowed secret name, with hardcoded shadow-key sets and inline-cmd regex scans). After codex's pre-landing review surfaced multiple bypass paths the closed enum invited (cmd-inline assignments, missed shadow names like `GBRAIN_DIRECT_DATABASE_URL`), the deeper question was: what threat does the allowlist actually defend? On a single-uid topology the agent and worker share trust; refusing to let the agent inherit `voyage_api_key` because it's not on a hand-curated list is paternalism, not security. The shipped design opens `inherit:` to any snake_case config-key. The closed enum is the failure mode this skill avoids. + +## To take advantage of v0.36.5.0 + +`gbrain upgrade` does this automatically — no migrations to run, no host action required. + +1. **Verify the new pattern works on your worker host:** + ```bash + gbrain jobs submit shell --params '{"cmd":"gbrain stats","cwd":"/tmp","inherit":["database_url"]}' --follow + ``` + Expect: page count, exit 0. If you see `shell: inherit requested "database_url" but worker has no database_url configured`, run `gbrain config set database_url ` on the worker host once. +2. **Update your agent code** if it submits shell jobs with `env: { GBRAIN_DATABASE_URL: ... }`. Swap to `inherit: ["database_url"]`. The error message tells you exactly what to change. +3. **Check the doctor for worktree hygiene:** + ```bash + gbrain doctor --json | grep -A1 home_dir_in_worktree + ``` + Warn means `~/.gbrain/` lives inside a git worktree. The retroactive `.gitignore` blocks casual `git add`, but consider moving the brain out or setting `GBRAIN_HOME` if you also need backup/screenshare hygiene. +4. **If any step fails**, file an issue: https://github.com/garrytan/gbrain/issues with the output of `gbrain doctor` and the contents of `~/.gbrain/upgrade-errors.jsonl` if it exists. This feedback loop is how the wedge incidents get caught. + +Credit: this wave exists because @garrytan-agents filed PR #1137 documenting the env-stripping gap. The doc made the underlying problem visible enough to fix in code. Thank you. ## [0.36.4.0] - 2026-05-18 **Your agent can now drive your brain to 90/100 by itself, on a cron, without you watching.** @@ -616,17 +726,17 @@ GBrain v0.36 retires the managed-block install model. `gbrain skillpack install` ### What you can now do -**Scaffold a skill into your real agent repo and own it outright.** `cd ~/git/wintermute && gbrain skillpack scaffold book-mirror` copies the skill markdown, any sibling routing-eval fixtures, and any paired source files declared in the skill's frontmatter `sources:` array. No managed-block fence written to your `RESOLVER.md`. No `cumulative-slugs` receipt. No `.gbrain-skillpack.lock` file. The files are yours. Edit freely; re-scaffolding refuses to overwrite anything that exists. +**Scaffold a skill into your real agent repo and own it outright.** `cd ~/git/PR #1137 author && gbrain skillpack scaffold book-mirror` copies the skill markdown, any sibling routing-eval fixtures, and any paired source files declared in the skill's frontmatter `sources:` array. No managed-block fence written to your `RESOLVER.md`. No `cumulative-slugs` receipt. No `.gbrain-skillpack.lock` file. The files are yours. Edit freely; re-scaffolding refuses to overwrite anything that exists. **Read the diff against gbrain's bundle as reference, not as a force-update.** `gbrain skillpack reference book-mirror` prints per-file status (`identical` / `differs` / `missing`) plus unified diffs for any drift, framed with an agent-readable header: "These files live at as reference. Read them and decide what (if anything) to integrate into your local skills/. Your local edits are intentional — do not blindly overwrite." For a one-line-per-skill sweep, `reference --all`. For two-way auto-apply of non-conflicting hunks, `reference --apply-clean-hunks` (with a documented two-way merge limitation — no scaffold-time base tracking, by design). **Migrate off the old managed block with one command.** `gbrain skillpack migrate-fence` strips `` / `end -->` markers and the manifest receipt comment from your resolver file, preserving every row inside the fence verbatim as user-owned routing. Cumulative-slugs receipt missing or stale → falls back to row-parsing with a loud warning. Idempotent; re-runs are no-ops. Once your agent confirms it walks frontmatter `triggers:` for routing, `gbrain skillpack scrub-legacy-fence-rows` tears down the bridge — removes legacy rows whose skill exists AND declares non-empty triggers, preserves anything user-added. -**Lift a proven skill back into gbrain so other clients can scaffold it.** `gbrain skillpack harvest my-skill --from ~/git/wintermute` is the inverse loop. Reads the host skill's `skills//` + any paired source files, copies into gbrain's tree, updates `openclaw.plugin.json` (sorted), and runs a default-on privacy linter against `~/.gbrain/harvest-private-patterns.txt` (built-in defaults catch `\bWintermute\b`, common email regex, Slack channel patterns). Any match → rollback (delete the copy) and exit non-zero so the editorial pass surfaces the leak. Symlinks in the host skill dir are rejected; canonical-path containment prevents path traversal. The companion editorial skill `skillpack-harvest` walks the genericization checklist (scrub fork names, generalize triggers, lift fork-specific conventions to references). +**Lift a proven skill back into gbrain so other clients can scaffold it.** `gbrain skillpack harvest my-skill --from ~/git/PR #1137 author` is the inverse loop. Reads the host skill's `skills//` + any paired source files, copies into gbrain's tree, updates `openclaw.plugin.json` (sorted), and runs a default-on privacy linter against `~/.gbrain/harvest-private-patterns.txt` (built-in defaults catch `\bPR #1137 author\b`, common email regex, Slack channel patterns). Any match → rollback (delete the copy) and exit non-zero so the editorial pass surfaces the leak. Symlinks in the host skill dir are rejected; canonical-path containment prevents path traversal. The companion editorial skill `skillpack-harvest` walks the genericization checklist (scrub fork names, generalize triggers, lift fork-specific conventions to references). **Gate CI on bundle drift without breaking interactive use.** `gbrain skillpack check` defaults to informational (exit 0 even with drift) when invoked as a subcommand. Pass `--strict` to opt into exit-1 on action-needed for CI gating. Top-level `gbrain skillpack-check` (the cron entry point) keeps its existing exit-1 behavior for backwards compat. -**Auto-detect a target workspace in non-OpenClaw repos.** `autoDetectSkillsDir` gains a `cwd_walk_up` tier ahead of the `~/.openclaw/workspace` fallback. `cd ~/git/wintermute && gbrain skillpack scaffold ...` finds wintermute automatically. `$OPENCLAW_WORKSPACE` precedence preserved when explicitly set — the precedence regression (R5) is pinned by a test. +**Auto-detect a target workspace in non-OpenClaw repos.** `autoDetectSkillsDir` gains a `cwd_walk_up` tier ahead of the `~/.openclaw/workspace` fallback. `cd ~/git/PR #1137 author && gbrain skillpack scaffold ...` finds PR #1137 author automatically. `$OPENCLAW_WORKSPACE` precedence preserved when explicitly set — the precedence regression (R5) is pinned by a test. ### Migration @@ -1114,7 +1224,7 @@ The judge now sees the page-level `effective_date` for each chunk via a `(from: **Cap the cost of the post-bump re-judge.** The `PROMPT_VERSION` change invalidates the entire judge cache (the prompt shape changed, old verdicts no longer apply). Before that re-judge spends any tokens, the runner prints an upper-bound cost estimate and waits 10 seconds in TTY for Ctrl-C; set `GBRAIN_NO_PROBE_PROMPT=1` to skip or `GBRAIN_PROBE_PROMPT_GRACE_SECONDS=0` to proceed immediately. Non-TTY (autopilot) auto-proceeds with a stderr note. `--budget-usd N` hard-caps the run when cumulative cost exceeds the cap. The judge model now resolves through `models.eval.contradictions_judge` (defaults to Haiku tier), so `gbrain config set models.eval.contradictions_judge` works the same way as every other model config key. `gbrain models doctor` surfaces the new touchpoint. -**Block PII in `docs/proposals/*.md` at CI time.** A new `scripts/check-proposal-pii.sh` (wired into `bun run verify` and `bun run check:all`) catches the specific PII classes that surfaced in past RFC drafts: private repo references (`garrytan/brain`), personal-relationship vocabulary (`trial separation`, `couples session`, `divorce attorney`, etc.), death-context phrases, and the literal `Wintermute` agent name. Bare common words (`separation`, `funeral`) are intentionally not banned. The denylist names patterns rather than real names, so the script's own source doesn't re-introduce PII into the repo. +**Block PII in `docs/proposals/*.md` at CI time.** A new `scripts/check-proposal-pii.sh` (wired into `bun run verify` and `bun run check:all`) catches the specific PII classes that surfaced in past RFC drafts: private repo references (`garrytan/brain`), personal-relationship vocabulary (`trial separation`, `couples session`, `divorce attorney`, etc.), death-context phrases, and the literal `PR #1137 author` agent name. Bare common words (`separation`, `funeral`) are intentionally not banned. The denylist names patterns rather than real names, so the script's own source doesn't re-introduce PII into the repo. ### Itemized changes @@ -1128,7 +1238,7 @@ The judge now sees the page-level `effective_date` for each chunk via a `(from: - `date-filter.ts`: `shouldSkipForDateMismatch` accepts optional `effectiveDateA` and `effectiveDateB`. When both are non-null, returns `skip=false` with new `'both_have_effective_date'` reason. Other rules preserved. - `src/commands/eval-suspected-contradictions.ts`: new `--budget-usd N` hard cap (was pre-existing for runtime; help text now explains it), new cost-estimate prompt wired via `src/core/eval-contradictions/cost-prompt.ts`. `--severity` accepts `info`. `--severity` output shows `[verdict]` tag. Judge model routes through `resolveModel({configKey: 'models.eval.contradictions_judge', tier: 'utility', envVar: 'GBRAIN_CONTRADICTIONS_JUDGE_MODEL'})`. Human summary and trend chart show the per-verdict breakdown. - `src/commands/models.ts` registers `models.eval.contradictions_judge` as a tracked per-task model key so `gbrain models` and `gbrain models doctor` surface it. -- `scripts/check-proposal-pii.sh` + denylist (structural patterns only, no real names) + 15-case test in `test/scripts/check-proposal-pii.test.ts`. Wired into `bun run verify`, `bun run check:all`, and as the new `bun run check:proposal-pii` standalone target. Two privacy-guard test fixtures naming `Wintermute` allowlisted in `scripts/check-test-real-names.sh`; the new privacy-guard scripts themselves allowlisted in `scripts/check-privacy.sh`. +- `scripts/check-proposal-pii.sh` + denylist (structural patterns only, no real names) + 15-case test in `test/scripts/check-proposal-pii.test.ts`. Wired into `bun run verify`, `bun run check:all`, and as the new `bun run check:proposal-pii` standalone target. Two privacy-guard test fixtures naming `PR #1137 author` allowlisted in `scripts/check-test-real-names.sh`; the new privacy-guard scripts themselves allowlisted in `scripts/check-privacy.sh`. - Six IRON-RULE regression test suites pin the wave's invariants: R1 date-filter rule 3 relaxation preserves rules 1+2 (6 cases), R2 cache invalidation on `PROMPT_VERSION` bump, R3 verdict-enum migration (compile-time via `tsc --noEmit`), R4 runner emit predicate fires for every non-`no_contradiction` verdict (6 cases), R5 cache key tuple stays 5 fields, R6 contradiction severity unchanged (4 cases). Plus 9 cases on the cost-prompt helper. - The source RFC (`docs/proposals/temporal-contradiction-probe.md`) and PR title/body/commit/branch-name all scrubbed of PII at draft time. The new CI lint prevents the next one from slipping through. @@ -2613,7 +2723,7 @@ The bigger swing (chunk-level `revises` field + ranking change + synthesize-prom **Time, place, and what you're doing — reinjected on every turn, no matter how hard the session got compacted.** **A new OpenClaw context engine that kills the "time warp" bug class with zero LLM calls and <5ms overhead.** -When a long session compacts, the LLM loses track of what time it is, where Garry is, and what he's working on. The headline incident: Wintermute responded to a Sunday-night photo as if it were Monday morning, and reported Pacific Time while Garry was in Toronto. Both are downstream of the same architectural gap — compaction discards live state, and there was no mechanism to put it back. +When a long session compacts, the LLM loses track of what time it is, where Garry is, and what he's working on. The headline incident: PR #1137 author responded to a Sunday-night photo as if it were Monday morning, and reported Pacific Time while Garry was in Toronto. Both are downstream of the same architectural gap — compaction discards live state, and there was no mechanism to put it back. v0.32.5 ships `gbrain-context`, an OpenClaw plugin that owns the `systemPromptAddition` slot on every `assemble()` call. It reads `memory/heartbeat-state.json`, `memory/upcoming-flights.json`, `memory/calendar-cache.json`, and `ops/tasks.md` from the agent workspace, and injects a structured block: current local time + day-of-week (computed from the location's timezone, not hardcoded US/Pacific), current city + country, home time when traveling, active flight + route when in transit, the meeting Garry is in right now, the next three calendar events within a 4-hour window, and unchecked tasks under "Today." Compaction can be as aggressive as it wants — the next turn rebuilds the block from disk in under 5ms. @@ -3733,7 +3843,7 @@ Tests: 4570 unit pass / 1 pre-existing master flake (`BrainRegistry — lazy ini **Thin-client mode actually works now. `gbrain init --mcp-only` is no longer a half-built bridge.** **Every read + write + admin op routes through MCP; refused commands carry pinpoint hints.** -Hermes/Neuromancer hit this in production: thin-client install of wintermute (102k pages, +Hermes/Neuromancer hit this in production: thin-client install of PR #1137 author (102k pages, 265k chunks). Every CLI search returned zero rows. Exit code 0. No warning. The agent configured `gbrain init --mcp-only`, then walked into a wall of "no results found" against a brain that had everything it needed. The CLI was opening the empty local PGLite, @@ -3802,7 +3912,7 @@ warns about the new `oauth_client_scopes_probe` check: gbrain stats ``` Both should now show real numbers and an identity banner like - `[thin-client → wintermute:3131 · brain: 102k pages, 265k chunks · v0.31.1]`. + `[thin-client → PR #1137 author:3131 · brain: 102k pages, 265k chunks · v0.31.1]`. 4. **If any step fails or the numbers look wrong**, file an issue at https://github.com/garrytan/gbrain/issues with `gbrain remote doctor` output. @@ -4029,7 +4139,7 @@ PGLite test (CI default) and a Postgres parity test The v0.30 dream cycle has been stalled since May 2 for one user — daily aggregated transcripts at 2.7-4.5MB each generate 1.7M-token Anthropic prompts, which hit the 1M-token hard limit and 400. The subagent handler treated those failures as renewable, so every doomed transcript stalled three times before dead-lettering, and every new cycle re-discovered and re-submitted the same fat transcripts. Six days of synth backlog, queue full of doomed work. -v0.30.2 adds model-aware chunking + terminal-error classification + a poison-pill-free skip path. Wintermute's [PR #748](https://github.com/garrytan/gbrain/pull/748) supplied the boundary heuristics (`## Topic:` → `---` → `\n` ladder) and the `dream.synthesize.max_prompt_tokens` config surface. Garry's branch extended that with model-aware budgets, deterministic chunk identity for partial-progress safety, orchestrator-side slug rewriting for zero Sonnet trust on collisions, and doctor-surface visibility. +v0.30.2 adds model-aware chunking + terminal-error classification + a poison-pill-free skip path. PR #1137's [PR #748](https://github.com/garrytan/gbrain/pull/748) supplied the boundary heuristics (`## Topic:` → `---` → `\n` ladder) and the `dream.synthesize.max_prompt_tokens` config surface. Garry's branch extended that with model-aware budgets, deterministic chunk identity for partial-progress safety, orchestrator-side slug rewriting for zero Sonnet trust on collisions, and doctor-surface visibility. ### What you can now do @@ -4500,7 +4610,7 @@ path mirroring `gbrain reindex-code`. https://github.com/garrytan/gbrain/issues Co-Authored-By: Claude Opus 4.7 (1M context) -Co-Authored-By: Wintermute +Co-Authored-By: PR #1137 author ## [0.29.0] - 2026-05-03 @@ -6328,7 +6438,7 @@ If you're a contributor: - The privacy gate (`scripts/check-privacy.sh`) was previously only in the now-removed `bun run test` chain. It now runs via `bun run verify` and CI's `.github/workflows/test.yml` calls `bun run verify` directly — single source of truth for "what's the ship gate." #### CI tightening -- **`.github/workflows/test.yml`** now runs `bun run verify` (was: 4 specific scripts inlined). Privacy check now actually fires on every CI run; previously it ran only when somebody manually invoked `bun run test`. The pre-existing `Wintermute` references in `src/core/mounts-cache.ts:6` and `:324` (introduced in earlier commits and surviving every CI green) were caught by the now-firing gate and replaced with `your OpenClaw` per the privacy rule. +- **`.github/workflows/test.yml`** now runs `bun run verify` (was: 4 specific scripts inlined). Privacy check now actually fires on every CI run; previously it ran only when somebody manually invoked `bun run test`. The pre-existing `PR #1137 author` references in `src/core/mounts-cache.ts:6` and `:324` (introduced in earlier commits and surviving every CI green) were caught by the now-firing gate and replaced with `your OpenClaw` per the privacy rule. #### Failure-first logging - **`.context/test-failures.log`** — extracted failure blocks per shard, prefixed with `--- shard N: ---`. Cleared at the start of every wrapper run. Falls back to `/tmp/gbrain-test-failures.log` if `.context/` is unwritable. diff --git a/CLAUDE.md b/CLAUDE.md index c5a8045f2..8260725b0 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -144,7 +144,12 @@ strict behavior when unset. - `src/core/minions/spawn-helpers.ts` (v0.28.1) — pure `detectTini()` + `buildSpawnInvocation()` helpers consumed by both `supervisor.ts` and `autopilot.ts`. Resolves the DRY violation between the two spawn sites and makes the tini wrapping testable without `mock.module()` (rule R2 of `scripts/check-test-isolation.sh`). `detectTini()` calls `execFileSync('which', ['tini'])` with explicit `env: process.env` so Bun sees runtime PATH mutations (the env-snapshot bug fix). `buildSpawnInvocation(tiniPath, cmd, args)` returns `{cmd, args}` with tini prepended when present, or the bare invocation otherwise. Pinned by `test/spawn-helpers.test.ts` (5 cases) and `test/supervisor-tini.test.ts` (4 cases). - `src/core/minions/types.ts` — `MinionJobInput` + `MinionJobStatus` + handler context types. `MinionJobInput.max_stalled` (new in v0.14.1) is optional; omitted values let the schema DEFAULT (5) kick in, provided values are clamped to `[1, 100]`. - `src/core/minions/protected-names.ts` — side-effect-free constant module exporting `PROTECTED_JOB_NAMES` + `isProtectedJobName()`. Kept pure so queue core can import without loading handler modules. -- `src/core/minions/handlers/shell.ts` — `shell` job handler. Spawns `/bin/sh -c cmd` (absolute path, PATH-override-safe) or `argv[0] argv[1..]` (no shell). Env allowlist: `PATH, HOME, USER, LANG, TZ, NODE_ENV` + caller `env:` overrides. UTF-8-safe stdout/stderr tail via `string_decoder.StringDecoder`. Abort (either `ctx.signal` or `ctx.shutdownSignal`) fires SIGTERM → 5s grace → SIGKILL on child. Requires `GBRAIN_ALLOW_SHELL_JOBS=1` on worker (gated by `registerBuiltinHandlers`). +- `src/core/minions/handlers/shell.ts` — `shell` job handler. Spawns `/bin/sh -c cmd` (absolute path, PATH-override-safe) or `argv[0] argv[1..]` (no shell). Env allowlist: `PATH, HOME, USER, LANG, TZ, NODE_ENV` + caller `env:` overrides + (v0.36.5.0) `inherit:`-resolved keys. UTF-8-safe stdout/stderr tail via `string_decoder.StringDecoder`. Abort (either `ctx.signal` or `ctx.shutdownSignal`) fires SIGTERM → 5s grace → SIGKILL on child. Requires `GBRAIN_ALLOW_SHELL_JOBS=1` on worker (gated by `registerBuiltinHandlers`). **v0.36.5.0:** `ShellJobParams.inherit?: string[]` is a free-form list of snake_case config-key names. The worker resolves each via `loadConfig()` and injects the value under the derived env key (`database_url` → `GBRAIN_DATABASE_URL`; everything else uppercased). Names persist in `minion_jobs.data` (and the shell-audit JSONL); values never do. The canonical validator `validateShellJobParams` (sibling file `shell-validate.ts`) runs **pre-enqueue** in both submit surfaces — `gbrain jobs submit shell` (jobs.ts:271) AND `submit_job` op for `name='shell'` (operations.ts:2085). The handler-entry re-validation here is defense-in-depth. Closes the codex F-CDX-1 load-bearing bug class where validation in the handler ran AFTER `queue.add()` persisted the row. The validator does NOT police which config keys the agent inherits — same-uid trust model treats the agent as a peer of the worker. +- `src/core/minions/handlers/shell-inherit.ts` (v0.36.5.0, NEW) — three small helpers, no closed enum. `INHERIT_NAME_RE` (`/^[a-z][a-z0-9_]*$/`) is the snake_case shape guard used by the validator; rejects `__proto__`, leading-underscore, uppercase, and path-traversal shapes so audit logs stay readable and prototype-pollution lookups can't smuggle through. `deriveEnvKey(name)` maps config-key → child-env-key (`name.toUpperCase()` with one override: `database_url` → `GBRAIN_DATABASE_URL` because plain `DATABASE_URL` is ambiguous). `resolveInheritValue(cfg, name)` is the value lookup; uses `Object.hasOwn` to defeat prototype-pollution lookups, returns undefined for missing / non-string / empty-string values. An earlier closed-enum design (hardcoded `INHERITABLE` record with shadow-keys per name) was abandoned because the agent and worker share a uid — refusing to let the agent inherit arbitrary config keys defends nothing in that trust model. +- `src/core/minions/handlers/shell-validate.ts` (v0.36.5.0, NEW) — `validateShellJobParams(data, opts?)` shared pre-enqueue validator. Throws `UnrecoverableError` with paste-ready operator hints on every failure path. Three rules: (1) existing cmd/argv/cwd/env shape, (2) inherit array shape + snake_case regex per element (prototype-pollution defense), (3) fail-fast on missing config value with `gbrain config set ` hint. Also accepts optional `redact_secrets?: boolean` for output-side scrubbing. The validator deliberately does NOT police WHICH secrets the agent passes — single-uid trust model. Test seam: `opts.config` lets unit tests drive the validator hermetically without mocking the module. The defense-in-depth re-call at `shell.ts` handler entry catches pre-existing rows submitted before v0.36.5.0. +- `src/core/minions/handlers/shell-redact.ts` (v0.36.5.0, NEW) — opt-in output-side scrubbing for shell-job stdout/stderr. Pure `redactSecretsInText(text, secrets)` function: string-mode `replaceAll` so regex metacharacters in values stay literal. When the caller passes `redact_secrets: true` (or `--redact-secrets` on the CLI), the handler builds a Map of inherit-name → resolved-value and post-processes both tails before throw/return, so the persisted `result.stdout_tail` / `result.stderr_tail` / `error_text` carry `` instead of the value. Only `inherit:`-resolved values are scrubbed; caller-supplied `env:` values stay through (those are the agent's "fine in the row" channel). Heuristic — defeats the common-case `echo "$GBRAIN_DATABASE_URL"` echo, not adversarial encode-then-print. Default `false` for back-compat. +- `src/core/config.ts:ensureGitignore` (v0.36.5.0) — idempotent retroactive writer of `~/.gbrain/.gitignore` (single line `*`). Called from `saveConfig()` so every config-writing path lays it down, AND from `runPostUpgrade()` so existing users pick it up on next `gbrain upgrade`. Never clobbers a user-customized `.gitignore` (checks file exists + content non-empty before writing). Honest scope, named in CHANGELOG: blocks casual `git add ~/.gbrain` from inside an enclosing worktree, but does NOT cover already-tracked files, screenshots, backups (Time Machine / iCloud / Dropbox), or `git add -f`. The doctor check `home_dir_in_worktree` surfaces what `.gitignore` can't. +- `src/commands/doctor.ts:home_dir_in_worktree` (v0.36.5.0) — filesystem check walking up from `gbrainPath()` toward `$HOME` looking for either a `.git` directory (main repo) or `.git` file (linked worktree pointer; Conductor + git-worktrees topology). Walk terminates at `$HOME` so a `.git` above the user's home doesn't false-positive. Honors `GBRAIN_HOME` (gbrain appends `.gbrain` to the override). Warn (not fail) with worktree-root path + paste-ready fix pointing at `GBRAIN_HOME` override or moving the brain. - `src/core/minions/handlers/shell-audit.ts` — per-submission JSONL audit trail at `~/.gbrain/audit/shell-jobs-YYYY-Www.jsonl` (ISO-week rotation; override via `GBRAIN_AUDIT_DIR`). Best-effort: `mkdirSync(recursive)` + `appendFileSync`; failures logged to stderr, submission not blocked. Logs cmd (first 80 chars) or argv (JSON array). Never logs env values. - `src/core/minions/handlers/supervisor-audit.ts` — supervisor lifecycle JSONL audit at `~/.gbrain/audit/supervisor-YYYY-Www.jsonl` (ISO-week rotation; shares `computeIsoWeekName()` helper with `shell-audit.ts`). `writeSupervisorEvent(emission, supervisorPid)` appends one line per supervisor event (`started`, `worker_spawned`, `worker_exited`, `backoff`, `health_warn`, `health_error`, `max_crashes_exceeded`, `shutting_down`, `stopped`, `worker_spawn_failed`). `readSupervisorEvents({sinceMs})` is the readback path for `gbrain doctor`. **v0.35.5.0:** new exports `isCrashExit(event)`, `summarizeCrashes(events)`, `CrashSummary` type, and `CLEAN_EXIT_CAUSES` denylist (`'clean_exit' | 'graceful_shutdown'`). Single regression point — both `gbrain doctor` (Lane D supervisor check at `doctor.ts:1011-1043`) and `gbrain jobs supervisor status` (`jobs.ts:803-826`) import from here so the two CLI surfaces cannot drift. `isCrashExit` classifies a single `worker_exited` event against the denylist: `clean_exit` / `graceful_shutdown` are NON-crashes; everything else (`runtime_error`, `oom_or_external_kill`, `unknown`, AND any future `likely_cause` value added upstream in `child-worker-supervisor.ts`) is a crash. Pre-v0.34 audit lines lacking `likely_cause` fall back to `code !== 0`. `summarizeCrashes` returns `{total, by_cause: {runtime_error, oom_or_external_kill, unknown, legacy}, clean_exits}` so dashboards bind to named buckets — the `legacy` bucket catches BOTH pre-v0.34 fallback entries AND future unrecognized `likely_cause` values, fail-loud instead of silent underreport. Denylist-over-allowlist was a codex outside-voice catch during `/plan-eng-review` — the bug being fixed (read sites counting every `worker_exited` as a crash, inflating to 120+/day on healthy brains after v0.34.3.0 watchdog drains) was itself an allowlist-of-event-names. Pinned by `test/supervisor-audit.test.ts` (14 cases: 9-case `isCrashExit` branch matrix including denylist regression guard for unrecognized future causes + non-exit-event defensive case, 5-case `summarizeCrashes` aggregator including unrecognized-cause routing to legacy + null-code edge case) and 4 source-grep wiring assertions in `test/doctor.test.ts` guarding both surfaces against drift. - `src/core/minions/backpressure-audit.ts` (v0.19.1) — sibling of shell-audit.ts for `maxWaiting` coalesce events. JSONL at `~/.gbrain/audit/backpressure-YYYY-Www.jsonl`. Fires one line per coalesce with `(queue, name, waiting_count, max_waiting, returned_job_id, ts)`. Closes the silent-drop vector the v0.19.0 maxWaiting guard introduced. diff --git a/VERSION b/VERSION index a1aada581..acfbf4c89 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.36.4.0 \ No newline at end of file +0.36.5.0 diff --git a/docs/UPGRADING_DOWNSTREAM_AGENTS.md b/docs/UPGRADING_DOWNSTREAM_AGENTS.md index 18312948a..053eb22c3 100644 --- a/docs/UPGRADING_DOWNSTREAM_AGENTS.md +++ b/docs/UPGRADING_DOWNSTREAM_AGENTS.md @@ -538,3 +538,75 @@ To check what your fork is missing: diff <(grep -A3 "Based on gbrain" ~//skills/brain-ops/SKILL.md) \ <(grep "v[0-9]" ~/gbrain/skills/migrations/ | tail -3) ``` + + +## v0.36.5.0 — Free-form secret inheritance for shell jobs calling `gbrain` CLI + +**The change.** Shell-job params get a new `inherit:` field. Pass any +snake_case config-key name on it; the worker resolves the value from its +`loadConfig()` at child-spawn time and injects it into the child env. Names +land in the row; values never persist from `inherit:`. Validation runs +**pre-enqueue** in both submit paths (CLI + `submit_job` op), so a malformed +payload never lands in `minion_jobs.data`. + +**Why.** Pre-v0.36.5.0, agents that wanted to call `gbrain` from shell jobs +had to either write `database_url` to `~/.gbrain/config.json` plaintext or +pass `env: { GBRAIN_DATABASE_URL: "..." }` per-job. Both left plaintext +secrets somewhere — disk or DB row. `inherit:` keeps names in the row and +resolves values at spawn time. + +**What your agent can do.** `inherit:` is free-form. Pass any config-key: + +```jsonc +{ + "cmd": "gbrain sync --skip-failed && gbrain embed --stale", + "cwd": "/data/gbrain", + "inherit": ["database_url", "anthropic_api_key", "voyage_api_key"] +} +``` + +The env-key name in the child is derived by uppercasing the config-key: +`database_url` → `GBRAIN_DATABASE_URL`, `anthropic_api_key` → +`ANTHROPIC_API_KEY`, `voyage_api_key` → `VOYAGE_API_KEY`, etc. The validator +does NOT police which config keys you inherit — the agent is in the same +uid as the worker, so it's the agent's call. + +**You can still use `env:`.** v0.36.5.0 does not forbid `env:{ ANYTHING }`. +If you have a reason to put a value in the row plaintext (a non-secret +correlation token, or a secret you know is OK to persist), pass it via +`env:`. Prefer `inherit:` when you want the value out of the row. + +**Worker setup** (one-time, per host): + +- `gbrain config set database_url postgresql://...` (or any other key you + want available for inherit) +- OR put the key in `~/.gbrain/config.json` directly +- OR set `GBRAIN_DATABASE_URL` / `DATABASE_URL` / per-provider env on the + worker process + +If the worker can't resolve a requested name, the validator fail-fasts at +submit time with `gbrain config set ` hint. No more silent "No database +URL" failures in child stderr minutes after submission. + +**Also new.** A `gbrain doctor` check `home_dir_in_worktree` warns if +`~/.gbrain/` lives inside a git worktree. A retroactive `~/.gbrain/.gitignore` +(single line `*`) is now laid down by every `saveConfig()` call AND by +`gbrain post-upgrade`, so existing users get coverage without re-running +`gbrain init`. Honest scope: the `.gitignore` covers casual `git add` but does +NOT cover already-tracked files, screenshots, backups, or `git add -f`. + +**Strategy framing.** For agent-to-gbrain calls, the new canonical guide is +`docs/guides/agent-to-gbrain.md`. Two distinct surfaces: HTTP MCP via OAuth +for ops with MCP equivalents (`search`, `query`, `put_page`, etc.), and shell +job + `inherit:` for `localOnly` admin ops (`sync`, `embed`, `dream`, +`doctor`, etc.). Not a fallback hierarchy — pick by op. + +**Errors to handle** (your agent submits shell jobs; surface these clearly): + +| Error | What it means | Agent action | +|---|---|---| +| `shell: inherit must be an array of config-key names` | `inherit` wasn't an array. | Pass `"inherit": ["database_url", ...]`. | +| `shell: inherit entries must be non-empty strings` | Element was empty, non-string, or null. | Use snake_case config-key names. | +| `shell: inherit name "" must match [a-z][a-z0-9_]*` | Name failed snake_case regex (uppercase, leading underscore, etc.). | Use the config-key verbatim — `database_url`, not `DATABASE_URL`. | +| `shell: inherit requested "" but worker has no configured` | Worker can't resolve the name from its `loadConfig()`. | Run `gbrain config set ` on the worker host. | + diff --git a/docs/guides/agent-to-gbrain.md b/docs/guides/agent-to-gbrain.md new file mode 100644 index 000000000..20627b2ab --- /dev/null +++ b/docs/guides/agent-to-gbrain.md @@ -0,0 +1,199 @@ +# How a downstream agent should talk to gbrain + +This guide is for authors of downstream agents (hermes, openclaw, future +forks) that need to call gbrain operations from their own runtime. Reading +this first will save you a debugging cycle: gbrain has **two distinct +surfaces**, and which one you pick depends on the operation. + +## The two surfaces + +``` + ┌─────────────────────────────────────────────┐ + │ gbrain process │ + │ │ + Agent (hermes, │ ┌──────────────────┐ ┌────────────────┐ │ + openclaw, fork) ────┼──▶ MCP ops surface │ │ localOnly │ │ + │ │ (HTTP + OAuth) │ │ admin ops │ │ + │ │ │ │ │ │ + │ │ search, query, │ │ sync, embed, │ │ + │ │ put_page, │ │ dream, doctor,│ │ + │ │ get_page, │ │ autopilot, │ │ + │ │ find_experts, │ │ init, secrets │ │ + │ │ ... │ │ │ │ + │ └──────────────────┘ └────────────────┘ │ + │ ▲ ▲ │ + │ │ │ │ + │ │ │ │ + │ thin-client OAuth shell-job `inherit:`│ + │ (preferred for (only path for │ + │ MCP-equivalent ops) localOnly ops) │ + └─────────────────────────────────────────────┘ +``` + +The two surfaces are **not interchangeable**. Pick by op, not by preference. + +## Surface 1 — MCP ops over HTTP (thin-client + OAuth) + +Use for any operation that has an MCP equivalent: `search`, `query`, +`put_page`, `get_page`, `find_experts`, `find_orphans`, `find_anomalies`, +`get_recent_salience`, `find_trajectory`, and so on. The canonical list is +the set of ops in `src/core/operations.ts` whose `localOnly` flag is unset +(or `false`). + +### Setup + +The host runs gbrain as a long-lived HTTP server: + +```bash +GBRAIN_ALLOW_SHELL_JOBS=1 gbrain serve --http --port 3131 +``` + +The agent registers as an OAuth client (one-time): + +```bash +gbrain auth register-client hermes \ + --grant-types client_credentials \ + --scopes read,write +# Prints client_id + client_secret one-time. Store securely. +``` + +The agent's runtime calls `/mcp` with a bearer token from `client_credentials` +grant. Secrets stay in the gbrain serve process; the agent never sees +DATABASE_URL or API keys. + +Thin-client mode (`gbrain init --mcp-only`) gives the agent the same +client-credentials wiring, plus the `gbrain` CLI itself routes MCP-eligible +commands through the configured remote MCP. The agent can call +`gbrain search` / `gbrain query` directly and the CLI does the OAuth dance. + +### Why this is preferred for MCP ops + +- Secrets never leave the server process. +- OAuth scopes give you `read`, `write`, `admin` separation — agent only gets + what it needs. +- Source-scoped tokens (`--source dept-x` on `register-client`) confine the + agent to a specific source within a federated brain. +- One audit surface (`mcp_request_log`) covers every op call uniformly. + +## Surface 2 — localOnly admin ops via shell-job `inherit:` + +Some operations are flagged `localOnly: true` in `src/core/operations.ts` and +are **refused** in thin-client mode at `src/cli.ts:isThinClient`. The full +list (as of v0.36.5.0) includes: + +- `sync` (filesystem walks need local FS access) +- `embed` (orchestrates the embed pipeline) +- `extract` (walks markdown files) +- `dream` (synthesis cycle) +- `doctor` (filesystem hygiene checks) +- `autopilot` (background daemon orchestration) +- `init` (creates `~/.gbrain/`) +- `secrets` (config management) + +For these, the agent cannot route through HTTP MCP. The only path is to run +`gbrain` as a CLI subprocess. The recommended pattern is to submit the +subprocess as a shell job to the gbrain Minions worker so retry / backoff / +DLQ / audit trail all come for free. + +### Setup + +```bash +gbrain jobs submit shell --params '{ + "cmd": "gbrain sync --skip-failed && gbrain embed --stale", + "cwd": "/data/gbrain", + "inherit": ["database_url"] +}' +``` + +The `inherit: ["database_url"]` field tells the worker to look up +`database_url` from its `loadConfig()` and inject the value into the child +env as `GBRAIN_DATABASE_URL`. The DB row in `minion_jobs.data` carries the +names only — `inherit: ["database_url"]` — never the value. See +[minions-shell-jobs.md#secrets](./minions-shell-jobs.md#secrets) for the +full validation rules and error catalog. + +### Why this is preferred over writing secrets into `env:` per-job + +- Pre-v0.36.5.0 callers passed `env: { GBRAIN_DATABASE_URL: "postgresql://..." }` + per job. The URL landed plaintext in `minion_jobs.data` and the shell-audit + JSONL. Anyone with brain-DB read access (or a brain dump, or a shared brain + via mounts) saw the URL. As of v0.36.5.0, this is rejected at pre-enqueue + validation. The error message names `inherit: ["database_url"]` as the + replacement. + +### Worker setup (one-time, per host) + +The agent's host needs a worker that processes shell jobs: + +```bash +# One-shot inline execution (PGLite or Postgres): +gbrain jobs submit shell --params '{...}' --follow + +# Persistent worker (Postgres only — PGLite uses --follow inline): +GBRAIN_ALLOW_SHELL_JOBS=1 gbrain jobs work +``` + +`GBRAIN_ALLOW_SHELL_JOBS=1` is the worker-side opt-in. Without it, shell jobs +sit in `waiting` indefinitely. Set it on the worker process env (or in your +deploy unit / launchd plist), not per-submission — submitter env is a weak +proxy for worker env. + +## Decision table + +| Operation | Surface | Why | +|---|---|---| +| `search` / `query` | HTTP MCP via thin-client | Has MCP op; OAuth-scoped. | +| `get_page` / `list_pages` | HTTP MCP | Same. | +| `put_page` | HTTP MCP | Same; respects subagent allow-list when applicable. | +| `find_experts` / `find_orphans` | HTTP MCP | Same. | +| `sync` / `embed` / `extract` | Shell job + `inherit:` | `localOnly: true`. | +| `dream` | Shell job + `inherit:` | `localOnly: true`. | +| `doctor` | Shell job + `inherit:` (or no inherit if no DB) | `localOnly: true`. | +| `autopilot` | Run as a daemon directly on the host | Long-lived, not job-shaped. | +| `init` / `secrets` | One-time host setup | Operator action, not agent action. | + +## Recommended patterns + +- **Prefer `inherit:` for secrets you don't want in the row.** Names land in + `minion_jobs.data`; values resolve at child-spawn from the worker's config. + If a brain DB ever traverses a trust boundary, secrets stay out. +- **Free-form names.** `inherit:` accepts any snake_case config-key on your + worker — `database_url`, `anthropic_api_key`, `openai_api_key`, + `voyage_api_key`, `groq_api_key`, `zeroentropy_api_key`, or any custom + field you stuff into `~/.gbrain/config.json`. The agent picks what it + needs. +- **`env:` still works** for non-secret values, or for cases where you + WANT the value in the row (e.g. an opaque correlation token your audit + flow needs to read back later). The validator doesn't second-guess you. +- **Never try to route a `localOnly` op through thin-client MCP.** It will + fail with `localOnly op refused in thin-client mode`. Use shell-job + + `inherit:` (for secrets) or `env:` (for non-secrets). + +## Migration: from pre-v0.36.5.0 + +If your agent submits shell jobs that pass secrets via `env:`: + +```jsonc +// Pre-v0.36.5.0: works but URL persists in minion_jobs.data plaintext. +{ + "cmd": "gbrain sync --skip-failed", + "cwd": "/data/gbrain", + "env": { "GBRAIN_DATABASE_URL": "postgresql://..." } +} +``` + +Switch to (recommended): + +```jsonc +// v0.36.5.0+: name in row, value resolved at child-spawn from worker config. +{ + "cmd": "gbrain sync --skip-failed", + "cwd": "/data/gbrain", + "inherit": ["database_url"] +} +``` + +Make sure the worker host has `database_url` configured (either via +`gbrain config set database_url ` or via `GBRAIN_DATABASE_URL` / +`DATABASE_URL` env on the worker process). If the worker can't resolve the +key, the validator rejects the job at submit time with a paste-ready hint. diff --git a/docs/guides/minions-shell-jobs.md b/docs/guides/minions-shell-jobs.md index 7879278aa..62496e890 100644 --- a/docs/guides/minions-shell-jobs.md +++ b/docs/guides/minions-shell-jobs.md @@ -46,10 +46,13 @@ pass: **What the env allowlist does AND does not do.** Shell jobs run with a minimal env: `PATH, HOME, USER, LANG, TZ, NODE_ENV`. Your secrets like `OPENAI_API_KEY` and `DATABASE_URL` are NOT passed to the child. You opt-in additional keys per -job via `env: { ... }`. This stops accidental `$OPENAI_API_KEY` interpolation in -a user-authored script. It does **not** sandbox filesystem reads: a shell -script can `cat ~/.env` or any file the worker process can read. The operator -picks a safe `cwd`. That is the trust boundary. +job via `env: { ... }` (non-secret values only — see "Secrets" below) or via +`inherit: ["database_url"]` (recommended for secrets — names only in the row, +values resolved at child-spawn from `gbrain config set`). This stops accidental +`$OPENAI_API_KEY` interpolation in a user-authored script. It does **not** +sandbox filesystem reads: a shell script can `cat ~/.env` or any file the +worker process can read. The operator picks a safe `cwd`. That is the trust +boundary. **Audit trail, not forensic insurance.** Every submission writes a JSONL line to `~/.gbrain/audit/shell-jobs-YYYY-Www.jsonl` (ISO-week rotation; override @@ -106,6 +109,115 @@ Note: `--follow` blocks the crontab slot until the job finishes. If 14 shell crons land at the same minute and each takes 30s, they serialize through crontab's spawning limits. Postgres + persistent worker scales better. +### Calling `gbrain` itself from a shell job — use `inherit:` for DATABASE_URL {#secrets} + +A common pattern is submitting shell jobs that run `gbrain` CLI commands: + +```bash +gbrain jobs submit shell --params '{ + "cmd": "gbrain sync --skip-failed && gbrain embed --stale", + "cwd": "/data/gbrain", + "inherit": ["database_url"] +}' +``` + +`inherit: ["database_url"]` tells the worker to look up `database_url` from its +own `loadConfig()` (file + env merged) and inject the value into the child's +env as `GBRAIN_DATABASE_URL`. The job row in `minion_jobs.data` stores +`inherit: ["database_url"]` — **names only, never values**. The shell-audit +JSONL records the same. Pre-enqueue validation rejects the submission if the +worker can't resolve the requested key, with a paste-ready +`gbrain config set database_url ` hint. + +**Why not just write the URL into `env:` directly?** Pre-v0.36.5.0 callers +wrote things like: + +```jsonc +// ❌ Deprecated as of v0.36.5.0 — REJECTED at submit time. +{ + "cmd": "gbrain stats", + "cwd": "/data/gbrain", + "env": { "GBRAIN_DATABASE_URL": "postgresql://..." } +} +``` + +This planted plaintext secrets in `minion_jobs.data` (DB row) and in the +shell-audit JSONL. Anyone with read access to the brain DB (or a brain dump, +or a shared brain via the mounts feature) saw the URL. v0.36.5.0 doesn't +forbid that pattern — the validator trusts the agent — but **prefer +`inherit:`** for any secret you want kept out of the row. Names land in the +row; values resolve at child-spawn from the worker's config. + +**Scope:** v0.36.5.0 `inherit:` is **free-form**. Pass any snake_case +config-key name and the worker resolves the value from `loadConfig()` at +child-spawn time: + +- `inherit: ["database_url"]` → child env `GBRAIN_DATABASE_URL` +- `inherit: ["anthropic_api_key"]` → child env `ANTHROPIC_API_KEY` +- `inherit: ["openai_api_key"]` → child env `OPENAI_API_KEY` +- `inherit: ["voyage_api_key"]` → child env `VOYAGE_API_KEY` +- `inherit: ["groq_api_key", "zeroentropy_api_key"]` → both injected +- Or any arbitrary config-key your worker has (`my_custom_field` → + `MY_CUSTOM_FIELD`) + +The env-key name is derived by uppercasing the config-key name. The one +override is `database_url` → `GBRAIN_DATABASE_URL` (plain `DATABASE_URL` is +ambiguous in most Postgres-app contexts). + +Pre-enqueue validation fail-fasts if the worker can't resolve a requested +name. The validator does NOT police which secrets you choose to inherit — +the agent submitting the minion is in the same uid as the worker, so it's +your call. + +**Output-side leakage (read this).** The `inherit:` allowlist prevents +secrets from landing in the JOB ROW INPUT fields (`data.cmd`, `data.argv`, +`data.env`). By default it does NOT scrub the OUTPUT fields — if your +script prints the secret to stdout or stderr (`echo "$GBRAIN_DATABASE_URL"`, +`psql "$GBRAIN_DATABASE_URL"` echoing the URL on error), the value lands +plaintext in `result.stdout_tail` / `result.stderr_tail` / `error_text`, +and from there into the brain DB row. + +**`redact_secrets: true` opts into output-side scrubbing.** Set it per-job +(or pass `--redact-secrets` on the CLI): + +```bash +gbrain jobs submit shell --params '{ + "cmd": "gbrain sync --skip-failed", + "cwd": "/data/gbrain", + "inherit": ["database_url"], + "redact_secrets": true +}' + +# Or, equivalently: +gbrain jobs submit shell \ + --params '{"cmd":"gbrain sync --skip-failed","cwd":"/data/gbrain","inherit":["database_url"]}' \ + --redact-secrets +``` + +When `redact_secrets: true`, the worker resolves each name in `inherit:` to +a value, runs the child, then string-replaces every occurrence of those +values in `stdout_tail` / `stderr_tail` (and in the `error_text` derived from +`stderr_tail` on non-zero exit) with `` before persistence. +Only `inherit:`-resolved values are scrubbed; caller-supplied `env:` values +are not (those are the "I'm fine with this in the row" channel by design). + +**Heuristic, not perfect.** The redactor uses literal string-replace. A +script that base64-encodes the secret before printing, or that emits it +one character at a time, will bypass the scrub. Those are adversarial +shapes — the agent + the script are in the same trust domain, so this +layer defends against accidental echo (the common case), not deliberate +exfiltration. + +**Three rules for shell-job authors who deal with secrets:** + +- **Prefer not to echo secrets at all.** Even with `redact_secrets`, less + output means less risk if the redactor ever has an edge-case miss. +- **Wrap noisy CLI tools to suppress URLs on error.** `psql --quiet`, + `pg_dump --quiet`, or pipe through + `2>&1 | sed 's|postgresql://[^@]*@|postgresql://REDACTED@|g'`. +- **Inspect with `gbrain jobs get ` after a failure** to verify what + actually persisted. + ### Submitting with `argv` (no shell interpolation) For programmatic callers assembling commands from JSON, use `argv` instead of @@ -161,6 +273,11 @@ gbrain jobs list --status waiting --name shell | `shell: cwd is required and must be an absolute path` | `cwd` must be a string starting with `/`. | Set `cwd` in `--params` to an absolute path. | | `shell: argv must be an array of strings` | `argv` has a non-string entry or isn't an array. | Pass `argv: ["bin","arg1","arg2"]`. | | `shell: env values must all be strings` | `env` has a number/bool/object value. | Stringify: `"env":{"COUNT":"3"}` not `"env":{"COUNT":3}`. | +| `shell: inherit must be an array of config-key names` | `inherit` wasn't an array. | Pass `"inherit": ["database_url", ...]`. | +| `shell: inherit entries must be non-empty strings` | An element of `inherit` was empty, non-string, or null. | Use snake_case config-key names like `database_url`, `anthropic_api_key`. | +| `shell: inherit name "" must match [a-z][a-z0-9_]*` | Name failed snake_case regex (uppercase, leading digit/underscore, special char). | Use the config-key name verbatim — `database_url`, not `DATABASE_URL`. | +| `shell: inherit requested "" but worker has no configured` | Worker can't resolve the requested name from `loadConfig()`. | Run `gbrain config set ` on the worker host, OR check the config file at `~/.gbrain/config.json`. | +| `shell: redact_secrets must be a boolean if set` | Caller passed a non-boolean for `redact_secrets`. | Pass `true` or `false` (or omit). The CLI `--redact-secrets` flag sets it automatically. | | `permission_denied: shell jobs cannot be submitted over MCP` | An MCP client tried to submit a shell job. By design CLI-only. | Submit from CLI or via a trusted operation handler (`ctx.remote === false`). | | `protected job name 'shell' requires CLI or operation-local submitter` | A caller invoked `MinionQueue.add('shell', ...)` without the `trusted` opt-in. | Pass `{ allowProtectedSubmit: true }` as the 4th arg. CLI and `submit_job` do this automatically. | | `aborted: timeout` / `aborted: cancel` / `aborted: shutdown` / `aborted: lock-lost` | The worker's abort signal fired mid-execution. Child got SIGTERM, 5s grace, then SIGKILL. | Expected: timeout / user cancel / deploy restart / stall. Inspect `gbrain jobs get` to see which. | diff --git a/llms-full.txt b/llms-full.txt index 6d775013c..49191e4d2 100644 --- a/llms-full.txt +++ b/llms-full.txt @@ -280,7 +280,12 @@ strict behavior when unset. - `src/core/minions/spawn-helpers.ts` (v0.28.1) — pure `detectTini()` + `buildSpawnInvocation()` helpers consumed by both `supervisor.ts` and `autopilot.ts`. Resolves the DRY violation between the two spawn sites and makes the tini wrapping testable without `mock.module()` (rule R2 of `scripts/check-test-isolation.sh`). `detectTini()` calls `execFileSync('which', ['tini'])` with explicit `env: process.env` so Bun sees runtime PATH mutations (the env-snapshot bug fix). `buildSpawnInvocation(tiniPath, cmd, args)` returns `{cmd, args}` with tini prepended when present, or the bare invocation otherwise. Pinned by `test/spawn-helpers.test.ts` (5 cases) and `test/supervisor-tini.test.ts` (4 cases). - `src/core/minions/types.ts` — `MinionJobInput` + `MinionJobStatus` + handler context types. `MinionJobInput.max_stalled` (new in v0.14.1) is optional; omitted values let the schema DEFAULT (5) kick in, provided values are clamped to `[1, 100]`. - `src/core/minions/protected-names.ts` — side-effect-free constant module exporting `PROTECTED_JOB_NAMES` + `isProtectedJobName()`. Kept pure so queue core can import without loading handler modules. -- `src/core/minions/handlers/shell.ts` — `shell` job handler. Spawns `/bin/sh -c cmd` (absolute path, PATH-override-safe) or `argv[0] argv[1..]` (no shell). Env allowlist: `PATH, HOME, USER, LANG, TZ, NODE_ENV` + caller `env:` overrides. UTF-8-safe stdout/stderr tail via `string_decoder.StringDecoder`. Abort (either `ctx.signal` or `ctx.shutdownSignal`) fires SIGTERM → 5s grace → SIGKILL on child. Requires `GBRAIN_ALLOW_SHELL_JOBS=1` on worker (gated by `registerBuiltinHandlers`). +- `src/core/minions/handlers/shell.ts` — `shell` job handler. Spawns `/bin/sh -c cmd` (absolute path, PATH-override-safe) or `argv[0] argv[1..]` (no shell). Env allowlist: `PATH, HOME, USER, LANG, TZ, NODE_ENV` + caller `env:` overrides + (v0.36.5.0) `inherit:`-resolved keys. UTF-8-safe stdout/stderr tail via `string_decoder.StringDecoder`. Abort (either `ctx.signal` or `ctx.shutdownSignal`) fires SIGTERM → 5s grace → SIGKILL on child. Requires `GBRAIN_ALLOW_SHELL_JOBS=1` on worker (gated by `registerBuiltinHandlers`). **v0.36.5.0:** `ShellJobParams.inherit?: string[]` is a free-form list of snake_case config-key names. The worker resolves each via `loadConfig()` and injects the value under the derived env key (`database_url` → `GBRAIN_DATABASE_URL`; everything else uppercased). Names persist in `minion_jobs.data` (and the shell-audit JSONL); values never do. The canonical validator `validateShellJobParams` (sibling file `shell-validate.ts`) runs **pre-enqueue** in both submit surfaces — `gbrain jobs submit shell` (jobs.ts:271) AND `submit_job` op for `name='shell'` (operations.ts:2085). The handler-entry re-validation here is defense-in-depth. Closes the codex F-CDX-1 load-bearing bug class where validation in the handler ran AFTER `queue.add()` persisted the row. The validator does NOT police which config keys the agent inherits — same-uid trust model treats the agent as a peer of the worker. +- `src/core/minions/handlers/shell-inherit.ts` (v0.36.5.0, NEW) — three small helpers, no closed enum. `INHERIT_NAME_RE` (`/^[a-z][a-z0-9_]*$/`) is the snake_case shape guard used by the validator; rejects `__proto__`, leading-underscore, uppercase, and path-traversal shapes so audit logs stay readable and prototype-pollution lookups can't smuggle through. `deriveEnvKey(name)` maps config-key → child-env-key (`name.toUpperCase()` with one override: `database_url` → `GBRAIN_DATABASE_URL` because plain `DATABASE_URL` is ambiguous). `resolveInheritValue(cfg, name)` is the value lookup; uses `Object.hasOwn` to defeat prototype-pollution lookups, returns undefined for missing / non-string / empty-string values. An earlier closed-enum design (hardcoded `INHERITABLE` record with shadow-keys per name) was abandoned because the agent and worker share a uid — refusing to let the agent inherit arbitrary config keys defends nothing in that trust model. +- `src/core/minions/handlers/shell-validate.ts` (v0.36.5.0, NEW) — `validateShellJobParams(data, opts?)` shared pre-enqueue validator. Throws `UnrecoverableError` with paste-ready operator hints on every failure path. Three rules: (1) existing cmd/argv/cwd/env shape, (2) inherit array shape + snake_case regex per element (prototype-pollution defense), (3) fail-fast on missing config value with `gbrain config set ` hint. Also accepts optional `redact_secrets?: boolean` for output-side scrubbing. The validator deliberately does NOT police WHICH secrets the agent passes — single-uid trust model. Test seam: `opts.config` lets unit tests drive the validator hermetically without mocking the module. The defense-in-depth re-call at `shell.ts` handler entry catches pre-existing rows submitted before v0.36.5.0. +- `src/core/minions/handlers/shell-redact.ts` (v0.36.5.0, NEW) — opt-in output-side scrubbing for shell-job stdout/stderr. Pure `redactSecretsInText(text, secrets)` function: string-mode `replaceAll` so regex metacharacters in values stay literal. When the caller passes `redact_secrets: true` (or `--redact-secrets` on the CLI), the handler builds a Map of inherit-name → resolved-value and post-processes both tails before throw/return, so the persisted `result.stdout_tail` / `result.stderr_tail` / `error_text` carry `` instead of the value. Only `inherit:`-resolved values are scrubbed; caller-supplied `env:` values stay through (those are the agent's "fine in the row" channel). Heuristic — defeats the common-case `echo "$GBRAIN_DATABASE_URL"` echo, not adversarial encode-then-print. Default `false` for back-compat. +- `src/core/config.ts:ensureGitignore` (v0.36.5.0) — idempotent retroactive writer of `~/.gbrain/.gitignore` (single line `*`). Called from `saveConfig()` so every config-writing path lays it down, AND from `runPostUpgrade()` so existing users pick it up on next `gbrain upgrade`. Never clobbers a user-customized `.gitignore` (checks file exists + content non-empty before writing). Honest scope, named in CHANGELOG: blocks casual `git add ~/.gbrain` from inside an enclosing worktree, but does NOT cover already-tracked files, screenshots, backups (Time Machine / iCloud / Dropbox), or `git add -f`. The doctor check `home_dir_in_worktree` surfaces what `.gitignore` can't. +- `src/commands/doctor.ts:home_dir_in_worktree` (v0.36.5.0) — filesystem check walking up from `gbrainPath()` toward `$HOME` looking for either a `.git` directory (main repo) or `.git` file (linked worktree pointer; Conductor + git-worktrees topology). Walk terminates at `$HOME` so a `.git` above the user's home doesn't false-positive. Honors `GBRAIN_HOME` (gbrain appends `.gbrain` to the override). Warn (not fail) with worktree-root path + paste-ready fix pointing at `GBRAIN_HOME` override or moving the brain. - `src/core/minions/handlers/shell-audit.ts` — per-submission JSONL audit trail at `~/.gbrain/audit/shell-jobs-YYYY-Www.jsonl` (ISO-week rotation; override via `GBRAIN_AUDIT_DIR`). Best-effort: `mkdirSync(recursive)` + `appendFileSync`; failures logged to stderr, submission not blocked. Logs cmd (first 80 chars) or argv (JSON array). Never logs env values. - `src/core/minions/handlers/supervisor-audit.ts` — supervisor lifecycle JSONL audit at `~/.gbrain/audit/supervisor-YYYY-Www.jsonl` (ISO-week rotation; shares `computeIsoWeekName()` helper with `shell-audit.ts`). `writeSupervisorEvent(emission, supervisorPid)` appends one line per supervisor event (`started`, `worker_spawned`, `worker_exited`, `backoff`, `health_warn`, `health_error`, `max_crashes_exceeded`, `shutting_down`, `stopped`, `worker_spawn_failed`). `readSupervisorEvents({sinceMs})` is the readback path for `gbrain doctor`. **v0.35.5.0:** new exports `isCrashExit(event)`, `summarizeCrashes(events)`, `CrashSummary` type, and `CLEAN_EXIT_CAUSES` denylist (`'clean_exit' | 'graceful_shutdown'`). Single regression point — both `gbrain doctor` (Lane D supervisor check at `doctor.ts:1011-1043`) and `gbrain jobs supervisor status` (`jobs.ts:803-826`) import from here so the two CLI surfaces cannot drift. `isCrashExit` classifies a single `worker_exited` event against the denylist: `clean_exit` / `graceful_shutdown` are NON-crashes; everything else (`runtime_error`, `oom_or_external_kill`, `unknown`, AND any future `likely_cause` value added upstream in `child-worker-supervisor.ts`) is a crash. Pre-v0.34 audit lines lacking `likely_cause` fall back to `code !== 0`. `summarizeCrashes` returns `{total, by_cause: {runtime_error, oom_or_external_kill, unknown, legacy}, clean_exits}` so dashboards bind to named buckets — the `legacy` bucket catches BOTH pre-v0.34 fallback entries AND future unrecognized `likely_cause` values, fail-loud instead of silent underreport. Denylist-over-allowlist was a codex outside-voice catch during `/plan-eng-review` — the bug being fixed (read sites counting every `worker_exited` as a crash, inflating to 120+/day on healthy brains after v0.34.3.0 watchdog drains) was itself an allowlist-of-event-names. Pinned by `test/supervisor-audit.test.ts` (14 cases: 9-case `isCrashExit` branch matrix including denylist regression guard for unrecognized future causes + non-exit-event defensive case, 5-case `summarizeCrashes` aggregator including unrecognized-cause routing to legacy + null-code edge case) and 4 source-grep wiring assertions in `test/doctor.test.ts` guarding both surfaces against drift. - `src/core/minions/backpressure-audit.ts` (v0.19.1) — sibling of shell-audit.ts for `maxWaiting` coalesce events. JSONL at `~/.gbrain/audit/backpressure-YYYY-Www.jsonl`. Fires one line per coalesce with `(queue, name, waiting_count, max_waiting, returned_job_id, ts)`. Closes the silent-drop vector the v0.19.0 maxWaiting guard introduced. @@ -5988,4 +5993,75 @@ diff <(grep -A3 "Based on gbrain" ~//skills/brain-ops/SKILL.md) \ <(grep "v[0-9]" ~/gbrain/skills/migrations/ | tail -3) ``` + +## v0.36.5.0 — Free-form secret inheritance for shell jobs calling `gbrain` CLI + +**The change.** Shell-job params get a new `inherit:` field. Pass any +snake_case config-key name on it; the worker resolves the value from its +`loadConfig()` at child-spawn time and injects it into the child env. Names +land in the row; values never persist from `inherit:`. Validation runs +**pre-enqueue** in both submit paths (CLI + `submit_job` op), so a malformed +payload never lands in `minion_jobs.data`. + +**Why.** Pre-v0.36.5.0, agents that wanted to call `gbrain` from shell jobs +had to either write `database_url` to `~/.gbrain/config.json` plaintext or +pass `env: { GBRAIN_DATABASE_URL: "..." }` per-job. Both left plaintext +secrets somewhere — disk or DB row. `inherit:` keeps names in the row and +resolves values at spawn time. + +**What your agent can do.** `inherit:` is free-form. Pass any config-key: + +```jsonc +{ + "cmd": "gbrain sync --skip-failed && gbrain embed --stale", + "cwd": "/data/gbrain", + "inherit": ["database_url", "anthropic_api_key", "voyage_api_key"] +} +``` + +The env-key name in the child is derived by uppercasing the config-key: +`database_url` → `GBRAIN_DATABASE_URL`, `anthropic_api_key` → +`ANTHROPIC_API_KEY`, `voyage_api_key` → `VOYAGE_API_KEY`, etc. The validator +does NOT police which config keys you inherit — the agent is in the same +uid as the worker, so it's the agent's call. + +**You can still use `env:`.** v0.36.5.0 does not forbid `env:{ ANYTHING }`. +If you have a reason to put a value in the row plaintext (a non-secret +correlation token, or a secret you know is OK to persist), pass it via +`env:`. Prefer `inherit:` when you want the value out of the row. + +**Worker setup** (one-time, per host): + +- `gbrain config set database_url postgresql://...` (or any other key you + want available for inherit) +- OR put the key in `~/.gbrain/config.json` directly +- OR set `GBRAIN_DATABASE_URL` / `DATABASE_URL` / per-provider env on the + worker process + +If the worker can't resolve a requested name, the validator fail-fasts at +submit time with `gbrain config set ` hint. No more silent "No database +URL" failures in child stderr minutes after submission. + +**Also new.** A `gbrain doctor` check `home_dir_in_worktree` warns if +`~/.gbrain/` lives inside a git worktree. A retroactive `~/.gbrain/.gitignore` +(single line `*`) is now laid down by every `saveConfig()` call AND by +`gbrain post-upgrade`, so existing users get coverage without re-running +`gbrain init`. Honest scope: the `.gitignore` covers casual `git add` but does +NOT cover already-tracked files, screenshots, backups, or `git add -f`. + +**Strategy framing.** For agent-to-gbrain calls, the new canonical guide is +`docs/guides/agent-to-gbrain.md`. Two distinct surfaces: HTTP MCP via OAuth +for ops with MCP equivalents (`search`, `query`, `put_page`, etc.), and shell +job + `inherit:` for `localOnly` admin ops (`sync`, `embed`, `dream`, +`doctor`, etc.). Not a fallback hierarchy — pick by op. + +**Errors to handle** (your agent submits shell jobs; surface these clearly): + +| Error | What it means | Agent action | +|---|---|---| +| `shell: inherit must be an array of config-key names` | `inherit` wasn't an array. | Pass `"inherit": ["database_url", ...]`. | +| `shell: inherit entries must be non-empty strings` | Element was empty, non-string, or null. | Use snake_case config-key names. | +| `shell: inherit name "" must match [a-z][a-z0-9_]*` | Name failed snake_case regex (uppercase, leading underscore, etc.). | Use the config-key verbatim — `database_url`, not `DATABASE_URL`. | +| `shell: inherit requested "" but worker has no configured` | Worker can't resolve the name from its `loadConfig()`. | Run `gbrain config set ` on the worker host. | + --- diff --git a/package.json b/package.json index 66b71b205..d6028cc70 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "gbrain", - "version": "0.36.4.0", + "version": "0.36.5.0", "description": "Postgres-native personal knowledge brain with hybrid RAG search", "type": "module", "main": "src/core/index.ts", diff --git a/skills/migrations/v0.36.5.0.md b/skills/migrations/v0.36.5.0.md new file mode 100644 index 000000000..faf1a6cb0 --- /dev/null +++ b/skills/migrations/v0.36.5.0.md @@ -0,0 +1,115 @@ +--- +version: v0.36.5.0 +date: 2026-05-18 +feature_pitch: > + Shell jobs grow `inherit: [...]` — agent passes any snake_case config-key + names; worker resolves values from its own `loadConfig()` at child-spawn. + Names persist to the row; values resolve fresh. Validated pre-enqueue. +--- + +# v0.36.5.0 — Free-form secret inheritance for shell jobs calling `gbrain` CLI + +## What changed + +- **Shell-job pre-enqueue validator** (`src/core/minions/handlers/shell-validate.ts`, + NEW). Called from BOTH submit surfaces BEFORE `MinionQueue.add()`: + `gbrain jobs submit shell` (CLI) and `submit_job` MCP op for `name='shell'`. + Validates cmd/argv/cwd/env shape, `inherit` array of snake_case names, and + fail-fasts if the worker can't resolve a requested name. No rejected + payload ever lands in `minion_jobs.data`. +- **`inherit: [...]` ShellJobParams field — free-form.** Pass any snake_case + config-key name (`database_url`, `anthropic_api_key`, `voyage_api_key`, + `groq_api_key`, `zeroentropy_api_key`, `my_custom_field`, etc.). The + validator does NOT police which keys you choose — same-uid trust model + treats the agent as a peer. +- **Env-key derivation**: name uppercased by default. One override: + `database_url` → `GBRAIN_DATABASE_URL` (plain `DATABASE_URL` is ambiguous + in most Postgres-app contexts). +- **Prototype-pollution defense**: snake_case regex blocks `__proto__` / + `_leading` / uppercase. Value-resolver uses `Object.hasOwn`. +- **Defense-in-depth re-validation** in the shell handler at job-pickup + catches pre-existing rows AND any future submit path that forgets the + pre-enqueue call. +- **`gbrain doctor home_dir_in_worktree` check** warns when `~/.gbrain/` + lives inside a git worktree. Handles `.git` as directory or file. +- **`~/.gbrain/.gitignore` retroactive** via `ensureGitignore()` called from + `saveConfig()` AND `gbrain post-upgrade`. Idempotent, never clobbers. +- **Output-side redaction (opt-in)**: set `redact_secrets: true` on the job + params (or pass `--redact-secrets` on the CLI) and the worker scrubs every + occurrence of resolved `inherit:` values from `stdout_tail` / + `stderr_tail` / `error_text` before persistence. Replacement token: + ``. Literal-string replace — defeats accidental + `echo "$GBRAIN_DATABASE_URL"`, not adversarial encode-then-print. Default + false (back-compat). +- New canonical guide: `docs/guides/agent-to-gbrain.md`. Two-domain framing + for downstream agent authors: MCP ops via thin-client OAuth vs `localOnly` + admin ops via shell-job `inherit:`. + +## What downstream agents can do + +The agent picks which secrets to inherit, per job, by name: + +```jsonc +{ + "cmd": "gbrain sync --skip-failed && gbrain embed --stale", + "cwd": "/data/gbrain", + "inherit": ["database_url", "anthropic_api_key", "voyage_api_key"] +} +``` + +Worker resolves each name from `loadConfig()` and injects into the child env +under the derived key (`database_url` → `GBRAIN_DATABASE_URL`, +`anthropic_api_key` → `ANTHROPIC_API_KEY`, etc.). Names land in +`minion_jobs.data` and the shell-audit JSONL; values resolve fresh on every +spawn and don't persist from `inherit:` itself. + +If the worker can't resolve a name, the validator fail-fasts at submit time +with a paste-ready `gbrain config set ` hint. + +You can still use `env:` for non-secret values or for secrets you've decided +are OK to persist in the row (e.g. correlation tokens). v0.36.5.0 doesn't +forbid that — the validator trusts the agent. + +## Verify + +```bash +# 1. The new pattern works on your worker: +gbrain jobs submit shell --params \ + '{"cmd":"gbrain stats","cwd":"/tmp","inherit":["database_url"]}' --follow +# Expect: page count, exit 0. + +# 2. The doctor surfaces worktree risk if it exists: +gbrain doctor --json | grep -A1 home_dir_in_worktree + +# 3. The retroactive gitignore landed: +test -f ~/.gbrain/.gitignore && cat ~/.gbrain/.gitignore +# Expect: file exists, contents = "*\n" + +# 4. The audit-log records names not values: +tail -1 ~/.gbrain/audit/shell-jobs-*.jsonl | grep -o '"inherit":\[[^]]*\]' +# Expect: ["database_url"] (no URL value) +``` + +## Why this exists + +PR #1137 documented two workarounds for the env-stripping behavior of shell +jobs: write `database_url` plaintext to `~/.gbrain/config.json`, or pass +`env: { GBRAIN_DATABASE_URL: ... }` per-job. Both work. Both leave plaintext +secrets either on disk or in `minion_jobs.data` rows that travel with brain +DB dumps and shared brains. + +The `/cso` audit cut an earlier proposal (encrypted vault + Unix-socket +broker + SO_PEERCRED + per-call tokens) as theater for a single-uid topology. +Codex's pre-landing review then caught the load-bearing bug in the rewritten +"minimum scope" plan: validation in the handler runs AFTER `queue.add()`, +so the headline "input-secrets never persist" was technically false. Fixed +pre-implementation by lifting the validator to a shared pre-enqueue module +called from both submit surfaces. + +An interim draft used a closed `INHERITABLE` enum (hardcoded list of +allowed secret names with explicit shadow-key sets and inline-cmd regex +scans). Codex flagged bypasses; the deeper question surfaced: what does the +closed enum actually defend on a single-uid topology? Same-uid trust = same +trust domain; refusing to let the agent inherit `voyage_api_key` because +it's not on a hand-curated list is paternalism. The shipped design opens +`inherit:` to any snake_case config-key. The agent decides. diff --git a/src/commands/doctor.ts b/src/commands/doctor.ts index 0f84c4f28..555f2d877 100644 --- a/src/commands/doctor.ts +++ b/src/commands/doctor.ts @@ -9,6 +9,7 @@ import { compareVersions } from './migrations/index.ts'; import { createProgress, startHeartbeat, type ProgressReporter } from '../core/progress.ts'; import { getCliOptions, cliOptsToProgressOptions } from '../core/cli-options.ts'; import type { DbUrlSource } from '../core/config.ts'; +import { gbrainPath } from '../core/config.ts'; import { dirname, isAbsolute, join, resolve as resolvePath } from 'path'; import { fileURLToPath } from 'url'; import { existsSync, readFileSync, readdirSync, statSync } from 'fs'; @@ -1634,6 +1635,69 @@ export async function runDoctor(engine: BrainEngine | null, args: string[], dbSo // Best-effort; audit-log read failure shouldn't stop doctor. } + // 3e. home_dir_in_worktree (v0.35.8.0). Walks up from `gbrainPath()` + // looking for a `.git` directory OR file. If found, warns: `~/.gbrain/` + // lives inside a git worktree, so an accidental `git add` from the + // worktree root could stage the brain. Pairs with the retroactive + // `~/.gbrain/.gitignore` (single-line `*`) laid down by saveConfig + + // post-upgrade. Honest scope: the .gitignore covers casual `git add` + // but NOT already-tracked files, screenshots, backups, or `git add -f`. + // + // Walk termination: stops at $HOME (don't keep walking into / on a user + // who set GBRAIN_HOME=/tmp/something). Handles `.git` as both a directory + // (main repo) and a file (linked worktree pointing at parent's worktrees/). + // Honors GBRAIN_HOME via gbrainPath(). + try { + const gbrainHome = gbrainPath(); + const home = process.env.HOME || ''; + let worktreeRoot: string | null = null; + if (gbrainHome && home && gbrainHome.startsWith(home + '/')) { + // Walk up from gbrainHome's parent toward $HOME, stopping at $HOME. + // We don't check gbrainHome itself: a `.git` directly inside ~/.gbrain + // isn't a containing-worktree, it would be a brain repo cloned there. + let cur = dirname(gbrainHome); + while (cur && cur.length >= home.length) { + const gitPath = join(cur, '.git'); + try { + const st = statSync(gitPath); + // Either a directory (main repo) or a file (linked worktree pointer). + if (st.isDirectory() || st.isFile()) { + worktreeRoot = cur; + break; + } + } catch { + // No .git at this level; continue. + } + if (cur === home) break; + const parent = dirname(cur); + if (parent === cur) break; + cur = parent; + } + } + if (worktreeRoot) { + const homeEnvHint = process.env.GBRAIN_HOME + ? `# Or move \`~/.gbrain\` outside the worktree by setting GBRAIN_HOME elsewhere.` + : `# Fix: \`export GBRAIN_HOME=/some/path/outside/the/worktree\` (gbrain appends \`.gbrain\`).`; + checks.push({ + name: 'home_dir_in_worktree', + status: 'warn', + message: + `~/.gbrain lives inside git worktree at ${worktreeRoot}. ` + + `Config + brain DB could be committed by accident. ` + + `A retroactive ~/.gbrain/.gitignore blocks casual \`git add\`, but does NOT cover ` + + `already-tracked files, screenshots, backups, or \`git add -f\`. ${homeEnvHint}`, + }); + } else { + checks.push({ + name: 'home_dir_in_worktree', + status: 'ok', + message: 'gbrain home is outside any enclosing git worktree.', + }); + } + } catch { + // Best-effort filesystem-hygiene check; never block doctor. + } + // 3b-multi-source. Multi-source drift (v0.31.8 — D8 + D17 + OV12 + OV13). // Pre-v0.30.3 putPage misrouted multi-source writes to (default, slug). // For each non-default source with local_path set, walk the FS and surface diff --git a/src/commands/jobs.ts b/src/commands/jobs.ts index e338078a4..6793836e3 100644 --- a/src/commands/jobs.ts +++ b/src/commands/jobs.ts @@ -126,6 +126,8 @@ USAGE [--backoff-type fixed|exponential] [--backoff-delay Nms] [--backoff-jitter 0..1] [--timeout-ms Nms] [--idempotency-key K] [--queue Q] [--dry-run] + [--redact-secrets] (shell only; scrubs inherit + values from stdout/stderr) gbrain jobs list [--status S] [--queue Q] [--limit N] gbrain jobs get gbrain jobs cancel @@ -237,6 +239,12 @@ HANDLER TYPES (built in) const queueName = parseFlag(args, '--queue') ?? 'default'; const dryRun = hasFlag(args, '--dry-run'); const follow = hasFlag(args, '--follow'); + // v0.36.5.0: --redact-secrets is a CLI convenience that merges + // `redact_secrets: true` into the params before validation. Equivalent + // to passing it in --params JSON; flag form is faster to type. + if (hasFlag(args, '--redact-secrets') && name.trim() === 'shell') { + data.redact_secrets = true; + } if (dryRun) { console.log(`[DRY RUN] Would submit job:`); @@ -268,6 +276,23 @@ HANDLER TYPES (built in) // future protected name forces explicit opt-in at the call site. const { isProtectedJobName } = await import('../core/minions/protected-names.ts'); const trusted = isProtectedJobName(name) ? { allowProtectedSubmit: true } : undefined; + + // v0.35.8.0: pre-enqueue shell-job validation. Validates `inherit:` + // closed enum, rejects secret env-keys, fail-fasts on missing config. + // Throws UnrecoverableError BEFORE `queue.add` so a bad payload never + // lands in `minion_jobs.data`. Defense-in-depth re-validation happens + // in the worker handler. See: src/core/minions/handlers/shell-validate.ts + if (name.trim() === 'shell') { + try { + const { validateShellJobParams } = await import('../core/minions/handlers/shell-validate.ts'); + validateShellJobParams(data); + } catch (e) { + const msg = e instanceof Error ? e.message : String(e); + console.error(`Error: ${msg}`); + process.exit(1); + } + } + const job = await queue.add(name, data, { priority, delay: delay > 0 ? delay : undefined, @@ -286,6 +311,9 @@ HANDLER TYPES (built in) try { const { logShellSubmission } = await import('../core/minions/handlers/shell-audit.ts'); if (name.trim() === 'shell') { + const inheritNames = Array.isArray(data.inherit) + ? (data.inherit as unknown[]).filter((s): s is string => typeof s === 'string') + : undefined; logShellSubmission({ caller: 'cli', remote: false, @@ -295,6 +323,7 @@ HANDLER TYPES (built in) argv_display: Array.isArray(data.argv) ? (data.argv as unknown[]).filter((a): a is string => typeof a === 'string').map((a) => a.slice(0, 80)) : undefined, + inherit: inheritNames && inheritNames.length > 0 ? inheritNames : undefined, }); } } catch { /* audit failures never block submission */ } diff --git a/src/commands/upgrade.ts b/src/commands/upgrade.ts index 2e1d4f018..75ba2952a 100644 --- a/src/commands/upgrade.ts +++ b/src/commands/upgrade.ts @@ -241,6 +241,16 @@ export async function runPostUpgrade(args: string[] = []): Promise { console.log('Idempotent — safe to re-run any time.'); return; } + + // v0.35.8.0: lay down ~/.gbrain/.gitignore retroactively. Existing users + // never re-run `gbrain init`, so init-only coverage misses them entirely + // (codex F-CDX-8). Idempotent + non-clobbering — safe to run every upgrade. + try { + const { ensureGitignore } = await import('../core/config.ts'); + ensureGitignore(); + } catch { + // Best-effort hygiene; never block upgrade. + } // Cosmetic: print feature pitches for migrations newer than the prior binary. try { const statePath = join(process.env.HOME || '', '.gbrain', 'upgrade-state.json'); diff --git a/src/core/config.ts b/src/core/config.ts index d78998cbd..c54777390 100644 --- a/src/core/config.ts +++ b/src/core/config.ts @@ -328,6 +328,60 @@ export function saveConfig(config: GBrainConfig): void { } catch { // chmod may fail on some platforms } + // v0.35.8.0: ensure the per-home `.gitignore` exists on every config-write + // path. Cheap, idempotent, doesn't clobber user edits. Catches the case + // where `~/.gbrain/` lives inside a git worktree (Conductor + gstack + // workspaces hit this) so `git add` doesn't accidentally stage the brain. + // The doctor check `home_dir_in_worktree` surfaces vectors this can't + // close (already-tracked files, screenshots, backups, `git add -f`). + ensureGitignore(); +} + +/** + * Idempotently lay down `~/.gbrain/.gitignore` containing the single line `*`. + * Honors GBRAIN_HOME via `configDir()`. Best-effort: errors are logged to + * stderr and never block the caller. Never clobbers a `.gitignore` whose + * content the user has customized. + * + * Called from: + * - `saveConfig()` so any config-writing path lays it down. + * - `gbrain post-upgrade` so existing users get it on next upgrade. + * + * What this DOES cover: a casual `git add ~/.gbrain` from inside an enclosing + * worktree — the directory-local `.gitignore` blocks everything below it. + * + * What this does NOT cover (the CHANGELOG names these honestly): + * - Files already tracked before the .gitignore landed (no remediation here). + * - Screenshots, sync folders (Dropbox/iCloud), Time Machine backups. + * - `git add -f ~/.gbrain` (deliberate force-add bypasses .gitignore). + * - Out-of-band copy operations (rsync, cp -r, scp). + * + * The doctor check `home_dir_in_worktree` surfaces these vectors at audit + * time so the user can act on them. + */ +export function ensureGitignore(): void { + try { + const dir = configDir(); + const file = join(dir, '.gitignore'); + mkdirSync(dir, { recursive: true }); + if (existsSync(file)) { + // Don't clobber user customization. Only write when the file is missing + // OR when its content is empty (zero-byte placeholder). + try { + const existing = readFileSync(file, 'utf-8'); + if (existing.trim().length > 0) return; + } catch { + // Read failed but file exists — leave it alone to be safe. + return; + } + } + writeFileSync(file, '*\n', { mode: 0o600 }); + try { chmodSync(file, 0o600); } catch { /* platform-specific */ } + } catch (e) { + // Best-effort: log to stderr, never block the caller. + const msg = e instanceof Error ? e.message : String(e); + process.stderr.write(`[gbrain] ensureGitignore failed (${msg}); continuing\n`); + } } export function toEngineConfig(config: GBrainConfig): EngineConfig { diff --git a/src/core/minions/handlers/shell-audit.ts b/src/core/minions/handlers/shell-audit.ts index 44586c18b..06bf35c48 100644 --- a/src/core/minions/handlers/shell-audit.ts +++ b/src/core/minions/handlers/shell-audit.ts @@ -25,6 +25,9 @@ export interface ShellAuditEvent { cwd: string; cmd_display?: string; // first 80 chars of cmd; may contain inline tokens argv_display?: string[]; // each arg truncated individually to preserve separation + /** Names of inheritable secrets requested via `inherit:` (v0.35.8.0). + * Names only — values never appear here. */ + inherit?: string[]; } /** Compute `shell-jobs-YYYY-Www.jsonl` using ISO-8601 week numbering. diff --git a/src/core/minions/handlers/shell-inherit.ts b/src/core/minions/handlers/shell-inherit.ts new file mode 100644 index 000000000..13c652d06 --- /dev/null +++ b/src/core/minions/handlers/shell-inherit.ts @@ -0,0 +1,69 @@ +/** + * Free-form secret inheritance for `shell` job `inherit:` field (v0.36.5.0). + * + * Design choice: the agent spawning a minion is in the same trust domain as the + * worker (same uid on the same machine). When the agent asks for `inherit:[X]`, + * it KNOWS what X means and what it's for. The validator's job is to make the + * mechanism work, not to second-guess which config fields are "OK to inherit." + * + * What's guaranteed: + * - The agent passes config-key NAMES, not values, in `inherit:`. + * - Names persist to `minion_jobs.data` and the shell-audit JSONL. + * - Values resolve at child-spawn time from the worker's `loadConfig()`. + * - If a requested name has no value on the worker, the validator fail-fasts. + * + * What's NOT guaranteed: + * - No closed enum of "approved" secrets — any config key works. + * - No env-shadow rejection — caller can also use `env:` for the same name + * if they want; that's their call. Names land in the row plaintext via env: + * if you do that, so prefer `inherit:` for hygiene. + * - No output-side scrub — if your script prints the value, it persists in + * `result.stdout_tail`. Script author's responsibility. + */ +import type { GBrainConfig } from '../../config.ts'; + +/** + * Snake-case config-key shape. Pinned by regex to: + * - prevent prototype-pollution shapes (`__proto__`, `constructor`) + * - prevent path-traversal-looking names in audit logs + * - match the `GBrainConfig` field-name convention + */ +export const INHERIT_NAME_RE = /^[a-z][a-z0-9_]*$/; + +/** + * Optional env-key overrides. For most config keys we derive `ENV_KEY` by + * uppercasing the name (`anthropic_api_key` → `ANTHROPIC_API_KEY`). For a few + * gbrain-flavored names we use a gbrain-prefixed form so they don't collide + * with provider conventions: `database_url` becomes `GBRAIN_DATABASE_URL` + * because plain `DATABASE_URL` is ambiguous (every Postgres app uses it). + */ +const ENV_KEY_OVERRIDES: Readonly> = Object.freeze({ + database_url: 'GBRAIN_DATABASE_URL', +}); + +/** + * Derive the child-env key name for a given config key. Falls back to + * `name.toUpperCase()` when no override is set. Example: + * deriveEnvKey('database_url') === 'GBRAIN_DATABASE_URL' + * deriveEnvKey('anthropic_api_key') === 'ANTHROPIC_API_KEY' + * deriveEnvKey('voyage_api_key') === 'VOYAGE_API_KEY' + */ +export function deriveEnvKey(name: string): string { + return ENV_KEY_OVERRIDES[name] ?? name.toUpperCase(); +} + +/** + * Resolve a config-key name to its string value on `cfg`. Returns undefined + * when the field is unset, non-string, or empty. Uses `Object.hasOwn` to + * defeat prototype-pollution lookups (`__proto__`, `constructor`, etc.). + */ +export function resolveInheritValue( + cfg: GBrainConfig | null, + name: string, +): string | undefined { + if (cfg === null || typeof cfg !== 'object') return undefined; + if (!Object.hasOwn(cfg, name)) return undefined; + const value = (cfg as unknown as Record)[name]; + if (typeof value !== 'string' || value.length === 0) return undefined; + return value; +} diff --git a/src/core/minions/handlers/shell-redact.ts b/src/core/minions/handlers/shell-redact.ts new file mode 100644 index 000000000..9ca87b802 --- /dev/null +++ b/src/core/minions/handlers/shell-redact.ts @@ -0,0 +1,51 @@ +/** + * Output-side redaction for shell-job stdout/stderr (v0.36.5.0). + * + * Honest defense for the documented limitation: `inherit:` keeps values out + * of the JOB ROW INPUT fields (`data.cmd`, `data.argv`, `data.env`), but if + * the script prints the value to stdout or stderr, it lands in + * `result.stdout_tail` / `result.stderr_tail` / `error_text` and from there + * into `minion_jobs.result` plaintext. + * + * This module scrubs resolved inherit values out of output text before the + * shell handler returns or throws. Opt-in via `redact_secrets: true` on the + * job params (or `--redact-secrets` on the CLI). + * + * What gets redacted: only the resolved values of names listed in `inherit:`. + * The agent identified those by name as "secret"-class. Caller-supplied + * `env:` values are NOT redacted — those are the agent's chosen "I'm fine + * with this in the row" channel. + * + * Heuristic, not perfect: a determined script can encode-then-print + * (base64, hex-split, character-by-character) and bypass the literal-string + * replace. The agent + script are in the same trust domain, so this layer + * defends against accidental echo (the common case), not adversarial print. + * + * Replacement token: `` (the inherit name, human-readable) + * so the operator inspecting the row knows WHICH secret was scrubbed. + */ + +/** + * Scrub every resolved inherit value out of the given text. Returns the + * scrubbed text; original is not mutated. + * + * @param text The stdout/stderr/error text to scrub. + * @param secrets Map of inherit-name → resolved value. Empty values are skipped. + * Order doesn't matter (each value is independently replaced). + */ +export function redactSecretsInText( + text: string, + secrets: ReadonlyMap, +): string { + if (text.length === 0 || secrets.size === 0) return text; + let result = text; + for (const [name, value] of secrets) { + if (value.length === 0) continue; + // String-mode replaceAll: no regex interpretation, so special chars in + // the value (?, *, +, parens, etc.) replace as literal substrings. This + // matches the threat model: the value is whatever the worker had in + // config; we want the exact byte sequence scrubbed. + result = result.replaceAll(value, ``); + } + return result; +} diff --git a/src/core/minions/handlers/shell-validate.ts b/src/core/minions/handlers/shell-validate.ts new file mode 100644 index 000000000..a872edba9 --- /dev/null +++ b/src/core/minions/handlers/shell-validate.ts @@ -0,0 +1,181 @@ +/** + * Pre-enqueue validator for `shell` job params (v0.36.5.0). + * + * Called from BOTH submit surfaces BEFORE `MinionQueue.add()`: + * - `src/commands/jobs.ts` — `gbrain jobs submit shell` CLI handler + * - `src/core/operations.ts` — `submit_job` op handler for name='shell' + * + * Correctness property: a rejected payload NEVER lands in `minion_jobs.data`. + * Pre-v0.36.5.0 validation ran in the worker handler AFTER `queue.add()` had + * already persisted the row; this module exists to close that window. + * + * Trust model (read this once): the agent that submits the job is in the same + * uid as the worker that runs it. We do NOT police WHICH secrets the agent + * chooses to pass — that's the agent's call. We only validate: + * + * 1. Shape — `cmd` XOR `argv`, `cwd` absolute, `env` is string→string + * 2. `inherit` is an array of snake_case names (prevents prototype-pollution + * lookups like `__proto__` and keeps audit logs readable) + * 3. Every `inherit` name resolves to a non-empty string on the worker's + * `loadConfig()` (UX guardrail — fail at submit time, not minutes later + * in opaque child-process stderr) + * + * What we deliberately do NOT do: + * - No closed-enum allowlist of "approved" secrets. Agent decides. + * - No shadow-rejection (caller can also set the same key in `env:` if they + * want — that puts the value in the row plaintext, which is their call). + * - No inline-`cmd: "X=value ..."` scan. Same reasoning. + */ + +import * as path from 'node:path'; +import { UnrecoverableError } from '../types.ts'; +import { + INHERIT_NAME_RE, + resolveInheritValue, +} from './shell-inherit.ts'; +import { loadConfig, type GBrainConfig } from '../../config.ts'; + +/** Validated, narrowed shell-job params. */ +export interface ValidatedShellJobParams { + cmd?: string; + argv?: string[]; + cwd: string; + env?: Record; + inherit?: string[]; + redact_secrets?: boolean; +} + +export interface ValidateShellJobOpts { + /** + * Loaded gbrain config used to verify every `inherit` name resolves to a + * value. Pass `null` to fail-fast on any inherit request. Defaults to + * calling `loadConfig()` when undefined — the typical CLI / op-handler path. + * + * Test seam: pass `{ config }` explicitly to drive the validator with a + * stubbed config in hermetic unit tests instead of mocking the module. + */ + config?: GBrainConfig | null; +} + +/** + * Validate raw shell-job submission `data`. Returns the narrowed shape on + * success; throws `UnrecoverableError` with an operator-facing message on + * every failure path. Validation errors are never retry-worthy. + */ +export function validateShellJobParams( + data: Record, + opts: ValidateShellJobOpts = {}, +): ValidatedShellJobParams { + const hasCmd = typeof data.cmd === 'string' && (data.cmd as string).length > 0; + const hasArgv = Array.isArray(data.argv) && (data.argv as unknown[]).length > 0; + + if (hasCmd && hasArgv) { + throw new UnrecoverableError( + 'shell: specify exactly one of cmd or argv (see: docs/guides/minions-shell-jobs.md#errors)', + ); + } + if (!hasCmd && !hasArgv) { + throw new UnrecoverableError( + 'shell: specify exactly one of cmd or argv (see: docs/guides/minions-shell-jobs.md#errors)', + ); + } + if (hasArgv) { + const argvOk = (data.argv as unknown[]).every((a) => typeof a === 'string'); + if (!argvOk) { + throw new UnrecoverableError( + 'shell: argv must be an array of strings (see: docs/guides/minions-shell-jobs.md#errors)', + ); + } + } + if (typeof data.cwd !== 'string' || (data.cwd as string).length === 0) { + throw new UnrecoverableError( + 'shell: cwd is required and must be an absolute path (see: docs/guides/minions-shell-jobs.md#errors)', + ); + } + if (!path.isAbsolute(data.cwd as string)) { + throw new UnrecoverableError( + 'shell: cwd is required and must be an absolute path (see: docs/guides/minions-shell-jobs.md#errors)', + ); + } + if (data.env !== undefined) { + if (typeof data.env !== 'object' || data.env === null || Array.isArray(data.env)) { + throw new UnrecoverableError( + 'shell: env must be an object of string values (see: docs/guides/minions-shell-jobs.md#errors)', + ); + } + for (const v of Object.values(data.env as Record)) { + if (typeof v !== 'string') { + throw new UnrecoverableError( + 'shell: env values must all be strings (see: docs/guides/minions-shell-jobs.md#errors)', + ); + } + } + } + + // ---- `inherit` shape validation ---- + // Free-form list of config-key names. The closed enum of v0.35-RC was + // overcautious for the single-uid trust model — the agent knows what it + // needs to pass to the child. We only enforce shape (snake_case) so audit + // logs stay readable and prototype-pollution shapes (`__proto__`) can't + // sneak through. + let inherit: string[] | undefined; + if (data.inherit !== undefined) { + if (!Array.isArray(data.inherit)) { + throw new UnrecoverableError( + 'shell: inherit must be an array of config-key names ' + + '(see: docs/guides/minions-shell-jobs.md#secrets)', + ); + } + const items = data.inherit as unknown[]; + for (const item of items) { + if (typeof item !== 'string' || item.length === 0) { + throw new UnrecoverableError( + 'shell: inherit entries must be non-empty strings ' + + '(see: docs/guides/minions-shell-jobs.md#secrets)', + ); + } + if (!INHERIT_NAME_RE.test(item)) { + throw new UnrecoverableError( + `shell: inherit name "${item}" must match [a-z][a-z0-9_]* ` + + '(snake_case config-key shape; see: docs/guides/minions-shell-jobs.md#secrets)', + ); + } + } + inherit = items as string[]; + } + + // ---- Fail-fast on missing config value ---- + // UX guardrail: if the worker can't resolve a requested name, fail at + // submit-time with a paste-ready fix. Without this, the child gets an + // unset env var and fails minutes later with a less precise error. + if (inherit !== undefined && inherit.length > 0) { + const cfg = opts.config !== undefined ? opts.config : loadConfig(); + for (const name of inherit) { + const value = resolveInheritValue(cfg, name); + if (value === undefined) { + throw new UnrecoverableError( + `shell: inherit requested "${name}" but worker has no ${name} configured. ` + + `Fix: \`gbrain config set ${name} \` or set the value in the worker's config file. ` + + '(see: docs/guides/minions-shell-jobs.md#secrets)', + ); + } + } + } + + // ---- `redact_secrets` shape check ---- + if (data.redact_secrets !== undefined && typeof data.redact_secrets !== 'boolean') { + throw new UnrecoverableError( + 'shell: redact_secrets must be a boolean if set ' + + '(see: docs/guides/minions-shell-jobs.md#secrets)', + ); + } + + return { + cmd: hasCmd ? (data.cmd as string) : undefined, + argv: hasArgv ? (data.argv as string[]) : undefined, + cwd: data.cwd as string, + env: data.env as Record | undefined, + inherit, + redact_secrets: data.redact_secrets as boolean | undefined, + }; +} diff --git a/src/core/minions/handlers/shell.ts b/src/core/minions/handlers/shell.ts index 9ffd5e83c..9079ecc03 100644 --- a/src/core/minions/handlers/shell.ts +++ b/src/core/minions/handlers/shell.ts @@ -28,9 +28,12 @@ import { spawn, type ChildProcess } from 'node:child_process'; import { StringDecoder } from 'node:string_decoder'; -import * as path from 'node:path'; import type { MinionJobContext } from '../types.ts'; import { UnrecoverableError } from '../types.ts'; +import { deriveEnvKey, resolveInheritValue } from './shell-inherit.ts'; +import { validateShellJobParams } from './shell-validate.ts'; +import { redactSecretsInText } from './shell-redact.ts'; +import { loadConfig } from '../../config.ts'; /** Environment variables passed through to shell children by default. Callers * that need additional keys (e.g. a specific API token for a cron) must name @@ -54,8 +57,31 @@ export interface ShellJobParams { /** Working directory. REQUIRED, must be an absolute path. The operator chooses * this; it's the trust boundary for what files the script can read/write. */ cwd: string; - /** Additional env vars to pass to the child. Merged on top of SHELL_ENV_ALLOWLIST. */ + /** Additional env vars to pass to the child. Merged on top of SHELL_ENV_ALLOWLIST. + * Cannot contain secret env keys (GBRAIN_DATABASE_URL, DATABASE_URL, etc.) — + * use `inherit:` instead. Enforced pre-enqueue by `validateShellJobParams`. */ env?: Record; + /** + * Free-form list of config-key names to inherit from the worker's + * `loadConfig()` into the child env (v0.36.5.0). Each name must match + * `[a-z][a-z0-9_]*`; the env key is derived via `deriveEnvKey` (e.g. + * `database_url` → `GBRAIN_DATABASE_URL`, `anthropic_api_key` → + * `ANTHROPIC_API_KEY`). Names persist to `minion_jobs.data` + the + * shell-audit JSONL; values resolve at child-spawn time and never persist + * anywhere from `inherit:` itself. Pre-enqueue validation fail-fasts when + * the worker can't resolve a requested name. See: + * `src/core/minions/handlers/shell-inherit.ts` and `shell-validate.ts`. + */ + inherit?: string[]; + /** + * Opt-in (v0.36.5.0): scrub resolved `inherit:` values from + * `stdout_tail` / `stderr_tail` / `error_text` before persistence. + * Replacement token: ``. Only `inherit:`-resolved values + * are scrubbed; caller-supplied `env:` values are not (those are the + * agent's "fine in the row" channel by design). Heuristic — defeats the + * common-case echo, not adversarial encode-then-print. + */ + redact_secrets?: boolean; } export interface ShellJobResult { @@ -66,72 +92,43 @@ export interface ShellJobResult { pid: number; } -/** Validate and narrow `job.data` to ShellJobParams. Throws UnrecoverableError - * for misshapen input — validation failures are not retry-worthy. */ -function validateParams(data: Record): ShellJobParams { - const hasCmd = typeof data.cmd === 'string' && data.cmd.length > 0; - const hasArgv = Array.isArray(data.argv) && data.argv.length > 0; - - if (hasCmd && hasArgv) { - throw new UnrecoverableError( - 'shell: specify exactly one of cmd or argv (see: docs/guides/minions-shell-jobs.md#errors)', - ); - } - if (!hasCmd && !hasArgv) { - throw new UnrecoverableError( - 'shell: specify exactly one of cmd or argv (see: docs/guides/minions-shell-jobs.md#errors)', - ); - } - if (hasArgv) { - const argvOk = (data.argv as unknown[]).every((a) => typeof a === 'string'); - if (!argvOk) { - throw new UnrecoverableError( - 'shell: argv must be an array of strings (see: docs/guides/minions-shell-jobs.md#errors)', - ); - } - } - if (typeof data.cwd !== 'string' || data.cwd.length === 0) { - throw new UnrecoverableError( - 'shell: cwd is required and must be an absolute path (see: docs/guides/minions-shell-jobs.md#errors)', - ); - } - if (!path.isAbsolute(data.cwd)) { - throw new UnrecoverableError( - 'shell: cwd is required and must be an absolute path (see: docs/guides/minions-shell-jobs.md#errors)', - ); - } - if (data.env !== undefined) { - if (typeof data.env !== 'object' || data.env === null || Array.isArray(data.env)) { - throw new UnrecoverableError( - 'shell: env must be an object of string values (see: docs/guides/minions-shell-jobs.md#errors)', - ); - } - for (const v of Object.values(data.env as Record)) { - if (typeof v !== 'string') { - throw new UnrecoverableError( - 'shell: env values must all be strings (see: docs/guides/minions-shell-jobs.md#errors)', - ); - } - } - } - - return { - cmd: hasCmd ? (data.cmd as string) : undefined, - argv: hasArgv ? (data.argv as string[]) : undefined, - cwd: data.cwd, - env: (data.env as Record | undefined), - }; -} - -/** Build the child process env: SHELL_ENV_ALLOWLIST picked from process.env, - * overlaid with caller-supplied `job.data.env`. Prevents accidental leak of - * OPENAI_API_KEY / DATABASE_URL / etc. into user-authored scripts. */ -function buildChildEnv(override: Record | undefined): Record { +/** Build the child process env. Layering (low to high precedence): + * 1. `SHELL_ENV_ALLOWLIST` picked from `process.env` (worker process env). + * 2. Resolved `inherit:` values — each config-key name is looked up on the + * worker's `loadConfig()`. The child-env key is derived via + * `deriveEnvKey` (e.g. `database_url` → `GBRAIN_DATABASE_URL`). + * Pre-enqueue validation fail-fasted on missing names, so we reach this + * branch only when every name resolves. + * 3. Caller-supplied `job.data.env` overlay (free-form; trust model is + * same-uid agent + worker, so the agent decides what to pass). + * + * Trust boundary is the operator's choice of `cwd`. Resolution uses + * `Object.hasOwn` (see `resolveInheritValue`) so prototype-pollution lookups + * like `inherit:["__proto__"]` can't return a value. + */ +function buildChildEnv( + override: Record | undefined, + inherit: string[] | undefined, +): Record { const env: Record = {}; for (const key of SHELL_ENV_ALLOWLIST) { const v = process.env[key]; if (typeof v === 'string') env[key] = v; } + if (inherit && inherit.length > 0) { + const cfg = loadConfig(); + for (const name of inherit) { + const value = resolveInheritValue(cfg, name); + if (value !== undefined) { + env[deriveEnvKey(name)] = value; + } + // Missing values are not silently dropped in production — the + // pre-enqueue validator fail-fasts at submit time. This branch only + // hits in legacy rows that bypassed pre-enqueue validation; the + // defense-in-depth re-validation in shellHandler catches them before + // this code path runs in practice. + } + } if (override) { for (const [k, v] of Object.entries(override)) env[k] = v; } @@ -217,8 +214,30 @@ export async function shellHandler(ctx: MinionJobContext): Promise(); + if (params.redact_secrets && params.inherit && params.inherit.length > 0) { + const cfg = loadConfig(); + for (const name of params.inherit) { + const value = resolveInheritValue(cfg, name); + if (value !== undefined) redactionMap.set(name, value); + } + } + const startedAt = Date.now(); let proc: ChildProcess; @@ -298,8 +317,18 @@ export async function shellHandler(ctx: MinionJobContext): Promise 0) { + stdout_tail = redactSecretsInText(stdout_tail, redactionMap); + stderr_tail = redactSecretsInText(stderr_tail, redactionMap); + } // If we sent SIGTERM/SIGKILL in response to an abort, surface that as the // error rather than the exit code — clearer for debugging. Worker catch diff --git a/src/core/operations.ts b/src/core/operations.ts index f0c67fac0..023219391 100644 --- a/src/core/operations.ts +++ b/src/core/operations.ts @@ -2092,13 +2092,57 @@ const submit_job: Operation = { // Trusted flag fires ONLY for an explicit local CLI submission of a protected // name. Strict `=== false` so an untyped/cast context can't escalate. const trusted = ctx.remote === false && isProtectedJobName(name) ? { allowProtectedSubmit: true } : undefined; - return queue.add(name, (p.data as Record) || {}, { + + const jobData = (p.data as Record) || {}; + + // v0.35.8.0: pre-enqueue shell-job validation, parity with the CLI submit + // path. Closes the bug class where shell.ts handler-time validation ran + // AFTER queue.add() persisted the row (codex F-CDX-1). Note: this branch + // only fires for trusted local submitters (`ctx.remote === false` AND + // protected-name allowlist), so remote MCP callers never reach it — but + // it stays here as defense-in-depth in case a future code path widens + // the trust gate above. + if (name === 'shell' && trusted) { + const { validateShellJobParams } = await import('./minions/handlers/shell-validate.ts'); + validateShellJobParams(jobData); + } + + const job = await queue.add(name, jobData, { queue: (p.queue as string) || 'default', priority: (p.priority as number) || 0, max_attempts: (p.max_attempts as number) || 3, delay: (p.delay as number) || undefined, timeout_ms: (p.timeout_ms as number) || undefined, }, trusted); + + // v0.35.8.0: submit_job audit-log parity with the CLI path (codex F-CDX-4). + // Pre-v0.35.8.0 the op handler bypassed the shell-audit JSONL writer + // entirely. Lift the call here so both submit surfaces produce one + // operational-trace line per shell submission. Best-effort; audit + // failures never block submission. + if (name === 'shell' && trusted) { + try { + const { logShellSubmission } = await import('./minions/handlers/shell-audit.ts'); + const inheritNames = Array.isArray(jobData.inherit) + ? (jobData.inherit as unknown[]).filter((s): s is string => typeof s === 'string') + : undefined; + logShellSubmission({ + caller: 'mcp', + // Gated on `trusted` (which requires ctx.remote === false), so + // we know this path is a local trusted submitter — log it that way. + remote: false, + job_id: job.id, + cwd: typeof jobData.cwd === 'string' ? jobData.cwd : '', + cmd_display: typeof jobData.cmd === 'string' ? (jobData.cmd as string).slice(0, 80) : undefined, + argv_display: Array.isArray(jobData.argv) + ? (jobData.argv as unknown[]).filter((a): a is string => typeof a === 'string').map((a) => a.slice(0, 80)) + : undefined, + inherit: inheritNames && inheritNames.length > 0 ? inheritNames : undefined, + }); + } catch { /* audit failures never block submission */ } + } + + return job; }, }; diff --git a/test/config-ensure-gitignore.test.ts b/test/config-ensure-gitignore.test.ts new file mode 100644 index 000000000..573fd153c --- /dev/null +++ b/test/config-ensure-gitignore.test.ts @@ -0,0 +1,93 @@ +/** + * Tests for `ensureGitignore()` in `src/core/config.ts` (v0.35.8.0). + * + * Idempotent retroactive coverage: every config-writing path (saveConfig + + * post-upgrade) lays down `~/.gbrain/.gitignore` containing the single line + * `*`. The helper MUST NOT clobber a `.gitignore` whose content the user + * has customized. + */ + +import { describe, test, expect, beforeEach, afterEach } from 'bun:test'; +import { existsSync, readFileSync, writeFileSync, mkdirSync, rmSync } from 'fs'; +import { join } from 'path'; +import { tmpdir } from 'os'; +import { withEnv } from './helpers/with-env.ts'; +import { ensureGitignore, configDir } from '../src/core/config.ts'; + +let testHome: string; + +beforeEach(() => { + testHome = join(tmpdir(), `gbrain-eg-${Date.now()}-${Math.random().toString(36).slice(2)}`); + mkdirSync(testHome, { recursive: true }); +}); + +afterEach(() => { + try { rmSync(testHome, { recursive: true, force: true }); } catch { /* best-effort */ } +}); + +describe('ensureGitignore', () => { + test('creates ~/.gbrain/.gitignore with single * when missing', async () => { + await withEnv({ GBRAIN_HOME: testHome }, async () => { + ensureGitignore(); + const file = join(configDir(), '.gitignore'); + expect(existsSync(file)).toBe(true); + expect(readFileSync(file, 'utf-8')).toBe('*\n'); + }); + }); + + test('no-op when .gitignore already exists with non-empty content (user customization preserved)', async () => { + await withEnv({ GBRAIN_HOME: testHome }, async () => { + // Prime: user wrote their own .gitignore first. + const dir = configDir(); + mkdirSync(dir, { recursive: true }); + const file = join(dir, '.gitignore'); + const userContent = '*.tmp\n# my custom gitignore\nlogs/\n'; + writeFileSync(file, userContent); + // Now call ensureGitignore. + ensureGitignore(); + // Assert: untouched. + expect(readFileSync(file, 'utf-8')).toBe(userContent); + }); + }); + + test('writes default when .gitignore exists but is empty', async () => { + await withEnv({ GBRAIN_HOME: testHome }, async () => { + const dir = configDir(); + mkdirSync(dir, { recursive: true }); + const file = join(dir, '.gitignore'); + writeFileSync(file, ''); + ensureGitignore(); + expect(readFileSync(file, 'utf-8')).toBe('*\n'); + }); + }); + + test('idempotent: second call is a no-op', async () => { + await withEnv({ GBRAIN_HOME: testHome }, async () => { + ensureGitignore(); + const file = join(configDir(), '.gitignore'); + const first = readFileSync(file, 'utf-8'); + ensureGitignore(); + const second = readFileSync(file, 'utf-8'); + expect(second).toBe(first); + }); + }); + + test('honors GBRAIN_HOME: writes under $GBRAIN_HOME/.gbrain/, not $HOME', async () => { + await withEnv({ GBRAIN_HOME: testHome }, async () => { + ensureGitignore(); + const expected = join(testHome, '.gbrain', '.gitignore'); + expect(existsSync(expected)).toBe(true); + }); + }); + + test('does not throw on read failure of existing file (best-effort)', async () => { + // Hard to simulate cleanly cross-platform; the contract is that ANY + // unexpected fs error is logged to stderr and the function returns + // normally without throwing. Tested indirectly by the idempotency case + // above and by the surface-level "function returns void, never throws" + // contract (no expect.toThrow on any call). + await withEnv({ GBRAIN_HOME: testHome }, async () => { + expect(() => ensureGitignore()).not.toThrow(); + }); + }); +}); diff --git a/test/doctor-home-dir-in-worktree.test.ts b/test/doctor-home-dir-in-worktree.test.ts new file mode 100644 index 000000000..e5f77a5d8 --- /dev/null +++ b/test/doctor-home-dir-in-worktree.test.ts @@ -0,0 +1,156 @@ +/** + * Tests for the `home_dir_in_worktree` doctor check (v0.35.8.0). + * + * Hermetic — drives the file system + GBRAIN_HOME + HOME envs directly via + * `withEnv`, then invokes `runDoctor(null, ['--fast', '--json'])` and parses + * the resulting JSON `checks` array. Skips the DB phase (engine=null + --fast). + * + * Covers F4 edge cases nailed in plan-eng-review: + * - .git as DIRECTORY (main repo) — warns + * - .git as FILE (linked worktree) — warns + * - walk terminates at $HOME — no false positive past it + * - GBRAIN_HOME override outside any worktree — ok + */ + +import { describe, test, expect, beforeEach, afterEach } from 'bun:test'; +import { mkdirSync, writeFileSync, rmSync } from 'fs'; +import { join } from 'path'; +import { tmpdir } from 'os'; +import { withEnv } from './helpers/with-env.ts'; +import { runDoctor } from '../src/commands/doctor.ts'; + +let scratch: string; + +beforeEach(() => { + scratch = join(tmpdir(), `gbrain-doctor-hw-${Date.now()}-${Math.random().toString(36).slice(2)}`); + mkdirSync(scratch, { recursive: true }); +}); + +afterEach(() => { + try { rmSync(scratch, { recursive: true, force: true }); } catch { /* best-effort */ } +}); + +/** Run the local doctor (no DB; null engine + --fast) under a stubbed HOME + + * GBRAIN_HOME, capture stdout AND prevent runDoctor's `process.exit(N)` from + * killing the test runner. Returns the check matching `name`. */ +async function getCheck(name: string, env: Record) { + const captured: string[] = []; + // Patch console.log directly — Bun's console.log doesn't route through the + // current process.stdout.write reference (it appears to cache the binding + // at module load), so monkey-patching write() doesn't catch it. console.log + // is the canonical doctor JSON-output channel. + const origLog = console.log; + console.log = (...args: unknown[]) => { + captured.push(args.map(a => typeof a === 'string' ? a : JSON.stringify(a)).join(' ') + '\n'); + }; + const origExit = process.exit; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (process as any).exit = (code?: number) => { + // Throw a tagged error so the test's try-block sees it; runDoctor's + // own try/catch doesn't catch this because it's outside its scope. + throw new Error(`__doctor_exit__:${code ?? 0}`); + }; + try { + await withEnv(env, async () => { + try { + await runDoctor(null, ['--fast', '--json']); + } catch (e) { + // Swallow the synthetic __doctor_exit__ sentinel; rethrow other errors. + if (!(e instanceof Error) || !e.message.startsWith('__doctor_exit__:')) throw e; + } + }); + } finally { + console.log = origLog; + process.exit = origExit; + } + const text = captured.join(''); + // The doctor's JSON envelope is the last `{` block containing `"checks":`. + const idx = text.lastIndexOf('"checks"'); + const objStart = idx >= 0 ? text.lastIndexOf('{', idx) : text.lastIndexOf('{'); + const jsonStr = objStart >= 0 ? text.slice(objStart) : text; + let parsed: { checks: { name: string; status: string; message: string }[] }; + try { + parsed = JSON.parse(jsonStr); + } catch { + throw new Error(`Could not parse doctor JSON; saw: ${text.slice(-500)}`); + } + return parsed.checks.find(c => c.name === name); +} + +describe('home_dir_in_worktree doctor check', () => { + test('gbrain home outside any worktree → ok', async () => { + // scratch/.gbrain — no parent has a .git, scratch IS our fake $HOME + const home = scratch; + const gbrainParent = home; + const check = await getCheck('home_dir_in_worktree', { + HOME: home, + GBRAIN_HOME: gbrainParent, + }); + expect(check).toBeDefined(); + expect(check!.status).toBe('ok'); + }); + + test('gbrain home inside dir-style .git worktree → warn', async () => { + // scratch/home/myrepo/.git/ (directory) + // scratch/home/myrepo/.gbrain/ ← gbrain home is inside the worktree + const home = join(scratch, 'home'); + const repo = join(home, 'myrepo'); + mkdirSync(join(repo, '.git'), { recursive: true }); + mkdirSync(repo, { recursive: true }); + const check = await getCheck('home_dir_in_worktree', { + HOME: home, + GBRAIN_HOME: repo, + }); + expect(check).toBeDefined(); + expect(check!.status).toBe('warn'); + expect(check!.message).toContain('myrepo'); + }); + + test('gbrain home inside .git-AS-FILE linked worktree → warn (F4)', async () => { + // Linked worktrees use a `.git` FILE (not a directory) containing + // `gitdir: /path/to/main/.git/worktrees/`. Doctor MUST recognize + // both shapes — this is the Conductor + git-worktrees topology our + // dev environment runs in. + const home = join(scratch, 'home'); + const repo = join(home, 'linked-wt'); + mkdirSync(repo, { recursive: true }); + writeFileSync(join(repo, '.git'), 'gitdir: /some/other/path/.git/worktrees/linked-wt\n'); + const check = await getCheck('home_dir_in_worktree', { + HOME: home, + GBRAIN_HOME: repo, + }); + expect(check).toBeDefined(); + expect(check!.status).toBe('warn'); + expect(check!.message).toContain('linked-wt'); + }); + + test('walk terminates at $HOME — .git ABOVE $HOME does NOT trigger warn (F4)', async () => { + // scratch/.git/ (ABOVE the fake $HOME — should be ignored) + // scratch/home/ (fake $HOME) + // scratch/home/.gbrain/ (no worktree below $HOME) + mkdirSync(join(scratch, '.git'), { recursive: true }); + const home = join(scratch, 'home'); + mkdirSync(home, { recursive: true }); + const check = await getCheck('home_dir_in_worktree', { + HOME: home, + GBRAIN_HOME: home, + }); + expect(check).toBeDefined(); + // OK because the .git is above $HOME, outside our walk scope. + expect(check!.status).toBe('ok'); + }); + + test('GBRAIN_HOME override pointing outside any worktree → ok', async () => { + // Real $HOME might be inside a worktree, but the user pointed + // GBRAIN_HOME at a clean location. Doctor should report ok. + const home = scratch; + const safe = join(scratch, 'safe-elsewhere'); + mkdirSync(safe, { recursive: true }); + const check = await getCheck('home_dir_in_worktree', { + HOME: home, + GBRAIN_HOME: safe, + }); + expect(check).toBeDefined(); + expect(check!.status).toBe('ok'); + }); +}); diff --git a/test/e2e/minions-shell-pglite.test.ts b/test/e2e/minions-shell-pglite.test.ts index b15b27ef4..0d8033975 100644 --- a/test/e2e/minions-shell-pglite.test.ts +++ b/test/e2e/minions-shell-pglite.test.ts @@ -93,6 +93,283 @@ describe('E2E: Minions shell handler on PGLite (--follow inline path)', () => { } }, 30000); + test('v0.35.8.0: inherit:["database_url"] resolves DATABASE_URL into child env, names-only in row + audit', async () => { + // Hermetic — drive `inherit` directly against the PGLite path. The + // production submit flow runs `validateShellJobParams` pre-enqueue, but + // here we exercise the handler-side resolution by submitting a row that + // already passed pre-enqueue validation upstream. Validates that: + // 1. The child env carries GBRAIN_DATABASE_URL with the resolved value. + // 2. The persisted row's `data.inherit` is ["database_url"] (names only). + // 3. The persisted row JSON does NOT contain the URL substring anywhere. + const { writeFileSync, mkdirSync, rmSync, existsSync } = await import('node:fs'); + const { join } = await import('node:path'); + const { tmpdir } = await import('node:os'); + + const tmpHome = join(tmpdir(), `gbrain-inh-${Date.now()}-${Math.random().toString(36).slice(2)}`); + mkdirSync(join(tmpHome, '.gbrain'), { recursive: true }); + const testDbUrl = 'postgresql://test:T0P_5ECR3T@localhost:5432/inherit_e2e_db'; + writeFileSync( + join(tmpHome, '.gbrain', 'config.json'), + JSON.stringify({ engine: 'postgres', database_url: testDbUrl }) + '\n', + ); + + const savedHome = process.env.GBRAIN_HOME; + const savedGbrainUrl = process.env.GBRAIN_DATABASE_URL; + const savedDbUrl = process.env.DATABASE_URL; + process.env.GBRAIN_HOME = tmpHome; + // loadConfig() merges env vars OVER config.json, so we must drop both env + // names while the worker reads its own config. When the E2E suite runs + // with a real DATABASE_URL set in the parent process, the inherited value + // would otherwise be the suite's postgres URL, not the test's. + delete process.env.GBRAIN_DATABASE_URL; + delete process.env.DATABASE_URL; + + try { + const queue = new MinionQueue(engine); + const job = await queue.add( + 'shell', + // `printenv` reflects the child env to stdout — proves the inherited + // secret reached the child without us having to leak it via the test. + { cmd: 'printenv GBRAIN_DATABASE_URL', cwd: '/tmp', inherit: ['database_url'] }, + {}, + { allowProtectedSubmit: true }, + ); + + // T7 regression assertion: the persisted row carries names only, NEVER values. + const persisted = await queue.getJob(job.id); + expect((persisted!.data as Record).inherit).toEqual(['database_url']); + // T1 + T7 negative-shape: the URL substring must not appear ANYWHERE in + // the persisted row's data JSON. Pinpoint the load-bearing R1 invariant. + const rowJson = JSON.stringify(persisted!.data); + expect(rowJson).not.toContain('T0P_5ECR3T'); + expect(rowJson).not.toContain(testDbUrl); + + const worker = new MinionWorker(engine, { pollInterval: 100, lockDuration: 30000 }); + await registerBuiltinHandlers(worker, engine); + const runPromise = worker.start(); + try { + const status = await waitTerminal(queue, job.id, 20000); + expect(status).toBe('completed'); + const final = await queue.getJob(job.id); + // The child saw GBRAIN_DATABASE_URL = the configured URL. + expect((final!.result as Record).stdout_tail).toBe(testDbUrl + '\n'); + } finally { + worker.stop(); + await runPromise; + } + } finally { + if (savedHome === undefined) delete process.env.GBRAIN_HOME; + else process.env.GBRAIN_HOME = savedHome; + if (savedGbrainUrl === undefined) delete process.env.GBRAIN_DATABASE_URL; + else process.env.GBRAIN_DATABASE_URL = savedGbrainUrl; + if (savedDbUrl === undefined) delete process.env.DATABASE_URL; + else process.env.DATABASE_URL = savedDbUrl; + if (existsSync(tmpHome)) rmSync(tmpHome, { recursive: true, force: true }); + } + }, 30000); + + test('v0.36.5.0: inherit:["anthropic_api_key"] also resolves (free-form, any config key)', async () => { + // v0.36.5.0 free-form design: inherit accepts ANY snake_case config-key + // name, not a closed enum. Same single-uid trust model — the agent + // decides what to pass. This test exercises the non-database_url path + // to prove the mechanism is genuinely free-form. + const { writeFileSync, mkdirSync, rmSync, existsSync } = await import('node:fs'); + const { join } = await import('node:path'); + const { tmpdir } = await import('node:os'); + + const tmpHome = join(tmpdir(), `gbrain-anth-${Date.now()}-${Math.random().toString(36).slice(2)}`); + mkdirSync(join(tmpHome, '.gbrain'), { recursive: true }); + const fakeKey = 'sk-ant-test-FAKE-KEY-FOR-E2E'; + writeFileSync( + join(tmpHome, '.gbrain', 'config.json'), + JSON.stringify({ + engine: 'postgres', + database_url: 'postgresql://x:y@h/d', + anthropic_api_key: fakeKey, + }) + '\n', + ); + + const savedHome = process.env.GBRAIN_HOME; + const savedAnth = process.env.ANTHROPIC_API_KEY; + process.env.GBRAIN_HOME = tmpHome; + delete process.env.ANTHROPIC_API_KEY; + + try { + const queue = new MinionQueue(engine); + const job = await queue.add( + 'shell', + { cmd: 'printenv ANTHROPIC_API_KEY', cwd: '/tmp', inherit: ['anthropic_api_key'] }, + {}, + { allowProtectedSubmit: true }, + ); + + // Row carries name only — not value. + const persisted = await queue.getJob(job.id); + expect((persisted!.data as Record).inherit).toEqual(['anthropic_api_key']); + const rowJson = JSON.stringify(persisted!.data); + expect(rowJson).not.toContain('sk-ant-test-FAKE-KEY-FOR-E2E'); + + const worker = new MinionWorker(engine, { pollInterval: 100, lockDuration: 30000 }); + await registerBuiltinHandlers(worker, engine); + const runPromise = worker.start(); + try { + const status = await waitTerminal(queue, job.id, 20000); + expect(status).toBe('completed'); + const final = await queue.getJob(job.id); + // Child saw ANTHROPIC_API_KEY = configured key (derived env-name from snake_case) + expect((final!.result as Record).stdout_tail).toBe(fakeKey + '\n'); + } finally { + worker.stop(); + await runPromise; + } + } finally { + if (savedHome === undefined) delete process.env.GBRAIN_HOME; + else process.env.GBRAIN_HOME = savedHome; + if (savedAnth === undefined) delete process.env.ANTHROPIC_API_KEY; + else process.env.ANTHROPIC_API_KEY = savedAnth; + if (existsSync(tmpHome)) rmSync(tmpHome, { recursive: true, force: true }); + } + }, 30000); + + test('v0.36.5.0: redact_secrets:true scrubs inherit values from stdout_tail', async () => { + // Honest defense for the documented output-side leakage: when the script + // echoes the inherited value, redact_secrets:true ensures the persisted + // result.stdout_tail carries the token instead of the + // plaintext value. The agent opts in per-job; default is false (back-compat). + const { writeFileSync, mkdirSync, rmSync, existsSync } = await import('node:fs'); + const { join } = await import('node:path'); + const { tmpdir } = await import('node:os'); + + const tmpHome = join(tmpdir(), `gbrain-redact-${Date.now()}-${Math.random().toString(36).slice(2)}`); + mkdirSync(join(tmpHome, '.gbrain'), { recursive: true }); + const fakeKey = 'sk-ant-FAKE-REDACT-E2E'; + const fakeUrl = 'postgresql://user:R3DACT_ME@host:5432/redactdb'; + writeFileSync( + join(tmpHome, '.gbrain', 'config.json'), + JSON.stringify({ + engine: 'postgres', + database_url: fakeUrl, + anthropic_api_key: fakeKey, + }) + '\n', + ); + + const savedHome = process.env.GBRAIN_HOME; + const savedDbUrl = process.env.GBRAIN_DATABASE_URL; + const savedAnth = process.env.ANTHROPIC_API_KEY; + process.env.GBRAIN_HOME = tmpHome; + delete process.env.GBRAIN_DATABASE_URL; + delete process.env.DATABASE_URL; + delete process.env.ANTHROPIC_API_KEY; + + try { + const queue = new MinionQueue(engine); + const job = await queue.add( + 'shell', + { + cmd: 'printenv GBRAIN_DATABASE_URL && printenv ANTHROPIC_API_KEY', + cwd: '/tmp', + inherit: ['database_url', 'anthropic_api_key'], + redact_secrets: true, + }, + {}, + { allowProtectedSubmit: true }, + ); + + const worker = new MinionWorker(engine, { pollInterval: 100, lockDuration: 30000 }); + await registerBuiltinHandlers(worker, engine); + const runPromise = worker.start(); + try { + const status = await waitTerminal(queue, job.id, 20000); + expect(status).toBe('completed'); + const final = await queue.getJob(job.id); + const stdoutTail = (final!.result as Record).stdout_tail as string; + // The actual values must NOT appear in the persisted row. + expect(stdoutTail).not.toContain('R3DACT_ME'); + expect(stdoutTail).not.toContain(fakeUrl); + expect(stdoutTail).not.toContain(fakeKey); + // Redaction tokens point at WHICH inherit name was scrubbed. + expect(stdoutTail).toContain(''); + expect(stdoutTail).toContain(''); + } finally { + worker.stop(); + await runPromise; + } + } finally { + if (savedHome === undefined) delete process.env.GBRAIN_HOME; + else process.env.GBRAIN_HOME = savedHome; + if (savedDbUrl === undefined) delete process.env.GBRAIN_DATABASE_URL; + else process.env.GBRAIN_DATABASE_URL = savedDbUrl; + if (savedAnth === undefined) delete process.env.ANTHROPIC_API_KEY; + else process.env.ANTHROPIC_API_KEY = savedAnth; + if (existsSync(tmpHome)) rmSync(tmpHome, { recursive: true, force: true }); + } + }, 30000); + + test('v0.36.5.0: redact_secrets:false (default) does NOT scrub — back-compat', async () => { + // The previous tests already prove default behavior (no scrubbing) by + // asserting stdout_tail equals the literal URL when redact_secrets is + // absent. This case is the explicit-false twin: passing false should + // be identical to omitting. + const { writeFileSync, mkdirSync, rmSync, existsSync } = await import('node:fs'); + const { join } = await import('node:path'); + const { tmpdir } = await import('node:os'); + + const tmpHome = join(tmpdir(), `gbrain-rd-off-${Date.now()}-${Math.random().toString(36).slice(2)}`); + mkdirSync(join(tmpHome, '.gbrain'), { recursive: true }); + const fakeUrl = 'postgresql://u:NO_REDACT@h:5432/nrdb'; + writeFileSync( + join(tmpHome, '.gbrain', 'config.json'), + JSON.stringify({ engine: 'postgres', database_url: fakeUrl }) + '\n', + ); + + const savedHome = process.env.GBRAIN_HOME; + const savedDbUrl = process.env.GBRAIN_DATABASE_URL; + const savedPlainDbUrl = process.env.DATABASE_URL; + process.env.GBRAIN_HOME = tmpHome; + delete process.env.GBRAIN_DATABASE_URL; + delete process.env.DATABASE_URL; + + try { + const queue = new MinionQueue(engine); + const job = await queue.add( + 'shell', + { + cmd: 'printenv GBRAIN_DATABASE_URL', + cwd: '/tmp', + inherit: ['database_url'], + redact_secrets: false, + }, + {}, + { allowProtectedSubmit: true }, + ); + + const worker = new MinionWorker(engine, { pollInterval: 100, lockDuration: 30000 }); + await registerBuiltinHandlers(worker, engine); + const runPromise = worker.start(); + try { + const status = await waitTerminal(queue, job.id, 20000); + expect(status).toBe('completed'); + const final = await queue.getJob(job.id); + const stdoutTail = (final!.result as Record).stdout_tail as string; + // Plaintext URL DOES appear when redact is off — back-compat with + // earlier inherit:["database_url"] tests in this file. + expect(stdoutTail).toBe(fakeUrl + '\n'); + expect(stdoutTail).not.toContain(' { // v0.20.3+: shell handler is always registered (so claimed jobs emit a clear // rejection log), but the runtime env guard lives inside the handler itself. diff --git a/test/minions-shell-inherit.test.ts b/test/minions-shell-inherit.test.ts new file mode 100644 index 000000000..dff66343d --- /dev/null +++ b/test/minions-shell-inherit.test.ts @@ -0,0 +1,120 @@ +/** + * Tests for `src/core/minions/handlers/shell-inherit.ts` — free-form helpers + * for `inherit:` secret resolution (v0.36.5.0). + * + * Two pure functions and one regex. Properties under test: + * - `INHERIT_NAME_RE` matches snake_case shapes, rejects everything else. + * - `deriveEnvKey` returns the right ENV-key name per convention. + * - `resolveInheritValue` uses `Object.hasOwn` (prototype-pollution defense), + * returns undefined for missing / non-string / empty-string values, and + * handles `null` config. + */ + +import { describe, test, expect } from 'bun:test'; +import { + INHERIT_NAME_RE, + deriveEnvKey, + resolveInheritValue, +} from '../src/core/minions/handlers/shell-inherit.ts'; +import type { GBrainConfig } from '../src/core/config.ts'; + +describe('INHERIT_NAME_RE', () => { + test.each([ + 'database_url', + 'anthropic_api_key', + 'openai_api_key', + 'voyage_api_key', + 'groq_api_key', + 'zeroentropy_api_key', + 'remote_mcp_oauth_client_secret', + 'field2', + 'a', + 'a_b_c_d_e', + ])('accepts snake_case shape: %s', (name) => { + expect(INHERIT_NAME_RE.test(name)).toBe(true); + }); + + test.each([ + '__proto__', // leading underscore (prototype pollution) + '_underscore_first', // leading underscore + 'CamelCase', // uppercase letters + 'UPPER_CASE', // all uppercase + '0_leading_digit', // leading digit + 'has-dash', // hyphen + 'has.dot', // dot + '../traversal', // path-traversal shape + 'has space', // whitespace + '', // empty + 'has\nnewline', // newline + ])('rejects non-snake_case shape: %s', (name) => { + expect(INHERIT_NAME_RE.test(name)).toBe(false); + }); +}); + +describe('deriveEnvKey', () => { + test('database_url → GBRAIN_DATABASE_URL (override, less ambiguous)', () => { + expect(deriveEnvKey('database_url')).toBe('GBRAIN_DATABASE_URL'); + }); + test('anthropic_api_key → ANTHROPIC_API_KEY (provider-standard uppercase)', () => { + expect(deriveEnvKey('anthropic_api_key')).toBe('ANTHROPIC_API_KEY'); + }); + test('openai_api_key → OPENAI_API_KEY', () => { + expect(deriveEnvKey('openai_api_key')).toBe('OPENAI_API_KEY'); + }); + test('voyage_api_key → VOYAGE_API_KEY', () => { + expect(deriveEnvKey('voyage_api_key')).toBe('VOYAGE_API_KEY'); + }); + test('groq_api_key → GROQ_API_KEY', () => { + expect(deriveEnvKey('groq_api_key')).toBe('GROQ_API_KEY'); + }); + test('arbitrary_field → ARBITRARY_FIELD (default uppercase)', () => { + expect(deriveEnvKey('arbitrary_field')).toBe('ARBITRARY_FIELD'); + }); +}); + +describe('resolveInheritValue', () => { + test('returns string when field exists and is a non-empty string', () => { + const cfg: GBrainConfig = { engine: 'postgres', database_url: 'postgresql://x' }; + expect(resolveInheritValue(cfg, 'database_url')).toBe('postgresql://x'); + }); + test('returns undefined when field is unset', () => { + expect(resolveInheritValue({ engine: 'postgres' }, 'database_url')).toBeUndefined(); + }); + test('returns undefined when field is empty string', () => { + const cfg: GBrainConfig = { engine: 'postgres', database_url: '' }; + expect(resolveInheritValue(cfg, 'database_url')).toBeUndefined(); + }); + test('returns undefined for null config', () => { + expect(resolveInheritValue(null, 'database_url')).toBeUndefined(); + }); + test('returns undefined when field is non-string (e.g. object)', () => { + const cfg = { engine: 'postgres', remote_mcp: { issuer_url: 'x' } } as unknown as GBrainConfig; + expect(resolveInheritValue(cfg, 'remote_mcp')).toBeUndefined(); + }); + test('prototype-pollution defense: __proto__ returns undefined even though Object.prototype has the property', () => { + expect(resolveInheritValue({ engine: 'postgres' }, '__proto__')).toBeUndefined(); + }); + test('prototype-pollution defense: constructor returns undefined', () => { + expect(resolveInheritValue({ engine: 'postgres' }, 'constructor')).toBeUndefined(); + }); + test('prototype-pollution defense: toString returns undefined', () => { + expect(resolveInheritValue({ engine: 'postgres' }, 'toString')).toBeUndefined(); + }); +}); + +describe('integration: deriveEnvKey + resolveInheritValue work together', () => { + const cfg: GBrainConfig = { + engine: 'postgres', + database_url: 'postgresql://x', + anthropic_api_key: 'sk-ant-x', + openai_api_key: 'sk-x', + }; + test.each([ + ['database_url', 'GBRAIN_DATABASE_URL', 'postgresql://x'], + ['anthropic_api_key', 'ANTHROPIC_API_KEY', 'sk-ant-x'], + ['openai_api_key', 'OPENAI_API_KEY', 'sk-x'], + ])('name %s resolves to envKey %s with value %s', (name, expectedEnvKey, expectedValue) => { + expect(deriveEnvKey(name)).toBe(expectedEnvKey); + expect(resolveInheritValue(cfg, name)).toBe(expectedValue); + }); +}); diff --git a/test/minions-shell-redact.test.ts b/test/minions-shell-redact.test.ts new file mode 100644 index 000000000..bdd1a505c --- /dev/null +++ b/test/minions-shell-redact.test.ts @@ -0,0 +1,112 @@ +/** + * Tests for `src/core/minions/handlers/shell-redact.ts` — output-side + * redaction of inherit-resolved values from shell-job stdout/stderr. + * + * Pure function under test. Properties: + * - Replaces every occurrence of each map value with ``. + * - Empty input or empty map → identity. + * - Empty values in map are skipped (defensive). + * - String-mode replaceAll: regex metacharacters in values are literal. + * - Multiple secrets are independently scrubbed. + * - Substring overlap: a longer secret containing a shorter one redacts + * the longer one first only if iteration order places it first (Map + * iteration is insertion order). Test the realistic case. + */ + +import { describe, test, expect } from 'bun:test'; +import { redactSecretsInText } from '../src/core/minions/handlers/shell-redact.ts'; + +describe('redactSecretsInText', () => { + test('empty text passes through unchanged', () => { + expect(redactSecretsInText('', new Map([['database_url', 'postgresql://x:y@h/d']]))).toBe(''); + }); + + test('empty map passes text through unchanged', () => { + expect(redactSecretsInText('postgresql://x:y@h/d landed in logs', new Map())).toBe( + 'postgresql://x:y@h/d landed in logs', + ); + }); + + test('single secret value redacted with name token', () => { + const out = redactSecretsInText( + 'DB URL is postgresql://x:y@h/d', + new Map([['database_url', 'postgresql://x:y@h/d']]), + ); + expect(out).toBe('DB URL is '); + }); + + test('multiple occurrences of the same secret all redacted', () => { + const out = redactSecretsInText( + 'first: postgresql://x:y@h/d, again: postgresql://x:y@h/d, done.', + new Map([['database_url', 'postgresql://x:y@h/d']]), + ); + expect(out).toBe( + 'first: , again: , done.', + ); + }); + + test('multiple secrets each independently redacted', () => { + const text = + 'connecting to postgresql://x:y@h/d with key sk-ant-test123 done'; + const out = redactSecretsInText(text, new Map([ + ['database_url', 'postgresql://x:y@h/d'], + ['anthropic_api_key', 'sk-ant-test123'], + ])); + expect(out).toBe('connecting to with key done'); + }); + + test('empty-value entries in map are skipped (defensive)', () => { + const text = 'postgresql://x:y@h/d landed in logs'; + const out = redactSecretsInText(text, new Map([ + ['weird_empty', ''], + ['database_url', 'postgresql://x:y@h/d'], + ])); + expect(out).toBe(' landed in logs'); + }); + + test('regex metacharacters in value are treated as literal', () => { + // String-mode replaceAll. If the value contains regex chars like .*+?() + // they don't expand. Critical for safety. + const trickyValue = 'pgpass.*foo+bar?(baz)'; + const out = redactSecretsInText( + `dump: ${trickyValue} end`, + new Map([['foo', trickyValue]]), + ); + expect(out).toBe('dump: end'); + }); + + test('text without the secret value is unchanged', () => { + const out = redactSecretsInText( + 'no secrets here, just normal log output', + new Map([['database_url', 'postgresql://x:y@h/d']]), + ); + expect(out).toBe('no secrets here, just normal log output'); + }); + + test('value across newlines is redacted (replaceAll handles \\n)', () => { + // If a JWT-like secret happens to contain a newline somehow, replaceAll + // still works because it's string-mode. + const v = 'line1\nline2'; + const out = redactSecretsInText(`pre ${v} post`, new Map([['multi', v]])); + expect(out).toBe('pre post'); + }); + + test('substring overlap: shorter value inside longer value', () => { + // If `short` is a substring of `long` AND both are in the map, the + // iteration-order winner replaces first. Map preserves insertion order, + // so the test reflects that explicitly. Real-world expectation: callers + // should not have overlapping secrets; if they do, longest-first is + // typically what they want (which requires the caller to insert long + // before short). This test pins the behavior, doesn't claim a policy. + const longV = 'token_with_inner_token'; + const shortV = 'inner_token'; + const text = `outer: ${longV}, inner: ${shortV}`; + const out = redactSecretsInText(text, new Map([ + ['long_token', longV], + ['short_token', shortV], + ])); + // Long replaced first → its substring stays as REDACTED token, short + // then replaces the standalone occurrence. + expect(out).toBe('outer: , inner: '); + }); +}); diff --git a/test/minions-shell-validate.test.ts b/test/minions-shell-validate.test.ts new file mode 100644 index 000000000..354d800f8 --- /dev/null +++ b/test/minions-shell-validate.test.ts @@ -0,0 +1,247 @@ +/** + * Tests for `src/core/minions/handlers/shell-validate.ts` — the pre-enqueue + * validator. + * + * v0.36.5.0 design: `inherit:` is free-form (any snake_case config-key name). + * No closed-enum check, no env-shadow rejection, no inline-cmd scan. The + * single-uid trust model treats the agent as a peer of the worker — it + * decides which secrets to pass. + * + * What the validator DOES check: + * - Shape (cmd XOR argv, cwd absolute, env is string→string) + * - `inherit` is array of snake_case strings (prototype-pollution defense) + * - Every `inherit` name resolves on `loadConfig()` (UX fail-fast) + * + * The T1 regression guard at the bottom pins the load-bearing invariant: + * validation throws BEFORE any persistence call. + */ + +import { describe, test, expect } from 'bun:test'; +import { validateShellJobParams } from '../src/core/minions/handlers/shell-validate.ts'; +import { UnrecoverableError } from '../src/core/minions/types.ts'; +import type { GBrainConfig } from '../src/core/config.ts'; + +const dbUrl = 'postgresql://test:test@localhost:5432/test'; +const fakeCfg: GBrainConfig = { + engine: 'postgres', + database_url: dbUrl, + anthropic_api_key: 'sk-ant-test', + openai_api_key: 'sk-test', +}; + +describe('validateShellJobParams — existing param shape checks', () => { + test('cmd XOR argv: both → reject', () => { + expect(() => validateShellJobParams({ cmd: 'echo', argv: ['echo'], cwd: '/tmp' }, { config: fakeCfg })) + .toThrow(UnrecoverableError); + }); + test('cmd XOR argv: neither → reject', () => { + expect(() => validateShellJobParams({ cwd: '/tmp' }, { config: fakeCfg })) + .toThrow(UnrecoverableError); + }); + test('cwd must be absolute', () => { + expect(() => validateShellJobParams({ cmd: 'echo', cwd: 'relative/path' }, { config: fakeCfg })) + .toThrow(/absolute path/); + }); + test('cwd required', () => { + expect(() => validateShellJobParams({ cmd: 'echo', cwd: '' }, { config: fakeCfg })) + .toThrow(/cwd/); + }); + test('env must be object of string values', () => { + expect(() => validateShellJobParams({ cmd: 'echo', cwd: '/tmp', env: 'oops' as unknown as Record }, { config: fakeCfg })) + .toThrow(/env/); + expect(() => validateShellJobParams({ cmd: 'echo', cwd: '/tmp', env: { K: 1 as unknown as string } }, { config: fakeCfg })) + .toThrow(/string/); + }); + test('happy path: cmd + cwd accepted', () => { + const p = validateShellJobParams({ cmd: 'echo hi', cwd: '/tmp' }, { config: fakeCfg }); + expect(p.cmd).toBe('echo hi'); + expect(p.argv).toBeUndefined(); + expect(p.cwd).toBe('/tmp'); + }); +}); + +describe('inherit — free-form config-key names (v0.36.5.0)', () => { + test('inherit:["database_url"] accepted', () => { + const p = validateShellJobParams( + { cmd: 'echo', cwd: '/tmp', inherit: ['database_url'] }, + { config: fakeCfg }, + ); + expect(p.inherit).toEqual(['database_url']); + }); + test('inherit:["anthropic_api_key"] accepted (was scope-creep in closed enum)', () => { + const p = validateShellJobParams( + { cmd: 'echo', cwd: '/tmp', inherit: ['anthropic_api_key'] }, + { config: fakeCfg }, + ); + expect(p.inherit).toEqual(['anthropic_api_key']); + }); + test('inherit multiple keys at once', () => { + const p = validateShellJobParams( + { cmd: 'echo', cwd: '/tmp', inherit: ['database_url', 'anthropic_api_key', 'openai_api_key'] }, + { config: fakeCfg }, + ); + expect(p.inherit).toEqual(['database_url', 'anthropic_api_key', 'openai_api_key']); + }); + test('inherit must be an array', () => { + expect(() => validateShellJobParams( + { cmd: 'echo', cwd: '/tmp', inherit: 'database_url' as unknown as string[] }, + { config: fakeCfg }, + )).toThrow(/inherit must be an array/); + }); + test('inherit non-string element rejected', () => { + expect(() => validateShellJobParams( + { cmd: 'echo', cwd: '/tmp', inherit: [1] as unknown as string[] }, + { config: fakeCfg }, + )).toThrow(/non-empty strings/); + }); + test('inherit empty string rejected', () => { + expect(() => validateShellJobParams( + { cmd: 'echo', cwd: '/tmp', inherit: [''] }, + { config: fakeCfg }, + )).toThrow(/non-empty strings/); + }); +}); + +describe('inherit — snake_case shape guard (prototype-pollution defense)', () => { + test('"__proto__" rejected (not snake_case shape)', () => { + expect(() => validateShellJobParams( + { cmd: 'echo', cwd: '/tmp', inherit: ['__proto__'] }, + { config: fakeCfg }, + )).toThrow(/must match \[a-z\]/); + }); + test('"constructor" rejected (uppercase letters)', () => { + expect(() => validateShellJobParams( + { cmd: 'echo', cwd: '/tmp', inherit: ['constructor'] as unknown as string[] }, + { config: fakeCfg }, + )).toThrow(/worker has no constructor configured/); + }); + test('path-traversal-looking name rejected', () => { + expect(() => validateShellJobParams( + { cmd: 'echo', cwd: '/tmp', inherit: ['../etc/passwd'] }, + { config: fakeCfg }, + )).toThrow(/must match \[a-z\]/); + }); + test('"FOO_BAR" (uppercase) rejected — config keys are snake_case', () => { + expect(() => validateShellJobParams( + { cmd: 'echo', cwd: '/tmp', inherit: ['FOO_BAR'] }, + { config: fakeCfg }, + )).toThrow(/must match \[a-z\]/); + }); + test('digits-after-letter allowed (matches regex)', () => { + // Won't actually resolve since fakeCfg doesn't have field2 — fail-fast hits + expect(() => validateShellJobParams( + { cmd: 'echo', cwd: '/tmp', inherit: ['field2'] }, + { config: fakeCfg }, + )).toThrow(/worker has no field2 configured/); + }); +}); + +describe('inherit — fail-fast on missing config value', () => { + test('inherit:["database_url"] + config without database_url → reject with set-hint', () => { + expect(() => validateShellJobParams( + { cmd: 'echo', cwd: '/tmp', inherit: ['database_url'] }, + { config: { engine: 'postgres' } as GBrainConfig }, + )).toThrow(/gbrain config set database_url/); + }); + test('inherit:["database_url"] + null config → reject', () => { + expect(() => validateShellJobParams( + { cmd: 'echo', cwd: '/tmp', inherit: ['database_url'] }, + { config: null }, + )).toThrow(/worker has no database_url/); + }); + test('inherit:["voyage_api_key"] when not set → reject (fakeCfg has only db_url + anthropic + openai)', () => { + expect(() => validateShellJobParams( + { cmd: 'echo', cwd: '/tmp', inherit: ['voyage_api_key'] }, + { config: fakeCfg }, + )).toThrow(/worker has no voyage_api_key configured/); + }); + test('inherit:["database_url"] + empty-string database_url → reject', () => { + expect(() => validateShellJobParams( + { cmd: 'echo', cwd: '/tmp', inherit: ['database_url'] }, + { config: { engine: 'postgres', database_url: '' } }, + )).toThrow(/worker has no database_url/); + }); +}); + +describe('what the validator deliberately does NOT do (agency for the agent)', () => { + test('caller can use env: for ANY key, including ones with secret-looking names', () => { + // No shadow rejection. Agent decides if they want to put a URL in env: + // directly (and accept that it lands in the row plaintext). v0.36.5.0 + // honors the agent's call. + const p = validateShellJobParams( + { cmd: 'echo', cwd: '/tmp', env: { GBRAIN_DATABASE_URL: dbUrl, DATABASE_URL: dbUrl } }, + { config: fakeCfg }, + ); + expect(p.env).toEqual({ GBRAIN_DATABASE_URL: dbUrl, DATABASE_URL: dbUrl }); + }); + test('caller can put inline secret-key=value in cmd', () => { + // No inline-cmd scan. The agent knows what it's writing. + const p = validateShellJobParams( + { cmd: 'GBRAIN_DATABASE_URL=postgresql://... gbrain sync', cwd: '/tmp' }, + { config: fakeCfg }, + ); + expect(p.cmd).toContain('GBRAIN_DATABASE_URL'); + }); + test('inherit + env: with overlapping intent both work (last write wins per overlay order)', () => { + // Agent might want inherit for value-from-config AND env: for an + // additional non-secret. Both are honored. + const p = validateShellJobParams( + { cmd: 'echo', cwd: '/tmp', inherit: ['database_url'], env: { MY_FLAG: '1' } }, + { config: fakeCfg }, + ); + expect(p.inherit).toEqual(['database_url']); + expect(p.env).toEqual({ MY_FLAG: '1' }); + }); +}); + +describe('redact_secrets shape check', () => { + test('redact_secrets: true accepted', () => { + const p = validateShellJobParams( + { cmd: 'echo', cwd: '/tmp', inherit: ['database_url'], redact_secrets: true }, + { config: fakeCfg }, + ); + expect(p.redact_secrets).toBe(true); + }); + test('redact_secrets: false accepted', () => { + const p = validateShellJobParams( + { cmd: 'echo', cwd: '/tmp', redact_secrets: false }, + { config: fakeCfg }, + ); + expect(p.redact_secrets).toBe(false); + }); + test('redact_secrets: undefined is fine (default)', () => { + const p = validateShellJobParams( + { cmd: 'echo', cwd: '/tmp' }, + { config: fakeCfg }, + ); + expect(p.redact_secrets).toBeUndefined(); + }); + test('redact_secrets: non-boolean rejected', () => { + expect(() => validateShellJobParams( + { cmd: 'echo', cwd: '/tmp', redact_secrets: 'yes' as unknown as boolean }, + { config: fakeCfg }, + )).toThrow(/redact_secrets must be a boolean/); + }); +}); + +describe('T1 regression guard: validation runs BEFORE persistence', () => { + // This test pins the load-bearing invariant codex caught: validation must + // throw before any persistence call. If a future refactor moves the + // validation call back into the shell.ts handler, this test fails. + test('bad payload throws synchronously, no queue.add could have been called', () => { + let queueAddCalled = false; + const fakeQueueAdd = () => { queueAddCalled = true; }; + + // Bad shape: inherit name doesn't pass snake_case regex. + const data = { cmd: 'echo', cwd: '/tmp', inherit: ['NotSnake'] }; + let validatorThrew = false; + try { + validateShellJobParams(data, { config: fakeCfg }); + fakeQueueAdd(); + } catch { + validatorThrew = true; + } + expect(validatorThrew).toBe(true); + expect(queueAddCalled).toBe(false); + }); +});