From 668365a848aecb0e7b59586e84abe8fcfe56d47e Mon Sep 17 00:00:00 2001 From: jdalton Date: Mon, 31 Aug 2026 13:43:18 -0400 Subject: [PATCH] fix(build): resolve vendored blessed requires The published package shipped bare require('blessed/...') calls in external/blessed-contrib/lib/widget/{table,charts/bar,charts/line}.js and external/blessed/vendor/tng.js. Node resolves bare specifiers only through node_modules, and external/ is not one, so `socket threat-feed` crashed on launch with "Cannot find module 'blessed/lib/widgets/box'". `socket analytics` and `socket audit-log` load the same widgets. copyExternalPackages() already rewired those requires, but it runs in the first config's writeBundle, and the blessed-contrib configs then bundle back out over the same files with 'blessed' marked external. Give that bundle its own rewrite so the emitted requires are relative when written, and widen the copy pass to blessed's own tree so vendor/tng.js is covered. Adds a test over the built external/ tree, scoped to the packages the build vendors there. Refs SURF-1445, SURF-1639 --- .config/rollup.dist.config.mjs | 47 ++++++++++---- test/external-bare-requires.test.mts | 91 ++++++++++++++++++++++++++++ 2 files changed, 127 insertions(+), 11 deletions(-) create mode 100644 test/external-bare-requires.test.mts diff --git a/.config/rollup.dist.config.mjs b/.config/rollup.dist.config.mjs index 52eab0bec..154474889 100644 --- a/.config/rollup.dist.config.mjs +++ b/.config/rollup.dist.config.mjs @@ -74,6 +74,22 @@ const SOCKET_SECURITY_REGISTRY = '@socketsecurity/registry' const UTILS = 'utils' const VENDOR = 'vendor' +// A fresh regexp per call: socketModifyPlugin advances lastIndex, so a shared +// instance would make chunks skip each other's matches. +function newBareBlessedRequireRegExp() { + return /(?<=require[$\w]*(?:\.resolve)?\(["'])blessed(?=(?:\/[^"']+)?["']\))/g +} + +// '.' rather than '' keeps a require from a file sitting in blessed's own root +// from becoming an absolute-looking specifier. +function relativeBlessedPath(filepath) { + return ( + normalizePath( + path.relative(path.dirname(filepath), constants.blessedPath), + ) || '.' + ) +} + async function copyInitGradle() { const filepath = path.join(constants.srcPath, 'commands/manifest/init.gradle') const destPath = path.join(constants.distPath, 'init.gradle') @@ -197,22 +213,26 @@ async function copyExternalPackages() { await removeEmptyDirs(thePath) }), ) - // Rewire 'blessed' inside 'blessed-contrib'. + // Rewire 'blessed' inside 'blessed-contrib', and inside blessed's own vendor + // files, which reach for it by bare name too. await Promise.all( - ( - await fastGlob.glob(['**/*.js'], { + [blessedPath, blessedContribPath].map(async cwd => { + const filepaths = await fastGlob.glob(['**/*.js'], { absolute: true, - cwd: blessedContribPath, + cwd, ignore: [NODE_MODULES_GLOB_RECURSIVE], }) - ).map(async p => { - const relPath = path.relative(path.dirname(p), blessedPath) - const content = await fs.readFile(p, 'utf8') - const modded = content.replace( - /(?<=require\(["'])blessed(?=(?:\/[^"']+)?["']\))/g, - () => relPath, + await Promise.all( + filepaths.map(async p => { + const relPath = relativeBlessedPath(p) + const content = await fs.readFile(p, 'utf8') + const modded = content.replace( + newBareBlessedRequireRegExp(), + () => relPath, + ) + await fs.writeFile(p, modded, 'utf8') + }), ) - await fs.writeFile(p, modded, 'utf8') }), ) } @@ -563,6 +583,11 @@ export default async () => { ) }, plugins: [ + // Runs after copyExternalPackages() and overwrites what it rewired. + socketModifyPlugin({ + find: newBareBlessedRequireRegExp(), + replace: () => relativeBlessedPath(path.join(rootPath, relPath)), + }), nodeResolve({ exportConditions: ['node'], extensions: ['.mjs', '.js', '.json'], diff --git a/test/external-bare-requires.test.mts b/test/external-bare-requires.test.mts new file mode 100644 index 000000000..40fc951eb --- /dev/null +++ b/test/external-bare-requires.test.mts @@ -0,0 +1,91 @@ +/** + * Guards that nothing under external/ reaches for a bundled package by bare + * name. Node resolves bare specifiers only through node_modules directories, + * and external/ is not one, so such a require throws "Cannot find module" in + * the published package even though the file ships correctly on disk. + */ +import { existsSync, readFileSync, readdirSync } from 'node:fs' +import path from 'node:path' +import { fileURLToPath } from 'node:url' + +import { describe, expect, it } from 'vitest' + +const rootPath = path.join(path.dirname(fileURLToPath(import.meta.url)), '..') +const externalPath = path.join(rootPath, 'external') + +// Mirrors EXTERNAL_PACKAGES in .config/rollup.base.config.mjs. Scoped to what +// the build vendors into external/, because unbundled optional peers reached by +// bare name there (blessed's pty.js/term.js terminal widget, node-gyp under +// @socketsecurity/registry) are a separate, longstanding question. +const bundledNames = new Set([ + '@socketsecurity/registry', + 'blessed', + 'blessed-contrib', +]) + +const bareRequireRegExp = + /require[$\w]*(?:\.resolve)?\(\s*['"]([^'"]+)['"]\s*\)/g + +function findScripts(dirPath: string): string[] { + const scriptPaths: string[] = [] + for (const entry of readdirSync(dirPath, { withFileTypes: true })) { + const entryPath = path.join(dirPath, entry.name) + if (entry.isDirectory()) { + scriptPaths.push(...findScripts(entryPath)) + } else if (entry.name.endsWith('.js')) { + scriptPaths.push(entryPath) + } + } + return scriptPaths.sort() +} + +function packageNameFromSpecifier(specifier: string): string | undefined { + if ( + !specifier || + specifier.startsWith('.') || + specifier.startsWith('#') || + specifier.startsWith('node:') || + path.isAbsolute(specifier) + ) { + return undefined + } + const segments = specifier.split('/') + return specifier.startsWith('@') + ? segments.slice(0, 2).join('/') + : segments[0] +} + +describe('external bare requires', () => { + it('never name a bundled package', () => { + if (!existsSync(externalPath)) { + throw new Error( + `Missing build output at ${externalPath}.\n` + + `→ This test checks what ships, so it needs a built external/.\n` + + `→ Run: pnpm build:dist:src`, + ) + } + const scriptPaths = findScripts(externalPath) + expect(scriptPaths.length).toBeGreaterThan(0) + + const findings: string[] = [] + for (const scriptPath of scriptPaths) { + const relPath = path.relative(rootPath, scriptPath).replace(/\\/g, '/') + const source = readFileSync(scriptPath, 'utf8') + bareRequireRegExp.lastIndex = 0 + let match + while ((match = bareRequireRegExp.exec(source)) !== null) { + const specifier = match[1]! + const pkgName = packageNameFromSpecifier(specifier) + if (!pkgName || !bundledNames.has(pkgName)) { + continue + } + const finding = `${relPath} requires "${specifier}"` + if (!findings.includes(finding)) { + findings.push(finding) + } + } + } + + expect(findings).toEqual([]) + }) +})