From 24ecc2a513cde6e3ae053cc878ba31d55ef7a937 Mon Sep 17 00:00:00 2001 From: 0xfandom Date: Mon, 13 Jul 2026 13:21:42 +0530 Subject: [PATCH 01/12] fix(bash): convert BRE interval braces when previewing sed edits MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit convertBrePatternToJs rewrites a POSIX BRE sed pattern into a JS regex so the permission dialog can preview what a 'sed -i s/.../.../' edit will do. It unescapes the BRE metacharacters that flip escaping between BRE and JS, but the membership sets listed only +?|() and omitted { }. In BRE '\{n,m\}' is the interval quantifier and a bare '{' is literal — the reverse of JS — so both were handled backwards: '\{2\}' was emitted as a JS literal (matched nothing) and a bare '{2}' became a JS quantifier. The result is a misleading diff at an approval gate: 'sed -i s/a\{2\}/X/' previews as no change while the command the user approves rewrites the file. Add { } to both sets. Add coverage for escaped intervals, escaped ranges, and bare-brace literals. --- src/tools/BashTool/sedEditParser.test.ts | 22 ++++++++++++++++++++++ src/tools/BashTool/sedEditParser.ts | 7 +++++-- 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/src/tools/BashTool/sedEditParser.test.ts b/src/tools/BashTool/sedEditParser.test.ts index f9c8732648..719d385b6e 100644 --- a/src/tools/BashTool/sedEditParser.test.ts +++ b/src/tools/BashTool/sedEditParser.test.ts @@ -38,3 +38,25 @@ test('BRE mode preserves escaped backslashes', () => { expect(result).toBe('backslash-match foo/bar') }) + +test('BRE mode treats escaped braces as an interval quantifier', () => { + // `a\{2\}` is the BRE interval quantifier (exactly two a's). Before the fix + // braces were omitted from the metacharacter set, so it was emitted as the + // JS literal `a\{2\}` and matched nothing — the preview showed no change + // while real sed rewrote the file. + const result = applySedSubstitution('aa and a', sedInfo('a\\{2\\}', 'X')) + expect(result).toBe('X and a') +}) + +test('BRE mode treats bare braces as literals', () => { + // A bare `{2}` is literal in BRE, so it must not become a JS quantifier. + const result = applySedSubstitution('a{2} and aa', sedInfo('a{2}', 'X')) + expect(result).toBe('X and aa') +}) + +test('BRE mode supports escaped interval ranges', () => { + // `b\{1,3\}` matches one-to-three b's; on "bbbc" it consumes the three b's + // (the upper bound) and leaves the trailing "c". + const result = applySedSubstitution('bbbc', sedInfo('b\\{1,3\\}', 'X')) + expect(result).toBe('Xc') +}) diff --git a/src/tools/BashTool/sedEditParser.ts b/src/tools/BashTool/sedEditParser.ts index 02be50a742..552eee5656 100644 --- a/src/tools/BashTool/sedEditParser.ts +++ b/src/tools/BashTool/sedEditParser.ts @@ -35,7 +35,10 @@ function convertBrePatternToJs(pattern: string): string { } if (next === '\\') { result += '\\\\' - } else if ('+?|()'.includes(next)) { + } else if ('+?|(){}'.includes(next)) { + // BRE metacharacters that are special only when escaped: unescape them + // for JS. Braces belong here too — in BRE `\{n,m\}` is the interval + // quantifier and a bare `{` is literal, the reverse of JS. result += next } else { result += `\\${next}` @@ -44,7 +47,7 @@ function convertBrePatternToJs(pattern: string): string { continue } - if ('+?|()'.includes(char)) { + if ('+?|(){}'.includes(char)) { result += `\\${char}` continue } From 848bd885c4247b9422adbc8b37fb658a5b4ae588 Mon Sep 17 00:00:00 2001 From: 0xfandom Date: Tue, 14 Jul 2026 15:06:35 +0530 Subject: [PATCH 02/12] fix(bash): only unescape sed interval braces when they form a valid count \{,m\}, \{\} and \{n,m,k\} are literal brace runs in sed, not interval quantifiers. Emitting them as JS braces produced a bogus quantifier (or a literal that matched nothing), so the sed-edit preview showed no change while real sed rewrote the file. Detect the enclosed count and only convert legal BRE intervals (n, n,, n,m, and the GNU ,m extension, normalized to {0,m}); otherwise keep the braces escaped as literals. Extract the shared metachar set and cover ? | ( ) and the ERE brace path. --- src/tools/BashTool/sedEditParser.test.ts | 65 ++++++++++++++++++++++++ src/tools/BashTool/sedEditParser.ts | 59 +++++++++++++++++++-- 2 files changed, 119 insertions(+), 5 deletions(-) diff --git a/src/tools/BashTool/sedEditParser.test.ts b/src/tools/BashTool/sedEditParser.test.ts index 719d385b6e..da11be1c49 100644 --- a/src/tools/BashTool/sedEditParser.test.ts +++ b/src/tools/BashTool/sedEditParser.test.ts @@ -60,3 +60,68 @@ test('BRE mode supports escaped interval ranges', () => { const result = applySedSubstitution('bbbc', sedInfo('b\\{1,3\\}', 'X')) expect(result).toBe('Xc') }) + +test('BRE mode supports the open lower-bound interval \\{,m\\}', () => { + // GNU sed accepts `\{,3\}` (up to three). JS has no `{,3}` quantifier, so it + // must be normalized to `{0,3}`; otherwise the preview showed no change while + // sed rewrote the run. A single (non-global) substitution isolates the + // upper-bound behaviour from `{0,m}`'s zero-width matches. + const result = applySedSubstitution('aaaab', { + filePath: 'example.txt', + pattern: 'a\\{,3\\}', + replacement: 'X', + flags: '', + extendedRegex: false, + }) + expect(result).toBe('Xab') +}) + +test('BRE mode supports the open upper-bound interval \\{n,\\}', () => { + const result = applySedSubstitution('aaab', sedInfo('a\\{2,\\}', 'X')) + expect(result).toBe('Xb') +}) + +test('BRE mode treats an empty interval \\{\\} as literal braces', () => { + // `\{\}` is not a legal count, so sed matches the literal `{}` characters. + const result = applySedSubstitution('a{} and aa', sedInfo('a\\{\\}', 'X')) + expect(result).toBe('X and aa') +}) + +test('BRE mode treats a malformed interval \\{n,m,k\\} as literal braces', () => { + const result = applySedSubstitution( + 'a{1,2,3} and aa', + sedInfo('a\\{1,2,3\\}', 'X'), + ) + expect(result).toBe('X and aa') +}) + +test('BRE mode treats escaped alternation as an operator', () => { + const result = applySedSubstitution( + 'cat and dog', + sedInfo('cat\\|dog', 'X'), + ) + expect(result).toBe('X and X') +}) + +test('BRE mode treats escaped groups and question marks as operators', () => { + const result = applySedSubstitution('abab', sedInfo('\\(ab\\)\\+', 'X')) + expect(result).toBe('X') + + const optional = applySedSubstitution('ac abc', sedInfo('ab\\?c', 'X')) + expect(optional).toBe('X X') +}) + +test('BRE mode applies g across multiple interval quantifiers', () => { + const result = applySedSubstitution( + 'aabb aabb', + sedInfo('a\\{2\\}b\\{2\\}', 'X'), + ) + expect(result).toBe('X X') +}) + +test('ERE mode uses native brace intervals', () => { + // Under -E the braces are already the JS quantifier form and must pass + // through untouched. + const result = applySedSubstitution('aaab', sedInfo('a{2}', 'X', true)) + expect(result).toBe('Xab') +}) diff --git a/src/tools/BashTool/sedEditParser.ts b/src/tools/BashTool/sedEditParser.ts index 552eee5656..97c1370f0d 100644 --- a/src/tools/BashTool/sedEditParser.ts +++ b/src/tools/BashTool/sedEditParser.ts @@ -21,6 +21,29 @@ export type SedEditInfo = { extendedRegex: boolean } +// BRE metacharacters that are special only when escaped: `\+ \? \| \( \)` are +// the operator forms and the bare characters are literal — the reverse of JS. +// Braces are handled separately because they only form an operator (the +// interval quantifier) when they enclose a valid count. +const BRE_ESCAPE_FLIP_METACHARS = '+?|()' + +/** + * Translate the body of a BRE interval `\{...\}` to its JS quantifier form, + * or null when the body is not a legal interval. GNU sed accepts `n`, `n,`, + * `n,m`, and the extension `,m`; anything else (empty, extra commas, + * non-numeric) is a literal brace run in sed, so we must not turn it into a + * quantifier. `,m` has no JS spelling, so it is normalized to `{0,m}`. + */ +function breIntervalBodyToJs(body: string): string | null { + if (/^[0-9]+(,[0-9]*)?$/.test(body)) { + return `{${body}}` + } + if (/^,[0-9]+$/.test(body)) { + return `{0${body}}` + } + return null +} + function convertBrePatternToJs(pattern: string): string { let result = '' @@ -33,12 +56,32 @@ function convertBrePatternToJs(pattern: string): string { result += '\\\\' continue } + if (next === '{') { + // `\{...\}` is the BRE interval quantifier, but only when the body is a + // legal count. Otherwise sed treats the braces as literals, so keep them + // escaped instead of emitting a bogus JS quantifier. + const close = pattern.indexOf('\\}', i + 2) + if (close !== -1) { + const js = breIntervalBodyToJs(pattern.slice(i + 2, close)) + if (js !== null) { + result += js + i = close + 1 // consume through the closing `\}` + continue + } + } + result += '\\{' + i++ + continue + } + if (next === '}') { + // A stray escaped closing brace with no matching interval open: literal. + result += '\\}' + i++ + continue + } if (next === '\\') { result += '\\\\' - } else if ('+?|(){}'.includes(next)) { - // BRE metacharacters that are special only when escaped: unescape them - // for JS. Braces belong here too — in BRE `\{n,m\}` is the interval - // quantifier and a bare `{` is literal, the reverse of JS. + } else if (BRE_ESCAPE_FLIP_METACHARS.includes(next)) { result += next } else { result += `\\${next}` @@ -47,7 +90,13 @@ function convertBrePatternToJs(pattern: string): string { continue } - if ('+?|(){}'.includes(char)) { + if (BRE_ESCAPE_FLIP_METACHARS.includes(char)) { + result += `\\${char}` + continue + } + + // Bare braces are literal in BRE (the reverse of JS), so escape them. + if (char === '{' || char === '}') { result += `\\${char}` continue } From 0c26e3d65546060951ff3f39e59e817674369dbc Mon Sep 17 00:00:00 2001 From: 0xfandom Date: Wed, 15 Jul 2026 15:21:46 +0530 Subject: [PATCH 03/12] fix(bash): decline to simulate sed edits that cannot be reproduced faithfully The preview is what the user approves, so it must either match what sed writes or not be rendered at all. Three cases could not match: - Zero-minimum quantifiers under g. sed and JS advance differently past an empty match: s/a\{0,3\}/X/g turns aaaab into XXbX in sed but XXXbX in JS, and the same holds for the pre-existing s/a*/X/g (XbX vs XXbX). - Bracket expressions. \{ and \} are ordinary members inside [...], not an interval, and a backslash is a literal member in POSIX brackets but an escape in a JS character class, so [\{,3\}] cannot be mapped across. - Illegal interval bodies. sed rejects \{\} and \{1,2,3\} outright (Invalid content of \{\}) and leaves the file untouched, so they are not literal braces and there is no edit to show. parseSedEditCommand now returns null for these, falling back to ordinary bash rendering. The same gate catches patterns whose translation is not a valid JS regex, which previously threw and was swallowed into a silent no-change diff. Verified differentially against GNU sed 4.10: of 22 expressions, the 15 still simulated match sed byte-for-byte and the 7 divergent ones now decline. --- src/tools/BashTool/sedEditParser.test.ts | 75 +++++++++--- src/tools/BashTool/sedEditParser.ts | 142 ++++++++++++++++++----- 2 files changed, 171 insertions(+), 46 deletions(-) diff --git a/src/tools/BashTool/sedEditParser.test.ts b/src/tools/BashTool/sedEditParser.test.ts index da11be1c49..22ae23be51 100644 --- a/src/tools/BashTool/sedEditParser.test.ts +++ b/src/tools/BashTool/sedEditParser.test.ts @@ -1,6 +1,10 @@ -import { expect, test } from 'bun:test' +import { describe, expect, test } from 'bun:test' -import { applySedSubstitution, type SedEditInfo } from './sedEditParser.js' +import { + applySedSubstitution, + parseSedEditCommand, + type SedEditInfo, +} from './sedEditParser.js' function sedInfo(pattern: string, replacement: string, extendedRegex = false): SedEditInfo { return { @@ -64,8 +68,8 @@ test('BRE mode supports escaped interval ranges', () => { test('BRE mode supports the open lower-bound interval \\{,m\\}', () => { // GNU sed accepts `\{,3\}` (up to three). JS has no `{,3}` quantifier, so it // must be normalized to `{0,3}`; otherwise the preview showed no change while - // sed rewrote the run. A single (non-global) substitution isolates the - // upper-bound behaviour from `{0,m}`'s zero-width matches. + // sed rewrote the run. Without `g` there is a single match, so sed's and JS's + // differing empty-match advance cannot come into play. const result = applySedSubstitution('aaaab', { filePath: 'example.txt', pattern: 'a\\{,3\\}', @@ -81,18 +85,16 @@ test('BRE mode supports the open upper-bound interval \\{n,\\}', () => { expect(result).toBe('Xb') }) -test('BRE mode treats an empty interval \\{\\} as literal braces', () => { - // `\{\}` is not a legal count, so sed matches the literal `{}` characters. - const result = applySedSubstitution('a{} and aa', sedInfo('a\\{\\}', 'X')) - expect(result).toBe('X and aa') -}) - -test('BRE mode treats a malformed interval \\{n,m,k\\} as literal braces', () => { - const result = applySedSubstitution( - 'a{1,2,3} and aa', - sedInfo('a\\{1,2,3\\}', 'X'), - ) - expect(result).toBe('X and aa') +test('BRE mode declines malformed intervals rather than guessing', () => { + // GNU sed rejects both of these outright ("Invalid content of \{\}"), so the + // command aborts and the file is untouched. They are not literal braces, and + // there is no edit to render. + expect( + parseSedEditCommand("sed -i '' 's/a\\{\\}/X/g' example.txt"), + ).toBeNull() + expect( + parseSedEditCommand("sed -i '' 's/a\\{1,2,3\\}/X/g' example.txt"), + ).toBeNull() }) test('BRE mode treats escaped alternation as an operator', () => { @@ -125,3 +127,44 @@ test('ERE mode uses native brace intervals', () => { const result = applySedSubstitution('aaab', sedInfo('a{2}', 'X', true)) expect(result).toBe('Xab') }) + +describe('declines to simulate what it cannot reproduce faithfully', () => { + const cmd = (expr: string) => `sed -i '' '${expr}' example.txt` + + test('rejects zero-minimum intervals under g', () => { + // sed and JS advance differently past an empty match, so a global + // zero-minimum quantifier genuinely diverges: GNU sed turns "aaaab" into + // "XXbX" for both of these, while a JS global replace yields "XXXbX". + // Rendering that as an approved file diff would show the user a change that + // is not what sed writes, so no sed edit is claimed at all. + expect(parseSedEditCommand(cmd('s/a\\{0,3\\}/X/g'))).toBeNull() + expect(parseSedEditCommand(cmd('s/a\\{,3\\}/X/g'))).toBeNull() + // Same class, pre-dating interval support: `*` also matches empty. + expect(parseSedEditCommand(cmd('s/a*/X/g'))).toBeNull() + }) + + test('still simulates zero-minimum intervals without g', () => { + // Only one substitution happens, so the empty-match advance never matters. + expect(parseSedEditCommand(cmd('s/a\\{0,3\\}/X/'))).not.toBeNull() + }) + + test('rejects interval syntax inside a bracket expression', () => { + // `[\{,3\}]` is a bracket expression whose members are ordinary characters; + // GNU sed turns "0,{3}" into "0XXXX". A backslash is a literal member in + // POSIX brackets but an escape in a JS character class, so this cannot be + // mapped across and must not be parsed as an interval. + expect(parseSedEditCommand(cmd('s/[\\{,3\\}]/X/g'))).toBeNull() + }) + + test('still simulates ordinary bracket expressions', () => { + const info = parseSedEditCommand(cmd('s/[abc]/X/g')) + expect(info).not.toBeNull() + expect(applySedSubstitution('abcd', info!)).toBe('XXXd') + }) + + test('rejects a pattern whose translation is not a valid regex', () => { + // Previously `new RegExp` threw and the catch returned the original + // content, rendering an invalid pattern as a silent "no change" diff. + expect(parseSedEditCommand(cmd('s/a*\\{2\\}/X/g'))).toBeNull() + }) +}) diff --git a/src/tools/BashTool/sedEditParser.ts b/src/tools/BashTool/sedEditParser.ts index 97c1370f0d..90213f4dee 100644 --- a/src/tools/BashTool/sedEditParser.ts +++ b/src/tools/BashTool/sedEditParser.ts @@ -28,11 +28,12 @@ export type SedEditInfo = { const BRE_ESCAPE_FLIP_METACHARS = '+?|()' /** - * Translate the body of a BRE interval `\{...\}` to its JS quantifier form, - * or null when the body is not a legal interval. GNU sed accepts `n`, `n,`, - * `n,m`, and the extension `,m`; anything else (empty, extra commas, - * non-numeric) is a literal brace run in sed, so we must not turn it into a - * quantifier. `,m` has no JS spelling, so it is normalized to `{0,m}`. + * Translate the body of a BRE interval `\{...\}` to its JS quantifier form, or + * null when the body is not a legal interval. GNU sed accepts `n`, `n,`, `n,m`, + * and the extension `,m`. `,m` has no JS spelling, so it is normalized to + * `{0,m}`. Anything else — empty, extra commas, non-numeric — is rejected by + * sed itself ("Invalid content of \{\}"), which aborts the command and leaves + * the file untouched; we cannot render that as an edit, so it declines. */ function breIntervalBodyToJs(body: string): string | null { if (/^[0-9]+(,[0-9]*)?$/.test(body)) { @@ -44,12 +45,51 @@ function breIntervalBodyToJs(body: string): string | null { return null } -function convertBrePatternToJs(pattern: string): string { +/** + * Find the index of the `]` closing the BRE bracket expression that starts at + * `open`, or -1 when it is never closed. A `]` in the first position (after an + * optional leading `^`) is an ordinary member, not the terminator. + */ +function findBracketEnd(pattern: string, open: number): number { + let i = open + 1 + if (pattern[i] === '^') i++ + if (pattern[i] === ']') i++ + for (; i < pattern.length; i++) { + if (pattern[i] === ']') return i + } + return -1 +} + +/** + * Convert a BRE pattern to its JS equivalent, or null when it cannot be + * translated faithfully and the caller must decline to simulate the edit. + */ +function convertBrePatternToJs(pattern: string): string | null { let result = '' for (let i = 0; i < pattern.length; i++) { const char = pattern[i]! + if (char === '[') { + // Inside a bracket expression `\{`/`\}` are ordinary members rather than + // an interval, so the interval scan below must not see them. A backslash + // is itself a literal member in a POSIX bracket expression but an escape + // in a JS character class, so those cannot be mapped across — decline + // instead of silently changing which characters match. + const end = findBracketEnd(pattern, i) + if (end === -1) { + // Never closed: a lone `[` is literal in BRE. + result += '\\[' + continue + } + const body = pattern.slice(i, end + 1) + if (body.includes('\\')) return null + // Members are literal in both dialects, so the body carries over as-is. + result += body + i = end + continue + } + if (char === '\\') { const next = pattern[i + 1] if (next === undefined) { @@ -57,20 +97,15 @@ function convertBrePatternToJs(pattern: string): string { continue } if (next === '{') { - // `\{...\}` is the BRE interval quantifier, but only when the body is a - // legal count. Otherwise sed treats the braces as literals, so keep them - // escaped instead of emitting a bogus JS quantifier. + // `\{...\}` is the BRE interval quantifier. An unterminated or + // illegal-bodied interval is an error in sed rather than a literal, so + // decline instead of emitting braces that would match something else. const close = pattern.indexOf('\\}', i + 2) - if (close !== -1) { - const js = breIntervalBodyToJs(pattern.slice(i + 2, close)) - if (js !== null) { - result += js - i = close + 1 // consume through the closing `\}` - continue - } - } - result += '\\{' - i++ + if (close === -1) return null + const js = breIntervalBodyToJs(pattern.slice(i + 2, close)) + if (js === null) return null + result += js + i = close + 1 // consume through the closing `\}` continue } if (next === '}') { @@ -302,6 +337,13 @@ export function parseSedEditCommand(command: string): SedEditInfo | null { return null } + // Only claim this is a renderable sed edit if we can reproduce it faithfully. + // Declining falls back to ordinary bash rendering, which is far better than + // showing the user a diff that does not match what sed will write. + if (!canSimulateFaithfully(pattern, flags, extendedRegex)) { + return null + } + return { filePath, pattern, @@ -311,6 +353,49 @@ export function parseSedEditCommand(command: string): SedEditInfo | null { } } +/** + * Convert the sed pattern to the JS regex source this module would run, or null + * if it cannot be translated. + */ +function toJsPatternSource( + pattern: string, + extendedRegex: boolean, +): string | null { + const unescaped = pattern.replace(/\\\//g, '/') + return extendedRegex ? unescaped : convertBrePatternToJs(unescaped) +} + +/** + * Whether the simulated substitution is guaranteed to match what sed does. + * + * Two cases are declined: + * - the pattern does not translate, or the translation is not a valid JS regex + * (previously this threw and was swallowed into a "no change" preview); + * - the pattern can match the empty string under `g`. sed and JS advance + * differently after an empty match, so the results genuinely differ: + * `s/a*​/X/g` on "aaaab" is "XbX" in sed but "XXbX" in JS, and + * `s/a\{0,3\}/X/g` is "XXbX" in sed but "XXXbX" in JS. + */ +function canSimulateFaithfully( + pattern: string, + flags: string, + extendedRegex: boolean, +): boolean { + const jsPattern = toJsPatternSource(pattern, extendedRegex) + if (jsPattern === null) return false + + let regex: RegExp + try { + regex = new RegExp(jsPattern) + } catch { + return false + } + + if (flags.includes('g') && regex.test('')) return false + + return true +} + /** * Apply a sed substitution to file content * Returns the new content after applying the substitution @@ -337,17 +422,14 @@ export function applySedSubstitution( regexFlags += 'm' } - // Convert sed pattern to JavaScript regex pattern - let jsPattern = sedInfo.pattern - // Unescape \/ to / - .replace(/\\\//g, '/') - - // In BRE mode (no -E flag), metacharacters have opposite escaping: - // BRE: \+ means "one or more", + is literal - // ERE/JS: + means "one or more", \+ is literal - // We need to convert BRE escaping to ERE for JavaScript regex - if (!sedInfo.extendedRegex) { - jsPattern = convertBrePatternToJs(jsPattern) + // Convert sed pattern to JavaScript regex pattern. In BRE mode (no -E flag) + // metacharacters have opposite escaping: BRE `\+` means "one or more" and `+` + // is literal, the reverse of ERE/JS. + const jsPattern = toJsPatternSource(sedInfo.pattern, sedInfo.extendedRegex) + if (jsPattern === null) { + // Not translatable. parseSedEditCommand rejects these up front, so this is + // only reachable for a hand-built SedEditInfo; leave the content untouched. + return content } // Unescape sed-specific escapes in replacement From 1cce4973060809c30cec30e2c5c1662972df7684 Mon Sep 17 00:00:00 2001 From: 0xfandom Date: Thu, 16 Jul 2026 14:31:56 +0530 Subject: [PATCH 04/12] fix(bash): restrict the sed preview to portable, per-line-faithful patterns MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tighten the faithful-simulation gate on every axis where the preview could disagree with what sed writes on the user's platform: - Apply the substitution once per line, as sed does: without g, sed replaces the first match on EVERY line, so a whole-buffer replace previewed s/a\{2\}/X/ on 'aa\naa\n' as 'X\naa\n' where sed writes 'X\nX\n'. This also makes ^ and $ anchor per line, matching sed. - Decline GNU-only operators: \+ \? \| and the \{,m\} interval are extensions that BSD/macOS sed treats as literals (or rejects outright), and this parser explicitly supports macOS through its -i '' handling — one platform's operator is the other's literal, so no single preview can be right for both. Alternation is additionally unfaithful even on GNU: POSIX selects the leftmost-longest branch where JavaScript takes the first that matches. - Decline unterminated bracket expressions (an error in sed, not a literal) and POSIX [:class:]/[=equiv=]/[.collate.] constructs, which JavaScript would silently read as plain character sets. ERE patterns, which previously carried over verbatim, are screened for the same constructs. Verified differentially against GNU sed 4.10 and macOS BSD sed across 27 expressions including multi-line inputs: every still-simulated pattern matches both implementations byte-for-byte, and every declined pattern demonstrably differs between platforms, differs from JavaScript, or errors in sed. --- src/tools/BashTool/sedEditParser.test.ts | 99 +++++++++++------ src/tools/BashTool/sedEditParser.ts | 134 +++++++++++++++++------ 2 files changed, 168 insertions(+), 65 deletions(-) diff --git a/src/tools/BashTool/sedEditParser.test.ts b/src/tools/BashTool/sedEditParser.test.ts index 22ae23be51..3fcb6c05c9 100644 --- a/src/tools/BashTool/sedEditParser.test.ts +++ b/src/tools/BashTool/sedEditParser.test.ts @@ -25,13 +25,16 @@ test('BRE mode keeps unescaped plus literal', () => { expect(result).toBe('literal-plus and aaab') }) -test('BRE mode treats escaped plus as one-or-more', () => { - const result = applySedSubstitution( - 'abbb and a+b', - sedInfo('ab\\+', 'one-or-more'), - ) - - expect(result).toBe('one-or-more and a+b') +test('BRE mode declines the GNU-only escaped plus and question mark', () => { + // `\+` and `\?` are GNU extensions: BSD/macOS sed matches them as literal + // `+`/`?`, so one platform's operator is the other's literal and a single + // preview cannot be right for both. + expect( + parseSedEditCommand("sed -i '' 's/ab\\+/X/' example.txt"), + ).toBeNull() + expect( + parseSedEditCommand("sed -i '' 's/ab\\?c/X/' example.txt"), + ).toBeNull() }) test('BRE mode preserves escaped backslashes', () => { @@ -65,19 +68,14 @@ test('BRE mode supports escaped interval ranges', () => { expect(result).toBe('Xc') }) -test('BRE mode supports the open lower-bound interval \\{,m\\}', () => { - // GNU sed accepts `\{,3\}` (up to three). JS has no `{,3}` quantifier, so it - // must be normalized to `{0,3}`; otherwise the preview showed no change while - // sed rewrote the run. Without `g` there is a single match, so sed's and JS's - // differing empty-match advance cannot come into play. - const result = applySedSubstitution('aaaab', { - filePath: 'example.txt', - pattern: 'a\\{,3\\}', - replacement: 'X', - flags: '', - extendedRegex: false, - }) - expect(result).toBe('Xab') +test('BRE mode declines the GNU-only open lower-bound interval \\{,m\\}', () => { + // `\{,3\}` is a GNU extension: BSD/macOS sed rejects it and leaves the file + // untouched, and this parser explicitly supports macOS via its `-i ''` + // handling. Previewing a {0,3} result would show an edit on platforms where + // the real command fails, so no sed edit is claimed even without `g`. + expect( + parseSedEditCommand("sed -i '' 's/a\\{,3\\}/X/' example.txt"), + ).toBeNull() }) test('BRE mode supports the open upper-bound interval \\{n,\\}', () => { @@ -97,20 +95,23 @@ test('BRE mode declines malformed intervals rather than guessing', () => { ).toBeNull() }) -test('BRE mode treats escaped alternation as an operator', () => { - const result = applySedSubstitution( - 'cat and dog', - sedInfo('cat\\|dog', 'X'), - ) - expect(result).toBe('X and X') +test('BRE mode declines alternation rather than mis-selecting a branch', () => { + // POSIX regex picks the leftmost-longest alternative while JavaScript picks + // the first that matches: sed writes `\(a\|aa\)` on "aa" as "X", a JS + // preview as "Xa". Selection cannot be reproduced, so no edit is claimed. + expect( + parseSedEditCommand("sed -i '' 's/cat\\|dog/X/g' example.txt"), + ).toBeNull() + expect( + parseSedEditCommand("sed -i '' 's/\\(a\\|aa\\)\\{1\\}/X/' example.txt"), + ).toBeNull() }) -test('BRE mode treats escaped groups and question marks as operators', () => { - const result = applySedSubstitution('abab', sedInfo('\\(ab\\)\\+', 'X')) - expect(result).toBe('X') - - const optional = applySedSubstitution('ac abc', sedInfo('ab\\?c', 'X')) - expect(optional).toBe('X X') +test('BRE mode keeps escaped groups with portable interval quantifiers', () => { + // `\(...\)` and `\{n\}` are POSIX BRE, supported identically by GNU and + // BSD sed, so groups repeated via an interval still simulate. + const result = applySedSubstitution('abab cd', sedInfo('\\(ab\\)\\{2\\}', 'X')) + expect(result).toBe('X cd') }) test('BRE mode applies g across multiple interval quantifiers', () => { @@ -167,4 +168,38 @@ describe('declines to simulate what it cannot reproduce faithfully', () => { // content, rendering an invalid pattern as a silent "no change" diff. expect(parseSedEditCommand(cmd('s/a*\\{2\\}/X/g'))).toBeNull() }) + + test('rejects an unterminated bracket expression', () => { + // GNU sed rejects `s/[/X/g` with an unterminated-address error and leaves + // the file untouched; rendering `[` as a literal would persist an edit the + // command cannot perform. + expect(parseSedEditCommand(cmd('s/[/X/g'))).toBeNull() + }) + + test('rejects POSIX character classes JavaScript cannot interpret', () => { + // `[[:digit:]]` is a digit class to sed but a plain character set to a JS + // regex, so the two produce different files ("1a2": sed XaX, JS unchanged). + expect(parseSedEditCommand(cmd('s/[[:digit:]]/X/g'))).toBeNull() + expect( + parseSedEditCommand("sed -i '' -E 's/[[:alpha:]]+/X/g' example.txt"), + ).toBeNull() + }) + + test('rejects ERE alternation for the same POSIX-selection reason', () => { + expect( + parseSedEditCommand("sed -i '' -E 's/(a|aa)/X/' example.txt"), + ).toBeNull() + }) + + test('applies the substitution once per line like sed, not once per file', () => { + // sed substitutes the first match on EVERY line even without `g`. + const info = parseSedEditCommand(cmd('s/a\\{2\\}/X/')) + expect(info).not.toBeNull() + expect(applySedSubstitution('aa\naa\n', info!)).toBe('X\nX\n') + // With `g`, all matches on every line. + const g = parseSedEditCommand(cmd('s/a\\{2\\}/X/g')) + expect(applySedSubstitution('aaaa\naa b aa\n', g!)).toBe('XX\nX b X\n') + // A trailing newline is preserved and never treated as an extra empty line. + expect(applySedSubstitution('aa', info!)).toBe('X') + }) }) diff --git a/src/tools/BashTool/sedEditParser.ts b/src/tools/BashTool/sedEditParser.ts index 90213f4dee..323a3f981b 100644 --- a/src/tools/BashTool/sedEditParser.ts +++ b/src/tools/BashTool/sedEditParser.ts @@ -21,27 +21,32 @@ export type SedEditInfo = { extendedRegex: boolean } -// BRE metacharacters that are special only when escaped: `\+ \? \| \( \)` are -// the operator forms and the bare characters are literal — the reverse of JS. -// Braces are handled separately because they only form an operator (the -// interval quantifier) when they enclose a valid count. -const BRE_ESCAPE_FLIP_METACHARS = '+?|()' +// Escaped forms that are portable POSIX BRE operators: `\(` `\)` (grouping). +// The escaped forms of the characters below are NOT portable — `\+` `\?` `\|` +// are GNU extensions that BSD/macOS sed matches literally — so they decline +// instead (see the branch handling them). +const BRE_PORTABLE_OPERATOR_ESCAPES = '()' + +// JS regex metacharacters that are literal when bare in BRE (the reverse of +// JS), so they must be escaped in the translation. Braces are handled +// separately because they only form an operator when escaped around a valid +// count. +const BRE_BARE_LITERAL_METACHARS = '+?|()' /** * Translate the body of a BRE interval `\{...\}` to its JS quantifier form, or - * null when the body is not a legal interval. GNU sed accepts `n`, `n,`, `n,m`, - * and the extension `,m`. `,m` has no JS spelling, so it is normalized to - * `{0,m}`. Anything else — empty, extra commas, non-numeric — is rejected by - * sed itself ("Invalid content of \{\}"), which aborts the command and leaves - * the file untouched; we cannot render that as an edit, so it declines. + * null when it cannot be previewed faithfully. Only the POSIX-portable forms + * `n`, `n,` and `n,m` are accepted: the GNU-only `,m` extension is rejected by + * BSD/macOS sed (which this parser explicitly supports via its `-i ''` + * handling), so previewing it would show an edit on platforms where the real + * command fails and changes nothing. Anything else — empty, extra commas, + * non-numeric — is rejected by sed itself ("Invalid content of \{\}"), which + * aborts the command and leaves the file untouched. */ function breIntervalBodyToJs(body: string): string | null { if (/^[0-9]+(,[0-9]*)?$/.test(body)) { return `{${body}}` } - if (/^,[0-9]+$/.test(body)) { - return `{0${body}}` - } return null } @@ -72,19 +77,19 @@ function convertBrePatternToJs(pattern: string): string | null { if (char === '[') { // Inside a bracket expression `\{`/`\}` are ordinary members rather than - // an interval, so the interval scan below must not see them. A backslash - // is itself a literal member in a POSIX bracket expression but an escape - // in a JS character class, so those cannot be mapped across — decline - // instead of silently changing which characters match. + // an interval, so the interval scan below must not see them. Several + // bracket constructs cannot be previewed faithfully and must decline: + // - an unterminated `[` is an error in sed (file untouched), not a + // literal; + // - a backslash is a literal member in a POSIX bracket expression but an + // escape in a JS character class; + // - `[:class:]`, `[=equiv=]` and `[.collate.]` constructs mean nothing + // to JS, which would read them as a plain set of characters. const end = findBracketEnd(pattern, i) - if (end === -1) { - // Never closed: a lone `[` is literal in BRE. - result += '\\[' - continue - } + if (end === -1) return null const body = pattern.slice(i, end + 1) - if (body.includes('\\')) return null - // Members are literal in both dialects, so the body carries over as-is. + if (body.includes('\\') || body.slice(1).includes('[')) return null + // Remaining members are literal in both dialects; carry the body as-is. result += body i = end continue @@ -114,9 +119,23 @@ function convertBrePatternToJs(pattern: string): string | null { i++ continue } + if (next === '|') { + // GNU alternation extension. Doubly unfaithful: BSD/macOS sed matches + // `\|` as a literal pipe, and even on GNU, POSIX regex selects the + // leftmost-longest alternative while JavaScript takes the first one + // that matches (`\(a\|aa\)` previews `aa` as `Xa` where GNU sed writes + // `X`). Decline. + return null + } + if (next === '+' || next === '?') { + // GNU extensions: BSD/macOS sed matches `\+` and `\?` as literal + // `+`/`?`, so one platform's operator is the other's literal and a + // single preview cannot be right for both. Decline. + return null + } if (next === '\\') { result += '\\\\' - } else if (BRE_ESCAPE_FLIP_METACHARS.includes(next)) { + } else if (BRE_PORTABLE_OPERATOR_ESCAPES.includes(next)) { result += next } else { result += `\\${next}` @@ -125,7 +144,7 @@ function convertBrePatternToJs(pattern: string): string | null { continue } - if (BRE_ESCAPE_FLIP_METACHARS.includes(char)) { + if (BRE_BARE_LITERAL_METACHARS.includes(char)) { result += `\\${char}` continue } @@ -353,24 +372,59 @@ export function parseSedEditCommand(command: string): SedEditInfo | null { } } +/** + * Reject ERE constructs whose behavior in a JS regex is not the POSIX behavior. + * The BRE converter declines these itself; ERE patterns carry over verbatim, so + * they need the same screening: + * - alternation: POSIX picks the leftmost-longest alternative, JS the first + * that matches; + * - bracket expressions holding a backslash (literal member in POSIX, escape + * in JS), a POSIX `[:class:]`-style construct, or no terminator at all + * (an error in sed, not a literal). + */ +function ereHasUnfaithfulConstructs(pattern: string): boolean { + for (let i = 0; i < pattern.length; i++) { + const char = pattern[i]! + if (char === '\\') { + i++ + continue + } + if (char === '|') return true + if (char === '[') { + const end = findBracketEnd(pattern, i) + if (end === -1) return true + const body = pattern.slice(i + 1, end) + if (body.includes('\\') || body.includes('[')) return true + i = end + } + } + return false +} + /** * Convert the sed pattern to the JS regex source this module would run, or null - * if it cannot be translated. + * if it cannot be translated faithfully. */ function toJsPatternSource( pattern: string, extendedRegex: boolean, ): string | null { const unescaped = pattern.replace(/\\\//g, '/') - return extendedRegex ? unescaped : convertBrePatternToJs(unescaped) + if (extendedRegex) { + return ereHasUnfaithfulConstructs(unescaped) ? null : unescaped + } + return convertBrePatternToJs(unescaped) } /** * Whether the simulated substitution is guaranteed to match what sed does. * - * Two cases are declined: - * - the pattern does not translate, or the translation is not a valid JS regex - * (previously this threw and was swallowed into a "no change" preview); + * Declined here: + * - the pattern does not translate — GNU-only interval forms, alternation, + * untranslatable or unterminated bracket expressions (see + * convertBrePatternToJs / ereHasUnfaithfulConstructs) — or the translation + * is not a valid JS regex (previously this threw and was swallowed into a + * "no change" preview); * - the pattern can match the empty string under `g`. sed and JS advance * differently after an empty match, so the results genuinely differ: * `s/a*​/X/g` on "aaaab" is "XbX" in sed but "XXbX" in JS, and @@ -447,11 +501,25 @@ export function applySedSubstitution( // Convert placeholder back to literal & .replace(new RegExp(ESCAPED_AMP_PLACEHOLDER, 'g'), '&') + let regex: RegExp try { - const regex = new RegExp(jsPattern, regexFlags) - return content.replace(regex, jsReplacement) + regex = new RegExp(jsPattern, regexFlags) } catch { // If regex is invalid, return original content return content } + + // sed applies s/// to each line of the pattern space independently: without + // `g` it substitutes the first match on EVERY line, not the first match in + // the file. A single whole-buffer replace previewed `sed 's/a\{2\}/X/'` on + // "aa\naa\n" as "X\naa\n" where sed writes "X\nX\n". Apply per line. A + // trailing newline produces a final empty split element that corresponds to + // no input line, so it is carried over untouched. + const endsWithNewline = content.endsWith('\n') + const body = endsWithNewline ? content.slice(0, -1) : content + const result = body + .split('\n') + .map(line => line.replace(regex, jsReplacement)) + .join('\n') + return endsWithNewline ? result + '\n' : result } From 356a52bf3e72cdb9e563cd5659415167e7019e0a Mon Sep 17 00:00:00 2001 From: 0xfandom Date: Thu, 16 Jul 2026 14:51:19 +0530 Subject: [PATCH 05/12] fix(bash): keep empty files empty in the per-line sed simulation An empty file has no lines, so sed never runs the substitution and the output stays empty. Splitting '' fabricated one empty line, letting anchored patterns like s/^/X/ preview an edit the real command does not make. Return early before splitting; verified against GNU sed 4.10 and BSD/macOS sed (both leave the file at 0 bytes). --- src/tools/BashTool/sedEditParser.test.ts | 8 ++++++++ src/tools/BashTool/sedEditParser.ts | 7 +++++++ 2 files changed, 15 insertions(+) diff --git a/src/tools/BashTool/sedEditParser.test.ts b/src/tools/BashTool/sedEditParser.test.ts index 3fcb6c05c9..b290468ec0 100644 --- a/src/tools/BashTool/sedEditParser.test.ts +++ b/src/tools/BashTool/sedEditParser.test.ts @@ -202,4 +202,12 @@ describe('declines to simulate what it cannot reproduce faithfully', () => { // A trailing newline is preserved and never treated as an extra empty line. expect(applySedSubstitution('aa', info!)).toBe('X') }) + + test('leaves an empty file empty like sed', () => { + // An empty file has no lines: sed never runs the substitution, so even an + // anchored pattern that matches the empty string writes nothing. + const info = parseSedEditCommand(cmd('s/^/X/')) + expect(info).not.toBeNull() + expect(applySedSubstitution('', info!)).toBe('') + }) }) diff --git a/src/tools/BashTool/sedEditParser.ts b/src/tools/BashTool/sedEditParser.ts index 323a3f981b..96c42f9352 100644 --- a/src/tools/BashTool/sedEditParser.ts +++ b/src/tools/BashTool/sedEditParser.ts @@ -509,6 +509,13 @@ export function applySedSubstitution( return content } + // An empty file has no lines, so sed never runs the substitution and the + // output stays empty; splitting '' would fabricate one empty line and let + // anchored patterns like s/^/X/ preview an edit sed does not make. + if (content.length === 0) { + return content + } + // sed applies s/// to each line of the pattern space independently: without // `g` it substitutes the first match on EVERY line, not the first match in // the file. A single whole-buffer replace previewed `sed 's/a\{2\}/X/'` on From 8d449bed138ed28293ea2d9ead775cf32c38bfe7 Mon Sep 17 00:00:00 2001 From: 0xfandom Date: Mon, 20 Jul 2026 18:48:19 +0530 Subject: [PATCH 06/12] fix(bash): only simulate the sed subset that translates faithfully MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The preview replaces the command once approved, so every accepted pattern must produce exactly what sed writes. Move from screening known-bad constructs to admitting only ones with matching semantics: - flags: accept g/i/I only. 1-9 select the Nth match per line (the simulator always rewrites the first, so s/a\{2\}/X/2 on "aaaa" previewed "Xaa" where sed writes "aaX"); p prints; m/M redefine ^ and $. - replacements: require literal text. \1-\9, &, \n and \U are sed syntax the simulator passes through verbatim, so s/\(a\)\{2\}/\1/ wrote the literal characters \1 where sed writes a. - escapes: allow only the portable set. \< and \> are word boundaries in GNU sed but literal angle brackets in JS; \d is the converse. - anchors: bare ^ and $ only anchor at a BRE boundary, so s/a^b/X/ previewed no change while sed rewrote the literal text. - ERE intervals: validate the body there too — JS reads a{,3} as literal braces while GNU sed -E rewrites "aaaab" to "XXbX". - character semantics: compile with u, so a quantifier counts characters as sed does rather than UTF-16 code units. - CRLF: compile with s, so . matches the carriage return the pattern space holds. - an empty pattern declines: sed has no previous regexp to reuse and errors. Verified against GNU sed 4.10 and BSD/macOS sed: all 12 accepted expressions match both byte-for-byte. --- src/tools/BashTool/sedEditParser.test.ts | 73 +++++++++++++ src/tools/BashTool/sedEditParser.ts | 133 +++++++++++++++++++---- 2 files changed, 185 insertions(+), 21 deletions(-) diff --git a/src/tools/BashTool/sedEditParser.test.ts b/src/tools/BashTool/sedEditParser.test.ts index b290468ec0..9ac41542fd 100644 --- a/src/tools/BashTool/sedEditParser.test.ts +++ b/src/tools/BashTool/sedEditParser.test.ts @@ -191,6 +191,79 @@ describe('declines to simulate what it cannot reproduce faithfully', () => { ).toBeNull() }) + test('rejects numeric occurrence flags it does not model', () => { + // `2` selects the second match on each line, but the simulator always + // rewrites the first: sed turns "aaaa" into "aaX", a preview into "Xaa". + expect(parseSedEditCommand(cmd('s/a\\{2\\}/X/2'))).toBeNull() + expect(parseSedEditCommand(cmd('s/a/X/9'))).toBeNull() + // `p` prints and `m`/`M` redefine ^ and $ inside the pattern space. + expect(parseSedEditCommand(cmd('s/a/X/p'))).toBeNull() + expect(parseSedEditCommand(cmd('s/a/X/m'))).toBeNull() + }) + + test('rejects replacements carrying sed-specific syntax', () => { + // `\1` is a backreference to sed but two literal characters to the + // simulator: sed rewrites "aa" as "a", the preview as "\1". + expect(parseSedEditCommand(cmd('s/\\(a\\)\\{2\\}/\\1/'))).toBeNull() + expect(parseSedEditCommand(cmd('s/a/\\n/'))).toBeNull() + // A plain literal replacement still simulates. + expect(parseSedEditCommand(cmd('s/a/X/'))).not.toBeNull() + }) + + test('rejects escapes whose sed meaning is not the JavaScript meaning', () => { + // GNU sed reads `\<`/`\>` as word boundaries; JS reads literal angle + // brackets, so the preview shows no change while sed rewrites the file. + expect(parseSedEditCommand(cmd('s/\\/X/g'))).toBeNull() + // The converse: `\d` is a digit class in JS but a literal `d` in BRE. + expect(parseSedEditCommand(cmd('s/\\d/X/g'))).toBeNull() + expect(parseSedEditCommand(cmd('s/\\w/X/g'))).toBeNull() + }) + + test('rejects ^ and $ where BRE treats them as literals', () => { + // Bare `^`/`$` only anchor at a BRE boundary; elsewhere sed matches them + // literally while JS always reads an anchor. + expect(parseSedEditCommand(cmd('s/a^b/X/'))).toBeNull() + expect(parseSedEditCommand(cmd('s/a$b/X/'))).toBeNull() + // Genuine anchors still simulate. + expect(parseSedEditCommand(cmd('s/^a/X/'))).not.toBeNull() + expect(parseSedEditCommand(cmd('s/a$/X/'))).not.toBeNull() + }) + + test('validates interval bodies on the ERE path too', () => { + // JS reads `a{,3}` as literal braces; GNU sed -E applies its extension and + // rewrites "aaaab" to "XXbX", and BSD has no portable behavior. + expect( + parseSedEditCommand("sed -i '' -E 's/a{,3}/X/g' example.txt"), + ).toBeNull() + expect( + parseSedEditCommand("sed -i '' -E 's/a{1,2,3}/X/g' example.txt"), + ).toBeNull() + }) + + test('counts characters like sed, not UTF-16 code units', () => { + // Verified against GNU sed 4.10: `s/.\{2\}/X/` on the emoji + "a" writes a + // bare "X". Without a unicode-aware matcher the quantifier consumes only + // the emoji's surrogate pair and leaves the "a". + const info = parseSedEditCommand(cmd('s/.\\{2\\}/X/')) + expect(info).not.toBeNull() + expect(applySedSubstitution('\u{1F600}a', info!)).toBe('X') + }) + + test('matches a carriage return the way sed pattern space does', () => { + // Verified against GNU sed 4.10: `s/./X/g` on "a\r\n" writes "XX\n" — the + // CR is an ordinary character in the pattern space. A JS `.` excludes + // carriage returns and would leave it in place. + const info = parseSedEditCommand(cmd('s/./X/g')) + expect(info).not.toBeNull() + expect(applySedSubstitution('a\r\n', info!)).toBe('XX\n') + }) + + test('rejects a standalone empty pattern', () => { + // `s//X/` has no previous regular expression to reuse, so sed errors and + // leaves the file untouched; an empty JS regex would prefix every line. + expect(parseSedEditCommand(cmd('s//X/'))).toBeNull() + }) + test('applies the substitution once per line like sed, not once per file', () => { // sed substitutes the first match on EVERY line even without `g`. const info = parseSedEditCommand(cmd('s/a\\{2\\}/X/')) diff --git a/src/tools/BashTool/sedEditParser.ts b/src/tools/BashTool/sedEditParser.ts index 96c42f9352..a642e64b08 100644 --- a/src/tools/BashTool/sedEditParser.ts +++ b/src/tools/BashTool/sedEditParser.ts @@ -70,6 +70,7 @@ function findBracketEnd(pattern: string, open: number): number { * translated faithfully and the caller must decline to simulate the edit. */ function convertBrePatternToJs(pattern: string): string | null { + if (!breAnchorsAreUnambiguous(pattern)) return null let result = '' for (let i = 0; i < pattern.length; i++) { @@ -137,8 +138,11 @@ function convertBrePatternToJs(pattern: string): string | null { result += '\\\\' } else if (BRE_PORTABLE_OPERATOR_ESCAPES.includes(next)) { result += next - } else { + } else if (BRE_PORTABLE_LITERAL_ESCAPES.includes(next)) { result += `\\${next}` + } else { + // Not a portable escape: its sed meaning is not the JS meaning. + return null } i++ continue @@ -161,6 +165,59 @@ function convertBrePatternToJs(pattern: string): string | null { return result } +/** + * Only replacements that are pure literal text can be previewed. + * + * sed's replacement syntax has its own meanings that this module does not + * translate: `\1`-`\9` are backreferences, `&` is the whole match, `\n`/`\t` + * are escapes, and `\U`/`\L` (GNU) case-fold. The simulator passes the + * replacement to String.replace as-is, so `s/\(a\)\{2\}/\1/` writes the two + * literal characters `\1` where sed writes `a`. Decline anything carrying a + * backslash or an unescaped `&`. + */ +function isFaithfullyLiteralReplacement(replacement: string): boolean { + return !replacement.includes('\\') && !replacement.includes('&') +} + +/** + * Escapes that mean the same thing in a POSIX BRE and in the emitted JS regex. + * + * Everything outside this set declines. GNU sed reads `\<`/`\>` as word + * boundaries while JS reads them as literal angle brackets, so `s/\/X/g` + * previews no change while sed rewrites the file; `\d` has the converse problem + * (a digit class in JS, a literal `d` in BRE). Carrying unhandled escapes + * through means the emitted regex is not the pattern sed was given. + */ +const BRE_PORTABLE_LITERAL_ESCAPES = '.*[]^$/' + +/** + * `^` and `$` are only anchors at the start/end of a BRE or of a `\( \)` + * subexpression; anywhere else sed matches them literally, while JS always + * reads them as anchors. `s/a^b/X/` would therefore be accepted and preview no + * edit while sed rewrites the literal text. Rather than model every + * subexpression boundary, decline when either character appears outside the + * positions where both dialects agree it anchors. + */ +function breAnchorsAreUnambiguous(pattern: string): boolean { + for (let i = 0; i < pattern.length; i++) { + const char = pattern[i]! + if (char === '\\') { + i++ + continue + } + if (char === '[') { + const end = findBracketEnd(pattern, i) + if (end === -1) return false + i = end + continue + } + // A leading `^` and a trailing `$` are anchors in both dialects. + if (char === '^' && i !== 0) return false + if (char === '$' && i !== pattern.length - 1) return false + } + return true +} + /** * Check if a command is a sed in-place edit command * Returns true only for simple sed -i 's/pattern/replacement/flags' file commands @@ -350,8 +407,13 @@ export function parseSedEditCommand(command: string): SedEditInfo | null { return null } - // Validate flags - only allow safe substitution flags - const validFlags = /^[gpimIM1-9]*$/ + // Only the flags this module actually models. `1`-`9` select the Nth match on + // each line and `p` prints, neither of which the simulator implements — it + // always rewrites the first match (or every match under `g`), so + // `s/a\{2\}/X/2` on "aaaa" would be previewed as "Xaa" where sed writes + // "aaX". `m`/`M` redefine `^`/`$` per line inside the pattern space. Decline + // all of them rather than approve a write that differs from the command. + const validFlags = /^[giI]*$/ if (!validFlags.test(flags)) { return null } @@ -363,6 +425,10 @@ export function parseSedEditCommand(command: string): SedEditInfo | null { return null } + if (!isFaithfullyLiteralReplacement(replacement)) { + return null + } + return { filePath, pattern, @@ -390,6 +456,20 @@ function ereHasUnfaithfulConstructs(pattern: string): boolean { continue } if (char === '|') return true + if (char === '^' && i !== 0) return true + if (char === '$' && i !== pattern.length - 1) return true + if (char === '{') { + // JS reads an illegal interval body as literal braces, but GNU sed either + // errors or applies its own extension: `sed -E 's/a{,3}/X/g'` rewrites + // "aaaab" to "XXbX" while JS matches the literal text "a{,3}". BSD has no + // portable behavior here either, so anything outside the POSIX forms + // declines. + const close = pattern.indexOf('}', i + 1) + if (close === -1) return true + if (breIntervalBodyToJs(pattern.slice(i + 1, close)) === null) return true + i = close + continue + } if (char === '[') { const end = findBracketEnd(pattern, i) if (end === -1) return true @@ -435,12 +515,19 @@ function canSimulateFaithfully( flags: string, extendedRegex: boolean, ): boolean { + // A standalone `s//X/` has no previous regular expression to reuse, so sed + // errors and leaves the file untouched. JS would compile an empty regex that + // matches at every position and prefix every line. + if (pattern === '') return false + const jsPattern = toJsPatternSource(pattern, extendedRegex) if (jsPattern === null) return false let regex: RegExp try { - regex = new RegExp(jsPattern) + // Same flags the simulator will use, so a source that only compiles without + // them cannot slip through the gate. + regex = new RegExp(jsPattern, jsRegexFlags(flags)) } catch { return false } @@ -450,6 +537,26 @@ function canSimulateFaithfully( return true } +/** + * The JS flags that reproduce sed's matching for the accepted flag subset. + * + * `u` is not optional: in the UTF-8 locales sed runs in, a quantifier counts + * characters, while a non-unicode JS regex counts UTF-16 code units — without + * it `s/.\{2\}/X/` turns "\u{1F600}a" into "Xa" (consuming only the emoji's + * surrogate pair) where sed writes "X". + * + * `s` makes `.` match every character in the line. sed's pattern space holds a + * lone `\r` on CRLF input and `.` matches it, but a JS `.` excludes carriage + * returns, so `sed 's/./X/g'` on "a\r\n" writes "XX" where an unflagged + * simulation writes "X\r". + */ +function jsRegexFlags(sedFlags: string): string { + let flags = 'us' + if (sedFlags.includes('g')) flags += 'g' + if (sedFlags.includes('i') || sedFlags.includes('I')) flags += 'i' + return flags +} + /** * Apply a sed substitution to file content * Returns the new content after applying the substitution @@ -458,23 +565,7 @@ export function applySedSubstitution( content: string, sedInfo: SedEditInfo, ): string { - // Convert sed pattern to JavaScript regex - let regexFlags = '' - - // Handle global flag - if (sedInfo.flags.includes('g')) { - regexFlags += 'g' - } - - // Handle case-insensitive flag (i or I in sed) - if (sedInfo.flags.includes('i') || sedInfo.flags.includes('I')) { - regexFlags += 'i' - } - - // Handle multiline flag (m or M in sed) - if (sedInfo.flags.includes('m') || sedInfo.flags.includes('M')) { - regexFlags += 'm' - } + const regexFlags = jsRegexFlags(sedInfo.flags) // Convert sed pattern to JavaScript regex pattern. In BRE mode (no -E flag) // metacharacters have opposite escaping: BRE `\+` means "one or more" and `+` From ce19172cdb7d860aa2f5ef26811cefe3166cb9da Mon Sep 17 00:00:00 2001 From: 0xfandom Date: Tue, 21 Jul 2026 17:35:24 +0530 Subject: [PATCH 07/12] fix(bash): decline bracket expressions opening with a ] member POSIX treats the first ] in [] ] / [^] ] as an ordinary member, so GNU sed rewrites "a]b" to "aXb". JavaScript reads it as the class terminator and matches nothing, so the preview showed no change while sed rewrote the file. findBracketEnd already skipped the leading ] to locate the real terminator, but the body was then carried into the JS regex verbatim. Decline on both the BRE and ERE paths. --- src/tools/BashTool/sedEditParser.test.ts | 11 +++++++++++ src/tools/BashTool/sedEditParser.ts | 15 +++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/src/tools/BashTool/sedEditParser.test.ts b/src/tools/BashTool/sedEditParser.test.ts index 9ac41542fd..1a20d09527 100644 --- a/src/tools/BashTool/sedEditParser.test.ts +++ b/src/tools/BashTool/sedEditParser.test.ts @@ -169,6 +169,17 @@ describe('declines to simulate what it cannot reproduce faithfully', () => { expect(parseSedEditCommand(cmd('s/a*\\{2\\}/X/g'))).toBeNull() }) + test('rejects bracket expressions with a leading ] member', () => { + // POSIX treats the first `]` as an ordinary member, so GNU sed rewrites + // "a]b" to "aXb"; JavaScript reads it as the class terminator and matches + // nothing, leaving the text untouched. + expect(parseSedEditCommand(cmd('s/[]]/X/g'))).toBeNull() + expect(parseSedEditCommand(cmd('s/[^]]/X/g'))).toBeNull() + expect( + parseSedEditCommand("sed -i '' -E 's/[]]/X/g' example.txt"), + ).toBeNull() + }) + test('rejects an unterminated bracket expression', () => { // GNU sed rejects `s/[/X/g` with an unterminated-address error and leaves // the file untouched; rendering `[` as a literal would persist an edit the diff --git a/src/tools/BashTool/sedEditParser.ts b/src/tools/BashTool/sedEditParser.ts index a642e64b08..4ff21d33cf 100644 --- a/src/tools/BashTool/sedEditParser.ts +++ b/src/tools/BashTool/sedEditParser.ts @@ -65,6 +65,19 @@ function findBracketEnd(pattern: string, open: number): number { return -1 } +/** + * True when a bracket expression opens with a `]` member (`[]]`, `[^]]`). + * + * POSIX treats that first `]` as an ordinary member, so `s/[]]/X/g` rewrites + * "a]b" to "aXb". JavaScript reads it as the class terminator instead, leaving + * the text untouched, so the two dialects disagree and the pattern declines. + */ +function bracketHasLeadingCloseMember(pattern: string, open: number): boolean { + let i = open + 1 + if (pattern[i] === '^') i++ + return pattern[i] === ']' +} + /** * Convert a BRE pattern to its JS equivalent, or null when it cannot be * translated faithfully and the caller must decline to simulate the edit. @@ -88,6 +101,7 @@ function convertBrePatternToJs(pattern: string): string | null { // to JS, which would read them as a plain set of characters. const end = findBracketEnd(pattern, i) if (end === -1) return null + if (bracketHasLeadingCloseMember(pattern, i)) return null const body = pattern.slice(i, end + 1) if (body.includes('\\') || body.slice(1).includes('[')) return null // Remaining members are literal in both dialects; carry the body as-is. @@ -473,6 +487,7 @@ function ereHasUnfaithfulConstructs(pattern: string): boolean { if (char === '[') { const end = findBracketEnd(pattern, i) if (end === -1) return true + if (bracketHasLeadingCloseMember(pattern, i)) return true const body = pattern.slice(i + 1, end) if (body.includes('\\') || body.includes('[')) return true i = end From 61507699e5713b12eb23411464aab741588aeed4 Mon Sep 17 00:00:00 2001 From: 0xfandom Date: Wed, 22 Jul 2026 19:24:35 +0530 Subject: [PATCH 08/12] fix(bash): close three sed preview divergences before approval All three let an approved preview differ from what the command writes, which is the failure this simulator exists to avoid -- the preview is persisted directly once the user approves it. Dollar tokens: $ is an ordinary character in a sed replacement but a substitution token to String.replace, so s/\(a\)/$1/ previewed the matched text where sed writes the two characters $1. Double each $ for the JS replacement. ERE escapes: the ERE screen skipped every backslash escape and then used the source verbatim, so -E 's/\d/X/g' on "1d2" previewed "XdX" while sed writes "1X2" -- GNU sed reads \d as a literal d. \w, \s and \u{...} diverge the same way. Admit only the escapes that are the same literal in both dialects, mirroring the BRE allowlist. Case-insensitive matching: the emitted regex needs u for the quantifier fix, and u + i selects ECMAScript Unicode case folding rather than sed's locale matching, so s/k/X/I rewrote a Kelvin sign that GNU sed under C.UTF-8 leaves alone. Decline i/I until that can be modeled. Verified the accepted set against GNU sed 4.10: 11 expressions, including each dollar case above, byte-identical. --- src/tools/BashTool/sedEditParser.test.ts | 91 ++++++++++++++++++++++++ src/tools/BashTool/sedEditParser.ts | 45 ++++++++++-- 2 files changed, 132 insertions(+), 4 deletions(-) diff --git a/src/tools/BashTool/sedEditParser.test.ts b/src/tools/BashTool/sedEditParser.test.ts index 1a20d09527..68a4373c8e 100644 --- a/src/tools/BashTool/sedEditParser.test.ts +++ b/src/tools/BashTool/sedEditParser.test.ts @@ -295,3 +295,94 @@ describe('declines to simulate what it cannot reproduce faithfully', () => { expect(applySedSubstitution('', info!)).toBe('') }) }) + +describe('replacement text is written literally, as sed writes it', () => { + const cmd = (expr: string) => `sed -i '' '${expr}' example.txt` + + test('writes dollar tokens literally instead of expanding them', () => { + // `$` is an ordinary character in a sed replacement but a substitution + // token to String.replace. GNU sed 4.10 writes the two characters `$1` + // here; before the fix the preview expanded it to the matched text, so an + // approved diff differed from what the command performs. + const one = parseSedEditCommand(cmd('s/\\(a\\)/$1/')) + expect(one).not.toBeNull() + expect(applySedSubstitution('a', one!)).toBe('$1') + + const dollars = parseSedEditCommand(cmd('s/a/$$/')) + expect(dollars).not.toBeNull() + expect(applySedSubstitution('a', dollars!)).toBe('$$') + + // $` and $' expand to the text before/after the match in JS. + const before = parseSedEditCommand(cmd('s/b/$`/')) + expect(before).not.toBeNull() + expect(applySedSubstitution('abc', before!)).toBe('a$`c') + + // `$'` cannot go through the single-quoted fixture above, so drive it + // directly: in JS it expands to the text following the match. + expect(applySedSubstitution('abc', sedInfo('b', "$'"))).toBe("a$'c") + + // `&` is sed's own whole-match token and is declined outright, so a + // replacement spelling JS's `$&` never reaches the simulator. + expect(parseSedEditCommand(cmd('s/b/$&/'))).toBeNull() + }) + + test('a plain replacement is unaffected', () => { + const info = parseSedEditCommand(cmd('s/a/USD 5/g')) + expect(info).not.toBeNull() + expect(applySedSubstitution('a b a', info!)).toBe('USD 5 b USD 5') + }) +}) + +describe('ERE patterns are screened for JS-only escapes', () => { + const ere = (expr: string) => `sed -i '' -E '${expr}' example.txt` + + test('declines escapes that mean a character class only in JS', () => { + // GNU sed reads `\d` as a literal `d`, so `-E 's/\d/X/g'` on "1d2" writes + // "1X2"; a JS regex reads a digit class and would preview "XdX". The same + // divergence applies to the other JS-only escapes, and several are not + // valid sed at all. + for (const expr of [ + 's/\\d/X/g', + 's/\\w/X/g', + 's/\\s/X/g', + 's/\\S/X/g', + 's/\\b/X/g', + 's/\\u{41}/X/g', + 's/\\p{L}/X/g', + 's/\\x41/X/g', + ]) { + expect(parseSedEditCommand(ere(expr))).toBeNull() + } + }) + + test('still accepts escapes that are the same literal in both dialects', () => { + const dot = parseSedEditCommand(ere('s/a\\.b/X/g')) + expect(dot).not.toBeNull() + expect(applySedSubstitution('a.b axb', dot!)).toBe('X axb') + + const plus = parseSedEditCommand(ere('s/a\\+/X/g')) + expect(plus).not.toBeNull() + expect(applySedSubstitution('a+ aa', plus!)).toBe('X aa') + }) + + test('declines a trailing lone backslash', () => { + expect(parseSedEditCommand(ere('s/a\\/X/g'))).toBeNull() + }) +}) + +describe('case-insensitive matching is declined', () => { + test('declines the i and I flags rather than folding by Unicode rules', () => { + // The emitted regex needs `u` for the quantifier fix, and `u` + `i` is + // ECMAScript Unicode case folding, not sed's locale matching: `s/k/X/I` + // would rewrite a Kelvin sign that GNU sed under C.UTF-8 leaves alone. + for (const expr of ["s/k/X/I", "s/k/X/i", "s/k/X/gI", "s/k/X/gi"]) { + expect( + parseSedEditCommand(`sed -i '' '${expr}' example.txt`), + ).toBeNull() + } + // The g flag on its own is still simulated. + expect( + parseSedEditCommand("sed -i '' 's/k/X/g' example.txt"), + ).not.toBeNull() + }) +}) diff --git a/src/tools/BashTool/sedEditParser.ts b/src/tools/BashTool/sedEditParser.ts index 4ff21d33cf..3eae2f15ec 100644 --- a/src/tools/BashTool/sedEditParser.ts +++ b/src/tools/BashTool/sedEditParser.ts @@ -193,6 +193,17 @@ function isFaithfullyLiteralReplacement(replacement: string): boolean { return !replacement.includes('\\') && !replacement.includes('&') } +/** + * `$` is an ordinary character in a sed replacement but a substitution token in + * a JS one: `$1`, `$$`, `` $` `` and `$'` all expand. `s/\(a\)/$1/` is a + * literal replacement by the rule above, yet String.replace would write the + * matched text where sed writes the two characters `$1`. Doubling each `$` is + * the documented way to emit one literally. + */ +function escapeJsReplacement(replacement: string): string { + return replacement.replaceAll('$', '$$$$') +} + /** * Escapes that mean the same thing in a POSIX BRE and in the emitted JS regex. * @@ -427,7 +438,13 @@ export function parseSedEditCommand(command: string): SedEditInfo | null { // `s/a\{2\}/X/2` on "aaaa" would be previewed as "Xaa" where sed writes // "aaX". `m`/`M` redefine `^`/`$` per line inside the pattern space. Decline // all of them rather than approve a write that differs from the command. - const validFlags = /^[giI]*$/ + // + // `i`/`I` are declined for a subtler reason: the emitted regex needs `u` for + // the quantifier fix, and `u` + `i` selects ECMAScript Unicode case folding + // rather than sed's locale-based matching. `s/k/X/I` would then rewrite a + // Kelvin sign, which GNU sed under C.UTF-8 leaves alone. Until that folding + // can be modeled, an approved preview would not be the edit sed performs. + const validFlags = /^g*$/ if (!validFlags.test(flags)) { return null } @@ -452,6 +469,19 @@ export function parseSedEditCommand(command: string): SedEditInfo | null { } } +/** + * Escapes that denote the same literal character in a POSIX ERE and in the + * emitted JS regex: the ERE metacharacters, plus the delimiter and a literal + * backslash. + * + * ERE patterns are handed to JS verbatim, so an escape outside this set is not + * the pattern sed was given. `\d` is the clearest case -- GNU sed reads it as a + * literal `d`, JS as a digit class, so `sed -E 's/\d/X/g'` on "1d2" previews + * "XdX" while sed writes "1X2". `\w`, `\s`, `\b` and `\u{...}` diverge the same + * way, and several are not valid sed at all. + */ +const ERE_PORTABLE_LITERAL_ESCAPES = '.*[]^$/+?(){}|\\' + /** * Reject ERE constructs whose behavior in a JS regex is not the POSIX behavior. * The BRE converter declines these itself; ERE patterns carry over verbatim, so @@ -460,12 +490,17 @@ export function parseSedEditCommand(command: string): SedEditInfo | null { * that matches; * - bracket expressions holding a backslash (literal member in POSIX, escape * in JS), a POSIX `[:class:]`-style construct, or no terminator at all - * (an error in sed, not a literal). + * (an error in sed, not a literal); + * - escapes outside the set that is literal in both dialects (see + * ERE_PORTABLE_LITERAL_ESCAPES). */ function ereHasUnfaithfulConstructs(pattern: string): boolean { for (let i = 0; i < pattern.length; i++) { const char = pattern[i]! if (char === '\\') { + const escaped = pattern[i + 1] + if (escaped === undefined) return true + if (!ERE_PORTABLE_LITERAL_ESCAPES.includes(escaped)) return true i++ continue } @@ -564,11 +599,13 @@ function canSimulateFaithfully( * lone `\r` on CRLF input and `.` matches it, but a JS `.` excludes carriage * returns, so `sed 's/./X/g'` on "a\r\n" writes "XX" where an unflagged * simulation writes "X\r". + * + * There is deliberately no `i` here: `u` + `i` is Unicode case folding, not + * sed's locale matching, so `i`/`I` are declined at the flag gate instead. */ function jsRegexFlags(sedFlags: string): string { let flags = 'us' if (sedFlags.includes('g')) flags += 'g' - if (sedFlags.includes('i') || sedFlags.includes('I')) flags += 'i' return flags } @@ -597,7 +634,7 @@ export function applySedSubstitution( // Use a unique placeholder with random salt to prevent injection attacks const salt = randomBytes(8).toString('hex') const ESCAPED_AMP_PLACEHOLDER = `___ESCAPED_AMPERSAND_${salt}___` - const jsReplacement = sedInfo.replacement + const jsReplacement = escapeJsReplacement(sedInfo.replacement) // Unescape \/ to / .replace(/\\\//g, '/') // First escape \& to a placeholder From a7df3231f3cae4f9d655c0fbe487ddb47d58b671 Mon Sep 17 00:00:00 2001 From: 0xfandom Date: Thu, 23 Jul 2026 12:27:07 +0530 Subject: [PATCH 09/12] fix(bash): gate the sed preview on locale and bound its matching Three follow-ups to the preview-fidelity work. The emitted regex always carries the u flag so a quantifier counts characters, but sed inherits the process locale and counts bytes in a byte locale: LC_ALL=C 's/.\{2\}/X/' on an emoji consumes two of its bytes and leaves the rest in the file. Claim a sed edit only when the resolved locale (LC_ALL, then LC_CTYPE, then LANG, per POSIX) names a UTF-8 codeset. Interval support also made nested quantifiers translatable: \(a\{1,\}\)\{1,\}b becomes (a{1,}){1,}b, which backtracks exponentially on a run of a's with no b. applySedSubstitution runs synchronously while the permission request renders, so that stalls the approval UI before the user can decide. Decline a quantified group that already contains a quantifier. The replacement gate rejected every backslash, including \/ and \&, which the translation right below already handles faithfully -- so ordinary commands like s/foo/path\/to/ lost their file diff for no reason. Admit those two, keep declining backreferences, case folding and a bare &. Verified against GNU sed 4.10: the newly readmitted escapes and the still-accepted single-quantifier groups all match byte-for-byte. --- src/tools/BashTool/sedEditParser.test.ts | 113 ++++++++++++++++++++++- src/tools/BashTool/sedEditParser.ts | 105 ++++++++++++++++++++- 2 files changed, 214 insertions(+), 4 deletions(-) diff --git a/src/tools/BashTool/sedEditParser.test.ts b/src/tools/BashTool/sedEditParser.test.ts index 68a4373c8e..f342a57c04 100644 --- a/src/tools/BashTool/sedEditParser.test.ts +++ b/src/tools/BashTool/sedEditParser.test.ts @@ -1,11 +1,27 @@ -import { describe, expect, test } from 'bun:test' +import { afterAll, beforeAll, describe, expect, test } from 'bun:test' import { applySedSubstitution, parseSedEditCommand, + sedLocaleCountsCharacters, type SedEditInfo, } from './sedEditParser.js' +// Simulation is only claimed in a UTF-8 locale (see sedLocaleCountsCharacters), +// and CI machines do not reliably set one. Pin it so these cases exercise the +// translation rather than the locale gate; the gate has its own tests below. +const savedLcAll = process.env.LC_ALL +beforeAll(() => { + process.env.LC_ALL = 'en_US.UTF-8' +}) +afterAll(() => { + if (savedLcAll === undefined) { + delete process.env.LC_ALL + } else { + process.env.LC_ALL = savedLcAll + } +}) + function sedInfo(pattern: string, replacement: string, extendedRegex = false): SedEditInfo { return { filePath: 'example.txt', @@ -386,3 +402,98 @@ describe('case-insensitive matching is declined', () => { ).not.toBeNull() }) }) + +describe('locale-sensitive matching is declined outside UTF-8', () => { + test('declines when sed would count bytes rather than characters', () => { + // The emitted regex carries `u` so a quantifier counts characters. That is + // sed's behavior in a UTF-8 locale only: under LC_ALL=C, `s/.\{2\}/X/` on + // "😀a" consumes two bytes of the emoji and leaves the rest in the file. + for (const locale of ['C', 'POSIX', 'en_US.ISO-8859-1']) { + expect(sedLocaleCountsCharacters({ LC_ALL: locale })).toBe(false) + } + // POSIX resolves an unset or empty locale to C. + expect(sedLocaleCountsCharacters({})).toBe(false) + expect(sedLocaleCountsCharacters({ LC_ALL: '' , LANG: ''})).toBe(false) + }) + + test('accepts the UTF-8 spellings that actually occur', () => { + expect(sedLocaleCountsCharacters({ LC_ALL: 'en_US.UTF-8' })).toBe(true) + expect(sedLocaleCountsCharacters({ LANG: 'de_DE.utf8' })).toBe(true) + // macOS sets the bare codeset for LC_CTYPE. + expect(sedLocaleCountsCharacters({ LC_CTYPE: 'UTF-8' })).toBe(true) + expect(sedLocaleCountsCharacters({ LANG: 'fr_FR.UTF-8@euro' })).toBe(true) + }) + + test('honours the POSIX precedence order', () => { + // LC_ALL overrides everything; LC_CTYPE overrides LANG. + expect( + sedLocaleCountsCharacters({ LC_ALL: 'C', LC_CTYPE: 'en_US.UTF-8' }), + ).toBe(false) + expect(sedLocaleCountsCharacters({ LC_CTYPE: 'C', LANG: 'en_US.UTF-8' })).toBe( + false, + ) + expect(sedLocaleCountsCharacters({ LANG: 'en_US.UTF-8' })).toBe(true) + }) + + test('claims no sed edit at all under a byte locale', () => { + const saved = process.env.LC_ALL + process.env.LC_ALL = 'C' + try { + expect( + parseSedEditCommand("sed -i '' 's/a\\{2\\}/X/g' example.txt"), + ).toBeNull() + } finally { + process.env.LC_ALL = saved + } + }) +}) + +describe('patterns that can stall the approval UI are declined', () => { + const cmd = (expr: string) => `sed -i '' '${expr}' example.txt` + + test('declines a quantified group that already contains a quantifier', () => { + // `(a{1,}){1,}b` backtracks exponentially on a run of `a`s with no `b`, and + // applySedSubstitution runs synchronously while the permission request is + // rendered -- so this freezes the approval UI before the user can decide. + expect(parseSedEditCommand(cmd('s/\\(a\\{1,\\}\\)\\{1,\\}b/X/'))).toBeNull() + expect(parseSedEditCommand(cmd('s/\\(a*\\)\\{2\\}b/X/'))).toBeNull() + expect( + parseSedEditCommand("sed -i '' -E 's/(a+)+b/X/' example.txt"), + ).toBeNull() + }) + + test('still accepts a quantified group with no inner quantifier', () => { + const info = parseSedEditCommand(cmd('s/\\(ab\\)\\{2\\}/X/g')) + expect(info).not.toBeNull() + expect(applySedSubstitution('abab ab', info!)).toBe('X ab') + }) + + test('still accepts an inner quantifier when the group is not quantified', () => { + const info = parseSedEditCommand(cmd('s/\\(a\\{2\\}\\)b/X/g')) + expect(info).not.toBeNull() + expect(applySedSubstitution('aab ab', info!)).toBe('X ab') + }) +}) + +describe('literal replacement escapes keep their preview', () => { + const cmd = (expr: string) => `sed -i '' '${expr}' example.txt` + + test('accepts the two escapes the translation already handles', () => { + // Rejecting these sent ordinary commands back to the generic bash approval + // even though applySedSubstitution translates both faithfully. + const slash = parseSedEditCommand(cmd('s/foo/path\\/to/')) + expect(slash).not.toBeNull() + expect(applySedSubstitution('foo\n', slash!)).toBe('path/to\n') + + const amp = parseSedEditCommand(cmd('s/foo/a\\&b/')) + expect(amp).not.toBeNull() + expect(applySedSubstitution('foo\n', amp!)).toBe('a&b\n') + }) + + test('still declines backreferences, case folding and a bare &', () => { + expect(parseSedEditCommand(cmd('s/\\(a\\)/\\1/'))).toBeNull() + expect(parseSedEditCommand(cmd('s/a/\\U&/'))).toBeNull() + expect(parseSedEditCommand(cmd('s/a/\\n/'))).toBeNull() + expect(parseSedEditCommand(cmd('s/a/x&y/'))).toBeNull() + }) +}) diff --git a/src/tools/BashTool/sedEditParser.ts b/src/tools/BashTool/sedEditParser.ts index 3eae2f15ec..182a3a96e1 100644 --- a/src/tools/BashTool/sedEditParser.ts +++ b/src/tools/BashTool/sedEditParser.ts @@ -186,11 +186,26 @@ function convertBrePatternToJs(pattern: string): string | null { * translate: `\1`-`\9` are backreferences, `&` is the whole match, `\n`/`\t` * are escapes, and `\U`/`\L` (GNU) case-fold. The simulator passes the * replacement to String.replace as-is, so `s/\(a\)\{2\}/\1/` writes the two - * literal characters `\1` where sed writes `a`. Decline anything carrying a - * backslash or an unescaped `&`. + * literal characters `\1` where sed writes `a`. + * + * `\/` and `\&` are the exceptions: both denote a literal character, and the + * translation below already handles them, so rejecting them would send + * ordinary commands like `s/foo/path\/to/` back to the generic bash approval + * for no reason. Every other backslash escape, and a bare `&` (sed's + * whole-match token), declines. */ function isFaithfullyLiteralReplacement(replacement: string): boolean { - return !replacement.includes('\\') && !replacement.includes('&') + for (let i = 0; i < replacement.length; i++) { + const char = replacement[i]! + if (char === '\\') { + const escaped = replacement[i + 1] + if (escaped !== '/' && escaped !== '&') return false + i++ + continue + } + if (char === '&') return false + } + return true } /** @@ -449,6 +464,13 @@ export function parseSedEditCommand(command: string): SedEditInfo | null { return null } + // The emitted regex counts characters, which is only what sed does in a + // UTF-8 locale; under a byte locale it counts bytes and writes a different + // file. + if (!sedLocaleCountsCharacters()) { + return null + } + // Only claim this is a renderable sed edit if we can reproduce it faithfully. // Declining falls back to ordinary bash rendering, which is far better than // showing the user a diff that does not match what sed will write. @@ -584,9 +606,59 @@ function canSimulateFaithfully( if (flags.includes('g') && regex.test('')) return false + if (hasNestedQuantifier(jsPattern)) return false + return true } +/** + * Whether a quantifier is applied to a group that already contains one. + * + * Interval support means `\(a\{1,\}\)\{1,\}b` translates to `(a{1,}){1,}b` and + * otherwise passes this gate. On a run of `a`s with no `b`, matching that + * backtracks exponentially -- and applySedSubstitution runs synchronously while + * the permission request is being rendered, so a command plus a repository file + * can stall the approval UI before the user gets to decide. There is no useful + * preview to salvage here, so decline. + */ +function hasNestedQuantifier(jsSource: string): boolean { + const isQuantifierStart = (char: string | undefined): boolean => + char === '*' || char === '+' || char === '?' || char === '{' + + for (let i = 0; i < jsSource.length; i++) { + const char = jsSource[i]! + if (char === '\\') { + i++ + continue + } + if (char !== ')' || !isQuantifierStart(jsSource[i + 1])) continue + + // Walk back to this group's opening paren, then look for a quantifier + // inside it. Escaped parens are literals and do not open or close a group. + let depth = 0 + for (let j = i; j >= 0; j--) { + if (j > 0 && jsSource[j - 1] === '\\') continue + const inner = jsSource[j]! + if (inner === ')') depth++ + else if (inner === '(') { + depth-- + if (depth === 0) { + const body = jsSource.slice(j + 1, i) + for (let k = 0; k < body.length; k++) { + if (body[k] === '\\') { + k++ + continue + } + if (isQuantifierStart(body[k])) return true + } + break + } + } + } + } + return false +} + /** * The JS flags that reproduce sed's matching for the accepted flag subset. * @@ -609,6 +681,33 @@ function jsRegexFlags(sedFlags: string): string { return flags } +/** + * Whether the locale sed will inherit makes character-wise matching correct. + * + * The emitted regex always carries `u`, which is required for a quantifier to + * count characters the way sed does in a UTF-8 locale. But the command inherits + * the process locale, and in a byte locale (`LC_ALL=C`) sed counts bytes + * instead: `s/.\{2\}/X/` on "😀a" consumes two bytes of the emoji and leaves + * the rest of it in the file, where the simulation consumes the whole + * character. Since an approved preview is written directly, that is a different + * file. + * + * POSIX resolves the locale as LC_ALL, then LC_CTYPE, then LANG, and an unset + * or empty locale means the C locale -- so require an explicit UTF-8 codeset + * rather than assuming one. Declining costs only the specialized diff; the + * command still renders as an ordinary bash approval. + * + * Exported for testing. + */ +export function sedLocaleCountsCharacters( + env: Record = process.env, +): boolean { + const locale = env.LC_ALL || env.LC_CTYPE || env.LANG || '' + // Either a codeset suffix ("en_US.UTF-8", "de_DE.utf8@euro") or the bare + // codeset, which is what macOS sets for LC_CTYPE. + return /(^|[.@])utf-?8($|@)/i.test(locale) +} + /** * Apply a sed substitution to file content * Returns the new content after applying the substitution From 0ee98be2ff39e923ef4060000243c1a5cc2c927f Mon Sep 17 00:00:00 2001 From: 0xfandom Date: Wed, 5 Aug 2026 16:42:11 +0530 Subject: [PATCH 10/12] fix(bash): decline a repeated s/// g flag the way sed does GNU sed rejects a duplicated flag ("multiple `g' options to `s' command"), so tighten the accepted-flags pattern from /^g*$/ to /^g?$/. A preview that rendered `s/a/X/gg` as a successful global rewrite would diverge from the command sed refuses to run. --- src/tools/BashTool/sedEditParser.test.ts | 5 +++++ src/tools/BashTool/sedEditParser.ts | 5 ++++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/tools/BashTool/sedEditParser.test.ts b/src/tools/BashTool/sedEditParser.test.ts index f342a57c04..08d54fdc21 100644 --- a/src/tools/BashTool/sedEditParser.test.ts +++ b/src/tools/BashTool/sedEditParser.test.ts @@ -226,6 +226,11 @@ describe('declines to simulate what it cannot reproduce faithfully', () => { // `p` prints and `m`/`M` redefine ^ and $ inside the pattern space. expect(parseSedEditCommand(cmd('s/a/X/p'))).toBeNull() expect(parseSedEditCommand(cmd('s/a/X/m'))).toBeNull() + // A repeated `g` is not a global rewrite: GNU sed rejects it outright, so a + // preview of a successful edit would diverge from the failing command. + expect(parseSedEditCommand(cmd('s/a/X/gg'))).toBeNull() + // A single `g` still parses. + expect(parseSedEditCommand(cmd('s/a/X/g'))).not.toBeNull() }) test('rejects replacements carrying sed-specific syntax', () => { diff --git a/src/tools/BashTool/sedEditParser.ts b/src/tools/BashTool/sedEditParser.ts index 182a3a96e1..829162b8f6 100644 --- a/src/tools/BashTool/sedEditParser.ts +++ b/src/tools/BashTool/sedEditParser.ts @@ -459,7 +459,10 @@ export function parseSedEditCommand(command: string): SedEditInfo | null { // rather than sed's locale-based matching. `s/k/X/I` would then rewrite a // Kelvin sign, which GNU sed under C.UTF-8 leaves alone. Until that folding // can be modeled, an approved preview would not be the edit sed performs. - const validFlags = /^g*$/ + // At most one `g`: GNU sed rejects a repeated flag ("multiple `g' options to + // `s' command"), so a `gg` that this module previewed as a successful global + // rewrite would diverge from the command sed actually refuses to run. + const validFlags = /^g?$/ if (!validFlags.test(flags)) { return null } From 6ed2ddafb9690f01fe74b2d0540059aa844bd624 Mon Sep 17 00:00:00 2001 From: 0xfandom Date: Thu, 6 Aug 2026 17:28:04 +0530 Subject: [PATCH 11/12] test(bash): scope the sed preview test to the CRLF-normalized gate The permission path normalizes CRLF to LF before calling applySedSubstitution, so the approved preview never sees a raw carriage return. Replace the raw-\r\n assertion (which claimed a fidelity the gate does not exercise) with one over LF content, and document that raw-CR bytes are out of scope. --- src/tools/BashTool/sedEditParser.test.ts | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/tools/BashTool/sedEditParser.test.ts b/src/tools/BashTool/sedEditParser.test.ts index 08d54fdc21..d4978975a9 100644 --- a/src/tools/BashTool/sedEditParser.test.ts +++ b/src/tools/BashTool/sedEditParser.test.ts @@ -281,13 +281,17 @@ describe('declines to simulate what it cannot reproduce faithfully', () => { expect(applySedSubstitution('\u{1F600}a', info!)).toBe('X') }) - test('matches a carriage return the way sed pattern space does', () => { - // Verified against GNU sed 4.10: `s/./X/g` on "a\r\n" writes "XX\n" — the - // CR is an ordinary character in the pattern space. A JS `.` excludes - // carriage returns and would leave it in place. + test('substitutes every character on a line, matching sed under LF content', () => { + // The permission path (SedEditPermissionRequest) normalizes CRLF to LF + // before it ever calls the simulator, so the preview it approves only ever + // sees `\n`-terminated lines. On that normalized content `s/./X/g` rewrites + // each character exactly as GNU sed does. Raw-CR fidelity is deliberately + // out of scope: the simulator never receives a `\r`, so this asserts the + // behavior the approval gate actually exercises rather than a raw-byte case + // production cannot reach. const info = parseSedEditCommand(cmd('s/./X/g')) expect(info).not.toBeNull() - expect(applySedSubstitution('a\r\n', info!)).toBe('XX\n') + expect(applySedSubstitution('ab', info!)).toBe('XX') }) test('rejects a standalone empty pattern', () => { From 80f81993d4a32cf927cefd6ed168f1c60a5abae9 Mon Sep 17 00:00:00 2001 From: 0xfandom Date: Fri, 7 Aug 2026 14:56:27 +0530 Subject: [PATCH 12/12] fix(bash): decline ERE (?...) groups sed does not implement ereHasUnfaithfulConstructs screened escapes, alternation, anchors, intervals and bracket bodies but treated grouping as implicitly safe. POSIX/GNU sed -E only supports plain capturing (...); a (? opens JavaScript-only syntax -- (?:), lookaround, named groups -- that new RegExp compiles but GNU sed rejects. The preview would render a concrete edit for a command sed refuses to run, so decline as soon as an unescaped ( is followed by ?. --- src/tools/BashTool/sedEditParser.test.ts | 23 +++++++++++++++++++++++ src/tools/BashTool/sedEditParser.ts | 9 +++++++++ 2 files changed, 32 insertions(+) diff --git a/src/tools/BashTool/sedEditParser.test.ts b/src/tools/BashTool/sedEditParser.test.ts index d4978975a9..95523d37d5 100644 --- a/src/tools/BashTool/sedEditParser.test.ts +++ b/src/tools/BashTool/sedEditParser.test.ts @@ -218,6 +218,29 @@ describe('declines to simulate what it cannot reproduce faithfully', () => { ).toBeNull() }) + test('rejects ERE (?...) group extensions GNU sed does not implement', () => { + // `sed -E` supports only plain capturing `(...)`. `(?` opens JS-only syntax + // -- non-capturing, lookaround, named groups -- that JavaScript compiles + // but GNU sed rejects, so a preview would edit a file the real command + // leaves untouched. Every `(?` form must decline. + for (const pattern of [ + '(?:a)b', + '(?=a)', + '(?!a)b', + '(?<=a)b', + '(?a)', + ]) { + expect( + parseSedEditCommand(`sed -i '' -E 's/${pattern}/X/' example.txt`), + ).toBeNull() + } + // A plain capturing group is faithful and still parses. + expect( + parseSedEditCommand("sed -i '' -E 's/(a)b/X/' example.txt"), + ).not.toBeNull() + }) + test('rejects numeric occurrence flags it does not model', () => { // `2` selects the second match on each line, but the simulator always // rewrites the first: sed turns "aaaa" into "aaX", a preview into "Xaa". diff --git a/src/tools/BashTool/sedEditParser.ts b/src/tools/BashTool/sedEditParser.ts index 829162b8f6..75fed8bc11 100644 --- a/src/tools/BashTool/sedEditParser.ts +++ b/src/tools/BashTool/sedEditParser.ts @@ -532,6 +532,15 @@ function ereHasUnfaithfulConstructs(pattern: string): boolean { if (char === '|') return true if (char === '^' && i !== 0) return true if (char === '$' && i !== pattern.length - 1) return true + if (char === '(' && pattern[i + 1] === '?') { + // POSIX/GNU `sed -E` supports only plain capturing groups `(...)`. A `(?` + // opens JavaScript-only syntax -- non-capturing `(?:)`, lookaround + // `(?=)`/`(?!)`/`(?<=)`/`(?)` groups -- which JS + // compiles but GNU sed rejects ("Invalid preceding regular expression"). + // The simulator would render a concrete edit for a command sed refuses to + // run, so decline. + return true + } if (char === '{') { // JS reads an illegal interval body as literal braces, but GNU sed either // errors or applies its own extension: `sed -E 's/a{,3}/X/g'` rewrites