Skip to content

Install, uninstall: identify a running Insomnia by path or bundle id, not by name - #18

Open
krishhgg wants to merge 20 commits into
mainfrom
fix/app-detection-bundle-id
Open

krishhgg wants to merge 20 commits into
mainfrom
fix/app-detection-bundle-id

Conversation

@krishhgg

@krishhgg krishhgg commented Oct 1, 2026 •

Copy link
Copy Markdown
Owner

This body replaces the one the PR was opened with. That one described a different change (freeze-all defaults), because the draft file in /tmp was overwritten by another branch's draft between writing and gh pr create.

Why

scripts/install.sh and scripts/uninstall.sh decided the app was running with pgrep -x Insomnia. The Insomnia API client's executable is also named Insomnia, and its bundle id is not com.kgarg.insomnia. With the client open, the AppleScript quit did nothing, the scripts waited out the quit timeout, and they refused with a message that sounded like this app would not quit. Anyone who already uses the API client could neither install nor uninstall. (Launch-readiness review, High.)

What

  • Both scripts check each pid that pgrep -x Insomnia reports, in every account, by its executable path from ps -o comm= and its owner from ps -o uid=. For an app LaunchServices launched, that is the full path. I checked this read-only on the maintainer's Mac, where the installed app shows ~/Applications/Insomnia.app/Contents/MacOS/Insomnia.
  • A process is this app when its path is the installed bundle's binary, or lies in a bundle whose Info.plist declares com.kgarg.insomnia. A development build with that bundle id shares the journal and the lock, so it counts.
  • Only the API client's bundle id, com.insomnia.app (from its Homebrew cask), proves a process is another app. Such a process is reported and left alone: "Ignoring 1 process(es) named Insomnia that are not this app: pid 4242 (/Applications/Insomnia.app/Contents/MacOS/Insomnia, bundle id com.insomnia.app)." (130032b; before, any readable id other than ours did.)
  • A process named Insomnia that is not proven to be either app counts as this app until it exits. That covers any other bundle id, no path from ps, a path outside any bundle, an Info.plist that does not parse, and in uninstall an Info.plist that does not answer within CALL_TIMEOUT_SECONDS. The scripts never signal it. They wait for it, then refuse, naming each such pid, its path and why it could not be identified.
  • No Info.plist read can hold the recovery lock. uninstall.sh runs it through its bounded helper. install.sh has no time limit for a call, so once it holds the lock it reads no Info.plist: it reuses the ids it read before the lock for the same pid and path, and a process it first sees under the lock counts as unverified. (c21f832, d2c5087) Qualified in 8d5184c: this holds for the scripts' process identity reads. The backstop.sh each script runs under the lock is main's: it has no time limit and reads the installed app's Info.plist itself. Since e9e3037 it holds for the backstop.sh each script runs under the lock too: both scripts read InsomniaResumeFrozenVersion and the file's identity before the lock and pass both down, and the backstop uses the version only while a bounded stat under the lock shows the same file. A backstop sealed in a bundle from an earlier release, which an uninstall from that release's zip runs, does not know these variables and still reads the Info.plist itself (see Codex review 6, R8-4). uninstall.sh now reads InsomniaResumeFrozenVersion before the lock, bounded, and under the lock only a bounded stat checks that the file is unchanged.
  • The AppleScript quit goes to BUNDLE_ID, and only when a copy was identified by path or bundle id. An unverified process alone never sends a quit to the real app.
  • Every refusal ("still running", "started again") names the pid and executable path of each copy that is still running.
  • A copy of this app in another account, or a process there that cannot be told apart from one, stops install before the sudoers step and uninstall before anything is removed. It is named with its uid and never asked to quit or signalled. (13a633b, see the Codex review.)
  • Uninstall reads /etc/sudoers.d/insomnia through sudo and removes it only when it is the rule install.sh writes for the calling account. Otherwise it keeps the file and says why. (13a633b)
  • Install reads the existing rule through sudo the same way before it writes a new one. A line counts as this account's only when its sudoers user field is exactly $USER or # and this account's uid, with no comma after it that carries on the user list. Any other line stops the install with nothing changed. For a grant to another name or user ID, the message names it and says to uninstall Insomnia there first, or to remove the rule with sudo rm if that account no longer exists. Any other form (a group, netgroup, alias, list, quoted name, ALL, Defaults, an include, a continued line) gets a message that quotes the line, since the new file would drop it. A rule that sudo cannot read stops it too. The commands on a line do not matter, so this account's rule from any version of install.sh is still replaced. (acf1141 and 38a9d1d, see the Codex review.)
  • Both scripts write or remove the rule only if it is still what they read, as far as the other writer takes the same lock. The compare and the write (or removal) are one sudo /bin/bash -c call that holds a lock only root can create, /var/run/insomnia-sudoers.lock file in the rule's own folder, /etc/sudoers.d/.insomnia-sudoers.lock (8d5184c), which root uses only after checking that only root can have made or changed it. A rule that changed since the read is left as it is and the run stops. (729a91c, see Codex review 4. Codex review 5 found the /var/run lock was not root-only; 8d5184c replaced it.) Since e9e3037 root also stops at an access control list (ACL) that allows more than reading, or one it cannot read in full, on any folder from the rule's up to /, on the lock file or on the rule. It checks before it creates or opens the lock file and again just before the change. Just before mv or rm it reads the rule a second time through a new descriptor, with cat's exit status checked, and changes the rule only when that read gives the same bytes from the same file. (Codex review 6, R8-1 and R8-2.)
  • After the merge of main's bounded() (b4b6d05), uninstall.sh's process check also reads no Info.plist under the recovery lock (see the qualification above). A pgrep that fails stops install before the sudoers step and uninstall before anything is removed. (b4b6d05, f20c230)
  • 8d5184c (Codex review 5): a process whose owner ps -o uid= cannot read (it fails, does not answer, or prints no user ID) is no longer judged as this account's. Both scripts look once more and, if its owner or identity is still unknown, stop before their first sudo call, the password prompt included. Only a process positively identified as the API client is ignored. uninstall.sh asks for the password with sudo -v before the recovery lock, and each sudo call under the lock is sudo -n and bounded. The reads under the lock that the review listed are bounded, and one that fails or does not answer is never taken as a clean answer. Codex review 6 found two gaps here, both fixed: a failed raw journal read in uninstall.sh could count as clean (e9e3037, R8-3), and a SIGTERM to the run's process group ended the supervisor of a bounded sudo call, so the lock could be released while that call still ran (c97535a, R8-7).
  • The Swift app does not look for other instances by process name anywhere in Sources/, so the app does not change.
  • README (install details, uninstall), spec section 2, and docs/release-validation.md (two new "Not run" rows). 8d5184c rewrites the lock text in all three and adds three "Not run" rows. e9e3037 adds the ACL check, root's second read, the backstop's limits and the journal read rules to all three, and three more "Not run" rows.

Review fixes

Tests

New in RecoveryScriptTests. They run fixture copies of the scripts with fake pgrep, ps, osascript, sudo and launchctl, so nothing real runs.

  • testInstallIgnoresAForeignProcessNamedInsomnia
  • testUninstallIgnoresAForeignProcessNamedInsomnia
  • testInstallQuitsACopyWithThisBundleIdAtAnotherPath
  • testUninstallRefusalNamesTheRunningCopyAndIgnoresTheOther
  • testProcessWithoutAnIdentifiablePathBlocksUninstall
  • testInstallRefusesWhileACopyWithAnUnreadableInfoPlistRuns
  • testUninstallRefusesWhileACopyWithAnUnreadableInfoPlistRunsAndStillIgnoresTheClient
  • testInstallSendsNoQuitWhenOnlyAnUnverifiedProcessRuns
  • testUninstallSendsNoQuitWhenOnlyAnUnverifiedProcessRuns

The fake pgrep prints pids when it reports a match, and the fake ps answers -o comm= -p <pid> from a table. The old exit-status sequences still work. With the old script logic the first two tests fail with "Insomnia is still running", the symptom from the review.

New in 13a633b:

  • testUninstallStopsForInsomniaInAnotherAccountAndNeverSignalsIt: a copy of this app and an unidentifiable process, both in another account. Exit 1, no osascript, kill, pkill, sudo or launchctl call, both pids named with their uid, the app, LaunchAgent, rule and journal untouched.
  • testInstallStopsBeforeTheSudoersStepForInsomniaInAnotherAccount: no sudo call at all, this account's own running copy is not asked to quit, the rule, bundle and LaunchAgent unchanged.
  • testUninstallIgnoresAnotherAppInAnotherAccount: the API client in another account is still ignored.
  • testUninstallReadsTheRuleBackAndRemovesItWhenThisAccountsInstallWroteIt: sudo cat then sudo rm -f.
  • testUninstallKeepsARuleThatGrantsAnotherAccount: the file is kept byte for byte, the message names the other account, and the rest of the uninstall goes on.
  • testUninstallKeepsARuleThatIsNotWhatInstallWritesForThisAccount: an extra grant, a file with no grant, and a line install.sh never writes are each kept with the reason.
  • testUninstallStopsWhenTheRuleCannotBeReadThroughSudo: exit 1, the rule and the app kept (mode 000, skipped under root).

