main: bypass the command-type cache when PATH_DIRS is removed for sudo

Per Copilot review on #987: _zsh_highlight_main__type's cache is keyed
on the command name alone, so classifying a name once under PATH_DIRS
(e.g. as a plain top-level command) would silently poison later
lookups of that same name after sudo -- and vice versa -- regardless
of the PATH_DIRS removal, since the cache lookup happens before
$options_to_set is even consulted.

Give _zsh_highlight_main__type an explicit no_cache parameter, and
add _zsh_highlight_main__type_maybe_no_pathdirs(), a small wrapper
that only shadows $options_to_set (and only bypasses the cache) when
PATH_DIRS was actually present to remove -- so a user without
PATH_DIRS set sees this codepath do nothing at all. Both call sites
in the main word-classification block now go through this wrapper
instead of duplicating the shadowing logic inline.

Strengthened sudo-path_dirs.zsh to classify the same name twice in
one buffer (plain, then sudo-prefixed) specifically to exercise this
cache interaction, not just the PATH_DIRS removal in isolation.

Verified via WSL zsh 5.9: make test passes (same 28 pre-existing
TODO failures, zero new), and the strengthened test fails as expected
(observes "command" instead of "unknown-token" on the sudo-prefixed
occurrence) when run against the pre-fix highlighter.
This commit is contained in:
Manish Tiwari 2026-08-25 09:57:05 +05:30
parent 71b1101166
commit 3d15f057c1
2 changed files with 50 additions and 21 deletions

View File

@ -158,6 +158,12 @@ _zsh_highlight_main_calculate_fallback() {
# The result will be stored in REPLY.
_zsh_highlight_main__type() {
integer -r aliases_allowed=${2-1}
# $3: if non-zero, bypass the cache entirely (neither read nor write it).
# Needed by callers that vary $options_to_set per call (see
# _zsh_highlight_main__type_maybe_no_pathdirs below) -- the cache is keyed
# on the command name alone, so without this a result computed under one
# $options_to_set could be served back under a different one.
integer -r no_cache=${3-0}
# We won't cache replies of anything that exists as an alias at all, to
# ensure the cached value is correct regardless of $aliases_allowed.
#
@ -166,7 +172,7 @@ _zsh_highlight_main__type() {
integer may_cache=1
# Cache lookup
if (( $+_zsh_highlight_main__command_type_cache )); then
if (( ! no_cache )) && (( $+_zsh_highlight_main__command_type_cache )); then
REPLY=$_zsh_highlight_main__command_type_cache[(e)$1]
if [[ -n "$REPLY" ]]; then
return
@ -231,13 +237,41 @@ _zsh_highlight_main__type() {
fi
# Cache population
if (( may_cache )) && (( $+_zsh_highlight_main__command_type_cache )); then
if (( ! no_cache )) && (( may_cache )) && (( $+_zsh_highlight_main__command_type_cache )); then
_zsh_highlight_main__command_type_cache[(e)$1]=$REPLY
fi
[[ -n $REPLY ]]
return $?
}
# Wrapper around _zsh_highlight_main__type() for a word that may be the
# target of a recognised precommand (sudo, env, nice, ...; see :sudo_opt: in
# the main loop below). Those precommands spawn their target via
# execvp(3)-style lookup, which (unlike the shell's own PATH_DIRS option)
# never searches $path for a name containing a slash -- so PATH_DIRS must
# not be honoured while classifying such a word, or e.g. "sudo foo/bar" gets
# highlighted as a valid command when sudo itself would fail to find it
# (issue #595).
#
# When PATH_DIRS is actually removed for this call, also bypasses the
# command-type cache: it's keyed on the command name alone, so a result
# computed with PATH_DIRS off must not be read back (or written) as if it
# applied unconditionally -- that would either poison a later plain-command
# lookup of the same name, or (going the other way) let an earlier
# plain-command lookup's cached "command" leak into this precommand-target
# classification, defeating the PATH_DIRS removal above entirely.
_zsh_highlight_main__type_maybe_no_pathdirs() {
integer no_cache=0
if [[ $this_word == *':sudo_opt:'* ]]; then
local -a filtered=( ${options_to_set:#PATH_DIRS} )
if (( $#filtered != $#options_to_set )); then
local -a options_to_set=( $filtered )
no_cache=1
fi
fi
_zsh_highlight_main__type "$1" "$2" $no_cache
}
# Checks whether $1 is something that can be run.
#
# Return 0 if runnable, 1 if not runnable, 2 if trouble.
@ -706,18 +740,7 @@ _zsh_highlight_main_highlighter_highlight_list()
if [[ $this_word == *':start:'* ]] && ! (( in_redirection )); then
# Expand aliases.
# An alias is ineligible for expansion while it's being expanded (see #652/#653).
() {
# :sudo_opt: marks every word from a recognised precommand (sudo,
# env, nice, ...) up to and including its actual command word. Those
# precommands spawn their target via execvp(3)-style lookup, which
# (unlike the shell's own PATH_DIRS option) never searches $path for
# a name containing a slash -- so PATH_DIRS must not be honoured
# while classifying this word, or e.g. "sudo foo/bar" gets
# highlighted as a valid command when sudo itself would fail to find
# it (issue #595).
[[ $this_word == *':sudo_opt:'* ]] && local -a options_to_set=( ${options_to_set:#PATH_DIRS} )
_zsh_highlight_main__type "$arg" "$(( ! ${+seen_alias[$arg]} ))"
}
_zsh_highlight_main__type_maybe_no_pathdirs "$arg" "$(( ! ${+seen_alias[$arg]} ))"
local res="$REPLY"
if [[ $res == "alias" ]]; then
# Mark insane aliases as unknown-token (cf. #263).
@ -748,10 +771,7 @@ _zsh_highlight_main_highlighter_highlight_list()
continue
else
_zsh_highlight_main_highlighter_expand_path $arg
() {
[[ $this_word == *':sudo_opt:'* ]] && local -a options_to_set=( ${options_to_set:#PATH_DIRS} )
_zsh_highlight_main__type "$REPLY" 0
}
_zsh_highlight_main__type_maybe_no_pathdirs "$REPLY" 0
res="$REPLY"
fi
fi

View File

@ -32,6 +32,13 @@
# PATH_DIRS option (see option-path_dirs.zsh). So even though PATH_DIRS makes
# 'bar/testing-issue-228' resolve to a real command when run directly, the
# same word after 'sudo' should not, since sudo itself wouldn't find it (#595).
#
# The two occurrences in one buffer are deliberate: _zsh_highlight_main__type's
# command-type cache is keyed on the command name alone and persists for the
# whole buffer (reset only on precmd), so this also guards against the first,
# plain-command occurrence (correctly classified "command" under PATH_DIRS)
# poisoning the cache and leaking that result into the second, sudo-prefixed
# occurrence of the exact same name.
if [[ $OSTYPE == msys ]]; then
skip_test='Cannot chmod +x in msys2'
else
@ -41,10 +48,12 @@ else
chmod +x foo/bar/testing-issue-228
path+=( "$PWD"/foo )
BUFFER='sudo bar/testing-issue-228'
BUFFER='bar/testing-issue-228; sudo bar/testing-issue-228'
expected_region_highlight=(
"1 4 precommand" # sudo
"6 26 unknown-token" # bar/testing-issue-228 -- not found via sudo's own lookup
"1 21 command" # bar/testing-issue-228 (plain, PATH_DIRS applies)
"22 22 commandseparator" # ;
"24 27 precommand" # sudo
"29 49 unknown-token" # bar/testing-issue-228 -- not found via sudo's own lookup, even though the name was just cached as "command" above
)
fi