From 1d38eae536a67bcd91431d43ca33441941524db7 Mon Sep 17 00:00:00 2001 From: unraid Date: Fri, 3 Apr 2026 04:08:04 +0800 Subject: [PATCH] fix: address CodeRabbit review findings - webhookSanitizer: redact before truncate to avoid split secrets at boundary - webhookSanitizer: return safe placeholder on error instead of raw content - peerSessions: use discriminated union return type for type safety Co-Authored-By: Claude Opus 4.6 (1M context) --- src/bridge/peerSessions.ts | 2 +- src/bridge/webhookSanitizer.ts | 20 ++++++++++---------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/bridge/peerSessions.ts b/src/bridge/peerSessions.ts index dad8477f7..4d792c06d 100644 --- a/src/bridge/peerSessions.ts +++ b/src/bridge/peerSessions.ts @@ -18,7 +18,7 @@ import { toCompatSessionId } from './sessionIdCompat.js' export async function postInterClaudeMessage( target: string, message: string, -): Promise<{ ok: boolean; error?: string }> { +): Promise<{ ok: true } | { ok: false; error: string }> { try { const handle = getReplBridgeHandle() if (!handle) { diff --git a/src/bridge/webhookSanitizer.ts b/src/bridge/webhookSanitizer.ts index cb48f974d..a2999b07c 100644 --- a/src/bridge/webhookSanitizer.ts +++ b/src/bridge/webhookSanitizer.ts @@ -5,7 +5,7 @@ * Strips known secret patterns (tokens, API keys, credentials) while preserving * the meaningful content (PR titles, descriptions, commit messages, etc.). * - * Must be synchronous and never throw — on error, returns the original content. + * Must be synchronous and never throw — on error, returns a safe placeholder. */ /** Patterns that match known secret/token formats. */ @@ -37,21 +37,21 @@ export function sanitizeInboundWebhookContent(content: string): string { let sanitized = content - // Truncate excessively large payloads - if (sanitized.length > MAX_CONTENT_LENGTH) { - sanitized = sanitized.slice(0, MAX_CONTENT_LENGTH) + '\n... [truncated]' - } - - // Redact known secret patterns + // Redact known secret patterns first (before truncation to avoid + // splitting a secret across the truncation boundary) for (const { pattern, replacement } of SECRET_PATTERNS) { - // Reset lastIndex for global regexes pattern.lastIndex = 0 sanitized = sanitized.replace(pattern, replacement) } + // Truncate excessively large payloads after redaction + if (sanitized.length > MAX_CONTENT_LENGTH) { + sanitized = sanitized.slice(0, MAX_CONTENT_LENGTH) + '\n... [truncated]' + } + return sanitized } catch { - // Never throw — return original content on any error - return content + // Never throw, never return raw content — return a safe placeholder + return '[webhook content redacted due to sanitization error]' } }