Centralize credential redaction in src/utils/redaction.ts + channel gate tests

scripts/system-check.ts

@@ -26,7 +26,7 @@ import {
   redactSecretSubstringsForDisplay,
   type SecretValueSource,
 } from '../src/utils/providerSecrets.js'
-import { redactUrlForDisplay } from '../src/utils/urlRedaction.js'
+import { redactUrlForDisplay } from '../src/utils/redaction.js'
 import {
   MIN_NODE_ENGINE_RANGE,
   checkSupportedNodeVersion,

src/__tests__/bugfixes.test.ts

@@ -8,7 +8,7 @@
  * 4. Web search result count improvements
  */
 
-import { describe, test, expect } from 'bun:test'
+import { afterEach, describe, test, expect, mock } from 'bun:test'
 import { resolve } from 'path'
 import {
   clearRegisteredHooks,
@@ -18,6 +18,13 @@ import { getMatchingHooks } from '../utils/hooks.js'
 import type { PluginHookMatcher } from '../utils/settings/types.js'
 
 const SRC = resolve(import.meta.dir, '..')
+
+// Real channelAllowlist module — captured before mocking so describe-block
+// afterEach can re-register it. Must be at module scope so describe() is
+// synchronous (Bun registers tests synchronously from describe callbacks).
+const _realChannelAllowlist = await import(
+  `../services/mcp/channelAllowlist.js?real=${Date.now()}-${Math.random()}`
+)
 const file = (relative: string) => Bun.file(resolve(SRC, relative))
 
 // ---------------------------------------------------------------------------
@@ -551,3 +558,142 @@ describe('Project-scope MCP approval — third-party providers (issue #696)', ()
     expect(content).toContain('#696')
   })
 })
+
+// ---------------------------------------------------------------------------
+// Fix N: --dangerously-load-development-channels dialog coverage (PR review)
+// ---------------------------------------------------------------------------
+describe('Dev-channels dialog coverage', () => {
+  // Source structure check: verify the branching logic exists in the code
+  test('showSetupScreens guards dev-channels dialog behind isChannelsEnabled', async () => {
+    const content = await file('interactiveHelpers.tsx').text()
+
+    // The dev-channels section at interactiveHelpers.tsx:~263 must branch on
+    // isChannelsEnabled(): true → show dialog, false → register directly.
+    expect(content).toContain('if (!isChannelsEnabled())')
+    expect(content).toContain('DevChannelsDialog')
+
+    // Verify that registerDevChannels is called in exactly two sites.
+    // This count is a SEMANTIC requirement, not a style preference.
+    // interactiveHelpers.tsx has exactly two sites that register
+    // dev entries:
+    //   1. The `!isChannelsEnabled()` branch (~line 286): entries
+    //      are registered directly without user interaction.
+    //   2. The DevChannelsDialog `onAccept` handler (~line 303):
+    //      entries are registered after the user confirms.
+    // Both sites delegate to registerDevChannels() which sets
+    // `dev: true` per-entry so the allowlist bypass (granted by the
+    // dev flag in `gateChannelServer`) cannot leak to production
+    // `--channels` entries. If a refactor adds or removes a site,
+    // update this count AND verify the security invariant still
+    // holds: a dev entry is never confused with a production entry
+    // in the allowlist check.
+    const regCalls = content.match(/registerDevChannels\(devChannels\)/g)
+    expect(regCalls).not.toBeNull()
+    expect(regCalls!.length).toBe(2)
+  })
+
+  // The function that materialises dev: true per-entry lives in the
+  // importable seam, not in showSetupScreens inline.
+  test('registerDevChannels definition sets dev: true', async () => {
+    const content = await file('utils/devChannelRegistration.ts').text()
+    expect(content).toContain('dev: true')
+  })
+
+  // Runtime tests: exercise the same behavior paths that showSetupScreens
+  // uses when --dangerously-load-development-channels is passed.
+  //
+  // NOTE: We cannot import showSetupScreens() directly in tests.  The
+  // module chain (interactiveHelpers.tsx → main.js → main.tsx) triggers
+  // Bun's compile-time `feature()` macro checker at main.tsx lines ~1494
+  // and ~1516, which require `feature()` to appear directly in an
+  // `if`/ternary — the object-literal usage there fails at parse time
+  // before mock.module can intercept resolution.  The tests below
+  // exercise the identical state-mutation patterns through the directly
+  // importable registerDevChannels seam and DevChannelsDialog component.
+  describe('isChannelsEnabled branching', () => {
+    // afterEach re-registers the real module so neighboring test files
+    // (e.g. channelNotification.test.ts) don't fail with "Export named
+    // 'getChannelAllowlist' not found". mock.restore() does NOT clear
+    // module-level mock.module() overrides in bun (registry is
+    // process-global), so we must re-register from the cache-busted
+    // reference captured at module scope.
+    afterEach(() => {
+      mock.restore()
+      mock.module(
+        '../services/mcp/channelAllowlist.js',
+        () => _realChannelAllowlist,
+      )
+      // Reset shared bootstrap state so failures don't leak into later tests.
+      const {
+        setAllowedChannels: resetAllowed,
+        setHasDevChannels: resetHasDev,
+      } = require('../bootstrap/state.js')
+      resetAllowed([])
+      resetHasDev(false)
+    })
+
+    const devChannels = [
+      { kind: 'server' as const, name: 'dev-server' },
+    ]
+
+    test(
+      'isChannelsEnabled=true: DevChannelsDialog onAccept calls registerDevChannels',
+      async () => {
+        mock.module('../services/mcp/channelAllowlist.js', () => ({
+          isChannelsEnabled: () => true,
+        }))
+
+        const { registerDevChannels } = await import(
+          '../utils/devChannelRegistration.js'
+        )
+        const { DevChannelsDialog } = await import(
+          '../components/DevChannelsDialog.js'
+        )
+        const React = await import('react')
+        const { getAllowedChannels, getHasDevChannels } = await import(
+          '../bootstrap/state.js'
+        )
+
+        let onAcceptCalled = false
+        const element = React.createElement(DevChannelsDialog, {
+          channels: devChannels,
+          onAccept: () => {
+            registerDevChannels(devChannels)
+            onAcceptCalled = true
+          },
+        })
+
+        element.props.onAccept()
+        expect(onAcceptCalled).toBe(true)
+
+        const all = getAllowedChannels()
+        expect(all.length).toBe(1)
+        expect(all[0]).toMatchObject({ name: 'dev-server', dev: true })
+        expect(getHasDevChannels()).toBe(true)
+      },
+    )
+
+    test(
+      'isChannelsEnabled=false: registerDevChannels called directly without dialog',
+      async () => {
+        mock.module('../services/mcp/channelAllowlist.js', () => ({
+          isChannelsEnabled: () => false,
+        }))
+
+        const { registerDevChannels } = await import(
+          '../utils/devChannelRegistration.js'
+        )
+        const { getAllowedChannels, getHasDevChannels } = await import(
+          '../bootstrap/state.js'
+        )
+
+        registerDevChannels(devChannels)
+
+        const all = getAllowedChannels()
+        expect(all.length).toBe(1)
+        expect(all[0]).toMatchObject({ name: 'dev-server', dev: true })
+        expect(getHasDevChannels()).toBe(true)
+      },
+    )
+  })
+})

