feat: config: validate metrics config (#4130)

This change adds validation for metrics config.
In particular, the metrics path is checked to
ensure it starts with a / and is not one of the
disallowed paths.

Signed-off-by: Vishwas Rajashekar <dev@vrajashkr.com>

errors/errors.go

@@ -212,4 +212,7 @@ var (
 	ErrCertificateWatcherAlreadyRunning = errors.New("certificate watcher is already running")
 	ErrInvalidEndSessionEndpoint        = errors.New("end_session_endpoint must be an absolute http(s) URL")
 	ErrPolicyConditionNotCompiled       = errors.New("policy condition not compiled")
+	ErrDisallowedMetricsPath            = errors.New("provided metrics path is disallowed")
+	ErrInvalidMetricsPathPrefix         = errors.New("metrics path must start with /")
+	ErrInvalidMetricsPath               = errors.New("invalid metrics path")
 )

pkg/cli/server/root.go

@@ -536,6 +536,32 @@ func validateRemoteSessionStoreConfig(cfg *config.Config, logger zlog.Logger) er
 	return nil
 }
 
+func validateMetricsConfig(cfg *extconf.ExtensionConfig) error {
+	metricsCfg := cfg.GetMetricsPrometheusConfig()
+	if metricsCfg == nil {
+		return nil
+	}
+
+	cleanedPath := path.Clean(metricsCfg.Path)
+	// The path when cleaned should be exactly the same as in config
+	// to avoid invalid paths from being used for metrics.
+	if metricsCfg.Path != cleanedPath {
+		return zerr.ErrInvalidMetricsPath
+	}
+
+	if !strings.HasPrefix(metricsCfg.Path, "/") {
+		return zerr.ErrInvalidMetricsPathPrefix
+	}
+
+	disallowedMetricsPaths := []string{"/", "/v2"}
+
+	if slices.Contains(disallowedMetricsPaths, metricsCfg.Path) {
+		return zerr.ErrDisallowedMetricsPath
+	}
+
+	return nil
+}
+
 func validateExtensionsConfig(cfg *config.Config, logger zlog.Logger) error {
 	extensionsConfig := cfg.CopyExtensionsConfig()
 	if extensionsConfig != nil && extensionsConfig.Mgmt != nil {
@@ -547,6 +573,15 @@ func validateExtensionsConfig(cfg *config.Config, logger zlog.Logger) error {
 			"are now configurable in the HTTP settings.")
 	}
 
+	if extensionsConfig != nil {
+		if metricsValErr := validateMetricsConfig(extensionsConfig); metricsValErr != nil {
+			joinedErr := errors.Join(zerr.ErrBadConfig, metricsValErr)
+			logger.Error().Err(joinedErr).Msg("invalid metrics config")
+
+			return joinedErr
+		}
+	}
+
 	if extensionsConfig.IsUIEnabled() {
 		// it would make sense to also check for mgmt and user prefs to be enabled,
 		// but those are both enabled by having the search and ui extensions enabled

pkg/cli/server/root_test.go

@@ -3446,3 +3446,216 @@ func TestBearerASMConfigValidation(t *testing.T) {
 		})
 	})
 }
