Skip to content

fix: use path.sep for wiki path-traversal check on Windows#23

Closed
drewperd1000 wants to merge 1 commit intoHouseofmvps:mainfrom
drewperd1000:fix/wiki-windows-path-separator
Closed

fix: use path.sep for wiki path-traversal check on Windows#23
drewperd1000 wants to merge 1 commit intoHouseofmvps:mainfrom
drewperd1000:fix/wiki-windows-path-separator

Conversation

@drewperd1000
Copy link
Copy Markdown
Contributor

Summary

readWikiArticle hardcoded a POSIX forward-slash when verifying that the resolved article path stays inside the wiki directory:

if (!resolved.startsWith(wikiDir + "/") && resolved !== wikiDir) return null;

On Windows, path.resolve() returns backslash-delimited paths, so this check always returned false and every wiki article lookup fell through to return null. Both codesight_get_wiki_index and codesight_get_wiki_article MCP tools were broken for Windows users. Other tools (codesight_get_summary, codesight_lint_wiki, etc.) were unaffected because they don't route through readWikiArticle.

Fix

Use path.sep so the boundary check works cross-platform:

const { resolve, sep } = await import("node:path");
...
if (!resolved.startsWith(wikiDir + sep) && resolved !== wikiDir) return null;

Two-line change in src/generators/wiki.ts.

Tests

Added tests/wiki.test.ts with three regression tests:

  • readWikiArticle returns content for an existing article (with and without .md extension, covering both index and a regular article)
  • readWikiArticle rejects path-traversal attempts (../../../etc/passwd, ../package.json)
  • readWikiArticle returns null for nonexistent articles

Updated package.json test script to include the new file:

"test": "pnpm build && tsx --test tests/detectors.test.ts tests/wiki.test.ts"

All three new tests pass on Windows. Repro locally with pnpm test.

Manual repro on Windows

  1. codesight scan in any project
  2. Call codesight_get_wiki_index via the MCP tool
  3. Before: returns null (tool reports "wiki not found")
  4. After: returns the index content

Scope note

While running the suite I hit three pre-existing failures in tests/detectors.test.ts on main for Python workspace subdirectory detection. They are the same class of Windows path-separator issue but on the test-assertion side — they compare hardcoded services/foo strings against actual services\foo results. I deliberately left them out of this PR to keep it surgical and happy to send a follow-up if you'd like.

🤖 Generated with Claude Code

readWikiArticle hardcoded a POSIX forward-slash when verifying that
the resolved article path stays inside the wiki directory. On Windows,
path.resolve() returns backslash-delimited paths, so
resolved.startsWith(wikiDir + "/") always returned false and every
wiki article lookup returned null. This broke both
codesight_get_wiki_index and codesight_get_wiki_article MCP tools for
Windows users.

Switch to path.sep so the boundary check works cross-platform, and
add regression tests for the happy path, path-traversal rejection,
and missing-article fallback.

Co-Authored-By: Claude <noreply@anthropic.com>
Houseofmvps added a commit that referenced this pull request Apr 12, 2026
readWikiArticle hardcoded "/" in the path-traversal boundary check, causing
path.resolve() on Windows (which returns backslash paths) to always return null
for every wiki article lookup. Both codesight_get_wiki_index and
codesight_get_wiki_article MCP tools were broken for Windows users.

Fix: use path.sep so the check works cross-platform.
Add tests/wiki.test.ts with three regression tests covering: existing article
lookup (with/without .md extension), path-traversal rejection, and missing articles.

Co-authored-by: drewperd1000 <drewperd1000@users.noreply.github.com>
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
@Houseofmvps
Copy link
Copy Markdown
Owner

Applied manually in commit a0d9d97 alongside fixes for issues #21, #22, and #24. The fix is identical — path.sep instead of hardcoded "/" in the path-traversal guard. Also added the test suite from this PR (tests/wiki.test.ts), all 3 tests pass. Thanks for the clean, surgical fix and the note about the pre-existing Windows test failures — those are on the radar for a follow-up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants