Skip to content

Warn about nonrecursive rm on important system paths - #3616

Open
hakarrr wants to merge 3 commits into
koalaman:masterfrom
hakarrr:fix-3600-nonrecursive-rm
Open

hakarrr wants to merge 3 commits into
koalaman:masterfrom
hakarrr:fix-3600-nonrecursive-rm

Conversation

@hakarrr

@hakarrr hakarrr commented Nov 10, 2026

Copy link
Copy Markdown

Fixes #3600.

checkCatastrophicRm only inspected rm arguments when a recursive flag was present. That misses both rm /bin/*, which deletes the directory's files without recursion, and rm /bin, which can unlink the symlink on a merged-/usr system.

Remove the recursive-flag gate while keeping the existing important-path list, brace expansion, path normalization and guarded-variable handling. Add nine regression properties covering literal paths, globs, brace expansion and an unguarded variable, plus guarded-variable and ordinary-path controls.

Validation on Linux with GHC 9.6.3:

  • The new properties fail on the original code in precisely the five positive cases and pass with the change.
  • Full stack test --fast and cabal v1-test suites pass; the executable builds.
  • End-to-end JSON checks confirm SC2114 for the literal/glob/brace examples and SC2115 for the unguarded-variable example. The negative controls emit neither code.
  • git diff --check is clean. No destructive rm commands were executed.

This deliberately warns on important literal paths even on hosts where they are real directories and nonrecursive rm would fail, since those same paths can be symlinks on other hosts. Path normalization is unchanged; #3614 addresses that separate behavior.

hakarrr and others added 3 commits November 10, 2026 22:03
Stage the tested Commands.hs replacement on the fix branch before moving it into its final path.

Co-authored-by: generaltony007 <144252170+GeneralTony007@users.noreply.github.com>
Remove the original path before moving the staged tested replacement into place.

Co-authored-by: generaltony007 <144252170+GeneralTony007@users.noreply.github.com>
Move the tested replacement into the Commands.hs path. Nonrecursive rm can delete files in system directories and unlink system-directory symlinks. Keep the existing path and guarded-variable handling.

Add regression coverage for literal, glob, brace and unguarded-variable cases, with guarded-variable and ordinary-path controls.

Co-authored-by: generaltony007 <144252170+GeneralTony007@users.noreply.github.com>
@hakarrr

hakarrr commented Nov 13, 2026

Copy link
Copy Markdown
Author

Could a maintainer review this warning for nonrecursive rm on important system paths? Please let me know if this warning is in scope or if the path selection or cases need changing before review. Thanks.

This branch has not been deployed

No deployments
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.

SC2114 is not issued to rm /bin/* (glob but without --recursive)

1 participant