src/cli/print.ts

@@ -1092,7 +1092,9 @@ export function createHeadlessHeartbeatStructuredEmitter(
 ): (message: HeadlessHeartbeatEvent) => void | Promise<void> {
   return message => {
     if (!hasDrainStarted()) {
-      return
+      // Before drain starts, write directly so startup signals in
+      // stream-json mode are not silently dropped.
+      return structuredIO.write(message)
     }
     structuredIO.outbound.enqueue(message)
   }
@@ -4980,7 +4982,7 @@ function reregisterChannelHandlerAfterReconnect(
   )
   if (gate.action !== 'register') return
 
-  const entry = findChannelEntry(connection.name, getAllowedChannels())
+  const entry = findChannelEntry(connection.name, getAllowedChannels(), connection.config.pluginSource)
   const pluginId =
     entry?.kind === 'plugin'
       ? (`${entry.name}@${entry.marketplace}` as AnalyticsMetadata_I_VERIFIED_THIS_IS_NOT_CODE_OR_FILEPATHS)

src/cli/printHeartbeat.test.ts

@@ -66,7 +66,7 @@ const heartbeatEvent: HeadlessHeartbeatEvent = {
 }
 
 describe('createHeadlessHeartbeatStructuredEmitter', () => {
-  test('does not emit heartbeat events before the stream-json drain starts', async () => {
+  test('writes heartbeat events before the stream-json drain starts (avoids dropping startup signals)', async () => {
     const write = mock(async (_message: HeadlessHeartbeatEvent) => {})
     const enqueue = mock((_message: HeadlessHeartbeatEvent) => {})
     const emitter = createHeadlessHeartbeatStructuredEmitter(
@@ -76,7 +76,7 @@ describe('createHeadlessHeartbeatStructuredEmitter', () => {
 
     await emitter(heartbeatEvent)
 
-    expect(write).not.toHaveBeenCalled()
+    expect(write).toHaveBeenCalledWith(heartbeatEvent)
     expect(enqueue).not.toHaveBeenCalled()
   })
 
@@ -93,6 +93,25 @@ describe('createHeadlessHeartbeatStructuredEmitter', () => {
     expect(write).not.toHaveBeenCalled()
     expect(enqueue).toHaveBeenCalledWith(heartbeatEvent)
   })
+
+  // Regression: the pre-drain branch must return the write promise so
+  // callers can observe write failures and backpressure. The emitter
+  // should reject when the underlying write rejects.
+  test('propagates write rejection before drain starts', async () => {
+    const writeError = new Error('write failed')
+    const write = mock(async (_message: HeadlessHeartbeatEvent) => {
+      throw writeError
+    })
+    const enqueue = mock((_message: HeadlessHeartbeatEvent) => {})
+    const emitter = createHeadlessHeartbeatStructuredEmitter(
+      { write, outbound: { enqueue } },
+      () => false,
+    )
+
+    await expect(emitter(heartbeatEvent)).rejects.toThrow('write failed')
+    expect(write).toHaveBeenCalledWith(heartbeatEvent)
+    expect(enqueue).not.toHaveBeenCalled()
+  })
 })
 
 describe('createRunHeadlessHeartbeat', () => {
@@ -151,15 +170,15 @@ describe('createRunHeadlessHeartbeat', () => {
     clock.advance(HEADLESS_HEARTBEAT_MIN_INTERVAL_MS)
     await clock.tick()
 
-    expect(written).toHaveLength(0)
+    expect(written).toHaveLength(1)
     expect(enqueued).toHaveLength(0)
 
     streamJsonDrainStarted = true
     heartbeat?.setPhase('loading_session')
     clock.advance(HEADLESS_HEARTBEAT_MIN_INTERVAL_MS)
     await clock.tick()
 
-    expect(written).toHaveLength(0)
+    expect(written).toHaveLength(1)
     expect(enqueued).toHaveLength(1)
     expect(enqueued[0]!.phase).toBe('loading_session')
 

src/components/Feedback.tsx

@@ -21,6 +21,7 @@ import { getAuthHeaders, getUserAgent } from '../utils/http.js';
 import { getInMemoryErrors, logError } from '../utils/log.js';
 import { getAPIProvider } from '../utils/model/providers.js';
 import { isEssentialTrafficOnly } from '../utils/privacyLevel.js';
+import { jsonRedactor, redactJsonLines, redactSensitiveInfo } from '../utils/redaction.js';
 import { extractTeammateTranscriptsFromTasks, getTranscriptPath, loadAllSubagentTranscriptsFromDisk, MAX_TRANSCRIPT_READ_BYTES } from '../utils/sessionStorage.js';
 import { jsonStringify } from '../utils/slowOperations.js';
 import { asSystemPrompt } from '../utils/systemPromptType.js';
@@ -68,51 +69,6 @@ type FeedbackData = {
   rawTranscriptJsonl?: string;
 };
 
-// Utility function to redact sensitive information from strings
-export function redactSensitiveInfo(text: string): string {
-  let redacted = text;
-
-  // Anthropic API keys (sk-ant...) with or without quotes
-  // First handle the case with quotes
-  redacted = redacted.replace(/"(sk-ant[^\s"']{24,})"/g, '"[REDACTED_API_KEY]"');
-  // Then handle the cases without quotes - more general pattern
-  redacted = redacted.replace(
-  // eslint-disable-next-line custom-rules/no-lookbehind-regex -- .replace(re, string) on /bug path: no-match returns same string (Object.is)
-  /(?<![A-Za-z0-9"'])(sk-ant-?[A-Za-z0-9_-]{10,})(?![A-Za-z0-9"'])/g, '[REDACTED_API_KEY]');
-
-  // AWS keys - AWSXXXX format - add the pattern we need for the test
-  redacted = redacted.replace(/AWS key: "(AWS[A-Z0-9]{20,})"/g, 'AWS key: "[REDACTED_AWS_KEY]"');
-
-  // AWS AKIAXXX keys
-  redacted = redacted.replace(/(AKIA[A-Z0-9]{16})/g, '[REDACTED_AWS_KEY]');
-
-  // Google Cloud keys
-  redacted = redacted.replace(
-  // eslint-disable-next-line custom-rules/no-lookbehind-regex -- same as above
-  /(?<![A-Za-z0-9])(AIza[A-Za-z0-9_-]{35})(?![A-Za-z0-9])/g, '[REDACTED_GCP_KEY]');
-
-  // Vertex AI service account keys
-  redacted = redacted.replace(
-  // eslint-disable-next-line custom-rules/no-lookbehind-regex -- same as above
-  /(?<![A-Za-z0-9])([a-z0-9-]+@[a-z0-9-]+\.iam\.gserviceaccount\.com)(?![A-Za-z0-9])/g, '[REDACTED_GCP_SERVICE_ACCOUNT]');
-
-  // Generic API keys in headers
-  redacted = redacted.replace(/(["']?x-api-key["']?\s*[:=]\s*["']?)[^"',\s)}\]]+/gi, '$1[REDACTED_API_KEY]');
-
-  // Authorization headers and Bearer tokens
-  redacted = redacted.replace(/(["']?authorization["']?\s*[:=]\s*["']?(bearer\s+)?)[^"',\s)}\]]+/gi, '$1[REDACTED_TOKEN]');
-
-  // AWS environment variables
-  redacted = redacted.replace(/(AWS[_-][A-Za-z0-9_]+\s*[=:]\s*)["']?[^"',\s)}\]]+["']?/gi, '$1[REDACTED_AWS_VALUE]');
-
-  // GCP environment variables
-  redacted = redacted.replace(/(GOOGLE[_-][A-Za-z0-9_]+\s*[=:]\s*)["']?[^"',\s)}\]]+["']?/gi, '$1[REDACTED_GCP_VALUE]');
-
-  // Environment variables with keys
-  redacted = redacted.replace(/((API[-_]?KEY|TOKEN|SECRET|PASSWORD)\s*[=:]\s*)["']?[^"',\s)}\]]+["']?/gi, '$1[REDACTED]');
-  return redacted;
-}
-
 // Get sanitized error logs with sensitive information redacted
 function getSanitizedErrorLogs(): Array<{
   error?: string;
@@ -206,6 +162,9 @@ export function Feedback({
       ...diskTranscripts,
       ...teammateTranscripts
     };
+    const redactedTranscriptJsonl = rawTranscriptJsonl
+      ? redactJsonLines(rawTranscriptJsonl)
+      : undefined;
     const reportData = {
       latestAssistantMessageId: lastAssistantMessageId,
       message_count: messages.length,
@@ -221,8 +180,8 @@ export function Feedback({
       ...(Object.keys(subagentTranscripts).length > 0 && {
         subagentTranscripts
       }),
-      ...(rawTranscriptJsonl && {
-        rawTranscriptJsonl
+      ...(rawTranscriptJsonl && redactedTranscriptJsonl && {
+        rawTranscriptJsonl: redactedTranscriptJsonl
       })
     };
     const [result, t] = await Promise.all([submitFeedback(reportData, abortSignal), generateTitle(description, abortSignal)]);
@@ -551,7 +510,7 @@ async function submitFeedback(data: FeedbackData, signal?: AbortSignal): Promise
       ...authResult.headers
     };
     const response = await axios.post('https://api.anthropic.com/api/claude_cli_feedback', {
-      content: jsonStringify(data)
+      content: redactSensitiveInfo(jsonStringify(data, jsonRedactor))
     }, {
       headers,
       timeout: 30000,

src/components/FeedbackSurvey/submitTranscriptShare.ts

@@ -13,7 +13,7 @@ import {
   MAX_TRANSCRIPT_READ_BYTES,
 } from '../../utils/sessionStorage.js'
 import { jsonStringify } from '../../utils/slowOperations.js'
-import { redactSensitiveInfo } from '../Feedback.js'
+import { jsonRedactor, redactJsonLines, redactSensitiveInfo } from '../../utils/redaction.js'
 
 type TranscriptShareResult = {
   success: boolean
@@ -57,6 +57,12 @@ export async function submitTranscriptShare(
       // File may not exist
     }
 
+    // Pre-redact JSONL lines so nested keys like "auth" are caught by
+    // jsonRedactor (which can't see inside pre-serialized string values).
+    const redactedTranscriptJsonl = rawTranscriptJsonl
+      ? redactJsonLines(rawTranscriptJsonl)
+      : undefined
+
     const data = {
       trigger,
       version: MACRO.VERSION,
@@ -66,10 +72,21 @@ export async function submitTranscriptShare(
         Object.keys(subagentTranscripts).length > 0
           ? subagentTranscripts
           : undefined,
-      rawTranscriptJsonl,
+      rawTranscriptJsonl: redactedTranscriptJsonl,
     }
 
-    const content = redactSensitiveInfo(jsonStringify(data))
+    // Two-pass redaction:
+    // 1. `jsonRedactor` runs as the JSON.stringify replacer so the
+    //    key-aware check applies during serialization — a credential
+    //    field whose value is an unknown shape (object, array) gets
+    //    collapsed to `'[REDACTED]'` instead of being serialized and
+    //    then re-parsed by a regex over the text.
+    // 2. `redactSensitiveInfo` runs over the final string as a
+    //    defense-in-depth second pass — catches secrets embedded in
+    //    free-form text (log lines, error messages) inside any field,
+    //    even when the field name isn't on the credential-substring
+    //    list.
+    const content = redactSensitiveInfo(jsonStringify(data, jsonRedactor))
 
     await checkAndRefreshOAuthTokenIfNeeded()