Skip to content

fix(virusforget): correct --clean abort and --user validation#368

Open
maybebyte wants to merge 2 commits into
Kicksecure:masterfrom
maybebyte:fix/virusforget-error-handling
Open

fix(virusforget): correct --clean abort and --user validation#368
maybebyte wants to merge 2 commits into
Kicksecure:masterfrom
maybebyte:fix/virusforget-error-handling

Conversation

@maybebyte

Copy link
Copy Markdown
Contributor

Summary

virusforget --clean aborts partway through on exactly the input it exists
to remove, and an invalid --user is detected but not acted on. These are
two small, independent error-handling fixes to usr/libexec/security-misc/virusforget.

Changes

  • Guard restore_file against a missing backup. Root cause: an
    extraneous file (present in $HOME, absent from the committed backup) is
    deleted and then unconditionally passed to restore_file, which runs cp
    from a backup path that does not exist. Under set -e this aborts the
    entire --clean run on the first such file, leaving the system partially
    cleaned. The fix returns early when no backup exists — deleting the
    extraneous file is the correct action and is already done by the caller,
    which keeps a forensic copy under dangerous/.
  • Exit when --user does not exist. The nonexistent-user branch printed
    a warning to stdout but, unlike every sibling validation branch, neither
    wrote to stderr nor exited, so a mistyped --user proceeded against a
    bogus /home/<user>. It now writes to stderr and exit 1.

Testing

  • bash -n usr/libexec/security-misc/virusforget — no syntax errors.
  • Manually traced both walk paths (the direct process_files call chain and
    the process_folders find | while subshell). Because the script uses
    set -e without errtrace, the subshell path aborts via set -e
    propagation on the failed pipe rather than the ERR trap, but both paths
    stop on the first extraneous file pre-fix and complete after the guard.

Notes for reviewers

  • The two commits are independent and can be reviewed/reverted separately.
  • The restore_file guard is transparent to its other two callers
    (missing-file and modified-file), which always have a backup present.
  • Behavior-only diff (9 insertions, 1 deletion); no reformatting.

An extraneous file (present in the home directory but absent from the
committed backup) is deleted and then unconditionally passed to
restore_file, which runs `cp` from a backup path that does not exist.
Under `set -e` this aborts the entire `--clean` run on the first such
file, leaving the system partially cleaned -- the exact
malware-persistence scenario the tool exists to handle.

Return early from restore_file when no backup exists. Deleting the
extraneous file is the correct action and is already done by the caller
(which keeps a forensic copy under the dangerous/ folder). The guard is
transparent to the missing-file and modified-file callers, which always
have a backup present.
The nonexistent-user branch printed a warning but, unlike every sibling
validation branch, neither exited nor wrote to stderr, so a mistyped
--user proceeded to operate on a bogus /home/<user> path. Send the error
to stderr and exit 1.
@maybebyte

Copy link
Copy Markdown
Contributor Author

Used Claude Code to find this and assist, but manually reviewed.

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