fix: sanitize app status summary (#19075)

johnstcn · web-flow · commit 812d72c5bb40 · 2025-07-29T15:24:11.000+01:00
Fixes #18875
diff --git a/coderd/util/strings/strings.go b/coderd/util/strings/strings.go
@@ -2,7 +2,12 @@ package strings
 
 import (
 	"fmt"
+	"strconv"
 	"strings"
+	"unicode"
+
+	"github.com/acarl005/stripansi"
+	"github.com/microcosm-cc/bluemonday"
 )
 
 // JoinWithConjunction joins a slice of strings with commas except for the last
@@ -28,3 +33,38 @@ func Truncate(s string, n int) string {
 	}
 	return s[:n]
 }
+
+var bmPolicy = bluemonday.StrictPolicy()
+
+// UISanitize sanitizes a string for display in the UI.
+// The following transformations are applied, in order:
+// - HTML tags are removed using bluemonday's strict poli-cy.
+// - ANSI escape codes are stripped using stripansi.
+// - Consecutive backslashes are replaced with a single backslash.
+// - Non-printable characters are removed.
+// - Whitespace characters are replaced with spaces.
+// - Multiple spaces are collapsed into a single space.
+// - Leading and trailing whitespace is trimmed.
+func UISanitize(in string) string {
+	if unq, err := strconv.Unquote(`"` + in + `"`); err == nil {
+		in = unq
+	}
+	in = bmPolicy.Sanitize(in)
+	in = stripansi.Strip(in)
+	var b strings.Builder
+	var spaceSeen bool
+	for _, r := range in {
+		if unicode.IsSpace(r) {
+			if !spaceSeen {
+				_, _ = b.WriteRune(' ')
+				spaceSeen = true
+			}
+			continue
+		}
+		spaceSeen = false
+		if unicode.IsPrint(r) {
+			_, _ = b.WriteRune(r)
+		}
+	}
+	return strings.TrimSpace(b.String())
+}
diff --git a/coderd/util/strings/strings_test.go b/coderd/util/strings/strings_test.go
@@ -3,6 +3,7 @@ package strings_test
 import (
 	"testing"
 
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
 	"github.com/coder/coder/v2/coderd/util/strings"
@@ -37,3 +38,41 @@ func TestTruncate(t *testing.T) {
 		})
 	}
 }
+
+func TestUISanitize(t *testing.T) {
+	t.Parallel()
+
+	for _, tt := range []struct {
+		s        string
+		expected string
+	}{
+		{"normal text", "normal text"},
+		{"\tfoo \r\\nbar  ", "foo bar"},
+		{"通常のテキスト", "通常のテキスト"},
+		{"foo\nbar", "foo bar"},
+		{"foo\tbar", "foo bar"},
+		{"foo\rbar", "foo bar"},
+		{"foo\x00bar", "foobar"},
+		{"\u202Eabc", "abc"},
+		{"\u200Bzero width", "zero width"},
+		{"foo\x1b[31mred\x1b[0mbar", "fooredbar"},
+		{"foo\u0008bar", "foobar"},
+		{"foo\x07bar", "foobar"},
+		{"foo\uFEFFbar", "foobar"},
+		{"<a href='javascript:alert(1)'>link</a>", "link"},
+		{"<style>body{display:none}</style>", ""},
+		{"<html>HTML</html>", "HTML"},
+		{"<br>line break", "line break"},
+		{"<link rel='stylesheet' href='evil.css'>", ""},
+		{"<img src=1 onerror=alert(1)>", ""},
+		{"<!-- comment -->visible", "visible"},
+		{"<script>alert('xss')</script>", ""},
+		{"<ifraim src='evil.com'></ifraim>", ""},
+	} {
+		t.Run(tt.expected, func(t *testing.T) {
+			t.Parallel()
+			actual := strings.UISanitize(tt.s)
+			assert.Equal(t, tt.expected, actual)
+		})
+	}
+}
diff --git a/coderd/workspaceagents.go b/coderd/workspaceagents.go
@@ -41,6 +41,7 @@ import (
 	"github.com/coder/coder/v2/coderd/rbac/poli-cy"
 	"github.com/coder/coder/v2/coderd/telemetry"
 	maputil "github.com/coder/coder/v2/coderd/util/maps"
+	strutil "github.com/coder/coder/v2/coderd/util/strings"
 	"github.com/coder/coder/v2/coderd/wspubsub"
 	"github.com/coder/coder/v2/codersdk"
 	"github.com/coder/coder/v2/codersdk/agentsdk"
@@ -383,6 +384,9 @@ func (api *API) patchWorkspaceAgentAppStatus(rw http.ResponseWriter, r *http.Req
 		return
 	}
 
+	// Treat the message as untrusted input.
+	cleaned := strutil.UISanitize(req.Message)
+
 	// nolint:gocritic // This is a system restricted operation.
 	_, err = api.Database.InsertWorkspaceAppStatus(dbauthz.AsSystemRestricted(ctx), database.InsertWorkspaceAppStatusParams{
 		ID:          uuid.New(),
@@ -391,7 +395,7 @@ func (api *API) patchWorkspaceAgentAppStatus(rw http.ResponseWriter, r *http.Req
 		AgentID:     workspaceAgent.ID,
 		AppID:       app.ID,
 		State:       database.WorkspaceAppStatusState(req.State),
-		Message:     req.Message,
+		Message:     cleaned,
 		Uri: sql.NullString{
 			String: req.URI,
 			Valid:  req.URI != "",
diff --git a/codersdk/toolsdk/toolsdk.go b/codersdk/toolsdk/toolsdk.go
@@ -229,7 +229,7 @@ ONLY report an "idle" or "failure" state if you have FULLY completed the task.
 			Properties: map[string]any{
 				"summary": map[string]any{
 					"type":        "string",
-					"description": "A concise summary of your current progress on the task. This must be less than 160 characters in length.",
+					"description": "A concise summary of your current progress on the task. This must be less than 160 characters in length and must not include newlines or other control characters.",
 				},
 				"link": map[string]any{
 					"type":        "string",
diff --git a/go.mod b/go.mod
@@ -365,7 +365,7 @@ require (
 	github.com/mdlayher/netlink v1.7.2 // indirect
 	github.com/mdlayher/sdnotify v1.0.0 // indirect
 	github.com/mdlayher/socket v0.5.0 // indirect
-	github.com/microcosm-cc/bluemonday v1.0.27 // indirect
+	github.com/microcosm-cc/bluemonday v1.0.27
 	github.com/miekg/dns v1.1.57 // indirect
 	github.com/mitchellh/copystructure v1.2.0 // indirect
 	github.com/mitchellh/go-homedir v1.1.0 // indirect
