Fix opsgenie validation (cortexproject#5045)

* Validate OpsGenie alertmanager configuration Signed-off-by: Friedrich Gonzalez <friedrichg@gmail.com> * Update changelog Signed-off-by: Friedrich Gonzalez <friedrichg@gmail.com> * Validate global config too Signed-off-by: Friedrich Gonzalez <friedrichg@gmail.com> * fix pr number in changelog Signed-off-by: Friedrich Gonzalez <friedrichg@gmail.com> Signed-off-by: Friedrich Gonzalez <friedrichg@gmail.com> Signed-off-by: Alex Le <leqiyue@amazon.com>
alexqyle · May 2, 2023 · 1f060d6 · 1f060d6
1 parent 6cb8a34
commit 1f060d6
Show file tree

Hide file tree

Showing 3 changed files with 50 additions and 0 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,6 +1,7 @@
 # Changelog
 
 ## master / unreleased
+* [CHANGE] Alertmanager: Local file disclosure vulnerability in OpsGenie configuration has been fixed. #5045
 * [ENHANCEMENT] Update Go version to 1.19.3. #4988
 * [ENHANCEMENT] Querier: limit series query to only ingesters if `start` param is not specified. #4976
 * [ENHANCEMENT] Query-frontend/scheduler: add a new limit `frontend.max-outstanding-requests-per-tenant` for configuring queue size per tenant. Started deprecating two flags `-query-scheduler.max-outstanding-requests-per-tenant` and `-querier.max-outstanding-requests-per-tenant`, and change their value default to 0. Now if both the old flag and new flag are specified, the old flag's queue size will be picked. #5005

diff --git a/pkg/alertmanager/api.go b/pkg/alertmanager/api.go
@@ -45,6 +45,7 @@ var (
 	errTLSFileNotAllowed             = errors.New("setting TLS ca_file, cert_file and key_file is not allowed")
 	errSlackAPIURLFileNotAllowed     = errors.New("setting Slack api_url_file and global slack_api_url_file is not allowed")
 	errVictorOpsAPIKeyFileNotAllowed = errors.New("setting VictorOps api_key_file is not allowed")
+	errOpsGenieAPIKeyFileNotAllowed  = errors.New("setting OpsGenie api_key_file is not allowed")
 )
 
 // UserConfig is used to communicate a users alertmanager configs
@@ -336,6 +337,11 @@ func validateAlertmanagerConfig(cfg interface{}) error {
 			return err
 		}
 
+	case reflect.TypeOf(config.OpsGenieConfig{}):
+		if err := validateOpsGenieConfig(v.Interface().(config.OpsGenieConfig)); err != nil {
+			return err
+		}
+
 	case reflect.TypeOf(commoncfg.TLSConfig{}):
 		if err := validateReceiverTLSConfig(v.Interface().(commoncfg.TLSConfig)); err != nil {
 			return err
@@ -426,12 +432,24 @@ func validateReceiverTLSConfig(cfg commoncfg.TLSConfig) error {
 // validateGlobalConfig validates the Global config and returns an error if it contains
 // settings now allowed by Cortex.
 func validateGlobalConfig(cfg config.GlobalConfig) error {
+	if cfg.OpsGenieAPIKeyFile != "" {
+		return errOpsGenieAPIKeyFileNotAllowed
+	}
 	if cfg.SlackAPIURLFile != "" {
 		return errSlackAPIURLFileNotAllowed
 	}
 	return nil
 }
 
+// validateOpsGenieConfig validates the OpsGenie config and returns an error if it contains
+// settings now allowed by Cortex.
+func validateOpsGenieConfig(cfg config.OpsGenieConfig) error {
+	if cfg.APIKeyFile != "" {
+		return errOpsGenieAPIKeyFileNotAllowed
+	}
+	return nil
+}
+
 // validateSlackConfig validates the Slack config and returns an error if it contains
 // settings now allowed by Cortex.
 func validateSlackConfig(cfg config.SlackConfig) error {

diff --git a/pkg/alertmanager/api_test.go b/pkg/alertmanager/api_test.go
@@ -371,6 +371,23 @@ alertmanager_config: |
 `,
 			err: errors.Wrap(errOAuth2SecretFileNotAllowed, "error validating Alertmanager config"),
 		},
+		{
+			name: "Should return error if global opsgenie_api_key_file is set",
+			cfg: `
+alertmanager_config: |
+  global:
+    opsgenie_api_key_file: /secrets
+
+  receivers:
+    - name: default-receiver
+      webhook_configs:
+        - url: http://localhost
+
+  route:
+    receiver: 'default-receiver'
+`,
+			err: errors.Wrap(errOpsGenieAPIKeyFileNotAllowed, "error validating Alertmanager config"),
+		},
 		{
 			name: "Should return error if global slack_api_url_file is set",
 			cfg: `
@@ -402,6 +419,20 @@ alertmanager_config: |
 `,
 			err: errors.Wrap(errSlackAPIURLFileNotAllowed, "error validating Alertmanager config"),
 		},
+		{
+			name: "Should return error if OpsGenie api_key_file is set",
+			cfg: `
+alertmanager_config: |
+  receivers:
+    - name: default-receiver
+      opsgenie_configs:
+        - api_key_file: /secrets
+
+  route:
+    receiver: 'default-receiver'
+`,
+			err: errors.Wrap(errOpsGenieAPIKeyFileNotAllowed, "error validating Alertmanager config"),
+		},
 		{
 			name: "Should return error if VictorOps api_key_file is set",
 			cfg: `