From 015b8b6bec138c6e36f57a636110da831beb0f43 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 23 May 2025 10:45:43 +0000 Subject: [PATCH 1/5] Initial plan for issue From e2f6b4476307b7bed68b01c9400b7e36c7685a65 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 23 May 2025 10:51:08 +0000 Subject: [PATCH 2/5] Add content filtering package Co-authored-by: SamMorrowDrums <4811358+SamMorrowDrums@users.noreply.github.com> --- pkg/filtering/content_filter.go | 133 ++++++++++++++++++++ pkg/filtering/content_filter_test.go | 173 +++++++++++++++++++++++++++ 2 files changed, 306 insertions(+) create mode 100644 pkg/filtering/content_filter.go create mode 100644 pkg/filtering/content_filter_test.go diff --git a/pkg/filtering/content_filter.go b/pkg/filtering/content_filter.go new file mode 100644 index 00000000..2d01c4bd --- /dev/null +++ b/pkg/filtering/content_filter.go @@ -0,0 +1,133 @@ +package filtering + +import ( + "regexp" + "strings" +) + +var ( + // Invisible Unicode characters + // This includes zero-width spaces, zero-width joiners, zero-width non-joiners, + // bidirectional marks, and other invisible unicode characters + invisibleCharsRegex = regexp.MustCompile(`[\x{200B}-\x{200F}\x{2028}-\x{202E}\x{2060}-\x{2064}\x{FEFF}]`) + + // HTML comments + htmlCommentsRegex = regexp.MustCompile(``) + + // HTML elements that could contain hidden content + // This is a simple approach that targets specific dangerous tags + // Go's regexp doesn't support backreferences, so we list each tag explicitly + htmlScriptRegex = regexp.MustCompile(`]*>[\s\S]*?`) + htmlStyleRegex = regexp.MustCompile(`]*>[\s\S]*?`) + htmlIframeRegex = regexp.MustCompile(`]*>[\s\S]*?`) + htmlObjectRegex = regexp.MustCompile(`]*>[\s\S]*?`) + htmlEmbedRegex = regexp.MustCompile(`]*>[\s\S]*?`) + htmlSvgRegex = regexp.MustCompile(`]*>[\s\S]*?`) + htmlMathRegex = regexp.MustCompile(`]*>[\s\S]*?`) + htmlLinkRegex = regexp.MustCompile(`]*>[\s\S]*?`) + + // HTML attributes that might be used for hiding content + htmlAttributesRegex = regexp.MustCompile(`<[^>]*(?:style|data-[\w-]+|hidden|class)="[^"]*"[^>]*>`) + + // Detect collapsed sections (details/summary) + collapsedSectionsRegex = regexp.MustCompile(`
[\s\S]*?
`) + + // Very small text (font-size or similar CSS tricks) + smallTextRegex = regexp.MustCompile(`<[^>]*style="[^"]*font-size:\s*(?:0|0\.\d+|[0-3])(?:px|pt|em|%)[^"]*"[^>]*>[\s\S]*?]+>`) + + // Excessive whitespace (more than 3 consecutive newlines) + excessiveWhitespaceRegex = regexp.MustCompile(`\n{4,}`) +) + +// Config holds configuration for content filtering +type Config struct { + // DisableContentFiltering disables all content filtering when true + DisableContentFiltering bool +} + +// DefaultConfig returns the default content filtering configuration +func DefaultConfig() *Config { + return &Config{ + DisableContentFiltering: false, + } +} + +// FilterContent filters potentially hidden content from the input text +// This includes invisible Unicode characters, HTML comments, and other methods of hiding content +func FilterContent(input string, cfg *Config) string { + if cfg != nil && cfg.DisableContentFiltering { + return input + } + + if input == "" { + return input + } + + // Process the input text through each filter + result := input + + // Remove invisible characters + result = invisibleCharsRegex.ReplaceAllString(result, "") + + // Replace HTML comments with a marker + result = htmlCommentsRegex.ReplaceAllString(result, "[HTML_COMMENT]") + + // Replace potentially dangerous HTML elements + result = htmlScriptRegex.ReplaceAllString(result, "[HTML_ELEMENT]") + result = htmlStyleRegex.ReplaceAllString(result, "[HTML_ELEMENT]") + result = htmlIframeRegex.ReplaceAllString(result, "[HTML_ELEMENT]") + result = htmlObjectRegex.ReplaceAllString(result, "[HTML_ELEMENT]") + result = htmlEmbedRegex.ReplaceAllString(result, "[HTML_ELEMENT]") + result = htmlSvgRegex.ReplaceAllString(result, "[HTML_ELEMENT]") + result = htmlMathRegex.ReplaceAllString(result, "[HTML_ELEMENT]") + result = htmlLinkRegex.ReplaceAllString(result, "[HTML_ELEMENT]") + + // Replace HTML attributes that might be used for hiding + result = htmlAttributesRegex.ReplaceAllStringFunc(result, cleanHTMLAttributes) + + // Replace collapsed sections with visible indicator + result = collapsedSectionsRegex.ReplaceAllStringFunc(result, makeCollapsedSectionVisible) + + // Replace very small text with visible indicator + result = smallTextRegex.ReplaceAllString(result, "[SMALL_TEXT]") + + // Normalize excessive whitespace + result = excessiveWhitespaceRegex.ReplaceAllString(result, "\n\n\n") + + return result +} + +// cleanHTMLAttributes removes potentially dangerous attributes from HTML tags +func cleanHTMLAttributes(tag string) string { + // This is a simple implementation that removes style, data-* and hidden attributes + // A more sophisticated implementation would parse the HTML and selectively remove attributes + tagWithoutStyle := regexp.MustCompile(`\s+(?:style|data-[\w-]+|hidden|class)="[^"]*"`).ReplaceAllString(tag, "") + return tagWithoutStyle +} + +// makeCollapsedSectionVisible transforms a
section to make it visible +func makeCollapsedSectionVisible(detailsSection string) string { + // Extract the summary if present + summaryRegex := regexp.MustCompile(`(.*?)`) + summaryMatches := summaryRegex.FindStringSubmatch(detailsSection) + + summary := "Collapsed section" + if len(summaryMatches) > 1 { + summary = summaryMatches[1] + } + + // Extract the content (everything after and before
) + parts := strings.SplitN(detailsSection, "", 2) + content := detailsSection + if len(parts) > 1 { + content = parts[1] + content = strings.TrimSuffix(content, "") + } else { + // No summary tag found, remove the details tags + content = strings.TrimPrefix(content, "
") + content = strings.TrimSuffix(content, "
") + } + + // Format as a visible section + return "\n\n**" + summary + ":**\n" + content + "\n\n" +} \ No newline at end of file diff --git a/pkg/filtering/content_filter_test.go b/pkg/filtering/content_filter_test.go new file mode 100644 index 00000000..bcc859b2 --- /dev/null +++ b/pkg/filtering/content_filter_test.go @@ -0,0 +1,173 @@ +package filtering + +import ( + "testing" +) + +func TestFilterContent(t *testing.T) { + tests := []struct { + name string + input string + expected string + cfg *Config + }{ + { + name: "Empty string", + input: "", + expected: "", + cfg: DefaultConfig(), + }, + { + name: "Normal text without hidden content", + input: "This is normal text without any hidden content.", + expected: "This is normal text without any hidden content.", + cfg: DefaultConfig(), + }, + { + name: "Text with invisible characters", + input: "Hidden\u200Bcharacters\u200Bin\u200Bthis\u200Btext", + expected: "Hiddencharactersinthistext", + cfg: DefaultConfig(), + }, + { + name: "Text with HTML comments", + input: "This has a in it.", + expected: "This has a [HTML_COMMENT] in it.", + cfg: DefaultConfig(), + }, + { + name: "Text with HTML elements", + input: "This has scripts.", + expected: "This has [HTML_ELEMENT] scripts.", + cfg: DefaultConfig(), + }, + { + name: "Text with details/summary", + input: "Collapsed content:
Click meHidden content
", + expected: "Collapsed content: \n\n**Click me:**\nHidden content\n\n", + cfg: DefaultConfig(), + }, + { + name: "Text with small font", + input: "This has hidden tiny text in it.", + expected: "This has hidden tiny text in it.", + cfg: DefaultConfig(), + }, + { + name: "Text with excessive whitespace", + input: "Line 1\n\n\n\n\n\nLine 2", + expected: "Line 1\n\n\nLine 2", + cfg: DefaultConfig(), + }, + { + name: "Text with HTML attributes", + input: "

Hidden paragraph

", + expected: "

Hidden paragraph

", + cfg: DefaultConfig(), + }, + { + name: "Filtering disabled", + input: "Hidden\u200Bcharacters and ", + expected: "Hidden\u200Bcharacters and ", + cfg: &Config{DisableContentFiltering: true}, + }, + { + name: "Nil config uses default (filtering enabled)", + input: "Hidden\u200Bcharacters", + expected: "Hiddencharacters", + cfg: nil, + }, + { + name: "Normal markdown with code blocks", + input: "# Title\n\n```go\nfunc main() {\n fmt.Println(\"Hello, world!\")\n}\n```", + expected: "# Title\n\n```go\nfunc main() {\n fmt.Println(\"Hello, world!\")\n}\n```", + cfg: DefaultConfig(), + }, + { + name: "GitHub flavored markdown with tables", + input: "| Header 1 | Header 2 |\n| -------- | -------- |\n| Cell 1 | Cell 2 |", + expected: "| Header 1 | Header 2 |\n| -------- | -------- |\n| Cell 1 | Cell 2 |", + cfg: DefaultConfig(), + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + result := FilterContent(tc.input, tc.cfg) + if result != tc.expected { + t.Errorf("FilterContent() = %q, want %q", result, tc.expected) + } + }) + } +} + +func TestMakeCollapsedSectionVisible(t *testing.T) { + tests := []struct { + name string + input string + expected string + }{ + { + name: "Simple details/summary", + input: "
Click meHidden content
", + expected: "\n\n**Click me:**\nHidden content\n\n", + }, + { + name: "Details without summary", + input: "
Hidden content
", + expected: "\n\n**Collapsed section:**\nHidden content\n\n", + }, + { + name: "Nested content", + input: "
OuterContent
InnerNested
", + expected: "\n\n**Outer:**\nContent
InnerNested
\n\n", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + result := makeCollapsedSectionVisible(tc.input) + if result != tc.expected { + t.Errorf("makeCollapsedSectionVisible() = %q, want %q", result, tc.expected) + } + }) + } +} + +func TestCleanHTMLAttributes(t *testing.T) { + tests := []struct { + name string + input string + expected string + }{ + { + name: "Tag with style attribute", + input: "

Hidden

", + expected: "

Hidden

", + }, + { + name: "Tag with data attribute", + input: "

Hidden

", + expected: "

Hidden

", + }, + { + name: "Tag with multiple attributes", + input: "

Hidden

", + expected: "

Hidden

", + }, + { + name: "Tag with allowed attributes", + input: "Link", + expected: "Link", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + result := cleanHTMLAttributes(tc.input) + if result != tc.expected { + t.Errorf("cleanHTMLAttributes() = %q, want %q", result, tc.expected) + } + }) + } +} \ No newline at end of file From 09c5a0a77347adb88cfd04b33c3ef95094b8edac Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 23 May 2025 10:52:28 +0000 Subject: [PATCH 3/5] Add content filtering flag to CLI and server configuration Co-authored-by: SamMorrowDrums <4811358+SamMorrowDrums@users.noreply.github.com> --- cmd/github-mcp-server/main.go | 21 ++++++++++++--------- internal/ghmcp/server.go | 21 ++++++++++++++------- 2 files changed, 26 insertions(+), 16 deletions(-) diff --git a/cmd/github-mcp-server/main.go b/cmd/github-mcp-server/main.go index fb716f78..bee5e6b0 100644 --- a/cmd/github-mcp-server/main.go +++ b/cmd/github-mcp-server/main.go @@ -44,15 +44,16 @@ var ( } stdioServerConfig := ghmcp.StdioServerConfig{ - Version: version, - Host: viper.GetString("host"), - Token: token, - EnabledToolsets: enabledToolsets, - DynamicToolsets: viper.GetBool("dynamic_toolsets"), - ReadOnly: viper.GetBool("read-only"), - ExportTranslations: viper.GetBool("export-translations"), - EnableCommandLogging: viper.GetBool("enable-command-logging"), - LogFilePath: viper.GetString("log-file"), + Version: version, + Host: viper.GetString("host"), + Token: token, + EnabledToolsets: enabledToolsets, + DynamicToolsets: viper.GetBool("dynamic_toolsets"), + ReadOnly: viper.GetBool("read-only"), + DisableContentFiltering: viper.GetBool("disable-content-filtering"), + ExportTranslations: viper.GetBool("export-translations"), + EnableCommandLogging: viper.GetBool("enable-command-logging"), + LogFilePath: viper.GetString("log-file"), } return ghmcp.RunStdioServer(stdioServerConfig) @@ -73,6 +74,7 @@ func init() { rootCmd.PersistentFlags().Bool("enable-command-logging", false, "When enabled, the server will log all command requests and responses to the log file") rootCmd.PersistentFlags().Bool("export-translations", false, "Save translations to a JSON file") rootCmd.PersistentFlags().String("gh-host", "", "Specify the GitHub hostname (for GitHub Enterprise etc.)") + rootCmd.PersistentFlags().Bool("disable-content-filtering", false, "Disable filtering of invisible characters and hidden content from GitHub issues, PRs, and comments") // Bind flag to viper _ = viper.BindPFlag("toolsets", rootCmd.PersistentFlags().Lookup("toolsets")) @@ -82,6 +84,7 @@ func init() { _ = viper.BindPFlag("enable-command-logging", rootCmd.PersistentFlags().Lookup("enable-command-logging")) _ = viper.BindPFlag("export-translations", rootCmd.PersistentFlags().Lookup("export-translations")) _ = viper.BindPFlag("host", rootCmd.PersistentFlags().Lookup("gh-host")) + _ = viper.BindPFlag("disable-content-filtering", rootCmd.PersistentFlags().Lookup("disable-content-filtering")) // Add subcommands rootCmd.AddCommand(stdioCmd) diff --git a/internal/ghmcp/server.go b/internal/ghmcp/server.go index a75a9e0c..e290a588 100644 --- a/internal/ghmcp/server.go +++ b/internal/ghmcp/server.go @@ -43,6 +43,9 @@ type MCPServerConfig struct { // ReadOnly indicates if we should only offer read-only tools ReadOnly bool + // DisableContentFiltering disables filtering of invisible characters and hidden content + DisableContentFiltering bool + // Translator provides translated text for the server tooling Translator translations.TranslationHelperFunc } @@ -160,6 +163,9 @@ type StdioServerConfig struct { // ReadOnly indicates if we should only register read-only tools ReadOnly bool + // DisableContentFiltering disables filtering of invisible characters and hidden content + DisableContentFiltering bool + // ExportTranslations indicates if we should export translations // See: https://github.com/github/github-mcp-server?tab=readme-ov-file#i18n--overriding-descriptions ExportTranslations bool @@ -180,13 +186,14 @@ func RunStdioServer(cfg StdioServerConfig) error { t, dumpTranslations := translations.TranslationHelper() ghServer, err := NewMCPServer(MCPServerConfig{ - Version: cfg.Version, - Host: cfg.Host, - Token: cfg.Token, - EnabledToolsets: cfg.EnabledToolsets, - DynamicToolsets: cfg.DynamicToolsets, - ReadOnly: cfg.ReadOnly, - Translator: t, + Version: cfg.Version, + Host: cfg.Host, + Token: cfg.Token, + EnabledToolsets: cfg.EnabledToolsets, + DynamicToolsets: cfg.DynamicToolsets, + ReadOnly: cfg.ReadOnly, + DisableContentFiltering: cfg.DisableContentFiltering, + Translator: t, }) if err != nil { return fmt.Errorf("failed to create MCP server: %w", err) From 67d6012e7f25a12809c9ca875f776286682d0ff6 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 23 May 2025 11:46:23 +0000 Subject: [PATCH 4/5] Implement content filtering for issues, PRs, and comments Co-authored-by: SamMorrowDrums <4811358+SamMorrowDrums@users.noreply.github.com> --- README.md | 35 ++++ internal/ghmcp/server.go | 5 +- pkg/github/filtering.go | 205 +++++++++++++++++++++ pkg/github/filtering_test.go | 345 +++++++++++++++++++++++++++++++++++ pkg/github/issues.go | 42 ++++- pkg/github/pullrequests.go | 36 +++- pkg/github/server.go | 19 +- 7 files changed, 677 insertions(+), 10 deletions(-) create mode 100644 pkg/github/filtering.go create mode 100644 pkg/github/filtering_test.go diff --git a/README.md b/README.md index 352bb50e..ea8a821a 100644 --- a/README.md +++ b/README.md @@ -219,6 +219,41 @@ docker run -i --rm \ ghcr.io/github/github-mcp-server ``` +## Content Filtering + +The GitHub MCP Server includes a content filtering feature that removes invisible characters and hidden content from GitHub issues, PRs, and comments. This helps prevent potential security risks and ensures better readability of content. + +### What Gets Filtered + +- **Invisible Unicode Characters**: Zero-width spaces, zero-width joiners, zero-width non-joiners, bidirectional marks, and other invisible Unicode characters +- **HTML Comments**: Comments that might contain hidden information +- **Hidden HTML Elements**: Script, style, iframe, and other potentially dangerous HTML elements +- **Collapsed Sections**: Details/summary elements that might hide content +- **Very Small Text**: Content with extremely small font size + +### Controlling Content Filtering + +Content filtering is enabled by default. You can disable it using the `--disable-content-filtering` flag: + +```bash +github-mcp-server --disable-content-filtering +``` + +Or using the environment variable: + +```bash +GITHUB_DISABLE_CONTENT_FILTERING=1 github-mcp-server +``` + +When using Docker, you can set the environment variable: + +```bash +docker run -i --rm \ + -e GITHUB_PERSONAL_ACCESS_TOKEN= \ + -e GITHUB_DISABLE_CONTENT_FILTERING=1 \ + ghcr.io/github/github-mcp-server +``` + ## GitHub Enterprise Server The flag `--gh-host` and the environment variable `GITHUB_HOST` can be used to set diff --git a/internal/ghmcp/server.go b/internal/ghmcp/server.go index e290a588..f5906da5 100644 --- a/internal/ghmcp/server.go +++ b/internal/ghmcp/server.go @@ -94,7 +94,10 @@ func NewMCPServer(cfg MCPServerConfig) (*server.MCPServer, error) { OnBeforeInitialize: []server.OnBeforeInitializeFunc{beforeInit}, } - ghServer := github.NewServer(cfg.Version, server.WithHooks(hooks)) + ghServer := github.NewServerWithConfig(github.ServerConfig{ + Version: cfg.Version, + DisableContentFiltering: cfg.DisableContentFiltering, + }, server.WithHooks(hooks)) enabledToolsets := cfg.EnabledToolsets if cfg.DynamicToolsets { diff --git a/pkg/github/filtering.go b/pkg/github/filtering.go new file mode 100644 index 00000000..1c645a40 --- /dev/null +++ b/pkg/github/filtering.go @@ -0,0 +1,205 @@ +package github + +import ( + "github.com/github/github-mcp-server/pkg/filtering" + "github.com/google/go-github/v69/github" +) + +// ContentFilteringConfig holds configuration for content filtering +type ContentFilteringConfig struct { + // DisableContentFiltering disables all content filtering when true + DisableContentFiltering bool +} + +// DefaultContentFilteringConfig returns the default content filtering configuration +func DefaultContentFilteringConfig() *ContentFilteringConfig { + return &ContentFilteringConfig{ + DisableContentFiltering: false, + } +} + +// FilterIssue applies content filtering to issue bodies and titles +func FilterIssue(issue *github.Issue, cfg *ContentFilteringConfig) *github.Issue { + if issue == nil { + return nil + } + + // Don't modify the original issue, create a copy + filteredIssue := *issue + + // Filter the body if present + if issue.Body != nil { + filteredBody := filtering.FilterContent(*issue.Body, &filtering.Config{ + DisableContentFiltering: cfg.DisableContentFiltering, + }) + filteredIssue.Body = github.Ptr(filteredBody) + } + + // Filter the title if present + if issue.Title != nil { + filteredTitle := filtering.FilterContent(*issue.Title, &filtering.Config{ + DisableContentFiltering: cfg.DisableContentFiltering, + }) + filteredIssue.Title = github.Ptr(filteredTitle) + } + + return &filteredIssue +} + +// FilterIssues applies content filtering to a list of issues +func FilterIssues(issues []*github.Issue, cfg *ContentFilteringConfig) []*github.Issue { + if issues == nil { + return nil + } + + filteredIssues := make([]*github.Issue, len(issues)) + for i, issue := range issues { + filteredIssues[i] = FilterIssue(issue, cfg) + } + + return filteredIssues +} + +// FilterPullRequest applies content filtering to pull request bodies and titles +func FilterPullRequest(pr *github.PullRequest, cfg *ContentFilteringConfig) *github.PullRequest { + if pr == nil { + return nil + } + + // Don't modify the original PR, create a copy + filteredPR := *pr + + // Filter the body if present + if pr.Body != nil { + filteredBody := filtering.FilterContent(*pr.Body, &filtering.Config{ + DisableContentFiltering: cfg.DisableContentFiltering, + }) + filteredPR.Body = github.Ptr(filteredBody) + } + + // Filter the title if present + if pr.Title != nil { + filteredTitle := filtering.FilterContent(*pr.Title, &filtering.Config{ + DisableContentFiltering: cfg.DisableContentFiltering, + }) + filteredPR.Title = github.Ptr(filteredTitle) + } + + return &filteredPR +} + +// FilterPullRequests applies content filtering to a list of pull requests +func FilterPullRequests(prs []*github.PullRequest, cfg *ContentFilteringConfig) []*github.PullRequest { + if prs == nil { + return nil + } + + filteredPRs := make([]*github.PullRequest, len(prs)) + for i, pr := range prs { + filteredPRs[i] = FilterPullRequest(pr, cfg) + } + + return filteredPRs +} + +// FilterIssueComment applies content filtering to issue comment bodies +func FilterIssueComment(comment *github.IssueComment, cfg *ContentFilteringConfig) *github.IssueComment { + if comment == nil { + return nil + } + + // Don't modify the original comment, create a copy + filteredComment := *comment + + // Filter the body if present + if comment.Body != nil { + filteredBody := filtering.FilterContent(*comment.Body, &filtering.Config{ + DisableContentFiltering: cfg.DisableContentFiltering, + }) + filteredComment.Body = github.Ptr(filteredBody) + } + + return &filteredComment +} + +// FilterIssueComments applies content filtering to a list of issue comments +func FilterIssueComments(comments []*github.IssueComment, cfg *ContentFilteringConfig) []*github.IssueComment { + if comments == nil { + return nil + } + + filteredComments := make([]*github.IssueComment, len(comments)) + for i, comment := range comments { + filteredComments[i] = FilterIssueComment(comment, cfg) + } + + return filteredComments +} + +// FilterPullRequestComment applies content filtering to pull request comment bodies +func FilterPullRequestComment(comment *github.PullRequestComment, cfg *ContentFilteringConfig) *github.PullRequestComment { + if comment == nil { + return nil + } + + // Don't modify the original comment, create a copy + filteredComment := *comment + + // Filter the body if present + if comment.Body != nil { + filteredBody := filtering.FilterContent(*comment.Body, &filtering.Config{ + DisableContentFiltering: cfg.DisableContentFiltering, + }) + filteredComment.Body = github.Ptr(filteredBody) + } + + return &filteredComment +} + +// FilterPullRequestComments applies content filtering to a list of pull request comments +func FilterPullRequestComments(comments []*github.PullRequestComment, cfg *ContentFilteringConfig) []*github.PullRequestComment { + if comments == nil { + return nil + } + + filteredComments := make([]*github.PullRequestComment, len(comments)) + for i, comment := range comments { + filteredComments[i] = FilterPullRequestComment(comment, cfg) + } + + return filteredComments +} + +// FilterPullRequestReview applies content filtering to pull request review bodies +func FilterPullRequestReview(review *github.PullRequestReview, cfg *ContentFilteringConfig) *github.PullRequestReview { + if review == nil { + return nil + } + + // Don't modify the original review, create a copy + filteredReview := *review + + // Filter the body if present + if review.Body != nil { + filteredBody := filtering.FilterContent(*review.Body, &filtering.Config{ + DisableContentFiltering: cfg.DisableContentFiltering, + }) + filteredReview.Body = github.Ptr(filteredBody) + } + + return &filteredReview +} + +// FilterPullRequestReviews applies content filtering to a list of pull request reviews +func FilterPullRequestReviews(reviews []*github.PullRequestReview, cfg *ContentFilteringConfig) []*github.PullRequestReview { + if reviews == nil { + return nil + } + + filteredReviews := make([]*github.PullRequestReview, len(reviews)) + for i, review := range reviews { + filteredReviews[i] = FilterPullRequestReview(review, cfg) + } + + return filteredReviews +} \ No newline at end of file diff --git a/pkg/github/filtering_test.go b/pkg/github/filtering_test.go new file mode 100644 index 00000000..8a8a97e1 --- /dev/null +++ b/pkg/github/filtering_test.go @@ -0,0 +1,345 @@ +package github + +import ( + "testing" + + "github.com/google/go-github/v69/github" +) + +func TestFilterIssue(t *testing.T) { + tests := []struct { + name string + issue *github.Issue + filterOn bool + expected *github.Issue + }{ + { + name: "nil issue", + issue: nil, + filterOn: true, + expected: nil, + }, + { + name: "no invisible characters", + issue: &github.Issue{ + Title: github.Ptr("Test Issue"), + Body: github.Ptr("This is a test issue"), + }, + filterOn: true, + expected: &github.Issue{ + Title: github.Ptr("Test Issue"), + Body: github.Ptr("This is a test issue"), + }, + }, + { + name: "with invisible characters", + issue: &github.Issue{ + Title: github.Ptr("Test\u200BIssue"), + Body: github.Ptr("This\u200Bis a test issue"), + }, + filterOn: true, + expected: &github.Issue{ + Title: github.Ptr("TestIssue"), + Body: github.Ptr("Thisis a test issue"), + }, + }, + { + name: "with HTML comments", + issue: &github.Issue{ + Title: github.Ptr("Test Issue"), + Body: github.Ptr("This is a test issue"), + }, + filterOn: true, + expected: &github.Issue{ + Title: github.Ptr("Test Issue"), + Body: github.Ptr("This is a [HTML_COMMENT] test issue"), + }, + }, + { + name: "with filtering disabled", + issue: &github.Issue{ + Title: github.Ptr("Test\u200BIssue"), + Body: github.Ptr("This\u200Bis a test issue"), + }, + filterOn: false, + expected: &github.Issue{ + Title: github.Ptr("Test\u200BIssue"), + Body: github.Ptr("This\u200Bis a test issue"), + }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + cfg := &ContentFilteringConfig{ + DisableContentFiltering: !tc.filterOn, + } + result := FilterIssue(tc.issue, cfg) + + // For nil input, we expect nil output + if tc.issue == nil { + if result != nil { + t.Fatalf("FilterIssue() = %v, want %v", result, nil) + } + return + } + + // Check title + if *result.Title != *tc.expected.Title { + t.Errorf("FilterIssue().Title = %q, want %q", *result.Title, *tc.expected.Title) + } + + // Check body + if *result.Body != *tc.expected.Body { + t.Errorf("FilterIssue().Body = %q, want %q", *result.Body, *tc.expected.Body) + } + }) + } +} + +func TestFilterPullRequest(t *testing.T) { + tests := []struct { + name string + pr *github.PullRequest + filterOn bool + expected *github.PullRequest + }{ + { + name: "nil pull request", + pr: nil, + filterOn: true, + expected: nil, + }, + { + name: "no invisible characters", + pr: &github.PullRequest{ + Title: github.Ptr("Test PR"), + Body: github.Ptr("This is a test PR"), + }, + filterOn: true, + expected: &github.PullRequest{ + Title: github.Ptr("Test PR"), + Body: github.Ptr("This is a test PR"), + }, + }, + { + name: "with invisible characters", + pr: &github.PullRequest{ + Title: github.Ptr("Test\u200BPR"), + Body: github.Ptr("This\u200Bis a test PR"), + }, + filterOn: true, + expected: &github.PullRequest{ + Title: github.Ptr("TestPR"), + Body: github.Ptr("Thisis a test PR"), + }, + }, + { + name: "with HTML comments", + pr: &github.PullRequest{ + Title: github.Ptr("Test PR"), + Body: github.Ptr("This is a test PR"), + }, + filterOn: true, + expected: &github.PullRequest{ + Title: github.Ptr("Test PR"), + Body: github.Ptr("This is a [HTML_COMMENT] test PR"), + }, + }, + { + name: "with filtering disabled", + pr: &github.PullRequest{ + Title: github.Ptr("Test\u200BPR"), + Body: github.Ptr("This\u200Bis a test PR"), + }, + filterOn: false, + expected: &github.PullRequest{ + Title: github.Ptr("Test\u200BPR"), + Body: github.Ptr("This\u200Bis a test PR"), + }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + cfg := &ContentFilteringConfig{ + DisableContentFiltering: !tc.filterOn, + } + result := FilterPullRequest(tc.pr, cfg) + + // For nil input, we expect nil output + if tc.pr == nil { + if result != nil { + t.Fatalf("FilterPullRequest() = %v, want %v", result, nil) + } + return + } + + // Check title + if *result.Title != *tc.expected.Title { + t.Errorf("FilterPullRequest().Title = %q, want %q", *result.Title, *tc.expected.Title) + } + + // Check body + if *result.Body != *tc.expected.Body { + t.Errorf("FilterPullRequest().Body = %q, want %q", *result.Body, *tc.expected.Body) + } + }) + } +} + +func TestFilterIssueComment(t *testing.T) { + tests := []struct { + name string + comment *github.IssueComment + filterOn bool + expected *github.IssueComment + }{ + { + name: "nil comment", + comment: nil, + filterOn: true, + expected: nil, + }, + { + name: "no invisible characters", + comment: &github.IssueComment{ + Body: github.Ptr("This is a test comment"), + }, + filterOn: true, + expected: &github.IssueComment{ + Body: github.Ptr("This is a test comment"), + }, + }, + { + name: "with invisible characters", + comment: &github.IssueComment{ + Body: github.Ptr("This\u200Bis a test comment"), + }, + filterOn: true, + expected: &github.IssueComment{ + Body: github.Ptr("Thisis a test comment"), + }, + }, + { + name: "with HTML comments", + comment: &github.IssueComment{ + Body: github.Ptr("This is a test comment"), + }, + filterOn: true, + expected: &github.IssueComment{ + Body: github.Ptr("This is a [HTML_COMMENT] test comment"), + }, + }, + { + name: "with filtering disabled", + comment: &github.IssueComment{ + Body: github.Ptr("This\u200Bis a test comment"), + }, + filterOn: false, + expected: &github.IssueComment{ + Body: github.Ptr("This\u200Bis a test comment"), + }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + cfg := &ContentFilteringConfig{ + DisableContentFiltering: !tc.filterOn, + } + result := FilterIssueComment(tc.comment, cfg) + + // For nil input, we expect nil output + if tc.comment == nil { + if result != nil { + t.Fatalf("FilterIssueComment() = %v, want %v", result, nil) + } + return + } + + // Check body + if *result.Body != *tc.expected.Body { + t.Errorf("FilterIssueComment().Body = %q, want %q", *result.Body, *tc.expected.Body) + } + }) + } +} + +func TestFilterPullRequestComment(t *testing.T) { + tests := []struct { + name string + comment *github.PullRequestComment + filterOn bool + expected *github.PullRequestComment + }{ + { + name: "nil comment", + comment: nil, + filterOn: true, + expected: nil, + }, + { + name: "no invisible characters", + comment: &github.PullRequestComment{ + Body: github.Ptr("This is a test comment"), + }, + filterOn: true, + expected: &github.PullRequestComment{ + Body: github.Ptr("This is a test comment"), + }, + }, + { + name: "with invisible characters", + comment: &github.PullRequestComment{ + Body: github.Ptr("This\u200Bis a test comment"), + }, + filterOn: true, + expected: &github.PullRequestComment{ + Body: github.Ptr("Thisis a test comment"), + }, + }, + { + name: "with HTML comments", + comment: &github.PullRequestComment{ + Body: github.Ptr("This is a test comment"), + }, + filterOn: true, + expected: &github.PullRequestComment{ + Body: github.Ptr("This is a [HTML_COMMENT] test comment"), + }, + }, + { + name: "with filtering disabled", + comment: &github.PullRequestComment{ + Body: github.Ptr("This\u200Bis a test comment"), + }, + filterOn: false, + expected: &github.PullRequestComment{ + Body: github.Ptr("This\u200Bis a test comment"), + }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + cfg := &ContentFilteringConfig{ + DisableContentFiltering: !tc.filterOn, + } + result := FilterPullRequestComment(tc.comment, cfg) + + // For nil input, we expect nil output + if tc.comment == nil { + if result != nil { + t.Fatalf("FilterPullRequestComment() = %v, want %v", result, nil) + } + return + } + + // Check body + if *result.Body != *tc.expected.Body { + t.Errorf("FilterPullRequestComment().Body = %q, want %q", *result.Body, *tc.expected.Body) + } + }) + } +} \ No newline at end of file diff --git a/pkg/github/issues.go b/pkg/github/issues.go index 68e7a36c..a3cba5f7 100644 --- a/pkg/github/issues.go +++ b/pkg/github/issues.go @@ -70,7 +70,14 @@ func GetIssue(getClient GetClientFn, t translations.TranslationHelperFunc) (tool return mcp.NewToolResultError(fmt.Sprintf("failed to get issue: %s", string(body))), nil } - r, err := json.Marshal(issue) + // Apply content filtering + filterCfg := &ContentFilteringConfig{ + DisableContentFiltering: false, // Default to enabled + } + // TODO: Pass server configuration through client context once it's available + filteredIssue := FilterIssue(issue, filterCfg) + + r, err := json.Marshal(filteredIssue) if err != nil { return nil, fmt.Errorf("failed to marshal issue: %w", err) } @@ -232,6 +239,21 @@ func SearchIssues(getClient GetClientFn, t translations.TranslationHelperFunc) ( return mcp.NewToolResultError(fmt.Sprintf("failed to search issues: %s", string(body))), nil } + // Apply content filtering + filterCfg := &ContentFilteringConfig{ + DisableContentFiltering: false, // Default to enabled + } + // TODO: Pass server configuration through client context once it's available + + // Apply filtering to both issues and pull requests in the search results + if result.Issues != nil { + filteredItems := make([]*github.Issue, len(result.Issues)) + for i, issue := range result.Issues { + filteredItems[i] = FilterIssue(issue, filterCfg) + } + result.Issues = filteredItems + } + r, err := json.Marshal(result) if err != nil { return nil, fmt.Errorf("failed to marshal response: %w", err) @@ -476,7 +498,14 @@ func ListIssues(getClient GetClientFn, t translations.TranslationHelperFunc) (to return mcp.NewToolResultError(fmt.Sprintf("failed to list issues: %s", string(body))), nil } - r, err := json.Marshal(issues) + // Apply content filtering + filterCfg := &ContentFilteringConfig{ + DisableContentFiltering: false, // Default to enabled + } + // TODO: Pass server configuration through client context once it's available + filteredIssues := FilterIssues(issues, filterCfg) + + r, err := json.Marshal(filteredIssues) if err != nil { return nil, fmt.Errorf("failed to marshal issues: %w", err) } @@ -705,7 +734,14 @@ func GetIssueComments(getClient GetClientFn, t translations.TranslationHelperFun return mcp.NewToolResultError(fmt.Sprintf("failed to get issue comments: %s", string(body))), nil } - r, err := json.Marshal(comments) + // Apply content filtering + filterCfg := &ContentFilteringConfig{ + DisableContentFiltering: false, // Default to enabled + } + // TODO: Pass server configuration through client context once it's available + filteredComments := FilterIssueComments(comments, filterCfg) + + r, err := json.Marshal(filteredComments) if err != nil { return nil, fmt.Errorf("failed to marshal response: %w", err) } diff --git a/pkg/github/pullrequests.go b/pkg/github/pullrequests.go index d6dd3f96..bb0d94f5 100644 --- a/pkg/github/pullrequests.go +++ b/pkg/github/pullrequests.go @@ -68,7 +68,14 @@ func GetPullRequest(getClient GetClientFn, t translations.TranslationHelperFunc) return mcp.NewToolResultError(fmt.Sprintf("failed to get pull request: %s", string(body))), nil } - r, err := json.Marshal(pr) + // Apply content filtering + filterCfg := &ContentFilteringConfig{ + DisableContentFiltering: false, // Default to enabled + } + // TODO: Pass server configuration through client context once it's available + filteredPR := FilterPullRequest(pr, filterCfg) + + r, err := json.Marshal(filteredPR) if err != nil { return nil, fmt.Errorf("failed to marshal response: %w", err) } @@ -413,7 +420,14 @@ func ListPullRequests(getClient GetClientFn, t translations.TranslationHelperFun return mcp.NewToolResultError(fmt.Sprintf("failed to list pull requests: %s", string(body))), nil } - r, err := json.Marshal(prs) + // Apply content filtering + filterCfg := &ContentFilteringConfig{ + DisableContentFiltering: false, // Default to enabled + } + // TODO: Pass server configuration through client context once it's available + filteredPRs := FilterPullRequests(prs, filterCfg) + + r, err := json.Marshal(filteredPRs) if err != nil { return nil, fmt.Errorf("failed to marshal response: %w", err) } @@ -788,7 +802,14 @@ func GetPullRequestComments(getClient GetClientFn, t translations.TranslationHel return mcp.NewToolResultError(fmt.Sprintf("failed to get pull request comments: %s", string(body))), nil } - r, err := json.Marshal(comments) + // Apply content filtering + filterCfg := &ContentFilteringConfig{ + DisableContentFiltering: false, // Default to enabled + } + // TODO: Pass server configuration through client context once it's available + filteredComments := FilterPullRequestComments(comments, filterCfg) + + r, err := json.Marshal(filteredComments) if err != nil { return nil, fmt.Errorf("failed to marshal response: %w", err) } @@ -850,7 +871,14 @@ func GetPullRequestReviews(getClient GetClientFn, t translations.TranslationHelp return mcp.NewToolResultError(fmt.Sprintf("failed to get pull request reviews: %s", string(body))), nil } - r, err := json.Marshal(reviews) + // Apply content filtering + filterCfg := &ContentFilteringConfig{ + DisableContentFiltering: false, // Default to enabled + } + // TODO: Pass server configuration through client context once it's available + filteredReviews := FilterPullRequestReviews(reviews, filterCfg) + + r, err := json.Marshal(filteredReviews) if err != nil { return nil, fmt.Errorf("failed to marshal response: %w", err) } diff --git a/pkg/github/server.go b/pkg/github/server.go index e4c24171..dcbcffdf 100644 --- a/pkg/github/server.go +++ b/pkg/github/server.go @@ -9,9 +9,24 @@ import ( "github.com/mark3labs/mcp-go/server" ) -// NewServer creates a new GitHub MCP server with the specified GH client and logger. +// ServerConfig holds configuration for the GitHub MCP server +type ServerConfig struct { + // Version of the server + Version string + + // DisableContentFiltering disables filtering of invisible characters and hidden content + DisableContentFiltering bool +} +// NewServer creates a new GitHub MCP server with the specified GH client and logger. func NewServer(version string, opts ...server.ServerOption) *server.MCPServer { + return NewServerWithConfig(ServerConfig{ + Version: version, + }, opts...) +} + +// NewServerWithConfig creates a new GitHub MCP server with the specified configuration and options. +func NewServerWithConfig(cfg ServerConfig, opts ...server.ServerOption) *server.MCPServer { // Add default options defaultOpts := []server.ServerOption{ server.WithToolCapabilities(true), @@ -23,7 +38,7 @@ func NewServer(version string, opts ...server.ServerOption) *server.MCPServer { // Create a new MCP server s := server.NewMCPServer( "github-mcp-server", - version, + cfg.Version, opts..., ) return s From 2e27e2aca3695fd67634208c2c351dce14688376 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 23 May 2025 21:40:21 +0000 Subject: [PATCH 5/5] Add filters for excessive spaces and tabs Co-authored-by: SamMorrowDrums <4811358+SamMorrowDrums@users.noreply.github.com> --- pkg/filtering/content_filter.go | 12 ++++++++++++ pkg/filtering/content_filter_test.go | 12 ++++++++++++ 2 files changed, 24 insertions(+) diff --git a/pkg/filtering/content_filter.go b/pkg/filtering/content_filter.go index 2d01c4bd..c79c7652 100644 --- a/pkg/filtering/content_filter.go +++ b/pkg/filtering/content_filter.go @@ -37,6 +37,12 @@ var ( // Excessive whitespace (more than 3 consecutive newlines) excessiveWhitespaceRegex = regexp.MustCompile(`\n{4,}`) + + // Excessive spaces (15 or more consecutive spaces) + excessiveSpacesRegex = regexp.MustCompile(` {15,}`) + + // Excessive tabs (6 or more consecutive tabs) + excessiveTabsRegex = regexp.MustCompile(`\t{6,}`) ) // Config holds configuration for content filtering @@ -93,6 +99,12 @@ func FilterContent(input string, cfg *Config) string { // Normalize excessive whitespace result = excessiveWhitespaceRegex.ReplaceAllString(result, "\n\n\n") + + // Normalize excessive spaces + result = excessiveSpacesRegex.ReplaceAllString(result, " ") + + // Normalize excessive tabs + result = excessiveTabsRegex.ReplaceAllString(result, " ") return result } diff --git a/pkg/filtering/content_filter_test.go b/pkg/filtering/content_filter_test.go index bcc859b2..719fd4a7 100644 --- a/pkg/filtering/content_filter_test.go +++ b/pkg/filtering/content_filter_test.go @@ -59,6 +59,18 @@ func TestFilterContent(t *testing.T) { expected: "Line 1\n\n\nLine 2", cfg: DefaultConfig(), }, + { + name: "Text with excessive spaces", + input: "Normal Excessive", + expected: "Normal Excessive", + cfg: DefaultConfig(), + }, + { + name: "Text with excessive tabs", + input: "Normal\t\t\t\t\t\t\t\tExcessive", + expected: "Normal Excessive", + cfg: DefaultConfig(), + }, { name: "Text with HTML attributes", input: "

Hidden paragraph

",