From e0c194cfe1e0a9be2776dfd855d6d95a95c6adb1 Mon Sep 17 00:00:00 2001 From: Huanqi Cao Date: Tue, 29 Sep 2026 16:12:42 +0800 Subject: [PATCH] fix(windows): classify after the subtree scan and surface report failures The classification was recorded before the subtree scan, so a run that found and removed package ACEs under the requested directory still reported NOT_THIS_CLASS or PRECONDITION with an empty packageObjects, contradicting README and SKILL. The scan now runs first, so the verdict and packageObjects describe what the run actually repairs. A report-file write failure now prints REPORT_IO_FAILED with the path and the error on stderr instead of silently leaving an incomplete report behind. --- .../scripts/diagnose-windows-sandbox-acl.ps1 | 49 +++++++++++-------- .../tests/diagnose-script.spec.ts | 8 ++- 2 files changed, 35 insertions(+), 22 deletions(-) diff --git a/packages/sandbox/sandbox-windows-acl/assets/diagnose-windows-sandbox-acl/scripts/diagnose-windows-sandbox-acl.ps1 b/packages/sandbox/sandbox-windows-acl/assets/diagnose-windows-sandbox-acl/scripts/diagnose-windows-sandbox-acl.ps1 index bead02a280..81c742c233 100644 --- a/packages/sandbox/sandbox-windows-acl/assets/diagnose-windows-sandbox-acl/scripts/diagnose-windows-sandbox-acl.ps1 +++ b/packages/sandbox/sandbox-windows-acl/assets/diagnose-windows-sandbox-acl/scripts/diagnose-windows-sandbox-acl.ps1 @@ -87,7 +87,13 @@ function Write-Report { # tail, so a long run's early records survive only there. if ($null -ne $script:reportWriter) { try { $script:reportWriter.WriteLine($json); $script:reportWriter.Flush() } - catch [System.IO.IOException] { $script:reportWriter = $null } + catch [System.IO.IOException] { + # stdout keeps only its tail, so this file is the complete record: a write failure + # has to reach the caller, on stderr, instead of leaving a silently incomplete file. + [Console]::Error.WriteLine('REPORT_IO_FAILED {0}: {1}' -f $script:reportPath, $_.Exception.Message) + try { $script:reportWriter.Dispose() } catch { } + $script:reportWriter = $null + } } [Console]::Out.WriteLine('REPORT ' + $json) } @@ -565,6 +571,27 @@ try { $needsPrecondition = (-not $unreadable) -and ((-not $targetFacts.HasWriteDac) -or (-not $targetFacts.HasWriteOwner)) + # A directory the caller cannot provision is the workspace root half the time: the sandbox + # reports its own failure there while the conflicting entry sits deeper. Examine that + # subtree before the verdict so the classification and its packageObjects describe what + # this run will actually repair. + $ownsRoot = $full -ieq (Get-NormalizedPath $AllowRoot) + if (-not $Restore -and $targetFacts.Errors.Count -eq 0 -and [System.IO.Directory]::Exists($full) -and ($needsPrecondition -or $ownsRoot)) { + $scan = Get-SubtreePackageSources -Root $full -Limit $SCAN_LIMIT + foreach ($object in $scan.sources) { + if ($packageTargets -notcontains $object) { $packageTargets += $object } + if (-not $factsByPath.ContainsKey($object)) { + $scanned = Get-ObjectFacts -FullPath $object -MeSid $meSid + $factsByPath[$scanned.Object] = $scanned + } + } + Write-Report observation subtree_scan $full $(if ($scan.failed -gt 0) { 'partial' } else { 'read' }) 'The caller cannot provision this directory, or it is the authorized root, so explicit package allow ACEs under it were collected in the same run; no reparse point or managed application tree is entered.' @{ + visited = $scan.visited; packageSources = @($scan.sources); truncated = [bool]$scan.truncated; unreadable = [int]$scan.failed + } + Write-Line ('SCAN {0} VISITED={1} PACKAGE_SOURCES={2}{3}{4}' -f $full, $scan.visited, @($scan.sources).Count, $(if ($scan.truncated) { ' TRUNCATED' } else { '' }), $(if ($scan.failed -gt 0) { " UNREADABLE=$($scan.failed)" } else { '' })) + if ($scan.truncated) { $script:scanTruncated = $true } + } + if ($targetFacts.Errors.Count -gt 0) { $verdict = 'INCOMPLETE' $reason = 'Some observations failed; missing evidence is not evidence that a right is absent or a repair is safe.' @@ -595,26 +622,6 @@ try { break paths } - # A directory the caller cannot provision is the workspace root half the time: the - # sandbox reports its own failure there while the conflicting entry sits deeper. - # Examine that subtree in the same run instead of spending a second approval. - $ownsRoot = $full -ieq (Get-NormalizedPath $AllowRoot) - if ([System.IO.Directory]::Exists($full) -and ($needsPrecondition -or $ownsRoot)) { - $scan = Get-SubtreePackageSources -Root $full -Limit $SCAN_LIMIT - foreach ($object in $scan.sources) { - if ($packageTargets -notcontains $object) { $packageTargets += $object } - if (-not $factsByPath.ContainsKey($object)) { - $scanned = Get-ObjectFacts -FullPath $object -MeSid $meSid - $factsByPath[$scanned.Object] = $scanned - } - } - Write-Report observation subtree_scan $full $(if ($scan.failed -gt 0) { 'partial' } else { 'read' }) 'The caller cannot provision this directory, or it is the authorized root, so explicit package allow ACEs under it were collected in the same run; no reparse point or managed application tree is entered.' @{ - visited = $scan.visited; packageSources = @($scan.sources); truncated = [bool]$scan.truncated; unreadable = [int]$scan.failed - } - Write-Line ('SCAN {0} VISITED={1} PACKAGE_SOURCES={2}{3}{4}' -f $full, $scan.visited, @($scan.sources).Count, $(if ($scan.truncated) { ' TRUNCATED' } else { '' }), $(if ($scan.failed -gt 0) { " UNREADABLE=$($scan.failed)" } else { '' })) - if ($scan.truncated) { $script:scanTruncated = $true } - } - # Nothing is changed while a package source or a grant target is out of reach: a # repair that cannot finish would leave the tree half repaired before refusing. foreach ($packagePath in $packageTargets) { diff --git a/packages/sandbox/sandbox-windows-acl/tests/diagnose-script.spec.ts b/packages/sandbox/sandbox-windows-acl/tests/diagnose-script.spec.ts index 1240d7b078..e6f9a7b55b 100644 --- a/packages/sandbox/sandbox-windows-acl/tests/diagnose-script.spec.ts +++ b/packages/sandbox/sandbox-windows-acl/tests/diagnose-script.spec.ts @@ -349,7 +349,13 @@ describe.skipIf(!isWin32 || !pwshAvailable())('diagnose-windows-sandbox-acl scri const run = runScript(['-Path', root, '-AllowRoot', root, '-Out', join(scratch, 'out')]) expect(run.code, run.output).toBe(0) - expect(run.output).toContain('VERDICT=PRECONDITION') + // The classification describes what this run repairs: the scanned sources are part + // of it, not discovered after the verdict was recorded. + expect(run.output).toContain('VERDICT=BOTH') + expect(reports(run)).toContainEqual(containingObject({ + kind: 'decision', operation: 'classify', path: root, status: 'BOTH', + details: containingObject({ packageObjects: containingArray([deep, leaf]) }), + })) expect(run.output).toContain(`GRANTED ${root} SID=${meSid}`) expect(run.output).toContain(`FIXED ${deep} SID=${PACKAGE_SID}`) expect(run.output).toContain(`FIXED ${leaf} SID=${OTHER_PACKAGE_SID}`)