Fix #143: ask for the Maintenance admin password once - #145
Merged
Merged
Conversation
Contributor
Author
Each admin task spawned a new osascript process, so macOS could not cache credentials. Run do shell script in-process via NSAppleScript.
iliyami
force-pushed
the
fix/maintenance-auth-once
branch
from
September 16, 2026 17:47
f137d1b to
90d6856
Compare
Owner
|
Excellent fix, and thank you. Running the admin do-shell-script in-process via NSAppleScript so macOS's per-process credential cache covers the sequential tasks is exactly the right approach, no persistent root helper, same standard prompt. I audited it carefully: commands still come only from the fixed MaintenanceTask enum (no user input), the two-layer escaping is preserved (sh quote + AppleScript string), it's serialized off-main so long jobs don't freeze the UI, the missing-binary gate still runs before the prompt, and cancel now uses the real error number. 17 tests, strong coverage. Rebased, bumped to 1.19.2, full suite green at 819. Merging. Thanks @Borisserz! |
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #143.
The bug
On Maintenance, every admin task asks for the password again. Reporter on macOS 14.7.5 / Mac Sai 1.19.0:
Root cause: each admin task spawned a new
/usr/bin/osascriptprocess fordo shell script … with administrator privileges. macOS caches that authorization for about five minutes per process, so a fresh osascript cannot reuse the previous password. "Run Safe Tasks" already runs sequentially (issue #82) to avoid stacked dialogs, but it still prompted three times (RAM, purgeable space, maintenance scripts).Fix
Run the same AppleScript in-process via
NSAppleScripton a dedicated serial queue.MaintenanceTaskenum (never user input). POSIX quoting + AppleScript string escaping is unchanged, now inMaintenanceShell.appleScriptSource.-128) still maps to the existing friendly message.periodic) stay off the main thread so the UI does not freeze.NSAppleScriptcalls are serialized on one queue because the class is not thread-safe.No privileged helper / XPC daemon. That matches the current "standard macOS admin prompt" design.
Verification
compileAndReturnErroron every adminsystemCommand.testSequentialAdminTasksReuseTheSameRunnerproves routing through one in-process runner (the OS credential cache itself cannot be asserted in CI without a GUI password).testDoShellScriptWorksOffMainThreadruns realNSAppleScriptdo shell script "echo ok"on a background queue (no admin).bash scripts/check-version-sync.sh && swift build && swift test→ 815 tests, 3 skipped, 0 failures. Same commands as.github/workflows/ci.yml.Test plan
-128Choices worth your review
NSAppleScript, not a privileged helper. A LaunchDaemon would auth once at install, but this repo already chose the standard admin dialog over a persistent root helper. This PR keeps that boundary.executeAndReturnErroron the main thread would freeze the window for the length ofperiodic. SecurityAgent still presents the password dialog from a background wait.