-
Notifications
You must be signed in to change notification settings - Fork 11
Security: Prevent ANSI injection attacks in user-facing output #12
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Security: Prevent ANSI injection attacks in user-facing output #12
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Co-authored-by: tikazyq <[email protected]>
Co-authored-by: tikazyq <[email protected]>
Co-authored-by: tikazyq <[email protected]>
Co-authored-by: tikazyq <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR implements security hardening to prevent ANSI injection attacks by sanitizing user-controlled input before displaying it in the terminal. The changes introduce a centralized sanitization utility and apply it consistently across all commands that display user-provided content.
- Adds
sanitizeUserInput()function to strip ANSI escape sequences and control characters from user input - Wraps all user-controlled output (spec names, paths, tags, assignees, queries) with sanitization
- Provides comprehensive test coverage for various injection attack vectors
Reviewed Changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/utils/safe-output.ts | New security module with sanitization utilities and safe output functions |
| src/safe-output.test.ts | Comprehensive tests for sanitization and safe output functions (327 lines, 42 test cases) |
| src/utils/ui.ts | Re-exports sanitizeUserInput and adds documentation notes about pre-sanitization |
| src/commands/update.ts | Sanitizes spec paths in error and success messages |
| src/commands/search.ts | Sanitizes query strings, filter options, and search result metadata |
| src/commands/files.ts | Sanitizes spec names and file names in directory listings |
| src/commands/deps.ts | Sanitizes dependency paths in dependency tree output |
| src/commands/create.ts | Sanitizes spec directory and file paths in creation messages |
| src/commands/check.ts | Sanitizes conflicting spec paths in validation output |
| src/commands/board.ts | Sanitizes spec paths, assignees, and tags in board view |
| src/commands/archive.ts | Sanitizes spec paths in archive operations |
| package.json | Adds strip-ansi@^7.1.2 dependency |
| pnpm-lock.yaml | Lock file update for new dependency |
| specs/.../README.md | Updates spec status to complete |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| for (const file of documents) { | ||
| const size = formatSize(file.size); | ||
| console.log(chalk.cyan(` ✓ ${file.name.padEnd(20)} (${size})`)); | ||
| console.log(chalk.cyan(` ✓ ${sanitizeUserInput(file.name).padEnd(20)} (${size})`)); |
Copilot
AI
Nov 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Calling .padEnd(20) on the sanitized filename can cause misalignment when the filename contains multi-byte unicode characters or emojis. The sanitization preserves unicode/emojis but .padEnd() counts by string length, not visual width. Consider sanitizing after padding or using a display-width-aware padding function.
| for (const file of assets) { | ||
| const size = formatSize(file.size); | ||
| console.log(chalk.yellow(` ✓ ${file.name.padEnd(20)} (${size})`)); | ||
| console.log(chalk.yellow(` ✓ ${sanitizeUserInput(file.name).padEnd(20)} (${size})`)); |
Copilot
AI
Nov 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Calling .padEnd(20) on the sanitized filename can cause misalignment when the filename contains multi-byte unicode characters or emojis. The sanitization preserves unicode/emojis but .padEnd() counts by string length, not visual width. Consider sanitizing after padding or using a display-width-aware padding function.
…dd-4cfb-921f-3403c4059d59
…90-efb63b60-92dd-4cfb-921f-3403c4059d59 Security: Prevent ANSI injection attacks in user-facing output
User-controlled input (spec names, paths, tags, assignees) flows through chalk formatting without sanitization, enabling ANSI injection attacks that can manipulate terminal behavior, hide output, or inject malicious control sequences.
Changes
Security Module
src/utils/safe-output.ts: Centralized sanitization usingstrip-ansito remove ANSI codes and control characters while preserving unicode/emojissrc/safe-output.test.ts: 42 tests covering injection vectors (CSI sequences, OSC commands, cursor control, bell spam, hyperlink injection)Command Migration
Sanitize user input before display in 8 commands:
create.ts,archive.ts,update.ts: spec pathsboard.ts: spec paths, tags, assigneessearch.ts: queries, results, metadatafiles.ts: spec/file namesdeps.ts: dependency pathscheck.ts: conflict pathsExample
Attack example prevented:
Dependencies
[email protected](no known vulnerabilities)Original prompt
Created from VS Code via the GitHub Pull Request extension.
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.