Skip to content

Commit bb3f5b6

Browse files
ericfitzclaude
andcommitted
fix(api): allow template-like patterns in markdown content fields
Remove template injection validation from markdown content fields (Note, TriageNote). These are free-text fields that legitimately contain ${...}, {{...}}, and similar patterns (e.g., Terraform variable references, shell variables). TMI does not evaluate templates in stored content, so SSTI checks were producing false positives that blocked tmi-tf import. Non-markdown fields (names, titles, metadata) retain template injection checks via CheckHTMLInjection. Fixes #223 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent b06b915 commit bb3f5b6

6 files changed

Lines changed: 40 additions & 163 deletions

File tree

.version

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"major": 1,
33
"minor": 3,
4-
"patch": 3,
4+
"patch": 4,
55
"prerelease": ""
66
}

api/markdown_sanitizer.go

Lines changed: 0 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ package api
33
import (
44
"html"
55
"regexp"
6-
"strings"
76

87
"github.com/microcosm-cc/bluemonday"
98
)
@@ -95,12 +94,6 @@ func SanitizeMarkdownContent(content string) string {
9594
return markdownPolicy.Sanitize(content)
9695
}
9796

98-
// codeBlockRegex matches fenced code blocks (```...```)
99-
var codeBlockRegex = regexp.MustCompile("(?s)```[^`]*```")
100-
101-
// inlineCodeRegex matches inline code (`...`)
102-
var inlineCodeRegex = regexp.MustCompile("`[^`]+`")
103-
10497
// SanitizePlainText strips ALL HTML tags from a string, leaving only text content.
10598
// Use this for plain-text fields (e.g., metadata values) that should never contain HTML.
10699
// Unlike SanitizeMarkdownContent, this does not preserve any HTML elements.
@@ -211,23 +204,3 @@ func SanitizePatchOperations(operations []PatchOperation, paths []string) {
211204
}
212205
}
213206
}
214-
215-
// validateTemplateInjectionInMarkdown checks for server-side template injection
216-
// patterns in markdown content. Code blocks are stripped first to avoid false
217-
// positives from code examples. This covers patterns that bluemonday does not
218-
// handle (template expressions are not HTML).
219-
func validateTemplateInjectionInMarkdown(content string) error {
220-
// Remove code blocks to avoid false positives
221-
contentWithoutCodeBlocks := codeBlockRegex.ReplaceAllString(content, "")
222-
contentWithoutCode := inlineCodeRegex.ReplaceAllString(contentWithoutCodeBlocks, "")
223-
224-
// Check template injection patterns (reuses patterns from html_injection_checker.go)
225-
for _, tp := range templateInjectionPatterns {
226-
if strings.Contains(contentWithoutCode, tp.pattern) {
227-
return InvalidInputError(
228-
"Field 'content' contains potentially unsafe " + tp.desc +
229-
" pattern (" + tp.pattern + ")")
230-
}
231-
}
232-
return nil
233-
}

api/markdown_sanitizer_test.go

Lines changed: 20 additions & 83 deletions
Original file line numberDiff line numberDiff line change
@@ -422,95 +422,32 @@ func TestSanitizePatchOperations(t *testing.T) {
422422
})
423423
}
424424

