feat(packaged): forward the raw daemon log tail in startup-crash telemetry

apps/packaged/src/startup-telemetry.ts

@@ -23,11 +23,15 @@
 // opted-out users (and the main process cannot read daemon consent anyway,
 // since the daemon isn't up). The Settings → Privacy copy MUST call this out.
 //
-// Payload PII: the free-form crash fields (error_message, error_stack) and every
-// path we send (log_path, native_module_path) run through `scrubUserPaths` to
-// strip the user's home dir, and the free-form text is length-capped. Startup
-// errors are module-resolution / daemon-exit messages, not user content, so this
-// bounds the exposure to build/OS strings rather than anything the user typed.
+// Payload PII: the free-form fields (error_message, error_stack, sidecar_log_tail)
+// and every path (log_path, native_module_path) run through `scrubUserPaths`.
+// sidecar_log_tail is additionally WHITELISTED to error/stack-shaped lines
+// (`extractSidecarErrorLines`), so config / connection / KEY=value lines — where
+// the daemon's config/connector/MCP secrets live — are structurally excluded
+// rather than chased by an ever-incomplete denylist. `scrubSecrets` then runs on
+// all free-form fields as defense in depth (a token could still appear inside an
+// error message), and the text is length-capped. This event is sent even for
+// opted-out users, hence the layered structural + denylist controls.
 
 import { readFile } from "node:fs/promises";
 import { readFileSync, statSync } from "node:fs";
@@ -161,12 +165,92 @@ export function scrubUserPaths(value: string): string {
     // running across lines in a multi-line stack. Runs before the POSIX rule
     // because that rule also matches the "/Users/" inside a `C:/Users/` path.
     .replace(/([A-Za-z]:[\\/]Users[\\/])[^\\/\r\n]+/g, "$1<redacted>")
+    // Any `…/Users/<name>`, `…/Profiles/<name>` or `…/home/<name>` segment
+    // regardless of prefix — covers UNC shares (`\\CORP-FS\Profiles\jdoe\…`) and
+    // roaming layouts that the drive-anchored rule above misses. Space-tolerant
+    // (Windows names can contain spaces), line-bounded.
+    .replace(/([\\/](?:Users|Profiles)[\\/])[^\\/\r\n]+/g, "$1<redacted>")
     // POSIX home dirs. Real macOS/Linux home segments cannot contain spaces, so
     // the whitespace boundary is correct here and avoids over-redacting a
     // following word in free-form crash text.
     .replace(/\/(Users|home)\/[^/\s]+/g, "/$1/<redacted>");
 }
 
