Skip to content

Commit 3127769

Browse files
authored
fix: comprehensive quality scan fixes across codebase (#111)
Critical fixes: - process-lock: atomic mkdir without recursive to prevent TOCTOU race - http-request: add res.on('error') handler to prevent hanging promises - dlx/manifest: atomic writes via renameSync in all write paths - promise-queue: clear() rejects pending tasks, event-driven onIdle() Security fixes: - json-parser: deep prototype pollution check via JSON.parse reviver - esbuild-config: bundle shared deps inline (no extraneous devDeps in dist) Bug fixes: - abort: use AbortSignal.timeout() to prevent timer leaks - cache-with-ttl: guard JSON.parse for corrupted cache entries - isolation: wrap JSON.parse with file-path context in error - memoization: fix expired entry cleanup, implement clearAllCaches, prevent thundering herd in memoizeAsync, fix memoizeWeak undefined - ansi: fix ST pattern to correctly separate BEL|ESC\|0x9C - argv/parse: remove ambiguous short-flag inference from hasFlag - progress: guard division by zero when total=0 - promise-queue: maxQueueLength check uses !== undefined - promises: pRetry captures latest error instead of first - github: remove double JSON.stringify/parse in cacheFetchGhsa - ipc: unref waitForIpc timer to prevent blocking exit Script fixes: - fix/commonjs-exports, fix/path-aliases, test/filter: fix parent directory traversal (off-by-one in __dirname resolution) - test/main: deduplicate NODE_OPTIONS in runIsolatedTests - test/cover: clean exit on build failure - validate/no-extraneous-deps: add .catch() on main() - orchestrator: use logger instead of console.log - esbuild-config: remove console from drop list, add node18 comment
1 parent 0d19a99 commit 3127769

25 files changed

Lines changed: 216 additions & 158 deletions

scripts/build-externals/esbuild-config.mjs

Lines changed: 3 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -177,21 +177,6 @@ function createStubPlugin(stubMap = STUB_MAP) {
177177
}
178178
}
179179

