From 4f7a2e170e8fd7099d1244f5b7e426082ec13761 Mon Sep 17 00:00:00 2001 From: "posthog[bot]" <206114724+posthog[bot]@users.noreply.github.com> Date: Sat, 29 Aug 2026 19:14:42 +0000 Subject: [PATCH 1/2] fix(cli): stop leaking user paths into error tracking npm prints the user's home directory and name in its `npm error path` and `npm error command` lines. The CLI install step wrapped that raw stderr in an Error and reported it, so personal data reached error tracking and the per-machine path gave every failure its own fingerprint. Sanitize npm output before it becomes an Error: drop the path and command lines, redact the home directory, and give the Error a stable message so the failures group into one issue. Keep the sanitized, truncated output in a `detail` property for diagnosis. Generated-By: PostHog Desktop Task-Id: dcf8f9a1-835d-4277-be7e-7d2704907c64 --- .../index.ts | 2 +- .../__tests__/install-cli-steering.test.ts | 37 ++++++++++++++++ src/steps/install-cli-steering/index.ts | 43 +++++++++++++++---- 3 files changed, 73 insertions(+), 9 deletions(-) diff --git a/src/lib/programs/error-tracking-upload-source-maps/index.ts b/src/lib/programs/error-tracking-upload-source-maps/index.ts index 80db004f5..e4fb26f4a 100644 --- a/src/lib/programs/error-tracking-upload-source-maps/index.ts +++ b/src/lib/programs/error-tracking-upload-source-maps/index.ts @@ -41,7 +41,7 @@ function ensurePostHogCli(variant: SkillVariant): void { analytics.captureException( result.errorObject ?? new Error(`posthog-cli pre-install failed: ${result.error}`), - { source: 'source_maps_cli_preinstall', variant }, + { source: 'source_maps_cli_preinstall', variant, detail: result.detail }, ); getUI().log.warn( `Could not pre-install posthog-cli (${result.error}). Your release build ` + diff --git a/src/steps/install-cli-steering/__tests__/install-cli-steering.test.ts b/src/steps/install-cli-steering/__tests__/install-cli-steering.test.ts index 19a0b9c8f..f1f9b307a 100644 --- a/src/steps/install-cli-steering/__tests__/install-cli-steering.test.ts +++ b/src/steps/install-cli-steering/__tests__/install-cli-steering.test.ts @@ -82,6 +82,43 @@ describe('install-cli-steering', () => { expect(result.error).toContain('install failed'); }); + it('gives every npm failure a stable exception message so they group', () => { + spawnSyncMock.mockReturnValue({ + status: 1, + stdout: '', + stderr: `npm error path ${os.homedir()}\\AppData\\Roaming\\npm\n`, + }); + + const result = installOrUpdatePostHogCli(); + expect(result.errorObject?.message).toBe( + 'npm install --global @posthog/cli@latest failed', + ); + }); + + it('drops personal data from the reported detail', () => { + const home = os.homedir(); + spawnSyncMock.mockReturnValue({ + status: 1, + stdout: '', + stderr: [ + 'npm error code E404', + 'npm error 404 Not Found - GET https://registry.npmjs.org/@posthog/cli', + `npm error path ${home}\\AppData\\Roaming\\npm`, + `npm error command ${home}\\node.exe install`, + `npm error A complete log is in ${home}\\npm-cache\\log`, + ].join('\n'), + }); + + const result = installOrUpdatePostHogCli(); + expect(result.detail).not.toContain(home); + expect(result.detail).not.toContain('npm error path'); + expect(result.detail).not.toContain('npm error command'); + // Keeps the diagnosable bits: HTTP status and the failing registry URL. + expect(result.detail).toContain('E404'); + expect(result.detail).toContain('registry.npmjs.org'); + expect(result.error).not.toContain(home); + }); + it('explains when npm itself cannot be run', () => { spawnSyncMock.mockReturnValue({ error: new Error('spawn npm ENOENT'), diff --git a/src/steps/install-cli-steering/index.ts b/src/steps/install-cli-steering/index.ts index 45e133219..3c393b903 100644 --- a/src/steps/install-cli-steering/index.ts +++ b/src/steps/install-cli-steering/index.ts @@ -73,6 +73,12 @@ export interface CliInstallResult { * false. */ errorObject?: Error; + /** + * Sanitized npm output for callers to attach as an exception property. Keeps + * the HTTP status and CLI version out of the grouping message while the + * message itself stays stable. Truncated and free of personal data. + */ + detail?: string; } const spawnOptions = { @@ -82,6 +88,31 @@ const spawnOptions = { shell: process.platform === 'win32', }; +/** Stable message so npm install failures group into one error-tracking issue. */ +const NPM_INSTALL_FAILED_MESSAGE = + 'npm install --global @posthog/cli@latest failed'; + +/** Cap on the sanitized npm output stored as an exception property. */ +const NPM_FAILURE_DETAIL_LIMIT = 1000; + +/** + * Strip personal data from npm's output before it reaches error tracking. npm + * prints `npm error path ` and `npm error command ` lines that carry + * the user's name and home directory, so drop those lines and redact any home + * directory that remains. + */ +function sanitizeNpmFailure(raw: string): string { + const home = os.homedir(); + const cleaned = raw + .split('\n') + .filter((line) => { + const normalized = line.trim().toLowerCase(); + return !/^npm (error|err!) (path|command)\b/.test(normalized); + }) + .join('\n'); + return (home ? cleaned.split(home).join('~') : cleaned).trim(); +} + /** * Install or update the PostHog CLI in the user's environment. `npm install * --global @posthog/cli@latest` covers both first-time installs and upgrades @@ -101,16 +132,12 @@ export function installOrUpdatePostHogCli(): CliInstallResult { }; } if (result.status !== 0) { - const detail = (result.stderr || result.stdout || '').trim(); - const message = - detail || - `npm install --global @posthog/cli@latest exited with status ${ - result.status ?? 'unknown' - }`; + const detail = sanitizeNpmFailure(result.stderr || result.stdout || ''); return { success: false, - error: message, - errorObject: new Error(message), + error: detail || NPM_INSTALL_FAILED_MESSAGE, + errorObject: new Error(NPM_INSTALL_FAILED_MESSAGE), + detail: detail.slice(0, NPM_FAILURE_DETAIL_LIMIT) || undefined, }; } return { success: true }; From 1409fde792c6d4698cd9064d1be4fc5ea2ace679 Mon Sep 17 00:00:00 2001 From: "posthog[bot]" <206114724+posthog[bot]@users.noreply.github.com> Date: Sat, 29 Aug 2026 19:37:46 +0000 Subject: [PATCH 2/2] fix(cli): keep exit status when npm fails silently MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A quieted npm (e.g. `--loglevel=silent` from a user's .npmrc) can exit non-zero with no output. In that case `sanitizeNpmFailure` returned an empty string, so `error` fell back to the constant grouping message and `detail` was dropped entirely — losing the exit code, which is the only diagnostic available for a silent failure. Restore a fallback that reports `result.status` (or the terminating `result.signal`) and feed it to both `error` and `detail`. The stable `errorObject` message is untouched, so error-tracking grouping is preserved; neither the exit code nor the signal name is personal data. Adds tests for the silent-exit and signal-termination cases. Generated-By: PostHog Desktop Task-Id: 96741072-cf69-4a7e-ab4f-68b3d6797503 --- .../__tests__/install-cli-steering.test.ts | 37 +++++++++++++++++++ src/steps/install-cli-steering/index.ts | 14 ++++++- 2 files changed, 49 insertions(+), 2 deletions(-) diff --git a/src/steps/install-cli-steering/__tests__/install-cli-steering.test.ts b/src/steps/install-cli-steering/__tests__/install-cli-steering.test.ts index f1f9b307a..ee72eb6a3 100644 --- a/src/steps/install-cli-steering/__tests__/install-cli-steering.test.ts +++ b/src/steps/install-cli-steering/__tests__/install-cli-steering.test.ts @@ -119,6 +119,43 @@ describe('install-cli-steering', () => { expect(result.error).not.toContain(home); }); + it('keeps the exit status when npm fails with no output', () => { + spawnSyncMock.mockReturnValue({ + status: 1, + signal: null, + stdout: '', + stderr: '', + }); + + const result = installOrUpdatePostHogCli(); + expect(result.success).toBe(false); + // Grouping message stays stable... + expect(result.errorObject?.message).toBe( + 'npm install --global @posthog/cli@latest failed', + ); + // ...but the exit status survives in the error and detail so a silent + // failure is still distinguishable. + expect(result.error).toContain('exited with status 1'); + expect(result.detail).toContain('exited with status 1'); + }); + + it('keeps the terminating signal when npm is killed with no output', () => { + spawnSyncMock.mockReturnValue({ + status: null, + signal: 'SIGKILL', + stdout: '', + stderr: '', + }); + + const result = installOrUpdatePostHogCli(); + expect(result.success).toBe(false); + expect(result.errorObject?.message).toBe( + 'npm install --global @posthog/cli@latest failed', + ); + expect(result.error).toContain('terminated by signal SIGKILL'); + expect(result.detail).toContain('terminated by signal SIGKILL'); + }); + it('explains when npm itself cannot be run', () => { spawnSyncMock.mockReturnValue({ error: new Error('spawn npm ENOENT'), diff --git a/src/steps/install-cli-steering/index.ts b/src/steps/install-cli-steering/index.ts index 3c393b903..12d7451e6 100644 --- a/src/steps/install-cli-steering/index.ts +++ b/src/steps/install-cli-steering/index.ts @@ -133,11 +133,21 @@ export function installOrUpdatePostHogCli(): CliInstallResult { } if (result.status !== 0) { const detail = sanitizeNpmFailure(result.stderr || result.stdout || ''); + // A quieted npm (e.g. `--loglevel=silent` from .npmrc) can exit non-zero + // with no output. The exit status or terminating signal is then the only + // diagnostic left, so fall back to it instead of a bare constant. Neither + // is personal data, and the stable `errorObject` message still groups these + // failures into one issue regardless of the fallback text. + const exitDetail = result.signal + ? `npm install --global @posthog/cli@latest terminated by signal ${result.signal}` + : `npm install --global @posthog/cli@latest exited with status ${ + result.status ?? 'unknown' + }`; return { success: false, - error: detail || NPM_INSTALL_FAILED_MESSAGE, + error: detail || exitDetail, errorObject: new Error(NPM_INSTALL_FAILED_MESSAGE), - detail: detail.slice(0, NPM_FAILURE_DETAIL_LIMIT) || undefined, + detail: detail.slice(0, NPM_FAILURE_DETAIL_LIMIT) || exitDetail, }; } return { success: true };