7 commits

Author	SHA1	Message	Date
Daniel Han	5a5bf64130	Reduce antivirus false positives in the desktop installers (#8586) ... * Windows setup: install uv from a pinned release instead of running remote script text studio/setup.ps1 piped astral's install.ps1 straight into Invoke-Expression. That download-and-execute shape is the single construct AMSI providers and cloud ML scanners score hardest, and install.ps1 already replaced it with a pinned-SHA-256 archive download. Port the same implementation across. Progress goes to the pipeline rather than the console, so the quiet path swallows it exactly as it swallowed astral's installer output and the printed lines around the call site are unchanged. * Windows: stop pairing a hidden window with a bypassed execution policy The Studio shortcut launched launch-studio.ps1 with -WindowStyle Hidden and -ExecutionPolicy Bypass on the same command line. That pair is what Microsoft's own detections key on, and studio/src-tauri/src/install.rs already refuses it for the app's own launch of install.ps1. The installer writes launch-studio.ps1 itself, so the file carries no mark-of-the-web and RemoteSigned loads it. The hidden window is unchanged, so the shortcut behaves exactly as before. The generated launcher's own child launch moves to RemoteSigned for the same reason: it runs an inline -Command against an executable, where no script file is loaded and the two policies are equivalent. Also refresh a stale comment in studio/setup.ps1 that attributed the PSModulePath fix to astral's uv installer, which no longer runs in-process. * Installers: keep download-and-run command lines out of the shipped script text AMSI scans install.ps1 in full before a single line of it runs, and generic script classifiers read install.sh the same way inside the Linux bundle. Both headers rehearsed the piped web one-liner five times over, plus a scriptblock form and an execution-policy bypass, none of which anything in the scripts reads and all of which the README already documents. Point at the README instead and reword the in-body comments that quoted the one-liner as shorthand. Every printed line is untouched: the remediation text the installers show users still spells out the command in full. Same treatment for scripts/uninstall.ps1's header. * Windows: resolve process image paths with one Win32_Process query install.ps1's venv-holder probe opened a handle to every running PID through inline C# compiled at runtime. Opening a handle per process is a shape AV heuristics score hard, and it bought nothing: Win32_Process reports ExecutablePath for exactly the processes those handles could be opened against, and answers for all of them in a single query instead of once per PID. The remaining file-canonicalisation imports stay -- handle-based resolution of linked ancestors has no faithful Windows PowerShell 5.1 equivalent, and it runs on security-relevant paths. Falls back to the per-process .Path when the query is unavailable, so a degraded WMI repository degrades exactly as the old code did on a process it could not open. * Desktop: say who blocked the install when AMSI stops the script PowerShell hands the whole top-level script block to AMSI while compiling it, so a security product's verdict arrives as a parse error over the entire file before install.ps1 runs a statement: no [TAURI:ERROR] marker, no phase log, and a stderr tail the user cannot act on. unsloth#8523 shows what that looks like in the UI -- "Installation failed: + FullyQualifiedErrorId : ScriptContainedMaliciousContent". Recognise the two stable error ids on either stream and append what the user actually needs: nothing was installed, nothing was changed, it is a false positive, update definitions and retry, do not turn off endpoint protection. The raw id stays in the message, because the diagnostics report and any vendor submission both need it. Matches the id, never the message text, which is localized, and tolerates the cmdlet suffix the Invoke-Expression form carries. * Desktop: ship each bundle only the installer it can run resolve_install_script picks install.sh on unix and install.ps1 everywhere else, but the shared Tauri config bundled both into every target. The Linux AppImage therefore carried 280 KB of Windows PowerShell it can never execute -- and it is the largest script body a generic classifier walking the squashfs reads, which is where Microsoft's Trojan:Script/Wacatac.B!ml verdict on 0.1.701-beta landed. Move the resource map into the per-platform configs. The clean-machine job already fails when a Linux bundle ships no install.sh; it now also fails when one ships install.ps1, so the split cannot silently regress in either direction. The .deb scanned clean with the same payload, so this is surface reduction rather than a proven fix for that verdict. * POSIX installers: install uv from a pinned release before falling back install.sh downloaded astral's install.sh to a temp file, ran it and deleted the file; studio/setup.sh piped it straight into a shell. Both are, shape for shape, what a dropper does, and generic ML script classifiers score them accordingly -- the 0.1.701-beta Linux AppImage came back Trojan:Script/Wacatac.B!ml while the .deb carrying the same scripts came back clean. Fetch the pinned release archive and verify a hardcoded SHA-256 instead, matching what install.ps1 already does on Windows. Only the four mainstream targets are pinned: musl, armv7 and any host without a digest tool keep the path they have today, because guessing a target triple wrong would break the install outright and that costs far more than the heuristic score of the fallback. Destination, PATH handling and every printed line are unchanged, so a host that takes either path ends up in the same state it did before. * tests: pin the installer shapes antivirus heuristics score One file collecting what was removed, so it cannot drift back: no remote script run in-process, no encoded or base64 payload, no hidden window paired with a bypassed execution policy, no handle opened against another process, and no new runtime-compiled native import outside an allowlist that carries a reason for each entry that stays. The last test is the other half of the contract. Hardening must not change what a user sees, so the remediation lines the installers print -- which still spell out the web one-liner in full -- are asserted verbatim. Removing the one-liner from comments is the point; removing it from what the user is told to run would be a regression. Runs on the existing discovery-based pytest step, no workflow list to update. * release: emit a false-positive submission packet for whatever gets flagged The build job assembles a Microsoft submission packet, but only for the Windows -setup.exe. The detection that actually arrived on 0.1.701-beta was Trojan:Script/Wacatac.B!ml on the Linux AppImage, so nothing was produced for the one asset that needed it. The VirusTotal job already knows which assets were flagged and by which engines, so put the packet there: hash, size and both portals, for every flagged asset whatever platform it came from, with a note that clearance is per hash and per vendor. Engine names are not repeated -- they are third-party text and already appear escaped under Flagging engines. The gate stays advisory; this only makes acting on it take seconds. * Revert "Windows: resolve process image paths with one Win32_Process query" This reverts commit 7897865c9. tests/python/test_windows_installer_concurrency_guard.py bans Get-CimInstance and $process.Path from Get-RunningStudioVenvProcesses outright, and requires the native image-path lookup. That contract came out of #7764, which closed a set of races where the installer inferred "in use" from something other than a confirmed executable identity and blocked installs that should have proceeded. Win32_Process.ExecutablePath does answer the same question, but a wrongly blocked install costs far more than the heuristic weight of three native imports. Record the imports in the AV-shapes allowlist with that reasoning instead, and keep the ban on the process-memory APIs, which the installer has no use for. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Tighten the comments added by this branch Opening comment-reduction pass over the PR diff: same intent, fewer lines. Cut hardest on the prose that restated the PR description rather than explaining the code next to it. Comments and docstrings only, verified with comment_tools.py check --strip-docstrings across every Python file in the diff. * Drop an unused helper from the uv pinned-release test * Fix three review findings on the installer hardening Stray-resource check aborted the step it was meant to assert. grep exits 1 when it selects nothing, and under this step's set -o pipefail plus the runner's bash -e that kills the assignment outright, so every correctly split .deb failed clean-machine CI before reaching the check. Both lookups take || true now: no match is the passing case for the stray one, and for install.sh it was swallowing the explicit annotation in favour of a bare exit 1. studio/setup.sh skipped astral's XDG_DATA_HOME/../bin destination tier, which install.sh, install.ps1 and studio/setup.ps1 all honour. A host that configured an XDG location got uv under ~/.local/bin instead, where no later shell looks for it. The session PATH prepend hid it at install time. The AMSI guidance claimed nothing was changed even when the block landed on the nested studio/setup.ps1, which install.ps1 launches through the same inherited pipes after the venv, PyTorch and the packages are already on disk. Split the wording on whether a [TAURI:STEP] marker has been seen: a pre-start block produces none, so the reassurance is only given where it is true. * Key the submission packet on the flagged count, not the engine list stats and results are separate fields of the same VirusTotal response, so an asset can carry a flagged count with no readable results map. The summary table reports that asset and the packet skipped it, which is exactly the one that needs a packet. Select on stats.flagged and keep the engine list for the Flagging engines section, which is correctly keyed on having engines to name. * Drop the bundle stray-resource assertion from clean-machine CI That job downloads a published release, never a bundle built from the branch, so asserting the new resource split there turns every run red until a release ships with it. The split is a property of the Tauri config, and tests/studio/test_tauri_installer_resource_contract.py already enforces it at the right layer. The || true on the install.sh lookup stays: it is what lets the explicit annotation print instead of the step dying on grep's exit 1 under pipefail. * Cover the uv host matrix and repeat application in the pinned-release test The pinned path picks an archive per host triple, and a wrong pick installs a binary that cannot execute, which is worse than not installing at all. Drive _uv_pinned_asset over 20 host combinations and require each one to return its own triple or decline to the fallback. Also run the installer three times over one HOME and require an identical tree, and require a stale uv at the destination to be replaced rather than joined by a second copy: the installer is re-run on every upgrade and every repair. * Pick the pinned uv archive off a positive libc check, not the absence of musl An independent audit pass found the Linux selector accepts any host whose ldd output does not say musl. That is not the same question astral's installer asks: it checks a minimum glibc and drops to its musl-static archive below it, so three hosts that worked before this branch now get a GNU binary that cannot exec, and the helper reports success so the fallback never runs. aarch64 with glibc below 2.28 (Ubuntu 18.04) x86_64 with glibc below 2.17 (RHEL 6) a musl image with no ldd at all, where the probe simply finds nothing Read the version instead, from ldd or getconf, and require it to clear astral's floor for the triple. Anything unreadable declines to the fallback. Also ask the userland for its bitness rather than trusting uname on a 64-bit kernel running a 32-bit userland, and follow astral in reading hw.optional.arm64 so a translated shell under Rosetta 2 still gets the native macOS build. Three more from the same pass: Report success only when the destination uv is executable. A copy onto a busy or read-only destination could leave a file that is not, and reporting success there skipped the fallback. Nothing is unwound on the failure path on purpose: the fallback installs over whatever is at the destination, and deleting there would take out a working uv the host already had. Clear the mark of the web on the launcher we author. WriteAllText replaces the unnamed data stream and leaves other NTFS streams alone, so a launch-studio.ps1 that somehow carried one would keep it across the rewrite, and RemoteSigned refuses a marked unsigned script. Store the security-block kind and resolve its wording in message(). stdout and stderr are read by independent threads, so a [TAURI:STEP] written before a block can be observed after it, and freezing the wording at observation time could tell a user nothing was changed on a run that had already installed PyTorch. Also require the error id to appear as the value of a FullyQualifiedErrorId field, so a scanner log that merely names it cannot attach antivirus guidance to whatever fails next. The host matrix in the shell test grows to 28 rows covering every case above, and removing the new gate fails 8 of them. install.rs gains two tests: 37 pass. * Replace a symlinked uv destination instead of writing through it Three from the review on the previous head. cp onto a destination that is a symlink follows the link, so installing over `~/.local/bin/uv -> /opt/homebrew/bin/uv` rewrote the Homebrew binary in place and left the link pointing at a file another package manager owns. Stage next to the destination and rename over it: rename replaces the link itself, and it is atomic, so a concurrent reader never sees a half-written uv either. The staging file is removed when the rename fails, so a failed run leaves no debris. Verify the Windows copy the same way the shell scripts now do. Copy-Item is non-terminating under the caller's ErrorActionPreference, so a locked or ACL-denied destination let execution reach `$haveUv = $true` and the function reported success over whatever was already there. Compare the destination against the archive we just verified, so a stale uv.exe cannot pass for the one we meant to install. install.ps1 carried the same shape and gets the same treatment. Point the header links at the heading that exists. The README has no "Install Unsloth Studio"; it is "Unsloth Studio (web UI)", whose anchor is #unsloth-studio-web-ui. Three test cases cover the symlink: the file behind the link is untouched, the link itself is replaced, and no staging file survives. Reverting the fix fails two of them. * Fail the build when the uv pin drifts from a version floor Before the pin, astral's endpoint always delivered the newest uv, so raising UV_MIN_VERSION was safe on its own. It is not any more: a floor above the pin means a host with no uv gets 0.12.1 installed and then judged too old by the same script that installed it, on the one path where the pin is what runs. Two checks. All four installers must name the same uv, or which version a machine ends up with depends on which script reached it first. And the pin must clear every floor in the tree (UV_MIN_VERSION, UV_OFFLINE_MIN_VERSION, $UvMinVersion). Raising a floor past the pin fails the first, bumping one installer's pin alone fails the second. * Write the shell profile entry the pinned uv path no longer gets for free The P1 here is a real regression and it took a second look to see why. install.sh decides whether to add ~/.local/bin to the user's shell profile with `case ":$PATH:"`, near the end of the run. By then this process has prepended that directory twice, once for the uv bootstrap and once for the venv, so the guard answers yes for a login shell that would answer no and the profile line is never written. That was survivable while astral's installer ran, because it wrote its own profile line and its env file. The pinned path writes neither, so on a fresh account whose login PATH lacks ~/.local/bin the install succeeds, the current shell works, and the next terminal cannot find `unsloth` or `uv`. Snapshot the inherited PATH before anything prepends to it and test the guard against that. Two more from the same review. Honour a configured uv mirror exclusively. UV_INSTALLER_GHE_BASE_URL and UV_INSTALLER_GITHUB_BASE_URL already win outright in both PowerShell installers and in astral's own; the shell path ignored them and tried the public hosts first. A restricted network sets one precisely because those hosts are unreachable, and download() has no timeout, so it would hang rather than reach the fallback. Do not let the twin of an earlier clear erase a later AMSI verdict. Clear-TauriInstallError writes one logical clear to BOTH streams (install.ps1:198) and independent threads read them, so a block observed between a clear and its own twin was discarded by the twin. Ignore a clear identical to the one just processed; a genuine later recovery carries different text and still clears. Two tests cover both directions, 39 install tests pass. * Stage the uv copy under a per-process name An audit pass reproduced a race I introduced with the symlink fix. Both POSIX helpers staged through a fixed destination-side name, so two installers targeting one directory shared it: A finishes copying the staging file B opens the same path with truncation A renames that inode into place as uv B keeps writing through its open descriptor, which is now the published uv The published uv was observable at zero bytes until B resumed, which makes the claim in the comment about a concurrent reader flatly wrong. install.ps1 is covered by its named mutex, but nothing serialises the POSIX helpers, and studio/setup.sh runs standalone on every studio update. mktemp in the destination directory instead. Each rename then publishes a file no other process can still be writing, which is what the atomicity argument needed all along. The loser cleans up its own staging file and declines, so the caller falls back rather than reporting a success it did not achieve. * Keep a default install as quiet as it was when a uv mirror misbehaves Two console regressions from the audit pass, both on paths the install still recovers from. download() runs curl -LsSf, and -S deliberately prints its own errors. The fallback ran under run_maybe_quiet, so a failed download printed nothing before; the pinned attempts run outside that wrapper, so an unreachable mirror now put two curl: (N) lines on the console of a default install that then succeeded. Redirect stderr on the speculative attempts only, leaving download() untouched for every other caller. [TAURI:WARN] is a marker level install.sh has never emitted, and the app forwards unknown markers to its progress UI verbatim (install.rs:639), so a digest mismatch would have surfaced as raw text in the desktop window. Make it a verbose only stderr line: the next mirror or the fallback still runs, so a default install has nothing to say here. Printed-string diff against the merge base is back to additions inside $(...) capture plus that one verbose-gated line, with nothing removed or changed. * Ask the installed uv whether it runs before skipping the fallback The libc gate reads a glibc version from ldd or getconf and treats that as proof a GNU binary will execute. It is not. A stripped NixOS-derived image without nix-ld reports a glibc version through getconf while its loader lives in the Nix store, so the pinned x86_64 uv asks for /lib64/ld-linux-x86-64.so.2 and gets nothing. Every static check passed, so the helper reported success, the astral fallback was skipped, and the first real uv call failed with No such file or directory. astral's installer fails its own glibc probe on that host and ships the fully static musl archive, which runs. The user went from a working uv to none. The archive is digest-verified astral uv by the time it is placed, so ask it: run --version and require it to succeed. One exec closes the whole class rather than this one host, covering a wrong triple, a loader that is not where the binary looks, and a destination we could not really write. A test drives an archive whose uv cannot execute and requires the helper to decline; removing the exec check fails it. * Pair every clear with its twin, not just the previous one install.ps1 clears after each recovered step, so a lagging reader can be several clears behind when a block lands. With clears A then B on one stream and A's twin arriving on the other after the verdict, asking only whether this is the message just seen answers no, and the delayed twin discarded the verdict the guidance exists to explain. Each logical clear emits exactly two markers, so count unpaired ones by message: the first sighting is the clear, the next pairs with it. A test drives the A, B, verdict, A', B' ordering; 40 install tests pass. * Validate the staged uv before it replaces a working one My own exec check was on the wrong side of the rename. The sequence that bites: a host has a uv good enough for UV_OFFLINE_MIN_VERSION but below UV_MIN_VERSION, so the block runs with _uv_present_before true; the pinned path renames over that working binary; the --version check then fails because the loader is missing or the destination is mounted noexec; the fallback download also fails. The installer neither restores the old uv nor reports that none is available, and every later command runs the broken one. Test the staging file instead, before the rename. It sits on the destination filesystem, so it answers the noexec question too, and a binary that cannot run here never gets to replace one that could. Two tests: a working incumbent uv survives an archive whose uv cannot execute, and the rejected staging file is cleaned up. Moving the check back after the rename fails the first. * Stop the AMSI guidance claiming more than it knows Two of these are honesty defects in text a blocked user reads. "nothing was changed on this machine" is false. Rust starts a diagnostics attempt and its phase log before PowerShell is ever spawned, and spawn_script can create ~/.unsloth first, so a pre-start block has already written to disk. The honest claim is that no installation step ran. "This is a false positive" is not something the classifier can know. It proves the output carries a PowerShell error id and nothing about the script's integrity, and install.ps1 can sit in a user-writable directory, so a locally modified copy can earn a genuine verdict. Telling someone to report a correct detection to their vendor is worse than telling them to reinstall from an official package first and only escalate if an unmodified copy is still blocked. Two smaller ones from the same pass. The matcher tested for the field name and the id independently, so a line naming both in prose qualified; it now requires the id to follow the colon and end at a comma or whitespace, which is what the comment always claimed. And the clear-pairing map is bounded: legitimate producers use a small fixed label set, and child output must not be able to grow it without limit. 42 install tests pass. * Honour astral's download override, and stop Unblock-File asking Three from the second audit round. Unblock-File declares SupportsShouldProcess at the default Medium impact, so a profile that sets $ConfirmPreference to Medium or Low gets a prompt from the line I added, even for a launcher that never carried the stream. -ErrorAction does not suppress a ShouldProcess prompt, and a noninteractive host turns it into an error that skips shortcut setup entirely. -Confirm:$false. UV_DOWNLOAD_URL and its older alias INSTALLER_DOWNLOAD_URL outrank the mirror variables in astral's installer, and the merge-base path inherited that because it ran astral's script. All four implementations now honour them first and exclusively. My earlier comment argued they point at a version the pin would reject, but that reasoning had it backwards: a host sets one because it cannot reach the public endpoints, so ignoring it meant public egress first and, with no timeout on the download, a hang instead of a fallback. The pin still applies, so a source serving a different build fails the digest and the caller falls back to astral's installer, which honours the same variable. chmod 0755 on the staging file rather than +x. cp gives it the umask default and +x then adds execute only where the umask allowed read, so a umask of 077 left uv unusable for every other account on a shared machine. astral ships them 0755. Four checks pin the override precedence across all four installers and the mode across both shell ones, with the behaviour verified against a stubbed downloader. * Validate uv before it replaces an incumbent on Windows, and bound the probe install.ps1 and studio/setup.ps1 copied the extracted uv.exe straight over the destination and only asked whether it ran afterwards. A host with a working older uv and a policy (AppLocker, WDAC, endpoint protection) that refuses the new one was left with neither. Run the extracted binary where it landed first, then keep a copy of the incumbent across the publish and restore it if the published copy will not run, since Windows has no atomic replace for a file that may be open. The probe itself is bounded: Start-Process with a 20s WaitForExit and redirected streams, and on POSIX no stdin plus a 20s ceiling where timeout exists. A binary this installer just downloaded must not be able to hang an unattended install by prompting or by never exiting. install.sh and studio/setup.sh also published the pinned uvx after rejecting the pinned uv, leaving a pairing that is never built or tested. A uv that fails to stage, copy or run now abandons the whole placement. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Verify each uv mirror, and persist PATH when the account has no rc file Both PowerShell installers checked the archive digest once, after the download loop had already broken out. A captive portal or a proxy answering 200 with its own body is a successful download by every measure Invoke-WebRequest has, so the first mirror consumed the only attempt and the second, healthy one was never tried. The digest now decides whether a mirror counts as served. install.sh picked a shell profile from .zshrc, .bashrc or .profile and did nothing when none existed. A fresh account has none: astral's installer used to create its own PATH setup there, the pinned path does not, so the next terminal resolved neither unsloth nor uv. Fall back to creating ~/.profile, which every POSIX login shell reads. The existing content guard keeps it written once. * Remove the install.sh a Windows upgrade would otherwise keep forever Windows bundles now carry only install.ps1, but NSIS writes the current resource manifest and deletes nothing, and the uninstaller deletes only what is in that manifest. An in-place upgrade from a release that bundled both installers left install.sh in $INSTDIR permanently, which also made the non-recursive RMDir "$INSTDIR" fail at uninstall. The pre-install and pre-uninstall hooks now delete it, so the population most likely to upgrade actually gets the split. Also silence the speculative mktemp -d in the pinned uv path: its failure falls back to astral's installer, so an unusable TMPDIR printed a line the user could not act on and that the merge base did not print. * Remove the pinned uv temporaries when an install is interrupted The pinned path unpacks a 40 MB archive into a work directory and stages the binary next to the destination, but only cleaned both up when the helper returned normally. A Ctrl-C in between left the archive behind and left a staging file inside a directory that is on PATH. Both paths are now published to the exit and signal traps as they are created and cleared when the helper releases them. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Persist the PATH the way each shell actually reads it, and fail a half-published pair Four follow-ups from review: A uvx that the archive carried but that could not be staged or renamed left uv published next to a stale or missing uvx and still reported success, skipping the fallback that would have installed both. Either half failing now fails the placement, in install.sh and studio/setup.sh. studio/setup.sh had none of the interrupt cleanup install.sh gained: a Ctrl-C left the unpacked archive behind and a staging file inside a directory on PATH. It now owns HUP, INT, TERM and EXIT for the duration of the pinned install and hands them back on the way out. fish sources none of the POSIX rc files, so the ~/.profile fallback was a no-op for a fish user. The persistence helper writes a conf.d drop-in with fish_add_path there, and honours ZDOTDIR for zsh. UV_INSTALL_DIR, UV_UNMANAGED_INSTALL, XDG_BIN_HOME and XDG_DATA_HOME can put uv somewhere other than ~/.local/bin, and astral's installer wrote a PATH line for whichever it picked. The pinned path now persists its own destination too, with UV_NO_MODIFY_PATH honoured as astral honours it. * Make the Windows uv publish a real transaction, and quote persisted paths The companion copies ran bare: under install.ps1's Stop preference a locked or ACL-denied destination threw past the rollback and left a mismatched set with the backups still on disk, and under setup.ps1's Continue preference it kept a stale companion and reported success. Both now copy under -ErrorAction Stop inside the transaction, so any failure unwinds like the others. A failed restore also used to delete the backup anyway, which is the one path in this block that could leave the host with less than it started with: the two things that make a restore fail, an open incumbent and a denied ACL, are the same two that made the replace risky. The backup is now kept and named. fish takes an unquoted path with a space as two directories, neither of which exists, so the drop-in single-quotes it; and the rc line is written inside double quotes, so a uv directory holding a dollar or a backtick is escaped. The second test caught a doubled backslash in the escaper itself. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Tighten the comments added by this branch Comments only, no code touched: 161 comment lines become 106 across install.sh, install.ps1, studio/setup.sh, studio/setup.ps1 and the NSIS hooks. Each one keeps the reason it was written for, said once. Verified with the PowerShell AST parser, sh -n and bash -n, the 50-check uv pinned release suite and 114 installer tests, and by confirming the diff contains no non-comment line. * Tighten the install.rs comments too Comments only: 35 lines become 27, each keeping the reason it was written for. 42 install tests pass and the diff contains no non-comment line. * Abort on a companion that cannot be backed up, and pair clears by stream A uvx.exe that could not be copied aside, because it is locked or its ACL denies reads, was skipped and the new uv.exe published anyway, so the function reported success with a mismatched pair and the fallback never ran. Any backup failure now fails the placement and runs the rollback, in install.ps1 and studio/setup.ps1. The ERROR_CLEAR pairing keyed only on the message, so two real clears of one label on one stream were taken for a clear and its twin. That happens: _install_torch_default_index emits its recovery during the install and again during the ROCm repair. A verdict landing between them was then erased by the genuinely later clear arriving on the other stream. The map is keyed by stream as well, so only the opposite stream's copy can consume a pending marker. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Do not fail an install because the uv probe could not get an answer Three clean-machine CI legs that pass on main failed on this branch: arm64 and two Windows containers, all three with winget unavailable, which is the only condition under which the pinned fallback runs. Each downloaded the right asset, passed the digest, and then failed the probe. Start-Process -NoNewWindow with redirected streams does not behave in a container or on the arm64 image the way it does in a desktop session, and a boolean probe reported that as a broken binary and aborted the install. The probe is now tri-state. Only the binary answering non-zero is a failure. A launch that throws or a wait that times out is inconclusive, and since the digest already proved the bytes are astral's pinned release, an inconclusive probe publishes as the pre-pin code did. Every path prints why, with the captured stderr and the exit code, so the next occurrence is not opaque. Also from review: the POSIX path now stages both binaries and publishes them together with the incumbents saved aside, so a failed uvx rename restores the uv it replaced instead of leaving a new uv beside a stale uvx; the Windows rollback records the destination before the copy that can truncate it; and UV_UNMANAGED_INSTALL suppresses the profile write, as it does for astral. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Give setup.sh the same pair publish and PATH persistence as install.sh studio/setup.sh published uv and then uvx one after the other, so a failed uvx rename left a new uv beside the host's stale one, and the remote fallback can be unavailable. It now stages both, validates uv, and publishes the two renames back to back with the incumbents saved aside, restoring them if the second fails. setup.sh is also run directly for local and Colab setup, where astral's installer used to write the profile line for whichever destination it chose. Without one the PATH export died with that shell and every later run reinstalled uv. It now persists its own destination, with fish handled on its own terms and both of astral's opt-outs honoured. * Treat an empty uv exit code as no verdict, not as a failure The arm64 clean-machine leg still failed on the tri-state probe, and the diagnostic that came with it said why: "uv --version exited ." with no number. WaitForExit(ms) can return before the exit code is cached, so ExitCode was empty and an empty value is not 0, which read a working uv as broken. The parameterless WaitForExit settles it and returns at once because the process has already exited, and a code that is still missing is inconclusive rather than a failure, which is the same rule the launch and timeout paths already follow. Verified against pwsh that a real non-zero exit and a real launch failure still classify as failed and unknown respectively. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Eight review fixes across the uv publish and PATH persistence The fish escaper in studio/setup.sh reached sed as an invalid expression, so a fish user running setup directly would have had setup killed under set -e right after uv was published. It now matches the one in install.sh, and the test runs both escapers rather than reading them. An incumbent that cannot be hard-linked or copied cannot be restored either, so publishing over it would be a one-way move. Both shells now decline. Writing that test turned up that my own rollback deleted both incumbents when nothing had been published, since the no-predecessor branch cannot tell the two cases apart; the rollback is now reached only after a publish was attempted. A rollback with no predecessor removes the binary it published, rather than leaving half a pair the host never had. A signal between the two renames left the undo copy as the only reference to the incumbent, and the handler deleted it. It restores it now, in both shells. setup.sh prepended ~/.local/bin unconditionally after a successful pinned install, so a stale uv there could shadow a custom UV_INSTALL_DIR destination and the rest of setup would run the wrong one. That prepend is now only for astral's installer, which is what writes there. PATH entries are compared literally rather than as case patterns, so a destination holding *, ? or [ is not mistaken for an unrelated entry. On Windows, a .unsloth-old left behind by a failed restore is the only copy of a working uv, and the next run reused that exact name. It takes a distinct one. * Keep the pinned uv first on PATH, and only count an active profile entry install.sh prepends ~/.local/bin after the uv bootstrap, and astral's env file does too, so a custom UV_INSTALL_DIR destination was pushed behind a stale uv sitting in the home directory and every bare uv below picked the wrong one. The pinned destination goes back in front. setup.sh had the same shape and was fixed in defd2292a. The profile check treated any occurrence of the destination text as proof the PATH entry was already there, so a commented-out old export, or /opt/uv-old when the destination is /opt/uv, suppressed the write and left the next shell without uv. Comments are stripped and the directory has to appear as a whole entry. * Gate the NSIS tidy-up, and remove an orphan uv on signal The pre-install hook runs before the user can still cancel, and $INSTDIR can be a directory they picked in the GUI, so deleting install.sh there could take a file that was never ours. Both hooks now only act where our own executable already is. A signal between the two renames restored a predecessor but did nothing when there was none, leaving a 0.12.1 uv beside whatever uvx the machine had. It now removes what it published, which is what the ordinary rollback already does. * Write the uv PATH entry to every startup file astral's installer wired astral's uv installer wires ~/.profile, each of .bashrc, .bash_profile and .bash_login that exists, .zshrc or .zshenv under ZDOTDIR, and a fish drop-in under ~/.config. Replacing that installer with a pinned archive meant the PATH entry only reached the one file for whichever shell happened to be running, so a bash user whose .bash_profile does not source .bashrc, a /bin/sh login, or anyone who later switched shells would have no uv on PATH where they used to. Both POSIX installers now write the same set, once each, with the existing whole-entry check keeping a re-run idempotent. Files that do not exist are not created, apart from ~/.profile, which astral creates too. * Cut the uv publish back to what the common case needs The rollback machinery that grew over the review rounds covered cases a user is very unlikely to meet: an incumbent that cannot be hard-linked, a signal landing between two renames, a restore that itself fails, a second installer racing the first. It was 281 net lines, and every finding in the last two rounds was in it rather than in the hardening. What stays is what the common case needs. POSIX stages both binaries, runs the staged uv, and publishes the pair with two renames; a failure anywhere before them leaves the destination untouched, and the caller falls back to astral's installer exactly as before. Windows probes the extracted uv.exe before touching the destination, then copies the three under -ErrorAction Stop and re-checks the digest at the destination. The staging files are still removed on a signal, since they live in a directory that is on PATH. 64 shell checks and 114 installer tests cover the rest. * Match the exact fish entry, and let a UNC launcher load The fish drop-in is the only thing that puts uv on a fish user's PATH, since fish reads none of the POSIX files, and its check treated any occurrence of the directory as proof: /opt/uv-old suppressed /opt/uv. It now matches the exact fish_add_path line it would write. A launcher on a UNC share is a remote script to PowerShell, and RemoteSigned refuses an unsigned one, so a roaming profile got a shortcut that exits without starting Studio. That case, and only that case, uses Bypass, and drops -WindowStyle Hidden with it so the pair the detections key on never appears. * Wire every startup file on a DEFAULT install too, and give setup.ps1 a fallback The all-profile PATH write was gated on the uv destination differing from ~/.local/bin, which is exactly where a normal install puts it, so every ordinary machine still got the single-file write the shim path has always done. Three independent audits found this. The gate is gone, and the idempotency check now also matches the $HOME-relative spelling the shim block writes, so the default case does not end up with two lines for one directory. studio/setup.ps1 replaced astral's installer with the pinned archive and had nothing to fall back to. A failed pinned install therefore left UseUv false and silently ran torch, bitsandbytes, Triton and the rest through pip: a different resolver, not just a different download. winget is the fallback, as install.ps1 already does, rather than the remote script this branch exists to remove. * Read the pinned install's real result, and three narrower publish guards The winget fallback I added last round read Invoke-SetupCommand's return value, which is [int]$LASTEXITCODE rather than the function's $true, so it fired on every run: a redundant managed install, and a second copy on a machine that asked for UV_UNMANAGED_INSTALL. The function records its own success on the script scope and the fallback reads that. A directory named uv at the destination looked like a published binary: mv moves into it and reports success, and a searchable directory passes -x, so the install reported success and the first later uv call failed instead. Both shells refuse a directory target, as the installer already does for its own shim. The PATH idempotency pattern escaped only part of the ERE metacharacter set, so a destination holding + ( or | did not match itself and every reinstall appended another block to every profile. * Restrict the profile duplicate check to PATH lines, and cover mapped drives for PR #8586 * Match only PATH-setting lines in the profile duplicate check for PR #8586 * Tighten the installer comments added by PR #8586 --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: danielhanchen <danielhanchen@users.noreply.github.com>	2026-08-13 07:02:18 -07:00
Daniel Han	de12348a66	Stop writing the desktop build provenance section onto the release (#8340)	2026-08-10 06:01:11 -07:00
Wasim Yousef Said	67af9aa825	Desktop: unify normal release updater flow (#8298) ... * CI: point desktop updater test at fork * Studio: simplify desktop setup progress UI * CI: limit updater A/B build to macOS and Windows * CI: publish macOS and Windows updater test assets * CI: build Linux and Windows updater test assets * CI: pin updater test to Windows 2022 * Fix latest-main updater test workflow merge * Resolve latest-main startup message merge * Desktop: unify normal release updater flow * Desktop: validate updater signatures * Desktop: preserve generated updater signature * Restore macOS and target the existing v release for PR #8298 Restores the macOS leg that was dropped from the release pipeline: the macos-latest matrix entry, the .dmg and .app.tar.gz assets, the darwin-aarch64 platform entries in latest.json, and darwin-aarch64 in the required families of both release-desktop.yml and publish-desktop-updater.yml. Without them no macOS bundle is published and macOS clients find no matching platform in the manifest, so they stop updating entirely. Targets the v{version} release that already exists instead of creating it. The tag is cut when main is tagged, before this workflow is dispatched, so the old "tag already exists" guard failed every run on this repository; it only passed on a fork where the tags were absent. The guard now requires the release to exist and refuses only when it already carries desktop assets, naming the delete-asset commands to recover a failed publish. Provenance is appended to the release body rather than replacing it, since that body is the changelog. Windows step conditions follow the restored windows-latest matrix entry. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Address the review on PR #8298 Gate the asset and manifest uploads on the draft input. The target release is already public, so a validation-only run was publishing unapproved binaries and latest.json to it. Move the provenance edit after the uploads and replace any earlier section instead of skipping it, so a retry that follows a partial upload records the digests that actually shipped rather than the previous build's. Reject a prerelease target in both guards. GitHub cannot mark a prerelease latest, so catching it only at promotion left the bundles already public. Re-read GitHub latest immediately before promotion. The downgrade check runs before a build that can take an hour, and promoting past a newer release would hand every client an older manifest. Build from the release tag rather than the dispatch ref. The release is published before the workflow runs and main keeps moving, so the bundles could come from unrelated source and provenance could record a SHA that is not the tag's. Skip the updater validation when the release carries no latest.json. The v release is published before the bundles land, so the release event fired first and failed on every release; it now fails closed only when the release cannot be read. Scope the signature sweep to the desktop bundles, since the release is shared. Add the AGPL-3.0 header to the two new files, in the style the repository uses. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Keep updater discovery on desktop metadata and record the built commit for PR #8298 * Send make_latest as the documented string for PR #8298 * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Repair the release-creation tests and the publish-side guard for PR #8298 * Fail closed on promotion, order numbered prereleases and keep the pointer forwardable for PR #8298 * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Resolve the newest desktop release lazily and record the updater pointer gap for PR #8298 --------- Co-authored-by: danielhanchen <danielhanchen@gmail.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>	2026-08-10 01:01:39 -07:00
Daniel Han	0cd73cf3fb	Make the desktop release contract tests fail when the contract breaks (#8228) ... * Realign the desktop release tests with the post-publish VirusTotal job * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Tie the release contract tests to what they are meant to guard Checkpoint of in-progress work, mutation testing still outstanding. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Scope the release wait and scan assertions to the mechanism they guard The wait checks searched the whole step, so a one-shot jobs API read beside an unrelated loop passed; they now run against the poll loop body and also require a break. The scan job's condition was accepted on a success() substring, which let a disjunction through; it now has to require success() conjunctively. And the scan's directory was compared by leaf between two download steps, so moving both under a new parent, or repointing the script argument, scanned an empty directory and still reported clean; the argument the step passes is now compared against the download path. * Tighten the comments on the release contract tests * Reject any job-level condition on the scan and require a live poll loop Accepting a condition that merely opened with success() let success() && false through, which skips the sweep after a publication that succeeded, so no job-level if: is accepted at all now: reaching virustotal-scan is needs:'s decision alone. The wait helper likewise selected a loop by shape, so turning while :; do into while false; do kept every assertion green while the shell skipped the API reads and fell through to a download that races the matrix; the helper now only selects an unconditional poll. --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>	2026-08-09 04:48:38 -07:00
Daniel Han	b844e55d44	Repo tests: pin the post-publish VirusTotal contract instead of the old pre-flight one (#8240) ... The desktop release workflow no longer runs a VirusTotal step inside publish-release. #8194 moved it into its own virustotal-scan job that runs after the release is published, and #8193 replaced publish-release's `needs: build` with a "Wait for the build matrix" step so the job can queue for its runner during the build. Five tests still encoded the old shape and fail on main for every PR: TestWorkflowOrdering::test_scan_runs_after_the_release_is_validated TestWorkflowOrdering::test_release_creation_is_deferred_until_after_the_scan TestWorkflowOrdering::test_scan_runs_before_the_assets_are_published TestWorkflowOrdering::test_the_scan_script_is_checked_out_first test_build_matrix_hands_off_assets_without_release_credentials Rewrite them against the current layout rather than dropping the assertions. The scan is a post-publish sweep and is advisory by design, so what is worth pinning is that it cannot be quietly lost: - virustotal-scan exists and `needs: [publish-release]`, so deleting the job or the dependency is red, - the job carries no `if:`, and no step does either except the summary, which is `if: always()` so the verdict survives a failed scan, - the sparse checkout of scripts/virustotal_scan.py is asserted by mechanism rather than by step name, along with the guard that exits 1 when the script is absent, - the scan step does not swallow the script's exit status: no continue-on-error, no `|| true`, no `exit 0`, and no `--fail-threshold` pinned to a value the script treats as never-fail, - continue-on-error appears on exactly one job and on no step of any job that handles a bundle, - the downloaded artifact pattern matches what the build matrix uploads, - publish-release does not re-inline the scan. For the permissions test, the build-to-publish handoff is now gated by the waiter, so assert that instead of `needs: build`: it covers every matrix leg by name, refuses to publish a leg that did not succeed, and refuses to publish a leg whose job record never appeared. Add a check that the new scan job holds no release credentials, since it handles the bundles and uploads them off-box. Docstrings say plainly that this is a post-publish sweep, so a future reader does not go looking for a pre-publish gate that is not there. Co-authored-by: danielhanchen <unslothshared@gmail.com>	2026-08-09 01:52:12 -07:00
oobabooga	38dec58e08	Desktop: refuse to republish an existing release version (#7941) ... * Desktop: refuse to republish an existing release version * Desktop: harden immutable release publishing * Desktop: detect existing draft releases before publishing * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Desktop: verify reserved release tag * Desktop: update VirusTotal workflow assertions --------- Co-authored-by: Wasim Yousef Said <wasimysdev@gmail.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>	2026-08-08 13:38:11 +02:00
Daniel Han	ee64eec51a	release-desktop: add a VirusTotal pre-flight scan of the release bundles (#8089) ... * release-desktop: add a VirusTotal pre-flight scan of the release bundles * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * virustotal_scan: register the signed upload URL with add-mask * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * virustotal_scan: check out the script, stop replaying single-use upload URLs, bound every request by the deadline * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * virustotal_scan: fail closed on malformed hash lookups and cap pacing by the deadline * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Bound VirusTotal socket calls to the deadline and scan only validated releases - Pass a per-call socket timeout through the transport, clamped to the remaining scan deadline, so a request starting just before the deadline cannot consume the full 300s cushion ahead of the step timeout. - Retry a malformed upload acknowledgement instead of aborting, since the disclosure cost of the upload has already been paid at that point. - Move the scan after 'Create or validate versioned release' so a run that is rejected has not already uploaded all four bundles. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Defer non-draft release creation past the scan and cap retry backoff - Split 'Create or validate versioned release' into a validation step that runs before the scan and a creation step that runs after it. A dispatch with draft=false and a new tag previously published an empty release that stayed assetless for the length of the scan, and permanently so if the run was cancelled part way through. - Clamp the exponential retry backoff to the remaining deadline, so a 429 or 5xx arriving late cannot sleep past --timeout-seconds before the loop notices and writes its summary. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Keep release notes unconditional, fail closed on lookup errors, fix the permission test - Write desktop-release-notes.md in the validation step, which always runs. The updater metadata step reads it on every run, so leaving the write in the conditional create step broke reruns against an existing release. - Only treat a lookup as a missing release when gh reports 'release not found'. Any other failure now fails the step, rather than proceeding to disclose the bundles for a run that cannot publish. - Point test_release_desktop_permissions at the renamed validation step and assert the deferred create step and its gate. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Escape third-party text in the VirusTotal warning annotations Engine names, detection labels and API error strings are third-party data written straight into a workflow command. Actions truncates an annotation at the first newline and mis-parses a bare %, so a crafted or merely awkward detection string could drop the engine list exactly when the scan is trying to alert a maintainer. Mirrors _gha_escape in lockfile_supply_chain_audit.py, including the replace-% first ordering. * Never report an unanalysed bundle as clean, and escape the summary - A hash known to VirusTotal can have no completed analysis, in which case last_analysis_stats is absent and parse_stats yields all zeros. That row read as 'known to VirusTotal' with zero detections, which looks like 70 engines cleared a bundle that none of them scanned. Such a row now reports 'no completed analysis' with stats left unset, so it renders as dashes and cannot trip the threshold. The upload path polls until status is completed, so it only requires a stats object. - Escape third-party engine names, detection labels and error strings in the job summary. It is appended to GITHUB_STEP_SUMMARY and rendered as Markdown, so a newline ended the row and | opened a new cell. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Avoid a CodeQL clear-text-logging false positive on the skip message Interpolating API_KEY_ENV into the skip log trips CodeQL's py/clear-text-logging-sensitive-data rule at high severity, because the constant's name ends in _KEY. It only ever holds the env var name, never the value, but the repo uses CodeQL default setup so there is no config to filter the query on. Write the name out literally and pin it against the constant in test_missing_key_skips_without_failing so the two cannot drift. --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>	2026-08-07 03:27:08 -07:00