fix(daemon): capture operator PATH into EnvironmentFile for systemd shell tool - #1565
Merged
Aaronontheweb merged 7 commits intoJul 3, 2026
Merged
Conversation
…hell tool A systemd --user service starts with a sanitized environment and does not inherit the operator's login-shell PATH, so the agent's shell tool cannot resolve `netclaw`, `dotnet`, or `~/.local/bin` binaries. `netclaw daemon install` previously baked a hardcoded PATH list into the unit, which can never anticipate every environment (netclaw-dev#1544: `~/.dotnet` was invisible). Instead of guessing, capture the operator's real PATH from the CLI process itself (a child of the operator's shell, so no shell is spawned and no dotfiles are sourced) and hand it to the daemon via a netclaw-owned EnvironmentFile: - `NetclawPaths.DaemonEnvironmentFilePath` (config/daemon.env) - `DaemonPathEnvironmentFile`: single capture/compose/render/parse contract shared by the installer (producer), doctor --fix (rehydrator), and the doctor check (validator) - install writes `PATH=<installDir>:<captured>` and wires the unit via `EnvironmentFile=-…` (tolerant load); the inline `Environment=PATH=` is gone - uninstall removes the env file (extracted `RemoveDaemonEnvironmentFile`) - `netclaw doctor --fix` rehydrates the file from the current shell PATH, independent of netclaw.json, and instructs a restart (never restarts the daemon implicitly) - `SystemdUnitPathDoctorCheck` validates the EnvironmentFile wiring + contents; legacy inline-PATH units are routed to reinstall PATH is confirmed security-neutral here: the shell command policy matches the literal typed verb token and never resolves against $PATH, so widening PATH cannot bypass a deny or widen an allow. Docs (SPEC-011, PRD-004) and the netclaw-operations skill updated. Closes netclaw-dev#1544
Follow-up to the capture-not-guess change, fixing defects surfaced by an xhigh code review: - Restore a guaranteed system-directory floor. Composition is now `installDir : <captured> : /usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin`, de-duplicated. Dropping the old unit-baked floor meant an empty/unset capture yielded `PATH=installDir` alone (silent total shell-tool break, which the doctor check still passed), and a desktop login PATH omitting /usr/sbin,/sbin lost admin-tool resolution. - Drop empty PATH elements. A POSIX empty element (`::`, from `PATH="$PATH:"`) resolves to the current directory, letting a binary planted in an agent-controlled workspace shadow a system command. - doctor --fix: ApplyAsync now creates the parent directory before writing (was throwing DirectoryNotFoundException and aborting the run if ~/.netclaw/config had been removed), and the EnvironmentFile= path comparison is guarded against malformed values crashing Path.GetFullPath. - Doctor check no longer false-warns a functional legacy inline-PATH unit (e.g. after an in-place binary upgrade without reinstall); it passes with a migration note and only warns when the inline PATH lacks the install dir. - Reconcile the spec deltas with the implementation: doctor --fix owns only the env file (unwired/legacy units route to reinstall), and document the floor + empty-element sanitization. Tests added for empty-capture floor, empty-element stripping, functional vs broken legacy units, config-dir-removed rehydration, and malformed unit path. 286 Daemon+Doctor tests pass; slopwatch 0.
The DoctorFixService daemon-PATH tests built ExecStart via Path.Combine, which yields backslashes on Windows. TryGetInstallDir correctly parses POSIX `/` (systemd units are always POSIX), so install-dir parsing failed on windows-latest and no rehydration fix was planned, failing 4 tests. Build the test units with forward slashes and a POSIX install-dir literal, matching real systemd units and the (already-passing) SystemdUnitPathDoctorCheck tests. No production change.
…onstant Replace the six copy-pasted `const string installDir = "/opt/netclaw"` blocks (and comment) in DoctorFixServiceTests with a single class-level `InstallDir` constant; drop the now-redundant WriteWiredUnit parameter.
Aaronontheweb
commented
Jul 3, 2026
Aaronontheweb
left a comment
Collaborator
Author
There was a problem hiding this comment.
LGTM - let's give it a try in the beta channel
| var captured = DaemonPathEnvironmentFile.CaptureCurrentPath(); | ||
| var updated = DaemonPathEnvironmentFile.Render(installDir, captured); | ||
|
|
||
| fixes.Add(new DoctorFileFix( |
| public async Task ApplyAsync(DoctorFixPlan plan, CancellationToken cancellationToken = default) | ||
| { | ||
| foreach (var fix in plan.Fixes) | ||
| { |
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.
Problem
Closes #1544.
A systemd
--userservice starts with a sanitized, non-interactive environment and does not inherit the operator's login-shellPATH.netclaw daemon installcompensated by baking a hardcodedEnvironment=PATH=list into the unit — which can never anticipate a real operator's environment. The trigger:~/.dotnet/dotnetwas invisible to the daemon's shell tool during a CI restore because~/.dotnetwasn't in the hardcoded list.Approach
The operator's shell already knows the correct
PATH, and so does anynetclawCLI process launched from it. Capture that instead of guessing — with zero shell execution and no dotfile sourcing — and hand it to the daemon via a netclaw-ownedEnvironmentFile.DaemonPathEnvironmentFile— one contract (capture / compose / render / parse) shared by the producer, rehydrator, and validator, so the file format andEnvironmentFile=wiring stay in lockstep.daemon installcapturesEnvironment.GetEnvironmentVariable("PATH"), writesPATH=<installDir>:<captured>to~/.netclaw/config/daemon.env, and wires the unit viaEnvironmentFile=-…(tolerant load). The inlineEnvironment=PATH=is removed.daemon uninstallremoves the env file (extractedRemoveDaemonEnvironmentFile).doctor --fixrehydrates the file from the current shellPATH— independent ofnetclaw.json— and instructssystemctl --user restart netclaw. It writes files only; it never restarts the daemon implicitly.SystemdUnitPathDoctorCheckvalidates theEnvironmentFile=wiring + contents; legacy inline-PATH units are routed to reinstall.Why this is safe
The shell command policy is PATH-independent:
ShellCommandPolicymatches the literal typed verb token (never resolves against$PATH), and there is no resolved-path allow-list. Widening the daemon'sPATHchanges only bare-name resolution (ergonomics), never a security decision — so it cannot bypass a deny or widen an allow.Behavior notes / trade-offs
PATHis current as of the lastinstall/doctor --fix. Installed a new tool afterward? Re-run either and restart. The doctor check nudges when the install dir is missing fromPATH.EnvironmentFile=-: a missingdaemon.envdegrades tool resolution (flagged by the doctor check) rather than bricking the whole daemon over a PATH helper file. Rationale recorded in the change'sdesign.md(D8).mise/asdf/direnv); manualnetclaw daemon startalready inheritsPATHfor free.Testing
DaemonPathEnvironmentFileTests(contract hub +BuildDaemonUnitContent+ uninstall removal).SystemdUnitPathDoctorCheckTestsincl. a producer→consumer contract test.DoctorFixServiceTests(rehydrate when missing/stale, incl. no-netclaw.json; no-op when healthy/legacy; restart instruction; applies to disk).InstallAsync/UninstallAsyncaren't driven end-to-end (realsystemctl/loginctlagainst the live service); the pure builders + extracted seam they use are tested directly.dotnet slopwatch analyze→ 0 issues · copyright headers verified · 281 Daemon+Doctor tests pass.Planning artifacts are included under
openspec/changes/systemd-daemon-path-capture/(proposal / design / spec deltas / tasks) and can be archived after merge.