Only set the NapiModuleMeta when it is a bundler plugin and properly check that the input to napiModule in JS is indeed a bun native plugihn

src/bun.js/bindings/BunProcess.cpp

@@ -428,12 +428,19 @@ JSC_DEFINE_HOST_FUNCTION(Process_functionDlopen,
     EncodedJSValue exportsValue = JSC::JSValue::encode(exports);
     JSC::JSValue resultValue = JSValue::decode(napi_register_module_v1(globalObject, exportsValue));
 
-    // TODO: think about the finalizer here
-    // currently we do not dealloc napi modules so we don't have to worry about it right now
-    auto* meta = new Bun::NapiModuleMeta(globalObject->m_pendingNapiModuleDlopenHandle);
-    Bun::NapiExternal* napi_external = Bun::NapiExternal::create(vm, globalObject->NapiExternalStructure(), meta, nullptr, nullptr);
-    bool success = resultValue.getObject()->putDirect(vm, WebCore::builtinNames(vm).napiDlopenHandlePrivateName(), napi_external, JSC::PropertyAttribute::DontDelete | JSC::PropertyAttribute::ReadOnly);
-    ASSERT(success);
+    if (auto resultObject = resultValue.getObject()) {
+        // If this is a native bundler plugin we want to store the handle from dlopen
+        // as we are going to call `dlsym()` on it later to get the plugin implementation.
+        const char** pointer_to_plugin_name = (const char**)dlsym(handle, "BUN_PLUGIN_NAME");
+        if (pointer_to_plugin_name) {
+            // TODO: think about the finalizer here
+            // currently we do not dealloc napi modules so we don't have to worry about it right now
+            auto* meta = new Bun::NapiModuleMeta(globalObject->m_pendingNapiModuleDlopenHandle);
+            Bun::NapiExternal* napi_external = Bun::NapiExternal::create(vm, globalObject->NapiExternalStructure(), meta, nullptr, nullptr);
+            bool success = resultObject->putDirect(vm, WebCore::builtinNames(vm).napiDlopenHandlePrivateName(), napi_external, JSC::PropertyAttribute::DontDelete | JSC::PropertyAttribute::ReadOnly);
+            ASSERT(success);
+        }
+    }
 
     RETURN_IF_EXCEPTION(scope, {});
 

src/js/builtins/BundlerPlugin.ts

@@ -1,11 +1,4 @@
-import type {
-  BuildConfig,
-  BunPlugin,
-  OnLoadCallback,
-  OnResolveCallback,
-  PluginBuilder,
-  PluginConstraints,
-} from "bun";
+import type { BuildConfig, BunPlugin, OnLoadCallback, OnResolveCallback, PluginBuilder, PluginConstraints } from "bun";
 type AnyFunction = (...args: any[]) => any;
 
 interface BundlerPlugin {
@@ -73,8 +66,8 @@ export function runSetupFunction(
     if (map === onBeforeParsePlugins) {
       isOnBeforeParse = true;
       // TODO: how to check if it a napi module here?
-      if (!callback) {
-        throw new TypeError("onBeforeParse `napiModule` must be a Napi module");
+      if (!callback || !$isObject(callback) || !callback.$napiDlopenHandle) {
+        throw new TypeError("onBeforeParse `napiModule` must be a Napi module which exports the `BUN_PLUGIN_SYMBOL`");
       }
 
       if (typeof symbol !== "string") {
@@ -134,7 +127,7 @@ export function runSetupFunction(
 
   const self = this;
   function onStart(callback) {
-    if(isBake) {
+    if (isBake) {
       throw new TypeError("onStart() is not supported in Bake yet");
     }
     if (!$isCallable(callback)) {
@@ -370,7 +363,14 @@ export function runOnResolvePlugins(this: BundlerPlugin, specifier, inputNamespa
   }
 }
 
-export function runOnLoadPlugins(this: BundlerPlugin, internalID, path, namespace, defaultLoaderId, isServerSide: boolean) {
+export function runOnLoadPlugins(
+  this: BundlerPlugin,
+  internalID,
+  path,
+  namespace,
+  defaultLoaderId,
+  isServerSide: boolean,
+) {
   const LOADERS_MAP = $LoaderLabelToId;
   const loaderName = $LoaderIdToLabel[defaultLoaderId];
 
@@ -411,15 +411,15 @@ export function runOnLoadPlugins(this: BundlerPlugin, internalID, path, namespac
         }
 
         var { contents, loader = defaultLoader } = result as any;
-        if ((loader as any) === 'object') {
-          if (!('exports' in result)) {
+        if ((loader as any) === "object") {
+          if (!("exports" in result)) {
             throw new TypeError('onLoad plugin returning loader: "object" must have "exports" property');
           }
           try {
             contents = JSON.stringify(result.exports);
-            loader = 'json';
+            loader = "json";
           } catch (e) {
-            throw new TypeError('When using Bun.build, onLoad plugin must return a JSON-serializable object: ' + e) ;
+            throw new TypeError("When using Bun.build, onLoad plugin must return a JSON-serializable object: " + e);
           }
         }
 

test/bundler/native-plugin.test.ts

@@ -2,6 +2,7 @@ import { BunFile, Loader, plugin } from "bun";
 import { afterEach, beforeAll, beforeEach, describe, expect, it } from "bun:test";
 import path, { dirname, join, resolve } from "path";
 import source from "./native_plugin.cc" with { type: "file" };
+import notAPlugin from "./not_native_plugin.cc" with { type: "file" };
 import bundlerPluginHeader from "../../packages/bun-native-bundler-plugin-api/bundler_plugin.h" with { type: "file" };
 import { bunEnv, bunExe, tempDirWithFiles } from "harness";
 import { itBundled } from "bundler/expectBundled";
@@ -15,6 +16,7 @@ describe("native-plugins", async () => {
     const files = {
       "bun-native-bundler-plugin-api/bundler_plugin.h": await Bun.file(bundlerPluginHeader).text(),
       "plugin.cc": await Bun.file(source).text(),
+      "not_a_plugin.cc": await Bun.file(notAPlugin).text(),
       "package.json": JSON.stringify({
         "name": "fake-plugin",
         "module": "index.ts",
@@ -48,6 +50,11 @@ values;`,
             "target_name": "xXx123_foo_counter_321xXx",
             "sources": [ "plugin.cc" ],
             "include_dirs": [ "." ]
+          },
+          {
+            "target_name": "not_a_plugin",
+            "sources": [ "not_a_plugin.cc" ],
+            "include_dirs": [ "." ]
           }
         ]
       }`,
@@ -491,6 +498,32 @@ const many_foo = ["foo","foo","foo","foo","foo","foo","foo"]
     expect(compilationCtxFreedCount).toBe(0);
   });
 
+  it("should fail gracefully when passing something that is NOT a bunler plugin", async () => {
+    const not_plugins = [require(path.join(tempdir, "build/Release/not_a_plugin.node")), 420, "hi", {}];
+
+    for (const napiModule of not_plugins) {
+      try {
+        await Bun.build({
+          outdir,
+          entrypoints: [path.join(tempdir, "index.ts")],
+          plugins: [
+            {
+              name: "not_a_plugin",
+              setup(build) {
+                build.onBeforeParse({ filter: /\.ts/ }, { napiModule, symbol: "plugin_impl" });
+              },
+            },
+          ],
+        });
+        expect.unreachable();
+      } catch (e) {
+        expect(e.toString()).toContain(
+          "onBeforeParse `napiModule` must be a Napi module which exports the `BUN_PLUGIN_SYMBOL`",
+        );
+      }
+    }
+  });
+
   it("should use result of the first plugin that runs and doesn't execute the others", async () => {
     const filter = /\.ts/;