feat(mcp): add zift mcp subcommand exposing scanner, prompt, and Rego primitives over stdio#21
feat(mcp): add zift mcp subcommand exposing scanner, prompt, and Rego primitives over stdio#21
Conversation
… primitives over stdio Implements PR 2 from plans/done/02-pr2-mcp-server.md — a Tier-1 deep-scan transport that lets agent hosts (Claude Code, Cursor, Continue, Cline, Zed, …) drive Zift over the Model Context Protocol. The agent host owns the model; Zift owns the authz expertise. Hand-rolled JSON-RPC 2.0 over stdio in src/mcp/jsonrpc.rs rather than pulling rmcp + tokio into a fully-blocking codebase. Pinned to MCP protocol version 2024-11-05. Tools (7): scan_authz, get_finding_context, list_rules, get_rule, suggest_rego, validate_rego, analyze_snippet — the last is the headline primitive: it returns the rendered prompt + JSON Schema so the agent host's model produces the response and Zift never sees it. Resources: prompt://system, prompt://schema, category://<slug>, rule://<id>. prompt://system and prompt://schema round-trip the canonical crate::deep::prompt constants verbatim — drift between deep-scan and MCP paths becomes a test failure. Tests: 35 unit tests across the mcp/ module plus 6 stdio integration tests in tests/mcp_stdio_integration.rs that spawn zift mcp as a subprocess and drive real pipes (catches accidental println! to stdout from anywhere on the call path).
… read loop, no-unwrap rule
- suggest_rego: build_template_vars now aliases the first extracted
literal under plan_value, perm_value, and permission, so rules using
those placeholders (e.g. ts-feature-gate-check, *-shiro-*, *-permissions)
render fully instead of leaving literal {{plan_value}} in the output.
New regression test asserts no `{{` survives.
- jsonrpc::read_request: loop over blank-line keepalives instead of
recursing, so a peer pumping blanks can't grow the stack.
- resolve_inside_scan_root / ensure_inside_scan_root: drop redundant
scan_root.canonicalize() on every call. The stored ctx.scan_root is
canonicalized once at startup; trust the invariant.
- ServerContext: remove dead rules_dir field (set but never read).
- Introduce a no-unwrap-in-production rule for the mcp module:
- Add clippy.toml with allow-unwrap-in-tests / allow-expect-in-tests.
- Add #![warn(clippy::unwrap_used)] to mcp/mod.rs with a docstring.
- Convert 8 production .unwrap() calls to .expect("…") with messages
documenting why each is infallible (5 in server.rs handler dispatch,
3 in resources.rs pretty-print).
All checks clean: cargo fmt, cargo clippy --all-targets -- -D warnings,
cargo test (253 + 18 + 6, +1 from the placeholder regression test).
📝 WalkthroughWalkthroughAdds a stdio JSON-RPC MCP (Model Context Protocol) server to Zift, exposing seven tools, resources (system/schema prompts, category taxonomy, per-rule URIs), protocol initialization, tool/resource listing and dispatch, CLI plumbing, and integration tests for stdio operation. Changes
Sequence Diagram(s)sequenceDiagram
participant Agent as Agent
participant Stdio as Zift MCP Server (stdio)
participant Dispatcher as Dispatcher
participant Tools as Tools Module
participant Resources as Resources Module
Agent->>Stdio: initialize (JSON-RPC)
Stdio->>Dispatcher: parse & handle initialize
Dispatcher-->>Stdio: initialize response
Agent->>Stdio: tools/list (JSON-RPC)
Stdio->>Dispatcher: dispatch -> list_tools()
Dispatcher->>Tools: list_tools()
Tools-->>Dispatcher: ToolsListResult
Dispatcher-->>Stdio: tools/list response
Agent->>Stdio: tools/call validate_rego (JSON-RPC)
Stdio->>Dispatcher: dispatch -> validate_rego
Dispatcher->>Tools: validate_rego(args)
Tools->>Tools: validate via embedded regorus
Tools-->>Dispatcher: ToolsCallResult
Dispatcher-->>Stdio: tools/call response
Agent->>Stdio: resources/list (JSON-RPC)
Stdio->>Dispatcher: dispatch -> list_resources()
Dispatcher->>Resources: list_resources(ctx)
Resources-->>Dispatcher: ResourcesListResult
Dispatcher-->>Stdio: resources/list response
Agent->>Stdio: resources/read (JSON-RPC)
Stdio->>Dispatcher: dispatch -> read_resource(uri)
Dispatcher->>Resources: read_resource(ctx, uri)
Resources-->>Dispatcher: ResourceContent
Dispatcher-->>Stdio: resources/read response
Agent->>Stdio: close stdin (EOF)
Stdio->>Stdio: clean shutdown
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
Review rate limit: 4/5 reviews remaining, refill in 12 minutes. Comment |
There was a problem hiding this comment.
This PR implements a comprehensive MCP (Model Context Protocol) server for Zift, exposing authz scanning primitives over JSON-RPC 2.0/stdio. The implementation is well-architected with proper security measures and extensive test coverage.
Key strengths:
- Proper path traversal protection via canonicalization checks in scan_root boundaries
- Clean separation between transport layer (MCP module) and business logic (scanner/deep modules)
- Comprehensive error handling with graceful degradation (continues serving after parse errors)
- No-unwrap policy enforced with
.expect()+ justification messages - Extensive test coverage (35 unit tests + 6 stdio integration tests)
- Protocol compliance verified with real subprocess integration tests
Test coverage highlights:
- JSON-RPC framing and error handling
- All 7 tools with path containment validation
- Resources (prompt://system, prompt://schema) round-trip canonical constants
- Subprocess integration catches stdout contamination regressions
The code is production-ready and follows Rust best practices throughout.
You can now have the agent implement changes and create commits directly on your pull request's source branch. Simply comment with /q followed by your request in natural language to ask the agent to make changes.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@README.md`:
- Line 127: The README's initialize smoke-test expectation is incorrect: it
asserts that "the seven tools are listed in the capabilities" though the
initialize response only contains capability flags; tool descriptors are
returned by the tools/list endpoint. Update the wording around the initialize
expectation (the sentence mentioning serverInfo.name == "zift") to state that
initialize returns capability flags (not tool descriptors), and add/clarify that
the seven tool descriptors are available from the tools/list API (referencing
tools/list) rather than from initialize.
In `@src/mcp/resources.rs`:
- Around line 43-50: The ResourceDescriptor for category URIs advertises
mime_type "text/plain" but read_resource returns ResourceContent with
"application/json", causing inconsistency; update the descriptor creation in the
loop over ALL_CATEGORIES (the ResourceDescriptor constructed with uri:
format!("category://{}", category_slug(*category)), name: format!("AuthCategory:
{category}"), description: category_description(*category).to_string(),
mime_type: "text/plain") to use mime_type "application/json" to match
read_resource's ResourceContent output (and ensure any consumers expecting JSON
are satisfied), or alternatively change read_resource's returned ResourceContent
mime_type to "text/plain" if the content is actually plain text — pick the JSON
option and make the mime_type values consistent between ResourceDescriptor and
read_resource.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d44ce492-94c6-4d09-89b0-48388689485b
📒 Files selected for processing (15)
README.mdclippy.tomlplans/done/02-pr2-mcp-server.mdplans/todo/00-deep-mode-overview.mdsrc/cli.rssrc/commands/mcp.rssrc/commands/mod.rssrc/lib.rssrc/mcp/jsonrpc.rssrc/mcp/mod.rssrc/mcp/protocol.rssrc/mcp/resources.rssrc/mcp/server.rssrc/mcp/tools.rstests/mcp_stdio_integration.rs
…IME type - README: clarify that `initialize` returns capability flags, not tool descriptors; tool list comes from `tools/list`. - resources.rs: align ResourceDescriptor mime_type for category:// URIs with the application/json content returned by read_resource.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/mcp/resources.rs (1)
119-223: Consider consolidating category metadata into a single static table.
ALL_CATEGORIES,category_slug,category_from_slug,category_description, andcategory_examplesare tightly coupled and can drift over time. A single metadata source would reduce maintenance risk.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/mcp/resources.rs` around lines 119 - 223, The current code duplicates category data across ALL_CATEGORIES, category_slug, category_from_slug, category_description, and category_examples which can drift; replace these with a single static metadata table (e.g., a static array/slice of a CategoryMeta struct containing fields: kind: AuthCategory, slug: &'static str, description: &'static str, examples: &'static [&'static str]) and update the functions ALL_CATEGORIES (or remove and derive it from the table), category_slug, category_from_slug, category_description, and category_examples to read from that table (lookup by AuthCategory or slug) so all metadata lives in one source of truth and the utilities simply index into it.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/mcp/resources.rs`:
- Around line 119-223: The current code duplicates category data across
ALL_CATEGORIES, category_slug, category_from_slug, category_description, and
category_examples which can drift; replace these with a single static metadata
table (e.g., a static array/slice of a CategoryMeta struct containing fields:
kind: AuthCategory, slug: &'static str, description: &'static str, examples:
&'static [&'static str]) and update the functions ALL_CATEGORIES (or remove and
derive it from the table), category_slug, category_from_slug,
category_description, and category_examples to read from that table (lookup by
AuthCategory or slug) so all metadata lives in one source of truth and the
utilities simply index into it.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b86f38c2-3f9a-43a5-b903-7b6bf060eded
📒 Files selected for processing (2)
README.mdsrc/mcp/resources.rs
Summary
Adds
zift mcp— Tier 1 deep-scan transport from the deep-mode plan. Runs Zift as an MCP (Model Context Protocol) server over stdio so any agent host (Claude Code, Cursor, Continue, Cline, Zed, …) can call Zift's authz primitives. The agent host owns the model; Zift owns the authz expertise — no LLM client lives in Zift on this path.Hand-rolled JSON-RPC 2.0 over stdio (~250 lines) instead of pulling in
rmcp— the codebase is fully blocking and forcing tokio fragmentation wasn't worth it. Protocol version pinned to2024-11-05. The MCP server is a true transport: every primitive (system prompt, output schema, candidate kind, context expansion, Rego rendering, regorus validator) is imported verbatim from the existingdeep::*/rego::*modules — no parallel implementations.Tools exposed
scan_authz--scan-root; returns findings + enforcement-point countget_finding_contextlist_rulesget_rulesuggest_regovalidate_regoregorusengineanalyze_snippetResources exposed
prompt://systemSYSTEM_PROMPTconstant (round-tripped verbatim — asserted in tests)prompt://schemaoutput_schema()JSON Schemacategory://<auth_category>rule://<rule_id>Notable decisions
--scan-rootand rejects results outside it. Same defense asdeep::expand_finding.prompt://system/prompt://schemaround-trip the canonical constants verbatim — drift between the deep-scan path and the MCP path becomes a test failure, not silent inconsistency.validate_regouses embeddedregorus— the dep was already present, so zero-install UX was free.scan_authzpayload fits comfortably in one response; revisit if multi-thousand-finding scans become a real workload.mcpmodule (introduced after self-review):clippy.tomlwithallow-unwrap-in-tests = true, plus#![warn(clippy::unwrap_used)]onmcp/mod.rs. Production code uses.expect("…")with messages documenting why each is infallible.Tests
src/mcp/*(jsonrpc framing, server dispatch, every tool, every resource).tests/mcp_stdio_integration.rsthat spawnzift mcpas a subprocess and drive the protocol over real pipes — catches the class of regressions in-process tests can't (e.g. an accidentalprintln!to stdout from anywhere on the call path, or a tracing subscriber that writes to stdout instead of stderr).Test plan
cargo fmt --checkcargo clippy --all-targets -- -D warningscargo test— 253 lib + 18 + 6 (mcp_stdio_integration), all passingtools/listinteractivelySummary by CodeRabbit
New Features
Documentation
Tests