diff --git a/src/main/pty/legacy-terminal-posix-tombstone.ts b/src/main/pty/legacy-terminal-posix-tombstone.ts index abe82ac485e..7c910b82217 100644 --- a/src/main/pty/legacy-terminal-posix-tombstone.ts +++ b/src/main/pty/legacy-terminal-posix-tombstone.ts @@ -73,16 +73,14 @@ const POSIX_TOMBSTONE = String.raw`#!__ORCA_INTERPRETER__ set -u command_name="__ORCA_COMMAND__" -# Why: parameter expansion, not the external dirname — if that were unresolvable the substitution -# would be empty and cd into it succeeds, silently making wrapper_dir the cwd so the wrapper -# fails to exclude itself from PATH. +# Why this shape, rather than dirname: an unresolvable external would leave the substitution empty +# and cd into it succeeds, silently making wrapper_dir the cwd. With no slash the %/* strip yields the +# file name rather than a directory, so that case takes $PWD. CDPATH is cleared because cd searches +# it for a relative operand and echoes where it landed, which the substitution would capture. +# Each of these made self-exclusion miss the wrapper's own directory, so the lookup resolved back +# to this script. wrapper_src="${SHELL_DOLLAR}{BASH_SOURCE[0]}" case "$wrapper_src" in - # Why: with no slash the %/* strip yields the file name, not a directory, so self-exclusion - # would miss the wrapper's own dir and the lookup would resolve back to this script. - # Why: with CDPATH set, cd searches it for a relative operand and echoes the directory it lands - # in, which the command substitution then captures — wrapper_dir ends up wrong and doubled. - # Clear it for this one command. */*) wrapper_dir="$(CDPATH= cd -P -- "${SHELL_DOLLAR}{wrapper_src%/*}" 2>/dev/null && pwd)" ;; *) wrapper_dir="$PWD" ;; esac @@ -99,7 +97,7 @@ filter_path() { local filtered_path="" local separator="" path_entry_kept=0 - local entry normalized candidate has_more + local entry normalized has_more while true; do if [[ "$remaining" == *:* ]]; then entry="${SHELL_DOLLAR}{remaining%%:*}" @@ -113,14 +111,13 @@ filter_path() { while [[ "$normalized" != "/" && "$normalized" == */ ]]; do normalized="${SHELL_DOLLAR}{normalized%/}" done - candidate="${SHELL_DOLLAR}{entry:-.}" if [[ "$entry" != /* ]]; then # Why: an empty or relative PATH element resolves against the current directory, so keeping # it would let a repository-local git/gh win the lookup below. : elif [[ -n "$legacy_target" && "$normalized" == "$legacy_target" ]]; then : - elif [[ "$candidate" -ef "$wrapper_dir" ]]; then + elif [[ "$entry" -ef "$wrapper_dir" ]]; then : else filtered_path+="$separator$entry" diff --git a/src/main/pty/legacy-terminal-shim-dir.test.ts b/src/main/pty/legacy-terminal-shim-dir.test.ts index 47a3b96825b..442dd77a6dc 100644 --- a/src/main/pty/legacy-terminal-shim-dir.test.ts +++ b/src/main/pty/legacy-terminal-shim-dir.test.ts @@ -707,6 +707,25 @@ describe('legacy terminal shim neutralization', () => { expect(env).toEqual({ PATH: '/usr/local/bin:/usr/bin', HOME: '/home/u' }) }) + it('strips the captured shim directory when PATH spells it with a trailing separator', () => { + // Why: the captured value and the PATH entry can differ by a trailing separator; comparing + // them literally left the shim directory on PATH and the wrapper reachable. + const posix: Record = { + PATH: '/custom/elsewhere/:/usr/bin', + ORCA_ATTRIBUTION_SHIM_DIR: '/custom/elsewhere' + } + stripLegacyTerminalShimEnv(posix, 'linux') + expect(posix.PATH).toBe('/usr/bin') + + // And the reverse spelling, plus Windows slash style. + const win: Record = { + Path: 'C:\\Custom\\Else;C:\\Windows', + ORCA_ATTRIBUTION_SHIM_DIR: 'C:\\Custom\\Else\\' + } + stripLegacyTerminalShimEnv(win, 'win32') + expect(win.Path).toBe('C:\\Windows') + }) + it('uses the captured POSIX shim directory literally when it contains a colon', () => { const shimDir = '/tmp/orca:user/orca-terminal-attribution/posix' const env: Record = { diff --git a/src/main/pty/legacy-terminal-shim-dir.ts b/src/main/pty/legacy-terminal-shim-dir.ts index 0d380bcfd65..128d88c1e9c 100644 --- a/src/main/pty/legacy-terminal-shim-dir.ts +++ b/src/main/pty/legacy-terminal-shim-dir.ts @@ -126,6 +126,22 @@ function writeFileAtomically(filePath: string, contents: string, mode: number): } } +// Why: the captured directory and the PATH entry naming it can differ by a trailing separator (or +// its slash style on Windows). A literal compare missed that and left the shim dir on PATH. The +// literal compare itself has to stay: a shim path may contain the PATH delimiter, which splitting +// would fragment. +function pathEntrySpellings(dir: string, windows: boolean): string[] { + const base = dir.replace(/[\\/]+$/, '') + if (!base) { + return [dir] + } + const spellings = new Set([dir, base, `${base}/`]) + if (windows) { + spellings.add(`${base}\\`) + } + return [...spellings] +} + export function isLegacyTerminalShimPathEntry(entry: string): boolean { const normalized = entry.replaceAll('\\', '/').replace(/\/+$/, '').toLowerCase() return ( @@ -164,7 +180,11 @@ export function stripLegacyTerminalShimEnv( continue } const withoutExplicitDirs = explicitShimDirs.reduce( - (pathValue, shimDir) => removeLiteralPathEntry(pathValue, shimDir, delimiter, windows), + (pathValue, shimDir) => + pathEntrySpellings(shimDir, windows).reduce( + (value, spelling) => removeLiteralPathEntry(value, spelling, delimiter, windows), + pathValue + ), current ) const cleaned = withoutExplicitDirs diff --git a/src/main/pty/legacy-terminal-windows-tombstone.ts b/src/main/pty/legacy-terminal-windows-tombstone.ts index 8bff863ec18..d482d1479f2 100644 --- a/src/main/pty/legacy-terminal-windows-tombstone.ts +++ b/src/main/pty/legacy-terminal-windows-tombstone.ts @@ -6,6 +6,10 @@ setlocal set "orca_real=%ORCA_REAL___ORCA_UPPER_COMMAND__%" set "orca_wrapper_dir=%~dp0" set "orca_legacy_wrapper_dir=%ORCA_ATTRIBUTION_SHIM_DIR%" +rem Why: normalize once here rather than per PATH entry; the value cannot change mid-run. +set "orca_legacy_norm=" +if defined orca_legacy_wrapper_dir for %%G in ("%orca_legacy_wrapper_dir%") do set "orca_legacy_norm=%%~fG" +if defined orca_legacy_norm if "%orca_legacy_norm:~-1%"=="%orca_sep%" set "orca_legacy_norm=%orca_legacy_norm:~0,-1%" set "orca_clean_path=" rem Why: holds a single separator so the trailing-separator tests below need neither a literal rem backslash before a quote (which breaks cmd parsing) nor a sentinel character (which would @@ -80,8 +84,6 @@ if defined orca_clean_path (set "orca_clean_path=%orca_clean_path%;%~1") else se exit /b :orca_reject_legacy_dir -for %%G in ("%orca_legacy_wrapper_dir%") do set "orca_legacy_norm=%%~fG" -if "%orca_legacy_norm:~-1%"=="%orca_sep%" set "orca_legacy_norm=%orca_legacy_norm:~0,-1%" if /I "%orca_path_entry_dir%"=="%orca_legacy_norm%\" set "orca_skip_entry=1" exit /b `