Worktree fix+compound value emission#144
Merged
mrdailey99 merged 4 commits intodevelopfrom May 6, 2026
Merged
Conversation
… in strings
Req: When {VarName} was mixed with surrounding literal text in a step argument, the generator emitted a plain valueClass="string" element; Provar runtime passes these tokens literally rather than resolving the variable reference. The validator also lacked a rule for this compound-variable pattern.
Fix: Add buildCompoundValue() in testCaseGenerate to emit class="compound"/<parts> when a string mixes literals and {VarName} tokens; split VAR-REF-001/002 in testCaseValidate to detect both pure and embedded variable misuse.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…eployManual, and testPlanTools Req: PR 143 review identified three bugs: pathPolicy containment check produced double separators for root paths (e.g. C:\ or //), DeployManual used tail -1 on a descending-sorted tag list so it selected the oldest tag instead of newest, and testPlanTools did not call assertPathAllowed on test_case_path before file access allowing directory traversal. Fix: Strip trailing separator before prefix check in pathPolicy; change tail -1 to head -1 in DeployManual; add assertPathAllowed(absoluteTestCasePath) in testPlanTools; add path traversal unit test covering the escape-via-dotdot case. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR tightens MCP filesystem safety for test plan operations, improves Windows path-policy behavior, and adds support for emitting/validating compound XML values when {VarName} tokens are embedded within larger strings.
Changes:
- Enforce
assertPathAllowedforprovar_testplan_add-instance.test_case_pathand add a unit test covering path escape attempts. - Add compound value generation (
class="compound"with<parts>) for embedded{VarName}tokens, plus new validator ruleVAR-REF-002and accompanying tests. - Update path policy docs/tests for Windows case-insensitive containment and expand CI to include Windows + a unit test step.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/mcp/tools/testPlanTools.ts |
Adds path-policy enforcement for test_case_path before filesystem access. |
test/unit/mcp/testPlanTools.test.ts |
Adds a unit test ensuring escaping test_case_path is rejected. |
src/mcp/security/pathPolicy.ts |
Makes containment checks case-insensitive on Windows to avoid drive-letter casing mismatches. |
test/unit/mcp/pathPolicy.test.ts |
Adds Windows-only tests for case-insensitive path comparison behavior. |
src/mcp/tools/testCaseGenerate.ts |
Emits class="compound" XML for strings with embedded {VarName} tokens. |
test/unit/mcp/testCaseGenerate.test.ts |
Adds tests verifying compound emission for embedded variables/system vars and preserves pure-variable behavior. |
src/mcp/tools/testCaseValidate.ts |
Adds VAR-REF-002 for embedded variables in string values; refactors VAR-REF-001 detection. |
test/unit/mcp/testCaseValidate.test.ts |
Adds tests for VAR-REF-002 and non-firing cases (pure var / correct compound). |
docs/mcp.md |
Documents Windows case-insensitive path comparisons. |
docs/mcp-pilot-guide.md |
Documents Windows case-insensitive path comparisons. |
.github/workflows/CI_Execution.yml |
Adds Windows to the matrix and runs unit tests during CI. |
.github/workflows/DeployManual.yml |
Refines previous-tag detection and filters changelog commit subjects more aggressively. |
package.json |
Bumps version to 1.5.0-beta.16. |
server.json |
Bumps version to 1.5.0-beta.16 and reformats runtime arguments. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
262
to
265
| // Resolve testcase absolute path and enforce path policy before any fs access | ||
| const absoluteTestCasePath = path.join(projectRoot, test_case_path); | ||
| assertPathAllowed(absoluteTestCasePath, config.allowedPaths); | ||
| if (!fs.existsSync(absoluteTestCasePath)) { |
| sf plugins link . | ||
| yarn prepack | ||
| - name: Unit tests | ||
| run: node node_modules/.bin/mocha "test/**/*.test.ts" --timeout 30000 |
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>
…ath.join normalizes '..' away Req: path.join(projectRoot, test_case_path) normalizes '..' segments before assertPathAllowed inspects the path, so a traversal input like '../escape.testcase' bypassed the PATH_TRAVERSAL check and was only caught by containment — meaning it went undetected entirely when allowedPaths is empty (unrestricted mode). Fix: Explicitly reject '..' segments in normalizedTestCasePath before calling path.join(), ensuring PATH_TRAVERSAL is raised regardless of allowedPaths configuration; update the unit test to use an unrestricted server (allowedPaths: []) to prove the fix covers the unrestricted-mode gap. 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.
No description provided.