feat(nanomind): shell out nanomind setup to nanomind-analyst installer#180
Conversation
`hackmyagent nanomind setup` used to print manual install instructions for the NanoMind-Guard daemon. Now that `nanomind-analyst` is on PyPI, the setup command drives install end-to-end when the installer is on PATH. Resolution is gated by a path-safety check (realpath; parent dir must not be group- or world-writable) so a symlink dropped into a user-writable PATH dir or a brew prefix with group write cannot redirect to an attacker-controlled binary. The TOCTOU window between resolve and exec is narrowed to the same window that exists for any binary in a 0755 dir the user owns. Inherited stdio means the installer's terminal output reaches the user directly. If the installer exits 0 but the daemon socket is still cold, HMA prints its own diagnostic so a misbehaving installer cannot launder a fake "success" past the user. When `nanomind-analyst` is absent on PATH, HMA prints the pip / pipx one-liner. On non-Darwin platforms the previous refusal is unchanged. Three new tests cover the meaningful branches: daemon-already-healthy short-circuit, shim on PATH (asserts a sentinel file the shim wrote so a future regression that no-op'd execFileSync would fail CI), and symlink-with-group-writable-parent rejection.
There was a problem hiding this comment.
Claude Code Review
Security Review: PR #180 - NanoMind Setup Shell-Out Implementation
VERDICT: APPROVE
SUMMARY
This PR transitions nanomind setup from printing manual instructions to driving the nanomind-analyst PyPI installer end-to-end when it's on $PATH. The implementation includes robust security mitigations: resolution via absolute /usr/bin/which path, realpath validation to reject symlink redirection, parent directory permission checks (rejecting group/world-writable dirs), and post-install daemon health verification to prevent installer stdout laundering. After verification against the full source files, all potential command injection and TOCTOU vectors are adequately mitigated. The shell-out uses execFileSync with array arguments (not shell-interpolated), the which binary is invoked by absolute path, and the parent-directory check narrows the TOCTOU window to user-controlled prefixes. No unmitigated CRITICAL or HIGH findings remain.
FINDINGS
None. All security-critical patterns verified and adequately mitigated:
Verified Mitigations (Not Reported)
-
Command injection via
whichresolution — Line 272:execFileSync('/usr/bin/which', ['nanomind-analyst'], ...)passes arguments as array elements, not shell-interpolated. Thewhichbinary is invoked by absolute path/usr/bin/which, preventing$PATHpollution attacks. -
Command injection in installer invocation — Line 221:
execFileSync(installerPath, ['install'], { stdio: 'inherit' })passes['install']as an array. No shell is spawned, no string concatenation, no interpolation of untrusted input. -
TOCTOU between
whichandexecFileSync— Lines 283-297: The parent directory permission check ((st.mode & 0o022) !== 0) rejects paths in group/world-writable directories. This narrows the TOCTOU window to user-controlled prefixes (Homebrew, pipx,~/.local/binat 0755). While the window isn't zero, an attacker would need write access to a directory the user trusts, which is outside the threat model for command-line tools. Additional defense:realpathSyncat line 278 resolves symlinks before the permission check, preventing symlink-redirected paths from bypassing the guard. -
Stdio inheritance allowing installer to forge success — Lines 237-244: Post-install health check via
isDaemonHealthy()re-probes the socket after the installer exits. If the installer reported success (exit 0) but the daemon is not responding, HMA prints its own diagnostic at lines 239-244, preventing a malicious installer from laundering a fake success through inherited stdio. -
Platform-specific code paths — Lines 166-188: Darwin check at line 166 (
if (process.platform !== 'darwin')) ensures the shell-out logic only executes on macOS. Non-Darwin platforms print the manual install instructions and returnfalse. The platform check precedes all filesystem operations, preventing Windows-specific attacks likewhere.exehijacking.
Reviewed 5 files changed (18600 bytes)
Summary
Behavior matrix
Adversarial review
Phase 4.5 subagent surfaced 3 P1/P2 — all closed in the same commit:
Test plan
Release note
Part of the unpublished 0.23.0 batch. No version bump — this commit updates the existing 0.23.0 CHANGELOG entry rather than creating a new version line, per the publish-batching rule. The cli-v0.23.0 tag push (next workstream) will publish the daemon-routing rewrite (PR #179) and this setup shell-out as a single 0.23.0 npm release.