Skip to content

bug: prevent paccy empty selection crashes#86

Open
ALX99 wants to merge 4 commits into
masterfrom
audit-20260709
Open

bug: prevent paccy empty selection crashes#86
ALX99 wants to merge 4 commits into
masterfrom
audit-20260709

Conversation

@ALX99

@ALX99 ALX99 commented Jul 8, 2026

Copy link
Copy Markdown
Owner

Bug

paccy crashes or performs the wrong package-manager action when an interactive fzf picker is cancelled or returns no selected rows.

The affected flows are:

  • .local/bin/paccy -rt / --remove-tracked
  • .local/bin/paccy -ru / --remove-untracked
  • .local/bin/paccy -it / --install-tracked
  • .local/bin/paccy -s

Severity and impact

This is a reliability bug in an interactive package-management helper. Cancelling an fzf selection is a normal user flow and should be a no-op. Instead, the remove/install-tracked paths can crash under set -u, while the search path can proceed to yay -S with an empty package list. The practical impact is noisy failures and a confusing package-manager prompt/action after the user deliberately selected nothing.

Evidence

Relevant code:

  • .local/bin/paccy, set -eou pipefail enables nounset behavior.
  • .local/bin/paccy, the -rt, -ru, and -it branches append to pkgs only inside a while read loop fed by fzf, then immediately evaluate ${#pkgs[@]}.
  • .local/bin/paccy, the -s branch appends to pkgs from yay -Slq | fzf, but previously had no empty-selection guard before calling yay -S "${pkgs[@]}".

Observed incorrect behavior before the fix, as captured by the added regression test:

  • A stubbed fzf consumes stdin and exits 130, matching the normal cancel path.
  • For -rt, -ru, and -it, no pkgs array element is ever assigned; with set -u, checking ${#pkgs[@]} is an unbound-variable crash.
  • For -s, the cancelled picker leaves no selected package, but the script still reaches the install command. The test's stubbed yay treats any package-manager action other than the initial yay -Slq listing as unexpected.

This is a real defect rather than a hypothetical edge case because fzf cancellation is a standard interactive outcome, and the script already tries to handle empty selections in several branches with [ ${#pkgs[@]} -eq 0 ] && exit; the implementation just forgot to ensure pkgs exists and forgot the equivalent guard in search mode.

Reproduce

Setup:

git checkout master

Create a temporary test harness with stubbed commands:

  • fzf reads stdin and exits 130.
  • yay -Slq prints a couple of candidate packages; any other yay invocation is recorded as unexpected.
  • pacman prints deterministic installed/tracked package lists.
  • sudo records any invocation as unexpected.

Exact command added in this PR:

bash tests/paccy-empty-selection.test.sh

Actual result before the fix:

  • paccy -rt, paccy -ru, or paccy -it exits nonzero with an unbound pkgs array error after the cancelled picker; or
  • paccy -s reaches yay -S even though no package was selected.

Expected result:

  • Cancelling the picker exits successfully as a no-op.
  • No install/remove command is invoked when no package was selected.

Root cause

pkgs was created implicitly only when the while read loop appended at least one package. If the picker returned no rows, pkgs remained unset and set -u made ${#pkgs[@]} fatal. The search branch also lacked the same empty-selection guard used by the remove/install-tracked branches.

Fix

Declare pkgs as an empty local array at the start of main, making all ${#pkgs[@]} checks safe when no rows are selected. Add the missing empty-selection guard in the -s branch before calling yay -S or logging installed packages.

Validation

Commands run / checks performed:

  • compare master...audit-20260709 via GitHub compare API — result: branch is 2 commits ahead, 0 behind; changed files are only .local/bin/paccy and tests/paccy-empty-selection.test.sh.
  • Duplicate search: repo:ALX99/dotfiles is:pr paccy fzf selection pkgs — no existing PR found for this issue.

Regression test added:

  • tests/paccy-empty-selection.test.sh covers cancelled selections for -rt, -ru, -it, and -s using stubbed fzf, yay, pacman, and sudo commands.

I could not execute the shell test locally in this connector runtime because direct container execution and repository cloning were unavailable. The test is self-contained and can be run by a reviewer with the command above.

Scope

This PR intentionally does not change package-list parsing, package-manager command behavior for real selections, the -S direct install path, unrelated paccy options, or any other dotfile scripts.

@ALX99 ALX99 force-pushed the master branch 4 times, most recently from 5cf571d to dd748db Compare July 10, 2026 12:22
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.

1 participant