Skip to content

fix: sanitize app status summary #19075

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 29, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 40 additions & 0 deletions coderd/util/strings/strings.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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())
}
39 changes: 39 additions & 0 deletions coderd/util/strings/strings_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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>", ""},
{"<iframe src='evil.com'></iframe>", ""},
} {
t.Run(tt.expected, func(t *testing.T) {
t.Parallel()
actual := strings.UISanitize(tt.s)
assert.Equal(t, tt.expected, actual)
})
}
}
6 changes: 5 additions & 1 deletion coderd/workspaceagents.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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(),
Expand All @@ -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 != "",
Expand Down
2 changes: 1 addition & 1 deletion codersdk/toolsdk/toolsdk.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading