1
0
mirror of https://github.com/tw93/Mole.git synced 2026-03-22 16:45:07 +00:00

fix: correct glob expansion in downloads cleanup, add purge index bounds guard, update security audit

- lib/clean/user.sh: quote glob patterns in _clean_incomplete_downloads so
  they are not expanded at array assignment time; filenames with spaces would
  previously be word-split before reaching safe_clean, causing silent failures
- lib/clean/project.sh: replace silent array fallback with explicit bounds
  check before reading PURGE_CATEGORY_FULL_PATHS_ARRAY, guarding against
  future index drift if menu filtering is added
- SECURITY_AUDIT.md: document double validatePath in analyze delete, native
  PAM passthrough for sudo prompts, dry-run dedup by filesystem identity,
  atomic purge config write, pre-commit hook mirroring CI, and new test suites
This commit is contained in:
Tw93
2026-03-21 14:17:52 +08:00
parent d6b9d9f3f3
commit 694c191c6f
3 changed files with 19 additions and 6 deletions

View File

@@ -52,6 +52,7 @@ Core controls include:
- paths containing control characters are rejected - paths containing control characters are rejected
- raw `find ... -delete` is avoided for security-sensitive cleanup logic - raw `find ... -delete` is avoided for security-sensitive cleanup logic
- removal flows use guarded helpers such as `safe_remove()`, `safe_sudo_remove()`, `safe_find_delete()`, and `safe_sudo_find_delete()` - removal flows use guarded helpers such as `safe_remove()`, `safe_sudo_remove()`, `safe_find_delete()`, and `safe_sudo_find_delete()`
- incomplete download cleanup skips files currently open (lsof check) and uses quoted glob patterns to prevent word-splitting on filenames that contain spaces
Blocked paths remain protected even with sudo. Examples include: Blocked paths remain protected even with sudo. Examples include:
@@ -180,6 +181,7 @@ Path traversal handling is also explicit:
- non-absolute paths are rejected for destructive helpers - non-absolute paths are rejected for destructive helpers
- `..` is rejected when it appears as a path component - `..` is rejected when it appears as a path component
- legitimate names containing `..` inside a single path element remain allowed to avoid false positives for real application data - legitimate names containing `..` inside a single path element remain allowed to avoid false positives for real application data
- `mo analyze` delete validates the raw user-supplied path before `filepath.Abs` resolves it, then validates the resolved absolute path a second time, closing a window where traversal segments could survive `Abs` normalization
## Privilege Escalation and Sudo Boundaries ## Privilege Escalation and Sudo Boundaries
@@ -193,6 +195,7 @@ Key properties:
- sudo deletion uses the same path validation gate as non-sudo deletion - sudo deletion uses the same path validation gate as non-sudo deletion
- sudo cleanup skips or reports denied operations instead of widening scope - sudo cleanup skips or reports denied operations instead of widening scope
- authentication, SIP/MDM, and read-only filesystem failures are classified separately in file-operation results - authentication, SIP/MDM, and read-only filesystem failures are classified separately in file-operation results
- sudo credential prompting passes through the system's native PAM prompt rather than a hardcoded string, ensuring correct behavior across locales and PAM configurations
When sudo is denied or unavailable, Mole prefers skipping privileged cleanup to forcing execution through unsafe fallback behavior. When sudo is denied or unavailable, Mole prefers skipping privileged cleanup to forcing execution through unsafe fallback behavior.
@@ -223,8 +226,10 @@ This reduces the risk of incorrectly classifying active software as orphaned dat
Mole exposes multiple safety controls before and during destructive actions: Mole exposes multiple safety controls before and during destructive actions:
- `--dry-run` previews are available for major destructive commands - `--dry-run` previews are available for major destructive commands
- dry-run output deduplicates targets by filesystem identity (device+inode), so aliased paths and symlinks do not appear as separate items
- interactive high-risk flows require explicit confirmation before deletion - interactive high-risk flows require explicit confirmation before deletion
- purge marks recent projects conservatively and leaves them unselected by default - purge marks recent projects conservatively and leaves them unselected by default
- purge configuration is written atomically (mktemp then rename) to prevent partial writes if the process is interrupted
- analyzer delete uses Finder Trash rather than direct permanent removal - analyzer delete uses Finder Trash rather than direct permanent removal
- operation logs are written to `~/Library/Logs/mole/operations.log` unless disabled with `MO_NO_OPLOG=1` - operation logs are written to `~/Library/Logs/mole/operations.log` unless disabled with `MO_NO_OPLOG=1`
- timeouts bound external commands so stalled discovery or uninstall operations do not silently hang the entire flow - timeouts bound external commands so stalled discovery or uninstall operations do not silently hang the entire flow
@@ -243,9 +248,10 @@ Mole treats release trust as part of its security posture, not just a packaging
Repository-level signals include: Repository-level signals include:
- weekly Dependabot updates for Go modules and GitHub Actions - weekly Dependabot updates for Go modules and GitHub Actions
- pre-commit hook that mirrors GitHub CI checks locally (shell syntax, shfmt, shellcheck, Go vet)
- CI checks for unsafe `rm -rf` usage patterns and core protection behavior - CI checks for unsafe `rm -rf` usage patterns and core protection behavior
- targeted tests for path validation, purge boundaries, symlink behavior, dry-run flows, and destructive helpers - targeted tests for path validation, purge boundaries, symlink behavior, dry-run flows, and destructive helpers
- CodeQL scanning for Go and GitHub Actions workflows - CodeQL scanning for Go and GitHub Actions workflows, with workflow permission hardening
- curated changelog-driven release notes with a dedicated `Safety-related changes` section - curated changelog-driven release notes with a dedicated `Safety-related changes` section
- published SHA-256 checksums for release assets - published SHA-256 checksums for release assets
- GitHub artifact attestations for release assets - GitHub artifact attestations for release assets
@@ -271,8 +277,11 @@ Key coverage areas include:
- path validation rejects empty, relative, traversal, and system paths - path validation rejects empty, relative, traversal, and system paths
- symlinked directories are rejected for destructive scans - symlinked directories are rejected for destructive scans
- purge protects shallow or ambiguous paths and filters nested artifacts - purge protects shallow or ambiguous paths and filters nested artifacts
- dry-run flows preview actions without applying them - dry-run flows preview actions without applying them and do not emit duplicate targets
- confirmation flows exist for high-risk interactive operations - confirmation flows exist for high-risk interactive operations
- sudo credential prompting and session management (`tests/manage_sudo.bats`)
- purge config path discovery and write behavior (`tests/purge_config_paths.bats`)
- hint and cleanup-hint flows (`tests/clean_hints.bats`)
## Known Limitations and Future Work ## Known Limitations and Future Work

