diff --git a/native/windows/README.md b/native/windows/README.md index 428b33a5ca..9d96ca7eb0 100644 --- a/native/windows/README.md +++ b/native/windows/README.md @@ -111,7 +111,9 @@ automatically. Alternatively, build it using the Linux/MinGW lane above. - Cleanup walks locked directory objects, removes files by handle, and unlinks reparse entries without traversing their targets. Failed removal retains the root and ancestor leases for retry; a partially removed runtime accepts only - cleanup, and its registry entry remains until removal succeeds. Normal close, cancellation, + cleanup, and its registry entry remains until removal succeeds. The owned root + handle includes attribute-write access so cleanup can clear a read-only marker, + matching the child cleanup handles. Normal close, cancellation, startup failure, and timeout release provider resources. Abrupt owner death kills the owned job. The Android provider additionally uses a detached helper that owns its private runtime directory and cleans it when the owner pipe @@ -156,8 +158,11 @@ npm run verify:windows-native npm run verify:ghidra:windows ``` -The native inspection probe retains its directory owner if deletion fails. A -later availability query retries that cleanup before creating another probe; +An unpublished runtime retains its directory and parent handles if validation, +result publication, or rollback fails. Errors retain the original failure and +any rollback failure. Later runtime creation or native inspection retries that +exact owner before allocating another directory. The same owner covers the +native inspection probe. A later availability query retries failed probe cleanup; successful admission and static artifact failures remain cached. Environment teardown makes one final cleanup attempt and reports a persistent failure to stderr rather than claiming the directory was removed. @@ -174,9 +179,11 @@ On Windows, after compiling the runtime: node scripts/verify-windows-native-inspection.mjs ``` -This artifact injects one access-denied disposition result while using a real -NTFS directory. The verifier checks retained ownership, recovery on a later -query, cached successful admission, and cleanup on environment teardown. It +This artifact injects an access-denied disposition result while using real +NTFS directories, and separately denies post-create validation and rollback. +The verifier checks retained ownership after inspection and direct runtime +creation failures, recovery on later admission, cached successful inspection, +and cleanup on environment teardown. It renames the retained probe and recreates its old path, checking that cleanup removes the owned inode while preserving the replacement directory. It does not reproduce or identify the host-dependent cause reported in #1701. diff --git a/native/windows/src/filesystem.cc b/native/windows/src/filesystem.cc index d4b5d0c4a8..ab67325d55 100644 --- a/native/windows/src/filesystem.cc +++ b/native/windows/src/filesystem.cc @@ -384,7 +384,7 @@ void createRuntime(const std::wstring& parentPath, const std::wstring& prefix, owner = std::move(result); try { owner->directory = createRelative(owner->parents.back().get(), name, - DELETE | FILE_LIST_DIRECTORY | FILE_READ_ATTRIBUTES | READ_CONTROL, + DELETE | FILE_LIST_DIRECTORY | FILE_READ_ATTRIBUTES | FILE_WRITE_ATTRIBUTES | READ_CONTROL, true, security, owner->path); verifyPrivate(owner->directory.get(), owner->path, true); // The owned child pins the parent nonempty before write sharing is relaxed. @@ -469,10 +469,7 @@ static void writeBytes(HANDLE file, const void* bytes, DWORD count, const std::w static void dispose(HANDLE handle, const std::wstring& path) { FILE_BASIC_INFO info{}; require(GetFileInformationByHandleEx(handle, FileBasicInfo, &info, sizeof(info)), "Read cleanup attributes failed", path); - // READONLY prevents file deletion but is not honored on directories. Clearing - // it on a directory would need attribute-write rights cleanup does not need. - if ((info.FileAttributes & FILE_ATTRIBUTE_READONLY) != 0 && - (info.FileAttributes & FILE_ATTRIBUTE_DIRECTORY) == 0) { + if ((info.FileAttributes & FILE_ATTRIBUTE_READONLY) != 0) { info.FileAttributes &= ~FILE_ATTRIBUTE_READONLY; require(SetFileInformationByHandle(handle, FileBasicInfo, &info, sizeof(info)), "Clear owned readonly attribute failed", path); } diff --git a/scripts/verify-windows-native.mjs b/scripts/verify-windows-native.mjs index 6e405cba54..b121b714bd 100644 --- a/scripts/verify-windows-native.mjs +++ b/scripts/verify-windows-native.mjs @@ -316,6 +316,7 @@ process.stdout.write(JSON.stringify({executable:basename(process.execPath),abiVe ]); const sentinel = join(sourceDirectory, "sentinel"); await writeFile(sentinel, "preserve"); + const runtimeVerificationErrors = []; try { const acl = JSON.parse( execFileSync( @@ -587,7 +588,7 @@ process.stdout.write(JSON.stringify({executable:basename(process.execPath),abiVe "-NoProfile", "-NonInteractive", "-Command", - "$ErrorActionPreference='Stop'; $items=@($env:REA_TEST_READONLY_CHILDREN | ConvertFrom-Json); @($items | ForEach-Object { [IO.File]::SetAttributes($_,[IO.FileAttributes]([int][IO.File]::GetAttributes($_) -bor 1)); $a=[int][IO.File]::GetAttributes($_); @{readonly=(($a -band 1) -ne 0); directory=(($a -band 16) -ne 0)} }) | ConvertTo-Json -Compress", + "$ErrorActionPreference='Stop'; $items=ConvertFrom-Json -InputObject $env:REA_TEST_READONLY_CHILDREN; @($items | ForEach-Object { [IO.File]::SetAttributes($_,[IO.FileAttributes]([int][IO.File]::GetAttributes($_) -bor 1)); $a=[int][IO.File]::GetAttributes($_); @{readonly=(($a -band 1) -ne 0); directory=(($a -band 16) -ne 0)} }) | ConvertTo-Json -Compress", ], { encoding: "utf8", @@ -606,9 +607,21 @@ process.stdout.write(JSON.stringify({executable:basename(process.execPath),abiVe { readonly: true, directory: true }, { readonly: true, directory: false }, ]); - } finally { - native.call("runtime_close", [runtime.handle]); + } catch (failure) { + runtimeVerificationErrors.push(failure); } + try { + native.call("runtime_close", [runtime.handle]); + } catch (cleanupFailure) { + runtimeVerificationErrors.push(cleanupFailure); + } + if (runtimeVerificationErrors.length === 1) + throw runtimeVerificationErrors[0]; + if (runtimeVerificationErrors.length > 1) + throw new AggregateError( + runtimeVerificationErrors, + "Native runtime verification and cleanup failed", + ); assert.equal(await exists(runtime.path), false); assert.equal(await readFile(sentinel, "utf8"), "preserve"); report.controls.cleanupDoesNotTraverseJunctions = true;