PER-9666: avoid require binding name that breaks the packaged binary

.semgrepignore

@@ -38,3 +38,16 @@ packages/core/src/api.js
 # in the file-load helper anyway. No user input flows here.
 packages/core/test/unit/maestro-hierarchy.test.js
 packages/core/test/unit/maestro-hierarchy.parity.test.js
+
+# Regression guard for the packaged-binary crash (PER-9666): it statically
+# scans the repo's own source tree for the `require = createRequire` footgun.
+# To walk that tree it builds paths with path.join()/path.resolve() inside its
+# findRepoRoot() and collectSourceFiles() helpers, which trips semgrep's
+# javascript.lang.security.audit.path-traversal.path-join-resolve-traversal
+# rule. No user input flows into those joins — every path is derived from
+# process.cwd() and fs.readdir() of the repo's own packages/ directory, with a
+# fixed allowlist of source extensions and a skip-list of build/dep dirs. It is
+# a build-time test, never shipped or exposed to external input. The rule
+# cannot follow that the inputs are filesystem-internal, so it false-positives
+# on every join. Suppress at the file level with this rationale.
+packages/cli-command/test/noRequireBinding.test.js

packages/cli-command/src/lockfileDiff.js

@@ -6,15 +6,21 @@ import logger from '@percy/logger';
 // time — that way importing this module never throws on older Node versions
 // (or when the optional install was skipped for any other reason). Cached on
 // first successful load so the require only resolves once per process.
