From e75a69c9e15164d090e080fa001dc515495ec350 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Fri, 6 Nov 2026 22:56:32 -0800 Subject: [PATCH] fix: keep the gbrain database URL off argv; redact more credential formats (GST-1747) (#4114) * fix: keep the gbrain database URL off argv; redact more credential formats (GST-1747) /setup-gbrain runs gstack-gbrain-source-wireup without --database-url; the helper reads ~/.gbrain/config.json once at startup and pins the URL. A new guard test keeps setup-gbrain docs from passing the URL on argv. lib/redact-patterns.ts adds HIGH patterns for OpenRouter keys, GitHub App ghu_/ghr_ tokens, Stripe live restricted keys, Supabase sb_secret_ and sbp_ tokens, and matches temporary ASIA AWS key IDs in aws.access_key. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * chore: name the changelog fragment after the branch (GST-1747) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix: pre-landing review fixes Clear GBRAIN_DATABASE_URL and DATABASE_URL for the setup-gbrain source wireup call so the helper pins the ~/.gbrain/config.json URL, as the --database-url flag did before. Guard test added; changelog fragment scoped to the wireup step. Review-Finding: p1/specialist-batched#1 reader=security Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix: match full-length GitHub refresh tokens in the redaction gate github.refresh now accepts a body of 36 or more characters, so the 76-character refresh tokens GitHub issues match and are masked whole. Test samples use a 76-character body; the 35-character near-miss stays negative. Review-Finding: p3/adversarial-native#1 reader=adversarial-native Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --------- Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- changelog.d/capy-keep-the-gbrain-database.md | 2 + lib/redact-patterns.ts | 46 ++++++++++++++++- setup-gbrain/SKILL.md | 23 ++++----- setup-gbrain/SKILL.md.tmpl | 23 ++++----- test/redact-diff-prefix.test.ts | 6 +++ test/redact-engine.test.ts | 27 ++++++++++ test/setup-gbrain-db-url-argv.test.ts | 53 ++++++++++++++++++++ 7 files changed, 150 insertions(+), 30 deletions(-) create mode 100644 changelog.d/capy-keep-the-gbrain-database.md create mode 100644 test/setup-gbrain-db-url-argv.test.ts diff --git a/changelog.d/capy-keep-the-gbrain-database.md b/changelog.d/capy-keep-the-gbrain-database.md new file mode 100644 index 000000000..159e70c7c --- /dev/null +++ b/changelog.d/capy-keep-the-gbrain-database.md @@ -0,0 +1,2 @@ +- **/setup-gbrain's source wireup step no longer puts the gbrain database URL on the command line.** The step now runs `gstack-gbrain-source-wireup --strict` without `--database-url`, with `GBRAIN_DATABASE_URL` and `DATABASE_URL` cleared for the call, so the helper reads `~/.gbrain/config.json` once at startup and pins that URL for every `gbrain` call it makes, as the flag did before; a `DATABASE_URL` in the user's shell can't redirect it. The URL holds the database password, and command-line arguments are visible to other local users in `ps`. A new test keeps every setup-gbrain doc from passing the URL to the helper again. (GST-1747) +- **The redaction gate blocks more credential formats.** `lib/redact-patterns.ts` adds HIGH patterns for OpenRouter keys (`sk-or-v1-`), GitHub App user-to-server and refresh tokens (`ghu_`, `ghr_`), Stripe live restricted keys (`rk_live_`), Supabase secret keys (`sb_secret_`) and personal access tokens (`sbp_`), and `aws.access_key` now also matches temporary `ASIA` key IDs. `gstack-redact`, the pre-push hook and every skill that scans through the redaction engine pick them up. (GST-1747) diff --git a/lib/redact-patterns.ts b/lib/redact-patterns.ts index 1833ec1c9..1d0a54f7e 100644 --- a/lib/redact-patterns.ts +++ b/lib/redact-patterns.ts @@ -1161,8 +1161,8 @@ export const PATTERNS: RedactPattern[] = [ id: "aws.access_key", tier: "HIGH", category: "secret", - description: "AWS access key ID (AKIA…)", - regex: /\b(AKIA[0-9A-Z]{16})\b/, + description: "AWS access key ID (AKIA… long-term, ASIA… temporary)", + regex: /\b((?:AKIA|ASIA)[0-9A-Z]{16})\b/, }, { id: "aws.secret_key", @@ -1194,6 +1194,20 @@ export const PATTERNS: RedactPattern[] = [ description: "GitHub server-to-server token", regex: /\b(ghs_[A-Za-z0-9]{36})\b/, }, + { + id: "github.user_to_server", + tier: "HIGH", + category: "secret", + description: "GitHub App user-to-server token", + regex: /\b(ghu_[A-Za-z0-9]{36})\b/, + }, + { + id: "github.refresh", + tier: "HIGH", + category: "secret", + description: "GitHub App refresh token", + regex: /\b(ghr_[A-Za-z0-9]{36,})\b/, + }, { id: "github.fine_grained", tier: "HIGH", @@ -1289,6 +1303,13 @@ export const PATTERNS: RedactPattern[] = [ regex: /\b(sk-(?:proj|svcacct|admin)-[A-Za-z0-9_-]{20,}|sk-[A-Za-z0-9]{32,})\b/, }, + { + id: "openrouter.key", + tier: "HIGH", + category: "secret", + description: "OpenRouter API key", + regex: /\b(sk-or-v1-[a-f0-9]{64})\b/, + }, { id: "sendgrid.key", tier: "HIGH", @@ -1303,6 +1324,27 @@ export const PATTERNS: RedactPattern[] = [ description: "Stripe live SECRET key", regex: /\b(sk_live_[A-Za-z0-9]{24,})\b/, }, + { + id: "stripe.restricted", + tier: "HIGH", + category: "secret", + description: "Stripe live restricted key", + regex: /\b(rk_live_[A-Za-z0-9]{24,})\b/, + }, + { + id: "supabase.secret", + tier: "HIGH", + category: "secret", + description: "Supabase secret API key", + regex: /\b(sb_secret_[A-Za-z0-9_-]{20,})\b/, + }, + { + id: "supabase.pat", + tier: "HIGH", + category: "secret", + description: "Supabase personal access token", + regex: /\b(sbp_[a-f0-9]{40})\b/, + }, { id: "slack.token", tier: "HIGH", diff --git a/setup-gbrain/SKILL.md b/setup-gbrain/SKILL.md index c7ba1b728..97b60bed2 100644 --- a/setup-gbrain/SKILL.md +++ b/setup-gbrain/SKILL.md @@ -485,22 +485,17 @@ any gbrain client. The helper creates a `git worktree` of `~/.gstack/`, registers it as a federated source via `gbrain sources add --path --federated`, and runs an initial `gbrain sync`. Local-stdio paths only. -Capture the database URL out of `~/.gbrain/config.json` first and pass it -explicitly so the wireup is robust against any other process rewriting -`~/.gbrain/config.json` mid-sync (e.g., concurrent `gbrain init` runs -elsewhere on the machine): +The helper reads the database URL from `~/.gbrain/config.json` once at +startup and pins it for every `gbrain` call it makes, so another process +rewriting that file mid-sync (e.g., a concurrent `gbrain init`) can't redirect +it. Don't pass the URL as an argument: it holds the database password, and +arguments are visible to other local users in `ps`. Clear +`GBRAIN_DATABASE_URL` and `DATABASE_URL` for the call: the helper ranks both +above `config.json`, and a shell's `DATABASE_URL` often points at an app's +own database. ```bash -GBRAIN_URL=$(python3 -c " -import json, os, sys -try: - c = json.load(open(os.path.expanduser('~/.gbrain/config.json'))) - print(c.get('database_url', '')) -except Exception: - pass -") -~/.claude/skills/gstack/bin/gstack-gbrain-source-wireup --strict \ - ${GBRAIN_URL:+--database-url "$GBRAIN_URL"} +GBRAIN_DATABASE_URL= DATABASE_URL= ~/.claude/skills/gstack/bin/gstack-gbrain-source-wireup --strict ``` `--strict` exits non-zero on missing prereqs (gbrain not installed, < 0.18.0, diff --git a/setup-gbrain/SKILL.md.tmpl b/setup-gbrain/SKILL.md.tmpl index 5e3a96fd8..6301aac2e 100644 --- a/setup-gbrain/SKILL.md.tmpl +++ b/setup-gbrain/SKILL.md.tmpl @@ -400,22 +400,17 @@ any gbrain client. The helper creates a `git worktree` of `~/.gstack/`, registers it as a federated source via `gbrain sources add --path --federated`, and runs an initial `gbrain sync`. Local-stdio paths only. -Capture the database URL out of `~/.gbrain/config.json` first and pass it -explicitly so the wireup is robust against any other process rewriting -`~/.gbrain/config.json` mid-sync (e.g., concurrent `gbrain init` runs -elsewhere on the machine): +The helper reads the database URL from `~/.gbrain/config.json` once at +startup and pins it for every `gbrain` call it makes, so another process +rewriting that file mid-sync (e.g., a concurrent `gbrain init`) can't redirect +it. Don't pass the URL as an argument: it holds the database password, and +arguments are visible to other local users in `ps`. Clear +`GBRAIN_DATABASE_URL` and `DATABASE_URL` for the call: the helper ranks both +above `config.json`, and a shell's `DATABASE_URL` often points at an app's +own database. ```bash -GBRAIN_URL=$(python3 -c " -import json, os, sys -try: - c = json.load(open(os.path.expanduser('~/.gbrain/config.json'))) - print(c.get('database_url', '')) -except Exception: - pass -") -~/.claude/skills/gstack/bin/gstack-gbrain-source-wireup --strict \ - ${GBRAIN_URL:+--database-url "$GBRAIN_URL"} +GBRAIN_DATABASE_URL= DATABASE_URL= ~/.claude/skills/gstack/bin/gstack-gbrain-source-wireup --strict ``` `--strict` exits non-zero on missing prereqs (gbrain not installed, < 0.18.0, diff --git a/test/redact-diff-prefix.test.ts b/test/redact-diff-prefix.test.ts index 5e4c25212..b7a2bdacd 100644 --- a/test/redact-diff-prefix.test.ts +++ b/test/redact-diff-prefix.test.ts @@ -19,6 +19,8 @@ const SAMPLES: Record = { "github.pat": j("ghp_", "a1B2c3D4e5F6g7H8i9J0k1L2m3N4o5P6q7R8"), "github.oauth": j("gho_", "a1B2c3D4e5F6g7H8i9J0k1L2m3N4o5P6q7R8"), "github.server": j("ghs_", "a1B2c3D4e5F6g7H8i9J0k1L2m3N4o5P6q7R8"), + "github.user_to_server": j("ghu_", "a1B2c3D4e5F6g7H8i9J0k1L2m3N4o5P6q7R8"), + "github.refresh": j("ghr_", "a1B2c3D4e5F6g7H8i9J0".repeat(3), "k1L2m3N4o5P6q7R8"), "github.fine_grained": j("github_pat_", "a1B2c3D4e5F6g7H8i9J0".repeat(4), "kL"), "gitlab.token": j("glpat-", "a1B2c3D4e5F6g7H8i9J0"), "groq.key": j("gsk_", "a1B2c3D4e5F6g7H8i9J0k1L2"), @@ -31,7 +33,11 @@ const SAMPLES: Record = { "anthropic.key": j("sk-ant-", "api03-a1B2c3D4e5F6g7H8i9J0k1L2"), "openai.key": j("sk-proj-", "a1B2c3D4e5F6g7H8i9J0k1L2m3N4o5P6q7R8s9T0u1V2w3X4"), "sendgrid.key": j("SG.", "a1B2c3D4e5F6g7H8i9J0k1", ".", "a1B2c3D4e5F6g7H8i9J0k1L2m3N4o5P6q7R8s9T0u1V"), + "openrouter.key": j("sk-or-v1-", hex32, hex32), "stripe.secret": j("sk_live_", "a1B2c3D4e5F6g7H8i9J0k1L2"), + "stripe.restricted": j("rk_live_", "a1B2c3D4e5F6g7H8i9J0k1L2"), + "supabase.secret": j("sb_secret_", "tR4i6cyMIrYTE3s6OyHGHw_ppx2p6WK"), + "supabase.pat": j("sbp_", hex32, "9f8e7d6c"), "slack.token": j("xoxb-", "12345", "67890-a1B2c3D4e5F6"), "slack.webhook": j("https://hooks.slack.com/services/", "T0ABCDEFG/B0ABCDEFG/", "a1B2c3D4e5F6g7H8i9J0k1L2"), "discord.webhook": j("https://discord.com/api/webhooks/", "123456789012345678/", "a1B2c3D4e5F6g7H8i9J0k1L2m3N4o5P6q7R8s9T0u1V2w3X4y5Z6a7B8c9D0e1F2"), diff --git a/test/redact-engine.test.ts b/test/redact-engine.test.ts index 0bdef1556..98bf89f50 100644 --- a/test/redact-engine.test.ts +++ b/test/redact-engine.test.ts @@ -62,6 +62,13 @@ describe("HIGH credential patterns", () => { ], ["google.oauth_client_secret", 'client_secret: "GOCSPX-' + "Ab3xQ9zLmNp2RtVw7YkD1sHf" + '"'], ["telegram.bot_token", "TELEGRAM_TOKEN=8326208591:AA" + "HdqRy9Lm2ZpXvKb4NcQw8TuEr6YoP1sVg"], + ["aws.access_key", "temporary key ASIA" + "ZQ3XN7P2LM4RT8WV"], + ["github.user_to_server", "ghu_" + "a1B2c3D4e5F6g7H8i9J0k1L2m3N4o5P6q7R8"], + ["github.refresh", "ghr_" + "a1B2c3D4e5F6g7H8i9J0".repeat(3) + "k1L2m3N4o5P6q7R8"], + ["openrouter.key", "OPENROUTER sk-or-v1-" + "9f8e7d6c5b4a39281706f5e4d3c2b1a0".repeat(2)], + ["stripe.restricted", "rk_live_" + "a1B2c3D4e5F6g7H8i9J0k1L2"], + ["supabase.secret", "sb_secret_" + "tR4i6cyMIrYTE3s6OyHGHw_ppx2p6WK"], + ["supabase.pat", "sbp_" + "9f8e7d6c5b4a39281706f5e4d3c2b1a09f8e7d6c"], ]; for (const [id, text] of cases) { test(`flags ${id}`, () => { @@ -463,6 +470,20 @@ describe("#1946 pattern negatives (placeholders never fire)", () => { }); }); +describe("OpenRouter, GitHub ghu_/ghr_, Stripe restricted, Supabase and AWS temporary key negatives", () => { + test("near-miss shapes never fire", () => { + expect(ids("sk-or-v1-" + "9f8e7d6c5b4a39281706f5e4d3c2b1a0")).not.toContain("openrouter.key"); + expect(ids("ghu_" + "a1B2c3D4e5F6g7H8i9J0k1L2m3N4o5P6q7R")).not.toContain("github.user_to_server"); + expect(ids("ghr_" + "a1B2c3D4e5F6g7H8i9J0k1L2m3N4o5P6q7R")).not.toContain("github.refresh"); + expect(ids("rk_test_" + "a1B2c3D4e5F6g7H8i9J0k1L2")).not.toContain("stripe.restricted"); + expect(ids("sb_publishable_" + "tR4i6cyMIrYTE3s6OyHGHw_ppx2p6WK")).not.toContain("supabase.secret"); + expect(ids("sb_secret_key")).not.toContain("supabase.secret"); + expect(ids("sbp_" + "9f8e7d6c5b4a39281706f5e4d3c2b1a09f8e7d6")).not.toContain("supabase.pat"); + expect(ids("sbp_" + "Zf8e7d6c5b4a39281706f5e4d3c2b1a09f8e7d6c")).not.toContain("supabase.pat"); + expect(ids("ASIA" + "ZQ3XN7P2LM4RT8W")).not.toContain("aws.access_key"); + }); +}); + describe("google.oauth_client_secret / telegram.bot_token negatives", () => { test("undersized and placeholder shapes never fire", () => { // Length floor keeps short repo fixtures quiet (e.g. the 19-char body in @@ -749,6 +770,12 @@ describe("redactFindingSpans — machine-egress masking (#1947)", () => { expect(out).toBe("auth rejected"); }); + test("a 76-character GitHub refresh token is masked whole", () => { + const token = "ghr_" + "a1B2c3D4e5F6g7H8i9J0".repeat(3) + "k1L2m3N4o5P6q7R8"; + const out = redactFindingSpans(`auth ${token} rejected`, { repoVisibility: "private" }); + expect(out).toBe("auth rejected"); + }); + test("multiple findings are all replaced (right-to-left splice keeps offsets valid)", () => { const pat = "ghp_" + "1234567890abcdefghijklmnopqrstuvwxyz"; const aws = "AKIA1234567890ABCDEF"; diff --git a/test/setup-gbrain-db-url-argv.test.ts b/test/setup-gbrain-db-url-argv.test.ts new file mode 100644 index 000000000..e27691241 --- /dev/null +++ b/test/setup-gbrain-db-url-argv.test.ts @@ -0,0 +1,53 @@ +// setup-gbrain keeps the gbrain database URL off the process command line. +// +// The URL carries the Postgres password. A value passed as an argument is +// readable by every local user through `ps` for as long as the process runs, +// so gstack passes credentials through env or a config file, never argv (the +// same rule lib/gbrain-supabase-provision.ts states for DB_PASS). The wireup +// helper already reads ~/.gbrain/config.json once at startup and pins it for +// every child gbrain call (covered in test/gstack-gbrain-source-wireup.test.ts), +// so no skill doc needs to hand it the URL. + +import { describe, test, expect } from 'bun:test'; +import * as fs from 'fs'; +import * as path from 'path'; + +const ROOT = path.resolve(import.meta.dir, '..'); +const SKILL_DIR = path.join(ROOT, 'setup-gbrain'); +const SECTIONS_DIR = path.join(SKILL_DIR, 'sections'); + +const docs: Array<[string, string]> = [ + 'SKILL.md.tmpl', + 'SKILL.md', + ...fs + .readdirSync(SECTIONS_DIR) + .filter((f) => f.endsWith('.md.tmpl') || f.endsWith('.md')) + .sort() + .map((f) => path.join('sections', f)), +].map((rel) => [rel, fs.readFileSync(path.join(SKILL_DIR, rel), 'utf-8')]); + +// One shell command: the helper name plus any backslash-continued lines. +const wireupCommand = /gstack-gbrain-source-wireup\b(?:[^\n]*\\\r?\n)*[^\n]*/g; + +describe('setup-gbrain passes no database URL on argv', () => { + test('the wireup command is still documented (guard is not vacuous)', () => { + const invocations = docs.flatMap(([, text]) => text.match(wireupCommand) ?? []); + expect(invocations.some((cmd) => cmd.includes('--strict'))).toBe(true); + }); + + test('no gstack-gbrain-source-wireup invocation carries --database-url', () => { + const offenders = docs.flatMap(([rel, text]) => + (text.match(wireupCommand) ?? []).filter((cmd) => cmd.includes('--database-url')).map((cmd) => `${rel}: ${cmd}`), + ); + expect(offenders).toEqual([]); + }); + + test('the wireup command clears ambient database URLs so config.json wins', () => { + const lines = docs.flatMap(([rel, text]) => + text.split(/\r?\n/).filter((l) => /bin\/gstack-gbrain-source-wireup --strict/.test(l)).map((l) => `${rel}: ${l.trim()}`), + ); + expect(lines.length).toBeGreaterThan(0); + const offenders = lines.filter((l) => !/: GBRAIN_DATABASE_URL= DATABASE_URL= ~\/\.claude\//.test(l)); + expect(offenders).toEqual([]); + }); +});