mirror of
https://github.com/tw93/Mole.git
synced 2026-02-14 18:47:28 +00:00
fix: harden cleanup path validation
This commit is contained in:
@@ -2,7 +2,7 @@
|
|||||||
|
|
||||||
<div align="center">
|
<div align="center">
|
||||||
|
|
||||||
**Status:** PASSED | **Risk Level:** LOW | **Version:** 1.22.1 (2026-01-17)
|
**Status:** PASSED | **Risk Level:** LOW | **Version:** 1.23.2 (2026-01-26)
|
||||||
|
|
||||||
</div>
|
</div>
|
||||||
|
|
||||||
@@ -12,24 +12,31 @@
|
|||||||
|
|
||||||
| Attribute | Details |
|
| Attribute | Details |
|
||||||
|-----------|---------|
|
|-----------|---------|
|
||||||
| Audit Date | January 17, 2026 |
|
| Audit Date | January 26, 2026 |
|
||||||
| Audit Conclusion | **PASSED** |
|
| Audit Conclusion | **PASSED** |
|
||||||
| Mole Version | V1.22.0 |
|
| Mole Version | V1.23.2 |
|
||||||
| Audited Branch | `main` (HEAD) |
|
| Audited Branch | `main` (HEAD) |
|
||||||
| Scope | Shell scripts, Go binaries, Configuration |
|
| Scope | Shell scripts, Go binaries, Configuration |
|
||||||
| Methodology | Static analysis, Threat modeling, Code review |
|
| Methodology | Static analysis, Threat modeling, Code review |
|
||||||
| Review Cycle | Every 6 months or after major feature additions |
|
| Review Cycle | Every 6 months or after major feature additions |
|
||||||
| Next Review | June 2026 |
|
| Next Review | July 2026 |
|
||||||
|
|
||||||
**Key Findings:**
|
**Key Findings:**
|
||||||
|
|
||||||
- Multi-layer validation effectively blocks risky system modifications.
|
- Multi-layer validation effectively blocks risky system modifications.
|
||||||
- Conservative cleaning logic ensures safety (e.g., 60-day dormancy rule).
|
- Conservative cleaning logic ensures safety (e.g., 60-day dormancy rule).
|
||||||
- Comprehensive protection for VPNs, AI tools, and core system components.
|
- Comprehensive protection for VPNs, AI tools, and core system components.
|
||||||
|
- Operations logging improves traceability while remaining optional (MO_NO_OPLOG=1).
|
||||||
- Atomic operations prevent state corruption during crashes.
|
- Atomic operations prevent state corruption during crashes.
|
||||||
- Dry-run and whitelist features give users full control.
|
- Dry-run and whitelist features give users full control.
|
||||||
- Installer cleanup scans safely and requires user confirmation.
|
- Installer cleanup scans safely and requires user confirmation.
|
||||||
|
|
||||||
|
**Recent Remediations:**
|
||||||
|
|
||||||
|
- Symlink cleanup in `bin/clean.sh` now routes through `safe_remove` for target validation.
|
||||||
|
- Orphaned helper cleanup in `lib/clean/apps.sh` now uses `safe_sudo_remove`.
|
||||||
|
- ByHost preference cleanup in `lib/uninstall/batch.sh` validates bundle IDs and deletes via `safe_remove`.
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
## Security Philosophy
|
## Security Philosophy
|
||||||
|
|||||||
100
bin/clean.sh
100
bin/clean.sh
@@ -69,10 +69,10 @@ if [[ -f "$HOME/.config/mole/whitelist" ]]; then
|
|||||||
fi
|
fi
|
||||||
|
|
||||||
case "$line" in
|
case "$line" in
|
||||||
/ | /System | /System/* | /bin | /bin/* | /sbin | /sbin/* | /usr/bin | /usr/bin/* | /usr/sbin | /usr/sbin/* | /etc | /etc/* | /var/db | /var/db/*)
|
/ | /System | /System/* | /bin | /bin/* | /sbin | /sbin/* | /usr/bin | /usr/bin/* | /usr/sbin | /usr/sbin/* | /etc | /etc/* | /var/db | /var/db/*)
|
||||||
WHITELIST_WARNINGS+=("Protected system path: $line")
|
WHITELIST_WARNINGS+=("Protected system path: $line")
|
||||||
continue
|
continue
|
||||||
;;
|
;;
|
||||||
esac
|
esac
|
||||||
|
|
||||||
duplicate="false"
|
duplicate="false"
|
||||||
@@ -86,7 +86,7 @@ if [[ -f "$HOME/.config/mole/whitelist" ]]; then
|
|||||||
fi
|
fi
|
||||||
[[ "$duplicate" == "true" ]] && continue
|
[[ "$duplicate" == "true" ]] && continue
|
||||||
WHITELIST_PATTERNS+=("$line")
|
WHITELIST_PATTERNS+=("$line")
|
||||||
done <"$HOME/.config/mole/whitelist"
|
done < "$HOME/.config/mole/whitelist"
|
||||||
else
|
else
|
||||||
WHITELIST_PATTERNS=("${DEFAULT_WHITELIST_PATTERNS[@]}")
|
WHITELIST_PATTERNS=("${DEFAULT_WHITELIST_PATTERNS[@]}")
|
||||||
fi
|
fi
|
||||||
@@ -140,7 +140,7 @@ cleanup() {
|
|||||||
fi
|
fi
|
||||||
CLEANUP_DONE=true
|
CLEANUP_DONE=true
|
||||||
|
|
||||||
stop_inline_spinner 2>/dev/null || true
|
stop_inline_spinner 2> /dev/null || true
|
||||||
|
|
||||||
if [[ -t 1 ]]; then
|
if [[ -t 1 ]]; then
|
||||||
printf "\r\033[K" >&2 || true
|
printf "\r\033[K" >&2 || true
|
||||||
@@ -166,8 +166,8 @@ start_section() {
|
|||||||
|
|
||||||
if [[ "$DRY_RUN" == "true" ]]; then
|
if [[ "$DRY_RUN" == "true" ]]; then
|
||||||
ensure_user_file "$EXPORT_LIST_FILE"
|
ensure_user_file "$EXPORT_LIST_FILE"
|
||||||
echo "" >>"$EXPORT_LIST_FILE"
|
echo "" >> "$EXPORT_LIST_FILE"
|
||||||
echo "=== $1 ===" >>"$EXPORT_LIST_FILE"
|
echo "=== $1 ===" >> "$EXPORT_LIST_FILE"
|
||||||
fi
|
fi
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -220,7 +220,7 @@ normalize_paths_for_cleanup() {
|
|||||||
done
|
done
|
||||||
fi
|
fi
|
||||||
[[ "$is_child" == "true" ]] || result_paths+=("$path")
|
[[ "$is_child" == "true" ]] || result_paths+=("$path")
|
||||||
done <<<"$sorted_paths"
|
done <<< "$sorted_paths"
|
||||||
|
|
||||||
if [[ ${#result_paths[@]} -gt 0 ]]; then
|
if [[ ${#result_paths[@]} -gt 0 ]]; then
|
||||||
printf '%s\n' "${result_paths[@]}"
|
printf '%s\n' "${result_paths[@]}"
|
||||||
@@ -232,9 +232,9 @@ get_cleanup_path_size_kb() {
|
|||||||
local path="$1"
|
local path="$1"
|
||||||
|
|
||||||
if [[ -f "$path" && ! -L "$path" ]]; then
|
if [[ -f "$path" && ! -L "$path" ]]; then
|
||||||
if command -v stat >/dev/null 2>&1; then
|
if command -v stat > /dev/null 2>&1; then
|
||||||
local bytes
|
local bytes
|
||||||
bytes=$(stat -f%z "$path" 2>/dev/null || echo "0")
|
bytes=$(stat -f%z "$path" 2> /dev/null || echo "0")
|
||||||
if [[ "$bytes" =~ ^[0-9]+$ && "$bytes" -gt 0 ]]; then
|
if [[ "$bytes" =~ ^[0-9]+$ && "$bytes" -gt 0 ]]; then
|
||||||
echo $(((bytes + 1023) / 1024))
|
echo $(((bytes + 1023) / 1024))
|
||||||
return 0
|
return 0
|
||||||
@@ -243,9 +243,9 @@ get_cleanup_path_size_kb() {
|
|||||||
fi
|
fi
|
||||||
|
|
||||||
if [[ -L "$path" ]]; then
|
if [[ -L "$path" ]]; then
|
||||||
if command -v stat >/dev/null 2>&1; then
|
if command -v stat > /dev/null 2>&1; then
|
||||||
local bytes
|
local bytes
|
||||||
bytes=$(stat -f%z "$path" 2>/dev/null || echo "0")
|
bytes=$(stat -f%z "$path" 2> /dev/null || echo "0")
|
||||||
if [[ "$bytes" =~ ^[0-9]+$ && "$bytes" -gt 0 ]]; then
|
if [[ "$bytes" =~ ^[0-9]+$ && "$bytes" -gt 0 ]]; then
|
||||||
echo $(((bytes + 1023) / 1024))
|
echo $(((bytes + 1023) / 1024))
|
||||||
else
|
else
|
||||||
@@ -465,9 +465,9 @@ safe_clean() {
|
|||||||
[[ ! "$size" =~ ^[0-9]+$ ]] && size=0
|
[[ ! "$size" =~ ^[0-9]+$ ]] && size=0
|
||||||
|
|
||||||
if [[ "$size" -gt 0 ]]; then
|
if [[ "$size" -gt 0 ]]; then
|
||||||
echo "$size 1" >"$temp_dir/result_${idx}"
|
echo "$size 1" > "$temp_dir/result_${idx}"
|
||||||
else
|
else
|
||||||
echo "0 0" >"$temp_dir/result_${idx}"
|
echo "0 0" > "$temp_dir/result_${idx}"
|
||||||
fi
|
fi
|
||||||
|
|
||||||
((idx++))
|
((idx++))
|
||||||
@@ -492,17 +492,17 @@ safe_clean() {
|
|||||||
[[ ! "$size" =~ ^[0-9]+$ ]] && size=0
|
[[ ! "$size" =~ ^[0-9]+$ ]] && size=0
|
||||||
local tmp_file="$temp_dir/result_${idx}.$$"
|
local tmp_file="$temp_dir/result_${idx}.$$"
|
||||||
if [[ "$size" -gt 0 ]]; then
|
if [[ "$size" -gt 0 ]]; then
|
||||||
echo "$size 1" >"$tmp_file"
|
echo "$size 1" > "$tmp_file"
|
||||||
else
|
else
|
||||||
echo "0 0" >"$tmp_file"
|
echo "0 0" > "$tmp_file"
|
||||||
fi
|
fi
|
||||||
mv "$tmp_file" "$temp_dir/result_${idx}" 2>/dev/null || true
|
mv "$tmp_file" "$temp_dir/result_${idx}" 2> /dev/null || true
|
||||||
) &
|
) &
|
||||||
pids+=($!)
|
pids+=($!)
|
||||||
((idx++))
|
((idx++))
|
||||||
|
|
||||||
if ((${#pids[@]} >= MOLE_MAX_PARALLEL_JOBS)); then
|
if ((${#pids[@]} >= MOLE_MAX_PARALLEL_JOBS)); then
|
||||||
wait "${pids[0]}" 2>/dev/null || true
|
wait "${pids[0]}" 2> /dev/null || true
|
||||||
pids=("${pids[@]:1}")
|
pids=("${pids[@]:1}")
|
||||||
((completed++))
|
((completed++))
|
||||||
|
|
||||||
@@ -515,7 +515,7 @@ safe_clean() {
|
|||||||
|
|
||||||
if [[ ${#pids[@]} -gt 0 ]]; then
|
if [[ ${#pids[@]} -gt 0 ]]; then
|
||||||
for pid in "${pids[@]}"; do
|
for pid in "${pids[@]}"; do
|
||||||
wait "$pid" 2>/dev/null || true
|
wait "$pid" 2> /dev/null || true
|
||||||
((completed++))
|
((completed++))
|
||||||
|
|
||||||
if [[ "$show_spinner" == "true" && -t 1 ]]; then
|
if [[ "$show_spinner" == "true" && -t 1 ]]; then
|
||||||
@@ -536,15 +536,11 @@ safe_clean() {
|
|||||||
for path in "${existing_paths[@]}"; do
|
for path in "${existing_paths[@]}"; do
|
||||||
local result_file="$temp_dir/result_${idx}"
|
local result_file="$temp_dir/result_${idx}"
|
||||||
if [[ -f "$result_file" ]]; then
|
if [[ -f "$result_file" ]]; then
|
||||||
read -r size count <"$result_file" 2>/dev/null || true
|
read -r size count < "$result_file" 2> /dev/null || true
|
||||||
local removed=0
|
local removed=0
|
||||||
if [[ "$DRY_RUN" != "true" ]]; then
|
if [[ "$DRY_RUN" != "true" ]]; then
|
||||||
if [[ -L "$path" ]]; then
|
if safe_remove "$path" true; then
|
||||||
rm "$path" 2>/dev/null && removed=1
|
removed=1
|
||||||
else
|
|
||||||
if safe_remove "$path" true; then
|
|
||||||
removed=1
|
|
||||||
fi
|
|
||||||
fi
|
fi
|
||||||
else
|
else
|
||||||
removed=1
|
removed=1
|
||||||
@@ -581,12 +577,8 @@ safe_clean() {
|
|||||||
|
|
||||||
local removed=0
|
local removed=0
|
||||||
if [[ "$DRY_RUN" != "true" ]]; then
|
if [[ "$DRY_RUN" != "true" ]]; then
|
||||||
if [[ -L "$path" ]]; then
|
if safe_remove "$path" true; then
|
||||||
rm "$path" 2>/dev/null && removed=1
|
removed=1
|
||||||
else
|
|
||||||
if safe_remove "$path" true; then
|
|
||||||
removed=1
|
|
||||||
fi
|
|
||||||
fi
|
fi
|
||||||
else
|
else
|
||||||
removed=1
|
removed=1
|
||||||
@@ -643,9 +635,9 @@ safe_clean() {
|
|||||||
local size=0
|
local size=0
|
||||||
|
|
||||||
if [[ -n "${temp_dir:-}" && -f "$temp_dir/result_${idx}" ]]; then
|
if [[ -n "${temp_dir:-}" && -f "$temp_dir/result_${idx}" ]]; then
|
||||||
read -r size count <"$temp_dir/result_${idx}" 2>/dev/null || true
|
read -r size count < "$temp_dir/result_${idx}" 2> /dev/null || true
|
||||||
else
|
else
|
||||||
size=$(get_cleanup_path_size_kb "$path" 2>/dev/null || echo "0")
|
size=$(get_cleanup_path_size_kb "$path" 2> /dev/null || echo "0")
|
||||||
fi
|
fi
|
||||||
|
|
||||||
[[ "$size" == "0" || -z "$size" ]] && {
|
[[ "$size" == "0" || -z "$size" ]] && {
|
||||||
@@ -653,7 +645,7 @@ safe_clean() {
|
|||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
|
|
||||||
echo "$(dirname "$path")|$size|$path" >>"$paths_temp"
|
echo "$(dirname "$path")|$size|$path" >> "$paths_temp"
|
||||||
((idx++))
|
((idx++))
|
||||||
done
|
done
|
||||||
fi
|
fi
|
||||||
@@ -684,9 +676,9 @@ safe_clean() {
|
|||||||
' | while IFS='|' read -r display_path total_size child_count; do
|
' | while IFS='|' read -r display_path total_size child_count; do
|
||||||
local size_human=$(bytes_to_human "$((total_size * 1024))")
|
local size_human=$(bytes_to_human "$((total_size * 1024))")
|
||||||
if [[ $child_count -gt 1 ]]; then
|
if [[ $child_count -gt 1 ]]; then
|
||||||
echo "$display_path # $size_human, $child_count items" >>"$EXPORT_LIST_FILE"
|
echo "$display_path # $size_human, $child_count items" >> "$EXPORT_LIST_FILE"
|
||||||
else
|
else
|
||||||
echo "$display_path # $size_human" >>"$EXPORT_LIST_FILE"
|
echo "$display_path # $size_human" >> "$EXPORT_LIST_FILE"
|
||||||
fi
|
fi
|
||||||
done
|
done
|
||||||
|
|
||||||
@@ -726,7 +718,7 @@ start_cleanup() {
|
|||||||
SYSTEM_CLEAN=false
|
SYSTEM_CLEAN=false
|
||||||
|
|
||||||
ensure_user_file "$EXPORT_LIST_FILE"
|
ensure_user_file "$EXPORT_LIST_FILE"
|
||||||
cat >"$EXPORT_LIST_FILE" <<EOF
|
cat > "$EXPORT_LIST_FILE" << EOF
|
||||||
# Mole Cleanup Preview - $(date '+%Y-%m-%d %H:%M:%S')
|
# Mole Cleanup Preview - $(date '+%Y-%m-%d %H:%M:%S')
|
||||||
#
|
#
|
||||||
# How to protect files:
|
# How to protect files:
|
||||||
@@ -742,7 +734,7 @@ EOF
|
|||||||
fi
|
fi
|
||||||
|
|
||||||
if [[ -t 0 ]]; then
|
if [[ -t 0 ]]; then
|
||||||
if sudo -n true 2>/dev/null; then
|
if sudo -n true 2> /dev/null; then
|
||||||
SYSTEM_CLEAN=true
|
SYSTEM_CLEAN=true
|
||||||
echo -e "${GREEN}${ICON_SUCCESS}${NC} Admin access already available"
|
echo -e "${GREEN}${ICON_SUCCESS}${NC} Admin access already available"
|
||||||
echo ""
|
echo ""
|
||||||
@@ -782,7 +774,7 @@ EOF
|
|||||||
else
|
else
|
||||||
echo ""
|
echo ""
|
||||||
echo "Running in non-interactive mode"
|
echo "Running in non-interactive mode"
|
||||||
if sudo -n true 2>/dev/null; then
|
if sudo -n true 2> /dev/null; then
|
||||||
SYSTEM_CLEAN=true
|
SYSTEM_CLEAN=true
|
||||||
echo " ${ICON_LIST} System-level cleanup enabled, sudo session active"
|
echo " ${ICON_LIST} System-level cleanup enabled, sudo session active"
|
||||||
else
|
else
|
||||||
@@ -1033,7 +1025,7 @@ perform_cleanup() {
|
|||||||
echo "# Potential cleanup: ${freed_gb}GB"
|
echo "# Potential cleanup: ${freed_gb}GB"
|
||||||
echo "# Items: $files_cleaned"
|
echo "# Items: $files_cleaned"
|
||||||
echo "# Categories: $total_items"
|
echo "# Categories: $total_items"
|
||||||
} >>"$EXPORT_LIST_FILE"
|
} >> "$EXPORT_LIST_FILE"
|
||||||
|
|
||||||
summary_details+=("Detailed file list: ${GRAY}$EXPORT_LIST_FILE${NC}")
|
summary_details+=("Detailed file list: ${GRAY}$EXPORT_LIST_FILE${NC}")
|
||||||
summary_details+=("Use ${GRAY}mo clean --whitelist${NC} to add protection rules")
|
summary_details+=("Use ${GRAY}mo clean --whitelist${NC} to add protection rules")
|
||||||
@@ -1085,18 +1077,18 @@ perform_cleanup() {
|
|||||||
main() {
|
main() {
|
||||||
for arg in "$@"; do
|
for arg in "$@"; do
|
||||||
case "$arg" in
|
case "$arg" in
|
||||||
"--debug")
|
"--debug")
|
||||||
export MO_DEBUG=1
|
export MO_DEBUG=1
|
||||||
;;
|
;;
|
||||||
"--dry-run" | "-n")
|
"--dry-run" | "-n")
|
||||||
DRY_RUN=true
|
DRY_RUN=true
|
||||||
export MOLE_DRY_RUN=1
|
export MOLE_DRY_RUN=1
|
||||||
;;
|
;;
|
||||||
"--whitelist")
|
"--whitelist")
|
||||||
source "$SCRIPT_DIR/../lib/manage/whitelist.sh"
|
source "$SCRIPT_DIR/../lib/manage/whitelist.sh"
|
||||||
manage_whitelist "clean"
|
manage_whitelist "clean"
|
||||||
exit 0
|
exit 0
|
||||||
;;
|
;;
|
||||||
esac
|
esac
|
||||||
done
|
done
|
||||||
|
|
||||||
|
|||||||
@@ -500,7 +500,7 @@ clean_orphaned_system_services() {
|
|||||||
if [[ "$orphan_file" == *.plist ]]; then
|
if [[ "$orphan_file" == *.plist ]]; then
|
||||||
sudo launchctl unload "$orphan_file" 2> /dev/null || true
|
sudo launchctl unload "$orphan_file" 2> /dev/null || true
|
||||||
fi
|
fi
|
||||||
if sudo rm -f "$orphan_file" 2> /dev/null; then
|
if safe_sudo_remove "$orphan_file"; then
|
||||||
debug_log "Removed orphaned service: $orphan_file"
|
debug_log "Removed orphaned service: $orphan_file"
|
||||||
fi
|
fi
|
||||||
fi
|
fi
|
||||||
|
|||||||
@@ -502,8 +502,14 @@ batch_uninstall_applications() {
|
|||||||
fi
|
fi
|
||||||
|
|
||||||
# ByHost preferences (machine-specific).
|
# ByHost preferences (machine-specific).
|
||||||
if [[ -d ~/Library/Preferences/ByHost ]]; then
|
if [[ -d "$HOME/Library/Preferences/ByHost" ]]; then
|
||||||
find ~/Library/Preferences/ByHost -maxdepth 1 -name "${bundle_id}.*.plist" -delete 2> /dev/null || true
|
if [[ "$bundle_id" =~ ^[A-Za-z0-9._-]+$ ]]; then
|
||||||
|
while IFS= read -r -d '' plist_file; do
|
||||||
|
safe_remove "$plist_file" true > /dev/null || true
|
||||||
|
done < <(command find "$HOME/Library/Preferences/ByHost" -maxdepth 1 -type f -name "${bundle_id}.*.plist" -print0 2> /dev/null || true)
|
||||||
|
else
|
||||||
|
debug_log "Skipping ByHost cleanup, invalid bundle id: $bundle_id"
|
||||||
|
fi
|
||||||
fi
|
fi
|
||||||
fi
|
fi
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user