fleet-mcp: document required Fleet API endpoints and verify them at startup - #50817
fleet-mcp: document required Fleet API endpoints and verify them at startup#50817lukeheath wants to merge 1 commit into
Conversation
…tartup Deployments that lock the API-only user down with endpoint restrictions had no way to catch allowlist drift when the MCP toolset grew: tools failed at runtime with opaque 403s. Add a requiredEndpoints list as the source of truth, a non-fatal background self-check that probes each route at startup and warns on 403s, a drift test that fails when the package starts calling a Fleet API route missing from the list, and a README section enumerating the routes to allowlist. Resolves #50814
WalkthroughThe MCP server now defines and probes all required Fleet API endpoints during startup. The check uses a 30-second overall timeout, classifies forbidden and transport failures separately, drains responses, and uses side-effect-free POST requests. Startup continues without waiting for verification. Tests validate endpoint behavior, path normalization, POST probe safety, and coverage of Fleet API routes used by the source. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cmd/fleet-mcp/startup_check_test.go`:
- Around line 150-190: Update the startup endpoint validation around callRe to
resolve local endpoint variables assigned Fleet API paths, preserving their
associated HTTP method when checking coveredMethod and requiredEndpoints. Use Go
AST analysis with local assignment tracking or shared method/path descriptors,
and add a fixture that verifies a variable endpoint path with a mismatched
method is rejected.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 40f08067-ef2d-4163-b20e-e22032e65333
⛔ Files ignored due to path filters (1)
cmd/fleet-mcp/README.mdis excluded by!**/*.md
📒 Files selected for processing (3)
cmd/fleet-mcp/main.gocmd/fleet-mcp/startup_check.gocmd/fleet-mcp/startup_check_test.go
| litRe := regexp.MustCompile(`"(/api/v1/fleet/[^"]*)"`) | ||
| // Method-aware variant for call sites where the method and path literal | ||
| // share a line, e.g. makeFleetRequest(ctx, "POST", "/api/v1/fleet/...", | ||
| // or with the path wrapped in fmt.Sprintf. | ||
| callRe := regexp.MustCompile(`makeFleetRequest\(ctx, "(GET|POST|PUT|PATCH|DELETE)",\s*(?:fmt\.Sprintf\()?"(/api/v1/fleet/[^"]*)"`) | ||
| files, err := filepath.Glob("*.go") | ||
| if err != nil { | ||
| t.Fatal(err) | ||
| } | ||
| for _, f := range files { | ||
| // startup_check.go defines the list itself; its probe literals are | ||
| // not additional API usage. | ||
| if strings.HasSuffix(f, "_test.go") || f == "startup_check.go" { | ||
| continue | ||
| } | ||
| src, err := os.ReadFile(f) | ||
| if err != nil { | ||
| t.Fatal(err) | ||
| } | ||
| for _, m := range litRe.FindAllStringSubmatch(string(src), -1) { | ||
| lit := m[1] | ||
| norm := normalizeAPIPath(lit) | ||
| if _, ok := exemptPaths[norm]; ok { | ||
| continue | ||
| } | ||
| if _, ok := covered[norm]; !ok { | ||
| t.Errorf("%s uses Fleet API path %q which is not listed in requiredEndpoints (startup_check.go); add it there and to the README endpoint list", f, lit) | ||
| } | ||
| } | ||
| for _, m := range callRe.FindAllStringSubmatch(string(src), -1) { | ||
| method, lit := m[1], m[2] | ||
| norm := normalizeAPIPath(lit) | ||
| if _, ok := exemptPaths[norm]; ok { | ||
| continue | ||
| } | ||
| if _, ok := exemptMethodPaths[method+" "+norm]; ok { | ||
| continue | ||
| } | ||
| if _, ok := coveredMethod[method+" "+norm]; !ok { | ||
| t.Errorf("%s calls %s %s which is not listed (with that method) in requiredEndpoints (startup_check.go); add it there and to the README endpoint list", f, method, lit) | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Validate methods for variable endpoint paths.
callRe does not match calls that pass a local path variable. For example, cmd/fleet-mcp/fleet_integration.go:344-354 passes endpoint to makeFleetRequest after assigning the Fleet path on an earlier line.
The generic litRe check still finds the path. A future change from GET to POST can therefore pass this test while requiredEndpoints continues to probe GET. The startup check can then report success although the live tool requires a blocked method.
Use Go AST analysis with local assignment tracking, or define the tool routes from shared method-and-path descriptors. Add a fixture for a variable endpoint path with a method mismatch.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cmd/fleet-mcp/startup_check_test.go` around lines 150 - 190, Update the
startup endpoint validation around callRe to resolve local endpoint variables
assigned Fleet API paths, preserving their associated HTTP method when checking
coveredMethod and requiredEndpoints. Use Go AST analysis with local assignment
tracking or shared method/path descriptors, and add a fixture that verifies a
variable endpoint path with a mismatched method is rejected.
There was a problem hiding this comment.
Pull request overview
Adds a canonical, documented allowlist of Fleet API routes required by cmd/fleet-mcp, plus a non-fatal startup self-check that probes those routes with the configured API-only token and logs actionable warnings for any HTTP 403 blocks (without delaying readiness).
Changes:
- Introduces
requiredEndpointsand a background startup probe (verifyRequiredEndpoints) to detect allowlist drift early. - Adds tests to validate probe behavior and guard against endpoint-list drift from source usage.
- Documents the required endpoint list and the startup self-check behavior in
cmd/fleet-mcp/README.md.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| cmd/fleet-mcp/startup_check.go | Adds the required endpoint list and startup probing/logging logic. |
| cmd/fleet-mcp/startup_check_test.go | Adds tests for probing outcomes and drift/side-effect safety assertions. |
| cmd/fleet-mcp/README.md | Documents required endpoints and explains the startup self-check and exclusions. |
| cmd/fleet-mcp/main.go | Starts the self-check concurrently after API-only verification. |
Suppressed comments (1)
cmd/fleet-mcp/startup_check_test.go:92
- TestRequiredEndpoints_ProbesAreSideEffectFree currently only checks that POST probes have a non-nil body, but it doesn’t verify that the body is actually the intended empty JSON object ({}). That means a future change could accidentally add query SQL or other fields and the test would still pass, contradicting the test’s own comment and the PR description.
// Every POST probe must carry a body that fails Fleet-side validation
// before anything is created or executed: no query SQL, no real
// host/query IDs in the path.
realIDRe := regexp.MustCompile(`^[1-9][0-9]*$`)
for _, e := range requiredEndpoints {
if e.method == "GET" {
continue
}
if e.body == nil {
t.Errorf("POST probe %s must send a JSON body", e.route)
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Drain a bounded amount so the connection can be reused. | ||
| _, _ = io.Copy(io.Discard, io.LimitReader(resp.Body, 4096)) | ||
| resp.Body.Close() |
| import ( | ||
| "net/http" | ||
| "net/http/httptest" | ||
| "os" | ||
| "path/filepath" | ||
| "regexp" | ||
| "slices" | ||
| "strings" | ||
| "testing" | ||
| ) |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #50817 +/- ##
=======================================
Coverage 68.52% 68.53%
=======================================
Files 3977 3977
Lines 256094 256142 +48
Branches 13658 13658
=======================================
+ Hits 175489 175536 +47
+ Misses 64987 64986 -1
- Partials 15618 15620 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Related issue: Resolves #50814
Checklist for submitter
SELECT *is avoided, SQL injection is prevented (using placeholders for values in statements), JS inline code is prevented especially for url redirects, and untrusted data interpolated into shell scripts/commands is validated against shell metacharacters.Testing
Details
The dogfood Fleet Slack bot runs fleet-mcp with a least-privilege API-only user restricted to specific API endpoints. The MCP toolset has grown since that allowlist was written, so tools started failing with opaque 403s that nothing caught at deploy time.
This PR:
requiredEndpoints(newstartup_check.go): the canonical list of the 19 Fleet API routes the MCP toolset depends on, matching the list enumerated in the issue./healthzon SSE, stdin on stdio) never waits on a slow Fleet (30s cap). If Fleet becomes unreachable mid-check, the summary reports the check as incomplete rather than claiming an all-clear, and both blocked and unverified counts are reported when both occur.0or a sentinel identifier that never exists. The two POST probes send empty JSON bodies that Fleet rejects during validation:POST /reports/runwith no query/query_id errors inNewDistributedQueryCampaignbefore any query row or campaign is created, andPOST /hosts/0/queryfails the host lookup inrunLiveQueryOnHostByIDEndpointbefore the query is examined. A test (TestRequiredEndpoints_ProbesAreSideEffectFree) enforces this shape.TestRequiredEndpointsCoverSourcePathsscans the package source for Fleet API path literals and fails if one is missing fromrequiredEndpoints, so adding a tool that calls a new Fleet route forces the list (and README) to be updated. Where the HTTP method is extractable (method and path literal on the samemakeFleetRequestline), the check is method-aware, so a new POST on an already-listed GET path is caught too. Exempt:GET /api/v1/fleet/results/websocket(raw handler, not subject to endpoint restrictions) andPOST /api/v1/fleet/reports(developer-only-seedmode).No new dependencies; the module's go.mod is unchanged.
Manual QA
Built the binary and ran it against a fake Fleet server:
startup self-check: all 19 required Fleet API endpoints are reachable with the configured token, and the fake server's request log confirms all 19 probes with{}POST bodies and no other traffic.... returned HTTP 403 — blocked by the API-only user's endpoint restrictions or the token's role; MCP tools that call it will fail) plus the summary, and the server still starts.startup self-check: incomplete — 16 of 19 ... could not be probedsummary; the server still starts.GET /healthzreturns 200 immediately (readiness does not wait on the check) while the warnings land in the log.Summary by CodeRabbit