Require an execute bit before selecting a Unix apphost - #10641
Draft
Jakub Jareš (nohwnd) wants to merge 1 commit into
Draft
Require an execute bit before selecting a Unix apphost#10641Jakub Jareš (nohwnd) wants to merge 1 commit into
Jakub Jareš (nohwnd) wants to merge 1 commit into
Conversation
The server-mode client picks a sibling apphost for a managed .dll by asking File.Exists, which says yes to a file that cannot be launched. A payload built on a Windows agent and run on a Linux machine arrives with its extensionless apphost stripped of POSIX permission bits, because a zip written on Windows records none. Process.Start then throws Permission denied and aborts the run instead of falling back. Gate the choice on IsUsableApphost, which additionally requires an execute bit on Unix. File.GetUnixFileMode is .NET 7+, so net462 and netstandard2.0 consumers keep the existence-only check, which stays correct because the apphost path probe is already OS-aware. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Restores Unix apphost validation in the source-only server-mode client after vstest adopted it.
Changes:
- Requires Unix sibling apphosts to have an execute bit.
- Falls back to
dotnet <dll>for unusable apphosts. - Adds cross-platform launch-selection tests.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
MtpServerProcess.cs |
Adds apphost usability validation and fallback logic. |
MtpServerProcessTests.cs |
Covers Windows and Unix apphost selection. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+370
to
+374
| #if NETCOREAPP | ||
| // File.GetUnixFileMode is .NET 7+. Consumers compiling the netstandard2.0 slice (net462, and | ||
| // net5.0-net7.0 which select that slice) fall back to the existence check above. That stays correct | ||
| // because GetAppHostPath already refuses to consider a Windows ".exe" on Unix, so the only case left | ||
| // uncovered there is an extensionless file that lost its execute bit. |
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.
When launching a managed
.dll, the server-mode client prefers a sibling apphost and decides withFile.Exists, which says yes to a file that cannot be started. A test payload built on a Windows agent and executed on a Linux machine arrives with its extensionless apphost stripped of POSIX permission bits, because a zip written on Windows records none.Process.Startthen throwsPermission deniedand aborts the run rather than falling back.BuildLaunchnow gates the choice onIsUsableApphost, which additionally requires an execute bit on Unix, so an unusable candidate falls back todotnet <dll>.File.GetUnixFileModeis .NET 7+, so it sits behind the package's modern-.NET fence and net462, netstandard2.0 and net5.0–net7.0 consumers keep the existence-only check. That stays correct because the apphost path probe is already OS-aware and never offers a Windows.exeon Unix.This restores the second half of the fix in microsoft/vstest#16336, which was lost when vstest deleted its own client in favour of this package in microsoft/vstest#16300.
Reproduced end to end before fixing:
dotnet publish -r linux-x64on Windows emits an extensionless apphost beside the.dll; a Windows-written zip storesExternalAttributes = 0; extracting on Linux yields mode 0644;Process.StartthrowsPermission denied. The four alternatives were all checked and none holds — the check is absent frommain, the layout is reachable,Process.Startdoes not degrade gracefully, and the.dllbranch is live for consumers even though testfx's own callers pass an apphost.Verified:
build.cmdpasses; the new tests pass on Windows net8.0 and net462 and on Linux net8.0, and reverting the guard makes the Unix fallback test fail.Follow-up in microsoft/vstest, once a package with this ships: bump
MicrosoftTestingPlatformServerModeClientSourcesVersionineng/Versions.propsand add an acceptance test for the layout.🤖
🤖