The highlight.js clone interpolates a path into a shell string

Context

Third and last item from the code-scanning triage @tobiu surfaced. CodeQL alert 64, rule js/shell-command-injection-from-environment, at buildScripts/build/highlightJs.mjs:53.

This was left as an explicitly unowned lane, not overlooked. #15818 — "FileSystemService interpolates a sandboxed path into a shell — argv, not escaping" — fixed the identical rule elsewhere and scoped this one out by name:

The other 7 open dev code-scanning alerts (… js/shell-command-injection-from-environment in buildScripts/build/highlightJs.mjs). They remain an unowned triage lane; this ticket takes the one rule I verified rather than bundling six unexamined findings behind a confirmed fix.

This ticket takes it.

I initially called this a false positive and that was wrong — worth recording because the reasoning was true. neoPath derives from __dirname, not from the environment, so the rule's name does not literally describe the source. That is correct and it does not make an unquoted shell interpolation safe. The rule name was not the hazard.

Live latest-open sweep: checked latest 12 open issues at 2026-08-21T18:2x UTC; gh search issues for highlightJs OR execSync OR shell-command-injection returned zero open; corpus grep surfaced only the closed #15818 above, which scopes this out rather than covering it.

The Problem

buildScripts/build/highlightJs.mjs:52-53:

const cloneCommand = `${gitCmd} clone --depth 1 https://github.com/highlightjs/highlight.js.git ${tempDir}`;
execSync(cloneCommand, { stdio: 'inherit' });

tempDir is path.resolve(neoPath, 'tmp/highlightjs'), and neoPath is path.resolve(__dirname, '../../') — the checkout location. It is interpolated into a string that execSync hands to a shell, unquoted.

Verified, with the shell doing the parsing rather than my reading of it. Simulating a checkout at /Users/Shared/my repos/neo:

command: git clone --depth 1 https://…/highlight.js.git /Users/Shared/my repos/neo/tmp/highlightjs
clone target argv: ["https://…/highlight.js.git", "/Users/Shared/my", "repos/neo/tmp/highlightjs"]
BROKEN: path split into 3 arguments

Two consequences, and the mundane one is the more likely:

1. Robustness. Any checkout path containing a space breaks the build. On macOS that is an ordinary directory name, not an exotic one.
2. Injection. A checkout path containing shell metacharacters reaches a shell. The threat model is narrow — it needs control of the directory name plus a maintainer running the build — but it is the same class #15818 treated as real.

Scoped precisely: one site. :58 and :69 pass cwd: tempDir as an option rather than interpolating it, and their command strings (npm install, the build script) carry no path. They are already safe and are not touched. A repository-wide grep for the same shape (execSync with a path interpolated into a template literal) found no other occurrence in buildScripts/.

The gitCmd / npmCmd half of the alert is a false positive: both are two-way choices between hardcoded literals ('git.exe' / 'git'). That half is worth stating so the fix is not mistaken for sanitising them.

The Architectural Reality

• buildScripts/build/highlightJs.mjs:52-53 — the sole defective site
• buildScripts/build/highlightJs.mjs:10,16,18,20 — neoPath, tempDir, gitCmd, npmCmd derivations

The house remedy is already established, by #15818 and by its own cited precedent (da7b5a2d3a, replacing bash -c with spawnSync):

Remove the shell rather than sanitise the interpolation. "A shell string reached for to get a capability an argv invocation seemed to lack."

This site is exactly that shape — the command needs no shell feature at all. No pipes, no redirection, no globbing.

The Fix

Replace the interpolated execSync with an argv-array invocation:

execFileSync(gitCmd, ['clone', '--depth', '1', 'https://github.com/highlightjs/highlight.js.git', tempDir], {stdio: 'inherit'});

No shell, so no quoting question and no metacharacter question — the path is one argument by construction. Consistent with the two prior fixes rather than a third approach to the same problem.

Acceptance Criteria

AC amended 2026-08-21 after review PRR_kwDODSospM8AAAABKdJ2lA. Two criteria below were wrong as originally written and are corrected in place rather than quietly marked done. The originals are struck through so the change is auditable.

• :53 invokes git via argv
• No interpolated string reaches execSync anywhere in the file — including via an intermediate variable. no path is interpolated into a shell string anywhere in the file — the original wording described only the inline shape. The real defect was two-step (const cmd = \…${tempDir}`; execSync(cmd)`), so a guard satisfying the original text stayed green against the actual regression path.
• RED-first: the source-scan arm reads the module from disk and is red against the exact original two-step shape — recorded result "via binding: cloneCommand". and fails on current dev — the three argv arms import the module, which on dev has no exports and runs a build on import, so they cannot execute there. One arm satisfies the criterion literally; the other three cannot, and that is a property of the pre-fix module rather than of the coverage.
• The assertion is on the resulting argv, not on a quoted string — a test pinning an escaped string passes for any consistent-but-wrong quoting
• All three exec sites use argv; execSync is no longer imported. :58 and :69 are unchanged, with a control proving cwd-passed paths were never the defect — see Out of Scope amendment below.
• CodeQL alert 64 closes on the merge commit

Out of Scope

Scope amended 2026-08-21. :58 and :69 were originally scoped out as "already safe". They were safe — ${npmCmd} install interpolates one of two hardcoded literals — but they were not checkable. A guard that covers identifier-mediated construction flags them too, and the only way to keep them out is to special-case which interpolations count as safe. That rebuilds the exact blind-spot class this ticket is about: the next unsafe value need only avoid being named like a path. Making the file's invariant absolute is what makes it enforceable, so all three sites are now argv. Deliberate widening, recorded rather than folded in silently.

• Alerts 62/63 (js/prototype-pollution-utility, src/Neo.mjs:563,565) — deliberate prototype enrichment. These want a documented dismissal decision from @tobiu, not a code change; not something an agent should action against the security tab unilaterally.
• Alert js/cors-permissive-configuration, still unowned from #15818's list.
• Any rework of the highlight.js build beyond the exec call.

Avoided Traps

Sanitising the interpolation. Quoting or escaping tempDir would close the alert and keep the shell, leaving the next path-bearing command to re-introduce it. Both prior fixes removed the shell; a third variant here would fragment the remedy.

Dismissing on the rule's name. js/shell-command-injection-from-environment names an environment source, and this path comes from __dirname — a true observation that does not reach the hazard. Recorded because it is the reasoning that nearly closed this as a false positive.

Related

• #15818 — same rule, established the argv remedy, scoped this alert out by name
• da7b5a2d3a — the original bash -c → spawnSync precedent
• #17484 / PR #17485 — sibling code-scanning lane (alerts 41/42)

Retrieval Hint: highlightJs execSync tempDir shell interpolation argv execFileSync code-scanning 64

Origin Session ID: 752da6ac-a6c3-447f-8847-1da4ce49deb8

Decision Record impact: none — applies an established remedy in a build script; no ADR authority touched. Structure-map gate: N/A, no ai/ surface and no file placement.