centralise sanitisation for inputs and outputs
CodeQL / Analyze (go) (push) Has been cancelled
CodeQL / Analyze (actions) (push) Has been cancelled
CodeQL / Analyze (javascript) (push) Has been cancelled
Build and Test Go Project / build (macos-latest) (push) Has been cancelled
Build and Test Go Project / build (ubuntu-latest) (push) Has been cancelled
Build and Test Go Project / build (windows-latest) (push) Has been cancelled

This commit is contained in:
RossTarrant
2026-07-27 09:55:57 +01:00
parent eb088dfe9d
commit 35cca24ea2
32 changed files with 2609 additions and 417 deletions
@@ -32,6 +32,9 @@
},
"sort": {
"description": "Sort field ('indexed' only)",
"enum": [
"indexed"
],
"type": "string"
}
},
@@ -46,6 +46,9 @@
},
"sort": {
"description": "Sort field ('indexed' only)",
"enum": [
"indexed"
],
"type": "string"
}
},
+8 -8
View File
@@ -84,7 +84,7 @@ func handleFailedJobLogs(ctx context.Context, client *github.Client, owner, repo
// Continue with other jobs even if one fails
jobResult = map[string]any{
"job_id": job.GetID(),
"job_name": job.GetName(),
"job_name": sanitizeOutputText(job.GetName()),
"error": err.Error(),
}
// Enable reporting of status codes and error causes
@@ -139,7 +139,7 @@ func getJobLogData(ctx context.Context, client *github.Client, owner, repo strin
"job_id": jobID,
}
if jobName != "" {
result["job_name"] = jobName
result["job_name"] = sanitizeOutputText(jobName)
}
if returnContent {
@@ -788,7 +788,7 @@ func getWorkflow(ctx context.Context, client *github.Client, owner, repo, resour
}
defer func() { _ = resp.Body.Close() }()
r, err := json.Marshal(workflow)
r, err := json.Marshal(sanitizedWorkflowCopy(workflow))
if err != nil {
return nil, nil, fmt.Errorf("failed to marshal workflow: %w", err)
}
@@ -802,7 +802,7 @@ func getWorkflowRun(ctx context.Context, client *github.Client, owner, repo stri
return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to get workflow run", resp, err), nil, nil
}
defer func() { _ = resp.Body.Close() }()
r, err := json.Marshal(workflowRun)
r, err := json.Marshal(sanitizedWorkflowRunCopy(workflowRun))
if err != nil {
return nil, nil, fmt.Errorf("failed to marshal workflow run: %w", err)
}
@@ -815,7 +815,7 @@ func getWorkflowJob(ctx context.Context, client *github.Client, owner, repo stri
return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to get workflow job", resp, err), nil, nil
}
defer func() { _ = resp.Body.Close() }()
r, err := json.Marshal(workflowJob)
r, err := json.Marshal(sanitizedWorkflowJobCopy(workflowJob))
if err != nil {
return nil, nil, fmt.Errorf("failed to marshal workflow job: %w", err)
}
@@ -834,7 +834,7 @@ func listWorkflows(ctx context.Context, client *github.Client, owner, repo strin
}
defer func() { _ = resp.Body.Close() }()
r, err := json.Marshal(workflows)
r, err := json.Marshal(sanitizedWorkflowsCopy(workflows))
if err != nil {
return nil, nil, fmt.Errorf("failed to marshal workflows: %w", err)
}
@@ -884,7 +884,7 @@ func listWorkflowRuns(ctx context.Context, client *github.Client, args map[strin
}
defer func() { _ = resp.Body.Close() }()
r, err := json.Marshal(workflowRuns)
r, err := json.Marshal(sanitizedWorkflowRunsCopy(workflowRuns))
if err != nil {
return nil, nil, fmt.Errorf("failed to marshal workflow runs: %w", err)
}
@@ -919,7 +919,7 @@ func listWorkflowJobs(ctx context.Context, client *github.Client, args map[strin
}
response := map[string]any{
"jobs": workflowJobs,
"jobs": sanitizedWorkflowJobsCopy(workflowJobs),
}
defer func() { _ = resp.Body.Close() }()
+109 -16
View File
@@ -114,22 +114,66 @@ func Test_ActionsList_ListWorkflows(t *testing.T) {
}
}
func unsafeWorkflowRunFixture() *github.WorkflowRun {
return &github.WorkflowRun{
ID: github.Ptr(int64(12345)),
Name: github.Ptr(baselineUnsafeText),
DisplayTitle: github.Ptr(baselineUnsafeText),
HeadBranch: github.Ptr("feature/exact<script>"),
HeadSHA: github.Ptr("abc123"),
HeadCommit: &github.HeadCommit{
Message: github.Ptr(baselineUnsafeText),
Author: &github.CommitAuthor{
Name: github.Ptr(baselineUnsafeText),
Email: github.Ptr("author@example.com"),
},
Added: []string{"src/exact<script>.go"},
},
PullRequests: []*github.PullRequest{{
Title: github.Ptr(baselineUnsafeText),
Body: github.Ptr(baselineUnsafeText),
Labels: []*github.Label{{Name: github.Ptr(baselineUnsafeText), Description: github.Ptr(baselineUnsafeText)}},
Milestone: &github.Milestone{Title: github.Ptr(baselineUnsafeText), Description: github.Ptr(baselineUnsafeText)},
Head: &github.PullRequestBranch{Repo: &github.Repository{Description: github.Ptr(baselineUnsafeText)}},
Base: &github.PullRequestBranch{Repo: &github.Repository{Description: github.Ptr(baselineUnsafeText)}},
}},
Repository: &github.Repository{Description: github.Ptr(baselineUnsafeText)},
HeadRepository: &github.Repository{Description: github.Ptr(baselineUnsafeText)},
Status: github.Ptr("completed"),
Conclusion: github.Ptr("success"),
}
}
func assertSanitizedWorkflowRun(t *testing.T, run *github.WorkflowRun) {
t.Helper()
expected := sanitizeOutputText(baselineUnsafeText)
assert.Equal(t, expected, run.GetName())
assert.Equal(t, expected, run.GetDisplayTitle())
assert.Equal(t, expected, run.HeadCommit.GetMessage())
assert.Equal(t, expected, run.HeadCommit.Author.GetName())
assert.Equal(t, "author@example.com", run.HeadCommit.Author.GetEmail())
assert.Equal(t, []string{"src/exact<script>.go"}, run.HeadCommit.Added)
assert.Equal(t, expected, run.PullRequests[0].GetTitle())
assert.Equal(t, expected, run.PullRequests[0].Labels[0].GetName())
assert.Equal(t, expected, run.PullRequests[0].Milestone.GetTitle())
assert.Equal(t, expected, run.PullRequests[0].Head.Repo.GetDescription())
assert.Equal(t, expected, run.PullRequests[0].Base.Repo.GetDescription())
assert.Equal(t, expected, run.Repository.GetDescription())
assert.Equal(t, expected, run.HeadRepository.GetDescription())
assert.Equal(t, "feature/exact<script>", run.GetHeadBranch())
assert.Equal(t, "abc123", run.GetHeadSHA())
}
func Test_ActionsList_ListWorkflowRuns(t *testing.T) {
toolDef := ActionsList(translations.NullTranslationHelper)
t.Run("successful workflow runs list", func(t *testing.T) {
workflowRun := unsafeWorkflowRunFixture()
mockedClient := MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
GetReposActionsWorkflowsRunsByOwnerByRepoByWorkflowID: http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
runs := &github.WorkflowRuns{
TotalCount: github.Ptr(1),
WorkflowRuns: []*github.WorkflowRun{
{
ID: github.Ptr(int64(123)),
Name: github.Ptr("CI"),
Status: github.Ptr("completed"),
Conclusion: github.Ptr("success"),
},
},
TotalCount: github.Ptr(1),
WorkflowRuns: []*github.WorkflowRun{workflowRun},
}
w.WriteHeader(http.StatusOK)
_ = json.NewEncoder(w).Encode(runs)
@@ -158,6 +202,9 @@ func Test_ActionsList_ListWorkflowRuns(t *testing.T) {
err = json.Unmarshal([]byte(textContent.Text), &response)
require.NoError(t, err)
assert.NotNil(t, response.TotalCount)
require.Len(t, response.WorkflowRuns, 1)
assertSanitizedWorkflowRun(t, response.WorkflowRuns[0])
assert.Equal(t, baselineUnsafeText, workflowRun.GetName())
})
t.Run("list all workflow runs without resource_id", func(t *testing.T) {
@@ -209,6 +256,55 @@ func Test_ActionsList_ListWorkflowRuns(t *testing.T) {
})
}
func Test_ActionsList_ListWorkflowJobs(t *testing.T) {
toolDef := ActionsList(translations.NullTranslationHelper)
workflowJob := &github.WorkflowJob{
ID: github.Ptr(int64(123)),
Name: github.Ptr(baselineUnsafeText),
WorkflowName: github.Ptr(baselineUnsafeText),
RunnerName: github.Ptr(baselineUnsafeText),
RunnerGroupName: github.Ptr(baselineUnsafeText),
Steps: []*github.TaskStep{{Name: github.Ptr(baselineUnsafeText)}},
}
mockedClient := MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
GetReposActionsRunsJobsByOwnerByRepoByRunID: http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
w.WriteHeader(http.StatusOK)
_ = json.NewEncoder(w).Encode(&github.Jobs{
TotalCount: github.Ptr(1),
Jobs: []*github.WorkflowJob{workflowJob},
})
}),
})
client := mustNewGHClient(t, mockedClient)
deps := BaseDeps{Client: client}
handler := toolDef.Handler(deps)
request := createMCPRequest(map[string]any{
"method": "list_workflow_jobs",
"owner": "owner",
"repo": "repo",
"resource_id": "123",
})
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
require.NoError(t, err)
require.False(t, result.IsError)
var response struct {
Jobs *github.Jobs `json:"jobs"`
}
require.NoError(t, json.Unmarshal([]byte(getTextResult(t, result).Text), &response))
require.NotNil(t, response.Jobs)
require.Len(t, response.Jobs.Jobs, 1)
returnedJob := response.Jobs.Jobs[0]
assert.Equal(t, sanitizeOutputText(baselineUnsafeText), returnedJob.GetName())
assert.Equal(t, sanitizeOutputText(baselineUnsafeText), returnedJob.GetWorkflowName())
assert.Equal(t, baselineUnsafeText, returnedJob.GetRunnerName())
assert.Equal(t, baselineUnsafeText, returnedJob.GetRunnerGroupName())
assert.Equal(t, sanitizeOutputText(baselineUnsafeText), returnedJob.Steps[0].GetName())
assert.Equal(t, baselineUnsafeText, workflowJob.GetName())
assert.Equal(t, baselineUnsafeText, workflowJob.GetRunnerName())
}
func Test_ActionsGet(t *testing.T) {
// Verify tool definition once
toolDef := ActionsGet(translations.NullTranslationHelper)
@@ -271,16 +367,11 @@ func Test_ActionsGet_GetWorkflowRun(t *testing.T) {
toolDef := ActionsGet(translations.NullTranslationHelper)
t.Run("successful workflow run get", func(t *testing.T) {
workflowRun := unsafeWorkflowRunFixture()
mockedClient := MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
GetReposActionsRunsByOwnerByRepoByRunID: http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
run := &github.WorkflowRun{
ID: github.Ptr(int64(12345)),
Name: github.Ptr("CI"),
Status: github.Ptr("completed"),
Conclusion: github.Ptr("success"),
}
w.WriteHeader(http.StatusOK)
_ = json.NewEncoder(w).Encode(run)
_ = json.NewEncoder(w).Encode(workflowRun)
}),
})
@@ -307,6 +398,8 @@ func Test_ActionsGet_GetWorkflowRun(t *testing.T) {
require.NoError(t, err)
assert.NotNil(t, response.ID)
assert.Equal(t, int64(12345), *response.ID)
assertSanitizedWorkflowRun(t, &response)
assert.Equal(t, baselineUnsafeText, workflowRun.GetName())
})
}
+8 -2
View File
@@ -84,7 +84,7 @@ func GetCodeScanningAlert(t translations.TranslationHelperFunc) inventory.Server
return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to get alert", resp, body), nil, nil
}
r, err := json.Marshal(alert)
r, err := json.Marshal(sanitizedCodeScanningAlertCopy(alert))
if err != nil {
return utils.NewToolResultErrorFromErr("failed to marshal alert", err), nil, nil
}
@@ -164,10 +164,16 @@ func ListCodeScanningAlerts(t translations.TranslationHelperFunc) inventory.Serv
if err != nil {
return utils.NewToolResultError(err.Error()), nil, nil
}
if err := validateEnumParam("state", state, "open", "closed", "dismissed", "fixed"); err != nil {
return utils.NewToolResultError(err.Error()), nil, nil
}
severity, err := OptionalParam[string](args, "severity")
if err != nil {
return utils.NewToolResultError(err.Error()), nil, nil
}
if err := validateEnumParam("severity", severity, "critical", "high", "medium", "low", "warning", "note", "error"); err != nil {
return utils.NewToolResultError(err.Error()), nil, nil
}
toolName, err := OptionalParam[string](args, "tool_name")
if err != nil {
return utils.NewToolResultError(err.Error()), nil, nil
@@ -209,7 +215,7 @@ func ListCodeScanningAlerts(t translations.TranslationHelperFunc) inventory.Serv
return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to list alerts", resp, body), nil, nil
}
r, err := json.Marshal(alerts)
r, err := json.Marshal(sanitizedCodeScanningAlertsCopy(alerts))
if err != nil {
return utils.NewToolResultErrorFromErr("failed to marshal alerts", err), nil, nil
}
+22
View File
@@ -209,6 +209,28 @@ func Test_ListCodeScanningAlerts(t *testing.T) {
expectError: false,
expectedAlerts: mockAlerts,
},
{
name: "invalid state is rejected before request",
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{}),
requestArgs: map[string]any{
"owner": "owner",
"repo": "repo",
"state": "archived",
},
expectError: true,
expectedErrMsg: "state must be one of: open, closed, dismissed, fixed",
},
{
name: "invalid severity is rejected before request",
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{}),
requestArgs: map[string]any{
"owner": "owner",
"repo": "repo",
"severity": "extreme",
},
expectError: true,
expectedErrMsg: "severity must be one of: critical, high, medium, low, warning, note, error",
},
{
name: "alerts listing fails",
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
+8 -2
View File
@@ -85,7 +85,7 @@ func GetDependabotAlert(t translations.TranslationHelperFunc) inventory.ServerTo
return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to get alert", resp, body), nil, nil
}
r, err := json.Marshal(alert)
r, err := json.Marshal(sanitizedDependabotAlertCopy(alert))
if err != nil {
return utils.NewToolResultErrorFromErr("failed to marshal alert", err), nil, err
}
@@ -153,10 +153,16 @@ func ListDependabotAlerts(t translations.TranslationHelperFunc) inventory.Server
if err != nil {
return utils.NewToolResultError(err.Error()), nil, nil
}
if err := validateEnumParam("state", state, "open", "fixed", "dismissed", "auto_dismissed"); err != nil {
return utils.NewToolResultError(err.Error()), nil, nil
}
severity, err := OptionalParam[string](args, "severity")
if err != nil {
return utils.NewToolResultError(err.Error()), nil, nil
}
if err := validateEnumParam("severity", severity, "low", "medium", "high", "critical"); err != nil {
return utils.NewToolResultError(err.Error()), nil, nil
}
pagination, err := OptionalCursorPaginationParams(args)
if err != nil {
@@ -194,7 +200,7 @@ func ListDependabotAlerts(t translations.TranslationHelperFunc) inventory.Server
}
response := map[string]any{
"alerts": alerts,
"alerts": sanitizedDependabotAlertsCopy(alerts),
"pageInfo": buildPageInfo(resp),
}
+22
View File
@@ -256,6 +256,28 @@ func Test_ListDependabotAlerts(t *testing.T) {
expectedAlerts: []*github.DependabotAlert{&criticalAlert},
expectedNextCursor: "nextcursor123",
},
{
name: "invalid state is rejected before request",
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{}),
requestArgs: map[string]any{
"owner": "owner",
"repo": "repo",
"state": "archived",
},
expectError: true,
expectedErrMsg: "state must be one of: open, fixed, dismissed, auto_dismissed",
},
{
name: "invalid severity is rejected before request",
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{}),
requestArgs: map[string]any{
"owner": "owner",
"repo": "repo",
"severity": "extreme",
},
expectError: true,
expectedErrMsg: "severity must be one of: low, medium, high, critical",
},
{
name: "alerts listing fails",
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
+52 -55
View File
@@ -65,6 +65,20 @@ type NodeFragment struct {
URL githubv4.String `graphql:"url"`
}
type discussionDetailFragment struct {
Number githubv4.Int
Title githubv4.String
Body githubv4.String
CreatedAt githubv4.DateTime
Closed githubv4.Boolean
IsAnswered githubv4.Boolean
AnswerChosenAt *githubv4.DateTime
URL githubv4.String `graphql:"url"`
Category struct {
Name githubv4.String
} `graphql:"category"`
}
type PageInfoFragment struct {
HasNextPage bool
HasPreviousPage bool
@@ -99,7 +113,7 @@ type WithCategoryNoOrder struct {
func fragmentToDiscussion(fragment NodeFragment) *github.Discussion {
return &github.Discussion{
Number: github.Ptr(int(fragment.Number)),
Title: github.Ptr(string(fragment.Title)),
Title: github.Ptr(sanitizeOutputText(string(fragment.Title))),
HTMLURL: github.Ptr(string(fragment.URL)),
CreatedAt: &github.Timestamp{Time: fragment.CreatedAt.Time},
UpdatedAt: &github.Timestamp{Time: fragment.UpdatedAt.Time},
@@ -107,11 +121,40 @@ func fragmentToDiscussion(fragment NodeFragment) *github.Discussion {
Login: github.Ptr(string(fragment.Author.Login)),
},
DiscussionCategory: &github.DiscussionCategory{
Name: github.Ptr(string(fragment.Category.Name)),
Name: github.Ptr(sanitizeOutputText(string(fragment.Category.Name))),
},
}
}
func discussionDetailResponse(d discussionDetailFragment) map[string]any {
response := map[string]any{
"number": int(d.Number),
"title": sanitizeOutputText(string(d.Title)),
"body": sanitizeOutputText(string(d.Body)),
"url": string(d.URL),
"closed": bool(d.Closed),
"isAnswered": bool(d.IsAnswered),
"createdAt": d.CreatedAt.Time,
"category": map[string]any{
"name": sanitizeOutputText(string(d.Category.Name)),
},
}
if d.AnswerChosenAt != nil {
response["answerChosenAt"] = d.AnswerChosenAt.Time
}
return response
}
func convertToMinimalDiscussionComment(id githubv4.ID, body githubv4.String, isAnswer githubv4.Boolean) MinimalDiscussionComment {
return MinimalDiscussionComment{
ID: fmt.Sprintf("%v", id),
Body: sanitizeOutputText(string(body)),
IsAnswer: bool(isAnswer),
}
}
func getQueryType(useOrdering bool, categoryID *githubv4.ID) any {
if categoryID != nil && useOrdering {
return &WithCategoryAndOrder{}
@@ -329,19 +372,7 @@ func GetDiscussion(t translations.TranslationHelperFunc) inventory.ServerTool {
var q struct {
Repository struct {
Discussion struct {
Number githubv4.Int
Title githubv4.String
Body githubv4.String
CreatedAt githubv4.DateTime
Closed githubv4.Boolean
IsAnswered githubv4.Boolean
AnswerChosenAt *githubv4.DateTime
URL githubv4.String `graphql:"url"`
Category struct {
Name githubv4.String
} `graphql:"category"`
} `graphql:"discussion(number: $discussionNumber)"`
Discussion discussionDetailFragment `graphql:"discussion(number: $discussionNumber)"`
} `graphql:"repository(owner: $owner, name: $repo)"`
}
vars := map[string]any{
@@ -352,29 +383,7 @@ func GetDiscussion(t translations.TranslationHelperFunc) inventory.ServerTool {
if err := client.Query(ctx, &q, vars); err != nil {
return utils.NewToolResultError(err.Error()), nil, nil
}
d := q.Repository.Discussion
// Build response as map to include fields not present in go-github's Discussion struct.
// The go-github library's Discussion type lacks isAnswered and answerChosenAt fields,
// so we use map[string]interface{} for the response (consistent with other functions
// like ListDiscussions and GetDiscussionComments).
response := map[string]any{
"number": int(d.Number),
"title": string(d.Title),
"body": string(d.Body),
"url": string(d.URL),
"closed": bool(d.Closed),
"isAnswered": bool(d.IsAnswered),
"createdAt": d.CreatedAt.Time,
"category": map[string]any{
"name": string(d.Category.Name),
},
}
// Add optional timestamp fields if present
if d.AnswerChosenAt != nil {
response["answerChosenAt"] = d.AnswerChosenAt.Time
}
response := discussionDetailResponse(q.Repository.Discussion)
out, err := json.Marshal(response)
if err != nil {
@@ -520,18 +529,10 @@ func GetDiscussionComments(t translations.TranslationHelperFunc) inventory.Serve
return utils.NewToolResultError(err.Error()), nil, nil
}
for _, c := range q.Repository.Discussion.Comments.Nodes {
comment := MinimalDiscussionComment{
ID: fmt.Sprintf("%v", c.ID),
Body: string(c.Body),
IsAnswer: bool(c.IsAnswer),
ReplyTotalCount: c.Replies.TotalCount,
}
comment := convertToMinimalDiscussionComment(c.ID, c.Body, c.IsAnswer)
comment.ReplyTotalCount = c.Replies.TotalCount
for _, r := range c.Replies.Nodes {
comment.Replies = append(comment.Replies, MinimalDiscussionComment{
ID: fmt.Sprintf("%v", r.ID),
Body: string(r.Body),
IsAnswer: bool(r.IsAnswer),
})
comment.Replies = append(comment.Replies, convertToMinimalDiscussionComment(r.ID, r.Body, r.IsAnswer))
}
comments = append(comments, comment)
}
@@ -562,11 +563,7 @@ func GetDiscussionComments(t translations.TranslationHelperFunc) inventory.Serve
return utils.NewToolResultError(err.Error()), nil, nil
}
for _, c := range q.Repository.Discussion.Comments.Nodes {
comments = append(comments, MinimalDiscussionComment{
ID: fmt.Sprintf("%v", c.ID),
Body: string(c.Body),
IsAnswer: bool(c.IsAnswer),
})
comments = append(comments, convertToMinimalDiscussionComment(c.ID, c.Body, c.IsAnswer))
}
pageInfo = q.Repository.Discussion.Comments.PageInfo
totalCount = q.Repository.Discussion.Comments.TotalCount
@@ -1077,7 +1074,7 @@ func ListDiscussionCategories(t translations.TranslationHelperFunc) inventory.Se
for _, c := range q.Repository.DiscussionCategories.Nodes {
categories = append(categories, map[string]string{
"id": fmt.Sprint(c.ID),
"name": string(c.Name),
"name": sanitizeOutputText(string(c.Name)),
})
}
+2 -2
View File
@@ -820,7 +820,7 @@ func Test_ListDiscussionCategories(t *testing.T) {
"repository": map[string]any{
"discussionCategories": map[string]any{
"nodes": []map[string]any{
{"id": "123", "name": "CategoryOne"},
{"id": "123", "name": baselineUnsafeText},
{"id": "456", "name": "CategoryTwo"},
},
"pageInfo": map[string]any{
@@ -873,7 +873,7 @@ func Test_ListDiscussionCategories(t *testing.T) {
expectError: false,
expectedCount: 2,
expectedCategories: []map[string]string{
{"id": "123", "name": "CategoryOne"},
{"id": "123", "name": sanitizeOutputText(baselineUnsafeText)},
{"id": "456", "name": "CategoryTwo"},
},
},
+56
View File
@@ -0,0 +1,56 @@
package github
import (
"fmt"
"slices"
"strings"
)
func validateEnumParam(name, value string, allowed ...string) error {
if value == "" {
return nil
}
if slices.Contains(allowed, value) {
return nil
}
return fmt.Errorf("%s must be one of: %s", name, strings.Join(allowed, ", "))
}
func validateRepoRelativePath(name, path string) error {
if path == "" {
return fmt.Errorf("%s must not be empty", name)
}
if strings.HasPrefix(path, "/") {
return fmt.Errorf("%s must be relative to the repository root (no leading '/')", name)
}
if slices.Contains(strings.Split(path, "/"), "..") {
return fmt.Errorf("%s must not contain '..' segments", name)
}
for _, r := range path {
if r < 0x20 || r == 0x7f {
return fmt.Errorf("%s must not contain control characters", name)
}
}
return nil
}
func validateOptionalRepoRelativePath(name, path string) error {
if path == "" {
return nil
}
return validateRepoRelativePath(name, path)
}
func validateRepoRelativePathOrRoot(name, path string) error {
if path == "" || path == "/" {
return nil
}
return validateRepoRelativePath(name, path)
}
func normalizeRepoRelativePathOrRoot(path string) string {
if path == "/" {
return ""
}
return path
}
+9 -19
View File
@@ -739,16 +739,6 @@ func GetIssue(ctx context.Context, client *github.Client, deps ToolDependencies,
}
}
// Sanitize title/body on response
if issue != nil {
if issue.Title != nil {
issue.Title = github.Ptr(sanitize.Sanitize(*issue.Title))
}
if issue.Body != nil {
issue.Body = github.Ptr(sanitize.Sanitize(*issue.Body))
}
}
minimalIssue := convertToMinimalIssue(issue)
// Always drop the verbose REST IssueFieldValues; enrich with the GraphQL
@@ -931,7 +921,7 @@ func GetSubIssues(ctx context.Context, client *github.Client, deps ToolDependenc
subIssues = filteredSubIssues
}
r, err := json.Marshal(subIssues)
r, err := json.Marshal(sanitizedSubIssuesCopy(subIssues))
if err != nil {
return nil, fmt.Errorf("failed to marshal response: %w", err)
}
@@ -1048,9 +1038,9 @@ func GetIssueLabels(ctx context.Context, client *githubv4.Client, owner string,
for i, label := range query.Repository.Issue.Labels.Nodes {
issueLabels[i] = map[string]any{
"id": fmt.Sprintf("%v", label.ID),
"name": string(label.Name),
"name": sanitizeOutputText(string(label.Name)),
"color": string(label.Color),
"description": string(label.Description),
"description": sanitizeOutputText(string(label.Description)),
}
}
@@ -1131,7 +1121,7 @@ func ListIssueTypes(t translations.TranslationHelperFunc) inventory.ServerTool {
return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to list issue types", resp, body), nil, nil
}
r, err := json.Marshal(issueTypes)
r, err := json.Marshal(sanitizedIssueTypesCopy(issueTypes))
if err != nil {
return utils.NewToolResultErrorFromErr("failed to marshal issue types", err), nil, nil
}
@@ -1155,7 +1145,7 @@ func ListIssueTypes(t translations.TranslationHelperFunc) inventory.ServerTool {
return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to list issue types", resp, body), nil, nil
}
r, err := json.Marshal(issueTypes)
r, err := json.Marshal(sanitizedIssueTypesCopy(issueTypes))
if err != nil {
return utils.NewToolResultErrorFromErr("failed to marshal issue types", err), nil, nil
}
@@ -1507,7 +1497,7 @@ func AddSubIssue(ctx context.Context, client *github.Client, owner string, repo
return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to add sub-issue", resp, body), nil
}
r, err := json.Marshal(subIssue)
r, err := json.Marshal(sanitizedSubIssueCopy(subIssue))
if err != nil {
return nil, fmt.Errorf("failed to marshal response: %w", err)
}
@@ -1538,7 +1528,7 @@ func RemoveSubIssue(ctx context.Context, client *github.Client, owner string, re
return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to remove sub-issue", resp, body), nil
}
r, err := json.Marshal(subIssue)
r, err := json.Marshal(sanitizedSubIssueCopy(subIssue))
if err != nil {
return nil, fmt.Errorf("failed to marshal response: %w", err)
}
@@ -1587,7 +1577,7 @@ func ReprioritizeSubIssue(ctx context.Context, client *github.Client, owner stri
return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to reprioritize sub-issue", resp, body), nil
}
r, err := json.Marshal(subIssue)
r, err := json.Marshal(sanitizedSubIssueCopy(subIssue))
if err != nil {
return nil, fmt.Errorf("failed to marshal response: %w", err)
}
@@ -1788,7 +1778,7 @@ type SearchIssueResult struct {
// MarshalJSON serializes SearchIssueResult, suppressing the raw issue_field_values from the
// embedded REST response in favour of the normalized field_values populated via GraphQL enrichment.
func (r SearchIssueResult) MarshalJSON() ([]byte, error) {
issueBytes, err := json.Marshal(r.Issue)
issueBytes, err := json.Marshal(sanitizedIssueCopy(r.Issue))
if err != nil {
return nil, err
}
+52 -30
View File
@@ -3730,9 +3730,9 @@ func Test_GetIssueLabels(t *testing.T) {
"nodes": []any{
map[string]any{
"id": githubv4.ID("label-1"),
"name": githubv4.String("bug"),
"name": githubv4.String(baselineUnsafeText),
"color": githubv4.String("d73a4a"),
"description": githubv4.String("Something isn't working"),
"description": githubv4.String(baselineUnsafeText),
},
},
"totalCount": githubv4.Int(1),
@@ -3772,6 +3772,16 @@ func Test_GetIssueLabels(t *testing.T) {
}
} else {
assert.False(t, result.IsError)
var response struct {
Labels []struct {
Name string `json:"name"`
Description string `json:"description"`
} `json:"labels"`
}
require.NoError(t, json.Unmarshal([]byte(getTextResult(t, result).Text), &response))
require.Len(t, response.Labels, 1)
assert.Equal(t, sanitizeOutputText(baselineUnsafeText), response.Labels[0].Name)
assert.Equal(t, sanitizeOutputText(baselineUnsafeText), response.Labels[0].Description)
}
})
}
@@ -3961,8 +3971,8 @@ func Test_AddSubIssue(t *testing.T) {
// Setup mock issue for success case (matches GitHub API response format)
mockIssue := &github.Issue{
Number: github.Ptr(42),
Title: github.Ptr("Parent Issue"),
Body: github.Ptr("This is the parent issue with a sub-issue"),
Title: github.Ptr(baselineUnsafeText),
Body: github.Ptr(baselineUnsafeText),
State: github.Ptr("open"),
HTMLURL: github.Ptr("https://github.com/owner/repo/issues/42"),
User: &github.User{
@@ -3970,9 +3980,9 @@ func Test_AddSubIssue(t *testing.T) {
},
Labels: []*github.Label{
{
Name: github.Ptr("enhancement"),
Name: github.Ptr(baselineUnsafeText),
Color: github.Ptr("84b6eb"),
Description: github.Ptr("New feature or request"),
Description: github.Ptr(baselineUnsafeText),
},
},
}
@@ -4157,11 +4167,13 @@ func Test_AddSubIssue(t *testing.T) {
err = json.Unmarshal([]byte(textContent.Text), &returnedIssue)
require.NoError(t, err)
assert.Equal(t, *tc.expectedIssue.Number, *returnedIssue.Number)
assert.Equal(t, *tc.expectedIssue.Title, *returnedIssue.Title)
assert.Equal(t, *tc.expectedIssue.Body, *returnedIssue.Body)
assert.Equal(t, sanitizeOutputText(tc.expectedIssue.GetTitle()), returnedIssue.GetTitle())
assert.Equal(t, sanitizeOutputText(tc.expectedIssue.GetBody()), returnedIssue.GetBody())
assert.Equal(t, *tc.expectedIssue.State, *returnedIssue.State)
assert.Equal(t, *tc.expectedIssue.HTMLURL, *returnedIssue.HTMLURL)
assert.Equal(t, *tc.expectedIssue.User.Login, *returnedIssue.User.Login)
assert.Equal(t, sanitizeOutputText(tc.expectedIssue.Labels[0].GetName()), returnedIssue.Labels[0].GetName())
assert.Equal(t, baselineUnsafeText, mockIssue.GetTitle())
})
}
}
@@ -4186,8 +4198,8 @@ func Test_GetSubIssues(t *testing.T) {
mockSubIssues := []*github.Issue{
{
Number: github.Ptr(123),
Title: github.Ptr("Sub-issue 1"),
Body: github.Ptr("This is the first sub-issue"),
Title: github.Ptr(baselineUnsafeText),
Body: github.Ptr(baselineUnsafeText),
State: github.Ptr("open"),
HTMLURL: github.Ptr("https://github.com/owner/repo/issues/123"),
User: &github.User{
@@ -4195,9 +4207,9 @@ func Test_GetSubIssues(t *testing.T) {
},
Labels: []*github.Label{
{
Name: github.Ptr("bug"),
Name: github.Ptr(baselineUnsafeText),
Color: github.Ptr("d73a4a"),
Description: github.Ptr("Something isn't working"),
Description: github.Ptr(baselineUnsafeText),
},
},
},
@@ -4386,17 +4398,22 @@ func Test_GetSubIssues(t *testing.T) {
for i, subIssue := range returnedSubIssues {
if i < len(tc.expectedSubIssues) {
assert.Equal(t, *tc.expectedSubIssues[i].Number, *subIssue.Number)
assert.Equal(t, *tc.expectedSubIssues[i].Title, *subIssue.Title)
assert.Equal(t, sanitizeOutputText(tc.expectedSubIssues[i].GetTitle()), subIssue.GetTitle())
assert.Equal(t, *tc.expectedSubIssues[i].State, *subIssue.State)
assert.Equal(t, *tc.expectedSubIssues[i].HTMLURL, *subIssue.HTMLURL)
assert.Equal(t, *tc.expectedSubIssues[i].User.Login, *subIssue.User.Login)
if tc.expectedSubIssues[i].Body != nil {
assert.Equal(t, *tc.expectedSubIssues[i].Body, *subIssue.Body)
assert.Equal(t, sanitizeOutputText(tc.expectedSubIssues[i].GetBody()), subIssue.GetBody())
}
if len(tc.expectedSubIssues[i].Labels) > 0 {
assert.Equal(t, sanitizeOutputText(tc.expectedSubIssues[i].Labels[0].GetName()), subIssue.Labels[0].GetName())
assert.Equal(t, sanitizeOutputText(tc.expectedSubIssues[i].Labels[0].GetDescription()), subIssue.Labels[0].GetDescription())
}
}
}
assert.Equal(t, baselineUnsafeText, mockSubIssues[0].GetTitle())
})
}
}
@@ -4675,8 +4692,8 @@ func Test_RemoveSubIssue(t *testing.T) {
// Setup mock issue for success case (matches GitHub API response format - the updated parent issue)
mockIssue := &github.Issue{
Number: github.Ptr(42),
Title: github.Ptr("Parent Issue"),
Body: github.Ptr("This is the parent issue after sub-issue removal"),
Title: github.Ptr(baselineUnsafeText),
Body: github.Ptr(baselineUnsafeText),
State: github.Ptr("open"),
HTMLURL: github.Ptr("https://github.com/owner/repo/issues/42"),
User: &github.User{
@@ -4684,9 +4701,9 @@ func Test_RemoveSubIssue(t *testing.T) {
},
Labels: []*github.Label{
{
Name: github.Ptr("enhancement"),
Name: github.Ptr(baselineUnsafeText),
Color: github.Ptr("84b6eb"),
Description: github.Ptr("New feature or request"),
Description: github.Ptr(baselineUnsafeText),
},
},
}
@@ -4854,11 +4871,13 @@ func Test_RemoveSubIssue(t *testing.T) {
err = json.Unmarshal([]byte(textContent.Text), &returnedIssue)
require.NoError(t, err)
assert.Equal(t, *tc.expectedIssue.Number, *returnedIssue.Number)
assert.Equal(t, *tc.expectedIssue.Title, *returnedIssue.Title)
assert.Equal(t, *tc.expectedIssue.Body, *returnedIssue.Body)
assert.Equal(t, sanitizeOutputText(tc.expectedIssue.GetTitle()), returnedIssue.GetTitle())
assert.Equal(t, sanitizeOutputText(tc.expectedIssue.GetBody()), returnedIssue.GetBody())
assert.Equal(t, *tc.expectedIssue.State, *returnedIssue.State)
assert.Equal(t, *tc.expectedIssue.HTMLURL, *returnedIssue.HTMLURL)
assert.Equal(t, *tc.expectedIssue.User.Login, *returnedIssue.User.Login)
assert.Equal(t, sanitizeOutputText(tc.expectedIssue.Labels[0].GetName()), returnedIssue.Labels[0].GetName())
assert.Equal(t, baselineUnsafeText, mockIssue.GetTitle())
})
}
}
@@ -4883,8 +4902,8 @@ func Test_ReprioritizeSubIssue(t *testing.T) {
// Setup mock issue for success case (matches GitHub API response format - the updated parent issue)
mockIssue := &github.Issue{
Number: github.Ptr(42),
Title: github.Ptr("Parent Issue"),
Body: github.Ptr("This is the parent issue with reprioritized sub-issues"),
Title: github.Ptr(baselineUnsafeText),
Body: github.Ptr(baselineUnsafeText),
State: github.Ptr("open"),
HTMLURL: github.Ptr("https://github.com/owner/repo/issues/42"),
User: &github.User{
@@ -4892,9 +4911,9 @@ func Test_ReprioritizeSubIssue(t *testing.T) {
},
Labels: []*github.Label{
{
Name: github.Ptr("enhancement"),
Name: github.Ptr(baselineUnsafeText),
Color: github.Ptr("84b6eb"),
Description: github.Ptr("New feature or request"),
Description: github.Ptr(baselineUnsafeText),
},
},
}
@@ -5114,11 +5133,13 @@ func Test_ReprioritizeSubIssue(t *testing.T) {
err = json.Unmarshal([]byte(textContent.Text), &returnedIssue)
require.NoError(t, err)
assert.Equal(t, *tc.expectedIssue.Number, *returnedIssue.Number)
assert.Equal(t, *tc.expectedIssue.Title, *returnedIssue.Title)
assert.Equal(t, *tc.expectedIssue.Body, *returnedIssue.Body)
assert.Equal(t, sanitizeOutputText(tc.expectedIssue.GetTitle()), returnedIssue.GetTitle())
assert.Equal(t, sanitizeOutputText(tc.expectedIssue.GetBody()), returnedIssue.GetBody())
assert.Equal(t, *tc.expectedIssue.State, *returnedIssue.State)
assert.Equal(t, *tc.expectedIssue.HTMLURL, *returnedIssue.HTMLURL)
assert.Equal(t, *tc.expectedIssue.User.Login, *returnedIssue.User.Login)
assert.Equal(t, sanitizeOutputText(tc.expectedIssue.Labels[0].GetName()), returnedIssue.Labels[0].GetName())
assert.Equal(t, baselineUnsafeText, mockIssue.GetTitle())
})
}
}
@@ -5138,8 +5159,8 @@ func Test_ListIssueTypes(t *testing.T) {
mockIssueTypes := []*github.IssueType{
{
ID: github.Ptr(int64(1)),
Name: github.Ptr("bug"),
Description: github.Ptr("Something isn't working"),
Name: github.Ptr(baselineUnsafeText),
Description: github.Ptr(baselineUnsafeText),
Color: github.Ptr("d73a4a"),
},
{
@@ -5265,11 +5286,12 @@ func Test_ListIssueTypes(t *testing.T) {
if tc.expectedIssueTypes != nil {
require.Equal(t, len(tc.expectedIssueTypes), len(returnedIssueTypes))
for i, expected := range tc.expectedIssueTypes {
assert.Equal(t, *expected.Name, *returnedIssueTypes[i].Name)
assert.Equal(t, *expected.Description, *returnedIssueTypes[i].Description)
assert.Equal(t, sanitizeOutputText(expected.GetName()), returnedIssueTypes[i].GetName())
assert.Equal(t, sanitizeOutputText(expected.GetDescription()), returnedIssueTypes[i].GetDescription())
assert.Equal(t, *expected.Color, *returnedIssueTypes[i].Color)
assert.Equal(t, *expected.ID, *returnedIssueTypes[i].ID)
}
assert.Equal(t, baselineUnsafeText, mockIssueTypes[0].GetName())
}
})
}
+7 -7
View File
@@ -96,9 +96,9 @@ func GetLabel(t translations.TranslationHelperFunc) inventory.ServerTool {
label := map[string]any{
"id": fmt.Sprintf("%v", query.Repository.Label.ID),
"name": string(query.Repository.Label.Name),
"name": sanitizeOutputText(string(query.Repository.Label.Name)),
"color": string(query.Repository.Label.Color),
"description": string(query.Repository.Label.Description),
"description": sanitizeOutputText(string(query.Repository.Label.Description)),
}
out, err := json.Marshal(label)
@@ -193,9 +193,9 @@ func ListLabels(t translations.TranslationHelperFunc) inventory.ServerTool {
for i, labelNode := range query.Repository.Labels.Nodes {
labels[i] = map[string]any{
"id": fmt.Sprintf("%v", labelNode.ID),
"name": string(labelNode.Name),
"name": sanitizeOutputText(string(labelNode.Name)),
"color": string(labelNode.Color),
"description": string(labelNode.Description),
"description": sanitizeOutputText(string(labelNode.Description)),
}
}
@@ -336,7 +336,7 @@ func LabelWrite(t translations.TranslationHelperFunc) inventory.ServerTool {
return ghErrors.NewGitHubGraphQLErrorResponse(ctx, "Failed to create label", err), nil, nil
}
return utils.NewToolResultText(fmt.Sprintf("label '%s' created successfully", mutation.CreateLabel.Label.Name)), nil, nil
return utils.NewToolResultText(fmt.Sprintf("label '%s' created successfully", sanitizeOutputText(string(mutation.CreateLabel.Label.Name)))), nil, nil
case "update":
// Validate required params for update
@@ -379,7 +379,7 @@ func LabelWrite(t translations.TranslationHelperFunc) inventory.ServerTool {
return ghErrors.NewGitHubGraphQLErrorResponse(ctx, "Failed to update label", err), nil, nil
}
return utils.NewToolResultText(fmt.Sprintf("label '%s' updated successfully", mutation.UpdateLabel.Label.Name)), nil, nil
return utils.NewToolResultText(fmt.Sprintf("label '%s' updated successfully", sanitizeOutputText(string(mutation.UpdateLabel.Label.Name)))), nil, nil
case "delete":
// Get the label ID
@@ -402,7 +402,7 @@ func LabelWrite(t translations.TranslationHelperFunc) inventory.ServerTool {
return ghErrors.NewGitHubGraphQLErrorResponse(ctx, "Failed to delete label", err), nil, nil
}
return utils.NewToolResultText(fmt.Sprintf("label '%s' deleted successfully", name)), nil, nil
return utils.NewToolResultText(fmt.Sprintf("label '%s' deleted successfully", sanitizeOutputText(name))), nil, nil
default:
return utils.NewToolResultError(fmt.Sprintf("unknown method: %s. Supported methods are: create, update, delete", method)), nil, nil
+32 -6
View File
@@ -2,6 +2,7 @@ package github
import (
"context"
"encoding/json"
"net/http"
"testing"
@@ -60,9 +61,9 @@ func TestGetLabel(t *testing.T) {
"repository": map[string]any{
"label": map[string]any{
"id": githubv4.ID("test-label-id"),
"name": githubv4.String("bug"),
"name": githubv4.String(baselineUnsafeText),
"color": githubv4.String("d73a4a"),
"description": githubv4.String("Something isn't working"),
"description": githubv4.String(baselineUnsafeText),
},
},
}),
@@ -133,6 +134,13 @@ func TestGetLabel(t *testing.T) {
}
} else {
assert.False(t, result.IsError)
var label struct {
Name string `json:"name"`
Description string `json:"description"`
}
require.NoError(t, json.Unmarshal([]byte(getTextResult(t, result).Text), &label))
assert.Equal(t, sanitizeOutputText(baselineUnsafeText), label.Name)
assert.Equal(t, sanitizeOutputText(baselineUnsafeText), label.Description)
}
})
}
@@ -188,9 +196,9 @@ func TestListLabels(t *testing.T) {
"nodes": []any{
map[string]any{
"id": githubv4.ID("label-1"),
"name": githubv4.String("bug"),
"name": githubv4.String(baselineUnsafeText),
"color": githubv4.String("d73a4a"),
"description": githubv4.String("Something isn't working"),
"description": githubv4.String(baselineUnsafeText),
},
map[string]any{
"id": githubv4.ID("label-2"),
@@ -231,6 +239,16 @@ func TestListLabels(t *testing.T) {
}
} else {
assert.False(t, result.IsError)
var response struct {
Labels []struct {
Name string `json:"name"`
Description string `json:"description"`
} `json:"labels"`
}
require.NoError(t, json.Unmarshal([]byte(getTextResult(t, result).Text), &response))
require.NotEmpty(t, response.Labels)
assert.Equal(t, sanitizeOutputText(baselineUnsafeText), response.Labels[0].Name)
assert.Equal(t, sanitizeOutputText(baselineUnsafeText), response.Labels[0].Description)
}
})
}
@@ -256,6 +274,7 @@ func TestWriteLabel(t *testing.T) {
mockedClient *http.Client
expectToolError bool
expectedToolErrMsg string
expectedOutput string
}{
{
name: "successful label creation",
@@ -304,13 +323,14 @@ func TestWriteLabel(t *testing.T) {
"createLabel": map[string]any{
"label": map[string]any{
"id": githubv4.ID("new-label-id"),
"name": githubv4.String("new-label"),
"name": githubv4.String(baselineUnsafeText),
},
},
}),
),
),
expectToolError: false,
expectedOutput: sanitizeOutputText(baselineUnsafeText),
},
{
name: "create label without color",
@@ -377,13 +397,14 @@ func TestWriteLabel(t *testing.T) {
"updateLabel": map[string]any{
"label": map[string]any{
"id": githubv4.ID("bug-label-id"),
"name": githubv4.String("defect"),
"name": githubv4.String(baselineUnsafeText),
},
},
}),
),
),
expectToolError: false,
expectedOutput: sanitizeOutputText(baselineUnsafeText),
},
{
name: "update label without any changes",
@@ -484,6 +505,11 @@ func TestWriteLabel(t *testing.T) {
}
} else {
assert.False(t, result.IsError)
if tc.expectedOutput != "" {
textContent := getTextResult(t, result)
assert.Contains(t, textContent.Text, tc.expectedOutput)
assert.NotContains(t, textContent.Text, "<script>")
}
}
})
}
+114 -58
View File
@@ -618,11 +618,58 @@ type MinimalPullRequestReview struct {
// Helper functions
func sanitizeOutputText(s string) string {
return sanitize.Sanitize(s)
}
func sanitizeOutputValue(value any) any {
switch v := value.(type) {
case string:
return sanitizeOutputText(v)
case []string:
sanitized := make([]string, len(v))
for i, item := range v {
sanitized[i] = sanitizeOutputText(item)
}
return sanitized
default:
return v
}
}
func convertToMinimalRepository(repo *github.Repository) MinimalRepository {
minimalRepo := MinimalRepository{
ID: repo.GetID(),
Name: repo.GetName(),
FullName: repo.GetFullName(),
Description: sanitizeOutputText(repo.GetDescription()),
HTMLURL: repo.GetHTMLURL(),
Language: repo.GetLanguage(),
Stars: repo.GetStargazersCount(),
Forks: repo.GetForksCount(),
OpenIssues: repo.GetOpenIssuesCount(),
Private: repo.GetPrivate(),
Fork: repo.GetFork(),
Archived: repo.GetArchived(),
DefaultBranch: repo.GetDefaultBranch(),
}
if repo.UpdatedAt != nil {
minimalRepo.UpdatedAt = repo.UpdatedAt.Format("2006-01-02T15:04:05Z")
}
if repo.CreatedAt != nil {
minimalRepo.CreatedAt = repo.CreatedAt.Format("2006-01-02T15:04:05Z")
}
if repo.Topics != nil {
minimalRepo.Topics = repo.Topics
}
return minimalRepo
}
func convertToMinimalPullRequestReview(review *github.PullRequestReview) MinimalPullRequestReview {
m := MinimalPullRequestReview{
ID: review.GetID(),
State: review.GetState(),
Body: review.GetBody(),
Body: sanitizeOutputText(review.GetBody()),
HTMLURL: review.GetHTMLURL(),
User: convertToMinimalUser(review.GetUser()),
CommitID: review.GetCommitID(),
@@ -639,8 +686,8 @@ func convertToMinimalPullRequestReview(review *github.PullRequestReview) Minimal
func convertToMinimalIssue(issue *github.Issue) MinimalIssue {
m := MinimalIssue{
Number: issue.GetNumber(),
Title: issue.GetTitle(),
Body: issue.GetBody(),
Title: sanitizeOutputText(issue.GetTitle()),
Body: sanitizeOutputText(issue.GetBody()),
State: issue.GetState(),
StateReason: issue.GetStateReason(),
Draft: issue.GetDraft(),
@@ -663,7 +710,7 @@ func convertToMinimalIssue(issue *github.Issue) MinimalIssue {
for _, label := range issue.Labels {
if label != nil {
m.Labels = append(m.Labels, label.GetName())
m.Labels = append(m.Labels, sanitizeOutputText(label.GetName()))
}
}
@@ -678,11 +725,11 @@ func convertToMinimalIssue(issue *github.Issue) MinimalIssue {
}
if milestone := issue.GetMilestone(); milestone != nil {
m.Milestone = milestone.GetTitle()
m.Milestone = sanitizeOutputText(milestone.GetTitle())
}
if issueType := issue.GetType(); issueType != nil {
m.IssueType = issueType.GetName()
m.IssueType = sanitizeOutputText(issueType.GetName())
}
for _, fv := range issue.IssueFieldValues {
@@ -693,12 +740,12 @@ func convertToMinimalIssue(issue *github.Issue) MinimalIssue {
IssueFieldID: fv.IssueFieldID,
NodeID: fv.NodeID,
DataType: fv.DataType,
Value: fv.Value,
Value: sanitizeOutputValue(fv.Value),
}
if opt := fv.SingleSelectOption; opt != nil {
mfv.SingleSelectOption = &MinimalIssueFieldValueSingleSelectOption{
ID: opt.ID,
Name: opt.Name,
Name: sanitizeOutputText(opt.Name),
Color: opt.Color,
}
}
@@ -725,8 +772,8 @@ func convertToMinimalIssue(issue *github.Issue) MinimalIssue {
func fragmentToMinimalIssue(fragment IssueFragment) MinimalIssue {
m := MinimalIssue{
Number: int(fragment.Number),
Title: sanitize.Sanitize(string(fragment.Title)),
Body: sanitize.Sanitize(string(fragment.Body)),
Title: sanitizeOutputText(string(fragment.Title)),
Body: sanitizeOutputText(string(fragment.Body)),
State: string(fragment.State),
Comments: int(fragment.Comments.TotalCount),
CreatedAt: fragment.CreatedAt.Format(time.RFC3339),
@@ -737,7 +784,7 @@ func fragmentToMinimalIssue(fragment IssueFragment) MinimalIssue {
}
for _, label := range fragment.Labels.Nodes {
m.Labels = append(m.Labels, string(label.Name))
m.Labels = append(m.Labels, sanitizeOutputText(string(label.Name)))
}
for _, fv := range fragment.IssueFieldValues.Nodes {
@@ -755,23 +802,23 @@ func fragmentToMinimalFieldValue(fv IssueFieldValueFragment) (MinimalFieldValue,
switch fv.TypeName {
case "IssueFieldDateValue":
return MinimalFieldValue{
Field: fv.DateValue.Field.Name(),
Field: sanitizeOutputText(fv.DateValue.Field.Name()),
Value: string(fv.DateValue.Value),
}, true
case "IssueFieldNumberValue":
return MinimalFieldValue{
Field: fv.NumberValue.Field.Name(),
Field: sanitizeOutputText(fv.NumberValue.Field.Name()),
Value: strconv.FormatFloat(float64(fv.NumberValue.Value), 'f', -1, 64),
}, true
case "IssueFieldSingleSelectValue":
return MinimalFieldValue{
Field: fv.SingleSelectValue.Field.Name(),
Value: string(fv.SingleSelectValue.Value),
Field: sanitizeOutputText(fv.SingleSelectValue.Field.Name()),
Value: sanitizeOutputText(string(fv.SingleSelectValue.Value)),
}, true
case "IssueFieldTextValue":
return MinimalFieldValue{
Field: fv.TextValue.Field.Name(),
Value: string(fv.TextValue.Value),
Field: sanitizeOutputText(fv.TextValue.Field.Name()),
Value: sanitizeOutputText(string(fv.TextValue.Value)),
}, true
}
return MinimalFieldValue{}, false
@@ -798,7 +845,7 @@ func convertToMinimalIssuesResponse(fragment IssueQueryFragment) MinimalIssuesRe
func convertToMinimalIssueComment(comment *github.IssueComment) MinimalIssueComment {
m := MinimalIssueComment{
ID: comment.GetID(),
Body: comment.GetBody(),
Body: sanitizeOutputText(comment.GetBody()),
HTMLURL: comment.GetHTMLURL(),
User: convertToMinimalUser(comment.GetUser()),
AuthorAssociation: comment.GetAuthorAssociation(),
@@ -847,13 +894,13 @@ func convertToMinimalFileContentResponse(resp *github.RepositoryContentResponse)
m.Commit = &MinimalFileCommit{
SHA: resp.Commit.GetSHA(),
Message: resp.Commit.GetMessage(),
Message: sanitizeCommitMessage(resp.Commit.GetMessage()),
HTMLURL: resp.Commit.GetHTMLURL(),
}
if author := resp.Commit.Author; author != nil {
m.Commit.Author = &MinimalCommitAuthor{
Name: author.GetName(),
Name: sanitizeOutputText(author.GetName()),
Email: author.GetEmail(),
}
if author.Date != nil {
@@ -867,8 +914,8 @@ func convertToMinimalFileContentResponse(resp *github.RepositoryContentResponse)
func convertToMinimalPullRequest(pr *github.PullRequest) MinimalPullRequest {
m := MinimalPullRequest{
Number: pr.GetNumber(),
Title: pr.GetTitle(),
Body: pr.GetBody(),
Title: sanitizeOutputText(pr.GetTitle()),
Body: sanitizeOutputText(pr.GetBody()),
State: pr.GetState(),
Draft: pr.GetDraft(),
Merged: pr.GetMerged(),
@@ -897,7 +944,7 @@ func convertToMinimalPullRequest(pr *github.PullRequest) MinimalPullRequest {
for _, label := range pr.Labels {
if label != nil {
m.Labels = append(m.Labels, label.GetName())
m.Labels = append(m.Labels, sanitizeOutputText(label.GetName()))
}
}
@@ -926,7 +973,7 @@ func convertToMinimalPullRequest(pr *github.PullRequest) MinimalPullRequest {
}
if milestone := pr.GetMilestone(); milestone != nil {
m.Milestone = milestone.GetTitle()
m.Milestone = sanitizeOutputText(milestone.GetTitle())
}
return m
@@ -945,7 +992,7 @@ func convertToMinimalPRBranch(branch *github.PullRequestBranch) *MinimalPRBranch
if repo := branch.GetRepo(); repo != nil {
b.Repo = &MinimalPRBranchRepo{
FullName: repo.GetFullName(),
Description: repo.GetDescription(),
Description: sanitizeOutputText(repo.GetDescription()),
}
}
@@ -962,15 +1009,15 @@ func convertToMinimalProject(fullProject *github.ProjectV2) *MinimalProject {
NodeID: github.Ptr(fullProject.GetNodeID()),
Owner: convertToMinimalUser(fullProject.GetOwner()),
Creator: convertToMinimalUser(fullProject.GetCreator()),
Title: github.Ptr(fullProject.GetTitle()),
Description: github.Ptr(fullProject.GetDescription()),
Title: github.Ptr(sanitizeOutputText(fullProject.GetTitle())),
Description: github.Ptr(sanitizeOutputText(fullProject.GetDescription())),
Public: github.Ptr(fullProject.GetPublic()),
ClosedAt: github.Ptr(fullProject.GetClosedAt()),
CreatedAt: github.Ptr(fullProject.GetCreatedAt()),
UpdatedAt: github.Ptr(fullProject.GetUpdatedAt()),
DeletedAt: github.Ptr(fullProject.GetDeletedAt()),
Number: github.Ptr(fullProject.GetNumber()),
ShortDescription: github.Ptr(fullProject.GetShortDescription()),
ShortDescription: github.Ptr(sanitizeOutputText(fullProject.GetShortDescription())),
DeletedBy: convertToMinimalUser(fullProject.GetDeletedBy()),
}
}
@@ -1026,7 +1073,7 @@ func convertIssueToMinimalProjectItemContent(issue *github.Issue) *MinimalProjec
ID: issue.GetID(),
NodeID: issue.GetNodeID(),
Number: issue.GetNumber(),
Title: issue.GetTitle(),
Title: sanitizeOutputText(issue.GetTitle()),
State: issue.GetState(),
StateReason: issue.GetStateReason(),
HTMLURL: issue.GetHTMLURL(),
@@ -1048,11 +1095,11 @@ func convertIssueToMinimalProjectItemContent(issue *github.Issue) *MinimalProjec
}
for _, label := range issue.Labels {
if label != nil {
m.Labels = append(m.Labels, label.GetName())
m.Labels = append(m.Labels, sanitizeOutputText(label.GetName()))
}
}
if milestone := issue.GetMilestone(); milestone != nil {
m.Milestone = milestone.GetTitle()
m.Milestone = sanitizeOutputText(milestone.GetTitle())
}
return m
@@ -1063,7 +1110,7 @@ func convertPullRequestToMinimalProjectItemContent(pr *github.PullRequest) *Mini
ID: pr.GetID(),
NodeID: pr.GetNodeID(),
Number: pr.GetNumber(),
Title: pr.GetTitle(),
Title: sanitizeOutputText(pr.GetTitle()),
State: pr.GetState(),
HTMLURL: pr.GetHTMLURL(),
Repository: pullRequestRepositoryFullName(pr),
@@ -1086,11 +1133,11 @@ func convertPullRequestToMinimalProjectItemContent(pr *github.PullRequest) *Mini
}
for _, label := range pr.Labels {
if label != nil {
m.Labels = append(m.Labels, label.GetName())
m.Labels = append(m.Labels, sanitizeOutputText(label.GetName()))
}
}
if milestone := pr.GetMilestone(); milestone != nil {
m.Milestone = milestone.GetTitle()
m.Milestone = sanitizeOutputText(milestone.GetTitle())
}
return m
@@ -1100,7 +1147,7 @@ func convertDraftIssueToMinimalProjectItemContent(draftIssue *github.ProjectV2Dr
m := &MinimalProjectItemContent{
ID: draftIssue.GetID(),
NodeID: draftIssue.GetNodeID(),
Title: draftIssue.GetTitle(),
Title: sanitizeOutputText(draftIssue.GetTitle()),
CreatedAt: formatProjectTimestamp(draftIssue.CreatedAt),
UpdatedAt: formatProjectTimestamp(draftIssue.UpdatedAt),
}
@@ -1120,7 +1167,7 @@ func convertToMinimalProjectItemFields(fields []*github.ProjectV2ItemFieldValue)
}
minimalFields = append(minimalFields, MinimalProjectItemFieldValue{
ID: field.GetID(),
Name: field.GetName(),
Name: sanitizeOutputText(field.GetName()),
DataType: field.GetDataType(),
Value: minimalProjectFieldValue(field.GetValue()),
})
@@ -1132,10 +1179,16 @@ func minimalProjectFieldValue(value any) any {
switch v := value.(type) {
case nil:
return nil
case string, bool, int, int8, int16, int32, int64, uint, uint8, uint16, uint32, uint64, float32, float64:
case string:
return sanitizeOutputText(v)
case bool, int, int8, int16, int32, int64, uint, uint8, uint16, uint32, uint64, float32, float64:
return v
case []string:
return v
strings := make([]string, len(v))
for i, value := range v {
strings[i] = sanitizeOutputText(value)
}
return strings
case map[string]any:
return minimalProjectMapValue(v)
case []any:
@@ -1143,23 +1196,23 @@ func minimalProjectFieldValue(value any) any {
case *github.User:
return v.GetLogin()
case *github.Label:
return v.GetName()
return sanitizeOutputText(v.GetName())
case *github.Repository:
return v.GetFullName()
case *github.Milestone:
return v.GetTitle()
return sanitizeOutputText(v.GetTitle())
case *github.PullRequest:
return minimalProjectPullRequestRefFromPullRequest(v)
case *github.ProjectV2FieldOption:
return minimalProjectOptionValue{
ID: v.GetID(),
Name: projectTextContentString(v.GetName()),
Name: sanitizeOutputText(projectTextContentString(v.GetName())),
Color: v.GetColor(),
}
case *github.ProjectV2FieldIteration:
return minimalProjectIterationValue{
ID: v.GetID(),
Title: projectTextContentString(v.GetTitle()),
Title: sanitizeOutputText(projectTextContentString(v.GetTitle())),
StartDate: v.GetStartDate(),
Duration: v.GetDuration(),
}
@@ -1175,7 +1228,7 @@ func minimalProjectFieldValue(value any) any {
names := make([]string, 0, len(v))
for _, label := range v {
if label != nil {
names = append(names, label.GetName())
names = append(names, sanitizeOutputText(label.GetName()))
}
}
return names
@@ -1194,7 +1247,7 @@ func minimalProjectFieldValue(value any) any {
func minimalProjectMapValue(value map[string]any) any {
if text := minimalProjectTextValue(value); text != "" {
return text
return sanitizeOutputText(text)
}
if repo := fullNameFromMap(value); repo != "" {
return repo
@@ -1212,10 +1265,10 @@ func minimalProjectMapValue(value map[string]any) any {
return iteration
}
if title := stringFromMap(value, "title"); title != "" {
return title
return sanitizeOutputText(title)
}
if name := stringFromMap(value, "name"); name != "" {
return name
return sanitizeOutputText(name)
}
compact := make(map[string]any)
@@ -1239,6 +1292,9 @@ func minimalProjectArrayValue(values []any) any {
return strings
}
if strings, ok := minimalProjectStringsFromArray(values, "name"); ok {
for i, value := range strings {
strings[i] = sanitizeOutputText(value)
}
return strings
}
@@ -1273,7 +1329,7 @@ func minimalProjectOptionFromMap(value map[string]any) (minimalProjectOptionValu
}
return minimalProjectOptionValue{
ID: stringFromMap(value, "id"),
Name: name,
Name: sanitizeOutputText(name),
Color: color,
}, true
}
@@ -1286,7 +1342,7 @@ func minimalProjectIterationFromMap(value map[string]any) (minimalProjectIterati
}
return minimalProjectIterationValue{
ID: stringFromMap(value, "id"),
Title: textContentStringFromMap(value, "title"),
Title: sanitizeOutputText(textContentStringFromMap(value, "title")),
StartDate: startDate,
Duration: duration,
}, true
@@ -1359,7 +1415,7 @@ func minimalProjectPullRequestRefFromPullRequest(pr *github.PullRequest) minimal
}
return minimalProjectPullRequestRef{
Number: pr.GetNumber(),
Title: pr.GetTitle(),
Title: sanitizeOutputText(pr.GetTitle()),
State: pr.GetState(),
HTMLURL: pr.GetHTMLURL(),
Repository: pullRequestRepositoryFullName(pr),
@@ -1381,7 +1437,7 @@ func minimalProjectPullRequestRefFromMap(value map[string]any) minimalProjectPul
return minimalProjectPullRequestRef{
Number: intFromAny(value["number"]),
Title: stringFromMap(value, "title"),
Title: sanitizeOutputText(stringFromMap(value, "title")),
State: stringFromMap(value, "state"),
HTMLURL: htmlURL,
Repository: repository,
@@ -1541,12 +1597,12 @@ func newMinimalCommitFromCore(sha, htmlURL string, commit *github.Commit, author
if commit != nil {
minimalCommit.Commit = &MinimalCommitInfo{
Message: commit.GetMessage(),
Message: sanitizeCommitMessage(commit.GetMessage()),
}
if commit.Author != nil {
minimalCommit.Commit.Author = &MinimalCommitAuthor{
Name: commit.Author.GetName(),
Name: sanitizeOutputText(commit.Author.GetName()),
Email: commit.Author.GetEmail(),
}
if commit.Author.Date != nil {
@@ -1556,7 +1612,7 @@ func newMinimalCommitFromCore(sha, htmlURL string, commit *github.Commit, author
if commit.Committer != nil {
minimalCommit.Commit.Committer = &MinimalCommitAuthor{
Name: commit.Committer.GetName(),
Name: sanitizeOutputText(commit.Committer.GetName()),
Email: commit.Committer.GetEmail(),
}
if commit.Committer.Date != nil {
@@ -1744,7 +1800,7 @@ func convertToMinimalPullRequestCommits(commits []*github.RepositoryCommit) []Mi
}
if commit.Commit != nil {
minimalCommit.Message = commit.Commit.GetMessage()
minimalCommit.Message = sanitizeCommitMessage(commit.Commit.GetMessage())
minimalCommit.Author = convertToMinimalCommitAuthor(commit.Commit.Author)
}
@@ -1759,7 +1815,7 @@ func convertToMinimalCommitAuthor(author *github.CommitAuthor) *MinimalCommitAut
}
minimalAuthor := &MinimalCommitAuthor{
Name: author.GetName(),
Name: sanitizeOutputText(author.GetName()),
Email: author.GetEmail(),
}
if author.Date != nil {
@@ -1782,8 +1838,8 @@ func convertToMinimalRelease(release *github.RepositoryRelease) MinimalRelease {
m := MinimalRelease{
ID: release.GetID(),
TagName: release.GetTagName(),
Name: release.GetName(),
Body: release.GetBody(),
Name: sanitizeOutputText(release.GetName()),
Body: sanitizeOutputText(release.GetBody()),
HTMLURL: release.GetHTMLURL(),
Prerelease: release.GetPrerelease(),
Draft: release.GetDraft(),
@@ -1886,7 +1942,7 @@ func convertToMinimalReviewThread(thread reviewThreadNode) MinimalReviewThread {
func convertToMinimalReviewComment(c reviewCommentNode) MinimalReviewComment {
m := MinimalReviewComment{
Body: string(c.Body),
Body: sanitizeOutputText(string(c.Body)),
Path: string(c.Path),
Author: string(c.Author.Login),
HTMLURL: c.URL.String(),
+656
View File
@@ -0,0 +1,656 @@
package github
import (
"net/mail"
"strconv"
"strings"
"github.com/google/go-github/v89/github"
)
func sanitizeOutputStringPtr(s *string) *string {
if s == nil {
return nil
}
sanitized := sanitizeOutputText(*s)
return &sanitized
}
type protectedCommitTrailerAddress struct {
placeholder string
address string
}
func sanitizeCommitMessage(message string) string {
lines := strings.Split(message, "\n")
protected := make([]protectedCommitTrailerAddress, 0)
for i, line := range lines {
address, ok := commitTrailerAddress(line)
if !ok {
continue
}
placeholder := commitTrailerAddressPlaceholder(message, len(protected))
addressIndex := strings.LastIndex(line, address)
lines[i] = line[:addressIndex] + placeholder + line[addressIndex+len(address):]
protected = append(protected, protectedCommitTrailerAddress{
placeholder: placeholder,
address: address,
})
}
sanitized := sanitizeOutputText(strings.Join(lines, "\n"))
for _, trailerAddress := range protected {
sanitized = strings.ReplaceAll(sanitized, trailerAddress.placeholder, trailerAddress.address)
}
return sanitized
}
func sanitizeCommitMessagePtr(message *string) *string {
if message == nil {
return nil
}
sanitized := sanitizeCommitMessage(*message)
return &sanitized
}
func commitTrailerAddress(line string) (string, bool) {
colonIndex := strings.IndexByte(line, ':')
if colonIndex <= 0 || !isCommitTrailerToken(line[:colonIndex]) {
return "", false
}
value := strings.TrimSpace(line[colonIndex+1:])
if !strings.HasSuffix(value, ">") {
return "", false
}
openIndex := strings.LastIndexByte(value, '<')
if openIndex < 0 || openIndex == len(value)-1 {
return "", false
}
email := value[openIndex+1 : len(value)-1]
address, err := mail.ParseAddress(value)
if err != nil || address.Address != email {
return "", false
}
return "<" + email + ">", true
}
func isCommitTrailerToken(token string) bool {
if token == "" {
return false
}
for i := range len(token) {
c := token[i]
if c >= 'a' && c <= 'z' || c >= 'A' && c <= 'Z' || c >= '0' && c <= '9' || c == '-' {
continue
}
return false
}
return true
}
func commitTrailerAddressPlaceholder(message string, index int) string {
placeholder := "GITHUBMCPCOMMITTRAILEREMAIL" + strconv.Itoa(index) + "PLACEHOLDER"
for strings.Contains(message, placeholder) {
placeholder += "X"
}
return placeholder
}
func sanitizedIssueCopy(issue *github.Issue) *github.Issue {
if issue == nil {
return nil
}
issueCopy := *issue
issueCopy.Title = sanitizeOutputStringPtr(issue.Title)
issueCopy.Body = sanitizeOutputStringPtr(issue.Body)
issueCopy.Labels = sanitizedLabelsCopy(issue.Labels)
issueCopy.Milestone = sanitizedMilestoneCopy(issue.Milestone)
issueCopy.Type = sanitizedIssueTypeCopy(issue.Type)
return &issueCopy
}
func sanitizedSubIssueCopy(issue *github.SubIssue) *github.SubIssue {
if issue == nil {
return nil
}
issueCopy := *issue
issueCopy.Title = sanitizeOutputStringPtr(issue.Title)
issueCopy.Body = sanitizeOutputStringPtr(issue.Body)
issueCopy.Labels = sanitizedLabelsCopy(issue.Labels)
issueCopy.Milestone = sanitizedMilestoneCopy(issue.Milestone)
issueCopy.Type = sanitizedIssueTypeCopy(issue.Type)
return &issueCopy
}
func sanitizedSubIssuesCopy(issues []*github.SubIssue) []*github.SubIssue {
if issues == nil {
return nil
}
issuesCopy := make([]*github.SubIssue, len(issues))
for i, issue := range issues {
issuesCopy[i] = sanitizedSubIssueCopy(issue)
}
return issuesCopy
}
func sanitizedIssuesSearchResultCopy(result *github.IssuesSearchResult) *github.IssuesSearchResult {
if result == nil {
return nil
}
resultCopy := *result
if result.Issues != nil {
resultCopy.Issues = make([]*github.Issue, len(result.Issues))
for i, issue := range result.Issues {
resultCopy.Issues[i] = sanitizedIssueCopy(issue)
}
}
return &resultCopy
}
func sanitizedLabelsCopy(labels []*github.Label) []*github.Label {
if labels == nil {
return nil
}
labelsCopy := make([]*github.Label, len(labels))
for i, label := range labels {
if label == nil {
continue
}
labelCopy := *label
labelCopy.Name = sanitizeOutputStringPtr(label.Name)
labelCopy.Description = sanitizeOutputStringPtr(label.Description)
labelsCopy[i] = &labelCopy
}
return labelsCopy
}
func sanitizedMilestoneCopy(milestone *github.Milestone) *github.Milestone {
if milestone == nil {
return nil
}
milestoneCopy := *milestone
milestoneCopy.Title = sanitizeOutputStringPtr(milestone.Title)
milestoneCopy.Description = sanitizeOutputStringPtr(milestone.Description)
return &milestoneCopy
}
func sanitizedIssueTypeCopy(issueType *github.IssueType) *github.IssueType {
if issueType == nil {
return nil
}
issueTypeCopy := *issueType
issueTypeCopy.Name = sanitizeOutputStringPtr(issueType.Name)
issueTypeCopy.Description = sanitizeOutputStringPtr(issueType.Description)
return &issueTypeCopy
}
func sanitizedIssueTypesCopy(issueTypes []*github.IssueType) []*github.IssueType {
if issueTypes == nil {
return nil
}
issueTypesCopy := make([]*github.IssueType, len(issueTypes))
for i, issueType := range issueTypes {
issueTypesCopy[i] = sanitizedIssueTypeCopy(issueType)
}
return issueTypesCopy
}
func sanitizedRepositoryCopy(repo *github.Repository) *github.Repository {
if repo == nil {
return nil
}
repoCopy := *repo
repoCopy.Description = sanitizeOutputStringPtr(repo.Description)
return &repoCopy
}
func sanitizedRepositoriesSearchResultCopy(result *github.RepositoriesSearchResult) *github.RepositoriesSearchResult {
if result == nil {
return nil
}
resultCopy := *result
if result.Repositories != nil {
resultCopy.Repositories = make([]*github.Repository, len(result.Repositories))
for i, repo := range result.Repositories {
resultCopy.Repositories[i] = sanitizedRepositoryCopy(repo)
}
}
return &resultCopy
}
func sanitizedReleaseCopy(release *github.RepositoryRelease) *github.RepositoryRelease {
if release == nil {
return nil
}
releaseCopy := *release
releaseCopy.Name = sanitizeOutputStringPtr(release.Name)
releaseCopy.Body = sanitizeOutputStringPtr(release.Body)
return &releaseCopy
}
func sanitizedCommitAuthorCopy(author *github.CommitAuthor) *github.CommitAuthor {
if author == nil {
return nil
}
authorCopy := *author
authorCopy.Name = sanitizeOutputStringPtr(author.Name)
return &authorCopy
}
func sanitizedCommitCopy(commit *github.Commit) *github.Commit {
if commit == nil {
return nil
}
commitCopy := *commit
commitCopy.Message = sanitizeCommitMessagePtr(commit.Message)
commitCopy.Author = sanitizedCommitAuthorCopy(commit.Author)
commitCopy.Committer = sanitizedCommitAuthorCopy(commit.Committer)
return &commitCopy
}
func sanitizedHeadCommitCopy(commit *github.HeadCommit) *github.HeadCommit {
if commit == nil {
return nil
}
commitCopy := *commit
commitCopy.Message = sanitizeCommitMessagePtr(commit.Message)
commitCopy.Author = sanitizedCommitAuthorCopy(commit.Author)
commitCopy.Committer = sanitizedCommitAuthorCopy(commit.Committer)
return &commitCopy
}
func sanitizedProjectV2TextContentCopy(content *github.ProjectV2TextContent) *github.ProjectV2TextContent {
if content == nil {
return nil
}
contentCopy := *content
contentCopy.Raw = sanitizeOutputStringPtr(content.Raw)
contentCopy.HTML = sanitizeOutputStringPtr(content.HTML)
return &contentCopy
}
func sanitizedProjectV2FieldOptionCopy(option *github.ProjectV2FieldOption) *github.ProjectV2FieldOption {
if option == nil {
return nil
}
optionCopy := *option
optionCopy.Name = sanitizedProjectV2TextContentCopy(option.Name)
optionCopy.Description = sanitizedProjectV2TextContentCopy(option.Description)
return &optionCopy
}
func sanitizedProjectV2FieldIterationCopy(iteration *github.ProjectV2FieldIteration) *github.ProjectV2FieldIteration {
if iteration == nil {
return nil
}
iterationCopy := *iteration
iterationCopy.Title = sanitizedProjectV2TextContentCopy(iteration.Title)
return &iterationCopy
}
func sanitizedProjectV2FieldConfigurationCopy(configuration *github.ProjectV2FieldConfiguration) *github.ProjectV2FieldConfiguration {
if configuration == nil {
return nil
}
configurationCopy := *configuration
if configuration.Iterations != nil {
configurationCopy.Iterations = make([]*github.ProjectV2FieldIteration, len(configuration.Iterations))
for i, iteration := range configuration.Iterations {
configurationCopy.Iterations[i] = sanitizedProjectV2FieldIterationCopy(iteration)
}
}
return &configurationCopy
}
func sanitizedProjectV2FieldCopy(field *github.ProjectV2Field) *github.ProjectV2Field {
if field == nil {
return nil
}
fieldCopy := *field
fieldCopy.Name = sanitizeOutputStringPtr(field.Name)
if field.Options != nil {
fieldCopy.Options = make([]*github.ProjectV2FieldOption, len(field.Options))
for i, option := range field.Options {
fieldCopy.Options[i] = sanitizedProjectV2FieldOptionCopy(option)
}
}
fieldCopy.Configuration = sanitizedProjectV2FieldConfigurationCopy(field.Configuration)
return &fieldCopy
}
func sanitizedProjectV2FieldsCopy(fields []*github.ProjectV2Field) []*github.ProjectV2Field {
if fields == nil {
return nil
}
fieldsCopy := make([]*github.ProjectV2Field, len(fields))
for i, field := range fields {
fieldsCopy[i] = sanitizedProjectV2FieldCopy(field)
}
return fieldsCopy
}
func sanitizedPullRequestCopy(pr *github.PullRequest) *github.PullRequest {
if pr == nil {
return nil
}
prCopy := *pr
prCopy.Title = sanitizeOutputStringPtr(pr.Title)
prCopy.Body = sanitizeOutputStringPtr(pr.Body)
prCopy.Labels = sanitizedLabelsCopy(pr.Labels)
prCopy.Milestone = sanitizedMilestoneCopy(pr.Milestone)
prCopy.Head = sanitizedPullRequestBranchCopy(pr.Head)
prCopy.Base = sanitizedPullRequestBranchCopy(pr.Base)
return &prCopy
}
func sanitizedPullRequestBranchCopy(branch *github.PullRequestBranch) *github.PullRequestBranch {
if branch == nil {
return nil
}
branchCopy := *branch
branchCopy.Repo = sanitizedRepositoryCopy(branch.Repo)
return &branchCopy
}
func sanitizedPullRequestsCopy(prs []*github.PullRequest) []*github.PullRequest {
if prs == nil {
return nil
}
prsCopy := make([]*github.PullRequest, len(prs))
for i, pr := range prs {
prsCopy[i] = sanitizedPullRequestCopy(pr)
}
return prsCopy
}
func sanitizedPullRequestCommentCopy(comment *github.PullRequestComment) *github.PullRequestComment {
if comment == nil {
return nil
}
commentCopy := *comment
commentCopy.Body = sanitizeOutputStringPtr(comment.Body)
return &commentCopy
}
func sanitizedCombinedStatusCopy(status *github.CombinedStatus) *github.CombinedStatus {
if status == nil {
return nil
}
statusCopy := *status
if status.Statuses != nil {
statusCopy.Statuses = make([]*github.RepoStatus, len(status.Statuses))
for i, repoStatus := range status.Statuses {
statusCopy.Statuses[i] = sanitizedRepoStatusCopy(repoStatus)
}
}
return &statusCopy
}
func sanitizedRepoStatusCopy(status *github.RepoStatus) *github.RepoStatus {
if status == nil {
return nil
}
statusCopy := *status
statusCopy.Description = sanitizeOutputStringPtr(status.Description)
return &statusCopy
}
func sanitizedWorkflowCopy(workflow *github.Workflow) *github.Workflow {
if workflow == nil {
return nil
}
workflowCopy := *workflow
workflowCopy.Name = sanitizeOutputStringPtr(workflow.Name)
return &workflowCopy
}
func sanitizedWorkflowsCopy(workflows *github.Workflows) *github.Workflows {
if workflows == nil {
return nil
}
workflowsCopy := *workflows
if workflows.Workflows != nil {
workflowsCopy.Workflows = make([]*github.Workflow, len(workflows.Workflows))
for i, workflow := range workflows.Workflows {
workflowsCopy.Workflows[i] = sanitizedWorkflowCopy(workflow)
}
}
return &workflowsCopy
}
func sanitizedWorkflowRunCopy(run *github.WorkflowRun) *github.WorkflowRun {
if run == nil {
return nil
}
runCopy := *run
runCopy.Name = sanitizeOutputStringPtr(run.Name)
runCopy.DisplayTitle = sanitizeOutputStringPtr(run.DisplayTitle)
runCopy.PullRequests = sanitizedPullRequestsCopy(run.PullRequests)
runCopy.HeadCommit = sanitizedHeadCommitCopy(run.HeadCommit)
runCopy.Repository = sanitizedRepositoryCopy(run.Repository)
runCopy.HeadRepository = sanitizedRepositoryCopy(run.HeadRepository)
return &runCopy
}
func sanitizedWorkflowRunsCopy(runs *github.WorkflowRuns) *github.WorkflowRuns {
if runs == nil {
return nil
}
runsCopy := *runs
if runs.WorkflowRuns != nil {
runsCopy.WorkflowRuns = make([]*github.WorkflowRun, len(runs.WorkflowRuns))
for i, run := range runs.WorkflowRuns {
runsCopy.WorkflowRuns[i] = sanitizedWorkflowRunCopy(run)
}
}
return &runsCopy
}
func sanitizedWorkflowJobCopy(job *github.WorkflowJob) *github.WorkflowJob {
if job == nil {
return nil
}
jobCopy := *job
jobCopy.Name = sanitizeOutputStringPtr(job.Name)
jobCopy.WorkflowName = sanitizeOutputStringPtr(job.WorkflowName)
if job.Steps != nil {
jobCopy.Steps = make([]*github.TaskStep, len(job.Steps))
for i, step := range job.Steps {
if step == nil {
continue
}
stepCopy := *step
stepCopy.Name = sanitizeOutputStringPtr(step.Name)
jobCopy.Steps[i] = &stepCopy
}
}
return &jobCopy
}
func sanitizedWorkflowJobsCopy(jobs *github.Jobs) *github.Jobs {
if jobs == nil {
return nil
}
jobsCopy := *jobs
if jobs.Jobs != nil {
jobsCopy.Jobs = make([]*github.WorkflowJob, len(jobs.Jobs))
for i, job := range jobs.Jobs {
jobsCopy.Jobs[i] = sanitizedWorkflowJobCopy(job)
}
}
return &jobsCopy
}
func sanitizedSecurityAdvisoryCopy(advisory *github.SecurityAdvisory) *github.SecurityAdvisory {
if advisory == nil {
return nil
}
advisoryCopy := *advisory
advisoryCopy.Summary = sanitizeOutputStringPtr(advisory.Summary)
advisoryCopy.Description = sanitizeOutputStringPtr(advisory.Description)
advisoryCopy.PrivateFork = sanitizedRepositoryCopy(advisory.PrivateFork)
advisoryCopy.CollaboratingTeams = sanitizedTeamsCopy(advisory.CollaboratingTeams)
return &advisoryCopy
}
func sanitizedTeamsCopy(teams []*github.Team) []*github.Team {
if teams == nil {
return nil
}
teamsCopy := make([]*github.Team, len(teams))
for i, team := range teams {
teamsCopy[i] = sanitizedTeamCopy(team)
}
return teamsCopy
}
func sanitizedTeamCopy(team *github.Team) *github.Team {
if team == nil {
return nil
}
teamCopy := *team
teamCopy.Name = sanitizeOutputStringPtr(team.Name)
teamCopy.Description = sanitizeOutputStringPtr(team.Description)
teamCopy.Parent = sanitizedTeamCopy(team.Parent)
return &teamCopy
}
func sanitizedSecurityAdvisoriesCopy(advisories []*github.SecurityAdvisory) []*github.SecurityAdvisory {
if advisories == nil {
return nil
}
advisoriesCopy := make([]*github.SecurityAdvisory, len(advisories))
for i, advisory := range advisories {
advisoriesCopy[i] = sanitizedSecurityAdvisoryCopy(advisory)
}
return advisoriesCopy
}
func sanitizedGlobalSecurityAdvisoryCopy(advisory *github.GlobalSecurityAdvisory) *github.GlobalSecurityAdvisory {
if advisory == nil {
return nil
}
advisoryCopy := *advisory
advisoryCopy.SecurityAdvisory = *sanitizedSecurityAdvisoryCopy(&advisory.SecurityAdvisory)
return &advisoryCopy
}
func sanitizedGlobalSecurityAdvisoriesCopy(advisories []*github.GlobalSecurityAdvisory) []*github.GlobalSecurityAdvisory {
if advisories == nil {
return nil
}
advisoriesCopy := make([]*github.GlobalSecurityAdvisory, len(advisories))
for i, advisory := range advisories {
advisoriesCopy[i] = sanitizedGlobalSecurityAdvisoryCopy(advisory)
}
return advisoriesCopy
}
func sanitizedDependabotAlertCopy(alert *github.DependabotAlert) *github.DependabotAlert {
if alert == nil {
return nil
}
alertCopy := *alert
alertCopy.DismissedComment = sanitizeOutputStringPtr(alert.DismissedComment)
alertCopy.Repository = sanitizedRepositoryCopy(alert.Repository)
if alert.SecurityAdvisory != nil {
advisoryCopy := *alert.SecurityAdvisory
advisoryCopy.Summary = sanitizeOutputStringPtr(alert.SecurityAdvisory.Summary)
advisoryCopy.Description = sanitizeOutputStringPtr(alert.SecurityAdvisory.Description)
alertCopy.SecurityAdvisory = &advisoryCopy
}
return &alertCopy
}
func sanitizedDependabotAlertsCopy(alerts []*github.DependabotAlert) []*github.DependabotAlert {
if alerts == nil {
return nil
}
alertsCopy := make([]*github.DependabotAlert, len(alerts))
for i, alert := range alerts {
alertsCopy[i] = sanitizedDependabotAlertCopy(alert)
}
return alertsCopy
}
func sanitizedCodeScanningAlertCopy(alert *github.Alert) *github.Alert {
if alert == nil {
return nil
}
alertCopy := *alert
alertCopy.Repository = sanitizedRepositoryCopy(alert.Repository)
alertCopy.RuleDescription = sanitizeOutputStringPtr(alert.RuleDescription)
alertCopy.Rule = sanitizedCodeScanningRuleCopy(alert.Rule)
alertCopy.DismissedComment = sanitizeOutputStringPtr(alert.DismissedComment)
alertCopy.MostRecentInstance = sanitizedCodeScanningInstanceCopy(alert.MostRecentInstance)
if alert.Instances != nil {
alertCopy.Instances = make([]*github.MostRecentInstance, len(alert.Instances))
for i, instance := range alert.Instances {
alertCopy.Instances[i] = sanitizedCodeScanningInstanceCopy(instance)
}
}
return &alertCopy
}
func sanitizedCodeScanningRuleCopy(rule *github.Rule) *github.Rule {
if rule == nil {
return nil
}
ruleCopy := *rule
ruleCopy.Name = sanitizeOutputStringPtr(rule.Name)
ruleCopy.Description = sanitizeOutputStringPtr(rule.Description)
ruleCopy.FullDescription = sanitizeOutputStringPtr(rule.FullDescription)
ruleCopy.Help = sanitizeOutputStringPtr(rule.Help)
return &ruleCopy
}
func sanitizedCodeScanningAlertsCopy(alerts []*github.Alert) []*github.Alert {
if alerts == nil {
return nil
}
alertsCopy := make([]*github.Alert, len(alerts))
for i, alert := range alerts {
alertsCopy[i] = sanitizedCodeScanningAlertCopy(alert)
}
return alertsCopy
}
func sanitizedCodeScanningInstanceCopy(instance *github.MostRecentInstance) *github.MostRecentInstance {
if instance == nil {
return nil
}
instanceCopy := *instance
if instance.Message != nil {
messageCopy := *instance.Message
messageCopy.Text = sanitizeOutputStringPtr(instance.Message.Text)
instanceCopy.Message = &messageCopy
}
return &instanceCopy
}
func sanitizedSecretScanningAlertCopy(alert *github.SecretScanningAlert) *github.SecretScanningAlert {
if alert == nil {
return nil
}
alertCopy := *alert
alertCopy.Repository = sanitizedRepositoryCopy(alert.Repository)
alertCopy.ResolutionComment = sanitizeOutputStringPtr(alert.ResolutionComment)
alertCopy.PushProtectionBypassRequestComment = sanitizeOutputStringPtr(alert.PushProtectionBypassRequestComment)
alertCopy.PushProtectionBypassRequestReviewerComment = sanitizeOutputStringPtr(alert.PushProtectionBypassRequestReviewerComment)
return &alertCopy
}
func sanitizedSecretScanningAlertsCopy(alerts []*github.SecretScanningAlert) []*github.SecretScanningAlert {
if alerts == nil {
return nil
}
alertsCopy := make([]*github.SecretScanningAlert, len(alerts))
for i, alert := range alerts {
alertsCopy[i] = sanitizedSecretScanningAlertCopy(alert)
}
return alertsCopy
}
+5 -5
View File
@@ -134,7 +134,7 @@ func convertToMinimalStatusUpdate(node statusUpdateNode) MinimalProjectStatusUpd
return MinimalProjectStatusUpdate{
ID: fmt.Sprintf("%v", node.ID),
Body: derefString(node.Body),
Body: sanitizeOutputText(derefString(node.Body)),
Status: derefString(node.Status),
CreatedAt: node.CreatedAt.Time.Format(time.RFC3339),
StartDate: derefString(node.StartDate),
@@ -926,7 +926,7 @@ func listProjectFields(ctx context.Context, client *github.Client, args map[stri
defer func() { _ = resp.Body.Close() }()
response := map[string]any{
"fields": projectFields,
"fields": sanitizedProjectV2FieldsCopy(projectFields),
"pageInfo": buildPageInfo(resp),
}
@@ -1099,7 +1099,7 @@ func getProjectField(ctx context.Context, client *github.Client, owner, ownerTyp
}
return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to get project field", resp, body), nil, nil
}
r, err := json.Marshal(projectField)
r, err := json.Marshal(sanitizedProjectV2FieldCopy(projectField))
if err != nil {
return nil, nil, fmt.Errorf("failed to marshal response: %w", err)
}
@@ -1880,7 +1880,7 @@ func createIterationField(ctx context.Context, gqlClient *githubv4.Client, owner
for _, iter := range field.Configuration.Iterations {
iterResults = append(iterResults, map[string]any{
"id": iter.ID,
"title": iter.Title,
"title": sanitizeOutputText(iter.Title),
"start_date": iter.StartDate,
"duration": iter.Duration,
})
@@ -1888,7 +1888,7 @@ func createIterationField(ctx context.Context, gqlClient *githubv4.Client, owner
result := map[string]any{
"id": field.ID,
"name": field.Name,
"name": sanitizeOutputText(field.Name),
"configuration": map[string]any{
"iterations": iterResults,
},
+53 -4
View File
@@ -9,6 +9,7 @@ import (
"github.com/github/github-mcp-server/internal/githubv4mock"
"github.com/github/github-mcp-server/internal/toolsnaps"
"github.com/github/github-mcp-server/pkg/translations"
gogithub "github.com/google/go-github/v89/github"
"github.com/google/jsonschema-go/jsonschema"
"github.com/shurcooL/githubv4"
"github.com/stretchr/testify/assert"
@@ -129,10 +130,46 @@ func Test_ProjectsList_ListProjects(t *testing.T) {
}
}
func projectFieldSanitizationFixture() map[string]any {
return map[string]any{
"id": 101,
"name": baselineUnsafeText,
"data_type": "iteration",
"options": []map[string]any{{
"id": "option-1",
"name": map[string]any{"raw": baselineUnsafeText, "html": baselineUnsafeText},
"description": map[string]any{"raw": baselineUnsafeText, "html": baselineUnsafeText},
}},
"configuration": map[string]any{
"iterations": []map[string]any{{
"id": "iteration-1",
"title": map[string]any{"raw": baselineUnsafeText, "html": baselineUnsafeText},
"start_date": "2026-01-01",
"duration": 14,
}},
},
}
}
func assertSanitizedProjectField(t *testing.T, field map[string]any) {
t.Helper()
expected := sanitizeOutputText(baselineUnsafeText)
assert.Equal(t, expected, field["name"])
options := field["options"].([]any)
option := options[0].(map[string]any)
assert.Equal(t, expected, option["name"].(map[string]any)["raw"])
assert.Equal(t, expected, option["description"].(map[string]any)["raw"])
configuration := field["configuration"].(map[string]any)
iterations := configuration["iterations"].([]any)
assert.Equal(t, expected, iterations[0].(map[string]any)["title"].(map[string]any)["raw"])
}
func Test_ProjectsList_ListProjectFields(t *testing.T) {
toolDef := ProjectsList(translations.NullTranslationHelper)
fields := []map[string]any{{"id": 101, "name": "Status", "data_type": "single_select"}}
fields := []map[string]any{projectFieldSanitizationFixture()}
t.Run("success organization", func(t *testing.T) {
mockedClient := MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
@@ -162,6 +199,7 @@ func Test_ProjectsList_ListProjectFields(t *testing.T) {
fieldsList, ok := response["fields"].([]any)
require.True(t, ok)
assert.Equal(t, 1, len(fieldsList))
assertSanitizedProjectField(t, fieldsList[0].(map[string]any))
})
t.Run("missing project_number", func(t *testing.T) {
@@ -727,7 +765,7 @@ func Test_ProjectsGet_IFC_InsidersMode(t *testing.T) {
func Test_ProjectsGet_GetProjectField(t *testing.T) {
toolDef := ProjectsGet(translations.NullTranslationHelper)
field := map[string]any{"id": 101, "name": "Status", "data_type": "single_select"}
field := projectFieldSanitizationFixture()
t.Run("success organization", func(t *testing.T) {
mockedClient := MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
@@ -756,6 +794,7 @@ func Test_ProjectsGet_GetProjectField(t *testing.T) {
err = json.Unmarshal([]byte(textContent.Text), &response)
require.NoError(t, err)
assert.NotNil(t, response["id"])
assertSanitizedProjectField(t, response)
})
t.Run("missing field_id", func(t *testing.T) {
@@ -1323,10 +1362,20 @@ func TestMinimalProjectFieldValue(t *testing.T) {
{
name: "labels",
value: []any{
map[string]any{"name": "bug", "url": "https://api.github.com/repos/cli/cli/labels/bug"},
map[string]any{"name": baselineUnsafeText, "url": "https://api.github.com/repos/cli/cli/labels/bug"},
map[string]any{"name": "help wanted", "url": "https://api.github.com/repos/cli/cli/labels/help%20wanted"},
},
want: []string{"bug", "help wanted"},
want: []string{sanitizeOutputText(baselineUnsafeText), "help wanted"},
},
{
name: "typed labels",
value: []*gogithub.Label{{Name: gogithub.Ptr(baselineUnsafeText)}},
want: []string{sanitizeOutputText(baselineUnsafeText)},
},
{
name: "string labels",
value: []string{baselineUnsafeText},
want: []string{sanitizeOutputText(baselineUnsafeText)},
},
{
name: "repository",
+6 -2
View File
@@ -199,12 +199,12 @@ func updateFieldIterationResponse() githubv4mock.GQLResponse {
"updateProjectV2Field": map[string]any{
"projectV2Field": map[string]any{
"id": "PVTIF_field1",
"name": "Sprint",
"name": baselineUnsafeText,
"configuration": map[string]any{
"iterations": []any{
map[string]any{
"id": "PVTI_iter1",
"title": "Sprint 1",
"title": baselineUnsafeText,
"startDate": "2025-01-20",
"duration": 7,
},
@@ -295,6 +295,10 @@ func Test_ProjectsWrite_CreateIterationField(t *testing.T) {
err = json.Unmarshal([]byte(textContent.Text), &response)
require.NoError(t, err)
assert.Equal(t, "PVTIF_field1", response["id"])
assert.Equal(t, sanitizeOutputText(baselineUnsafeText), response["name"])
configuration := response["configuration"].(map[string]any)
iterations := configuration["iterations"].([]any)
assert.Equal(t, sanitizeOutputText(baselineUnsafeText), iterations[0].(map[string]any)["title"])
})
t.Run("success without iterations", func(t *testing.T) {
+11 -25
View File
@@ -17,7 +17,6 @@ import (
"github.com/github/github-mcp-server/pkg/ifc"
"github.com/github/github-mcp-server/pkg/inventory"
"github.com/github/github-mcp-server/pkg/octicons"
"github.com/github/github-mcp-server/pkg/sanitize"
"github.com/github/github-mcp-server/pkg/scopes"
"github.com/github/github-mcp-server/pkg/translations"
"github.com/github/github-mcp-server/pkg/utils"
@@ -185,16 +184,6 @@ func GetPullRequest(ctx context.Context, client *github.Client, deps ToolDepende
return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to get pull request", resp, body), nil
}
// sanitize title/body on response
if pr != nil {
if pr.Title != nil {
pr.Title = github.Ptr(sanitize.Sanitize(*pr.Title))
}
if pr.Body != nil {
pr.Body = github.Ptr(sanitize.Sanitize(*pr.Body))
}
}
if ff.LockdownMode {
if restricted, err := authorLockdownResult(ctx, cache, owner, repo, pr.GetUser().GetLogin(), lockdownPullRequestRestrictedMessage); restricted != nil || err != nil {
return restricted, err
@@ -307,7 +296,7 @@ func GetPullRequestStatus(ctx context.Context, client *github.Client, owner, rep
return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to get combined status", resp, body), nil
}
r, err := json.Marshal(status)
r, err := json.Marshal(sanitizedCombinedStatusCopy(status))
if err != nil {
return nil, fmt.Errorf("failed to marshal response: %w", err)
}
@@ -1298,6 +1287,7 @@ func AddReplyToPullRequestComment(t translations.TranslationHelperFunc) inventor
return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to add reply to pull request comment", resp, bodyBytes), nil, nil
}
}
comment = sanitizedPullRequestCommentCopy(comment)
var result any
switch {
@@ -1420,6 +1410,9 @@ func listPullRequestsTool(t translations.TranslationHelperFunc, includeFields bo
if err != nil {
return utils.NewToolResultError(err.Error()), nil, nil
}
if err := validateEnumParam("state", state, "open", "closed", "all"); err != nil {
return utils.NewToolResultError(err.Error()), nil, nil
}
head, err := OptionalParam[string](args, "head")
if err != nil {
return utils.NewToolResultError(err.Error()), nil, nil
@@ -1432,10 +1425,16 @@ func listPullRequestsTool(t translations.TranslationHelperFunc, includeFields bo
if err != nil {
return utils.NewToolResultError(err.Error()), nil, nil
}
if err := validateEnumParam("sort", sort, "created", "updated", "popularity", "long-running"); err != nil {
return utils.NewToolResultError(err.Error()), nil, nil
}
direction, err := OptionalParam[string](args, "direction")
if err != nil {
return utils.NewToolResultError(err.Error()), nil, nil
}
if err := validateEnumParam("direction", direction, "asc", "desc"); err != nil {
return utils.NewToolResultError(err.Error()), nil, nil
}
var fields []string
if includeFields {
fields, err = OptionalStringArrayParam(args, "fields")
@@ -1482,19 +1481,6 @@ func listPullRequestsTool(t translations.TranslationHelperFunc, includeFields bo
return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to list pull requests", resp, bodyBytes), nil, nil
}
// sanitize title/body on each PR
for _, pr := range prs {
if pr == nil {
continue
}
if pr.Title != nil {
pr.Title = github.Ptr(sanitize.Sanitize(*pr.Title))
}
if pr.Body != nil {
pr.Body = github.Ptr(sanitize.Sanitize(*pr.Body))
}
}
minimalPRs := make([]MinimalPullRequest, 0, len(prs))
for _, pr := range prs {
if pr != nil {
+55 -20
View File
@@ -696,7 +696,7 @@ func Test_ListPullRequests(t *testing.T) {
requestArgs: map[string]any{
"owner": "owner",
"repo": "repo",
"state": "invalid",
"state": "open",
},
expectError: true,
expectedErrMsg: "failed to list pull requests",
@@ -891,11 +891,19 @@ func Test_SearchPullRequests(t *testing.T) {
Issues: []*github.Issue{
{
Number: github.Ptr(42),
Title: github.Ptr("Test PR 1"),
Body: github.Ptr("Updated tests."),
Title: github.Ptr(baselineUnsafeText),
Body: github.Ptr(baselineUnsafeText),
State: github.Ptr("open"),
HTMLURL: github.Ptr("https://github.com/owner/repo/pull/1"),
Comments: github.Ptr(5),
Labels: []*github.Label{{
Name: github.Ptr(baselineUnsafeText),
Description: github.Ptr(baselineUnsafeText),
}},
Milestone: &github.Milestone{
Title: github.Ptr(baselineUnsafeText),
Description: github.Ptr(baselineUnsafeText),
},
User: &github.User{
Login: github.Ptr("user1"),
},
@@ -913,6 +921,7 @@ func Test_SearchPullRequests(t *testing.T) {
},
},
}
expectedSearchResult := sanitizedIssuesSearchResultCopy(mockSearchResult)
tests := []struct {
name string
@@ -946,7 +955,7 @@ func Test_SearchPullRequests(t *testing.T) {
"perPage": float64(30),
},
expectError: false,
expectedResult: mockSearchResult,
expectedResult: expectedSearchResult,
},
{
name: "pull request search with owner and repo parameters",
@@ -972,7 +981,7 @@ func Test_SearchPullRequests(t *testing.T) {
"order": "asc",
},
expectError: false,
expectedResult: mockSearchResult,
expectedResult: expectedSearchResult,
},
{
name: "pull request search with only owner parameter (should ignore it)",
@@ -993,7 +1002,7 @@ func Test_SearchPullRequests(t *testing.T) {
"owner": "test-owner",
},
expectError: false,
expectedResult: mockSearchResult,
expectedResult: expectedSearchResult,
},
{
name: "pull request search with only repo parameter (should ignore it)",
@@ -1014,7 +1023,7 @@ func Test_SearchPullRequests(t *testing.T) {
"repo": "test-repo",
},
expectError: false,
expectedResult: mockSearchResult,
expectedResult: expectedSearchResult,
},
{
name: "pull request search with minimal parameters",
@@ -1025,7 +1034,7 @@ func Test_SearchPullRequests(t *testing.T) {
"query": "is:pr repo:owner/repo is:open",
},
expectError: false,
expectedResult: mockSearchResult,
expectedResult: expectedSearchResult,
},
{
name: "query with existing is:pr filter - no duplication",
@@ -1045,7 +1054,7 @@ func Test_SearchPullRequests(t *testing.T) {
"query": "is:pr repo:github/github-mcp-server is:open draft:false",
},
expectError: false,
expectedResult: mockSearchResult,
expectedResult: expectedSearchResult,
},
{
name: "query with existing repo: filter and conflicting owner/repo params - uses query filter",
@@ -1067,7 +1076,7 @@ func Test_SearchPullRequests(t *testing.T) {
"repo": "different-repo",
},
expectError: false,
expectedResult: mockSearchResult,
expectedResult: expectedSearchResult,
},
{
name: "complex query with existing is:pr filter and OR operators",
@@ -1087,7 +1096,7 @@ func Test_SearchPullRequests(t *testing.T) {
"query": "is:pr repo:github/github-mcp-server (label:bug OR label:enhancement OR label:feature)",
},
expectError: false,
expectedResult: mockSearchResult,
expectedResult: expectedSearchResult,
},
{
name: "search pull requests fails",
@@ -1146,10 +1155,22 @@ func Test_SearchPullRequests(t *testing.T) {
for i, issue := range returnedResult.Issues {
assert.Equal(t, *tc.expectedResult.Issues[i].Number, *issue.Number)
assert.Equal(t, *tc.expectedResult.Issues[i].Title, *issue.Title)
assert.Equal(t, *tc.expectedResult.Issues[i].Body, *issue.Body)
assert.Equal(t, *tc.expectedResult.Issues[i].State, *issue.State)
assert.Equal(t, *tc.expectedResult.Issues[i].HTMLURL, *issue.HTMLURL)
assert.Equal(t, *tc.expectedResult.Issues[i].User.Login, *issue.User.Login)
if len(tc.expectedResult.Issues[i].Labels) > 0 {
assert.Equal(t, tc.expectedResult.Issues[i].Labels[0].GetName(), issue.Labels[0].GetName())
assert.Equal(t, tc.expectedResult.Issues[i].Labels[0].GetDescription(), issue.Labels[0].GetDescription())
}
if tc.expectedResult.Issues[i].Milestone != nil {
assert.Equal(t, tc.expectedResult.Issues[i].Milestone.GetTitle(), issue.Milestone.GetTitle())
assert.Equal(t, tc.expectedResult.Issues[i].Milestone.GetDescription(), issue.Milestone.GetDescription())
}
}
assert.Equal(t, baselineUnsafeText, mockSearchResult.Issues[0].GetTitle())
assert.Equal(t, baselineUnsafeText, mockSearchResult.Issues[0].GetBody())
assert.Equal(t, baselineUnsafeText, mockSearchResult.Issues[0].Labels[0].GetName())
})
}
@@ -1401,9 +1422,9 @@ func Test_GetPullRequestCommits(t *testing.T) {
SHA: github.Ptr("abc123def456"),
HTMLURL: github.Ptr("https://github.com/owner/repo/commit/abc123def456"),
Commit: &github.Commit{
Message: github.Ptr("feat: add commit listing"),
Message: github.Ptr(baselineUnsafeText),
Author: &github.CommitAuthor{
Name: github.Ptr("Test User"),
Name: github.Ptr(baselineUnsafeText),
Email: github.Ptr("test@example.com"),
Date: &github.Timestamp{Time: authorDate},
},
@@ -1543,10 +1564,12 @@ func Test_GetPullRequestCommits(t *testing.T) {
for i, commit := range returnedCommits {
assert.Equal(t, tc.expectedCommits[i].GetSHA(), commit.SHA)
assert.Equal(t, tc.expectedCommits[i].GetHTMLURL(), commit.HTMLURL)
assert.Equal(t, tc.expectedCommits[i].GetCommit().GetMessage(), commit.Message)
assert.Equal(t, sanitizeCommitMessage(tc.expectedCommits[i].GetCommit().GetMessage()), commit.Message)
}
assert.Equal(t, sanitizeOutputText(baselineUnsafeText), returnedCommits[0].Author.Name)
assert.Equal(t, authorDate.Format(time.RFC3339), returnedCommits[0].Author.Date)
assert.Equal(t, baselineUnsafeText, mockCommits[0].GetCommit().GetMessage())
})
}
}
@@ -1590,8 +1613,8 @@ func Test_GetPullRequestStatus(t *testing.T) {
Statuses: []*github.RepoStatus{
{
State: github.Ptr("success"),
Context: github.Ptr("continuous-integration/travis-ci"),
Description: github.Ptr("Build succeeded"),
Context: github.Ptr("continuous-integration/<script>"),
Description: github.Ptr(baselineUnsafeText),
TargetURL: github.Ptr("https://travis-ci.org/owner/repo/builds/123"),
},
{
@@ -1712,8 +1735,9 @@ func Test_GetPullRequestStatus(t *testing.T) {
for i, status := range returnedStatus.Statuses {
assert.Equal(t, *tc.expectedStatus.Statuses[i].State, *status.State)
assert.Equal(t, *tc.expectedStatus.Statuses[i].Context, *status.Context)
assert.Equal(t, *tc.expectedStatus.Statuses[i].Description, *status.Description)
assert.Equal(t, sanitizeOutputText(*tc.expectedStatus.Statuses[i].Description), *status.Description)
}
assert.Equal(t, baselineUnsafeText, mockStatus.Statuses[0].GetDescription())
})
}
}
@@ -4138,7 +4162,7 @@ func TestAddReplyToPullRequestComment(t *testing.T) {
// Setup mock reply comment for success case
mockReplyComment := &github.PullRequestComment{
ID: github.Ptr(int64(456)),
Body: github.Ptr("This is a reply to the comment"),
Body: github.Ptr(baselineUnsafeText),
InReplyTo: github.Ptr(int64(123)),
HTMLURL: github.Ptr("https://github.com/owner/repo/pull/42#discussion_r456"),
User: &github.User{
@@ -4365,8 +4389,19 @@ func TestAddReplyToPullRequestComment(t *testing.T) {
// Parse the result and verify it's not an error
require.False(t, result.IsError)
textContent := getTextResult(t, result)
if _, ok := tc.requestArgs["body"]; ok {
assert.Contains(t, textContent.Text, "This is a reply to the comment")
if _, hasBody := tc.requestArgs["body"]; hasBody {
var returnedComment github.PullRequestComment
if _, hasReaction := tc.requestArgs["reaction"]; hasReaction {
var combinedResult struct {
Comment github.PullRequestComment `json:"comment"`
}
require.NoError(t, json.Unmarshal([]byte(textContent.Text), &combinedResult))
returnedComment = combinedResult.Comment
} else {
require.NoError(t, json.Unmarshal([]byte(textContent.Text), &returnedComment))
}
assert.Equal(t, sanitizeOutputText(baselineUnsafeText), returnedComment.GetBody())
assert.Equal(t, baselineUnsafeText, mockReplyComment.GetBody())
}
if _, ok := tc.requestArgs["reaction"]; ok {
assert.Contains(t, textContent.Text, "789")
+56 -76
View File
@@ -7,7 +7,6 @@ import (
"fmt"
"io"
"net/http"
"slices"
"strconv"
"strings"
@@ -235,6 +234,9 @@ func listCommitsTool(t translations.TranslationHelperFunc, includeFields bool) i
if err != nil {
return utils.NewToolResultError(err.Error()), nil, nil
}
if err := validateOptionalRepoRelativePath("path", path); err != nil {
return utils.NewToolResultError(err.Error()), nil, nil
}
var fields []string
if includeFields {
fields, err = OptionalStringArrayParam(args, "fields")
@@ -500,6 +502,9 @@ SHA MUST be provided for existing file updates.
if err != nil {
return utils.NewToolResultError(err.Error()), nil, nil
}
if err := validateRepoRelativePath("path", path); err != nil {
return utils.NewToolResultError(err.Error()), nil, nil
}
content, err := RequiredParam[string](args, "content")
if err != nil {
return utils.NewToolResultError(err.Error()), nil, nil
@@ -840,6 +845,10 @@ func getFileContentsTool(t translations.TranslationHelperFunc, includeFields boo
return utils.NewToolResultError(err.Error()), nil, nil
}
path = strings.TrimPrefix(path, "/")
if err := validateRepoRelativePathOrRoot("path", path); err != nil {
return utils.NewToolResultError(err.Error()), nil, nil
}
path = normalizeRepoRelativePathOrRoot(path)
ref, err := OptionalParam[string](args, "ref")
if err != nil {
@@ -1166,6 +1175,9 @@ func DeleteFile(t translations.TranslationHelperFunc) inventory.ServerTool {
if err != nil {
return utils.NewToolResultError(err.Error()), nil, nil
}
if err := validateRepoRelativePath("path", path); err != nil {
return utils.NewToolResultError(err.Error()), nil, nil
}
message, err := RequiredParam[string](args, "message")
if err != nil {
return utils.NewToolResultError(err.Error()), nil, nil
@@ -1284,7 +1296,7 @@ func DeleteFile(t translations.TranslationHelperFunc) inventory.ServerTool {
// Create a response similar to what the DeleteFile API would return
response := map[string]any{
"commit": newCommit,
"commit": sanitizedCommitCopy(newCommit),
"content": nil,
}
@@ -1489,6 +1501,34 @@ func PushFiles(t translations.TranslationHelperFunc) inventory.ServerTool {
return utils.NewToolResultError("files parameter must be an array of objects with path and content"), nil, nil
}
entries := make([]*github.TreeEntry, 0, len(filesObj))
for _, file := range filesObj {
fileMap, ok := file.(map[string]any)
if !ok {
return utils.NewToolResultError("each file must be an object with path and content"), nil, nil
}
path, ok := fileMap["path"].(string)
if !ok || path == "" {
return utils.NewToolResultError("each file must have a path"), nil, nil
}
if err := validateRepoRelativePath("path", path); err != nil {
return utils.NewToolResultError(err.Error()), nil, nil
}
content, ok := fileMap["content"].(string)
if !ok {
return utils.NewToolResultError("each file must have content"), nil, nil
}
entries = append(entries, &github.TreeEntry{
Path: github.Ptr(path),
Mode: github.Ptr("100644"),
Type: github.Ptr("blob"),
Content: github.Ptr(content),
})
}
client, err := deps.GetClient(ctx)
if err != nil {
return nil, nil, fmt.Errorf("failed to get GitHub client: %w", err)
@@ -1562,34 +1602,6 @@ func PushFiles(t translations.TranslationHelperFunc) inventory.ServerTool {
baseCommit = base
}
// Create tree entries for all files (or remaining files if empty repo)
var entries []*github.TreeEntry
for _, file := range filesObj {
fileMap, ok := file.(map[string]any)
if !ok {
return utils.NewToolResultError("each file must be an object with path and content"), nil, nil
}
path, ok := fileMap["path"].(string)
if !ok || path == "" {
return utils.NewToolResultError("each file must have a path"), nil, nil
}
content, ok := fileMap["content"].(string)
if !ok {
return utils.NewToolResultError("each file must have content"), nil, nil
}
// Create a tree entry for the file
entries = append(entries, &github.TreeEntry{
Path: github.Ptr(path),
Mode: github.Ptr("100644"), // Regular file mode
Type: github.Ptr("blob"),
Content: github.Ptr(content),
})
}
// Create a new tree with the file entries (baseCommit is now guaranteed to exist)
newTree, resp, err := client.Git.CreateTree(ctx, owner, repo, *baseCommit.Tree.SHA, entries)
if err != nil {
@@ -2062,7 +2074,7 @@ func GetLatestRelease(t translations.TranslationHelperFunc) inventory.ServerTool
return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to get latest release", resp, body), nil, nil
}
r, err := json.Marshal(release)
r, err := json.Marshal(sanitizedReleaseCopy(release))
if err != nil {
return nil, nil, fmt.Errorf("failed to marshal response: %w", err)
}
@@ -2148,7 +2160,7 @@ func GetReleaseByTag(t translations.TranslationHelperFunc) inventory.ServerTool
return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to get release by tag", resp, body), nil, nil
}
r, err := json.Marshal(release)
r, err := json.Marshal(sanitizedReleaseCopy(release))
if err != nil {
return nil, nil, fmt.Errorf("failed to marshal response: %w", err)
}
@@ -2208,10 +2220,16 @@ func ListStarredRepositories(t translations.TranslationHelperFunc) inventory.Ser
if err != nil {
return utils.NewToolResultError(err.Error()), nil, nil
}
if err := validateEnumParam("sort", sort, "created", "updated"); err != nil {
return utils.NewToolResultError(err.Error()), nil, nil
}
direction, err := OptionalParam[string](args, "direction")
if err != nil {
return utils.NewToolResultError(err.Error()), nil, nil
}
if err := validateEnumParam("direction", direction, "asc", "desc"); err != nil {
return utils.NewToolResultError(err.Error()), nil, nil
}
pagination, err := OptionalPaginationParams(args)
if err != nil {
return utils.NewToolResultError(err.Error()), nil, nil
@@ -2265,28 +2283,10 @@ func ListStarredRepositories(t translations.TranslationHelperFunc) inventory.Ser
// Convert to minimal format
minimalRepos := make([]MinimalRepository, 0, len(repos))
for _, starredRepo := range repos {
repo := starredRepo.Repository
minimalRepo := MinimalRepository{
ID: repo.GetID(),
Name: repo.GetName(),
FullName: repo.GetFullName(),
Description: repo.GetDescription(),
HTMLURL: repo.GetHTMLURL(),
Language: repo.GetLanguage(),
Stars: repo.GetStargazersCount(),
Forks: repo.GetForksCount(),
OpenIssues: repo.GetOpenIssuesCount(),
Private: repo.GetPrivate(),
Fork: repo.GetFork(),
Archived: repo.GetArchived(),
DefaultBranch: repo.GetDefaultBranch(),
if starredRepo == nil || starredRepo.Repository == nil {
continue
}
if repo.UpdatedAt != nil {
minimalRepo.UpdatedAt = repo.UpdatedAt.Format("2006-01-02T15:04:05Z")
}
minimalRepos = append(minimalRepos, minimalRepo)
minimalRepos = append(minimalRepos, convertToMinimalRepository(starredRepo.Repository))
}
r, err := json.Marshal(minimalRepos)
@@ -2541,26 +2541,6 @@ type blameCommitFragment struct {
} `graphql:"blame(path: $path)"`
}
// validateBlamePath rejects empty, leading-slash, traversal-laden, or
// control-character paths before any network call is made.
func validateBlamePath(p string) error {
if strings.TrimSpace(p) == "" {
return fmt.Errorf("path must not be empty")
}
if strings.HasPrefix(p, "/") {
return fmt.Errorf("path must be relative to the repository root (no leading '/')")
}
if slices.Contains(strings.Split(p, "/"), "..") {
return fmt.Errorf("path must not contain '..' segments")
}
for _, r := range p {
if r < 0x20 || r == 0x7f {
return fmt.Errorf("path must not contain control characters")
}
}
return nil
}
func GetFileBlame(t translations.TranslationHelperFunc) inventory.ServerTool {
st := NewTool(
ToolsetMetadataRepos,
@@ -2624,7 +2604,7 @@ func GetFileBlame(t translations.TranslationHelperFunc) inventory.ServerTool {
if err != nil {
return utils.NewToolResultError(err.Error()), nil, nil
}
if err := validateBlamePath(path); err != nil {
if err := validateRepoRelativePath("path", path); err != nil {
return utils.NewToolResultError(err.Error()), nil, nil
}
ref, err := OptionalParam[string](args, "ref")
@@ -2805,13 +2785,13 @@ func GetFileBlame(t translations.TranslationHelperFunc) inventory.ServerTool {
if idx := strings.IndexByte(headline, '\n'); idx >= 0 {
headline = headline[:idx]
}
headline = strings.TrimRight(headline, " \t\r")
headline = sanitizeOutputText(strings.TrimRight(headline, " \t\r"))
bc := BlameCommit{
SHA: sha,
MessageHeadline: headline,
CommittedDate: r.Commit.CommittedDate.Format("2006-01-02T15:04:05Z"),
Author: BlameAuthor{
Name: string(r.Commit.Author.Name),
Name: sanitizeOutputText(string(r.Commit.Author.Name)),
Email: string(r.Commit.Author.Email),
},
}
+49 -18
View File
@@ -1174,9 +1174,9 @@ func Test_GetCommit(t *testing.T) {
mockCommit := &github.RepositoryCommit{
SHA: github.Ptr("abc123def456"),
Commit: &github.Commit{
Message: github.Ptr("First commit"),
Message: github.Ptr(baselineUnsafeText),
Author: &github.CommitAuthor{
Name: github.Ptr("Test User"),
Name: github.Ptr(baselineUnsafeText),
Email: github.Ptr("test@example.com"),
Date: &github.Timestamp{Time: time.Now().Add(-48 * time.Hour)},
},
@@ -1277,9 +1277,11 @@ func Test_GetCommit(t *testing.T) {
require.NoError(t, err)
assert.Equal(t, *tc.expectedCommit.SHA, *returnedCommit.SHA)
assert.Equal(t, *tc.expectedCommit.Commit.Message, *returnedCommit.Commit.Message)
assert.Equal(t, sanitizeCommitMessage(tc.expectedCommit.Commit.GetMessage()), returnedCommit.Commit.GetMessage())
assert.Equal(t, sanitizeOutputText(tc.expectedCommit.Commit.Author.GetName()), returnedCommit.Commit.Author.GetName())
assert.Equal(t, *tc.expectedCommit.Author.Login, *returnedCommit.Author.Login)
assert.Equal(t, *tc.expectedCommit.HTMLURL, *returnedCommit.HTMLURL)
assert.Equal(t, baselineUnsafeText, mockCommit.Commit.GetMessage())
})
}
}
@@ -1430,9 +1432,9 @@ func Test_ListCommits(t *testing.T) {
{
SHA: github.Ptr("abc123def456"),
Commit: &github.Commit{
Message: github.Ptr("First commit"),
Message: github.Ptr(baselineUnsafeText),
Author: &github.CommitAuthor{
Name: github.Ptr("Test User"),
Name: github.Ptr(baselineUnsafeText),
Email: github.Ptr("test@example.com"),
Date: &github.Timestamp{Time: time.Now().Add(-48 * time.Hour)},
},
@@ -1690,7 +1692,10 @@ func Test_ListCommits(t *testing.T) {
assert.Equal(t, tc.expectedCommits[i].GetSHA(), commit.SHA)
assert.Equal(t, tc.expectedCommits[i].GetHTMLURL(), commit.HTMLURL)
if tc.expectedCommits[i].Commit != nil {
assert.Equal(t, tc.expectedCommits[i].Commit.GetMessage(), commit.Commit.Message)
assert.Equal(t, sanitizeCommitMessage(tc.expectedCommits[i].Commit.GetMessage()), commit.Commit.Message)
if tc.expectedCommits[i].Commit.Author != nil {
assert.Equal(t, sanitizeOutputText(tc.expectedCommits[i].Commit.Author.GetName()), commit.Commit.Author.Name)
}
}
if tc.expectedCommits[i].Author != nil {
assert.Equal(t, tc.expectedCommits[i].Author.GetLogin(), commit.Author.Login)
@@ -1736,9 +1741,9 @@ func Test_CreateOrUpdateFile(t *testing.T) {
},
Commit: github.Commit{
SHA: github.Ptr("def456abc789"),
Message: github.Ptr("Add example file"),
Message: github.Ptr(baselineUnsafeText),
Author: &github.CommitAuthor{
Name: github.Ptr("Test User"),
Name: github.Ptr(baselineUnsafeText),
Email: github.Ptr("test@example.com"),
Date: &github.Timestamp{Time: time.Now()},
},
@@ -2058,12 +2063,12 @@ func Test_CreateOrUpdateFile(t *testing.T) {
// Verify commit
assert.Equal(t, tc.expectedContent.Commit.GetSHA(), returnedContent.Commit.SHA)
assert.Equal(t, tc.expectedContent.Commit.GetMessage(), returnedContent.Commit.Message)
assert.Equal(t, sanitizeCommitMessage(tc.expectedContent.Commit.GetMessage()), returnedContent.Commit.Message)
assert.Equal(t, tc.expectedContent.Commit.GetHTMLURL(), returnedContent.Commit.HTMLURL)
// Verify commit author
require.NotNil(t, returnedContent.Commit.Author)
assert.Equal(t, tc.expectedContent.Commit.Author.GetName(), returnedContent.Commit.Author.Name)
assert.Equal(t, sanitizeOutputText(tc.expectedContent.Commit.Author.GetName()), returnedContent.Commit.Author.Name)
assert.Equal(t, tc.expectedContent.Commit.Author.GetEmail(), returnedContent.Commit.Author.Email)
assert.NotEmpty(t, returnedContent.Commit.Author.Date)
})
@@ -3164,7 +3169,7 @@ func Test_DeleteFile(t *testing.T) {
mockNewCommit := &github.Commit{
SHA: github.Ptr("jkl012"),
Message: github.Ptr("Delete example file"),
Message: github.Ptr(baselineUnsafeText),
HTMLURL: github.Ptr("https://github.com/owner/repo/commit/jkl012"),
}
@@ -3304,6 +3309,8 @@ func Test_DeleteFile(t *testing.T) {
commitSHA, ok := commit["sha"].(string)
require.True(t, ok)
assert.Equal(t, tc.expectedCommitSHA, commitSHA)
assert.Equal(t, sanitizeOutputText(baselineUnsafeText), commit["message"])
assert.Equal(t, baselineUnsafeText, mockNewCommit.GetMessage())
})
}
}
@@ -4556,7 +4563,7 @@ func Test_ListStarredRepositories(t *testing.T) {
ID: github.Ptr(int64(12345)),
Name: github.Ptr("awesome-repo"),
FullName: github.Ptr("owner/awesome-repo"),
Description: github.Ptr("An awesome repository"),
Description: github.Ptr(baselineUnsafeText),
HTMLURL: github.Ptr("https://github.com/owner/awesome-repo"),
Language: github.Ptr("Go"),
StargazersCount: github.Ptr(100),
@@ -4630,6 +4637,24 @@ func Test_ListStarredRepositories(t *testing.T) {
expectError: false,
expectedCount: 2,
},
{
name: "invalid sort is rejected before request",
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{}),
requestArgs: map[string]any{
"sort": "pushed",
},
expectError: true,
expectedErrMsg: "sort must be one of: created, updated",
},
{
name: "invalid direction is rejected before request",
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{}),
requestArgs: map[string]any{
"direction": "sideways",
},
expectError: true,
expectedErrMsg: "direction must be one of: asc, desc",
},
{
name: "list fails",
mockedClient: NewMockedHTTPClient(
@@ -4664,7 +4689,9 @@ func Test_ListStarredRepositories(t *testing.T) {
// Verify results
if tc.expectError {
require.NoError(t, err)
require.NotNil(t, result)
require.True(t, result.IsError)
textResult, ok := result.Content[0].(*mcp.TextContent)
require.True(t, ok, "Expected text content")
assert.Contains(t, textResult.Text, tc.expectedErrMsg)
@@ -4684,6 +4711,8 @@ func Test_ListStarredRepositories(t *testing.T) {
if tc.expectedCount > 0 {
assert.Equal(t, "awesome-repo", returnedRepos[0].Name)
assert.Equal(t, "owner/awesome-repo", returnedRepos[0].FullName)
assert.Equal(t, sanitizeOutputText(baselineUnsafeText), returnedRepos[0].Description)
assert.Equal(t, baselineUnsafeText, mockStarredRepos[0].Repository.GetDescription())
}
}
})
@@ -4948,10 +4977,10 @@ func Test_GetFileBlame(t *testing.T) {
"startingLine": 1, "endingLine": 5, "age": 2,
"commit": map[string]any{
"oid": "abc123def456",
"message": "Initial commit\n\nLong body that should not appear in the response.",
"message": baselineUnsafeText + "\nLong body that should not appear in the response.",
"committedDate": "2024-01-01T12:00:00Z",
"author": map[string]any{
"name": "John Doe", "email": "john@example.com",
"name": baselineUnsafeText, "email": "john@example.com",
"user": map[string]any{"login": "johndoe", "url": "https://github.com/johndoe"},
},
},
@@ -4961,10 +4990,10 @@ func Test_GetFileBlame(t *testing.T) {
"startingLine": 6, "endingLine": 7, "age": 2,
"commit": map[string]any{
"oid": "abc123def456",
"message": "Initial commit\n\nLong body that should not appear in the response.",
"message": baselineUnsafeText + "\nLong body that should not appear in the response.",
"committedDate": "2024-01-01T12:00:00Z",
"author": map[string]any{
"name": "John Doe", "email": "john@example.com",
"name": baselineUnsafeText, "email": "john@example.com",
"user": map[string]any{"login": "johndoe", "url": "https://github.com/johndoe"},
},
},
@@ -5011,7 +5040,9 @@ func Test_GetFileBlame(t *testing.T) {
require.Contains(t, br.Commits, "abc123def456")
require.Contains(t, br.Commits, "def456ghi789")
// Multi-line message must be reduced to its headline.
assert.Equal(t, "Initial commit", br.Commits["abc123def456"].MessageHeadline)
assert.Equal(t, sanitizeOutputText(strings.SplitN(baselineUnsafeText, "\n", 2)[0]), br.Commits["abc123def456"].MessageHeadline)
assert.Equal(t, sanitizeOutputText(baselineUnsafeText), br.Commits["abc123def456"].Author.Name)
assert.Equal(t, "john@example.com", br.Commits["abc123def456"].Author.Email)
assert.NotContains(t, result, "Long body that should not appear")
// Login/URL pointers populated.
require.NotNil(t, br.Commits["abc123def456"].Author.Login)
@@ -5408,7 +5439,7 @@ func Test_GetFileBlame(t *testing.T) {
path string
want string
}{
{"empty", " ", "must not be empty"},
{"empty", "", "missing required parameter: path"},
{"absolute", "/etc/passwd", "must be relative"},
{"traversal", "src/../../../etc/passwd", "must not contain '..'"},
{"control char", "src/\x00bad.go", "control characters"},
+924
View File
@@ -0,0 +1,924 @@
package github
import (
"context"
"encoding/base64"
"encoding/json"
"net/http"
"net/http/httptest"
"net/url"
"strings"
"testing"
"time"
"github.com/github/github-mcp-server/internal/githubv4mock"
"github.com/github/github-mcp-server/pkg/sanitize"
"github.com/github/github-mcp-server/pkg/translations"
gogithub "github.com/google/go-github/v89/github"
"github.com/modelcontextprotocol/go-sdk/mcp"
"github.com/shurcooL/githubv4"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
const baselineUnsafeText = "<script>alert(1)</script>keep\u200B ```go onclick=alert(1)\nfmt.Println(\"x\")\n```"
func TestSanitizationIssueOutputPaths(t *testing.T) {
t.Run("rest get issue sanitizes title and body", func(t *testing.T) {
mockIssue := &gogithub.Issue{
Number: gogithub.Ptr(42),
Title: gogithub.Ptr(baselineUnsafeText),
Body: gogithub.Ptr(baselineUnsafeText),
State: gogithub.Ptr("open"),
HTMLURL: gogithub.Ptr("https://github.com/owner/repo/issues/42"),
User: &gogithub.User{Login: gogithub.Ptr("author")},
}
client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
GetReposIssuesByOwnerByRepoByIssueNumber: mockResponse(t, http.StatusOK, mockIssue),
}))
deps := BaseDeps{
Client: client,
GQLClient: defaultGQLClient,
RepoAccessCache: stubRepoAccessCache(nil, 15*time.Minute),
Flags: stubFeatureFlags(map[string]bool{"lockdown-mode": false}),
}
serverTool := IssueRead(translations.NullTranslationHelper)
handler := serverTool.Handler(deps)
result, err := handler(ContextWithDeps(context.Background(), deps), &mcp.CallToolRequest{
Params: createMCPRequest(map[string]any{
"method": "get",
"owner": "owner",
"repo": "repo",
"issue_number": float64(42),
}).Params,
})
require.NoError(t, err)
require.False(t, result.IsError)
var issue MinimalIssue
require.NoError(t, json.Unmarshal([]byte(getTextResult(t, result).Text), &issue))
assert.Equal(t, sanitize.Sanitize(baselineUnsafeText), issue.Title)
assert.Equal(t, sanitize.Sanitize(baselineUnsafeText), issue.Body)
assert.Equal(t, baselineUnsafeText, mockIssue.GetTitle())
assert.Equal(t, baselineUnsafeText, mockIssue.GetBody())
})
t.Run("rest search issue sanitizes embedded display fields without mutating source object", func(t *testing.T) {
sourceIssue := &gogithub.Issue{
Number: gogithub.Ptr(42),
Title: gogithub.Ptr(baselineUnsafeText),
Body: gogithub.Ptr(baselineUnsafeText),
State: gogithub.Ptr("open"),
Labels: []*gogithub.Label{{
Name: gogithub.Ptr(baselineUnsafeText),
Description: gogithub.Ptr(baselineUnsafeText),
}},
Milestone: &gogithub.Milestone{
Title: gogithub.Ptr(baselineUnsafeText),
Description: gogithub.Ptr(baselineUnsafeText),
},
}
searchHit := SearchIssueResult{Issue: sourceIssue}
raw, err := json.Marshal(searchHit)
require.NoError(t, err)
var issue map[string]any
require.NoError(t, json.Unmarshal(raw, &issue))
assert.Equal(t, sanitize.Sanitize(baselineUnsafeText), issue["title"])
assert.Equal(t, sanitize.Sanitize(baselineUnsafeText), issue["body"])
labels := issue["labels"].([]any)
label := labels[0].(map[string]any)
assert.Equal(t, sanitize.Sanitize(baselineUnsafeText), label["name"])
assert.Equal(t, sanitize.Sanitize(baselineUnsafeText), label["description"])
milestone := issue["milestone"].(map[string]any)
assert.Equal(t, sanitize.Sanitize(baselineUnsafeText), milestone["title"])
assert.Equal(t, sanitize.Sanitize(baselineUnsafeText), milestone["description"])
assert.Equal(t, baselineUnsafeText, sourceIssue.GetTitle())
assert.Equal(t, baselineUnsafeText, sourceIssue.GetBody())
assert.Equal(t, baselineUnsafeText, sourceIssue.Labels[0].GetName())
assert.Equal(t, baselineUnsafeText, sourceIssue.Milestone.GetTitle())
})
t.Run("graphql issue fragment conversion sanitizes title and body", func(t *testing.T) {
now := githubv4.DateTime{Time: time.Date(2026, 7, 3, 9, 0, 0, 0, time.UTC)}
fragment := IssueFragment{
Number: githubv4.Int(42),
Title: githubv4.String(baselineUnsafeText),
Body: githubv4.String(baselineUnsafeText),
State: githubv4.String("OPEN"),
CreatedAt: now,
UpdatedAt: now,
}
fragment.Author.Login = githubv4.String("author")
fragment.Labels.Nodes = append(fragment.Labels.Nodes, struct {
Name githubv4.String
ID githubv4.String
Description githubv4.String
}{Name: githubv4.String(baselineUnsafeText)})
issue := fragmentToMinimalIssue(fragment)
assert.Equal(t, sanitize.Sanitize(baselineUnsafeText), issue.Title)
assert.Equal(t, sanitize.Sanitize(baselineUnsafeText), issue.Body)
assert.Equal(t, []string{sanitize.Sanitize(baselineUnsafeText)}, issue.Labels)
})
}
func TestSanitizationPullRequestOutputPaths(t *testing.T) {
t.Run("rest get pull request sanitizes title and body without mutating source object", func(t *testing.T) {
mockPR := &gogithub.PullRequest{
Number: gogithub.Ptr(42),
Title: gogithub.Ptr(baselineUnsafeText),
Body: gogithub.Ptr(baselineUnsafeText),
State: gogithub.Ptr("open"),
HTMLURL: gogithub.Ptr("https://github.com/owner/repo/pull/42"),
User: &gogithub.User{Login: gogithub.Ptr("author")},
Labels: []*gogithub.Label{{
Name: gogithub.Ptr(baselineUnsafeText),
}},
Milestone: &gogithub.Milestone{
Title: gogithub.Ptr(baselineUnsafeText),
},
Head: &gogithub.PullRequestBranch{
Ref: gogithub.Ptr("feature"),
SHA: gogithub.Ptr("abc123"),
Repo: &gogithub.Repository{
FullName: gogithub.Ptr("owner/repo"),
Description: gogithub.Ptr(baselineUnsafeText),
},
},
}
client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
GetReposPullsByOwnerByRepoByPullNumber: mockResponse(t, http.StatusOK, mockPR),
}))
deps := BaseDeps{
Client: client,
GQLClient: githubv4.NewClient(githubv4mock.NewMockedHTTPClient()),
RepoAccessCache: stubRepoAccessCache(nil, 5*time.Minute),
Flags: stubFeatureFlags(map[string]bool{"lockdown-mode": false}),
}
serverTool := PullRequestRead(translations.NullTranslationHelper)
handler := serverTool.Handler(deps)
result, err := handler(ContextWithDeps(context.Background(), deps), &mcp.CallToolRequest{
Params: createMCPRequest(map[string]any{
"method": "get",
"owner": "owner",
"repo": "repo",
"pullNumber": float64(42),
}).Params,
})
require.NoError(t, err)
require.False(t, result.IsError)
var pr MinimalPullRequest
require.NoError(t, json.Unmarshal([]byte(getTextResult(t, result).Text), &pr))
assert.Equal(t, sanitize.Sanitize(baselineUnsafeText), pr.Title)
assert.Equal(t, sanitize.Sanitize(baselineUnsafeText), pr.Body)
assert.Equal(t, []string{sanitize.Sanitize(baselineUnsafeText)}, pr.Labels)
assert.Equal(t, sanitize.Sanitize(baselineUnsafeText), pr.Milestone)
require.NotNil(t, pr.Head)
require.NotNil(t, pr.Head.Repo)
assert.Equal(t, sanitize.Sanitize(baselineUnsafeText), pr.Head.Repo.Description)
assert.Equal(t, baselineUnsafeText, mockPR.GetTitle())
assert.Equal(t, baselineUnsafeText, mockPR.GetBody())
assert.Equal(t, baselineUnsafeText, mockPR.Labels[0].GetName())
assert.Equal(t, baselineUnsafeText, mockPR.Milestone.GetTitle())
assert.Equal(t, baselineUnsafeText, mockPR.Head.Repo.GetDescription())
})
t.Run("rest list pull requests sanitizes title and body without mutating source objects", func(t *testing.T) {
mockPRs := []*gogithub.PullRequest{
{
Number: gogithub.Ptr(42),
Title: gogithub.Ptr(baselineUnsafeText),
Body: gogithub.Ptr(baselineUnsafeText),
State: gogithub.Ptr("open"),
HTMLURL: gogithub.Ptr("https://github.com/owner/repo/pull/42"),
},
{
Number: gogithub.Ptr(43),
Title: gogithub.Ptr("safe title"),
Body: gogithub.Ptr("safe body"),
State: gogithub.Ptr("closed"),
HTMLURL: gogithub.Ptr("https://github.com/owner/repo/pull/43"),
},
}
client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
GetReposPullsByOwnerByRepo: expectQueryParams(t, map[string]string{
"state": "all",
"sort": "created",
"direction": "desc",
"per_page": "30",
"page": "1",
}).andThen(mockResponse(t, http.StatusOK, mockPRs)),
}))
deps := BaseDeps{Client: client}
serverTool := ListPullRequests(translations.NullTranslationHelper)
handler := serverTool.Handler(deps)
result, err := handler(ContextWithDeps(context.Background(), deps), &mcp.CallToolRequest{
Params: createMCPRequest(map[string]any{
"owner": "owner",
"repo": "repo",
"state": "all",
"sort": "created",
"direction": "desc",
"perPage": float64(30),
"page": float64(1),
}).Params,
})
require.NoError(t, err)
require.False(t, result.IsError)
var prs []MinimalPullRequest
require.NoError(t, json.Unmarshal([]byte(getTextResult(t, result).Text), &prs))
require.Len(t, prs, 2)
assert.Equal(t, sanitize.Sanitize(baselineUnsafeText), prs[0].Title)
assert.Equal(t, sanitize.Sanitize(baselineUnsafeText), prs[0].Body)
assert.Equal(t, "safe title", prs[1].Title)
assert.Equal(t, "safe body", prs[1].Body)
assert.Equal(t, baselineUnsafeText, mockPRs[0].GetTitle())
assert.Equal(t, baselineUnsafeText, mockPRs[0].GetBody())
})
}
func TestSanitizationCollaborationTextPaths(t *testing.T) {
sanitizedUnsafeText := sanitize.Sanitize(baselineUnsafeText)
comment := convertToMinimalIssueComment(&gogithub.IssueComment{
ID: gogithub.Ptr(int64(1)),
Body: gogithub.Ptr(baselineUnsafeText),
HTMLURL: gogithub.Ptr("https://github.com/owner/repo/issues/1#issuecomment-1"),
})
assert.Equal(t, sanitizedUnsafeText, comment.Body)
review := convertToMinimalPullRequestReview(&gogithub.PullRequestReview{
ID: gogithub.Ptr(int64(2)),
State: gogithub.Ptr("COMMENTED"),
Body: gogithub.Ptr(baselineUnsafeText),
HTMLURL: gogithub.Ptr("https://github.com/owner/repo/pull/1#pullrequestreview-2"),
})
assert.Equal(t, sanitizedUnsafeText, review.Body)
reviewURL, err := url.Parse("https://github.com/owner/repo/pull/1#discussion_r1")
require.NoError(t, err)
reviewComment := convertToMinimalReviewComment(reviewCommentNode{
Body: baselineUnsafeText,
Path: "README.md",
URL: githubv4.URI{URL: reviewURL},
})
assert.Equal(t, sanitizedUnsafeText, reviewComment.Body)
assert.Equal(t, "README.md", reviewComment.Path)
discussion := fragmentToDiscussion(NodeFragment{
Number: githubv4.Int(1),
Title: githubv4.String(baselineUnsafeText),
URL: githubv4.String("https://github.com/owner/repo/discussions/1"),
CreatedAt: githubv4.DateTime{Time: time.Date(2026, 7, 3, 9, 0, 0, 0, time.UTC)},
UpdatedAt: githubv4.DateTime{Time: time.Date(2026, 7, 3, 9, 0, 0, 0, time.UTC)},
})
assert.Equal(t, sanitizedUnsafeText, discussion.GetTitle())
detail := discussionDetailFragment{
Number: githubv4.Int(1),
Title: githubv4.String(baselineUnsafeText),
Body: githubv4.String(baselineUnsafeText),
URL: githubv4.String("https://github.com/owner/repo/discussions/1"),
CreatedAt: githubv4.DateTime{Time: time.Date(2026, 7, 3, 9, 0, 0, 0, time.UTC)},
}
detail.Category.Name = githubv4.String(baselineUnsafeText)
detailResponse := discussionDetailResponse(detail)
assert.Equal(t, sanitizedUnsafeText, detailResponse["title"])
assert.Equal(t, sanitizedUnsafeText, detailResponse["body"])
assert.Equal(t, sanitizedUnsafeText, detailResponse["category"].(map[string]any)["name"])
discussionComment := convertToMinimalDiscussionComment(githubv4.ID("DC_1"), githubv4.String(baselineUnsafeText), githubv4.Boolean(true))
assert.Equal(t, sanitizedUnsafeText, discussionComment.Body)
}
func TestSanitizationProjectDisplayTextPaths(t *testing.T) {
sanitizedUnsafeText := sanitize.Sanitize(baselineUnsafeText)
project := convertToMinimalProject(&gogithub.ProjectV2{
Title: gogithub.Ptr(baselineUnsafeText),
Description: gogithub.Ptr(baselineUnsafeText),
ShortDescription: gogithub.Ptr(baselineUnsafeText),
})
require.NotNil(t, project)
require.NotNil(t, project.Title)
require.NotNil(t, project.Description)
require.NotNil(t, project.ShortDescription)
assert.Equal(t, sanitizedUnsafeText, *project.Title)
assert.Equal(t, sanitizedUnsafeText, *project.Description)
assert.Equal(t, sanitizedUnsafeText, *project.ShortDescription)
content := convertIssueToMinimalProjectItemContent(&gogithub.Issue{
Number: gogithub.Ptr(42),
Title: gogithub.Ptr(baselineUnsafeText),
State: gogithub.Ptr("open"),
Labels: []*gogithub.Label{{Name: gogithub.Ptr(baselineUnsafeText)}},
})
require.NotNil(t, content)
assert.Equal(t, sanitizedUnsafeText, content.Title)
assert.Equal(t, []string{sanitizedUnsafeText}, content.Labels)
prContent := convertPullRequestToMinimalProjectItemContent(&gogithub.PullRequest{
Number: gogithub.Ptr(43),
Title: gogithub.Ptr(baselineUnsafeText),
State: gogithub.Ptr("open"),
Labels: []*gogithub.Label{{Name: gogithub.Ptr(baselineUnsafeText)}},
})
require.NotNil(t, prContent)
assert.Equal(t, sanitizedUnsafeText, prContent.Title)
assert.Equal(t, []string{sanitizedUnsafeText}, prContent.Labels)
draftIssueContent := convertDraftIssueToMinimalProjectItemContent(&gogithub.ProjectV2DraftIssue{
Title: gogithub.Ptr(baselineUnsafeText),
})
require.NotNil(t, draftIssueContent)
assert.Equal(t, sanitizedUnsafeText, draftIssueContent.Title)
fields := convertToMinimalProjectItemFields([]*gogithub.ProjectV2ItemFieldValue{
{
ID: gogithub.Ptr(int64(1)),
Name: gogithub.Ptr(baselineUnsafeText),
DataType: gogithub.Ptr("text"),
Value: baselineUnsafeText,
},
})
require.Len(t, fields, 1)
assert.Equal(t, sanitizedUnsafeText, fields[0].Name)
assert.Equal(t, sanitizedUnsafeText, fields[0].Value)
option := minimalProjectFieldValue(&gogithub.ProjectV2FieldOption{
ID: gogithub.Ptr("option-id"),
Name: &gogithub.ProjectV2TextContent{Raw: gogithub.Ptr(baselineUnsafeText)},
})
assert.Equal(t, minimalProjectOptionValue{ID: "option-id", Name: sanitizedUnsafeText}, option)
body := githubv4.String(baselineUnsafeText)
status := githubv4.String("ON_TRACK")
statusUpdate := convertToMinimalStatusUpdate(statusUpdateNode{
ID: githubv4.ID("SU_1"),
Body: &body,
Status: &status,
CreatedAt: githubv4.DateTime{Time: time.Date(2026, 7, 3, 9, 0, 0, 0, time.UTC)},
})
assert.Equal(t, sanitizedUnsafeText, statusUpdate.Body)
assert.Equal(t, "ON_TRACK", statusUpdate.Status)
}
func TestSanitizeCommitMessage(t *testing.T) {
t.Run("preserves valid trailer addresses while sanitizing the message", func(t *testing.T) {
message := "<script>alert(1)</script>Subject\n\n" +
"Co-authored-by: Copilot <copilot@github.com>\n" +
"Signed-off-by: \"Copilot App\" <223556219+Copilot@users.noreply.github.com>"
assert.Equal(t, "Subject\n\n"+
"Co-authored-by: Copilot <copilot@github.com>\n"+
"Signed-off-by: &#34;Copilot App&#34; <223556219+Copilot@users.noreply.github.com>",
sanitizeCommitMessage(message))
})
t.Run("does not restore address-like text outside a trailer", func(t *testing.T) {
message := "Contact Copilot <copilot@github.com>"
assert.Equal(t, sanitizeOutputText(message), sanitizeCommitMessage(message))
})
t.Run("does not restore a trailer address with trailing HTML", func(t *testing.T) {
message := "Co-authored-by: Copilot <copilot@github.com><script>alert(1)</script>"
assert.Equal(t, sanitizeOutputText(message), sanitizeCommitMessage(message))
})
t.Run("does not restore HTML smuggled through address syntax", func(t *testing.T) {
displayNameHTML := `Co-authored-by: "><img src=x onerror=alert(1)>" <copilot@github.com>`
sanitized := sanitizeCommitMessage(displayNameHTML)
assert.NotContains(t, sanitized, "onerror")
assert.NotContains(t, sanitized, "alert(1)")
assert.Contains(t, sanitized, "<copilot@github.com>")
addressHTML := `Co-authored-by: Copilot <"<script>alert(1)</script>"@example.com>`
assert.Equal(t, sanitizeOutputText(addressHTML), sanitizeCommitMessage(addressHTML))
})
t.Run("avoids collisions with placeholder-like message text", func(t *testing.T) {
message := "GITHUBMCPCOMMITTRAILEREMAIL0PLACEHOLDER\n\nCo-authored-by: Copilot <copilot@github.com>"
assert.Equal(t, message, sanitizeCommitMessage(message))
})
}
func TestSanitizationOutputBypassesAndOutliers(t *testing.T) {
sanitizedUnsafeText := sanitize.Sanitize(baselineUnsafeText)
repo := &gogithub.Repository{
ID: gogithub.Ptr(int64(1)),
Name: gogithub.Ptr("repo"),
FullName: gogithub.Ptr("owner/repo"),
Description: gogithub.Ptr(baselineUnsafeText),
HTMLURL: gogithub.Ptr("https://github.com/owner/repo"),
}
minimalRepo := convertToMinimalRepository(repo)
assert.Equal(t, sanitizedUnsafeText, minimalRepo.Description)
assert.Equal(t, baselineUnsafeText, repo.GetDescription())
searchResult := sanitizedRepositoriesSearchResultCopy(&gogithub.RepositoriesSearchResult{
Repositories: []*gogithub.Repository{repo},
})
require.Len(t, searchResult.Repositories, 1)
assert.Equal(t, sanitizedUnsafeText, searchResult.Repositories[0].GetDescription())
assert.Equal(t, baselineUnsafeText, repo.GetDescription())
release := &gogithub.RepositoryRelease{
Name: gogithub.Ptr(baselineUnsafeText),
Body: gogithub.Ptr(baselineUnsafeText),
}
minimalRelease := convertToMinimalRelease(release)
assert.Equal(t, sanitizedUnsafeText, minimalRelease.Name)
assert.Equal(t, sanitizedUnsafeText, minimalRelease.Body)
sanitizedRelease := sanitizedReleaseCopy(release)
assert.Equal(t, sanitizedUnsafeText, sanitizedRelease.GetName())
assert.Equal(t, sanitizedUnsafeText, sanitizedRelease.GetBody())
assert.Equal(t, baselineUnsafeText, release.GetName())
assert.Equal(t, baselineUnsafeText, release.GetBody())
securityAdvisory := &gogithub.SecurityAdvisory{
Summary: gogithub.Ptr(baselineUnsafeText),
Description: gogithub.Ptr(baselineUnsafeText),
CollaboratingTeams: []*gogithub.Team{{
Name: gogithub.Ptr(baselineUnsafeText),
Description: gogithub.Ptr(baselineUnsafeText),
Slug: gogithub.Ptr("team\u200B<script>"),
Parent: &gogithub.Team{
Name: gogithub.Ptr(baselineUnsafeText),
Description: gogithub.Ptr(baselineUnsafeText),
Slug: gogithub.Ptr("parent-exact"),
},
}},
}
advisory := sanitizedSecurityAdvisoryCopy(securityAdvisory)
assert.Equal(t, sanitizedUnsafeText, advisory.GetSummary())
assert.Equal(t, sanitizedUnsafeText, advisory.GetDescription())
require.Len(t, advisory.CollaboratingTeams, 1)
assert.Equal(t, sanitizedUnsafeText, advisory.CollaboratingTeams[0].GetName())
assert.Equal(t, sanitizedUnsafeText, advisory.CollaboratingTeams[0].GetDescription())
assert.Equal(t, "team\u200B<script>", advisory.CollaboratingTeams[0].GetSlug())
assert.Equal(t, sanitizedUnsafeText, advisory.CollaboratingTeams[0].Parent.GetName())
assert.Equal(t, sanitizedUnsafeText, advisory.CollaboratingTeams[0].Parent.GetDescription())
assert.Equal(t, "parent-exact", advisory.CollaboratingTeams[0].Parent.GetSlug())
assert.Equal(t, baselineUnsafeText, securityAdvisory.CollaboratingTeams[0].GetName())
assert.Equal(t, baselineUnsafeText, securityAdvisory.CollaboratingTeams[0].Parent.GetName())
dependabotAlert := &gogithub.DependabotAlert{
SecurityAdvisory: &gogithub.DependabotSecurityAdvisory{
Summary: gogithub.Ptr(baselineUnsafeText),
Description: gogithub.Ptr(baselineUnsafeText),
},
DismissedComment: gogithub.Ptr(baselineUnsafeText),
Repository: repo,
}
sanitizedDependabotAlert := sanitizedDependabotAlertCopy(dependabotAlert)
assert.Equal(t, sanitizedUnsafeText, sanitizedDependabotAlert.SecurityAdvisory.GetSummary())
assert.Equal(t, sanitizedUnsafeText, sanitizedDependabotAlert.SecurityAdvisory.GetDescription())
assert.Equal(t, sanitizedUnsafeText, sanitizedDependabotAlert.GetDismissedComment())
assert.Equal(t, sanitizedUnsafeText, sanitizedDependabotAlert.Repository.GetDescription())
assert.Equal(t, baselineUnsafeText, dependabotAlert.SecurityAdvisory.GetSummary())
assert.Equal(t, baselineUnsafeText, dependabotAlert.GetDismissedComment())
codeAlert := &gogithub.Alert{
RuleDescription: gogithub.Ptr(baselineUnsafeText),
DismissedComment: gogithub.Ptr(baselineUnsafeText),
Rule: &gogithub.Rule{
Name: gogithub.Ptr(baselineUnsafeText),
Description: gogithub.Ptr(baselineUnsafeText),
FullDescription: gogithub.Ptr(baselineUnsafeText),
Help: gogithub.Ptr(baselineUnsafeText),
},
MostRecentInstance: &gogithub.MostRecentInstance{
Message: &gogithub.Message{Text: gogithub.Ptr(baselineUnsafeText)},
},
}
sanitizedCodeAlert := sanitizedCodeScanningAlertCopy(codeAlert)
assert.Equal(t, sanitizedUnsafeText, sanitizedCodeAlert.GetRuleDescription())
assert.Equal(t, sanitizedUnsafeText, sanitizedCodeAlert.GetDismissedComment())
assert.Equal(t, sanitizedUnsafeText, sanitizedCodeAlert.Rule.GetName())
assert.Equal(t, sanitizedUnsafeText, sanitizedCodeAlert.Rule.GetDescription())
assert.Equal(t, sanitizedUnsafeText, sanitizedCodeAlert.Rule.GetFullDescription())
assert.Equal(t, sanitizedUnsafeText, sanitizedCodeAlert.Rule.GetHelp())
assert.Equal(t, sanitizedUnsafeText, sanitizedCodeAlert.MostRecentInstance.Message.GetText())
assert.Equal(t, baselineUnsafeText, codeAlert.GetRuleDescription())
assert.Equal(t, baselineUnsafeText, codeAlert.Rule.GetHelp())
secretAlert := &gogithub.SecretScanningAlert{
Secret: gogithub.Ptr(baselineUnsafeText),
ResolutionComment: gogithub.Ptr(baselineUnsafeText),
PushProtectionBypassRequestComment: gogithub.Ptr(baselineUnsafeText),
PushProtectionBypassRequestHTMLURL: gogithub.Ptr("https://github.com/owner/repo/security/secret-scanning/1"),
PushProtectionBypassRequestReviewer: &gogithub.User{Login: gogithub.Ptr("reviewer")},
PushProtectionBypassRequestReviewerComment: gogithub.Ptr(baselineUnsafeText),
}
sanitizedSecretAlert := sanitizedSecretScanningAlertCopy(secretAlert)
assert.Equal(t, baselineUnsafeText, sanitizedSecretAlert.GetSecret())
assert.Equal(t, sanitizedUnsafeText, sanitizedSecretAlert.GetResolutionComment())
assert.Equal(t, sanitizedUnsafeText, sanitizedSecretAlert.GetPushProtectionBypassRequestComment())
assert.Equal(t, sanitizedUnsafeText, sanitizedSecretAlert.GetPushProtectionBypassRequestReviewerComment())
assert.Equal(t, baselineUnsafeText, secretAlert.GetResolutionComment())
combinedStatus := &gogithub.CombinedStatus{
Name: gogithub.Ptr("branch\u200B<script>"),
SHA: gogithub.Ptr("abc123"),
Statuses: []*gogithub.RepoStatus{
{
Description: gogithub.Ptr(baselineUnsafeText),
Context: gogithub.Ptr("ci\u200B<script>"),
TargetURL: gogithub.Ptr("https://example.com/status<script>"),
},
nil,
},
}
sanitizedStatus := sanitizedCombinedStatusCopy(combinedStatus)
assert.Equal(t, "branch\u200B<script>", sanitizedStatus.GetName())
assert.Equal(t, "abc123", sanitizedStatus.GetSHA())
require.Len(t, sanitizedStatus.Statuses, 2)
assert.Equal(t, sanitizedUnsafeText, sanitizedStatus.Statuses[0].GetDescription())
assert.Equal(t, "ci\u200B<script>", sanitizedStatus.Statuses[0].GetContext())
assert.Equal(t, "https://example.com/status<script>", sanitizedStatus.Statuses[0].GetTargetURL())
assert.Nil(t, sanitizedStatus.Statuses[1])
assert.Equal(t, baselineUnsafeText, combinedStatus.Statuses[0].GetDescription())
workflow := &gogithub.Workflow{Name: gogithub.Ptr(baselineUnsafeText)}
sanitizedWorkflow := sanitizedWorkflowCopy(workflow)
assert.Equal(t, sanitizedUnsafeText, sanitizedWorkflow.GetName())
assert.Equal(t, baselineUnsafeText, workflow.GetName())
workflowRun := &gogithub.WorkflowRun{
Name: gogithub.Ptr(baselineUnsafeText),
DisplayTitle: gogithub.Ptr(baselineUnsafeText),
HeadCommit: &gogithub.HeadCommit{
Message: gogithub.Ptr(baselineUnsafeText),
Author: &gogithub.CommitAuthor{Name: gogithub.Ptr(baselineUnsafeText), Email: gogithub.Ptr("author@example.com")},
Committer: &gogithub.CommitAuthor{Name: gogithub.Ptr(baselineUnsafeText), Email: gogithub.Ptr("committer@example.com")},
Added: []string{"src/exact<script>.go"},
},
PullRequests: []*gogithub.PullRequest{{
Title: gogithub.Ptr(baselineUnsafeText),
Body: gogithub.Ptr(baselineUnsafeText),
Labels: []*gogithub.Label{{Name: gogithub.Ptr(baselineUnsafeText), Description: gogithub.Ptr(baselineUnsafeText)}},
Milestone: &gogithub.Milestone{Title: gogithub.Ptr(baselineUnsafeText), Description: gogithub.Ptr(baselineUnsafeText)},
Head: &gogithub.PullRequestBranch{Repo: repo},
Base: &gogithub.PullRequestBranch{Repo: repo},
}},
Repository: repo,
}
sanitizedRun := sanitizedWorkflowRunCopy(workflowRun)
assert.Equal(t, sanitizedUnsafeText, sanitizedRun.GetName())
assert.Equal(t, sanitizedUnsafeText, sanitizedRun.GetDisplayTitle())
assert.Equal(t, sanitizedUnsafeText, sanitizedRun.PullRequests[0].GetTitle())
assert.Equal(t, sanitizedUnsafeText, sanitizedRun.PullRequests[0].Labels[0].GetName())
assert.Equal(t, sanitizedUnsafeText, sanitizedRun.PullRequests[0].Milestone.GetTitle())
assert.Equal(t, sanitizedUnsafeText, sanitizedRun.PullRequests[0].Head.Repo.GetDescription())
assert.Equal(t, sanitizedUnsafeText, sanitizedRun.PullRequests[0].Base.Repo.GetDescription())
assert.Equal(t, sanitizedUnsafeText, sanitizedRun.HeadCommit.GetMessage())
assert.Equal(t, sanitizedUnsafeText, sanitizedRun.HeadCommit.Author.GetName())
assert.Equal(t, sanitizedUnsafeText, sanitizedRun.HeadCommit.Committer.GetName())
assert.Equal(t, "author@example.com", sanitizedRun.HeadCommit.Author.GetEmail())
assert.Equal(t, []string{"src/exact<script>.go"}, sanitizedRun.HeadCommit.Added)
assert.Equal(t, sanitizedUnsafeText, sanitizedRun.Repository.GetDescription())
assert.Equal(t, baselineUnsafeText, workflowRun.GetDisplayTitle())
assert.Equal(t, baselineUnsafeText, workflowRun.HeadCommit.GetMessage())
assert.Equal(t, baselineUnsafeText, workflowRun.PullRequests[0].Labels[0].GetName())
commitMessage := "Subject\n\nCo-authored-by: Copilot <copilot@github.com>"
sanitizedCommit := sanitizedCommitCopy(&gogithub.Commit{Message: gogithub.Ptr(commitMessage)})
assert.Equal(t, commitMessage, sanitizedCommit.GetMessage())
sanitizedHeadCommit := sanitizedHeadCommitCopy(&gogithub.HeadCommit{Message: gogithub.Ptr(commitMessage)})
assert.Equal(t, commitMessage, sanitizedHeadCommit.GetMessage())
workflowJob := &gogithub.WorkflowJob{
Name: gogithub.Ptr(baselineUnsafeText),
WorkflowName: gogithub.Ptr(baselineUnsafeText),
RunnerName: gogithub.Ptr(baselineUnsafeText),
RunnerGroupName: gogithub.Ptr(baselineUnsafeText),
Steps: []*gogithub.TaskStep{{Name: gogithub.Ptr(baselineUnsafeText)}},
}
sanitizedJob := sanitizedWorkflowJobCopy(workflowJob)
assert.Equal(t, sanitizedUnsafeText, sanitizedJob.GetName())
assert.Equal(t, sanitizedUnsafeText, sanitizedJob.GetWorkflowName())
assert.Equal(t, baselineUnsafeText, sanitizedJob.GetRunnerName())
assert.Equal(t, baselineUnsafeText, sanitizedJob.GetRunnerGroupName())
assert.Equal(t, sanitizedUnsafeText, sanitizedJob.Steps[0].GetName())
assert.Equal(t, baselineUnsafeText, workflowJob.GetName())
}
func TestSanitizationRawContentOutputsPreserveExactText(t *testing.T) {
t.Run("file contents preserve exact text", func(t *testing.T) {
rawContent := []byte(baselineUnsafeText)
client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
GetReposGitRefByOwnerByRepoByRef: mockResponse(t, http.StatusOK, `{"ref":"refs/heads/main","object":{"sha":""}}`),
GetReposByOwnerByRepo: mockResponse(t, http.StatusOK, `{"name":"repo","default_branch":"main"}`),
GetReposContentsByOwnerByRepoByPath: func(w http.ResponseWriter, _ *http.Request) {
w.WriteHeader(http.StatusOK)
fileContent := &gogithub.RepositoryContent{
Name: gogithub.Ptr("README.md"),
Path: gogithub.Ptr("README.md"),
SHA: gogithub.Ptr("abc123"),
Type: gogithub.Ptr("file"),
Content: gogithub.Ptr(base64.StdEncoding.EncodeToString(rawContent)),
Size: gogithub.Ptr(len(rawContent)),
Encoding: gogithub.Ptr("base64"),
}
require.NoError(t, json.NewEncoder(w).Encode(fileContent))
},
}))
deps := BaseDeps{Client: client}
serverTool := GetFileContents(translations.NullTranslationHelper)
handler := serverTool.Handler(deps)
result, err := handler(ContextWithDeps(context.Background(), deps), &mcp.CallToolRequest{
Params: createMCPRequest(map[string]any{
"owner": "owner",
"repo": "repo",
"path": "README.md",
"ref": "refs/heads/main",
}).Params,
})
require.NoError(t, err)
require.False(t, result.IsError)
resource := getResourceResult(t, result)
assert.Equal(t, baselineUnsafeText, resource.Text)
})
t.Run("diff and patch outputs preserve exact text", func(t *testing.T) {
patch := "@@ -1 +1 @@\n-unsafe\n+" + baselineUnsafeText
prFiles := convertToMinimalPRFiles([]*gogithub.CommitFile{{Filename: gogithub.Ptr("README.md"), Patch: gogithub.Ptr(patch)}})
require.Len(t, prFiles, 1)
assert.Equal(t, patch, prFiles[0].Patch)
commit := convertToMinimalCommit(&gogithub.RepositoryCommit{
SHA: gogithub.Ptr("abc123"),
HTMLURL: gogithub.Ptr("https://github.com/owner/repo/commit/abc123"),
Files: []*gogithub.CommitFile{{Filename: gogithub.Ptr("README.md"), Patch: gogithub.Ptr(patch)}},
}, commitDetailFullPatch)
require.Len(t, commit.Files, 1)
assert.Equal(t, patch, commit.Files[0].Patch)
})
t.Run("workflow logs preserve exact text", func(t *testing.T) {
logContent := "line 1\n" + baselineUnsafeText + "\nline 3"
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
_, _ = w.Write([]byte(logContent))
}))
t.Cleanup(server.Close)
content, originalLength, resp, err := downloadLogContent(context.Background(), server.URL, 10, 10)
if resp != nil {
defer func() { _ = resp.Body.Close() }()
}
require.NoError(t, err)
assert.Equal(t, logContent, content)
assert.Equal(t, len(strings.Split(logContent, "\n")), originalLength)
})
t.Run("code search text matches preserve exact text", func(t *testing.T) {
client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
GetSearchCode: expectQueryParams(t, map[string]string{
"q": "repo:owner/repo unsafe",
"page": "1",
"per_page": "30",
}).withHeaders(map[string]string{"Accept": "text-match"}).andThen(mockResponse(t, http.StatusOK, &gogithub.CodeSearchResult{
Total: gogithub.Ptr(1),
IncompleteResults: gogithub.Ptr(false),
CodeResults: []*gogithub.CodeResult{{
Name: gogithub.Ptr("main.go"),
Path: gogithub.Ptr("main.go"),
SHA: gogithub.Ptr("abc123"),
Repository: &gogithub.Repository{FullName: gogithub.Ptr("owner/repo")},
TextMatches: []*gogithub.TextMatch{{
Fragment: gogithub.Ptr(baselineUnsafeText),
}},
}},
})),
}))
deps := BaseDeps{Client: client}
serverTool := SearchCode(translations.NullTranslationHelper)
handler := serverTool.Handler(deps)
result, err := handler(ContextWithDeps(context.Background(), deps), &mcp.CallToolRequest{
Params: createMCPRequest(map[string]any{
"query": "repo:owner/repo unsafe",
}).Params,
})
require.NoError(t, err)
require.False(t, result.IsError)
var searchResult MinimalCodeSearchResult
require.NoError(t, json.Unmarshal([]byte(getTextResult(t, result).Text), &searchResult))
require.Len(t, searchResult.Items, 1)
require.Len(t, searchResult.Items[0].TextMatches, 1)
assert.Equal(t, baselineUnsafeText, searchResult.Items[0].TextMatches[0].GetFragment())
})
}
func TestSanitizationInputContentPreservation(t *testing.T) {
t.Run("create issue preserves title and body request content", func(t *testing.T) {
client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
PostReposIssuesByOwnerByRepo: expectRequestBody(t, map[string]any{
"title": baselineUnsafeText,
"body": baselineUnsafeText,
"assignees": []any{},
"labels": []any{},
}).andThen(mockResponse(t, http.StatusCreated, &gogithub.Issue{
ID: gogithub.Ptr(int64(1)),
HTMLURL: gogithub.Ptr("https://github.com/owner/repo/issues/1"),
})),
}))
result, err := CreateIssue(context.Background(), client, "owner", "repo", baselineUnsafeText, baselineUnsafeText, []string{}, []string{}, 0, "", nil)
require.NoError(t, err)
require.False(t, result.IsError)
})
t.Run("issue comment preserves body request content", func(t *testing.T) {
client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
PostReposIssuesCommentsByOwnerByRepoByIssueNumber: expectRequestBody(t, map[string]any{
"body": baselineUnsafeText,
}).andThen(mockResponse(t, http.StatusCreated, &gogithub.IssueComment{
ID: gogithub.Ptr(int64(1)),
HTMLURL: gogithub.Ptr("https://github.com/owner/repo/issues/1#issuecomment-1"),
})),
}))
deps := BaseDeps{Client: client}
serverTool := AddIssueComment(translations.NullTranslationHelper)
handler := serverTool.Handler(deps)
result, err := handler(ContextWithDeps(context.Background(), deps), &mcp.CallToolRequest{
Params: createMCPRequest(map[string]any{
"owner": "owner",
"repo": "repo",
"issue_number": float64(1),
"body": baselineUnsafeText,
}).Params,
})
require.NoError(t, err)
require.False(t, result.IsError)
})
t.Run("discussion comment preserves body request content", func(t *testing.T) {
gqlClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient(
githubv4mock.NewQueryMatcher(
struct {
Repository struct {
Discussion struct {
ID githubv4.ID
} `graphql:"discussion(number: $discussionNumber)"`
} `graphql:"repository(owner: $owner, name: $repo)"`
}{},
map[string]any{
"owner": githubv4.String("owner"),
"repo": githubv4.String("repo"),
"discussionNumber": githubv4.Int(1),
},
githubv4mock.DataResponse(map[string]any{
"repository": map[string]any{
"discussion": map[string]any{"id": "D_1"},
},
}),
),
githubv4mock.NewMutationMatcher(
struct {
AddDiscussionComment struct {
Comment struct {
ID githubv4.ID
URL githubv4.String `graphql:"url"`
}
} `graphql:"addDiscussionComment(input: $input)"`
}{},
githubv4.AddDiscussionCommentInput{
DiscussionID: githubv4.ID("D_1"),
Body: githubv4.String(baselineUnsafeText),
},
nil,
githubv4mock.DataResponse(map[string]any{
"addDiscussionComment": map[string]any{
"comment": map[string]any{
"id": "DC_1",
"url": "https://github.com/owner/repo/discussions/1#discussioncomment-1",
},
},
}),
),
))
result, _, err := addDiscussionComment(context.Background(), gqlClient, map[string]any{
"owner": "owner",
"repo": "repo",
"discussionNumber": float64(1),
"body": baselineUnsafeText,
})
require.NoError(t, err)
require.False(t, result.IsError)
})
t.Run("search query preserves syntax except intentional issue qualifier prefix", func(t *testing.T) {
query, opts, err := prepareSearchArgs(map[string]any{
"query": `repo:github/github-mcp-server label:critical OR "exact phrase" field.priority:P1`,
}, "issue")
require.NoError(t, err)
assert.Equal(t, `is:issue repo:github/github-mcp-server label:critical OR "exact phrase" field.priority:P1`, query)
require.NotNil(t, opts.AdvancedSearch)
assert.True(t, *opts.AdvancedSearch)
})
}
func TestSanitizationInputPolicyValidation(t *testing.T) {
t.Run("repo-relative path validation rejects traversal leading slash and control characters", func(t *testing.T) {
require.NoError(t, validateRepoRelativePath("path", "docs/readme.md"))
require.NoError(t, validateRepoRelativePath("path", " "))
require.NoError(t, validateRepoRelativePath("path", "\u00a0"))
assert.EqualError(t, validateRepoRelativePath("path", ""), "path must not be empty")
assert.EqualError(t, validateRepoRelativePath("path", "/docs/readme.md"), "path must be relative to the repository root (no leading '/')")
assert.EqualError(t, validateRepoRelativePath("path", "docs/../secrets.txt"), "path must not contain '..' segments")
assert.EqualError(t, validateRepoRelativePath("path", "docs/readme.md\x00"), "path must not contain control characters")
require.NoError(t, validateRepoRelativePathOrRoot("path", "/"))
})
t.Run("enum validation rejects undeclared control values", func(t *testing.T) {
require.NoError(t, validateEnumParam("sort", "", "created", "updated"))
require.NoError(t, validateEnumParam("sort", "created", "created", "updated"))
assert.EqualError(t, validateEnumParam("sort", "pushed", "created", "updated"), "sort must be one of: created, updated")
})
t.Run("search issue validates sort but preserves query syntax", func(t *testing.T) {
query, opts, err := prepareSearchArgs(map[string]any{
"query": `repo:github/github-mcp-server label:critical OR "exact phrase" field.priority:P1`,
"sort": "updated",
"order": "desc",
}, "issue")
require.NoError(t, err)
assert.Equal(t, `is:issue repo:github/github-mcp-server label:critical OR "exact phrase" field.priority:P1`, query)
assert.Equal(t, "updated", opts.Sort)
assert.Equal(t, "desc", opts.Order)
_, _, err = prepareSearchArgs(map[string]any{
"query": "unsafe",
"sort": "pushed",
}, "issue")
assert.EqualError(t, err, "sort must be one of: comments, reactions, reactions-+1, reactions--1, reactions-smile, reactions-thinking_face, reactions-heart, reactions-tada, interactions, created, updated")
})
t.Run("file handlers reject invalid paths before content mutation", func(t *testing.T) {
deps := BaseDeps{}
getFileTool := GetFileContents(translations.NullTranslationHelper)
getFileHandler := getFileTool.Handler(deps)
result, err := getFileHandler(ContextWithDeps(context.Background(), deps), &mcp.CallToolRequest{
Params: createMCPRequest(map[string]any{
"owner": "owner",
"repo": "repo",
"path": "../README.md",
}).Params,
})
require.NoError(t, err)
require.True(t, result.IsError)
assert.Contains(t, getTextResult(t, result).Text, "path must not contain '..' segments")
createTool := CreateOrUpdateFile(translations.NullTranslationHelper)
createHandler := createTool.Handler(deps)
result, err = createHandler(ContextWithDeps(context.Background(), deps), &mcp.CallToolRequest{
Params: createMCPRequest(map[string]any{
"owner": "owner",
"repo": "repo",
"path": "/README.md",
"content": baselineUnsafeText,
"message": baselineUnsafeText,
"branch": "main",
}).Params,
})
require.NoError(t, err)
require.True(t, result.IsError)
assert.Contains(t, getTextResult(t, result).Text, "path must be relative to the repository root")
pushTool := PushFiles(translations.NullTranslationHelper)
pushHandler := pushTool.Handler(deps)
result, err = pushHandler(ContextWithDeps(context.Background(), deps), &mcp.CallToolRequest{
Params: createMCPRequest(map[string]any{
"owner": "owner",
"repo": "repo",
"branch": "main",
"message": baselineUnsafeText,
"files": []any{
map[string]any{"path": "bad\x00path", "content": baselineUnsafeText},
},
}).Params,
})
require.NoError(t, err)
require.True(t, result.IsError)
assert.Contains(t, getTextResult(t, result).Text, "path must not contain control characters")
})
}
+27 -28
View File
@@ -68,10 +68,16 @@ func SearchRepositories(t translations.TranslationHelperFunc) inventory.ServerTo
if err != nil {
return utils.NewToolResultError(err.Error()), nil, nil
}
if err := validateEnumParam("sort", sort, "stars", "forks", "help-wanted-issues", "updated"); err != nil {
return utils.NewToolResultError(err.Error()), nil, nil
}
order, err := OptionalParam[string](args, "order")
if err != nil {
return utils.NewToolResultError(err.Error()), nil, nil
}
if err := validateEnumParam("order", order, "asc", "desc"); err != nil {
return utils.NewToolResultError(err.Error()), nil, nil
}
pagination, err := OptionalPaginationParams(args)
if err != nil {
return utils.NewToolResultError(err.Error()), nil, nil
@@ -116,33 +122,7 @@ func SearchRepositories(t translations.TranslationHelperFunc) inventory.ServerTo
if minimalOutput {
minimalRepos := make([]MinimalRepository, 0, len(result.Repositories))
for _, repo := range result.Repositories {
minimalRepo := MinimalRepository{
ID: repo.GetID(),
Name: repo.GetName(),
FullName: repo.GetFullName(),
Description: repo.GetDescription(),
HTMLURL: repo.GetHTMLURL(),
Language: repo.GetLanguage(),
Stars: repo.GetStargazersCount(),
Forks: repo.GetForksCount(),
OpenIssues: repo.GetOpenIssuesCount(),
Private: repo.GetPrivate(),
Fork: repo.GetFork(),
Archived: repo.GetArchived(),
DefaultBranch: repo.GetDefaultBranch(),
}
if repo.UpdatedAt != nil {
minimalRepo.UpdatedAt = repo.UpdatedAt.Format("2006-01-02T15:04:05Z")
}
if repo.CreatedAt != nil {
minimalRepo.CreatedAt = repo.CreatedAt.Format("2006-01-02T15:04:05Z")
}
if repo.Topics != nil {
minimalRepo.Topics = repo.Topics
}
minimalRepos = append(minimalRepos, minimalRepo)
minimalRepos = append(minimalRepos, convertToMinimalRepository(repo))
}
minimalResult := &MinimalSearchRepositoriesResult{
@@ -156,7 +136,7 @@ func SearchRepositories(t translations.TranslationHelperFunc) inventory.ServerTo
return utils.NewToolResultErrorFromErr("failed to marshal minimal response", err), nil, nil
}
} else {
r, err = json.Marshal(result)
r, err = json.Marshal(sanitizedRepositoriesSearchResultCopy(result))
if err != nil {
return utils.NewToolResultErrorFromErr("failed to marshal full response", err), nil, nil
}
@@ -229,6 +209,7 @@ func searchCodeTool(t translations.TranslationHelperFunc, includeFields bool) in
"sort": {
Type: "string",
Description: "Sort field ('indexed' only)",
Enum: []any{"indexed"},
},
"order": {
Type: "string",
@@ -267,10 +248,16 @@ func searchCodeTool(t translations.TranslationHelperFunc, includeFields bool) in
if err != nil {
return utils.NewToolResultError(err.Error()), nil, nil
}
if err := validateEnumParam("sort", sort, "indexed"); err != nil {
return utils.NewToolResultError(err.Error()), nil, nil
}
order, err := OptionalParam[string](args, "order")
if err != nil {
return utils.NewToolResultError(err.Error()), nil, nil
}
if err := validateEnumParam("order", order, "asc", "desc"); err != nil {
return utils.NewToolResultError(err.Error()), nil, nil
}
var fields []string
if includeFields {
fields, err = OptionalStringArrayParam(args, "fields")
@@ -391,10 +378,16 @@ func userOrOrgHandler(ctx context.Context, accountType string, deps ToolDependen
if err != nil {
return utils.NewToolResultError(err.Error()), nil, nil
}
if err := validateEnumParam("sort", sort, "followers", "repositories", "joined"); err != nil {
return utils.NewToolResultError(err.Error()), nil, nil
}
order, err := OptionalParam[string](args, "order")
if err != nil {
return utils.NewToolResultError(err.Error()), nil, nil
}
if err := validateEnumParam("order", order, "asc", "desc"); err != nil {
return utils.NewToolResultError(err.Error()), nil, nil
}
pagination, err := OptionalPaginationParams(args)
if err != nil {
return utils.NewToolResultError(err.Error()), nil, nil
@@ -601,10 +594,16 @@ func SearchCommits(t translations.TranslationHelperFunc) inventory.ServerTool {
if err != nil {
return utils.NewToolResultError(err.Error()), nil, nil
}
if err := validateEnumParam("sort", sort, "author-date", "committer-date"); err != nil {
return utils.NewToolResultError(err.Error()), nil, nil
}
order, err := OptionalParam[string](args, "order")
if err != nil {
return utils.NewToolResultError(err.Error()), nil, nil
}
if err := validateEnumParam("order", order, "asc", "desc"); err != nil {
return utils.NewToolResultError(err.Error()), nil, nil
}
pagination, err := OptionalPaginationParams(args)
if err != nil {
return utils.NewToolResultError(err.Error()), nil, nil
+76 -4
View File
@@ -359,6 +359,7 @@ func Test_SearchCode(t *testing.T) {
assert.Contains(t, schema.Properties, "perPage")
assert.Contains(t, schema.Properties, "page")
assert.Contains(t, schema.Properties, "fields")
assert.Equal(t, []any{"indexed"}, schema.Properties["sort"].Enum)
assert.ElementsMatch(t, schema.Required, []string{"query"})
// Setup mock search results
@@ -444,6 +445,16 @@ func Test_SearchCode(t *testing.T) {
expectError: false,
expectedResult: mockSearchResult,
},
{
name: "invalid sort is rejected before search",
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{}),
requestArgs: map[string]any{
"query": "fmt.Println language:go",
"sort": "updated",
},
expectError: true,
expectedErrMsg: "sort must be one of: indexed",
},
{
name: "search code fails",
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
@@ -789,6 +800,26 @@ func Test_SearchUsers(t *testing.T) {
expectError: true,
expectedErrMsg: "failed to search users",
},
{
name: "invalid sort is rejected before search",
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{}),
requestArgs: map[string]any{
"query": "octocat",
"sort": "stars",
},
expectError: true,
expectedErrMsg: "sort must be one of: followers, repositories, joined",
},
{
name: "invalid order is rejected before search",
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{}),
requestArgs: map[string]any{
"query": "octocat",
"order": "newest",
},
expectError: true,
expectedErrMsg: "order must be one of: asc, desc",
},
}
for _, tc := range tests {
@@ -952,6 +983,26 @@ func Test_SearchOrgs(t *testing.T) {
expectError: true,
expectedErrMsg: "failed to search orgs",
},
{
name: "invalid sort is rejected before search",
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{}),
requestArgs: map[string]any{
"query": "github",
"sort": "stars",
},
expectError: true,
expectedErrMsg: "sort must be one of: followers, repositories, joined",
},
{
name: "invalid order is rejected before search",
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{}),
requestArgs: map[string]any{
"query": "github",
"order": "newest",
},
expectError: true,
expectedErrMsg: "order must be one of: asc, desc",
},
}
for _, tc := range tests {
@@ -1026,9 +1077,9 @@ func Test_SearchCommits(t *testing.T) {
SHA: github.Ptr("abc123commit"),
HTMLURL: github.Ptr("https://github.com/owner/repo/commit/abc123commit"),
Commit: &github.Commit{
Message: github.Ptr("Initial commit"),
Message: github.Ptr(baselineUnsafeText),
Author: &github.CommitAuthor{
Name: github.Ptr("Author Name"),
Name: github.Ptr(baselineUnsafeText),
Email: github.Ptr("author@example.com"),
Date: &github.Timestamp{Time: now},
},
@@ -1102,6 +1153,26 @@ func Test_SearchCommits(t *testing.T) {
expectError: true,
expectedErrMsg: "failed to search commits",
},
{
name: "invalid sort is rejected before search",
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{}),
requestArgs: map[string]any{
"query": "repo:owner/repo fix",
"sort": "updated",
},
expectError: true,
expectedErrMsg: "sort must be one of: author-date, committer-date",
},
{
name: "invalid order is rejected before search",
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{}),
requestArgs: map[string]any{
"query": "repo:owner/repo fix",
"order": "newest",
},
expectError: true,
expectedErrMsg: "order must be one of: asc, desc",
},
}
for _, tc := range tests {
@@ -1134,8 +1205,8 @@ func Test_SearchCommits(t *testing.T) {
assert.Equal(t, tc.expectedResult.GetTotal(), returnedResult.TotalCount)
assert.Len(t, returnedResult.Items, len(tc.expectedResult.Commits))
assert.Equal(t, *tc.expectedResult.Commits[0].SHA, returnedResult.Items[0].SHA)
assert.Equal(t, *tc.expectedResult.Commits[0].Commit.Message, returnedResult.Items[0].Commit.Message)
assert.Equal(t, *tc.expectedResult.Commits[0].Commit.Author.Name, returnedResult.Items[0].Commit.Author.Name)
assert.Equal(t, sanitizeCommitMessage(tc.expectedResult.Commits[0].Commit.GetMessage()), returnedResult.Items[0].Commit.Message)
assert.Equal(t, sanitizeOutputText(tc.expectedResult.Commits[0].Commit.Author.GetName()), returnedResult.Items[0].Commit.Author.Name)
assert.Equal(t, now.Format(time.RFC3339), returnedResult.Items[0].Commit.Author.Date)
assert.Equal(t, *tc.expectedResult.Commits[0].Author.Login, returnedResult.Items[0].Author.Login)
@@ -1144,6 +1215,7 @@ func Test_SearchCommits(t *testing.T) {
require.NotNil(t, returnedResult.Items[0].Repository)
assert.Equal(t, "owner/repo", returnedResult.Items[0].Repository.FullName)
assert.Equal(t, "https://github.com/owner/repo", returnedResult.Items[0].Repository.HTMLURL)
assert.Equal(t, baselineUnsafeText, mockSearchResult.Commits[0].Commit.GetMessage())
// Second commit has no resolved GitHub user for author/committer
// and no commit-level author block — the handler must not panic
+24 -5
View File
@@ -105,10 +105,28 @@ func prepareSearchArgs(args map[string]any, searchType string) (string, *github.
if err != nil {
return "", nil, err
}
if err := validateEnumParam("sort", sort,
"comments",
"reactions",
"reactions-+1",
"reactions--1",
"reactions-smile",
"reactions-thinking_face",
"reactions-heart",
"reactions-tada",
"interactions",
"created",
"updated",
); err != nil {
return "", nil, err
}
order, err := OptionalParam[string](args, "order")
if err != nil {
return "", nil, err
}
if err := validateEnumParam("order", order, "asc", "desc"); err != nil {
return "", nil, err
}
pagination, err := OptionalPaginationParams(args)
if err != nil {
return "", nil, err
@@ -166,16 +184,17 @@ func searchHandler(
return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, errorPrefix, resp, body), nil
}
sanitizedResult := sanitizedIssuesSearchResultCopy(result)
filtered := false
var payload any = result
var payload any = sanitizedResult
if len(cfg.fields) > 0 {
filteredItems, err := filterEachField(result.Issues, cfg.fields)
filteredItems, err := filterEachField(sanitizedResult.Issues, cfg.fields)
if err != nil {
return utils.NewToolResultErrorFromErr(errorPrefix+": failed to filter results", err), nil
}
payload = map[string]any{
"total_count": result.Total,
"incomplete_results": result.IncompleteResults,
"total_count": sanitizedResult.Total,
"incomplete_results": sanitizedResult.IncompleteResults,
"items": filteredItems,
}
filtered = true
@@ -187,7 +206,7 @@ func searchHandler(
}
if cfg.fieldsTool != "" {
recordFieldsUsageFor(ctx, cfg.fieldsDeps, cfg.fieldsTool, result, filtered, len(r))
recordFieldsUsageFor(ctx, cfg.fieldsDeps, cfg.fieldsTool, sanitizedResult, filtered, len(r))
}
callResult := utils.NewToolResultText(string(r))
+8 -2
View File
@@ -85,7 +85,7 @@ func GetSecretScanningAlert(t translations.TranslationHelperFunc) inventory.Serv
return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to get alert", resp, body), nil, nil
}
r, err := json.Marshal(alert)
r, err := json.Marshal(sanitizedSecretScanningAlertCopy(alert))
if err != nil {
return nil, nil, fmt.Errorf("failed to marshal alert: %w", err)
}
@@ -156,6 +156,9 @@ func ListSecretScanningAlerts(t translations.TranslationHelperFunc) inventory.Se
if err != nil {
return utils.NewToolResultError(err.Error()), nil, nil
}
if err := validateEnumParam("state", state, "open", "resolved"); err != nil {
return utils.NewToolResultError(err.Error()), nil, nil
}
secretType, err := OptionalParam[string](args, "secret_type")
if err != nil {
return utils.NewToolResultError(err.Error()), nil, nil
@@ -164,6 +167,9 @@ func ListSecretScanningAlerts(t translations.TranslationHelperFunc) inventory.Se
if err != nil {
return utils.NewToolResultError(err.Error()), nil, nil
}
if err := validateEnumParam("resolution", resolution, "false_positive", "wont_fix", "revoked", "pattern_edited", "pattern_deleted", "used_in_tests"); err != nil {
return utils.NewToolResultError(err.Error()), nil, nil
}
pagination, err := OptionalPaginationParams(args)
if err != nil {
@@ -200,7 +206,7 @@ func ListSecretScanningAlerts(t translations.TranslationHelperFunc) inventory.Se
return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to list alerts", resp, body), nil, nil
}
r, err := json.Marshal(alerts)
r, err := json.Marshal(sanitizedSecretScanningAlertsCopy(alerts))
if err != nil {
return nil, nil, fmt.Errorf("failed to marshal alerts: %w", err)
}
+22
View File
@@ -216,6 +216,28 @@ func Test_ListSecretScanningAlerts(t *testing.T) {
expectError: false,
expectedAlerts: []*github.SecretScanningAlert{&openAlert},
},
{
name: "invalid state is rejected before request",
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{}),
requestArgs: map[string]any{
"owner": "owner",
"repo": "repo",
"state": "dismissed",
},
expectError: true,
expectedErrMsg: "state must be one of: open, resolved",
},
{
name: "invalid resolution is rejected before request",
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{}),
requestArgs: map[string]any{
"owner": "owner",
"repo": "repo",
"resolution": "ignored",
},
expectError: true,
expectedErrMsg: "resolution must be one of: false_positive, wont_fix, revoked, pattern_edited, pattern_deleted, used_in_tests",
},
{
name: "alerts listing fails",
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
+34 -9
View File
@@ -87,11 +87,6 @@ func ListGlobalSecurityAdvisories(t translations.TranslationHelperFunc) inventor
},
[]scopes.Scope{scopes.SecurityEvents},
func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) {
client, err := deps.GetClient(ctx)
if err != nil {
return nil, nil, fmt.Errorf("failed to get GitHub client: %w", err)
}
ghsaID, err := OptionalParam[string](args, "ghsaId")
if err != nil {
return utils.NewToolResultError(fmt.Sprintf("invalid ghsaId: %v", err)), nil, nil
@@ -101,6 +96,9 @@ func ListGlobalSecurityAdvisories(t translations.TranslationHelperFunc) inventor
if err != nil {
return utils.NewToolResultError(fmt.Sprintf("invalid type: %v", err)), nil, nil
}
if err := validateEnumParam("type", typ, "reviewed", "malware", "unreviewed"); err != nil {
return utils.NewToolResultError(err.Error()), nil, nil
}
cveID, err := OptionalParam[string](args, "cveId")
if err != nil {
@@ -111,11 +109,17 @@ func ListGlobalSecurityAdvisories(t translations.TranslationHelperFunc) inventor
if err != nil {
return utils.NewToolResultError(fmt.Sprintf("invalid ecosystem: %v", err)), nil, nil
}
if err := validateEnumParam("ecosystem", eco, "actions", "composer", "erlang", "go", "maven", "npm", "nuget", "other", "pip", "pub", "rubygems", "rust"); err != nil {
return utils.NewToolResultError(err.Error()), nil, nil
}
sev, err := OptionalParam[string](args, "severity")
if err != nil {
return utils.NewToolResultError(fmt.Sprintf("invalid severity: %v", err)), nil, nil
}
if err := validateEnumParam("severity", sev, "unknown", "low", "medium", "high", "critical"); err != nil {
return utils.NewToolResultError(err.Error()), nil, nil
}
cwes, err := OptionalStringArrayParam(args, "cwes")
if err != nil {
@@ -147,6 +151,11 @@ func ListGlobalSecurityAdvisories(t translations.TranslationHelperFunc) inventor
return utils.NewToolResultError(fmt.Sprintf("invalid modified: %v", err)), nil, nil
}
client, err := deps.GetClient(ctx)
if err != nil {
return nil, nil, fmt.Errorf("failed to get GitHub client: %w", err)
}
opts := &github.ListGlobalSecurityAdvisoriesOptions{}
if ghsaID != "" {
@@ -199,7 +208,7 @@ func ListGlobalSecurityAdvisories(t translations.TranslationHelperFunc) inventor
return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to list advisories", resp, body), nil, nil
}
r, err := json.Marshal(advisories)
r, err := json.Marshal(sanitizedGlobalSecurityAdvisoriesCopy(advisories))
if err != nil {
return nil, nil, fmt.Errorf("failed to marshal advisories: %w", err)
}
@@ -277,6 +286,9 @@ func ListRepositorySecurityAdvisories(t translations.TranslationHelperFunc) inve
if err != nil {
return utils.NewToolResultError(err.Error()), nil, nil
}
if err := validateRepositorySecurityAdvisoryListParams(direction, sortField, state); err != nil {
return utils.NewToolResultError(err.Error()), nil, nil
}
client, err := deps.GetClient(ctx)
if err != nil {
@@ -308,7 +320,7 @@ func ListRepositorySecurityAdvisories(t translations.TranslationHelperFunc) inve
return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to list repository advisories", resp, body), nil, nil
}
r, err := json.Marshal(advisories)
r, err := json.Marshal(sanitizedSecurityAdvisoriesCopy(advisories))
if err != nil {
return nil, nil, fmt.Errorf("failed to marshal advisories: %w", err)
}
@@ -375,7 +387,7 @@ func GetGlobalSecurityAdvisory(t translations.TranslationHelperFunc) inventory.S
return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to get advisory", resp, body), nil, nil
}
r, err := json.Marshal(advisory)
r, err := json.Marshal(sanitizedGlobalSecurityAdvisoryCopy(advisory))
if err != nil {
return nil, nil, fmt.Errorf("failed to marshal advisory: %w", err)
}
@@ -443,6 +455,9 @@ func ListOrgRepositorySecurityAdvisories(t translations.TranslationHelperFunc) i
if err != nil {
return utils.NewToolResultError(err.Error()), nil, nil
}
if err := validateRepositorySecurityAdvisoryListParams(direction, sortField, state); err != nil {
return utils.NewToolResultError(err.Error()), nil, nil
}
client, err := deps.GetClient(ctx)
if err != nil {
@@ -474,7 +489,7 @@ func ListOrgRepositorySecurityAdvisories(t translations.TranslationHelperFunc) i
return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to list organization repository advisories", resp, body), nil, nil
}
r, err := json.Marshal(advisories)
r, err := json.Marshal(sanitizedSecurityAdvisoriesCopy(advisories))
if err != nil {
return nil, nil, fmt.Errorf("failed to marshal advisories: %w", err)
}
@@ -490,6 +505,16 @@ func ListOrgRepositorySecurityAdvisories(t translations.TranslationHelperFunc) i
)
}
func validateRepositorySecurityAdvisoryListParams(direction, sortField, state string) error {
if err := validateEnumParam("direction", direction, "asc", "desc"); err != nil {
return err
}
if err := validateEnumParam("sort", sortField, "created", "updated", "published"); err != nil {
return err
}
return validateEnumParam("state", state, "triage", "draft", "published", "closed")
}
// allAdvisoriesPublished reports whether every advisory in the slice is in the
// "published" state. Repository security advisories can also be in draft,
// triage, or closed states, none of which are world-readable even on a public
+96 -14
View File
@@ -62,19 +62,32 @@ func Test_ListGlobalSecurityAdvisories(t *testing.T) {
expectedAdvisories: []*github.GlobalSecurityAdvisory{mockAdvisory},
},
{
name: "invalid severity value",
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
GetAdvisories: http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
w.WriteHeader(http.StatusBadRequest)
_, _ = w.Write([]byte(`{"message": "Bad Request"}`))
}),
}),
name: "invalid type is rejected before request",
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{}),
requestArgs: map[string]any{
"type": "private",
},
expectError: true,
expectedErrMsg: "type must be one of: reviewed, malware, unreviewed",
},
{
name: "invalid ecosystem is rejected before request",
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{}),
requestArgs: map[string]any{
"ecosystem": "cargo",
},
expectError: true,
expectedErrMsg: "ecosystem must be one of: actions, composer, erlang, go, maven, npm, nuget, other, pip, pub, rubygems, rust",
},
{
name: "invalid severity value",
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{}),
requestArgs: map[string]any{
"type": "reviewed",
"severity": "extreme",
},
expectError: true,
expectedErrMsg: "failed to list global security advisories",
expectedErrMsg: "severity must be one of: unknown, low, medium, high, critical",
},
{
name: "API error handling",
@@ -105,8 +118,13 @@ func Test_ListGlobalSecurityAdvisories(t *testing.T) {
// Verify results
if tc.expectError {
require.Error(t, err)
assert.Contains(t, err.Error(), tc.expectedErrMsg)
if err != nil {
assert.Contains(t, err.Error(), tc.expectedErrMsg)
return
}
require.NotNil(t, result)
require.True(t, result.IsError)
assert.Contains(t, getErrorResult(t, result).Text, tc.expectedErrMsg)
return
}
@@ -260,6 +278,11 @@ func Test_ListRepositorySecurityAdvisories(t *testing.T) {
Summary: github.Ptr("Repo advisory one"),
Description: github.Ptr("First repo advisory."),
Severity: github.Ptr("high"),
CollaboratingTeams: []*github.Team{{
Name: github.Ptr(baselineUnsafeText),
Description: github.Ptr(baselineUnsafeText),
Slug: github.Ptr("security<script>"),
}},
}
adv2 := &github.SecurityAdvisory{
GHSAID: github.Ptr("GHSA-2222-2222-2222"),
@@ -317,6 +340,39 @@ func Test_ListRepositorySecurityAdvisories(t *testing.T) {
expectError: false,
expectedAdvisories: []*github.SecurityAdvisory{adv1},
},
{
name: "invalid direction is rejected before request",
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{}),
requestArgs: map[string]any{
"owner": "owner",
"repo": "repo",
"direction": "sideways",
},
expectError: true,
expectedErrMsg: "direction must be one of: asc, desc",
},
{
name: "invalid sort is rejected before request",
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{}),
requestArgs: map[string]any{
"owner": "owner",
"repo": "repo",
"sort": "severity",
},
expectError: true,
expectedErrMsg: "sort must be one of: created, updated, published",
},
{
name: "invalid state is rejected before request",
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{}),
requestArgs: map[string]any{
"owner": "owner",
"repo": "repo",
"state": "withdrawn",
},
expectError: true,
expectedErrMsg: "state must be one of: triage, draft, published, closed",
},
{
name: "advisories listing fails",
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
@@ -348,8 +404,13 @@ func Test_ListRepositorySecurityAdvisories(t *testing.T) {
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
if tc.expectError {
require.Error(t, err)
assert.Contains(t, err.Error(), tc.expectedErrMsg)
if err != nil {
assert.Contains(t, err.Error(), tc.expectedErrMsg)
return
}
require.NotNil(t, result)
require.True(t, result.IsError)
assert.Contains(t, getErrorResult(t, result).Text, tc.expectedErrMsg)
return
}
@@ -366,7 +427,13 @@ func Test_ListRepositorySecurityAdvisories(t *testing.T) {
assert.Equal(t, *tc.expectedAdvisories[i].Summary, *advisory.Summary)
assert.Equal(t, *tc.expectedAdvisories[i].Description, *advisory.Description)
assert.Equal(t, *tc.expectedAdvisories[i].Severity, *advisory.Severity)
if len(tc.expectedAdvisories[i].CollaboratingTeams) > 0 {
assert.Equal(t, sanitizeOutputText(tc.expectedAdvisories[i].CollaboratingTeams[0].GetName()), advisory.CollaboratingTeams[0].GetName())
assert.Equal(t, sanitizeOutputText(tc.expectedAdvisories[i].CollaboratingTeams[0].GetDescription()), advisory.CollaboratingTeams[0].GetDescription())
assert.Equal(t, tc.expectedAdvisories[i].CollaboratingTeams[0].GetSlug(), advisory.CollaboratingTeams[0].GetSlug())
}
}
assert.Equal(t, baselineUnsafeText, adv1.CollaboratingTeams[0].GetName())
})
}
}
@@ -574,6 +641,16 @@ func Test_ListOrgRepositorySecurityAdvisories(t *testing.T) {
expectError: false,
expectedAdvisories: []*github.SecurityAdvisory{adv1},
},
{
name: "invalid state is rejected before request",
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{}),
requestArgs: map[string]any{
"org": "octo",
"state": "withdrawn",
},
expectError: true,
expectedErrMsg: "state must be one of: triage, draft, published, closed",
},
{
name: "listing fails",
mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
@@ -604,8 +681,13 @@ func Test_ListOrgRepositorySecurityAdvisories(t *testing.T) {
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
if tc.expectError {
require.Error(t, err)
assert.Contains(t, err.Error(), tc.expectedErrMsg)
if err != nil {
assert.Contains(t, err.Error(), tc.expectedErrMsg)
return
}
require.NotNil(t, result)
require.True(t, result.IsError)
assert.Contains(t, getErrorResult(t, result).Text, tc.expectedErrMsg)
return
}