Copilot MCP sanctioning: reliable server identification + telemetry

[anonpran]

Summary

Makes Copilot MCP-server identification reliable so Copilot MCP tool calls resolve to their server and reach the gateway's sanction check the same way Claude Code & Cursor already do — plus telemetry to measure the residual. Purely progressive: no enforcement-direction change — an unidentifiable tool still returns {} (allow) exactly as before.

Copilot emits sanitized <server>-<tool> tool names (no mcp__ prefix, no structured server field), so the hook must reverse-engineer the server from local MCP config. When it resolves, the gateway makes the identical allow/deny decision it makes for CC/Cursor. This PR maximizes how often it resolves.

Changes (all in copilot/hooks/unbound.py + tests)

• Identification: honor COPILOT_HOME for the CLI config dir (expanduser only); parse the flat top-level server form; fall back to the SessionStart-logged cwd when PreToolUse omits it (and log SessionStart so the fallback works — also fixes a latent get_session_start_model miss); defensive structured-field read (future-proof, dead today).
• Telemetry: per-surface (vscode/cli) resolution categories — resolved / unresolved_with_config / unresolved_no_config — names only, over a separate rate-limit budget so it can't suppress real error/bypass reports.

Deliberately NOT in this PR (deferred)

The fail-secure DENY for unidentifiable-but-MCP-configured calls is intentionally not here. It needs (a) a gateway mcp_sanctioning_active signal so it never over-blocks orgs with no allow-list configured, and (b) a telemetry soak first. It lands in a follow-up (hook deny + gateway signal) once unresolved_with_config is measured ~0.

Known residual (tracked, not closed here)

Until that follow-up, an unidentifiable MCP call on Copilot still allows (the pre-existing bypass). This PR shrinks it (better identification) and measures it; it does not close it.

Test plan

• 33 integration tests at the process_pre_tool_use/main() layer: both surfaces, cwd via event + SessionStart fallback, COPILOT_HOME, flat-form (+ inputs exclusion), infra-vs-identity, never-crash, telemetry redaction + budget isolation, SessionStart-preservation cleanup.

Touches the policy-enforcement hook + adds control-plane telemetry (names-only) — worth a SOC-2-aware glance, though no deny decisions are added.

🤖 Generated with Claude Code

Greptile Summary

This PR hardens Copilot MCP-server tool identification in copilot/hooks/unbound.py to fix a silent bypass where unresolved tools were allowed without any observable signal. It adds cwd fallback via SessionStart audit-log records, COPILOT_HOME support, flat top-level server-form parsing, a separate rate-limit budget for resolution telemetry (so routine resolved/unresolved events can never starve genuine error reports), and cleanup_old_logs preservation of the SessionStart record so the fallback survives log rotation.

• Identification hardening: detect_mcp_call now receives a cwd recovered from the session's logged SessionStart when the PreToolUse event omits it; COPILOT_HOME redirects the CLI config directory; flat top-level JSON form is parsed with a _FLAT_FORM_NON_SERVER_KEYS exclusion list to prevent VS Code metadata blocks from being misread as servers.
• Telemetry: Per-surface (vscode/cli) resolution categories (resolved, unresolved_no_config, unresolved_with_config) use a dedicated throttle file (LAST_RESOLUTION_REPORT_FILE) that is entirely separate from the error/bypass budget.
• Behavior on merge: All unresolved tools still return {} (allow) — the PR description and rollout plan reference a UNBOUND_COPILOT_MCP_DENY_UNRESOLVED fail-secure gate, but that flag and deny path are not present in the committed code.

Confidence Score: 3/5

Safe to merge in terms of immediate runtime behavior (all unresolved tools continue to be allowed, no new denies), but the PR description and rollout plan describe a deny gate that does not exist in the code — a SOC 2 approver would be acting on an inaccurate description of what enforcement capability is present.

The identification and telemetry changes are well-engineered and tested. However, the PR title, description, and rollout plan all promise a UNBOUND_COPILOT_MCP_DENY_UNRESOLVED fail-secure gate that is entirely absent from the committed code. For a change explicitly tagged SOC 2 with a named-approver requirement, the disconnect between the stated security posture and the actual implementation is a meaningful gap — the team would believe they can flip a flag to enable deny behavior in step 3 of the rollout when no such flag exists.

copilot/hooks/unbound.py — specifically process_pre_tool_use around the unresolved-tool return path, where the described deny gate is absent

Security Review

