fix(sessions): PATHEXT-aware Windows executable resolution on both spawn paths

[mwiebe]

What was the problem/requirement? (What/Why)

Background: how do you turn command: python into a running process on Windows?

When an OpenJD template says command: python, something has to figure out
which file on disk that means. On Windows this is surprisingly subtle,
because Windows has two rules that Linux doesn't:

1. Extensions matter. python might be python.exe, but a runnable
   command can also be a .bat, .cmd, or other extension. Windows keeps
   the list of runnable extensions in the PATHEXT environment variable.
2. The canonical search order is "directory first, extension second".
   When cmd.exe (or where.exe, or Python's shutil.which) looks up a bare
   name, it walks the PATH directories one at a time and tries every
   PATHEXT extension in each directory before moving to the next. So a
   tool.bat in an early PATH directory beats a tool.exe in a later one.

Our sessions runtime spawned commands with Rust's std::process::Command,
which does not follow those rules. It only ever tries .exe for a bare
name, and — worse — when the child's PATH has no match, the underlying
CreateProcessW call quietly falls back to a legacy search: the application
directory, the system directories, and the parent process's PATH (even
when we explicitly cleared the child's environment).

Exploratory testing confirmed three real defects:

1. Wrong binary chosen. With dirA\tool.bat and dirB\tool.exe on
   PATH=dirA;dirB, every standard Windows resolver picks the .bat in
   dirA; we picked the .exe in dirB. A bare name whose only match was
   a .bat/.cmd didn't resolve at all.
2. PATH leak. A command that was not on the action's PATH could still
   run, silently resolved through the worker agent's own environment. An
   action's command should see exactly the environment the action defines.
3. Working directory not searched. The Python reference implementation
   searches the session working directory first (working_dir;PATH), so a
   script materialized as an embedded file is runnable by bare name. We
   never searched it.

The crate already contained a module for this (win32_locate.rs, mirroring
Python's _locate_executable.py), but it was dead code — never wired into
the spawn paths — and had a bug in its PATH fallback. The specs described
Windows cross-user support as "partially implemented", which was also stale:
the embedded cross-user helper is fully implemented and CI-tested on Windows.

What was the solution? (How)

Resolve the command to an absolute path before spawning. Absolute paths
bypass all of CreateProcessW's fallback machinery, so getting the lookup
right once fixes all three defects at the same time.

There are two spawn paths, and each resolves in the right place:

• Same-user actions (run_subprocess in subprocess.rs) resolve
  host-side via the rewritten win32_locate::locate_windows_executable:
  PATHEXT-aware search over {working_dir};{PATH}, earliest directory wins,
  and "not found" is now a hard error with the same message Python raises
  (Could not find executable file: <command>).

• Cross-user actions resolve inside the embedded helper binary
  (helper/src/runner_win.rs::locate_executable), which runs as the target
  user. That placement matters for two reasons:

  • Permissions: the helper can find executables in directories that
    only the target user can read (Python achieves the same by spawning a
    shutil.which probe as the target user; our helper just does it
    in-process since it already is that user).
  • PATH source: when the action doesn't set PATH, the child inherits
    the target user's PATH (from its environment block) — so resolution
    must use that PATH, not the service user's. Host-side resolution would
    look at the wrong one.

One rule worth calling out: the PATH used for the search comes from exactly
one source, chosen up front — if the action supplies a PATH it is used
exclusively (even if the search then finds nothing), and only a completely
absent PATH falls back to the base environment. It is never a
try-one-then-the-other chain. This mirrors how the environment merge works
at spawn, so resolution always searches the PATH the child actually sees.

Both paths compile the same search implementation from one shared source
file (helper/src/win32_which.rs, included into the session crate via
#[path]), so their semantics are identical by construction. The search is
hand-written with shutil.which semantics rather than using the which
crate, because which cannot honor the action's environment: it always
reads PATHEXT from the resolving process (an action setting PATHEXT=.EXE
would still match a .bat), and it accepts any existing file when the
command has an explicit extension (script.ps1 would resolve and then die
at spawn with error 193, where shutil.which correctly reports not-found).
Like PATH, PATHEXT follows the either/or source rule: the action's value is
used exclusively when set, else the base environment's.

A test-harness fix rode along: the CLI tests shimmed python with a
python.cmd wrapper, which the (previously broken) resolution never found.
Once resolution started finding it, tests with multi-line python -c
one-liners failed with batch file arguments are invalid: since the
BatBadBut fix (CVE-2024-24576), Rust's std refuses to spawn a .bat/.cmd
with any argument it cannot safely escape for cmd.exe, and cmd.exe has no
escape for an embedded newline (passed raw, it silently truncates the
argument at the newline). The shim is now a hard-linked python.exe.

What is the impact of this change?

• Windows actions now resolve commands with canonical Windows semantics,
  matching cmd.exe, where.exe, shutil.which, and the Python OpenJD
  implementation. Templates using .bat/.cmd commands by bare name, or
  scripts placed in the session working directory, now work.
• A command not on the action's PATH now fails fast with a clear error
  instead of silently running a binary from the worker's own environment.
  This is a behavior change: jobs that (likely unknowingly) depended on the
  legacy fallback search will now fail with Could not find executable file: <command> — which is the same behavior as the Python
  implementation, and the failure message makes the cause obvious.
• The specs no longer describe Windows support as "partially implemented";
  they document what is actually built and tested.

How was this change tested?

• Have you run the unit tests?

Yes — cargo test --workspace and cargo clippy --all-features --all-targets --workspace -- -D warnings (plus the helper sub-crate's own
clippy, as CI runs it) are clean on Windows. The two pre-existing failures
on this (domain-joined) dev machine — test_with_password_nonexistent_user_ returns_logon_failure and test_tempdir_windows_nonvalid_principal_raises_ error — fail identically on main and are environmental (LogonUserW
returns "domain isn't available" instead of ERROR_LOGON_FAILURE); they
pass on CI's standalone runners.

New coverage, written to fail before the fix and pass after:

• Unit tests in win32_locate.rs: PATHEXT precedence, working-dir
  precedence, case-insensitive PATH key lookup, process-PATH fallback,
  absolute-path passthrough, not-found error.
• tests/integration/test_win32_locate.rs: the same behaviors
  end-to-end through Session::run_subprocess, including the "absent from
  the action's PATH must fail" guarantee that proves the legacy fallback
  search is bypassed.
• tests/integration/test_helper.rs: drives the helper binary directly
  over its stdin/stdout protocol — PATHEXT precedence, cwd-first,
  not-found error over the protocol, helper-PATH fallback.
• tests/integration/test_cross_user_windows.rs (CI test-user job):
  working-dir .bat by bare name through the helper, not-found via the
  protocol, and resolution of an executable in a directory with a
  protected DACL readable by the target user — the case that specifically
  requires target-user-vantage resolution.

Was this change documented?

• Are relevant docstrings in the code base updated?

Yes. Doc comments on both resolution functions state the search rules and
the either/or PATH-source selection. Spec updates: win32-locate.md
(rewritten around the integrated design), subprocess.md (new resolution
section), cross-user.md and architecture.md (stale "partially
implemented" / "not yet integrated" claims replaced with the actual state,
including CI testing), and embedded-cross-user-helper.md (new "Windows
runner: executable resolution" section).

Is this a breaking change?

No public API changes. There is one intentional behavior change on Windows,
described above: commands not on the action's PATH now fail with a clear
error instead of silently resolving through the worker process's
environment. This aligns with the Python implementation's behavior.

Does this change impact security?

This change provides the following improvement: previously, an action whose
PATH did not contain its command could silently execute a binary from the
worker agent's own PATH, the application directory, or the system
directories — an environment the job never declared. Resolution to an
absolute path before spawn eliminates that ambient lookup on both spawn
paths. No new files or directories are created; the helper binary's
protections (protected DACL, Job Object) are unchanged.

---

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

[leongdl]

Aoproved.

[crowecawcaw]

From the PR description, it sounded like this change is supposed to fix resolution issues so we don't need stuff like this? I might be misinterpreting something though.

[mwiebe]

I'm of a mind to remove this python proxy thing entirely, but I haven't looked closely at what it's doing.

[crowecawcaw]

Is it possible to run which in a separate process that mirrors the action so we don't have this reimplementation?

[mwiebe]

which isn't a program generally available on Windows, and running a subprocess here would be a bigger performance degradation than I think is reasonable.