fix(security): harden MCP execution and proxy boundaries - #8
Conversation
There was a problem hiding this comment.
Deterministic Validator approval for exact head 7effe67d0fde1101277f8b74458fa2487c30d179.
Ticket: PLF-2216
Correlation ID: PLF-2216-mcp-safety-skills-v2
Model: zai/glm-5.3
Reviewed diff chunks: 6
Advisory LLM verdict: REQUEST_CHANGES
Advisory summary: Reviewed all 6 diff chunk(s). The provided PR chunk contains no reviewable diff, logs, or metadata (content appears redacted/empty). Without visible code or test results, approval cannot be granted. | The visible content consists of a single redacted environment variable line with no executable code, workflow, or secret exposure to assess. | Chunk 3 of 6: git2mcp proxy hardening — capability gates (mutation/execute/remote-write) via env flags, path traversal validation for stage/checkpoints, ref validation, safe tar import via manager, URL credential redaction, run-tests timeout bounds. Good regression tests for traversal and disabled-by-default capabilities. | Chunk adds capability-gating env checks on mutating endpoints, path validation via _repo_path/_validate_git_arg, timeouts and output truncation on run-tests, token redaction in clone errors, and resets the git remote URL to the unauthenticated form after clone. The skills server adds sync/execute gates, path-confined repo resolution, and hardened tar extraction that rejects symlinks/hardlinks/devices and validates members against the target root. These are security-hardening improvements. | Chunk shows solid security hardening in mcp-skills: path traversal mitigated via _resolve_repo_path/_resolve_descendant, safe tar extraction replacing extractall, operator capability gates (_require_sync/_require_execute) defaulting to disabled with 403 handler, and matching regression tests including archive traversal rejection. WebUI gains capability-gated mutations. | Chunk adds capability gating (_require_capability) to mutating/execute/secret-write WebUI endpoints, removes token from CLI fetch results and prefill (reducing token exposure), and adds security regression tests asserting deny-by-default behavior. Changes are consistent and low-risk.
Advisory findings: No diff content or test result data was provided; the change cannot be semantically assessed.
The LLM output above is advisory and was not used as the approval trust root.
Merge will be attempted after this approval when explicitly authorized.
Decision record (recomputable)
DECISION D-216-1027
TICKET PLF-2216
HEAD_SHA 7effe67d0fde1101277f8b74458fa2487c30d179
CORRELATION_ID PLF-2216-mcp-safety-skills-v2
ACTOR agent:ifuri-validator-agent[bot]
APPLIED_RULE P-CORE-015
INPUT author_login = "tom-sapletta-com"
INPUT observed_checks = ["verify=PASS"]
INPUT required_checks = ["verify"]
INPUT required_checks_source = "protected registry (env/request)"
INPUT reviewer_login = "ifuri-validator-agent[bot]"
INPUT superseded_checks = []
VERDICT APPROVE AUTHORITY DETERMINISTIC
REJECTED REQUEST_CHANGES BECAUSE NO_UNSAFE_CHANGE_REASON_FOUND
ADVISORY llm_verdict = "REQUEST_CHANGES" MODEL "zai/glm-5.3"
ASSERT VERDICT_AUTHORITY != "ADVISORY"
Summary
Validation
Hosted verify must validate the exact branch head before merge. The branch is currently behind main; reconciliation must preserve both security commits and rerun checks. Independent Validator approval remains required.
Ticket: PLF-2216
Correlation ID: PLF-2216-mcp-safety-skills