• Missing deny-gate implementation (copilot/hooks/unbound.py): The PR description and rollout plan state that UNBOUND_COPILOT_MCP_DENY_UNRESOLVED is implemented (OFF by default) and can be flipped ON. The flag does not exist in the code; all unresolved MCP tool calls continue to pass through unconditionally. A SOC 2 approver relying on the description to understand what deny capability is being deployed would have an inaccurate picture of the current enforcement posture.

Important Files Changed

Filename	Overview
copilot/hooks/unbound.py	Core policy-enforcement hook: adds MCP tool identification (cwd fallback, flat-form parsing, COPILOT_HOME, structured-server forward-compat), separate resolution telemetry budget, and cleanup_old_logs SessionStart preservation — but the UNBOUND_COPILOT_MCP_DENY_UNRESOLVED fail-secure gate described in the PR is absent from the code
copilot/hooks/test_pretool_mcp.py	New integration test suite (584 lines, 38 tests): covers both surfaces, cwd-event and SessionStart-cwd fallback, COPILOT_HOME, flat-form, known-builtin suppression, infra-vs-identity separation, never-crash, and budget isolation with real throttle-file I/O — no TestFailSecureFlagOn class (consistent with the deny gate not being implemented)

_{Reviews (2): Last reviewed commit: "Reduce scope to identification + telemet..." | Re-trigger Greptile}

Greptile also left 1 inline comment on this PR.

[vigneshsubbiah16]

🛡️ Automated Security Review (consensus)

4 findings — 1 high-confidence, 3 to triage. Reviewers: Cursor, Claude, Semgrep, Gitleaks.

---

🔴 Residual MCP policy bypass until kill-switch enabled

copilot/hooks/unbound.py (DENY_UNRESOLVED_MCP, process_pre_tool_use, _sanitize_copilot_server_name, detect_mcp_call)

Impact: Unidentified MCP tools on MCP-configured hosts still return {} (allow) and skip gateway policy checks by default; unverified server-name sanitization can widen the gap so genuine MCP calls fail to resolve to mcp__server__tool.
Fix: Track copilot_mcp_unresolved_with_config during soak, validate prefix derivation against Copilot fixtures, then enable UNBOUND_COPILOT_MCP_DENY_UNRESOLVED in production (or default ON once soak is clean).
Flagged by: Cursor, Claude

---

🟡 Overly permissive file permissions ($BITS)

copilot/hooks/unbound.py:1526

Impact: World/group-writable or overly broad modes on hook state files could let other local users tamper with throttle/audit data.
Fix: Tighten to owner-only where writes are needed (e.g. 0o600); use 0o644 only when shared read is intentional.
Flagged by: Semgrep

---

🟡 Overly permissive directory/file mode (0o755)

copilot/hooks/unbound.py:1745

Impact: 0o755 exposes created paths to group/other on multi-user hosts; may be broader than required for hook runtime artifacts.
Fix: Prefer 0o700 for directories and 0o600/0o644 for files unless cross-user read is required.
Flagged by: Semgrep

---

🟡 COPILOT_HOME path uses shell env expansion

copilot/hooks/unbound.py (_copilot_cli_config_dir)

Impact: os.path.expandvars on COPILOT_HOME resolves $VAR from the process environment before reading MCP config, adding unnecessary path-resolution surface in attacker-influenced launch contexts.
Fix: Use expanduser on the literal value only; drop expandvars unless variable interpolation is a documented requirement.
Flagged by: Claude

---

Clean scans: Gitleaks — no secrets detected. No new injection, IDOR, or sensitive-data-in-telemetry issues identified (telemetry is name-only; config reads are size-capped and exception-safe).

---

🤖 consensus review · reviewers: Cursor,Claude,Semgrep,Gitleaks · head e9316ca1 · 2026-06-19T19:11Z

[anonpran]

Updated in bbf7209 — scope reduced to identification + telemetry only per this review (the gated DENY is removed and deferred to a follow-up). Responses:

@greptile-apps P1 — denied_unresolved telemetry throttled: ✅ Resolved. Component 2 (the gated DENY) is removed, so copilot_mcp_denied_unresolved no longer exists; exactly one resolution-telemetry call now fires per invocation (resolved XOR unresolved_with_config XOR unresolved_no_config) — the shared-budget self-throttle is structurally gone.

@vigneshsubbiah16 🔴 Residual MCP policy bypass until kill-switch enabled: Acknowledged, deliberately deferred (not closed here). Removing the default-OFF deny also removed an over-block risk we found: the hook's deny couldn't tell whether the org even has an MCP allow-list configured, so flag-ON would have over-blocked unresolved calls in non-sanctioning orgs (the gateway allows them — isMcpServerSanctioned → applies:false). The deny will land in a follow-up that adds a gateway mcp_sanctioning_active signal so it denies only when an allow-list is active. This PR shrinks the bypass via better identification and adds telemetry to measure the residual first.

