From 3414f81cb63d588de60685ad64eae06bb6cbcfbb Mon Sep 17 00:00:00 2001 From: Rootiest Date: Mon, 21 Sep 2026 21:26:55 -0400 Subject: [PATCH] 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. --- docs/function-classification-schema.md | 20 +++++ functions/__fish_config_sync_logging.fish | 3 + functions/__fish_real_command.fish | 3 + functions/__fish_user_dots_link.fish | 2 +- functions/_agents_init_ensure_gitignore.fish | 3 + functions/_agents_repo_ensure_symlink.fish | 3 + functions/_fish_deps_install.fish | 8 +- functions/_fish_deps_marktext_appimage.fish | 4 +- functions/_fish_deps_update.fish | 8 +- functions/_scrollback_prune_junk.fish | 2 +- functions/agents-init.fish | 11 ++- functions/agents-vault.fish | 3 + functions/antigravity-ide.fish | 3 + functions/cffetch.fish | 3 + functions/cleanup.fish | 3 + functions/config-help.fish | 5 +- functions/dng2avif.fish | 3 + functions/fc.fish | 2 +- functions/ffetch.fish | 3 + functions/gi.fish | 2 +- functions/logs.fish | 2 +- functions/mkrep.fish | 2 +- functions/qr.fish | 2 +- functions/sbver.fish | 3 + functions/smart_exit.fish | 2 +- tests/run-tests.fish | 83 ++++++++++++++++++++ 26 files changed, 165 insertions(+), 23 deletions(-) diff --git a/docs/function-classification-schema.md b/docs/function-classification-schema.md index 5bf75a1..9631b44 100644 --- a/docs/function-classification-schema.md +++ b/docs/function-classification-schema.md @@ -35,6 +35,26 @@ it empty as a placeholder. break this function's logic: timestamps leaking into a parsed capture, `-i` prompting on a path meant to run unattended, structural output changes breaking a `string`/`sed` parse, etc. +- **`self-limiting(name[,name...])`** — calls a shadowed command bare, and + it's safe not because the caller did anything but because *the shadow's + own logic* already neutralizes the override for this call. Verify the + actual condition per shadow, it's not the same check for each one: + - `rm` falls back to `command rm` for any flag **except** a bare `-r`, + `-R`, or `--recursive` (those still route to `trash put`) — so + `rm -f`/`rm -rf` qualify, but `rm -r $dir` alone does not. + - `mkdir` falls back to `command mkdir -p` for *any* flag at all, no + exception. + - `--color=auto`/`bat`'s own tty auto-detection (`grep`, `fgrep`, + `egrep`, `dir`, `vdir`, `cat` — verified byte-identical to stock when + piped or captured, since none of these force color on a + non-terminal). + + Document it explicitly rather than leaving the bare call untagged: if a + shadow's bypass condition is ever weakened, narrowed, or removed, every + `self-limiting` site is one grep away instead of silently wrong. + Don't use this for `ls` — eza's long-format/icon layout is structural, + not tty-gated, so it stays different from stock `ls` even piped; a + bare `ls` call still needs `uses-shadow(ls)` or a real bypass. - **`destructive`** — can irreversibly delete or overwrite data: `rm -f`, `rm -rf`, truncating or force-overwriting a file, `git push --force`. Routine cleanup of the function's own `$tmpdir`/`$_tmpdir`/`mktemp` diff --git a/functions/__fish_config_sync_logging.fish b/functions/__fish_config_sync_logging.fish index f515a59..58e1708 100644 --- a/functions/__fish_config_sync_logging.fish +++ b/functions/__fish_config_sync_logging.fish @@ -4,6 +4,9 @@ # COMPONENT # logging/terminal-capture # +# CLASSIFICATION +# self-limiting(rm,mkdir) +# # SYNOPSIS # __fish_config_sync_logging # diff --git a/functions/__fish_real_command.fish b/functions/__fish_real_command.fish index d515c8b..9669ee2 100644 --- a/functions/__fish_real_command.fish +++ b/functions/__fish_real_command.fish @@ -1,6 +1,9 @@ # Copyright (C) 2026 Rootiest # SPDX-License-Identifier: AGPL-3.0-or-later +# CLASSIFICATION +# self-limiting(grep) +# # SYNOPSIS # __fish_real_command # diff --git a/functions/__fish_user_dots_link.fish b/functions/__fish_user_dots_link.fish index d7026b8..848fd87 100644 --- a/functions/__fish_user_dots_link.fish +++ b/functions/__fish_user_dots_link.fish @@ -5,7 +5,7 @@ # autoexec/sync # # CLASSIFICATION -# destructive +# self-limiting(rm), destructive # # SYNOPSIS # __fish_user_dots_link diff --git a/functions/_agents_init_ensure_gitignore.fish b/functions/_agents_init_ensure_gitignore.fish index f1f5549..ca1d737 100644 --- a/functions/_agents_init_ensure_gitignore.fish +++ b/functions/_agents_init_ensure_gitignore.fish @@ -1,6 +1,9 @@ # Copyright (C) 2026 Rootiest # SPDX-License-Identifier: AGPL-3.0-or-later +# CLASSIFICATION +# self-limiting(grep) +# # SYNOPSIS # _agents_init_ensure_gitignore