Skip to content

Commit ba6435f

Browse files
authored
feat(packaged): attach crash-scene evidence to packaged_runtime_failed (#5224)
* feat(packaged): attach crash-scene evidence to packaged_runtime_failed The pre-daemon startup crash class (`packaged_runtime_failed`, #4696) reports only structured buckets today: failure_kind, exit_code, error_name, and a log-tail-parsed error_code/missing_module. That is enough to see THAT startup failed, but not WHY on a given machine: - The mac `daemon-start` failures are better-sqlite3 ERR_MODULE_NOT_FOUND, yet the shipped 0.13.0 DMG's `better_sqlite3.node` is verified present, arm64, signed, notarized and resolvable — so this is a machine-side subset, not a build defect, and we have no per-machine signal to explain the subset. - The Windows `unknown` bucket (the single largest slice) carries no exit code and no daemon log to parse, so today it is a dead end. Enrich the event with on-machine crash evidence: the scrubbed + truncated top-level error message/stack (the only signal the `unknown` bucket has), and a best-effort probe of the daemon's better-sqlite3 binding on THIS machine (present + size), which separates "file missing" from "file present but unloadable" (arch mismatch / quarantine / AV). All paths and free-form text are run through the existing `scrubUserPaths` and length-capped before send. - startup-telemetry.ts: add `error_message`, `error_stack`, `native_module_present/size/path`; new `nativeModulePath` arg + injectable `statNativeModule` dep. - index.ts: pass the packaged binding path (`getAppPath()/node_modules/better-sqlite3/build/Release/better_sqlite3.node`, layout verified against the shipped DMG) through the fatal-exit report. - contracts: extend `PackagedRuntimeFailedProps` with the optional fields. - tests: red-spec first — assert scrubbed message/stack and the native probe (present/missing) via the injected `statNativeModule`. * fix(packaged): scrub full Windows profile segment even with spaces Review (PerishCode/Looper): the new free-form error_message/error_stack fields run through scrubUserPaths, but its Windows branch matched `[^\\\s]+` — it stopped at the first whitespace. A Windows profile dir can contain a space ("C:\Users\John Doe\..."), so it produced "C:\Users\<redacted> Doe\..." and leaked the rest of the segment, breaking this PR's stated privacy bound. Consume the whole segment up to the next backslash (`[^\\]+`). POSIX home segments cannot contain spaces and file:// URLs percent-encode them, so only this backslash form needs the whitespace-tolerant boundary. Add fixture coverage for the spaced profile — both standalone and embedded in a crash message — asserting the surname never survives. * fix(packaged): make Windows-home scrub separator-agnostic Review round 2 (PerishCode/Looper): the prior fix covered the backslash form, but slash-normalized Windows paths ("C:/Users/John Doe/...", which JS/Electron/ Node diagnostics commonly emit) still leaked the surname — the POSIX "/Users/" rule matched them and stopped at the first space, producing "C:/Users/<redacted> Doe/...". Replace the backslash-only Windows rule with a separator-agnostic one that matches `<drive>:[\\/]Users[\\/]` and consumes the whole segment up to the next slash OR backslash (spaces allowed), running before the POSIX rule. The `\r\n` in the class keeps it from running across lines in a multi-line stack. POSIX segments can't contain spaces, so that rule is left with its whitespace boundary. Fixtures added for the slash form (standalone + embedded) and a multi-line no-over-redaction case.
1 parent 1eb3898 commit ba6435f

4 files changed

Lines changed: 220 additions & 5 deletions

File tree

apps/packaged/src/index.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ let startupTelemetryContext:
5555
namespace: string;
5656
source: string;
5757
installationRoot: string;
58+
nativeModulePath: string | null;
5859
}
5960
| null = null;
6061

@@ -129,6 +130,18 @@ async function main(): Promise<void> {
129130
// Pass installationRoot explicitly: OD_INSTALLATION_DIR is only set in the
130131
// daemon child env, not this parent process (see startup-telemetry.ts).
131132
installationRoot: paths.installationRoot,
133+
// Absolute path where the daemon's better-sqlite3 binding ships in the
134+
// packaged bundle (`Contents/Resources/app/node_modules/...` — layout
135+
// verified against the shipped 0.13.0 DMG). The fatal-exit report probes
136+
// this to record whether the .node actually exists on the crashing machine.
137+
nativeModulePath: join(
138+
app.getAppPath(),
139+
"node_modules",
140+
"better-sqlite3",
141+
"build",
142+
"Release",
143+
"better_sqlite3.node",
144+
),
132145
};
133146

134147
await ensurePackagedNamespacePaths(paths);
@@ -284,6 +297,7 @@ void main().catch(async (error: unknown) => {
284297
appVersion: startupTelemetryContext.appVersion,
285298
namespace: startupTelemetryContext.namespace,
286299
source: startupTelemetryContext.source,
300+
nativeModulePath: startupTelemetryContext.nativeModulePath,
287301
});
288302
}
289303
process.exit(1);