The fixture's fake ps answers -o uid= -p <pid> from a table, padded like the real column, and gives every other listed pid the test account's uid. The fake sudo runs cat on fixture paths. installMachinery writes the exact rule install.sh would write for the account running the tests, in place of the word "rule". Two install tests whose pgrep answer sequences assumed one fewer check gained an answer for the new check before the sudoers step.

New in c21f832, 130032b and d2c5087:

  • testUninstallTreatsAnInfoPlistThatDoesNotAnswerUnderTheLockAsUnverified: the fixture's plutil hangs on one bundle's Info.plist. Uninstall stops the read after 1 s without passing the lock to it, exits 1 with "did not answer within 1s", and removes nothing. On the previous uninstall.sh it held the lock for 60 s.
  • testInstallReadsNoInfoPlistUnderTheRecoveryLock: a process first seen under the lock, whose bundle id would have shown the API client, is never read and stops the install before launchctl. On the previous install.sh the plist was read and the install went on.
  • testInstallIgnoresAForeignProcessNamedInsomnia also checks that the client's Info.plist is read once, before the lock.
  • testInstallDoesNotCarryAnIdentityOverToANewProcessAtTheSamePath (d2c5087): the client as pid 4242 before the lock, another process at its path as pid 5151 under it. The install stops naming pid 5151. It fails on c21f832's cache. The fake pgrep takes a pids:A,B line for this.
  • testInstallRefusesWhileAProcessWithAnUnknownBundleIdRuns and testUninstallRefusesWhileProcessesWithAnUnknownBundleIdRun: a bundle declaring com.example.insomnia-copy blocks install, and blocks uninstall in this account and in another, with no quit, signal or removal. Both fail on the previous scripts.

The fixture's plutil is now a wrapper around the real tool, patched in as each script's PLUTIL. It records every Info.plist it is asked to read and can make one read hang.

New in acf1141:

  • testInstallRefusesWhenTheRuleGrantsAnotherAccount: the rule install.sh writes, for alice. Exit 1 with "grants alice, not " and the uninstall-first message, sudo cat called, no sudo -n, sudo visudo, sudo install, osascript or launchctl call, and the rule, app and LaunchAgent unchanged byte for byte.
  • testInstallRefusesWhenTheRuleGrantsAnotherUserID (38a9d1d): the same rule written for # and another uid. Exit 1 with "grants user ID , not ", no sudo -n, sudo visudo, sudo install, osascript or launchctl call, and the rule and app unchanged. It fails on acf1141, which installed over that rule.
  • testInstallRefusesARuleWithAnyLineNotForThisAccount: this account's rule plus one more line, each refused with the rule unchanged. The extra lines are a grant to bob, a %admin grant, Defaults:bob, an ALL line and an #include. 38a9d1d adds a grant to another uid, #-1, %#20, +staff, an alias name and a User_Alias line, a quoted name, <account>,bob, <account> , bob, #includedir, @include, and a continued line. On acf1141 the other uid, #-1 and <account> , bob were installed over, and the alias name was called an account.
  • testInstallReplacesThisAccountsOwnRuleWhateverItsCommands: this account's rule with three grants and another header, the way a later install.sh might write it, is replaced and the install finishes. 38a9d1d makes one of the grants to # and this account's uid, separates another with a tab, and adds comment lines # and #--- 502 ....
  • testUninstallKeepsARuleThatGrantsAnotherUserID (38a9d1d): uninstall keeps the rule written for another uid and says it grants #<uid>. Uninstall's check compares whole lines, so it already did.
  • testInstallStopsWhenTheRuleCannotBeReadThroughSudo: a mode-000 rule stops the install with "Could not read ... Nothing was changed." (skipped under root).

All four fail on 4eeae1e. The install tests now run with USER set to ScriptFixture.account, the account that installMachinery's rule names, in place of tester. With tester, ten of them would have stopped at the new check.

New in f20c230:

  • testUninstallReadsNoInfoPlistUnderTheRecoveryLock (was testUninstallTreatsAnInfoPlistThatDoesNotAnswerUnderTheLockAsUnverified): a process first seen under the lock is never read, and the uninstall stops naming it as "first seen under the recovery lock, where no Info.plist is read".
  • testUninstallStopsAnInfoPlistReadThatDoesNotAnswerBeforeTheLock: the read before the lock is stopped at the 5 s limit without the lock, the process counts as unverified, and the run goes on.
  • testInstallStopsBeforeTheSudoersStepWhenPgrepFails and testUninstallStopsWhenPgrepFails: pgrep exiting 3 stops each script with nothing changed.

New in 729a91c. The fake sudo runs the transaction unprivileged only when the rule it would write or remove and the lock in its script are inside the fixture. A FAKE_SUDO_GATE file makes the transaction wait before it starts, so a test can act between a run's read and its write.

  • testInstallKeepsARuleAnotherAccountWroteAfterItsRead: two accounts, two homes, two recovery locks. Both read no rule. This account's write waits at the gate while bob's install runs to the end and writes bob's rule. Released, this account's install exits 1 with "changed after this install read it", bob's rule is intact, and this account's app and LaunchAgent are not installed. A rerun refuses bob's rule.
  • testUninstallKeepsARuleWrittenAfterItsRead: the uninstall reads its own rule and waits at the gate while bob's rule replaces it. It keeps bob's rule and the app, exits 1 and asks for a rerun; the rerun keeps bob's rule and finishes.
  • testUninstallAndAnotherAccountsInstallNeverRemoveOrReplaceTheOthersRule: while this account's uninstall waits to remove its rule, bob's install in another home reads that rule and refuses it with nothing changed. The uninstall then removes its own rule, and bob's rerun installs.
  • testInstallStopsWhileAnotherRunHoldsTheSudoersLock and testUninstallKeepsItsRuleWhileAnotherRunHoldsTheSudoersLock: the sudoers lock held past the timeout. Install changes nothing; uninstall keeps the rule and the app, and its rerun removes both.
  • testInstallKeepsTheRuleWhenItsCopyBesideItFailsVisudo and testInstallKeepsTheRuleWhenTheRenameOverItFails: the copy beside the rule fails visudo, or the rename over the rule fails. The rule is unchanged and no copy is left beside it.
  • testInstallWritesTheRuleInOneLockedCompareAndRename: a fresh install expects no rule, stages the copy as root:wheel, checks it with visudo and leaves the rule 0440 and the lock 0600; a rerun expects the bytes it read.
  • testUninstallRemovesTheThreeCommandRuleOfALaterInstall: Sudoers: drop passwordless disablesleep 1; Start asks for the administrator password #32's three-command rule for this account is still removed.
  • testInstallAndUninstallTakeTheSameRootOnlySudoersLock: both scripts name /var/run/insomnia-sudoers.lock once and refuse a symlink there. Since 8d5184c the name is /private/etc/sudoers.d/.insomnia-sudoers.lock, the root-side checks are one text in both scripts, and neither script removes, replaces or repairs the lock.

Pre-fix check: in a scratch worktree at f20c230, the two race tests ran against the previous scripts, with the fake sudo's old bare-name arms gated the same way on install and rm. Both failed. This account's install exited 0 after bob wrote his rule and replaced it with its own, and the uninstall exited 0 and removed bob's rule, which it had never read.

New in 8d5184c. The fake sudo still runs the root shell unprivileged, only on fixture paths, with ROOT_UID set to the account running the tests. Nothing here is a real root or two-account run.

  • testInstallRefusesASudoersLockOnlyRootCouldNotHaveMade: a lock file with mode 0666 or 0640, a symlink, a FIFO, a hard link, a directory, another owner, a folder above it with mode 0777, the folder replaced by a symlink, and a rule outside the lock's folder. Each stops the install with exit 1 and the reason, without blocking on an open, with the lock's lstat (mode, owner, links, inode) and the rule's bytes unchanged, and no copy left beside the rule.
  • testInstallTakesARegularSudoersLockThatIsAlreadyThereAsItIs: an existing root 0600 lock file is used and kept, same inode.
  • testUninstallKeepsTheRuleWhenTheSudoersLockIsNotOnlyRoots: the same refusals in uninstall keep the rule.
  • testInstallKeepsARuleOnlyRootCouldNotHaveWritten: a rule with mode 0666, a hard link or a symlink is not replaced.
  • testInstallKeepsARuleReplacedWhileTheNewOneWasStaged: another account's rule written while the new copy is staged (by a rerun, or a fresh install) is kept.
  • testARuleChangedInPlaceAfterItsReadIsKept and testARuleWithANulByteIsKept: root compares the exact bytes it read through its own descriptor with the text the run judged, in both scripts.
  • testInstallKeepsTheRuleWhenStagingTheNewOneFails, testInstallStopsWhenWhetherTheRuleWasReplacedIsNotKnown and testInstallReportsAnUnknownResultWhenItsRenameIsKilled: a staging failure says the rule was not replaced; a root shell or an mv that ends by a signal after the rename says the result is not known, and the app and the LaunchAgent stay.
  • testUninstallKeepsTheAppAndJournalWhenTheRuleRemovalDoesNotAnswer, testUninstallKeepsTheAppAndJournalWhenItsRuleRemovalIsKilled and testUninstallLeavesTheLockToASudoCallThatIgnoresSigterm: a removal that hangs before or after its change, or whose rm ends by a signal, keeps the app and the journal; a sudo that ignores SIGTERM keeps the recovery lock until it ends and is named by pid.
  • testUninstallStopsWhenTheInstalledVersionCannotBeRead and testUninstallStopsWhenTheInfoPlistChangesAfterItsVersionWasRead: a version read that hangs, fails or meets a plist that does not parse, and an Info.plist replaced after its read, stop before any backstop runs.
  • testInstallMovesNeitherBundleWhenThePinnedRequirementReadDoesNotAnswer, ...ReadFails, ...ThePlistOnDiskDoesNotParse and testInstallStopsWhenTheNewPlistsLintDoesNotAnswerOrFails: install's reads under the lock.
  • testUninstallStopsWhenAJournalReadDoesNotAnswerOrFails and testUninstallKeepsTheJournalWhenReadingItAgainForAKeptBrightnessFails: uninstall's journal reads.
  • testInstallStopsBeforeAnySudoWhileAProcessOwnerCannotBeRead, testUninstallStopsBeforeAnySudoWhileAProcessOwnerOrIdentityCannotBeRead and testAnAPIClientWhoseOwnerCannotBeReadIsIgnored: a ps -o uid= that fails, does not answer, prints root? or prints nothing, and in uninstall an unknown bundle id, mean no sudo call at all, with the rule, bundle and agent unchanged; the API client is still ignored.

