v1.5.0-beta.16 fix(mcp): Windows path case-insensitivity + CI hardening#143
Merged
mrdailey99 merged 13 commits intodevelopfrom May 6, 2026
Merged
v1.5.0-beta.16 fix(mcp): Windows path case-insensitivity + CI hardening#143mrdailey99 merged 13 commits intodevelopfrom
mrdailey99 merged 13 commits intodevelopfrom
Conversation
RCA: The git log extraction picked up all feat/fix commits including internal process commits (PR review addressing, merge conflict resolution, CI tooling, and implementation details that are meaningless to end users). Fix: Add two extra grep filters — one to strip (ci)-scoped commits entirely, and one deny-list for patterns that indicate purely internal work: review comments, merge conflicts, feedback items, pre-landing fixes, session references, technical implementation tokens (testCasePath, planitem, uuid lookup, buildTestCase, server.tool, registerTool, awk regex). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…se notes RCA: git tag --sort=-creatordate | head -1 picks the most recently created tag regardless of branch ancestry. When a dispatch runs from a branch that diverged before the last tag was cut on develop/main, the range includes commits already shipped in the previous release. Fix: Replace with git describe --tags --abbrev=0 which walks the ancestry from HEAD and returns only a tag that is a true ancestor, guaranteeing the range only covers commits introduced since the last release on this line. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…etters RCA: On Windows, fs.realpathSync does not canonicalise drive-letter case, so assertPathAllowed failed when Copilot sent lowercase c:\ paths but allowed paths used uppercase C:\. Fix: Normalise both sides of the path comparison to lowercase on win32; Linux/macOS behaviour is unchanged. Four regression tests added to pathPolicy.test.ts. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
RCA: In bash, using || "" attempts to execute an empty string as a command, which fails with "command not found" in strict environments and produces noisy stderr output in CI logs.
Fix: Replace || "" with || true so PREV is empty string when no ancestor tag exists; the ${PREV:+${PREV}..}HEAD range expansion already handles the empty case correctly.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…to CI matrix RCA: Three issues in CI workflows — $RANGE unquoted in git log allows shell injection via crafted tag names; dot.notation in grep filter was an unescaped regex matching any character; unit tests never ran in CI so Windows path-policy regression tests had zero automated coverage. Fix: Quote "$RANGE" in DeployManual.yml git log call; escape dot\.notation in grep -Eiv pattern; add windows-latest to CI_Execution.yml default OS matrix and add a unit test step before smoke tests so all platform-specific code paths (including Windows case-insensitive path comparison) run on every PR. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
RCA: CI improvements and Windows path-policy fix constitute a release-worthy change set that requires a new beta version. Fix: Increment beta suffix from .15 to .16 in package.json and server.json; both top-level version and packages[0].version updated to stay in sync. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…urity model RCA: The pathPolicy change (case-insensitive comparison on win32) was undocumented in the security model sections of mcp.md and mcp-pilot-guide.md, leaving users and integrators with no explanation of why c:\ and C:\ paths are treated as equivalent. Fix: Add one paragraph to each doc's path-policy section explaining the Windows case-insensitive behavior and the fs.realpathSync limitation that motivates it. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the MCP path security policy to treat Windows paths case-insensitively (to avoid false PATH_NOT_ALLOWED errors due to drive-letter casing), and hardens release/CI workflows to reduce brittleness and improve Windows coverage.
Changes:
- Update
assertPathAllowedto compare resolved paths case-insensitively on Windows, plus add targeted Windows regression tests. - Harden the manual deploy workflow changelog extraction (quoting, safer tag lookup, and regex fixes) and expand grep filters for noisy commits.
- Expand CI execution matrix to include
windows-latestand add a unit test step.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/mcp/security/pathPolicy.ts |
Case-insensitive containment comparison on Windows when checking resolved paths against allowed roots. |
test/unit/mcp/pathPolicy.test.ts |
Adds Windows-only regression tests to validate drive-letter/directory casing behavior. |
.github/workflows/DeployManual.yml |
Hardens changelog extraction (range quoting, safer previous-tag resolution, regex fixes/filters). |
.github/workflows/CI_Execution.yml |
Adds windows-latest to matrix and introduces a unit-test step in CI. |
docs/mcp.md |
Documents Windows case-insensitive path comparison behavior in the security model section. |
docs/mcp-pilot-guide.md |
Documents Windows case-insensitive path comparison behavior in the security model section. |
package.json |
Bumps package version to 1.5.0-beta.16. |
server.json |
Bumps registry metadata versions to 1.5.0-beta.16 and reformats runtime arguments. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| !resolvedAllowed.some((base) => resolved === base || resolved.startsWith(base + path.sep)) | ||
| !resolvedAllowed.some((base) => { | ||
| const baseKey = normalizeForCompare(base); | ||
| return resolvedKey === baseKey || resolvedKey.startsWith(baseKey + path.sep); |
| sf plugins link . | ||
| yarn prepack | ||
| - name: Unit tests | ||
| run: node node_modules/.bin/mocha "test/**/*.test.ts" --timeout 30000 |
Comment on lines
+68
to
+73
| # Release event: HEAD is the new tag — find the nearest ancestor tag before it | ||
| PREV=$(git describe --tags --abbrev=0 HEAD^ 2>/dev/null || git tag --sort=-version:refname | tail -1) | ||
| else | ||
| # For a manual dispatch use the most recent existing tag | ||
| PREV=$(git tag --sort=-creatordate | head -1) | ||
| # Manual dispatch: find the nearest ancestor tag from HEAD | ||
| # (git describe respects branch ancestry; avoids pulling in commits from sibling branches) | ||
| PREV=$(git describe --tags --abbrev=0 HEAD 2>/dev/null || true) |
…alization RCA: Two pre-existing bugs were exposed by adding macOS to CI matrix and the unit test step. On macOS, os.tmpdir() returns /var/... which is a symlink to /private/var/...; the path policy fallback only tried one parent level and fell back to path.resolve() which does not follow symlinks, causing allowed-path comparisons to fail for non-existent output paths. Separately, testPlanTools built absolute paths with path.join() before normalizing backslashes, so Windows-style paths from test fixtures resolved incorrectly on macOS/Linux where backslash is a literal character. Fix: Replace single-level parent fallback with an ancestor walk-up loop in assertPathAllowed so realpathSync is called on the deepest existing ancestor, preserving symlink resolution. Hoist normalizedTestCasePath (using existing toForwardSlashes helper) to before path.join() in testPlanTools add-instance handler so backslashes are normalized before any path ops. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ol descriptions RCA: Three tools that generate or edit XML test steps (provar_testcase_generate, provar_testcase_step_edit, provar_testcase_validate) lacked the fallback instruction to consult the provar://docs/step-reference MCP resource when the corpus API returns count:0 with a warning field. Agents calling these tools directly (outside a loop prompt) had no guidance on where to look for step structure when corpus is unavailable. Fix: Add the standard corpus fallback sentence to all three tool descriptions, matching the pattern already used in loopPrompts.ts and migrationPrompts.ts. Update MockMcpServer/CapturingServer in the three test files to capture descriptions, and add one describe block per tool verifying the step-reference URI appears. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…compatibility RCA: node_modules/.bin/mocha is a bash shebang script on all platforms. Calling it as `node node_modules/.bin/mocha` works on Unix because npm adds a proper launcher, but on Windows Node.js tries to parse the bash script as JavaScript and throws SyntaxError. CI started failing on the windows-latest runner as soon as the unit tests step was added to the matrix. Fix: Replace `node node_modules/.bin/mocha` with `npx mocha`. npx resolves the platform-correct entry point automatically: .cmd wrapper on Windows, bash script on Unix. Uses the locally installed version (no registry download). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…tion RCA: Tools lacked compound XML generation and embedded variable detection Fix: Add buildCompoundValue() to testCaseGenerate and VAR-REF-002 rule to testCaseValidate with full test coverage (888 passing) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…le strings RCA: docs/mcp.md lacked a VAR-REF-002 entry after the rule was added to the validator; CLAUDE.md requires new error codes to be documented. Fix: Add VAR-REF-002 bullet to the testcase validate error code section explaining compound XML usage and provar_testcase_generate auto-handling. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Collaborator
Author
|
@copilot none of the PR review comments from the re-review posted |
mrdailey99
added a commit
that referenced
this pull request
May 6, 2026
Req: PR #143 was merged to develop after this branch diverged, bringing in compound value generation, VAR-REF-002 validation, macOS symlink walk-up loop in pathPolicy, and backslash normalization before path.join in testPlanTools. This branch had duplicate feature code and conflicting path handling that broke the backslash normalization test on Linux CI. Fix: Merge develop to absorb duplicated feature commits; resolve testPlanTools conflict by combining develop's toForwardSlashes normalization with this branch's assertPathAllowed on absoluteTestCasePath; take develop's testCaseValidate.test.ts to eliminate duplicate VAR-REF-002 tests. Co-Authored-By: Claude Sonnet 4.6 <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.
Summary
MCP path security & CI hardening:
pathPolicy.tsrealpathSyncis called on the deepest existing ancestor$RANGE, fix dot-notation regex in Slack release notes, add unit tests to CI matrixMCP tool description completeness:
provar://docs/step-referenceMCP resource fallback guidance toprovar_testcase_generate,provar_testcase_step_edit, andprovar_testcase_validatetool descriptionscount:0with a warning field (zero token cost on happy path)Docs:
mcp.mdandmcp-pilot-guide.mdTest Coverage
877 unit tests passing (0 failures). 3 new
describeblocks added to testcase tool test files verifyingprovar://docs/step-referenceURI appears in each tool description.Pre-Landing Review
Prior review (2026-05-06, commit 346d50c) — CLEAR. 4 commits since review are low-risk: version bump, docs, macOS symlink fix, description string additions.
Test plan
🤖 Generated with Claude Code