Skip to content

fix(criteria,engine): route shell commands through shellexec, not direct sh -c#69

Merged
tzone85 merged 1 commit into
mainfrom
fix/shellexec-bypass-criteria-investigator
Jun 11, 2026
Merged

fix(criteria,engine): route shell commands through shellexec, not direct sh -c#69
tzone85 merged 1 commit into
mainfrom
fix/shellexec-bypass-criteria-investigator

Conversation

@tzone85

@tzone85 tzone85 commented Jun 11, 2026

Copy link
Copy Markdown
Owner

Summary

criteria/db.go (evaluateMigrationSucceeds) and engine/investigator.go (runCommand) both shelled out via exec.CommandContext(ctx, "sh", "-c", cmd). On native Windows sh isn't on PATH, so both paths silently broke — even though shellexec exists for exactly this case (sh on Unix, cmd.exe on Windows, NXD_SHELL override).

Changes

Replace both with shellexec.CommandContext(ctx, cmd). No behavior change on Unix; Windows pipelines now use the configured shell.

These were the last two exec.Command*("sh", ...) direct invocations outside internal/shellexec itself.

Test plan

  • go build ./..., go vet ./..., go test ./... -count=1 -timeout 240s all green locally.
  • Existing criteria + investigator tests continue to pass.

Audit traceability

Architecture finding ARCH-H5 (2026-06-11 sweep).

…ect sh -c

criteria/db.go (evaluateMigrationSucceeds) and engine/investigator.go
(runCommand) both shelled out via exec.CommandContext(ctx, "sh", "-c",
cmd). On native Windows `sh` is not on PATH, so both paths silently
broke even for read-only commands the doctor / status / report commands
indirectly invoke. shellexec already exists for exactly this — it picks
sh on Unix and cmd.exe on Windows, honouring the NXD_SHELL override.

Replace both with shellexec.CommandContext(ctx, cmd). No behaviour
change on Unix; Windows pipelines now use the configured shell.

These were the last two `exec.Command*("sh", ...)` direct invocations
outside internal/shellexec itself. Future regressions can be caught by
the existing grep-style audit.

Surfaced by the 2026-06-11 architecture audit (ARCH-H5).
@tzone85 tzone85 merged commit 786142d into main Jun 11, 2026
9 of 10 checks passed
@tzone85 tzone85 deleted the fix/shellexec-bypass-criteria-investigator branch June 11, 2026 11:22
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