Resolve Copilot CLI & VS Code built-in/hosted MCP identity (sanction/policy coverage)#181
Resolve Copilot CLI & VS Code built-in/hosted MCP identity (sanction/policy coverage)#181anonpran wants to merge 1 commit into
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 86c3704. Configure here.
| parsed_server, _, parsed_tool = body.partition('_') | ||
| if parsed_server and parsed_tool: | ||
| mcp_server, mcp_tool = parsed_server, parsed_tool | ||
| canonical = f"mcp__{mcp_server}__{mcp_tool}" |
There was a problem hiding this comment.
Ambiguous MCP names misparsed
Medium Severity
The unbound_hook function attempts to parse a server and tool from raw VS Code MCP input even when _resolve_vscode_mcp explicitly returns unresolved due to ambiguity. This bypasses the resolver's careful checks, potentially mis-identifying the server for sanctioning.
Reviewed by Cursor Bugbot for commit 86c3704. Configure here.
| if len(builtin_candidates) != 1: | ||
| return (None, None, None) | ||
| bc = builtin_candidates[0] | ||
| return (bc[2], bc[3], _copy_if_builtin(COPILOT_VSCODE_BUILTIN_MCP[bc[2]])) |
There was a problem hiding this comment.
Builtin fallback matches bare github
Medium Severity
The VS Code built-in fallback reuses truncated alias matching, so tool bodies like github_get_me can match the github-mcp-server alias github_mcp_server via alias.startswith('github') when no configured server matched. That attaches the hosted built-in URL to tokens meant for a separate github server entry.
Reviewed by Cursor Bugbot for commit 86c3704. Configure here.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| body = raw_tool[len('mcp_'):] | ||
| parsed_server, _, parsed_tool = body.partition('_') | ||
| if parsed_server and parsed_tool: | ||
| mcp_server, mcp_tool = parsed_server, parsed_tool | ||
| canonical = f"mcp__{mcp_server}__{mcp_tool}" | ||
| log_error(f"copilot vscode mcp UNRESOLVED session={session_id} tool={raw_tool}", 'mcp_match') |
There was a problem hiding this comment.
The log_error call on line 1037 always fires with the message "copilot vscode mcp UNRESOLVED", but when parsed_server and parsed_tool is truthy (lines 1034-1036), mcp_server and mcp_tool are populated and a full mcp__server__tool canonical is built and forwarded to the gateway. From logs alone it's impossible to tell whether the gateway received an identity (fail-secure block possible) or not (degenerate token, no identity forwarded, allow). Two distinct log messages — one for each branch — would let on-call engineers reconstruct exactly what happened from a failing session.
| @@ -652,7 +706,7 @@ def detect_mcp_call(raw_tool, mcp_servers): | |||
|
|
|||
| if best is None: | |||
| return (None, None, None) | |||
| return (best[1], best[2], mcp_servers.get(best[1])) | |||
| return (best[1], best[2], _copy_if_builtin(merged.get(best[1]))) | |||
There was a problem hiding this comment.
Three new observable flows land in this PR but none emit metrics: (1) CLI built-in candidate merged and matched (detect_mcp_call now also resolves from COPILOT_CLI_BUILTIN_MCP), (2) VS Code built-in fallback resolved/unresolved (_resolve_vscode_mcp builtin path), and (3) the new fail-secure unresolved forwarding path in process_pre_tool_use. Per the observability rules, every new flow must ship at minimum a success/failure counter and a workflow-specific metric. Suggested metrics: mcp_builtin_resolution_total{surface="cli|vscode", outcome="resolved|unresolved"} and mcp_failsecure_forwarded_total{surface="vscode"}.
Context Used: P0 — Critical (must block merge)
Django / Backend ... (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
…y fingerprinting Copilot's runtime-provisioned built-in github-mcp-server (CLI: bare github-mcp-server-<tool> not in mcp-config.json; VS Code: hosted server absent from mcp.json) reached the gateway with no/garbled identity → null/wrong fingerprint → sanction evasion or over-block + canonical-group policy/analytics gaps. Same class as #180 (Claude Code), now for Copilot (shared hook). - CLI (detect_mcp_call): COPILOT_CLI_BUILTIN_MCP merged into the candidate map ({**builtins, **mcp_servers}, configured wins) so github-mcp-server out-ranks a coincidental shorter `github` entry → correct url + un-garbled tool. - VS Code (_resolve_vscode_mcp): COPILOT_VSCODE_BUILTIN_MCP fallback (fires only when no configured server matches; setup#168 + ambiguity guard preserved). - VS Code unresolved mcp_<server>_<tool> now forwards a best-effort identity so deny-by-default applies (fail-open → fail-secure), only with a non-empty tool segment; degenerate mcp_ tokens left as-is. - Built-in registry configs returned as copies (no in-process mutation). - 27 hermetic tests. Deferred (noted): CLI fail-secure on BARE unresolved tokens stays return {} — a delimiter-less name can't be split safely; blind-forwarding would over-block native tools. Backend dependency (gateway-data, out of scope): classify api.individual.githubcopilot.com/mcp(+/readonly) into the GitHub canonical group so the CLI built-in is allowed when GitHub is sanctioned (VS Code url already cataloged). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
anonpran
force-pushed
the
nanda/copilot-builtin-mcp-resolution
branch
from
86c3704 to
18a004e
Compare
🛡️ Automated Security Review (consensus)2 findings — 2 high-confidence, 0 to triage. Reviewers: Cursor, Claude, Semgrep, Gitleaks. Ambiguous VS Code MCP tokens re-parsed after resolver declined
VS Code built-in fallback prefix-matches unrelated
|
2 participants
Summary
Closes the same MCP-identity gap PR #180 fixed for Claude Code — now for GitHub Copilot's built-in / hosted MCP servers (CLI + VS Code, shared hook). The runtime-provisioned
github-mcp-serverreaches the gateway with no or garbled identity → null/wrong fingerprint → sanction evasion or over-block, and canonical-group BLOCK/WARN/AUDIT policies + MCP analytics are wrong. This is an original identity bug exposed (not caused) by the sanction allow-list.All changes are in one production file (
copilot/hooks/unbound.py) + its tests. No gateway / gateway-data / sibling-hook changes.Root cause (verified by live E2E)
github-mcp-server-<tool>;github-mcp-serveris not in~/.copilot/mcp-config.json.detect_mcp_callmis-attributed it to a coincidental shortergithubconfig entry → garbled tool (mcp-server-list_issues) + wrong URL (api.githubcopilot.com/mcp); with no coincidental entry →return {}(fail-open allow-list bypass).mcp.json→_resolve_vscode_mcpreturns(None,None,None)→ the raw single-underscore name reaches the gateway unrecognized → fail-open allow.Fix
COPILOT_CLI_BUILTIN_MCP) merged intodetect_mcp_call's candidate map ({**builtins, **mcp_servers}— configured servers still win). The built-ingithub-mcp-server(len 17) out-ranks the coincidentalgithub(len 6) via the existing longest-prefix logic → correct identity (url:api.individual.githubcopilot.com/mcp/readonly) + un-garbled tool.COPILOT_VSCODE_BUILTIN_MCP) in_resolve_vscode_mcp, firing only when no configured server matches (setup#168 + the ambiguity guard fully preserved) → hosted github (url:api.githubcopilot.com/mcp, already in the GitHub canonical group → no backend change).mcp_<server>_<tool>now forwards a best-effort parsed identity so the gateway applies deny-by-default — only when a non-empty tool segment is present; degeneratemcp_-tokens are left as-is (not a regression). Safe becausemcp_is a reliable MCP signal (native tools never start with it). This intentionally flips one prior fail-open test to fail-secure.api.individual.githubcopilot.com/mcp/readonly) ≠ VS Code url (api.githubcopilot.com/mcp) — same logical server, different endpoints, intentional.Deliberately deferred (noted, not a bug)
return {}. A delimiter-less<server>-<tool>name has no reliable split; blind-forwarding would over-block benign native tools fleet-wide the moment any group is sanctioned. Documented as a follow-up.For the CLI built-in to be allowed when GitHub is sanctioned, gateway-data must classify
url:api.individual.githubcopilot.com/mcpand.../mcp/readonlyinto the GitHub canonical group. It is absent today (the classifier rule isapi.githubcopilot.com/*, which doesn't cover theindividualsubdomain), so until that lands the CLI built-in resolves to a correct-but-uncataloged identity → fail-secure deny under an active allow-list. The VS Code hosted url is already cataloged → no backend dep.Scope
copilot/hooks/unbound.pyonly. Cursor (project-scoped.cursor/mcp.json) is a separate follow-up.Test plan
test_pretool_mcp.py): CLI built-in out-ranks coincidentalgithub+ un-garbled tool; built-in resolves with no config; configured server wins (precedence); unrelated bare token still unresolved; VS Code hosted built-in fallback resolves when unconfigured; configured VS Code server still wins (setup#168); fallback doesn't rescue a real ambiguity; sanctioned hosted built-in allowed e2e; unresolvedmcp_now fail-secure blocked; degenerate empty-tool token not forwarded / not falsely blocked.py_compile+pyflakesclean.github-mcp-server-*allowed when GitHub sanctioned.🤖 Generated with Claude Code
Note
Medium Risk
Changes pre-tool MCP identity and sanction behavior (fail-open to fail-secure for some VS Code mcp_ calls), which can block tools under active allow-lists; CLI built-in allow still depends on gateway-data classifying the individual subdomain URL.
Overview
Fixes Copilot CLI and VS Code pre-tool hooks so runtime built-in
github-mcp-servercalls get correct MCP identity and sanction/policy coverage, instead of wrong attribution, missing config, or slipping the allow-list.CLI: Adds
COPILOT_CLI_BUILTIN_MCPand merges it intodetect_mcp_call(user config still wins on name collision). Baregithub-mcp-server-<tool>now resolves to the built-in readonly URL and beats a shorter coincidentalgithubentry via longest-prefix matching.VS Code: Adds
COPILOT_VSCODE_BUILTIN_MCPand a built-in-only fallback in_resolve_vscode_mcpwhen no configured server matches (configured matches and ambiguity rules unchanged). Shared matching moves to_vscode_match_servers; built-in configs are returned as copies via_copy_if_builtin.Fail-secure: Unresolved
mcp_<server>_<tool>tokens with a non-empty tool now forward best-effortmcp_server/mcp_toolto the gateway so deny-by-default can block; degenerate tokens likemcp_xstill forward nothing. Bare unresolved CLI MCP names remain fail-open (documented deferral).Tests in
test_pretool_mcp.pycover built-in precedence, fallback, sanction e2e, and the flipped unresolved behavior.Reviewed by Cursor Bugbot for commit 18a004e. Bugbot is set up for automated code reviews on this repo. Configure here.
Greptile Summary
This PR fixes MCP identity mis-attribution for GitHub Copilot's built-in
github-mcp-serveron both the CLI and VS Code surfaces, closing a gap where absent or garbled identity could cause sanction bypass or over-block. All changes are confined tocopilot/hooks/unbound.pyand its test file.COPILOT_CLI_BUILTIN_MCPis merged intodetect_mcp_call's candidate map so that the baregithub-mcp-server-<tool>form resolves to the correct readonly URL (api.individual.githubcopilot.com/mcp/readonly) instead of being mis-attributed to a shorter configuredgithubentry via longest-prefix.COPILOT_VSCODE_BUILTIN_MCPis tried in_resolve_vscode_mcponly when no configured server produces candidates (ambiguity guard fully preserved from setup#168); unresolvedmcp_<server>_<tool>tokens now forward a best-effort server/tool identity for deny-by-default instead of silently failing open, while degenerate tokens with no tool segment remain unchanged.Confidence Score: 4/5
Safe to merge from a correctness standpoint; the identity resolution logic is sound, precedence rules are properly ordered, and the fail-secure flip is well-tested — but the new resolution flows ship without instrumentation, leaving the three new paths (CLI builtin match, VS Code builtin fallback, fail-secure forwarding) invisible in production metrics.
The core logic change is correct and thoroughly covered by 27 hermetic tests. The built-in registry merge preserves configured-server precedence, the VS Code ambiguity guard from setup#168 is intact, and the fail-secure behavior for degenerate tokens is explicitly guarded. The gap is that none of the three new observable flows emit counters or histograms, making it impossible to track resolution rates or detect regressions in production without adding instrumentation.
copilot/hooks/unbound.py — the three new resolution flows (CLI builtin merge, VS Code builtin fallback, fail-secure unresolved forwarding) each need success/failure counters before going live.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[raw_tool arrives] --> B{starts with mcp_ not mcp__?} B -- Yes VS Code --> C[_resolve_vscode_mcp] B -- No CLI bare --> D{is_mcp already?} D -- No --> E[detect_mcp_call merged builtins plus config] E --> F{match found?} F -- Yes --> G[forward with config to gateway] F -- No --> H[return empty - fail-open for bare CLI] C --> I[match against configured servers] I --> J{candidates found?} J -- Yes --> K{ambiguity guard passes?} K -- Yes --> G K -- No --> L[unresolved path] J -- No --> M[match against COPILOT_VSCODE_BUILTIN_MCP] M --> N{exactly 1 builtin match?} N -- Yes --> G N -- No --> L L --> O{first underscore split yields both server and tool?} O -- Yes --> P[best-effort identity forwarded - fail-secure deny] O -- No --> Q[no identity forwarded - degenerate token allow]%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% flowchart TD A[raw_tool arrives] --> B{starts with mcp_ not mcp__?} B -- Yes VS Code --> C[_resolve_vscode_mcp] B -- No CLI bare --> D{is_mcp already?} D -- No --> E[detect_mcp_call merged builtins plus config] E --> F{match found?} F -- Yes --> G[forward with config to gateway] F -- No --> H[return empty - fail-open for bare CLI] C --> I[match against configured servers] I --> J{candidates found?} J -- Yes --> K{ambiguity guard passes?} K -- Yes --> G K -- No --> L[unresolved path] J -- No --> M[match against COPILOT_VSCODE_BUILTIN_MCP] M --> N{exactly 1 builtin match?} N -- Yes --> G N -- No --> L L --> O{first underscore split yields both server and tool?} O -- Yes --> P[best-effort identity forwarded - fail-secure deny] O -- No --> Q[no identity forwarded - degenerate token allow]Reviews (2): Last reviewed commit: "Resolve Copilot CLI & VS Code built-in/h..." | Re-trigger Greptile