feat(#1733): normalize-path-in-content production AST rule + fix Windows agent-skills content leak (Phase 5) (#1736)
* feat(#1733): normalize-path-in-content production AST rule (Phase 5) ADR-1703 Phase 5 — the first production-code rule. local/normalize-path-in-content (src/**/*.cts, @typescript-eslint/parser): flags a path-returning fn result (path.basename excluded — returns a separator-less filename) interpolated into an @-reference / config-dir markdown body without .replace(/\\/g,'/') normalization, per RULESET.CONTENT-PATH-NORMALIZATION / DEFECT.WINDOWS-PATH-LEAK-IN-MARKDOWN-CONTENT. Build-and-assess found the canonical defect site (computePathPrefix) already compliant and only 1 src/ hit — a false positive (path.basename in a status message) — eliminated by narrowing (exclude basename; require a real @-ref/ config-dir marker, not bare .md). 0 src/ violations: clean forward-prevention. The out-of-band disable-ban now scans src/**/*.cts too (typescript-estree) so the production rule also cannot be eslint-disabled. Registered (error) + PROTECTED_RULES; CONTEXT.md predicates + how-to doc updated. - RuleTester suite (26 cases) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(#1733): add changeset for Windows agent-skills path-leak fix Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix: harden mutation-matrix.cjs stdin read against EAGAIN on non-blocking pipe scripts/mutation-matrix.cjs read piped stdin via readFileSync(process.stdin.fd). On macOS libuv marks the stdin pipe fd non-blocking, so a synchronous read can throw EAGAIN before the writer fills the pipe — intermittently, under heavy CI shard load — aborting the script (status 2) and flaking mutation-matrix-ratchet. Replace with readStdinSync(): an fs.readSync loop that retries on EAGAIN (1ms synchronous Atomics.wait yield), stops on 0-byte/EOF, and rethrows other errors. Deterministic regression test injects EAGAIN via an fs.readSync monkeypatch. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * ci: re-run golden-install-parity on src/lib + installer changes (close drift guard) golden-install-parity hashes every installed bin/lib/*.cjs per runtime, so it must re-run whenever the built lib could change. ci-test-scope selected it for neither src/** nor installer changes, so a source-only edit (e.g. #1691's milestone.cts/roadmap.cts) recompiled bin/lib and silently drifted the golden fixtures past the scoped lane. Add golden-install-parity.test.cjs to both the 'TS runtime sources' and 'installer and package layout' selection rules, with behavioral regression tests for each. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: review-bot <review-bot@gsd>
This commit is contained in:
Tom Boucher
@@ -547,6 +547,34 @@ describe('test-full shard matrix parity (#1212)', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('golden-install-parity selection (#1691 drift guard)', () => {
|
||||
// Regression: a src/*.cts-only edit recompiles bin/lib/*.cjs (changing installed
|
||||
// hashes), but the scoped CI lane was not re-running golden-install-parity —
|
||||
// causing golden fixtures to silently drift (#1691 milestone/roadmap cts change).
|
||||
// Both the 'TS runtime sources' and 'installer and package layout' rules must now
|
||||
// select tests/golden-install-parity.test.cjs.
|
||||
|
||||
test('src/*.cts change selects golden-install-parity (TS runtime sources rule)', () => {
|
||||
const result = scopeFor(['src/milestone.cts']);
|
||||
assert.strictEqual(result.code_changed, true,
|
||||
`expected code_changed=true for src/ change, got: ${JSON.stringify(result)}`);
|
||||
assert.ok(
|
||||
result.targeted_tests.includes('tests/golden-install-parity.test.cjs'),
|
||||
`expected golden-install-parity in targeted_tests for src/*.cts change, got: ${JSON.stringify(result.targeted_tests)}`,
|
||||
);
|
||||
});
|
||||
|
||||
test('bin/install.js change selects golden-install-parity (installer and package layout rule)', () => {
|
||||
const result = scopeFor(['bin/install.js']);
|
||||
assert.strictEqual(result.code_changed, true,
|
||||
`expected code_changed=true for bin/ change, got: ${JSON.stringify(result)}`);
|
||||
assert.ok(
|
||||
result.targeted_tests.includes('tests/golden-install-parity.test.cjs'),
|
||||
`expected golden-install-parity in targeted_tests for bin/install.js change, got: ${JSON.stringify(result.targeted_tests)}`,
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('code_changed=false implies clean output invariant', () => {
|
||||
// Fix 1: when code_changed is false, full_matrix, targeted_tests, windows_tests
|
||||
// must ALL be empty/false — even if a docs path coincidentally
|
||||
|
||||
93
tests/mutation-matrix-stdin-eagain.test.cjs
Normal file
93
tests/mutation-matrix-stdin-eagain.test.cjs
Normal file
@@ -0,0 +1,93 @@
|
||||
'use strict';
|
||||
|
||||
/**
|
||||
* tests/mutation-matrix-stdin-eagain.test.cjs
|
||||
*
|
||||
* Regression tests for the EAGAIN-resilient readStdinSync() helper added to
|
||||
* scripts/mutation-matrix.cjs (issue #1733).
|
||||
*
|
||||
* Background: On macOS, libuv sets a piped stdin fd to non-blocking mode.
|
||||
* Under heavy CI shard load a synchronous fs.readFileSync(process.stdin.fd)
|
||||
* can throw EAGAIN before the writer has filled the pipe, aborting the script
|
||||
* with status 2. readStdinSync() retries on EAGAIN; these tests verify that
|
||||
* contract deterministically by monkeypatching fs.readSync (never via chmod /
|
||||
* permission tricks — see cross-platform IO-failure-injection convention).
|
||||
*/
|
||||
|
||||
const { test, describe } = require('node:test');
|
||||
const assert = require('node:assert/strict');
|
||||
const fs = require('fs');
|
||||
const path = require('path');
|
||||
|
||||
const matrix = require(path.resolve(__dirname, '../scripts/mutation-matrix.cjs'));
|
||||
|
||||
describe('readStdinSync: EAGAIN resilience', () => {
|
||||
test('exports readStdinSync as a function', () => {
|
||||
assert.strictEqual(
|
||||
typeof matrix.readStdinSync,
|
||||
'function',
|
||||
'mutation-matrix.cjs must export readStdinSync'
|
||||
);
|
||||
});
|
||||
|
||||
test('retries on EAGAIN then returns the full payload', () => {
|
||||
// Arrange: stub fs.readSync driven by a closure counter.
|
||||
// Call 1 → throw EAGAIN (simulates non-blocking pipe not ready)
|
||||
// Call 2 → write payload into buffer, return byte length
|
||||
// Call 3+ → return 0 (clean EOF)
|
||||
const payload = 'src/core-utils.cts\nsrc/adr-parser.cts\n';
|
||||
const payloadBuf = Buffer.from(payload, 'utf8');
|
||||
let callCount = 0;
|
||||
|
||||
const origReadSync = fs.readSync;
|
||||
try {
|
||||
fs.readSync = (fd, buf, offset, _length, _position) => {
|
||||
callCount++;
|
||||
if (callCount === 1) {
|
||||
throw Object.assign(new Error('EAGAIN: resource temporarily unavailable'), { code: 'EAGAIN' });
|
||||
}
|
||||
if (callCount === 2) {
|
||||
payloadBuf.copy(buf, offset, 0, payloadBuf.length);
|
||||
return payloadBuf.length;
|
||||
}
|
||||
// Call 3+: EOF
|
||||
return 0;
|
||||
};
|
||||
|
||||
const result = matrix.readStdinSync();
|
||||
|
||||
assert.strictEqual(
|
||||
result,
|
||||
payload,
|
||||
'readStdinSync must return the full payload after retrying the EAGAIN'
|
||||
);
|
||||
assert.ok(
|
||||
callCount >= 3,
|
||||
`expected at least 3 fs.readSync calls (EAGAIN + data + EOF), got ${callCount}`
|
||||
);
|
||||
} finally {
|
||||
fs.readSync = origReadSync;
|
||||
}
|
||||
});
|
||||
|
||||
test('non-EAGAIN errors propagate (are not swallowed)', () => {
|
||||
// Arrange: stub fs.readSync to throw a non-retryable error.
|
||||
const origReadSync = fs.readSync;
|
||||
try {
|
||||
fs.readSync = () => {
|
||||
throw Object.assign(new Error('EACCES: permission denied'), { code: 'EACCES' });
|
||||
};
|
||||
|
||||
assert.throws(
|
||||
() => matrix.readStdinSync(),
|
||||
(err) => {
|
||||
assert.strictEqual(err.code, 'EACCES');
|
||||
return true;
|
||||
},
|
||||
'readStdinSync must rethrow non-EAGAIN errors'
|
||||
);
|
||||
} finally {
|
||||
fs.readSync = origReadSync;
|
||||
}
|
||||
});
|
||||
});
|
||||
541
tests/normalize-path-in-content.rule.test.cjs
Normal file
541
tests/normalize-path-in-content.rule.test.cjs
Normal file
@@ -0,0 +1,541 @@
|
||||
'use strict';
|
||||
|
||||
/**
|
||||
* normalize-path-in-content.rule.test.cjs
|
||||
*
|
||||
* RuleTester unit tests for the local/normalize-path-in-content ESLint rule.
|
||||
*
|
||||
* Rule: flag a path-returning fn result (PATH_RETURNING_FNS minus path.basename)
|
||||
* interpolated into a content template literal (`${ … }`) without POSIX
|
||||
* normalization. "Content" is identified by two shapes:
|
||||
*
|
||||
* Shape (a) — quasis contain an @-reference or home-dir prefix marker:
|
||||
* `@~/`, `@$`, `@/`, `$HOME`, `~/` in the template's static parts.
|
||||
* A bare `@` not followed by `~`, `$`, or `/` is NOT a marker.
|
||||
*
|
||||
* Shape (b) — per-expression quasi check: quasis[i+1].raw starts with `/`
|
||||
* and contains `.md` or `.json` at the end of a path component. Catches
|
||||
* `${computePathPrefix(t)}/commands/gsd/x.md` without config-dir markers.
|
||||
*
|
||||
* Config-dir substrings (`/.claude`, `/commands`, `/skills`, etc.) are NOT
|
||||
* content markers — removed to eliminate diagnostic/log FPs.
|
||||
*
|
||||
* DEFECT category: DEFECT.WINDOWS-PATH-LEAK-IN-MARKDOWN-CONTENT
|
||||
* RULESET: RULESET.CONTENT-PATH-NORMALIZATION
|
||||
*
|
||||
* INVALID (violation expected):
|
||||
* - path.join(home, '.claude') interpolated into a template containing @~/
|
||||
* - computePathPrefix(...) interpolated into a template with /commands/gsd/x.md
|
||||
* (shape b: quasi immediately after starts with /…\.md)
|
||||
* - getGlobalConfigDir() in @${fn()}/commands/gsd/help.md (shape b)
|
||||
*
|
||||
* VALID (no violation):
|
||||
* - path.basename(p) in any content template — basename cannot contain separators (N1).
|
||||
* - Path normalized via .replace(/\\/g, '/') before interpolation
|
||||
* - Path normalized via String(...).replace(...) before interpolation
|
||||
* - Path in console.log() — log message, not content
|
||||
* - Path in fs.writeFileSync() first arg — real FS path, not content
|
||||
* - Path in new Error() — diagnostic, not content
|
||||
* - throw Error(...) — bare Error() call is a diagnostic, not content (W2)
|
||||
* - Template literal with NO content markers — e.g. just a log string
|
||||
* - path.basename(outputPath) in a status message with bare .md prose — N2
|
||||
* - Template with a bare `@` that is not an @-reference (@-narrowing precision)
|
||||
* - path.resolve(configDir) in `${fn()}/.claude/x` — no .md/.json and no
|
||||
* @-ref markers → not flagged (narrowed from Tier 2 / config-dir markers)
|
||||
* - os.homedir() in `${fn()}/.claude/gsd-core/commands` — same: no .md/.json
|
||||
*/
|
||||
|
||||
const { test, describe } = require('node:test');
|
||||
const assert = require('node:assert/strict');
|
||||
const { RuleTester } = require('eslint');
|
||||
|
||||
const normalizePathInContent = require('../eslint-rules/normalize-path-in-content.cjs');
|
||||
|
||||
const ruleTester = new RuleTester({
|
||||
languageOptions: {
|
||||
ecmaVersion: 2022,
|
||||
sourceType: 'commonjs',
|
||||
},
|
||||
});
|
||||
|
||||
// ─── module shape ─────────────────────────────────────────────────────────────
|
||||
|
||||
describe('normalize-path-in-content rule module', () => {
|
||||
test('exports meta and create', () => {
|
||||
assert.strictEqual(typeof normalizePathInContent.meta, 'object');
|
||||
assert.strictEqual(typeof normalizePathInContent.create, 'function');
|
||||
assert.strictEqual(normalizePathInContent.meta.type, 'problem');
|
||||
assert.ok(normalizePathInContent.meta.messages.pathInContent);
|
||||
});
|
||||
});
|
||||
|
||||
// ─── INVALID cases (violation expected) ───────────────────────────────────────
|
||||
|
||||
describe('normalize-path-in-content invalid cases', () => {
|
||||
test('invalid: path.join(home, ".claude") in @~/ home-dir reference template', () => {
|
||||
ruleTester.run('normalize-path-in-content', normalizePathInContent, {
|
||||
valid: [],
|
||||
invalid: [
|
||||
{
|
||||
// Canonical defect shape: path.join result into a markdown @~/ reference
|
||||
// without normalization. On Windows, path.join emits backslashes.
|
||||
// The '@~/' quasi prefix is the genuine @-reference marker (not bare '@').
|
||||
code: "const ref = `@~/${path.join(home, '.claude')}/x.md`;",
|
||||
errors: [{ messageId: 'pathInContent' }],
|
||||
},
|
||||
],
|
||||
});
|
||||
});
|
||||
|
||||
test('invalid: computePathPrefix result in content template containing .md reference', () => {
|
||||
ruleTester.run('normalize-path-in-content', normalizePathInContent, {
|
||||
valid: [],
|
||||
invalid: [
|
||||
{
|
||||
// computePathPrefix is a PATH_RETURNING_FNS entry; its result used
|
||||
// directly in a template with a .md path reference is flagged.
|
||||
code: `const body = \`\${computePathPrefix(t)}/commands/gsd/x.md\`;`,
|
||||
errors: [{ messageId: 'pathInContent' }],
|
||||
},
|
||||
],
|
||||
});
|
||||
});
|
||||
|
||||
test('valid (narrowed): path.resolve(configDir) in template with /.claude/x — no .md/.json after expr and no @-ref marker → NOT flagged after Tier 2 marker removal', () => {
|
||||
// /.claude was a Tier 2 marker but was removed to eliminate diagnostic FPs.
|
||||
// Shape (b) requires the quasi immediately after the expression to start with
|
||||
// /…\.md or /…\.json — /.claude/x does NOT end in .md/.json so this is now VALID.
|
||||
ruleTester.run('normalize-path-in-content', normalizePathInContent, {
|
||||
valid: [
|
||||
{
|
||||
code: `const prefix = \`\${path.resolve(configDir)}/.claude/x\`;`,
|
||||
},
|
||||
],
|
||||
invalid: [],
|
||||
});
|
||||
});
|
||||
|
||||
test('invalid: getGlobalConfigDir("claude") in @${fn()}/commands/gsd/help.md — shape (b) fires: quasi after expr starts with /commands/gsd/help.md', () => {
|
||||
ruleTester.run('normalize-path-in-content', normalizePathInContent, {
|
||||
valid: [],
|
||||
invalid: [
|
||||
{
|
||||
// Shape (b): quasi[1] = '/commands/gsd/help.md' starts with '/' and ends with '.md'
|
||||
// → QUASI_MD_JSON_RE matches → path call is flagged regardless of config-dir markers.
|
||||
// Note: bare '@' in quasi[0] is NOT a content marker (only @~, @$, @/ are);
|
||||
// this case is detected by shape (b) alone.
|
||||
code: `const ref = \`@\${getGlobalConfigDir('claude')}/commands/gsd/help.md\`;`,
|
||||
errors: [{ messageId: 'pathInContent' }],
|
||||
},
|
||||
],
|
||||
});
|
||||
});
|
||||
|
||||
test('valid (narrowed): os.homedir() in template /.claude/gsd-core/commands — no .md/.json and no @-ref → NOT flagged after Tier 2 marker removal', () => {
|
||||
// /.claude was a Tier 2 marker but was removed to eliminate diagnostic FPs.
|
||||
// Shape (b) requires /…\.md or /…\.json immediately after the expression.
|
||||
// /.claude/gsd-core/commands has no .md/.json → VALID.
|
||||
ruleTester.run('normalize-path-in-content', normalizePathInContent, {
|
||||
valid: [
|
||||
{
|
||||
code: `const body = \`\${os.homedir()}/.claude/gsd-core/commands\`;`,
|
||||
},
|
||||
],
|
||||
invalid: [],
|
||||
});
|
||||
});
|
||||
|
||||
test('invalid: String() wrapping without normalization is still flagged', () => {
|
||||
ruleTester.run('normalize-path-in-content', normalizePathInContent, {
|
||||
valid: [],
|
||||
invalid: [
|
||||
{
|
||||
// String() alone is not a POSIX normalizer — the replace() is still needed.
|
||||
// Uses @~/ marker so the content heuristic fires (bare '@' is no longer a marker).
|
||||
code: "const ref = `@~/${String(path.join(home, '.claude'))}/x.md`;",
|
||||
errors: [{ messageId: 'pathInContent' }],
|
||||
},
|
||||
],
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
// ─── VALID cases (no violation) ───────────────────────────────────────────────
|
||||
|
||||
describe('normalize-path-in-content valid cases', () => {
|
||||
test('valid: path normalized with .replace(/\\\\/g, "/") before interpolation', () => {
|
||||
ruleTester.run('normalize-path-in-content', normalizePathInContent, {
|
||||
valid: [
|
||||
{
|
||||
// Correct POSIX normalization via .replace before interpolation into @~/ reference.
|
||||
// Uses '@~/' marker so the content heuristic fires, then confirms no violation.
|
||||
code: "const ref = `@~/${String(path.join(home, '.claude')).replace(/\\\\/g, '/')}/x.md`;",
|
||||
},
|
||||
],
|
||||
invalid: [],
|
||||
});
|
||||
});
|
||||
|
||||
test('valid: toPosixPath() wrapper before interpolation', () => {
|
||||
ruleTester.run('normalize-path-in-content', normalizePathInContent, {
|
||||
valid: [
|
||||
{
|
||||
code: `const body = \`\${toPosixPath(path.resolve(configDir))}/commands/gsd/x.md\`;`,
|
||||
},
|
||||
],
|
||||
invalid: [],
|
||||
});
|
||||
});
|
||||
|
||||
test('valid: console.log with path — log line, not content (no content markers = no flag)', () => {
|
||||
ruleTester.run('normalize-path-in-content', normalizePathInContent, {
|
||||
valid: [
|
||||
{
|
||||
// Log line: no @, $HOME, .md, /gsd etc. in the template quasis
|
||||
code: `console.log(\`built \${path.join(a, b)}\`);`,
|
||||
},
|
||||
],
|
||||
invalid: [],
|
||||
});
|
||||
});
|
||||
|
||||
test('valid: fs.writeFileSync with path join as first arg — real FS path, not content', () => {
|
||||
ruleTester.run('normalize-path-in-content', normalizePathInContent, {
|
||||
valid: [
|
||||
{
|
||||
// The ENTIRE template is an arg to fs.writeFileSync → suppressed
|
||||
// as a real filesystem path argument, not markdown content
|
||||
code: `fs.writeFileSync(\`\${path.join(a, b)}/foo\`, data);`,
|
||||
},
|
||||
],
|
||||
invalid: [],
|
||||
});
|
||||
});
|
||||
|
||||
test('valid: new Error with path — diagnostic, not content', () => {
|
||||
ruleTester.run('normalize-path-in-content', normalizePathInContent, {
|
||||
valid: [
|
||||
{
|
||||
// Diagnostic / error message: not markdown content
|
||||
code: `throw new Error(\`File not found: \${path.join(dir, file)}\`);`,
|
||||
},
|
||||
],
|
||||
invalid: [],
|
||||
});
|
||||
});
|
||||
|
||||
test('valid: template literal with no content markers does not flag path', () => {
|
||||
ruleTester.run('normalize-path-in-content', normalizePathInContent, {
|
||||
valid: [
|
||||
{
|
||||
// No @, $HOME, ~/., .md, /gsd, etc. → not considered content
|
||||
code: `const msg = \`Processing \${path.join(a, b)} now\`;`,
|
||||
},
|
||||
],
|
||||
invalid: [],
|
||||
});
|
||||
});
|
||||
|
||||
test('valid: String() + .replace() chain normalized before interpolation', () => {
|
||||
ruleTester.run('normalize-path-in-content', normalizePathInContent, {
|
||||
valid: [
|
||||
{
|
||||
// The real fix used in runtime-artifact-conversion.cts line 2193
|
||||
code: `const posixTarget = String(resolvedTarget).replace(/\\\\/g, '/');
|
||||
const ref = \`@\${posixTarget}/.claude/x.md\`;`,
|
||||
},
|
||||
],
|
||||
invalid: [],
|
||||
});
|
||||
});
|
||||
|
||||
test('valid: resolveAgentDir result already POSIX-normalized via toPosixPath', () => {
|
||||
ruleTester.run('normalize-path-in-content', normalizePathInContent, {
|
||||
valid: [
|
||||
{
|
||||
code: `const body = \`@\${toPosixPath(resolveAgentDir('opencode', { homedir: () => home }))}/agents/gsd.md\`;`,
|
||||
},
|
||||
],
|
||||
invalid: [],
|
||||
});
|
||||
});
|
||||
|
||||
test('valid: path used in require() — module load, not content', () => {
|
||||
ruleTester.run('normalize-path-in-content', normalizePathInContent, {
|
||||
valid: [
|
||||
{
|
||||
// require() is a suppressed context
|
||||
code: `const m = require(\`\${path.join(libDir, 'helpers')}\`);`,
|
||||
},
|
||||
],
|
||||
invalid: [],
|
||||
});
|
||||
});
|
||||
|
||||
// ── N1: path.basename exclusion ─────────────────────────────────────────────
|
||||
//
|
||||
// path.basename() returns only the final filename component (no directory
|
||||
// separators), so it cannot leak backslashes into content regardless of OS.
|
||||
// It is excluded from CONTENT_PATH_FNS for this rule.
|
||||
|
||||
test('valid (N1): path.basename(p) in a template with @~/ reference marker — no flag because basename cannot contain separators', () => {
|
||||
ruleTester.run('normalize-path-in-content', normalizePathInContent, {
|
||||
valid: [
|
||||
{
|
||||
// path.basename returns a filename with no directory separators;
|
||||
// interpolating it into @~/ reference content is safe on all platforms.
|
||||
// Uses '@~/' marker so the content heuristic fires, then confirms no
|
||||
// violation because path.basename is excluded from CONTENT_PATH_FNS.
|
||||
code: "const label = `@~/${path.basename(outputPath)}/x.md`;",
|
||||
},
|
||||
],
|
||||
invalid: [],
|
||||
});
|
||||
});
|
||||
|
||||
test('valid (N1): path.basename(p) in a template with /commands/ marker — not flagged', () => {
|
||||
ruleTester.run('normalize-path-in-content', normalizePathInContent, {
|
||||
valid: [
|
||||
{
|
||||
code: `const ref = \`/commands/\${path.basename(outputPath)}/help.md\`;`,
|
||||
},
|
||||
],
|
||||
invalid: [],
|
||||
});
|
||||
});
|
||||
|
||||
// ── N2: tightened content heuristic ─────────────────────────────────────────
|
||||
//
|
||||
// A bare `.md` token in plain prose (no @-ref, ~/., $HOME, config-dir, or
|
||||
// artifact-path segment) must NOT qualify as "content". This test mirrors
|
||||
// the false-positive from src/profile-output.cts:1205.
|
||||
|
||||
test('valid (N2): path.basename(outputPath) in a status-message template with bare .md prose — not flagged', () => {
|
||||
ruleTester.run('normalize-path-in-content', normalizePathInContent, {
|
||||
valid: [
|
||||
{
|
||||
// Mirrors the src/profile-output.cts:1205 false positive. The template
|
||||
// contains "PROJECT.md / REQUIREMENTS.md" (bare prose), but the quasi
|
||||
// after the expression does NOT start with '/' → shape (b) does not fire.
|
||||
// And no @-ref / $HOME / ~/ markers → shape (a) does not fire.
|
||||
code: `const msg = \`Left existing \${path.basename(outputPath)} untouched (no GSD markers found). Broad project context lives in PROJECT.md / REQUIREMENTS.md; pass --force to overwrite.\`;`,
|
||||
},
|
||||
],
|
||||
invalid: [],
|
||||
});
|
||||
});
|
||||
|
||||
test('valid (N2): path.join result in prose with bare .md mention — quasi after expr does not start with / → not flagged', () => {
|
||||
ruleTester.run('normalize-path-in-content', normalizePathInContent, {
|
||||
valid: [
|
||||
{
|
||||
// path.join IS in CONTENT_PATH_FNS, but the quasi after the expression is
|
||||
// ': see README.md for details.' which does NOT start with '/' → shape (b)
|
||||
// does not fire. No @-ref / $HOME / ~/ markers either.
|
||||
code: `const note = \`Generated \${path.join(dir, 'output')}: see README.md for details.\`;`,
|
||||
},
|
||||
],
|
||||
invalid: [],
|
||||
});
|
||||
});
|
||||
|
||||
// ── N4: canonical INVALID shapes still flag after N1/N2 changes ─────────────
|
||||
|
||||
test('invalid (N4 confirm): @~/${path.join(home,".claude")}/x.md — still flagged after N1/N2/@-narrowing', () => {
|
||||
ruleTester.run('normalize-path-in-content', normalizePathInContent, {
|
||||
valid: [],
|
||||
invalid: [
|
||||
{
|
||||
// @~/ prefix marker in quasis → genuine @-reference content → path.join flagged.
|
||||
// Confirms that narrowing '@' to '@~'/'@$'/'@/' leaves the canonical case working.
|
||||
code: "const ref = `@~/${path.join(home, '.claude')}/x.md`;",
|
||||
errors: [{ messageId: 'pathInContent' }],
|
||||
},
|
||||
],
|
||||
});
|
||||
});
|
||||
|
||||
test('invalid (N4 confirm): ${computePathPrefix(t)}/commands/gsd/x.md — still flagged after N1/N2 via shape (b)', () => {
|
||||
ruleTester.run('normalize-path-in-content', normalizePathInContent, {
|
||||
valid: [],
|
||||
invalid: [
|
||||
{
|
||||
// Shape (b): quasi[1] = '/commands/gsd/x.md' starts with '/' and ends with '.md'
|
||||
// → QUASI_MD_JSON_RE matches → flagged. (/commands/ is no longer in CONTENT_MARKERS
|
||||
// but shape (b) per-expression check catches this case.)
|
||||
code: `const body = \`\${computePathPrefix(t)}/commands/gsd/x.md\`;`,
|
||||
errors: [{ messageId: 'pathInContent' }],
|
||||
},
|
||||
],
|
||||
});
|
||||
});
|
||||
|
||||
// ── W2: bare Error(...) / TypeError(...) suppression ─────────────────────────
|
||||
//
|
||||
// A bare Error(...) call (CallExpression, not NewExpression) with a path inside
|
||||
// must be suppressed — it is a diagnostic, not content.
|
||||
|
||||
test('valid (W2): throw Error(`...${path.join(...)}`) — Error() is diagnostic; also /.claude is no longer a content marker', () => {
|
||||
ruleTester.run('normalize-path-in-content', normalizePathInContent, {
|
||||
valid: [
|
||||
{
|
||||
// W2: bare Error() call (CallExpression). Additionally, /.claude is no
|
||||
// longer in CONTENT_MARKERS (removed to eliminate diagnostic FPs), so this
|
||||
// template has no content markers at all — not flagged on either ground.
|
||||
code: "throw Error(`Cannot install to /.claude: ${path.join(a, b)}`);",
|
||||
},
|
||||
],
|
||||
invalid: [],
|
||||
});
|
||||
});
|
||||
|
||||
test('valid (W2): throw TypeError(`...${path.join(...)}`) — TypeError() is suppressed; also no content markers', () => {
|
||||
ruleTester.run('normalize-path-in-content', normalizePathInContent, {
|
||||
valid: [
|
||||
{
|
||||
// W2: /.claude no longer a content marker; also in an Error() call context.
|
||||
code: "throw TypeError(`Bad path /.claude: ${path.join(a, b)}`);",
|
||||
},
|
||||
],
|
||||
invalid: [],
|
||||
});
|
||||
});
|
||||
|
||||
// ── W3: chained and right-deep concatenation with nested BinaryExpression ──────
|
||||
//
|
||||
// The BinaryExpression check recursively scans the FULL concat tree for both:
|
||||
// (1) a content marker anywhere in the tree (via isContentString recursion)
|
||||
// (2) an unnormalized path call anywhere in the non-content subtree
|
||||
// (via scanAndReport recursion — the "right-deep FN" fix)
|
||||
//
|
||||
// Cases:
|
||||
// '@~/' + name + path.join(home, '.claude')
|
||||
// → parse tree: ('@~/' + name) + path.join(...)
|
||||
// → outer left is BinaryExpression → isContentString descends → finds '@~/'
|
||||
// → outer right is path.join → isUnnormalizedPathExpression → flagged
|
||||
//
|
||||
// '@~/' + (name + path.join(home, '.claude'))
|
||||
// → parse tree: '@~/' + (name + path.join(...))
|
||||
// → left is '@~/' → content marker found
|
||||
// → right is BinaryExpression; scanAndReport descends → finds path.join → flagged
|
||||
|
||||
test('invalid (W3): "@~/" + name + path.join(home, ".claude") — chained concat flagged via recursive scan', () => {
|
||||
ruleTester.run('normalize-path-in-content', normalizePathInContent, {
|
||||
valid: [],
|
||||
invalid: [
|
||||
{
|
||||
// W3 left-deep: left side is '@~/' + name (nested BinaryExpression).
|
||||
// Without recursive descent, the marker '@~/' in the inner literal
|
||||
// is invisible to the outer + node and the violation is missed.
|
||||
code: "const s = '@~/' + name + path.join(home, '.claude');",
|
||||
errors: [{ messageId: 'pathInContent' }],
|
||||
},
|
||||
],
|
||||
});
|
||||
});
|
||||
|
||||
test('invalid (W3-right-deep): "@~/" + (name + path.join(home, ".claude")) — right-deep path call flagged', () => {
|
||||
ruleTester.run('normalize-path-in-content', normalizePathInContent, {
|
||||
valid: [],
|
||||
invalid: [
|
||||
{
|
||||
// W3 right-deep: content marker is in the left literal '@~/' and the path
|
||||
// call is nested inside the right-side BinaryExpression (name + path.join).
|
||||
// The recursive scanAndReport must descend into the right subtree to find
|
||||
// path.join and report it.
|
||||
code: "const s = '@~/' + (name + path.join(home, '.claude'));",
|
||||
errors: [{ messageId: 'pathInContent' }],
|
||||
},
|
||||
],
|
||||
});
|
||||
});
|
||||
|
||||
test('valid (W3): "log " + path.join(a, b) — no content marker in any sub-literal → not flagged', () => {
|
||||
ruleTester.run('normalize-path-in-content', normalizePathInContent, {
|
||||
valid: [
|
||||
{
|
||||
// No content marker anywhere in the concat tree → not flagged.
|
||||
code: "const s = 'log ' + path.join(a, b);",
|
||||
},
|
||||
],
|
||||
invalid: [],
|
||||
});
|
||||
});
|
||||
|
||||
// ── '@' narrowing precision ────────────────────────────────────────────────────
|
||||
//
|
||||
// A bare '@' that is not @~, @$, or @/ must NOT trigger the content heuristic
|
||||
// (e.g. email addresses, @author attributions in comments / strings).
|
||||
|
||||
test('valid (@-narrowing): bare "@" in string (e.g. email) is not an @-reference marker', () => {
|
||||
ruleTester.run('normalize-path-in-content', normalizePathInContent, {
|
||||
valid: [
|
||||
{
|
||||
// The string contains '@' but no @~, @$, @/ and no ~/., $HOME markers.
|
||||
// Bare '@' is not in CONTENT_MARKERS (only @~, @$, @/ are).
|
||||
// Also: quasi after expression is ';' — no /…\.md shape (b) match.
|
||||
code: "const msg = `Contact author@example.com for ${path.join(a, b)}`;",
|
||||
},
|
||||
],
|
||||
invalid: [],
|
||||
});
|
||||
});
|
||||
|
||||
test('valid (@-narrowing): @~/${path.join(home, ".claude")}/x.md WITH normalization — not flagged', () => {
|
||||
ruleTester.run('normalize-path-in-content', normalizePathInContent, {
|
||||
valid: [
|
||||
{
|
||||
// Confirms that after @-narrowing, a proper @~/ reference WITH POSIX
|
||||
// normalization is still correctly NOT flagged.
|
||||
code: "const ref = `@~/${String(path.join(home, '.claude')).replace(/\\\\/g, '/')}/x.md`;",
|
||||
},
|
||||
],
|
||||
invalid: [],
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
// ── TS-parser RuleTester ───────────────────────────────────────────────────────
|
||||
//
|
||||
// Run representative fixtures through the @typescript-eslint/parser to confirm
|
||||
// the rule works under the production parser (used on src/**/*.cts). This
|
||||
// catches any AST-shape differences between espree and ts-estree.
|
||||
|
||||
const { RuleTester: TSRuleTester } = require('eslint');
|
||||
const tsParser = require('@typescript-eslint/parser');
|
||||
|
||||
const tsRuleTester = new TSRuleTester({
|
||||
languageOptions: {
|
||||
parser: tsParser,
|
||||
ecmaVersion: 2022,
|
||||
sourceType: 'commonjs',
|
||||
},
|
||||
});
|
||||
|
||||
describe('normalize-path-in-content — @typescript-eslint/parser (TS syntax fixtures)', () => {
|
||||
test('TS-parser INVALID: @~/${path.join(home as string, ".claude")}/x.md — flagged under TS parser', () => {
|
||||
tsRuleTester.run('normalize-path-in-content', normalizePathInContent, {
|
||||
valid: [],
|
||||
invalid: [
|
||||
{
|
||||
// TS-specific syntax: `home as string` type assertion inside path.join arg.
|
||||
// The expression inside ${ } is still path.join (a CallExpression) → flagged.
|
||||
code: "const ref = `@~/${path.join(home as string, '.claude')}/x.md`;",
|
||||
errors: [{ messageId: 'pathInContent' }],
|
||||
},
|
||||
],
|
||||
});
|
||||
});
|
||||
|
||||
test('TS-parser VALID: @~/${toPosixPath(path.join(home as string, ".claude"))}/x.md — not flagged', () => {
|
||||
tsRuleTester.run('normalize-path-in-content', normalizePathInContent, {
|
||||
valid: [
|
||||
{
|
||||
// Same TS syntax but POSIX-normalized via toPosixPath — must NOT be flagged.
|
||||
code: "const ref = `@~/${toPosixPath(path.join(home as string, '.claude'))}/x.md`;",
|
||||
},
|
||||
],
|
||||
invalid: [],
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -26,6 +26,7 @@ const assert = require('node:assert/strict');
|
||||
const fs = require('fs');
|
||||
const path = require('path');
|
||||
const espree = require('espree');
|
||||
const tsEstree = require('@typescript-eslint/typescript-estree');
|
||||
const { globSync } = require('glob');
|
||||
|
||||
// ── Protected portability rules (grows with each ADR-1703 phase) ──────────────
|
||||
@@ -38,6 +39,8 @@ const PROTECTED_RULES = [
|
||||
'no-hardcoded-tmp',
|
||||
'no-bare-npm-exec',
|
||||
'require-userprofile-with-home',
|
||||
// ADR-1703 Phase 5 rule (issue #1733) — applies to src/**/*.cts (production sources)
|
||||
'normalize-path-in-content',
|
||||
];
|
||||
|
||||
// ── Detect disable directives via the comment text ───────────────────────────
|
||||
@@ -81,14 +84,25 @@ function classifyComment(commentValue) {
|
||||
return null;
|
||||
}
|
||||
|
||||
// ── Collect test files ────────────────────────────────────────────────────────
|
||||
// ── Collect scanned files ─────────────────────────────────────────────────────
|
||||
//
|
||||
// The disable-ban covers:
|
||||
// - tests/**/*.test.cjs — test sources (phase 1–4 scope)
|
||||
// - src/**/*.cts — production TypeScript sources (extended in phase 5 to
|
||||
// protect normalize-path-in-content, which applies to
|
||||
// src/**/*.cts; an eslint-disable there would bypass the
|
||||
// production rule entirely)
|
||||
|
||||
const SELF_ABS = __filename;
|
||||
|
||||
function collectTestFiles() {
|
||||
return globSync('tests/**/*.test.cjs', { cwd: path.join(__dirname, '..') })
|
||||
.map(rel => path.join(__dirname, '..', rel))
|
||||
const root = path.join(__dirname, '..');
|
||||
const testFiles = globSync('tests/**/*.test.cjs', { cwd: root })
|
||||
.map(rel => path.join(root, rel))
|
||||
.filter(absPath => absPath !== SELF_ABS);
|
||||
const srcFiles = globSync('src/**/*.cts', { cwd: root })
|
||||
.map(rel => path.join(root, rel));
|
||||
return [...testFiles, ...srcFiles];
|
||||
}
|
||||
|
||||
// ── Scan ──────────────────────────────────────────────────────────────────────
|
||||
@@ -101,15 +115,23 @@ function scanFile(absPath) {
|
||||
throw new Error(`Could not read ${absPath}: ${err.message}`);
|
||||
}
|
||||
|
||||
// .cts files use TypeScript syntax — use @typescript-eslint/typescript-estree.
|
||||
// .cjs files use plain JS — use espree (the original parser).
|
||||
const isCts = absPath.endsWith('.cts');
|
||||
|
||||
let ast;
|
||||
try {
|
||||
ast = espree.parse(src, {
|
||||
comment: true,
|
||||
ecmaVersion: 2022,
|
||||
loc: true,
|
||||
range: true,
|
||||
tolerant: true,
|
||||
});
|
||||
if (isCts) {
|
||||
ast = tsEstree.parse(src, { comment: true, loc: true, range: true });
|
||||
} else {
|
||||
ast = espree.parse(src, {
|
||||
comment: true,
|
||||
ecmaVersion: 2022,
|
||||
loc: true,
|
||||
range: true,
|
||||
tolerant: true,
|
||||
});
|
||||
}
|
||||
} catch (parseErr) {
|
||||
// C5: fail CLOSED on parse error — a file that fails to parse must FAIL the
|
||||
// test with its path, not be silently skipped. Silent skip is a false-green:
|
||||
@@ -135,7 +157,7 @@ function scanFile(absPath) {
|
||||
// ── C5: parse-error fail-closed ───────────────────────────────────────────────
|
||||
|
||||
describe('C5 — scanFile fails closed on parse error', () => {
|
||||
test('C5: scanFile throws on parse error instead of silently returning empty result', () => {
|
||||
test('C5a: scanFile throws on parse error instead of silently returning empty result (.cjs path, espree)', () => {
|
||||
// Inject a parse error deterministically by monkeypatching espree.parse.
|
||||
// This is the cross-platform approach (works under root/Docker too).
|
||||
const origParse = espree.parse;
|
||||
@@ -154,6 +176,51 @@ describe('C5 — scanFile fails closed on parse error', () => {
|
||||
espree.parse = origParse;
|
||||
}
|
||||
});
|
||||
|
||||
test('W1/C5b: scanFile throws on parse error for .cts path (tsEstree path — fail-closed)', () => {
|
||||
// W1: The existing C5a test only exercises the espree (.cjs) path. This test
|
||||
// exercises the tsEstree (.cts) path by monkeypatching tsEstree.parse and
|
||||
// pointing scanFile at a synthetic .cts-suffixed path.
|
||||
//
|
||||
// Cross-platform approach: monkeypatch the module method, not chmod/permissions
|
||||
// (chmod 0o000 is bypassed by root in Docker and behaves differently per OS).
|
||||
//
|
||||
// tsEstree exports 'parse' via a configurable getter (no setter), so we use
|
||||
// Object.defineProperty to inject a throwing stub, then restore the original
|
||||
// descriptor in the finally block.
|
||||
const tsEstreeModule = require('@typescript-eslint/typescript-estree');
|
||||
const origDescriptor = Object.getOwnPropertyDescriptor(tsEstreeModule, 'parse');
|
||||
const injected = () => { throw new SyntaxError('injected tsEstree parse error for W1/C5b test'); };
|
||||
Object.defineProperty(tsEstreeModule, 'parse', {
|
||||
value: injected,
|
||||
writable: true,
|
||||
configurable: true,
|
||||
enumerable: true,
|
||||
});
|
||||
|
||||
// Also monkeypatch fs.readFileSync to return dummy content for the fake .cts
|
||||
// path, so the .cts branch in scanFile runs without needing a real file.
|
||||
const origReadFileSync = fs.readFileSync;
|
||||
fs.readFileSync = (p, enc) => {
|
||||
if (typeof p === 'string' && p.endsWith('.cts')) return '// dummy cts content';
|
||||
return origReadFileSync.call(fs, p, enc);
|
||||
};
|
||||
|
||||
try {
|
||||
assert.throws(
|
||||
() => scanFile(path.join(__dirname, 'dummy-fixture.cts')),
|
||||
(err) => {
|
||||
return err instanceof Error &&
|
||||
err.message.includes('injected tsEstree parse error for W1/C5b test');
|
||||
},
|
||||
'scanFile must throw on tsEstree parse error for .cts files (fail-closed)'
|
||||
);
|
||||
} finally {
|
||||
fs.readFileSync = origReadFileSync;
|
||||
// Restore original descriptor (getter-only)
|
||||
Object.defineProperty(tsEstreeModule, 'parse', origDescriptor);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
// ── Tests ─────────────────────────────────────────────────────────────────────
|
||||
|
||||
Reference in New Issue
Block a user