🟡 COPILOT_HOME expandvars: ✅ Fixed — expanduser on the literal only; expandvars dropped.

🟡 file modes (~1526 / ~1745): Pre-existing — git diff origin/main adds zero file-mode lines; those 0o755/chmod calls live in _replace_self / _dispatch_discovery, untouched by this PR. Left out of scope.

[greptile-apps]

Thanks for the detailed breakdown — reviewed bbf7209, all items are addressed:

P1 (budget self-throttle) ✅ — structurally gone. With the DENY component removed, exactly one _report_mcp_resolution call fires per invocation (resolved XOR unresolved_with_config XOR unresolved_no_config). The separate LAST_RESOLUTION_REPORT_FILE budget + TestResolutionTelemetryBudgetIsolation exercising the real throttle-file logic is solid — that test class is the right place to catch this class of regression.

COPILOT_HOME (expandvars) ✅ — _copilot_cli_config_dir() uses os.path.expanduser(override) only. Confirmed.

Residual bypass / deferred deny ✅ — the rationale holds: the hook has no way to distinguish "unresolved on a sanctioning org" from "unresolved on an org where isMcpServerSanctioned returns applies:false", so a default-OFF deny was the right call to defer. The unresolved_with_config soak gives you the signal to gate the follow-up safely.

File modes — pre-existing / out of scope, accepted.

One minor observation (non-blocking): _reporting_error is a shared global that gates both error reports and resolution telemetry through the same report_error_to_gateway function. In single-threaded Python this is fine and the finally reset is correct — just worth a note if this hook ever gets parallelized.

Code is clean and the 33-test suite covers the right surface. LGTM. 🟢

Tip: You can customize Greptile's behavior for this repo with .greptile/rules.md and .greptile/config.json.

[vigneshsubbiah16]

🛡️ Automated Security Review (consensus)

2 findings — 1 high-confidence, 1 to triage. Reviewers: Cursor, Claude, Semgrep, Gitleaks.

---

1. Policy identity minted from unvalidated client event fields

copilot/hooks/unbound.py:1014 · 🔴 HIGH

Impact: The forward-compat structured_server branch builds mcp__<server>__<tool> from client-supplied mcp_server/serverName/server and tool keys without requiring a configured MCP server match (mcp_server_config may be None while is_mcp is still set), enabling policy-decision spoofing if this path becomes reachable.

Fix: Only trust structured server fields when they resolve to a known entry in mcp_servers; otherwise fall through to heuristic/unresolved handling. Add a test that an unknown structured_server does not produce a trusted canonical identity.

Flagged by: Claude, Cursor

---

2. Overly permissive file permissions on hook-local files

copilot/hooks/unbound.py:1526, copilot/hooks/unbound.py:1745 · 🟡 TRIAGE

Impact: Wide permission bits ($BITS / 0o755) may expose hook state or logs to other local users on shared machines.

Fix: Tighten to least privilege (e.g. 0o644 for files, 0o700 for sensitive dirs) where writes are not required by other principals.

Flagged by: Semgrep (full-file scan; lines appear outside this PR’s diff hunks — confirm whether introduced here or pre-existing)

---

Clean scans: Gitleaks — no secrets detected. Telemetry paths redact args (names-only, JSON-encoded) and use a separate throttle budget so routine MCP resolution telemetry cannot suppress error/bypass reports.

---

🤖 consensus review · reviewers: Cursor,Claude,Semgrep,Gitleaks · head bbf72094 · 2026-06-19T19:34Z

[greptile-apps]

Fail-secure flag described in PR but not implemented

The PR description, title, and rollout plan all claim UNBOUND_COPILOT_MCP_DENY_UNRESOLVED is implemented (defaulted OFF) so the team can "flip it ON" in step 3. The code here unconditionally returns {} for unresolved tools with the comment "no deny is issued here" — the env-var gate, the copilot_mcp_denied_unresolved telemetry category, and the deny path referenced in the rollout plan do not exist anywhere in this file.

This is a direct conflict: a named SOC 2 approver reading the PR description would believe a reversible deny gate is available, but step 3 of the rollout ("Flip UNBOUND_COPILOT_MCP_DENY_UNRESOLVED on") would silently do nothing. Either the description needs to be updated to reflect that the deny gate was deferred (and the rollout plan rewritten), or the flag implementation needs to be added back before merge.