-const require = createRequire(import.meta.url);
+// Bound to a non-`require` name on purpose: when this ESM file is transpiled to
+// CommonJS for the packaged binary, naming it `require` collides with two Babel
+// transforms at once — preset-env renames the local `require` to `_require`, and
+// transform-import-meta expands `import.meta.url` into a `require('url')` call
+// that gets renamed to `_require` too, producing `_require(...)` inside its own
+// initializer (TypeError: _require is not a function). See PER smartsnap binary.
+const cjsRequire = createRequire(import.meta.url);
 let _snykModule;
 /* istanbul ignore next: snyk-backed path — the parser requires Node >=18 while
    CI runs the suite on Node 14, so these lines can't execute there; they're
    exercised by the describeSnyk tests on Node >=18 */
 function loadSnyk() {
   if (_snykModule) return _snykModule;
   try {
-    _snykModule = require('snyk-nodejs-lockfile-parser');
+    _snykModule = cjsRequire('snyk-nodejs-lockfile-parser');
     return _snykModule;
   } catch (e) {
     // Surface the underlying require failure so callers can distinguish

packages/cli-command/src/smartsnap.js

@@ -8,11 +8,16 @@ import { diffLockfileDeps } from './lockfileDiff.js';
 import { renderGraphTraceHtml } from './graphTrace.js';
 
 // stream-json is CommonJS — load via createRequire to avoid named-import interop issues.
-const require = createRequire(import.meta.url);
-const { parser } = require('stream-json');
-const { pick } = require('stream-json/filters/Pick');
-const { streamArray } = require('stream-json/streamers/StreamArray');
-const { streamValues } = require('stream-json/streamers/StreamValues');
+// Bound to a non-`require` name on purpose: in the CommonJS-transpiled binary,
+// naming it `require` collides with Babel's preset-env + transform-import-meta,
+// which rewrite `import.meta.url` into a `require('url')` call that then gets
+// renamed alongside the local binding to `_require(...)` inside its own
+// initializer (TypeError: _require is not a function).
+const cjsRequire = createRequire(import.meta.url);
+const { parser } = cjsRequire('stream-json');
+const { pick } = cjsRequire('stream-json/filters/Pick');
+const { streamArray } = cjsRequire('stream-json/streamers/StreamArray');
+const { streamValues } = cjsRequire('stream-json/streamers/StreamValues');
 
 // Webpack/Vite stats prefix loader-resolved virtual modules with a NUL byte
 // (e.g. "\u0000/path/to/file?commonjs-es-import"). Strip it so the path lines

packages/cli-command/test/noRequireBinding.test.js

@@ -0,0 +1,99 @@
+import fs from 'fs';
+import path from 'path';
+
+// Regression guard for the packaged-binary crash fixed in
+// fix/cli-command-createRequire-binary-crash.
+//
+// In an ESM source file, declaring the createRequire result with the binding
+// name `require`:
+//
+//   const require = createRequire(import.meta.url);
+//
+// is fine under Node ESM, but breaks the `pkg`-built executable. When the file
+// is transpiled to CommonJS for the binary, two Babel transforms collide:
+//   - preset-env renames the local `require` binding to `_require`, and
+//   - transform-import-meta expands `import.meta.url` into a `require('url')`
+//     call that is *also* renamed to `_require`.
+// The result is `_require(...)` evaluated inside its own initializer →
+// `TypeError: _require is not a function`, thrown on startup before any command
+// runs. The fix is simply to bind to a non-`require` name (e.g. `cjsRequire`).
+//
+// This is a static source scan — no build step, no new tooling, just fs + a
+// regex over every package's published source. It runs inside the existing
+// Jasmine node suite.
+
+// Walk up from the package cwd to the monorepo root (the dir holding lerna.json
+// and packages/), so the scan covers the whole repo regardless of which package
+// the suite happens to run from.
+function findRepoRoot() {
+  let dir = process.cwd();
+  for (;;) {
+    if (fs.existsSync(path.join(dir, 'lerna.json')) &&
+        fs.existsSync(path.join(dir, 'packages'))) return dir;
+    let parent = path.dirname(dir);
+    if (parent === dir) throw new Error('could not locate monorepo root');
+    dir = parent;
+  }
+}
+
+// Collect every source file under packages/<pkg>/src. Build output (dist/build),
+// dependencies, tests and coverage are excluded — only authored source matters.
+function collectSourceFiles(root) {
+  const SKIP_DIRS = new Set(['node_modules', 'dist', 'build', 'coverage', 'test', '.nyc_output']);
+  const SRC_EXT = new Set(['.js', '.cjs', '.mjs']);
+  const files = [];
+
+  const walk = (dir) => {
+    for (const entry of fs.readdirSync(dir, { withFileTypes: true })) {
+      if (entry.isDirectory()) {
+        if (!SKIP_DIRS.has(entry.name)) walk(path.join(dir, entry.name));
+      } else if (SRC_EXT.has(path.extname(entry.name))) {
+        files.push(path.join(dir, entry.name));
+      }
+    }
+  };
+
+  const packagesDir = path.join(root, 'packages');
+  for (const pkg of fs.readdirSync(packagesDir, { withFileTypes: true })) {
+    if (!pkg.isDirectory()) continue;
+    const src = path.join(packagesDir, pkg.name, 'src');
+    if (fs.existsSync(src)) walk(src);
+  }
+
+  return files;
+}
+
+// Matches a const/let/var binding literally named `require` assigned from
+// createRequire(...). `\s` spans newlines, so a wrapped declaration is caught.
+const FORBIDDEN = /\b(?:const|let|var)\s+require\s*=\s*createRequire\b/;
+
+describe('source: no `require = createRequire` binding', () => {
+  const root = findRepoRoot();
+  const files = collectSourceFiles(root);
+
+  it('scans a non-trivial number of source files', () => {
+    // Guards against the walk silently finding nothing (wrong cwd, refactor).
+    expect(files.length).toBeGreaterThan(20);
+  });
+
+  it('never binds createRequire to a name `require` (breaks the packaged binary)', () => {
+    const violations = [];
+
+    for (const file of files) {
+      const lines = fs.readFileSync(file, 'utf8').split('\n');
+      lines.forEach((line, i) => {
+        if (FORBIDDEN.test(line)) {
+          violations.push(`${path.relative(root, file)}:${i + 1}: ${line.trim()}`);
+        }
+      });
+    }
+
+    expect(violations)
+      .withContext(
+        'Bind createRequire to a non-`require` name (e.g. `const cjsRequire = ' +
+        'createRequire(import.meta.url)`). Naming it `require` collides with Babel ' +
+        'transforms and crashes the pkg binary with "_require is not a function":\n' +
+        violations.join('\n'))
+      .toEqual([]);
+  });
+});