Files
Sam Morrow 912cce687b Sanitize remaining issue-ref and blame headline response paths
Route every MinimalIssueRef/MinimalPullRequestRef construction through shared
constructors that sanitize the user-authored title, so issue_dependency_read,
issue_dependency_write and find_duplicate no longer forward raw issue titles.

Also sanitize the get_file_blame commit message headline, after truncation so
the headline is still cut at the author's real first line break.

Extends the sanitization regression suite with the project status update body,
both ref constructors and the dependency ref, and adds tool-level regression
tests for find_duplicate and get_file_blame.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
2026-08-19 15:20:33 +02:00

386 lines
13 KiB
Go

package github
import (
"context"
"encoding/json"
"net/http"
"net/url"
"testing"
"github.com/github/github-mcp-server/internal/toolsnaps"
"github.com/github/github-mcp-server/pkg/translations"
"github.com/google/jsonschema-go/jsonschema"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
const endpointSemanticallySimilar = EndpointPattern("GET /repos/{owner}/{repo}/issues/{issue_number}/semantically_similar")
func Test_FindDuplicate(t *testing.T) {
// Verify tool definition once (flag-gated variant snap).
serverTool := FindDuplicate(translations.NullTranslationHelper)
tool := serverTool.Tool
require.NoError(t, toolsnaps.Test(tool.Name+"_ff_"+FeatureFlagDuplicateDetection, tool))
require.Equal(t, FeatureFlagDuplicateDetection, serverTool.FeatureFlagEnable)
assert.Equal(t, "find_duplicate", tool.Name)
assert.NotEmpty(t, tool.Description)
assert.True(t, tool.Annotations.ReadOnlyHint)
assert.ElementsMatch(t, serverTool.RequiredScopes, []string{"repo"})
schema := tool.InputSchema.(*jsonschema.Schema)
assert.Contains(t, schema.Properties, "owner")
assert.Contains(t, schema.Properties, "repo")
assert.Contains(t, schema.Properties, "issue_number")
assert.Contains(t, schema.Properties, "confidence_threshold")
assert.Contains(t, schema.Properties, "page")
assert.Contains(t, schema.Properties, "perPage")
assert.ElementsMatch(t, schema.Required, []string{"owner", "repo", "issue_number"})
}
func Test_FindDuplicate_RankedResults(t *testing.T) {
serverTool := FindDuplicate(translations.NullTranslationHelper)
rankedResults := []map[string]any{
{
"issue": map[string]any{
"number": 456,
"title": "Example failure when saving",
"state": "open",
"html_url": "https://github.com/owner/repo/issues/456",
},
"score": 0.95,
"confidence": "high",
"likely_duplicate": true,
},
{
"issue": map[string]any{
"number": 789,
"title": "Possibly related",
"state": "closed",
"html_url": "https://github.com/owner/repo/issues/789",
},
"score": nil, // score is nullable
"confidence": "low",
"likely_duplicate": false,
},
}
var capturedURL *url.URL
var capturedMethod string
handler := func(w http.ResponseWriter, r *http.Request) {
capturedURL = r.URL
capturedMethod = r.Method
w.WriteHeader(http.StatusOK)
_, _ = w.Write(MustMarshal(rankedResults))
}
client := mustNewGHClient(t, NewMockedHTTPClient(WithRequestMatchHandler(endpointSemanticallySimilar, http.HandlerFunc(handler))))
deps := BaseDeps{Client: client}
toolHandler := serverTool.Handler(deps)
request := createMCPRequest(map[string]any{
"owner": "owner",
"repo": "repo",
"issue_number": float64(123),
"confidence_threshold": float64(0.8),
"perPage": float64(10),
"page": float64(1),
})
result, err := toolHandler(ContextWithDeps(context.Background(), deps), &request)
require.NoError(t, err)
require.False(t, result.IsError, "expected result to not be an error")
// The tool must be read-only: only a GET is issued.
assert.Equal(t, http.MethodGet, capturedMethod)
// confidence_threshold maps to threshold; perPage maps to per_page; page is forwarded.
require.NotNil(t, capturedURL)
assert.Equal(t, "0.8", capturedURL.Query().Get("threshold"))
assert.Equal(t, "10", capturedURL.Query().Get("per_page"))
assert.Equal(t, "1", capturedURL.Query().Get("page"))
text := getTextResult(t, result)
var candidates []duplicateCandidate
require.NoError(t, json.Unmarshal([]byte(text.Text), &candidates))
require.Len(t, candidates, 2)
assert.Equal(t, "high", candidates[0].Confidence)
assert.True(t, candidates[0].LikelyDuplicate)
require.NotNil(t, candidates[0].Score)
assert.InDelta(t, 0.95, *candidates[0].Score, 0.0001)
assert.Equal(t, 456, candidates[0].Issue.Number)
assert.Equal(t, "Example failure when saving", candidates[0].Issue.Title)
assert.Equal(t, "open", candidates[0].Issue.State)
assert.Equal(t, "https://github.com/owner/repo/issues/456", candidates[0].Issue.URL)
// A null score must decode successfully.
assert.Nil(t, candidates[1].Score)
assert.Equal(t, "low", candidates[1].Confidence)
assert.False(t, candidates[1].LikelyDuplicate)
}
// Test_FindDuplicate_SanitizesIssueTitle asserts that candidate issue titles, which are
// user-authored content from an arbitrary repository, are sanitized before being returned.
// Without this the tool would forward hidden-instruction payloads straight to the model.
func Test_FindDuplicate_SanitizesIssueTitle(t *testing.T) {
serverTool := FindDuplicate(translations.NullTranslationHelper)
rankedResults := []map[string]any{
{
"issue": map[string]any{
"number": 456,
"title": maliciousText,
"state": "open",
"html_url": "https://github.com/owner/repo/issues/456",
},
"score": 0.95,
"confidence": "high",
"likely_duplicate": true,
},
}
handler := func(w http.ResponseWriter, _ *http.Request) {
w.WriteHeader(http.StatusOK)
_, _ = w.Write(MustMarshal(rankedResults))
}
client := mustNewGHClient(t, NewMockedHTTPClient(WithRequestMatchHandler(endpointSemanticallySimilar, http.HandlerFunc(handler))))
deps := BaseDeps{Client: client}
toolHandler := serverTool.Handler(deps)
request := createMCPRequest(map[string]any{
"owner": "owner",
"repo": "repo",
"issue_number": float64(123),
})
result, err := toolHandler(ContextWithDeps(context.Background(), deps), &request)
require.NoError(t, err)
require.False(t, result.IsError, "expected result to not be an error")
text := getTextResult(t, result)
var candidates []duplicateCandidate
require.NoError(t, json.Unmarshal([]byte(text.Text), &candidates))
require.Len(t, candidates, 1)
assert.Equal(t, sanitizedText, candidates[0].Issue.Title)
assert.NotContains(t, text.Text, "<script>")
}
func Test_FindDuplicate_OmitsUnsetParams(t *testing.T) {
serverTool := FindDuplicate(translations.NullTranslationHelper)
var capturedURL *url.URL
handler := func(w http.ResponseWriter, r *http.Request) {
capturedURL = r.URL
w.WriteHeader(http.StatusOK)
_, _ = w.Write([]byte(`[]`))
}
client := mustNewGHClient(t, NewMockedHTTPClient(WithRequestMatchHandler(endpointSemanticallySimilar, http.HandlerFunc(handler))))
deps := BaseDeps{Client: client}
toolHandler := serverTool.Handler(deps)
request := createMCPRequest(map[string]any{
"owner": "owner",
"repo": "repo",
"issue_number": float64(123),
})
result, err := toolHandler(ContextWithDeps(context.Background(), deps), &request)
require.NoError(t, err)
require.False(t, result.IsError)
require.NotNil(t, capturedURL)
q := capturedURL.Query()
_, hasThreshold := q["threshold"]
_, hasPerPage := q["per_page"]
_, hasPage := q["page"]
assert.False(t, hasThreshold, "threshold should be omitted when unset")
assert.False(t, hasPerPage, "per_page should be omitted when unset")
assert.False(t, hasPage, "page should be omitted when unset")
}
func Test_FindDuplicate_EmptyResults(t *testing.T) {
serverTool := FindDuplicate(translations.NullTranslationHelper)
client := mustNewGHClient(t, NewMockedHTTPClient(WithRequestMatch(endpointSemanticallySimilar, []map[string]any{})))
deps := BaseDeps{Client: client}
toolHandler := serverTool.Handler(deps)
request := createMCPRequest(map[string]any{
"owner": "owner",
"repo": "repo",
"issue_number": float64(123),
})
result, err := toolHandler(ContextWithDeps(context.Background(), deps), &request)
require.NoError(t, err)
require.False(t, result.IsError, "empty results is a successful search")
text := getTextResult(t, result)
var candidates []duplicateCandidate
require.NoError(t, json.Unmarshal([]byte(text.Text), &candidates))
assert.Empty(t, candidates)
}
func Test_FindDuplicate_LegacyBareIssueResponse(t *testing.T) {
serverTool := FindDuplicate(translations.NullTranslationHelper)
// When ranked duplicate detection is disabled the endpoint returns bare
// issue resources (no ranking metadata), which must fail clearly.
bareIssues := []map[string]any{
{
"number": 456,
"title": "Example",
"state": "open",
"html_url": "https://github.com/owner/repo/issues/456",
},
}
client := mustNewGHClient(t, NewMockedHTTPClient(WithRequestMatch(endpointSemanticallySimilar, bareIssues)))
deps := BaseDeps{Client: client}
toolHandler := serverTool.Handler(deps)
request := createMCPRequest(map[string]any{
"owner": "owner",
"repo": "repo",
"issue_number": float64(123),
})
result, err := toolHandler(ContextWithDeps(context.Background(), deps), &request)
require.NoError(t, err)
getErrorResult(t, result)
}
func Test_FindDuplicate_Errors(t *testing.T) {
serverTool := FindDuplicate(translations.NullTranslationHelper)
t.Run("missing required param", func(t *testing.T) {
client := mustNewGHClient(t, NewMockedHTTPClient())
deps := BaseDeps{Client: client}
toolHandler := serverTool.Handler(deps)
request := createMCPRequest(map[string]any{
"owner": "owner",
"repo": "repo",
})
result, err := toolHandler(ContextWithDeps(context.Background(), deps), &request)
require.NoError(t, err)
getErrorResult(t, result)
})
t.Run("API error is surfaced", func(t *testing.T) {
client := mustNewGHClient(t, NewMockedHTTPClient(
WithRequestMatchHandler(endpointSemanticallySimilar, http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
w.WriteHeader(http.StatusNotFound)
_, _ = w.Write([]byte(`{"message": "Not Found"}`))
})),
))
deps := BaseDeps{Client: client}
toolHandler := serverTool.Handler(deps)
request := createMCPRequest(map[string]any{
"owner": "owner",
"repo": "repo",
"issue_number": float64(123),
})
result, err := toolHandler(ContextWithDeps(context.Background(), deps), &request)
require.NoError(t, err)
getErrorResult(t, result)
})
}
func Test_FindDuplicate_IFCLabels(t *testing.T) {
serverTool := FindDuplicate(translations.NullTranslationHelper)
rankedResults := []map[string]any{
{
"issue": map[string]any{
"number": 585,
"title": "Improve the onboarding flow for new users",
"state": "open",
"html_url": "https://github.com/owner/repo/issues/585",
},
"score": 1.93,
"confidence": "high",
"likely_duplicate": true,
},
}
// makeClient serves the semantic-similarity endpoint plus the repo lookup
// that the IFC labeler uses to resolve visibility.
makeClient := func(isPrivate bool, repoStatus int) *http.Client {
handlers := map[string]http.HandlerFunc{
string(endpointSemanticallySimilar): mockResponse(t, http.StatusOK, rankedResults),
}
if repoStatus != 0 && repoStatus != http.StatusOK {
handlers[GetReposByOwnerByRepo] = mockResponse(t, repoStatus, "boom")
} else {
handlers[GetReposByOwnerByRepo] = mockResponse(t, http.StatusOK, map[string]any{
"name": "repo",
"private": isPrivate,
})
}
return MockHTTPClientWithHandlers(handlers)
}
req := map[string]any{
"owner": "owner",
"repo": "repo",
"issue_number": float64(769),
}
t.Run("flag disabled omits ifc label", func(t *testing.T) {
deps := BaseDeps{Client: mustNewGHClient(t, makeClient(false, 0))}
handler := serverTool.Handler(deps)
request := createMCPRequest(req)
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
require.NoError(t, err)
require.False(t, result.IsError)
assert.Nil(t, result.Meta)
})
t.Run("flag enabled on public repo emits public untrusted", func(t *testing.T) {
deps := BaseDeps{
Client: mustNewGHClient(t, makeClient(false, 0)),
featureChecker: featureCheckerFor(FeatureFlagIFCLabels),
}
handler := serverTool.Handler(deps)
request := createMCPRequest(req)
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
require.NoError(t, err)
require.False(t, result.IsError)
require.NotNil(t, result.Meta)
ifcMap := unmarshalIFC(t, result.Meta["ifc"])
assert.Equal(t, "untrusted", ifcMap["integrity"])
assert.Equal(t, "public", ifcMap["confidentiality"])
})
t.Run("flag enabled on private repo emits private trusted", func(t *testing.T) {
deps := BaseDeps{
Client: mustNewGHClient(t, makeClient(true, 0)),
featureChecker: featureCheckerFor(FeatureFlagIFCLabels),
}
handler := serverTool.Handler(deps)
request := createMCPRequest(req)
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
require.NoError(t, err)
require.False(t, result.IsError)
require.NotNil(t, result.Meta)
ifcMap := unmarshalIFC(t, result.Meta["ifc"])
assert.Equal(t, "trusted", ifcMap["integrity"])
assert.Equal(t, "private", ifcMap["confidentiality"])
})
t.Run("visibility lookup failure omits label but still succeeds", func(t *testing.T) {
deps := BaseDeps{
Client: mustNewGHClient(t, makeClient(false, http.StatusInternalServerError)),
featureChecker: featureCheckerFor(FeatureFlagIFCLabels),
}
handler := serverTool.Handler(deps)
request := createMCPRequest(req)
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
require.NoError(t, err)
require.False(t, result.IsError, "tool call should still succeed when visibility lookup fails")
if result.Meta != nil {
_, hasIFC := result.Meta["ifc"]
assert.False(t, hasIFC, "label must be omitted on visibility lookup failure")
}
})
}