Files
gbrain/test/minions-shell-redact.test.ts
T
e227965024 v0.36.5.0 feat: secure DATABASE_URL access for shell jobs (inherit: ["database_url"]) (#1192)
* 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 <noreply@anthropic.com>

* 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 <noreply@anthropic.com>

* 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 `<REDACTED:name>` 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 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
2026-05-19 13:12:40 -07:00

113 lines
4.6 KiB
TypeScript

/**
* 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 `<REDACTED:name>`.
* - 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 <REDACTED:database_url>');
});
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: <REDACTED:database_url>, again: <REDACTED:database_url>, 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 <REDACTED:database_url> with key <REDACTED:anthropic_api_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('<REDACTED:database_url> 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: <REDACTED:foo> 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 <REDACTED:multi> 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: <REDACTED:long_token>, inner: <REDACTED:short_token>');
});
});