From 6cfdd7501015c5617d6f437c4e2dcf5a7a3c1199 Mon Sep 17 00:00:00 2001 From: DavertMik Date: Sat, 11 Jul 2026 13:44:54 +0300 Subject: [PATCH 1/2] fix: redact interpolated secrets from telemetry and experience files MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Knowledge files interpolate ${env.SECRET} into cleartext at load time; those resolved values flowed into two persisted/exported sinks — the Langfuse trace input and on-disk (often git-committed) experience files. Separately, ${config.*} could expand credential leaves like ${config.ai.apiKey} into every prompt and both sinks. Add src/utils/secrets.ts (a process-global secret registry + redactSecrets). The env interpolation branch registers credential-named values (still returning cleartext, since the model must type the real password) and both sinks now run redactSecrets on the serialized/content string. Credential-named ${config.*} paths are blocked from interpolating at all. Deferred (see plan 003 Maintenance notes): masking in the LLM request itself, the AI SDK experimental_telemetry inputs, and the Testomat.io reporter. Co-Authored-By: Claude Opus 4.8 --- src/experience-tracker.ts | 3 ++- src/knowledge-tracker.ts | 8 ++++++- src/observability.ts | 3 ++- src/utils/secrets.ts | 29 ++++++++++++++++++++++ tests/unit/knowledge-tracker.test.ts | 28 ++++++++++++++++++++++ tests/unit/secrets.test.ts | 36 ++++++++++++++++++++++++++++ 6 files changed, 104 insertions(+), 3 deletions(-) create mode 100644 src/utils/secrets.ts create mode 100644 tests/unit/secrets.test.ts diff --git a/src/experience-tracker.ts b/src/experience-tracker.ts index 3ae2b6e..7560815 100644 --- a/src/experience-tracker.ts +++ b/src/experience-tracker.ts @@ -8,6 +8,7 @@ import { KnowledgeTracker } from './knowledge-tracker.js'; import type { WebPageState } from './state-manager.js'; import { createDebug, tag } from './utils/logger.js'; import { mdq } from './utils/markdown-query.js'; +import { redactSecrets } from './utils/secrets.js'; import { isNonReusableCode } from './utils/step-analyzer.ts'; import { extractStatePath } from './utils/url-matcher.js'; @@ -109,7 +110,7 @@ export class ExperienceTracker { return; } const filePath = this.getExperienceFilePath(stateHash); - const fileContent = matter.stringify(content, frontmatter || {}); + const fileContent = matter.stringify(redactSecrets(content), frontmatter || {}); writeFileSync(filePath, fileContent, 'utf8'); } diff --git a/src/knowledge-tracker.ts b/src/knowledge-tracker.ts index d37ed9c..44d4b67 100644 --- a/src/knowledge-tracker.ts +++ b/src/knowledge-tracker.ts @@ -5,6 +5,7 @@ import { ActionResult } from './action-result.js'; import { ConfigParser } from './config.js'; import { getCliName } from './utils/cli-name.ts'; import { createDebug } from './utils/logger.js'; +import { isSecretName, registerSecret } from './utils/secrets.js'; const debugLog = createDebug('explorbot:knowledge-tracker'); @@ -138,9 +139,14 @@ export class KnowledgeTracker { const namespace = expr.slice(0, dotIndex); const key = expr.slice(dotIndex + 1); - if (namespace === 'env') return process.env[key] ?? ''; + if (namespace === 'env') { + const value = process.env[key] ?? ''; + if (isSecretName(key)) registerSecret(value); + return value; + } if (namespace === 'config') { + if (isSecretName(key)) return ''; const config = ConfigParser.getInstance().getConfig(); const value = key.split('.').reduce((obj: any, k) => obj?.[k], config); if (value !== undefined && typeof value !== 'object') return String(value); diff --git a/src/observability.ts b/src/observability.ts index bb1b6ab..cb9e36d 100644 --- a/src/observability.ts +++ b/src/observability.ts @@ -1,4 +1,5 @@ import { type Span, trace } from '@opentelemetry/api'; +import { redactSecrets } from './utils/secrets.js'; type TelemetryMetadata = Record; @@ -84,7 +85,7 @@ function buildRootSpanAttributes(name: string, metadata: TelemetryMetadata): Rec attributes['langfuse.trace.tags'] = metadata.tags as string[]; } if (metadata.input !== undefined) { - attributes['langfuse.trace.input'] = JSON.stringify(metadata.input); + attributes['langfuse.trace.input'] = redactSecrets(JSON.stringify(metadata.input)); } return attributes; diff --git a/src/utils/secrets.ts b/src/utils/secrets.ts new file mode 100644 index 0000000..a469101 --- /dev/null +++ b/src/utils/secrets.ts @@ -0,0 +1,29 @@ +const MIN_SECRET_LENGTH = 4; +const secretValues = new Set(); + +export function registerSecret(value: string): void { + if (!value) return; + if (value.length < MIN_SECRET_LENGTH) return; + secretValues.add(value); +} + +export function redactSecrets(text: string): string { + if (!text) return text; + let result = text; + for (const value of secretValues) { + if (!result.includes(value)) continue; + result = result.split(value).join('***REDACTED***'); + } + return result; +} + +export function clearRegisteredSecrets(): void { + secretValues.clear(); +} + +const SECRET_NAME_TOKENS = ['password', 'passwd', 'secret', 'token', 'apikey', 'api_key', 'credential', 'private_key', 'privatekey', 'access_key']; + +export function isSecretName(name: string): boolean { + const lower = name.toLowerCase(); + return SECRET_NAME_TOKENS.some((token) => lower.includes(token)); +} diff --git a/tests/unit/knowledge-tracker.test.ts b/tests/unit/knowledge-tracker.test.ts index 6626096..7f46100 100644 --- a/tests/unit/knowledge-tracker.test.ts +++ b/tests/unit/knowledge-tracker.test.ts @@ -4,6 +4,7 @@ import { mkdirSync, writeFileSync } from 'node:fs'; import matter from 'gray-matter'; import { ConfigParser } from '../../src/config'; import { KnowledgeTracker } from '../../src/knowledge-tracker'; +import { clearRegisteredSecrets, redactSecrets } from '../../src/utils/secrets'; const knowledgeDir = '/tmp/explorbot-test-knowledge'; @@ -124,5 +125,32 @@ describe('KnowledgeTracker', () => { expect(content[0]).toContain('value:'); expect(content[0]).not.toContain('${config.'); }); + + it('should block credential-named config keys from interpolating', () => { + (ConfigParser.getInstance() as any).config.ai.apiKey = 'sk-should-not-leak'; + writeKnowledgeFile('page.md', '/page', 'key: ${config.ai.apiKey}'); + + const tracker = new KnowledgeTracker(); + const content = tracker.getKnowledgeForUrl('/page'); + + expect(content[0]).not.toContain('sk-should-not-leak'); + expect(content[0]).not.toContain('${config.'); + }); + + it('should register env secrets so they are redacted at sinks', () => { + clearRegisteredSecrets(); + process.env.APP_PASSWORD = 'hunter2secret'; + + writeKnowledgeFile('login.md', '/login', 'password: ${env.APP_PASSWORD}'); + + const tracker = new KnowledgeTracker(); + const content = tracker.getKnowledgeForUrl('/login'); + + expect(content[0]).toContain('password: hunter2secret'); + expect(redactSecrets('typed hunter2secret into the field')).toBe('typed ***REDACTED*** into the field'); + + process.env.APP_PASSWORD = undefined; + clearRegisteredSecrets(); + }); }); }); diff --git a/tests/unit/secrets.test.ts b/tests/unit/secrets.test.ts new file mode 100644 index 0000000..778c1c4 --- /dev/null +++ b/tests/unit/secrets.test.ts @@ -0,0 +1,36 @@ +import { beforeEach, describe, expect, it } from 'bun:test'; +import { clearRegisteredSecrets, isSecretName, redactSecrets, registerSecret } from '../../src/utils/secrets'; + +describe('secrets', () => { + beforeEach(() => { + clearRegisteredSecrets(); + }); + + it('redacts a registered value', () => { + registerSecret('hunter2secret'); + expect(redactSecrets('login with hunter2secret now')).toBe('login with ***REDACTED*** now'); + }); + + it('passes unregistered values through unchanged', () => { + registerSecret('hunter2secret'); + expect(redactSecrets('nothing to hide here')).toBe('nothing to hide here'); + }); + + it('does not redact short values', () => { + registerSecret('12'); + expect(redactSecrets('code 12 is fine')).toBe('code 12 is fine'); + }); + + it('replaces every occurrence', () => { + registerSecret('topsecret'); + expect(redactSecrets('topsecret and topsecret again')).toBe('***REDACTED*** and ***REDACTED*** again'); + }); + + it('detects credential-named keys', () => { + expect(isSecretName('APP_PASSWORD')).toBe(true); + expect(isSecretName('ai.apiKey')).toBe(true); + expect(isSecretName('SESSION_TOKEN')).toBe(true); + expect(isSecretName('BASE_URL')).toBe(false); + expect(isSecretName('playwright.browser')).toBe(false); + }); +}); From 5857a37343901f4b030bd522193e1a78ede55433 Mon Sep 17 00:00:00 2001 From: DavertMik Date: Sat, 11 Jul 2026 16:32:23 +0300 Subject: [PATCH 2/2] refactor: redact longest secrets first; group secret consts Per re-review: iterate registered secrets longest-first in redactSecrets so a secret that is a substring of another no longer leaves a partial-secret fragment (genuine hardening for a security fix). Group SECRET_NAME_TOKENS with the other module consts. Co-Authored-By: Claude Opus 4.8 --- src/utils/secrets.ts | 5 ++--- tests/unit/secrets.test.ts | 6 ++++++ 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/utils/secrets.ts b/src/utils/secrets.ts index a469101..1ef1726 100644 --- a/src/utils/secrets.ts +++ b/src/utils/secrets.ts @@ -1,4 +1,5 @@ const MIN_SECRET_LENGTH = 4; +const SECRET_NAME_TOKENS = ['password', 'passwd', 'secret', 'token', 'apikey', 'api_key', 'credential', 'private_key', 'privatekey', 'access_key']; const secretValues = new Set(); export function registerSecret(value: string): void { @@ -10,7 +11,7 @@ export function registerSecret(value: string): void { export function redactSecrets(text: string): string { if (!text) return text; let result = text; - for (const value of secretValues) { + for (const value of [...secretValues].sort((a, b) => b.length - a.length)) { if (!result.includes(value)) continue; result = result.split(value).join('***REDACTED***'); } @@ -21,8 +22,6 @@ export function clearRegisteredSecrets(): void { secretValues.clear(); } -const SECRET_NAME_TOKENS = ['password', 'passwd', 'secret', 'token', 'apikey', 'api_key', 'credential', 'private_key', 'privatekey', 'access_key']; - export function isSecretName(name: string): boolean { const lower = name.toLowerCase(); return SECRET_NAME_TOKENS.some((token) => lower.includes(token)); diff --git a/tests/unit/secrets.test.ts b/tests/unit/secrets.test.ts index 778c1c4..0dc847b 100644 --- a/tests/unit/secrets.test.ts +++ b/tests/unit/secrets.test.ts @@ -26,6 +26,12 @@ describe('secrets', () => { expect(redactSecrets('topsecret and topsecret again')).toBe('***REDACTED*** and ***REDACTED*** again'); }); + it('redacts a longer secret fully even when a registered secret is its substring', () => { + registerSecret('hunter2'); + registerSecret('hunter2extended'); + expect(redactSecrets('login with hunter2extended now')).toBe('login with ***REDACTED*** now'); + }); + it('detects credential-named keys', () => { expect(isSecretName('APP_PASSWORD')).toBe(true); expect(isSecretName('ai.apiKey')).toBe(true);