shishantbiswas/coder
1
0
Fork 0
mirror of https://github.com/coder/coder.git synced 2026-06-02 20:48:20 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: escape appearance values in HTML output (#25804)

Browse Source
This commit is contained in:
Jon Ayers
2026-06-02 13:19:16 -05:00
committed by GitHub
parent 2f011fd2a3
commit ec19bc41d8
4 changed files with 120 additions and 5 deletions
Download Patch File Download Diff File Expand all files Collapse all files
coderd/notifications/dispatch/smtp/html.gotmpl
+1 -1
View File
@@ -8,7 +8,7 @@
<body style="margin: 0; padding: 0; font-family: -apple-system, system-ui, BlinkMacSystemFont, 'Segoe UI', 'Roboto', 'Oxygen', 'Ubuntu', 'Cantarell', 'Fira Sans', 'Droid Sans', 'Helvetica Neue', sans-serif; color: #020617; background: #f8fafc;">
<div style="max-width: 600px; margin: 20px auto; padding: 60px; border: 1px solid #e2e8f0; border-radius: 8px; background-color: #fff; text-align: left; font-size: 14px; line-height: 1.5;">
<div style="text-align: center;">
<img src="{{ logo_url }}" alt="{{ app_name }} Logo" style="height: 40px;" />
<img src="{{ logo_url | html }}" alt="{{ app_name | html }} Logo" style="height: 40px;" />
</div>
<h1 style="text-align: center; font-size: 24px; font-weight: 400; margin: 8px 0 32px; line-height: 1.5;">
{{ .Labels._subject }}
coderd/notifications/dispatch/smtp_internal_test.go
+37
View File
@@ -1,11 +1,48 @@
package dispatch
import (
"html"
"strings"
"testing"
"github.com/stretchr/testify/require"
"github.com/coder/coder/v2/coderd/notifications/render"
"github.com/coder/coder/v2/coderd/notifications/types"
)
func TestSMTPHTMLTemplateEscapesAppearanceHelpers(t *testing.T) {
t.Parallel()
const (
appName = `Coder"><script>alert(1)</script>`
logoURL = `https://example.com/logo.png"><img src=x onerror=alert(1)>`
)
payload := types.MessagePayload{
NotificationTemplateID: "00000000-0000-0000-0000-000000000000",
UserName: "Test User",
Labels: map[string]string{
"_subject": "Test notification",
"_body": "<p>Test body</p>",
},
}
helpers := map[string]any{
"base_url": func() string { return "https://coder.example.com" },
"current_year": func() string { return "2026" },
"logo_url": func() string { return logoURL },
"app_name": func() string { return appName },
}
got, err := render.GoTemplate(htmlTemplate, payload, helpers)
require.NoError(t, err)
require.True(t, strings.Contains(got, html.EscapeString(appName)), "application name must be HTML escaped")
require.True(t, strings.Contains(got, html.EscapeString(logoURL)), "logo URL must be HTML escaped")
require.False(t, strings.Contains(got, appName), "raw application name must not be rendered")
require.False(t, strings.Contains(got, logoURL), "raw logo URL must not be rendered")
}
func TestValidateFromAddr(t *testing.T) {
t.Parallel()
site/site.go
+4 -4
View File
@@ -397,8 +397,8 @@ func (h *Handler) renderHTMLWithState(r *http.Request, filePath string, state ht
// nolint:gocritic // User is not expected to be signed in.
ctx := dbauthz.AsSystemRestricted(r.Context())
cfg, _ = af.Fetch(ctx)
state.ApplicationName = applicationNameOrDefault(cfg)
state.LogoURL = cfg.LogoURL
state.ApplicationName = html.EscapeString(applicationNameOrDefault(cfg))
state.LogoURL = html.EscapeString(cfg.LogoURL)
return execTmpl(tmpl, state)
}
@@ -488,8 +488,8 @@ func (h *Handler) populateHTMLState(
appr, err := json.Marshal(cfg)
if err == nil {
state.Appearance = html.EscapeString(string(appr))
state.ApplicationName = applicationNameOrDefault(cfg)
state.LogoURL = cfg.LogoURL
state.ApplicationName = html.EscapeString(applicationNameOrDefault(cfg))
state.LogoURL = html.EscapeString(cfg.LogoURL)
}
}
})
site/site_test.go
+78
View File
@@ -14,6 +14,7 @@ import (
"path/filepath"
"strconv"
"strings"
"sync/atomic"
"testing"
"testing/fstest"
"time"
@@ -26,6 +27,7 @@ import (
"github.com/stretchr/testify/require"
"golang.org/x/exp/maps"
"github.com/coder/coder/v2/coderd/appearance"
"github.com/coder/coder/v2/coderd/database"
"github.com/coder/coder/v2/coderd/database/db2sdk"
"github.com/coder/coder/v2/coderd/database/dbgen"
@@ -39,6 +41,82 @@ import (
"github.com/coder/coder/v2/testutil"
)
type staticAppearanceFetcher struct {
cfg codersdk.AppearanceConfig
}
func (f staticAppearanceFetcher) Fetch(context.Context) (codersdk.AppearanceConfig, error) {
return f.cfg, nil
}
func TestInjectionAppearanceEscapesMetaAttributes(t *testing.T) {
t.Parallel()
const (
applicationName = `Coder"><script>alert(1)</script>`
logoURL = `https://example.com/logo.png"><img src=x onerror=alert(1)>`
)
tests := []struct {
name string
authenticated bool
}{
{
name: "unauthenticated",
},
{
name: "authenticated",
authenticated: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
siteFS := fstest.MapFS{
"index.html": &fstest.MapFile{
Data: []byte(`<meta name="application-name" content="{{ .ApplicationName }}" /><meta property="logo-url" content="{{ .LogoURL }}" />`),
},
}
db, _ := dbtestutil.NewDB(t)
var appearanceFetcher atomic.Pointer[appearance.Fetcher]
fetcher := appearance.Fetcher(staticAppearanceFetcher{cfg: codersdk.AppearanceConfig{
ApplicationName: applicationName,
LogoURL: logoURL,
}})
appearanceFetcher.Store(&fetcher)
handler, err := site.New(&site.Options{
Telemetry: telemetry.NewNoop(),
Database: db,
SiteFS: siteFS,
AppearanceFetcher: &appearanceFetcher,
})
require.NoError(t, err)
r := httptest.NewRequest("GET", "/", nil)
if tt.authenticated {
user := dbgen.User(t, db, database.User{})
_, token := dbgen.APIKey(t, db, database.APIKey{
UserID: user.ID,
ExpiresAt: time.Now().Add(time.Hour),
})
r.Header.Set(codersdk.SessionTokenHeader, token)
}
rw := httptest.NewRecorder()
handler.ServeHTTP(rw, r)
require.Equal(t, http.StatusOK, rw.Code)
body := rw.Body.String()
require.True(t, strings.Contains(body, html.EscapeString(applicationName)), "application name must be HTML escaped")
require.True(t, strings.Contains(body, html.EscapeString(logoURL)), "logo URL must be HTML escaped")
require.False(t, strings.Contains(body, applicationName), "raw application name must not be rendered")
require.False(t, strings.Contains(body, logoURL), "raw logo URL must not be rendered")
})
}
}
func TestInjection(t *testing.T) {
t.Parallel()