feat(mcp_server): discover shadow MCP servers and correlate to identities#64
Open
leet-c1 wants to merge 10 commits into
Open
feat(mcp_server): discover shadow MCP servers and correlate to identities#64leet-c1 wants to merge 10 commits into
leet-c1 wants to merge 10 commits into
Conversation
…ties Adds shadow-MCP-server discovery to baton-crowdstrike. It reads CrowdStrike EDR detections (Alerts API), identifies unsanctioned Model Context Protocol servers by command-line signature (@modelcontextprotocol/server-*, mcp-server-*, mcp_server_*), and models them in ConductorOne as: - `mcp_server` resources — App-trait profile (server, package, launcher, transport, host, aid, local_ip, endpoint_user, sample_command_line, detection_count, ioa_rule, falcon_link, sanctioned, first_seen) plus a security-insight issue bound to the resolved identity; - `endpoint_user` accounts — the OS user that ran the server, matched to a c1 identity by email or assigned manually; - a `runner` assignment entitlement on each `mcp_server`, granted to the endpoint_user account that ran it. Chain: c1 identity -> endpoint_user account -> runner grant -> mcp_server. Detection is driven by a Custom IOA Process-Creation rule matching the signature; see README. Requires the Alerts: Read scope (plus the existing Identity Protection scopes for correlation). Robustness: - server-side FQL product filter + offset pagination (bounded, logs if capped) so busy tenants are scanned exhaustively rather than the newest 1000 alerts; - one syncID-cached detection snapshot + identity index shared across the two syncers, so grants are consistent and Alerts/Identity-Protection are not re-queried per resource; - stable resource IDs derived from the deduped server name (not a mutable package spelling); - ambiguous samAccountName / email-local-part matches resolve to no identity rather than silently attaching a finding to the wrong person; - detections lacking an AID or user name are dropped. Unit tests cover the command-line parser and ambiguous-identity resolution. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
Connector PR Review: feat(mcp_server): discover shadow MCP servers and correlate to identitiesBlocking Issues: 0 | Suggestions: 0 | Threads Resolved: 0 Review SummaryThe full PR diff was scanned for security and correctness; the new commits are scoped to identity_protection.go and mcp_server.go. The previous finding — a discarded url.Parse error in the Identity Protection client — is now addressed: GetIdentityRiskScores parses the endpoint per call and propagates the error with a wrapped message. The new commits also bound buildIdentityIndex with a 100-page cap (partial-index Warn on overflow) and fix the per-sync cache so empty detection/identity results memoize instead of re-scanning. No new issues found. Security IssuesNone found. Correctness IssuesNone found. SuggestionsNone. |
- goconst: extract launcherUnknown constant - nonamedreturns: drop named returns from parseMCPServer - gosec G704: annotate the Alerts client request (host is configured, not user input) - regenerate baton_capabilities.json / config_schema.json to include the new mcp_server and endpoint_user resource types Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…gosec - docs/connector.mdx: add mcp_server / endpoint_user to the capabilities table and a 'Detect shadow MCP servers' section covering the Alerts: Read scope and the Custom IOA detection rule (satisfies the docs-match-metadata CI check) - identity_protection.go: annotate the pre-existing gosec G704 the shared lint config now enforces (host is configured, not user input) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
CI pins golangci-lint v2.11.4, which does not raise gosec G704 on the config-derived httpClient.Do calls (only the older local v2.10.1 did), so the //nolint directives were flagged unused by nolintlint. Remove them; reverts identity_protection.go to its original state. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
leet-c1
marked this pull request as ready for review
July 15, 2026 22:37
Addresses PR review feedback on the Alerts client: - cap the alert scan at 10 pages so the largest offset stays under CrowdStrike's ~10,000-record deep-pagination limit (offset >= 10000 is rejected) - retry 429 and 5xx responses with exponential backoff honoring Retry-After, so a transient rate-limit no longer aborts the whole sync Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
leet-c1
dismissed stale reviews from github-actions[bot], github-actions[bot], and github-actions[bot]
July 15, 2026 23:29
Resolved in 935a398: offset pagination capped below CrowdStrike's ~10k deep-pagination limit, and 429/5xx responses now retry with exponential backoff honoring Retry-After. Both inline suggestions addressed and their threads resolved; the bot's latest review on this commit is non-blocking (COMMENTED). Dismissing this stale changes-requested from an earlier commit.
…by default) Both new capabilities are now gated behind default-false config flags, so the connector's core user/role sync is unchanged and the new features only run when explicitly enabled: - crowdstrike-ingest-risk-scores → gates the security_insight (Identity Protection risk score) syncer. - crowdstrike-detect-shadow-mcp → gates the mcp_server + endpoint_user (shadow-MCP detection) syncers. Gating is at execution, not registration: the syncers stay registered (so baton_capabilities.json is unchanged and the capabilities remain advertised), but when their flag is off, security_insight.List and mcpSource.detections/ identityIndex short-circuit to empty and make NO CrowdStrike API call. This also fixes the prior always-attempted behavior, where a client without the Alerts / Identity Protection scopes would 403 on those resource types. Docs: added "Turn on ..." steps to both setup sections; capabilities table already labels these opt-in. config_schema.json regenerated (two new fields); baton_capabilities.json unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
leet-c1
added a commit
that referenced
this pull request
Jul 22, 2026
Extend the Identity Protection GraphQL query and security_insight mapping to
surface per-account password risk from CrowdStrike Falcon Identity Protection:
- Query: request `passwordAttributes { exposed strength }` on each user account
descriptor fragment in the entities query.
- Model: capture it as PasswordAttributes on AccountData.
- Map: an exposed (compromised/breached) password becomes a high-severity
EXPOSED_PASSWORD risk factor on the account's security insight; a weak
password becomes a medium-severity WEAK_PASSWORD factor. Entity-level risk
factors are unchanged; password factors are additive per account.
Verified against the live Identity Protection GraphQL schema (introspection):
the doc-suggested `assessment { compromisedPassword }` / COMPROMISED_PASSWORD
do not exist; the real signal is `passwordAttributes.exposed` (Boolean) +
`strength` (PasswordStrength: UNKNOWN|WEAK|STRONG) on the account descriptors,
confirmed populated live. No new scopes (same Identity Protection GraphQL
query) and no new resource type — enriches the existing security_insight.
Tests: passwordRiskFactors mapping (exposed/weak/none/case-insensitive) and a
resource-level assertion that EXPOSED_PASSWORD lands on the built insight.
Stacked on #64 (leet/shadow-mcp-detection).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Flip crowdstrike-ingest-risk-scores to default true so Identity Protection risk-score ingestion (security_insight) runs by default — it's the connector's headline "external insights" capability. It remains a config flag, so it can be turned off. Shadow-MCP detection (crowdstrike-detect-shadow-mcp) stays default false (opt-in). The execution gate is unchanged; only the default flips. Docs: capabilities table drops the "(opt-in)" qualifier on Insights; the setup section now describes it as on-by-default with an opt-out. config_schema.json regenerated (defaultValue: true). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
leet-c1
added a commit
that referenced
this pull request
Jul 22, 2026
Extend the Identity Protection GraphQL query and security_insight mapping to
surface per-account password risk from CrowdStrike Falcon Identity Protection:
- Query: request `passwordAttributes { exposed strength }` on each user account
descriptor fragment in the entities query.
- Model: capture it as PasswordAttributes on AccountData.
- Map: an exposed (compromised/breached) password becomes a high-severity
EXPOSED_PASSWORD risk factor on the account's security insight; a weak
password becomes a medium-severity WEAK_PASSWORD factor. Entity-level risk
factors are unchanged; password factors are additive per account.
Verified against the live Identity Protection GraphQL schema (introspection):
the doc-suggested `assessment { compromisedPassword }` / COMPROMISED_PASSWORD
do not exist; the real signal is `passwordAttributes.exposed` (Boolean) +
`strength` (PasswordStrength: UNKNOWN|WEAK|STRONG) on the account descriptors,
confirmed populated live. No new scopes (same Identity Protection GraphQL
query) and no new resource type — enriches the existing security_insight.
Tests: passwordRiskFactors mapping (exposed/weak/none/case-insensitive) and a
resource-level assertion that EXPOSED_PASSWORD lands on the built insight.
Stacked on #64 (leet/shadow-mcp-detection).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…drop custom retry The Identity Protection (GraphQL) and Alerts clients rolled their own HTTP: raw net/http + golang.org/x/oauth2/clientcredentials + a custom transport + manual JSON + a bespoke 429/5xx retry loop. Replace all of it with the SDK's uhttp: - Build one shared uhttp.BaseHttpClient in New() via uhttp.NewOAuth2ClientCredentials(...).GetClient(ctx, WithUserAgent(...)) + NewBaseHttpClientWithContext (both clients share the same OAuth2 grant/host). - Requests use bc.NewRequest(WithJSONBody/WithAcceptJSONHeader) + bc.Do( WithJSONResponse). uhttp.Do maps 429/5xx to codes.Unavailable and attaches rate-limit metadata. - DELETE the custom retry (doJSON loop, waitBackoff, retryAfterSeconds, mcpMaxRetries) — errors are returned up so baton's sync retryer handles retries with rate-limit-reset awareness. Also delete the hand-rolled OAuth2 transport and the unused RefreshContext. Kept: the connector-wide RateLimitInfo annotation convention (extracted from the uhttp response), GraphQL 200-level error handling, pagination bounds, and all sync behavior. User-Agent "baton-crowdstrike" preserved via uhttp.WithUserAgent. go.mod: x/oauth2 is now indirect (via uhttp). Verified: build, go test, golangci-lint v2.11.4 (0 issues); the uhttp OAuth2 + GraphQL path live-verified against a Falcon Identity Protection tenant. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
leet-c1
added a commit
that referenced
this pull request
Jul 22, 2026
Extend the Identity Protection GraphQL query and security_insight mapping to
surface per-account password risk from CrowdStrike Falcon Identity Protection:
- Query: request `passwordAttributes { exposed strength }` on each user account
descriptor fragment in the entities query.
- Model: capture it as PasswordAttributes on AccountData.
- Map: an exposed (compromised/breached) password becomes a high-severity
EXPOSED_PASSWORD risk factor on the account's security insight; a weak
password becomes a medium-severity WEAK_PASSWORD factor. Entity-level risk
factors are unchanged; password factors are additive per account.
Verified against the live Identity Protection GraphQL schema (introspection):
the doc-suggested `assessment { compromisedPassword }` / COMPROMISED_PASSWORD
do not exist; the real signal is `passwordAttributes.exposed` (Boolean) +
`strength` (PasswordStrength: UNKNOWN|WEAK|STRONG) on the account descriptors,
confirmed populated live. No new scopes (same Identity Protection GraphQL
query) and no new resource type — enriches the existing security_insight.
Tests: passwordRiskFactors mapping (exposed/weak/none/case-insensitive) and a
resource-level assertion that EXPOSED_PASSWORD lands on the built insight.
Stacked on #64 (leet/shadow-mcp-detection).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… url.Parse panic) Three findings from an adversarial review of the shadow-MCP + uhttp changes: 1. mcpSource cold-cache duplicate fetch: detections()/identityIndex() used check-unlock-fetch-lock, so the mcp_server and endpoint_user syncers (which run in parallel sharing one mcpSource) both missed the cache at sync start and each ran a full Alerts scan / identity-index build — 2x API cost and possible grant drift. Hold the lock across the fetch so exactly one runs per sync and the sibling blocks on and reuses the result. (Sequential, non-nested calls — no lock-ordering hazard.) 2. buildIdentityIndex looped over Identity Protection pages with NO cap — on a large tenant one sync becomes an unbounded sequence of GraphQL round trips. Bound it with mcpIdentityIndexMaxPages (100) + a truncation warning, mirroring mcpAlertMaxPages. 3. NewIdentityProtectionClient discarded the url.Parse error, leaving endpoint nil and panicking in uhttp.NewRequest on first use if host were malformed. Store host and parse the endpoint in GetIdentityRiskScores where the error is returned instead of panicking. Verified: build, go test, golangci-lint v2.11.4 (0 issues); IdP path re-verified live after the endpoint-parse change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
leet-c1
added a commit
that referenced
this pull request
Jul 22, 2026
Extend the Identity Protection GraphQL query and security_insight mapping to
surface per-account password risk from CrowdStrike Falcon Identity Protection:
- Query: request `passwordAttributes { exposed strength }` on each user account
descriptor fragment in the entities query.
- Model: capture it as PasswordAttributes on AccountData.
- Map: an exposed (compromised/breached) password becomes a high-severity
EXPOSED_PASSWORD risk factor on the account's security insight; a weak
password becomes a medium-severity WEAK_PASSWORD factor. Entity-level risk
factors are unchanged; password factors are additive per account.
Verified against the live Identity Protection GraphQL schema (introspection):
the doc-suggested `assessment { compromisedPassword }` / COMPROMISED_PASSWORD
do not exist; the real signal is `passwordAttributes.exposed` (Boolean) +
`strength` (PasswordStrength: UNKNOWN|WEAK|STRONG) on the account descriptors,
confirmed populated live. No new scopes (same Identity Protection GraphQL
query) and no new resource type — enriches the existing security_insight.
Tests: passwordRiskFactors mapping (exposed/weak/none/case-insensitive) and a
resource-level assertion that EXPOSED_PASSWORD lands on the built insight.
Stacked on #64 (leet/shadow-mcp-detection).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
An empty Alerts scan (or empty identity index) returned a nil slice/map, and the cache guard checked the data for non-nil — so a sync that found no shadow-MCP activity re-ran the scan on every caller (mcp_server.List and endpoint_user.List), defeating the one-scan-per-sync guarantee. Track an explicit per-sync fetched flag instead, so an empty result is a valid cached value. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
leet-c1
added a commit
that referenced
this pull request
Jul 22, 2026
Extend the Identity Protection GraphQL query and security_insight mapping to
surface per-account password risk from CrowdStrike Falcon Identity Protection:
- Query: request `passwordAttributes { exposed strength }` on each user account
descriptor fragment in the entities query.
- Model: capture it as PasswordAttributes on AccountData.
- Map: an exposed (compromised/breached) password becomes a high-severity
EXPOSED_PASSWORD risk factor on the account's security insight; a weak
password becomes a medium-severity WEAK_PASSWORD factor. Entity-level risk
factors are unchanged; password factors are additive per account.
Verified against the live Identity Protection GraphQL schema (introspection):
the doc-suggested `assessment { compromisedPassword }` / COMPROMISED_PASSWORD
do not exist; the real signal is `passwordAttributes.exposed` (Boolean) +
`strength` (PasswordStrength: UNKNOWN|WEAK|STRONG) on the account descriptors,
confirmed populated live. No new scopes (same Identity Protection GraphQL
query) and no new resource type — enriches the existing security_insight.
Tests: passwordRiskFactors mapping (exposed/weak/none/case-insensitive) and a
resource-level assertion that EXPOSED_PASSWORD lands on the built insight.
Stacked on #64 (leet/shadow-mcp-detection).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
What
Adds shadow-MCP-server discovery to
baton-crowdstrike. It reads CrowdStrike EDR detections, identifies unsanctioned Model Context Protocol servers by command-line signature (@modelcontextprotocol/server-*,mcp-server-*,mcp_server_*), and models them in ConductorOne so a shadow MCP shows up against a real person in access reviews:mcp_server(App trait) — one per deduped (host, user, server). Profile carries server/endpoint/identity metadata (server_name,package,launcher,transport,host,aid,local_ip,endpoint_user,sample_command_line,detection_count,ioa_rule,falcon_link,sanctioned,first_seen, andidentity_*when resolved), plus a security-insight issue bound to the resolved identity.endpoint_user(User trait) — the OS user that ran the server, as an app account; auto-matched to a c1 identity by email when it resolves in Identity Protection, otherwise assignable manually.runnerassignment entitlement on eachmcp_server, granted to theendpoint_useraccount.Result: a c1 identity →
endpoint_useraccount →runnergrant →mcp_serverchain.How
product:'epp'filter and paginated to exhaustion (bounded, logs if capped); matches the MCP signature client-side.samAccountName/ email local-part.mcp_serverandendpoint_usersyncers share one per-sync (SyncID-keyed) detection snapshot + identity index, so grants are consistent and Alerts/Identity-Protection aren't re-queried per resource.Scopes
Adds Alerts: Read (documented in the README). Correlation reuses the existing Identity Protection scopes.
Detecting the servers
CrowdStrike raises the detections this connector reads via a Custom IOA Process-Creation rule (Detect disposition) matching the signature; documented in the README. MCP servers are benign to default detections, so this rule is the prerequisite for any findings.
Testing
parseMCPServer) and the ambiguous-identity guard (resolveIdentity).mcp_serverresources +endpoint_useraccounts +runnergrants, with each server correlated to the identity of the OS user that ran it.Notes
Hardened after an adversarial review: server-side-filtered + paginated detection fetch (was newest-1000-only), shared per-sync snapshot (removes per-resource re-fetch and grant drift), stable resource IDs derived from the deduped server name, ambiguous identity matches resolve to no identity (avoids attributing a finding to the wrong person), and unattributable detections (missing AID/user) are dropped.
Opened as a draft.
🤖 Generated with Claude Code