fix(npm): execute native binaries from npm package bin entries#34375
Conversation
Previously, `deno run npm:somepkg` where `somepkg`'s bin entry pointed to a native executable (Mach-O / ELF / PE) would fail with a JavaScript SyntaxError, because Deno fed the binary bytes into the JS module loader. Detect this case after npm install and spawn the executable as a subprocess, matching the behavior `deno x` already implements for the same scenario. JavaScript bin entries continue to use the existing module-loader path, no behavior change for them. Closes #27114 Closes #31432
fibibot
left a comment
fibibot
left a comment
There was a problem hiding this comment.
The native-bin path is only wired into the non-watch run_script flow, so deno run --watch npm:<native-bin> still reaches create_main_worker() with the binary entrypoint and will hit the same JS loader failure this PR fixes.
run_with_watchresolvesmain_moduleand callsmaybe_npm_install(&factory).await?, but it never callstry_run_npm_bin_executable; add the same interception beforewatch_paths()/create_main_worker()or explicitly reject native npm bins under watch with a clear error.
…er --watch `read_bin_value` only returns `BinValue::JsFile` for npx-shim files; any other file that doesn't start with `#!` (e.g. a plain `cli.mjs`) is returned as `BinValue::Executable`. The previous check therefore tried to spawn ordinary JS bins as native executables, tripping the `--allow-run` permission check across most `npm:<pkg>` invocations. Re-check the bin file's actual ELF / Mach-O / PE magic bytes before intercepting, and let everything else fall through to the JS module loader. Also reject native-bin npm packages under `deno run --watch` with a clear error rather than letting them reach `create_main_worker()`, where the same JS-loader failure this PR fixes would resurface.
fibibot
left a comment
fibibot
left a comment
There was a problem hiding this comment.
The new head addresses my previous blocker: run_with_watch() now detects a native npm: bin after maybe_npm_install() and returns a clear error before creating a worker, so deno run --watch npm:<native-bin> no longer falls through to the JS module loader. The non-watch path still spawns the native bin through run_bin_value(), preserving the existing --allow-run check.
Holding approval until CI finishes; one debug node_compat shard is red but logs are not available while the run is still in progress.
fibibot
left a comment
fibibot
left a comment
There was a problem hiding this comment.
CI is red now; flake-watcher will triage. The only failing shard is test node_compat (3/3) debug macos-aarch64, which is outside this PR's native npm-bin dispatch path in cli/tools/run and cli/tools/x, so I'm not turning this into a code blocker.
fibibot
left a comment
fibibot
left a comment
There was a problem hiding this comment.
CI is fully green now and the new head is just a Merge branch 'main' (no code change), promoting my prior review to APPROVE. My one blocker — native npm bins under --watch — was resolved on the previous head.
…and#34375) Some npm packages (e.g. supabase, lightningcss-cli) ship a platform-native CLI binary as their `bin` entry and rely on the install step to drop the right Mach-O / ELF / PE file in place. `deno run npm:<pkg>` resolved the bin path correctly but then fed the binary bytes into the JavaScript module loader, producing `SyntaxError: Unexpected character '?'` on the first byte. After `maybe_npm_install` we now look up the package's bin map via the same helpers `deno x` uses, and when the resolved bin is classified as a native executable we spawn it as a subprocess (subject to the existing `--allow-run` check). JavaScript bin entries return early and continue through the module loader, so behavior for those is unchanged. The check only fires for bare `npm:<pkg>` invocations; if the user passed a sub-path we leave resolution alone. Manually verified against `npm:supabase` (Mach-O on macOS); the existing `npm::deno_run_bin*` spec suite still passes. Shipping a portable native binary fixture for a new spec test isn't practical, so coverage relies on manual repro plus the no-regression sweep over the existing JS-bin specs. Closes denoland#27114 Closes denoland#31432
2 participants
Some npm packages (e.g. supabase, lightningcss-cli) ship a platform-native
CLI binary as their
binentry and rely on the install step to drop theright Mach-O / ELF / PE file in place.
deno run npm:<pkg>resolved thebin path correctly but then fed the binary bytes into the JavaScript
module loader, producing
SyntaxError: Unexpected character '?'on thefirst byte.
After
maybe_npm_installwe now look up the package's bin map via thesame helpers
deno xuses, and when the resolved bin is classified as anative executable we spawn it as a subprocess (subject to the existing
--allow-runcheck). JavaScript bin entries return early and continuethrough the module loader, so behavior for those is unchanged. The check
only fires for bare
npm:<pkg>invocations; if the user passed asub-path we leave resolution alone.
Manually verified against
npm:supabase(Mach-O on macOS); the existingnpm::deno_run_bin*spec suite still passes. Shipping a portable nativebinary fixture for a new spec test isn't practical, so coverage relies on
manual repro plus the no-regression sweep over the existing JS-bin specs.
Closes #27114
Closes #31432