diff --git a/.github/workflows/studio-windows-inference-smoke.yml b/.github/workflows/studio-windows-inference-smoke.yml index d821664327..e263243e6b 100644 --- a/.github/workflows/studio-windows-inference-smoke.yml +++ b/.github/workflows/studio-windows-inference-smoke.yml @@ -27,6 +27,11 @@ on: - 'install.ps1' - 'pyproject.toml' - 'tests/studio_setup_ps1/**' + # The PowerShell unit tests this workflow runs live here. Without this a + # PR touching only one of them never exercises it, and no other workflow + # runs them. Scoped to *.ps1 so python-only changes under tests/studio do + # not pull in the GGUF smoke jobs. + - 'tests/studio/*.ps1' - '.github/workflows/studio-windows-inference-smoke.yml' push: branches: [main, pip] @@ -78,10 +83,20 @@ jobs: if ($errs) { $errs | ForEach-Object { $_.ToString() }; exit 1 } Write-Host "$f parsed with no errors" } + # Propagate each child test's failure before continuing: the step only + # inherits the LAST command's exit code, so without these every test + # but the last one could fail and still report green (studio-windows- + # update-smoke.yml already does this). pwsh -NoProfile -File tests/studio/test_resolve_cuda_toolkit.ps1 + if ($LASTEXITCODE) { exit $LASTEXITCODE } pwsh -NoProfile -File tests/studio/test_torch_flavor.ps1 + if ($LASTEXITCODE) { exit $LASTEXITCODE } pwsh -NoProfile -File tests/studio/test_node_decision.ps1 + if ($LASTEXITCODE) { exit $LASTEXITCODE } pwsh -NoProfile -File tests/studio/test_node_probe_guard.ps1 + if ($LASTEXITCODE) { exit $LASTEXITCODE } + pwsh -NoProfile -File tests/studio/test_path_probe_access_denied.ps1 + if ($LASTEXITCODE) { exit $LASTEXITCODE } # uninstall.ps1: native uninstall must keep the shared unsloth.ico while a # WSL shortcut still references it (dual install), else that shortcut blanks. diff --git a/studio/setup.ps1 b/studio/setup.ps1 index bf766198d1..37d67677ad 100644 --- a/studio/setup.ps1 +++ b/studio/setup.ps1 @@ -289,16 +289,127 @@ function Remove-AgentInstructionFiles { } } +# ERROR_ACCESS_DENIED in any of its disguises. PowerShell wraps .NET exceptions +# in MethodInvocationException, so walk the chain instead of catching by type. +function Test-AccessDeniedError { + param($ErrorRecord) + + $ex = if ($ErrorRecord -is [System.Management.Automation.ErrorRecord]) { $ErrorRecord.Exception } else { $ErrorRecord } + while ($ex) { + if ($ex -is [System.UnauthorizedAccessException]) { return $true } + # IOException carries ERROR_ACCESS_DENIED as an HRESULT; Win32Exception + # keeps E_FAIL there and puts the code in NativeErrorCode instead. + if ($ex.HResult -eq -2147024891) { return $true } + if ($ex -is [System.ComponentModel.Win32Exception] -and $ex.NativeErrorCode -eq 5) { return $true } + $ex = $ex.InnerException + } + if ($ErrorRecord -is [System.Management.Automation.ErrorRecord]) { + return ($ErrorRecord.CategoryInfo.Category -eq [System.Management.Automation.ErrorCategory]::PermissionDenied) + } + return $false +} + +# Test-Path throws UnauthorizedAccessException (it does not return $false) when +# an ACL denies the probe, and this script runs under "Stop", so a denied path +# aborted setup with a raw error. "Denied" is kept distinct from "Absent" +# because a denial needs reporting, not a silent retry. +function Get-PathState { + param( + [Parameter(Mandatory = $true)][AllowEmptyString()][string]$Path, + [ValidateSet("Any", "Leaf", "Container")][string]$PathType = "Any" + ) + + if ([string]::IsNullOrWhiteSpace($Path)) { return "Absent" } + try { + if (Test-Path -LiteralPath $Path -PathType $PathType -ErrorAction Stop) { return "Present" } + return "Absent" + } catch { + if (Test-AccessDeniedError $_) { return "Denied" } + # Malformed path, offline drive, dangling link: nothing usable there. + return "Absent" + } +} + +# Non-throwing Test-Path for paths inside install trees we do not control. +# Callers that must react to a denial use Get-PathState instead. +function Test-PathQuiet { + param( + [Parameter(Mandatory = $true)][AllowEmptyString()][string]$Path, + [ValidateSet("Any", "Leaf", "Container")][string]$PathType = "Any" + ) + + return ((Get-PathState -Path $Path -PathType $PathType) -eq "Present") +} + +# Names the link target of a denied dir, since that is where the user must look. +# Empty/null tolerated: this runs while reporting a failure and must not add one. +function Get-PathDenialDetail { + param([Parameter(Mandatory = $true)][AllowNull()][AllowEmptyString()][string]$Path) + + if ([string]::IsNullOrWhiteSpace($Path)) { return "" } + $item = Get-Item -LiteralPath $Path -Force -ErrorAction SilentlyContinue + if (-not $item) { return "" } + # Non-filesystem providers expose an unrelated .Attributes with no -band + # overload, and throwing here would replace the failure we are reporting. + if ($item -isnot [System.IO.FileSystemInfo]) { return "" } + if (-not ($item.Attributes -band [System.IO.FileAttributes]::ReparsePoint)) { return "" } + $target = $null + try { $target = $item.Target } catch { $target = $null } + # PS 5.1 exposes .Target as a collection; PS 7 as a string. + if ($target) { return " (it is a link to $(@($target) -join ', '))" } + return " (it is a link)" +} + +# One stop for every unreadable install tree. Nothing downstream (validate, +# replace, junction, source build, swap) can work without this access, and the +# folder outlives an app reinstall, so retrying cannot help. +function Exit-PathAccessDenied { + param( + [Parameter(Mandatory = $true)][AllowNull()][AllowEmptyString()][string]$Path, + [Parameter(Mandatory = $true)][AllowEmptyString()][string]$Label, + # "delete it, we reinstall it" is true of the managed cache and wrong for + # a tree the user pointed us at. Never tell them to delete their build. + [switch]$UserSupplied, + # Same rule, one step weaker: the ownership guard stops because it could + # not read the marker, so it cannot claim the tree is ours either. It + # already says "move it aside" when it CAN prove the tree is not ours. + [switch]$OwnershipUnverified + ) + + step "permissions" "$Label at $Path cannot be read: access is denied$(Get-PathDenialDetail -Path $Path)" "Red" + if ($UserSupplied) { + substep "Unsloth will not touch a directory you pointed it at, so this has to be fixed at the source" "Yellow" + substep "Restore access with these two in an elevated PowerShell, or point UNSLOTH_LOCAL_LLAMA_CPP_DIR at a readable build:" "Yellow" + } elseif ($OwnershipUnverified) { + substep "Unsloth cannot confirm this folder is its own install while it is unreadable, so it will not tell you to remove it" "Yellow" + substep "Restore access with these two in an elevated PowerShell, or move the folder aside and re-run setup:" "Yellow" + } else { + substep "This folder lives outside the app, so reinstalling Unsloth Studio, to any drive, reuses it and fails the same way" "Yellow" + substep "Simplest fix: close Unsloth, delete or rename $Path, then re-run setup (it is a managed cache and gets reinstalled)" "Yellow" + substep "If deleting is also denied, run these two in an elevated PowerShell, then re-run setup:" "Yellow" + } + substep "takeown /F `"$Path`" /R /D Y" "Yellow" + substep "icacls `"$Path`" /reset /T" "Yellow" + substep "Antivirus or Controlled folder access can deny this path too; allow or exclude it, then retry" "Yellow" + if ($UserSupplied) { + Exit-SetupFailure "Access denied reading $Label at $Path. Restore access with takeown/icacls, or point UNSLOTH_LOCAL_LLAMA_CPP_DIR at a readable build, then re-run setup." + } + if ($OwnershipUnverified) { + Exit-SetupFailure "Access denied reading $Label at $Path. Unsloth cannot confirm that folder is its own install while it is unreadable: restore access with takeown/icacls, or move it aside, then re-run setup." + } + Exit-SetupFailure "Access denied reading the existing $Label at $Path. Delete or rename that folder (Unsloth reinstalls it) or restore access with takeown/icacls, then re-run setup. Reinstalling the app does not reset it." +} + function Get-InstalledLlamaPrebuiltRelease { param([string]$InstallDir) $metadataPath = Join-Path $InstallDir "UNSLOTH_PREBUILT_INFO.json" - if (-not (Test-Path $metadataPath)) { + if (-not (Test-PathQuiet $metadataPath)) { return $null } try { - $payload = Get-Content $metadataPath -Raw | ConvertFrom-Json + $payload = Get-Content -LiteralPath $metadataPath -Raw | ConvertFrom-Json } catch { return $null } @@ -1764,7 +1875,13 @@ if (-not $HasGit) { if ($_localLlamaDir) { # Same layout candidates as the reuse check in Phase 4. foreach ($_c in @("llama-server.exe", "build\bin\llama-server.exe", "build\bin\Release\llama-server.exe")) { - if (Test-Path -LiteralPath (Join-Path $_localLlamaDir $_c)) { $_localLlamaBuilt = $true; break } + # Denied here terminated the run under "Stop" long before Phase 4's + # guarded probes, so this scan needs the same three-state handling. + $_cState = Get-PathState -Path (Join-Path $_localLlamaDir $_c) + if ($_cState -eq "Denied") { + Exit-PathAccessDenied -Path $_localLlamaDir -Label "the UNSLOTH_LOCAL_LLAMA_CPP_DIR build" -UserSupplied + } + if ($_cState -eq "Present") { $_localLlamaBuilt = $true; break } } } if (-not $_localLlamaBuilt) { @@ -2810,20 +2927,52 @@ $StudioHomeIsCustom = ($_studioHomeCanon -ne $LegacyStudioHome) # llama.cpp or whisper.cpp predating the .unsloth-studio-owned marker (see # setup.sh). Only Unsloth prebuilt markers count; source builds are # indistinguishable from a user clone on Windows and stay under the strict guard. +# "Yes" / "No" / "Denied". A denied marker is not evidence of absence: reading it +# as "No" makes the guard below call an Unsloth tree an unrelated directory and +# tell the user to move it aside, when the real problem is permissions. +function Get-StudioAdoptableState { + param([Parameter(Mandatory = $true)][string]$Path) + $denied = $false + foreach ($marker in @("UNSLOTH_PREBUILT_INFO.json", "UNSLOTH_WHISPER_PREBUILT_INFO.json")) { + switch (Get-PathState -Path (Join-Path $Path $marker) -PathType Leaf) { + "Present" { return "Yes" } + "Denied" { $denied = $true } + } + } + if ($denied) { return "Denied" } + return "No" +} +# Boolean view for callers that only gate a cosmetic cleanup on adoption. function Test-StudioOwnedAdoptable { param([Parameter(Mandatory = $true)][string]$Path) - if (Test-Path -LiteralPath (Join-Path $Path "UNSLOTH_PREBUILT_INFO.json") -PathType Leaf) { return $true } - if (Test-Path -LiteralPath (Join-Path $Path "UNSLOTH_WHISPER_PREBUILT_INFO.json") -PathType Leaf) { return $true } - return $false + return ((Get-StudioAdoptableState -Path $Path) -eq "Yes") } function Assert-StudioOwnedOrAbsent { param( [Parameter(Mandatory = $true)][string]$Path, [Parameter(Mandatory = $true)][string]$Label ) - if (-not (Test-Path -LiteralPath $Path -PathType Container)) { return } - if ($StudioHomeIsCustom -and -not (Test-Path -LiteralPath (Join-Path $Path $StudioOwnedMarker) -PathType Leaf)) { - if (Test-StudioOwnedAdoptable $Path) { + # Denied is not Absent: a root we cannot read cannot be proven ours, and + # returning here would let the caller replace it. Both stops stay gated on + # $StudioHomeIsCustom, as before; a default-home denial is reported by the + # phase that owns the path. + $pathState = Get-PathState -Path $Path -PathType Container + if ($pathState -ne "Present") { + if ($StudioHomeIsCustom -and $pathState -eq "Denied") { + Exit-PathAccessDenied -Path $Path -Label $Label -OwnershipUnverified + } + return + } + $markerState = Get-PathState -Path (Join-Path $Path $StudioOwnedMarker) -PathType Leaf + if ($StudioHomeIsCustom -and $markerState -eq "Denied") { + Exit-PathAccessDenied -Path $Path -Label $Label -OwnershipUnverified + } + if ($StudioHomeIsCustom -and $markerState -ne "Present") { + $adoptState = Get-StudioAdoptableState -Path $Path + if ($adoptState -eq "Denied") { + Exit-PathAccessDenied -Path $Path -Label $Label -OwnershipUnverified + } + if ($adoptState -eq "Yes") { Mark-StudioOwned $Path return } @@ -3847,7 +3996,13 @@ if ($LlamaPr) { $LocalLlamaCppLinked = $false $LocalLlamaCppSrc = $env:UNSLOTH_LOCAL_LLAMA_CPP_DIR if ($LocalLlamaCppSrc) { - if (-not (Test-Path -LiteralPath $LocalLlamaCppSrc -PathType Container)) { + # Unreadable is not missing: reporting "does not exist" would send the user + # looking for the wrong problem. + $localSrcState = Get-PathState -Path $LocalLlamaCppSrc -PathType Container + if ($localSrcState -eq "Denied") { + Exit-PathAccessDenied -Path $LocalLlamaCppSrc -Label "the UNSLOTH_LOCAL_LLAMA_CPP_DIR directory" -UserSupplied + } + if ($localSrcState -ne "Present") { step "llama.cpp" "UNSLOTH_LOCAL_LLAMA_CPP_DIR does not exist: $LocalLlamaCppSrc" "Red" Exit-SetupFailure "UNSLOTH_LOCAL_LLAMA_CPP_DIR does not exist: $LocalLlamaCppSrc" } @@ -3857,13 +4012,24 @@ if ($LocalLlamaCppSrc) { # layout LlamaCppBackend._layout_candidates() resolves (root-level, build\bin, # or build\bin\Release) so the flag never rejects a tree Unsloth could run. $LocalLlamaServerFound = $false + $LocalIsCanonical = ($ResolvedLocal -eq $LlamaCppDir) foreach ($_cand in @( (Join-Path $ResolvedLocal "llama-server.exe"), (Join-Path $ResolvedLocal "build\bin\llama-server.exe"), (Join-Path $ResolvedLocal "build\bin\Release\llama-server.exe"))) { - if (Test-Path -LiteralPath $_cand) { $LocalLlamaServerFound = $true; break } + # Denied must not read as "nothing built here": the canonical branch + # below would then hand the tree to the prebuilt installer, which + # replaces the very build this flag asked to reuse. + $candState = Get-PathState -Path $_cand + if ($candState -eq "Denied") { + # -UserSupplied even when this is the canonical location: the + # override says the tree is the user's build, so never advise + # deleting it, managed path or not. + Exit-PathAccessDenied -Path $ResolvedLocal -Label "the UNSLOTH_LOCAL_LLAMA_CPP_DIR build" -UserSupplied + } + if ($candState -eq "Present") { $LocalLlamaServerFound = $true; break } } - if ($ResolvedLocal -eq $LlamaCppDir) { + if ($LocalIsCanonical) { # Points at the canonical install location itself: never delete-then-link # onto itself. Reuse an existing build here (skip prebuilt + source) so the # staged prebuilt installer can't replace a build the user asked to reuse; @@ -3893,17 +4059,31 @@ if ($LocalLlamaCppSrc) { # can remove it and relink to a new valid directory. $existing = Get-Item -LiteralPath $LlamaCppDir -Force -ErrorAction SilentlyContinue if ($existing -and ($existing.Attributes -band [System.IO.FileAttributes]::ReparsePoint)) { - $existing.Delete() + # A link reads Present, so the probe below cannot cover a denied + # unlink; report it here rather than terminate on the raw throw. + try { $existing.Delete() } + catch { + if (Test-AccessDeniedError $_) { Exit-PathAccessDenied -Path $LlamaCppDir -Label "llama.cpp install" } + throw + } } if ($StudioHomeIsCustom) { Assert-StudioOwnedOrAbsent -Path $LlamaCppDir -Label "llama.cpp install" } - if (Test-Path -LiteralPath $LlamaCppDir) { + # The destination is about to be deleted and replaced, so a denial here + # must stop rather than throw raw: under a default home nothing above + # has probed it three-state. + $destState = Get-PathState -Path $LlamaCppDir + if ($destState -eq "Denied") { + Exit-PathAccessDenied -Path $LlamaCppDir -Label "llama.cpp install" + } + if ($destState -eq "Present") { Remove-Item -Recurse -Force -LiteralPath $LlamaCppDir -ErrorAction SilentlyContinue # A locked/in-use tree can silently survive removal (SilentlyContinue # masks it). Don't then junction/copy over a half-present dir; mirror the # prebuilt path's active-process handling and stop with a clear message. - if (Test-Path -LiteralPath $LlamaCppDir) { + # Denied counts as surviving: unreadable is not gone. + if ((Get-PathState -Path $LlamaCppDir) -ne "Absent") { step "llama.cpp" "install blocked by active llama.cpp process" "Yellow" substep "Close Unsloth or other llama.cpp users and retry" "Yellow" Exit-SetupFailure "llama.cpp install is blocked by an active llama.cpp process" 3 @@ -3938,15 +4118,27 @@ if ($LocalLlamaCppLinked) { substep "Skipping prebuilt install -- falling back to source build" "Yellow" } else { Write-Host "" - if (Test-Path -LiteralPath $LlamaCppDir) { + # Denied on the dir itself means an unreadable parent; Denied on the file + # below means an unreadable install. Either way nothing here can proceed, + # and the bare probes used to die under "Stop" with a raw + # "Test-Path : Access is denied" and exit 1. + $llamaDirState = Get-PathState -Path $LlamaCppDir + if ($llamaDirState -eq "Denied") { + Exit-PathAccessDenied -Path $LlamaCppDir -Label "llama.cpp install" + } + if ($llamaDirState -eq "Present") { substep "Existing llama.cpp install detected -- validating staged prebuilt update before replacement" # If the existing install is the wrong kind (e.g. windows-cpu on a ROCm # machine that should have windows-rocm), remove it so the installer is # forced to download the correct variant rather than skipping on tag match. $existingMetaPath = Join-Path $LlamaCppDir "UNSLOTH_PREBUILT_INFO.json" - if (Test-Path $existingMetaPath) { + $existingMetaState = Get-PathState -Path $existingMetaPath -PathType Leaf + if ($existingMetaState -eq "Denied") { + Exit-PathAccessDenied -Path $LlamaCppDir -Label "llama.cpp install" + } + if ($existingMetaState -eq "Present") { try { - $existingMeta = Get-Content $existingMetaPath -Raw | ConvertFrom-Json + $existingMeta = Get-Content -LiteralPath $existingMetaPath -Raw | ConvertFrom-Json $existingKind = $existingMeta.install_kind # A ROCm host may legitimately carry the fork's windows-rocm bundle # or the upstream windows-hip fallback, so accept either and never @@ -4092,7 +4284,7 @@ if ($LocalLlamaCppLinked) { (Join-Path $LlamaCppDir "llama-server.exe"), (Join-Path $LlamaCppDir "build\bin\llama-server.exe"), (Join-Path $LlamaCppDir "build\bin\Release\llama-server.exe"))) { - if (Test-Path -LiteralPath $_cand) { $PreservedLlamaServerFound = $true; break } + if (Test-PathQuiet $_cand) { $PreservedLlamaServerFound = $true; break } } if (-not $PreservedLlamaServerFound) { $script:LlamaCppDegraded = $true } # A preserved CUDA/ROCm/CPU server does not satisfy an explicit Vulkan @@ -4192,7 +4384,7 @@ if ($env:WHISPER_SERVER_PATH -or $env:UNSLOTH_WHISPER_CPP_PATH) { } $installedWhisperLlamaTag = "unknown" $llamaMarker = Join-Path $LlamaCppDir "UNSLOTH_PREBUILT_INFO.json" - if (Test-Path -LiteralPath $llamaMarker -PathType Leaf) { + if (Test-PathQuiet $llamaMarker "Leaf") { try { $markerPayload = Get-Content -LiteralPath $llamaMarker -Raw | ConvertFrom-Json if ($markerPayload.release_tag) { $installedWhisperLlamaTag = $markerPayload.release_tag } @@ -4501,7 +4693,15 @@ if ($LocalLlamaCppLinked) { $UseConcreteRef = ($ResolvedSourceRef -ne "latest" -and -not [string]::IsNullOrWhiteSpace($ResolvedSourceRef)) - if (Test-Path -LiteralPath (Join-Path $LlamaCppDir ".git")) { + # Denied must not read as "no checkout here": the fresh-clone branch ends in + # a swap that recursively removes this tree and moves the temp one over it, + # under "Continue" and unchecked, so an unreadable child would leave a + # half-deleted install behind. Stop while that is still avoidable. + $llamaGitState = Get-PathState -Path (Join-Path $LlamaCppDir ".git") + if ($llamaGitState -eq "Denied") { + Exit-PathAccessDenied -Path $LlamaCppDir -Label "llama.cpp install" + } + if ($llamaGitState -eq "Present") { # why: in-place git mutation (remote set-url, checkout -B, clean -fdx) # rewrites $LlamaCppDir; mirror the prebuilt and temp-dir-swap guards # so an unrelated workspace .git tree is never silently overwritten. @@ -4818,8 +5018,30 @@ if ($LocalLlamaCppLinked) { # Swap temp build dir into final location (only if we built in a temp dir) if ($BuildOk -and $LlamaCppDir -ne $OriginalLlamaCppDir) { Assert-StudioOwnedOrAbsent -Path $OriginalLlamaCppDir -Label "llama.cpp install" - if (Test-Path -LiteralPath $OriginalLlamaCppDir) { Remove-Item -LiteralPath $OriginalLlamaCppDir -Recurse -Force } + if ((Get-PathState -Path $OriginalLlamaCppDir) -ne "Absent") { + Remove-Item -LiteralPath $OriginalLlamaCppDir -Recurse -Force -ErrorAction SilentlyContinue + # Any unreadable or locked child survives the removal, and Move-Item + # then nests the build *inside* the leftovers instead of replacing + # them. Both are non-terminating here, so check before destroying + # more: the temp build is still whole at this point. + $swapState = Get-PathState -Path $OriginalLlamaCppDir + if ($swapState -eq "Denied") { + Exit-PathAccessDenied -Path $OriginalLlamaCppDir -Label "llama.cpp install" + } + if ($swapState -ne "Absent") { + step "llama.cpp" "could not replace the existing install at $OriginalLlamaCppDir" "Red" + substep "Part of it survived removal; the new build is intact at $LlamaCppDir" "Yellow" + substep "Close Unsloth and other llama.cpp users, or move that folder aside, then re-run setup" "Yellow" + Exit-SetupFailure "llama.cpp install at $OriginalLlamaCppDir could not be replaced; the new build is at $LlamaCppDir" 3 + } + } Move-Item -LiteralPath $LlamaCppDir -Destination $OriginalLlamaCppDir + # A failed move is non-terminating too; without this setup would report + # a build it never installed. + if ((Get-PathState -Path $LlamaCppDir) -ne "Absent") { + step "llama.cpp" "could not move the new build into $OriginalLlamaCppDir" "Red" + Exit-SetupFailure "llama.cpp build at $LlamaCppDir could not be moved into $OriginalLlamaCppDir" 3 + } $LlamaCppDir = $OriginalLlamaCppDir $BuildDir = Join-Path $LlamaCppDir "build" $LlamaServerBin = Join-Path $BuildDir "bin\Release\llama-server.exe" @@ -4865,7 +5087,7 @@ $llamaCppItem = Get-Item -LiteralPath $LlamaCppDir -Force -ErrorAction SilentlyC $llamaCppIsLink = $llamaCppItem -and ($llamaCppItem.Attributes -band [System.IO.FileAttributes]::ReparsePoint) if (-not $llamaCppIsLink -and ( -not $StudioHomeIsCustom -or - (Test-Path -LiteralPath (Join-Path $LlamaCppDir $StudioOwnedMarker) -PathType Leaf) -or + (Test-PathQuiet (Join-Path $LlamaCppDir $StudioOwnedMarker) "Leaf") -or (Test-StudioOwnedAdoptable $LlamaCppDir) )) { Remove-AgentInstructionFiles -Roots @($LlamaCppDir) diff --git a/tests/python/test_windows_git_gate.py b/tests/python/test_windows_git_gate.py index 60de430191..678a95c7bf 100644 --- a/tests/python/test_windows_git_gate.py +++ b/tests/python/test_windows_git_gate.py @@ -36,11 +36,30 @@ def _git_gate_block() -> str: raise AssertionError("Unclosed git gate block in setup.ps1") +def _function(name: str) -> str: + """Inject the real helper the block calls; an undefined one is a silent no-op + under Continue, which would let the layout scan always report 'nothing built'.""" + source = SETUP_PS1.read_text(encoding = "utf-8") + start = source.index(f"function {name} {{") + depth = 0 + for index in range(source.index("{", start), len(source)): + if source[index] == "{": + depth += 1 + elif source[index] == "}": + depth -= 1 + if depth == 0: + return source[start : index + 1] + raise AssertionError(f"Unclosed function {name} in setup.ps1") + + def _script() -> str: return f""" $DefaultLlamaPrForce = "0" $DefaultLlamaSource = "https://github.com/ggml-org/llama.cpp" $DefaultLlamaTag = "latest" +{_function("Test-AccessDeniedError")} +{_function("Get-PathState")} +function Exit-PathAccessDenied {{ param($Path, $Label, [switch]$UserSupplied) throw "denied: $Path" }} {_git_gate_block()} Write-Output $gitNeeded """ diff --git a/tests/sh/test_with_llama_cpp_dir_flag.sh b/tests/sh/test_with_llama_cpp_dir_flag.sh index cee158bf40..cb8b817d73 100644 --- a/tests/sh/test_with_llama_cpp_dir_flag.sh +++ b/tests/sh/test_with_llama_cpp_dir_flag.sh @@ -159,7 +159,10 @@ assert_contains \ "$SETUP_SH" 'if [ "$_RESOLVED_LOCAL" = "$_CANON_LLAMA_CPP_DIR" ]; then' assert_contains \ "setup.ps1: ignores a local dir equal to the canonical install location" \ - "$SETUP_PS1" 'if ($ResolvedLocal -eq $LlamaCppDir) {' + "$SETUP_PS1" '$LocalIsCanonical = ($ResolvedLocal -eq $LlamaCppDir)' +assert_contains \ + "setup.ps1: the canonical check actually gates the no-op branch" \ + "$SETUP_PS1" 'if ($LocalIsCanonical) {' echo "" echo "=== Results ===" diff --git a/tests/studio/install/test_setup_denied_install_tree.py b/tests/studio/install/test_setup_denied_install_tree.py new file mode 100644 index 0000000000..648b221824 --- /dev/null +++ b/tests/studio/install/test_setup_denied_install_tree.py @@ -0,0 +1,267 @@ +# SPDX-License-Identifier: AGPL-3.0-only +"""Contract checks for setup.ps1 on an unreadable llama.cpp install tree. + +Windows Test-Path raises UnauthorizedAccessException instead of returning +$false when an ACL denies the probe. setup.ps1 runs under "Stop", so the bare +probe of the prebuilt metadata aborted setup with a raw "Test-Path : Access is +denied" and exit code 1, which the desktop app showed as "unsloth studio setup +failed (exit code 1)". + +~/.unsloth/llama.cpp lives beside the app rather than inside it, so a reinstall, +even to another drive, reused the unreadable folder and failed on the same line. +The probes now go through Get-PathState / Test-PathQuiet and a denial produces +an actionable [TAURI:ERROR] message. + +Behavioural coverage against a real ACL-denied directory lives in +tests/studio/test_path_probe_access_denied.ps1. +""" + +import re +from pathlib import Path + + +ROOT = Path(__file__).resolve().parents[3] +SETUP_PS1 = (ROOT / "studio" / "setup.ps1").read_text(encoding = "utf-8") + + +def test_setup_defines_non_throwing_path_probes(): + for name in ("Test-AccessDeniedError", "Get-PathState", "Test-PathQuiet"): + assert re.search(rf"^function {re.escape(name)} \{{", SETUP_PS1, re.M), name + # Get-PathState must keep the three-way answer: collapsing "Denied" into + # "Absent" would hide the failure again instead of reporting it. + for state in ('return "Present"', 'return "Absent"', 'return "Denied"'): + assert state in SETUP_PS1 + + +def test_prebuilt_metadata_probe_cannot_terminate_setup(): + assert "if (Test-Path $existingMetaPath)" not in SETUP_PS1 + assert "$existingMetaState = Get-PathState -Path $existingMetaPath -PathType Leaf" in SETUP_PS1 + assert '$existingMetaState -eq "Denied"' in SETUP_PS1 + assert '$existingMetaState -eq "Present"' in SETUP_PS1 + + +def test_every_denial_route_reports_instead_of_proceeding(): + """An unreadable parent dir, metadata file, .git checkout, or ownership root + must all stop. Treating any of them as absent lets the caller replace or + delete a tree it cannot read.""" + assert "$llamaDirState = Get-PathState -Path $LlamaCppDir" in SETUP_PS1 + assert '$llamaDirState -eq "Denied"' in SETUP_PS1 + assert '$llamaGitState = Get-PathState -Path (Join-Path $LlamaCppDir ".git")' in SETUP_PS1 + assert '$llamaGitState -eq "Denied"' in SETUP_PS1 + assert "$pathState = Get-PathState -Path $Path -PathType Container" in SETUP_PS1 + assert '$StudioHomeIsCustom -and $pathState -eq "Denied"' in SETUP_PS1 + # The junction path replaces this destination, so it needs its own stop. + assert "$destState = Get-PathState -Path $LlamaCppDir" in SETUP_PS1 + assert '$destState -eq "Denied"' in SETUP_PS1 + # Denied counts as surviving removal; collapsing it would junction over it. + assert '(Get-PathState -Path $LlamaCppDir) -ne "Absent"' in SETUP_PS1 + # Floor, not an exact count: losing a route is the bug, adding one is not. + # Each route above is pinned by name, so a swap cannot hide under the floor. + assert SETUP_PS1.count("Exit-PathAccessDenied -Path") >= 9 + + +def test_denied_install_reports_an_actionable_failure(): + body = SETUP_PS1.split("function Exit-PathAccessDenied", 1)[1].split("\nfunction ", 1)[0] + assert "cannot be read: access is denied" in body + # The reporter reinstalled to a different drive and hit the same line; the + # message has to say why that cannot help. + assert "reinstalling Unsloth Studio, to any drive, reuses it" in body + assert "delete or rename $Path" in body + assert "Controlled folder access" in body + assert 'Exit-SetupFailure "Access denied reading the existing $Label' in body + assert "Reinstalling the app does not reset it." in body + + +def test_recovery_commands_are_separately_runnable(): + """On one line "then" is not a PowerShell separator: takeown would take the + rest as arguments and icacls would never run.""" + body = SETUP_PS1.split("function Exit-PathAccessDenied", 1)[1].split("\nfunction ", 1)[0] + command_lines = [ + line for line in body.splitlines() if "takeown /F" in line or "/reset /T" in line + ] + assert len(command_lines) == 2, command_lines + assert not any("takeown" in line and "icacls" in line for line in command_lines), command_lines + + +def test_failure_reaches_the_desktop_ui(): + """Exit-SetupFailure is the only path that emits [TAURI:ERROR], which the + desktop app prefers over its generic exit-code message.""" + body = SETUP_PS1.split("function Exit-SetupFailure", 1)[1].split("\n}", 1)[0] + assert "UNSLOTH_TAURI_MODE" in body + assert "[TAURI:ERROR] $singleLine" in body + + +def test_ownership_guard_distinguishes_denied_from_unowned(): + guard = SETUP_PS1.split("function Assert-StudioOwnedOrAbsent", 1)[1].split("\nfunction ", 1)[0] + assert ( + "$markerState = Get-PathState -Path (Join-Path $Path $StudioOwnedMarker) -PathType Leaf" + in guard + ) + assert '$markerState -eq "Denied"' in guard + # The old wording blamed ownership, which is unknowable while the tree is + # unreadable; it must stay for the genuinely-unowned case only. + assert "is not marked as an Unsloth-owned $Label" in guard + # Both stops stay gated, so default-home installs behave exactly as before. + assert guard.count("$StudioHomeIsCustom -and") == 3 + + +def test_no_bare_test_path_probes_inside_the_llama_install_tree(): + """Probes that read *inside* a tree whose permissions we do not control are + the ones that throw; they must all go through the guarded helpers.""" + inside_tree = re.compile( + r"Test-Path\b[^\n]*(\$existingMetaPath|\$llamaMarker|\$_cand|Join-Path \$LlamaCppDir)" + ) + offenders = [ + f"{index}: {line.strip()}" + for index, line in enumerate(SETUP_PS1.splitlines(), start = 1) + if inside_tree.search(line) + ] + assert not offenders, offenders + + +def test_metadata_reads_are_literal_like_the_probes_that_gate_them(): + """A literal probe followed by a globbing read still fails on a path holding + [ or ], so the probe passes and the read throws into the catch.""" + offenders = [ + f"{index}: {line.strip()}" + for index, line in enumerate(SETUP_PS1.splitlines(), start = 1) + if re.search(r"Get-Content\b[^\n]*(\$metadataPath|\$existingMetaPath|\$llamaMarker)", line) + and "-LiteralPath" not in line + ] + assert not offenders, offenders + + +def test_whisper_phase_stays_non_fatal_on_a_denied_llama_tree(): + """whisper.cpp failures degrade to Transformers dictation by contract, so + its read of the llama.cpp marker must not be able to terminate setup.""" + assert 'if (Test-PathQuiet $llamaMarker "Leaf")' in SETUP_PS1 + + +def test_reporting_helpers_tolerate_an_empty_path(): + """These run while a failure is being reported. A mandatory [string] rejects + null and empty, so without these attributes the reporter would throw a + binding exception instead of printing the actionable message.""" + for name in ("Get-PathDenialDetail", "Exit-PathAccessDenied"): + body = SETUP_PS1.split(f"function {name}", 1)[1].split("\nfunction ", 1)[0] + head = body.split("$Path", 1)[0] + assert "[AllowNull()]" in head, name + assert "[AllowEmptyString()]" in head, name + detail = SETUP_PS1.split("function Get-PathDenialDetail", 1)[1].split("\nfunction ", 1)[0] + assert 'if ([string]::IsNullOrWhiteSpace($Path)) { return "" }' in detail + + +def test_local_llama_dir_probes_are_three_state(): + """--with-llama-cpp-dir pointed at the canonical location reuses whatever is + built there so the prebuilt installer cannot replace it. A denied binary read + as "nothing built" put that replacement back, which is what the branch exists + to prevent.""" + assert "$localSrcState = Get-PathState -Path $LocalLlamaCppSrc -PathType Container" in SETUP_PS1 + assert '$localSrcState -eq "Denied"' in SETUP_PS1 + local_block = SETUP_PS1.split("$LocalLlamaCppSrc = $env:UNSLOTH_LOCAL_LLAMA_CPP_DIR", 1)[1] + local_block = local_block.split("if ($LocalLlamaCppLinked) {", 1)[0] + assert "$candState = Get-PathState -Path $_cand" in local_block + assert '$candState -eq "Denied"' in local_block + assert '$candState -eq "Present"' in local_block + assert "Test-PathQuiet $_cand" not in local_block + # The disk-space branch keeps Test-PathQuiet on purpose: it only decides + # whether a preserved binary is usable, and an unreadable one is not. + + +def test_phase_1b_git_scan_is_guarded_too(): + """The git prerequisite scan probes the same candidate binaries in Phase 1b, + thousands of lines before the Phase 4 guards, and under "Stop". A denial + there reproduced the original raw termination.""" + scan = SETUP_PS1.split("$_localLlamaBuilt = $false", 1)[1].split( + "if (-not $_localLlamaBuilt) {", 1 + )[0] + assert "$_cState = Get-PathState -Path (Join-Path $_localLlamaDir $_c)" in scan + assert '$_cState -eq "Denied"' in scan + assert '$_cState -eq "Present"' in scan + assert "Test-Path -LiteralPath (Join-Path $_localLlamaDir $_c)" not in scan + assert "-UserSupplied" in scan + + +def test_adoption_markers_keep_their_denial(): + """A denied prebuilt marker is not evidence of absence. Collapsing it made the + ownership guard call an Unsloth tree an unrelated directory and tell the user + to move it aside, hiding a permissions problem.""" + assert "function Get-StudioAdoptableState" in SETUP_PS1 + state = SETUP_PS1.split("function Get-StudioAdoptableState", 1)[1].split("\nfunction ", 1)[0] + assert "Get-PathState -Path (Join-Path $Path $marker) -PathType Leaf" in state + for verdict in ('return "Yes"', 'return "No"', 'return "Denied"'): + assert verdict in state + guard = SETUP_PS1.split("function Assert-StudioOwnedOrAbsent", 1)[1].split("\nfunction ", 1)[0] + assert "$adoptState = Get-StudioAdoptableState -Path $Path" in guard + assert '$adoptState -eq "Denied"' in guard + # The denial must be reported before the "not Unsloth-owned" wording. + assert guard.index('$adoptState -eq "Denied"') < guard.index( + "is not marked as an Unsloth-owned" + ) + + +def test_user_supplied_paths_are_never_told_to_delete_themselves(): + """The managed advice ("delete it, Unsloth reinstalls it") is wrong for a tree + the user pointed us at with UNSLOTH_LOCAL_LLAMA_CPP_DIR.""" + body = SETUP_PS1.split("function Exit-PathAccessDenied", 1)[1].split("\nfunction ", 1)[0] + assert "[switch]$UserSupplied" in body + user_branch = body.split("if ($UserSupplied) {", 1)[1].split("} else {", 1)[0] + assert "delete or rename" not in user_branch + assert "managed cache" not in user_branch + assert "UNSLOTH_LOCAL_LLAMA_CPP_DIR at a readable build" in user_branch + # Every call site that reports a path the user pointed us at must pass the + # switch, including the canonical location: the override says that tree is + # the user's build, so "delete it, we reinstall it" is wrong there too. + for line in SETUP_PS1.splitlines(): + if "Exit-PathAccessDenied" in line and "UNSLOTH_LOCAL_LLAMA_CPP_DIR" in line: + assert "-UserSupplied" in line, line + # Keyed on the path being reported, not on position: this block also reports + # the managed destination ($LlamaCppDir), where "delete it" is the right advice. + local_block = SETUP_PS1.split("$LocalLlamaCppSrc = $env:UNSLOTH_LOCAL_LLAMA_CPP_DIR", 1)[1] + local_block = local_block.split("if ($LocalLlamaCppLinked) {", 1)[0] + for line in local_block.splitlines(): + if "Exit-PathAccessDenied" not in line: + continue + if "$ResolvedLocal" in line or "$LocalLlamaCppSrc" in line: + assert "-UserSupplied" in line, line + elif "$LlamaCppDir" in line: + assert "-UserSupplied" not in line, line + + +def test_the_ownership_guard_never_advises_deleting_an_unverified_tree(): + """The guard stops because it could not read the marker, so it cannot claim + the tree is ours. It already says "move it aside" when it can prove it.""" + guard = SETUP_PS1.split("function Assert-StudioOwnedOrAbsent", 1)[1].split("\nfunction ", 1)[0] + calls = [line.strip() for line in guard.splitlines() if "Exit-PathAccessDenied" in line] + assert len(calls) == 3, calls + for line in calls: + assert "-OwnershipUnverified" in line, line + body = SETUP_PS1.split("function Exit-PathAccessDenied", 1)[1].split("\nfunction ", 1)[0] + assert "[switch]$OwnershipUnverified" in body + branch = body.split("} elseif ($OwnershipUnverified) {", 1)[1].split("} else {", 1)[0] + assert "delete" not in branch.lower(), branch + assert "managed cache" not in branch, branch + assert "move the folder aside" in branch, branch + + +def test_the_reparse_point_unlink_reports_a_denied_delete(): + """A link probes Present, so the destination check below cannot cover it.""" + block = SETUP_PS1.split("$existing = Get-Item -LiteralPath $LlamaCppDir", 1)[1] + block = block.split("$destState", 1)[0] + assert "try { $existing.Delete() }" in block, block + assert "Test-AccessDeniedError" in block, block + + +def test_the_temp_dir_swap_checks_both_of_its_destructive_steps(): + """Remove-Item and Move-Item are both non-terminating here, and Move-Item + onto a surviving directory nests the build inside it rather than replacing + it, so setup would report a build it never installed.""" + swap = SETUP_PS1.split("# Swap temp build dir into final location", 1)[1] + swap = swap.split("} elseif (-not $BuildOk", 1)[0] + assert "$swapState = Get-PathState -Path $OriginalLlamaCppDir" in swap, swap + assert '$swapState -eq "Denied"' in swap, swap + # Stop before the move runs, while the temp build is still whole. + move = "Move-Item -LiteralPath $LlamaCppDir" + assert swap.index('$swapState -ne "Absent"') < swap.index(move), swap + # And catch a move that silently did not happen. + assert '(Get-PathState -Path $LlamaCppDir) -ne "Absent"' in swap.split(move, 1)[1], swap + assert "Test-Path -LiteralPath $OriginalLlamaCppDir" not in swap, swap diff --git a/tests/studio/test_ci_shell_suite_coverage.py b/tests/studio/test_ci_shell_suite_coverage.py index 50ed22f9de..4b7768a9b6 100644 --- a/tests/studio/test_ci_shell_suite_coverage.py +++ b/tests/studio/test_ci_shell_suite_coverage.py @@ -195,3 +195,128 @@ class TestBackendCiPathFilters: if __name__ == "__main__": pytest.main([__file__, "-v"]) + + +def _github_path_matcher(pattern: str) -> re.Pattern: + """GitHub path filters: ** crosses directories, * and ? do not.""" + out, i = [], 0 + while i < len(pattern): + c = pattern[i] + if pattern.startswith("**", i): + out.append(".*") + i += 2 + elif c == "*": + out.append("[^/]*") + i += 1 + elif c == "?": + out.append("[^/]") + i += 1 + else: + out.append(re.escape(c)) + i += 1 + return re.compile("^" + "".join(out) + "$") + + +def _workflows_running_powershell_tests(): + """Every workflow that invokes a tests/**.ps1 file, with its PR path filter.""" + found = {} + for workflow in sorted(_WORKFLOWS.glob("*.yml")): + text = workflow.read_text(encoding = "utf-8") + invoked = sorted(set(re.findall(r"pwsh -NoProfile -File (tests/[^\s`\"']+\.ps1)", text))) + if not invoked: + continue + parsed = yaml.safe_load(text) + # PyYAML parses the `on:` key as the boolean True. + triggers = parsed.get(True, parsed.get("on", {})) or {} + paths = (triggers.get("pull_request") or {}).get("paths") + found[workflow.name] = (invoked, paths) + return found + + +class TestGithubPathMatcher: + """The guard below is only as good as this matcher; a wrong one would pass + everything silently.""" + + @pytest.mark.parametrize( + "pattern,path,expected", + [ + ("tests/studio/*.ps1", "tests/studio/test_x.ps1", True), + ("tests/studio/*.ps1", "tests/studio/nested/test_x.ps1", False), + ("tests/studio/*.ps1", "tests/studio/test_x.py", False), + ("tests/studio/**", "tests/studio/nested/test_x.ps1", True), + ("studio/**", "studio/setup.ps1", True), + ("studio/**", "tests/studio/setup.ps1", False), + ( + "tests/studio/test_uninstall_*.ps1", + "tests/studio/test_uninstall_arg_guard.ps1", + True, + ), + ("tests/studio/test_uninstall_*.ps1", "tests/studio/test_node_decision.ps1", False), + ("install.ps1", "install.ps1", True), + ("install.ps1", "studio/install.ps1", False), + ], + ) + def test_matcher_semantics(self, pattern, path, expected): + assert bool(_github_path_matcher(pattern).match(path)) is expected + + +class TestPowerShellTestsRunOnAPr: + """tests/sh had this exact hole (see the module docstring) and so did the + Windows side: studio-windows-inference-smoke.yml ran six PowerShell tests + while its path filter matched none of them, so a PR fixing one of those + tests never ran it.""" + + def test_some_workflow_runs_powershell_tests(self): + assert ( + _workflows_running_powershell_tests() + ), "no workflow invokes a tests/*.ps1 file; did the invocation form change?" + + def test_every_invoked_powershell_test_triggers_its_workflow(self): + unguarded = [] + for name, (invoked, paths) in _workflows_running_powershell_tests().items(): + if paths is None: + continue # no filter at all means it always runs + matchers = [_github_path_matcher(p) for p in paths] + for test in invoked: + if not any(m.match(test) for m in matchers): + unguarded.append(f"{name} runs {test} but its paths filter never matches it") + assert not unguarded, ( + "these PowerShell tests can break without any PR running them; add the " + f"path (or a scoped glob) to the workflow's paths filter: {unguarded}" + ) + + def test_multi_test_steps_propagate_each_exit_code(self): + """A `shell: pwsh` step inherits only the LAST command's exit code, so a + step running several tests must check $LASTEXITCODE after each one. + Without it, test_resolve_cuda_toolkit.ps1 failed two checks on every + Windows run for as long as anyone can tell, and CI stayed green.""" + offenders = [] + for workflow in sorted(_WORKFLOWS.glob("*.yml")): + for block in re.findall( + r"run: \|\n(.*?)(?=\n [-a-zA-Z]|\Z)", + workflow.read_text(encoding = "utf-8"), + re.S, + ): + invocations = re.findall( + r"pwsh -NoProfile -File (tests/[^\s`\"']+\.ps1)[^\n]*\n(.*?)(?=pwsh -NoProfile -File|\Z)", + block, + re.S, + ) + if len(invocations) < 2: + continue # a single invocation's exit code is the step's + for test, following in invocations: + if "$LASTEXITCODE" not in following: + offenders.append(f"{workflow.name}: {test} runs without an exit-code check") + assert not offenders, ( + "these tests can fail without failing their step; add " + f"`if ($LASTEXITCODE) {{ exit $LASTEXITCODE }}` after each: {offenders}" + ) + + def test_every_invoked_powershell_test_exists(self): + missing = [ + f"{name} -> {test}" + for name, (invoked, _) in _workflows_running_powershell_tests().items() + for test in invoked + if not (REPO_ROOT / test).is_file() + ] + assert not missing, f"workflows invoke PowerShell tests that do not exist: {missing}" diff --git a/tests/studio/test_path_probe_access_denied.ps1 b/tests/studio/test_path_probe_access_denied.ps1 new file mode 100644 index 0000000000..96aa1d64a4 --- /dev/null +++ b/tests/studio/test_path_probe_access_denied.ps1 @@ -0,0 +1,243 @@ +# Regression test for setup.ps1 path probes on an ACL-denied install tree. +# +# Test-Path raises UnauthorizedAccessException instead of returning $false when +# an ACL denies the probe, and setup.ps1 runs under "Stop", so the bare probe of +# the llama.cpp prebuilt metadata aborted setup with a raw "Test-Path : Access +# is denied" and exit code 1. ~/.unsloth/llama.cpp outlives an app reinstall, so +# reinstalling, even to another drive, hit the same line again. +# +# The probes now go through Get-PathState / Test-PathQuiet, which never +# terminate and keep "Denied" distinct from "Absent". This runs the real +# functions against a genuinely unreadable directory (chmod on Unix, icacls deny +# on Windows). +$ErrorActionPreference = "Stop" +$script:failures = 0 +function Check($name, $cond) { + if ($cond) { Write-Host " PASS $name" } + else { Write-Host " FAIL $name" -ForegroundColor Red; $script:failures++ } +} + +$repoRoot = (Resolve-Path ([System.IO.Path]::Combine($PSScriptRoot, "..", ".."))).Path +$setupPath = [System.IO.Path]::Combine($repoRoot, "studio", "setup.ps1") +. ([System.IO.Path]::Combine($repoRoot, "tests", "studio_setup_ps1", "Get-FunctionSource.ps1")) + +foreach ($fn in @("Test-AccessDeniedError", "Get-PathState", "Test-PathQuiet", + "Get-PathDenialDetail", "Get-StudioAdoptableState", + "Test-StudioOwnedAdoptable")) { + $src = Get-FunctionSource -Path $setupPath -Name $fn + Check "setup.ps1 defines $fn" ($null -ne $src) + if ($src) { . ([scriptblock]::Create($src)) } +} + +# ── Source contract: the crash site must probe state, not bare Test-Path ── +$setupText = Get-Content -Raw -LiteralPath $setupPath +Check "prebuilt metadata probe no longer uses a bare Test-Path" ( + $setupText -notmatch '\n\s*if \(Test-Path \$existingMetaPath\)') +Check "prebuilt metadata probe goes through Get-PathState" ( + $setupText -match '\$existingMetaState = Get-PathState -Path \$existingMetaPath -PathType Leaf') +Check "a denied llama.cpp install fails with an actionable message" ( + $setupText -match '\$existingMetaState -eq "Denied"' -and + $setupText -match 'Exit-SetupFailure "Access denied reading the existing \$Label') +# Every denial route reports instead of proceeding: an unreadable parent dir, an +# unreadable metadata file, an unreadable .git checkout, and the ownership guard. +Check "the prebuilt phase stops on a denied llama.cpp dir" ( + $setupText -match '\$llamaDirState = Get-PathState -Path \$LlamaCppDir' -and + $setupText -match '\$llamaDirState -eq "Denied"') +Check "the source-build .git probe stops on a denied checkout" ( + $setupText -match '\$llamaGitState = Get-PathState -Path \(Join-Path \$LlamaCppDir "\.git"\)' -and + $setupText -match '\$llamaGitState -eq "Denied"') +Check "the ownership guard stops on a denied root instead of returning" ( + $setupText -match '\$pathState = Get-PathState -Path \$Path -PathType Container' -and + $setupText -match '\$StudioHomeIsCustom -and \$pathState -eq "Denied"') +Check "guidance says an app reinstall does not reset the folder" ( + $setupText -match 'reinstalling Unsloth Studio, to any drive, reuses it' -and + $setupText -match 'Reinstalling the app does not reset it\.') +Check "guidance names the concrete recovery commands" ( + $setupText -match 'takeown /F' -and $setupText -match 'icacls .* /reset /T') +Check "whisper marker probe cannot terminate the non-fatal whisper phase" ( + $setupText -match 'if \(Test-PathQuiet \$llamaMarker "Leaf"\)') + +# ── Behaviour against a real unreadable directory ── +$root = Join-Path ([System.IO.Path]::GetTempPath()) ("uns_acl_" + [guid]::NewGuid().ToString("N")) +$locked = Join-Path $root "llama.cpp" +New-Item -ItemType Directory -Force -Path $locked | Out-Null +$meta = Join-Path $locked "UNSLOTH_PREBUILT_INFO.json" +Set-Content -LiteralPath $meta -Value '{"release_tag":"app-1","published_repo":"unslothai/llama.cpp"}' + +$onWindows = ($env:OS -eq "Windows_NT") +function Set-Denied([bool]$on) { + if ($onWindows) { + $who = "$env:USERDOMAIN\$env:USERNAME" + if ($on) { icacls $locked /deny "${who}:(OI)(CI)(RX)" *>$null } + else { icacls $locked /remove:d "$who" *>$null } + } else { + if ($on) { chmod 000 $locked } else { chmod 755 $locked } + } +} + +try { + Check "readable metadata reports Present" ((Get-PathState -Path $meta -PathType Leaf) -eq "Present") + Check "readable install is adoptable" (Test-StudioOwnedAdoptable $locked) + Check "absent path reports Absent" ( + (Get-PathState -Path (Join-Path $root "missing.json") -PathType Leaf) -eq "Absent") + + Set-Denied $true + + # Negative control AND environment gate: the old unguarded form must blow up + # here, otherwise this host cannot produce a denial (root / admin bypass) + # and the assertions below would pass vacuously. + $oldFormTerminated = $false + try { $null = Test-Path $meta } catch { $oldFormTerminated = $true } + + if (-not $oldFormTerminated) { + Write-Host " SKIP cannot deny access on this host (running as root/admin?) -- behaviour checks skipped" -ForegroundColor Yellow + } else { + Check "bare Test-Path still terminates on a denied path (negative control)" $oldFormTerminated + $state = $null + $threw = $false + try { $state = Get-PathState -Path $meta -PathType Leaf } catch { $threw = $true } + Check "Get-PathState does not terminate on a denied path" (-not $threw) + Check "Get-PathState reports Denied (not Absent)" ($state -eq "Denied") + + $quiet = $null + $threw = $false + try { $quiet = Test-PathQuiet $meta } catch { $threw = $true } + Check "Test-PathQuiet does not terminate on a denied path" (-not $threw) + Check "Test-PathQuiet reports the path as unusable" ($quiet -eq $false) + + $threw = $false + $adoptable = $null + try { $adoptable = Test-StudioOwnedAdoptable $locked } catch { $threw = $true } + Check "Test-StudioOwnedAdoptable does not terminate on a denied tree" (-not $threw) + Check "Test-StudioOwnedAdoptable cannot adopt an unreadable tree" ($adoptable -eq $false) + } +} finally { + Set-Denied $false + Remove-Item -Recurse -Force -LiteralPath $root -ErrorAction SilentlyContinue +} + +# ── Test-Path parity: the regression-safety invariant ── +# Wherever the old bare probe returned a value instead of throwing, Get-PathState +# must agree. Denied may only appear where the old probe threw, so no path that +# used to work can take a different branch now. +$parityRoot = Join-Path ([System.IO.Path]::GetTempPath()) ("uns_par_" + [guid]::NewGuid().ToString("N")) +New-Item -ItemType Directory -Force -Path (Join-Path $parityRoot "tree/sub") | Out-Null +Set-Content -LiteralPath (Join-Path $parityRoot "tree/UNSLOTH_PREBUILT_INFO.json") -Value "{}" +Set-Content -LiteralPath (Join-Path $parityRoot "tree/sub/file.txt") -Value "x" +$parityProbes = @($parityRoot, (Join-Path $parityRoot "tree"), (Join-Path $parityRoot "tree/sub"), + (Join-Path $parityRoot "tree/UNSLOTH_PREBUILT_INFO.json"), (Join-Path $parityRoot "tree/sub/file.txt"), + (Join-Path $parityRoot "missing"), (Join-Path $parityRoot "missing/deeper.json")) +$mismatch = 0; $deniedWithoutThrow = 0; $probed = 0 +foreach ($p in $parityProbes) { + foreach ($t in @("Any", "Leaf", "Container")) { + $old = $null; $threw = $false + try { $old = [bool](Test-Path -LiteralPath $p -PathType $t -ErrorAction Stop) } catch { $threw = $true } + $new = Get-PathState -Path $p -PathType $t + $probed++ + if ($threw) { if ($new -notin @("Denied", "Absent")) { $mismatch++ } } + elseif ($new -eq "Denied") { $deniedWithoutThrow++ } + elseif ($old -ne ($new -eq "Present")) { $mismatch++ } + } +} +Remove-Item -Recurse -Force -LiteralPath $parityRoot -ErrorAction SilentlyContinue +Check "Get-PathState matches bare Test-Path on every non-throwing probe ($probed)" ($mismatch -eq 0) +Check "Denied never appears where the old probe did not throw" ($deniedWithoutThrow -eq 0) + +# ── A denied marker FILE under a readable directory ── +# Windows only: POSIX keeps a mode-000 file stat-able, so this state cannot be +# built on Unix. Collapsing it to "No" made the ownership guard report an +# Unsloth tree as an unrelated directory instead of a permissions problem. +$adoptRoot = Join-Path ([System.IO.Path]::GetTempPath()) ("uns_adopt_" + [guid]::NewGuid().ToString("N")) +New-Item -ItemType Directory -Force -Path $adoptRoot | Out-Null +$adoptMarker = Join-Path $adoptRoot "UNSLOTH_PREBUILT_INFO.json" +Set-Content -LiteralPath $adoptMarker -Value '{"release_tag":"app-1"}' +Check "a readable marker reports Yes" ((Get-StudioAdoptableState -Path $adoptRoot) -eq "Yes") +if ($onWindows) { + $who = "$env:USERDOMAIN\$env:USERNAME" + icacls $adoptMarker /deny "${who}:(R)" *>$null + try { + $markerThrew = $false + try { $null = Test-Path -LiteralPath $adoptMarker -PathType Leaf -ErrorAction Stop } catch { $markerThrew = $true } + if ($markerThrew) { + Check "a denied marker reports Denied, not No" ((Get-StudioAdoptableState -Path $adoptRoot) -eq "Denied") + Check "the boolean view still refuses to adopt it" (-not (Test-StudioOwnedAdoptable $adoptRoot)) + } else { + Write-Host " SKIP this host would not deny the marker file" -ForegroundColor Yellow + } + } finally { icacls $adoptMarker /remove:d "$who" *>$null } +} else { + Write-Host " SKIP denied marker file is a Windows-ACL-only state (POSIX keeps mode-000 files stat-able)" -ForegroundColor Yellow +} +Remove-Item -Recurse -Force -LiteralPath $adoptRoot -ErrorAction SilentlyContinue +Check "a missing marker reports No" ((Get-StudioAdoptableState -Path ([System.IO.Path]::GetTempPath())) -eq "No") + +# ── The reporting path must not itself fail ── +# Get-PathDenialDetail runs while a failure is being reported, so a null or empty +# path must not replace the actionable message with a binding exception. +foreach ($edge in @($null, "", " ")) { + $edgeOk = $true + try { $null = Get-PathDenialDetail -Path $edge } catch { $edgeOk = $false } + Check "Get-PathDenialDetail tolerates an empty/null path" $edgeOk +} + +# ── The desktop app must receive the reason, not just "exit code 1" ── +# The real Exit-PathAccessDenied with the real Exit-SetupFailure in Tauri mode: +# install.rs prefers a [TAURI:ERROR] line over its generic exit-code message, so +# this is what the user reads. +$exitDeniedSrc = Get-FunctionSource -Path $setupPath -Name Exit-PathAccessDenied +$exitSetupSrc = Get-FunctionSource -Path $setupPath -Name Exit-SetupFailure +Check "setup.ps1 defines Exit-PathAccessDenied" ($null -ne $exitDeniedSrc) +if ($exitDeniedSrc) { + $harness = @" +`$ErrorActionPreference = "Stop" +function step { param([string]`$Label, [string]`$Value, [string]`$Color = "Green") Write-Host " `$Label `$Value" } +function substep { param([string]`$Message, [string]`$Color = "DarkGray") Write-Host " `$Message" } +function Get-PathDenialDetail { param([string]`$Path) return "" } +$exitSetupSrc +$exitDeniedSrc +Exit-PathAccessDenied -Path "C:\Users\test\.unsloth\llama.cpp" -Label "llama.cpp install" +Write-Host "REACHED_UNREACHABLE" +"@ + $harnessFile = Join-Path ([System.IO.Path]::GetTempPath()) ("uns_denied_" + [guid]::NewGuid().ToString("N") + ".ps1") + Set-Content -LiteralPath $harnessFile -Value $harness -Encoding utf8 + $pwshExe = (Get-Command pwsh -ErrorAction SilentlyContinue).Source + if (-not $pwshExe) { $pwshExe = (Get-Command powershell).Source } + $savedMode = $env:UNSLOTH_TAURI_MODE + try { + $env:UNSLOTH_TAURI_MODE = "1" + $out = & $pwshExe -NoProfile -File $harnessFile 2>&1 | Out-String + $code = $LASTEXITCODE + } finally { + if ($null -eq $savedMode) { Remove-Item Env:UNSLOTH_TAURI_MODE -ErrorAction SilentlyContinue } + else { $env:UNSLOTH_TAURI_MODE = $savedMode } + Remove-Item -LiteralPath $harnessFile -ErrorAction SilentlyContinue + } + Check "the denial stops setup (exit 1)" ($code -eq 1) + Check "the denial does not fall through to the install" ($out -notmatch "REACHED_UNREACHABLE") + Check "the desktop app gets a [TAURI:ERROR] reason, not a bare exit code" ( + $out -match '\[TAURI:ERROR\] Access denied reading the existing llama\.cpp install') + Check "the reason names the folder to remove" ($out -match [regex]::Escape('C:\Users\test\.unsloth\llama.cpp')) + Check "the reason says a reinstall will not help" ($out -match 'Reinstalling the app does not reset it') + # takeown and icacls must be copy-pasteable. On one line, "then" is not a + # PowerShell separator and takeown would swallow the rest as arguments. + $takeownLines = @($out -split "`r?`n" | Where-Object { $_ -match 'takeown /F' }) + Check "takeown is printed on its own line" ($takeownLines.Count -eq 1) + Check "icacls is not appended to the takeown line" ( + $takeownLines.Count -eq 1 -and $takeownLines[0] -notmatch 'icacls') + Check "icacls is printed on its own line" ( + @($out -split "`r?`n" | Where-Object { $_ -match 'icacls .* /reset /T' }).Count -eq 1) +} + +# ── Denial classification ── +Check "UnauthorizedAccessException classifies as access denied" ( + Test-AccessDeniedError ([System.UnauthorizedAccessException]::new("denied"))) +Check "a wrapped UnauthorizedAccessException classifies as access denied" ( + Test-AccessDeniedError ([System.Exception]::new("outer", [System.UnauthorizedAccessException]::new("denied")))) +Check "an unrelated exception does not classify as access denied" ( + -not (Test-AccessDeniedError ([System.IO.FileNotFoundException]::new("missing")))) + +if ($script:failures -gt 0) { + Write-Host "$($script:failures) check(s) failed" -ForegroundColor Red + exit 1 +} +Write-Host "All checks passed" -ForegroundColor Green diff --git a/tests/studio/test_resolve_cuda_toolkit.ps1 b/tests/studio/test_resolve_cuda_toolkit.ps1 index 6be0adc621..c1114f7b38 100644 --- a/tests/studio/test_resolve_cuda_toolkit.ps1 +++ b/tests/studio/test_resolve_cuda_toolkit.ps1 @@ -31,6 +31,16 @@ $mismatchFn = $ast.FindAll({ param($n) if ($mismatchFn.Count -ne 1) { throw "expected exactly one Write-CudaDriverToolkitMismatch, found $($mismatchFn.Count)" } $mismatchText = $mismatchFn[0].Extent.Text +# -RequireOrExit leaves through Exit-SetupFailure, so the child needs the real +# one. Without it the call was an ignored command-not-found under +# ErrorActionPreference=Continue, the child fell through and exited 0, and the +# two "exits non-zero" checks failed on every run while CI stayed green. +$exitFn = $ast.FindAll({ param($n) + $n -is [System.Management.Automation.Language.FunctionDefinitionAst] -and $n.Name -eq "Exit-SetupFailure" +}, $true) +if ($exitFn.Count -ne 1) { throw "expected exactly one Exit-SetupFailure, found $($exitFn.Count)" } +$exitText = $exitFn[0].Extent.Text + # --- Spoof executables for driver/toolkit compatibility scenarios --- $work = Join-Path ([System.IO.Path]::GetTempPath()) ("rct_" + [guid]::NewGuid().ToString("N")) New-Item -ItemType Directory -Force -Path $work | Out-Null @@ -84,6 +94,8 @@ function Find-Nvcc { `$script:CudaToolkitReady = `$false `$script:NvccPath = `$null; `$script:CudaToolkitRoot = `$null; `$script:CudaArch = `$null +$exitText + $mismatchText $fnText diff --git a/tests/test_studio_install_workspace_guard.py b/tests/test_studio_install_workspace_guard.py index fa6c8afea4..aeb22cac45 100644 --- a/tests/test_studio_install_workspace_guard.py +++ b/tests/test_studio_install_workspace_guard.py @@ -253,7 +253,9 @@ def test_setup_ps1_prebuilt_llama_cpp_has_ownership_guard(): def test_setup_ps1_adopts_existing_whisper_prebuilt_marker(): text = SETUP_PS1.read_text(encoding = "utf-8") - helper_start = text.index("function Test-StudioOwnedAdoptable") + # The marker scan lives in Get-StudioAdoptableState; Test-StudioOwnedAdoptable + # is the boolean view of it. + helper_start = text.index("function Get-StudioAdoptableState") helper_end = text.index("function Assert-StudioOwnedOrAbsent", helper_start) helper = text[helper_start:helper_end] assert "UNSLOTH_WHISPER_PREBUILT_INFO.json" in helper @@ -379,7 +381,8 @@ def test_setup_helpers_gate_on_canonical_custom_root(): def test_setup_ps1_inplace_git_sync_marks_studio_owned(): """setup.ps1 in-place git-sync branch must Mark-StudioOwned after a successful sync.""" src = SETUP_PS1.read_text(encoding = "utf-8") - inplace_idx = src.index('Test-Path -LiteralPath (Join-Path $LlamaCppDir ".git")') + # Three-state probe so an ACL-denied tree stops instead of cloning over it. + inplace_idx = src.index('if ($llamaGitState -eq "Present") {') # The in-place branch ends just before the temp-dir clone branch. clone_idx = src.index("Cloning llama.cpp @", inplace_idx) inplace_block = src[inplace_idx:clone_idx] @@ -394,7 +397,8 @@ def test_setup_ps1_inplace_git_sync_marks_studio_owned(): def test_setup_ps1_inplace_git_sync_asserts_studio_owned_before_mutation(): """setup.ps1 in-place git-sync must Assert-StudioOwnedOrAbsent before any destructive git op.""" src = SETUP_PS1.read_text(encoding = "utf-8") - inplace_idx = src.index('Test-Path -LiteralPath (Join-Path $LlamaCppDir ".git")') + # Three-state probe so an ACL-denied tree stops instead of cloning over it. + inplace_idx = src.index('if ($llamaGitState -eq "Present") {') clone_idx = src.index("Cloning llama.cpp @", inplace_idx) inplace_block = src[inplace_idx:clone_idx] assert (