fix(daemon): survive bad commands, lock socket to owner - #4
Merged
Conversation
Two audit findings from PR #3. The daemon called the global die() (which exit()s) on any recoverable per-command error - an out-of-bounds coordinate, a non-numeric argument, or Mirroring being closed - so a single bad command killed the whole daemon and discarded the warm Vision model and held input session it exists to preserve. An agent doing exploratory taps triggered it constantly. Validation now throws CommandError: the one-shot CLI still turns that into die() (behavior unchanged, still exits 1), while the daemon returns it as an ERR reply and keeps running. Confirmed: an out-of-bounds tap, a bad arg, and an unknown verb all return ERR with the same warm daemon alive throughout. The daemon's UNIX socket was bound without an explicit chmod, so its connect-permission depended on the ambient umask; under a permissive umask another local user could drive the phone, write PNGs to arbitrary paths (shot), or read files as keystrokes (text). It is now chmod 0600 right after bind. Adds tests/test_daemon.bats (survival + socket perms) and a CI step to build the native engine so those tests run instead of skipping.
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
1 participant
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.
Summary
Fixes the two findings from the PR #3 self-audit.
Changes
Medium— one bad command killed it). Validation now throwsCommandErrorinstead of calling the globaldie()/exit(). The one-shot CLI catches it and dies as before (exit 1, unchanged); the daemon returnsERRand keeps its warm Vision model + held session. An out-of-bounds coordinate, a non-numeric arg, or Mirroring being closed no longer terminate the process — trivial for an agent to hit, and it was silently defeating the daemon's whole purpose.Low— perms depended on ambient umask).chmod 0600right afterbind(), so no other local user can connect and drive the phone, write PNGs anywhere (shot), or read files as keystrokes (text).tests/test_daemon.bats(survival across bad-command classes + socket perms), plus a CI step to build the native engine so those tests run instead of skipping on a fresh checkout.Testing
srw-------(0600)