fix(exec): resolve executables on Windows — PATHEXT, mode bits, and a swallowed failure - #9
Conversation
… swallowed failure
No command was ever resolvable by its bare name on Windows. Three defects
stack up to that, found while tracing five failing packages on go-inference's
windows CI lane back through `exec: "C:\...\acceptance source\git":
executable file not found in %PATH%`.
1. isExecutable asked `info.Mode()&0111 != 0`. Windows has no execute bit —
os.Stat synthesises 0666, or 0444 for a read-only file — so that test
rejects EVERY file on the platform, git.exe included. It now asks the
question Windows itself keys on: is the suffix one %PATHEXT% names.
2. lookPath never expanded %PATHEXT%. A command is written "git", not
"git.exe", so each candidate is now also tried with each extension, in the
order %PATHEXT% gives them (an unset value falls back to the .COM;.EXE;
.BAT;.CMD set Windows assumes). A name already carrying a listed extension
is not suffixed again.
3. commandContext swallowed the resolution failure and handed os/exec the bare
name. That is not a harmless fallback: with Dir set, Cmd.Start resolves a
separator-free Path RELATIVE TO Dir, so the error named a path nobody asked
for — a working directory, not PATH. It now returns the failure, which
prepare() surfaces, so callers get the honest "not found in PATH".
Also: the path-vs-name test was `Contains(file, string(PathSeparator))`, which
misses '/' on Windows — where Go accepts it — so "bin/tool" was hunted on PATH
instead of checked directly. containsSeparator tests both conventions.
POSIX behaviour is unchanged by construction: the extension list is empty
there, so candidates are the name alone and the mode bits still decide.
Receipts (macOS):
go test -count=1 ./... ok process 13.578s · exec 0.524s · pkg/api 2.624s
golangci-lint run ./exec/... 0 issues
gofmt -l, go vet: clean
This repo's CI is linux-only, so the Windows rules are pinned HERMETICALLY
instead: lookPathWith/isExecutableWith take the extension list as an argument,
and the new tests drive them with a fixture PATH and a fake %PATHEXT% on any
runner. TestExecInternal_lookPathWith_Good is the direct receipt — a 0644
"tool.exe" resolving from the bare name "tool", a file the old mode test would
have rejected outright. The live proof follows when go-inference bumps its pin
and its windows lane reports.
TestExecInternal_commandContext_Bad inverts an assertion that pinned the old
fallback ("expected raw name fallback") — that test encoded defect 3.
Co-Authored-By: Virgil <virgil@lethean.io>
📝 WalkthroughWalkthroughChangesExecutable resolution
Sequence Diagram(s)sequenceDiagram
participant prepare
participant commandContext
participant PATH
participant executableValidator
prepare->>commandContext: Resolve command name
commandContext->>PATH: Check direct path or PATH candidates
PATH->>executableValidator: Validate candidate
executableValidator-->>PATH: Return validation result
PATH-->>commandContext: Return resolved path or failure
commandContext-->>prepare: Return command or resolution error
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Warning Billing warning: we have not been able to collect payment for this subscription for more than 72 hours. Please update the payment method or pay any pending invoices in Billing to avoid service interruption. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. ℹ️ You can also turn on project coverage checks and project coverage reporting on Pull Request comment Thanks for integrating Codecov - We've got you covered ☂️ |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
go/exec/exec_internal_test.go (1)
92-325: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffUse
requirefor setup assertions andassertfor checks.The new tests use
t.Fatalandt.Fatalffor fixture setup and result checks. Userequirefor setup and preconditions that must stop the test. Useassertfor independent expected values.As per coding guidelines, use testify
requirefor test setup assertions andassertfor checks in test files.Also applies to: 361-390
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@go/exec/exec_internal_test.go` around lines 92 - 325, Update the tests around writeFixture, directory setup, and lookPathWith/isExecutableWith result checks to use testify require for setup or prerequisite failures that must stop execution, and assert for independent expected outcomes. Replace the corresponding t.Fatal/t.Fatalf calls while preserving each existing assertion message and test behavior; apply the same convention to the additionally referenced test block.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@go/exec/exec.go`:
- Around line 222-226: Update the command preparation flow around commandContext
and c.cmd assignment so relative direct paths such as ./tool are resolved
against c.opts.Dir before Path is finalized. Ensure c.cmd.Dir is established
before resolving the command, or explicitly resolve the path from c.opts.Dir,
while preserving existing handling for non-relative commands.
- Around line 368-369: Update the executable lookup logic around
hasExecutableExtension and the corresponding flow at the later PATHEXT handling
site so explicit filenames with extensions are attempted as-is before any
PATHEXT suffix expansion, without requiring the extension to appear in the
restricted list. Preserve suffix expansion for unresolved names, and add
coverage for an existing tool.exe when PATHEXT excludes .exe.
---
Nitpick comments:
In `@go/exec/exec_internal_test.go`:
- Around line 92-325: Update the tests around writeFixture, directory setup, and
lookPathWith/isExecutableWith result checks to use testify require for setup or
prerequisite failures that must stop execution, and assert for independent
expected outcomes. Replace the corresponding t.Fatal/t.Fatalf calls while
preserving each existing assertion message and test behavior; apply the same
convention to the additionally referenced test block.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: b0c370a8-7fc0-4319-88b7-65d5cb65c260
📒 Files selected for processing (2)
go/exec/exec.gogo/exec/exec_internal_test.go
| resolved := commandContext(c.ctx, c.name, c.args...) | ||
| if !resolved.OK { | ||
| return resolved | ||
| } | ||
| c.cmd = resolved.Value.(*core.Cmd) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate exec.go and relevant files =="
fd -a 'exec\.go$' . | sed 's#^\./##'
echo
echo "== git diff stat =="
git diff --stat || true
echo
echo "== inspect go/exec/exec.go around commandContext and prepare =="
file="go/exec/exec.go"
if [ -f "$file" ]; then
wc -l "$file"
sed -n '1,320p' "$file" | cat -n
fi
echo
echo "== search commandContext and prepare usages/definitions =="
rg -n "func commandContext|commandContext\\(|func .*prepare|\\.prepare\\(|\\.Dir|core\\.Cmd|New\\(" go/exec/go.mod go -S || trueRepository: dAppCore/go-process
Length of output: 15409
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== go/exec/exec.go remaining core/lookPath symbols =="
sed -n '320,462p' go/exec/exec.go | cat -n
echo
echo "== core.Cmd type/Dir handling in go package =="
rg -n "type Cmd|func .*Cmd\\(" go -S | head -100
echo
echo "== os/exec link commandContext implementation =="
sed -n '1,80p' go/os_exec_link.go | cat -n
echo
echo "== related commandContext patterns with Dir ordering =="
for f in go/program.go go/service.go go/exec/exec.go; do
echo "-- $f --"
sed -n '70,130p' "$f" 2>/dev/null | cat -n || true
sed -n '150,185p' "$f" 2>/dev/null | cat -n || true
done
echo "== inspect existing Windows test section around Cmd Dir behavior =="
sed -n '340,390p' go/exec/exec_internal_test.go | cat -nRepository: dAppCore/go-process
Length of output: 17789
🌐 Web query:
Go os/exec Cmd Windows relative Path Dir behavior "Path relative to Dir"
💡 Result:
In Go's os/exec package, the Cmd.Dir field specifies the working directory in which the command will execute [1][2]. It is important to note that Cmd.Dir does not influence the resolution of the command executable path itself [3]. When you provide a command name to exec.Command, Go's os/exec package resolves that executable independently of the directory specified in Cmd.Dir [3]. Specifically: 1. Executable Resolution: If the command name contains path separators (e.g.,./myprog or C:\bin\myprog), the package uses the name directly [3]. If the name does not contain path separators, the package uses LookPath to resolve the executable by searching the system PATH [3]. 2. Security and Current Directory: As of Go 1.19, os/exec will not resolve a program using an implicit or explicit path entry relative to the current directory [1][4]. Even if the system PATH configuration includes the current directory (or dot), LookPath will return an error (satisfying errors.Is(err, exec.ErrDot)) instead of resolving to a file in the current working directory [1][4]. 3. Relative Paths: To explicitly run a program located in a specific directory (such as the directory set in Cmd.Dir), you should construct the full path before calling exec.Command (e.g., using filepath.Join) or provide an explicit relative path like "./myprog" [3][5]. While "./myprog" works, it is evaluated relative to the calling process's current working directory, not the directory specified in Cmd.Dir [3]. In summary, Cmd.Dir only sets the environment for the child process once it starts; it does not change the search path for finding the executable binary [1][3]. If you intend to run an executable located inside the folder assigned to Cmd.Dir, you must provide the full path to that executable when creating the Cmd instance [3].
Citations:
- 1: https://pkg.go.dev/os/exec?GOOS=windows
- 2: https://go.dev/src/os/exec/exec.go
- 3: os/exec: Cmd.Dir is not checked when looking for executable on Windows golang/go#21675
- 4: https://pkg.go.dev/os/exec
- 5: https://go.dev/blog/path-security
🌐 Web query:
Go "When Path is set relative to Dir" os/exec Windows Cmd.Dir Path
💡 Result:
In Go's os/exec package, the Cmd.Path field is explicitly documented to be evaluated relative to the Cmd.Dir directory if the path is relative [1][2]. If you specify a relative path for an executable in Cmd.Path (e.g., "bin/prog.exe") and set Cmd.Dir to a specific directory (e.g., "C:\app"), the Go runtime will attempt to locate and execute the program at the resulting joined path ("C:\app\bin\prog.exe") [1][2]. It is important to note how Go handles executable resolution to avoid common pitfalls: 1. Executable Lookup: If you use exec.Command("name") without any path separators, Go uses LookPath to find the executable [1][3]. This process is separate from setting Cmd.Dir [3][4]. If LookPath resolves a command to a relative path (e.g., "./prog.exe") based on the current working directory of the calling process, that relative path remains relative when Cmd.Start is called [4]. If you subsequently set Cmd.Dir to a different directory, the command will fail because it will look for that relative path inside the new Cmd.Dir [4]. 2. Security (ErrDot): Since Go 1.19, os/exec will not resolve programs using implicit or explicit path entries relative to the current directory (the "dot" entry) during LookPath, specifically to prevent security vulnerabilities [1][2]. If a lookup would result in a path relative to the current directory, LookPath will return an error that satisfies errors.Is(err, ErrDot) [1][2]. To ensure robust behavior: - Always use absolute paths or explicitly constructed paths when setting Cmd.Path if you intend to execute a file relative to a specific directory. - Avoid relying on LookPath to resolve relative paths if you plan to change the working directory via Cmd.Dir [3]. - If you need to run a program located in a specific directory relative to your application, construct the path using filepath.Join with your target directory and the executable name before passing it to exec.Command [3].
Citations:
- 1: https://pkg.go.dev/os/exec?GOOS=windows
- 2: https://pkg.go.dev/os/exec
- 3: os/exec: Cmd.Dir is not checked when looking for executable on Windows golang/go#21675
- 4: os/exec: look in current directory for executable on all platforms? golang/go#7570
Resolve relative direct paths against Options.Dir before assigning c.cmd.Dir.
prepare assigns Path with commandContext before setting c.cmd.Dir = c.opts.Dir. For a direct relative path such as ./tool, Go evaluates Path relative to Cmd.Dir, so it can resolve to the caller's directory instead of the configured directory. Resolve ./tool from c.opts.Dir, or resolve after c.cmd.Dir is known.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@go/exec/exec.go` around lines 222 - 226, Update the command preparation flow
around commandContext and c.cmd assignment so relative direct paths such as
./tool are resolved against c.opts.Dir before Path is finalized. Ensure
c.cmd.Dir is established before resolving the command, or explicitly resolve the
path from c.opts.Dir, while preserving existing handling for non-relative
commands.
| if len(extensions) == 0 || hasExecutableExtension(base, extensions) { | ||
| return []string{base} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and inspect the relevant Go file and function ranges without executing repository code.
if [ -f go/exec/exec.go ]; then
echo "FOUND go/exec/exec.go"
wc -l go/exec/exec.go
echo "--- outline around exec.go ---"
ast-grep outline go/exec/exec.go || true
echo "--- relevant lines 330-390 ---"
sed -n '330,390p' go/exec/exec.go
echo "--- relevant lines 430-475 ---"
sed -n '430,475p' go/exec/exec.go
else
echo "go/exec/exec.go not found"
git ls-files | rg '(^|/)exec\.go$|exec/' || true
fi
echo "--- search for related symbols ---"
rg -n "hasExecutableExtension|PATHEXT|extensions|PATHEXT|Executable|ExpandPath|FindExecutable|isExecutable" go/exec -S || trueRepository: dAppCore/go-process
Length of output: 10575
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "--- full test functions around executable lookup / PATHEXT ---"
sed -n '180,290p' go/exec/exec_internal_test.go
echo "--- deterministic probe of lookup logic as implemented ---"
python3 - <<'PY'
from pathlib import Path
defaultPathExt = ".COM;EXE;.BAT;.CMD"
def parsePathExt(value):
extensions = []
for part in value.split(';'):
part = part.strip().lower()
if part:
if not part.startswith('.'):
part = '.' + part
if part not in extensions:
extensions.append(part)
return extensions or None if value == defaultPathExt else []
def hasExecutableExtension(base, extensions):
lowered = base.lower()
for extension in extensions:
if lowered.endswith(extension):
return True
return False
def executableExtensions(value=None):
if Path("/dev/null").exists() and False:
return None
return parsePathExt(value if value is not None else "")
def executableCandidates(base, extensions):
if len(extensions) == 0 or hasExecutableExtension(base, extensions):
return [base]
candidates = []
for extension in extensions:
candidates.append(base + extension)
return candidates
def candidateExists(c, filesystem):
for existing in filesystem:
if existing.lower() == c.lower():
return True
return False
def firstExecutable(base, extensions, filesystem):
for candidate in executableCandidates(base, extensions):
if candidateExists(candidate, filesystem):
return candidate
return ""
filesystem = {"tmp/tool.exe"}
pathextWithoutExe = ".COM;.BAT"
print("filesystem:", filesystem)
print("PATHEXT used:", pathextWithoutExe)
print("resolution_order:", executableCandidates("tool.exe", executableExtensions(pathextWithoutExe)))
print("resolved:", firstExecutable("tool.exe", executableExtensions(pathextWithoutExe), filesystem))
print("expected_go_windows_order_behavior:", ["tool.exe", "tool.exe.com", "tool.exe.bat"])
filesystem = {"tmp/tool.exe"}
pathextDefault = defaultPathExt
print("\nfilesystem:", filesystem)
print("PATHEXT used:", pathextDefault)
print("resolution_order:", executableCandidates("tool", executableExtensions(pathextDefault)))
print("resolved:", firstExecutable("tool", executableExtensions(pathextDefault), filesystem))
filesystem = {"tmp/tool.exe"}
pathextBadCaseNoExe = ".COM;.exe;.BAT;.CMD"
print("\nfilesystem:", filesystem)
print("PATHEXT used:", pathextBadCaseNoExe)
print("resolution_order:", executableCandidates("tool.exe", executableExtensions(pathextBadCaseNoExe)))
print("resolved:", firstExecutable("tool.exe", executableExtensions(pathextBadCaseNoExe), filesystem))
print("expected_go_windows_order_behavior:", ["tool.exe", "tool.exe.com", "tool.exe.exe", "tool.exe.bat", "tool.exe.cmd"])
PYRepository: dAppCore/go-process
Length of output: 5474
🌐 Web query:
Go os/exec Windows LookPath executable filename with extension before PATHEXT
💡 Result:
On Windows, Go’s os/exec.LookPath only treats a candidate as executable if it has an extension that matches PATHEXT; specifically, “the file must have an extension named by the PATHEXT environment variable”. If PATHEXT is unset, it falls back to requiring one of “.com”, “.exe”, “.bat”, or “.cmd”. [1][2] Therefore, when using os/exec on Windows, if you call LookPath with an executable filename that already includes an extension, Go will still apply its PATHEXT-based extension matching when searching PATH directories. If your provided extension is not in PATHEXT, LookPath will not treat it as a valid match via the PATH search. [1][3] Concretely, in the Windows implementation, LookPath builds the list of allowable extensions from PATHEXT (lowercased, ensuring each begins with “.”). When PATHEXT is empty, it uses the default extension list {“.com”, “.exe”, “.bat”, “.cmd”}. It then tries to find an executable by checking those extensions during PATH lookup. [3] Note: If the input contains a path separator (e.g., “C:...\tool.exe”), LookPath treats it differently: it tries the provided path directly (but still via findExecutable with the allowable extensions list). [3]
Citations:
- 1: https://go.dev/src/os/exec/lookpath.go
- 2: https://pkg.go.dev/os/exec?GOOS=windows
- 3: https://github.com/golang/go/blob/cb702a2a5670d4fa599f573b37b1a01abc9f995a/src/os/exec/lp_windows.go
🌐 Web query:
site:go.dev/src/os/exec/lp_windows.go func findExecutable os/exec lp_windows.go
💡 Result:
The Go source file is here: os/exec/lp_windows.go.
findExecutable is defined at line 37 and searches for an executable by checking the provided filename and Windows PATHEXT extensions. (go.dev)
Citations:
🌐 Web query:
"go/src/os/exec/lp_windows.go" "findExecutable" "try" "PATHEXT"
💡 Result:
In the Go source file src/os/exec/lp_windows.go, the findExecutable function is responsible for verifying if a file exists and is an executable by checking it directly and by appending extensions defined in the PATHEXT environment variable [1][2][3]. The logic inside findExecutable works as follows: 1. Handling Extensions: If no extensions (exts) are provided, it checks if the file exists using chkStat [1][3]. 2. Direct Check: If the file already has an extension (checked via hasExt), it first attempts to verify if that file exists using chkStat [1][3]. 3. PATHEXT Iteration: It iterates through the provided list of extensions (derived from PATHEXT in LookPath). For each extension, it appends it to the filename and checks if the resulting file exists using chkStat [1][2]. 4. Result: If a match is found during any of these checks, the absolute path to the file is returned [1][2]. If no match is found after checking all extensions, it returns an error (typically fs.ErrNotExist or ErrNotFound) [1][2][3]. The PATHEXT environment variable is processed in the LookPath function [1][2]. If PATHEXT is set, Go splits it by semicolons (;) and ensures each extension starts with a dot (.), creating the list of extensions used by findExecutable [1][2]. If PATHEXT is not set or is empty, Go defaults to [".com", ".exe", ".bat", ".cmd"] [1][2].
Citations:
- 1: https://github.com/golang/go/blob/go1.19.3/src/os/exec/lp_windows.go
- 2: https://github.com/golang/go/blob/1724077b789ad92972ab1ac03788389645306cbb/src/os/exec/lp_windows.go
- 3: https://golang.bg/src/os/exec/lp_windows.go
🌐 Web query:
"github.com/golang/go/src/os/exec/lp_windows.go" "findExecutable" "extensions"
💡 Result:
In the Go programming language, specifically within the implementation for Windows (found in lp_windows.go), the findExecutable function uses file extensions to locate an executable file [1][2]. The specific extensions considered depend on the PATHEXT environment variable [1][3]. The logic works as follows: 1. Environment Variable Check: The code retrieves the PATHEXT environment variable [1][4]. 2. If PATHEXT is set: The implementation splits the variable's value by semicolons (;) [1][5]. It converts these extensions to lowercase and ensures each starts with a dot (.) if it does not already [1][2]. 3. Default Extensions: If PATHEXT is not set or is empty, the implementation defaults to the following list: .com, .exe, .bat, and .cmd [1][4]. When searching for a file, findExecutable iterates through these determined extensions and appends them to the base filename to check for the existence of the file on the filesystem using chkStat [1][3]. This process is central to how os/exec.LookPath functions on Windows, ensuring that it respects the system's configured executable search behavior [2][6].
Citations:
- 1: https://github.com/golang/go/blob/go1.19.3/src/os/exec/lp_windows.go
- 2: https://go.googlesource.com/go/+/ca571803a87968b280d74b03cbc820ecf68e069d/src/os/exec/lp_windows.go
- 3: https://github.com/golang/go/blob/cb702a2a5670d4fa599f573b37b1a01abc9f995a/src/os/exec/lp_windows.go
- 4: https://github.com/golang/go/blob/1724077b789ad92972ab1ac03788389645306cbb/src/os/exec/lp_windows.go
- 5: https://golang.bg/src/os/exec/lp_windows.go
- 6: https://pkg.go.dev/os/exec
🌐 Web query:
github.com/golang/go/src/os/exec/lp_windows.go "findExecutable" "hasExt" "ex" "PATHEXT"
💡 Result:
In the Go standard library, specifically within src/os/exec/lp_windows.go, the findExecutable function is responsible for determining if a file path is a valid executable by checking it against a list of allowed extensions [1][2]. The function operates as follows: 1. It first checks if the provided file path already includes an extension by using the hasExt helper function [2]. hasExt determines if a file has an extension by checking if there is a dot (.) in the filename that appears after any directory separators (:\/) [2]. 2. If hasExt returns true, it verifies if the file exists at that exact path using chkStat [1][2]. 3. If the file does not have an extension or the previous check fails, it iterates through the provided slice of extensions (derived from the PATHEXT environment variable or defaulting to .com, .exe, .bat, .cmd) and attempts to locate the file by appending each extension to the original path [1][3][2]. The PATHEXT environment variable is processed in LookPath by splitting the string by semicolons, converting entries to lowercase, and ensuring they start with a dot [1][2]. If PATHEXT is unset, the library defaults to [".com", ".exe", ".bat", ".cmd"] [1][2]. As of Go 1.19, LookPath includes security measures that prevent it from implicitly resolving programs in the current directory, returning an error satisfying errors.Is(err, ErrDot) if the only executable found is in the current directory [4][5][6].
Citations:
- 1: https://github.com/golang/go/blob/go1.19.3/src/os/exec/lp_windows.go
- 2: https://github.com/golang/go/blob/cb702a2a5670d4fa599f573b37b1a01abc9f995a/src/os/exec/lp_windows.go
- 3: https://go.googlesource.com/go/+/ca571803a87968b280d74b03cbc820ecf68e069d/src/os/exec/lp_windows.go
- 4: https://pkg.go.dev/os/exec?GOOS=windows
- 5: https://github.com/golang/go/blob/7c72dc77a934cf5957e5667e204506a61457e66a/src/os/exec/exec.go
- 6: os/exec: return error when PATH lookup would use current directory golang/go#43724
Do not validate explicit filenames against a restricted PATHEXT list.
When an explicit filename has an extension, try that filename before applying remaining PATHEXT suffixes. With tool.exe present and %PATHEXT% as .com;.bat, the current lookup only checks .exe.com and .exe.bat, so the caller misses an executable. Keep suffix expansion for unresolved names. Add coverage for an existing tool.exe with a PATHEXT value that excludes .exe.
Also applies to lines 458-459.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@go/exec/exec.go` around lines 368 - 369, Update the executable lookup logic
around hasExecutableExtension and the corresponding flow at the later PATHEXT
handling site so explicit filenames with extensions are attempted as-is before
any PATHEXT suffix expansion, without requiring the extension to appear in the
restricted list. Preserve suffix expansion for unresolved names, and add
coverage for an existing tool.exe when PATHEXT excludes .exe.
1 participant
No command was ever resolvable by its bare name on Windows. Three defects stack up to that, found while tracing five failing packages on go-inference's windows CI lane back through
exec: "C:\...\acceptance source\git": executable file not found in %PATH%.The three defects
1.
isExecutableasked for a mode bit Windows does not have.info.Mode()&0111 != 0— but Windows has no execute bit;os.Statsynthesises0666, or0444for a read-only file. That test rejects every file on the platform,git.exeincluded. It now asks the question Windows itself keys on: is the suffix one%PATHEXT%names.2.
lookPathnever expanded%PATHEXT%. A command is writtengit, notgit.exe. Each candidate is now also tried with each extension, in%PATHEXT%order (an unset value falls back to the.COM;.EXE;.BAT;.CMDset Windows assumes). A name already carrying a listed extension is not suffixed again.3.
commandContextswallowed the resolution failure and handedos/execthe bare name. That is not a harmless fallback — withDirset,Cmd.Startresolves a separator-freePathrelative toDir, so the error named a path nobody asked for: a working directory, not PATH. It now returns the failure, whichprepare()surfaces, so callers get the honest "not found in PATH".Also: the path-vs-name test was
Contains(file, string(PathSeparator)), which misses/on Windows — where Go accepts it — sobin/toolwas hunted on PATH instead of checked directly.containsSeparatortests both conventions.POSIX behaviour is unchanged by construction: the extension list is empty there, so candidates are the name alone and the mode bits still decide.
How the Windows rules are proven without a Windows box
This repo's CI is linux-only, so a green lane here proves nothing about Windows. The rules are therefore pinned hermetically:
lookPathWith/isExecutableWithtake the extension list as an argument rather than reading the environment, and the new tests drive them with a fixture PATH and a fake%PATHEXT%on any runner.The direct receipt is
TestExecInternal_lookPathWith_Good— a 0644tool.exeresolving from the bare nametool. That file is one the old mode test would have rejected outright, so the test fails against the old logic for the right reason.TestExecInternal_lookPathWith_Uglypins the order-sensitivity (.combeats.exe, and reversing the list reverses the winner), the no-double-suffix rule, and the path-qualified case.TestExecInternal_isExecutableWith_Uglypins the rule swap directly: the same 0644 file fails the POSIX test and passes the Windows one, while a directory namedbundle.exeis neither.TestExecInternal_commandContext_Badinverts an assertion that pinned the old fallback ("expected raw name fallback") — that test encoded defect 3.Receipts (macOS)
24 internal tests pass, 20 of them new or rewritten.
The live proof
It follows when go-inference bumps its
dappco.re/go/processpin and its windows lane reports — that lane's 5-packageagent/*cluster is the thing this fix exists to clear. I'll post the before→after there.Summary by CodeRabbit
Bug Fixes
PATHEXTand executable checks for more reliable command discovery.Tests