🛡️ Sentinel: [HIGH] Fix terminal injection via ST sequence bypass#160
🛡️ Sentinel: [HIGH] Fix terminal injection via ST sequence bypass#160hongymagic wants to merge 1 commit into
Conversation
The ANSI stripping logic previously only recognized `\u0007` (Bell) as a terminator for OSC (Operating System Command) sequences. Malicious output could bypass sanitization by using the standard String Terminator (ST) `\u001b\`. This commit updates the ANSI regex to match both terminators, and fixes the streaming state logic to correctly handle partial chunks when terminators are split or back-to-back. Tests have been added to verify ST sequence handling. Co-authored-by: hongymagic <302730+hongymagic@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
There was a problem hiding this comment.
Pull request overview
This PR hardens the CLI’s ANSI/OSC sanitization by recognising the ST (ESC \) terminator for OSC sequences (e.g. hyperlinks) so they can’t bypass stripping, and documents the incident in the Sentinel log.
Changes:
- Extend
ANSI_REGEXto treatBELorSTas valid OSC terminators. - Adjust
createAnsiStripperto avoid treating a trailingESC \as the start of an incomplete sequence. - Add a vitest case covering an OSC hyperlink terminated with
ST.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/ansi.ts |
Updates OSC-stripping regex and tweaks streaming buffer logic around ESC \ terminators. |
tests/ansi.test.ts |
Adds a test case for stripping OSC hyperlinks terminated via ST. |
.jules/sentinel.md |
Records the vulnerability, learning, and prevention guidance for ST-terminated OSC sequences. |
| // Depending on whether it splits or is in one chunk, the result should be just the link text | ||
| expect(stripper(stSequence)).toBe("Link"); |
| // If the buffer ends with a partial ANSI sequence (starts with ESC), keep it in buffer | ||
| // ESC is \u001B or \u009B | ||
| const lastEscIndex = Math.max( | ||
| let lastEscIndex = Math.max( | ||
| buffer.lastIndexOf("\u001B"), | ||
| buffer.lastIndexOf("\u009B"), | ||
| ); | ||
|
|
||
| // If the last ESC is part of an ST terminator (\u001B\\), it's not the start | ||
| // of an incomplete sequence, it's the end of an OSC sequence. We should look | ||
| // for the ESC that precedes it to find the actual start of a potentially | ||
| // incomplete sequence. | ||
| if ( | ||
| lastEscIndex !== -1 && | ||
| buffer.slice(lastEscIndex, lastEscIndex + 2) === "\u001B\\" | ||
| ) { | ||
| let checkIndex = lastEscIndex; | ||
| while ( | ||
| checkIndex !== -1 && | ||
| buffer.slice(checkIndex, checkIndex + 2) === "\u001B\\" | ||
| ) { | ||
| checkIndex = Math.max( | ||
| buffer.lastIndexOf("\u001B", checkIndex - 1), | ||
| buffer.lastIndexOf("\u009B", checkIndex - 1), | ||
| ); | ||
| } | ||
| lastEscIndex = checkIndex; | ||
| } |
🚨 Severity: HIGH
💡 Vulnerability: Terminal Injection via ST (String Terminator) Sequences bypassing
stripAnsifilter🎯 Impact: An attacker could craft prompt injections that result in the AI emitting an OSC sequence (such as an invisible, clickable hyperlink to a malicious site) terminated with
\u001b\. Because the filter didn't recognize ST, the malicious link would be rendered in the victim's terminal unstripped.🔧 Fix:
ANSI_REGEXinsrc/ansi.tsto recognize(?:\u0007|\u001B\\)as valid OSC terminators.createAnsiStripperwhere it failed to find the start of an incomplete sequence if the buffer ended with\u001b\. The logic now iterates backwards past contiguous ST terminators.✅ Verification: Added dedicated ST test cases in
tests/ansi.test.ts. All existing suite tests and formatting checks pass securely.PR created automatically by Jules for task 17731770410998614750 started by @hongymagic