diff --git a/coderd/notifications/dispatch/smtp/html.gotmpl b/coderd/notifications/dispatch/smtp/html.gotmpl index 4e49c4239d..cecba560af 100644 --- a/coderd/notifications/dispatch/smtp/html.gotmpl +++ b/coderd/notifications/dispatch/smtp/html.gotmpl @@ -8,7 +8,7 @@
- {{ app_name }} Logo + {{ app_name | html }} Logo

{{ .Labels._subject }} diff --git a/coderd/notifications/dispatch/smtp_internal_test.go b/coderd/notifications/dispatch/smtp_internal_test.go index cc193673f0..2e7dff8cbe 100644 --- a/coderd/notifications/dispatch/smtp_internal_test.go +++ b/coderd/notifications/dispatch/smtp_internal_test.go @@ -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">` + logoURL = `https://example.com/logo.png">` + ) + + payload := types.MessagePayload{ + NotificationTemplateID: "00000000-0000-0000-0000-000000000000", + UserName: "Test User", + Labels: map[string]string{ + "_subject": "Test notification", + "_body": "

Test body

", + }, + } + 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() diff --git a/site/site.go b/site/site.go index 9ba43f79f9..b0a90ef0f0 100644 --- a/site/site.go +++ b/site/site.go @@ -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) } } }) diff --git a/site/site_test.go b/site/site_test.go index ef72ee0bc4..32d4bd27a2 100644 --- a/site/site_test.go +++ b/site/site_test.go @@ -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">` + logoURL = `https://example.com/logo.png">` + ) + + 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(``), + }, + } + 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()