fix(terminal): keep percent expressions out of the shim tombstone comments

cmd expands variables inside rem, so a comment naming the working directory
substituted a path into itself. Verified on Windows 11 that rem does not
re-parse the result -- a cwd of 'C:\x&pwned&rem' executed nothing and the real
git still ran -- so this was not exploitable, but rem handles separators
differently inside a parenthesized block and this script now has some. A test
now rejects any percent sign in an emitted rem line.

Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
Neil
2026-08-13 22:05:30 -07:00
co-authored by Orca
parent 8a7c0473ad
commit 41857351fc
2 changed files with 32 additions and 3 deletions
@@ -108,6 +108,27 @@ describe('legacy terminal shim neutralization', () => {
)
})
it('emits no percent expression inside a cmd rem comment', () => {
// Why: cmd expands variables inside rem, so a comment mentioning the working directory
// substitutes it into the comment text. Harmless at top level -- verified on Windows 11 that
// rem does not re-parse the result, so a cwd of `C:\\x&pwned&rem` executed nothing -- but rem
// treats separators differently inside a parenthesized block, and this script now has some.
const userData = makeUserDataDir()
const win32Dir = join(userData, 'orca-terminal-attribution', 'win32')
mkdirSync(win32Dir, { recursive: true })
neutralizeLegacyTerminalShimDir(userData)
for (const command of ['git', 'gh'] as const) {
const offending = readFileSync(join(win32Dir, `${command}.cmd`), 'utf8')
.split('\r\n')
.filter((line) => /^\s*rem\b/i.test(line) && line.includes('%'))
expect(offending, `rem comments with percent expressions: ${offending.join(' | ')}`).toEqual(
[]
)
}
})
it('keeps the Windows wrappers ASCII-only', () => {
// Why: cmd.exe seeks through a batch file in bytes but advances by decoded character count,
// so a single multi-byte character shifts every following line. Two em dashes in comments
@@ -1,6 +1,12 @@
// Why: kept beside the POSIX tombstone so the generated wrapper text for every platform lives
// in one place, and so the shim-dir module stays under the max-lines limit.
//
// Keep percent signs out of every emitted `rem` line. cmd expands variables inside rem, so a
// comment mentioning %CD% substitutes the working directory into itself. That is harmless at top
// level -- verified on Windows 11 that rem does not re-parse the result, so a cwd of
// `C:\x&pwned&rem` did not execute anything -- but rem handles separators differently inside a
// parenthesized block, and this script now has some.
//
// Keep both templates ASCII-only, comments included. cmd.exe tracks its position in a batch file
// in bytes but advances by decoded character count, so every extra UTF-8 byte shifts the whole
// file: two em dashes in comments made cmd drop the first four characters of every line and the
@@ -26,9 +32,11 @@ if defined orca_legacy_wrapper_dir call :orca_normalize_legacy_dir
rem Why: an empty PATH leaves the substitution below with an unbalanced quote, which
rem desynchronizes cmd parsing for the rest of the file. Skip the line entirely instead.
if not defined PATH goto :orca_path_walked
rem Why the variable: CALL re-expands its own command line, so a PATH entry holding a literal
rem %CD% would be turned into the current directory before the rooted check below ever sees it,
rem and the cwd would then be searched for __ORCA_COMMAND__. Proven on Windows 11.
rem Why the variable: CALL re-expands its own command line, so a PATH entry naming the current
rem directory through a percent expression would become that directory before the rooted check
rem below ever saw it, and the cwd would then be searched for __ORCA_COMMAND__. Proven on
rem Windows 11. The expression is spelled out only in the TypeScript comment above: cmd expands
rem percent signs inside rem, so writing one here would substitute a path into the comment.
for %%P in ("%PATH:;=" "%") do (
set "orca_entry=%%~P"
call :orca_append_path