Skip to content

fix(complete): skip unreadable files in fish shebang completion scan - #707

Merged
jdx merged 1 commit into
jdx:mainfrom
GrantD-ADSK:fix/fish-completion-init-unreadable-path-entries
Jul 6, 2026
Merged

fix(complete): skip unreadable files in fish shebang completion scan#707
jdx merged 1 commit into
jdx:mainfrom
GrantD-ADSK:fix/fish-completion-init-unreadable-path-entries

Conversation

@GrantD-ADSK

@GrantD-ADSK GrantD-ADSK commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Summary

  • The conf.d-sourced fish init script (usage g completion-init fish) scans every executable on $PATH to detect a usage shebang, peek-reading the first 128 bytes of each file.
  • Files that are executable but not readable by the current user (e.g. macOS's setuid-root sudo, or execute-only visudo) make fish's own redirection layer print a warning on every shell startup, since the existing read ... 2>/dev/null only silences the read builtin's stderr, not fish's own redirection-setup warning.
  • This adds a test -r $file; or continue guard alongside the existing executability check, so unreadable files are skipped before the peek-read is attempted.

Fixes #706

Test plan

  • cargo test -p usage-lib --lib complete::fish passes, snapshot updated via INSTA_UPDATE=always
  • cargo fmt --check -p usage-lib clean
  • Manually verified on macOS: applying the equivalent one-line patch to a locally generated ~/.config/fish/conf.d/usage.fish eliminates the /usr/bin/sudo / /usr/sbin/visudo warnings on new fish shells, with fish g completion-init fish output otherwise unchanged

Summary by CodeRabbit

  • Bug Fixes
    • Improved shell startup behavior by skipping unreadable PATH entries during fish initialization.
    • Reduced warning messages for execute-only or setuid-like binaries, especially in macOS-related command setups.

The conf.d-sourced fish init script scans every executable on $PATH
to detect a `usage` shebang, peek-reading the first 128 bytes of each
file. Files that are executable but not readable by the current user
(e.g. macOS's setuid-root sudo, or execute-only visudo) make fish's
own redirection layer print a warning on every shell startup, since
the read's stderr redirect doesn't cover that failure mode. Skip
files we can't read before attempting the peek-read.
@coderabbitai

coderabbitai Bot commented Jul 5, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Central YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro Plus

Run ID: 1835b689-ed90-4bc5-9207-46b2c38c4130

📥 Commits

Reviewing files that changed from the base of the PR and between 67c20c2 and f170bd3.

⛔ Files ignored due to path filters (1)
  • lib/src/complete/snapshots/usage__complete__fish__tests__complete_fish_init.snap is excluded by !**/*.snap
📒 Files selected for processing (1)
  • lib/src/complete/fish.rs

📝 Walkthrough

Walkthrough

The fish completion init script's PATH scanning loop now skips files that are not readable before attempting to peek-read their contents for shebang detection, preventing fish redirection warnings on execute-only or setuid binaries during shell startup.

Changes

Fish completion readability fix

Layer / File(s) Summary
Skip unreadable executables
lib/src/complete/fish.rs
Added test -r $file; or continue guard in the PATH scanning loop to skip files that aren't readable before reading their first bytes to detect the usage shebang.

Estimated code review effort: 1 (Trivial) | ~3 minutes

Possibly related issues

Poem

A rabbit hopped through PATH so wide,
Found sudo locked, and stepped aside.
No more warnings, no more fuss,
Just quiet shells for all of us.
🐇✨ test -r, and off we hop!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: skipping unreadable files during fish shebang completion scanning.
Linked Issues check ✅ Passed The patch adds the requested readability guard for the fish shebang scan, which should prevent warnings on unreadable executable PATH entries.
Out of Scope Changes check ✅ Passed Only the requested fish completion scan change is evident; no unrelated edits are shown in the reviewed files.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@greptile-apps

greptile-apps Bot commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

Guards the shebang peek-read in the generated fish init script with a test -r $file readability check so that execute-only binaries (e.g., macOS's sudo, visudo) are skipped before fish's redirection layer is invoked, eliminating spurious warnings on every shell startup.

  • lib/src/complete/fish.rs: Inserts test -r $file; or continue (with explanatory comment) immediately after the existing -f -x check and before read -l -n 128 first <$file, keeping the logic correct and the change minimal.
  • Snapshot updated: The insta snapshot for complete_fish_init is updated to reflect the four new lines, confirming the generated script matches the template.

Confidence Score: 5/5

This is a safe, single-line guard addition with no behavioral change for readable files.

The change is a one-line fish conditional inserted at the right point in the loop — after the executability check, before the file redirection — so files that already pass the -r test are completely unaffected. The comment is accurate, the snapshot is updated, and there is no logic that could regress existing behavior.

No files require special attention.

Important Files Changed

Filename Overview
lib/src/complete/fish.rs Adds test -r $file; or continue guard before the peek-read to skip execute-only files such as setuid binaries, with an explanatory comment.
lib/src/complete/snapshots/usage__complete__fish__tests__complete_fish_init.snap Snapshot updated to mirror the four new lines added to the generated fish init script; content matches the source template exactly.

Reviews (1): Last reviewed commit: "fix(complete): skip unreadable files in ..." | Re-trigger Greptile

@jdx
jdx merged commit 7e05325 into jdx:main Jul 6, 2026
5 checks passed
@mise-en-dev mise-en-dev mentioned this pull request Jul 6, 2026
tmeijn pushed a commit to tmeijn/dotfiles that referenced this pull request Jul 9, 2026
This MR contains the following updates:

| Package | Update | Change |
|---|---|---|
| [usage](https://github.com/jdx/usage) | patch | `3.5.2` → `3.5.4` |

MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot).

**Proposed changes to behavior should be submitted there as MRs.**

---

### Release Notes

<details>
<summary>jdx/usage (usage)</summary>

### [`v3.5.4`](https://github.com/jdx/usage/blob/HEAD/CHANGELOG.md#354---2026-07-06)

[Compare Source](jdx/usage@v3.5.3...v3.5.4)

##### 🐛 Bug Fixes

- **(complete)** skip unreadable files in fish shebang completion scan by [@&#8203;GrantD-ADSK](https://github.com/GrantD-ADSK) in [#&#8203;707](jdx/usage#707)

##### 🔍 Other Changes

- Enable Entire for Codex by [@&#8203;jdx](https://github.com/jdx) in [#&#8203;695](jdx/usage#695)
- Update sponsor references for jdx.dev by [@&#8203;jdx](https://github.com/jdx) in [#&#8203;702](jdx/usage#702)
- use release backends for cargo tools by [@&#8203;jdx](https://github.com/jdx) in [#&#8203;709](jdx/usage#709)

##### 📦️ Dependency Updates

- update jdx/mise-action action to v4.2.0 by [@&#8203;renovate\[bot\]](https://github.com/renovate\[bot]) in [#&#8203;698](jdx/usage#698)
- update rust-lang/crates-io-auth-action action to v1.0.5 by [@&#8203;renovate\[bot\]](https://github.com/renovate\[bot]) in [#&#8203;697](jdx/usage#697)
- update rust crate itertools to 0.15 by [@&#8203;renovate\[bot\]](https://github.com/renovate\[bot]) in [#&#8203;699](jdx/usage#699)
- update actions/checkout action to v7 by [@&#8203;renovate\[bot\]](https://github.com/renovate\[bot]) in [#&#8203;700](jdx/usage#700)
- lock file maintenance by [@&#8203;renovate\[bot\]](https://github.com/renovate\[bot]) in [#&#8203;701](jdx/usage#701)
- update zizmorcore/zizmor-action action to v0.5.7 by [@&#8203;renovate\[bot\]](https://github.com/renovate\[bot]) in [#&#8203;704](jdx/usage#704)
- update dependency cargo-binstall to v1.20.1 by [@&#8203;renovate\[bot\]](https://github.com/renovate\[bot]) in [#&#8203;703](jdx/usage#703)
- update rust crate tera to v2 by [@&#8203;renovate\[bot\]](https://github.com/renovate\[bot]) in [#&#8203;705](jdx/usage#705)
- lock file maintenance by [@&#8203;renovate\[bot\]](https://github.com/renovate\[bot]) in [#&#8203;708](jdx/usage#708)

##### New Contributors

- [@&#8203;GrantD-ADSK](https://github.com/GrantD-ADSK) made their first contribution in [#&#8203;707](jdx/usage#707)

### [`v3.5.3`](https://github.com/jdx/usage/blob/HEAD/CHANGELOG.md#353---2026-06-23)

[Compare Source](jdx/usage@v3.5.2...v3.5.3)

##### 🐛 Bug Fixes

- **(docs)** show negated flags in cli help by [@&#8203;jdx](https://github.com/jdx) in [#&#8203;694](jdx/usage#694)
- **(zsh)** preserve options for default completion by [@&#8203;jdx](https://github.com/jdx) in [#&#8203;693](jdx/usage#693)

##### 📦️ Dependency Updates

- update jdx/pr-closer action to v1.1.0 by [@&#8203;renovate\[bot\]](https://github.com/renovate\[bot]) in [#&#8203;689](jdx/usage#689)
- lock file maintenance lockfile maintenance by [@&#8203;renovate\[bot\]](https://github.com/renovate\[bot]) in [#&#8203;691](jdx/usage#691)

</details>

---

### Configuration

📅 **Schedule**: (UTC)

- Branch creation
  - At any time (no schedule defined)
- Automerge
  - At any time (no schedule defined)

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this MR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box

---

This MR has been generated by [Mend Renovate](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0My4yNTYuMCIsInVwZGF0ZWRJblZlciI6IjQzLjI1Ni4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJSZW5vdmF0ZSBCb3QiLCJhdXRvbWF0aW9uOmJvdC1hdXRob3JlZCIsImRlcGVuZGVuY3ktdHlwZTo6cGF0Y2giXX0=-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

completion-init fish: shebang-scan loop warns on unreadable-but-executable PATH entries (e.g. sudo, visudo)

2 participants