feat(tests): add shadow-classification lint; fix real cp/mv/less bugs
New Phase 1b in tests/run-tests.fish: catches a bare C1-shadowed-command call in a functions/*.fish body with no matching uses-shadow(name) or self-limiting(name) in that function's own CLASSIFICATION header. This is exactly the check discussed after the rm and cd audits -- runtime auto-unwrapping isn't viable in fish (there's no hook finer than shadowing itself, and rewriting behavior invisibly at runtime is its own footgun); a static lint using the CLASSIFICATION tag as the declared-intentional marker is. Scoped to functions/*.fish only: the one-function-per-file convention there makes body extraction exact with no block-depth parser needed. Added a new self-limiting(name) tag to the schema for the case a bare call is safe not because the caller did anything, but because the shadow's own logic already neutralizes the override: rm's and mkdir's flag checks (verified precisely -- rm falls back to command rm for any flag except a bare -r/-R/--recursive alone, which still routes to trash; mkdir falls back to command mkdir -p for any flag, no exception), and grep/fgrep/egrep/dir/vdir/cat's own tty auto-detection (--color=auto, and bat's default color behavior -- verified byte-identical to stock cat when piped, since bat also auto-disables highlighting on a non-terminal). Explicit and durable rather than a silent lint exemption: if a shadow's bypass condition is ever weakened, every self-limiting site is one grep away instead of silently wrong. Running the first draft of the lint surfaced three more real bugs, none previously audited: - config-help.fish's --man pager path checks `type -q less` (proving it wants the real less binary specifically, for less-only -R/+N flag syntax) then called it bare, routing through our own $PAGER -> ov -> less -> more -> cat fallback chain instead -- which could hand those less-specific flags to a completely different program. Now command less. - _fish_deps_install.fish and _fish_deps_update.fish's binary-upgrade paths cp a freshly downloaded binary over an already-installed one with no existence guard -- the update flow's target is guaranteed to already exist. Our cp shadow forces -i unconditionally (a plain alias, not flag-aware like rm's), so this would hang waiting on a confirmation prompt in any non-interactive run. Now command cp. Same two files' lazydocker install path piped curl output into bare bash, invoking our shell-switch wrapper instead of a plain subshell. Now command bash. - agents-init.fish's AGENTS.md/CLAUDE.md relocation calls mv bare in four places; each is already guarded by a preceding test -f check on the destination, so the -i alias was unlikely to ever fire in practice, but explicit command mv removes the reliance on that guard entirely rather than leaving it as the only thing standing between a file move and an unattended hang. The remaining ~65 flagged call sites across ~24 files were reviewed individually and tagged self-limiting(rm)/self-limiting(mkdir) (verified flagged with -f/-rf or -p) and self-limiting(grep)/ self-limiting(cat) (verified piped, captured, or -q/-c; none display color to a human), plus uses-shadow(ls) for two existence-check-only calls (cffetch.fish, ffetch.fish) whose output is redirected to /dev/null.
This commit is contained in:
@@ -53,6 +53,89 @@ if test $syntax_failed -ne 0 -o $indent_failed -ne 0
|
||||
set overall_failed 1
|
||||
end
|
||||
|
||||
# ---- Phase 1b: shadow-classification lint --------------------------------
|
||||
# Catches a bare C1-shadowed-command call in a function body with no
|
||||
# matching uses-shadow(name) or self-limiting(name) in that function's own
|
||||
# CLASSIFICATION header -- the exact bug class fixed across fc.fish,
|
||||
# dng2avif.fish, _scrollback_prune_junk.fish, mkcd.fish, and mkrep.fish. A
|
||||
# bare call is either declared (uses-shadow: wanted; self-limiting: safe
|
||||
# because the shadow's own logic neutralizes it, e.g. rm/mkdir's flag check
|
||||
# or grep/cat's tty-auto-detected color) or it's undocumented at best, a bug
|
||||
# at worst -- the lint never guesses which on its own; see
|
||||
# docs/function-classification-schema.md for the full tag definitions and
|
||||
# why the reasoning belongs in a tag, not in this script.
|
||||
#
|
||||
# Scoped to functions/*.fish only: the one-function-per-file convention
|
||||
# there makes "everything after the function line is its body" exact, with
|
||||
# no block-depth parser needed. conf.d/*.fish can define several functions
|
||||
# in one file and isn't covered -- see docs/function-classification-schema.md.
|
||||
echo
|
||||
echo "== Shadow-classification lint =="
|
||||
|
||||
# help and edit are deliberately excluded: help's real bypass is
|
||||
# __original_help (not command/builtin), and edit has no backing binary at
|
||||
# all to bypass to -- see docs/manual/08-components-reference/01-c1-command-shadows.md.
|
||||
set -l shadow_names ls cat cd rm less du top ping ssh rg mkdir bash cp mv wget grep fgrep egrep dir vdir claude
|
||||
|
||||
set -l class_checked 0
|
||||
set -l class_files_failed 0
|
||||
set -l class_issues 0
|
||||
|
||||
for f in $repo_root/functions/*.fish
|
||||
set -l lines (cat $f)
|
||||
|
||||
# Find the function line; everything before it is header, everything
|
||||
# from it onward is body (one function per file).
|
||||
set -l func_idx 0
|
||||
for i in (seq (count $lines))
|
||||
if string match -qr '^function ' -- $lines[$i]
|
||||
set func_idx $i
|
||||
break
|
||||
end
|
||||
end
|
||||
test $func_idx -eq 0; and continue
|
||||
set class_checked (math $class_checked + 1)
|
||||
|
||||
# Pull uses-shadow(...) and self-limiting(...) names from the
|
||||
# CLASSIFICATION tag line, if any -- either one accounts for a bare call.
|
||||
set -l declared
|
||||
for i in (seq (math $func_idx - 1))
|
||||
if test "$lines[$i]" = "# CLASSIFICATION"; and test $i -lt $func_idx
|
||||
set -l tagline $lines[(math $i + 1)]
|
||||
for tag in uses-shadow self-limiting
|
||||
set -l m (string match -r "$tag"'\(([^)]*)\)' -- $tagline)
|
||||
test -n "$m[2]"; and set -a declared (string trim -- (string split ',' -- $m[2]))
|
||||
end
|
||||
break
|
||||
end
|
||||
end
|
||||
|
||||
set -l file_failed 0
|
||||
for i in (seq $func_idx (count $lines))
|
||||
set -l line $lines[$i]
|
||||
# Strip quoted spans and comments so string literals (error
|
||||
# messages, --description text) never masquerade as a call.
|
||||
set -l stripped (string replace -ra '"[^"]*"' '' -- $line)
|
||||
set stripped (string replace -ra "'[^']*'" '' -- $stripped)
|
||||
set stripped (string replace -r '#.*$' '' -- $stripped)
|
||||
|
||||
for name in $shadow_names
|
||||
if string match -qr '(^|[;|(]|\band\b|\bor\b|\bnot\b|\bif\b|\bwhile\b|\bbegin\b)\s*'"$name"'(\s|$)' -- $stripped
|
||||
if not contains -- $name $declared
|
||||
echo " FAIL (shadow) "(string replace $repo_root/ '' $f)": line $i calls bare '$name' with no uses-shadow($name)/self-limiting($name)"
|
||||
set class_issues (math $class_issues + 1)
|
||||
set file_failed 1
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
test $file_failed -eq 1; and set class_files_failed (math $class_files_failed + 1)
|
||||
end
|
||||
echo (math $class_checked - $class_files_failed)"/$class_checked functions passed shadow-classification check"
|
||||
if test $class_issues -ne 0
|
||||
set overall_failed 1
|
||||
end
|
||||
|
||||
# ---- Phase 2: discover suites --------------------------------------------
|
||||
# Mode is declared by the suite, not by this driver. Detection is
|
||||
# case-insensitive so a near-miss like "# Mode: in-session" is caught rather
|
||||
|
||||
Reference in New Issue
Block a user