mirror of
https://github.com/stablyai/orca.git
synced 2026-10-24 00:04:01 +00:00
fix(terminal): strip the captured shim dir across trailing-separator spellings
The scrub compared the captured ORCA_ATTRIBUTION_SHIM_DIR to PATH entries literally, so a trailing-separator difference left the legacy shim directory on the spawned PATH. Same class already fixed in the generated wrappers. Also drops a dead default in the POSIX filter, consolidates comments that had accumulated across fixes, and normalizes the legacy directory once rather than per PATH entry in the cmd wrapper. The boundary scanner stays: a shim path can contain the PATH delimiter, which splitting would fragment.
This commit is contained in:
@@ -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"
|
||||
|
||||
@@ -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<string, string> = {
|
||||
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<string, string> = {
|
||||
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<string, string> = {
|
||||
|
||||
@@ -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<string>([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
|
||||
|
||||
@@ -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
|
||||
`
|
||||
|
||||
Reference in New Issue
Block a user