feat(engine): fixture-path dead links are SKIP, not WARN
A file living under test/, tests/, fixtures/, or a *golden* path is presumed to break its own links on purpose. nav-golden-escape/bundle/ index.md's deliberate `../../../../etc/passwd` escape pops the whole base path instead of resolving to null, so it read as a genuine WARN against three repos in the org — the check was at fault, not them. The finding still fires, as LINK-INTERNAL-FIXTURE at SKIP with file and line, so it is never silently dropped. Measured before shipping: 16 LINK-INTERNAL-* findings before, 16 after, across all 20 local clones — every one converted 1:1, none disappeared. 135 tests (was 129).
This commit is contained in:
parent
43a8d28f00
commit
9eb210bb01
4 changed files with 105 additions and 8 deletions
12
CLAUDE.md
12
CLAUDE.md
|
|
@ -80,6 +80,16 @@ would recreate, in data, exactly the drift this plugin exists to remove.
|
||||||
root it is a `WARN`: that is where session plans, agent working files and
|
root it is a `WARN`: that is where session plans, agent working files and
|
||||||
path-traversal fixtures with deliberately invalid targets live. Measured, 30
|
path-traversal fixtures with deliberately invalid targets live. Measured, 30
|
||||||
of 43 findings were down there and all were `ERROR`s.
|
of 43 findings were down there and all were `ERROR`s.
|
||||||
|
- **A fixture-path dead link is `SKIP`, not `WARN` — and never silently
|
||||||
|
dropped.** `test/`, `tests/`, `fixtures/` (exact segment) and `*golden*`
|
||||||
|
(substring) mark a path as presumed intentional; the finding still fires as
|
||||||
|
`LINK-INTERNAL-FIXTURE` with its file and line, it just isn't judged.
|
||||||
|
Grounded in `nav-golden-escape/bundle/index.md`'s deliberate
|
||||||
|
`../../../../etc/passwd` escape: the deep `..` pops the whole base path
|
||||||
|
rather than resolving to `null`, so it read as a genuine `WARN` — third tool
|
||||||
|
in the org to hit this exact pattern, which is the signal the check was at
|
||||||
|
fault. Measured before shipping: 16 findings before, 16 after, across all 20
|
||||||
|
local clones — every one converted 1:1, none disappeared.
|
||||||
- **A repo's name is its remote, not its directory.** `catalog/` holds
|
- **A repo's name is its remote, not its directory.** `catalog/` holds
|
||||||
`ktg-plugin-marketplace`. The basename left it unregistered with zero checks
|
`ktg-plugin-marketplace`. The basename left it unregistered with zero checks
|
||||||
run, against the one repo every catalog rule depends on.
|
run, against the one repo every catalog rule depends on.
|
||||||
|
|
@ -89,7 +99,7 @@ would recreate, in data, exactly the drift this plugin exists to remove.
|
||||||
## Commands
|
## Commands
|
||||||
|
|
||||||
```bash
|
```bash
|
||||||
npm test # 129 tests
|
npm test # 135 tests
|
||||||
node scripts/repo-standard-check.mjs --dir "$PWD" # gate one repo
|
node scripts/repo-standard-check.mjs --dir "$PWD" # gate one repo
|
||||||
node scripts/repo-standard-check.mjs --offline # no network call
|
node scripts/repo-standard-check.mjs --offline # no network call
|
||||||
node scripts/repo-standard-check.mjs --json # machine output
|
node scripts/repo-standard-check.mjs --json # machine output
|
||||||
|
|
|
||||||
|
|
@ -167,7 +167,7 @@ without that, a raw scan turns three dead names into about twenty.
|
||||||
npm test
|
npm test
|
||||||
```
|
```
|
||||||
|
|
||||||
129 tests over the pure classifiers. The reference fixtures are measured false
|
135 tests over the pure classifiers. The reference fixtures are measured false
|
||||||
positives, each with its expected verdict — the six that produced the
|
positives, each with its expected verdict — the six that produced the
|
||||||
three-outcome reference rule, plus the noise sources found by running the gate
|
three-outcome reference rule, plus the noise sources found by running the gate
|
||||||
against a real repository: regexes inside code spans that are markdown links to
|
against a real repository: regexes inside code spans that are markdown links to
|
||||||
|
|
|
||||||
|
|
@ -768,6 +768,20 @@ function linkLevelFor(path) {
|
||||||
return String(path).includes('/') ? 'WARN' : 'ERROR';
|
return String(path).includes('/') ? 'WARN' : 'ERROR';
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// A file living in a test/fixture path is presumed to break its own links on
|
||||||
|
// purpose — `nav-golden-escape/bundle/index.md` escapes with `../../../../etc/passwd`
|
||||||
|
// deliberately, and the deep `..` pops the whole base path rather than resolving
|
||||||
|
// to `null`, so it read as a genuine WARN. Third tool in the org to hit this
|
||||||
|
// exact pattern, which is the signal that the check was at fault, not the repos.
|
||||||
|
// Only `*golden*` is a substring glob; the other three are exact segment names,
|
||||||
|
// so `testing/` or `fixturesque/` — real directories — are not swept in.
|
||||||
|
function isFixturePath(path) {
|
||||||
|
return String(path)
|
||||||
|
.toLowerCase()
|
||||||
|
.split('/')
|
||||||
|
.some((seg) => seg === 'test' || seg === 'tests' || seg === 'fixtures' || seg.includes('golden'));
|
||||||
|
}
|
||||||
|
|
||||||
// Relative file links only. Anchor resolution depends on per-renderer heading
|
// Relative file links only. Anchor resolution depends on per-renderer heading
|
||||||
// slug rules and is a rabbit hole; external URLs need the network. Both are
|
// slug rules and is a rabbit hole; external URLs need the network. Both are
|
||||||
// deliberately out — a check that is sometimes wrong teaches people to ignore it.
|
// deliberately out — a check that is sometimes wrong teaches people to ignore it.
|
||||||
|
|
@ -804,12 +818,20 @@ export function checkInternalLinks({ files, present }) {
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
if (!have.has(resolved) && !haveDirs.has(resolved)) {
|
if (!have.has(resolved) && !haveDirs.has(resolved)) {
|
||||||
findings.push({
|
if (isFixturePath(path)) {
|
||||||
level: linkLevelFor(path),
|
findings.push({
|
||||||
code: 'LINK-INTERNAL-MISSING',
|
level: 'SKIP',
|
||||||
bucket: 'broken',
|
code: 'LINK-INTERNAL-FIXTURE',
|
||||||
msg: `${path}:${i + 1} — link points at \`${clean}\` (${resolved}), which is not a tracked file`,
|
msg: `${path}:${i + 1} — link points at \`${clean}\` (${resolved}), which is not a tracked file; ${path} is a test/fixture path, so this is presumed intentional and not judged`,
|
||||||
});
|
});
|
||||||
|
} else {
|
||||||
|
findings.push({
|
||||||
|
level: linkLevelFor(path),
|
||||||
|
code: 'LINK-INTERNAL-MISSING',
|
||||||
|
bucket: 'broken',
|
||||||
|
msg: `${path}:${i + 1} — link points at \`${clean}\` (${resolved}), which is not a tracked file`,
|
||||||
|
});
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
});
|
});
|
||||||
|
|
|
||||||
|
|
@ -1093,6 +1093,71 @@ test('WARN-only links must not also report that every link resolves', () => {
|
||||||
assert.equal(f.some((x) => x.code === 'LINKS-INTERNAL'), false);
|
assert.equal(f.some((x) => x.code === 'LINKS-INTERNAL'), false);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// ------------------------------------ fixture paths are presumed intentional
|
||||||
|
// `shared/examples/nav-golden-escape/bundle/index.md` in portfolio-optimiser
|
||||||
|
// deliberately escapes with `../../../../etc/passwd` — the deep `..` pops the
|
||||||
|
// whole base path and lands on `etc/passwd`, a path that is not `null` (still
|
||||||
|
// inside the repo by the resolver's arithmetic) and not tracked, so it read as
|
||||||
|
// a genuine WARN. It is the fixture doing its job, not a broken link. Third
|
||||||
|
// tool in the org to hit this same pattern — the check was the thing at fault.
|
||||||
|
|
||||||
|
test('a dead link inside a *golden* fixture path is SKIP with its own code, not WARN', () => {
|
||||||
|
const f = checkInternalLinks({
|
||||||
|
files: { 'shared/examples/nav-golden-escape/bundle/index.md': '[x](../../../../etc/passwd)' },
|
||||||
|
present: ['shared/examples/nav-golden-escape/bundle/index.md'],
|
||||||
|
});
|
||||||
|
assert.equal(f.some((x) => x.code === 'LINK-INTERNAL-MISSING'), false);
|
||||||
|
const hit = f.find((x) => x.code === 'LINK-INTERNAL-FIXTURE');
|
||||||
|
assert.equal(hit.level, 'SKIP');
|
||||||
|
assert.equal(hit.bucket, undefined);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('a dead link inside a tests/ or fixtures/ directory is SKIP, not judged', () => {
|
||||||
|
const f = checkInternalLinks({
|
||||||
|
files: { 'tests/fixtures/plan.md': '[x](gone.md)' },
|
||||||
|
present: ['tests/fixtures/plan.md'],
|
||||||
|
});
|
||||||
|
assert.equal(f.some((x) => x.code === 'LINK-INTERNAL-MISSING'), false);
|
||||||
|
assert.equal(f.some((x) => x.code === 'LINK-INTERNAL-FIXTURE' && x.level === 'SKIP'), true);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('a fixture-path dead link is never silently dropped — SKIP still names file and line', () => {
|
||||||
|
const f = checkInternalLinks({
|
||||||
|
files: { 'tests/plan.md': '[x](gone.md)' },
|
||||||
|
present: ['tests/plan.md'],
|
||||||
|
});
|
||||||
|
const hit = f.find((x) => x.code === 'LINK-INTERNAL-FIXTURE');
|
||||||
|
assert.match(hit.msg, /tests\/plan\.md:1/);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('SKIP-classified fixture links do not suppress the every-link-resolves OK line', () => {
|
||||||
|
// LINK-OUTSIDE-REPO already sits outside the LINK-INTERNAL-MISSING check that
|
||||||
|
// gates the OK line; LINK-INTERNAL-FIXTURE follows the same precedent.
|
||||||
|
const f = checkInternalLinks({
|
||||||
|
files: { 'tests/plan.md': '[x](gone.md)' },
|
||||||
|
present: ['tests/plan.md'],
|
||||||
|
});
|
||||||
|
assert.equal(f.some((x) => x.code === 'LINKS-INTERNAL'), true);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('a directory only substring-matching "test" or "fixtures" is not treated as a fixture path', () => {
|
||||||
|
// Exact segment match only for the literal names — "testing/" or
|
||||||
|
// "fixturesque/" are real directories, not the fixture convention. Only
|
||||||
|
// *golden* is a deliberate substring glob.
|
||||||
|
const f = checkInternalLinks({
|
||||||
|
files: { 'testing/plan.md': '[x](gone.md)' },
|
||||||
|
present: ['testing/plan.md'],
|
||||||
|
});
|
||||||
|
assert.equal(f.some((x) => x.code === 'LINK-INTERNAL-FIXTURE'), false);
|
||||||
|
assert.equal(f.some((x) => x.code === 'LINK-INTERNAL-MISSING' && x.level === 'WARN'), true);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('a root-level dead link is unaffected by the fixture heuristic', () => {
|
||||||
|
const f = checkInternalLinks({ files: { 'README.md': '[e](docs/gone.md)' }, present: ['README.md'] });
|
||||||
|
assert.equal(f.some((x) => x.code === 'LINK-INTERNAL-FIXTURE'), false);
|
||||||
|
assert.equal(f.find((x) => x.code === 'LINK-INTERNAL-MISSING').level, 'ERROR');
|
||||||
|
});
|
||||||
|
|
||||||
// ------------------------------------------------ the repo name is the remote
|
// ------------------------------------------------ the repo name is the remote
|
||||||
// `catalog/` is the working directory of the repo named `ktg-plugin-marketplace`.
|
// `catalog/` is the working directory of the repo named `ktg-plugin-marketplace`.
|
||||||
// Deriving the name from the directory basename left it REPO-UNREGISTERED and
|
// Deriving the name from the directory basename left it REPO-UNREGISTERED and
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue