fix(cli): strip terminal escapes from OAuth error text (GHSA-q9j4-4h4j-mv5m) - #382
Merged
Merged
Conversation
…j-mv5m) `argos login` printed the `error_description` of a failed OAuth callback verbatim. The authorization server is whatever `ARGOS_APP_BASE_URL` points at, so a hostile one could embed ANSI escape sequences and carriage returns that erase and repaint terminal lines, forging CLI output — a fake "run this command to continue" instruction under the CLI's own name. Sanitize remote text before it becomes an error message: drop escape sequences (CSI, OSC and the plain ones) along with the bytes that belong to them, turn the remaining control characters into spaces, drop the directional formatting that reorders what is displayed, collapse the result onto one line and bound its length. Applied to the loopback callback's `error`/`error_description` and to the token endpoint's error payload, which reaches the same terminal through `OAuthTokenError`; when nothing printable is left, the CLI falls back to a message of its own. Add unit tests for the sanitizer and a regression test driving the advisory's payload through the real callback server. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
🟢 Approval recommended
Untrusted OAuth messages are consistently sanitized, bounded, and covered by focused tests.
Pull request overview
Adds terminal-safe handling for untrusted OAuth errors, addressing GHSA-q9j4-4h4j-mv5m.
Changes:
- Adds terminal-text sanitization with length limits.
- Sanitizes callback and token-endpoint errors.
- Adds sanitizer and callback integration tests.
File summaries
| File | Description |
|---|---|
packages/cli/src/lib/terminal.ts |
Implements terminal-text sanitization. |
packages/cli/src/lib/terminal.test.ts |
Tests sanitization behavior and advisory payload. |
packages/cli/src/lib/oauth.ts |
Sanitizes token-endpoint errors. |
packages/cli/src/commands/login.ts |
Sanitizes OAuth callback errors. |
packages/cli/src/commands/login.test.ts |
Tests callback success and safe failures. |
Review details
- Files reviewed: 5/5 changed files
- Comments generated: 0
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes GHSA-q9j4-4h4j-mv5m.
argos loginprinted theerror_descriptionof a failed OAuth callback verbatim. The authorization server is whateverARGOS_APP_BASE_URLpoints at, so a hostile one could embed ANSI escape sequences and carriage returns that erase and repaint terminal lines — enough to forge a trusted-looking "run this command to continue" instruction under the CLI's own name.Fix
sanitizeTerminalText()(packages/cli/src/lib/terminal.ts) reduces remote text to something a terminal can only display:[2Kremnant is left as text;\rwould splicefoo\rbarinto one word);Applied to the loopback callback's
error/error_descriptionand to the token endpoint's error payload, which reaches the same terminal throughOAuthTokenError. When nothing printable is left, the CLI falls back to a message of its own instead of printing a blank error.Tests
src/lib/terminal.test.tscovers the sanitizer, including the advisory's payload.src/commands/login.test.tsdrives that payload through the real callback server and asserts the rejection carries no escape character, plus the success path and the empty-description fallback.🤖 Generated with Claude Code