Repository navigation
fix(reference-implementation): stop the unit storage guard recursing on Node >=24.15.0 - #107
Merged
Merged
Conversation
…on Node >=24.15.0 `driverRoots()` resolved each denied driver lazily from inside the guard's own `registerHooks` resolve hook, using `deniedDriverRoots.size > 0` as its only re-entrancy check. That check cannot latch during its own initialisation: the map stays empty until the resolves finish. Node 24.15.0 runs `require.resolve` through `module.registerHooks()` (nodejs/node#62028, commit 46cfad4138); before that it bypassed sync hooks. So those resolves began re-entering the hook, which called `driverRoots()` again on a still-empty map, recursing without bound. The guard's own test timed out after 120s with no child output; instrumenting the hook showed >300k nested resolutions of `pg` and `better-sqlite3` in 12s. Add an explicit `resolving` latch set before the resolves and a `rootsComputed` flag separate from map size, and prime the map before `registerHooks` so the steady-state path never depends on the latch. What the guard detects is unchanged: all six violation routes (builtin import, computed `getBuiltinModule` name, pre-resolved file URL, `createRequire` CJS driver, swallowed denial, real storage module) still fire on 24.14.1 and 24.21.0, with no driver evaluating. Verified non-vacuous by running each route with the guard off, where each reaches storage and exits 0. Assisted-by: AI Signed-off-by: Tim Nunamaker <tnunamak@gmail.com>
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
1 participant
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.
scripts/test-unit-preload.test.tstimes out after 120s with no child output on Node >= 24.15.0, so the reference-implementation workflow cannot move off 24.14.x. Nothing blocks. The process spins.driverRoots()inscripts/test-unit-preload.mjsresolves each denied driver lazily from inside the guard's ownregisterHooksresolve hook. Its only re-entrancy check wasdeniedDriverRoots.size > 0. That check can never latch during its own initialisation. The map is populated only after therequire.resolvecalls return. So every nested call sees an empty map and recurses again.Node 24.15.0 runs
require.resolvethroughmodule.registerHooks()(46cfad4138, nodejs/node#62028); before that it bypassed sync hooks. That is what closed the cycle. Instrumenting the hook with a depth counter showedmapsize 0at every level. The same trace counted 302,332 resolutions ofpgandbetter-sqlite3in 12s. On 24.14.1 it never exceeds depth 1.The fix adds a
resolvinglatch set before the resolves, plus arootsComputedflag that is separate from map size. It also primes the map beforeregisterHooksruns, so the steady-state path never depends on the latch. Priming alone stops the hang. But without the latch, a later first call would still recurse. Both parts ship together.Detection is unchanged. Six violation routes were tested: builtin import, computed
getBuiltinModulename, pre-resolvedfile://URL,createRequireCJS driver, swallowed denial, and real storage module. All six still fail the run on 24.14.1 and on 24.21.0, and no driver evaluates. Each was rerun withPDPP_TEST_UNIT_GUARD=0to confirm it reaches storage and exits 0 when unguarded. That is what makes the denials real rather than incidental failures. The swallowed-denial route still reportspass 1and still exits non-zero.This was tested only against the
memory-defaultprofile, and only on one Linux machine. Node 24.15.0 and 24.19.0 were not retested after the fix.Blocks #96, which cannot land until this does.
Assisted-by: AI