Fix shell write bypass of write-only denies

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

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(
@@ -391,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);
@@ -418,6 +420,24 @@ 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);
+    // Read-only access to the same write-denied path must remain permitted.
+    expect(readBlocked("/work/target.conf", pol)).toBe(false);
+  });
 });
 
 describe("redactText (defense-in-depth)", () => {
@@ -437,10 +457,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]"}',
+    );
   });
 });
 
@@ -457,7 +538,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));
     });

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,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) };
     }
   }
@@ -521,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. */