Skip to content

harden: uninstall wipes user-authored files — ownership inferred by name/dir while install is per-file (#230 audit) #236

Description

@edheltzel

From the post-merge audit of #230 (reviewer report finding 2; skill-dir shape found by the cross-model Codex pass and verified directly; commands shape flagged independently by 4 reviewers).

Root cause: uninstall infers ownership by name/directory while install manages per-file symlinks.

a. uninstall.sh:231 (remove_skills_from) does rm -rf "$skills_root/$name" for every RECALL_SKILL_NAMES entry. Install's collision rule (recall_link) backs up and replaces only the individual colliding file — so a user's pre-existing ~/.claude/skills/recall-add/ keeps its extra scripts/, references/ through install, then loses them all at uninstall. Silent, permanent. #230 grew the claimed namespace 4 → 9 names.
b. uninstall.sh:199-207 (remove_slash_commands) still rm -rfs all of ~/.claude/commands/Recall/ — destroying exactly the user files the #230 install/update contract now preserves (recall_unlink_if_managed + rmdir-if-empty). Also a third, cruder reimplementation beside the centralized recall_remove_legacy_slash_commands (DRY).

Fix: route both through the managed-symlink-only pattern (unlink managed links; remove dirs only when empty). If uninstall intentionally means "purge everything named recall-*", say so explicitly in the summary text (uninstall.sh:141 is currently inaccurate) and a comment.

Cheap adjacent hardenings from the same audit: compare managed links with readlink -f/-ef instead of string prefix (finding 6, P3); log_warn when non-symlink .md files remain after legacy cleanup (finding 8, P3 — cp-era installs).

Refs #230, #228.

Metadata

Metadata

Assignees

No one assigned

    Labels

    type:bugSomething isn't working

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions