feat: escape code blocks for markdown formatting #6089
Conversation
WalkthroughThe changes add a new utility function for safely formatting Markdown code blocks by escaping backticks and backslashes. The Changes
Sequence Diagram(s)sequenceDiagram
participant Caller as Caller
participant MF as MarkdownFormatter
participant ESC as escapeCodeBlockMarkdown
Caller->>MF: Call CreateCodeBlock(content)
MF->>ESC: Escape backticks and backslashes
ESC-->>MF: Return escaped content
MF->>Caller: Return formatted Markdown code block
sequenceDiagram
participant Caller as Caller
participant JF as Jira Formatter
Caller->>JF: Call CreateCodeBlock(content)
JF->>JF: Remove "{code}" from content
JF->>Caller: Return cleaned, formatted code block
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
pkg/reporting/exporters/markdown/util/markdown_formatter.go (1)
31-46: Well-designed escaping function with clear documentation.The
escapeCodeBlockMarkdownfunction is well-implemented and properly documented, focusing on the minimum set of characters that need escaping for security while preserving readability.One suggestion for a minor improvement:
func escapeCodeBlockMarkdown(text string) string { - minimalChars := []string{ - "\\", "`", - } + minimalChars := []string{"\\", "`"} result := text for _, char := range minimalChars { result = strings.ReplaceAll(result, char, "\\"+char) } return result }pkg/reporting/format/format_utils_test.go (1)
52-75: Excellent security test for Markdown injection.This test is a valuable addition that verifies protection against Markdown injection attacks in the report description. The test correctly checks that malicious payload patterns cannot break out of code blocks.
Minor suggestion: Consider removing the debug print statement on line 71:
result := CreateReportDescription(event, &util.MarkdownFormatter{}, false) - fmt.Println(result) require.NotContains(t, result, "```\r\n\r\nReferences:\r\n- https://rce.ee/pwned")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
pkg/reporting/exporters/markdown/util/markdown_formatter.go(3 hunks)pkg/reporting/exporters/markdown/util/markdown_utils_test.go(1 hunks)pkg/reporting/format/format_utils_test.go(2 hunks)pkg/reporting/trackers/jira/jira.go(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Tests (macOS-latest)
- GitHub Check: Tests (windows-latest)
- GitHub Check: Tests (ubuntu-latest)
🔇 Additional comments (6)
pkg/reporting/trackers/jira/jira.go (1)
31-33: Good implementation of content escaping for Jira code blocks.This change correctly prevents Jira formatting issues by removing any
{code}tags that might be present in the content, which could otherwise prematurely close code blocks.pkg/reporting/exporters/markdown/util/markdown_formatter.go (2)
4-5: Added required import for string operations.The addition of the strings package is necessary for the new escaping functionality.
14-16: Good implementation of code block content escaping.The code now properly escapes content passed to code blocks, preventing potential markdown injection vulnerabilities.
pkg/reporting/format/format_utils_test.go (2)
3-5: Added necessary import for new test function.The fmt package is correctly added for the new test.
11-12: Added necessary import for output package.This import is required for working with the ResultEvent type in the new test.
pkg/reporting/exporters/markdown/util/markdown_utils_test.go (1)
93-142: Comprehensive test coverage for the escaping function.Excellent test suite for the
escapeCodeBlockMarkdownfunction with thorough coverage of various edge cases:
- Regular text without special characters
- Text with backticks
- Text with backslashes
- Text with both backticks and backslashes
- Complete code blocks
- Already escaped characters
- Multiple consecutive backticks
This ensures the escaping logic is robust against various input patterns.
Proposed changes
Does escaping of code blocks
Checklist
Summary by CodeRabbit
New Features
Bug Fixes