From 88bf0fae482dc9b95934dfd470720aa1b0a8620d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20R=C3=BCger?= Date: Wed, 2 Sep 2026 14:14:31 +0200 Subject: [PATCH] web: escape values interpolated into the landing page MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit NewLandingPage rendered the page with text/template, so nothing it interpolated was escaped. Every field the template reads reaches the browser verbatim: Description, Version, each link's Text, Address and Description, and every form input's Value, Placeholder, Name and Type, including inside the href="..." and value="..." attributes. An exporter that puts a probe target, a query parameter or any other non-constant value on its landing page therefore has an injection point. Render the page with html/template instead, which escapes each value for the context it appears in. The two fields that are documented as carrying markup keep doing so by becoming template.HTML and template.CSS, which html/template emits verbatim by design. The generated stylesheet is still built with text/template: its output is injected into the page as a template.CSS value, and running it through html/template would escape it for an HTML context rather than a CSS one. Ordinary link addresses are unaffected. A query string such as /probe?module=http_2xx&target=example.com is written into the attribute as ...module=http_2xx&target=example.com, which browsers decode back to a single "&" when following the link. This changes the type of three LandingConfig fields from string to template.HTML or template.CSS. Callers assigning a string literal are unaffected, since an untyped constant converts on assignment; callers assigning a string variable need an explicit conversion. landing_page.go had no test coverage at all, so add tests for escaping, for the two verbatim fields, for link query strings, for the generated stylesheet, and for the handler's 404 path. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Manuel Rüger --- web/landing_page.go | 16 +++-- web/landing_page_test.go | 143 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 153 insertions(+), 6 deletions(-) create mode 100644 web/landing_page_test.go diff --git a/web/landing_page.go b/web/landing_page.go index f863e9d8..eca603eb 100644 --- a/web/landing_page.go +++ b/web/landing_page.go @@ -20,22 +20,23 @@ package web import ( "bytes" _ "embed" + "html/template" "net/http" "strings" - "text/template" + texttemplate "text/template" ) // Config represents the configuration of the web listener. type LandingConfig struct { RoutePrefix string // The route prefix for the exporter. HeaderColor string // Used for the landing page header. - CSS string // CSS style tag for the landing page. + CSS template.CSS // CSS style tag for the landing page. Name string // The name of the exporter, generally suffixed by _exporter. Description string // A short description about the exporter. Form LandingForm // A POST form. Links []LandingLinks // Links displayed on the landing page. - ExtraHTML string // Additional HTML to be embedded. - ExtraCSS string // Additional CSS to be embedded. + ExtraHTML template.HTML // Additional HTML to be embedded. + ExtraCSS template.CSS // Additional CSS to be embedded. Version string // The version displayed. Profiling string // If false, don't display profiling links. } @@ -92,11 +93,14 @@ func NewLandingPage(c LandingConfig) (*LandingPageHandler, error) { // Default to Prometheus orange. c.HeaderColor = "#e6522c" } - cssTemplate := template.Must(template.New("landing css").Parse(landingPagecssContent)) + // The CSS is generated with text/template: its output is injected + // into the page as a template.CSS value below, and html/template + // would escape it for an HTML context rather than a CSS one. + cssTemplate := texttemplate.Must(texttemplate.New("landing css").Parse(landingPagecssContent)) if err := cssTemplate.Execute(&buf, c); err != nil { return nil, err } - c.CSS = buf.String() + c.CSS = template.CSS(buf.String()) } if c.RoutePrefix == "" { c.RoutePrefix = "/" diff --git a/web/landing_page_test.go b/web/landing_page_test.go new file mode 100644 index 00000000..2910dfa8 --- /dev/null +++ b/web/landing_page_test.go @@ -0,0 +1,143 @@ +// Copyright The Prometheus Authors +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package web + +import ( + "html/template" + "net/http" + "net/http/httptest" + "strings" + "testing" +) + +func landingPageBody(t *testing.T, c LandingConfig) string { + t.Helper() + h, err := NewLandingPage(c) + if err != nil { + t.Fatalf("NewLandingPage: %v", err) + } + rec := httptest.NewRecorder() + h.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/", nil)) + if rec.Code != http.StatusOK { + t.Fatalf("got status %d, expected %d", rec.Code, http.StatusOK) + } + return rec.Body.String() +} + +// TestLandingPageEscapesConfiguredValues checks that values interpolated into +// the landing page are HTML-escaped. Exporters put non-constant data such as a +// probe target or a version string into these fields, so rendering them +// verbatim would be an injection point. +func TestLandingPageEscapesConfiguredValues(t *testing.T) { + const payload = `` + + body := landingPageBody(t, LandingConfig{ + Name: payload, + Description: payload, + Version: payload, + Links: []LandingLinks{{ + Address: `/metrics">`, + Text: payload, + Description: payload, + }}, + Form: LandingForm{ + Action: "/probe", + Inputs: []LandingFormInput{{ + Label: payload, + Type: "text", + Name: payload, + Placeholder: payload, + Value: payload, + }}, + }, + }) + + for _, unwanted := range []string{ + "", + "", + } { + if strings.Contains(body, unwanted) { + t.Errorf("landing page contains unescaped %q", unwanted) + } + } + if !strings.Contains(body, "<script>") { + t.Error("landing page does not contain the escaped payload at all") + } +} + +// TestLandingPageKeepsExtraHTMLAndCSS checks that the two fields documented as +// carrying markup are still emitted verbatim. +func TestLandingPageKeepsExtraHTMLAndCSS(t *testing.T) { + body := landingPageBody(t, LandingConfig{ + Name: "test_exporter", + ExtraHTML: template.HTML(`

extra & html

`), + ExtraCSS: template.CSS(`.extra { color: #e6522c; }`), + }) + + if !strings.Contains(body, `

extra & html

`) { + t.Error("ExtraHTML was not emitted verbatim") + } + if !strings.Contains(body, `.extra { color: #e6522c; }`) { + t.Error("ExtraCSS was not emitted verbatim") + } +} + +// TestLandingPageKeepsLinkQueryStrings checks that ordinary link addresses, +// including query strings, survive escaping intact. +func TestLandingPageKeepsLinkQueryStrings(t *testing.T) { + body := landingPageBody(t, LandingConfig{ + Name: "test_exporter", + Links: []LandingLinks{ + {Address: "/metrics", Text: "Metrics"}, + {Address: "/probe?module=http_2xx&target=example.com", Text: "Probe"}, + }, + }) + + for _, want := range []string{ + `href="/metrics"`, + // "&" is written as "&" in an HTML attribute; browsers decode it + // back to a single "&" when following the link. + `href="/probe?module=http_2xx&target=example.com"`, + } { + if !strings.Contains(body, want) { + t.Errorf("landing page does not contain %q", want) + } + } +} + +// TestLandingPageCSSIsRendered checks that the generated stylesheet reaches the +// page unescaped, including the configured header color. +func TestLandingPageCSSIsRendered(t *testing.T) { + body := landingPageBody(t, LandingConfig{Name: "test_exporter", HeaderColor: "#123456"}) + + if !strings.Contains(body, "background-color: #123456;") { + t.Error("header color was not rendered into the stylesheet") + } + if strings.Contains(body, "ZgotmplZ") { + t.Error("stylesheet was escaped as an HTML value") + } +} + +// TestLandingPageServeHTTPNotFound checks that only the route prefix is served. +func TestLandingPageServeHTTPNotFound(t *testing.T) { + h, err := NewLandingPage(LandingConfig{Name: "test_exporter"}) + if err != nil { + t.Fatalf("NewLandingPage: %v", err) + } + rec := httptest.NewRecorder() + h.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/nope", nil)) + if rec.Code != http.StatusNotFound { + t.Errorf("got status %d, expected %d", rec.Code, http.StatusNotFound) + } +}