+// Strip credentials/secrets from free-form telemetry text (daemon log tail,
+// error message/stack). The daemon loads config/connector/MCP tokens before it
+// reports ready, so a startup error can echo a connection string, an auth
+// header, or a `KEY=value` secret — and this event is sent even for opted-out
+// users, so the surface must be scrubbed, not just path-redacted. Best-effort
+// (denylist, never complete), applied ON TOP of scrubUserPaths.
+export function scrubSecrets(value: string): string {
+  return value
+    // Serialized object literals embedded in a message ("error parsing config:
+    // {…}") — a common carrier of config/secret values inside an otherwise-legit
+    // error line, and one the key=value/URL rules below can't reach through JSON
+    // quoting. Redact the whole `{…}` (greedy within a line, so nested objects
+    // go too). `[…]` is deliberately left alone — it's the shape of log prefixes
+    // like `[daemon]`, not where secrets hide.
+    .replace(/\{[^\r\n]*\}/g, "<redacted-object>")
+    // Credentials embedded in a URL / connection string: scheme://user:pass@host.
+    // The password may itself contain '@' (e.g. `user:p@ss@host`), so match the
+    // whole userinfo greedily up to the LAST '@' before the host rather than
+    // stopping at the first '@' (which left `…<redacted>@ss@host` leaking part
+    // of the password). Requires a ':' so a plain `scheme://host` isn't touched.
+    .replace(/([a-zA-Z][a-zA-Z0-9+.-]*:\/\/)[^/\s@]*:[^/\s]*@/g, "$1<redacted>@")
+    // Authorization header — redact the ENTIRE value (scheme + token) to
+    // end-of-line. The generic key=value rule below would only consume the
+    // scheme word ("Bearer"/"Basic") and leave the credential
+    // ("Authorization: Bearer abc" -> "<redacted> abc"), so handle it first.
+    .replace(/(\bAuthorization\s*[:=]\s*)\S[^\r\n]*/gi, "$1<redacted>")
+    // `key = value` / `key: value` secrets (password, token, secret, api_key,
+    // auth, …). `auth` is kept here for bare `auth=…` fields; it can't misfire on
+    // "Authorization"/"author" because a `[=:]` separator must immediately follow
+    // the matched word (the Authorization *header* is handled by the rule above).
+    .replace(
+      /\b(pass(?:word|wd)?|pwd|secret|token|api[_-]?key|access[_-]?key|client[_-]?secret|auth)(\s*[=:]\s*)(?:"[^"\r\n]*"|'[^'\r\n]*'|[^\s"'&]+)/gi,
+      "$1$2<redacted>",
+    )
+    // Inline Bearer/Basic token values not under an Authorization header.
+    .replace(/\b(Bearer|Basic)\s+[A-Za-z0-9._~+/=-]{8,}/g, "$1 <redacted>")
+    // Provider API keys by well-known prefix (OpenAI/Anthropic, PostHog, GitHub, Slack, …).
+    .replace(/\b(?:sk|pk|rk|phx|phc|ghp|gho|ghs|xox[baprs])[-_][A-Za-z0-9_-]{8,}/g, "<redacted-token>")
+    // Bare email addresses.
+    .replace(/[A-Za-z0-9._%+-]+@[A-Za-z0-9.-]+\.[A-Za-z]{2,}/g, "<redacted-email>");
+}
+
+// A line is "diagnostically relevant" if it looks like a thrown error, a stack
+// frame, a Node error code, an exit marker, OR a plain-English failure message
+// ("failed to …", "could not …", "unable to …"). The failure-verb phrasings are
+// how much of our own startup code reports problems (e.g. "daemon failed to
+// resolve listening port", "… was found but could not …"); including them keeps
+// the diagnostic value without re-opening the secret surface, because config /
+// connection / KEY=value lines don't contain those verbs.
+const ERROR_LINE_RE =
+  /(error|exception|fatal|\bpanic\b|assert|\bthrow|unhandled|reject|abort|failed|fail(?:ing|ure)|could ?n['’]?t|could not|cannot|unable|\bERR_[A-Z0-9_]+|^\s+at\s|\bE[A-Z]{2,}\b|\b(?:code|signal)\s*[=:]|exited|not found|refused|denied|timed?\s*out)/i;
+
+// Keep ONLY error/stack-shaped lines from a log tail. This is the primary PII
+// control for `sidecar_log_tail`: rather than trying to scrub every secret shape
+// out of arbitrary log text (a denylist that kept leaking — connection strings,
+// auth headers, KEY=value, quoted values), we structurally exclude the config
+// and connection lines where secrets live, since they don't match an error
+// shape. scrubUserPaths + scrubSecrets still run on what remains as defense in
+// depth (a token could appear inside an error message).
+// An `UPPER_CASE_ENV=value` assignment at line start — the shape of a config /
+// env dump. Excluded even when it matches ERROR_LINE_RE, because a config KEY or
+// value can contain a broad failure substring (`ERROR_REPORT_EMAIL=…`,
+// `SOME_ERROR_TOKEN=…`) and would otherwise re-admit the very secret-bearing
+// lines the whitelist exists to keep out. Real error/stack/exit lines are not
+// `UPPER=…` assignments (`code=1` is lowercase; `TypeError:`/`at …` have no
+// leading `KEY=`).
+const CONFIG_ASSIGNMENT_RE = /^\s*[A-Z][A-Z0-9_]{2,}\s*=/;
+
+export function extractSidecarErrorLines(logText: string): string {
+  return logText
+    .split(/\r?\n/)
+    .filter((line) => ERROR_LINE_RE.test(line) && !CONFIG_ASSIGNMENT_RE.test(line))
+    .join("\n");
+}
+
 function osName(platform: NodeJS.Platform = process.platform): string {
   if (platform === "darwin") return "Mac OS X";
   if (platform === "win32") return "Windows";
@@ -216,11 +300,24 @@ async function defaultReadLogTail(path: string): Promise<string | null> {
 // Keep the message/stack payload bounded (a stack can be arbitrarily long).
 const ERROR_MESSAGE_MAX = 1000;
 const ERROR_STACK_MAX = 2000;
+// The raw daemon log tail we forward. defaultReadLogTail already reads the last
+// 16KB of the file; this bounds what we actually ship to the fatal error and
+// its stack.
+const SIDECAR_LOG_TAIL_MAX = 2500;
 
 function truncateForTelemetry(value: string, max: number): string {
   return value.length > max ? `${value.slice(0, max)}…[+${value.length - max} chars]` : value;
 }
 
+// A log TAIL's most useful lines — the fatal error and its stack — sit at the
+// END, so keep the end when trimming, unlike truncateForTelemetry which keeps
+// the head of a single message/stack.
+function truncateTailForTelemetry(value: string, max: number): string {
+  return value.length > max
+    ? `…[+${value.length - max} chars]${value.slice(value.length - max)}`
+    : value;
+}
+
 // Best-effort probe: does the native module actually exist on THIS machine, and
 // how big is it? The field crash is a subset of machines rather than a build
 // defect (the shipped .node is present + signed + resolvable), so per-machine
@@ -338,12 +435,26 @@ export async function reportStartupFailure(
     const classification = classifyStartupFailure(args.error, args.isPathAccess);
     let errorCode: string | undefined;
     let missingModule: string | undefined;
+    let sidecarLogTail: string | null = null;
     if (classification.logPath) {
       const tail = await (deps.readLogTail ?? defaultReadLogTail)(classification.logPath);
       if (tail) {
         const parsed = parseDaemonLogTail(tail);
         errorCode = parsed.errorCode;
         missingModule = parsed.missingModule;
+        // parseDaemonLogTail only recognises ERR_* and missing-module lines, so
+        // it captures nothing for the majority of code=1 sidecar exits (a plain
+        // Error, a config parse failure, a port bind, an assertion). Emit the
+        // error/stack-shaped lines of the tail so ANY exit reason is diagnosable,
+        // not just those two shapes — but WHITELISTED to error shapes so config /
+        // connection / KEY=value lines (where secrets live) are structurally
+        // excluded, then scrubbed as defense in depth. Null when the tail had no
+        // error-shaped line. Carries whichever sidecar failed — partition on
+        // failure_kind.
+        const errorLines = extractSidecarErrorLines(tail);
+        sidecarLogTail = errorLines
+          ? truncateTailForTelemetry(scrubSecrets(scrubUserPaths(errorLines)), SIDECAR_LOG_TAIL_MAX)
+          : null;
       }
     }
     const rawMessage =
@@ -380,15 +491,16 @@ export async function reportStartupFailure(
       // payload. These crash-scene fields are why the mac subset can't resolve
       // the module and what the Windows `unknown` bucket actually threw.
       error_message: rawMessage
-        ? truncateForTelemetry(scrubUserPaths(rawMessage), ERROR_MESSAGE_MAX)
+        ? truncateForTelemetry(scrubSecrets(scrubUserPaths(rawMessage)), ERROR_MESSAGE_MAX)
         : null,
       error_stack: rawStack
-        ? truncateForTelemetry(scrubUserPaths(rawStack), ERROR_STACK_MAX)
+        ? truncateForTelemetry(scrubSecrets(scrubUserPaths(rawStack)), ERROR_STACK_MAX)
         : null,
       native_module_present: nativeModulePresent,
       native_module_size: nativeModuleSize,
       native_module_path: nativeModulePath,
       log_path: classification.logPath ? scrubUserPaths(classification.logPath) : null,
+      sidecar_log_tail: sidecarLogTail,
       app_version: args.appVersion,
       namespace: args.namespace,
       source: args.source,