+
+func TestMetricsConfigurationValidation(t *testing.T) {
+	Convey("Test metrics config", t, func() {
+		Convey("Allow no metrics config", func() {
+			content := `{
+				"storage": {"rootDirectory": "/tmp/zot"},
+				"http": {
+					"address": "127.0.0.1", "port": "8080"
+				},
+				"extensions": {}
+			}`
+			cfg := config.New()
+			tmpfile := MakeTempFileWithContent(t, "zot-test.json", content)
+			err := cli.LoadConfiguration(cfg, tmpfile)
+			So(err, ShouldBeNil)
+		})
+
+		Convey("Allow empty metrics config", func() {
+			content := `{
+				"storage": {"rootDirectory": "/tmp/zot"},
+				"http": {
+					"address": "127.0.0.1", "port": "8080"
+				},
+				"extensions": {
+					"metrics": {}
+				}
+			}`
+			cfg := config.New()
+			tmpfile := MakeTempFileWithContent(t, "zot-test.json", content)
+			err := cli.LoadConfiguration(cfg, tmpfile)
+			So(err, ShouldBeNil)
+		})
+
+		Convey("Allow only metrics enabled", func() {
+			content := `{
+				"storage": {"rootDirectory": "/tmp/zot"},
+				"http": {
+					"address": "127.0.0.1", "port": "8080"
+				},
+				"extensions": {
+					"metrics": {
+						"enable": true
+					}
+				}
+			}`
+			cfg := config.New()
+			tmpfile := MakeTempFileWithContent(t, "zot-test.json", content)
+			err := cli.LoadConfiguration(cfg, tmpfile)
+			So(err, ShouldBeNil)
+		})
+	})
+
+	Convey("Test metrics path validation", t, func() {
+		Convey("Reject / as metrics path", func() {
+			content := `{
+				"storage": {"rootDirectory": "/tmp/zot"},
+				"http": {
+					"address": "127.0.0.1", "port": "8080"
+				},
+				"extensions": {
+					"metrics": {
+						"enable": true,
+						"prometheus": {
+							"path": "/"
+						}
+					}
+				}
+			}`
+			cfg := config.New()
+			tmpfile := MakeTempFileWithContent(t, "zot-test.json", content)
+			err := cli.LoadConfiguration(cfg, tmpfile)
+			So(err, ShouldNotBeNil)
+			So(err, ShouldWrap, zerr.ErrBadConfig)
+			So(err, ShouldWrap, zerr.ErrDisallowedMetricsPath)
+		})
+
+		Convey("Reject /v2 as metrics path", func() {
+			content := `{
+				"storage": {"rootDirectory": "/tmp/zot"},
+				"http": {
+					"address": "127.0.0.1", "port": "8080"
+				},
+				"extensions": {
+					"metrics": {
+						"enable": true,
+						"prometheus": {
+							"path": "/v2"
+						}
+					}
+				}
+			}`
+			cfg := config.New()
+			tmpfile := MakeTempFileWithContent(t, "zot-test.json", content)
+			err := cli.LoadConfiguration(cfg, tmpfile)
+			So(err, ShouldNotBeNil)
+			So(err, ShouldWrap, zerr.ErrBadConfig)
+			So(err, ShouldWrap, zerr.ErrDisallowedMetricsPath)
+		})
+
+		Convey("Reject /v2/ as metrics path", func() {
+			content := `{
+				"storage": {"rootDirectory": "/tmp/zot"},
+				"http": {
+					"address": "127.0.0.1", "port": "8080"
+				},
+				"extensions": {
+					"metrics": {
+						"enable": true,
+						"prometheus": {
+							"path": "/v2/"
+						}
+					}
+				}
+			}`
+			cfg := config.New()
+			tmpfile := MakeTempFileWithContent(t, "zot-test.json", content)
+			err := cli.LoadConfiguration(cfg, tmpfile)
+			So(err, ShouldNotBeNil)
+			So(err, ShouldWrap, zerr.ErrBadConfig)
+			So(err, ShouldWrap, zerr.ErrInvalidMetricsPath)
+		})
+
+		Convey("Reject /abcd/.. as metrics path", func() {
+			content := `{
+				"storage": {"rootDirectory": "/tmp/zot"},
+				"http": {
+					"address": "127.0.0.1", "port": "8080"
+				},
+				"extensions": {
+					"metrics": {
+						"enable": true,
+						"prometheus": {
+							"path": "/abcd/.."
+						}
+					}
+				}
+			}`
+			cfg := config.New()
+			tmpfile := MakeTempFileWithContent(t, "zot-test.json", content)
+			err := cli.LoadConfiguration(cfg, tmpfile)
+			So(err, ShouldNotBeNil)
+			So(err, ShouldWrap, zerr.ErrBadConfig)
+			So(err, ShouldWrap, zerr.ErrInvalidMetricsPath)
+		})
+
+		Convey("Reject abcd as metrics path", func() {
+			content := `{
+				"storage": {"rootDirectory": "/tmp/zot"},
+				"http": {
+					"address": "127.0.0.1", "port": "8080"
+				},
+				"extensions": {
+					"metrics": {
+						"enable": true,
+						"prometheus": {
+							"path": "abcd"
+						}
+					}
+				}
+			}`
+			cfg := config.New()
+			tmpfile := MakeTempFileWithContent(t, "zot-test.json", content)
+			err := cli.LoadConfiguration(cfg, tmpfile)
+			So(err, ShouldNotBeNil)
+			So(err, ShouldWrap, zerr.ErrBadConfig)
+			So(err, ShouldWrap, zerr.ErrInvalidMetricsPathPrefix)
+		})
+
+		Convey("Reject blank metrics path", func() {
+			content := `{
+				"storage": {"rootDirectory": "/tmp/zot"},
+				"http": {
+					"address": "127.0.0.1", "port": "8080"
+				},
+				"extensions": {
+					"metrics": {
+						"enable": true,
+						"prometheus": {
+							"path": ""
+						}
+					}
+				}
+			}`
+			cfg := config.New()
+			tmpfile := MakeTempFileWithContent(t, "zot-test.json", content)
+			err := cli.LoadConfiguration(cfg, tmpfile)
+			So(err, ShouldNotBeNil)
+			So(err, ShouldWrap, zerr.ErrBadConfig)
+			So(err, ShouldWrap, zerr.ErrInvalidMetricsPath)
+		})
+
+		Convey("Allow valid metrics path", func() {
+			content := `{
+				"storage": {"rootDirectory": "/tmp/zot"},
+				"http": {
+					"address": "127.0.0.1", "port": "8080"
+				},
+				"extensions": {
+					"metrics": {
+						"enable": true,
+						"prometheus": {
+							"path": "/abcd"
+						}
+					}
+				}
+			}`
+			cfg := config.New()
+			tmpfile := MakeTempFileWithContent(t, "zot-test.json", content)
+			err := cli.LoadConfiguration(cfg, tmpfile)
+			So(err, ShouldBeNil)
+		})
+	})
+}