fix(plugins): stop secret scrub from eating plain URLs

Author: TheGreatAxios

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

@@ -66,6 +66,39 @@ MIIEpAIBAAKCAQEA7
       "DATABASE_URL=sqlite:///local.db",
     );
   });
+
+  test("leaves scoped npm URLs and email-query URLs readable", () => {
+    const registry = "REGISTRY_URL=https://registry.npmjs.org/@scope/pkg";
+    const docs = "DOCS_URL=https://medium.com/@octocat/hello";
+    const support =
+      "SUPPORT_URL=https://example.com/help?email=dev@example.com";
+    expect(scrubSecretShapedContent(registry)).toBe(registry);
+    expect(scrubSecretShapedContent(docs)).toBe(docs);
+    expect(scrubSecretShapedContent(support)).toBe(support);
+  });
+
+  test("redacts user:pass@host connection URLs", () => {
+    const text = "DATABASE_URL=postgres://admin:foo@db.internal:5432/appdb";
+    const out = scrubSecretShapedContent(text);
+    expect(out).toBe(`DATABASE_URL=${CREDENTIAL_REDACTION}`);
+    expect(out).not.toContain("admin:foo");
+  });
+
+  test("redacts the whole env value when it contains its own key", () => {
+    const text = "OPENAI_KEY=proj-OPENAI_KEY-live-value-9z";
+    const out = scrubSecretShapedContent(text);
+    expect(out).toBe(`OPENAI_KEY=${CREDENTIAL_REDACTION}`);
+    expect(out).not.toContain("proj-");
+    expect(out).not.toContain("live-value-9z");
+  });
+
+  test("redacts Password= DSNs that lack a scheme", () => {
+    const text =
+      "DATABASE_URL=Server=db.internal;Database=appdb;User Id=admin;Password=s3cret-pw;";
+    const out = scrubSecretShapedContent(text);
+    expect(out).toBe(`DATABASE_URL=${CREDENTIAL_REDACTION}`);
+    expect(out).not.toContain("s3cret-pw");
+  });
 });
 
 describe("scrubSecretShapedValue", () => {

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

@@ -36,24 +36,36 @@ function isConnectionStringEnvKey(key: string): boolean {
   );
 }
 
-// True when a connection-string value actually carries credentials: a URI with
-// userinfo (scheme://user:pass@host/...) or embedded password material.
+// True when a connection-string value actually carries credentials: URI
+// userinfo of the form scheme://user:pass@host (a colon in the authority
+// before @, and before the next / or ?), or embedded password= material.
+// ADO-style DSNs carry Password= without a scheme, so the password check
+// does not require ://. An @ in the path or query (npm scopes, profile
+// URLs, email query params) is not userinfo.
 function carriesConnectionCredentials(value: string): boolean {
-  if (!value.includes("://")) return false;
-  return (
-    value.slice(value.indexOf("://") + 3).includes("@") ||
-    /password=/i.test(value)
-  );
+  return /password=/i.test(value) || hasCredentialUserinfo(value);
+}
+
+function hasCredentialUserinfo(value: string): boolean {
+  const schemeSep = value.indexOf("://");
+  if (schemeSep < 0) return false;
+  const rest = value.slice(schemeSep + 3);
+  const path = rest.indexOf("/");
+  const query = rest.indexOf("?");
+  let authorityEnd = rest.length;
+  if (path >= 0) authorityEnd = Math.min(authorityEnd, path);
+  if (query >= 0) authorityEnd = Math.min(authorityEnd, query);
+  const authority = rest.slice(0, authorityEnd);
+  const at = authority.indexOf("@");
+  if (at < 0) return false;
+  return authority.slice(0, at).includes(":");
 }
 
 function isSecretEnvKey(key: string, value: string): boolean {
-  if (
-    key === "API_KEY" ||
-    /(?:SECRET|TOKEN|PASSWORD|PRIVATE|CREDENTIAL|AUTH)/.test(key)
-  ) {
+  if (/(?:SECRET|TOKEN|PASSWORD|PRIVATE|CREDENTIAL|AUTH)/.test(key)) {
     return true;
   }
-  // Bare-KEY class (OPENAI_KEY, STRIPE_KEY, *_API_KEY): ENV-assignment scope
+  // 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);
@@ -91,8 +103,8 @@ export function scrubSecretShapedContent(text: string): string {
     ENV_ASSIGNMENT,
     (match, key: string, value: string) => {
       if (!isSecretEnvKey(key, value)) return match;
-      const prefixEnd = match.lastIndexOf(key);
-      const prefix = match.slice(0, prefixEnd);
+      const assignment = `${key}=${value}`;
+      const prefix = match.slice(0, match.length - assignment.length);
       return `${prefix}${key}=${CREDENTIAL_REDACTION}`;
     },
   );