Skip to content

install.sh --uninstall skips & orphans a backed-up broken symlink (-O dereferences) #5

@chiro-hiro

Description

@chiro-hiro

Severity: low · Category: robustness
Location: install.sh — find_latest_backup(), lines 87-89 (and the [ -O "$f" ] test)

What's wrong

backup_if_exists() correctly captures a pre-existing broken symlink at ~/.vim or ~/.vimrc because its guard tests [ -e "$target" ] || [ -L "$target" ] (the -L catches dangling links). It moves it to <target>.bak.<ts>, preserving the dangling symlink. But on --uninstall, find_latest_backup() filters every candidate through [ ! -O "$f" ] && continue. The -O test dereferences the symlink (it stat()s the target, not the link), so a backup that is itself a broken symlink fails -O and is skipped. Result: the user's original (broken) ~/.vimrc/.vim is never restored, the documented "restore most recent backup" contract is violated, and the backup file is left orphaned with no further mention. The -O filter was added as a TOCTOU/shared-HOME defense (commit 0ac397b), but it also rejects the user's own legitimate broken-symlink backups.

Evidence

Backup side (line 30) captures broken symlinks via -L:
if [ -e "$target" ] || [ -L "$target" ]; then ... mv "$target" "$backup"
Restore side (lines 87-89) rejects them because -O follows the link:
if [ ! -O "$f" ]; then
continue
fi
Reproduced end-to-end: a .vimrc.bak.5000 -> /home/.../missing-config dangling link is matched by the glob, passes the integer-suffix check, then SKIPPED (not -O) -> find_latest_backup returns empty -> original never restored, backup orphaned. ([ -O broken_symlink ] returns false because the link target does not exist.)

Suggested fix

Test ownership of the link itself, not its target. Replace [ ! -O "$f" ] with a non-dereferencing check, e.g. compare stat -c %u "$f" (Linux) / stat -f %u "$f" (BSD/macOS) against id -u, or use find "$f" -maxdepth 0 -user "$(id -un)". Alternatively, since both the backup creation and these files live in the user's own $HOME, special-case symlinks: if [ -L "$f" ] and the name already passed the integer-suffix validation, accept it. This restores broken-symlink backups while keeping the TOCTOU intent for regular files.


Filed from an automated multi-agent source review (2026-05-29); finding adversarially verified at high confidence. Line numbers reflect the audit-fixes-2026-05 working tree.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions