From 6d157e2f4d0c3cd0f9672ff603b316fa9fb5c13f Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Fri, 16 Oct 2026 18:19:10 -0700 Subject: [PATCH 1/2] fix(plugins): widen secret-scrub to bare-KEY and credentialed URLs --- src/plugins/tool-result-secret-scrub.test.ts | 42 +++++++++++++++++ src/plugins/tool-result-secret-scrub.ts | 49 +++++++++++++++----- 2 files changed, 79 insertions(+), 12 deletions(-) diff --git a/src/plugins/tool-result-secret-scrub.test.ts b/src/plugins/tool-result-secret-scrub.test.ts index ae601fb1e..d42ae8c1c 100644 --- a/src/plugins/tool-result-secret-scrub.test.ts +++ b/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) diff --git a/src/plugins/tool-result-secret-scrub.ts b/src/plugins/tool-result-secret-scrub.ts index 9fffe104e..5ff6fcd41 100644 --- a/src/plugins/tool-result-secret-scrub.ts +++ b/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; } From 8a380351c1c6b295d5d699b66b0fb4b38cb13404 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Fri, 16 Oct 2026 18:33:52 -0700 Subject: [PATCH 2/2] fix(plugins): stop secret scrub from eating plain URLs --- src/plugins/tool-result-secret-scrub.test.ts | 33 ++++++++++++++++ src/plugins/tool-result-secret-scrub.ts | 40 +++++++++++++------- 2 files changed, 59 insertions(+), 14 deletions(-) diff --git a/src/plugins/tool-result-secret-scrub.test.ts b/src/plugins/tool-result-secret-scrub.test.ts index d42ae8c1c..c98df19a3 100644 --- a/src/plugins/tool-result-secret-scrub.test.ts +++ b/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", () => { diff --git a/src/plugins/tool-result-secret-scrub.ts b/src/plugins/tool-result-secret-scrub.ts index 5ff6fcd41..fea0d5741 100644 --- a/src/plugins/tool-result-secret-scrub.ts +++ b/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}`; }, );