Skip to content

Alertmanager: reject global mattermost_webhook_url_file - #7768

Open
friedrichg wants to merge 2 commits into
masterfrom
global-mattermost-file
Open

Alertmanager: reject global mattermost_webhook_url_file#7768
friedrichg wants to merge 2 commits into
masterfrom
global-mattermost-file

Conversation

@friedrichg

Copy link
Copy Markdown
Member

What this PR does: Adds mattermost_webhook_url_file to the global *_file checks in validateGlobalConfig, the only one of the eleven that was missing.

Not exploitable today. When a mattermost receiver is present, upstream copies the global value into the receiver's WebhookURLFile during unmarshal, which runs before validation, so the per-receiver check already rejected it. With no mattermost receiver the value was accepted but never read.

The coverage was accidental though, resting entirely on upstream doing that propagation in UnmarshalYAML. Validating the field directly means the guarantee no longer depends on upstream ordering.

Reuses the existing error and extends its message to name both settings, matching how the other dual-scope settings are reported.

Found while auditing every file-path field reachable from a tenant config; the other 42 are all rejected.

CHANGELOG entry needs a PR number.

validateGlobalConfig checks ten global *_file settings but omitted
MattermostWebhookURLFile, the only one missing.

This was not exploitable. When a mattermost receiver is present, upstream
copies the global value into the receiver's WebhookURLFile during
unmarshal (config.go:637), which happens inside config.Load and therefore
before validateAlertmanagerConfig runs, so validateMattermostConfig
already rejected it. With no mattermost receiver the global value was
accepted but never propagated, and nothing read it.

The coverage was accidental though: it depends entirely on upstream doing
that propagation in UnmarshalYAML. If that ever moves into the notifier
constructor the check silently stops working. Validate the field directly
so the guarantee does not rest on upstream ordering.

Reuse the existing error and extend its message to name both settings,
matching how slack_api_url_file, rocketchat_token_file and the other
dual-scope settings are reported.

Signed-off-by: Friedrich Gonzalez <1517449+friedrichg@users.noreply.github.com>
@friedrichg
friedrichg marked this pull request as ready for review August 14, 2026 21:23
Signed-off-by: Friedrich Gonzalez <1517449+friedrichg@users.noreply.github.com>
@friedrichg
friedrichg force-pushed the global-mattermost-file branch from fef55d2 to 8bdafbd Compare August 14, 2026 22:40

@SungJin1212 SungJin1212 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks

@dosubot dosubot Bot added the lgtm This PR has been approved by a maintainer label Aug 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component/alertmanager lgtm This PR has been approved by a maintainer size/M type/bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants