From 33202fcd3a820b4257fa4cfcf78463103f820474 Mon Sep 17 00:00:00 2001 From: Robbie Blaine Date: Wed, 1 Jul 2026 13:32:37 +0200 Subject: [PATCH 1/3] Fix shell write bypass of write-only denies The command/code path-token scan in `decide` only checked read-class denies (`includeWrite = false`), so a path protected by a `Write(...)`/`Edit(...)` rule with no matching `Read(...)` deny could still be modified through a shell or eval command. The embedded `Edit(~/.bashrc)` deny, for instance, did not stop a shell append to that file. Shell and code execution can both read and write, and the token scan cannot tell which, so it now checks read- and write-class denies. A read-allowed but write-denied template (`.env.example`) is conservatively blocked in shell context; the read tool still permits it directly. Also tighten a `loadPolicyEntries` test to assert the string "123" is absent, so an accidental stringification of an invalid non-string entry would be caught. Glory to the Omnissiah --- extensions/rules-guard/index.test.ts | 24 +++++++++++++++++++++--- extensions/rules-guard/index.ts | 14 ++++++++------ 2 files changed, 29 insertions(+), 9 deletions(-) diff --git a/extensions/rules-guard/index.test.ts b/extensions/rules-guard/index.test.ts index 55ecac8..cb35c46 100644 --- a/extensions/rules-guard/index.test.ts +++ b/extensions/rules-guard/index.test.ts @@ -123,10 +123,12 @@ describe(".env goal — allow templates, deny the rest", () => { // Write(**/.env*) has no write-class allow → writing .env.example blocked. expect(writeBlocked("/work/.env.example", pol)).toBe(true); }); - test("shell read of a template is allowed (cat .env.example)", () => { + test("shell reference to a write-denied template is conservatively blocked", () => { + // The token scan can't tell `cat` from `echo >`, so a Write(**/.env*) deny now + // blocks .env.example in shell context; the read tool still permits it (see above). expect( decide("bash", { command: "cat .env.example" }, "/work", pol).block, - ).toBe(false); + ).toBe(true); }); test("shell read of a real .env is still blocked", () => { expect( @@ -418,6 +420,22 @@ describe("decide — adversarial bypass vectors", () => { false, ); }); + test("shell/code write to a Write-only-denied path is blocked (finding 1)", () => { + const pol = buildPolicy(["Edit(/work/target.conf)"], []); + // No matching Read deny — before the fix this bypassed the guard. + expect( + decide( + "bash", + { command: "echo evil >> /work/target.conf" }, + "/work", + pol, + ).block, + ).toBe(true); + expect( + decide("eval", { code: "open('/work/target.conf','w')" }, "/work", pol) + .block, + ).toBe(true); + }); }); describe("redactText (defense-in-depth)", () => { @@ -457,7 +475,7 @@ describe("loadPolicyEntries (settings file merge)", () => { const { deny, allow } = loadPolicyEntries([f]); expect(deny).toContain("Read(/x/y)"); expect(deny).toContain("Read(**/.env*)"); - expect(deny).not.toContain(123 as unknown as string); + expect(deny).not.toContain("123"); expect(allow).toContain("Read(/x/y/z)"); expect(allow).toEqual(expect.arrayContaining(EMBEDDED_ALLOW)); }); diff --git a/extensions/rules-guard/index.ts b/extensions/rules-guard/index.ts index 5efbc30..275b2c8 100644 --- a/extensions/rules-guard/index.ts +++ b/extensions/rules-guard/index.ts @@ -490,11 +490,13 @@ export function decide( if (hit) return { block: true, reason: fileMsg(raw, hit.src) }; } - // Command / code fields (bash/python/eval/browser/…): denied-command patterns, - // then a best-effort path-token scan so a Read(...) deny can't be sidestepped by - // a shell read. The token scan is read-class only (its sole purpose), so a - // read-allow (e.g. `.env.example`) lets `cat .env.example` through; true secrets - // keep a read-deny, so shell writes to them are still caught here. + // Command / code fields (bash/python/eval/browser/…): first denied-command + // patterns, then a path-token scan. Shell/code can BOTH read and write, and the + // scan can't tell which, so it checks read- AND write-class denies + // (includeWrite = true) — a Write(...)-only deny (e.g. `Edit(~/.bashrc)`) is thus + // enforced against `echo >> ~/.bashrc`. Trade-off: a read-allowed-but-write-denied + // path (`.env.example`) is conservatively blocked here, though the read tool + // still permits it. const shellText = fieldValues(inp, SHELL_FIELDS); for (const text of shellText) { for (const seg of bashSegments(text)) { @@ -510,7 +512,7 @@ export function decide( } for (const text of [...shellText, ...fieldValues(inp, CODE_FIELDS)]) { for (const tok of pathTokens(text)) { - const hit = blocked(candidateAbsPaths(tok, cwd), false); + const hit = blocked(candidateAbsPaths(tok, cwd), true); if (hit) return { block: true, reason: fileMsg(tok, hit.src) }; } } From 0cf058a4f865b58e6f9f89db5beca77072b0e5fd Mon Sep 17 00:00:00 2001 From: Robbie Blaine Date: Wed, 1 Jul 2026 13:56:44 +0200 Subject: [PATCH 2/3] Expand secret-output redaction patterns Broaden `redactText`'s `SECRET_OUTPUT` list from a handful of shapes to cover the common credential formats a tool might echo, as defense in depth on `tool_result`. - Widen the GitHub token pattern from `ghp_` to every prefix the `gh` CLI writes (`gho_`, `ghu_`, `ghs_`, `ghr_`). - Detect AWS secret access keys, but only in context (an `aws_secret_access_key`-style label plus value), since a bare 40-char base64 string is indistinguishable from a git SHA. - Redact credentials embedded in a connection URL (`scheme://user:password@host/...`), including the host, port, path, and any query-string secret, so a leaked DSN is fully masked. - Add Stripe, GitLab, Slack app tokens and webhooks, Google API/OAuth, npm, PyPI, SendGrid, DigitalOcean, Shopify, Twilio, Discord, and JWT. Bare high-entropy strings, git SHAs, and UUIDs are deliberately left untouched to keep false positives out of normal tool output. Add positive tests for every new shape, negative tests guarding the false-positive cases, and document the full list as a bullet list in the extension `README.md`. Glory to the Omnissiah --- extensions/rules-guard/README.md | 20 +++++++-- extensions/rules-guard/index.test.ts | 65 +++++++++++++++++++++++++++- extensions/rules-guard/index.ts | 41 ++++++++++++++---- 3 files changed, 111 insertions(+), 15 deletions(-) diff --git a/extensions/rules-guard/README.md b/extensions/rules-guard/README.md index 8d81d13..eac2e72 100644 --- a/extensions/rules-guard/README.md +++ b/extensions/rules-guard/README.md @@ -61,10 +61,22 @@ Denied bash command patterns. `Bash(...)` rules are matched against each command of a shell-command field (`command`, `cmd`, `script`), so `rm -rf *` or `git push --force` is blocked in `bash` and any other tool that carries such a field. -Secret-shaped output redaction on `tool_result`, as defense in depth. Substrings that look -like credentials are replaced with `[REDACTED]`: Anthropic (`sk-ant-…`), OpenAI and Stripe -(`sk-…`, `pk-…`), AWS access-key ids (`AKIA…`), GitHub PATs (`ghp_…`, `github_pat_…`), -Slack tokens (`xox[baprs]-…`), and PEM private-key blocks. +Secret-shaped output redaction on `tool_result`, as defense in depth. Substrings that +look like credentials are replaced with `[REDACTED]`: + +- Anthropic (`sk-ant-...`), OpenAI (`sk-...`, `pk-...`) +- Stripe (`sk_live_...`, `rk_test_...`) +- AWS access-key ids (`AKIA...`) and, in context, secret access keys +- GitHub tokens (`ghp_`/`gho_`/`ghu_`/`ghs_`/`ghr_...`, `github_pat_...`), GitLab (`glpat-...`) +- Slack (`xox[baprs]-...`, `xapp-...`, webhook URLs) +- Google (`AIza...`, `ya29....`, OAuth client ids) +- npm (`npm_...`), PyPI (`pypi-...`) +- SendGrid (`SG....`), DigitalOcean (`dop_v1_...`), Shopify (`shpat_...`), Twilio (`SK...`), Discord bot tokens +- JWTs and PEM private-key blocks +- Credentials embedded in connection URLs (`scheme://user:password@host/...`) + +Bare high-entropy strings, git SHAs, and UUIDs are deliberately not redacted, to keep +false positives out of normal tool output. When a call is blocked the model gets a specific reason instead of a silent failure. The reason names the matched rule, for example `Blocked by deny policy: "..." matches diff --git a/extensions/rules-guard/index.test.ts b/extensions/rules-guard/index.test.ts index cb35c46..60d66d8 100644 --- a/extensions/rules-guard/index.test.ts +++ b/extensions/rules-guard/index.test.ts @@ -393,7 +393,7 @@ describe("decide — adversarial bypass vectors", () => { expect( decide("read", { path: "/work/pub/../vault/k" }, "/work", pol).block, ).toBe(true); - // …and traversal OUT of a denied dir must NOT false-positive. + // ...and traversal OUT of a denied dir must NOT false-positive. expect( decide("read", { path: "/work/vault/../pub/k" }, "/work", pol).block, ).toBe(false); @@ -455,10 +455,71 @@ describe("redactText (defense-in-depth)", () => { const blob = `a ghp_${"a".repeat(36)} b AKIA${"1234567890ABCDEF"} c`; expect(redactText(blob)).toBe("a [REDACTED] b [REDACTED] c"); }); - test("leaves non-secret / too-short text unchanged (negative)", () => { + test("leaves non-secret / too-short / credential-free text unchanged (negative)", () => { expect(redactText("hello world")).toBe("hello world"); expect(redactText("ghp_tooshort")).toBe("ghp_tooshort"); expect(redactText("")).toBe(""); + // FP guards — secret-adjacent shapes that must NOT be redacted. + expect(redactText("a1b2c3d4e5".repeat(4))).toBe("a1b2c3d4e5".repeat(4)); // git SHA (40-hex) + expect(redactText("12345678-1234-1234-1234-123456789012")).toBe( + "12345678-1234-1234-1234-123456789012", + ); // UUID + expect(redactText("https://user@github.com/x")).toBe( + "https://user@github.com/x", + ); // URL user, no password + expect(redactText("https://example.com:8080/path")).toBe( + "https://example.com:8080/path", + ); // port, not credentials + }); +}); + +describe("redactText — provider token shapes (positive)", () => { + test("redacts newly-added provider token shapes", () => { + for (const s of [ + `gho_${"a".repeat(36)}`, // GitHub CLI OAuth token + `glpat-${"a".repeat(20)}`, + `xapp-${"1234567890abc"}`, + `AIza${"a".repeat(35)}`, + `ya29.${"a".repeat(30)}`, + `npm_${"a".repeat(36)}`, + `pypi-${"a".repeat(20)}`, + `SG.${"a".repeat(22)}.${"b".repeat(43)}`, + `sk_live_${"a".repeat(24)}`, + `dop_v1_${"a1b2c3d4".repeat(8)}`, + `shpat_${"a1b2c3d4".repeat(4)}`, + `SK${"a1b2c3d4".repeat(4)}`, + `M${"a".repeat(23)}.${"a".repeat(6)}.${"a".repeat(27)}`, + `123456789-${"a".repeat(32)}.apps.googleusercontent.com`, + `eyJ${"a".repeat(10)}.eyJ${"a".repeat(10)}.${"a".repeat(20)}`, + ]) + expect(redactText(s)).toBe("[REDACTED]"); + // AWS secret access key only redacts in context (label + value). + expect(redactText(`aws_secret_access_key = ${"A".repeat(40)}`)).toBe( + "[REDACTED]", + ); + // Slack webhook keeps the scheme, redacts the secret path. + expect( + redactText(`https://hooks.slack.com/services/T0/B0/${"a".repeat(20)}`), + ).toBe("https://[REDACTED]"); + }); +}); + +describe("redactText — credential URLs (positive)", () => { + test("redacts credentials embedded in connection URLs", () => { + expect( + redactText( + "postgres://postgresAdmin:posgresPassword@postgres:5432/my-db", + ), + ).toBe("[REDACTED]"); + expect(redactText("rediss://:password@redis:6379/0")).toBe("[REDACTED]"); + // secret in the query string is swept in with the DSN. + expect(redactText("postgres://u:p@h/db?password=hunter2")).toBe( + "[REDACTED]", + ); + // only the URL is redacted; surrounding JSON delimiters are preserved. + expect(redactText('{"url":"mysql://a:b@db/x"}')).toBe( + '{"url":"[REDACTED]"}', + ); }); }); diff --git a/extensions/rules-guard/index.ts b/extensions/rules-guard/index.ts index 275b2c8..cda0c09 100644 --- a/extensions/rules-guard/index.ts +++ b/extensions/rules-guard/index.ts @@ -26,7 +26,7 @@ * * NOT a sandbox: extensions run in-process and bash/eval can read bytes in ways a * text scan cannot fully enumerate (a bare `cat server.key` with no path separator, - * base64, custom interpreters, …). The only hard boundary is OS filesystem + * base64, custom interpreters, ...). The only hard boundary is OS filesystem * permissions — run omp as a user without read access to these paths, or in a * container where they are not mounted. This guard stops the common/accidental * paths and hands the model a clear, actionable reason. @@ -452,7 +452,7 @@ export interface Decision { } /** Pure block/allow decision for one tool call. Detection is field-driven so it - * covers every tool (read/write/edit/find/grep/python/eval/browser/…) regardless + * covers every tool (read/write/edit/find/grep/python/eval/browser/...) regardless * of the exact registered name. Exported for tests. */ export function decide( toolName: string, @@ -490,7 +490,7 @@ export function decide( if (hit) return { block: true, reason: fileMsg(raw, hit.src) }; } - // Command / code fields (bash/python/eval/browser/…): first denied-command + // Command / code fields (bash/python/eval/browser/...): first denied-command // patterns, then a path-token scan. Shell/code can BOTH read and write, and the // scan can't tell which, so it checks read- AND write-class denies // (includeWrite = true) — a Write(...)-only deny (e.g. `Edit(~/.bashrc)`) is thus @@ -523,13 +523,36 @@ export function decide( // ── Output redaction (defense in depth) ─────────────────────────────────────── const SECRET_OUTPUT: RegExp[] = [ - /\bsk-ant-[A-Za-z0-9_-]{16,}\b/g, // Anthropic - /\b(?:sk|pk)-[A-Za-z0-9_-]{16,}\b/g, // OpenAI / Stripe style + /\bsk-ant-[A-Za-z0-9_-]{16,}/g, // Anthropic (incl. sk-ant-api...) + /\b(?:sk|pk)-[A-Za-z0-9_-]{16,}/g, // OpenAI (incl. sk-proj-...) + /\b(?:sk|rk)_(?:live|test)_[A-Za-z0-9]{16,}/g, // Stripe secret / restricted keys /\bAKIA[0-9A-Z]{16}\b/g, // AWS access key id - /\bghp_[A-Za-z0-9]{36}\b/g, // GitHub PAT (classic) - /\bgithub_pat_[A-Za-z0-9_]{20,}\b/g, // GitHub PAT (fine-grained) - /\bxox[baprs]-[A-Za-z0-9-]{10,}\b/g, // Slack tokens - /-----BEGIN (?:[A-Z ]+ )?PRIVATE KEY-----[\s\S]*?-----END (?:[A-Z ]+ )?PRIVATE KEY-----/g, + // AWS secret access key: bare 40-char base64 is indistinguishable from a git SHA, + // so match only in context (an aws-secret-ish label followed by `=`/`:`). + /\baws_?secret_?access_?key[ \t]*[:=][ \t]*["']?[A-Za-z0-9/+]{40}/gi, + // GitHub token — PAT ghp_, OAuth/CLI gho_, user-to-server ghu_, server ghs_, refresh ghr_. + /\bgh[oprsu]_[A-Za-z0-9]{36}\b/g, + /\bgithub_pat_[A-Za-z0-9_]{20,}/g, // GitHub PAT (fine-grained) + /\bglpat-[A-Za-z0-9_-]{20,}/g, // GitLab PAT + /\b(?:xox[baprs]|xapp)-[A-Za-z0-9-]{10,}/g, // Slack tokens (bot/user/app/...) + /\bhooks\.slack\.com\/services\/[\w-]+\/[\w-]+\/[\w-]+/g, // Slack incoming webhook + /\bAIza[0-9A-Za-z_-]{35}\b/g, // Google API key + /\bya29\.[0-9A-Za-z_-]{20,}/g, // Google OAuth access token + /\bnpm_[A-Za-z0-9]{36}\b/g, // npm access token + /\bpypi-[A-Za-z0-9_-]{16,}/g, // PyPI API token + /\bSG\.[A-Za-z0-9_-]{22}\.[A-Za-z0-9_-]{43}/g, // SendGrid API key + /\beyJ[A-Za-z0-9_-]{6,}\.eyJ[A-Za-z0-9_-]{6,}\.[A-Za-z0-9_-]{6,}/g, // JWT (header.payload.signature) + /\bdop_v1_[a-f0-9]{64}\b/g, // DigitalOcean PAT + /\bshp(?:at|ca|pa|ss)_[a-fA-F0-9]{32}\b/g, // Shopify access token + /\bSK[0-9a-fA-F]{32}\b/g, // Twilio API key SID + /\b[MNO][\w-]{23}\.[\w-]{6}\.[\w-]{27,}/g, // Discord bot token + /\b[0-9]+-[0-9A-Za-z_]{32}\.apps\.googleusercontent\.com\b/g, // Google OAuth client id + // Credentials embedded in a connection URL (scheme://[user]:password@host/...). The + // password is required (`+`), so `https://user@host` and bare URLs are left alone. + // Match runs through the path/query (stopping at whitespace/quote/bracket) so a + // credentialed DSN's host, port, db name, and query secrets are all redacted. + /\b[a-z][a-z0-9+.-]*:\/\/[^\s:@/]*:[^\s@/]+@[^\s'"`<>)]+/gi, + /-----BEGIN (?:[A-Z ]+ )?PRIVATE KEY-----[\s\S]*?-----END (?:[A-Z ]+ )?PRIVATE KEY-----/g, // PEM private key ]; /** Redact secret-shaped substrings from tool output text. Exported for tests. */ From 9e12e7974266d8f1dbc5a53bc829eea189bbc160 Mon Sep 17 00:00:00 2001 From: Robbie Blaine Date: Wed, 1 Jul 2026 14:06:38 +0200 Subject: [PATCH 3/3] Assert reads stay allowed under a write-only deny Add the complementary regression assertion flagged by CodeRabbit to the `decide` finding-1 test: alongside verifying that shell and `eval` writes to an `Edit(...)`-only-denied path are blocked, assert that a plain read of the same path is still permitted when no matching read deny exists. This locks in the read/write class asymmetry the write-bypass fix depends on. Glory to the Omnissiah --- extensions/rules-guard/index.test.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/extensions/rules-guard/index.test.ts b/extensions/rules-guard/index.test.ts index 60d66d8..425c2d0 100644 --- a/extensions/rules-guard/index.test.ts +++ b/extensions/rules-guard/index.test.ts @@ -435,6 +435,8 @@ describe("decide — adversarial bypass vectors", () => { decide("eval", { code: "open('/work/target.conf','w')" }, "/work", pol) .block, ).toBe(true); + // Read-only access to the same write-denied path must remain permitted. + expect(readBlocked("/work/target.conf", pol)).toBe(false); }); });