View File

@@ -767,7 +767,11 @@ select_purge_categories() {
printf "%s\n" "$clear_line" printf "%s\n" "$clear_line"
local current_index=$((top_index + cursor_pos)) local current_index=$((top_index + cursor_pos))
local current_full_path="${PURGE_CATEGORY_FULL_PATHS_ARRAY[current_index]:-}" local current_full_path=""
local paths_len="${#PURGE_CATEGORY_FULL_PATHS_ARRAY[@]}"
if [[ "$paths_len" -gt 0 && "$current_index" -lt "$paths_len" ]]; then
current_full_path="${PURGE_CATEGORY_FULL_PATHS_ARRAY[current_index]}"
fi
if [[ -n "$current_full_path" ]]; then if [[ -n "$current_full_path" ]]; then
printf "%s${GRAY}Full path:${NC} %s\n" "$clear_line" "$current_full_path" printf "%s${GRAY}Full path:${NC} %s\n" "$clear_line" "$current_full_path"
printf "%s\n" "$clear_line" printf "%s\n" "$clear_line"

View File

@@ -88,9 +88,9 @@ _clean_recent_items() {
# Internal: Clean incomplete browser downloads, skipping files currently open. # Internal: Clean incomplete browser downloads, skipping files currently open.
_clean_incomplete_downloads() { _clean_incomplete_downloads() {
local -a patterns=( local -a patterns=(
"$HOME/Downloads/"*.download "$HOME/Downloads/*.download"
"$HOME/Downloads/"*.crdownload "$HOME/Downloads/*.crdownload"
"$HOME/Downloads/"*.part "$HOME/Downloads/*.part"
) )
local labels=("Safari incomplete downloads" "Chrome incomplete downloads" "Partial incomplete downloads") local labels=("Safari incomplete downloads" "Chrome incomplete downloads" "Partial incomplete downloads")
local i=0 local i=0