Carry the denial through three more probes

Three review points, all reproduced before fixing:

- The canonical --with-llama-cpp-dir override still got the managed
  advice ("delete it, Unsloth reinstalls it"). The override means the
  user asked to reuse whatever is in that tree, so deleting it is wrong
  wherever it sits. Both candidate denials now pass -UserSupplied, which
  collapses the branch to one call.
- Phase 1b's git prerequisite scan probes the same candidate binaries
  with a bare Test-Path under "Stop", thousands of lines before the
  Phase 4 guards, so a denied override terminated the run with the raw
  error this change exists to replace. Reproduced, then guarded.
- Test-StudioOwnedAdoptable collapsed a denied prebuilt marker to $false,
  so Assert-StudioOwnedOrAbsent called an Unsloth tree an unrelated
  directory and told the user to move it aside. Get-StudioAdoptableState
  returns Yes/No/Denied and the guard reports the denial first;
  Test-StudioOwnedAdoptable stays as the boolean view for the cosmetic
  cleanup gate.

A denied file under a readable directory is a Windows-ACL-only state:
POSIX keeps a mode-000 file stat-able, and a symlink into a denied
directory still answers Test-Path. The local run injects that one state
at the lowest seam and lets the real functions run; the Windows leg of
test_path_probe_access_denied.ps1 builds it for real with icacls and
skips elsewhere with the reason.

test_setup_ps1_adopts_existing_whisper_prebuilt_marker sliced between two
function names and the marker scan moved, so its anchor now points at
Get-StudioAdoptableState. Its assertion is unchanged.

Reverting each fix individually puts the original behaviour back: the
ownership misdiagnosis, the raw "Access to the path ... is denied" from
Phase 1b, and the delete-your-own-build advice.
This commit is contained in:
Michael Han 2026-08-02 03:33:35 -07:00
parent 2a613431fb
commit de486ff873
4 changed files with 106 additions and 15 deletions

View file

@ -1860,7 +1860,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) {
@ -2906,12 +2912,25 @@ $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.
# Test-PathQuiet: an unreadable tree must reach the errors below, not throw here.
# "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-PathQuiet (Join-Path $Path "UNSLOTH_PREBUILT_INFO.json") "Leaf") { return $true }
if (Test-PathQuiet (Join-Path $Path "UNSLOTH_WHISPER_PREBUILT_INFO.json") "Leaf") { return $true }
return $false
return ((Get-StudioAdoptableState -Path $Path) -eq "Yes")
}
function Assert-StudioOwnedOrAbsent {
param(
@ -2934,7 +2953,11 @@ function Assert-StudioOwnedOrAbsent {
Exit-PathAccessDenied -Path $Path -Label $Label
}
if ($StudioHomeIsCustom -and $markerState -ne "Present") {
if (Test-StudioOwnedAdoptable $Path) {
$adoptState = Get-StudioAdoptableState -Path $Path
if ($adoptState -eq "Denied") {
Exit-PathAccessDenied -Path $Path -Label $Label
}
if ($adoptState -eq "Yes") {
Mark-StudioOwned $Path
return
}
@ -3984,10 +4007,10 @@ if ($LocalLlamaCppSrc) {
# replaces the very build this flag asked to reuse.
$candState = Get-PathState -Path $_cand
if ($candState -eq "Denied") {
if ($LocalIsCanonical) {
Exit-PathAccessDenied -Path $ResolvedLocal -Label "llama.cpp install"
}
Exit-PathAccessDenied -Path $ResolvedLocal -Label "the --with-llama-cpp-dir build" -UserSupplied
# -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 }
}

View file

@ -50,7 +50,7 @@ def test_every_denial_route_reports_instead_of_proceeding():
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
assert SETUP_PS1.count("Exit-PathAccessDenied -Path") == 8
assert SETUP_PS1.count("Exit-PathAccessDenied -Path") == 9
def test_denied_install_reports_an_actionable_failure():
@ -148,6 +148,38 @@ def test_local_llama_dir_probes_are_three_state():
# 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."""
@ -157,9 +189,14 @@ def test_user_supplied_paths_are_never_told_to_delete_themselves():
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 user-supplied call site must actually pass the switch.
# 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
if "Exit-PathAccessDenied" in line and "--with-llama-cpp-dir" in line:
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" in line:
assert "-UserSupplied" in line, line

View file

@ -22,7 +22,8 @@ $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", "Test-StudioOwnedAdoptable")) {
"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)) }
@ -142,6 +143,34 @@ Remove-Item -Recurse -Force -LiteralPath $parityRoot -ErrorAction SilentlyContin
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.

View file

@ -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