425-
func TestValidateTemplateInjectionInMarkdown(t *testing.T) {
425+
func TestValidateMarkdownContent_AllowsTemplatePatterns(t *testing.T) {
426+
// ValidateMarkdownContent no longer rejects template-like patterns in markdown
427+
// content fields. These fields store free-text data that may legitimately
428+
// contain such syntax (Terraform refs, shell variables, code examples).
426429
tests := []struct {
427-
name string
428-
content string
429-
expectError bool
430-
description string
430+
name string
431+
content string
431432
}{
432-
{
433-
name: "Clean content",
434-
content: "Hello world, this is markdown",
435-
expectError: false,
436-
description: "Plain text should pass",
437-
},
438-
{
439-
name: "Handlebars template expression",
440-
content: "Hello {{ user }} world",
441-
expectError: true,
442-
description: "Template expressions should be rejected",
443-
},
444-
{
445-
name: "Closing template expression",
446-
content: "Hello result }} here",
447-
expectError: true,
448-
description: "Closing template expressions should be rejected",
449-
},
450-
{
451-
name: "JavaScript template literal",
452-
content: "Hello ${ name } world",
453-
expectError: true,
454-
description: "Template interpolation should be rejected",
455-
},
456-
{
457-
name: "GitHub Actions context",
458-
content: "Token: ${{ github.token }}",
459-
expectError: true,
460-
description: "GitHub Actions expressions should be rejected",
461-
},
462-
{
463-
name: "JSP/ASP template tag",
464-
content: "Hello <% code %> world",
465-
expectError: true,
466-
description: "Server template tags should be rejected",
467-
},
468-
{
469-
name: "Spring EL expression",
470-
content: "Hello #{ expr } world",
471-
expectError: true,
472-
description: "Expression language should be rejected",
473-
},
474-
{
475-
name: "Template expression in fenced code block",
476-
content: "```\n{{ user }}\n```",
477-
expectError: false,
478-
description: "Template expressions in code blocks should be allowed",
479-
},
480-
{
481-
name: "Template expression in inline code",
482-
content: "Use `{{ template }}` syntax",
483-
expectError: false,
484-
description: "Template expressions in inline code should be allowed",
485-
},
486-
{
487-
name: "Template expression in code block with language",
488-
content: "```go\nfmt.Println(\"{{ .Name }}\")\n```",
489-
expectError: false,
490-
description: "Template expressions in language-tagged code blocks should be allowed",
491-
},
492-
{
493-
name: "Mixed: template in code block and clean content",
494-
content: "# Title\n\n```\n{{ user }}\n```\n\nSafe content here",
495-
expectError: false,
496-
description: "Template in code block with clean surrounding content should pass",
497-
},
498-
{
499-
name: "Empty content",
500-
content: "",
501-
expectError: false,
502-
description: "Empty content should pass",
503-
},
433+
{"Plain text", "Hello world, this is markdown"},
434+
{"Empty content", ""},
435+
{"Handlebars template expression", "Hello {{ user }} world"},
436+
{"JavaScript template literal", "Hello ${ name } world"},
437+
{"GitHub Actions context", "Token: ${{ github.token }}"},
438+
{"JSP/ASP template tag", "Hello <% code %> world"},
439+
{"Spring EL expression", "Hello #{ expr } world"},
440+
{"Terraform variable interpolation", "| OKE Cluster | `${var.name_prefix}-oke` |"},
441+
{"Terraform module reference", "cluster_id = '${module.kubernetes.cluster_id}'"},
442+
{"Shell variable", "echo ${HOME}/bin"},
443+
{"Template in code block", "```\n{{ user }}\n```"},
444+
{"Template in inline code", "Use `{{ template }}` syntax"},
504445
}
505446

506447
for _, tt := range tests {
507448
t.Run(tt.name, func(t *testing.T) {
508-
err := validateTemplateInjectionInMarkdown(tt.content)
509-
if tt.expectError {
510-
assert.Error(t, err, tt.description)
511-
} else {
512-
assert.NoError(t, err, tt.description)
513-
}
449+
err := ValidateMarkdownContent(tt.content)
450+
assert.NoError(t, err, "Markdown content should accept all template-like patterns")
514451
})
515452
}
516453
}

api/validation_registry.go

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -177,11 +177,12 @@ func ValidateNoHTMLInjection(data any) error {
177177

178178
// ValidateMarkdownContent validates a markdown content string.
179179
// HTML tags are allowed and will be sanitized by bluemonday in the handler layer
180-
// before storage. This function checks only for server-side template injection
181-
// patterns ({{, ${, <%, etc.) which bluemonday does not handle.
182-
// This is the shared validation core used by both Note and TriageNote validators.
183-
func ValidateMarkdownContent(content string) error {
184-
return validateTemplateInjectionInMarkdown(content)
180+
// before storage. Template-like patterns (${, {{, <%, etc.) are permitted in
181+
// markdown content because these fields store free-text data that may legitimately
182+
// contain such syntax (e.g., Terraform references, shell variables, code examples).
183+
// TMI does not evaluate templates in stored content, so SSTI is not a concern here.
184+
func ValidateMarkdownContent(_ string) error {
185+
return nil
185186
}
186187

187188
// ValidateNoteMarkdown validates Note.Content field for dangerous HTML.

api/validation_test.go

Lines changed: 12 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -400,25 +400,21 @@ func TestValidateNoteMarkdown(t *testing.T) {
400400
tests := []struct {
401401
name string
402402
content string
403-
expectError bool
404403
description string
405404
}{
406405
{
407406
name: "Valid Markdown with headings",
408407
content: "# Heading 1\n## Heading 2\n### Heading 3",
409-
expectError: false,
410408
description: "Standard Markdown headings should be allowed",
411409
},
412410
{
413411
name: "Valid Markdown with code block containing JSON",
414412
content: "```json\n{\"option_key_1\": true, \"option_key_2\": \"value\"}\n```",
415-
expectError: false,
416413
description: "JSON in code blocks should not trigger false positives",
417414
},
418415
{
419416
name: "Valid Markdown with inline code",
420417
content: "Use the `onclick` handler in your code",
421-
expectError: false,
422418
description: "Code references to event handlers should be allowed",
423419
},
424420
{
@@ -484,98 +480,77 @@ class LocalizationDeDuplicator:
484480
with open(self.locale_file_path, 'r', encoding='utf-8') as f:
485481
self.localization_data = json.load(f)
486482
` + "```" + ``,
487-
expectError: false,
488483
description: "Complex real-world Markdown example should be allowed",
489484
},
490485
{
491486
name: "HTML script tag allowed (sanitized in handler)",
492487
content: "This is a note with <script>alert('xss')</script> dangerous content",
493-
expectError: false,
494488
description: "Script tags pass validation; sanitized by bluemonday in the handler layer",
495489
},
496490
{
497491
name: "HTML with onclick handler allowed (sanitized in handler)",
498492
content: "Click <a href='#' onclick='alert(1)'>here</a>",
499-
expectError: false,
500493
description: "HTML with event handlers passes validation; sanitized by bluemonday in the handler layer",
501494
},
502495
{
503496
name: "Iframe tag allowed (sanitized in handler)",
504497
content: "Embedded content: <iframe src='http://evil.com'></iframe>",
505-
expectError: false,
506498
description: "Iframe tags pass validation; sanitized by bluemonday in the handler layer",
507499
},
508500
{
509501
name: "HTML img tag allowed (sanitized in handler)",
510502
content: "Image: <img src='x' onerror='alert(1)'>",
511-
expectError: false,
512503
description: "HTML img tags pass validation; sanitized by bluemonday in the handler layer",
513504
},
514505
{
515506
name: "Valid Markdown with link",
516507
content: "Check out [this link](https://example.com)",
517-
expectError: false,
518508
description: "Markdown links should be allowed",
519509
},
520510
{
521511
name: "Valid Markdown with image",
522512
content: "![alt text](https://example.com/image.png)",
523-
expectError: false,
524513
description: "Markdown images should be allowed",
525514
},
526515
{
527516
name: "Valid empty content",
528517
content: "",
529-
expectError: false,
530518
description: "Empty content should pass validation (required validation is separate)",
531519
},
532520
{
533521
name: "HTML paragraph tag allowed (sanitized in handler)",
534522
content: "<p>This is HTML</p>",
535-
expectError: false,
536523
description: "HTML paragraph tags pass validation; sanitized by bluemonday in the handler layer",
537524
},
538525
{
539526
name: "HTML div tag allowed (sanitized in handler)",
540527
content: "<div class='container'>Content</div>",
541-
expectError: false,
542528
description: "HTML div tags pass validation; sanitized by bluemonday in the handler layer",
543529
},
544530
{
545531
name: "Valid Markdown with special characters",
546532
content: "Special chars: & < > \" ' are allowed in plain text",
547-
expectError: false,
548533
description: "Special characters in plain text should be allowed",
549534
},
550535
{
551-
name: "Template expression rejected",
536+
name: "Template expression allowed in markdown",
552537
content: "Hello {{ user }} world",
553-
expectError: true,
554-
description: "Template expressions should be rejected",
538+
description: "Template expressions are permitted in markdown content",
555539
},
556540
{
557-
name: "Template expression in code block allowed",
558-
content: "```\n{{ user }}\n```",
559-
expectError: false,
560-
description: "Template expressions in code blocks should be allowed",
561-
},
562-
{
563-
name: "Template expression in inline code allowed",
564-
content: "Use `{{ template }}` syntax",
565-
expectError: false,
566-
description: "Template expressions in inline code should be allowed",
567-
},
568-
{
569-
name: "JavaScript template literal rejected",
541+
name: "JavaScript template literal allowed in markdown",
570542
content: "Hello ${ name } world",
571-
expectError: true,
572-
description: "JavaScript template interpolation should be rejected",
543+
description: "Template interpolation is permitted in markdown content",
573544
},
574545
{
575-
name: "Server template tag rejected",
546+
name: "Server template tag allowed in markdown",
576547
content: "Hello <% code %> world",
577-
expectError: true,
578-
description: "Server template tags should be rejected",
548+
description: "Server template tags are permitted in markdown content",
549+
},
550+
{
551+
name: "Terraform variable interpolation",
552+
content: "| OKE Cluster | `${var.name_prefix}-oke` |",
553+
description: "Terraform variable syntax is permitted in markdown content",
579554
},
580555
}
581556

@@ -587,17 +562,8 @@ class LocalizationDeDuplicator:
587562
Name: "Test Note",
588563
}
589564

590-
// Run validation
591565
err := ValidateNoteMarkdown(note)
592-
593-
if tt.expectError {
594-
assert.Error(t, err, tt.description)
595-
if err != nil {
596-
assert.Contains(t, err.Error(), "unsafe", "Error should mention unsafe content")
597-
}
598-
} else {
599-
assert.NoError(t, err, tt.description)
600-
}
566+
assert.NoError(t, err, tt.description)
601567
})
602568
}
603569
}

api/version.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ var (
5050
// Minor version number
5151
VersionMinor = "3"
5252
// Patch version number
53-
VersionPatch = "3"
53+
VersionPatch = "4"
5454
// VersionPreRelease is the pre-release label (e.g., "rc.0", "beta.1"), empty for stable releases
5555
VersionPreRelease = ""
5656
// GitCommit is the git commit hash from build

0 commit comments

Comments
 (0)