shell-review: weekly footgun scan

[github-actions]

shell-review: weekly footgun scan — 2026-08-18

Scope

git diff v4.12.2..HEAD — 20 shell files. Baseline assumed green (shellcheck / bash -n / zsh -n / behavioural suite all run in audit-core.sh); nothing below is a lint finding.

---

Risk (likely)

1. bin/clip:31 — the tmux claim doesn't describe the call tmux actually makes

The comment says "Inside tmux this needs no special handling: Core's tmux.conf sets set-clipboard on, so tmux consumes a pane's OSC 52 and forwards it." That is true for a pane process (nvim, optoken, the tmux-cheat popup). It is not true for the one binding that names clip:

tmux/tmux.reset.conf:80  bind -T copy-mode-vi y  send -X copy-pipe-and-cancel "clip"

copy-pipe runs the command through tmux's job_run() — a child of the daemonized server, which has called setsid() and has no controlling terminal, and whose stderr is dup2'd to /dev/null. So bin/clip:62's open of /dev/tty fails with ENXIO, the error at line 63 goes nowhere, and clip exits 1 silently. The clip OSC 52 branch is unreachable from the yank binding.

Prefix-y still copies today only because window_copy_copy_pipe() calls window_copy_copy_buffer(), which under set-clipboard on emits its own OSC 52 to the client. Two consequences:

• The comment credits the wrong mechanism, so the next person to touch set-clipboard (→ off, or a tmux built without it) silently loses copy-out on exactly the headless box this change was written for, with an in-file comment saying it's covered.
• clip's fallback is dead code on that path — a real backend absence there produces a silent exit 1.

Trigger: headless ssh box, no wl-copy/xclip, set-clipboard off (or clip invoked from any run-shell/copy-pipe hook) → yank copies nothing, no message anywhere.

Fix (one line): in _osc52_copy, when the tty write fails and $TMUX is set, fall through to tmux load-buffer -w - — which reaches the client from the server side and needs no tty — and correct the comment to say tmux's own set-clipboard is what covers copy-pipe.

2. zsh/50-op.zsh:49 — clip can now exit 0 without anything reaching a clipboard

_osc52_copy returns success as soon as the escape is written, not when a terminal accepts it. OSC 52 clipboard writes are refused by default in several emulators (xterm's disallowedWindowOps, and unimplemented in others), and the failure mode is a silent drop — the file's own comment says so for the >100 KB case, but the same is true for "this terminal just ignores OSC 52".

So on a headless box printf '%s' "$otp" | clip && _core_ok "TOTP copied to clipboard" now prints a green success for an empty clipboard. Before this change it printed the error. Same overclaim in tmux/scripts/tmux-cheat.sh:150 (copy() → "copy to clipboard").

Secondary, on the same path: with set-clipboard on, tmux accepts the pane's OSC 52 and creates a tmux paste buffer. The TOTP is then readable via tmux show-buffer by anything that can reach the tmux socket — a store 50-op.zsh:43's rationale ("so it never lands in your shell history/scrollback") doesn't cover.

Fix: soften the two success messages on the OSC 52 path (e.g. clip exports/echoes which backend won, or _core_ok becomes "sent to the terminal's clipboard (OSC 52)"), and mention the tmux-buffer copy in 50-op.zsh's note.

3. bin/clip:62 — the OSC 52 write is not a single write() for large payloads

printf … > "$tty" with $tty a character device is line-buffered, and the sequence contains no newline by construction. Anything past BUFSIZ (~4 KB of base64 ≈ 3 KB of copied text) therefore leaves the shell as several write() calls on a tty that the foreground TUI also owns.

clip is nvim's "+y provider (nvim/lua/gerrrt/config/clipboard.lua:33). If nvim redraws between two of those writes, the OSC 52 string is split — the terminal (or tmux's parser) sees a truncated sequence and copies a partial value, and the injected chunk can corrupt the screen. Likelihood is low in practice because nvim blocks on the provider job, but the window is real and grows with yank size.

Fix: avoid the interleaving window entirely — assemble the sequence in one variable and hand it to a single unbuffered write (printf '%s' "$seq" | dd of="$tty" bs=… 2>/dev/null), or accept it and say so in the comment next to the existing 100 KB warning.

---

Nits (documentation coherence)

• scripts/lib/common.sh:134 — bumped to "the 10-repo fan-out" while the same sweep changed every other count in the diff (audit-core.sh, bench-core.sh, ci-classify.sh, nvim-reachability.sh, release.sh, update-plugins.sh, verify-atuin-guard.sh, test-core.sh) to nine. scripts/os-repos.txt has nine entries. The +1 convention may be deliberate (fleet + Core) but it now reads as the one stale number in the set.
• zsh/30-functions.zsh:289 — the render's resolved block gained a git exec-path → … line (line 452) but _core_doctor_json's "resolved" object still carries only fd/bat. The two "resolved" views now disagree, and the comment at line 447 notes the parity test is blind to it by construction — so nothing will catch further drift. A "git_exec_path" key would keep them symmetric.

---

Clean

Read for the footgun classes, nothing found:

• zsh/00-tools.zsh — the $commands/exec-path block is genuinely fork-free (parameter lookup + ≤2 [[ -x ]] stats, brace expansion over the quoted prefix is correct, _gx unset at file scope, ${(t)…} export test short-circuits behind the -n guard). No CORE_BENCH_BUDGET_MS exposure. One comment overclaim only: "a non-Debian shell pays nothing" holds when git-absorb is on $PATH, not when it's simply absent — which is most boxes.
• zsh/30-functions.zsh — _core_git_exec_path's typeset -g survives _core_doctor_render's local … REPLY frame; the cache genuinely re-derives per report (core-doctor unsets on entry, TTY render is inside $(…) and prints its exec-path line after the tool loop that populates it); REPLY doubling as scratch in the git-* arm is safe because $1 outlives the nested call; _core_have on an absolute path is honest about the exec bit.
• bin/clip-paste — the asymmetry is correct and the DISPLAY guards are a strict improvement (they replace an exec xclip that was doomed to fail with a message that explains why).
• bin/clip detection ladder — the DISPLAY guards make the fallback reachable without breaking X11-over-ssh (which sets DISPLAY); base64 | tr -d '\n' avoids the BSD -w0 trap; the attempt-don't-probe rationale for /dev/tty is right ([[ -w ]] passes under setsid); pipefail interaction with the || { … return 1; } guard is correct.
• scripts/tag-release.sh — skip for the advanced-tip notice is consistent with the file's other uses and no summary is inflated; git tag -fa … -m fixes the gpgsign abort and stays compatible with the --atomic + --force-with-lease push below it.
• scripts/sync-core.sh, fleet-drift.sh, core-integrity.sh, os-repos.txt — the four fleet lists agree and test-core.sh:244 now asserts it.
• scripts/test-core.sh — new OSC 52 and exec-path suites are hermetic (unopenable CLIP_TTY default so a stray pass can't come from the runner's real tty; >file not $(…) for the cache-lifetime cases; PATH-first asserted via the stub's call log rather than source spelling).
• scripts/audit-core.sh, bench-core.sh, ci-classify.sh, nvim-reachability.sh, release.sh, update-plugins.sh, verify-atuin-guard.sh, tmux/scripts/tmux-cheat.sh — comment-only in this range.

No edits made. Say the word and I'll fix any of the above here in Core, with the CHANGELOG.md entry and a green make audit.

Filed by the claude-routines workflow. Report-first: review and act — nothing was changed.