Repository navigation
Fix Windows stdin-only pipe hang, launch attribute lifetime, and lost scaffolds - #188
Merged
Merged
Conversation
The Windows stdio preload replaced a pipe in stdout or stderr, and stdin only alongside them. spawn and spawnSync with ['pipe', 'inherit', 'inherit'] reached the real call, which retries the pipe an AppContainer refuses forever. The contained node process spun on one core and no timer fired. @prisma/client's postinstall spawns with that shape, so a contained npm install of a Prisma 6 project never returned. A pipe in any of the three slots now goes through the substitution. A probe runs all 27 shapes of pipe, inherit and ignore through spawn and spawnSync and checks where each stream arrives.
The attribute list, the security capabilities, the capability SIDs and the handle list were kept alive with blank assignments, which Go drops. A collection between building them and CreateProcess could free them and hand the memory out again. With collections forced throughout, 1000 launches failed 9 times with "The parameter is incorrect" and started 7 children whose token was not an AppContainer token. With runtime.KeepAlive, 1000 launches under the same conditions all ran contained. A probe runs a collection just before CreateProcess and checks the child's token. npm's resolution step that the sandbox never started was treated as npm failing, so NVX_YES or -y approved the install with only the named packages checked. It is now tried once more and then refused with exit 77, and no approval waives it.
A folder that is not a project and whose permission walk overran 1.5 s was replaced by the sandbox's home as the working directory. npm create vite@latest myvite printed Done, exited 0 and lost the project. A folder of 20,200 files was enough. Such a folder now gets one entry, written without the walk, that covers the folder and anything created in it. A command that may not run where it started (the home folder, above it, or inside NVX_HOME) still runs in the sandbox's home, now in an empty folder of its own. When it leaves anything there, nvx names it, deletes it and exits 77 where it exited 0.
writeProtectedDotenvACL attached its permission list to a security descriptor, which holds the list's address as bytes the garbage collector does not follow, and then wrote the descriptor. The list could be freed first. It is now kept alive with runtime.KeepAlive, as in writeEntryWithoutWalk and the launch. A source test fails on a blank assignment in a Windows file that uses unsafe code, and on any function that attaches a list to a descriptor without runtime.KeepAlive. Against origin/main it reports all three cases fixed on this branch.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Three Windows sandbox defects.
A piped stdin alone hung the contained process. The stdio preload substituted a pipe in stdout or stderr, and stdin only alongside them.
spawnorspawnSyncwith['pipe', 'inherit', 'inherit']reached the real call, which retries the pipe an AppContainer refuses forever.@prisma/client's postinstall uses that shape. A pipe in any slot is now substituted.Launch attributes could be freed before CreateProcess read them. They were kept alive with blank assignments, which Go drops. With collections forced throughout, 1000 launches failed 9 times with "The parameter is incorrect" and started 7 children whose token was not an AppContainer token. With
runtime.KeepAlive, 1000 launches under the same conditions all ran contained. A resolution step the sandbox never started is now retried once and then refused with exit 77, andNVX_YES/-yno longer approve it.A scaffolder could lose its project and exit 0. A non-project folder whose permission walk overran 1.5 s was swapped for the sandbox's home. It now gets one entry, written without the walk, covering the folder and anything created in it. A command that may not run where it started (home, above it, inside NVX_HOME) still runs in the sandbox's home, in an empty folder of its own. If it leaves anything there, nvx names it, deletes it and exits 77.
Verification (Windows 11, scratch NVX_HOME)
gofmt -l internal cmd: empty.go vet ./...for windows, linux, darwin: clean.go test ./internal/nvx -count=1: ok (126 s).NVX_PROBE=1for the new probes and the existing stdio probes: all pass.a-pipe-inherit-inheritBy hand:
nvx npm install ./pkgwhose postinstall spawns with['pipe','inherit','inherit']hung until killed at 90 s on main and finished in 3 s here. A scaffold run from a non-project folder of 20,200 files kept its folder (main lost it).