From 71b1101166d69025fe5654eb8230dd49574f1c91 Mon Sep 17 00:00:00 2001 From: Manish Tiwari Date: Mon, 24 Aug 2026 14:51:48 +0530 Subject: [PATCH 1/2] main: don't honour PATH_DIRS for a precommand's target command sudo(8) (and env, nice, and other entries in \$precommand_options) resolve their target command via execvp(3)-style lookup, which never searches \$path for a name containing a slash -- unlike the shell's own PATH_DIRS option. So "sudo foo/bar" was highlighted as a valid command whenever the user had PATH_DIRS set and some \$path element made "foo/bar" resolvable that way, even though sudo itself would fail to find it. :sudo_opt: already marks every word from a recognised precommand up to and including its actual command word (for any precommand in \$precommand_options, not just sudo). Key off that existing marker to shadow PATH_DIRS out of \$options_to_set for just those two _zsh_highlight_main__type calls, via an anonymous-function scope so the shadow reverts automatically and normal command-word classification elsewhere in the line is unaffected. Fixes #595. --- highlighters/main/main-highlighter.zsh | 18 ++++++- .../main/test-data/sudo-path_dirs.zsh | 50 +++++++++++++++++++ 2 files changed, 66 insertions(+), 2 deletions(-) create mode 100644 highlighters/main/test-data/sudo-path_dirs.zsh diff --git a/highlighters/main/main-highlighter.zsh b/highlighters/main/main-highlighter.zsh index 061e2a4..a76e64f 100644 --- a/highlighters/main/main-highlighter.zsh +++ b/highlighters/main/main-highlighter.zsh @@ -706,7 +706,18 @@ _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). - _zsh_highlight_main__type "$arg" "$(( ! ${+seen_alias[$arg]} ))" + () { + # :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]} ))" + } local res="$REPLY" if [[ $res == "alias" ]]; then # Mark insane aliases as unknown-token (cf. #263). @@ -737,7 +748,10 @@ _zsh_highlight_main_highlighter_highlight_list() continue else _zsh_highlight_main_highlighter_expand_path $arg - _zsh_highlight_main__type "$REPLY" 0 + () { + [[ $this_word == *':sudo_opt:'* ]] && local -a options_to_set=( ${options_to_set:#PATH_DIRS} ) + _zsh_highlight_main__type "$REPLY" 0 + } res="$REPLY" fi fi diff --git a/highlighters/main/test-data/sudo-path_dirs.zsh b/highlighters/main/test-data/sudo-path_dirs.zsh new file mode 100644 index 0000000..598bfae --- /dev/null +++ b/highlighters/main/test-data/sudo-path_dirs.zsh @@ -0,0 +1,50 @@ +# ------------------------------------------------------------------------------------------------- +# Copyright (c) 2015 zsh-syntax-highlighting contributors +# All rights reserved. +# +# Redistribution and use in source and binary forms, with or without modification, are permitted +# provided that the following conditions are met: +# +# * Redistributions of source code must retain the above copyright notice, this list of conditions +# and the following disclaimer. +# * Redistributions in binary form must reproduce the above copyright notice, this list of +# conditions and the following disclaimer in the documentation and/or other materials provided +# with the distribution. +# * Neither the name of the zsh-syntax-highlighting contributors nor the names of its contributors +# may be used to endorse or promote products derived from this software without specific prior +# written permission. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND ANY EXPRESS OR +# IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND +# FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR +# CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL +# DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER +# IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT +# OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +# ------------------------------------------------------------------------------------------------- +# -*- mode: zsh; sh-indentation: 2; indent-tabs-mode: nil; sh-basic-offset: 2; -*- +# vim: ft=zsh sw=2 ts=2 et +# ------------------------------------------------------------------------------------------------- + +# sudo(8) resolves its target command via execvp(3)-style lookup, which never +# searches $path for a name containing a slash -- unlike the shell's own +# 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). +if [[ $OSTYPE == msys ]]; then + skip_test='Cannot chmod +x in msys2' +else + setopt PATH_DIRS + mkdir -p foo/bar + touch foo/bar/testing-issue-228 + chmod +x foo/bar/testing-issue-228 + path+=( "$PWD"/foo ) + + BUFFER='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 + ) +fi From 3d15f057c1626b71c65c5619eb7509c02172bd24 Mon Sep 17 00:00:00 2001 From: Manish Tiwari Date: Tue, 25 Aug 2026 09:57:05 +0530 Subject: [PATCH 2/2] 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. --- highlighters/main/main-highlighter.zsh | 56 +++++++++++++------ .../main/test-data/sudo-path_dirs.zsh | 15 ++++- 2 files changed, 50 insertions(+), 21 deletions(-) diff --git a/highlighters/main/main-highlighter.zsh b/highlighters/main/main-highlighter.zsh index a76e64f..599f9c2 100644 --- a/highlighters/main/main-highlighter.zsh +++ b/highlighters/main/main-highlighter.zsh @@ -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 diff --git a/highlighters/main/test-data/sudo-path_dirs.zsh b/highlighters/main/test-data/sudo-path_dirs.zsh index 598bfae..d8b2fcc 100644 --- a/highlighters/main/test-data/sudo-path_dirs.zsh +++ b/highlighters/main/test-data/sudo-path_dirs.zsh @@ -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