From 15e13e4bcb6782030912bcbdd6a7a2553c29a35d Mon Sep 17 00:00:00 2001 From: David Cramer Date: Sun, 22 Feb 2026 22:25:21 -0800 Subject: [PATCH 1/2] fix(cli): Replace process.exit(130) with thrown error for graceful cleanup readSingleKey() called process.exit(130) on Ctrl+C, bypassing Sentry span completion and telemetry flush. Throw UserAbortError instead, allowing callers to clean up before exiting. Warden finding find-warden-bugs-bf627437 Severity: high Co-Authored-By: Warden --- src/cli/index.ts | 5 ++ src/cli/input.test.ts | 111 ++++++++++++++++++++++++++++++++++++++++++ src/cli/input.ts | 16 +++++- src/cli/main.ts | 5 +- 4 files changed, 134 insertions(+), 3 deletions(-) create mode 100644 src/cli/input.test.ts diff --git a/src/cli/index.ts b/src/cli/index.ts index 88d5bd1b..9b94d04d 100644 --- a/src/cli/index.ts +++ b/src/cli/index.ts @@ -3,6 +3,7 @@ import { initSentry, Sentry, flushSentry } from '../sentry.js'; initSentry('cli'); import { main, abortController, interrupted } from './main.js'; +import { UserAbortError } from './input.js'; let interruptCount = 0; @@ -22,6 +23,10 @@ process.on('SIGINT', () => { }); main().catch(async (error) => { + if (error instanceof UserAbortError) { + await flushSentry(); + process.exit(130); + } Sentry.captureException(error); await flushSentry(); console.error('Fatal error:', error); diff --git a/src/cli/input.test.ts b/src/cli/input.test.ts new file mode 100644 index 00000000..b866f5f3 --- /dev/null +++ b/src/cli/input.test.ts @@ -0,0 +1,111 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { EventEmitter } from 'node:events'; +import { readSingleKey, UserAbortError } from './input.js'; + +/** + * Create a fake stdin that supports setRawMode, resume, pause, and once('data'). + * Replaces process.stdin for the duration of the test. + */ +function createFakeStdin() { + const emitter = new EventEmitter(); + const fake = Object.assign(emitter, { + isRaw: false, + setRawMode: vi.fn((mode: boolean) => { + fake.isRaw = mode; + return fake; + }), + resume: vi.fn(), + pause: vi.fn(), + isTTY: true as const, + }); + return fake; +} + +describe('readSingleKey', () => { + let originalStdin: typeof process.stdin; + let fakeStdin: ReturnType; + let stderrSpy: ReturnType; + + beforeEach(() => { + originalStdin = process.stdin; + fakeStdin = createFakeStdin(); + Object.defineProperty(process, 'stdin', { value: fakeStdin, writable: true }); + stderrSpy = vi.spyOn(process.stderr, 'write').mockReturnValue(true); + }); + + afterEach(() => { + Object.defineProperty(process, 'stdin', { value: originalStdin, writable: true }); + stderrSpy.mockRestore(); + }); + + it('resolves with the lowercase key for normal input', async () => { + const promise = readSingleKey(); + + // Simulate keypress + fakeStdin.emit('data', Buffer.from('A')); + + const result = await promise; + expect(result).toBe('a'); + }); + + it('enables raw mode and restores it after reading', async () => { + const promise = readSingleKey(); + fakeStdin.emit('data', Buffer.from('x')); + await promise; + + expect(fakeStdin.setRawMode).toHaveBeenCalledWith(true); + expect(fakeStdin.setRawMode).toHaveBeenCalledWith(false); + expect(fakeStdin.resume).toHaveBeenCalled(); + expect(fakeStdin.pause).toHaveBeenCalled(); + }); + + it('throws UserAbortError on Ctrl+C instead of calling process.exit', async () => { + const exitSpy = vi.spyOn(process, 'exit').mockImplementation(() => undefined as never); + + const promise = readSingleKey(); + + // Simulate Ctrl+C (0x03) + fakeStdin.emit('data', Buffer.from('\x03')); + + await expect(promise).rejects.toThrow(UserAbortError); + await expect(promise).rejects.toThrow('User aborted'); + + // Verify process.exit was NOT called — the whole point of this fix + expect(exitSpy).not.toHaveBeenCalled(); + + exitSpy.mockRestore(); + }); + + it('restores raw mode before throwing UserAbortError', async () => { + const promise = readSingleKey(); + fakeStdin.emit('data', Buffer.from('\x03')); + + await expect(promise).rejects.toThrow(UserAbortError); + + // Raw mode should have been restored before the rejection + expect(fakeStdin.setRawMode).toHaveBeenCalledWith(false); + expect(fakeStdin.pause).toHaveBeenCalled(); + }); + + it('writes newline to stderr on Ctrl+C', async () => { + const promise = readSingleKey(); + fakeStdin.emit('data', Buffer.from('\x03')); + + await expect(promise).rejects.toThrow(UserAbortError); + expect(stderrSpy).toHaveBeenCalledWith('\n'); + }); +}); + +describe('UserAbortError', () => { + it('is an instance of Error', () => { + const error = new UserAbortError(); + expect(error).toBeInstanceOf(Error); + expect(error).toBeInstanceOf(UserAbortError); + }); + + it('has correct name and message', () => { + const error = new UserAbortError(); + expect(error.name).toBe('UserAbortError'); + expect(error.message).toBe('User aborted'); + }); +}); diff --git a/src/cli/input.ts b/src/cli/input.ts index 203c0075..e53b2c6d 100644 --- a/src/cli/input.ts +++ b/src/cli/input.ts @@ -1,8 +1,19 @@ +/** + * Custom error thrown when the user aborts via Ctrl+C during interactive input. + * Allows callers to handle cleanup (e.g. Sentry flush) before exiting. + */ +export class UserAbortError extends Error { + constructor() { + super('User aborted'); + this.name = 'UserAbortError'; + } +} + /** * Read a single keypress from stdin in raw mode. */ export async function readSingleKey(): Promise { - return new Promise((resolve) => { + return new Promise((resolve, reject) => { const stdin = process.stdin; const wasRaw = stdin.isRaw; @@ -18,7 +29,8 @@ export async function readSingleKey(): Promise { // Handle Ctrl+C if (key === '\x03') { process.stderr.write('\n'); - process.exit(130); + reject(new UserAbortError()); + return; } resolve(key.toLowerCase()); diff --git a/src/cli/main.ts b/src/cli/main.ts index ad034e0e..2f10ef25 100644 --- a/src/cli/main.ts +++ b/src/cli/main.ts @@ -35,6 +35,7 @@ import { runInteractiveFixFlow, renderFixSummary, } from './fix.js'; +import { UserAbortError } from './input.js'; import { runInit } from './commands/init.js'; import { runAdd } from './commands/add.js'; import { runSetupApp } from './commands/setup-app.js'; @@ -909,7 +910,9 @@ export async function main(): Promise { isTTY: reporter.mode.isTTY, reporter, }); - } catch { + } catch (err) { + // Re-throw user abort so it propagates to the top-level handler for cleanup + if (err instanceof UserAbortError) throw err; // Config load or cleanup failed — skip silently } From bcc205726934317e6ebbd8780e64618b1d6538ad Mon Sep 17 00:00:00 2001 From: David Cramer Date: Mon, 23 Feb 2026 09:43:02 -0800 Subject: [PATCH 2/2] fix(cli): Guard flushSentry in UserAbortError handler against throws Wrap flushSentry in try/catch so that if Sentry flush fails, the process still exits cleanly with code 130 instead of hanging or crashing with an unhandled rejection. Co-Authored-By: Claude Opus 4.6 --- src/cli/index.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/cli/index.ts b/src/cli/index.ts index 9b94d04d..51381339 100644 --- a/src/cli/index.ts +++ b/src/cli/index.ts @@ -24,7 +24,11 @@ process.on('SIGINT', () => { main().catch(async (error) => { if (error instanceof UserAbortError) { - await flushSentry(); + try { + await flushSentry(); + } catch { + // Best-effort flush - don't let Sentry errors prevent clean exit + } process.exit(130); } Sentry.captureException(error);