fix(guards): resolve markdown links against a normalised repository root - #192
Open
OnLocation-acumbal-contractor wants to merge 1 commit into
Open
Conversation
`git rev-parse --show-toplevel` reports POSIX separators on every platform. On Windows it answers `C:/repo` while `path.resolve` produces `C:\repo`, so the containment check in `targetExists` compared `C:\repo\SECURITY.md` against the prefix `C:/repo\` and bailed out before ever calling `existsSync`. Every local link failed: `node guards/run.mjs` on Windows reports all 56 links in this repository as missing, including `LICENSE` and `SECURITY.md`, and `pnpm verify` cannot go green on a Windows checkout. Normalise the root with `path.resolve` before comparing. The containment rule itself is unchanged, so targets escaping the repository are still rejected. The resolution step is extracted as `resolveWithinRoot` with an injectable `path` module, so the Windows behaviour is covered from any host. The tests assert the git-reported root, the native Windows root, POSIX parity, and that traversal outside the repository still returns null. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #
The bug
git rev-parse --show-toplevelreports POSIX separators on every platform. On Windows itanswers
C:/repowhilepath.resolveproducesC:\repo, so the containment check intargetExistscomparedwhich is
false, and returned beforeexistsSyncwas ever called. Every local link wasreported missing — on a Windows checkout
node guards/run.mjsflags all 56 links in thisrepository,
LICENSEandSECURITY.mdincluded, whileactions-pinnedpasses in the samerun and the same checkout is clean on Linux.
Since
pnpm verifyrunspnpm guards, a Windows contributor could not get a green localverify, and the output looks like a broken checkout rather than a platform bug.
The fix
Normalise the root with
path.resolvebefore the comparison. The containment rule isunchanged — targets escaping the repository are still rejected.
The resolution step is extracted as an exported
resolveWithinRoot(root, source, target, path)with the
pathmodule injectable, so Windows semantics can be exercised from any host vianode:path'swin32binding. That is the only structural change;targetExistskeeps itsbehaviour and its extension/
index.mdcandidates.Tests
scripts/markdown-links-guard.test.mjs, five cases, picked up by the existingpnpm test:dev(node --test scripts/*.test.mjs):C:/repo) resolves in-repo targets, including a../hopC:\repo) keeps workingnull, on both platformsThe traversal cases are there deliberately: the containment check is a security property, and
this change must not loosen it.
Verification
Confirmed on a Windows host, which is where the bug is observable.
Before this change,
node guards\run.mjson Windows failsmarkdown-linkswith 56violations — every local link in the repository,
LICENSEandSECURITY.mdincluded —while
actions-pinnedpasses in the same run. After applying this branch, same machine,same checkout:
On Linux:
Suggested follow-up
Not in this PR, but this is the third Windows issue on the quickstart path alongside #182 and
#167. A minimal Windows CI job running
pnpm guardsandpnpm test:devwould have caught allthree. Happy to open that separately if it's wanted.