Existing tests changed in 8d5184c: uninstall's sudo -v before the lock is allowed where a test asserted no sudo call (ScriptFixture.runsAsRoot); testProcessWithoutAnIdentifiablePathBlocksUninstall gives pid 4242 this account's uid, since an unknown owner now stops earlier; testUninstallReadsNoInfoPlistUnderTheRecoveryLock expects the one version read before the lock. No assertion was removed.

New in c97535a. The fake sudo logs the sender of each SIGTERM and SIGHUP it receives through a small perl receiver, and the tests send the group's signals one at a time, since the kernel keeps one sender per process.

  • testTheSharedSupervisorOutlivesItsRunAndGroupSignalsAndHoldsTheLockUntilItReapsTheCall: the harness holds the lock and makes one bounded sudo call, whose fake closes its fd 9 as sudo does. The test kills the harness with SIGKILL, then sends SIGTERM, SIGHUP and SIGINT to its process group. The supervisor survives, sends its own SIGTERM at the limit and never SIGKILL, and keeps the lock until the call has exited and been reaped. One SIGTERM came from the test and one from the supervisor.
  • testUninstallsRootTransactionKeepsTheLockThroughGroupSignalsUntilSudoEnds: the same for uninstall's root transaction, with a fake sudo that removes the rule and then ignores SIGTERM. The app and the journal stay.
  • testTheSharedSupervisorGivesItsCallTheDefaultSignalActions: the call gets back the SIGTERM and SIGHUP actions the script started with.

New in e9e3037. The fake ls can add, fail or garble an ACL listing on a chosen call, the fake cat can print every byte and then fail, and the fake cp and plutil can hang or fail on a chosen journal copy.

  • Sudoers ACLs (R8-1): testRootRefusesAnAccessControlListThatAllowsAChange (an allow entry on the rule's folder, on one above it, one that reaches only files made there later, on the lock or on the rule: exit 7, or 4 for the rule, nothing repaired, the rule and its inode kept, no staged copy left, in both scripts), testRootAcceptsAccessControlListsThatOnlyDenyOrAllowReading, testRootStopsWhenAnAccessControlListCannotBeReadInFull (an ls that fails, a + with no entry, a line the check does not know) and testRootChecksAccessControlListsAgainJustBeforeItChangesTheRule (an entry that appears after root's first checks stops the run with exit 7).
  • Second read (R8-2): testRootKeepsARuleChangedAfterItsFirstRead (changed in place after root judged it or while visudo checks the staged copy, or replaced by a rename: kept, and no staged copy left), testRootStopsWhenItsReadOfTheRuleFails (cat prints every byte of a rule with no final newline and then fails: exit 8 at either read) and testRootTextWithoutIndentationDefinesTheSameFunctions.
  • Uninstall's journal reads (R8-3): testUninstallStopsWhenAJournalReadIsRefusedCutShortOrHasANulByte, through the real caller on the left of ||: a journal its owner cannot open, a plutil that prints a NUL byte and exits 0, and one that prints one byte and exits 2. Also testUninstallStopsWhenTheJournalBecomesAFIFOBeforeItIsCopied (the bounded cp is stopped at its limit and the FIFO stays), testUninstallStopsWhenASessionFileWithoutReadPermissionIsKeptByAFailedUndo and testUninstallKeepsARefusedBrightnessInAJournalOnlyAnACLMakesReadable.
  • The backstop's reads (R8-4): testBackstopUndoesNothingWhenItsCopyOfTheJournalDoesNotAnswer (the cp ignores SIGTERM, is killed and reaped, and never had fd 9), testBackstopStopsWhenAReadOfItsJournalCopyDoesNotAnswer, testBackstopKeepsTheJournalWhenAReadFailsAfterAnUndo and testBackstopTreatsASessionWhoseCopyDoesNotAnswerAsEnded.
  • Version evidence (R8-4): testBackstopKeepsAMicrosecondEntryWhenItsOwnVersionReadDoesNotAnswer, testBackstopUsesTheVersionEvidenceItsCallerPassesDown, testUninstallPassesItsVersionEvidenceToTheBackstopAndPrintsItsOutput, testUninstallFromAZipPassesTheVersionEvidenceToTheSealedCopy, testInstallKeepsAMicrosecondEntryWhenTheInstalledVersionCannotBeRead and testInstallReadsTheVersionOfASetAsideAppItPutsBack.
  • The backstop's limit (R8-4): testUninstallLeavesABackstopThatIgnoresSIGTERMRunningWithTheLock (never SIGKILL: uninstall stops three seconds after the SIGTERM with nothing removed, and the supervisor keeps the lock until the backstop ends), testUninstallStopsABackstopAtItsLimitWhileItsSudoKeepsTheLock (the real backstop at its limit while a sudo pmset it started runs on: the sudo gets no signal, and the backstop's own supervisor keeps the lock until it ends), testUninstallsBackstopCallKeepsTheLockThroughACrashAndGroupSignals and testInstallStopsWhenItsBackstopDoesNotFinishInTime (124 and 125).
  • testTheScriptsShareTheirReadersTextForText: the read layer, the journal and session checks, the version reader and the backstop call are the same text in each script that has them.

New in 2e67600: testBackstopReadsAJournalOnlyAnACLMakesReadableAndLeavesTheEntry. The journal is mode 0200 and its owner reads it only through an ACL entry, with nothing to undo. The backstop reads it through its private copy and publishes nothing, and the journal keeps mode 0200 and its one entry, main's assertion. With a chmod -N of the journal added to the backstop for the check, the test fails; the backstop was then restored byte for byte.

Existing tests changed in c97535a, e9e3037 and 2e67600. No assertion was removed, and the only one loosened is the ACL entry count below, whose old value 2e67600 checks again in its own test:

  • Main's testTheSupervisorOutlivesItsRunAndGroupSignalsAndHoldsTheLockUntilItReapsTheCommand, the test that failed on hosted CI at 8d5184c, keeps its count of exactly two SIGTERMs and every lock, no-SIGKILL, reap and kept-state check. It now sends the group's signals one at a time, checks each, and checks that one SIGTERM came from the test and one from the supervisor (R8-5).
  • Main's testBackstopKeepsAnOwnerACLAndStillUndoesTheJournal is now testBackstopReadsAJournalOnlyAnACLMakesReadableAndPublishesItOwnerOnly. Its 0200 journal, readable only through an owner ACL entry, has sleep left disabled. On main the backstop undid sleep, but its cp could not copy the extended attributes, so the publish failed and the dirty journal kept its entry: the test asserted one entry. Now the backstop publishes from its cp -X copy, a new owner-only file with no entry, and the test asserts none. Every journal main's backstop publishes also has no entry, since its cp copies no ACL.
  • Ten sudoers message checks expect "could not be shown to be" where they expected "is not", since the message now also covers a list that could not be read, and the second-read messages add "could not be read in full". testInstallKeepsARuleReplacedWhileTheNewOneWasStaged expects "was replaced after root read it".
  • testInstallAndUninstallTakeTheSameRootOnlySudoersLock compares five more root functions between the scripts and expects more than 150 lines, up from 60. testScriptsAndAppReadTheSameSessionDates checks for read failures in both scripts' read layers, not only the bounded one.
  • Every script fixture links its fakes to shared files, and the fakes name their fixture through FAKE_ROOT instead of a literal path (R8-6).

Mutation checks, each against a temporary change restored byte for byte afterwards: without root's ACL check, its three tests fail; without the second read, testRootKeepsARuleChangedAfterItsFirstRead and testRootStopsWhenItsReadOfTheRuleFails fail; with cat's status unchecked, the second of those fails; without the supervisor's restore of the default signal actions, its test fails; with 8d5184c's supervisor, both group-signal tests fail.

Run on b4ba734, after the merge of origin/main:

