You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
fix(execution): refuse a relative configured command - #342
CommandResolver.Resolve turned a configured relative command into an absolute path by resolving it against the process's current directory, so a wip.yml committed to a repository someone had just cloned could point wip at a binary inside that repository and have it run with their privileges. #329 closed the launcher side; this path defeated that guard rather than tripping it, because the resolver had already absolutized the value before the guard saw it.
The rule
A configured wslc.command / compose.command must be a bare name, searched on PATH, or a fully qualified path. Anything else is refused before any resolver sees it.
This is what the setting is already documented to do — the wslc: block lists auto, "any other name: must be executable on PATH", and "an absolute path". A relative path was never documented, so this brings the code to the page rather than changing documented behaviour.
Three details:
The guard sits ahead of resolveExecutable, so an injected resolver cannot route around it. Covered by a test.
Path.IsPathFullyQualified, not IsPathRooted. IsPathRooted accepts the drive-relative C:wslc.exe and the root-relative \wslc.exe, which still depend on the current drive or directory. IsPathRooted is used only to detect that a separator-less value like C:wslc.exe is a path at all — as a bare name it would otherwise reach the PATH walk and come back as itself.
CommandNotFoundException is reused, keeping it the only failure callers have to handle, as that file's contract states. The message carries the meaning: the value, the rule, and why.
The candidates loop is untouched. Candidates are code-supplied (DefaultCandidates, ["git"] in RulesFetcher), never wip.yml content.
Tests
ExplicitRelativePathIsFrozenAsAbsolutePath asserted the behaviour being removed, so its setup became the refusal test. Added: the absolute path still resolving, a bare name still resolving on PATH, the drive- and root-relative forms, and the injected-resolver case. UnnormalizableCandidate… now uses an absolute path so it still exercises normalization rather than the new guard.
Each refusal test puts a real file where the value points, so only the guard can make it throw — not an absent file.
Verification
On the Windows host, SDK 10.0.403:
dotnet restore wip.slnx --locked-mode — up to date, no lock file changed
CommandResolverTests alone — 16 total, 0 failed, 0 skipped, so the Windows-only cases really ran
Mutation check: with the RejectRelativePath call commented out, 7 cases in that class fail (the refusal test, all four drive/root-relative forms, and both injected-resolver forms). The tests fail without the fix.
The same change was also run on Linux during implementation: 1746 total, 0 failed, 10 skipped — there the four Windows-only cases skip, which is why they were re-run here.
wslc.command and compose.command come from the project's wip.yml. A
relative path was resolved against the directory wip ran from, so a
cloned repository could point wip at a binary inside itself. Accept only
a bare name, searched on PATH, or a fully qualified path; drive- and
root-relative forms are refused too.
Closes#338
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
No actionable comments were generated in the recent review.
Walkthrough
CommandResolver.Resolve rejects configured relative paths before calling the executable resolver. On Windows, it also rejects rooted paths that are not drive-rooted. Tests cover rejected path forms, accepted absolute paths and bare names, and normalization failure.
Resolve rejects configured relative paths before invoking the resolver and rejects non-local rooted paths on Windows. Tests cover error messages, injected resolver behavior, accepted absolute paths and bare names, platform-specific paths, and normalization failure.
This change makes configured wslc.command and compose.command values refuse relative paths and non-local Windows paths, so a repository's wip.yml can no longer select a binary relative to the current directory. Bare names on PATH and fully qualified paths still work. No concrete defect was identified, and the change appears ready to merge.
Pre-merge checks: 4
Check name
Status
Explanation
Description Check
Check skipped - CodeRabbit’s high-level summary is enabled.
Title check
The title "fix(execution): refuse a relative configured command" matches the main change. CommandResolver.Resolve now refuses configured relative, drive-relative, and root-relative command values befo…
Linked Issues check
Issue #338 (direct) requires that a relative configured command cannot make wip run a binary resolved against the current directory. At head, CommandResolver.Resolve calls RejectRelativePath(configure…
Out of Scope Changes check
The change set at head consists of the guards and helpers in src/Wip.Core/Execution/CommandResolver.cs (RejectRelativePath, RejectNonLocalPath, IsDriveRoot) and changes in tests/Wip.Tests/CommandResol…
Full details: Title check
Explanation
The title "fix(execution): refuse a relative configured command" matches the main change. CommandResolver.Resolve now refuses configured relative, drive-relative, and root-relative command values before the executable resolver runs, and the tests cover these refusals. The title is specific and short, and it names the affected area (execution) and the behavior change (refusing a relative configured command). It does not mention the Windows UNC and device path rejection, but that is a secondary detail and the title does not need to cover it.
Full details: Linked Issues check
Explanation
Issue #338 (direct) requires that a relative configured command cannot make wip run a binary resolved against the current directory. At head, CommandResolver.Resolve calls RejectRelativePath(configured) and then RejectNonLocalPath(configured) before resolveExecutable for any value other than "auto". RejectRelativePath treats a value as a path when it contains a separator or is rooted, and refuses any path value that is not Path.IsPathFullyQualified. This covers the relative, drive-relative (C:wslc.exe) and root-relative (\wslc.exe) forms named in the issue. Bare names still go through ResolveExecutable, whose PATH walk skips non-fully-qualified PATH entries. Both guards run before the injected resolver, so an injected resolver cannot bypass them. The chosen rule is the refusal option from the issue. The head also refuses Windows UNC and device paths (RejectNonLocalPath), which the change since the previous review adds. This is consistent with the same threat model and with the PR objectives. Mapped drive roots such as Z:\ remain allowed through IsDriveRoot. Test contents were taken from the PR summary and the test summary; I did not read tests/Wip.Tests/CommandResolverTests.cs. The author reports the locked restore, Release build and full test run; I did not re-execute them. The documentation criterion is not a coding requirement, and I did not read the README text the PR cites.
Full details: Out of Scope Changes check
Explanation
The change set at head consists of the guards and helpers in src/Wip.Core/Execution/CommandResolver.cs (RejectRelativePath, RejectNonLocalPath, IsDriveRoot) and changes in tests/Wip.Tests/CommandResolverTests.cs. RejectNonLocalPath is a new addition since the previous review. It is connected to #338 because a UNC or device path in wip.yml also selects a binary outside the user's trusted local locations, so it is the same class of hazard. It is not a demonstrated unrelated change. The candidates loop and ResolveFile logic are unchanged and used only for code-supplied candidates. The PR description does not mention the UNC and device refusal, but the PR objectives do. This is a documentation gap in the description, not an out-of-scope code change. I did not read the test diff, so test-level scope is based on the summary.
Finishing Touches
Generate unit tests (beta)
Commit to this branch
Create a new PR
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Reviewing files that changed from the base of the PR and between fc48549 and 498cdf9.
Files selected for processing: 2
src/Wip.Core/Execution/CommandResolver.cs
tests/Wip.Tests/CommandResolverTests.cs
Included review availability: This review used your included allowance. 0 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 3 reviews per hour.
IsPathFullyQualified accepts \\server\share\wslc.exe, so a wip.yml could
run a binary over SMB without knowing where it was cloned. On Windows a
configured path must now have a drive root; a mapped drive stays allowed.
Also reword the relative-path refusal so the "compose command" label
no longer prints "command command".
Refs #338
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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
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.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Closes #338.
CommandResolver.Resolveturned a configured relative command into an absolute path by resolving it against the process's current directory, so awip.ymlcommitted to a repository someone had just cloned could point wip at a binary inside that repository and have it run with their privileges. #329 closed the launcher side; this path defeated that guard rather than tripping it, because the resolver had already absolutized the value before the guard saw it.The rule
A configured
wslc.command/compose.commandmust be a bare name, searched on PATH, or a fully qualified path. Anything else is refused before any resolver sees it.This is what the setting is already documented to do — the
wslc:block listsauto, "any other name: must be executable onPATH", and "an absolute path". A relative path was never documented, so this brings the code to the page rather than changing documented behaviour.Three details:
resolveExecutable, so an injected resolver cannot route around it. Covered by a test.Path.IsPathFullyQualified, notIsPathRooted.IsPathRootedaccepts the drive-relativeC:wslc.exeand the root-relative\wslc.exe, which still depend on the current drive or directory.IsPathRootedis used only to detect that a separator-less value likeC:wslc.exeis a path at all — as a bare name it would otherwise reach the PATH walk and come back as itself.CommandNotFoundExceptionis reused, keeping it the only failure callers have to handle, as that file's contract states. The message carries the meaning: the value, the rule, and why.The
candidatesloop is untouched. Candidates are code-supplied (DefaultCandidates,["git"]inRulesFetcher), neverwip.ymlcontent.Tests
ExplicitRelativePathIsFrozenAsAbsolutePathasserted the behaviour being removed, so its setup became the refusal test. Added: the absolute path still resolving, a bare name still resolving on PATH, the drive- and root-relative forms, and the injected-resolver case.UnnormalizableCandidate…now uses an absolute path so it still exercises normalization rather than the new guard.Each refusal test puts a real file where the value points, so only the guard can make it throw — not an absent file.
Verification
On the Windows host, SDK 10.0.403:
dotnet restore wip.slnx --locked-mode— up to date, no lock file changeddotnet build wip.slnx --configuration Release— 0 warnings, 0 errorsdotnet test— 1746 total, 0 failed, 1651 passed, 95 skippedCommandResolverTestsalone — 16 total, 0 failed, 0 skipped, so the Windows-only cases really ranMutation check: with the
RejectRelativePathcall commented out, 7 cases in that class fail (the refusal test, all four drive/root-relative forms, and both injected-resolver forms). The tests fail without the fix.The same change was also run on Linux during implementation: 1746 total, 0 failed, 10 skipped — there the four Windows-only cases skip, which is why they were re-run here.
🤖 Generated with Claude Code
https://claude.ai/code/session_01LhR19gpMx4ZBuq6pkP4fgY
Summary by CodeRabbit
PATH.