You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
VS Code Copilot names MCP tools mcp_<server>_<tool>. When the hook can't match that token to a configured server, the call reaches the gateway with no server and no config. No MCP policy runs, and analytics records the server as unknown.
This has persisted for weeks on individual machines. The only trace was a local error.log line, and its Sentry report was usually dropped by the hook's one-report-per-60s limit. From our side we couldn't tell where VS Code was loading those servers from.
What
Unresolved-MCP diagnostic. The Claude Code hook already has a null-fingerprint diagnostic; this ports it to Copilot. On an unresolved mcp_ call, the hook spawns a detached child, at most once per (server token, cwd) every 6h. The child uploads a text report to the existing /v1/hooks/mcp-diagnostics endpoint, which stores it in mcp_fingerprint_diagnostics and pings Slack on first sight. The report covers:
a resolution replay, with where each server name the hook knows came from
every source the hook reads, file by file
sources the hook does not read:
Claude Code: ~/.claude.json and installed plugins
Claude Desktop, Cursor, Windsurf
.vscode/mcp.json above the cwd
servers VS Code actually started. VS Code writes one mcpServer.<collection>.<server>.log per server, and the collection records where the definition came from (for example, mcp.config.usrlocal is the user mcp.json).
installed extensions that contribute MCP servers
copilot mcp list --json
exact and near name matches for the tool's server token, using VS Code's 13-char prefix truncation
Safety
The report never runs on the PreToolUse path. It builds in a detached child, and failures are only logged.
Server configs are summarized as type, URL host and path, and command basename. Args, env, headers and URL queries are never included.
The frozen binary needs no change: its mcp-diagnostic <tool> subcommand already calls _run_mcp_diagnostic_cli on any module that defines it.
Ran the report builder locally on macOS against real configs:
it flagged servers that exist only in Claude Code config or plugins
it matched copilot mcp list entries
it took 0.03–0.6s
End to end: dispatch spawned the detached child, the child POSTed to a local stub on /v1/hooks/mcp-diagnostics with the right auth and envelope, and the 6h stamp was written.
On a Windows 11 ARM VM (Python 3.13, real copilot.exe and VS Code):
The real profile built cleanly in ~9s, mostly copilot mcp list.
In a fixture profile with the unresolved server defined only in Claude Desktop, the report flagged that server as an exact match, marked VS Code's mcpServer.*.elasticsearch log, and listed an extension-contributed MCP provider.
Secrets planted in args, a URL query, headers and env did not appear in the report.
The DETACHED_PROCESS child ran, curl.exe POSTed to the local stub, and the 6h stamp was written.
Companion PR: websentry-ai/ai-gateway-data, the same branch name, which adds the tool name and hook source to the Slack alert.
Confidence Score: 4/5
The PR is not yet safe to merge because the new diagnostic workflow still lacks the required operation metrics.
The PR adds detached diagnostics for unresolved Copilot MCP calls, collecting sanitized configuration-source and runtime-resolution information and uploading it to the gateway on a six-hour cooldown.
Replays MCP server resolution across sources read and not read by the hook.
Collects VS Code MCP logs, extension providers, settings, and Copilot CLI output.
Moves diagnostic collection and upload outside the blocking pre-tool path.
The latest revision restricts Copilot CLI resolution to absolute PATH entries and rejects executables resolved inside the workspace.
Diagram
%%{init: {'theme': 'neutral'}}%%
flowchart LR
A[Unresolved Copilot MCP tool] --> B{Cooldown active?}
B -->|Yes| C[Continue policy flow]
B -->|No| D[Spawn detached diagnostic child]
D --> E[Inventory MCP configuration sources]
E --> F[Inspect VS Code logs and extensions]
F --> G[Run trusted Copilot CLI when available]
G --> H[Render bounded diagnostic report]
H --> I[Upload to MCP diagnostics endpoint]
D --> J[Write six-hour dispatch stamp]
The reason will be displayed to describe this comment to others. Learn more.
Diagnostic Metrics Missing
This new detached diagnostic flow has no operation metrics. This violates the repository directive that every new flow ship success/failure and workflow-specific metrics. Add a diagnostic outcome counter covering dispatch, build, upload success, and each failure stage, along with report-build and upload latency.
The reason will be displayed to describe this comment to others. Learn more.
I can’t access the referenced scratchpad attachment in the repository review context. The current PR diff still contains no diagnostic metrics, so the finding remains unresolved: add outcome counters for dispatch, build, upload success, and each failure stage, plus report-build and upload latency. Please paste the scratchpad contents or push the metrics implementation to the PR so I can verify the exact instrumentation.
The reason will be displayed to describe this comment to others. Learn more.
Failed Uploads Suppress Retries
The cooldown stamp is written immediately after the detached child is spawned, before the report is built or uploaded. If report construction, authentication, curl, or the gateway fails, the child only logs and exits while the stamp remains. Further diagnostics for that server and cwd are then suppressed for six hours, delaying diagnosis after a transient failure. Clear the stamp after failures or defer it until upload succeeds.
The reason will be displayed to describe this comment to others. Learn more.
Failure Logs Lack Context
Detached upload failures are logged only as a curl exit code or exception text, without the affected tool or server, a cwd hash, or a diagnostic dispatch identifier. Multiple concurrent diagnostics therefore cannot be correlated with the unresolved call that failed. This violates the repository's contextual-logging directive and must be addressed before merging by attaching a non-sensitive correlation ID and server/cwd identifiers to every build and upload outcome.
The reason will be displayed to describe this comment to others. Learn more.
I can’t access local scratchpad paths such as /private/tmp/.../r_logctx.txt from this review context. Please paste its contents or summarize the proposed logging changes so I can assess them against the correlation/context requirement.
Impact: The gateway API key is interpolated into curl's -H 'Authorization: Bearer …' argument; on Linux /proc/<pid>/cmdline is world-readable and on macOS ps exposes argv to same-user processes, so a local observer can steal a live gateway credential during the up-to-35s upload window.
Fix: Stop passing the bearer token via argv — use urllib.request, curl --config - with headers on stdin, or a temp config file with 0600 perms; drop -L from -fsSL on this POST.
Impact: The uploaded report (stored remotely and alerted to Slack) includes full cwd/home, config file paths, Claude project names, chat.mcp.* values, and verbatim URL paths from _mcp_diag_summarize; remote MCP endpoints often embed auth tokens in the path, and usernames/project layout leak tenant identity.
Fix: Apply redact_secrets to the final rendered report before upload; hash or home-relativize paths and replace URL paths with segment counts or hashes instead of literals.
Flagged by: Cursor, Claude, Greptile
🟡 Cooldown bypassable via attacker-controlled server token
copilot/hooks/unbound.py:6373
Impact: Diagnostics fire on unresolved mcp_<server>_<tool> names and the 6h stamp keys only on (server_hint, cwd) parsed from that name; a prompt-injected agent can vary the server token each call to spawn unbounded detached children (config sweeps, copilot mcp list, curl uploads) within the same cwd.
Fix: Add a per-cwd or global dispatch cap (e.g., max N per hour regardless of server token) and bound concurrent diagnostic children.
Flagged by: Claude
🟡 API key passed to detached child via environment
copilot/hooks/unbound.py:6360
Impact:UNBOUND_DIAG_API_KEY is injected into the child's full inherited environment, widening credential exposure to any subprocess the child spawns (e.g., copilot mcp list on PATH).
Fix: Omit UNBOUND_DIAG_API_KEY and let _run_mcp_diagnostic_cli call get_api_key() locally; pass a minimal env to child subprocesses.
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Agentic security review of PR #307 (vv/copilot-mcp-diagnostic).
One HIGH finding: Copilot MCP diagnostic URL summaries include URL paths and can send those values to the gateway and Slack. The Claude Code diagnostic this ports already redacts to host-only.
When a VS Code mcp_<server>_<tool> call matches no configured server, the
call goes to the gateway without a server or config, so no policy applies
and analytics records server 'unknown'. Until now the only trace was one
error.log line.
The hook now spawns a detached child, at most once per (server, cwd) every
6h, that builds a text report and uploads it to /v1/hooks/mcp-diagnostics,
the endpoint the Claude Code diagnostic already uses. The report covers:
- the hook's resolution replay and where each known name came from
- every source the hook reads, file by file
- sources it does not read: Claude Code (~/.claude.json and installed
plugins), Claude Desktop, Cursor, Windsurf, .vscode/mcp.json above the cwd
- servers VS Code actually started, from its mcpServer.<collection>.<server>
logs, and extensions that contribute MCP servers
- copilot mcp list --json
- exact and near matches for the tool's server token, applying VS Code's
13-char prefix truncation
Server configs are summarized as type, URL host and path, and command
basename; args, env and headers are never included. The frozen binary
already routes mcp-diagnostic <tool> to _run_mcp_diagnostic_cli.
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Agentic security review found one new high-severity issue on this head (cwd executable search with the diagnostic API key). The previously reported URL-path redaction gap in _mcp_diag_summarize is still present and was left open.
🔴 [HIGH] MCP server URL paths uploaded to gateway storage and Slack
copilot/hooks/unbound.py:5897
Impact:_mcp_diag_summarize keeps scheme://host plus the full URL path (unparseable values can pass through whole); many hosted MCP endpoints embed credentials in the path, which then land in mcp_fingerprint_diagnostics and first-sight Slack alerts despite the PR's "no secrets" intent.
Fix: Match the Claude Code diagnostic: keep only scheme://hostname[:port]; replace unparseable or scheme-less URLs with <unparseable-url>; if path shape matters, send a hash or segment count instead.
Impact: The API key is passed as -H 'Authorization: Bearer <key>' in subprocess.Popen arguments, visible to other local processes via ps//proc; on communicate timeout, str(exc) can include the full command and is written to error.log, which _mcp_diag_error_log_tail may re-upload.
Fix: POST with urllib.request or curl config on stdin (-K -) so the key never appears in argv; on subprocess errors log only type(exc).__name__, never str(exc).
Flagged by: Claude
🟡 [TRIAGE] Diagnostic report lacks privacy redaction before remote upload
copilot/hooks/unbound.py:6170
Impact: The rendered report includes full cwd/home, Claude Code project paths, config file paths, raw chat.mcp.* JSON fragments, and up to 15 error.log lines — usernames, project names, internal hostnames, and prior exception text can leave the machine for vendor storage and Slack with no final scrub pass.
Fix: Normalize paths (~ for home, hash project paths), drop or strictly filter the error-log section, and run a final secret-scrub over the rendered report (bearer/token patterns, URL userinfo/query strings) before _upload_mcp_diagnostic.
Flagged by: Claude
🟡 [TRIAGE] Windows detached child may execute workspace-planted copilot or curl
copilot/hooks/unbound.py:6109
Impact: The diagnostic child inherits the hook's cwd and calls bare shutil.which('copilot') and 'curl'; on Windows both can resolve to binaries in the workspace root, giving code execution (and access to UNBOUND_DIAG_API_KEY in the inherited env) when an unresolved mcp_ call fires.
Fix: Set cwd to a trusted user-owned directory in dispatch Popen, invoke curl by absolute path (%SystemRoot%\System32\curl.exe), set NoDefaultCurrentDirectoryInExePath=1 before which, and strip UNBOUND_DIAG_API_KEY from the child env after reading it.
MCP HTTP/SSE endpoints often carry a token or install id in the path, and
the report is stored server-side and pinged to Slack. Summarize a URL as
scheme://host[:port] only, and mark an unparseable one, matching the Claude
Code diagnostic's _mcp_diag_host_of.
Three review findings on the detached child:
- it kept UNBOUND_DIAG_API_KEY in its environment, so every command it ran
(copilot, curl) inherited the gateway key. Pop it on read instead.
- the child inherits the editor's workspace as its cwd, and Windows searches
that directory before PATH. shutil.which('copilot') there returns a
workspace copilot.cmd, so resolve from PATH and reject a hit in the cwd.
- curl was invoked bare for the same reason. Pin curl.exe on Windows, as the
discovery download path already does.
Why: Command-line arguments are visible to other local processes while the upload runs, for up to 30s. On Linux that is /proc/<pid>/cmdline unless hidepid is set. On Windows it is WMI or Task Manager. A local user or process can read the key and reuse it against the gateway. The change that pops UNBOUND_DIAG_API_KEY from the environment does not help here, because the key goes right back into argv.
Fix: Keep the header off the command line. Two options:
Pass curl a config on stdin (-K - with header = "Authorization: Bearer ...") and send the body from a temp file with 0600 permissions.
Better: do the POST with urllib.request inside the child process, so no subprocess is involved at all.
Flagged by: Claude (also raised earlier by Greptile, and still present in this revision)
[MEDIUM] 🟡 Uploaded report has no final scrub for sensitive data (copilot/hooks/unbound.py:6175)
What: The report is stored on the gateway and pinged to Slack. It includes:
raw cwd and home
full config file paths
Claude Code project <full path> labels
chat.mcp.* dict values, up to 200 chars each
the last 15 lines of error.log that mention "mcp", up to 300 chars each
Why: Paths reveal usernames, customer project names and repo names. The error.log lines hold free-form exception text, and ⚠️ other log calls may write URLs or config values into it. That code is not in this diff, so this is unverified. Either way, the text reaches Slack unfiltered. The URL fix is good: _mcp_diag_summarize now keeps only scheme://host[:port]. The log tail and settings dumps get around that fix.
Fix:
Replace the home prefix in every path with ~.
Hash or drop Claude project paths.
Run error_log_tail and vscode_settings values through a scrubber that strips URLs down to host and masks token-like strings.
Or drop both sections from the upload and keep them local only.
Flagged by: Claude (also raised by Greptile, whose latest reply says it is still valid)
[LOW] 🟡 copilot binary from the workspace can still run through relative PATH entries (copilot/hooks/unbound.py:6109)
What: _mcp_diag_path_binary rejects a binary only when its parent folder is exactly the cwd.
Why: A relative PATH entry (for example node_modules\.bin or bin) resolves inside the workspace. A cloned repo could then ship bin/copilot.exe, which passes the check and runs as the user whenever an unresolved mcp_* call happens. The API key is no longer in that binary's environment, but running code from the repo is enough to cause harm.
Fix: Also reject a binary when its resolved path is anywhere under the cwd (Path(found).resolve().is_relative_to(Path.cwd().resolve())). Or drop non-absolute PATH entries before calling shutil.which.
Flagged by: Claude
[LOW] 🟡 No global limit on diagnostic child processes (copilot/hooks/unbound.py:3877)
What: The cooldown key is (server token, cwd). Every new mcp_<token>_... name spawns a fresh detached child.
Why: Each child runs copilot mcp list (about 9s on Windows), scans the disk, uploads a report, and can trigger a Slack alert "on first sight". If a prompt-injected agent emits many made-up mcp_ tool names, it could flood the machine with processes and flood Slack with alerts. ⚠️ Not confirmed that Copilot sends PreToolUse for tool names that don't exist.
Fix: Add a global cap, such as at most N diagnostics per hour across all tokens, checked with a single stamp or counter file before spawning.
Flagged by: Claude
Pre-existing (not introduced by this PR)
Semgrep insecure-file-permissions at copilot/hooks/unbound.py:5567 (0o700): this line is not in the diff. The finding is also a false positive, because 0o700 is owner-only and stricter than the suggested 0o644. No action needed.
The gateway key sat in curl's argv for the length of the upload, readable by
any local process. Post it with urllib and the same no-redirect opener that
_request_skill_sync uses, so the header never leaves this process. Drops the
curl dependency (and its Windows path pinning) from this path.
🟡 [MEDIUM] Server summary can leak secrets passed inline in command — copilot/hooks/unbound.py:5892
What: _mcp_diag_summarize uploads os.path.basename(command)[:60]. It assumes command is only a path to a program.
Why: Many MCP configs put the whole command line in command, for example "docker run -e API_TOKEN=abc…" or "node srv.js --key sk-…". basename only splits on /, so the token survives and goes to the gateway diagnostics store and Slack. That breaks the PR's "no args, env or headers" promise. On Windows, C:\…\tool.exe --token x also passes through unchanged.
Fix: Keep only the first word, split on whitespace with shlex.split in a try block. Then take the basename of that, split on both / and \. If the string had more than one word, add a marker such as cmd=docker (+inline args).
Flagged by: Claude
🟡 [LOW] Report sends local paths, project names and raw log lines with no final redaction step — copilot/hooks/unbound.py (_build_mcp_diagnostic, _mcp_diag_unread_sources, _mcp_diag_error_log_tail)
What: The report includes these values unchanged:
the full cwd, home and hook paths
every config file path
every ~/.claude.json project key, which is a full project path, in source
chat.mcp.* dict values, up to 200 chars each
the last 15 error.log lines that mention "mcp", up to 300 chars each
Why: This goes to a remote store and to Slack. It exposes OS usernames, customer project and repo names, and whatever exception text earlier log_error calls wrote. That text can include URLs.
Fix: Replace the home directory with ~ everywhere. Hash or shorten project paths that are not the current cwd. Scrub the error.log lines for URL queries, userinfo and token-like strings before rendering.
Flagged by: Claude (also raised by an earlier Greptile review; ⚠️ the maintainer's reply points to a local scratchpad file that can't be read here, so it's unclear whether this was accepted by design)
🟡 [LOW] No overall cap on detached diagnostic processes — copilot/hooks/unbound.py (_dispatch_mcp_diagnostic)
What: The 6h cooldown is keyed on (server token, cwd). Every new unresolved mcp_<x>_… token starts a new detached child.
Why: A model steered by prompt injection could emit many distinct fake mcp_ tool names. Each one would start a child that runs copilot mcp list (about 9s on Windows), uploads up to 200KB, and can trigger a first-sight Slack ping. ⚠️ Not confirmed: it's unclear whether Copilot sends unregistered tool names to PreToolUse at all.
Fix: Add a global limit next to the per-key stamp, such as at most N dispatches per hour. Also skip dispatch while another diagnostic child is still running, using a lock file.
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Agentic security review of PR #307 (vv/copilot-mcp-diagnostic).
One MEDIUM finding remains on this head: the Copilot unresolved-MCP diagnostic uploads VS Code chat.mcp* settings dumps, MCP-related error.log lines, and raw host paths without the Claude Code diagnostic scrub pass. Prior HIGH issues (URL-path leakage in server summaries, Windows cwd binary hijack with the diagnostic API key) look addressed in the current diff.
The report carried two pieces of free text without the Claude Code
diagnostic's boundary: chat.mcp.* setting values and the error.log excerpt.
Reuse its _MCP_DIAG_SECRETISH pattern — a matching value becomes <redacted>
and a matching log line is dropped with a count of what was suppressed.
The reason will be displayed to describe this comment to others. Learn more.
🛡️ Automated Security Review (consensus)
3 findings — 0 high-confidence, 3 to triage. Reviewers: Claude (lead), Semgrep, Gitleaks.
[MEDIUM] Command-line secrets can leak through the cmd= summary — copilot/hooks/unbound.py:5894
What: _mcp_diag_summarize uploads os.path.basename(command)[:60] without scrubbing. Some MCP configs put the whole command line, with flags, into the command string.
Why: Take "command": "npx -y @acme/mcp-server --api-key sk-live-abc123". Its basename is mcp-server --api-key sk-live-abc123, and that text goes into the report. The report is stored remotely and sent to Slack. The PR says args are never included, but here they are. This path runs for every config source, including copilot mcp list.
Fix: Keep only the first whitespace-separated token of command, then take its basename. Also run the result through _mcp_diag_scrub_value.
Flagged by: Claude
[LOW] Error-log filter misses secrets that have no keyword — copilot/hooks/unbound.py (_mcp_diag_error_log_tail)
What: Lines are dropped only if they match _MCP_DIAG_SECRETISH, which looks for words like authorization, token or key. Raw key values (sk-…, ghp_…), URLs with query strings, and exception text that has no keyword all pass through. Up to 300 chars per line are kept.
Why: log_error calls elsewhere put {exc} and server details into lines that mention "mcp". A secret there that lacks a keyword gets uploaded in the report.
Fix: Also drop or redact lines that contain URL queries (?…=) or long high-entropy tokens. Or send only the log's category and timestamp, not the free text.
Flagged by: Claude
[LOW] copilot mcp list source field is not scrubbed — copilot/hooks/unbound.py (_mcp_diag_copilot_cli_list)
What: entry.get('source') from the CLI's JSON output is added to the report as-is, with no length cap and no scrub.
Why: That field is outside the hook's control. It may hold a file path or plugin identifier, and a future CLI version could return anything there.
Fix: Wrap it in _mcp_diag_scrub_value(...) and cap its length, as the settings values already are.
Flagged by: Claude
Previously acknowledged (not re-flagged)
Full paths in the report (cwd, home, hook path, config file paths, Claude Code project keys). Raised by Greptile and Cursor. The maintainer replied on the thread (the reply's text isn't visible here), and the latest commit added value and line filtering but deliberately kept paths. The PR description says per-file paths are needed for the diagnosis, so this is treated as by design.
Bearer token visible in curl's command line. The maintainer replied, and the code now POSTs from inside the process with urllib and a handler that refuses redirects.
Credential-bearing URL paths. The maintainer replied, and _mcp_diag_host_of now keeps only scheme://host[:port] and returns <unparseable-url> for URLs it can't parse.
copilot/curl binary hijack from the workspace folder. The maintainer replied. curl is gone, and _mcp_diag_path_binary refuses a match that sits in the current directory.
Semgrep insecure-file-permissions at line 5567. This line was not touched by this PR. 0o700 (owner-only access) is also stricter than the 0o644 Semgrep suggests, so it's a false positive.
Two gaps found by reading a stored report end to end:
- the match section compared the tool name with mcp_ stripped, so a server
whose own name starts with mcp_ never matched. A report could list the
server in its inventory and still say no match was found. Compare with and
without the prefix.
- the row and the Slack alert carried the token parsed out of the tool name
('es' for mcp_es_probe-echo). When exactly one on-disk server matches, file
the diagnostic under that real name instead.
The reason will be displayed to describe this comment to others. Learn more.
🛡️ Automated Security Review (consensus)
4 findings — 0 high-confidence, 4 to triage. Reviewers: Claude (lead), Semgrep, Gitleaks.
[LOW] 🟡 Uploaded report still carries local paths and project names — copilot/hooks/unbound.py:6204
What: _build_mcp_diagnostic sends raw cwd, home and the hook path. The report also lists every config file path, and each Claude Code project <path> label comes from ~/.claude.json. The upload envelope repeats cwd too.
Why: This data is stored on the gateway and can reach Slack. It exposes OS usernames, client or project directory names, and repo layout for every project in ~/.claude.json, not just the current one.
Fix: Swap the home prefix for ~ in all paths. Hash or drop the project paths from other Claude projects. Drop hook and term_program, or reduce them to a basename and sha. Add a test that checks no raw home path appears in the rendered report.
Flagged by: Claude, Cursor bot (earlier thread), Greptile (earlier thread). ⚠️ The maintainer's replies on this thread point to local scratchpad files that can't be read here. If they accepted this by design, ignore it.
[LOW] 🟡 Redaction only matches keywords, so some secrets get through — copilot/hooks/unbound.py (_mcp_diag_scrub_value, _mcp_diag_error_log_tail)
What: chat.mcp* settings values and error.log lines are dropped only when they contain words like token or apikey. Anything else goes out as-is, up to 200 or 300 characters.
Why: Some secrets have no such keyword. Examples: a URL like https://host/sse?key=abc123, a URL path with a secret in it, or user:pass@host. These pass the filter and leave the machine in the report.
Fix: Before the keyword check, run any URL-like substring through _mcp_diag_host_of. Add key=, sig=, auth and ://user:pass@ patterns to _MCP_DIAG_SECRETISH. Or send only the setting keys and value types, not the values.
Flagged by: Claude
[LOW] 🟡 copilot binary check can be bypassed through relative PATH entries — copilot/hooks/unbound.py:6109
What: _mcp_diag_path_binary rejects a match only when the binary's parent folder is exactly the cwd. A relative PATH entry such as node_modules/.bin or bin resolves under the workspace and passes the check.
Why: The child runs from the editor's workspace. On a machine with relative PATH entries, a cloned repo that ships node_modules/.bin/copilot runs as the hook user whenever an unresolved mcp_ call happens. The API key is removed from the env before this runs, which limits the damage. It is still code execution triggered by the repo.
Fix: Build the search path only from absolute PATH entries: os.pathsep.join(p for p in PATH.split(os.pathsep) if os.path.isabs(p)). Also reject any result that resolves under Path.cwd(), not only one whose parent equals it. Better still, set cwd= on the child to a neutral folder such as LOG_DIR.
Flagged by: Claude (improves on the earlier Cursor bot hijack finding, which the curl removal and this helper mostly fixed)
[LOW] 🟡 Server names from repo files flow into the Slack alert without escaping — copilot/hooks/unbound.py (_mcp_diag_reported_server, _mcp_diag_unread_sources)
What: The report now reads .vscode/mcp.json and .cursor/mcp.json from the workspace and its parent folders. A single exact match there becomes the server field in the upload, which the backend uses for its first-sight Slack ping.
Why: A malicious repo controls that name, which is capped only at 255 characters. If the backend doesn't escape it, the name could inject Slack mrkdwn such as <!channel> or a fake <https://evil|link> into an internal alert channel. ⚠️ The backend's escaping isn't in this diff, so this can't be confirmed from here.
Fix: On the hook side, limit server to [A-Za-z0-9._-]. Check that the companion ai-gateway-data PR escapes server, tool and cwd before posting to Slack.
The reason will be displayed to describe this comment to others. Learn more.
🛡️ Automated Security Review (consensus)
2 findings — 0 high-confidence, 2 to triage. Reviewers: Claude (lead), Semgrep, Gitleaks.
[MEDIUM] copilot binary can still resolve to a workspace-controlled file — copilot/hooks/unbound.py:6112
What:_mcp_diag_path_binary only rejects a binary that sits directly in the current working directory. Two cases still get through:
A relative PATH entry such as bin or .\tools resolves to <workspace>/bin/copilot.
On Windows, shutil.which may still search the working directory for other forms of the name.
Why: The detached child is started without a cwd=, so it inherits the editor's workspace. If Copilot emits any unresolved mcp_* tool name, the child runs a repository-shipped copilot as the user. The API key is popped from the environment first. But the spawned binary runs with the user's full privileges, so it can still read the key from the hook's config on disk.
Fix:
Pass cwd= set to a trusted directory (home or temp) in _dispatch_mcp_diagnostic's Popen.
In _mcp_diag_path_binary, search only absolute PATH entries.
Reject any result whose resolved path is inside cwd or UNBOUND_DIAG_CWD.
On Windows, consider the _windows_system32_path-style pinning already used for curl.exe.
Flagged by: Claude. Also raised in earlier bot threads (Greptile, Cursor). The maintainer's replies are unreadable local scratchpad links, so this is not treated as acknowledged.
[LOW] Diagnostic report sends local paths and every Claude Code project path to the gateway and Slack — copilot/hooks/unbound.py:6204 (_build_mcp_diagnostic / _mcp_diag_unread_sources)
What: These go into the uploaded report unredacted:
raw cwd, home, and the hook path
the full path of every config file
one Claude Code project <path> row for every project in ~/.claude.json that defines MCP servers, not just the current one
Why: The report is stored remotely and posted to Slack on first sight. It exposes usernames, customer or project directory names, and the user's list of other workspaces, well beyond what is needed to diagnose one unresolved server. The secret-keyword filter (_MCP_DIAG_SECRETISH) only catches keyword-labelled secrets. A bare token in an error.log line, such as ghp_… or ?key=…, would pass through.
Fix:
Replace the home prefix with ~.
Hash or drop project paths other than the current cwd.
Add a final scrub over the rendered report (home prefix, known token patterns) before upload.
Flagged by: Claude. Also raised by Cursor and Greptile. The maintainer's reply is an unreadable scratchpad link.
Pre-existing (not introduced by this PR)
The Semgrep insecure-file-permissions hit at copilot/hooks/unbound.py:5567 (0o700) is on a line this diff does not touch. 0o700 is owner-only and more restrictive than Semgrep's suggested 0o644, so it is a false positive.
The previous guard rejected only a binary sitting directly in the cwd, so a
relative PATH entry such as 'bin' still resolved inside the editor workspace
the detached child inherits. Resolve from absolute PATH entries only and
refuse any result under the cwd tree.
The reason will be displayed to describe this comment to others. Learn more.
🛡️ Automated Security Review (consensus)
3 findings — 0 high-confidence, 3 to triage. Reviewers: Claude (lead), Semgrep, Gitleaks.
[MEDIUM] Diagnostic report still sends local paths and project names off the machine — copilot/hooks/unbound.py:6208
What: The report is stored remotely and pinged to Slack. It still includes several unredacted values:
raw cwd and home
the full hook path
every config file path
every ~/.claude.json project path that has MCP servers, as Claude Code project <path>
Why: A single unresolved mcp_ call lists every Claude Code project directory with MCP servers on the laptop. That exposes usernames, client or repo names, and tenant identifiers to the gateway store and Slack. _mcp_diag_summarize also keeps os.path.basename(command)[:60]. If command is a full shell string, such as sh -c "FOO=abc …", part of that string can survive.
Fix: Before rendering, swap the home prefix for ~ in all paths. Hash or drop the Claude Code project keys, for example project#<sha8>. Remove hook and term_program. Run _mcp_diag_scrub_value on the command basename too.
Flagged by: Claude. This is what is left of the earlier Greptile and Cursor scrub thread. URL and settings scrubbing are now fixed. ⚠️ The maintainer's replies point to local scratchpad files that aren't visible here. If they accepted cwd/home as needed by design, ignore that part.
[LOW] Error-log redaction misses common credential words — copilot/hooks/unbound.py:6211
What: _mcp_diag_error_log_tail drops a line only when it matches authorization|bearer|api_key|token|secret|password|passwd|credential. Other lines are forwarded as-is, up to 300 chars each.
Why: Exception text that holds URLs with ?key=, ?sig=, ?code=, X-Api-Key, or access_key does not match the pattern. Those lines get uploaded and reach Slack.
Fix: Before the secret check, strip URL query strings and userinfo from each line, for example with re.sub(r'\?\S*', '?<q>', line). Also add key|sig|signature|code|session|cookie to _MCP_DIAG_SECRETISH.
Flagged by: Claude
[LOW] Workspace-binary check compares against the process cwd, not the event cwd — copilot/hooks/unbound.py:6109
What: _mcp_diag_path_binary rejects copilot only when it resolves under Path.cwd(). The workspace from the event (UNBOUND_DIAG_CWD) is never checked.
Why: The hook process cwd may not be the workspace root, for example in multi-root workspaces or when VS Code launches the hook elsewhere. In that case, a repo-controlled copilot reached through an absolute PATH entry passes the check. direnv's PATH_add bin and node_modules/.bin are common examples. It then runs during diagnostic collection.
Fix: Also reject binaries under Path(UNBOUND_DIAG_CWD).resolve(). Better, only allow known install locations, such as the npm global prefix or %LOCALAPPDATA%.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
vigneshsubbiah16
left a comment
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Why
VS Code Copilot names MCP tools
mcp_<server>_<tool>. When the hook can't match that token to a configured server, the call reaches the gateway with no server and no config. No MCP policy runs, and analytics records the server asunknown.This has persisted for weeks on individual machines. The only trace was a local
error.logline, and its Sentry report was usually dropped by the hook's one-report-per-60s limit. From our side we couldn't tell where VS Code was loading those servers from.What
Unresolved-MCP diagnostic. The Claude Code hook already has a null-fingerprint diagnostic; this ports it to Copilot. On an unresolved
mcp_call, the hook spawns a detached child, at most once per (server token, cwd) every 6h. The child uploads a text report to the existing/v1/hooks/mcp-diagnosticsendpoint, which stores it inmcp_fingerprint_diagnosticsand pings Slack on first sight. The report covers:~/.claude.jsonand installed plugins.vscode/mcp.jsonabove the cwdmcpServer.<collection>.<server>.logper server, and the collection records where the definition came from (for example,mcp.config.usrlocalis the usermcp.json).copilot mcp list --jsonSafety
mcp-diagnostic <tool>subcommand already calls_run_mcp_diagnostic_clion any module that defines it.Testing
tests/copilot,test_hook_contract,test_mcp_fingerprint_parity,binary/tests(1372 passed, 38 skipped).copilot mcp listentries/v1/hooks/mcp-diagnosticswith the right auth and envelope, and the 6h stamp was written.copilot.exeand VS Code):copilot mcp list.mcpServer.*.elasticsearchlog, and listed an extension-contributed MCP provider.DETACHED_PROCESSchild ran,curl.exePOSTed to the local stub, and the 6h stamp was written.Companion PR: websentry-ai/ai-gateway-data, the same branch name, which adds the tool name and hook source to the Slack alert.
The PR is not yet safe to merge because the new diagnostic workflow still lacks the required operation metrics.
Findings
Summary
The PR adds detached diagnostics for unresolved Copilot MCP calls, collecting sanitized configuration-source and runtime-resolution information and uploading it to the gateway on a six-hour cooldown.
Diagram
%%{init: {'theme': 'neutral'}}%% flowchart LR A[Unresolved Copilot MCP tool] --> B{Cooldown active?} B -->|Yes| C[Continue policy flow] B -->|No| D[Spawn detached diagnostic child] D --> E[Inventory MCP configuration sources] E --> F[Inspect VS Code logs and extensions] F --> G[Run trusted Copilot CLI when available] G --> H[Render bounded diagnostic report] H --> I[Upload to MCP diagnostics endpoint] D --> J[Write six-hour dispatch stamp]Reviews (8) · Last reviewed commit: "Keep workspace binaries off the diagnost..."