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 <3393101+tikazyq@users.noreply.github.com>
Co-authored-by: tikazyq <3393101+tikazyq@users.noreply.github.com>
Co-authored-by: tikazyq <3393101+tikazyq@users.noreply.github.com>
Co-authored-by: tikazyq <3393101+tikazyq@users.noreply.github.com>
There was a problem hiding this comment.
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})`)); |
There was a problem hiding this comment.
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})`)); |
There was a problem hiding this comment.
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
strip-ansi@7.1.2(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.