Skip to content

test(rules): mirror and cover MCP-029 - #172

Closed
bradAGI wants to merge 1 commit into
trustabl:mainfrom
bradAGI:feat/mcp-ts-path-safety
Closed

bradAGI wants to merge 1 commit into
trustabl:mainfrom
bradAGI:feat/mcp-ts-path-safety

Conversation

@bradAGI

@bradAGI bradAGI commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Engine half of a coordinated pair. Rules half: trustabl/agent-reliability-rules#94, on a branch of the same name, so the rules-sync job resolves the matching pack rather than main. Neither half should merge alone — check-rules-sync.sh fails if they do.

What the pair adds

MCP-005 covers the Python path-safety case; the TypeScript half was missing. MCP-029 mirrors CSDK-012 including its coarse-signal caveat, stated in the explanation: it flags any filesystem write, not only unnormalized paths, because TS path-normalization analysis isn't wired yet. Confidence 0.5 to match.

Deployment is what sharpens this for MCP. A stdio server is launched as a subprocess by whatever client the user is running, so it inherits that user's own filesystem permissions — a write escaping its intended directory reaches their home directory, dotfiles, and SSH keys. And the server can't see the injection: it receives a well-formed tools/call for a path it has no way to distinguish from a legitimate one, so containment has to be structural.

What this PR does

  1. Mirrors mcp/path_safety.yaml into testdata/rules-fixture/.
  2. Adds a fire case and a silent case to policyRuleCases, as TestPolicyRules_AllRulesCovered requires.

The silent case applies the remediation the fix text prescribes for the common shape — derive the filename server-side rather than accepting a path over the protocol — instead of merely deleting the write.

Verification

$ RULES_REPO=../trustabl-rules scripts/check-rules-sync.sh
rules fixture is in sync with production (86 files compared)

$ go vet ./internal/rules/
$ go test ./internal/rules/
ok  	github.com/trustabl/trustabl/internal/rules

@jhumel-code

Copy link
Copy Markdown
Collaborator

Thanks @bradAGI. Pairing the fixture mirror with the rules half on a same-named branch is the right way to do this, and the mirrored block in testdata/rules-fixture/mcp/path_safety.yaml is byte identical to trustabl/agent-reliability-rules#94, so rules-sync has nothing to complain about once both land.

The test cases are real rather than vacuous. Both snippets import from @modelcontextprotocol/sdk/server/mcp.js and bind s to new McpServer(...), so DiscoverTSMCPProper genuinely produces a KindMCPTool in each case. The fire case's writeFileSync is one of the callees tsHandlerFacts maps to writes_fs, and the silent case still discovers a tool but sets no fact, so it exercises the predicate rather than the import gate. Writing the silent case as the remediation the fix text prescribes, instead of just deleting the write, is a nice touch.

Two things. The diff also deletes a blank line at the end of policyRepoRuleCases, which is unrelated to this change and will conflict with other open PRs touching that file, so please drop it. And MCP-029 is contested: #124 and #135 both claim it for different rules. Main stops at MCP-022, so renumbering is cheap for whoever lands second.

Happy to take this once those two are handled.

@sairenchristianbuerano

Copy link
Copy Markdown
Collaborator

Selected for merge. This pair keeps MCP-029.

Closing trustabl/agent-reliability-rules#80 (PHP typed parameters) and trustabl/agent-reliability-rules#100 (PHP idempotency), which claimed the same id for different rules.

Several PRs claimed the same ids independently. We are resolving each id to a single pair rather than renumbering, so the competing PRs are being closed — the rules themselves were not rejected.

Merge the engine side first so a gap fails safe: a fixture for an unshipped rule is harmless, a rule with no fixture turns rules-sync red.

Engine half of a coordinated pair with trustabl/agent-reliability-rules#94, on a
branch of the same name so the rules-sync job resolves the matching pack
rather than main. Neither half should merge alone — check-rules-sync.sh
fails if they do.

Mirrors mcp/path_safety.yaml into testdata/rules-fixture and adds a fire
and a silent case to policyRuleCases, as TestPolicyRules_AllRulesCovered
requires.

MCP-005 covers the Python path-safety case; the TypeScript half was
missing. The silent case applies the remediation the fix text prescribes
for the common shape — derive the name server-side rather than accepting a
path over the protocol — instead of merely removing the write.
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.

3 participants