From 380dd5d10222bace7d72c3cf4c5de800eff0f0e3 Mon Sep 17 00:00:00 2001 From: Derek Lewis Date: Fri, 4 Sep 2026 04:41:38 +0000 Subject: [PATCH] =?UTF-8?q?=F0=9F=8F=97=EF=B8=8F=F0=9F=94=A7=EF=BC=9Aquote?= =?UTF-8?q?=20paths,=20read=20JSON5,=20fix=20the=20EC=20binary?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three faults found while porting this repository's build tasks into OpenINF/.github, all of them here too. `editorconfig-checker` moves to 7.0.0. The wrapper fetches its binary from the newest GitHub release and globs the asset by name, and 4.0.0 renamed every asset from `ec-*` to `editorconfig-checker-*`. So 6.1.1 asks for one that is no longer there, and `verify.validForEC` fails on any machine without a cached binary -- which is every CI runner, and no development machine, so it fails where nobody is watching and passes where they are. JSON5 goes to prettier rather than to Biome, which has no parser for it. Handed one, Biome reports the path as ignored and carries on, so naming `**/*.json5` beside the others read as coverage while being none: nothing was looking at `.renovaterc.json5` at all. The prettier override keeps the spelling renovate documents rather than restyling the file. Every path a task discovers is quoted before it reaches the shell. A task builds its command as one string, so a filename is shell text by the time the tool sees it: a space in one splits an argument in two, and a `$(...)` or a `;` is a command of somebody else's choosing, running in CI. `verify.filenames` objects to names like that, but it cannot be the guard -- it is one task among the rest, and a failing one does not stop the others being handed what it just objected to. The path to the vnu jar is quoted too, in both tasks that run it. It is an absolute path into node_modules rather than something glob found, so the first pass left it bare -- and a checkout under a directory with a space in its name would have split `java -jar` from its argument. The same bug this commit is about, in the one place it did not look. A name that looks like an option gets a leading `./` as well. Quoting settles what the shell does with a name and nothing about what the tool makes of it: `'--write.md'` reaches prettier as `--write.md`, which it reads as an option, printing an error and exiting 0 -- a check that passed having checked nothing. Signed-off-by: Derek Lewis Assisted-by: Claude-Code:claude-opus-5 --- .prettierrc.yml | 11 +++ build/tasks/format/format-dockerfile.mts | 4 +- build/tasks/format/format-js.mts | 4 +- build/tasks/format/format-json.mts | 22 +++--- build/tasks/format/format-liquid.mts | 4 +- build/tasks/format/format-md.mts | 6 +- build/tasks/format/format-scss.mts | 6 +- build/tasks/format/format-svg.mts | 4 +- build/tasks/format/format-toml.mts | 4 +- build/tasks/format/format-ts.mts | 4 +- build/tasks/format/format-yaml.mts | 4 +- build/tasks/verify/verify-dockerfile.mts | 4 +- .../verify/verify-html-valid-for-vnu.mts | 4 +- build/tasks/verify/verify-js.mts | 4 +- build/tasks/verify/verify-json.mts | 22 +++--- build/tasks/verify/verify-liquid.mts | 4 +- build/tasks/verify/verify-md.mts | 8 +-- build/tasks/verify/verify-scss.mts | 6 +- build/tasks/verify/verify-spelling.mts | 4 +- build/tasks/verify/verify-svg.mts | 6 +- build/tasks/verify/verify-toml.mts | 4 +- build/tasks/verify/verify-ts.mts | 4 +- build/tasks/verify/verify-unit.mts | 4 +- build/tasks/verify/verify-yaml.mts | 4 +- build/utils.mts | 31 +++++++++ build/utils.test.mts | 69 ++++++++++++++++++- package.json | 2 +- pnpm-lock.yaml | 12 ++-- 28 files changed, 190 insertions(+), 75 deletions(-) diff --git a/.prettierrc.yml b/.prettierrc.yml index f32467f9d..c0a15d332 100644 --- a/.prettierrc.yml +++ b/.prettierrc.yml @@ -23,6 +23,17 @@ overrides: parser: 'liquid-html' singleAttributePerLine: false singleQuote: false + - files: + - '*.json5' + options: + parser: json5 + # Biome has no JSON5 parser and skips these files without saying so, + # which left `verify.json` claiming a coverage it did not have. Prettier + # is what reads them now -- for syntax and layout, not to restyle them: + # `.renovaterc.json5` is written the way renovate documents it, and that + # spelling is not ours to change. + quoteProps: preserve + singleQuote: false - files: - '*.md' options: diff --git a/build/tasks/format/format-dockerfile.mts b/build/tasks/format/format-dockerfile.mts index b1bdc924b..13af0e929 100644 --- a/build/tasks/format/format-dockerfile.mts +++ b/build/tasks/format/format-dockerfile.mts @@ -5,7 +5,7 @@ * @module {type ES6Module} build/tasks/format/format-dockerfile */ -import { exec, glob } from '@openinf/portal/build/utils'; +import { exec, glob, quote } from '@openinf/portal/build/utils'; const dockerfileFiles = await glob([ '.devcontainer/**/Dockerfile', @@ -14,7 +14,7 @@ const dockerfileFiles = await glob([ ]); let exitCode = 0; -const scripts = [`dprint fmt ${dockerfileFiles.join(' ')}`]; +const scripts = [`dprint fmt ${quote(dockerfileFiles)}`]; for (const element of scripts) { exitCode = await exec(element); diff --git a/build/tasks/format/format-js.mts b/build/tasks/format/format-js.mts index be7e00add..5a0a44886 100644 --- a/build/tasks/format/format-js.mts +++ b/build/tasks/format/format-js.mts @@ -5,7 +5,7 @@ * @module {type ES6Module} build/tasks/format/format-js */ -import { exec, glob } from '@openinf/portal/build/utils'; +import { exec, glob, quote } from '@openinf/portal/build/utils'; const jsFiles = await glob([ '**/*.js', @@ -16,7 +16,7 @@ const jsFiles = await glob([ ]); let exitCode = 0; -const scripts = [`biome check --write ${jsFiles.join(' ')}`]; +const scripts = [`biome check --write ${quote(jsFiles)}`]; for (const element of scripts) { exitCode = await exec(element); diff --git a/build/tasks/format/format-json.mts b/build/tasks/format/format-json.mts index 1a2925f0a..b043d5128 100644 --- a/build/tasks/format/format-json.mts +++ b/build/tasks/format/format-json.mts @@ -5,18 +5,22 @@ * @module {type ES6Module} build/tasks/format/format-json */ -import { exec, glob } from '@openinf/portal/build/utils'; +import { exec, glob, quote } from '@openinf/portal/build/utils'; -const jsonFiles = await glob([ - '**/*.json', - '**/*.json5', - '**/*.jsonc', - '!_site/', - '!node_modules/', -]); +const EXCLUDED = ['!_site/', '!node_modules/']; + +const jsonFiles = await glob(['**/*.json', '**/*.jsonc', ...EXCLUDED]); +// Biome has no JSON5 parser. Handed one it reports the path as ignored and +// carries on with the rest, so listing `**/*.json5` beside the others read as +// coverage while being none: nothing looked at `.renovaterc.json5` at all. +// Prettier does have the parser. +const json5Files = await glob(['**/*.json5', ...EXCLUDED]); let exitCode = 0; -const scripts = [`biome check --write ${jsonFiles.join(' ')}`]; +const scripts = [ + `biome check --write ${quote(jsonFiles)}`, + ...(json5Files.length > 0 ? [`prettier --write ${quote(json5Files)}`] : []), +]; for (const element of scripts) { exitCode = await exec(element); diff --git a/build/tasks/format/format-liquid.mts b/build/tasks/format/format-liquid.mts index 80e4db50d..6a405b647 100644 --- a/build/tasks/format/format-liquid.mts +++ b/build/tasks/format/format-liquid.mts @@ -5,7 +5,7 @@ * @module {type ES6Module} build/tasks/format/format-liquid */ -import { exec, glob } from '@openinf/portal/build/utils'; +import { exec, glob, quote } from '@openinf/portal/build/utils'; const LiquidFiles = await glob([ '**/*.html', @@ -15,7 +15,7 @@ const LiquidFiles = await glob([ ]); let exitCode = 0; -const scripts = [`prettier --write ${LiquidFiles.join(' ')}`]; +const scripts = [`prettier --write ${quote(LiquidFiles)}`]; for (const element of scripts) { exitCode = await exec(element); diff --git a/build/tasks/format/format-md.mts b/build/tasks/format/format-md.mts index 2635a4534..bc5bb21fb 100644 --- a/build/tasks/format/format-md.mts +++ b/build/tasks/format/format-md.mts @@ -5,7 +5,7 @@ * @module {type ES6Module} build/tasks/format/format-md */ -import { exec, glob } from '@openinf/portal/build/utils'; +import { exec, glob, quote } from '@openinf/portal/build/utils'; const markdownFiles = await glob([ '**/*.md', @@ -24,8 +24,8 @@ const markdownFiles = await glob([ let exitCode = 0; const scripts = [ - `prettier --write ${markdownFiles.join(' ')}`, - `markdownlint-cli2 --fix ${markdownFiles.join(' ')}`, + `prettier --write ${quote(markdownFiles)}`, + `markdownlint-cli2 --fix ${quote(markdownFiles)}`, ]; for (const element of scripts) { diff --git a/build/tasks/format/format-scss.mts b/build/tasks/format/format-scss.mts index 06b528a2f..d2e1ef011 100644 --- a/build/tasks/format/format-scss.mts +++ b/build/tasks/format/format-scss.mts @@ -5,14 +5,14 @@ * @module {type ES6Module} build/tasks/format/format-scss */ -import { exec, glob } from '@openinf/portal/build/utils'; +import { exec, glob, quote } from '@openinf/portal/build/utils'; const scssFiles = await glob(['**/*.scss', '!_site/', '!node_modules/']); let exitCode = 0; const scripts = [ - `prettier --write ${scssFiles.join(' ')}`, - `stylelint --fix ${scssFiles.join(' ')}`, + `prettier --write ${quote(scssFiles)}`, + `stylelint --fix ${quote(scssFiles)}`, ]; for (const element of scripts) { diff --git a/build/tasks/format/format-svg.mts b/build/tasks/format/format-svg.mts index 60d9eab57..bde614b6d 100644 --- a/build/tasks/format/format-svg.mts +++ b/build/tasks/format/format-svg.mts @@ -5,12 +5,12 @@ * @module {type ES6Module} build/tasks/format/format-svg */ -import { exec, glob } from '@openinf/portal/build/utils'; +import { exec, glob, quote } from '@openinf/portal/build/utils'; const svgFiles = await glob(['**/*.svg', '!_site/', '!node_modules/']); let exitCode = 0; -const scripts = [`prettier --write ${svgFiles.join(' ')}`]; +const scripts = [`prettier --write ${quote(svgFiles)}`]; for (const element of scripts) { exitCode = await exec(element); diff --git a/build/tasks/format/format-toml.mts b/build/tasks/format/format-toml.mts index 587b06638..90bbba11b 100644 --- a/build/tasks/format/format-toml.mts +++ b/build/tasks/format/format-toml.mts @@ -5,12 +5,12 @@ * @module {type ES6Module} build/tasks/format/format-toml */ -import { exec, glob } from '@openinf/portal/build/utils'; +import { exec, glob, quote } from '@openinf/portal/build/utils'; const tomlFiles = await glob(['**/*.toml', '!_site/', '!node_modules/']); let exitCode = 0; -const scripts = [`dprint fmt ${tomlFiles.join(' ')}`]; +const scripts = [`dprint fmt ${quote(tomlFiles)}`]; for (const element of scripts) { exitCode = await exec(element); diff --git a/build/tasks/format/format-ts.mts b/build/tasks/format/format-ts.mts index 8ec8c5f48..4acd1f74b 100644 --- a/build/tasks/format/format-ts.mts +++ b/build/tasks/format/format-ts.mts @@ -5,7 +5,7 @@ * @module {type ES6Module} build/tasks/format/format-ts */ -import { exec, glob } from '@openinf/portal/build/utils'; +import { exec, glob, quote } from '@openinf/portal/build/utils'; const tsFiles = await glob([ '**/*.ts', @@ -15,7 +15,7 @@ const tsFiles = await glob([ ]); let exitCode = 0; -const scripts = [`biome check --write ${tsFiles.join(' ')}`]; +const scripts = [`biome check --write ${quote(tsFiles)}`]; for (const element of scripts) { exitCode = await exec(element); diff --git a/build/tasks/format/format-yaml.mts b/build/tasks/format/format-yaml.mts index a8dfb7976..088e8b301 100644 --- a/build/tasks/format/format-yaml.mts +++ b/build/tasks/format/format-yaml.mts @@ -5,7 +5,7 @@ * @module {type ES6Module} build/tasks/format/format-yaml */ -import { exec, glob } from '@openinf/portal/build/utils'; +import { exec, glob, quote } from '@openinf/portal/build/utils'; const yamlFiles = await glob([ '**/*.yml', @@ -15,7 +15,7 @@ const yamlFiles = await glob([ ]); let exitCode = 0; -const scripts = [`prettier --write ${yamlFiles.join(' ')}`]; +const scripts = [`prettier --write ${quote(yamlFiles)}`]; for (const element of scripts) { exitCode = await exec(element); diff --git a/build/tasks/verify/verify-dockerfile.mts b/build/tasks/verify/verify-dockerfile.mts index 7062ae44f..d5e689b36 100644 --- a/build/tasks/verify/verify-dockerfile.mts +++ b/build/tasks/verify/verify-dockerfile.mts @@ -5,7 +5,7 @@ * @module {type ES6Module} build/tasks/verify/verify-dockerfile */ -import { exec, glob } from '@openinf/portal/build/utils'; +import { exec, glob, quote } from '@openinf/portal/build/utils'; const dockerfileFiles = await glob([ '.devcontainer/**/Dockerfile', @@ -14,7 +14,7 @@ const dockerfileFiles = await glob([ ]); let exitCode = 0; -const scripts = [`dprint check ${dockerfileFiles.join(' ')}`]; +const scripts = [`dprint check ${quote(dockerfileFiles)}`]; for (const element of scripts) { exitCode = await exec(element); diff --git a/build/tasks/verify/verify-html-valid-for-vnu.mts b/build/tasks/verify/verify-html-valid-for-vnu.mts index ad5c2e063..d9381015e 100644 --- a/build/tasks/verify/verify-html-valid-for-vnu.mts +++ b/build/tasks/verify/verify-html-valid-for-vnu.mts @@ -5,7 +5,7 @@ * @module {type ES6Module} build/tasks/verify/verify-html-valid-for-vnu */ -import { exec, glob } from '@openinf/portal/build/utils'; +import { exec, glob, quote } from '@openinf/portal/build/utils'; import vnu from 'vnu-jar'; const htmlFiles = await glob(['_site/**/*.html', '!node_modules/']); @@ -19,7 +19,7 @@ if (htmlFiles.length === 0) { } let exitCode = 0; -const scripts = [`java -jar ${vnu} ${htmlFiles.join(' ')}`]; +const scripts = [`java -jar ${quote(vnu)} ${quote(htmlFiles)}`]; for (const element of scripts) { exitCode = await exec(element); diff --git a/build/tasks/verify/verify-js.mts b/build/tasks/verify/verify-js.mts index 9633d3d8d..3ec594207 100644 --- a/build/tasks/verify/verify-js.mts +++ b/build/tasks/verify/verify-js.mts @@ -5,7 +5,7 @@ * @module {type ES6Module} build/tasks/verify/verify-js */ -import { exec, glob } from '@openinf/portal/build/utils'; +import { exec, glob, quote } from '@openinf/portal/build/utils'; const jsFiles = await glob([ '**/*.js', @@ -16,7 +16,7 @@ const jsFiles = await glob([ ]); let exitCode = 0; -const scripts = [`biome check ${jsFiles.join(' ')}`]; +const scripts = [`biome check ${quote(jsFiles)}`]; for (const element of scripts) { exitCode = await exec(element); diff --git a/build/tasks/verify/verify-json.mts b/build/tasks/verify/verify-json.mts index 87133028d..dffb0f415 100644 --- a/build/tasks/verify/verify-json.mts +++ b/build/tasks/verify/verify-json.mts @@ -5,18 +5,22 @@ * @module {type ES6Module} build/tasks/verify/verify-json */ -import { exec, glob } from '@openinf/portal/build/utils'; +import { exec, glob, quote } from '@openinf/portal/build/utils'; -const jsonFiles = await glob([ - '**/*.json', - '**/*.json5', - '**/*.jsonc', - '!_site/', - '!node_modules/', -]); +const EXCLUDED = ['!_site/', '!node_modules/']; + +const jsonFiles = await glob(['**/*.json', '**/*.jsonc', ...EXCLUDED]); +// Biome has no JSON5 parser. Handed one it reports the path as ignored and +// carries on with the rest, so listing `**/*.json5` beside the others read as +// coverage while being none: nothing looked at `.renovaterc.json5` at all. +// Prettier does have the parser. +const json5Files = await glob(['**/*.json5', ...EXCLUDED]); let exitCode = 0; -const scripts = [`biome check ${jsonFiles.join(' ')}`]; +const scripts = [ + `biome check ${quote(jsonFiles)}`, + ...(json5Files.length > 0 ? [`prettier --check ${quote(json5Files)}`] : []), +]; for (const element of scripts) { exitCode = await exec(element); diff --git a/build/tasks/verify/verify-liquid.mts b/build/tasks/verify/verify-liquid.mts index d82369d85..cc67eeb23 100644 --- a/build/tasks/verify/verify-liquid.mts +++ b/build/tasks/verify/verify-liquid.mts @@ -5,7 +5,7 @@ * @module {type ES6Module} build/tasks/verify/verify-liquid */ -import { exec, glob } from '@openinf/portal/build/utils'; +import { exec, glob, quote } from '@openinf/portal/build/utils'; const liquidFiles = await glob([ '**/*.html', @@ -15,7 +15,7 @@ const liquidFiles = await glob([ ]); let exitCode = 0; -const scripts = [`prettier --check ${liquidFiles.join(' ')}`]; +const scripts = [`prettier --check ${quote(liquidFiles)}`]; for (const element of scripts) { exitCode = await exec(element); diff --git a/build/tasks/verify/verify-md.mts b/build/tasks/verify/verify-md.mts index 8c6d63378..5dd673a15 100644 --- a/build/tasks/verify/verify-md.mts +++ b/build/tasks/verify/verify-md.mts @@ -5,7 +5,7 @@ * @module {type ES6Module} build/tasks/verify/verify-md */ -import { exec, glob } from '@openinf/portal/build/utils'; +import { exec, glob, quote } from '@openinf/portal/build/utils'; const markdownFiles = await glob([ '**/*.md', @@ -24,9 +24,9 @@ const markdownFiles = await glob([ let exitCode = 0; const scripts = [ - `prettier --check ${markdownFiles.join(' ')}`, - `markdownlint-cli2 ${markdownFiles.join(' ')}`, - `remark -f --silently-ignore ${markdownFiles.join(' ')}`, + `prettier --check ${quote(markdownFiles)}`, + `markdownlint-cli2 ${quote(markdownFiles)}`, + `remark -f --silently-ignore ${quote(markdownFiles)}`, ]; for (const element of scripts) { diff --git a/build/tasks/verify/verify-scss.mts b/build/tasks/verify/verify-scss.mts index c17e21e56..afabbf023 100644 --- a/build/tasks/verify/verify-scss.mts +++ b/build/tasks/verify/verify-scss.mts @@ -5,14 +5,14 @@ * @module {type ES6Module} build/tasks/verify/verify-scss */ -import { exec, glob } from '@openinf/portal/build/utils'; +import { exec, glob, quote } from '@openinf/portal/build/utils'; const scssFiles = await glob(['**/*.scss', '!_site/', '!node_modules/']); let exitCode = 0; const scripts = [ - `prettier --check ${scssFiles.join(' ')}`, - `stylelint ${scssFiles.join(' ')}`, + `prettier --check ${quote(scssFiles)}`, + `stylelint ${quote(scssFiles)}`, ]; for (const element of scripts) { diff --git a/build/tasks/verify/verify-spelling.mts b/build/tasks/verify/verify-spelling.mts index 05b5c7462..7ef3ddbd9 100644 --- a/build/tasks/verify/verify-spelling.mts +++ b/build/tasks/verify/verify-spelling.mts @@ -5,7 +5,7 @@ * @module {type ES6Module} build/tasks/verify/verify-spelling */ -import { exec, glob } from '@openinf/portal/build/utils'; +import { exec, glob, quote } from '@openinf/portal/build/utils'; // Comments and template prose are read as often as the documentation is, and // cspell's `en` dictionary is the American one, so this is also what holds the @@ -30,4 +30,4 @@ const files = await glob([ '!collections/_pages/vision.md', ]); -process.exitCode = await exec(`cspell lint ${files.join(' ')}`); +process.exitCode = await exec(`cspell lint ${quote(files)}`); diff --git a/build/tasks/verify/verify-svg.mts b/build/tasks/verify/verify-svg.mts index 3fe9c6fdb..e89c31763 100644 --- a/build/tasks/verify/verify-svg.mts +++ b/build/tasks/verify/verify-svg.mts @@ -5,15 +5,15 @@ * @module {type ES6Module} build/tasks/verify/verify-svg */ -import { exec, glob } from '@openinf/portal/build/utils'; +import { exec, glob, quote } from '@openinf/portal/build/utils'; import vnu from 'vnu-jar'; const svgFiles = await glob(['**/*.svg', '!_site/', '!node_modules/']); let exitCode = 0; const scripts = [ - `prettier --check ${svgFiles.join(' ')}`, - `java -jar ${vnu} --svg ${svgFiles.join(' ')}`, + `prettier --check ${quote(svgFiles)}`, + `java -jar ${quote(vnu)} --svg ${quote(svgFiles)}`, ]; for (const element of scripts) { diff --git a/build/tasks/verify/verify-toml.mts b/build/tasks/verify/verify-toml.mts index 85f2e5e68..051625019 100644 --- a/build/tasks/verify/verify-toml.mts +++ b/build/tasks/verify/verify-toml.mts @@ -5,12 +5,12 @@ * @module {type ES6Module} build/tasks/verify/verify-toml */ -import { exec, glob } from '@openinf/portal/build/utils'; +import { exec, glob, quote } from '@openinf/portal/build/utils'; const tomlFiles = await glob(['**/*.toml', '!_site/', '!node_modules/']); let exitCode = 0; -const scripts = [`dprint check ${tomlFiles.join(' ')}`]; +const scripts = [`dprint check ${quote(tomlFiles)}`]; for (const element of scripts) { exitCode = await exec(element); diff --git a/build/tasks/verify/verify-ts.mts b/build/tasks/verify/verify-ts.mts index f34e09559..81d83e6b7 100644 --- a/build/tasks/verify/verify-ts.mts +++ b/build/tasks/verify/verify-ts.mts @@ -5,7 +5,7 @@ * @module {type ES6Module} build/tasks/verify/verify-ts */ -import { exec, glob } from '@openinf/portal/build/utils'; +import { exec, glob, quote } from '@openinf/portal/build/utils'; const tsFiles = await glob([ '**/*.ts', @@ -18,7 +18,7 @@ let exitCode = 0; // tsc reads its file list from tsconfig.json rather than taking one, and // `erasableSyntaxOnly` there is what stops syntax node refuses to strip from // reaching a task script. -const scripts = [`biome check ${tsFiles.join(' ')}`, 'tsc --noEmit']; +const scripts = [`biome check ${quote(tsFiles)}`, 'tsc --noEmit']; for (const element of scripts) { exitCode = await exec(element); diff --git a/build/tasks/verify/verify-unit.mts b/build/tasks/verify/verify-unit.mts index 79352fab0..a635486dd 100644 --- a/build/tasks/verify/verify-unit.mts +++ b/build/tasks/verify/verify-unit.mts @@ -5,7 +5,7 @@ * @module {type ES6Module} build/tasks/verify/verify-unit */ -import { exec, glob } from '@openinf/portal/build/utils'; +import { exec, glob, quote } from '@openinf/portal/build/utils'; const testFiles = await glob(['**/*.test.mts', '!_site/', '!node_modules/']); @@ -18,7 +18,7 @@ if (testFiles.length === 0) { process.exitCode = 1; } else { let exitCode = 0; - const scripts = [`node --test ${testFiles.join(' ')}`]; + const scripts = [`node --test ${quote(testFiles)}`]; for (const element of scripts) { exitCode = await exec(element); diff --git a/build/tasks/verify/verify-yaml.mts b/build/tasks/verify/verify-yaml.mts index 8c29be6b3..83662e5ce 100644 --- a/build/tasks/verify/verify-yaml.mts +++ b/build/tasks/verify/verify-yaml.mts @@ -5,7 +5,7 @@ * @module {type ES6Module} build/tasks/verify/verify-yaml */ -import { exec, glob } from '@openinf/portal/build/utils'; +import { exec, glob, quote } from '@openinf/portal/build/utils'; const yamlFiles = await glob([ '**/*.yml', @@ -15,7 +15,7 @@ const yamlFiles = await glob([ ]); let exitCode = 0; -const scripts = [`prettier --check ${yamlFiles.join(' ')}`]; +const scripts = [`prettier --check ${quote(yamlFiles)}`]; for (const element of scripts) { exitCode = await exec(element); diff --git a/build/utils.mts b/build/utils.mts index d8dc117ce..c8eb16233 100644 --- a/build/utils.mts +++ b/build/utils.mts @@ -20,6 +20,37 @@ import { execute } from '@yarnpkg/shell'; export const exec = catchWrap(execute, 99); +/** + * Quotes paths for the shell `exec` runs them through. Every task builds its + * command as one string, so a path is shell text by the time the tool sees it + * -- a space in a filename splits one argument into two, and a `$(...)` or a + * `;` in one is a command of somebody else's choosing running in CI. Names + * like that are what `verify.filenames` exists to catch, but it cannot be the + * guard here: it is one task among the rest, and a failing one does not stop + * the others from being handed what it just objected to. + * + * Single quotes, because inside them a shell expands nothing at all. The one + * character they cannot hold is a single quote, which is why an embedded one + * closes the run, escapes itself, and opens the next. + * @param {string | string[]} paths The paths to pass to a command. + * @returns {string} Them, quoted and joined by spaces, ready to interpolate. + */ +export const quote = (paths: string | string[]) => + [paths] + .flat() + .map((path) => { + // Quoting settles what the shell does with a name and nothing about + // what the tool then makes of it: `'--write.md'` arrives at prettier as + // `--write.md`, which it reads as an option. It answered that one by + // printing an error and exiting 0 -- a check that passed having checked + // nothing. A leading `./` says the argument is a path and costs a + // relative name two characters. + const safe = path.startsWith('-') ? `./${path}` : path; + + return `'${safe.replaceAll("'", "'\\''")}'`; + }) + .join(' '); + /** * Expands a trailing-slash directory pattern (e.g. `_site/`) to cover * everything beneath it. On its own, a trailing slash matches just the one diff --git a/build/utils.test.mts b/build/utils.test.mts index d3ad601d2..83e90b014 100644 --- a/build/utils.test.mts +++ b/build/utils.test.mts @@ -6,11 +6,11 @@ */ import { deepStrictEqual, ok } from 'node:assert/strict'; -import { mkdir, mkdtemp, writeFile } from 'node:fs/promises'; +import { mkdir, mkdtemp, readFile, writeFile } from 'node:fs/promises'; import { tmpdir } from 'node:os'; import { dirname, join as pathJoin } from 'node:path'; import { after, before, describe, test } from 'node:test'; -import { glob } from '@openinf/portal/build/utils'; +import { exec, glob, quote } from '@openinf/portal/build/utils'; // Every pattern a build task writes is relative to the directory the task // runs in, so the fixture has to become that directory. @@ -113,3 +113,68 @@ describe('glob', () => { ok(!(await glob(['**/*.md', '!skipped/'])).includes('skipped/.g.md')); }); }); + +describe('quote', () => { + test('keeps a path with a space as one argument', () => { + deepStrictEqual( + quote(['sub/a b.md', 'sub/c.md']), + "'sub/a b.md' 'sub/c.md'" + ); + }); + + test('takes a lone path as well as a list', () => { + deepStrictEqual(quote('a.md'), "'a.md'"); + }); + + test('gives the shell nothing to expand', () => { + // Inside single quotes a shell expands nothing, so each of these reaches + // the tool as the filename it is rather than as syntax. + for (const name of ['$(id).md', '`id`.md', ';id;.md', 'a|b.md', 'a&b.md']) { + deepStrictEqual(quote(name), `'${name}'`); + } + }); + + test('escapes a single quote by closing and reopening the run', () => { + // The one character single quotes cannot hold. `'\''` is a closing + // quote, an escaped quote, and an opening quote. + deepStrictEqual(quote("it's.md"), "'it'\\''s.md'"); + }); + + test('keeps a name that looks like an option from being read as one', () => { + // Quoting alone leaves `--write.md` arriving at the tool as `--write.md`. + deepStrictEqual(quote('--write.md'), "'./--write.md'"); + deepStrictEqual(quote('-'), "'./-'"); + }); + + test('leaves an ordinary path alone', () => { + deepStrictEqual(quote('doc/adr/0001-a.md'), "'doc/adr/0001-a.md'"); + deepStrictEqual(quote('/abs/path.jar'), "'/abs/path.jar'"); + }); + + test('survives a round trip through the shell it is written for', async () => { + // The escaping is only worth anything if the shell `exec` uses agrees + // with it, so this asks that shell rather than a model of it. + const home = await mkdtemp(pathJoin(tmpdir(), 'openinf-quote-')); + const names = ['a b.md', "it's.md", '$(id).md', ';id;.md']; + const here = process.cwd(); + + for (const name of names) await writeFile(pathJoin(home, name), ''); + + process.chdir(home); + + try { + // `ls -1` prints one name per line, redirected by the same shell that + // parsed the quoting, so what lands in the file is exactly what the + // command received: one argument each, and nothing expanded. + deepStrictEqual(await exec(`ls -1 ${quote(names)} > out.txt`), 0); + + const seen = (await readFile(pathJoin(home, 'out.txt'), 'utf8')) + .split('\n') + .filter(Boolean); + + deepStrictEqual(seen.sort(), [...names].sort()); + } finally { + process.chdir(here); + } + }); +}); diff --git a/package.json b/package.json index 47cd40de3..58f9d8683 100644 --- a/package.json +++ b/package.json @@ -40,7 +40,7 @@ "cspell": "10.1.0", "cssnano": "8.0.8", "dprint": "0.56.1", - "editorconfig-checker": "6.1.1", + "editorconfig-checker": "7.0.0", "html-minifier-terser": "7.2.0", "htmlparser2": "12.0.0", "js-yaml": "5.3.0", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 2e0f3438c..c9e1d94fe 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -57,8 +57,8 @@ importers: specifier: 0.56.1 version: 0.56.1 editorconfig-checker: - specifier: 6.1.1 - version: 6.1.1 + specifier: 7.0.0 + version: 7.0.0 html-minifier-terser: specifier: 7.2.0 version: 7.2.0 @@ -1635,9 +1635,9 @@ packages: eastasianwidth@0.2.0: resolution: {integrity: sha512-I88TYZWc9XiYHRQ4/3c5rjjfgkjhLyW2luGIheGERbNQ6OY7yTybanSpDXZa8y7VUP9YmDcYa+eyq4ca7iLqWA==} - editorconfig-checker@6.1.1: - resolution: {integrity: sha512-kiOb6qaWpMNt7Z/43ba0Pa1Inhr2/t9nKbvEKtCeXJ5AesztoM9AgLOOQVB4QUv/nGjgz3xkbx4pcogVRD2NWw==} - engines: {node: '>=20.11.0'} + editorconfig-checker@7.0.0: + resolution: {integrity: sha512-bqTCyzjz6Qbah4hBO032mQiwGstrHL/jwsnlFvlqrDcpv8rv2pSgLo00Fnji1u6PGtSdY6uydzVLUdHupbpNBQ==} + engines: {node: '>=24.14.0'} hasBin: true ee-first@1.1.1: @@ -5221,7 +5221,7 @@ snapshots: eastasianwidth@0.2.0: {} - editorconfig-checker@6.1.1: {} + editorconfig-checker@7.0.0: {} ee-first@1.1.1: {}