fix(node): harden native loader allowlist and simplify loader-hooks surface

Security fixes from consolidated code review of the native HTTP ESM loader:

- gate registration itself on the MF_NODE_NATIVE_LOADER kill switch so
  module.register() is never called when disabled
- share one in-flight ack per origin in allowOrigin() so concurrent callers
  cannot resolve before the hooks thread applied the allowlist update
- reject HTTP redirects in the hooks-thread fetch so an allowlisted origin
  cannot redirect module source to an un-allowlisted target
- bound the hooks fetch (30s abort) and allowlist acks (10s rejection) so a
  broken port fails loudly instead of hanging the process

Cleanup: reuse RemoteEntryExports from @module-federation/runtime/types,
drop the always-true state.enabled flag, fold state.ts into protocol.ts,
type the hooks with @types/node module hook types, stop re-exporting
test-only helpers, resolve the CJS register hooksUrl via defaultHooksUrl(),
share the log prefix constant, warn when smoke tests skip on missing dist,
and fix a stale comment in the node-esm-host example.

Author: ScriptedAlchemy

apps/node-esm-host/webpack.config.js

@@ -5,9 +5,10 @@ const path = require('path');
 // The host bundle itself is a native ES module (output.module +
 // chunkFormat/chunkLoading 'module'/'import'). The Module Federation runtime
 // is bundled in; the remote entry URL is imported at runtime with a native
