fix: resolve sudo workload install hang on macOS and visibility mismatch on Linux#521
fix: resolve sudo workload install hang on macOS and visibility mismatch on Linux#521agneszitte wants to merge 26 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR addresses an infinite uno-check --fix loop on Linux when .NET workloads are installed under an elevated context (root-owned locations) but checked under the current user context, leading to repeated “missing workload” detections.
Changes:
- Add a
sudo -n(no-prompt) wrapper and use it to probe root-owned workloads when the user workload list is empty. - Change workload install/repair to run as the current user first, retrying with
sudoonly when output suggests a permission issue. - Add unit tests for the new sudo-retry detection helper (
ShouldRetryWithSudo).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| UnoCheck/Util.cs | Adds a no-prompt sudo wrapper path (sudo -n) for non-interactive probing. |
| UnoCheck/DotNet/DotNetWorkloadManager.cs | Implements user-first install/repair with sudo fallback and adds sudo probing for workload listing when empty. |
| UnoCheck.Tests/DotNetWorkloadFeedbackTests.cs | Adds theory coverage for ShouldRetryWithSudo string matching behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
On apt-installed .NET (e.g. Ubuntu), workloads installed via sudo are not visible to the normal user and vice versa, causing an infinite fix loop. - Try workload install/repair as current user first - Probe sudo -n (non-interactive) to detect if sudo is available - Fall back to sudo only when exit code or stderr suggests permission denial (ShouldRetryWithSudo) - GetInstalledWorkloads also tries sudo -n fallback when user-level list returns empty but root may own the metadata Fixes #515
…etry patterns The .NET SDK uses 'Inadequate permissions. Run the command with elevated privileges.' on Linux/macOS CI, which was not matched by ShouldRetryWithSudo. This caused the user-first install to fail without falling back to sudo.
…back - Extract RetryWithSudo helper to centralize sudo fallback logic - Use sudo -n (non-interactive) when Util.NonInteractive or Util.CI is set to prevent hanging on password prompt in CI runners - Add WrapShellCommandWithSudoNoPrompt overload with CancellationToken
- Add IsSdkPathWritable proactive check before user-level install attempts to skip doomed installs on root-owned SDK paths (addresses download failures that were not caught by ShouldRetryWithSudo patterns) - RetryWithSudo now always tries sudo -n first (cached creds/NOPASSWD), then falls back to TTY-attached interactive sudo in non-CI mode - Add WrapShellCommandWithSudoInteractive (RedirectOutput=false) so sudo can prompt for a password without hanging - Add verbose-output pattern tests for ShouldRetryWithSudo - Add IsSdkPathWritable unit tests (writable dir, non-existent dir)
- Replace TTY-attached sudo with sudo -S -p '' to prevent the macOS exec_pty relay from hanging when started via .NET Process.Start - Add ReadPasswordFromConsole() for masked password input - Add FlushAndCloseInput() to signal EOF after piping password
agneszitte
force-pushed
the
dev/agzi/issue-515-workload-context-followup
branch
from
a37de9e to
200c149
Compare
agneszitte
changed the title
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Use unique GUID-based probe filename in IsSdkPathWritable to avoid collisions when multiple instances run concurrently - Escape single quotes in shell command args passed to sudo to prevent shell injection - Return actionable stderr message when ReadPasswordFromConsole returns null instead of a silent empty result - Guard FlushAndCloseInput against InvalidOperationException when standard input is not redirected
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…ompt isn't hidden (#515)
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 9 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…en't reported as missing
…K paths with spaces work
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 8 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…en-prompt regression can't recur
ajpinedam
force-pushed
the
dev/agzi/issue-515-workload-context-followup
branch
from
4b396be to
9e2afd0
Compare
… repair can't expire the cached ticket and fall back to a hidden prompt
…ble workload-list output stops masquerading as 'no workloads installed'
…oInteractive so a missing sudo binary returns an actionable failure instead of throwing
…Async so a missing sudo binary returns false instead of bypassing the friendly handshake-failed flow
…ard the wrong-password fix when sudo is denied or missing
…nsole so 'uno-check --fix > log.txt' doesn't send the prompt to the log and leave the user typing blind
|
@khaskoul @jhanvi03 @agneszitte This branch/fix is ready to be re-tested. I went through the comments for the previous tests and I fixed the issues mentioned. I also tested it locally on macOS and Linux (did not test on Windows so please pay attention to that one) and it worked as expected, I was able to see the Sudo happening accordingly to the specs and the requirements. |
… so ShouldRetryWithSudo and BuildCliFailureMessage substring matchers fire on non-en-US hosts; sudo paths invoke through env to outflank env_reset
| /// Builds an args list for invoking <paramref name="dotnetExe"/> through <c>env</c>, with | ||
| /// <see cref="WorkloadCliEnv"/> re-set inside the elevated child. Required for sudo paths | ||
| /// because sudo's default <c>env_reset</c> strips any var we set on the parent process. | ||
| /// <paramref name="dotnetExe"/> is shell-double-quoted so a path containing spaces | ||
| /// (e.g., <c>/Users/My Name/.dotnet/dotnet</c>) survives both the <c>sh -c '…'</c> | ||
| /// interpolation used by <see cref="Util.WrapShellCommandWithSudoNoPrompt"/> and the | ||
| /// .NET argv tokenizer used by <see cref="Util.WrapShellCommandWithSudoInteractive"/>. | ||
| /// </summary> | ||
| static string[] BuildEnglishCliSudoArgs(string dotnetExe, string[] args) | ||
| { | ||
| var result = new List<string>(args.Length + WorkloadCliEnv.Count + 1); | ||
| foreach (var kv in WorkloadCliEnv) | ||
| result.Add($"{kv.Key}={kv.Value}"); | ||
| result.Add(Util.ShellDoubleQuote(dotnetExe)); | ||
| result.AddRange(args); | ||
| return result.ToArray(); | ||
| } | ||
|
|
| "This can happen when standard input is redirected or the terminal is non-interactive. " + | ||
| "Please rerun this command in an interactive terminal or configure passwordless sudo." |
| @@ -0,0 +1,154 @@ | |||
| # Sudo elevation for `dotnet workload install` | |||
|
|
|||
| Status: accepted, implemented in `dev/agzi/issue-515-workload-context-followup` | |||
Summary
Fixes the macOS sudo PTY hang (#526) and the Linux workload visibility/permission mismatch (#515) by introducing a robust user-first workload install strategy with safe sudo fallback.
Fixes #515
Fixes #526
Problem
macOS (#526)
On macOS,
sudouses anexec_ptyrelay that hangs indefinitely when launched viaProcess.Startbecause there is no real TTY attached. This madeuno-check --fixunusable on macOS when workload install required elevated privileges.Linux (#515)
When workloads are installed as root (e.g. via CI or manual
sudo dotnet workload install), a subsequent user-leveluno-checkcannot see or repair them becausedotnet workload listonly shows workloads for the current user context. This caused false "not installed" reports and permission-denied failures during fix attempts.Solution
macOS sudo fix
sudo -S -p ""(read password from stdin, suppress prompt) instead of interactive TTY-attached sudoReadPasswordFromConsole()for masked password input from the userFlushAndCloseInput()onShellProcessRunnerto signal EOF after piping the password, preventing indefinite stdin waitsLinux workload visibility fix
dotnet workload installas the current user firstIsSdkPathWritable()detects permission issues before they cause cryptic failuressudo -n(non-interactive, no password) — works in CI and pre-authenticated sessionssudo -Swith piped stdinGetInstalledWorkloadsalso runssudo -n dotnet workload listto discover root-owned workloadsPattern matching
ShouldRetryWithSudo()detects permission-denied patterns:Testing
Automated (CI)
ShouldRetryWithSudotheory cases (8 simple patterns + 4 verbose output scenarios)IsSdkPathWritablefacts (writable dir + non-existent dir)Manual Testing (Linux)
Performed on Ubuntu 24.04 (WSL), .NET SDK 9.0.313. Two user contexts:
root(writable SDK at/root/.dotnet) andtestdev(non-writable SDK at/opt/dotnet, NOPASSWD sudo)./root/.dotnet(writable)/root/.dotnet(writable)-v)/opt/dotnet(non-writable)sudo -n dotnet workload install/opt/dotnet(non-writable)-v)sudo -n dotnet workload installKey observations:
IsSdkPathWritabledetects the restriction, skips user-first attempt, goes directly tosudo -n-v: showsSDK path '/opt/dotnet' is not writable by the current user. Using elevated privileges.-v: only heartbeat progress dots during install (no diagnostic noise)dotnet workload listshows them installed after fixManual Testing (macOS)
Performed on a mac mini (macOS, user
user, admin group, sudo requires password).~/dotnet-writableuno-check --fix --target wasm~/dotnet-writableuno-check --fix --target wasm -v~/dotnet-nonwritableuno-check --fix --target wasm~/dotnet-nonwritableuno-check --fix --target macosVerification: After tests 3 & 4, confirmed workload packs installed as
root:wheelin~/dotnet-nonwritable/metadata/workloads/(7 WebAssembly packs)..NET SDK: 9.0.313 (both test SDKs)
CI Test Coverage Notes
ShouldRetryWithSudopattern matching,IsSdkPathWritablelogicsudo -Swith password piping — requiresConsole.ReadKeywhich is unavailable when stdin is redirected. Code gracefully handles this viaConsole.IsInputRedirectedcheck.sudo -nfallback on non-writable SDK paths (CI runners have writable paths), interactivesudo -Swith piped password on macOSCommits
Core implementation
fix: use user-first workload install with sudo fallback on Linux— core logicfix: add 'inadequate permissions' and 'elevated privileges' to sudo retry patterns— broader detectionfix: use sudo -n in CI/non-interactive mode for workload install fallback— non-interactive firstfix: proactive SDK writability check and TTY-safe sudo fallback—IsSdkPathWritable+ safe TTY checkfix: use sudo -S piped stdin to avoid macOS PTY hang— macOS-specific fixReview follow-ups
fix: harden sudo helpers per review feedback— guard clauses, error handling, code cleanupfix: harden FlushAndCloseInput and fix doc comment per review— defensive pattern on StandardInput accessfix: correct RetryWithSudo doc comment to describe sudo -S piping— doc accuracyfix: check cancellation before blocking password prompt and remove test manifests— CancellationToken check + cleanupdocs: expand ReadPasswordFromConsole XML doc to cover all null-return cases— doc completeness