diff --git a/coderd/util/strings/strings.go b/coderd/util/strings/strings.go index f416bba463bbf..49aad579e83f5 100644 --- 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 policy. +// - 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 index 5172fb08e1e69..7a20a06a25f28 100644 --- 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"}, + {"link", "link"}, + {"", ""}, + {"HTML", "HTML"}, + {"
line break", "line break"}, + {"", ""}, + {"", ""}, + {"visible", "visible"}, + {"", ""}, + {"", ""}, + } { + 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 index 3ae57d8394d43..d600eff6ecfec 100644 --- a/coderd/workspaceagents.go +++ b/coderd/workspaceagents.go @@ -41,6 +41,7 @@ import ( "github.com/coder/coder/v2/coderd/rbac/policy" "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 index 862d0c34a5316..c6c37821e5234 100644 --- 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 index 8e48f67f65885..fcd76e07987e2 100644 --- 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