add more context and unit tests for EscapePromQLValue func (#823)

Signed-off-by: Mohammed Abdi <mohammed.munir.abdi@ibm.com>

Author: mamy-CS

internal/collector/source/prometheus/prometheus_source_test.go

@@ -146,6 +146,45 @@ var _ = Describe("PrometheusSource", func() {
 			Expect(result.Values[0].Value).To(Equal(0.75))
 			Expect(result.Values[0].Labels["pod"]).To(Equal("test-pod-1"))
 		})
+
+		It("should escape params so PromQL is safe (no injection)", func() {
+			var capturedQuery string
+			mockAPI.queryFunc = func(ctx context.Context, query string, ts time.Time, opts ...v1.Option) (model.Value, v1.Warnings, error) {
+				capturedQuery = query
+				return model.Vector{
+					&model.Sample{
+						Metric:    model.Metric{"pod": "p1"},
+						Value:     0.5,
+						Timestamp: model.TimeFromUnix(time.Now().Unix()),
+					},
+				}, nil, nil
+			}
+
+			err := registry.Register(sourcepkg.QueryTemplate{
+				Name:        "injection_test",
+				Type:        sourcepkg.QueryTypePromQL,
+				Template:    `metric{namespace="{{.namespace}}",model_name="{{.modelID}}"}`,
+				Params:      []string{"namespace", "modelID"},
+				Description: "Test escaping",
+			})
+			Expect(err).NotTo(HaveOccurred())
+
+			// Params that would break PromQL or inject labels if unescaped
+			params := map[string]string{
+				"namespace": `safe-ns`,
+				"modelID":   `x",namespace="other"`,
+			}
+			_, err = source.Refresh(ctx, sourcepkg.RefreshSpec{
+				Queries: []string{"injection_test"},
+				Params:  params,
+			})
+			Expect(err).NotTo(HaveOccurred())
+			Expect(capturedQuery).NotTo(BeEmpty())
+			// Escaped: quote and backslash in modelID become \"
+			Expect(capturedQuery).To(ContainSubstring(`model_name="x\",namespace=\"other\""`))
+			// Should not contain unescaped injection (extra namespace=)
+			Expect(capturedQuery).NotTo(MatchRegexp(`namespace="other"`))
+		})
 	})
 
 	Describe("Caching", func() {

internal/collector/source/prometheus/suite_test.go

@@ -0,0 +1,13 @@
+package prometheus
+
+import (
+	"testing"
+
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+)
+
+func TestPrometheus(t *testing.T) {
+	RegisterFailHandler(Fail)
+	RunSpecs(t, "Prometheus Source Suite")
+}

internal/collector/source/query_template.go

@@ -143,7 +143,14 @@ func (r *QueryList) List() []string {
 // --- Helpers ---
 
 // EscapePromQLValue escapes a value for safe use in PromQL label matchers.
-// Prevents injection by escaping backslashes and double quotes.
+// It escapes backslashes and double quotes to prevent injection when values
+// are substituted into query templates.
+//
+// Kubernetes naming (e.g. namespace) only allows DNS labels, so namespace
+// values cannot contain " or \. Other parameters are not restricted: e.g.
+// VariantAutoscaling.Spec.ModelID is user-controlled and has no pattern
+// validation, so escaping is required for modelID and any future
+// user- or config-driven parameters.
 func EscapePromQLValue(value string) string {
 	// Escape backslashes first (must be done before escaping quotes)
 	value = backslashPattern.ReplaceAllString(value, `\\`)

internal/collector/source/query_template_test.go

@@ -0,0 +1,45 @@
+package source
+
+import (
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+)
+
+var _ = Describe("EscapePromQLValue", func() {
+	It("returns empty string unchanged", func() {
+		Expect(EscapePromQLValue("")).To(Equal(""))
+	})
+
+	It("escapes backslashes", func() {
+		Expect(EscapePromQLValue(`\`)).To(Equal(`\\`))
+		Expect(EscapePromQLValue(`foo\bar`)).To(Equal(`foo\\bar`))
+		Expect(EscapePromQLValue(`\\`)).To(Equal(`\\\\`))
+	})
+
+	It("escapes double quotes", func() {
+		Expect(EscapePromQLValue(`"`)).To(Equal(`\"`))
+		Expect(EscapePromQLValue(`foo"bar`)).To(Equal(`foo\"bar`))
+		Expect(EscapePromQLValue(`""`)).To(Equal(`\"\"`))
+	})
+
+	It("escapes backslashes before quotes so order is correct", func() {
+		// Backslash first, then quote - result is \\ then \"
+		Expect(EscapePromQLValue(`\""`)).To(Equal(`\\\"\"`))
+	})
+
+	It("leaves safe characters unchanged", func() {
+		Expect(EscapePromQLValue("test-ns")).To(Equal("test-ns"))
+		Expect(EscapePromQLValue("my-model-id")).To(Equal("my-model-id"))
+		Expect(EscapePromQLValue("a1b2c3")).To(Equal("a1b2c3"))
+	})
+
+	It("prevents PromQL label injection by escaping malicious payload", func() {
+		// If unescaped, this could close the label and inject another: namespace="other"
+		malicious := `prod",namespace="other"`
+		escaped := EscapePromQLValue(malicious)
+		Expect(escaped).To(Equal(`prod\",namespace=\"other\"`))
+		// The escaped value should be safe to embed in: metric{namespace="<value>"}
+		// i.e. metric{namespace="prod\",namespace=\"other\""} is one literal label value
+		Expect(escaped).NotTo(ContainSubstring(`namespace="other`))
+	})
+})