180-
// Shared dependencies that exist as standalone bundle files in dist/external/.
181-
// These must be marked external in bundles that would otherwise inline them,
182-
// so that at runtime they resolve to the existing bundle wrappers.
183-
const SHARED_EXTERNAL_DEPS = [
184-
'debug',
185-
'has-flag',
186-
'p-map',
187-
'signal-exit',
188-
'spdx-correct',
189-
'spdx-expression-parse',
190-
'supports-color',
191-
'which',
192-
'yoctocolors-cjs',
193-
]
194-
195180
/**
196181
* Get package-specific esbuild options.
197182
*
@@ -209,12 +194,6 @@ export function getPackageSpecificOptions(packageName) {
209194
} else if (packageName === 'zod') {
210195
// Zod has localization files we don't need.
211196
opts.external = [...(opts.external || []), './locales/*']
212-
} else if (packageName === 'debug') {
213-
// Mark supports-color as external - it exists as a standalone bundle wrapper.
214-
opts.external = [...(opts.external || []), 'supports-color']
215-
} else if (packageName === 'pico-pack') {
216-
// Mark p-map as external - it has its own standalone bundle.
217-
opts.external = [...(opts.external || []), 'p-map']
218197
} else if (packageName === 'external-pack') {
219198
// Inquirer packages have heavy dependencies we can exclude.
220199
opts.external = [...(opts.external || []), 'rxjs/operators']
@@ -223,10 +202,6 @@ export function getPackageSpecificOptions(packageName) {
223202
opts.footer = {
224203
js: 'if (module.exports && module.exports.default && Object.keys(module.exports).length === 1) { module.exports = module.exports.default; }',
225204
}
226-
} else if (packageName === 'npm-pack') {
227-
// Mark shared deps as external - they exist as standalone bundle wrappers.
228-
// This eliminates ~100KB of duplication in the npm-pack bundle.
229-
opts.external = [...(opts.external || []), ...SHARED_EXTERNAL_DEPS]
230205
} else if (packageName === '@socketregistry/packageurl-js') {
231206
// packageurl-js imports from socket-lib, creating a circular dependency.
232207
// Mark socket-lib imports as external to avoid bundling issues.
@@ -256,6 +231,8 @@ export function getEsbuildConfig(entryPoint, outfile, packageOpts = {}) {
256231
entryPoints: [entryPoint],
257232
bundle: true,
258233
platform: 'node',
234+
// Intentionally conservative: node18 ensures maximum compatibility
235+
// for bundled externals consumed by downstream packages.
259236
target: 'node18',
260237
format: 'cjs',
261238
outfile,
@@ -296,7 +273,7 @@ export function getEsbuildConfig(entryPoint, outfile, packageOpts = {}) {
296273
keepNames: true,
297274
// Additional optimizations:
298275
pure: ['console.log', 'console.debug', 'console.warn'],
299-
drop: ['debugger', 'console'],
276+
drop: ['debugger'],
300277
ignoreAnnotations: false,
301278
// Define compile-time constants for dead code elimination.
302279
define: {

scripts/build-externals/orchestrator.mjs

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,14 @@ import { promises as fs } from 'node:fs'
77
import path from 'node:path'
88
import { fileURLToPath } from 'node:url'
99

10+
import { getDefaultLogger } from '@socketsecurity/lib-stable/logger'
11+
1012
import { bundlePackage } from './bundler.mjs'
1113
import { externalPackages, scopedPackages } from './config.mjs'
1214
import { ensureDir } from './copy-files.mjs'
1315

16+
const logger = getDefaultLogger()
17+
1418
const __dirname = path.dirname(fileURLToPath(import.meta.url))
1519
const rootDir = path.resolve(__dirname, '..', '..')
1620
const distExternalDir = path.join(rootDir, 'dist', 'external')
@@ -84,7 +88,7 @@ async function bundleAllPackages(options = {}) {
8488
}
8589
} catch {
8690
if (!quiet) {
87-
console.log(` Skipping optional package ${scope}/${name}`)
91+
logger.log(` Skipping optional package ${scope}/${name}`)
8892
}
8993
}
9094
} else {
@@ -123,7 +127,7 @@ async function bundleAllPackages(options = {}) {
123127
}
124128
} catch {
125129
if (!quiet) {
126-
console.log(` Skipping optional package ${scope}/${pkg}`)
130+
logger.log(` Skipping optional package ${scope}/${pkg}`)
127131
}
128132
}
129133
} else {
@@ -226,7 +230,7 @@ async function fixNodeGypStrings(dir, options = {}) {
226230
await fs.writeFile(filePath, fixed, 'utf8')
227231

228232
if (!quiet) {
229-
console.log(
233+
logger.log(
230234
` Fixed node-gyp string in ${path.relative(path.join(dir, '..', '..'), filePath)}`,
231235
)
232236
}

scripts/fix/commonjs-exports.mjs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ import { getDefaultLogger } from '@socketsecurity/lib-stable/logger'
1717
const logger = getDefaultLogger()
1818

1919
const __dirname = path.dirname(fileURLToPath(import.meta.url))
20-
const distDir = path.resolve(__dirname, '..', 'dist')
20+
const distDir = path.resolve(__dirname, '..', '..', 'dist')
2121

2222
/**
2323
* Process files in a directory and fix CommonJS exports.

scripts/fix/path-aliases.mjs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,8 @@ import { getDefaultLogger } from '@socketsecurity/lib-stable/logger'
1414
const logger = getDefaultLogger()
1515

1616
const __dirname = path.dirname(fileURLToPath(import.meta.url))
17-
const distDir = path.resolve(__dirname, '..', 'dist')
18-
const _srcDir = path.resolve(__dirname, '..', 'src')
17+
const distDir = path.resolve(__dirname, '..', '..', 'dist')
18+
const _srcDir = path.resolve(__dirname, '..', '..', 'src')
1919

2020
// Map of path aliases to their actual directories
2121
const pathAliases = {

scripts/test/cover.mjs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,10 @@ const buildResult = await spawn('node', ['scripts/build/main.mjs'], {
4343
},
4444
})
4545
if (buildResult.code !== 0) {
46-
throw new Error('Build with source maps failed')
46+
logger.error('Build with source maps failed')
47+
process.exitCode = 1
48+
// eslint-disable-next-line unicorn/no-process-exit
49+
process.exit()
4750
}
4851

4952
// Run vitest with coverage enabled, capturing output

scripts/test/filter.mjs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import { getDefaultLogger } from '@socketsecurity/lib-stable/logger'
1515

1616
const logger = getDefaultLogger()
1717
const __dirname = path.dirname(fileURLToPath(import.meta.url))
18-
const projectRoot = path.resolve(__dirname, '..')
18+
const projectRoot = path.resolve(__dirname, '..', '..')
1919

2020
// Find all coverage JSON files
2121
const coverageDir = path.join(projectRoot, 'coverage')

scripts/test/main.mjs

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -324,8 +324,13 @@ async function runIsolatedTests(options) {
324324
cwd: rootPath,
325325
env: {
326326
...process.env,
327-
NODE_OPTIONS:
328-
`${process.env.NODE_OPTIONS || ''} --max-old-space-size=${process.env.CI ? 8192 : 4096} --unhandled-rejections=warn`.trim(),
327+
NODE_OPTIONS: [
328+
...(process.env.NODE_OPTIONS || '')
329+
.split(/\s+/)
330+
.filter(opt => opt && !opt.startsWith('--max-old-space-size')),
331+
`--max-old-space-size=${process.env.CI ? 8192 : 4096}`,
332+
'--unhandled-rejections=warn',
333+
].join(' '),
329334
VITEST: '1',
330335
},
331336
stdio: 'inherit',

scripts/validate/no-extraneous-dependencies.mjs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -328,4 +328,7 @@ async function main() {
328328
}
329329
}
330330

331-
main()
331+
main().catch(error => {
332+
logger.fail(`Validation failed: ${error.message}`)
333+
process.exitCode = 1
334+
})

src/abort.ts

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,5 @@ export function createTimeoutSignal(ms: number): AbortSignal {
4444
if (ms <= 0) {
4545
throw new TypeError('timeout must be a positive number')
4646
}
47-
const controller = new AbortController()
48-
setTimeout(() => controller.abort(), ms)
49-
return controller.signal
47+
return AbortSignal.timeout(Math.ceil(ms))
5048
}

src/ansi.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ const ANSI_REGEX = /\x1b\[[0-9;]*m/g
2727
export function ansiRegex(options?: { onlyFirst?: boolean }): RegExp {
2828
const { onlyFirst } = options ?? {}
2929
// Valid string terminator sequences are BEL, ESC\, and 0x9c.
30-
const ST = '(?:\\u0007\\u001B\\u005C|\\u009C)'
30+
const ST = '(?:\\u0007|\\u001B\\u005C|\\u009C)'
3131
// OSC sequences only: ESC ] ... ST (non-greedy until the first ST).
3232
const osc = `(?:\\u001B\\][\\s\\S]*?${ST})`
3333
// CSI and related: ESC/C1, optional intermediates, optional params (supports ; and :) then final byte.

0 commit comments

Comments
 (0)