diff --git a/.editorconfig b/.editorconfig new file mode 100644 index 000000000..a6e3fc6a1 --- /dev/null +++ b/.editorconfig @@ -0,0 +1,10 @@ +root = true + +[*] +end_of_line = lf +charset = utf-8 +insert_final_newline = true +trim_trailing_whitespace = true + +[*.md] +trim_trailing_whitespace = false diff --git a/.gitattributes b/.gitattributes new file mode 100644 index 000000000..56654af1f --- /dev/null +++ b/.gitattributes @@ -0,0 +1,12 @@ +# Normalize line endings to LF on checkin; auto-detect binary (skipped). +* text=auto eol=lf +# Common binary types — never normalize. +*.png binary +*.jpg binary +*.jpeg binary +*.gif binary +*.ico binary +*.woff binary +*.woff2 binary +*.ttf binary +*.pdf binary diff --git a/CONTEXT.md b/CONTEXT.md index 59e116107..4c09f79c6 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -611,6 +611,12 @@ The canonical lint infrastructure adopted in ADR 452 (`docs/adr/452-eslint-lint- `DEFECT.TEST-SHELL-PIPELINE-NONPORTABLE.detect=test does readFileSync(md).match for a bash fence with literal \n, OR execFileSync('bash',...) gated only on a bash-presence probe; also verifying a new test with a file-scoped run instead of the full suite hides repo-wide static guards` `DEFECT.TEST-SHELL-PIPELINE-NONPORTABLE.fix-forward=match the fence with \r?\n and normalize the captured block to LF; gate pipeline execution on process.platform !== 'win32' && hasBash since the extraction LOGIC is platform-independent and POSIX coverage suffices; run the full suite (or the parity/lint guards) before push when adding a test file` +`DEFECT.WINDOWS-TEST-PORTABILITY.symptom=local gsd-test runs Mac+Linux only (no Windows host); Windows-only test failures (chmod exec-bit not honored for PATH-executing extension-less scripts in Git Bash msys2; / vs \ path-separator in assertions; Git Bash msys2 shell semantics) surface ONLY in CI test (windows-latest,*) / full test (windows-latest,*) lanes, never locally` +`DEFECT.WINDOWS-TEST-PORTABILITY.examples=PR #1084 (chmod 0o755 + bare-command execution failed on windows lane); test files that assert path.join result without normalizing to forward slashes` +`DEFECT.WINDOWS-TEST-PORTABILITY.detect=npm run lint:windows-test-portability (tripwire: flags tests combining chmod exec-bit with sh/bash -c and no platform guard); watch CI windows matrix green before declaring a PR done` +`DEFECT.WINDOWS-TEST-PORTABILITY.fix-forward=gate platform-specific execution with if (process.platform !== 'win32'); normalize path expectations to forward slashes with .replace(/\\/g, '/'); invoke scripts via explicit interpreter (sh ) rather than relying on exec-bit; annotate // windows-portability-ok: when a bypass is intentional` +`DEFECT.WINDOWS-TEST-PORTABILITY.prevention=run lint:ci before opening a PR; treat the CI windows lane as the only true Windows signal — gsd-test (Mac/Linux only) cannot substitute for it` + --- diff --git a/eslint.config.mjs b/eslint.config.mjs index dffad429b..65882ac86 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -196,6 +196,7 @@ export default tseslint.config( 'no-unsafe-finally': 'warn', // eslint-plugin-n rules 'n/no-process-exit': 'error', + 'n/no-path-concat': 'error', // Local rules — warn for now; flip to error after cleanup phases 'local/no-source-grep': 'warn', }, diff --git a/package.json b/package.json index 93e1bd2fb..96a5245f4 100644 --- a/package.json +++ b/package.json @@ -92,7 +92,8 @@ "pretest:coverage": "npm run build:lib && npm run lint:skill-deps", "lint": "eslint . --cache --cache-location node_modules/.cache/eslint/", "lint:fix": "eslint . --fix", - "lint:ci": "npm run lint && npm run lint:skill-deps && node scripts/lint-test-file-count.cjs && node scripts/lint-command-contract.cjs && node scripts/lint-pr-check-project-dir.cjs && npm run lint:legacy-name && node scripts/lint-regression-test-names.cjs", + "lint:ci": "npm run lint && npm run lint:skill-deps && node scripts/lint-test-file-count.cjs && node scripts/lint-command-contract.cjs && node scripts/lint-pr-check-project-dir.cjs && npm run lint:legacy-name && node scripts/lint-regression-test-names.cjs && node scripts/lint-windows-test-portability.cjs", + "lint:windows-test-portability": "node scripts/lint-windows-test-portability.cjs", "lint:regression-names": "node scripts/lint-regression-test-names.cjs", "lint:descriptions": "node scripts/lint-descriptions.cjs", "lint:skill-deps": "node scripts/lint-skill-deps.cjs", diff --git a/scripts/lint-windows-test-portability.cjs b/scripts/lint-windows-test-portability.cjs new file mode 100644 index 000000000..6994a447c --- /dev/null +++ b/scripts/lint-windows-test-portability.cjs @@ -0,0 +1,178 @@ +'use strict'; + +/** + * lint-windows-test-portability.cjs — flag tests that combine chmod exec-bit + * with bare sh/bash -c without a platform guard. + * + * ## Why + * + * Windows Git Bash (msys2) does not honour Node's chmod exec bit for + * PATH-executing extension-less scripts. A test that (a) makes a fixture + * executable via chmodSync and (b) runs it with `sh -c`/`bash -c` will pass + * on Mac/Linux but fail only in the CI `test (windows-latest, *)` / + * `full test (windows-latest, *)` lanes, producing a hard-to-diagnose + * false-negative gate. See CONTEXT.md → DEFECT.WINDOWS-TEST-PORTABILITY. + * + * ## What this enforces + * + * For every file in tests/**\/*.test.cjs (recursive, excluding node_modules): + * - makesExecutable: contains chmodSync?( with an exec-bit octal literal + * - shellDashC: contains a sh/bash -c invocation (array form or string literal) + * - guarded: contains a process.platform / os.platform() / win32 / isWindows guard + * - optOut: contains the literal `windows-portability-ok` + * VIOLATION = makesExecutable && shellDashC && !guarded && !optOut + * + * ## Remediation + * + * Gate the bare-command execution with `if (process.platform !== 'win32')`, + * or invoke via an explicit interpreter (`sh `), or annotate + * `// windows-portability-ok: `. + * + * ## Export contract (for unit tests) + * + * When required as a module (`require.main !== module`) this file exports + * `scanContent(source)` → { makesExecutable, shellDashC, guarded, optOut, violation }. + */ + +const fs = require('fs'); +const path = require('path'); + +// ─── Detection regexes ────────────────────────────────────────────────────── + +/** + * Match chmod/chmodSync( calls with an octal mode literal whose exec bits are + * set, e.g. `fs.chmodSync(p, 0o755)` or `chmod(file, 0o111)`. + */ +const CHMOD_RE = /chmod(?:Sync)?\s*\([^,;]+,\s*0o([0-7]{3})\b/g; + +/** + * Array form: execFileSync/spawnSync/exec* with 'sh' or 'bash' (optionally + * prefixed) as the first arg and '-c' as an element of the args array. + * e.g. execFileSync('bash', ['-c', ...]) or spawnSync('/bin/sh', ['-c', ...]) + */ +const SHELL_ARRAY_RE = + /(?:execFile(?:Sync)?|spawnSync|spawn|exec)\s*\(\s*['"`](?:\/(?:usr\/)?bin\/)?(?:bash|sh)['"`]\s*,\s*\[[^\]]*['"]-c['"]/; + +/** + * String-literal form: any string containing `bash -c` or `sh -c`. + */ +const SHELL_STRING_RE = /['"`][^'"`\n]*(?:bash|sh)\s+-c[^'"`\n]*['"`]/; + +/** Platform guard presence. */ +const GUARD_RE = /process\.platform|os\.platform\s*\(|\bwin32\b|\bisWindows\b/; + +/** Opt-out annotation. */ +const OPT_OUT_RE = /windows-portability-ok/; + +// ─── Pure scanning function (exported for unit tests) ──────────────────────── + +/** + * Scan a single file's source text and return detection flags. + * + * @param {string} source - The file contents as a string. + * @returns {{ makesExecutable: boolean, shellDashC: boolean, guarded: boolean, optOut: boolean, violation: boolean }} + */ +function scanContent(source) { + // Reset stateful regex before use. + CHMOD_RE.lastIndex = 0; + + let makesExecutable = false; + let match; + while ((match = CHMOD_RE.exec(source)) !== null) { + const oct = match[1]; + if ((parseInt(oct, 8) & 0o111) !== 0) { + makesExecutable = true; + break; + } + } + + const shellDashC = SHELL_ARRAY_RE.test(source) || SHELL_STRING_RE.test(source); + const guarded = GUARD_RE.test(source); + const optOut = OPT_OUT_RE.test(source); + const violation = makesExecutable && shellDashC && !guarded && !optOut; + + return { makesExecutable, shellDashC, guarded, optOut, violation }; +} + +// ─── Filesystem walker ─────────────────────────────────────────────────────── + +/** + * Recursively collect all *.test.cjs files under `dir`, excluding node_modules. + * + * @param {string} dir + * @param {string[]} [acc] + * @returns {string[]} + */ +function collectTestFiles(dir, acc) { + acc = acc || []; + let entries; + try { + entries = fs.readdirSync(dir, { withFileTypes: true }); + } catch { + return acc; + } + for (const entry of entries) { + if (entry.name === 'node_modules') continue; + const full = path.join(dir, entry.name); + if (entry.isDirectory()) { + collectTestFiles(full, acc); + } else if (entry.isFile() && entry.name.endsWith('.test.cjs')) { + acc.push(full); + } + } + return acc; +} + +// ─── Main ──────────────────────────────────────────────────────────────────── + +function main() { + const ROOT = path.join(__dirname, '..'); + const TESTS_DIR = path.join(ROOT, 'tests'); + + const files = collectTestFiles(TESTS_DIR); + const violations = []; + + for (const file of files) { + let source; + try { + source = fs.readFileSync(file, 'utf8'); + } catch { + continue; + } + const { violation } = scanContent(source); + if (violation) { + const rel = path.relative(ROOT, file).replace(/\\/g, '/'); + violations.push(rel); + } + } + + if (violations.length > 0) { + for (const rel of violations) { + process.stderr.write( + `${rel}: chmod-executable + sh/bash -c with no platform guard\n`, + ); + } + process.stderr.write( + '\nWindows Git Bash does not honor Node\'s chmod exec bit for ' + + 'PATH-executing extension-less scripts ' + + '(CONTEXT.md → DEFECT.WINDOWS-TEST-PORTABILITY). ' + + 'Gate the bare-command execution with ' + + '`if (process.platform !== \'win32\')`, or invoke via an explicit ' + + 'interpreter (`sh `), or annotate ' + + '`// windows-portability-ok: `.\n', + ); + process.exitCode = 1; + } else { + console.log( + `ok lint-windows-test-portability: ${files.length} file(s) scanned, no violations`, + ); + } +} + +// ─── Module boundary ───────────────────────────────────────────────────────── + +if (require.main === module) { + main(); +} else { + module.exports = { scanContent }; +} diff --git a/tests/lint-windows-test-portability.test.cjs b/tests/lint-windows-test-portability.test.cjs new file mode 100644 index 000000000..e3d925a6c --- /dev/null +++ b/tests/lint-windows-test-portability.test.cjs @@ -0,0 +1,136 @@ +// windows-portability-ok: fixture strings for the lint's own unit test, not real execution +'use strict'; + +/** + * Tests for scripts/lint-windows-test-portability.cjs + * + * Uses the exported `scanContent` pure function to avoid spawning real + * subprocesses or touching the filesystem. This keeps the test portable and + * prevents the lint from flagging itself (the opt-out comment above covers the + * chmod/bash-c fixture strings below). + */ + +const { describe, test } = require('node:test'); +const assert = require('node:assert/strict'); +const { scanContent } = require('../scripts/lint-windows-test-portability.cjs'); + +describe('lint-windows-test-portability: scanContent', () => { + test('(a) chmod 0o755 + bash -c with no guard => violation', () => { + const src = ` + 'use strict'; + fs.chmodSync(fixture, 0o755); + execFileSync('bash', ['-c', 'echo hi']); + `; + const result = scanContent(src); + assert.strictEqual(result.makesExecutable, true, 'makesExecutable'); + assert.strictEqual(result.shellDashC, true, 'shellDashC'); + assert.strictEqual(result.guarded, false, 'guarded'); + assert.strictEqual(result.optOut, false, 'optOut'); + assert.strictEqual(result.violation, true, 'violation'); + }); + + test('(b) chmod 0o755 + bash -c + process.platform guard => no violation', () => { + const src = ` + 'use strict'; + fs.chmodSync(fixture, 0o755); + execFileSync('bash', ['-c', 'echo hi']); + if (process.platform !== 'win32') { runIt(); } + `; + const result = scanContent(src); + assert.strictEqual(result.makesExecutable, true, 'makesExecutable'); + assert.strictEqual(result.shellDashC, true, 'shellDashC'); + assert.strictEqual(result.guarded, true, 'guarded'); + assert.strictEqual(result.violation, false, 'violation'); + }); + + test('(c) chmod 0o644 (no exec bit) + bash -c => no violation', () => { + const src = ` + 'use strict'; + fs.chmodSync(fixture, 0o644); + execFileSync('bash', ['-c', 'cat file']); + `; + const result = scanContent(src); + assert.strictEqual(result.makesExecutable, false, 'makesExecutable'); + assert.strictEqual(result.shellDashC, true, 'shellDashC'); + assert.strictEqual(result.violation, false, 'violation'); + }); + + test('(d) chmod 0o755 + execFileSync(sh, [path]) with no -c => no violation', () => { + const src = ` + 'use strict'; + fs.chmodSync(fixture, 0o755); + execFileSync('sh', [fixturePath]); + `; + const result = scanContent(src); + assert.strictEqual(result.makesExecutable, true, 'makesExecutable'); + assert.strictEqual(result.shellDashC, false, 'shellDashC'); + assert.strictEqual(result.violation, false, 'violation'); + }); + + test('(e) violation pattern + windows-portability-ok opt-out => no violation', () => { + const src = ` + // windows-portability-ok: intentional cross-platform test + 'use strict'; + fs.chmodSync(fixture, 0o755); + execFileSync('bash', ['-c', 'run']); + `; + const result = scanContent(src); + assert.strictEqual(result.makesExecutable, true, 'makesExecutable'); + assert.strictEqual(result.shellDashC, true, 'shellDashC'); + assert.strictEqual(result.optOut, true, 'optOut'); + assert.strictEqual(result.violation, false, 'violation'); + }); + + test('chmod 0o111 (pure exec bits) is detected as executable', () => { + const src = `fs.chmodSync(f, 0o111); spawnSync('sh', ['-c', 'x']);`; + const result = scanContent(src); + assert.strictEqual(result.makesExecutable, true, 'makesExecutable'); + assert.strictEqual(result.shellDashC, true, 'shellDashC'); + assert.strictEqual(result.violation, true, 'violation'); + }); + + test('chmod 0o444 (read-only) is not executable', () => { + const src = `fs.chmodSync(f, 0o444); execFileSync('bash', ['-c', 'x']);`; + const result = scanContent(src); + assert.strictEqual(result.makesExecutable, false, 'makesExecutable'); + assert.strictEqual(result.violation, false, 'violation'); + }); + + test('string-literal sh -c form is detected', () => { + const src = ` + fs.chmodSync(f, 0o755); + exec('sh -c "run.sh"'); + `; + const result = scanContent(src); + assert.strictEqual(result.shellDashC, true, 'shellDashC from string literal'); + assert.strictEqual(result.violation, true, 'violation'); + }); + + test('/bin/bash prefix in array form is detected', () => { + const src = ` + fs.chmodSync(f, 0o755); + execFileSync('/bin/bash', ['-c', 'run']); + `; + const result = scanContent(src); + assert.strictEqual(result.shellDashC, true, 'shellDashC with /bin/bash prefix'); + assert.strictEqual(result.violation, true, 'violation'); + }); + + test('isWindows guard suppresses violation', () => { + const src = ` + const isWindows = process.platform === 'win32'; + fs.chmodSync(f, 0o755); + execFileSync('bash', ['-c', 'run']); + `; + const result = scanContent(src); + assert.strictEqual(result.guarded, true, 'guarded via isWindows'); + assert.strictEqual(result.violation, false, 'violation'); + }); + + test('no chmod at all => no violation regardless of shell -c', () => { + const src = `execFileSync('bash', ['-c', 'echo hi']);`; + const result = scanContent(src); + assert.strictEqual(result.makesExecutable, false, 'makesExecutable'); + assert.strictEqual(result.violation, false, 'violation'); + }); +});