fix(plugins): widen secret-scrub to bare-KEY and credentialed URLs

Author: TheGreatAxios

src/plugins/tool-result-secret-scrub.test.ts

@@ -42,6 +42,30 @@ MIIEpAIBAAKCAQEA7
       "See docs/authentication.md for how API keys are issued and rotated. No secrets in this paragraph.";
     expect(scrubSecretShapedContent(text)).toBe(text);
   });
+
+  test("redacts bare-KEY env assignments (CL-10756)", () => {
+    for (const key of ["OPENAI_KEY", "STRIPE_KEY", "GITHUB_KEY"]) {
+      const text = `./app/.env:7:${key}=proj-live-value-9z`;
+      const out = scrubSecretShapedContent(text);
+      expect(out).toContain(`${key}=${CREDENTIAL_REDACTION}`);
+      expect(out).not.toContain("proj-live-value-9z");
+    }
+  });
+
+  test("redacts credential-bearing DATABASE_URL connection strings (CL-10756)", () => {
+    const text =
+      "./app/.env:2:DATABASE_URL=postgres://admin:s3cret-pw@db.internal:5432/appdb";
+    const out = scrubSecretShapedContent(text);
+    expect(out).toContain(`DATABASE_URL=${CREDENTIAL_REDACTION}`);
+    expect(out).not.toContain("s3cret-pw");
+  });
+
+  test("leaves non-secret KEY words and credential-less URLs alone (CL-10756)", () => {
+    expect(scrubSecretShapedContent("MONKEY=banana")).toBe("MONKEY=banana");
+    expect(scrubSecretShapedContent("DATABASE_URL=sqlite:///local.db")).toBe(
+      "DATABASE_URL=sqlite:///local.db",
+    );
+  });
 });
 
 describe("scrubSecretShapedValue", () => {
@@ -219,6 +243,24 @@ describe("toolResultSecretScrubPlugin", () => {
     expect(result.content).toContain(CREDENTIAL_REDACTION);
     expect(result.content).not.toContain("supersecretvalue");
   });
+  test("scrubs bare-KEY and DATABASE_URL through the shared middleware path (CL-10756)", async () => {
+    const plugin = toolResultSecretScrubPlugin();
+    if (plugin.middleware === undefined)
+      throw new Error("expected middleware plugin");
+    const handler = plugin.middleware(
+      next(
+        "secrets/.env:7:OPENAI_KEY=proj-live-value-9z\nsecrets/.env:2:DATABASE_URL=postgres://admin:s3cret-pw@db.internal:5432/appdb",
+      ),
+    );
+    const result = await handler(
+      { id: "c2", name: "read_file", arguments: { path: "secrets/.env" } },
+      new AbortController().signal,
+    );
+    expect(result.content).toContain(`OPENAI_KEY=${CREDENTIAL_REDACTION}`);
+    expect(result.content).toContain(`DATABASE_URL=${CREDENTIAL_REDACTION}`);
+    expect(result.content).not.toContain("proj-live-value-9z");
+    expect(result.content).not.toContain("s3cret-pw");
+  });
   test("scrubs error results without stringifying object content", async () => {
     const plugin = toolResultSecretScrubPlugin();
     if (plugin.middleware === undefined)

src/plugins/tool-result-secret-scrub.ts

@@ -27,15 +27,37 @@ const JSON_CREDENTIAL_FIELD =
 // Grep/shell lines often look like path:line:KEY=value
 const ENV_ASSIGNMENT = /(?:^|:)([A-Z][A-Z0-9_]+)=([^\n]+)/gm;
 
-// Connection-string keys (DATABASE_URL and friends) are deliberately NOT
-// matched here. Widening this shape-classifier would redact every benign
-// connection string in tool output — a false-positive blast radius on a scrub
-// path, not a prompt path. The gap stays documented, not silently fixed.
-function isSecretEnvKey(key: string): boolean {
+// Connection-string env names (DATABASE_URL and friends). Redaction additionally
+// requires a credential-bearing value (see carriesConnectionCredentials), so
+// benign sqlite paths and bare hostnames in tool output stay readable.
+function isConnectionStringEnvKey(key: string): boolean {
   return (
+    /(?:_|^)(?:URL|URI|DSN)$/.test(key) || key.includes("CONNECTION_STRING")
+  );
+}
+
+// True when a connection-string value actually carries credentials: a URI with
+// userinfo (scheme://user:pass@host/...) or embedded password material.
+function carriesConnectionCredentials(value: string): boolean {
+  if (!value.includes("://")) return false;
+  return (
+    value.slice(value.indexOf("://") + 3).includes("@") ||
+    /password=/i.test(value)
+  );
+}
+
+function isSecretEnvKey(key: string, value: string): boolean {
+  if (
     key === "API_KEY" ||
     /(?:SECRET|TOKEN|PASSWORD|PRIVATE|CREDENTIAL|AUTH)/.test(key)
-  );
+  ) {
+    return true;
+  }
+  // Bare-KEY class (OPENAI_KEY, STRIPE_KEY, *_API_KEY): ENV-assignment scope
+  // only. The (^|_) boundary keeps plain words ending in KEY (MONKEY) out.
+  if (/(?:^|_)KEY$/.test(key)) return true;
+  if (isConnectionStringEnvKey(key)) return carriesConnectionCredentials(value);
+  return false;
 }
 
 function replaceAll(
@@ -65,12 +87,15 @@ export function scrubSecretShapedContent(text: string): string {
       `${prefix}${CREDENTIAL_REDACTION}${suffix}`,
   );
 
-  result = result.replace(ENV_ASSIGNMENT, (match, key: string) => {
-    if (!isSecretEnvKey(key)) return match;
-    const prefixEnd = match.lastIndexOf(key);
-    const prefix = match.slice(0, prefixEnd);
-    return `${prefix}${key}=${CREDENTIAL_REDACTION}`;
-  });
+  result = result.replace(
+    ENV_ASSIGNMENT,
+    (match, key: string, value: string) => {
+      if (!isSecretEnvKey(key, value)) return match;
+      const prefixEnd = match.lastIndexOf(key);
+      const prefix = match.slice(0, prefixEnd);
+      return `${prefix}${key}=${CREDENTIAL_REDACTION}`;
+    },
+  );
 
   return result;
 }