Exclude shell-spawning ProcessRunner tests on Windows (EDR alerts) - #163
Merged
Conversation
Five tests spawn a shell to get a child process that outlives a deadline, overruns a capture limit, or spawns a grandchild for the process-tree kill to reach. On Unix that shell is `sh`; on Windows it is PowerShell, and the resulting command line trips Defender for Endpoint's "Suspicious PowerShell command line" rule on managed devices. That has raised two security incidents (30 Jul 2026, and 03 Aug 2026 / MSDetectID 24562977 — the latter quoting the `%TEMP%\treemon-process-runner <guid>\child.pid` path written by the timeout test). What scores is the scaffolding, not the code under test: `ping 127.0.0.1 -n 30` is a documented evasion sleep (MITRE T1497.003), surrounded by an inline `-Command`, a detached `-PassThru` spawn, a PID persisted under a GUID-named temp directory, and `Wait-Process`. Swapping the delay binary does not help — `timeout /t` and `waitfor /t` are the same technique, the surrounding shape still scores, and the rule is a heuristic that cannot be read, so no tweak can be confirmed short of provoking another incident. Excluded on Windows only, via `[<Platform(Exclude = "Win")>]`. CI runs ubuntu-latest, so the `sh` branches keep covering real-process timeout, process-tree termination, and the shell-driven truncation paths on every push; a Windows developer only loses local pre-push feedback on these five. The file comment records the re-enable plan (a small `src/TestChild` helper process replacing the shell scaffolding) so the fix is not lost. Verified on Windows: ProcessRunner filter 10 passed / 0 failed (was 15, the five excluded), Fast suite 1884 passed / 0 failed, and no `powershell` process is spawned by the suite.
There was a problem hiding this comment.
Pull request overview
Excludes five EDR-triggering PowerShell-based process tests on Windows while retaining Linux CI coverage.
Changes:
- Adds Windows exclusions to five shell-spawning tests.
- Documents the security rationale and re-enable plan.
- Centralizes the NUnit exclusion reason.
Show a summary per file
| File | Description |
|---|---|
src/Tests/ProcessRunnerTests.fs |
Adds Windows-specific exclusions and supporting documentation. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 0
- Review effort level: Balanced
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
Five tests in
ProcessRunnerTests.fsshell out to get a real child process that outlives a deadline, overruns a capture limit, or spawns a grandchild for the process-tree kill to reach.On Unix that shell is
shand is unremarkable. On Windows it is PowerShell, and the resulting command line trips Defender for Endpoint's "Suspicious PowerShell command line" rule on managed devices. This has now raised two security incidents:%TEMP%\treemon-process-runner <guid>\child.pidpath written by that test.Each incident requires a manual attestation, and the frequency scales with the number of parallel worktrees — which is precisely the workflow this repo exists to support.
Why not just change the command
What scores is the scaffolding, not the code under test.
ping 127.0.0.1 -n 30is a documented evasion sleep (MITRE ATT&CK T1497.003), and around it sit an inline-Command, a detached-PassThruspawn, a PID persisted under a GUID-named temp directory, andWait-Process— collectively the shape of a loader.Substituting the delay binary does not fix it:
timeout /tandwaitfor /tare the same technique.Start-Process powershell -Command 'Start-Sleep 30'is worse (PowerShell spawning PowerShell).Critically, the rule is a heuristic we cannot read, and the only way to test a tweak is to provoke another incident and wait days. Deliberately tuning a command line until it stops tripping EDR, on a monitored device, is also itself the most attacker-shaped activity in this story. There is no safe iteration loop, so the right move is to leave the detection space rather than try to score just under the threshold.
The 30 Jul investigation concluded the ping was "incidental" and left the test in place. Four days later, same test, same machine, second incident.
Change
Exclude those five tests on Windows only, via
[<Platform(Exclude = "Win")>].CI runs
ubuntu-latest, so theshbranches continue to cover real-process timeout, process-tree termination, and the shell-driven truncation paths on every push. No coverage is lost in CI — a Windows developer loses only local pre-push feedback on these five.A comment at the top of the file records the incidents, why tweaking the command does not work, and the re-enable plan, so the reasoning is not lost:
That approach was validated in a scratch probe before being written down: a
ProjectReferenceto anExecopies its apphost into the test output (Cli.exeandTreemon.exealready arrive that way, andFSharp.Core.dllis already present), andKill(entireProcessTree = true)reaches the grandchild exactly as it does today. It would also retire the fourIsOSPlatformcommand branches and both quote-escaping schemes in this file.Verification (Windows)
--filter FullyQualifiedName~ProcessRunner--filter Category=Fastpowershell(5.1) processes spawned by the suiteOn process sampling: the first sampled Fast run observed one
powershellprocess, which did not reproduce in a second run, and no reachable Fast-tier code path spawnspowershellafter this change (the terminal fixture that does isExplicit, see below). This machine has heavy ambient PowerShell activity, so that observation is attributed to noise rather than the suite.Remaining PowerShell spawns during the suite are
pwsh -NoProfile -Filefrom the post-fork hook tests. Those are deliberately left alone: they are file-based script execution with no inline-Command, no delay binary, and no PID harvesting, and they mirror exactly what production does atGitWorktree.fs:731— converting them would mean no longer testing the real feature.Linux behaviour is unchanged and is validated by CI on this PR.
Scope note
The terminal-session fixture (
TerminalSessionFixture.fs) enumerates processes viapowershell.exe+Get-CimInstance Win32_Process, which is a stronger detection shape than the ping. It is not touched here because its consumers are alreadyCategory("Local")+Explicit, so it never runs in routine or CI runs — only when invoked deliberately.