/usr/bin/lockf /private/tmp/insomnia-fable/swifttest.lock swift test --skip UIStatusTests --skip UIStartupTests   # 467 tests, 0 failures
swift build -c release -Xswiftc -warnings-as-errors   # ok
/bin/bash -n scripts/*.sh plus the CI grep for bash 4 constructs   # ok, /bin/bash is 3.2
shellcheck scripts/*.sh   # ok

UIStatusTests and UIStartupTests were skipped on purpose. They put real status items in the menu bar of the maintainer's Mac, and hosted CI runs them on every push.

The first full run on this commit had 6 failures, all in testCommandThatIgnoresSigtermIsNeverKilledAndKeepsTheLock (backstop pmset timeout, not touched here). Its log has a 46-second gap with no output, so the machine was likely paused. The test passed alone, and the full rerun above passed.

After cc5cca3 and 13a633b: /usr/bin/lockf /private/tmp/insomnia-fable/swifttest.lock swift test --skip UIStatusTests --skip UIStartupTests, 554 tests, 0 failures. Also swift build -c release -Xswiftc -warnings-as-errors, the 3.2 syntax check with the bash 4 grep, and shellcheck scripts/*.sh. The two menu bar suites were skipped for the reason above.

After c21f832 and 130032b: /usr/bin/lockf /private/tmp/insomnia-fable/swifttest.lock swift test --skip UIStatusTests --skip UIStartupTests, 558 tests, 0 failures. After d2c5087: 559 tests, 0 failures. Both runs skipped the two menu bar suites for the same reason. Also the release build with warnings as errors, the 3.2 syntax check with the bash 4 grep, and shellcheck scripts/*.sh. Main has since gained #46, with no conflict.

After 4eeae1e and acf1141: /usr/bin/lockf /private/tmp/insomnia-fable/swifttest.lock swift test --skip UIStatusTests --skip UIStartupTests, 624 tests, 0 failures. Also the release build with warnings as errors, the 3.2 syntax check with the bash 4 grep, and shellcheck scripts/*.sh. The two menu bar suites were skipped for the reason above.

After 38a9d1d: /usr/bin/lockf /private/tmp/insomnia-fable/swifttest.lock swift test --skip UIStatusTests --skip UIStartupTests, 626 tests, 0 failures, with the release build with warnings as errors, the 3.2 syntax check with the bash 4 grep, and shellcheck scripts/*.sh. The two menu bar suites were skipped again. The branch still merges into main at af9c4f6 without conflicts, so it was not merged again.

After b4b6d05, f20c230 and 729a91c, at 729a91c with a clean tree: /usr/bin/lockf -k /private/tmp/insomnia-fable/swifttest.lock swift test --skip UIStatusTests --skip UIStartupTests --skip KeychainStoreTests, 1192 tests, 0 failures, 0 skipped. The selection was listed first (1192 test IDs once the three skipped classes are left out), and every one of them started once and passed. That listing, swift test list --skip-build, ran without the three --skip flags. It runs no tests, and the 67 IDs of the three classes were removed from its output by name. Passing the flags would not have changed it: checked at 8d5184c, swift test list accepts them but still lists those classes. The three skipped classes are not run on the maintainer's Mac. Also swift build -c release -Xswiftc -warnings-as-errors, scripts/check-lid-simulation-gate.sh, CI's 3.2 syntax check with the bash 4 grep, and shellcheck scripts/*.sh (0.10.0). Before the commit, the 169 install and uninstall tests passed in a focused run, as did the ten new ones on their own.

At 8d5184c with a clean tree, 17:50:20Z to 18:14:46Z on 2026-10-08: /usr/bin/lockf -k /private/tmp/insomnia-fable/swifttest.lock swift test --skip UIStatusTests --skip UIStartupTests --skip KeychainStoreTests, 1216 tests, 0 failures, 0 skipped, exit 0. The head, tree and tracked file hashes were the same before and after. Then swift test list --skip-build --skip UIStatusTests --skip UIStartupTests --skip KeychainStoreTests, which runs no tests, exited 0 and listed 1283 IDs. The flags do not filter that listing: it still held the 67 IDs of the three classes, which were removed by name. The other 1216 are exactly the tests that started, each once, and all passed; none of the three classes started. The listings of this round's focused runs, and one more the run's own script made after it, were swift test list --skip-build without the flags, and the last one gave the same 1283 IDs. Every focused run itself passed all three --skip flags. At the committed scripts: swift build -c release -Xswiftc -warnings-as-errors (up to date from an earlier build of the same Swift sources), scripts/check-lid-simulation-gate.sh, CI's 3.2 syntax check with the bash 4 grep, and shellcheck scripts/*.sh (0.10.0), all passing. Before the commit, the 326 tests of RecoveryScriptTests, LidCloseCopyTests, ReleaseWorkflowTests and PackagingTests passed in a focused run, which included all 24 new tests. During that 326-test run a ShellCheck directive comment was added to install.sh, so the run covered the committed scripts except that comment line.

At 2e67600 with a clean tree, 01:27:01Z to 01:43:02Z on 2026-10-09: /usr/bin/lockf -k /private/tmp/insomnia-fable/swifttest.lock swift test --skip UIStatusTests --skip UIStartupTests --skip KeychainStoreTests, 1246 tests, 0 failures, 0 skipped, exit 0, in 959.9 XCTest seconds. The head, the tree and the hashes of the three scripts and the test sources were the same before and after, with no MERGE_HEAD and nothing uncommitted. Then swift test list --skip-build --skip UIStatusTests --skip UIStartupTests --skip KeychainStoreTests, which runs no tests, exited 0 and listed 1313 IDs. The flags do not filter that listing: it still held the 67 IDs of the three classes, which were removed by name. The other 1246 are exactly the tests that started, each once, and all passed; none of the three classes started. pmset -g log shows no sleep or wake from 18:16:35 local, ten minutes before the run, until after it. Every listing this round, for the full run and for each focused run, passed the three --skip flags. An earlier full run at e9e3037 was stopped by hand after 757 passes and no failures, because 2e67600's test change was still to come; it is not counted. At the committed scripts: swift build -c release -Xswiftc -warnings-as-errors, scripts/check-lid-simulation-gate.sh, CI's 3.2 syntax check with the bash 4 grep, and shellcheck scripts/*.sh (0.10.0), all passing. 2e67600 changes only a test file, which none of these builds. The workflows are unchanged; actionlint and zizmor are not installed on this Mac, so CI's workflow lint job checks them. Before each commit the new and changed tests passed in focused runs, and before e9e3037 the 411 tests of the eight classes that run the scripts passed, 411 of 411.

Codex review

  1. From a local Codex review of b4ba734. [P1] scripts/uninstall.sh:225: the -u "$UID_NUM" filter on pgrep let uninstall remove another account's active sudoers rule. Fixed in 13a633b.

    • The filter is gone from both scripts. Every process named Insomnia is checked, and its owner is read with ps -o uid=. A copy of this app in another account (the installed path or this bundle id), or a process there that cannot be told apart from one, is a blocker. Uninstall stops at its first step with nothing removed. Install stops before the sudoers step with nothing changed, and checks again at the quit step. The message names each pid with its uid and says the shared rule is left alone. Nothing is sent to that process: the AppleScript quit goes only to this account's copy, and only when no other account has one. A pid whose uid cannot be read is judged as this account's, so it still blocks as before. Since 8d5184c a pid whose uid cannot be read is not judged as this account's: after a second look it stops both scripts before their first sudo call (Codex review 5). A process proven to be another app stays ignored in any account.
    • Uninstall reads the rule with sudo cat and removes it only when every line is blank, the install.sh header comment, or one of the four pmset grants to this account (id -un, now through an ID constant). A grant to another account means that account installed last: the file is kept, and the message names that account and says to uninstall there. Any other line keeps the file with the line quoted and the sudo rm command to run if nothing needs it. A failed read stops before the app is removed.
    • The comparison is line by line, not a byte compare with install.sh's output. Install: build the sudoers rule from the account, show it, and confirm #20 rewrites the header comment ("Exactly 4 commands") and another open PR drops the disablesleep 1 grant, so a byte compare would refuse this account's own rule after either lands. Each line must still be one of the grants install.sh has written, to this account.
    • Greptile's comment on the same filter (r4167961287) is fixed by the same commit.
  2. From the Codex review of d2c5087. [P0] scripts/install.sh:181: the cross-account guard could revoke the grant needed to recover a crashed session. Fixed in acf1141.

    • The guard only looked for a running Insomnia in other accounts. After that account's app crashed, nothing was left to find, and install replaced the one rule at /etc/sudoers.d/insomnia. That account's recovery agent then lost the grant it needs to undo the session.
    • Install now decides from the rule itself, not from processes. Before it writes, it reads the existing rule through sudo cat, as uninstall does, and stops with nothing changed if any line is for an account other than $USER. The message names the account and says to uninstall in that account first, or to run sudo rm /etc/sudoers.d/insomnia if that account no longer exists. Uninstall already keeps a rule that grants another account and says to uninstall there, so both scripts treat that rule the same way.
    • A line is judged by its first field, the account it names. The pmset commands do not matter, so this account's rule from any version of install.sh, including Sudoers: drop passwordless disablesleep 1; Start asks for the administrator password #32's shorter one, is replaced as before. A line that names no single account (a %group, an alias, ALL, Defaults, an include) also stops the install, because the new file would drop it. So does a rule that sudo cannot read.
    • The process check for other accounts stays. It still keeps install from quitting or replacing anything while another account's copy runs, before any sudo call.
    • 38a9d1d replaced the first-field check described above, see 3.
  3. From the Codex review of acf1141. [P1] scripts/install.sh:173: numeric UID grants bypass the cross-account sudoers guard. Fixed in 38a9d1d.

    • The guard skipped every line starting with # as a comment. Sudoers reads #502 at the start of a line as user ID 502, so a rule for another account's uid passed and install replaced it. That account's backstop could then no longer restore sleep after a crash. The guard also looked only at the first word, so <account> , bob ALL=... passed as this account's line.
    • The guard now fails closed instead of listing forms. A line is this account's only when its user field is exactly $USER or #$UID_NUM and no comma after it carries on the user list. Any other field stops the install: another name or user ID, %group, %#gid, +netgroup, an alias, ALL, a quoted name, Defaults, an include, a continued line.
    • # starts a comment as it does for sudo (sudoers(5): a comment unless it is part of #include or is followed by digits in a user name). A # followed by a digit, or by - and a digit, is a user ID. #include and #includedir stop the install as before.
    • The check still reads only the user field, never the commands, so Sudoers: drop passwordless disablesleep 1; Start asks for the administrator password #32's three-line rule passes for its own account.
    • Uninstall already compared whole lines with the rule install.sh writes for id -un, so it kept a #502 rule. A test now covers that.
  4. From the Codex review of 38a9d1d. [P1] scripts/install.sh:236: the sudoers ownership check races with another account's installer. Fixed in 729a91c for runs that take its lock. Codex review 5 found that lock was not root-only; 8d5184c replaced it. Codex review 6 found that 8d5184c's checks left out ACLs and that its last check compared only the rule's inode; e9e3037 adds both checks.

    • Correction: until 729a91c this item said "Not changed; the maintainer accepted it as a known gap." That was not true. The maintainer did not accept or waive this finding, and it stayed open until 729a91c. The two bullets the item had are kept here, struck through, as the reasoning that was given then:
    • Two accounts would have to run install.sh at the same moment, and one would have to pause between the ownership read and the write while the other finishes, starts a session and its cached sudo credentials expire. Closing it needs a machine-wide lock that only root can own, shared by both scripts in every account.
    • main has no ownership check at all, so this PR is strictly safer than main for every ordering. The gap is listed under "Not covered".
    • The race: install.sh read the rule through sudo, judged it, and wrote it in a later sudo call; uninstall.sh read it and removed it the same way. Another account's install.sh could write its rule in between. The later write replaced that account's grant, or the removal deleted it, and that account's recovery agent could no longer undo a session.
    • The fix: the compare and the change are one sudo /bin/bash -c call. As root it takes /var/run/insomnia-sudoers.lock with lockf -t 10, under umask 077 and refusing a symlink there. /var/run is writable only by root and the daemon group, so no account can create, swap or hold the lock. (Not root-only: a member of the daemon group, or a lock file left there by anyone who could write it, defeats it. 8d5184c moved the lock and checks it; see Codex review 5.) Under the lock it checks the rule is still what the run read: absent, or the same bytes by cmp against the copy read through sudo /bin/cat. Only then does install.sh mktemp a copy beside the rule (sudo skips a name with a dot), chown root:wheel and chmod 0440 it, check that copy with visudo -cf, and mv -f it over the rule; uninstall.sh runs rm -f. A failure removes the copy and leaves the rule. The shell's script is the function's text after the scripts' fixed tool paths, and sudo resets the environment, so nothing root runs comes from PATH. 8d5184c replaced the cmp against a file read through sudo /bin/cat: the text the run judged is passed to root, which compares it with the bytes it reads through its own descriptor.
    • If the rule changed since the read, install.sh stops with "changed after this install read it ... Nothing was changed." and uninstall.sh keeps the rule and the app ("Kept ...: it changed after this uninstall read it."). Both ask for a rerun, and the rerun reads the new rule and refuses it if it grants another account. A lock still held after 10 s stops the run the same way. Neither script changes the rule unless it is still what that script read and judged, as far as the other writer takes the same lock. Since 8d5184c these messages end "The app and the LaunchAgent were not touched." instead of "Nothing was changed.", since the run may have created the lock file.
    • Unchanged: the other-account process stop runs before the first sudo call; the rule grants this account the same four pmset commands and nothing more; install.sh still checks the new rule with sudo visudo -cf before the transaction. Nothing new runs as root outside the install and uninstall themselves: no helper, daemon or extra sudoers grant.
    • Costs: a second account that installs while the first is between its read and its write now has to rerun. The lock is a 0-byte file in /var/run that stays once created. Since 8d5184c the lock is a 0-byte file, /etc/sudoers.d/.insomnia-sudoers.lock, that stays once created. A refused uninstall keeps the app until its rerun. The race was tested with fakes only (see Tests); a real two-account run is a "Not run" row in docs/release-validation.md.
  5. From the Codex review of 729a91c (round 6), which denied the merge. Answered in 8d5184c.

    • [P1] The sudoers lock was not root-only, and root compared a file the caller could still change. /var/run is group-writable for daemon, so a lock file there may not be root's. The lock is now /etc/sudoers.d/.insomnia-sudoers.lock, a dot-named file sudo's include skips, in the rule's own folder. Before any open, root checks every folder up to / is root's and not writable by group or others, and an existing lock file is a regular file of root's with mode 0600 and one link, so a FIFO or a link is never opened. After lockf, the descriptor and the path must still be that file. The file is created (umask 077, noclobber) only where nothing is, and nothing ever repairs, re-owns, re-modes, unlinks or replaces it or its folder: a check that fails stops the run with the rule and its grants exactly as they were. Root opens the rule on a descriptor after the same owner, mode and link checks, reads it there, compares the exact bytes with the text the run judged (passed as an argument, never a file), judges them again, and renames or removes only while the path still names that file. No real root exploit is claimed: the review's split-inode result modelled an actor allowed to replace the lock's folder entry, and the fixture tests above run the root shell unprivileged. Codex review 6 found that these checks left out ACLs and that the last one compared only the rule's inode; e9e3037 adds both (R8-1, R8-2).
    • [P1] Reads under the recovery lock. uninstall.sh reads InsomniaResumeFrozenVersion before the lock with bounded calls and the file's identity before and after, and under the lock only a bounded stat checks that it still applies; a read that fails, does not answer or sees a change stops it before any backstop runs. Under the lock, install's plist_pins_previous and candidate plutil -lint are bounded. Uninstall copies the journal into its own private folder with a bounded cp and reads that copy: the extract, type and json reads, the journal checks and the kept-brightness reread through bounded plutil calls, the raw text checks with the shell's own reads. A failed or silent reader is never clean. (Codex review 6 found a failed raw read could still count as clean; e9e3037 fixes that, R8-3.) Uninstall's sudo test, sudo cat and the removal run through bounded() with sudo -n; a sudo still running past the limit keeps the lock until it ends (125) and the run stops with the app and the journal kept. The messages that said only exit 0 changes the rule, or "Nothing was changed" after the root call, are narrowed: an mv or rm killed by a signal now reports an unknown result, and the lock file may have been created. The backstop.sh the scripts run under the lock is main's and is not bounded here. e9e3037 bounds it (Codex review 6, R8-4).
    • [P2] An unknown owner counted as this account's. See "What" above. The pid, bundle and id cache, the named messages, the absence of any Info.plist read in the process check under the lock, and the rule never to signal another account's process or send a broad quit are unchanged. A known own copy still gets install's authenticate-before-quit order. uninstall.sh asks this account's own copy to quit before sudo -v, as on main.
    • Old lock and older writers: 729a91c's /var/run/insomnia-sudoers.lock was never on main. 8d5184c neither reads nor removes it, so a file it left stays there, unused. A 729a91c script and an 8d5184c script take different locks and do not exclude each other, and neither excludes scripts of earlier releases or an administrator's own sudo. Against those, the checks narrow the window to the moments between root's last check and its rename or removal; they do not close it.
    • Costs, reported for the maintainer to decide, not waived: an administrator whose sudo policy lists only some commands and not /bin/bash cannot run the transaction, so cannot install or remove the rule this way. Options that keep the grant as it is: such an administrator adds /bin/bash to their own sudo policy, or the maintainer accepts that such accounts cannot use the installer. A root-owned helper with its own sudoers entry, or a fallback to separate non-transactional calls, would change the privilege design or reopen the race, so neither is done here. The root shell's text is long, about 6.8 KB for install and 5.1 KB for uninstall, and appears as the command in sudo's log; it is the functions' text only, with no comments. Since e9e3037 it is 7,859 bytes for install and 6,699 for uninstall (see Codex review 6).
  6. From the Codex review of 8d5184c (round 8), which found it needed changes. Answered in c97535a and e9e3037; 2e67600 brings back a main assertion e9e3037 had changed.

    • R8-1 [P1] The sudoers guard did not check access control lists. Fixed in e9e3037. Root reads /bin/ls -lde for every folder from the rule's up to /, for the lock file and for the rule. An allow entry with any right beyond read, execute, readattr, readextattr, readsecurity, list or search (inheritance flags aside) stops the run, whoever it names: exit 7 for a folder or the lock, 4 for the rule. Deny entries pass. A list ls cannot print, or one the check cannot parse, stops the run too. Root checks the folders before it creates the lock file, the lock file before it opens it, the rule before it opens it, and all of them again just before the change. Nothing is repaired: the entry stays, and the message names the path and the entry. Cost: the check goes by the rights an entry allows, not by whom it names, so an entry for root, or one a management profile added, stops the run too, though it may be harmless, until someone removes it by hand. On the Mac these scripts were tested on, /, /private, /private/etc and /private/etc/sudoers.d carry no ACL.
    • R8-2 [P1] The last check compared only the rule's inode. Fixed in e9e3037. Root checks cat's exit status when it reads the rule (the review's unchecked PINNED_TEXT read). Just before mv or rm it opens the rule again on a new descriptor and reads it a second time, and changes the rule only when that read completes, finds no NUL byte, and gives the same bytes from the same device and inode. Otherwise it keeps the rule: exit 8 for a read that fails, is cut short or holds a NUL byte, 4 for a change. A writer that takes no lock can still change the rule between that read and the rename or removal, and no check of the file's identity or bytes can see that. A rule that keeps changing keeps every run from changing it.
    • R8-3 [P1] A failed raw journal read could count as clean. Fixed in e9e3037. This also corrects the "Not covered" bullet that said such a read could not be faked and that set -e would end the check early: set -e is off at that caller, which runs the check on the left of ||. uninstall.sh copies each journal file once with a bounded cp -X into its private folder and checks the copy, and every read there has an explicit status and the 30 s limit, without relying on set -e. A read that fails, is cut short, prints a NUL byte or does not answer counts as a problem, never as a clean journal, and the uninstall stops before anything is removed. A failed reread for a kept brightness keeps the journal. A session.json still there after the backstop stops the uninstall, readable or not. Main's checks of the JSON shape, escapes, duplicate keys, number ranges, boot, true zero and kept brightness (Display: refuse the private brightness calls on an unmeasured macOS or a changed KeyboardBrightnessClient #43) are unchanged and still tested.
    • R8-4 [P1] The backstop each script runs under the recovery lock was not bounded. Fixed in e9e3037. The backstop reads the journal and the session through private copies made with a bounded cp -X, and every read has a checked status and a 30 s limit. It reads no Info.plist under the lock: install.sh and uninstall.sh read InsomniaResumeFrozenVersion and the file's identity before the lock and pass both down, and the backstop uses the version only while a bounded stat under the lock shows the same file. Run on its own by the agent, it makes the same reads itself before it takes the lock. Both scripts run the backstop through the shared supervisor with a 300 s limit and SIGTERM only, never SIGKILL. A backstop still running three seconds after its SIGTERM keeps the lock until it ends (125), and the run stops, names its pid and replaces or removes nothing. Each sudo pmset or app binary call the backstop started keeps the lock through the backstop's own supervisor until that call exits. Cost: a backstop sealed in a bundle from an earlier release, which an uninstall from that release's zip runs, does not know the new variables. It reads the Info.plist and the journal itself with no limit on each read, though the 300 s limit on its run still applies.
    • R8-5 [P2] The hosted supervisor test counted three SIGTERMs. Answered in c97535a. The fake sudo's wait loop ran $(date), and a command substitution child copies the logging SIGTERM trap, so one group SIGTERM could be logged twice. In a private probe of that loop with its sleep taken out, 7 of 21 trials logged one group SIGTERM twice, every extra line from a command substitution child; the new loop logged it once in 30 of 30. With the sleep left in, as in the fixture, the old loop logged it once in 60 of 60, so the probe shows how a third SIGTERM can appear, not that this caused the hosted one. The loop now waits on bash's SECONDS. The fake logs the sender of each SIGTERM and SIGHUP, and the test sends the group's signals one at a time. The count of exactly two SIGTERMs and every lock, no-SIGKILL, reap and kept-state check are unchanged, and the test now also checks that one SIGTERM came from the test and one from the supervisor. The hosted run itself has not been repeated here.
    • R8-6 [P2] The hosted suite exceeded its 20-minute watchdog. Answered in e9e3037. Most of the time went to macOS checking each new executable file the first time it runs (85 to 400 ms per file on the test Mac), and every fixture wrote its fakes as new files. Now each distinct fake text is one file per test process, and every fixture's bin links to it. On the 382 tests of the script-running classes that ran awake in both measured runs, the time fell from 1,718 s to 766 s, 55% less. The escaped name or UID uninstall test went from 86.8 s to 45.3 s. Across the two full runs, both awake and with the same command, the 1,215 tests common to 8d5184c and 2e67600 took 1,464 s and then 852 s, though the scripts at 2e67600 do more work; the 31 new tests add 108 s, for 960 s in all against 1,465 s. The valid kept-display uninstall test went from 35.1 s to 16.1 s, and the malformed kept-display uninstall test, unfinished when the hosted watchdog fired, from 39.5 s to 15.4 s. No case or assertion was removed, no deadline was loosened, no timeout was raised, and the workflow is unchanged. What is left: each fixture still writes its own app binary, backstop copies and build stub, patched with its own paths, and each of those pays the first-run check. Whether the hosted job now fits its watchdog is known only from a hosted run. If it does not, a larger budget or a split of the job is the maintainer's choice; this PR makes neither.
    • R8-7 [P1] A SIGTERM to the run's process group ended the shared supervisor. Fixed in c97535a. The supervisor in install.sh and uninstall.sh now ignores SIGTERM and SIGHUP, as backstop.sh's does, and gives its call the signal actions the script started with, so a sudo call still ends on its SIGTERM at the limit. A signal to the run's whole process group no longer ends the supervisor while a sudo call that closed fd 9 still runs, so the recovery lock stays held until that call exits and is reaped.
    • Root text: since e9e3037 the -c text root's shell runs is 7,859 bytes for install and 6,699 for uninstall. The ACL checks and the second read added to it, and dropping each line's leading spaces took some back (a test checks that bash reads the same functions from the shorter text). It is still the functions' text only, with no comments.
    • Unchanged and not waived: writers that take no lock or another one, an administrator whose sudo policy does not allow /bin/bash, the stop for an unknown owner or another account's copy before the first sudo (sudo -v included), the process cache, own-name and numeric-UID lines, and Sudoers: drop passwordless disablesleep 1; Start asks for the administrator password #32's three-command rule. Real root, real ACLs and real two-account runs are "Not run" rows.

Decisions

  • pgrep plus a per-pid ps -o comm= check instead of lsappinfo. It works without a GUI LaunchServices session (SSH), needs only tools the fixture already fakes, and matches by either the installed path or the bundle id, as the review asked. lsappinfo find bundleid=... prints nothing and exits 0 whether or not the app runs, so it would have needed more parsing for less information.
  • pgrep covers every account. Another account's copy does not share this account's journal, but it does share the one sudoers file and may run from this account's bundle, so it blocks. This account cannot quit it, so it is reported and the run stops. (This replaces the earlier decision to filter with -u "$UID_NUM", which the Codex review found unsafe.)
  • Only the API client's bundle id proves another app (130032b). A copy of this app with an edited Info.plist would still share this account's journal, and a bundle id is the only identity the scripts can read without trusting the process. The cost is that any other app named Insomnia, or a stray ./Insomnia run from a shell, holds the install or uninstall until it exits. The refusal names the pid, the path and the bundle id, so the user can find it.
  • Install refuses a rule that grants another account instead of keeping that account's lines in the new file (acf1141). Keeping them would mean writing grants for an account the installer does not own, from lines it would have to trust. The cost is that a second account cannot install until the first one uninstalls. With one rule per Mac that was already true, but before, the second install broke the first account's recovery without saying so.
  • install.sh gets no time-limit helper of its own. Not reading under the lock is enough there: a read that hangs before the lock stops only the installer, which the user can interrupt, and nothing else waits on it. (Main has since given install.sh bounded(); b4b6d05 runs find_insomnia's calls through it, and f20c230 gives uninstall.sh the same no-read rule under the lock.)
  • The sudoers lock is a file in /var/run in /etc/sudoers.d (8d5184c) that root opens, not a lock in either account's home. A lock in a home, or one any account could create, would let one account hold or replace it for another. It is taken only inside the sudo call, after the password prompt, and only uninstall.sh holds the recovery lock while it waits for it, so the two locks are always taken in the same order.

Not covered

🤖 Generated with Claude Code

RetriggerConfidence Score: 5/5

The PR appears safe to merge; no new actionable issue remains.

Summary

The PR distinguishes this app from the Insomnia API client and protects the shared sudoers rule across accounts. The latest changes strengthen recovery reads and the checks made before changing that rule.

  • Install and uninstall tell this app apart from other processes named Insomnia.
  • Install and uninstall keep the shared recovery rule with the account that owns it.
  • Recovery stops when a key read or command cannot finish cleanly.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Check running processes and owners] --> B[Read app version before recovery lock]
  B --> C[Take recovery lock]
  C --> D[Run backstop with time limit]
  D --> E{Recovery completed?}
  E -- No --> F[Keep app and recovery files]
  E -- Yes --> G[Check shared sudoers rule under root lock]
  G --> H[Check ACLs and reread rule]
  H --> I{Same file and bytes?}
  I -- No --> F
  I -- Yes --> J[Replace or remove rule]
Loading

Reviews (11) · Last reviewed commit: "Tests: keep main's check that the backst..." · Reviewed by Greptile

… not by name

Both scripts decided the app was running with `pgrep -x Insomnia`. The
Insomnia API client's executable has the same name, so with that client
open the AppleScript quit (sent by bundle id) did nothing, the scripts
waited out QUIT_WAIT_SECONDS and refused with a message that sounded like
this app would not quit. Anyone who already uses the API client hit this
on day one.

Each pid pgrep reports (restricted to this user) is now checked by its
executable path from `ps -o comm=`, which is the full path for an app
LaunchServices launched: it is this app when the path is the installed
bundle's binary or lies in a bundle whose Info.plist declares
com.kgarg.insomnia (a development build with the same bundle id shares
the journal and the lock, so it counts). Anything else is reported with
its pid, path and bundle id and left alone. Every refusal names the pid
and executable path of the copy that is still running.

The Swift app does not look for other instances by process name, so
nothing changes there.

Tests: the fake pgrep prints pids, the fake ps answers `-o comm=` from a
table, and five fixture tests cover the API client running during
install and uninstall, a same-bundle-id copy at another path, a pid ps
cannot describe, and the refusal naming the right process.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Comment thread scripts/install.sh Outdated
…d blocks

A process named Insomnia whose bundle id could not be read (no path from
ps, a path outside any bundle, or an Info.plist that does not parse) was
put in the same bucket as the Insomnia API client and ignored. A
development copy of this app whose Info.plist breaks while it runs would
then not hold up the installer, which replaced the installed bundle, or
the uninstaller, which removed the recovery files it still shared.

Only a readable bundle id that is not ours proves another app. Anything
else named Insomnia now counts as this app until it exits: it is never
signalled, the quit is still only sent to our bundle id, and both
scripts wait and then refuse, naming each unverified pid, its path and
why it could not be identified. The API client, with its readable
bundle id, is still reported and left alone.

Tests: the "not identifiable means not this app" uninstall test becomes
a refusal; new install and uninstall tests run a copy whose Info.plist
no longer parses, with the API client beside it in the uninstall case.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Comment thread scripts/install.sh
krishhgg and others added 2 commits October 2, 2026 00:41
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
When the only process named Insomnia was unverified, the scripts held
the install or uninstall as intended but still sent
`tell application id "com.kgarg.insomnia" to quit`. An unrelated
process with an unreadable identity could therefore ask the real app
to quit. The quit request now sits inside the same check as the
"Insomnia is running" message: it is sent only when a copy was
identified by path or bundle id. An unverified process is still waited
for and still blocks until it exits.

Tests: install and uninstall each run with one unverified process that
exits during the wait; neither sends osascript, and both go on.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Comment thread scripts/install.sh Outdated
krishhgg and others added 2 commits October 2, 2026 15:38
main moved to a066607 (#44, #24, #35, #42, #27, #45). Conflicts:

- scripts/uninstall.sh: main bounded app_running's pgrep with a time
  limit (a pgrep that does not answer counts as running), and this
  branch replaced app_running with find_insomnia, which checks each pid's
  bundle id. Kept find_insomnia and ran its pgrep and ps calls through
  main's bounded helper. A pgrep timeout prints main's message and
  blocks as an unverified process; a ps that fails or times out leaves
  the pid unverified, so it still blocks.
- RecoveryScriptTests.swift: the pgrep fake keeps main's "hang" line and
  this branch's pid list.
- docs/release-validation.md: kept both sides' new rows.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… that is not ours

Codex review of the previous head, [P1] uninstall.sh:225: the -u filter
on pgrep hid another account's running Insomnia, so uninstall could
remove the sudoers rule that copy needs to undo its session.

- pgrep lists processes named Insomnia in every account, and each pid's
  owner is read with ps -o uid=. A copy of this app in another account,
  or a process there that cannot be told apart from one, is reported
  and stops the run: uninstall before anything is removed, install
  before the sudoers step. It is never asked to quit or signalled. A
  process proven to be another app is still ignored in any account.
- uninstall reads /etc/sudoers.d/insomnia through sudo and removes it
  only when every line is blank, the install.sh header comment, or one
  of the pmset grants install.sh writes, to this account (id -un). A
  grant to another account keeps the file and names that account; any
  other line keeps it and quotes the line. A failed read stops before
  the app is removed.
- uninstall.sh calls id through a fixed-path ID constant.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Comment thread scripts/uninstall.sh Outdated
Comment thread scripts/uninstall.sh Outdated
krishhgg and others added 2 commits October 2, 2026 16:55
Greptile (r4170641462): find_insomnia read each bundle's Info.plist with
plutil and no time limit, and both scripts run it again after taking the
recovery lock. An Info.plist on a stalled volume could hold the lock that
the app and the backstop need.

uninstall.sh now reads it through `bounded` like every other call there.
A read that does not answer within CALL_TIMEOUT_SECONDS is stopped, the
process counts as unverified, and the message says the plist did not
answer.

install.sh has no time limit for a call, so it reads no Info.plist once
it holds the lock. Ids read before the lock are kept per bundle and used
again under it. A process first seen under the lock counts as unverified
and stops the install before the LaunchAgent is touched.

Tests: the fixture's plutil is now a wrapper around the real tool that
records Info.plist reads and can make one hang. New tests cover a hung
read under uninstall's lock and a process first seen under install's
lock. The foreign-process install test also checks that the client's
plist is read once.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Greptile (r4170641456): a copy of this app whose Info.plist declares some
other bundle id was taken for another app and left out of the quit guard,
although it would use this account's journal. Uninstall could then remove
the backstop while it ran.

Only com.insomnia.app, the Insomnia API client's id (from its Homebrew
cask), now proves a process is not this app. A process named Insomnia
with any other readable id counts as unverified, in this account, or as a
copy in another account. It blocks until it exits and is never asked to
quit. The message names the id.

Tests: an unknown id blocks the install, and blocks the uninstall in this
account and in another one, with no quit, signal or removal. The
API-client tests are unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Comment thread scripts/install.sh Outdated
krishhgg and others added 12 commits October 2, 2026 17:08
Greptile (r4170821609): c21f832 kept the bundle ids read before the
recovery lock per bundle path. If a copy of this app took the API client's
place at the same path while the installer waited for sudo or for a quit,
the check under the lock would still take it for the client and go on.

The cache is now keyed by pid and bundle path. A process that took a
bundle's place has another pid, so under the lock it counts as first seen
there and stops the install.

Test: the fake pgrep takes a "pids:A,B" line. The client at pid 4242
before the lock and another process at its path as pid 5151 under the
lock stop the install, and the client's plist is read once.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Main gained #46, #36, #23, #17 and #21 since a066607. Two conflicts:

- scripts/uninstall.sh: this branch added the ID constant and #23 added
  DATE, MKDIR, RM, RMDIR and MKTEMP in the same block. Kept all six.
- RecoveryScriptTests class comment: kept #23's note on the frozen date
  stamp and this branch's notes on id and the plutil wrapper.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codex review of d2c5087 (P0, scripts/install.sh:181): the cross-account
guard only looked for a running Insomnia in other accounts. Once that
account's app had crashed, nothing was left to find, and install replaced
the one rule at /etc/sudoers.d/insomnia, taking away the grant that
account's recovery agent needs to undo the session.

Install now reads the existing rule through sudo, the same way
uninstall.sh does, before it writes anything. It judges each line by the
account it names, not by its commands, so this account's rule from any
version of the script (including #32's shorter one) is still replaced.
A line for another account stops the install with nothing changed and
says to uninstall in that account first, or to remove the rule by hand if
that account no longer exists. A group, alias, Defaults or include line
also stops it, since the new file would drop that line, and so does a
rule that sudo cannot read.

The install tests now run with USER set to the account the fixture's
rule names, so the existing rule is this account's own.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codex review of acf1141 (P1, scripts/install.sh:173): the cross-account
guard skipped every line starting with "#" as a comment, but sudoers
reads "#502" at the start of a line as user ID 502. A rule written for
another account's uid passed, and install replaced it, which leaves that
account's backstop unable to restore sleep after a crash. The guard also
took "me , bob ALL=..." as this account's line, since it looked only at
the first word.

The guard now fails closed. A line is this account's only when its user
field is exactly $USER or #$UID_NUM and no comma after it carries on the
user list. Every other field refuses the install: another name or user
ID, %group, %#gid, +netgroup, an alias, ALL, a quoted name, Defaults, an
include, a continued line. "#" starts a comment as it does for sudo, not
when a digit or "-" and a digit follows, and not in #include or
#includedir. The check still looks only at the user field, so #32's
three-line rule passes for its own account.

Tests: a rule for another uid is refused with nothing changed, the
table test covers each of those forms, the own-rule test adds a grant to
this account's uid and comment lines starting "#-" and "#", and uninstall
keeps a rule for another uid.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Brings in #47, #38, #40, #30, #41, #25, #48 and #19. Two conflicts, both
resolved by keeping both sides:
- scripts/uninstall.sh constants: this branch's BUNDLE_ID and
  CLIENT_BUNDLE_ID, and #41's RESUME_FROZEN_VERSION.
- docs/release-validation.md: this branch's two install/uninstall rows
  and #19's permissions row.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Keeps both sides. Conflicts resolved beyond taking both:

- install.sh: main's file with this branch's process identity
  (find_insomnia, KNOWN_IDS, PLIST_READS), the other-account stop before
  the first sudo call, the sudoers ownership check and the APP_FOUND-only
  quit. find_insomnia now runs pgrep, ps and the pre-lock plutil read
  through main's bounded(), so no call it makes under the recovery lock is
  unbounded. A pgrep that fails or does not answer blocks (PGREP_PROBLEM):
  before the sudoers step it stops the run, and under the lock it keeps
  main's "whether Insomnia started again is unknown" message. The second
  BUNDLE_ID line is dropped; main's constant serves both uses.
- Tests: main's install tests run with USER set to the account running
  them (the ownership check refuses a rule for "tester"), expect the pgrep
  before the sudoers step, and read the fixture's real rule back.
- README, release validation: both texts.

testUninstallTreatsAnInfoPlistThatDoesNotAnswerUnderTheLockAsUnverified
fails at this commit: main's bounded() keeps fd 9 for a call under the
lock, so a bounded Info.plist read there now holds the lock. The next
commit stops uninstall reading any Info.plist under the lock.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…pgrep failure

Main's bounded() keeps fd 9 for a call made under the recovery lock, so
the bounded Info.plist read uninstall.sh made there now held the lock for
up to CALL_TIMEOUT_SECONDS. uninstall.sh now does what install.sh does:
bundle ids read before the lock are kept per pid and bundle (KNOWN_IDS)
and reused for the same process only, and under the lock no Info.plist is
read (PLIST_READS=0). A process first seen there counts as unverified and
stops the run.

A pgrep that exits with anything but 0 or 1 lists nothing, which does not
show that no copy runs. uninstall.sh treated it as "not running"; it now
blocks like a pgrep that does not answer (PGREP_PROBLEM), and the quit
step stops on it, as install.sh's sudoers step does.

Tests: the Info.plist test that failed after the merge now checks that no
Info.plist is read under the lock; a new one checks that the read before
the lock is stopped at the limit and the run goes on. Two more cover a
failing pgrep in install.sh and uninstall.sh.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…till what was read, under a root-only lock

/etc/sudoers.d/insomnia is one file for the whole Mac. install.sh read it
through sudo, judged whose it was, and wrote it in a later sudo call;
uninstall.sh read it and removed it the same way. Another account's
install.sh could write its rule in between, and the later write replaced
that account's grant, or the removal deleted it (Codex P1 on 38a9d1d,
which the PR body had wrongly called accepted).

Now the compare and the change are one `sudo /bin/bash -c` call. As root
it takes /var/run/insomnia-sudoers.lock with lockf (umask 077, a symlink
refused; /var/run is writable only by root and the daemon group), checks
the rule is still absent or the bytes this run read (cmp), and only then
writes or removes it. install.sh stages the new rule beside the old one
(a name with a dot, which sudo skips), makes it root:wheel 0440, checks
that copy with visudo and renames it over the rule; a failure removes
the copy. The root shell's script is the function's text after the
scripts' fixed tool paths, so nothing it runs comes from PATH. A rule
that changed since the read, or a lock held past LOCK_TIMEOUT_SECONDS,
stops the run: install.sh changes nothing, uninstall.sh keeps the rule
and the app, and both ask for a rerun. The sudo reads of the rule now use
fixed paths (/bin/test, /bin/cat, /usr/sbin/visudo).

The grants do not change, nothing new runs as root outside the two
scripts, and the other-account process stop still comes before the first
sudo call.

Tests: the fake sudo runs the transaction unprivileged only when the
rule and the lock in its script are inside the fixture, and a gate file
holds it between a run's read and its write. New tests cover two
accounts installing from two homes at once, an uninstall whose rule is
replaced while it waits, an uninstall racing another account's install,
the lock held by another run (install and uninstall), visudo rejecting
the staged copy, a failed rename, the shape of the transaction, #32's
three-command rule, and both scripts naming the same lock. The two race
tests fail on the previous scripts (rule overwritten, rule removed).

Docs: README, spec section 2, and two "Not run" rows for a real
two-account race and the root-side file modes.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…r the recovery lock, and no sudo while a process owner is unknown

Answers the round-6 review of 729a91c.

- The rule's lock is /etc/sudoers.d/.insomnia-sudoers.lock (sudo skips a
  dotted name). Before opening it, root checks every folder above it is
  root's and not group or other writable, and that an existing file is a
  regular file of root's with mode 0600 and one link; after lockf, that
  the descriptor and the path are still that file. It is created only
  where nothing is and never repaired, re-owned, re-moded or removed.
- Root opens the rule on a descriptor after owner, mode and link checks,
  compares the exact bytes it reads there with the text the run judged
  (passed as an argument), judges them again, and renames or removes only
  while the path still names that file. An mv or rm killed by a signal is
  an unknown result, not a failure. Messages after the root call no longer
  say nothing was changed, since the lock file may have been made.
- uninstall.sh reads InsomniaResumeFrozenVersion before the recovery lock
  (bounded, with the file's identity before and after) and under it only
  checks with a bounded stat that the file is unchanged. It asks for the
  password with sudo -v before the lock; every sudo call under the lock is
  sudo -n and bounded, and one still running keeps the lock (125) while the
  run keeps the app and the journal. Its journal reads and install's
  plist_pins_previous and candidate lint are bounded, and a failed or
  silent read is never clean.
- A process whose owner ps -o uid= cannot give is no longer taken as this
  account's: both scripts look again and stop before the first sudo call.
  Only a process identified as the API client is ignored.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The shared bounded-call supervisor in install.sh and uninstall.sh now
ignores SIGTERM and SIGHUP, as backstop.sh's does, and gives its call the
script's starting actions back. A signal sent to the run's whole process
group no longer ends the supervisor while a sudo call that closed fd 9
still runs, so the recovery lock stays held until that call exits and is
reaped (R8-7).

The fake sudo's hung mode waits on SECONDS instead of running $(date) in
its loop: a command substitution child copies the logging SIGTERM trap
and logged one group SIGTERM twice (R8-5). A perl signal receiver logs
the sender of each SIGTERM and SIGHUP, and group signals are sent one at
a time, since the kernel keeps one sender per process.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
… bounded journal reads in uninstall and the backstop

Answers the round-8 review of 8d5184c.

- Sudoers ACLs (R8-1): install.sh and uninstall.sh read /bin/ls -lde for
  every folder from the lock up to /, the lock file and the rule. An allow
  entry with any right beyond read, execute, readattr, readextattr,
  readsecurity, list or search stops the run, whoever it names; deny
  entries pass. A list ls cannot print or that the check cannot parse
  stops it too. The folders are checked before the lock file is created,
  the file before it is opened, and all again just before the change.
  Nothing is repaired.
- Second read (R8-2): root reads the rule with cat's status checked, and
  immediately before mv or rm opens it anew and reads it again. It changes
  the rule only when that read completes with no NUL byte and the same
  bytes from the same device and inode; otherwise it keeps the rule (exit
  8 for a failed, short or NUL read, 4 for a change). The root text drops
  each function's leading indentation. A writer that takes no lock can
  still change the rule after that read.
- Uninstall journal check (R8-3): each file is copied once by a bounded
  cp -X into a private directory and checked there, every step with an
  explicit status that does not rely on set -e. A read that fails, is cut
  short, holds a NUL byte or does not answer is a problem, never a clean
  journal, and a session.json the backstop left stops the uninstall.
- Backstop (R8-4): its reads under the lock are bounded with checked
  statuses. Install and uninstall read InsomniaResumeFrozenVersion and the
  Info.plist's identity before the lock and pass both down; the backstop
  uses the version only while a stat under the lock shows the same file.
  Both scripts run the backstop with a 300 s limit and SIGTERM only; one
  still running is reported with its pid (125) and keeps the lock.
- Fixtures (R8-6): fakes name their fixture through FAKE_ROOT, so equal
  texts are hard links to one file per test process and macOS checks each
  once, not on every fixture's first exec. On the 382 script tests that
  ran awake in both measured runs, time fell from 1,718 s to 766 s. No
  case or assertion was removed.
- Exit 4 and 7 messages say the path could not be shown to be one only
  root can change, which also covers a list that could not be read.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
e9e3037 renamed main's testBackstopKeepsAnOwnerACLAndStillUndoesTheJournal
and changed its TestACL.entries assertion from 1 to 0. On main that test
kept its entry only because the publish failed: cp could not copy the
extended attributes of a 0200 journal its owner reads through an ACL
entry. The backstop now publishes from its cp -X private copy, so the
cleared journal is a new owner-only file with no entry, as every journal
main's backstop publishes is (its cp copies no ACL).

A new test runs the same 0200 journal with nothing to undo: the backstop
reads it, publishes nothing, and the journal keeps mode 0200 and its one
entry, main's assertion. A backstop that strips the entry fails it.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
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.

1 participant