From b7b1b64aa88305bfc1f79f621a1032ec90f1902a Mon Sep 17 00:00:00 2001
From: "google-labs-jules[bot]"
<161369871+google-labs-jules[bot]@users.noreply.github.com>
Date: Tue, 27 Jan 2026 07:29:20 +0000
Subject: [PATCH] Fix Stored XSS in HTML Output
- Added `github.com/microcosm-cc/bluemonday` dependency.
- Implemented `bluemonday.UGCPolicy()` sanitization in `formatHTML` and `formatJSON`.
- Added `api/xss_test.go` regression test.
- Updated `.jules/sentinel.md`.
Co-authored-by: lucasew <15693688+lucasew@users.noreply.github.com>
---
.jules/sentinel.md | 8 +++++
api/index.go | 7 ++--
api/reconstruct_test.go | 12 +++----
api/xss_test.go | 80 +++++++++++++++++++++++++++++++++++++++++
go.mod | 3 ++
go.sum | 6 ++++
6 files changed, 108 insertions(+), 8 deletions(-)
create mode 100644 api/xss_test.go
diff --git a/.jules/sentinel.md b/.jules/sentinel.md
index 2c991fb..45990ee 100644
--- a/.jules/sentinel.md
+++ b/.jules/sentinel.md
@@ -13,3 +13,11 @@
**Learning:** `ip.IsPrivate()` and `ip.IsLoopback()` are not sufficient to block all local traffic. The concept of "Unspecified" addresses (all zeros) must also be explicitly handled when validating IPs for SSRF protection in Go.
**Prevention:** When implementing a safe dialer to prevent SSRF, always include `ip.IsUnspecified()` in the list of blocked IP characteristics, in addition to private, loopback, and link-local addresses.
+
+## 2026-01-27 - Fix Stored XSS in HTML Output
+
+**Vulnerability:** The application was vulnerable to Stored Cross-Site Scripting (XSS) because it rendered HTML content from the `go-readability` library without adequate sanitization. Although `go-readability` performs some cleaning, it does not guarantee safety against all XSS vectors (e.g., `onerror` handlers).
+
+**Learning:** Relying solely on a parsing library for security is insufficient. Libraries designed for content extraction (like `readability`) often prioritize preserving content structure over strict security sanitization. A dedicated sanitizer is required before rendering user-controlled HTML.
+
+**Prevention:** We integrated `bluemonday` with `UGCPolicy` to explicitly sanitize the HTML content before it is injected into the HTML template or JSON response. This ensures that any malicious tags or attributes missed by the parser are stripped.
diff --git a/api/index.go b/api/index.go
index b64870e..764ab3e 100644
--- a/api/index.go
+++ b/api/index.go
@@ -26,6 +26,7 @@ import (
"codeberg.org/readeck/go-readability/v2"
"github.com/mattn/godown"
+ "github.com/microcosm-cc/bluemonday"
"golang.org/x/net/html"
)
@@ -57,6 +58,8 @@ const Template = `
var (
DefaultTemplate = template.Must(template.New("article").Parse(Template))
ReadabilityParser = readability.NewParser()
+ // sanitizer is used to clean up HTML content to prevent XSS
+ sanitizer = bluemonday.UGCPolicy()
// httpClient used for fetching remote articles with timeouts and redirect policy
httpClient = &http.Client{
Transport: &http.Transport{
@@ -270,7 +273,7 @@ func formatHTML(w http.ResponseWriter, article readability.Article, contentBuf *
Content template.HTML
}{
Title: article.Title(),
- Content: template.HTML(contentBuf.String()),
+ Content: template.HTML(sanitizer.Sanitize(contentBuf.String())),
}
if err := DefaultTemplate.Execute(w, data); err != nil {
// at this point, we can't write a JSON error, so we log it
@@ -297,7 +300,7 @@ func formatJSON(w http.ResponseWriter, article readability.Article, buf *bytes.B
w.Header().Set("Content-Type", "application/json")
if err := json.NewEncoder(w).Encode(map[string]string{
"title": article.Title(),
- "content": buf.String(),
+ "content": sanitizer.Sanitize(buf.String()),
}); err != nil {
log.Printf("error encoding json: %v", err)
}
diff --git a/api/reconstruct_test.go b/api/reconstruct_test.go
index 369de89..22d44ff 100644
--- a/api/reconstruct_test.go
+++ b/api/reconstruct_test.go
@@ -63,12 +63,12 @@ func TestReconstructTargetURL(t *testing.T) {
gotU, _ := url.Parse(got)
expU, _ := url.Parse(tt.expected)
- if gotU == nil || expU == nil {
- if got != tt.expected {
- t.Errorf("reconstructTargetURL() = %v, want %v", got, tt.expected)
- }
- return
- }
+ if gotU == nil || expU == nil {
+ if got != tt.expected {
+ t.Errorf("reconstructTargetURL() = %v, want %v", got, tt.expected)
+ }
+ return
+ }
if gotU.Scheme != expU.Scheme || gotU.Host != expU.Host || gotU.Path != expU.Path {
t.Errorf("reconstructTargetURL() base mismatch = %v, want %v", got, tt.expected)
diff --git a/api/xss_test.go b/api/xss_test.go
new file mode 100644
index 0000000..a064a03
--- /dev/null
+++ b/api/xss_test.go
@@ -0,0 +1,80 @@
+package handler
+
+import (
+ "bytes"
+ "context"
+ "net/http"
+ "net/http/httptest"
+ "net/url"
+ "strings"
+ "testing"
+)
+
+func TestXSSVulnerability(t *testing.T) {
+ // Malicious HTML payload
+ htmlBody := `
+
+
+XSS Test
+
+ XSS Test
+ Safe content
+
+ 
+ Click me
+
+`
+
+ srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+ if _, err := w.Write([]byte(htmlBody)); err != nil {
+ t.Errorf("failed to write response: %v", err)
+ }
+ }))
+ defer srv.Close()
+
+ // Override httpClient
+ oldClient := httpClient
+ httpClient = srv.Client()
+ defer func() { httpClient = oldClient }()
+
+ u, err := url.Parse(srv.URL)
+ if err != nil {
+ t.Fatalf("failed to parse server URL: %v", err)
+ }
+
+ ctx := context.Background()
+ req := httptest.NewRequest("GET", "/", nil)
+
+ // Fetch and parse
+ art, err := fetchAndParse(ctx, u, req)
+ if err != nil {
+ t.Fatalf("fetchAndParse failed: %v", err)
+ }
+
+ contentBuf := &bytes.Buffer{}
+ if err := art.RenderHTML(contentBuf); err != nil {
+ t.Fatalf("failed to render HTML: %v", err)
+ }
+
+ // We must test the output of formatHTML, which applies the sanitization
+ rec := httptest.NewRecorder()
+ formatHTML(rec, art, contentBuf)
+
+ resp := rec.Result()
+ bodyBuf := new(bytes.Buffer)
+ bodyBuf.ReadFrom(resp.Body)
+ content := bodyBuf.String()
+
+ // Check for XSS vectors
+ t.Logf("Rendered content: %s", content)
+
+ if strings.Contains(content, "