Harden the trust boundary and validate untrusted daemon input - #19
Merged
quantizor merged 3 commits intoAug 9, 2026
Merged
Conversation
ServerSpec.validationErrors is the per-spec check the register seam runs, so a directly-registered spec is screened like a committed one (bad port, empty command, a name carrying the reserved '::'). ProjectConfigLoader.validate now also rejects an empty lifecycle command, the one config field it skipped. grepRejection refuses a pattern that nests one unbounded quantifier inside another (the (a+)+ family), measured to run for seconds on a short line and never on a long one under Swift's backtracking Regex; the match runs on the log actor, so screening it up front is the only defense. DaemonClient gains a SO_RCVTIMEO response deadline, raised for a command that carries its own timeout, so a wedged daemon fails a request in bounded time instead of hanging the CLI and the app forever.
…point prepareSpawn is now the one home for the trust boundary. Its userInitiated flag records trust for an explicit command acting on a committed server, and refuses an autonomous boot restore or watch sweep of a project whose config was never approved, so a cloned repo's devservers.json cannot start itself after a reboot. The scattered recordTrustIfNeeded calls collapse into it. register validates the spec before storing it, closing the one entry point that skipped the file validator. writeConfig refuses to write for a project devctl does not already track, so a wire client cannot drop a devservers.json at an arbitrary path. The remaining project-scoped arms (writeConfig, logs query, logs mark, events, why) canonicalize the path at the decode seam.
devctl switch validates the branch's devservers.json after the checkout and before running its lifecycle argv, so a config the daemon would refuse is no longer fed to the shell. ensure, wait, up, down, restart, and switch pass their own timeout to the client so a long but healthy run is given room while a wedged daemon still fails in bounded time. Documents the register/logs/switch/restore behavior in the CLI contract, maps the trust gate and the client deadline in AGENTS, and adds the changeset.
Merged
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 the security and input-validation gaps in the daemon's wire surface. devctl's one standing invariant is that it never acts on a project's committed config until you approve it, and the SessionStart hook never puts attacker-influenceable output into an agent's context. This PR makes that invariant real and hardens the paths around it. Stacked on #18; review that first.
The trust boundary, actually enforced
Boot restore was the one autonomous spawn path that skipped the trust check: after a reboot it would bring back a committed server for a project that was never approved, so a cloned repo's
devservers.jsoncould start itself. The trust gate now lives in one place,prepareSpawn, which every start-shaped path funnels through. An explicitstart/ensure/uprecords approval (the command is the approval, as before); an autonomous boot restore or watch sweep refuses an unapproved project. A server registered ad-hoc throughregistercarries its own approval and is never gated.Every config entry point validates
registerwas the one way a spec entered the daemon without running the file validator, so a specconfig checkwould reject (an out-of-range port, an empty command, a name containing the reserved::) could be registered and then started. It is screened now, through a newServerSpec.validationErrorsshared with the file path.switchvalidated nothing before running the branch'slifecyclecommands, andlifecyclewas the one config field the validator skipped. Both are fixed: the branch config is validated after checkout and before its argv reaches the shell, and an empty lifecycle command is now aconfig checkerror.writeConfigtook the project path raw, so a crafted request could drop adevservers.jsonat any path (AtomicFile.writecreates intermediate directories). It now writes only for a project devctl already tracks. The five project-scoped arms that skipped it (writeConfig,logs,mark,events,why) canonicalize the path at the decode seam.Denial-of-service and hangs
logs --greppattern that nests one unbounded quantifier inside another (the(a+)+family) makes Swift's backtrackingRegexrun for minutes on a single line, on the log actor. I measured it: 1s at 16 characters, 7s at 18, and an input cap cannot help because the blowup is exponential. Such a pattern is now refused before it runs, with a message that names the fix.devctland the menu bar app forever with no output. The client now sets a response deadline (raised to cover a command's own timeout), so a request fails in bounded time and points atdevctl daemon restart, while a long but healthyensure,wait, or group rollout is given the room it needs.Verification
make testpasses (10 new tests). The trust gate has a true red/green pair:untrustedConfigDefinedServerIsNotRestoredversus the existingrestoresConfigDefinedServerWithResumeIntent, identical setup, only the approval differs, opposite outcomes. New tests also cover the register and writeConfig gates, the lifecycle validation, and the ReDoS screen against both catastrophic and safe patterns.scripts/smoke.shpasses; the release build is clean; each of the three commits builds in isolation.Resolves seven backlog items: trust enforcement, the writeConfig path, the grep ReDoS, the switch lifecycle validation, register validation, the missing canonicalization, and the client receive timeout.