apps/packaged/src/startup-telemetry.ts

Lines changed: 82 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,15 @@
2222
// in apps/daemon/src/analytics.ts — stability data is retained even for
2323
// opted-out users (and the main process cannot read daemon consent anyway,
2424
// since the daemon isn't up). The Settings → Privacy copy MUST call this out.
25+
//
26+
// Payload PII: the free-form crash fields (error_message, error_stack) and every
27+
// path we send (log_path, native_module_path) run through `scrubUserPaths` to
28+
// strip the user's home dir, and the free-form text is length-capped. Startup
29+
// errors are module-resolution / daemon-exit messages, not user content, so this
30+
// bounds the exposure to build/OS strings rather than anything the user typed.
2531

2632
import { readFile } from "node:fs/promises";
27-
import { readFileSync } from "node:fs";
33+
import { readFileSync, statSync } from "node:fs";
2834
import { join } from "node:path";
2935
import { release } from "node:os";
3036

@@ -146,8 +152,19 @@ export function parseDaemonLogTail(logText: string): {
146152
// can't import the web module anyway, so we ship a focused scrubber here.
147153
export function scrubUserPaths(value: string): string {
148154
return value
149-
.replace(/\/(Users|home)\/[^/\s]+/g, "/$1/<redacted>")
150-
.replace(/([A-Za-z]:\\Users\\)[^\\\s]+/g, "$1<redacted>");
155+
// Windows profile dirs FIRST, either separator style (`C:\Users\…` or the
156+
// slash-normalized `C:/Users/…` that JS/Electron/Node diagnostics commonly
157+
// emit). Consume the WHOLE segment up to the next slash/backslash — a
158+
// profile dir can contain spaces ("John Doe"), and a whitespace boundary
159+
// would leak the tail ("<redacted> Doe/…"). Anchored on `<drive>:` so it
160+
// only fires on a real Windows home path; `\r\n` in the class stops it from
161+
// running across lines in a multi-line stack. Runs before the POSIX rule
162+
// because that rule also matches the "/Users/" inside a `C:/Users/` path.
163+
.replace(/([A-Za-z]:[\\/]Users[\\/])[^\\/\r\n]+/g, "$1<redacted>")
164+
// POSIX home dirs. Real macOS/Linux home segments cannot contain spaces, so
165+
// the whitespace boundary is correct here and avoids over-redacting a
166+
// following word in free-form crash text.
167+
.replace(/\/(Users|home)\/[^/\s]+/g, "/$1/<redacted>");
151168
}
152169

153170
function osName(platform: NodeJS.Platform = process.platform): string {
@@ -196,6 +213,28 @@ async function defaultReadLogTail(path: string): Promise<string | null> {
196213
}
197214
}
198215

216+
// Keep the message/stack payload bounded (a stack can be arbitrarily long).
217+
const ERROR_MESSAGE_MAX = 1000;
218+
const ERROR_STACK_MAX = 2000;
219+
220+
function truncateForTelemetry(value: string, max: number): string {
221+
return value.length > max ? `${value.slice(0, max)}…[+${value.length - max} chars]` : value;
222+
}
223+
224+
// Best-effort probe: does the native module actually exist on THIS machine, and
225+
// how big is it? The field crash is a subset of machines rather than a build
226+
// defect (the shipped .node is present + signed + resolvable), so per-machine
227+
// existence/size is the signal that separates "file missing" from "file present
228+
// but unloadable (arch/quarantine/AV)".
229+
function defaultStatNativeModule(path: string): { size: number } | null {
230+
try {
231+
const s = statSync(path);
232+
return s.isFile() ? { size: s.size } : null;
233+
} catch {
234+
return null;
235+
}
236+
}
237+
199238
export interface CaptureDeps {
200239
fetchImpl?: typeof fetch;
201240
timeoutMs?: number;
@@ -277,10 +316,15 @@ export interface ReportStartupFailureArgs {
277316
appVersion: string | null;
278317
namespace: string;
279318
source: string;
319+
// Absolute path to the daemon's better-sqlite3 native binding, so the report
320+
// can probe whether that .node exists on this machine. Optional: null skips
321+
// the probe (fields report null).
322+
nativeModulePath?: string | null;
280323
}
281324

282325
export interface ReportDeps extends CaptureDeps {
283326
readLogTail?: (path: string) => Promise<string | null>;
327+
statNativeModule?: (path: string) => { size: number } | null;
284328
}
285329

286330
// The single entry point index.ts's fatal-exit catch calls. Orchestrates
@@ -302,15 +346,48 @@ export async function reportStartupFailure(
302346
missingModule = parsed.missingModule;
303347
}
304348
}
349+
const rawMessage =
350+
args.error instanceof Error
351+
? args.error.message
352+
: args.error == null
353+
? ""
354+
: String(args.error);
355+
const rawStack =
356+
args.error instanceof Error && typeof args.error.stack === "string"
357+
? args.error.stack
358+
: null;
359+
// Probe the native module on THIS machine (existence + size). This is the
360+
// signal the `unknown` bucket (no daemon log to parse) otherwise lacks, and
361+
// it distinguishes "file missing" from "file present but unloadable".
362+
let nativeModulePresent: boolean | null = null;
363+
let nativeModuleSize: number | null = null;
364+
let nativeModulePath: string | null = null;
365+
if (args.nativeModulePath) {
366+
const stat = (deps.statNativeModule ?? defaultStatNativeModule)(args.nativeModulePath);
367+
nativeModulePresent = stat != null;
368+
nativeModuleSize = stat?.size ?? null;
369+
nativeModulePath = scrubUserPaths(args.nativeModulePath);
370+
}
305371
const properties: Record<string, unknown> = {
306372
failure_kind: classification.failureKind,
307373
exit_code: classification.exitCode,
308374
signal: classification.signal,
309375
error_name: args.error instanceof Error ? args.error.name : "unknown",
310376
error_code: errorCode ?? null,
311377
missing_module: missingModule ?? null,
312-
// Structured fields only — no raw message/stack. Scrub the one path we do
313-
// send so a user's home dir never reaches PostHog.
378+
// Scrub every path we send (message/stack/log/native path) so a user's
379+
// home dir never reaches PostHog; truncate free-form text to bound the
380+
// payload. These crash-scene fields are why the mac subset can't resolve
381+
// the module and what the Windows `unknown` bucket actually threw.
382+
error_message: rawMessage
383+
? truncateForTelemetry(scrubUserPaths(rawMessage), ERROR_MESSAGE_MAX)
384+
: null,
385+
error_stack: rawStack
386+
? truncateForTelemetry(scrubUserPaths(rawStack), ERROR_STACK_MAX)
387+
: null,
388+
native_module_present: nativeModulePresent,
389+
native_module_size: nativeModuleSize,
390+
native_module_path: nativeModulePath,
314391
log_path: classification.logPath ? scrubUserPaths(classification.logPath) : null,
315392
app_version: args.appVersion,
316393
namespace: args.namespace,

apps/packaged/tests/startup-telemetry.test.ts

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,47 @@ describe('scrubUserPaths', () => {
103103
'C:\\Users\\<redacted>\\AppData\\Roaming',
104104
);
105105
});
106+
107+
it('redacts the FULL Windows profile segment even when it contains spaces', () => {
108+
// A Windows profile dir can contain whitespace ("John Doe"). The redaction
109+
// must consume the whole segment up to the next separator, not stop at the
110+
// first space and leak the surname.
111+
const scrubbed = scrubUserPaths('C:\\Users\\John Doe\\AppData\\Roaming');
112+
expect(scrubbed).toBe('C:\\Users\\<redacted>\\AppData\\Roaming');
113+
expect(scrubbed).not.toContain('Doe');
114+
});
115+
116+
it('scrubs a spaced Windows profile embedded in a crash message/stack', () => {
117+
const raw =
118+
"Cannot find package 'better-sqlite3' imported from C:\\Users\\John Doe\\App\\Resources\\app\\daemon\\server.mjs";
119+
const scrubbed = scrubUserPaths(raw);
120+
expect(scrubbed).not.toContain('John Doe');
121+
expect(scrubbed).not.toContain('Doe');
122+
expect(scrubbed).toContain('C:\\Users\\<redacted>\\App\\Resources');
123+
});
124+
125+
it('redacts SLASH-separated Windows home paths with spaces (forward-slash form)', () => {
126+
// JS/Electron/Node diagnostics frequently normalize Windows paths to forward
127+
// slashes; the profile segment can still contain a literal space. The POSIX
128+
// "/Users/" rule alone would stop at the space and leak the tail.
129+
expect(scrubUserPaths('C:/Users/John Doe/AppData/Roaming')).toBe(
130+
'C:/Users/<redacted>/AppData/Roaming',
131+
);
132+
});
133+
134+
it('scrubs a slash-form spaced Windows profile embedded in a crash message', () => {
135+
const raw =
136+
"Cannot find package 'better-sqlite3' imported from C:/Users/John Doe/App/Resources/app/daemon/server.mjs";
137+
const scrubbed = scrubUserPaths(raw);
138+
expect(scrubbed).not.toContain('John Doe');
139+
expect(scrubbed).not.toContain('Doe');
140+
expect(scrubbed).toContain('C:/Users/<redacted>/App/Resources');
141+
});
142+
143+
it('does not over-redact across lines in a multi-line stack', () => {
144+
const scrubbed = scrubUserPaths('a C:\\Users\\John Doe\\x\nb /Users/bob/y');
145+
expect(scrubbed).toBe('a C:\\Users\\<redacted>\\x\nb /Users/<redacted>/y');
146+
});
106147
});
107148

