From 5fd40b376bc7a9ff2fdb0784736b72440a047322 Mon Sep 17 00:00:00 2001 From: Koji Wakayama Date: Sat, 7 Nov 2026 08:23:50 +0100 Subject: [PATCH 1/2] fix(api): load CommonJS route dependencies with native require in the compiled binary The compiled binary rewrote CommonJS dependency imports with regexes into calls to __vf_interopDefault, a helper the injected shim never exported, and loaded dynamic imports of CommonJS packages such as pg as ES modules. Both failed at runtime. Each CommonJS dependency a route imports now bundles as a one-line module that loads it through the runtime's createRequire, so esbuild's interop gives default, named, namespace and dynamic imports their usual meaning. ES module dependencies keep loading from file:// URLs. The require shim resolves with createRequire and keeps route-level requires inside the project root by realpath. The regex rewrite and its helpers are removed. --- .../external-import-rewriter.test.ts | 70 ++---- .../module-loader/external-import-rewriter.ts | 120 +++------- .../api/module-loader/loader-helpers.test.ts | 10 - .../api/module-loader/loader-helpers.ts | 17 -- src/routing/api/module-loader/loader.test.ts | 209 ++++++++++-------- src/routing/api/module-loader/loader.ts | 74 ++++++- src/transforms/import-rewriter/core.test.ts | 5 +- .../import-rewriter/route-adapter.ts | 106 +++------ .../integration/compiled-binary-e2e.suite.ts | 74 +++++++ 9 files changed, 335 insertions(+), 350 deletions(-) diff --git a/src/routing/api/module-loader/external-import-rewriter.test.ts b/src/routing/api/module-loader/external-import-rewriter.test.ts index 2da0bd57e5..424dc5a5ec 100644 --- a/src/routing/api/module-loader/external-import-rewriter.test.ts +++ b/src/routing/api/module-loader/external-import-rewriter.test.ts @@ -1,17 +1,11 @@ import "#veryfront/schemas/_test-setup.ts"; -import { - assert, - assertEquals, - assertStringIncludes, - assertThrows, -} from "#veryfront/testing/assert.ts"; +import { assertEquals, assertStringIncludes, assertThrows } from "#veryfront/testing/assert.ts"; import { describe, it } from "#veryfront/testing/bdd.ts"; import type { FileInfo } from "#veryfront/platform/adapters/base.ts"; import type { FileSystem } from "#veryfront/platform/compat/fs.ts"; import { generateCompiledBinaryRequireShim, getNodeExternalPackagesToResolve, - NODE_BUILTINS, resolveEsmUserDependencies, resolveVeryfrontPackageExport, rewriteCompiledBinaryUserDependencyImports, @@ -116,45 +110,17 @@ describe("external-import-rewriter", () => { describe("rewriteCompiledBinaryUserDependencyImports", () => { const deps = new Map([["lodash", "^4"]]); - it("rewrites a default import to interop require", () => { - const out = rewriteCompiledBinaryUserDependencyImports( + it("leaves a CommonJS dependency's imports to the bundler's native-require module", () => { + // createNativeCommonJSPlugin bundles them before this rewrite runs; the + // rewrite never turns them into require() calls of its own. + const code = [ `import _ from "lodash";`, - deps, - ); - assertEquals(out, `const _ = __vf_interopDefault(require("lodash"));`); - }); - - it("rewrites a namespace import to require", () => { - const out = rewriteCompiledBinaryUserDependencyImports( - `import * as _ from "lodash";`, - deps, - ); - assertEquals(out, `const _ = require("lodash");`); - }); - - it("rewrites a named import to a destructuring require", () => { - const out = rewriteCompiledBinaryUserDependencyImports( + `import * as ns from "lodash";`, `import { merge } from "lodash";`, - deps, - ); - assertStringIncludes(out, `= require("lodash")`); - assertStringIncludes(out, "merge"); - }); - - it("rewrites a dynamic import to a resolved require promise", () => { - const out = rewriteCompiledBinaryUserDependencyImports( + `import sub from "lodash/merge";`, `const m = import("lodash");`, - deps, - ); - assertStringIncludes(out, `Promise.resolve(require("lodash"))`); - }); - - it("rewrites a subpath import including the subpath in require", () => { - const out = rewriteCompiledBinaryUserDependencyImports( - `import merge from "lodash/merge";`, - deps, - ); - assertStringIncludes(out, `require("lodash/merge")`); + ].join("\n"); + assertEquals(rewriteCompiledBinaryUserDependencyImports(code, deps), code); }); it("leaves imports for packages not in userDeps untouched", () => { @@ -384,17 +350,15 @@ describe("external-import-rewriter", () => { }); describe("generateCompiledBinaryRequireShim", () => { - it("emits a self-contained CJS require shim referencing the project dir", () => { + it("binds the runtime's own require to the project", () => { const shim = generateCompiledBinaryRequireShim("/srv/app"); - // Declares the require shim and embeds the project package.json path. - assertStringIncludes(shim, "__vf_createRequire"); - assertStringIncludes(shim, "/srv/app/package.json"); - // Embeds the node builtin set used for routing builtin requires. - assertStringIncludes(shim, JSON.stringify(NODE_BUILTINS)); - // Defines a require function and an interop helper. - assertStringIncludes(shim, "var require = function(id)"); - assertStringIncludes(shim, "__vf_interopDefault"); - assert(shim.length > 0); + assertStringIncludes( + shim, + 'import { createRequire as __vf_createRequire } from "node:module";', + ); + assertStringIncludes(shim, '__vf_createRequire("/srv/app/package.json")'); + assertStringIncludes(shim, "function __vf_native_require(id)"); + assertStringIncludes(shim, "var require = function (id)"); }); }); diff --git a/src/routing/api/module-loader/external-import-rewriter.ts b/src/routing/api/module-loader/external-import-rewriter.ts index 00b9bd3931..6f3ae46bfb 100644 --- a/src/routing/api/module-loader/external-import-rewriter.ts +++ b/src/routing/api/module-loader/external-import-rewriter.ts @@ -51,105 +51,49 @@ export async function readProjectDependencies( } /** - * Generates a CJS module loader shim for compiled Deno binaries. + * The `require` of a route in a compiled Deno binary: the runtime's own + * `createRequire`, bound to the project, so CommonJS dependencies load with + * the runtime's resolution and module cache (`exports`, `#imports`, nested + * `node_modules`, `module.parent`, Node's error codes). * - * In compiled binaries, `createRequire()` can resolve module paths and load - * built-in modules (fs, path, etc.), but cannot load CJS files from disk - * (loadMaybeCjs fails with "path not found"). This shim works around that - * limitation by using `Deno.readTextFileSync` to read CJS files and - * `new Function` to evaluate them in a proper CJS wrapper with require, - * exports, module, __filename, and __dirname bindings. + * The route's own requires, and each CommonJS dependency the route imports + * (see {@link createNativeCommonJSPlugin}), resolve first and load only a file + * whose real path is inside the project (VULN-FS-5): a symlinked + * `node_modules` entry cannot pull in code from outside it. Requires a + * dependency makes itself are the runtime's own, as on every other runtime. + * + * `__vf_native_require` is the loader that the dependency wrappers call. */ export function generateCompiledBinaryRequireShim(projectDir: string): string { - const builtinSet = JSON.stringify(NODE_BUILTINS); - const safeProjectDir = JSON.stringify(projectDir + "/package.json"); + const safeManifest = JSON.stringify(projectDir + "/package.json"); const safeProjectRoot = JSON.stringify(pathHelper.resolve(projectDir)); return ` import { createRequire as __vf_createRequire } from "node:module"; -import { dirname as __vf_dirname, resolve as __vf_resolve } from "node:path"; -var __vf_builtinRequire = __vf_createRequire(${safeProjectDir}); -var __vf_builtinSet = new Set(${builtinSet}); +import { realpathSync as __vf_realpathSync } from "node:fs"; +import { isAbsolute as __vf_isAbsolute, sep as __vf_sep } from "node:path"; +var __vf_nativeRequire = __vf_createRequire(${safeManifest}); +// VULN-FS-5: compare canonical paths, so a project reached through a symlink +// still contains its own dependencies. var __vf_projectRoot = ${safeProjectRoot}; -// VULN-FS-5: Canonicalize the project root so containment checks using -// Deno.realPathSync(resolved) compare canonical-vs-canonical. Without this, -// when the project itself is opened via a symlink, the realpath'd resolved -// module path has a different prefix than the non-canonical projectRoot and -// legitimate dependencies would be rejected. -try { __vf_projectRoot = Deno.realPathSync(__vf_projectRoot); } catch (_) { /* expected: projectRoot may not exist at shim init in some environments */ } -var __vf_cache = Object.create(null); -// Resolve symlinks so containment compares canonical paths. A module that does -// not exist yet cannot be canonicalized, so fall back to its parent directory, -// which is what carries a symlinked project root; only if that fails too is the -// plain resolved path used. -function __vf_canonicalize(candidate) { - try { - return Deno.realPathSync(candidate); - } catch (_) { /* expected: candidate may not exist */ } - try { - var parent = __vf_dirname(candidate); - return Deno.realPathSync(parent) + candidate.slice(parent.length); - } catch (_) { /* expected: parent may not exist either */ } - return __vf_resolve(candidate); -} -function __vf_assertContained(resolved) { - var norm = __vf_resolve(resolved).replace(/\\\\/g, "/"); - var root = __vf_projectRoot.replace(/\\\\/g, "/"); - if (!norm.startsWith(root + "/") && norm !== root) { - throw new Error("CJS loader blocked path outside project: " + resolved); +try { __vf_projectRoot = __vf_realpathSync(__vf_projectRoot); } catch (_) { /* expected: the root may not exist yet */ } +function __vf_contained(id, options) { + var resolved = __vf_nativeRequire.resolve(id, options); + // A built-in resolves to its name. + if (!__vf_isAbsolute(resolved)) return resolved; + var real = __vf_realpathSync(resolved); + if (real !== __vf_projectRoot && !real.startsWith(__vf_projectRoot + __vf_sep)) { + throw new Error("CJS loader blocked path outside project: " + JSON.stringify(String(id))); } + return real; } -function __vf_loadCjs(id, parentDir) { - if (id.startsWith("node:")) return __vf_builtinRequire(id); - if (__vf_builtinSet.has(id)) return __vf_builtinRequire(id); - var slashIdx = id.indexOf("/"); - if (slashIdx > 0 && __vf_builtinSet.has(id.slice(0, slashIdx))) return __vf_builtinRequire(id); - var resolved; - if (id.startsWith(".") || id.startsWith("/")) { - resolved = __vf_resolve(parentDir, id); - if (!resolved.match(/\\.[a-zA-Z0-9]+$/)) { - var exts = [".js", ".cjs", ".json", "/index.js", "/index.cjs", "/index.json"]; - for (var i = 0; i < exts.length; i++) { - try { Deno.statSync(resolved + exts[i]); resolved += exts[i]; break; } catch (_) { /* expected: probing file extensions */ } - } - } - } else { - resolved = __vf_builtinRequire.resolve(id); - } - // VULN-FS-5: Canonicalize before asserting containment, so the candidate and - // the already-canonical project root are compared like with like, then assert - // once. Checking the non-canonical path first rejected dependencies genuinely - // inside a project whose root is reached through a symlink, which is the - // ordinary case for package managers, CI checkouts and macOS /tmp. - // Canonicalizing first also keeps the guard against symlinked node_modules - // entries pointing outside the root: the escape is only visible once resolved. - resolved = __vf_canonicalize(resolved); - __vf_assertContained(resolved); - if (resolved in __vf_cache) return __vf_cache[resolved]; - var code = Deno.readTextFileSync(resolved); - if (resolved.endsWith(".json")) { - var json = JSON.parse(code); - __vf_cache[resolved] = json; - return json; - } - var mod = { exports: {} }; - __vf_cache[resolved] = mod.exports; - var dir = __vf_dirname(resolved); - var childReq = function(childId) { return __vf_loadCjs(childId, dir); }; - childReq.resolve = function(childId) { - if (childId.startsWith(".") || childId.startsWith("/")) return __vf_resolve(dir, childId); - return __vf_builtinRequire.resolve(childId); - }; - childReq.ensure = function(mods, cb) { cb(); }; - var fn = new Function("exports", "require", "module", "__filename", "__dirname", "global", "globalThis", "Worker", code); - fn(mod.exports, childReq, mod, resolved, dir, globalThis, globalThis, undefined); - __vf_cache[resolved] = mod.exports; - return mod.exports; +function __vf_native_require(id) { + return __vf_nativeRequire(__vf_contained(id)); } -function __vf_interopDefault(m) { return m && m.__esModule && m.default !== undefined ? m.default : m; } -var require = function(id) { return __vf_loadCjs(id, ${JSON.stringify(projectDir)}); }; -require.resolve = function(id) { return __vf_builtinRequire.resolve(id); }; -require.ensure = function(mods, cb) { cb(); }; +var require = function (id) { return __vf_native_require(id); }; +require.resolve = function (id, options) { return __vf_contained(id, options); }; +require.resolve.paths = __vf_nativeRequire.resolve.paths; +require.cache = __vf_nativeRequire.cache; `.trim(); } diff --git a/src/routing/api/module-loader/loader-helpers.test.ts b/src/routing/api/module-loader/loader-helpers.test.ts index d58a6aebd7..91badec5dc 100644 --- a/src/routing/api/module-loader/loader-helpers.test.ts +++ b/src/routing/api/module-loader/loader-helpers.test.ts @@ -5,7 +5,6 @@ import { FILE_EXTENSIONS, getLoaderForFile, resolveExportEntry, - toCjsDestructureBindings, validateModulePath, } from "./loader-helpers.ts"; @@ -34,15 +33,6 @@ describe("routing/api/module-loader helpers", () => { assertEquals(resolveExportEntry({ require: "./cjs.js" }), undefined); }); - it("converts CJS destructuring bindings", () => { - assertEquals( - toCjsDestructureBindings("{ parse as parsePdf, version }"), - "{ parse: parsePdf, version }", - ); - assertEquals(toCjsDestructureBindings("{ foo, bar }"), "{ foo, bar }"); - assertEquals(toCjsDestructureBindings("{ }"), "{}"); - }); - it("returns the expected file loaders and extensions", () => { assertEquals( FILE_EXTENSIONS, diff --git a/src/routing/api/module-loader/loader-helpers.ts b/src/routing/api/module-loader/loader-helpers.ts index bfba9f40df..1663624ccb 100644 --- a/src/routing/api/module-loader/loader-helpers.ts +++ b/src/routing/api/module-loader/loader-helpers.ts @@ -46,23 +46,6 @@ export function resolveExportEntry(entry: unknown): string | undefined { return undefined; } -export function toCjsDestructureBindings(bindings: string): string { - const inner = bindings.trim().replace(/^\{\s*/, "").trimEnd().replace(/\}$/, ""); - if (!inner) return "{}"; - - const converted = inner - .split(",") - .map((part) => part.trim()) - .filter(Boolean) - .map((part) => { - const aliasMatch = part.match(/^([A-Za-z_$][\w$]*)\s+as\s+([A-Za-z_$][\w$]*)$/); - if (aliasMatch) return `${aliasMatch[1]}: ${aliasMatch[2]}`; - return part; - }); - - return `{ ${converted.join(", ")} }`; -} - export function getLoaderForFile(filePath: string): "tsx" | "jsx" | "ts" | "js" | "json" { const ext = filePath.split(".").pop() ?? ""; return EXT_TO_LOADER[ext] ?? "js"; diff --git a/src/routing/api/module-loader/loader.test.ts b/src/routing/api/module-loader/loader.test.ts index f1e2ff2cc9..907c0a9980 100644 --- a/src/routing/api/module-loader/loader.test.ts +++ b/src/routing/api/module-loader/loader.test.ts @@ -9,12 +9,13 @@ import { assertThrows, } from "#veryfront/testing/assert.ts"; import { afterAll, describe, it } from "#veryfront/testing/bdd.ts"; -import { join, toFileUrl } from "#veryfront/compat/path"; +import { basename, join, toFileUrl } from "#veryfront/compat/path"; import { buildFailure, bundledModuleScopeDiscriminator, bundlerForcesTypeScript, canDirectImportSpecifier, + createNativeCommonJSPlugin, generateCompiledBinaryRequireShim, getNodeExternalPackagesToResolve, getUserDependencies, @@ -31,7 +32,6 @@ import { rewriteDenoNodeBuiltinImports, rewriteDenoNpmDependencyImports, rewriteNodeExternalImports, - toCjsDestructureBindings, typeScriptBuildOptions, } from "./loader.ts"; import { __setCompiledBinaryForTests } from "#veryfront/security/sandbox/isolation-capability.ts"; @@ -1668,21 +1668,6 @@ describe("loadHandlerModule", { sanitizeResources: false, sanitizeOps: false }, assertEquals(typeof route?.GET, "function"); }); - it("converts aliased named imports to valid CJS destructuring", () => { - assertEquals( - toCjsDestructureBindings("{ parse as parsePdf, version }"), - "{ parse: parsePdf, version }", - ); - assertEquals( - toCjsDestructureBindings("{ default as foo, bar as baz }"), - "{ default: foo, bar: baz }", - ); - assertEquals( - toCjsDestructureBindings("{ foo, bar }"), - "{ foo, bar }", - ); - }); - it("builds the node external package resolution list without duplicates", () => { const packages = getNodeExternalPackagesToResolve( new Map([ @@ -1823,30 +1808,20 @@ describe("loadHandlerModule", { sanitizeResources: false, sanitizeOps: false }, assertMatch(rewritten, /import\("\.\/_vf_tool\.mjs"\)/); }); - it("rewrites compiled-binary user dependency imports to require-based shims", () => { + it("leaves compiled-binary CommonJS dependency imports to the native-require bundle", () => { + // createNativeCommonJSPlugin bundles these before the rewrite runs. const source = [ 'import thing from "my-lib";', 'import { alpha as beta } from "my-lib";', 'import * as namespace from "my-lib";', - 'import combo, { gamma } from "my-lib";', 'import widget from "my-lib/subpath";', 'const loaded = import("my-lib/subpath");', ].join("\n"); - const rewritten = rewriteCompiledBinaryUserDependencyImports( + assertEquals( + rewriteCompiledBinaryUserDependencyImports(source, new Map([["my-lib", "^1.0.0"]])), source, - new Map([["my-lib", "^1.0.0"]]), - ); - - assertMatch(rewritten, /const thing = __vf_interopDefault\(require\("my-lib"\)\)/); - assertMatch(rewritten, /const \{ alpha: beta \} = require\("my-lib"\)/); - assertMatch(rewritten, /const namespace = require\("my-lib"\)/); - assertMatch( - rewritten, - /const __vf_tmp_combo = require\("my-lib"\); const combo = __vf_interopDefault\(__vf_tmp_combo\); const \{ gamma \} = __vf_tmp_combo/, ); - assertMatch(rewritten, /const widget = require\("my-lib\/subpath"\)/); - assertMatch(rewritten, /Promise\.resolve\(require\("my-lib\/subpath"\)\)/); }); it("rewrites ESM-only user dependency imports to real file:// module URLs", () => { @@ -1895,7 +1870,7 @@ describe("loadHandlerModule", { sanitizeResources: false, sanitizeOps: false }, assertEquals(rewritten.includes('require("esm-lib")'), false); }); - it("keeps CJS deps on the require shim while ESM deps use file:// URLs", () => { + it("rewrites only ESM deps to file:// URLs, leaving CJS deps to the bundle", () => { const source = [ 'import esm from "esm-lib";', 'import cjs from "cjs-lib";', @@ -1914,7 +1889,7 @@ describe("loadHandlerModule", { sanitizeResources: false, sanitizeOps: false }, ); assertMatch(rewritten, /import esm from "file:\/\/\/proj\/node_modules\/esm-lib\/index\.mjs"/); - assertMatch(rewritten, /const cjs = __vf_interopDefault\(require\("cjs-lib"\)\)/); + assertMatch(rewritten, /import cjs from "cjs-lib"/); }); it("detects ESM dependencies via type:module and .mjs entry points", async () => { @@ -1976,7 +1951,7 @@ describe("loadHandlerModule", { sanitizeResources: false, sanitizeOps: false }, ); // The traversing entry must be rejected so no file:// import escaping the - // package directory is emitted (it falls back to the contained CJS shim). + // package directory is emitted (it loads through the contained native require). assertEquals(esmDeps.has("evil-lib"), false); }); @@ -5434,67 +5409,32 @@ async function importCompiledBinaryRequireShim( return module.vfRequire; } -// VULN-FS-5: compiled-binary CJS loader must enforce project-root containment -// on BOTH branches of __vf_loadCjs (relative/absolute ids AND bare-package -// ids), and must re-canonicalise via Deno.realPathSync so that a symlinked -// node_modules entry cannot escape the project root. -describe("generateCompiledBinaryRequireShim - static checks (VULN-FS-5)", () => { - it("emits a __vf_assertContained call after bare-package resolution", () => { - const shim = generateCompiledBinaryRequireShim("/fake/project"); - // The original (vulnerable) layout called __vf_assertContained only inside - // the relative/absolute branch. The fix moves the assertion to run after - // both branches (i.e. AFTER the `} else { resolved = ...resolve(id); }`). - const elseIdx = shim.indexOf("__vf_builtinRequire.resolve(id)"); - const assertIdx = shim.indexOf("__vf_assertContained(resolved)", elseIdx); - assertEquals( - assertIdx > elseIdx, - true, - "containment check must follow bare-package resolution", - ); - }); - - it("canonicalises the resolved path before checking containment", () => { - const shim = generateCompiledBinaryRequireShim("/fake/project"); - assertEquals(shim.includes("Deno.realPathSync"), true); - // The containment check must run on the canonical path. Asserting on the - // pre-canonical one instead compares a non-canonical candidate against an - // already-canonical root, which rejects legitimate dependencies whenever - // the project root is itself a symlink. - const canonIdx = shim.indexOf("resolved = __vf_canonicalize(resolved)"); - const assertIdx = shim.indexOf("__vf_assertContained(resolved)", canonIdx); - assertEquals( - canonIdx !== -1 && assertIdx > canonIdx, - true, - "the resolved path must be canonicalised before it is containment-checked", - ); - }); - - it("canonicalises __vf_projectRoot at shim init so symlinked project roots are not falsely rejected", () => { - // Regression: prior to the fix, __vf_projectRoot was only path.resolve()'d, - // so when realPathSync(resolved) returned a canonical path whose prefix - // differed from a symlinked projectRoot, every legitimate dep was blocked. - const shim = generateCompiledBinaryRequireShim("/fake/project"); - const rootInitIdx = shim.indexOf("var __vf_projectRoot ="); - const canonIdx = shim.indexOf( - "Deno.realPathSync(__vf_projectRoot)", - rootInitIdx, - ); - const fnIdx = shim.indexOf("function __vf_assertContained"); - assertEquals( - canonIdx > rootInitIdx && canonIdx < fnIdx, - true, - "__vf_projectRoot must be realPathSync'd between its declaration and the assertContained definition", - ); - }); - +// VULN-FS-5: the compiled-binary require shim must keep route-level requires +// inside the project root for relative/absolute ids AND bare-package ids, and +// must compare realpaths so that a symlinked node_modules entry cannot escape +// the project root. +describe("generateCompiledBinaryRequireShim - containment (VULN-FS-5)", () => { denoIt("the emitted containment check rejects paths outside the project root", async () => { // Execute the shim the generator actually emits: a local re-implementation - // would keep passing even if the emitted __vf_assertContained were gutted. + // would keep passing even if the emitted containment check were gutted. const projectDir = Deno.realPathSync(await makeTempDir()); const packageDir = join(projectDir, "node_modules", "ok"); await fs.mkdir(packageDir, { recursive: true }); await fs.writeTextFile(join(packageDir, "index.js"), "module.exports = { ok: true };"); + await fs.writeTextFile( + join(packageDir, "package.json"), + JSON.stringify({ name: "ok", main: "index.js" }), + ); + // Real files outside the project, so the containment check, not a missing + // file, is what refuses them. + const siblingDir = `${projectDir}ile`; + await fs.mkdir(siblingDir, { recursive: true }); + await fs.writeTextFile(join(siblingDir, "secret.js"), "module.exports = 'sibling';"); + const escapeName = `${basename(projectDir)}-escape.js`; + const escapeFile = join(projectDir, "..", escapeName); + await fs.writeTextFile(escapeFile, "module.exports = 'escaped';"); + const vfRequire = await importCompiledBinaryRequireShim(projectDir); assertEquals( @@ -5502,6 +5442,11 @@ describe("generateCompiledBinaryRequireShim - static checks (VULN-FS-5)", () => true, "in-project CJS must load", ); + assertEquals( + (vfRequire("ok") as { ok: boolean }).ok, + true, + "a bare package resolves from the project's node_modules", + ); assertThrows( () => vfRequire("/etc/passwd"), Error, @@ -5515,12 +5460,92 @@ describe("generateCompiledBinaryRequireShim - static checks (VULN-FS-5)", () => "prefix-sibling dirs must be refused", ); assertThrows( - () => vfRequire(join(projectDir, "node_modules", "..", "..", "escape.js")), + () => vfRequire(join(projectDir, "node_modules", "..", "..", escapeName)), Error, "blocked path outside project", "'..' segments must be normalised before the containment check", ); + try { + await fs.remove(projectDir, { recursive: true }); + await fs.remove(siblingDir, { recursive: true }); + await fs.remove(escapeFile); + } catch (_) { /* best effort */ } + }); +}); + +describe("createNativeCommonJSPlugin", () => { + denoIt("bundles CommonJS imports as native requires with esbuild interop", async () => { + // Regression for #5471: default, named, namespace, subpath and dynamic + // imports of a CommonJS dependency all load through the emitted shim, and + // the dependency's own source is never bundled. + const projectDir = Deno.realPathSync(await makeTempDir()); + const packageDir = join(projectDir, "node_modules", "cjs-lib"); + await fs.mkdir(packageDir, { recursive: true }); + await fs.writeTextFile( + join(packageDir, "package.json"), + JSON.stringify({ name: "cjs-lib", main: "index.js" }), + ); + await fs.writeTextFile( + join(packageDir, "index.js"), + "exports.named = typeof require; exports.greet = () => 'hi';", + ); + await fs.writeTextFile(join(packageDir, "extra.js"), "module.exports = { name: 'extra' };"); + const shimPath = join(projectDir, "vf-shim.mjs"); + await fs.writeTextFile( + shimPath, + `${ + generateCompiledBinaryRequireShim(projectDir) + }\nexport { require, __vf_native_require };\n`, + ); + + const { build } = await import("veryfront/extensions/bundler"); + const result = await build({ + bundle: true, + write: false, + format: "esm", + platform: "neutral", + target: "es2022", + inject: [shimPath], + external: ["node:*"], + plugins: [createNativeCommonJSPlugin(["cjs-lib"])], + stdin: { + contents: [ + 'import lib, { greet } from "cjs-lib";', + 'import * as ns from "cjs-lib";', + 'import extra from "cjs-lib/extra.js";', + "export async function run() {", + ' const dynamic = await import("cjs-lib");', + " return {", + " defaultNamed: lib.named,", + " greet: greet(),", + " namespace: ns.named,", + " extra: extra.name,", + " dynamic: dynamic.named,", + " dynamicDefault: dynamic.default.named,", + " };", + "}", + ].join("\n"), + loader: "ts", + resolveDir: projectDir, + sourcefile: "route.ts", + }, + }); + const output = result.outputFiles?.[0]?.text ?? ""; + assertEquals(output.includes("exports.greet"), false, "dependency source must not be bundled"); + + const routePath = join(projectDir, "route.mjs"); + await fs.writeTextFile(routePath, output); + const route = await import(toFileUrl(routePath).href) as { run: () => Promise }; + assertEquals(await route.run(), { + defaultNamed: "function", + greet: "hi", + namespace: "function", + extra: "extra", + dynamic: "function", + dynamicDefault: "function", + }); + try { await fs.remove(projectDir, { recursive: true }); } catch (_) { /* best effort */ } @@ -5535,9 +5560,9 @@ describe("generateCompiledBinaryRequireShim - symlink resistance (VULN-FS-5)", { // Create a project root, a decoy "evil" package whose entry file is a // symlink pointing at a file outside the project root. If the shim only // checked the pre-symlink path, the containment test would pass but the - // readTextFileSync would still leak the external file. The emitted shim is - // executed here so the realPathSync + second __vf_assertContained is the - // thing under test, not a copy of it. + // native require would still load the external file. The emitted shim is + // executed here so its realpath containment check is the thing under test, + // not a copy of it. const projectDir = Deno.realPathSync(await makeTempDir()); const outsideDir = Deno.realPathSync(await makeTempDir()); const outsideFile = join(outsideDir, "secret.txt"); diff --git a/src/routing/api/module-loader/loader.ts b/src/routing/api/module-loader/loader.ts index b707eb8509..3755be7ef6 100644 --- a/src/routing/api/module-loader/loader.ts +++ b/src/routing/api/module-loader/loader.ts @@ -46,6 +46,7 @@ import { generateCompiledBinaryRequireShim, NODE_BUILTINS, readProjectDependencies, + resolveEsmUserDependencies, rewriteExternalImports, } from "./external-import-rewriter.ts"; import { @@ -102,8 +103,6 @@ const MAX_TYPESCRIPT_CONFIG_BYTES = 1024 * 1024; const ROUTE_RUNTIME_SHIM_SPECIFIER = "veryfront:route-runtime-shim"; const ROUTE_RUNTIME_SHIM_NAMESPACE = "veryfront-route-runtime-shim"; -export { toCjsDestructureBindings } from "./loader-helpers.ts"; - export function loadHandlerModule(options: LoadHostModuleOptions): Promise { if (!isExplicitHostProjectCodeExecutionAllowed(options)) { return Promise.reject( @@ -1549,6 +1548,48 @@ function createRouteRuntimeShimPlugin(contents: string): Plugin { }; } +const NATIVE_COMMONJS_NAMESPACE = "vf-native-cjs"; + +/** + * The user dependencies a compiled binary loads as CommonJS: every one that + * is not an ES module package (see resolveEsmUserDependencies), which keeps + * loading from its file:// URL. + */ +async function compiledCommonJSDependencies( + projectDir: string, + fs: ProjectSourceSnapshot, + userDeps: Map, +): Promise> { + const esmDeps = await resolveEsmUserDependencies(projectDir, fs, userDeps); + return new Set([...userDeps.keys()].filter((name) => !esmDeps.has(name))); +} + +/** + * Bundles each CommonJS dependency the route imports as a one-line module + * that loads it through the runtime's own `require` (see + * generateCompiledBinaryRequireShim), so the bundler's interop gives default, + * named, namespace and dynamic imports their usual meaning. The dependency's + * own files are never bundled or rewritten. + */ +export function createNativeCommonJSPlugin(packages: Iterable): Plugin { + const names = new Set(packages); + return { + name: "vf-native-cjs", + setup(build) { + build.onResolve({ filter: /^[^./]/ }, (args) => { + const parts = args.path.split("/"); + const name = args.path.startsWith("@") ? parts.slice(0, 2).join("/") : parts[0]!; + if (!names.has(name)) return undefined; + return { path: args.path, namespace: NATIVE_COMMONJS_NAMESPACE }; + }); + build.onLoad({ filter: /.*/, namespace: NATIVE_COMMONJS_NAMESPACE }, (args) => ({ + contents: `module.exports = __vf_native_require(${JSON.stringify(args.path)});`, + loader: "js", + })); + }, + }; +} + async function validateBundledLocalWorkerEntries(options: { sourceSnapshot: ProjectSourceSnapshot; projectDir: string; @@ -2070,9 +2111,9 @@ function buildTranspiledModuleSource( // in the temp handler that Deno cannot resolve → "not a dependency and not in // import map" → 500. (The Node path already always-resolves zod via // getNodeExternalPackagesToResolve.) On the compiled binary, keeping zod in - // userDeps routes it through rewriteCompiledBinaryUserDependencyImports like - // any other npm package, so its import resolves from the project's - // node_modules via the createRequire shim. zod is still force-externalized + // userDeps routes it through the native-CommonJS bundle plugin (or the ESM + // file:// rewrite) like any other npm package, so its import resolves from + // the project's node_modules. zod is still force-externalized // below, never bundled inline. See veryfront-issue-inbox#217. const userDeps = getUserDependencies(allDeps); @@ -2083,8 +2124,16 @@ function buildTranspiledModuleSource( // Bundling CJS deps inline (especially complex ones like pdf-parse/pdf.js) // breaks their internal global state management during esbuild's CJS→ESM // conversion. + // A compiled binary loads a CommonJS dependency through the runtime's own + // require (see createNativeCommonJSPlugin); ESM dependencies stay external + // and load from file:// URLs. + const compiledBinary = isDeno && isCompiledBinary(); + const commonJSDeps = compiledBinary + ? await compiledCommonJSDependencies(projectDir, sourceSnapshot, userDeps) + : new Set(); const userExternals: string[] = []; for (const name of userDeps.keys()) { + if (commonJSDeps.has(name)) continue; userExternals.push(name, `${name}/*`); } @@ -2095,12 +2144,10 @@ function buildTranspiledModuleSource( // fail at runtime. Inject a createRequire-based shim so require() works in ESM. // Use projectDir as the resolve base so require() finds the project's node_modules. const safeProjectDir = JSON.stringify(projectDir + "/package.json"); - const requireShim = isDeno && isCompiledBinary() - ? generateCompiledBinaryRequireShim(projectDir) - : [ - 'import { createRequire as __vf_createRequire } from "node:module";', - `var require = __vf_createRequire(${safeProjectDir});`, - ].join("\n"); + const requireShim = compiledBinary ? generateCompiledBinaryRequireShim(projectDir) : [ + 'import { createRequire as __vf_createRequire } from "node:module";', + `var require = __vf_createRequire(${safeProjectDir});`, + ].join("\n"); const routeSourceUrl = pathHelper.toFileUrl(resolvedPath).href; const workerShim = [ `const __vf_routeSourceUrl = ${JSON.stringify(routeSourceUrl)};`, @@ -2113,7 +2160,9 @@ function buildTranspiledModuleSource( const routeRuntimeShim = [ requireShim, workerShim, - "export { require, Worker };", + compiledBinary + ? "export { require, Worker, __vf_native_require };" + : "export { require, Worker };", ].join("\n"); const buildOptions = typescriptDecoratorOptions @@ -2150,6 +2199,7 @@ function buildTranspiledModuleSource( }, plugins: [ createRouteRuntimeShimPlugin(routeRuntimeShim), + ...(commonJSDeps.size > 0 ? [createNativeCommonJSPlugin(commonJSDeps)] : []), createImportMapPlugin( projectDir, sourceSnapshot, diff --git a/src/transforms/import-rewriter/core.test.ts b/src/transforms/import-rewriter/core.test.ts index 208142d36d..ba68e0166d 100644 --- a/src/transforms/import-rewriter/core.test.ts +++ b/src/transforms/import-rewriter/core.test.ts @@ -143,12 +143,13 @@ describe("import rewrite compatibility golden tests", () => { rewriteCompiledBinaryVeryfrontImports(`import { x } from "veryfront/agent";`), `import { x } from "./_vf_agent.mjs";`, ); - assertStringIncludes( + // CommonJS dependencies are left for the native-require bundle plugin. + assertEquals( rewriteCompiledBinaryUserDependencyImports( `const m = import("lodash/merge");`, new Map([["lodash", "^4"]]), ), - `Promise.resolve(require("lodash/merge"))`, + `const m = import("lodash/merge");`, ); assertEquals( rewriteDenoNodeBuiltinImports(`import { readFile } from "fs";`), diff --git a/src/transforms/import-rewriter/route-adapter.ts b/src/transforms/import-rewriter/route-adapter.ts index 178bde8147..53abdae941 100644 --- a/src/transforms/import-rewriter/route-adapter.ts +++ b/src/transforms/import-rewriter/route-adapter.ts @@ -6,7 +6,6 @@ import { resolveContainedPackagePath } from "./package-resolution.ts"; import { isWithinDirectory } from "#veryfront/security/path-validation.ts"; import { resolveExportEntry as resolveRouteExportEntry, - toCjsDestructureBindings, } from "#veryfront/routing/api/module-loader/loader-helpers.ts"; import { rethrowProjectBoundaryViolation } from "#veryfront/routing/api/module-loader/project-source-snapshot.ts"; @@ -205,91 +204,46 @@ export function rewriteCompiledUserDependencyImportsForRoute( const escaped = name.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); // ESM-only dependencies are rewritten to real ES module file:// URLs so that - // import.meta, top-level await, etc. work. They must NOT be transpiled to - // CommonJS and evaluated via the `new Function` shim (see __vf_loadCjs). + // import.meta, top-level await, etc. work. A CommonJS dependency never + // reaches this point: it is bundled as a module that loads it through the + // runtime's own require (see createNativeCommonJSPlugin). const esm = esmDeps.get(name); - if (esm) { - // Resolve a subpath import to a contained file:// URL. The subpath comes - // from the handler source; reject any that escape the package directory - // (e.g. "pkg/../../secret") by leaving the import untouched so it fails to - // resolve rather than reading outside node_modules. - const subpathUrl = (subpath: string, original: string): string | null => { - const target = resolveContainedPackagePath(esm.packageDir, "." + subpath); - if (!target) { - logger.warn(`Skipping ESM subpath import that escapes package directory: ${original}`); - return null; - } - return pathHelper.toFileUrl(target).href; - }; - - transformed = transformed.replace( - new RegExp(`from\\s+["']${escaped}(/[^"']+)["']`, "g"), - (match, subpath) => { - const url = subpathUrl(subpath, match); - return url ? `from "${url}"` : match; - }, - ); - transformed = transformed.replace( - new RegExp(`from\\s+["']${escaped}["']`, "g"), - () => `from "${esm.entryUrl}"`, - ); - transformed = transformed.replace( - new RegExp(`import\\s*\\(\\s*["']${escaped}(/[^"']+)["']\\s*\\)`, "g"), - (match, subpath) => { - const url = subpathUrl(subpath, match); - return url ? `import("${url}")` : match; - }, - ); - transformed = transformed.replace( - new RegExp(`import\\s*\\(\\s*["']${escaped}["']\\s*\\)`, "g"), - () => `import("${esm.entryUrl}")`, - ); - continue; - } + if (!esm) continue; + + // Resolve a subpath import to a contained file:// URL. The subpath comes + // from the handler source; reject any that escape the package directory + // (e.g. "pkg/../../secret") by leaving the import untouched so it fails to + // resolve rather than reading outside node_modules. + const subpathUrl = (subpath: string, original: string): string | null => { + const target = resolveContainedPackagePath(esm.packageDir, "." + subpath); + if (!target) { + logger.warn(`Skipping ESM subpath import that escapes package directory: ${original}`); + return null; + } + return pathHelper.toFileUrl(target).href; + }; transformed = transformed.replace( - new RegExp(`import\\s+(\\w+)\\s+from\\s+["']${escaped}["']`, "g"), - (_, localName) => `const ${localName} = __vf_interopDefault(require("${name}"))`, - ); - transformed = transformed.replace( - new RegExp(`import\\s+(\\{[^}]+\\})\\s+from\\s+["']${escaped}["']`, "g"), - (_, bindings) => `const ${toCjsDestructureBindings(bindings)} = require("${name}")`, - ); - transformed = transformed.replace( - new RegExp(`import\\s+\\*\\s+as\\s+(\\w+)\\s+from\\s+["']${escaped}["']`, "g"), - (_, localName) => `const ${localName} = require("${name}")`, + new RegExp(`from\\s+["']${escaped}(/[^"']+)["']`, "g"), + (match, subpath) => { + const url = subpathUrl(subpath, match); + return url ? `from "${url}"` : match; + }, ); transformed = transformed.replace( - new RegExp( - `import\\s+(\\w+)\\s*,\\s*(\\{[^}]+\\})\\s+from\\s+["']${escaped}["']`, - "g", - ), - (_, defaultName, bindings) => { - const tmp = `__vf_tmp_${defaultName}`; - return `const ${tmp} = require("${name}"); const ${defaultName} = __vf_interopDefault(${tmp}); const ${ - toCjsDestructureBindings(bindings) - } = ${tmp}`; - }, + new RegExp(`from\\s+["']${escaped}["']`, "g"), + () => `from "${esm.entryUrl}"`, ); transformed = transformed.replace( - new RegExp( - `import\\s+(\\w+|\\*\\s+as\\s+\\w+|\\{[^}]+\\})\\s+from\\s+["']${escaped}(/[^"']+)["']`, - "g", - ), - (_, binding, subpath) => { - const trimmedBinding = String(binding).trim(); - if (trimmedBinding.startsWith("{")) { - return `const ${toCjsDestructureBindings(trimmedBinding)} = require("${name}${subpath}")`; - } - const name_ = trimmedBinding.startsWith("*") - ? trimmedBinding.replace(/\*\s+as\s+/, "") - : trimmedBinding; - return `const ${name_} = require("${name}${subpath}")`; + new RegExp(`import\\s*\\(\\s*["']${escaped}(/[^"']+)["']\\s*\\)`, "g"), + (match, subpath) => { + const url = subpathUrl(subpath, match); + return url ? `import("${url}")` : match; }, ); transformed = transformed.replace( - new RegExp(`import\\s*\\(\\s*["']${escaped}(/[^"']*)?["']\\s*\\)`, "g"), - (_, subpath) => `Promise.resolve(require("${name}${subpath || ""}"))`, + new RegExp(`import\\s*\\(\\s*["']${escaped}["']\\s*\\)`, "g"), + () => `import("${esm.entryUrl}")`, ); } diff --git a/tests/integration/compiled-binary-e2e.suite.ts b/tests/integration/compiled-binary-e2e.suite.ts index 20444175dd..119b04e787 100644 --- a/tests/integration/compiled-binary-e2e.suite.ts +++ b/tests/integration/compiled-binary-e2e.suite.ts @@ -780,6 +780,80 @@ export function GET() { }); }); + it("should load CommonJS dependencies in compiled-binary API routes (#5471)", async () => { + // A pg-shaped CommonJS package: a lazily required nested dependency, a + // Node built-in, a default export that is an instance, and a subpath. + const projectDir = await createTestProject( + "api-cjs-deps-test", + ` +export default function Home() { + return
Home Page
; +} +`, + { + "node_modules/cjs-db/package.json": JSON.stringify({ name: "cjs-db", main: "./lib" }), + "node_modules/cjs-db/lib/index.js": ` +const EventEmitter = require("events").EventEmitter; +class Pool extends EventEmitter {} +function Db() { this.Pool = Pool; } +Object.defineProperty(Db.prototype, "native", { + get() { return require("cjs-db-types").name; }, +}); +module.exports = new Db(); +module.exports.defaults = { port: 5432 }; +`, + "node_modules/cjs-db/lib/extra.js": `module.exports = { name: "extra" };`, + "node_modules/cjs-db/node_modules/cjs-db-types/package.json": JSON.stringify({ + name: "cjs-db-types", + }), + "node_modules/cjs-db/node_modules/cjs-db-types/index.js": + `exports.name = "types:" + typeof module.parent;`, + "pages/api/db.ts": ` +import db, { defaults } from "cjs-db"; +import * as namespace from "cjs-db"; +import extra from "cjs-db/lib/extra.js"; + +export async function GET() { + const dynamic = await import("cjs-db"); + return Response.json({ + pool: typeof db.Pool, + emitter: typeof new db.Pool().on, + native: db.native, + port: defaults.port, + namespace: typeof namespace.default.Pool, + extra: extra.name, + dynamic: typeof dynamic.default.Pool, + }); +} +`, + }, + ); + const packageJsonPath = join(projectDir, "package.json"); + const packageJson = JSON.parse(await Deno.readTextFile(packageJsonPath)); + packageJson.dependencies["cjs-db"] = "1.0.0"; + await Deno.writeTextFile(packageJsonPath, JSON.stringify(packageJson, null, 2)); + + await withServer(projectDir, async (server) => { + const response = await fetch(`http://127.0.0.1:${server.port}/api/db`); + const body = await response.text(); + + assertEquals( + response.status, + 200, + `Should return 200\nResponse body: ${body}\n${server.logs.join("").slice(-16000)}`, + ); + assertEquals(JSON.parse(body), { + pool: "function", + emitter: "function", + native: "types:object", + port: 5432, + namespace: "function", + extra: "extra", + dynamic: "function", + }); + }); + }); + it("fails closed when API isolation conflicts with a broad host execution grant", async () => { const projectDir = await createTestProject( "isolation-fail-closed-test", From bbb0cf506f8433fdc46c2b4820ff5a4e5b444449 Mon Sep 17 00:00:00 2001 From: Koji Wakayama Date: Sat, 7 Nov 2026 08:41:51 +0100 Subject: [PATCH 2/2] test(api): cover which dependencies a compiled binary loads through native require --- src/routing/api/module-loader/loader.test.ts | 23 +++++++++++--------- src/routing/api/module-loader/loader.ts | 4 ++-- 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/src/routing/api/module-loader/loader.test.ts b/src/routing/api/module-loader/loader.test.ts index 907c0a9980..6872fddb33 100644 --- a/src/routing/api/module-loader/loader.test.ts +++ b/src/routing/api/module-loader/loader.test.ts @@ -15,6 +15,7 @@ import { bundledModuleScopeDiscriminator, bundlerForcesTypeScript, canDirectImportSpecifier, + compiledCommonJSDependencies, createNativeCommonJSPlugin, generateCompiledBinaryRequireShim, getNodeExternalPackagesToResolve, @@ -1909,16 +1910,18 @@ describe("loadHandlerModule", { sanitizeResources: false, sanitizeOps: false }, }); await writePackage("cjs-lib", { main: "index.js" }); - const esmDeps = await resolveEsmUserDependencies( - tmpDir, - fs, - new Map([ - ["type-module-lib", "^1.0.0"], - ["mjs-main-lib", "^1.0.0"], - ["exports-import-lib", "^1.0.0"], - ["cjs-lib", "^1.0.0"], - ["missing-lib", "^1.0.0"], - ]), + const userDeps = new Map([ + ["type-module-lib", "^1.0.0"], + ["mjs-main-lib", "^1.0.0"], + ["exports-import-lib", "^1.0.0"], + ["cjs-lib", "^1.0.0"], + ["missing-lib", "^1.0.0"], + ]); + const esmDeps = await resolveEsmUserDependencies(tmpDir, fs, userDeps); + // A compiled binary loads every non-ESM dependency through native require. + assertEquals( + await compiledCommonJSDependencies(tmpDir, fs, userDeps), + new Set(["cjs-lib", "missing-lib"]), ); assertEquals(esmDeps.has("type-module-lib"), true); diff --git a/src/routing/api/module-loader/loader.ts b/src/routing/api/module-loader/loader.ts index 3755be7ef6..0f678c8937 100644 --- a/src/routing/api/module-loader/loader.ts +++ b/src/routing/api/module-loader/loader.ts @@ -1555,9 +1555,9 @@ const NATIVE_COMMONJS_NAMESPACE = "vf-native-cjs"; * is not an ES module package (see resolveEsmUserDependencies), which keeps * loading from its file:// URL. */ -async function compiledCommonJSDependencies( +export async function compiledCommonJSDependencies( projectDir: string, - fs: ProjectSourceSnapshot, + fs: Parameters[1], userDeps: Map, ): Promise> { const esmDeps = await resolveEsmUserDependencies(projectDir, fs, userDeps);