-// dynamic `import()` (see src/native-esm-load-entry-plugin.js), which is what
-// routes remote loading through Node's http(s) loader hooks instead of the
-// vm-based 'async-node' machinery used by the sibling node-host example.
+// dynamic `import()` (the node runtime plugin's `loadEntry` bridge, see
+// @module-federation/node/runtimePlugin), which is what routes remote loading
+// through Node's http(s) loader hooks instead of the vm-based 'async-node'
+// machinery used by the sibling node-host example.
 module.exports = (_env, argv = {}) => {
   const isProduction = argv.mode === 'production';
 

packages/node/src/__tests__/fixtures/native-loader-smoke.mjs

@@ -31,14 +31,14 @@ const origin = `http://127.0.0.1:${server.address().port}`;
 
 const distSrc = new URL(process.env.MF_NODE_DIST_SRC);
 const { getNativeHttpLoaderState } = await import(
-  new URL('loader-hooks/state.mjs', distSrc)
+  new URL('loader-hooks/protocol.mjs', distSrc)
 );
 const { loadEntryViaNativeHttpLoader } = await import(
   new URL('loader-hooks/entryLoader.mjs', distSrc)
 );
 
 const state = getNativeHttpLoaderState();
-assert.ok(state?.enabled, 'register entry point must create loader state');
+assert.ok(state, 'register entry point must create loader state');
 
 // Origins outside the allowlist must be refused.
 await assert.rejects(

packages/node/src/__tests__/loader-hooks.test.ts

@@ -1,4 +1,5 @@
 import { MessageChannel } from 'node:worker_threads';
+import type { LoadHookContext, ResolveHookContext } from 'node:module';
 import {
   initialize,
   load,
@@ -10,15 +11,12 @@ import {
 import {
   ACK_MESSAGE,
   ADD_ORIGIN_MESSAGE,
-  MF_NATIVE_LOADER_ENV_FLAG,
   isHttpUrl,
   normalizeOrigin,
-} from '../loader-hooks/protocol';
-import {
   getNativeHttpLoaderState,
   setNativeHttpLoaderState,
   clearNativeHttpLoaderStateForTesting,
-} from '../loader-hooks/state';
+} from '../loader-hooks/protocol';
 import { loadEntryViaNativeHttpLoader } from '../loader-hooks/entryLoader';
 import type { NativeHttpLoaderState } from '../loader-hooks/protocol';
 
@@ -33,14 +31,17 @@ const originalFetch = global.fetch;
 afterEach(() => {
   resetNativeHttpLoaderStateForTesting();
   clearNativeHttpLoaderStateForTesting();
-  delete process.env[MF_NATIVE_LOADER_ENV_FLAG];
   global.fetch = originalFetch;
   jest.clearAllMocks();
 });
 
 function mockFetchResponse(
   body: string,
-  init: { ok?: boolean; status?: number; contentType?: string | null } = {},
+  init: {
+    ok?: boolean;
+    status?: number;
+    contentType?: string | null;
+  } = {},
 ): jest.Mock {
   const { ok = true, status = 200, contentType = 'text/javascript' } = init;
   const fetchMock = jest.fn().mockResolvedValue({
@@ -94,7 +95,12 @@ describe('loader hooks: initialize', () => {
 });
 
 describe('loader hooks: resolve', () => {
-  const context = { conditions: [], importAttributes: {} };
+  const context: ResolveHookContext = {
+    conditions: [],
+    importAssertions: {},
+    importAttributes: {},
+    parentURL: undefined,
+  };
 
   it('short-circuits allowed http specifiers', async () => {
     initialize({ allowedOrigins: [ORIGIN] });
@@ -161,7 +167,12 @@ describe('loader hooks: resolve', () => {
 });
 
 describe('loader hooks: load', () => {
-  const context = { conditions: [], importAttributes: {} };
+  const context: LoadHookContext = {
+    conditions: [],
+    format: undefined,
+    importAssertions: {},
+    importAttributes: {},
+  };
 
   it('passes non-http URLs through to the default loader', async () => {
     nextLoad.mockResolvedValue({ format: 'module', source: '' });
@@ -178,7 +189,13 @@ describe('loader hooks: load', () => {
       source: 'export const x = 1;',
       shortCircuit: true,
     });
-    expect(fetchMock).toHaveBeenCalledWith(ENTRY_URL);
+    expect(fetchMock).toHaveBeenCalledWith(
+      ENTRY_URL,
+      expect.objectContaining({
+        redirect: 'manual',
+        signal: expect.any(AbortSignal),
+      }),
+    );
     expect(nextLoad).not.toHaveBeenCalled();
   });
 
@@ -199,6 +216,18 @@ describe('loader hooks: load', () => {
     expect(fetchMock).not.toHaveBeenCalled();
   });
 
+  it('refuses to follow redirects from allowlisted origins', async () => {
+    initialize({ allowedOrigins: [ORIGIN] });
+    const fetchMock = mockFetchResponse('export const stolen = 1;', {
+      ok: false,
+      status: 302,
+    });
+    await expect(load(ENTRY_URL, context, nextLoad)).rejects.toThrow(
+      /redirect/i,
+    );
+    expect(fetchMock).toHaveBeenCalledTimes(1);
+  });
+
   it('throws a descriptive error on non-OK responses', async () => {
     initialize({ allowedOrigins: [ORIGIN] });
     mockFetchResponse('', { ok: false, status: 404 });
@@ -220,7 +249,6 @@ describe('native loader global state', () => {
   it('stores and clears state on globalThis', () => {
     expect(getNativeHttpLoaderState()).toBeUndefined();
     const state: NativeHttpLoaderState = {
-      enabled: true,
       allowedOrigins: new Set(),
       allowOrigin: jest.fn().mockResolvedValue(undefined),
     };
@@ -236,7 +264,6 @@ describe('loadEntryViaNativeHttpLoader', () => {
 
   function activateState(): NativeHttpLoaderState {
     const state: NativeHttpLoaderState = {
-      enabled: true,
       allowedOrigins: new Set(),
       allowOrigin: jest.fn().mockResolvedValue(undefined),
     };
@@ -250,15 +277,6 @@ describe('loadEntryViaNativeHttpLoader', () => {
     );
   });
 
-  it('returns undefined when disabled through the env flag', async () => {
-    const state = activateState();
-    process.env[MF_NATIVE_LOADER_ENV_FLAG] = '0';
-    await expect(loadEntryViaNativeHttpLoader(remoteInfo)).resolves.toBe(
-      undefined,
-    );
-    expect(state.allowOrigin).not.toHaveBeenCalled();
-  });
-
   it('returns undefined for non-http entries', async () => {
     activateState();
     await expect(

packages/node/src/__tests__/register-smoke.test.ts

@@ -19,6 +19,12 @@ const hooksSupported =
   typeof (nodeModule as { register?: unknown }).register === 'function';
 
 const describeSmoke = distAvailable ? describe : describe.skip;
+if (!distAvailable) {
+  console.warn(
+    `[@module-federation/node] register-smoke tests skipped: built dist not ` +
+      `found at ${distSrc}. Run the package build first to exercise them.`,
+  );
+}
 
 function runNode(args: string[], env: NodeJS.ProcessEnv = {}): string {
   return execFileSync(process.execPath, args, {

packages/node/src/__tests__/register.test.ts

@@ -15,17 +15,20 @@ import {
   resetNativeHttpLoaderStateForTesting,
 } from '../loader-hooks/hooks';
 import {
+  MF_NATIVE_LOADER_ENV_FLAG,
   MF_NATIVE_LOADER_HOSTS_ENV,
+  clearNativeHttpLoaderStateForTesting,
+  getNativeHttpLoaderState,
   type NativeHttpLoaderInitializeData,
 } from '../loader-hooks/protocol';
-import { clearNativeHttpLoaderStateForTesting } from '../loader-hooks/state';
 
 const HOOKS_URL = 'file:///fake/loader-hooks/hooks.mjs';
 
 afterEach(() => {
   clearNativeHttpLoaderStateForTesting();
   resetNativeHttpLoaderStateForTesting();
   delete process.env[MF_NATIVE_LOADER_HOSTS_ENV];
+  delete process.env[MF_NATIVE_LOADER_ENV_FLAG];
   jest.clearAllMocks();
 });
 
@@ -34,6 +37,19 @@ describe('registerNativeHttpLoader', () => {
     expect(isNativeHttpLoaderSupported()).toBe(true);
   });
 
+  it.each(['0', 'false'])(
+    'is a no-op when disabled via %s in the env kill switch',
+    (flag) => {
+      process.env[MF_NATIVE_LOADER_ENV_FLAG] = flag;
+
+      const state = registerNativeHttpLoader({ hooksUrl: HOOKS_URL });
+
+      expect(state).toBeUndefined();
+      expect(mockRegister).not.toHaveBeenCalled();
+      expect(getNativeHttpLoaderState()).toBeUndefined();
+    },
+  );
+
   it('registers the hooks module once and seeds allowed origins', () => {
     process.env[MF_NATIVE_LOADER_HOSTS_ENV] =
       'https://cdn.example.com, invalid entry,';
@@ -54,19 +70,26 @@ describe('registerNativeHttpLoader', () => {
     );
     expect(options.transferList).toContain(options.data.port);
 
-    expect(state?.enabled).toBe(true);
+    expect(state).toBeDefined();
     expect(state?.allowedOrigins.has('https://cdn.example.com')).toBe(true);
   });
 
-  it('is idempotent and merges new origins into the existing registration', () => {
+  it('is idempotent and merges new origins into the existing registration', async () => {
     const first = registerNativeHttpLoader({ hooksUrl: HOOKS_URL });
+    // Wire the hooks side so the merge's allowOrigin round-trips get acked.
+    initialize(
+      mockRegister.mock.calls[0][1].data as NativeHttpLoaderInitializeData,
+    );
+
     const second = registerNativeHttpLoader({
       hooksUrl: HOOKS_URL,
       allowedOrigins: ['https://late.example.com'],
     });
 
     expect(second).toBe(first);
     expect(mockRegister).toHaveBeenCalledTimes(1);
+    // The merged origin only lands once the hooks thread acks it.
+    await first!.allowOrigin('https://late.example.com');
     expect(first?.allowedOrigins.has('https://late.example.com')).toBe(true);
   });
 
@@ -89,4 +112,35 @@ describe('registerNativeHttpLoader', () => {
       state!.allowOrigin('https://dynamic.example.com'),
     ).resolves.toBeUndefined();
   });
+
+  it('does not resolve concurrent allowOrigin calls for the same origin before the ack', async () => {
+    const state = registerNativeHttpLoader({ hooksUrl: HOOKS_URL });
+    const data = mockRegister.mock.calls[0][1]
+      .data as NativeHttpLoaderInitializeData;
+
+    // No hooks side wired up yet: no ack can arrive.
+    const first = state!.allowOrigin('https://dynamic.example.com');
+    const second = state!.allowOrigin('https://dynamic.example.com/x.js');
+
+    let settled = false;
+    void first.then(() => {
+      settled = true;
+    });
+    void second.then(() => {
+      settled = true;
+    });
+    await new Promise((r) => setTimeout(r, 20));
+    expect(settled).toBe(false);
+    expect(state!.allowedOrigins.has('https://dynamic.example.com')).toBe(
+      false,
+    );
+
+    // Wiring up the hooks side delivers the queued message and acks it; only
+    // then may callers proceed to import().
+    initialize(data);
+    await expect(first).resolves.toBeUndefined();
+    await expect(second).resolves.toBeUndefined();
+    expect(state!.allowedOrigins.has('https://dynamic.example.com')).toBe(true);
+    expect(isOriginAllowed('https://dynamic.example.com/chunk.js')).toBe(true);
+  });
 });

packages/node/src/loader-hooks/entryLoader.ts

@@ -1,45 +1,31 @@
-import { MF_NATIVE_LOADER_ENV_FLAG, isHttpUrl } from './protocol';
-import { getNativeHttpLoaderState } from './state';
-
-/**
- * Structural subset of the MF runtime's `RemoteEntryExports` /
- * `RemoteInfo` types, kept local so this module can be bundled into
- * application code without pulling runtime-core type dependencies.
- */
-export interface NativeRemoteEntryExports {
-  get: (id: string) => () => Promise<unknown>;
-  init: (...args: unknown[]) => void | Promise<void>;
-}
-
-export interface NativeLoaderRemoteInfo {
-  name: string;
-  entry: string;
-  type?: string;
-}
+import {
+  MF_NODE_LOG_PREFIX,
+  getNativeHttpLoaderState,
+  isHttpUrl,
+} from './protocol';
+import type { RemoteEntryExports } from '@module-federation/runtime/types';
 
 const NATIVE_COMPATIBLE_REMOTE_TYPES = new Set(['module', 'esm']);
 
 // Indirect dynamic import so bundlers (webpack/rspack) leave it untouched and
 // the request flows through Node's module loader — and thus our hooks.
+// Known limitation: this breaks under `--disallow-code-generation-from-strings`
+// (new Function is code generation from a string); there is no bundler-proof
+// alternative today.
 const dynamicImport = (url: string): Promise<Record<string, unknown>> =>
   new Function('u', 'return import(u)')(url);
 
-function isNativeLoaderDisabledByEnv(): boolean {
-  const flag = process.env[MF_NATIVE_LOADER_ENV_FLAG];
-  return flag === '0' || flag === 'false';
-}
-
 function pickContainer(
   namespace: Record<string, unknown>,
-): NativeRemoteEntryExports | undefined {
+): RemoteEntryExports | undefined {
   const candidates = [namespace, namespace['default']];
   for (const candidate of candidates) {
     if (
       candidate &&
-      typeof (candidate as NativeRemoteEntryExports).get === 'function' &&
-      typeof (candidate as NativeRemoteEntryExports).init === 'function'
+      typeof (candidate as RemoteEntryExports).get === 'function' &&
+      typeof (candidate as RemoteEntryExports).init === 'function'
     ) {
-      return candidate as NativeRemoteEntryExports;
+      return candidate as RemoteEntryExports;
     }
   }
   return undefined;
@@ -50,17 +36,16 @@ function pickContainer(
  * routed through the hooks registered by `registerNativeHttpLoader`).
  *
  * Returns `undefined` when the native path does not apply — no registration,
- * disabled via env, non-HTTP entry, or a remote entry format that requires
- * the default vm-based path — so the MF runtime falls back to its default
- * entry loading.
+ * non-HTTP entry, or a remote entry format that requires the default vm-based
+ * path — so the MF runtime falls back to its default entry loading.
  */
 export async function loadEntryViaNativeHttpLoader(remoteInfo: {
   name: string;
   entry: string;
   type?: string;
-}): Promise<NativeRemoteEntryExports | undefined> {
+}): Promise<RemoteEntryExports | undefined> {
   const state = getNativeHttpLoaderState();
-  if (!state?.enabled || isNativeLoaderDisabledByEnv()) {
+  if (!state) {
     return undefined;
   }
 
@@ -78,7 +63,7 @@ export async function loadEntryViaNativeHttpLoader(remoteInfo: {
 
   if (!container) {
     throw new Error(
-      `[@module-federation/node] Remote "${name}" was imported from "${entry}" via the ` +
+      `${MF_NODE_LOG_PREFIX} Remote "${name}" was imported from "${entry}" via the ` +
         `native HTTP loader but does not expose a federation container ` +
         `(expected "get" and "init" exports). Check that the remote is built ` +
         `with library.type "module".`,