108149
describe('resolveStartupDistinctId', () => {
@@ -292,4 +333,73 @@ describe('reportStartupFailure', () => {
292333
);
293334
expect(fetchImpl).not.toHaveBeenCalled();
294335
});
336+
337+
// Why these exist: the field crash is a SUBSET of machines (verified 2026-07-06
338+
// — the shipped 0.13.0 DMG's better_sqlite3.node is present, signed, notarized,
339+
// and resolvable, so it is NOT a build defect). To learn WHY a given machine
340+
// can't load it, the event must carry on-machine evidence: the scrubbed error
341+
// message/stack (the only signal for the `unknown` bucket, which has no daemon
342+
// log to parse) and whether the native module file actually exists there.
343+
it('captures scrubbed error message/stack + native-module probe (on-machine evidence)', async () => {
344+
const fetchImpl = vi.fn().mockResolvedValue(new Response('ok'));
345+
const error = new Error(
346+
"Cannot find package 'better-sqlite3' imported from /Users/liudetao/App/Contents/Resources/app/prebundled/daemon/server.mjs",
347+
);
348+
error.stack = `${error.message}\n at file:///Users/liudetao/App/Contents/Resources/app/prebundled/daemon/server.mjs:1:1`;
349+
await reportStartupFailure(
350+
{
351+
error,
352+
isPathAccess: false,
353+
posthogKey: 'phc_test',
354+
posthogHost: null,
355+
distinctId: 'd',
356+
appVersion: '0.13.0',
357+
namespace: 'release-stable',
358+
source: 'packaged',
359+
nativeModulePath:
360+
'/Users/liudetao/App/Contents/Resources/app/node_modules/better-sqlite3/build/Release/better_sqlite3.node',
361+
},
362+
{
363+
fetchImpl: fetchImpl as unknown as typeof fetch,
364+
readLogTail: async () => null,
365+
statNativeModule: (p) => (p.endsWith('better_sqlite3.node') ? { size: 1_234_567 } : null),
366+
},
367+
);
368+
const [, init] = fetchImpl.mock.calls[0] as [string, RequestInit];
369+
const props = (JSON.parse(init.body as string) as { properties: Record<string, unknown> }).properties;
370+
// scrubbed message/stack: content kept, home dir gone.
371+
expect(props.error_message).toContain("Cannot find package 'better-sqlite3'");
372+
expect(props.error_message).not.toContain('liudetao');
373+
expect(String(props.error_stack)).not.toContain('liudetao');
374+
// native-module probe answers "is the .node actually on THIS machine".
375+
expect(props.native_module_present).toBe(true);
376+
expect(props.native_module_size).toBe(1_234_567);
377+
expect(props.native_module_path).not.toContain('liudetao');
378+
});
379+
380+
it('reports native_module_present=false when the .node is missing on the machine', async () => {
381+
const fetchImpl = vi.fn().mockResolvedValue(new Response('ok'));
382+
await reportStartupFailure(
383+
{
384+
error: new Error('boom'),
385+
isPathAccess: false,
386+
posthogKey: 'phc_test',
387+
posthogHost: null,
388+
distinctId: 'd',
389+
appVersion: '0.13.0',
390+
namespace: 'release-stable',
391+
source: 'packaged',
392+
nativeModulePath: '/a/b/better_sqlite3.node',
393+
},
394+
{
395+
fetchImpl: fetchImpl as unknown as typeof fetch,
396+
readLogTail: async () => null,
397+
statNativeModule: () => null,
398+
},
399+
);
400+
const [, init] = fetchImpl.mock.calls[0] as [string, RequestInit];
401+
const props = (JSON.parse(init.body as string) as { properties: Record<string, unknown> }).properties;
402+
expect(props.native_module_present).toBe(false);
403+
expect(props.native_module_size).toBeNull();
404+
});
295405
});

packages/contracts/src/analytics/events/result-events.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -663,6 +663,20 @@ export interface PackagedRuntimeFailedProps {
663663
// The unresolved module when error_code is a module-resolution failure
664664
// (e.g. `better-sqlite3` for #4638).
665665
missing_module: string | null;
666+
// Crash-scene evidence added for the field-crash subset (#4638 follow-up): the
667+
// shipped build is verified-good, so these separate a machine-side "module
668+
// missing/unloadable" from a code path, and give the Windows `unknown` bucket
669+
// (which has no daemon log to parse) its only signal. All scrubbed of the
670+
// user's home dir and truncated before send.
671+
//
672+
// Free-form error text off the top-level thrown error (not the log tail).
673+
error_message?: string | null;
674+
error_stack?: string | null;
675+
// Probe of the daemon's better-sqlite3 native binding on THIS machine.
676+
// present=null when no path was supplied; size is bytes when present.
677+
native_module_present?: boolean | null;
678+
native_module_size?: number | null;
679+
native_module_path?: string | null;
666680
// Scrubbed of the user's home dir before send.
667681
log_path: string | null;
668682
app_version: string | null;

0 commit comments

Comments
 (0)