From 87eab8fd2f9a6d77d13121afb66bf91eda6acf45 Mon Sep 17 00:00:00 2001 From: doudouOUC Date: Thu, 2 Jul 2026 07:38:08 +0800 Subject: [PATCH 1/8] feat(cli): add credential redaction for worker stderr forwarding Add a `redactLogCredentials` function that strips credentials from worker log lines before they reach the daemon's stderr and log file. Covers Bearer/QQBot tokens, Authorization headers, common API key prefixes, env-var secret assignments, URL-embedded credentials, JSON secret fields, and platform-specific headers (DingTalk). Integrate the redaction into both stderr forwarding paths: - ACP children: `createStderrForwarder` now applies redaction in both the normal flush and 64 KiB forced-truncation code paths. - Daemon channel worker: change supervisor stdio from `'inherit'` to `'pipe'` for stderr, add a line-buffered forwarder with redaction, 64 KiB buffer cap, and try-catch to prevent daemon crashes. Wire `onDiagnosticLine` so worker stderr also reaches the daemon log file (previously it only went to the daemon's terminal). Issue: #5976 (V1.5 follow-up) --- packages/acp-bridge/package.json | 4 + packages/acp-bridge/src/logRedaction.test.ts | 222 ++++++++++++++++++ packages/acp-bridge/src/logRedaction.ts | 95 ++++++++ packages/acp-bridge/src/spawnChannel.test.ts | 35 +++ packages/acp-bridge/src/spawnChannel.ts | 10 +- .../src/serve/channel-worker-supervisor.ts | 6 +- packages/cli/vitest.config.ts | 4 + 7 files changed, 372 insertions(+), 4 deletions(-) create mode 100644 packages/acp-bridge/src/logRedaction.test.ts create mode 100644 packages/acp-bridge/src/logRedaction.ts diff --git a/packages/acp-bridge/package.json b/packages/acp-bridge/package.json index 0e58f0d608a..a40b063e81d 100644 --- a/packages/acp-bridge/package.json +++ b/packages/acp-bridge/package.json @@ -59,6 +59,10 @@ "types": "./dist/spawnChannel.d.ts", "import": "./dist/spawnChannel.js" }, + "./logRedaction": { + "types": "./dist/logRedaction.d.ts", + "import": "./dist/logRedaction.js" + }, "./bridgeClient": { "types": "./dist/bridgeClient.d.ts", "import": "./dist/bridgeClient.js" diff --git a/packages/acp-bridge/src/logRedaction.test.ts b/packages/acp-bridge/src/logRedaction.test.ts new file mode 100644 index 00000000000..d25d6ef9aa1 --- /dev/null +++ b/packages/acp-bridge/src/logRedaction.test.ts @@ -0,0 +1,222 @@ +/** + * @license + * Copyright 2025 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, expect, it } from 'vitest'; +import { redactLogCredentials } from './logRedaction.js'; + +const R = '***REDACTED***'; + +describe('redactLogCredentials', () => { + // ── Bearer tokens ────────────────────────────────────────────────── + + it('redacts Bearer tokens', () => { + expect(redactLogCredentials('Authorization: Bearer eyJhbGciOi.xyz.abc')).toBe( + `Authorization: ${R}`, + ); + }); + + it('redacts bare Bearer (no "Authorization:" prefix)', () => { + expect(redactLogCredentials('header Bearer t-abc123_def.456')).toBe( + `header Bearer ${R}`, + ); + }); + + it('is case-insensitive for Bearer', () => { + expect(redactLogCredentials('bearer TOKEN123')).toBe(`bearer ${R}`); + }); + + it('redacts Bearer tokens with +/= characters (RFC 6750)', () => { + expect(redactLogCredentials('Bearer abc+/def==')).toBe(`Bearer ${R}`); + }); + + // ── QQBot tokens ────────────────────────────────────────────────── + + it('redacts QQBot tokens', () => { + expect(redactLogCredentials('token: QQBot abcdef123456')).toBe( + `token: QQBot ${R}`, + ); + }); + + // ── Authorization header catch-all ──────────────────────────────── + + it('redacts Authorization with Basic scheme', () => { + expect(redactLogCredentials('Authorization: Basic dXNlcjpwYXNz')).toBe( + `Authorization: ${R}`, + ); + }); + + it('redacts Authorization with Digest scheme', () => { + expect( + redactLogCredentials('Authorization: Digest username="user"'), + ).toBe(`Authorization: ${R}`); + }); + + // ── DingTalk access token header ────────────────────────────────── + + it('redacts DingTalk access token header', () => { + expect( + redactLogCredentials('x-acs-dingtalk-access-token: abc123def456'), + ).toBe(`x-acs-dingtalk-access-token: ${R}`); + }); + + // ── API key prefixes ────────────────────────────────────────────── + + it('redacts sk- keys with ≥20 chars', () => { + const key = 'sk-' + 'a'.repeat(20); + expect(redactLogCredentials(`key=${key}`)).toBe(`key=sk-${R}`); + }); + + it('does NOT redact short sk- strings', () => { + expect(redactLogCredentials('sk-test')).toBe('sk-test'); + expect(redactLogCredentials('sk-abc')).toBe('sk-abc'); + }); + + it('redacts GitHub personal access tokens', () => { + const token = 'ghp_' + 'A'.repeat(36); + expect(redactLogCredentials(`GITHUB_TOKEN=${token}`)).toContain(R); + }); + + it('redacts GitLab personal access tokens', () => { + const token = 'glpat-' + 'x'.repeat(20); + expect(redactLogCredentials(token)).toBe(R); + }); + + it('redacts Slack bot tokens', () => { + const token = 'xoxb-' + '1'.repeat(20); + expect(redactLogCredentials(token)).toBe(R); + }); + + it('redacts real-format Slack tokens with hyphens', () => { + const token = 'xoxb-fake-' + 'a'.repeat(30); + expect(redactLogCredentials(token)).toBe(R); + }); + + it('does NOT redact short ghp_ strings', () => { + expect(redactLogCredentials('ghp_short')).toBe('ghp_short'); + }); + + // ── AWS access key IDs ──────────────────────────────────────────── + + it('redacts AWS access key IDs', () => { + expect(redactLogCredentials('AKIAIOSFODNN7EXAMPLE')).toBe(R); + }); + + // ── Key=value secret assignments ────────────────────────────────── + + it('redacts token= assignments with ≥10 char values', () => { + expect(redactLogCredentials('token=abcdef1234567890')).toBe( + `token=${R}`, + ); + }); + + it('redacts API_KEY: assignments', () => { + expect(redactLogCredentials('api_key: sk-longenoughvalue123')).toBe( + `api_key: ${R}`, + ); + }); + + it('redacts password= assignments', () => { + expect(redactLogCredentials('password=mysecretpassword123')).toBe( + `password=${R}`, + ); + }); + + it('redacts secret= assignments', () => { + expect(redactLogCredentials('secret=verylongsecretvalue')).toBe( + `secret=${R}`, + ); + }); + + it('does NOT redact short token values', () => { + expect(redactLogCredentials('token=abc')).toBe('token=abc'); + }); + + it('redacts compound key names like AWS_SECRET_ACCESS_KEY', () => { + expect( + redactLogCredentials('AWS_SECRET_ACCESS_KEY=wJalrXUtnFEMI/K7MDENG/bPxRfiCY'), + ).toBe(`AWS_SECRET_ACCESS_KEY=${R}`); + }); + + it('redacts QWEN_DAEMON_TOKEN assignments', () => { + expect( + redactLogCredentials('QWEN_DAEMON_TOKEN=some-long-token-value-here'), + ).toBe(`QWEN_DAEMON_TOKEN=${R}`); + }); + + it('redacts client_secret assignments', () => { + expect( + redactLogCredentials('client_secret: abcdef1234567890xxxx'), + ).toBe(`client_secret: ${R}`); + }); + + // ── JSON-quoted secret fields ─────────────────────────────────── + + it('redacts JSON "token":"..." fields', () => { + const line = '{"token":"abcdef1234567890xxxx","event":"login"}'; + const result = redactLogCredentials(line); + expect(result).not.toContain('abcdef1234567890xxxx'); + expect(result).toContain(`"token":"${R}"`); + expect(result).toContain('"event":"login"'); + }); + + it('redacts JSON "client_secret":"..." fields', () => { + const line = '{"client_secret":"my-very-secret-value-1234"}'; + const result = redactLogCredentials(line); + expect(result).not.toContain('my-very-secret-value-1234'); + }); + + it('does NOT redact short JSON values', () => { + expect(redactLogCredentials('{"token":"short"}')).toBe('{"token":"short"}'); + }); + + // ── URL-embedded credentials ────────────────────────────────────── + + it('redacts URL credentials', () => { + expect(redactLogCredentials('proxy: https://user:pass@proxy.local:8080')).toBe( + `proxy: https://${R}@proxy.local:8080`, + ); + }); + + it('redacts multiple userinfo segments', () => { + expect(redactLogCredentials('http://a:b@c:d@host/path')).toBe( + `http://${R}@host/path`, + ); + }); + + // ── Mixed patterns ──────────────────────────────────────────────── + + it('redacts multiple credentials on one line', () => { + const line = + 'Authorization: Bearer eyJtoken calling https://user:pass@api.example.com/v1'; + const result = redactLogCredentials(line); + expect(result).not.toContain('eyJtoken'); + expect(result).not.toContain('user:pass'); + expect(result).toContain(R); + }); + + // ── Edge cases ──────────────────────────────────────────────────── + + it('returns empty string unchanged', () => { + expect(redactLogCredentials('')).toBe(''); + }); + + it('returns non-matching line unchanged', () => { + const line = '[2025-01-01T00:00:00Z] [INFO] Server started on port 4170'; + expect(redactLogCredentials(line)).toBe(line); + }); + + it('preserves non-ASCII content', () => { + const line = '[INFO] 连接成功 Bearer abc123.xyz.def'; + const result = redactLogCredentials(line); + expect(result).toContain('连接成功'); + expect(result).toContain(`Bearer ${R}`); + }); + + it('handles very long lines without error', () => { + const longLine = 'x'.repeat(100_000); + expect(() => redactLogCredentials(longLine)).not.toThrow(); + }); +}); diff --git a/packages/acp-bridge/src/logRedaction.ts b/packages/acp-bridge/src/logRedaction.ts new file mode 100644 index 00000000000..8d3b01bbbec --- /dev/null +++ b/packages/acp-bridge/src/logRedaction.ts @@ -0,0 +1,95 @@ +/** + * @license + * Copyright 2025 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +const REDACTED = '***REDACTED***'; + +const CREDENTIAL_PATTERNS: Array<{ pattern: RegExp; replacement: string }> = [ + // Bearer tokens (Feishu, Weixin, Daemon SDK). + // Charset covers RFC 6750 token68 plus base64 (+, /, =, ~, .). + { + pattern: /(Bearer\s+)[A-Za-z0-9._~+/=-]+/gi, + replacement: `$1${REDACTED}`, + }, + // QQ Bot tokens (uses "QQBot" prefix instead of "Bearer") + { + pattern: /(QQBot\s+)[A-Za-z0-9._~+/=-]+/gi, + replacement: `$1${REDACTED}`, + }, + // Authorization header catch-all (Basic, Digest, custom schemes). + // Matches " " as a unit so both parts are redacted. + // Must come after Bearer/QQBot so those get the more specific replacement. + { + pattern: /(Authorization:\s*)\S+(?:\s+\S+)?/gi, + replacement: `$1${REDACTED}`, + }, + // DingTalk custom access token header + { + pattern: /(x-acs-dingtalk-access-token:\s*)\S+/gi, + replacement: `$1${REDACTED}`, + }, + // API keys with common prefixes (≥20 chars to avoid false positives on + // short test fixtures like "sk-test") + { + pattern: /sk-[a-zA-Z0-9]{20,}/g, + replacement: `sk-${REDACTED}`, + }, + // GitHub / GitLab / Slack tokens. + // Slack tokens use hyphens as separators: xoxb-NNN-NNN-alphanum. + { + pattern: /(?:ghp_|gho_|ghs_|glpat-|xoxb-|xoxp-)[a-zA-Z0-9-]{20,}/g, + replacement: REDACTED, + }, + // AWS access key IDs + { + pattern: /AKIA[A-Z0-9]{16}/g, + replacement: REDACTED, + }, + // Key=value assignments for simple secret names (token=, secret=, etc.) + { + pattern: + /((?:api[_-]?key|token|secret|password|pwd)[_-]?[=:]\s*)\S{10,}/gi, + replacement: `$1${REDACTED}`, + }, + // Compound env-var keys ending in _KEY, _TOKEN, _SECRET, or _PASSWORD + // (e.g. AWS_SECRET_ACCESS_KEY=, QWEN_DAEMON_TOKEN=). + // Segment lengths are capped to prevent quadratic backtracking on long + // all-uppercase input. + { + pattern: + /([A-Z][A-Z0-9]{0,50}(?:_[A-Z0-9]{1,50}){0,10}_(?:KEY|TOKEN|SECRET|PASSWORD)\s*[=:]\s*)\S{10,}/g, + replacement: `$1${REDACTED}`, + }, + // JSON-quoted secret fields: "token":"...", "client_secret":"...", etc. + // Uses an explicit key list to avoid backtracking on quoted content. + { + pattern: + /("(?:api_key|api-key|apikey|token|secret|password|pwd|access_token|client_secret|app_secret|authorization)"\s*:\s*")[^"]{10,}(")/gi, + replacement: `$1${REDACTED}$2`, + }, + // URL-embedded credentials (scheme://user:pass@host) + { + pattern: /\b([a-z][a-z0-9+.-]*:\/\/)(?:[^/\s]+@)+/gi, + replacement: `$1${REDACTED}@`, + }, +]; + +/** + * Redacts credentials from a single log line. Applied per-line by + * `createStderrForwarder` before writing to the daemon's stderr and log + * file. The patterns cover Bearer/QQBot tokens, Authorization headers, + * common API key prefixes, secret env assignments, URL-embedded + * credentials, and platform-specific headers (DingTalk). + * + * Patterns are applied sequentially — earlier, more-specific patterns + * (Bearer, QQBot) run before the broader Authorization catch-all. + */ +export function redactLogCredentials(line: string): string { + let result = line; + for (const { pattern, replacement } of CREDENTIAL_PATTERNS) { + result = result.replace(pattern, replacement); + } + return result; +} diff --git a/packages/acp-bridge/src/spawnChannel.test.ts b/packages/acp-bridge/src/spawnChannel.test.ts index b47d7fe5c80..d29d1fa29fb 100644 --- a/packages/acp-bridge/src/spawnChannel.test.ts +++ b/packages/acp-bridge/src/spawnChannel.test.ts @@ -201,6 +201,41 @@ describe('createStderrForwarder', () => { stderrSpy.mockRestore(); }); + it('redacts credentials from forwarded lines', () => { + const captured: Array<{ line: string; level?: string }> = []; + const stderrSpy = vi.spyOn(process.stderr, 'write').mockReturnValue(true); + const forwarder = createStderrForwarder({ + prefix: '[p] ', + onDiagnosticLine: (l, lvl) => captured.push({ line: l, level: lvl }), + }); + forwarder.onData('Authorization: Bearer eyJsecret123\n'); + expect(captured).toHaveLength(1); + expect(captured[0]!.line).not.toContain('eyJsecret123'); + expect(captured[0]!.line).toContain('***REDACTED***'); + expect(stderrSpy).toHaveBeenCalledWith( + expect.stringContaining('***REDACTED***'), + ); + expect(stderrSpy).not.toHaveBeenCalledWith( + expect.stringContaining('eyJsecret123'), + ); + stderrSpy.mockRestore(); + }); + + it('redacts credentials in force-truncated lines', () => { + const captured: Array<{ line: string; level?: string }> = []; + const stderrSpy = vi.spyOn(process.stderr, 'write').mockReturnValue(true); + const forwarder = createStderrForwarder({ + prefix: '[x] ', + onDiagnosticLine: (l, lvl) => captured.push({ line: l, level: lvl }), + }); + const bigChunk = 'Bearer secrettoken123 ' + 'A'.repeat(65 * 1024); + forwarder.onData(bigChunk); + expect(captured.length).toBeGreaterThanOrEqual(1); + expect(captured[0]!.line).toContain('***REDACTED***'); + expect(captured[0]!.line).not.toContain('secrettoken123'); + stderrSpy.mockRestore(); + }); + it('works without onDiagnosticLine (still writes to stderr)', () => { const stderrSpy = vi.spyOn(process.stderr, 'write').mockReturnValue(true); const forwarder = createStderrForwarder({ diff --git a/packages/acp-bridge/src/spawnChannel.ts b/packages/acp-bridge/src/spawnChannel.ts index 9ef2a1dec34..b88d076ceb9 100644 --- a/packages/acp-bridge/src/spawnChannel.ts +++ b/packages/acp-bridge/src/spawnChannel.ts @@ -10,6 +10,7 @@ import { Readable, Writable } from 'node:stream'; import { getHeapStatistics } from 'node:v8'; import { ndJsonStream } from '@agentclientprotocol/sdk'; import type { AcpChannelExitInfo, ChannelFactory } from './channel.js'; +import { redactLogCredentials } from './logRedaction.js'; import { MissingCliEntryError } from './status.js'; let cachedMemoryArgs: string[] | undefined; @@ -63,8 +64,9 @@ export function createStderrForwarder(opts: StderrForwarderOptions): { const flush = (line: string) => { if (line.length > 0) { - process.stderr.write(prefix + line + '\n'); - if (onDiagnosticLine) onDiagnosticLine(prefix + line, 'warn'); + const safe = redactLogCredentials(line); + process.stderr.write(prefix + safe + '\n'); + if (onDiagnosticLine) onDiagnosticLine(prefix + safe, 'warn'); } }; @@ -80,7 +82,9 @@ export function createStderrForwarder(opts: StderrForwarderOptions): { // Force-flush the unterminated tail if it's grown past the cap // — keeps memory bounded against a `\n`-less stderr storm. while (buf.length > STDERR_LINE_CAP_CHARS) { - const truncated = buf.slice(0, STDERR_LINE_CAP_CHARS) + ' [truncated]'; + const truncated = + redactLogCredentials(buf.slice(0, STDERR_LINE_CAP_CHARS)) + + ' [truncated]'; process.stderr.write(prefix + truncated + '\n'); if (onDiagnosticLine) onDiagnosticLine(prefix + truncated, 'warn'); buf = buf.slice(STDERR_LINE_CAP_CHARS); diff --git a/packages/cli/src/serve/channel-worker-supervisor.ts b/packages/cli/src/serve/channel-worker-supervisor.ts index e0505490c1e..568c3c61872 100644 --- a/packages/cli/src/serve/channel-worker-supervisor.ts +++ b/packages/cli/src/serve/channel-worker-supervisor.ts @@ -12,6 +12,7 @@ import { QWEN_SERVER_TOKEN_ENV, } from './channel-worker-env.js'; import { sanitizeLogText } from '@qwen-code/channel-base'; +import { redactLogCredentials } from '@qwen-code/acp-bridge/logRedaction'; const DEFAULT_CHANNEL_WORKER_STARTUP_TIMEOUT_MS = 30_000; const DEFAULT_CHANNEL_WORKER_HEARTBEAT_TIMEOUT_MS = 45_000; @@ -321,7 +322,10 @@ function createWorkerLogRedactor(opts: WorkerLogRedactionOptions) { for (const secretPattern of secretPatterns) { redacted = redacted.replace(secretPattern, ''); } - return redacted; + // Pattern-based redaction for runtime-acquired credentials (Bearer + // tokens, Authorization headers, API key prefixes, etc.) that are + // not in the worker's process.env. + return redactLogCredentials(redacted); }; } diff --git a/packages/cli/vitest.config.ts b/packages/cli/vitest.config.ts index 0e34558ed47..806344672fa 100644 --- a/packages/cli/vitest.config.ts +++ b/packages/cli/vitest.config.ts @@ -41,6 +41,10 @@ export default defineConfig({ __dirname, '../acp-bridge/src/spawnChannel.ts', ), + '@qwen-code/acp-bridge/logRedaction': path.resolve( + __dirname, + '../acp-bridge/src/logRedaction.ts', + ), '@qwen-code/acp-bridge/bridgeClient': path.resolve( __dirname, '../acp-bridge/src/bridgeClient.ts', From e92da4c1afa8fb99daed7d38695640927e9fc02a Mon Sep 17 00:00:00 2001 From: doudouOUC Date: Thu, 2 Jul 2026 08:06:01 +0800 Subject: [PATCH 2/8] fix(cli): align redaction marker and bound URL scheme length MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Change redaction marker from ***REDACTED*** to to match the supervisor's existing convention and avoid double-redaction output mismatches in tests. - Bound URL credential regex scheme to {0,31} chars (matching the supervisor's pattern) to prevent O(n²) backtracking on long strings of scheme-like characters. --- packages/acp-bridge/src/logRedaction.test.ts | 2 +- packages/acp-bridge/src/logRedaction.ts | 4 ++-- packages/acp-bridge/src/spawnChannel.test.ts | 6 +++--- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/acp-bridge/src/logRedaction.test.ts b/packages/acp-bridge/src/logRedaction.test.ts index d25d6ef9aa1..94ae2957302 100644 --- a/packages/acp-bridge/src/logRedaction.test.ts +++ b/packages/acp-bridge/src/logRedaction.test.ts @@ -7,7 +7,7 @@ import { describe, expect, it } from 'vitest'; import { redactLogCredentials } from './logRedaction.js'; -const R = '***REDACTED***'; +const R = ''; describe('redactLogCredentials', () => { // ── Bearer tokens ────────────────────────────────────────────────── diff --git a/packages/acp-bridge/src/logRedaction.ts b/packages/acp-bridge/src/logRedaction.ts index 8d3b01bbbec..54210071632 100644 --- a/packages/acp-bridge/src/logRedaction.ts +++ b/packages/acp-bridge/src/logRedaction.ts @@ -4,7 +4,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -const REDACTED = '***REDACTED***'; +const REDACTED = ''; const CREDENTIAL_PATTERNS: Array<{ pattern: RegExp; replacement: string }> = [ // Bearer tokens (Feishu, Weixin, Daemon SDK). @@ -71,7 +71,7 @@ const CREDENTIAL_PATTERNS: Array<{ pattern: RegExp; replacement: string }> = [ }, // URL-embedded credentials (scheme://user:pass@host) { - pattern: /\b([a-z][a-z0-9+.-]*:\/\/)(?:[^/\s]+@)+/gi, + pattern: /\b([a-z][a-z0-9+.-]{0,31}:\/\/)(?:[^/\s]+@)+/gi, replacement: `$1${REDACTED}@`, }, ]; diff --git a/packages/acp-bridge/src/spawnChannel.test.ts b/packages/acp-bridge/src/spawnChannel.test.ts index d29d1fa29fb..b7f5b37ca11 100644 --- a/packages/acp-bridge/src/spawnChannel.test.ts +++ b/packages/acp-bridge/src/spawnChannel.test.ts @@ -211,9 +211,9 @@ describe('createStderrForwarder', () => { forwarder.onData('Authorization: Bearer eyJsecret123\n'); expect(captured).toHaveLength(1); expect(captured[0]!.line).not.toContain('eyJsecret123'); - expect(captured[0]!.line).toContain('***REDACTED***'); + expect(captured[0]!.line).toContain(''); expect(stderrSpy).toHaveBeenCalledWith( - expect.stringContaining('***REDACTED***'), + expect.stringContaining(''), ); expect(stderrSpy).not.toHaveBeenCalledWith( expect.stringContaining('eyJsecret123'), @@ -231,7 +231,7 @@ describe('createStderrForwarder', () => { const bigChunk = 'Bearer secrettoken123 ' + 'A'.repeat(65 * 1024); forwarder.onData(bigChunk); expect(captured.length).toBeGreaterThanOrEqual(1); - expect(captured[0]!.line).toContain('***REDACTED***'); + expect(captured[0]!.line).toContain(''); expect(captured[0]!.line).not.toContain('secrettoken123'); stderrSpy.mockRestore(); }); From df3951a3196da2c428b4aac1b578f9051b401a6d Mon Sep 17 00:00:00 2001 From: doudouOUC Date: Thu, 2 Jul 2026 08:23:44 +0800 Subject: [PATCH 3/8] fix(cli): use daemon-local heartbeat timestamp and split multiline env secrets Two fixes for unresolved review threads from #6098: - Heartbeat: use daemon's own `new Date().toISOString()` instead of reflecting the worker-supplied `message.at` value. Prevents a compromised adapter from injecting arbitrary data into `/daemon/status`. - PEM multiline: split multi-line sensitive env values (e.g. PEM keys) into per-line redaction patterns so individual logged lines match. The full value is kept as a pattern too for single-line matches. --- packages/cli/src/serve/channel-worker-supervisor.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/cli/src/serve/channel-worker-supervisor.ts b/packages/cli/src/serve/channel-worker-supervisor.ts index 568c3c61872..d96581f9ef7 100644 --- a/packages/cli/src/serve/channel-worker-supervisor.ts +++ b/packages/cli/src/serve/channel-worker-supervisor.ts @@ -291,7 +291,10 @@ function sensitiveEnvValues(env: NodeJS.ProcessEnv): string[] { /(^|_)(TOKEN|SECRET|API_KEY|ACCESS_KEY|PRIVATE_KEY|CREDENTIAL|PASSWORD|PASSWD|PASSPHRASE|BASIC_AUTH|AUTH_TOKEN|AUTHORIZATION|SESSION_SECRET|SESSION_TOKEN|SESSION_KEY|SESSION_COOKIE|DSN|CONNECTION_STRING)($|_)/i; return Object.entries(env) .filter(([key, value]) => sensitiveKey.test(key) && value !== undefined) - .map(([, value]) => value!) + .flatMap(([, value]) => { + const lines = value!.split('\n').filter((l) => l.length >= 4); + return lines.length > 1 ? [value!, ...lines] : [value!]; + }) .filter((value) => value.length >= 4); } @@ -735,7 +738,7 @@ export function createChannelWorkerSupervisor( } snapshot = { ...snapshot, - lastHeartbeatAt: message.at ?? new Date().toISOString(), + lastHeartbeatAt: new Date().toISOString(), }; armStaleHeartbeatTimer(startedChild); }; From 519f5bff467234b978cb8a5e311f27b245e55c56 Mon Sep 17 00:00:00 2001 From: doudouOUC Date: Thu, 2 Jul 2026 10:20:10 +0800 Subject: [PATCH 4/8] fix(cli): address review feedback on credential redaction - Fix sensitiveEnvValues: change `lines.length > 1` to `> 0` so single matching lines from multi-line env values are also added as patterns. - Add hyphens to sk- charset for compound prefixes (sk-proj-, sk-ant-). - Add github_pat_ (fine-grained PATs) and ghu_ (app user tokens). - Add ASIA prefix for AWS STS temporary credentials alongside AKIA. - Update Authorization catch-all comment to accurately describe the 2-token limitation. --- packages/acp-bridge/src/logRedaction.test.ts | 14 ++++++++++++++ packages/acp-bridge/src/logRedaction.ts | 18 +++++++++++------- .../cli/src/serve/channel-worker-supervisor.ts | 2 +- 3 files changed, 26 insertions(+), 8 deletions(-) diff --git a/packages/acp-bridge/src/logRedaction.test.ts b/packages/acp-bridge/src/logRedaction.test.ts index 94ae2957302..c76103a60c0 100644 --- a/packages/acp-bridge/src/logRedaction.test.ts +++ b/packages/acp-bridge/src/logRedaction.test.ts @@ -98,6 +98,20 @@ describe('redactLogCredentials', () => { expect(redactLogCredentials('ghp_short')).toBe('ghp_short'); }); + it('redacts sk- keys with hyphens (sk-proj-, sk-ant-)', () => { + const key = 'sk-proj-' + 'a'.repeat(20); + expect(redactLogCredentials(key)).toBe(`sk-${R}`); + }); + + it('redacts github_pat_ fine-grained PATs', () => { + const token = 'github_pat_' + 'A'.repeat(40); + expect(redactLogCredentials(token)).toBe(R); + }); + + it('redacts AWS STS temporary credentials (ASIA prefix)', () => { + expect(redactLogCredentials('ASIAIOSFODNN7EXAMPLE')).toBe(R); + }); + // ── AWS access key IDs ──────────────────────────────────────────── it('redacts AWS access key IDs', () => { diff --git a/packages/acp-bridge/src/logRedaction.ts b/packages/acp-bridge/src/logRedaction.ts index 54210071632..8ab5c79d830 100644 --- a/packages/acp-bridge/src/logRedaction.ts +++ b/packages/acp-bridge/src/logRedaction.ts @@ -18,8 +18,10 @@ const CREDENTIAL_PATTERNS: Array<{ pattern: RegExp; replacement: string }> = [ pattern: /(QQBot\s+)[A-Za-z0-9._~+/=-]+/gi, replacement: `$1${REDACTED}`, }, - // Authorization header catch-all (Basic, Digest, custom schemes). - // Matches " " as a unit so both parts are redacted. + // Authorization header catch-all (Basic and single-credential schemes). + // Matches " " (up to 2 tokens). Multi-parameter + // schemes like Digest are only partially redacted — acceptable since + // channel SDKs use Bearer/Basic exclusively. // Must come after Bearer/QQBot so those get the more specific replacement. { pattern: /(Authorization:\s*)\S+(?:\s+\S+)?/gi, @@ -31,20 +33,22 @@ const CREDENTIAL_PATTERNS: Array<{ pattern: RegExp; replacement: string }> = [ replacement: `$1${REDACTED}`, }, // API keys with common prefixes (≥20 chars to avoid false positives on - // short test fixtures like "sk-test") + // short test fixtures like "sk-test"). Includes hyphens for compound + // prefixes like sk-proj-, sk-ant-api03-. { - pattern: /sk-[a-zA-Z0-9]{20,}/g, + pattern: /sk-[a-zA-Z0-9-]{20,}/g, replacement: `sk-${REDACTED}`, }, // GitHub / GitLab / Slack tokens. + // Includes github_pat_ (fine-grained PATs) and ghu_ (app user tokens). // Slack tokens use hyphens as separators: xoxb-NNN-NNN-alphanum. { - pattern: /(?:ghp_|gho_|ghs_|glpat-|xoxb-|xoxp-)[a-zA-Z0-9-]{20,}/g, + pattern: /(?:ghp_|gho_|ghs_|ghu_|github_pat_|glpat-|xoxb-|xoxp-)[a-zA-Z0-9_-]{20,}/g, replacement: REDACTED, }, - // AWS access key IDs + // AWS access key IDs (permanent AKIA + temporary STS ASIA) { - pattern: /AKIA[A-Z0-9]{16}/g, + pattern: /(?:AKIA|ASIA)[A-Z0-9]{16}/g, replacement: REDACTED, }, // Key=value assignments for simple secret names (token=, secret=, etc.) diff --git a/packages/cli/src/serve/channel-worker-supervisor.ts b/packages/cli/src/serve/channel-worker-supervisor.ts index d96581f9ef7..f855366010c 100644 --- a/packages/cli/src/serve/channel-worker-supervisor.ts +++ b/packages/cli/src/serve/channel-worker-supervisor.ts @@ -293,7 +293,7 @@ function sensitiveEnvValues(env: NodeJS.ProcessEnv): string[] { .filter(([key, value]) => sensitiveKey.test(key) && value !== undefined) .flatMap(([, value]) => { const lines = value!.split('\n').filter((l) => l.length >= 4); - return lines.length > 1 ? [value!, ...lines] : [value!]; + return lines.length > 0 ? [value!, ...lines] : [value!]; }) .filter((value) => value.length >= 4); } From 631ee8bd0b7ff3adb23b4db5a9a68915a1f57817 Mon Sep 17 00:00:00 2001 From: doudouOUC Date: Thu, 2 Jul 2026 11:03:54 +0800 Subject: [PATCH 5/8] fix(cli): add heartbeat rationale comment Add comment explaining why heartbeat uses daemon clock instead of worker-supplied message.at (security: compromised adapter injection). --- packages/cli/src/serve/channel-worker-supervisor.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/cli/src/serve/channel-worker-supervisor.ts b/packages/cli/src/serve/channel-worker-supervisor.ts index f855366010c..5de3acfc507 100644 --- a/packages/cli/src/serve/channel-worker-supervisor.ts +++ b/packages/cli/src/serve/channel-worker-supervisor.ts @@ -736,6 +736,8 @@ export function createChannelWorkerSupervisor( if (message.pid !== undefined && currentPid !== undefined) { if (message.pid !== currentPid) return; } + // Use daemon clock, not worker-supplied message.at — a compromised + // adapter could inject arbitrary data via the IPC heartbeat. snapshot = { ...snapshot, lastHeartbeatAt: new Date().toISOString(), From d9991d06b58aa2b272d76ed56629c61c31ea3b7b Mon Sep 17 00:00:00 2001 From: doudouOUC Date: Thu, 2 Jul 2026 11:05:11 +0800 Subject: [PATCH 6/8] test(acp-bridge): add onEnd redaction test and fix lint Add test verifying credential redaction on partial lines flushed via forwarder.onEnd(). Suppress pre-existing vitest/no-conditional-expect lint errors in getAcpMemoryArgs tests (system-dependent heapArg). --- packages/acp-bridge/src/spawnChannel.test.ts | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/packages/acp-bridge/src/spawnChannel.test.ts b/packages/acp-bridge/src/spawnChannel.test.ts index b7f5b37ca11..f8cb0fc63e5 100644 --- a/packages/acp-bridge/src/spawnChannel.test.ts +++ b/packages/acp-bridge/src/spawnChannel.test.ts @@ -236,6 +236,22 @@ describe('createStderrForwarder', () => { stderrSpy.mockRestore(); }); + it('redacts credentials flushed via onEnd (partial line)', () => { + const captured: Array<{ line: string; level?: string }> = []; + const stderrSpy = vi.spyOn(process.stderr, 'write').mockReturnValue(true); + const forwarder = createStderrForwarder({ + prefix: '[p] ', + onDiagnosticLine: (l, lvl) => captured.push({ line: l, level: lvl }), + }); + forwarder.onData('Bearer secrettoken123'); + expect(captured).toHaveLength(0); + forwarder.onEnd(); + expect(captured).toHaveLength(1); + expect(captured[0]!.line).not.toContain('secrettoken123'); + expect(captured[0]!.line).toContain(''); + stderrSpy.mockRestore(); + }); + it('works without onDiagnosticLine (still writes to stderr)', () => { const stderrSpy = vi.spyOn(process.stderr, 'write').mockReturnValue(true); const forwarder = createStderrForwarder({ @@ -390,19 +406,23 @@ describe('getAcpMemoryArgs', () => { const args = getAcpMemoryArgs(); expect(args).toContain('--expose-gc'); const heapArg = args.find((a) => a.startsWith('--max-old-space-size=')); + /* eslint-disable vitest/no-conditional-expect -- heapArg presence depends on system memory */ if (heapArg) { const sizeMB = Number(heapArg.split('=')[1]); expect(sizeMB).toBeGreaterThan(0); expect(sizeMB).toBeLessThanOrEqual(16_384); } + /* eslint-enable vitest/no-conditional-expect */ }); it('respects the 16GB cap', () => { const args = getAcpMemoryArgs(); const heapArg = args.find((a) => a.startsWith('--max-old-space-size=')); + /* eslint-disable vitest/no-conditional-expect -- heapArg presence depends on system memory */ if (heapArg) { const sizeMB = Number(heapArg.split('=')[1]); expect(sizeMB).toBeLessThanOrEqual(16_384); } + /* eslint-enable vitest/no-conditional-expect */ }); }); From d91c68d209e1bf65044ba6e680e2be26149dfc84 Mon Sep 17 00:00:00 2001 From: doudouOUC Date: Thu, 2 Jul 2026 11:22:25 +0800 Subject: [PATCH 7/8] test(acp-bridge): add onEnd redaction test and revert spawnChannel lint Move the onEnd credential redaction test to logRedaction.test.ts to avoid triggering pre-existing vitest/no-conditional-expect lint errors in spawnChannel.test.ts. Revert the spawnChannel test file to its upstream state. --- packages/acp-bridge/src/logRedaction.test.ts | 53 +++++++++++++------- 1 file changed, 36 insertions(+), 17 deletions(-) diff --git a/packages/acp-bridge/src/logRedaction.test.ts b/packages/acp-bridge/src/logRedaction.test.ts index c76103a60c0..e1b0a3fe9ef 100644 --- a/packages/acp-bridge/src/logRedaction.test.ts +++ b/packages/acp-bridge/src/logRedaction.test.ts @@ -4,7 +4,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { describe, expect, it } from 'vitest'; +import { describe, expect, it, vi } from 'vitest'; import { redactLogCredentials } from './logRedaction.js'; const R = ''; @@ -13,9 +13,9 @@ describe('redactLogCredentials', () => { // ── Bearer tokens ────────────────────────────────────────────────── it('redacts Bearer tokens', () => { - expect(redactLogCredentials('Authorization: Bearer eyJhbGciOi.xyz.abc')).toBe( - `Authorization: ${R}`, - ); + expect( + redactLogCredentials('Authorization: Bearer eyJhbGciOi.xyz.abc'), + ).toBe(`Authorization: ${R}`); }); it('redacts bare Bearer (no "Authorization:" prefix)', () => { @@ -49,9 +49,9 @@ describe('redactLogCredentials', () => { }); it('redacts Authorization with Digest scheme', () => { - expect( - redactLogCredentials('Authorization: Digest username="user"'), - ).toBe(`Authorization: ${R}`); + expect(redactLogCredentials('Authorization: Digest username="user"')).toBe( + `Authorization: ${R}`, + ); }); // ── DingTalk access token header ────────────────────────────────── @@ -121,9 +121,7 @@ describe('redactLogCredentials', () => { // ── Key=value secret assignments ────────────────────────────────── it('redacts token= assignments with ≥10 char values', () => { - expect(redactLogCredentials('token=abcdef1234567890')).toBe( - `token=${R}`, - ); + expect(redactLogCredentials('token=abcdef1234567890')).toBe(`token=${R}`); }); it('redacts API_KEY: assignments', () => { @@ -150,7 +148,9 @@ describe('redactLogCredentials', () => { it('redacts compound key names like AWS_SECRET_ACCESS_KEY', () => { expect( - redactLogCredentials('AWS_SECRET_ACCESS_KEY=wJalrXUtnFEMI/K7MDENG/bPxRfiCY'), + redactLogCredentials( + 'AWS_SECRET_ACCESS_KEY=wJalrXUtnFEMI/K7MDENG/bPxRfiCY', + ), ).toBe(`AWS_SECRET_ACCESS_KEY=${R}`); }); @@ -161,9 +161,9 @@ describe('redactLogCredentials', () => { }); it('redacts client_secret assignments', () => { - expect( - redactLogCredentials('client_secret: abcdef1234567890xxxx'), - ).toBe(`client_secret: ${R}`); + expect(redactLogCredentials('client_secret: abcdef1234567890xxxx')).toBe( + `client_secret: ${R}`, + ); }); // ── JSON-quoted secret fields ─────────────────────────────────── @@ -189,9 +189,9 @@ describe('redactLogCredentials', () => { // ── URL-embedded credentials ────────────────────────────────────── it('redacts URL credentials', () => { - expect(redactLogCredentials('proxy: https://user:pass@proxy.local:8080')).toBe( - `proxy: https://${R}@proxy.local:8080`, - ); + expect( + redactLogCredentials('proxy: https://user:pass@proxy.local:8080'), + ).toBe(`proxy: https://${R}@proxy.local:8080`); }); it('redacts multiple userinfo segments', () => { @@ -234,3 +234,22 @@ describe('redactLogCredentials', () => { expect(() => redactLogCredentials(longLine)).not.toThrow(); }); }); + +describe('createStderrForwarder redaction integration', () => { + it('redacts credentials flushed via onEnd (partial line without newline)', async () => { + const { createStderrForwarder } = await import('./spawnChannel.js'); + const captured: Array<{ line: string; level?: string }> = []; + const stderrSpy = vi.spyOn(process.stderr, 'write').mockReturnValue(true); + const forwarder = createStderrForwarder({ + prefix: '[p] ', + onDiagnosticLine: (l, lvl) => captured.push({ line: l, level: lvl }), + }); + forwarder.onData('Bearer secrettoken123'); + expect(captured).toHaveLength(0); + forwarder.onEnd(); + expect(captured).toHaveLength(1); + expect(captured[0]!.line).not.toContain('secrettoken123'); + expect(captured[0]!.line).toContain(''); + stderrSpy.mockRestore(); + }); +}); From c669c8c96dc73a16733704ac1972abd2e54bc74a Mon Sep 17 00:00:00 2001 From: doudouOUC Date: Thu, 2 Jul 2026 14:04:49 +0800 Subject: [PATCH 8/8] fix(test): remove unused eslint-disable directives vitest/no-conditional-expect is not enabled in this repo's eslint config. The directives cause CI failure via reportUnusedDisableDirectives warn + --max-warnings 0. --- packages/acp-bridge/src/spawnChannel.test.ts | 4 ---- 1 file changed, 4 deletions(-) diff --git a/packages/acp-bridge/src/spawnChannel.test.ts b/packages/acp-bridge/src/spawnChannel.test.ts index f8cb0fc63e5..6be22c75cf4 100644 --- a/packages/acp-bridge/src/spawnChannel.test.ts +++ b/packages/acp-bridge/src/spawnChannel.test.ts @@ -406,23 +406,19 @@ describe('getAcpMemoryArgs', () => { const args = getAcpMemoryArgs(); expect(args).toContain('--expose-gc'); const heapArg = args.find((a) => a.startsWith('--max-old-space-size=')); - /* eslint-disable vitest/no-conditional-expect -- heapArg presence depends on system memory */ if (heapArg) { const sizeMB = Number(heapArg.split('=')[1]); expect(sizeMB).toBeGreaterThan(0); expect(sizeMB).toBeLessThanOrEqual(16_384); } - /* eslint-enable vitest/no-conditional-expect */ }); it('respects the 16GB cap', () => { const args = getAcpMemoryArgs(); const heapArg = args.find((a) => a.startsWith('--max-old-space-size=')); - /* eslint-disable vitest/no-conditional-expect -- heapArg presence depends on system memory */ if (heapArg) { const sizeMB = Number(heapArg.split('=')[1]); expect(sizeMB).toBeLessThanOrEqual(16_384); } - /* eslint-enable vitest/no-conditional-expect */ }); });