Fix destructive and incorrect optimize tasks (macOS & Linux)#3
Fix destructive and incorrect optimize tasks (macOS & Linux)#3fschrhunt wants to merge 2 commits into
Conversation
Several `vole optimize` tasks could damage the system or misreport results: - network stack: `sudo route -n flush` deleted the default gateway route and dropped ALL connectivity until a network state change re-added it. Refresh only the ARP cache, which is the safe and sufficient neighbor-cache reset. - linux home ownership + macOS disk permissions: both resolved the repair target from raw $USER/$HOME/id, which become root/0 under `sudo vole optimize`. The home-ownership repair would then recursively chown the user's entire home to root:root and lock them out. Resolve via the SUDO_USER-aware get_invoking_* helpers and skip when there is no non-root invoking user. - usage tracking DB: stopped unlinking the live -wal/-shm from under knowledged (lost commits / corruption); checkpoint+truncate inside SQLite. - notification DB: delivered_date is a 2001-epoch Core Data timestamp, so the 30-day predicate matched every row and wiped the whole table; subtract the 2001 epoch to match the knowledgeC query. - broken prefs: move suspected-corrupt plists to Trash (recoverable) instead of rm -rf, since plutil can misjudge a valid third-party plist. - report success only when the sqlite/delete work actually succeeded. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a8a469eae7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| target_home="$(get_invoking_home)" | ||
| target_uidgid="$(get_invoking_uid):$(get_invoking_gid)" |
There was a problem hiding this comment.
When sudo vole optimize runs on systems where sudo resets $HOME to /root, this new target_home points at the real user's home but _opt_home_ownership_excludes still expands exclusions from $HOME. If ownership repair is triggered for a parent such as /home/alice/.local, _opt_chown_tree_preserving_home_excludes receives /root/.local/... exclusions and will not prune /home/alice/.local/share/containers, so the recursive chown can still rewrite rootless container storage despite the comment promising it is left untouched.
Useful? React with 👍 / 👎.
| for p in "$target_home" "$target_home/Library" "$target_home/Library/Preferences"; do | ||
| [[ -e "$p" && ! -w "$p" ]] && needs_repair=true |
There was a problem hiding this comment.
When the whole optimizer is launched via sudo, these paths are now the real user's directories, but [[ ! -w "$p" ]] is evaluated with root's privileges. If the home is owned by the user but, for example, ~/Library is not writable by that user, root still sees it as writable and the task reports permissions are already optimal instead of running diskutil resetUserPermissions for target_uid.
Useful? React with 👍 / 👎.
…ability as invoking user
- _opt_home_ownership_excludes now builds the container-storage exclusion
paths from the passed-in invoking home instead of $HOME. Under sudo $HOME
is /root, so the old exclusions never matched the real user's
~/.local/share/{containers,docker,podman} and the recursive chown could
still rewrite rootless container storage. (Codex P1)
- opt_mac_disk_permissions now probes writability as the invoking user
(`sudo -u "$target_user" test -w`) when running as root, since a plain
`-w` under sudo always sees paths as writable and skipped the repair.
(Codex P2)
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 participant
Summary
Hardening pass on
vole optimizeafter one of its tasks knocked a machine's internet offline. Found and fixed one connectivity bug plus a cluster of related destructive / incorrect-reporting issues in the optimize tasks.Fixes
Network stack —
route -n flushkilled all connectivity (critical).opt_mac_network_stackransudo route -n flush, which deletes the default gateway route along with everything else — the machine loses internet until a Wi-Fi toggle / DHCP renew re-adds it. Now refreshes only the ARP (neighbor) cache, the safe and sufficient reset.Home-ownership / disk-permissions repair targeted root under sudo (critical).
opt_linux_home_ownershipandopt_mac_disk_permissionsderived the repair target from raw$USER/$HOME/id, which areroot/0undersudo vole optimize. The Linux path would then recursivelychownthe invoking user's entire home toroot:root, locking them out. Both now resolve the real user via the existing SUDO_USER-awareget_invoking_*helpers and skip when there is no non-root invoking user.Usage-tracking DB — unlinked the live WAL (high).
opt_mac_usage_data_cleanuprm-edknowledgeC.db-wal/-shmwhileknowledgedholds the DB open, risking lost commits / corruption. Now checkpoints + truncates inside SQLite (PRAGMA wal_checkpoint(TRUNCATE)) instead of touching the sidecars.Notification DB — wiped the whole table (medium).
delivered_dateis a Core Data timestamp (seconds since 2001-01-01), but the 30-day predicate compared it to a Unix-epoch threshold, so every row matched and the entire notification history was deleted. Now subtracts the 2001 epoch, matching theknowledgeCquery.Broken-prefs — permanent deletion (low). Suspected-corrupt plists were
rm -rf-ed;plutilcan misjudge a valid third-party plist. Now moved to Trash (recoverable).Honest reporting. Quarantine / notification / usage tasks reported success unconditionally even when the
sqlite3/ delete step failed or was skipped. Now report success only on actual success, and warn otherwise.Testing
bash -n lib/optimize/tasks.sh— cleanshellcheck -S warning lib/optimize/tasks.sh— clean (exit 0)Fix #1 has already been applied to the live install that hit the bug.
🤖 Generated with Claude Code