Skip to content

Commit 831a22d

Browse files
BridgeARrochdev
authored andcommitted
chore(scripts): flag unquoted ** globs in npm scripts (#8061)
* chore(scripts): flag unquoted `**` globs in npm scripts POSIX sh — which is what both `npm run` and `yarn run` ultimately invoke — does not support globstar. An unquoted `**` in a package.json script therefore collapses to a single `*`, so the shell only passes one directory level through to mocha/glob and specs deeper than that are silently skipped. That is how a handful of suites were under-running without anyone noticing. Two coordinated pieces: 1. A new `findUnquotedGlobstar` helper (quote-state-aware, so it doesn't false-positive on patterns that are already quoted) runs over every `scripts` entry during `verify-exercised-tests` and fails the check with a pointer to the offending column if any unquoted `**` is found. Doing this via string scanning instead of a glob-expansion probe is important because `globSync` itself will happily expand `**` recursively regardless of quoting, so it can't distinguish "works locally because glob is being nice" from "broken in CI because sh already collapsed it". 2. Wrap the existing offenders in double quotes so they actually pass the check: `test:aiguard`, `test:debugger`, `test:trace:guardrails`, `test:esbuild`, `test:webpack`, `test:instrumentations:misc`, `test:core`, `test:code-origin`, `test:lambda`, `test:openfeature`, and `test:profiler`. * test(debugger): work around fake-timers hasOwnProperty regression Root cause is an upstream regression in fake-timers commit `9c56bfdc` ("fix(#534): remove memory leak and potential security issue"), which renamed what looked like a typo: - clock[method].hadOwnProperty = Object.prototype.hasOwnProperty.call( + clock[method].hasOwnProperty = Object.prototype.hasOwnProperty.call( The original name (`hadOwnProperty`) was load-bearing: the property holds a boolean flag used by `uninstall` to decide whether to `delete` the method or write it back. After the rename, every fake clock method has its inherited `Object.prototype.hasOwnProperty` shadowed by a boolean. When sinon later does `sinon.stub(process.hrtime, 'bigint')`, its `wrapMethod` goes const owned = object.hasOwnProperty ? object.hasOwnProperty(property) // ← boolean, not callable : hasOwnProperty(object, property); and crashes on the invocation.
1 parent f269ef2 commit 831a22d

4 files changed

Lines changed: 102 additions & 17 deletions

File tree

package.json

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -24,44 +24,44 @@
2424
"release:proposal": "node scripts/release/proposal",
2525
"services": "node ./scripts/install_plugin_modules && node packages/dd-trace/test/setup/services",
2626
"test": "echo '\nError: The root \"npm test\" command is intentionally disabled.\n\nInstead, run specific test suites:\n - npm run test:trace:core\n - npm run test:appsec\n - etc.\n\nOr run individual test files:\n npx mocha path/to/test.spec.js\n\nSee CONTRIBUTING.md (Testing section) for more details.\n' && exit 1",
27-
"test:aiguard": "mocha packages/dd-trace/test/aiguard/**/*.spec.js",
27+
"test:aiguard": "mocha \"packages/dd-trace/test/aiguard/**/*.spec.js\"",
2828
"test:aiguard:ci": "nyc --silent node init && nyc -- npm run test:aiguard",
2929
"test:appsec": "mocha --exclude \"packages/dd-trace/test/appsec/**/*.plugin.spec.js\" \"packages/dd-trace/test/appsec/**/*.spec.js\"",
3030
"test:appsec:ci": "nyc --silent node init && nyc -- npm run test:appsec",
3131
"test:appsec:plugins": "mocha \"packages/dd-trace/test/appsec/**/*.@(${PLUGINS}).plugin.spec.js\"",
3232
"test:appsec:plugins:ci": "yarn services && nyc --silent node init && nyc -- npm run test:appsec:plugins",
33-
"test:debugger": "mocha packages/dd-trace/test/debugger/**/*.spec.js",
33+
"test:debugger": "mocha \"packages/dd-trace/test/debugger/**/*.spec.js\"",
3434
"test:debugger:ci": "nyc --silent node init && nyc -- npm run test:debugger",
3535
"test:eslint-rules": "node eslint-rules/*.test.mjs",
3636
"test:trace:core": "node scripts/mocha-parallel-files.js --expose-gc --timeout 30000 -- \"packages/dd-trace/test/*.spec.js\" \"packages/dd-trace/test/{agent,ci-visibility,config,crashtracking,datastreams,encode,exporters,msgpack,opentelemetry,opentracing,payload-tagging,plugins,remote_config,service-naming,standalone,telemetry,external-logger}/**/*.spec.js\"",
3737
"test:trace:core:ci": "nyc --silent node init && nyc -- npm run test:trace:core",
38-
"test:trace:guardrails": "mocha packages/dd-trace/test/guardrails/**/*.spec.js",
38+
"test:trace:guardrails": "mocha \"packages/dd-trace/test/guardrails/**/*.spec.js\"",
3939
"test:trace:guardrails:ci": "nyc --silent node init && nyc -- npm run test:trace:guardrails",
40-
"test:esbuild": "mocha packages/datadog-esbuild/test/**/*.spec.js",
40+
"test:esbuild": "mocha \"packages/datadog-esbuild/test/**/*.spec.js\"",
4141
"test:esbuild:ci": "nyc --silent node init && nyc -- npm run test:esbuild",
42-
"test:webpack": "mocha packages/datadog-webpack/test/**/*.spec.js",
42+
"test:webpack": "mocha \"packages/datadog-webpack/test/**/*.spec.js\"",
4343
"test:webpack:ci": "nyc --silent node init && nyc -- npm run test:webpack",
4444
"test:instrumentations": "mocha \"packages/datadog-instrumentations/test/@(${PLUGINS}).spec.js\"",
4545
"test:instrumentations:ci": "yarn services && nyc --silent node init && nyc -- npm run test:instrumentations",
46-
"test:instrumentations:misc": "mocha packages/datadog-instrumentations/test/*/**/*.spec.js",
46+
"test:instrumentations:misc": "mocha \"packages/datadog-instrumentations/test/*/**/*.spec.js\"",
4747
"test:instrumentations:misc:ci": "nyc --silent node init && nyc -- npm run test:instrumentations:misc",
48-
"test:core": "node scripts/mocha-parallel-files.js --expose-gc --timeout 30000 -- packages/datadog-core/test/**/*.spec.js",
48+
"test:core": "node scripts/mocha-parallel-files.js --expose-gc --timeout 30000 -- \"packages/datadog-core/test/**/*.spec.js\"",
4949
"test:core:ci": "nyc --silent node init && nyc -- npm run test:core",
50-
"test:code-origin": "mocha packages/datadog-code-origin/test/**/*.spec.js",
50+
"test:code-origin": "mocha \"packages/datadog-code-origin/test/**/*.spec.js\"",
5151
"test:code-origin:ci": "nyc --silent node init && nyc -- npm run test:code-origin",
52-
"test:lambda": "mocha packages/dd-trace/test/lambda/**/*.spec.js",
52+
"test:lambda": "mocha \"packages/dd-trace/test/lambda/**/*.spec.js\"",
5353
"test:lambda:ci": "nyc --silent node init && nyc -- npm run test:lambda",
5454
"test:llmobs:sdk": "mocha --exclude \"packages/dd-trace/test/llmobs/plugins/**/*.spec.js\" \"packages/dd-trace/test/llmobs/**/*.spec.js\"",
5555
"test:llmobs:sdk:ci": "nyc --silent node init && nyc -- npm run test:llmobs:sdk",
5656
"test:llmobs:plugins": "mocha \"packages/dd-trace/test/llmobs/plugins/@(${PLUGINS})/*.spec.js\"",
5757
"test:llmobs:plugins:ci": "yarn services && nyc --silent node init && nyc -- npm run test:llmobs:plugins",
58-
"test:openfeature": "mocha packages/dd-trace/test/openfeature/**/*.spec.js",
58+
"test:openfeature": "mocha \"packages/dd-trace/test/openfeature/**/*.spec.js\"",
5959
"test:openfeature:ci": "nyc --silent node init && nyc -- npm run test:openfeature",
6060
"test:plugins": "node --expose-gc ./node_modules/mocha/bin/mocha.js \"packages/datadog-plugin-@(${PLUGINS})/test/**/${SPEC:-*}*.spec.js\"",
6161
"test:plugins:ci": "yarn services && nyc --silent node init && nyc -- npm run test:plugins",
6262
"test:plugins:ci:flaky": "yarn services && nyc --silent node init && nyc -- npm run test:plugins -- --bail --retries 2",
6363
"test:plugins:upstream": "node ./packages/dd-trace/test/plugins/suite.js",
64-
"test:profiler": "node scripts/mocha-parallel-files.js --expose-gc --timeout 30000 -- packages/dd-trace/test/profiling/**/*.spec.js",
64+
"test:profiler": "node scripts/mocha-parallel-files.js --expose-gc --timeout 30000 -- \"packages/dd-trace/test/profiling/**/*.spec.js\"",
6565
"test:profiler:ci": "nyc --silent node init && nyc -- npm run test:profiler",
6666
"test:integration": "mocha --timeout 60000 \"integration-tests/*.spec.js\"",
6767
"test:integration:aiguard": "mocha --timeout 60000 \"integration-tests/aiguard/*.spec.js\"",

packages/dd-trace/test/debugger/devtools_client/snapshot/collector-deadline.spec.js

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -63,11 +63,14 @@ describe('debugger -> devtools client -> snapshot collector deadline', function
6363
it('should mark properties with timeout when deadline is exceeded', async function () {
6464
// Override the hrtime stub to advance time on each call
6565
// This simulates time passing during collection
66+
// Drive process.hrtime.bigint() with a plain monotonic counter instead of reading
67+
// `clock.now`. This works around a sinon fake-timers regression.
68+
// TODO: Revert when a fix for https://github.com/sinonjs/fake-timers/pull/549#pullrequestreview-4158432441 lands.
6669
sinon.restore()
67-
clock = sinon.useFakeTimers()
70+
let nowNs = 0n
6871
sinon.stub(process.hrtime, 'bigint').callsFake(() => {
69-
const time = BigInt(clock.now) * 1_000_000n
70-
clock.tick(50) // Advance by 50ms after each call
72+
const time = nowNs
73+
nowNs += 50_000_000n // advance 50ms per call
7174
return time
7275
})
7376

@@ -108,12 +111,15 @@ describe('debugger -> devtools client -> snapshot collector deadline', function
108111
let hrtimeCallCount = 0
109112

110113
// Override the hrtime stub to track calls and advance time
114+
// Drive process.hrtime.bigint() with a plain monotonic counter instead of reading
115+
// `clock.now`. This works around a sinon fake-timers regression.
116+
// TODO: Revert when a fix for https://github.com/sinonjs/fake-timers/pull/549#pullrequestreview-4158432441 lands.
111117
sinon.restore()
112-
clock = sinon.useFakeTimers()
118+
let nowNs = 0n
113119
sinon.stub(process.hrtime, 'bigint').callsFake(() => {
114-
const time = BigInt(clock.now) * 1_000_000n
120+
const time = nowNs
115121
hrtimeCallCount++
116-
clock.tick(30) // Advance by 30ms after each call
122+
nowNs += 30_000_000n // advance 30ms per call
117123
return time
118124
})
119125

packages/dd-trace/test/debugger/index.spec.js

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,8 @@ describe('debugger/index', () => {
7070
tags: {
7171
'runtime-id': 'test-runtime-id',
7272
},
73+
version: '1.2.3',
74+
env: 'test-env',
7375
url: new URL('http://localhost:8126'),
7476
}
7577

@@ -225,6 +227,7 @@ describe('debugger/index', () => {
225227
dynamicInstrumentation: {
226228
enabled: true,
227229
},
230+
env: 'test-env',
228231
hostname: 'test-host',
229232
inputPath: '/debugger/v2/input',
230233
logLevel: 'info',
@@ -234,6 +237,7 @@ describe('debugger/index', () => {
234237
runtimeId: 'test-runtime-id',
235238
service: 'test-service',
236239
url: 'http://localhost:8126/',
240+
version: '1.2.3',
237241
})
238242
})
239243
})

scripts/verify-exercised-tests.js

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -265,6 +265,56 @@ function parseInlineAssignments (prefix) {
265265
return out
266266
}
267267

268+
/**
269+
* Find `**` occurrences in a script string that are NOT inside quotes.
270+
*
271+
* POSIX sh (which is what `npm run`/`yarn run` invokes) does NOT support globstar, so an
272+
* unquoted `**` collapses to `*` (single directory level). For recursive glob matching to
273+
* reach mocha/glob intact, the pattern must be quoted so the shell passes it through as a
274+
* literal string. This analyzer cannot rely on `globSync` alone for the check because it
275+
* expands `**` recursively regardless of quoting — hiding the bug that the shell actually
276+
* breaks unquoted patterns.
277+
*
278+
* @param {string} script
279+
* @returns {{ column: number, context: string }[]}
280+
*/
281+
function findUnquotedGlobstar (script) {
282+
/** @type {{ column: number, context: string }[]} */
283+
const out = []
284+
/** @type {'"'|"'"|null} */
285+
let quote = null
286+
287+
for (let i = 0; i < script.length; i++) {
288+
const ch = script[i]
289+
290+
if (quote) {
291+
// In double-quoted strings, `\` escapes the next character.
292+
if (quote === '"' && ch === '\\' && i + 1 < script.length) {
293+
i++
294+
continue
295+
}
296+
if (ch === quote) quote = null
297+
continue
298+
}
299+
300+
if (ch === '"' || ch === "'") {
301+
quote = ch
302+
continue
303+
}
304+
305+
if (ch === '*' && script[i + 1] === '*') {
306+
// Capture a short context window so the error message points to the offending token.
307+
const start = Math.max(0, i - 20)
308+
const end = Math.min(script.length, i + 25)
309+
out.push({ column: i, context: script.slice(start, end) })
310+
// Skip the second `*` to avoid double-reporting.
311+
i++
312+
}
313+
}
314+
315+
return out
316+
}
317+
268318
/**
269319
* Extract `export NAME=value` assignments from a multi-line `run:` string.
270320
* @param {string} run
@@ -927,6 +977,31 @@ function main () {
927977
const knownScripts = new Set(Object.keys(scripts))
928978
const scriptPrefixes = buildScriptPrefixSet(scripts)
929979

980+
// Detect `**` globs in scripts that are not wrapped in quotes. POSIX sh drops globstar, so
981+
// unquoted `**` degrades to `*` and only a single directory level is passed through to
982+
// mocha/globSync — silently missing any spec file deeper than one subdirectory.
983+
/** @type {string[]} */
984+
const unquotedGlobstar = []
985+
for (const scriptName of Object.keys(scripts).sort((a, b) => a.localeCompare(b, 'en'))) {
986+
const cmd = scripts[scriptName]
987+
if (typeof cmd !== 'string') continue
988+
989+
for (const hit of findUnquotedGlobstar(cmd)) {
990+
unquotedGlobstar.push(
991+
`package.json: script "${scriptName}" contains an unquoted "**" at column ${hit.column} ` +
992+
`(near "${hit.context.trim()}"). Wrap the glob in double quotes so POSIX sh passes ` +
993+
'it through to mocha/glob as a literal; otherwise `**` collapses to `*` and specs ' +
994+
'deeper than one subdirectory are silently skipped.'
995+
)
996+
}
997+
}
998+
999+
if (unquotedGlobstar.length) {
1000+
process.stderr.write('Unquoted `**` globs detected in package.json scripts:\n')
1001+
for (const msg of unquotedGlobstar) process.stderr.write(`- ${msg}\n`)
1002+
process.exit(1)
1003+
}
1004+
9301005
const testFiles = findTestFiles(repoRoot)
9311006
const { globs, matchedFiles } = expandScriptGlobs(repoRoot, scripts)
9321007

0 commit comments

Comments
 (0)