René's URL Explorer Experiment

Title: node-api: do not crash on module version mismatch by krassx · Pull Request #66019 · nodejs/node · GitHub

Open Graph Title: node-api: do not crash on module version mismatch by krassx · Pull Request #66019 · nodejs/node

X Title: node-api: do not crash on module version mismatch by krassx · Pull Request #66019 · nodejs/node

Description: Problem node_napi_env__::New() validates the Node-API version an add-on declares. When the add-on requires a version this binary does not support, it throws a descriptive error and returns nullptr: } else if (module_api_version > NODE_API_SUPPORTED_VERSION_MAX && module_api_version != NAPI_VERSION_EXPERIMENTAL) { node::Environment* node_env = node::Environment::GetCurrent(context); CHECK_NOT_NULL(node_env); v8impl::ThrowNodeApiVersionError( node_env, module_filename.c_str(), module_api_version); return nullptr; } napi_module_register_by_symbol() then dereferences that nullptr without checking it: napi_env env = node_napi_env__::New(context, module_filename, module_api_version); napi_value _exports = nullptr; env->CallIntoModule([&](napi_env env) { // <-- nullptr So loading such an add-on segfaults, and the error Node already built — "… requires Node-API version N, but this version of Node.js only supports version M add-ons." — is never delivered. require() cannot catch it, because the process is gone. Reproduction A ten-line add-on, no third-party tooling, plain node-gyp: #define NAPI_VERSION 99 #include static napi_value Init(napi_env env, napi_value exports) { return exports; } NAPI_MODULE(NODE_GYP_MODULE_NAME, Init) $ node -e "try { require('./build/Release/hello.node') } catch (e) { console.log('caught:', e.message) }" Segmentation fault Confirmed on v20.x, v22.x, v24.x and v26.8.2, and main, v22.x-staging and v24.x-staging all still lack the check. Fix Return when New() returns nullptr; the error it threw then propagates normally. Verified against a local build of this branch: $ ./out/Release/node -e "try { require('.../test_module_version_mismatch.node') } catch (e) { console.log('caught:', e.message) }" caught: file:///.../test_module_version_mismatch.node requires Node-API version 2147483646, but this version of Node.js only supports version 10 add-ons. Removing the four added lines and relinking the same tree turns that back into exit=139 with the catch never reached, so the new test fails without the fix and passes with it. node-api and js-native-api both run clean on the patched build: 102 tests, 0 failures. Test The error path had no coverage at all — "requires Node-API version" appears in exactly one file in the repository, src/node_api.cc, which is how it could be unreachable without anyone noticing. The new test is modelled on test/node-api/test_null_init, which covers the sibling early return in the same function. Notes for reviewers This is not a reprise of the Node-API versioning discussion in #57233. That issue was closed on the grounds that the add-on had been miscompiled by a third-party tool, which is fair — but it leaves the crash in place. The version policy is not in question here; only that a check which already detects the problem and writes an error should be able to deliver it. The reproduction above involves no external tooling. #63740 reports a superficially similar segfault in napi_register_module_v1 from a libnode.so ABI mismatch. That is a different root cause and this change does not address it. Closest precedent: #58459 added CHECK_NOT_NULL(node_env) to this same function for the node::Environment pointer. This change covers the napi_env returned by New() twelve lines below, which is the one that is nullable by design. Refs: #57233 AI use disclosure Per doc/contributing/ai-guidelines.md: this change was prepared with assistance from Claude Code. The commit message refers to it only as "a closed-source coding agent", following the guidance in Naming AI tools in disclosures to keep commercial brand names out of the commit log and name them here instead. The design, review and validation are my own. Specifically, what I verified personally rather than taking on trust: The mechanism, read directly in src/node_api.cc — node_napi_env__::New() returns nullptr on the version-mismatch branch, and napi_module_register_by_symbol() dereferences it with no check. Confirmed under a debugger: the faulting register holds 0. That the fix works, against a local build of this branch, by observing the intended error reach a try/catch around require(). That the new test can fail. I removed the four added lines, relinked the same tree, and confirmed the test fails there (exit=139, the catch never reached) and passes with them. A test that cannot fail would not be worth adding. That the test asserts the intended behaviour rather than the implementation's incidental shape: it matches the error text the version check produces, with the supported-version number left as \d+ so it does not need updating each time NODE_API_SUPPORTED_VERSION_MAX moves. No regressions: node-api and js-native-api run clean on the patched build, 102 tests, 0 failures. I can explain and defend the change during review, and will respond to feedback myself.

Open Graph Description: Problem node_napi_env__::New() validates the Node-API version an add-on declares. When the add-on requires a version this binary does not support, it throws a descriptive error and returns nullptr:...

X Description: Problem node_napi_env__::New() validates the Node-API version an add-on declares. When the add-on requires a version this binary does not support, it throws a descriptive error and returns nullptr:...

Opengraph URL: https://github.com/nodejs/node/pull/66019

X: @github

direct link

Domain: github.com

Links:

Viewport: width=device-width