-
Notifications
You must be signed in to change notification settings - Fork 706
fix: MCP error because types are not enforced. #2422
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 10 commits
aa707f4
355c3e9
a15fe81
819c3a3
771c3a8
ecd745f
e71e8d1
921f059
e655cbb
09e5d1e
a5d2526
d64b233
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
|
|
||
| [TestIntegration_MCP_SSE_Subprocess/GetVulnerabilityDetails - 1] | ||
| { | ||
| "content": [ | ||
| { | ||
| "type": "text", | ||
| "text": "{\n \"schema_version\": \"1.7.3\",\n \"id\": \"GO-2023-1558\",\n \"published\": \"2023-02-14T19:41:21Z\",\n \"modified\": \"2024-05-20T16:03:47Z\",\n \"aliases\": [\"CVE-2023-23626\", \"GHSA-2h6c-j3gf-xp9r\"],\n \"summary\": \"Denial of service via malformed size parameters in github.com/ipfs/go-bitfield\",\n \"details\": \"When feeding untrusted user input into the size parameter of NewBitfield and FromBytes functions, an attacker can trigger panics.\\n\\nThis happens when the size is a not a multiple of 8 or is negative.\\n\\nA workaround is to ensure size%8 == 0 \u0026\u0026 size \u003e= 0 yourself before calling NewBitfield or FromBytes.\",\n \"affected\": [\n {\n \"package\": {\n \"name\": \"github.com/ipfs/go-bitfield\",\n \"ecosystem\": \"Go\",\n \"purl\": \"pkg:golang/github.com/ipfs/go-bitfield\"\n },\n \"ranges\": [\n {\n \"type\": \"SEMVER\",\n \"events\": [\n {\n \"introduced\": \"0\"\n },\n {\n \"fixed\": \"1.1.0\"\n }\n ]\n }\n ],\n \"ecosystem_specific\": {\n \"imports\": [\n {\n \"path\": \"github.com/ipfs/go-bitfield\",\n \"symbols\": [\"FromBytes\", \"NewBitfield\"]\n }\n ]\n },\n \"database_specific\": {\n \"source\": \"https://vuln.go.dev/ID/GO-2023-1558.json\"\n }\n }\n ],\n \"references\": [\n {\n \"type\": \"ADVISORY\",\n \"url\": \"https://github.com/ipfs/go-bitfield/security/advisories/GHSA-2h6c-j3gf-xp9r\"\n },\n {\n \"type\": \"FIX\",\n \"url\": \"https://github.com/ipfs/go-bitfield/commit/5e1d256fe043fc4163343ccca83862c69c52e579\"\n }\n ],\n \"database_specific\": {\n \"review_status\": \"REVIEWED\",\n \"url\": \"https://pkg.go.dev/vuln/GO-2023-1558\"\n },\n \"credits\": [\n {\n \"name\": \"Jorropo\"\n }\n ]\n}\n" | ||
| } | ||
| ] | ||
| } | ||
| --- | ||
|
|
||
| [TestIntegration_MCP_SSE_Subprocess/ScanVulnerableDependencies - 1] | ||
|
|
||
| Total 1 package affected by 1 known vulnerability (0 Critical, 0 High, 1 Medium, 0 Low, 0 Unknown) from 1 ecosystem. | ||
| 1 vulnerability can be fixed. | ||
|
|
||
| Go | ||
|
|
||
| lockfile:<rootdir>/testdata/go-project/go.mod: found 1 package with issues | ||
|
|
||
| github.com/ipfs/go-bitfield@1.0.0 has the following known vulnerabilities: | ||
| GO-2023-1558: Denial of service via malformed size parameters in github.com/ipfs/go-bitfield | ||
| Severity: '5.9'; Minimal Fix Version: '1.1.0'; | ||
|
|
||
| 1 known vulnerability found in lockfile:<rootdir>/testdata/go-project/go.mod | ||
| Hiding 2 number of vulnerabilities deemed unimportant, use --all-vulns to show them. | ||
|
|
||
|
|
||
| --- |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,209 @@ | ||
| package mcp_test | ||
|
|
||
| import ( | ||
| "context" | ||
| "net" | ||
| "net/http" | ||
| "os" | ||
| "os/exec" | ||
| "path/filepath" | ||
| "runtime" | ||
| "testing" | ||
| "time" | ||
|
|
||
| "github.com/google/osv-scanner/v2/internal/cachedregexp" | ||
| "github.com/google/osv-scanner/v2/internal/testutility" | ||
| "github.com/modelcontextprotocol/go-sdk/mcp" | ||
| ) | ||
|
|
||
| // TestIntegration_MCP_SSE_Subprocess validates the experimental-mcp command by: | ||
| // 1. Building the binary. | ||
| // 2. Starting it as an MCP server. | ||
| // 3. Connecting a client. | ||
| // 4. Running tools (scan_vulnerable_dependencies, get_vulnerability_details). | ||
| // | ||
| //nolint:paralleltest // This test is not parallelizable | ||
| func TestIntegration_MCP_SSE_Subprocess(t *testing.T) { | ||
| if testing.Short() { | ||
| testutility.Skip(t, "skipping integration test in short mode") | ||
| } | ||
|
|
||
| binPath := buildTestBinary(t) | ||
| addr := findFreePort(t) | ||
|
|
||
| // Start the server | ||
| ctx, cancel := context.WithCancel(context.Background()) | ||
| defer cancel() | ||
|
|
||
| cmdRun := startMCPServer(t, ctx, binPath, addr) | ||
| defer func() { | ||
| cancel() | ||
| _ = cmdRun.Wait() | ||
| }() | ||
|
|
||
| // Wait for server to be ready | ||
| baseURL := "http://" + addr + "/sse" | ||
| waitForServer(t, baseURL) | ||
|
|
||
| // Connect Client | ||
| client := connectMCPClient(t, ctx, baseURL) | ||
| defer client.Close() | ||
|
|
||
| // Use persistent testdata/go-project | ||
| testDataPath, err := filepath.Abs("testdata/go-project") | ||
| if err != nil { | ||
| t.Fatalf("failed to get absolute path: %v", err) | ||
| } | ||
| if _, err := os.Stat(testDataPath); os.IsNotExist(err) { | ||
| t.Fatalf("testdata/go-project does not exist at %s", testDataPath) | ||
| } | ||
|
|
||
| var vulnID string | ||
|
|
||
| // Step 1: Scan for vulnerabilities | ||
| t.Run("ScanVulnerableDependencies", func(t *testing.T) { | ||
| scanResult, err := client.CallTool(ctx, &mcp.CallToolParams{ | ||
| Name: "scan_vulnerable_dependencies", | ||
| Arguments: map[string]any{ | ||
| "paths": []string{testDataPath}, | ||
| "recursive": true, | ||
| "ignore_glob_patterns": []string{}, | ||
| }, | ||
| }) | ||
| if err != nil { | ||
| t.Fatalf("call to scan_vulnerable_dependencies failed: %v", err) | ||
| } | ||
|
|
||
| if len(scanResult.Content) == 0 { | ||
| t.Fatal("scan result content is empty") | ||
| } | ||
|
|
||
| textRes, ok := scanResult.Content[0].(*mcp.TextContent) | ||
| if !ok { | ||
| t.Fatalf("expected TextContent, got %T", scanResult.Content[0]) | ||
| } | ||
|
|
||
| output := textRes.Text | ||
| t.Logf("Scan completed. Output length: %d", len(output)) | ||
| testutility.NewSnapshot().MatchText(t, output) | ||
|
|
||
| // Extract a vulnerability ID for the next step. | ||
| // Example: GHSA-f682-3w9h-r22r or GO-2023-1558 | ||
| re := cachedregexp.MustCompile(`(GHSA-[a-zA-Z0-9-]+|GO-\d{4}-\d+)`) | ||
|
another-rex marked this conversation as resolved.
Outdated
|
||
| vulnID = re.FindString(output) | ||
| }) | ||
|
|
||
| if vulnID == "" { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is redundant now?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah good point, removed. |
||
| t.Fatal("cannot test get_vulnerability_details without a valid ID found in scan output") | ||
| } | ||
| t.Logf("Found Vuln ID: %s", vulnID) | ||
|
|
||
| // Step 2: Get details for the found vulnerability | ||
| t.Run("GetVulnerabilityDetails", func(t *testing.T) { | ||
| detailsResult, err := client.CallTool(ctx, &mcp.CallToolParams{ | ||
| Name: "get_vulnerability_details", | ||
| Arguments: map[string]any{ | ||
| "vuln_id": vulnID, | ||
| }, | ||
| }) | ||
| if err != nil { | ||
| t.Fatalf("call to get_vulnerability_details failed: %v", err) | ||
| } | ||
|
|
||
| if len(detailsResult.Content) == 0 { | ||
| t.Log("Details Result Content is empty") | ||
| return | ||
| } | ||
|
|
||
| t.Logf("Details Result Content: %v", detailsResult.Content[0]) | ||
| testutility.NewSnapshot().MatchJSON(t, detailsResult) | ||
| }) | ||
| } | ||
|
|
||
| // buildTestBinary builds the osv-scanner binary to a temporary directory. | ||
| func buildTestBinary(t *testing.T) string { | ||
| t.Helper() | ||
| tempDir := t.TempDir() | ||
| binPath := filepath.Join(tempDir, "osv-scanner-mcp-test") | ||
| if runtime.GOOS == "windows" { | ||
| binPath += ".exe" | ||
| } | ||
|
|
||
| // We use the full package path to ensure we build the correct main package. | ||
| cmdBuild := exec.CommandContext(context.Background(), "go", "build", "-o", binPath, "github.com/google/osv-scanner/v2/cmd/osv-scanner") | ||
| cmdBuild.Stdout = os.Stdout | ||
| cmdBuild.Stderr = os.Stderr | ||
| if err := cmdBuild.Run(); err != nil { | ||
| t.Fatalf("failed to build binary: %v", err) | ||
| } | ||
|
|
||
| return binPath | ||
| } | ||
|
|
||
| // findFreePort lets the OS choose a free port and returns the address string (e.g. "127.0.0.1:12345"). | ||
| func findFreePort(t *testing.T) string { | ||
| t.Helper() | ||
| var lc net.ListenConfig | ||
| ln, err := lc.Listen(context.Background(), "tcp", "localhost:0") | ||
| if err != nil { | ||
| t.Fatalf("failed to listen: %v", err) | ||
| } | ||
| addr := ln.Addr().String() | ||
| ln.Close() | ||
|
|
||
| return addr | ||
| } | ||
|
|
||
| // startMCPServer starts the mcp server in a subprocess. | ||
| // | ||
| //nolint:revive // t should be the first argument | ||
| func startMCPServer(t *testing.T, ctx context.Context, binPath, addr string) *exec.Cmd { | ||
| t.Helper() | ||
| cmdRun := exec.CommandContext(ctx, binPath, "experimental-mcp", "--sse", addr) | ||
| cmdRun.Stderr = os.Stderr | ||
| cmdRun.Stdout = os.Stdout | ||
|
|
||
| t.Logf("Starting MCP server on %s", addr) | ||
| if err := cmdRun.Start(); err != nil { | ||
| t.Fatalf("failed to start server: %v", err) | ||
| } | ||
|
|
||
| return cmdRun | ||
| } | ||
|
|
||
| // connectMCPClient connects to the MCP server via SSE. | ||
| // | ||
| //nolint:revive // t should be the first argument | ||
| func connectMCPClient(t *testing.T, ctx context.Context, baseURL string) *mcp.ClientSession { | ||
| t.Helper() | ||
| transport := &mcp.SSEClientTransport{ | ||
| Endpoint: baseURL, | ||
| } | ||
|
|
||
| client := mcp.NewClient(&mcp.Implementation{ | ||
| Name: "test-client", | ||
| Version: "1.0.0", | ||
| }, nil) | ||
|
|
||
| session, err := client.Connect(ctx, transport, nil) | ||
| if err != nil { | ||
| t.Fatalf("failed to connect to MCP server: %v", err) | ||
| } | ||
|
|
||
| return session | ||
| } | ||
|
|
||
| func waitForServer(t *testing.T, url string) { | ||
| t.Helper() | ||
| deadline := time.Now().Add(10 * time.Second) | ||
| for time.Now().Before(deadline) { | ||
| //nolint:gosec,noctx // This is a test with a local URL | ||
| resp, err := http.Get(url) | ||
| if err == nil { | ||
| resp.Body.Close() | ||
| return | ||
| } | ||
| time.Sleep(100 * time.Millisecond) | ||
| } | ||
| t.Fatalf("server failed to start listening at %s within timeout", url) | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| module example.com/test | ||
|
|
||
| go 1.25.3 | ||
|
|
||
| require github.com/ipfs/go-bitfield v1.0.0 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| github.com/ipfs/go-bitfield v1.0.0 h1:y/XHm2GEmD9wKngheWNNCNL0pzrWXZwCdQGv1ikXknQ= | ||
| github.com/ipfs/go-bitfield v1.0.0/go.mod h1:N/UiujQy+K+ceU1EF5EkVd1TNqevLrCQMIcAEPrdtus= |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| package main | ||
|
|
||
| import ( | ||
| "github.com/ipfs/go-bitfield" | ||
| ) | ||
|
|
||
| func main() { | ||
| _ = bitfield.NewBitfield(1) | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| package mcp_test | ||
|
|
||
| import ( | ||
| "log/slog" | ||
| "testing" | ||
|
|
||
| "github.com/google/osv-scanner/v2/cmd/osv-scanner/internal/cmd" | ||
| "github.com/google/osv-scanner/v2/cmd/osv-scanner/internal/testcmd" | ||
| "github.com/google/osv-scanner/v2/cmd/osv-scanner/mcp" | ||
| "github.com/google/osv-scanner/v2/internal/testlogger" | ||
| "github.com/google/osv-scanner/v2/internal/testutility" | ||
| ) | ||
|
|
||
| func TestMain(m *testing.M) { | ||
| slog.SetDefault(slog.New(testlogger.New())) | ||
| // This is technically not necessary, as we are running mcp via a subprocess | ||
| testcmd.CommandsUnderTest = []cmd.CommandBuilder{mcp.Command} | ||
| m.Run() | ||
|
|
||
| testutility.CleanSnapshots(m) | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,4 +2,4 @@ | |
|
|
||
| set -ex | ||
|
|
||
| go run github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.6.1 run ./... | ||
| GOTOOLCHAIN=go1.25.5 go run github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.6.1 run ./... $@ | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is this intended? this will probably override the environment variable specified in a command.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is intended, though good point about it overriding, fixed now. Right now on our machines run_lint script won't work because it uses the system's go version rather than the projects go version. |
||
Uh oh!
There was an error while loading. Please reload this page.