[codex] Sanitize text generation CLI errors - #3431
Conversation
Co-authored-by: codex <codex@users.noreply.github.com>
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
ApprovabilityVerdict: Approved This is a small security improvement that prevents sensitive data from CLI errors (like access tokens) from being exposed in user-facing error messages. The change is minimal, well-tested, and reduces risk. You can customize Macroscope's approvability policy. Learn more. |
Summary
causeRoot cause
Generic CLI errors were copied into
TextGenerationError.detail, so provider output and credentials embedded in a process error could surface in public error messages even though the original error was already retained as the cause.Validation
vp test apps/server/src/textGeneration/TextGenerationPrompts.test.ts(14 tests)vp checkvp run typecheckNote
Medium Risk
Touches error surfacing for text generation; reduces information leakage but may hide CLI failure text from default user-visible messages (still available via cause).
Overview
Stops leaking raw CLI stderr/process errors into user-facing text generation failures.
normalizeCliErrorno longer appends the underlyingError.messagetoTextGenerationError.detailfor generic CLI failures. The publicdetail(and derivedmessage) now uses only the stable operation fallback, while the original error remains oncause. Missing-CLI-on-PATH handling is unchanged.A regression test asserts that a Codex-style error containing
access_token=secret-tokendoes not appear in the public message.Reviewed by Cursor Bugbot for commit 8be620d. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Sanitize
normalizeCliErroroutput to exclude sensitive CLI error detailsIn TextGenerationUtils.ts,
normalizeCliErrorpreviously appended the original error message to the fallback string when constructing aTextGenerationErrorfor non-ENOENT errors. This exposed internal CLI failure details in the publicdetailfield. The fix setsdetailto the fallback string only, dropping the original error message entirely.Macroscope summarized 8be620d.