diff --git a/alert/alert.go b/alert/alert.go index 0904b95c19..86f965d19a 100644 --- a/alert/alert.go +++ b/alert/alert.go @@ -61,7 +61,7 @@ func (a Alert) Normalize() (*Alert, error) { err := validate.Many( validate.Text("Summary", a.Summary, 1, MaxSummaryLength), validate.Text("Details", a.Details, 0, MaxDetailsLength), - validate.OneOf("Source", a.Source, SourceManual, SourceGrafana, SourceSite24x7, SourcePrometheusAlertmanager, SourceEmail, SourceGeneric, SourceUniversal), + validate.OneOf("Source", a.Source, SourceManual, SourceGrafana, SourceSite24x7, SourcePrometheusAlertmanager, SourceCloudwatch, SourceAzureMonitor, SourceEmail, SourceGeneric, SourceUniversal), validate.OneOf("Status", a.Status, StatusTriggered, StatusActive, StatusClosed), validate.UUID("ServiceID", a.ServiceID), ) diff --git a/alert/alertlog/legacylogs.go b/alert/alertlog/legacylogs.go index 1e68d79e42..38429ac165 100644 --- a/alert/alertlog/legacylogs.go +++ b/alert/alertlog/legacylogs.go @@ -103,6 +103,10 @@ func createdSubject(msg string) *Subject { return &Subject{Type: SubjectTypeIntegrationKey, Classifier: "Site24x7"} case "Created via: prometheusAlertmanager": return &Subject{Type: SubjectTypeIntegrationKey, Classifier: "PrometheusAlertmanager"} + case "Created via: cloudwatch": + return &Subject{Type: SubjectTypeIntegrationKey, Classifier: "CloudWatch"} + case "Created via: azureMonitor": + return &Subject{Type: SubjectTypeIntegrationKey, Classifier: "Azure Monitor"} case "Created via: manual": return &Subject{Type: SubjectTypeUser, Classifier: "Web"} case "Created via: generic": diff --git a/alert/alertlog/store.go b/alert/alertlog/store.go index 786d97e210..e98d56e164 100644 --- a/alert/alertlog/store.go +++ b/alert/alertlog/store.go @@ -384,6 +384,10 @@ func (s *Store) logEntry(ctx context.Context, tx *sql.Tx, _type Type, meta inter r.subject.classifier = "Grafana" case integrationkey.TypeSite24x7: r.subject.classifier = "Site24x7" + case integrationkey.TypeCloudwatch: + r.subject.classifier = "CloudWatch" + case integrationkey.TypeAzureMonitor: + r.subject.classifier = "Azure Monitor" case integrationkey.TypeEmail: r.subject.classifier = "Email" } diff --git a/alert/details.go b/alert/details.go new file mode 100644 index 0000000000..c2d4672221 --- /dev/null +++ b/alert/details.go @@ -0,0 +1,61 @@ +package alert + +import ( + "context" + + "github.com/target/goalert/gadb" + "github.com/target/goalert/permission" + "github.com/target/goalert/validation/validate" +) + +// LockDetailsTx returns an alert's current Details, locking its row for the rest +// of the transaction. +// +// This exists so an append can be done as a read-modify-write without losing a +// concurrent one: the lock makes a second appender wait for the first to commit +// and then read the value it wrote, instead of both reading the same original and +// one overwriting the other. +func (s Store) LockDetailsTx(ctx context.Context, db gadb.DBTX, alertID int) (string, error) { + err := permission.LimitCheckAny(ctx, permission.User, permission.Service) + if err != nil { + return "", err + } + + return gadb.New(db).Alert_LockOneAlertDetails(ctx, gadb.Alert_LockOneAlertDetailsParams{ + ID: int64(alertID), + ServiceID: permission.ServiceNullUUID(ctx), // only provide service_id restriction if request is from a service + }) +} + +// SetDetailsTx sets the Details for an existing alert, replacing it wholesale. +// +// Unlike SetMetadataTx this does not merge: Details is free text with no +// key/value structure to merge on, so a caller wanting to preserve existing +// content must read it first and pass the full text it wants stored. +func (s Store) SetDetailsTx(ctx context.Context, db gadb.DBTX, alertID int, details string) error { + err := permission.LimitCheckAny(ctx, permission.User, permission.Service) + if err != nil { + return err + } + + err = validate.Text("Details", details, 0, MaxDetailsLength) + if err != nil { + return err + } + + rowCount, err := gadb.New(db).Alert_SetDetails(ctx, gadb.Alert_SetDetailsParams{ + ID: int64(alertID), + Details: details, + ServiceID: permission.ServiceNullUUID(ctx), // only provide service_id restriction if request is from a service + }) + if err != nil { + return err + } + + if rowCount == 0 { + // shouldn't happen, but just in case + return permission.NewAccessDenied("alert closed, invalid, or wrong service") + } + + return nil +} diff --git a/alert/metadata.go b/alert/metadata.go index e0ded2c4a9..bae116ef9e 100644 --- a/alert/metadata.go +++ b/alert/metadata.go @@ -20,6 +20,22 @@ type metadataDBFormat struct { AlertMetaV1 map[string]string } +// LockMetadataTx locks the alert's row for the remainder of the transaction, so +// a metadata read-modify-write cannot interleave with a concurrent one. Returns +// sql.ErrNoRows if the alert does not exist. +// +// Callers must take this lock BEFORE calling Metadata, or the lock does nothing: +// locking after the read has already raced. +func (s Store) LockMetadataTx(ctx context.Context, db gadb.DBTX, alertID int) error { + err := permission.LimitCheckAny(ctx, permission.System, permission.User, permission.Service) + if err != nil { + return err + } + + _, err = gadb.New(db).Alert_LockOneAlertMetadata(ctx, int64(alertID)) + return err +} + // Metadata returns the metadata for a single alert. If err == nil, meta is guaranteed to be non-nil. If the alert has no metadata, an empty map is returned. func (s *Store) Metadata(ctx context.Context, db gadb.DBTX, alertID int) (meta map[string]string, err error) { err = permission.LimitCheckAny(ctx, permission.System, permission.User) diff --git a/alert/queries.sql b/alert/queries.sql index e51d6114ee..31285d4c12 100644 --- a/alert/queries.sql +++ b/alert/queries.sql @@ -104,6 +104,48 @@ ON CONFLICT (alert_id) WHERE alert_data.alert_id = $1; +-- name: Alert_LockOneAlertMetadata :one +-- Locks the alert's row for a metadata read-modify-write, so two concurrent +-- writers cannot both read the same starting document and one silently +-- overwrite the other. Locks alerts, not alert_data: alert_data has no row +-- before an alert's first metadata write, and FOR UPDATE against a table with +-- no matching row locks nothing, so it could not serialize the first-writer +-- case, which is the common one for a brand new alert. +SELECT + id +FROM + alerts +WHERE + id = $1 +FOR UPDATE; + +-- name: Alert_LockOneAlertDetails :one +-- Returns the details for the alert and locks its row, so that a read-modify-write +-- of details cannot interleave with a concurrent one. +SELECT + details +FROM + alerts +WHERE + id = $1 + -- ensure the alert is associated with the service, if coming from an integration + AND (service_id = $2 + OR $2 IS NULL) +FOR UPDATE; + +-- name: Alert_SetDetails :execrows +-- Sets the details for the alert. +UPDATE + alerts +SET + details = $2 +WHERE + id = $1 + AND status != 'closed' + -- ensure the alert is associated with the service, if coming from an integration + AND (service_id = $3 + OR $3 IS NULL); + -- name: Alert_ServiceEPHasSteps :one -- Returns true if the Escalation Policy for the provided service has at least one step. SELECT diff --git a/alert/source.go b/alert/source.go index 5c7da87706..8d58b6438c 100644 --- a/alert/source.go +++ b/alert/source.go @@ -14,6 +14,8 @@ const ( SourceGrafana Source = "grafana" // grafana alert SourceSite24x7 Source = "site24x7" // site24x7 alert SourcePrometheusAlertmanager Source = "prometheusAlertmanager" // prometheus alertmanager alert + SourceCloudwatch Source = "cloudwatch" // AWS CloudWatch alarm (via SNS) + SourceAzureMonitor Source = "azureMonitor" // Azure Monitor alert (action group webhook) SourceManual Source = "manual" // manually triggered SourceGeneric Source = "generic" // generic API SourceUniversal Source = "universal" // universal integration diff --git a/app/config.go b/app/config.go index 0f6108dd8e..2e5706e5b9 100644 --- a/app/config.go +++ b/app/config.go @@ -61,6 +61,11 @@ type Config struct { TwilioBaseURL string SlackBaseURL string + // CloudwatchBaseURL overrides the origin used for outbound SNS certificate + // and subscription-confirmation requests. Testing only; there is no flag for + // it. The host allowlist still runs against the message-supplied URL. + CloudwatchBaseURL string + DBURL string DBURLNext string diff --git a/app/inithttp.go b/app/inithttp.go index 8a62398df7..0438fade63 100644 --- a/app/inithttp.go +++ b/app/inithttp.go @@ -9,6 +9,8 @@ import ( "github.com/prometheus/client_golang/prometheus/promhttp" "github.com/target/goalert/app/csp" + "github.com/target/goalert/azuremonitor" + "github.com/target/goalert/cloudwatch" "github.com/target/goalert/config" "github.com/target/goalert/expflag" "github.com/target/goalert/genericapi" @@ -122,6 +124,15 @@ func (app *App) initHTTP(ctx context.Context) error { UserStore: app.UserStore, }) + cw, err := cloudwatch.NewHandler(cloudwatch.Config{ + AlertStore: app.AlertStore, + IntegrationKeyStore: app.IntegrationKeyStore, + BaseURL: app.cfg.CloudwatchBaseURL, + }) + if err != nil { + return err + } + mux.Handle("POST /api/graphql", app.graphql2.Handler()) mux.HandleFunc("GET /api/v2/config", app.ConfigStore.ServeConfig) @@ -148,6 +159,11 @@ func (app *App) initHTTP(ctx context.Context) error { mux.HandleFunc("POST /api/v2/grafana/incoming", grafana.GrafanaToEventsAPI(app.AlertStore, app.IntegrationKeyStore)) mux.HandleFunc("POST /api/v2/site24x7/incoming", site24x7.Site24x7ToEventsAPI(app.AlertStore, app.IntegrationKeyStore)) mux.HandleFunc("POST /api/v2/prometheusalertmanager/incoming", prometheus.PrometheusAlertmanagerEventsAPI(app.AlertStore, app.IntegrationKeyStore)) + mux.HandleFunc("POST /api/v2/cloudwatch/incoming", cw.ServeIncoming) + mux.HandleFunc("POST /api/v2/azuremonitor/incoming", azuremonitor.NewHandler(azuremonitor.Config{ + AlertStore: app.AlertStore, + IntegrationKeyStore: app.IntegrationKeyStore, + }).ServeIncoming) mux.HandleFunc("POST /api/v2/generic/incoming", generic.ServeCreateAlert) mux.HandleFunc("POST /api/v2/heartbeat/{heartbeatID}", generic.ServeHeartbeatCheck) diff --git a/auth/handler.go b/auth/handler.go index a829f4362b..dc368001ce 100644 --- a/auth/handler.go +++ b/auth/handler.go @@ -594,6 +594,10 @@ func (h *Handler) authWithToken(w http.ResponseWriter, req *http.Request, next h ctx, err = h.cfg.IntKeyStore.Authorize(ctx, *tok, integrationkey.TypeSite24x7) case "/api/v2/prometheusalertmanager/incoming": ctx, err = h.cfg.IntKeyStore.Authorize(ctx, *tok, integrationkey.TypePrometheusAlertmanager) + case "/api/v2/cloudwatch/incoming": + ctx, err = h.cfg.IntKeyStore.Authorize(ctx, *tok, integrationkey.TypeCloudwatch) + case "/api/v2/azuremonitor/incoming": + ctx, err = h.cfg.IntKeyStore.Authorize(ctx, *tok, integrationkey.TypeAzureMonitor) case "/api/v2/calendar": ctx, err = h.cfg.CalSubStore.Authorize(ctx, *tok) default: diff --git a/azuremonitor/azuremonitor.go b/azuremonitor/azuremonitor.go new file mode 100644 index 0000000000..5e7cd9326c --- /dev/null +++ b/azuremonitor/azuremonitor.go @@ -0,0 +1,140 @@ +// Package azuremonitor implements ingress for Azure Monitor alerts delivered by +// an action-group webhook receiver. +// +// # Trust model +// +// Azure signs nothing. Unlike the sibling cloudwatch integration -- where SNS +// signs every message with a verifiable certificate chain -- the integration key +// in the query string is the only credential, so the webhook URL is +// credential-grade: anyone holding it can create arbitrary alerts on the service +// the key identifies. There is no public key to verify against, so asymmetric +// verification is not possible; anything built would be a shared secret with +// extra steps. +// +// The handler makes no outbound requests at all, so there is no SSRF surface and +// nothing analogous to cloudwatch's host allowlist or certificate cache. +// +// # Service resolution +// +// The integration key selects the GoAlert service. Nothing here knows or can know +// which action group, subscription, or team it is serving -- the only inputs are +// the request body and the key. That is what lets one deployed handler serve any +// number of action groups with no routing table and no per-group code. +package azuremonitor + +import ( + "errors" + "io" + "net/http" + "time" + + "github.com/target/goalert/alert" + "github.com/target/goalert/integrationkey" + "github.com/target/goalert/permission" + "github.com/target/goalert/retry" + "github.com/target/goalert/util/errutil" + "github.com/target/goalert/util/log" +) + +// maxBodyBytes bounds the request body independently of the global +// maxBodySizeMiddleware, which is disabled when MaxReqBodyBytes is 0. +const maxBodyBytes = 256 * 1024 + +// Config configures a Handler. +type Config struct { + AlertStore *alert.Store + IntegrationKeyStore *integrationkey.Store +} + +// Handler serves the Azure Monitor ingress endpoint. +type Handler struct { + cfg Config +} + +// NewHandler returns a Handler for the given config. +func NewHandler(cfg Config) *Handler { return &Handler{cfg: cfg} } + +// ServeIncoming handles an Azure Monitor action-group webhook delivery. +func (h *Handler) ServeIncoming(w http.ResponseWriter, r *http.Request) { + ctx := r.Context() + + err := permission.LimitCheckAny(ctx, permission.Service) + if errutil.HTTPError(ctx, w, err) { + return + } + + // MaxBytesReader rather than io.LimitReader: errutil.HTTPError maps + // *http.MaxBytesError to a clean 413, whereas silent truncation would surface + // as a confusing 400. + r.Body = http.MaxBytesReader(w, r.Body, maxBodyBytes) + data, err := io.ReadAll(r.Body) + if errutil.HTTPErrorRetry(ctx, w, err) { + return + } + + a, meta, info, err := buildAlert(data) + switch { + case errors.Is(err, errLegacySchema): + // Deliberately not degraded to the best-effort path: a legacy-schema + // payload would yield alerts with no useful content and no indication + // why. 400 rather than 5xx so Azure does not retry a receiver that is + // permanently misconfigured. + log.Logf(ctx, "azuremonitor: %v", err) + http.Error(w, err.Error(), http.StatusBadRequest) + return + case err != nil: + log.Debugf(ctx, "azuremonitor: bad request body: %v", err) + http.Error(w, http.StatusText(http.StatusBadRequest), http.StatusBadRequest) + return + } + + // signalType alone does not identify the payload shape -- Platform and + // Prometheus metric alerts share it, as do Log Alerts V2 and Azure Backup -- + // so log the whole triple that actually determines parsing. + ctx = log.WithFields(ctx, log.Fields{ + "AzureSignalType": info.SignalType, + "AzureMonitorService": info.MonitorService, + "AzureConditionType": info.ConditionType, + "AzureMonitorCondition": meta["monitor_condition"], + }) + + // Nothing recognised alertContext, so this alert carries essentials only. That + // is a deliberate graceful degradation rather than a failure, but it is also + // how a newly-routed Azure alert type announces itself -- otherwise the only + // symptom is thin alerts that nobody notices. + if !info.ContextRendered { + log.Logf(ctx, "azuremonitor: unrecognised alertContext, alert built from essentials only") + } + + a.ServiceID = permission.ServiceID(ctx) + if len(meta) == 0 { + meta = nil + } + + var created *alert.Alert + err = retry.DoTemporaryError(func(int) error { + var err error + created, _, err = h.cfg.AlertStore.CreateOrUpdateWithMeta(ctx, &a, meta) + return err + }, + retry.Log(ctx), + retry.Limit(5), + retry.FibBackoff(250*time.Millisecond), + ) + // HTTPErrorRetry, not HTTPError: Azure's retryable set is 408, 429, 503 and 504 + // -- 500 is absent, so mapping an exhausted database failure to 500 would drop + // the alert with no retries at all, which is exactly the case most worth + // retrying (an Aurora failover). 503 is retried by Azure and by SNS. + if errutil.HTTPErrorRetry(ctx, w, err) { + return + } + + // created is nil with a nil error when the status was closed and no open alert + // held this dedup key -- a Resolved delivery for an alert we never opened, or + // already closed. Normal, so info level and a 2xx. + if created == nil { + log.Logf(ctx, "azuremonitor: no open alert to close") + } + + w.WriteHeader(http.StatusNoContent) +} diff --git a/azuremonitor/payload.go b/azuremonitor/payload.go new file mode 100644 index 0000000000..6351d75517 --- /dev/null +++ b/azuremonitor/payload.go @@ -0,0 +1,713 @@ +package azuremonitor + +import ( + "crypto/sha256" + "encoding/hex" + "encoding/json" + "errors" + "fmt" + "net/url" + "regexp" + "sort" + "strconv" + "strings" + + "github.com/target/goalert/alert" + "github.com/target/goalert/validation/validate" +) + +// commonAlertSchemaID is the only schema this integration parses. Azure's legacy +// schema is a different, flatter shape that varies by alert type; a receiver +// sending it is rejected with an actionable message rather than degraded. +const commonAlertSchemaID = "azureMonitorCommonAlertSchema" + +// conditionType values we render natively. Anything else routes to the +// best-effort fallback. +const ( + condSingleResourceMultipleMetric = "SingleResourceMultipleMetricCriteria" + condDynamicThreshold = "DynamicThresholdCriteria" + condLogQuery = "LogQueryCriteria" +) + +const ( + // maxSearchQueryLen bounds the KQL query. It is the one unbounded field that + // precedes the search-results link in details, so without a cap a long query + // would push the link past MaxDetailsLength and truncate it mid-URL. + maxSearchQueryLen = 1024 + + // maxExpressionLen bounds the PromQL expression, which is likewise unbounded. + maxExpressionLen = 1024 + + // maxDescriptionLen bounds essentials.description, which is rendered last. + maxDescriptionLen = 2048 + + // maxMetaValueLen is a first-pass bound on each value, in RUNES. It is not by + // itself sufficient to stay inside alert.ValidateMetadata's total cap, which + // sums BYTES: buildMeta populates up to a dozen keys, and a rune-heavy + // (multi-byte) value at this cap on several of them can still add up to more + // bytes than the total allows. cleanMeta enforces the real, byte-based budget + // as a second pass. + maxMetaValueLen = 1024 + + // maxMetaTotalBytes leaves headroom under alert.ValidateMetadata's 32768-byte + // cap for the metadata keys themselves -- short ASCII constants, but sized off + // as a margin rather than their exact total so this doesn't need updating + // every time a key is added to buildMeta. + maxMetaTotalBytes = 32000 +) + +// envelope is the Azure Monitor common alert schema. +// +// https://learn.microsoft.com/azure/azure-monitor/alerts/alerts-common-schema +type envelope struct { + SchemaID string `json:"schemaId"` + Data struct { + Essentials essentials `json:"essentials"` + AlertContext alertContext `json:"alertContext"` + CustomProps map[string]string `json:"customProperties"` + } `json:"data"` +} + +// essentials is present and identically shaped on every payload regardless of +// signal type, which is what makes the fallback path viable. +type essentials struct { + AlertID string `json:"alertId"` + AlertRule string `json:"alertRule"` + AlertRuleID string `json:"alertRuleId"` + Severity string `json:"severity"` + SignalType string `json:"signalType"` + MonitorCondition string `json:"monitorCondition"` + MonitoringService string `json:"monitoringService"` + AlertTargetIDs []string `json:"alertTargetIDs"` + ConfigurationItems []string `json:"configurationItems"` + OriginAlertID string `json:"originAlertId"` + FiredDateTime string `json:"firedDateTime"` + ResolvedDateTime string `json:"resolvedDateTime"` + Description string `json:"description"` + + // TargetResourceGroup and TargetResourceType are the routing/context fields + // Microsoft documents the essentials block as existing to provide. + TargetResourceGroup string `json:"targetResourceGroup"` + TargetResourceType string `json:"targetResourceType"` + + // InvestigationLink opens Azure Monitor's AI investigation experience + // (Observability Agent) for the alert -- NOT the alert itself. It is a + // supplement to the portal link, never a replacement for it. + InvestigationLink string `json:"investigationLink"` +} + +// alertContext varies by signal type. Only the fields we render are declared; +// conditionType is the discriminator, and its absence is expected (Service +// Health and activity-log payloads have no condition at all). +type alertContext struct { + ConditionType string `json:"conditionType"` + + Condition struct { + WindowSize string `json:"windowSize"` + WindowStartTime string `json:"windowStartTime"` + WindowEndTime string `json:"windowEndTime"` + AllOf []criterion `json:"allOf"` + } `json:"condition"` + + // Properties duplicates data.customProperties on some payloads, and carries + // the Service Health incident detail on others. + Properties map[string]json.RawMessage `json:"properties"` + + // Azure Managed Prometheus. This shape has no conditionType and no + // condition.allOf; the PromQL expression and the rule's annotations carry the + // diagnostic content instead. + Expression string `json:"expression"` + ExpressionValue string `json:"expressionValue"` + For string `json:"for"` + Interval string `json:"interval"` + Labels map[string]string `json:"labels"` + Annotations map[string]string `json:"annotations"` + RuleGroup string `json:"ruleGroup"` +} + +// criterion is one entry of condition.allOf. Metric and log criteria share this +// envelope, differing only in which fields are populated. +type criterion struct { + MetricName string `json:"metricName"` + MetricNamespace string `json:"metricNamespace"` + Operator string `json:"operator"` + TimeAggregation string `json:"timeAggregation"` + Dimensions []dimension `json:"dimensions"` + + // Threshold is a string while MetricValue is a number -- do not assume they + // share a type. + Threshold string `json:"threshold"` + MetricValue *float64 `json:"metricValue"` + + // DynamicThresholdCriteria only. Threshold is a sensitivity artifact for + // these, not a meaningful limit, so these are rendered instead. + AlertSensitivity string `json:"alertSensitivity"` + FailingPeriods *failingPeriods `json:"failingPeriods"` + + // WebtestLocationAvailabilityCriteria only. + WebTestName string `json:"webTestName"` + + // LogQueryCriteria only. + SearchQuery string `json:"searchQuery"` + MetricMeasureColumn string `json:"metricMeasureColumn"` + LinkToFilteredSearchResultsUI string `json:"linkToFilteredSearchResultsUI"` + LinkToSearchResultsUI string `json:"linkToSearchResultsUI"` +} + +type dimension struct { + Name string `json:"name"` + Value string `json:"value"` +} + +type failingPeriods struct { + NumberOfEvaluationPeriods *float64 `json:"numberOfEvaluationPeriods"` + MinFailingPeriodsToAlert *float64 `json:"minFailingPeriodsToAlert"` +} + +// errLegacySchema is returned for a payload that is not the common alert schema. +// The message names the fix because an operator seeing it needs to change an +// action-group setting, not debug GoAlert. +var errLegacySchema = fmt.Errorf( + "azuremonitor: unsupported schemaId; enable the common alert schema on this action group's webhook receiver") + +// parseInfo reports which shape a payload was recognised as, so the caller can +// log it. ContextRendered is the important one: false means the alert carries +// essentials only because nothing recognised alertContext, which is a graceful +// outcome but also the signal that a new Azure alert type has started arriving. +type parseInfo struct { + SignalType string + MonitorService string + ConditionType string + ContextRendered bool +} + +// buildAlert maps an Azure Monitor webhook body onto an alert. +// +// The returned alert has ServiceID unset; the caller fills it in from the +// integration key. Source is always alert.SourceAzureMonitor and Dedup is always +// non-nil -- a nil dedup silently falls back to a content hash that changes +// between the Fired and Resolved deliveries, which would break the close path. +// +// An error is returned only for a payload that is not the common alert schema. +// Every other input, however unrecognised, produces a best-effort alert built +// from essentials rather than a failure. +func buildAlert(body []byte) (alert.Alert, map[string]string, parseInfo, error) { + var env envelope + err := json.Unmarshal(body, &env) + + // Tolerate a type mismatch on an individual field: encoding/json still fills + // everything it could decode, so a numerically-typed threshold or dimension + // value costs that one value rather than the whole alert. Azure documents + // these as strings but is not consistent across shapes, and a hard failure + // here means a 400 -- which Azure does not retry -- so the page is lost. + var typeErr *json.UnmarshalTypeError + if err != nil && !errors.As(err, &typeErr) { + return alert.Alert{}, nil, parseInfo{}, err + } + if env.SchemaID != commonAlertSchemaID { + return alert.Alert{}, nil, parseInfo{}, errLegacySchema + } + + e := env.Data.Essentials + ctx := env.Data.AlertContext + + status := alert.StatusTriggered + // monitorCondition is the schema-stable field; alertContext.status can + // disagree with it (the incident resolved while the alert fired). + if strings.EqualFold(e.MonitorCondition, "Resolved") { + status = alert.StatusClosed + } + + // Rendered once and passed in, so the caller can also report whether anything + // recognised alertContext without parsing twice. + ctxLines := contextLines(ctx) + info := parseInfo{ + SignalType: e.SignalType, + MonitorService: e.MonitoringService, + ConditionType: ctx.ConditionType, + ContextRendered: len(ctxLines) > 0, + } + + summary := validate.SanitizeText(alertSummary(e), alert.MaxSummaryLength) + details := validate.SanitizeText(alertDetails(e, ctx, ctxLines, env.Data.CustomProps), alert.MaxDetailsLength) + + return alert.Alert{ + Summary: summary, + Details: details, + Source: alert.SourceAzureMonitor, + Status: status, + Dedup: alert.NewUserDedup(sha256Hex(dedupKey(e))), + }, buildMeta(e, ctx), info, nil +} + +// dedupKey returns the stable identity of one alert firing. +// +// alertId is the alert *instance* ID and is stable across the Fired and Resolved +// deliveries of one firing (Azure alerts are stateful), so it closes the alert it +// opened. Never use originAlertId: it is per-rule for metric alerts, so a single +// missed close would mute that rule forever. +// +// When alertId is absent the weaker identifiers are combined instead. Hashing "" +// would hand every such payload the same key, collapsing unrelated alerts onto a +// single one that any one Resolved delivery could then close. Every field used +// here is fixed for the life of a firing -- notably excluding resolvedDateTime, +// which appears only on the Resolved delivery and would stop it matching the +// Fired delivery it needs to close. +func dedupKey(e essentials) string { + if e.AlertID != "" { + return e.AlertID + } + + return strings.Join([]string{ + e.AlertRuleID, + e.AlertRule, + strings.Join(e.ConfigurationItems, ","), + e.FiredDateTime, + }, "|") +} + +// portalURL builds an Azure Portal deep link to the alert-details page from a +// full ARM alert ID. essentials.alertId is one: +// /subscriptions//providers/Microsoft.AlertsManagement/alerts/. +// +// The generic "#resource/" pattern does NOT work here: Microsoft.AlertsManagement +// alerts have no resource blade, so the portal cannot render one and drops the +// user on a default view. AlertDetailsTemplateBlade is the route that resolves, +// and the ID must be percent-encoded because it is carried as a single path +// segment. +func portalURL(alertID string) string { + if !strings.HasPrefix(alertID, "/subscriptions/") { + return "" + } + return "https://portal.azure.com/#blade/Microsoft_Azure_Monitoring/AlertDetailsTemplateBlade/alertId/" + + url.PathEscape(alertID) +} + +// alertSummary falls back through progressively weaker identifiers. Azure always +// sends alertRule in practice, but an empty summary does not error -- it creates +// a blank, unactionable alert -- so the fallback is a correctness requirement. +func alertSummary(e essentials) string { + // Each candidate is sanitized before the emptiness test, not TrimSpace: the + // caller re-sanitizes the return value anyway, but SanitizeText also strips + // non-printable control characters that TrimSpace leaves alone. Testing the + // raw value would let e.g. AlertRule == "\x01\x02" through as non-blank, only + // for the caller's sanitize pass to reduce it to "". That does NOT fail + // validation -- validate.Text treats an empty body as valid regardless of its + // minimum length -- so the remaining fallbacks would be silently skipped and + // the alert created with a blank Summary: real, but useless to whoever is + // paged. + if rule := validate.SanitizeText(e.AlertRule, alert.MaxSummaryLength); rule != "" { + return rule + } + if items := nonEmpty(e.ConfigurationItems); len(items) > 0 { + return "Azure Monitor alert on " + strings.Join(items, ", ") + } + if signal := validate.SanitizeText(e.SignalType, alert.MaxSummaryLength); signal != "" { + return "Azure Monitor " + signal + " alert" + } + return "Azure Monitor alert" +} + +// alertDetails renders short high-value fields first so that truncation at +// MaxDetailsLength eats the least useful content. The search-results link is +// kept whole by capping searchQuery upstream of it. +func alertDetails(e essentials, ctx alertContext, ctxLines []string, custom map[string]string) string { + var lines []string + add := func(label, value string) { + if value != "" { + lines = append(lines, label+": "+value) + } + } + + add("Severity", e.Severity) + add("Monitor condition", e.MonitorCondition) + add("Signal type", e.SignalType) + add("Monitoring service", e.MonitoringService) + add("Fired", e.FiredDateTime) + add("Resolved", e.ResolvedDateTime) + + // configurationItems are short resource names; alertTargetIDs are full ARM + // paths, so prefer the former and fall back only when absent. + if items := nonEmpty(e.ConfigurationItems); len(items) > 0 { + add("Resource", strings.Join(items, ", ")) + } else if targets := nonEmpty(e.AlertTargetIDs); len(targets) > 0 { + add("Resource", strings.Join(targets, ", ")) + } + add("Resource group", e.TargetResourceGroup) + add("Resource type", e.TargetResourceType) + // Both links, when available: they are different destinations. Portal is the + // alert itself; investigationLink opens the AI investigation agent, which is + // no substitute for the alert page and must not suppress it. + add("Portal", portalURL(e.AlertID)) + add("Investigate", e.InvestigationLink) + + if len(ctxLines) > 0 { + lines = append(lines, "") + lines = append(lines, ctxLines...) + } + + // customProperties is operator-settable per rule and is the natural place for + // a runbook URL -- the closest Azure analogue to CloudWatch's + // AlarmDescription. alertContext.properties duplicates it on some payloads. + props := custom + if len(props) == 0 { + props = stringProps(ctx.Properties) + } + if len(props) > 0 { + lines = append(lines, "") + for _, k := range sortedKeys(props) { + add(k, props[k]) + } + } + + if d := strings.TrimSpace(e.Description); d != "" { + lines = append(lines, "", truncRunes(d, maxDescriptionLen)) + } + + return strings.Join(lines, "\n") +} + +// contextLines renders alertContext by conditionType. An unrecognised or absent +// conditionType yields no lines at all, which is the fallback: essentials alone +// still produces a usable alert. +func contextLines(ctx alertContext) []string { + // Dispatch on the presence of condition.allOf rather than an allowlist of + // conditionType values. Every metric and log criteria shape shares this one + // envelope, so this covers MultipleResourceMultipleMetricCriteria (used by + // multi-resource and resource-group-scoped metric rules) and + // WebtestLocationAvailabilityCriteria without special-casing either, and + // stays correct for whatever Azure adds next. + // + // conditionType still selects the only two behaviours that genuinely differ: + // suppressing the meaningless dynamic threshold, and the log query lines. + if len(ctx.Condition.AllOf) == 0 { + // Prometheus rule groups have no condition either, but carry their own + // distinct fields. + if lines := prometheusLines(ctx); len(lines) > 0 { + return lines + } + // Service Health and activity-log payloads have no condition at all; + // they carry their detail in properties instead. + return serviceHealthLines(ctx) + } + + var lines []string + for _, c := range ctx.Condition.AllOf { + lines = append(lines, criterionLines(c, ctx.ConditionType, ctx.Condition.WindowSize)...) + } + return lines +} + +// criterionLines renders one condition.allOf entry. allOf is an array -- a rule +// can carry several criteria and all of them are rendered. +func criterionLines(c criterion, condType, windowSize string) []string { + var lines []string + + if condType == condLogQuery { + if q := strings.TrimSpace(c.SearchQuery); q != "" { + lines = append(lines, "Query: "+truncRunes(q, maxSearchQueryLen)) + } + if c.MetricMeasureColumn != "" { + lines = append(lines, "Measure column: "+c.MetricMeasureColumn) + } + } + + if head := c.headline(condType, windowSize); head != "" { + lines = append(lines, head) + } + + // The namespace disambiguates same-named metrics across resource types. + if c.MetricNamespace != "" { + lines = append(lines, "Namespace: "+c.MetricNamespace) + } + if c.WebTestName != "" { + lines = append(lines, "Web test: "+c.WebTestName) + } + + if condType == condDynamicThreshold { + // threshold is a sensitivity artifact here, not a limit, so render the + // sensitivity and failing-period counts instead of a misleading number. + if c.AlertSensitivity != "" { + lines = append(lines, "Sensitivity: "+c.AlertSensitivity) + } + } + if fp := c.FailingPeriods; fp != nil { + if s := fp.String(); s != "" { + lines = append(lines, "Failing periods: "+s) + } + } + + if dims := dimensionPairs(c.Dimensions); dims != "" { + lines = append(lines, "Dimensions: "+dims) + } + + // Exactly one link, and the UI one: it is dimension-scoped to this firing and + // opens in the portal. The *API variants are not clickable, and all four + // together exceed MaxDetailsLength on their own. + if link := c.LinkToFilteredSearchResultsUI; link != "" { + lines = append(lines, "Results: "+link) + } else if link := c.LinkToSearchResultsUI; link != "" { + lines = append(lines, "Results: "+link) + } + + return lines +} + +// headline renders the metric comparison line shared by metric and log criteria. +func (c criterion) headline(condType, windowSize string) string { + name := c.MetricName + if name == "" && condType == condLogQuery { + name = "Results" + } + if name == "" { + return "" + } + + var b strings.Builder + b.WriteString(name) + if c.Operator != "" { + b.WriteString(" " + c.Operator) + } + // Suppress the threshold for dynamic criteria: it is a sensitivity artifact. + if c.Threshold != "" && condType != condDynamicThreshold { + b.WriteString(" " + c.Threshold) + } + + var qual []string + if c.TimeAggregation != "" { + qual = append(qual, c.TimeAggregation) + } + if windowSize != "" { + qual = append(qual, windowSize) + } + if len(qual) > 0 { + b.WriteString(" (" + strings.Join(qual, ", ") + ")") + } + if c.MetricValue != nil { + b.WriteString(" = " + formatNum(*c.MetricValue)) + } + + return b.String() +} + +func (f failingPeriods) String() string { + if f.MinFailingPeriodsToAlert == nil || f.NumberOfEvaluationPeriods == nil { + return "" + } + return formatNum(*f.MinFailingPeriodsToAlert) + " of " + formatNum(*f.NumberOfEvaluationPeriods) +} + +// prometheusLines renders the Azure Managed Prometheus shape. The rule's +// annotations are author-written and are the most useful thing on the alert, so +// they lead; the PromQL expression and the value that tripped it follow. +func prometheusLines(ctx alertContext) []string { + if ctx.Expression == "" && ctx.RuleGroup == "" { + return nil + } + + var lines []string + + // summary and description are the Prometheus conventions and are rendered + // bare, as prose. Any other annotation is labelled. + for _, k := range []string{"summary", "description"} { + if v := strings.TrimSpace(ctx.Annotations[k]); v != "" { + lines = append(lines, v) + } + } + for _, k := range sortedKeys(ctx.Annotations) { + if k == "summary" || k == "description" { + continue + } + if v := strings.TrimSpace(ctx.Annotations[k]); v != "" { + lines = append(lines, k+": "+v) + } + } + + add := func(label, value string) { + if value != "" { + lines = append(lines, label+": "+value) + } + } + add("Expression", truncRunes(ctx.Expression, maxExpressionLen)) + add("Value", ctx.ExpressionValue) + add("For", ctx.For) + add("Interval", ctx.Interval) + + // Labels are Prometheus's equivalent of metric dimensions. + var pairs []string + for _, k := range sortedKeys(ctx.Labels) { + pairs = append(pairs, k+"="+ctx.Labels[k]) + } + add("Labels", strings.Join(pairs, ", ")) + + return lines +} + +// serviceHealthLines renders the Service Health / activity-log shape, which has +// no conditionType and carries its detail under properties. +func serviceHealthLines(ctx alertContext) []string { + props := stringProps(ctx.Properties) + if len(props) == 0 { + return nil + } + + var lines []string + add := func(label, key string) { + if v := strings.TrimSpace(props[key]); v != "" { + lines = append(lines, label+": "+v) + } + } + add("Title", "title") + add("Service", "service") + add("Region", "region") + add("Incident type", "incidentType") + add("Tracking ID", "trackingId") + add("Impact start", "impactStartTime") + add("Stage", "stage") + + return lines +} + +func buildMeta(e essentials, ctx alertContext) map[string]string { + return cleanMeta(map[string]string{ + // Prometheus payloads have no essentials.alertRuleId; ruleGroup is the + // equivalent handle back to the rule in Azure. + "rule_group": ctx.RuleGroup, + + "alert_id": e.AlertID, + "alert_rule": e.AlertRule, + "alert_rule_id": e.AlertRuleID, + "severity": e.Severity, + "signal_type": e.SignalType, + "monitoring_service": e.MonitoringService, + "monitor_condition": e.MonitorCondition, + "alert_target_ids": strings.Join(nonEmpty(e.AlertTargetIDs), ","), + "configuration_items": strings.Join(nonEmpty(e.ConfigurationItems), ","), + + "target_resource_group": e.TargetResourceGroup, + "target_resource_type": e.TargetResourceType, + }) +} + +// htmlTagRe strips markup from Service Health fields like `communication`, which +// are HTML and unreadable raw on a pager. +var htmlTagRe = regexp.MustCompile(`<[^>]*>`) + +// stringProps flattens a properties map to the string-valued entries only. +// +// Several Azure properties are strings that *contain* JSON (impactedServices, +// targetResourceTypes). They are deliberately kept as opaque strings -- decoding +// them into a struct fails. +func stringProps(raw map[string]json.RawMessage) map[string]string { + if len(raw) == 0 { + return nil + } + + out := make(map[string]string, len(raw)) + for k, v := range raw { + var s string + if err := json.Unmarshal(v, &s); err != nil { + // Non-string values (numbers, objects, null) are skipped rather than + // rendered as raw JSON. + continue + } + s = strings.TrimSpace(htmlTagRe.ReplaceAllString(s, "")) + if s != "" { + out[k] = s + } + } + if len(out) == 0 { + return nil + } + + return out +} + +func dimensionPairs(dims []dimension) string { + var pairs []string + for _, d := range dims { + if d.Name == "" && d.Value == "" { + continue + } + pairs = append(pairs, d.Name+"="+d.Value) + } + return strings.Join(pairs, ", ") +} + +func sortedKeys(m map[string]string) []string { + keys := make([]string, 0, len(m)) + for k := range m { + keys = append(keys, k) + } + // Stable output so details are deterministic across deliveries. + sort.Strings(keys) + return keys +} + +func nonEmpty(in []string) []string { + var out []string + for _, s := range in { + if strings.TrimSpace(s) != "" { + out = append(out, s) + } + } + return out +} + +// formatNum renders a float without a trailing .0, so counts read as integers. +func formatNum(f float64) string { return strconv.FormatFloat(f, 'f', -1, 64) } + +func truncRunes(s string, max int) string { + r := []rune(s) + if len(r) <= max { + return s + } + return string(r[:max]) +} + +func sha256Hex(s string) string { + sum := sha256.Sum256([]byte(s)) + return hex.EncodeToString(sum[:]) +} + +// cleanMeta drops empty values and bounds the rest so the map can never fail +// alert.ValidateMetadata's total-size check. Values are not sanitized: they are +// JSON-marshalled on write, so control characters are escaped rather than +// injected, and trimming would corrupt an ARM resource ID. +// +// The per-value rune cap alone is not enough: it bounds runes, but +// ValidateMetadata sums bytes, and enough multi-byte values at that cap can add +// up to more bytes than the total allows. The byte budget below is computed from +// however many keys are actually non-empty, rather than hardcoded to today's key +// count, so the total still fits if buildMeta gains another key later. +func cleanMeta(m map[string]string) map[string]string { + for k, v := range m { + if v == "" { + delete(m, k) + } + } + + perValueBudget := maxMetaTotalBytes + if n := len(m); n > 0 { + perValueBudget = maxMetaTotalBytes / n + } + + for k, v := range m { + v = truncRunes(v, maxMetaValueLen) + m[k] = truncBytes(v, perValueBudget) + } + return m +} + +// truncBytes truncates s to at most n bytes without splitting a UTF-8 rune's +// encoding -- a plain byte-slice cut can leave a trailing partial rune, which +// ToValidUTF8 then scrubs rather than emit invalid UTF-8 into the metadata. +func truncBytes(s string, n int) string { + if n <= 0 { + return "" + } + if len(s) <= n { + return s + } + return strings.ToValidUTF8(s[:n], "") +} diff --git a/azuremonitor/payload_test.go b/azuremonitor/payload_test.go new file mode 100644 index 0000000000..ffcc4e1608 --- /dev/null +++ b/azuremonitor/payload_test.go @@ -0,0 +1,948 @@ +package azuremonitor + +import ( + "encoding/json" + "fmt" + "strings" + "testing" + "unicode/utf8" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/target/goalert/alert" +) + +const ( + testAlertID = "/subscriptions/sub-1/providers/Microsoft.AlertsManagement/alerts/1db044ff-df8f-4064-a559-b9c9f5f4f000" + testServiceID = "3c1a1a44-8e7a-4d1b-9f4a-2b0e5c6d7f80" + testRunbook = "https://runbook.example.com/frontdoor" + + testPortalURL = "https://portal.azure.com/#blade/Microsoft_Azure_Monitoring/AlertDetailsTemplateBlade/alertId/" + + "%2Fsubscriptions%2Fsub-1%2Fproviders%2FMicrosoft.AlertsManagement%2Falerts%2F1db044ff-df8f-4064-a559-b9c9f5f4f000" +) + +// metricPayload is a SingleResourceMultipleMetricCriteria delivery. +func metricPayload(condition string) string { + return fmt.Sprintf(`{ + "schemaId": "azureMonitorCommonAlertSchema", + "data": { + "essentials": { + "alertId": %q, + "alertRule": "Too Many Frontdoor Exceptions", + "severity": "Sev2", + "signalType": "Metric", + "monitorCondition": %q, + "monitoringService": "Platform", + "alertTargetIDs": ["/subscriptions/sub-1/resourceGroups/rg/providers/Microsoft.Storage/storageAccounts/acct"], + "configurationItems": ["stage-www-frontdoor"], + "originAlertId": "sub-1_rg_Microsoft.Insights_metricAlerts_rule_604016583", + "firedDateTime": "2026-07-18T08:01:01.000Z", + "description": "Runbook: %s" + }, + "alertContext": { + "conditionType": "SingleResourceMultipleMetricCriteria", + "condition": { + "windowSize": "PT5M", + "allOf": [{ + "metricName": "Transactions", + "metricNamespace": "Microsoft.Storage/storageAccounts", + "operator": "GreaterThan", + "threshold": "0", + "timeAggregation": "Total", + "dimensions": [{"name": "ApiName", "value": "GetBlob"}], + "metricValue": 100, + "webTestName": null + }], + "windowStartTime": "2026-07-18T07:56:01.000Z", + "windowEndTime": "2026-07-18T08:01:01.000Z" + } + }, + "customProperties": {"runbook": %q} + } +}`, testAlertID, condition, testRunbook, testRunbook) +} + +func TestBuildAlert_Metric(t *testing.T) { + t.Run("golden fired alert", func(t *testing.T) { + a, meta, _, err := buildAlert([]byte(metricPayload("Fired"))) + require.NoError(t, err) + + assert.Equal(t, "Too Many Frontdoor Exceptions", a.Summary) + assert.Equal(t, alert.StatusTriggered, a.Status) + assert.Equal(t, alert.SourceAzureMonitor, a.Source) + + require.NotNil(t, a.Dedup) + assert.Equal(t, sha256Hex(testAlertID), a.Dedup.Payload) + + assert.Contains(t, a.Details, "Severity: Sev2") + assert.Contains(t, a.Details, "Signal type: Metric") + // configurationItems is preferred over the full ARM path. + assert.Contains(t, a.Details, "Resource: stage-www-frontdoor") + assert.NotContains(t, a.Details, "/resourceGroups/rg/providers") + assert.Contains(t, a.Details, "Transactions GreaterThan 0 (Total, PT5M) = 100") + assert.Contains(t, a.Details, "Dimensions: ApiName=GetBlob") + assert.Contains(t, a.Details, testRunbook) + + assert.Equal(t, "Metric", meta["signal_type"]) + assert.Equal(t, "Fired", meta["monitor_condition"]) + assert.Equal(t, testAlertID, meta["alert_id"]) + assert.Equal(t, "stage-www-frontdoor", meta["configuration_items"]) + }) + + // The close path is live for Azure (autoMitigate), so the Resolved delivery + // must produce the same dedup key or the alert never closes. + t.Run("resolved closes with the same dedup", func(t *testing.T) { + fired, _, _, err := buildAlert([]byte(metricPayload("Fired"))) + require.NoError(t, err) + resolved, meta, _, err := buildAlert([]byte(metricPayload("Resolved"))) + require.NoError(t, err) + + assert.Equal(t, alert.StatusClosed, resolved.Status) + require.NotNil(t, resolved.Dedup) + assert.Equal(t, fired.Dedup.Payload, resolved.Dedup.Payload) + assert.Equal(t, "Resolved", meta["monitor_condition"]) + }) + + // originAlertId is per-rule for metric alerts; using it would mute the rule + // forever after one missed close. + t.Run("dedup ignores originAlertId", func(t *testing.T) { + a, _, _, err := buildAlert([]byte(metricPayload("Fired"))) + require.NoError(t, err) + require.NotNil(t, a.Dedup) + assert.NotEqual(t, sha256Hex("sub-1_rg_Microsoft.Insights_metricAlerts_rule_604016583"), a.Dedup.Payload) + }) + + t.Run("blank alertRule falls back to resource", func(t *testing.T) { + body := strings.Replace(metricPayload("Fired"), `"alertRule": "Too Many Frontdoor Exceptions"`, `"alertRule": " "`, 1) + a, _, _, err := buildAlert([]byte(body)) + require.NoError(t, err) + assert.Equal(t, "Azure Monitor alert on stage-www-frontdoor", a.Summary) + }) + + t.Run("description preserved verbatim", func(t *testing.T) { + a, _, _, err := buildAlert([]byte(metricPayload("Fired"))) + require.NoError(t, err) + assert.Contains(t, a.Details, "Runbook: "+testRunbook) + }) + + t.Run("falls back to alertTargetIDs when no configurationItems", func(t *testing.T) { + body := strings.Replace(metricPayload("Fired"), `"configurationItems": ["stage-www-frontdoor"],`, "", 1) + a, _, _, err := buildAlert([]byte(body)) + require.NoError(t, err) + assert.Contains(t, a.Details, "Resource: /subscriptions/sub-1/resourceGroups/rg") + }) + + // allOf is an array; a rule can carry several criteria and all must render. + t.Run("renders every allOf entry", func(t *testing.T) { + body := strings.Replace(metricPayload("Fired"), + `"metricValue": 100, + "webTestName": null + }]`, + `"metricValue": 100, + "webTestName": null + }, { + "metricName": "Latency", + "operator": "GreaterThan", + "threshold": "500", + "timeAggregation": "Average", + "dimensions": [], + "metricValue": 900 + }]`, 1) + a, _, _, err := buildAlert([]byte(body)) + require.NoError(t, err) + assert.Contains(t, a.Details, "Transactions GreaterThan 0") + assert.Contains(t, a.Details, "Latency GreaterThan 500 (Average, PT5M) = 900") + }) + + // threshold is a string and metricValue a number -- neither may be assumed to + // be the other's type. + t.Run("integral metricValue renders without a decimal", func(t *testing.T) { + a, _, _, err := buildAlert([]byte(metricPayload("Fired"))) + require.NoError(t, err) + assert.Contains(t, a.Details, "= 100") + assert.NotContains(t, a.Details, "= 100.0") + }) +} + +func dynamicPayload() string { + return fmt.Sprintf(`{ + "schemaId": "azureMonitorCommonAlertSchema", + "data": { + "essentials": { + "alertId": %q, + "alertRule": "Dynamic Transactions", + "severity": "Sev3", + "signalType": "Metric", + "monitorCondition": "Fired", + "monitoringService": "Platform", + "configurationItems": ["test-storageAccount"] + }, + "alertContext": { + "conditionType": "DynamicThresholdCriteria", + "condition": { + "windowSize": "PT15M", + "allOf": [{ + "alertSensitivity": "Low", + "failingPeriods": {"numberOfEvaluationPeriods": 3, "minFailingPeriodsToAlert": 3}, + "ignoreDataBefore": null, + "metricName": "Transactions", + "operator": "GreaterThan", + "threshold": "0.3", + "timeAggregation": "Average", + "dimensions": [], + "metricValue": 78.09 + }] + } + } + } +}`, testAlertID) +} + +func TestBuildAlert_DynamicThreshold(t *testing.T) { + a, _, _, err := buildAlert([]byte(dynamicPayload())) + require.NoError(t, err) + + // The threshold is a sensitivity artifact, not a limit -- rendering "0.3" + // would mislead whoever is paged. + assert.NotContains(t, a.Details, "0.3") + assert.Contains(t, a.Details, "Sensitivity: Low") + assert.Contains(t, a.Details, "Failing periods: 3 of 3") + assert.Contains(t, a.Details, "Transactions GreaterThan (Average, PT15M) = 78.09") +} + +func logPayload(searchQuery, link string) string { + return fmt.Sprintf(`{ + "schemaId": "azureMonitorCommonAlertSchema", + "data": { + "essentials": { + "alertId": %q, + "alertRule": "Heartbeat missing", + "severity": "Sev1", + "signalType": "Log", + "monitorCondition": "Fired", + "monitoringService": "Log Alerts V2", + "configurationItems": ["test-computer"] + }, + "alertContext": { + "conditionType": "LogQueryCriteria", + "condition": { + "windowSize": "PT10M", + "allOf": [{ + "searchQuery": %q, + "metricMeasureColumn": null, + "targetResourceTypes": "['Microsoft.OperationalInsights/workspaces']", + "operator": "GreaterThan", + "threshold": "0", + "timeAggregation": "Count", + "dimensions": [{"name": "Computer", "value": "test-computer"}], + "metricValue": 3, + "failingPeriods": {"numberOfEvaluationPeriods": 1, "minFailingPeriodsToAlert": 1}, + "linkToSearchResultsUI": "https://portal.azure.com#@unfiltered", + "linkToFilteredSearchResultsUI": %q, + "linkToSearchResultsAPI": "https://api.loganalytics.io/v1/workspaces/unfiltered", + "linkToFilteredSearchResultsAPI": "https://api.loganalytics.io/v1/workspaces/filtered" + }] + } + } + } +}`, testAlertID, searchQuery, link) +} + +func TestBuildAlert_LogQuery(t *testing.T) { + const link = "https://portal.azure.com#@filtered-link" + + t.Run("golden log alert", func(t *testing.T) { + a, meta, _, err := buildAlert([]byte(logPayload("Heartbeat", link))) + require.NoError(t, err) + + assert.Equal(t, "Heartbeat missing", a.Summary) + assert.Equal(t, alert.StatusTriggered, a.Status) + assert.Contains(t, a.Details, "Query: Heartbeat") + assert.Contains(t, a.Details, "GreaterThan 0 (Count, PT10M) = 3") + assert.Contains(t, a.Details, "Failing periods: 1 of 1") + assert.Contains(t, a.Details, "Dimensions: Computer=test-computer") + assert.Equal(t, "Log", meta["signal_type"]) + }) + + // Exactly one link, and the filtered UI one. All four together exceed + // MaxDetailsLength on their own. + t.Run("exactly one link, the filtered UI one", func(t *testing.T) { + a, _, _, err := buildAlert([]byte(logPayload("Heartbeat", link))) + require.NoError(t, err) + + assert.Contains(t, a.Details, link) + assert.NotContains(t, a.Details, "unfiltered") + assert.NotContains(t, a.Details, "api.loganalytics.io") + assert.Equal(t, 1, strings.Count(a.Details, "Results: ")) + }) + + t.Run("falls back to unfiltered link when filtered is absent", func(t *testing.T) { + body := strings.Replace(logPayload("Heartbeat", link), `"linkToFilteredSearchResultsUI": "`+link+`",`, "", 1) + a, _, _, err := buildAlert([]byte(body)) + require.NoError(t, err) + assert.Contains(t, a.Details, "https://portal.azure.com#@unfiltered") + }) + + t.Run("metricMeasureColumn rendered only when set", func(t *testing.T) { + a, _, _, err := buildAlert([]byte(logPayload("Heartbeat", link))) + require.NoError(t, err) + assert.NotContains(t, a.Details, "Measure column") + + body := strings.Replace(logPayload("Heartbeat", link), `"metricMeasureColumn": null`, `"metricMeasureColumn": "Duration"`, 1) + a, _, _, err = buildAlert([]byte(body)) + require.NoError(t, err) + assert.Contains(t, a.Details, "Measure column: Duration") + }) + + // targetResourceTypes is a string containing a JSON array, not an array. + t.Run("string-containing-json targetResourceTypes does not error", func(t *testing.T) { + a, _, _, err := buildAlert([]byte(logPayload("Heartbeat", link))) + require.NoError(t, err) + assert.NotEmpty(t, a.Summary) + }) + + // The test that catches naive truncation: the link must survive whole, not be + // cut mid-URL, even when the query preceding it is enormous. + t.Run("long query keeps the link intact", func(t *testing.T) { + longQuery := strings.Repeat("Heartbeat | where Computer == 'x' | summarize count() ", 200) + longLink := "https://portal.azure.com#@" + strings.Repeat("q", 1000) + + a, _, _, err := buildAlert([]byte(logPayload(longQuery, longLink))) + require.NoError(t, err) + + assert.LessOrEqual(t, len([]rune(a.Details)), alert.MaxDetailsLength) + assert.Contains(t, a.Details, longLink, "the full link must survive truncation") + // The query is capped so it cannot crowd the link out. + assert.NotContains(t, a.Details, strings.Repeat("Heartbeat | where Computer == 'x' | summarize count() ", 20)) + }) +} + +func serviceHealthPayload() string { + return fmt.Sprintf(`{ + "schemaId": "azureMonitorCommonAlertSchema", + "data": { + "essentials": { + "alertId": %q, + "alertRule": "test-ServiceHealthAlertRule", + "severity": "Sev4", + "signalType": "Activity Log", + "monitorCondition": "Fired", + "monitoringService": "ServiceHealth", + "alertTargetIDs": ["/subscriptions/sub-1"] + }, + "alertContext": { + "authorization": null, "channels": 1, "claims": null, "caller": null, + "eventSource": 2, "level": 3, + "operationName": "Microsoft.ServiceHealth/incident/action", + "properties": { + "title": "Test Action Group - Test Service Health Alert", + "service": "Azure Service Name", + "region": "Global", + "communication": "

This is a test from Service Health Alert

", + "incidentType": "Incident", + "trackingId": "TEST-TTT", + "impactStartTime": "2026-07-31T13:00:00.000Z", + "impactedServices": "[{\"ImpactedRegions\":[{\"RegionName\":\"Global\"}],\"ServiceName\":\"Azure Service Name\"}]", + "stage": "Resolved", + "isHIR": "false" + }, + "status": "Resolved", "subStatus": null, "ResourceType": null + } + } +}`, testAlertID) +} + +// Service Health carries no conditionType at all, which is why conditionType +// dispatch must tolerate its absence. +func TestBuildAlert_ServiceHealth(t *testing.T) { + a, meta, _, err := buildAlert([]byte(serviceHealthPayload())) + require.NoError(t, err) + + assert.Equal(t, "test-ServiceHealthAlertRule", a.Summary) + assert.Contains(t, a.Details, "Title: Test Action Group - Test Service Health Alert") + assert.Contains(t, a.Details, "Tracking ID: TEST-TTT") + assert.Contains(t, a.Details, "Service: Azure Service Name") + assert.Contains(t, a.Details, "Stage: Resolved") + + // HTML is unreadable on a pager; tags are stripped. + assert.NotContains(t, a.Details, "

") + assert.NotContains(t, a.Details, "

") + + // monitorCondition wins over alertContext.status: the incident resolved while + // the alert fired. Service Health is where this trap actually bites. + assert.Equal(t, alert.StatusTriggered, a.Status) + assert.Equal(t, "Fired", meta["monitor_condition"]) +} + +func TestBuildAlert_SchemaGate(t *testing.T) { + tests := []struct{ name, schemaID string }{ + {name: "missing", schemaID: ""}, + {name: "legacy metric", schemaID: "AzureMonitorMetricAlert"}, + {name: "unrecognised", schemaID: "somethingElse"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + body := fmt.Sprintf(`{"schemaId": %q, "data": {"essentials": {"alertRule": "x"}}}`, tt.schemaID) + _, _, _, err := buildAlert([]byte(body)) + + // Rejected with an actionable message, not silently degraded to the + // best-effort path. + require.ErrorIs(t, err, errLegacySchema) + assert.Contains(t, err.Error(), "common alert schema") + }) + } +} + +// Multi-resource and resource-group-scoped metric rules emit +// MultipleResourceMultipleMetricCriteria, not SingleResource... . Ten of this +// tenant's metric rules use it, and before the envelope-based dispatch they +// rendered essentials only -- silently losing every metric field. +func TestBuildAlert_MultipleResourceMetric(t *testing.T) { + body := strings.Replace(metricPayload("Fired"), + `"conditionType": "SingleResourceMultipleMetricCriteria"`, + `"conditionType": "MultipleResourceMultipleMetricCriteria"`, 1) + + a, _, _, err := buildAlert([]byte(body)) + require.NoError(t, err) + + assert.Contains(t, a.Details, "Transactions GreaterThan 0 (Total, PT5M) = 100") + assert.Contains(t, a.Details, "Namespace: Microsoft.Storage/storageAccounts") + assert.Contains(t, a.Details, "Dimensions: ApiName=GetBlob") +} + +func webtestPayload() string { + return `{ + "schemaId": "azureMonitorCommonAlertSchema", + "data": { + "essentials": { + "alertId": "wt-1", "alertRule": "availability-check", + "monitorCondition": "Fired", "signalType": "Metric", "monitoringService": "Platform" + }, + "alertContext": { + "conditionType": "WebtestLocationAvailabilityCriteria", + "condition": {"windowSize": "PT5M", "allOf": [{ + "metricName": "Failed Location", "metricNamespace": null, + "operator": "GreaterThan", "threshold": "2", "timeAggregation": "Sum", + "dimensions": [], "metricValue": 5, + "webTestName": "myAvailabilityTest-myApplication" + }]} + } + } +}` +} + +func TestBuildAlert_WebtestAvailability(t *testing.T) { + a, _, _, err := buildAlert([]byte(webtestPayload())) + require.NoError(t, err) + + assert.Contains(t, a.Details, "Failed Location GreaterThan 2 (Sum, PT5M) = 5") + assert.Contains(t, a.Details, "Web test: myAvailabilityTest-myApplication") +} + +// Azure documents threshold and dimension values as strings but is not +// consistent across shapes. A hard unmarshal failure would 400, which Azure does +// not retry, so one oddly-typed field must not cost the whole page. +func TestBuildAlert_ToleratesTypeMismatch(t *testing.T) { + t.Run("numeric threshold", func(t *testing.T) { + body := strings.Replace(metricPayload("Fired"), `"threshold": "0"`, `"threshold": 25`, 1) + a, _, _, err := buildAlert([]byte(body)) + require.NoError(t, err) + + // The mistyped value is lost, everything else survives. + assert.Contains(t, a.Details, "Transactions GreaterThan") + assert.Contains(t, a.Details, "Resource: stage-www-frontdoor") + require.NotNil(t, a.Dedup) + assert.Equal(t, sha256Hex(testAlertID), a.Dedup.Payload) + }) + + t.Run("numeric dimension value", func(t *testing.T) { + body := strings.Replace(metricPayload("Fired"), + `"dimensions": [{"name": "ApiName", "value": "GetBlob"}]`, + `"dimensions": [{"name": "code", "value": 500}]`, 1) + a, _, _, err := buildAlert([]byte(body)) + require.NoError(t, err) + + assert.NotEmpty(t, a.Summary) + require.NotNil(t, a.Dedup) + }) +} + +// essentials fields that apply to every signal type, so they also improve the +// fallback path where they are the only content available. +func TestBuildAlert_EssentialsExtras(t *testing.T) { + body := strings.Replace(metricPayload("Fired"), + `"description": "Runbook: `+testRunbook+`"`, + `"description": "Runbook: `+testRunbook+`", + "alertRuleId": "/subscriptions/sub-1/resourceGroups/rg/providers/microsoft.insights/metricAlerts/rule", + "targetResourceGroup": "stage-rg", + "targetResourceType": "Microsoft.Storage/storageAccounts", + "investigationLink": "https://portal.azure.com/investigate/abc"`, 1) + + a, meta, _, err := buildAlert([]byte(body)) + require.NoError(t, err) + + assert.Contains(t, a.Details, "Resource group: stage-rg") + assert.Contains(t, a.Details, "Resource type: Microsoft.Storage/storageAccounts") + assert.Contains(t, a.Details, "Investigate: https://portal.azure.com/investigate/abc") + + // The rule's ARM ID is machine-facing, so it belongs in metadata rather than + // cluttering the pager text with a full resource path. + assert.Equal(t, "stage-rg", meta["target_resource_group"]) + assert.Equal(t, "Microsoft.Storage/storageAccounts", meta["target_resource_type"]) + assert.Contains(t, meta["alert_rule_id"], "metricAlerts/rule") +} + +func TestPortalURL(t *testing.T) { + tests := []struct{ name, resourceID, want string }{ + { + name: "valid arm resource id", + resourceID: testAlertID, + want: testPortalURL, + }, + {name: "empty"}, + {name: "not a resource id", resourceID: "not-a-resource-id"}, + {name: "relative path missing leading slash", resourceID: "subscriptions/sub-1/providers/x"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.want, portalURL(tt.resourceID)) + }) + } + + // The ID is one path segment, so its slashes must not survive as separators. + t.Run("percent-encodes the alert id", func(t *testing.T) { + got := portalURL(testAlertID) + assert.Contains(t, got, "%2Fsubscriptions%2Fsub-1%2F") + assert.NotContains(t, strings.TrimPrefix(got, "https://portal.azure.com/#blade/Microsoft_Azure_Monitoring/AlertDetailsTemplateBlade/alertId/"), "/") + }) +} + +// investigationLink opens Azure's AI investigation agent, not the alert, so it +// is additive: the portal link to the alert itself must always be present. +func TestBuildAlert_PortalLink(t *testing.T) { + t.Run("portal link is present when investigationLink is absent", func(t *testing.T) { + a, _, _, err := buildAlert([]byte(metricPayload("Fired"))) + require.NoError(t, err) + + assert.Contains(t, a.Details, "Portal: "+testPortalURL) + assert.NotContains(t, a.Details, "Investigate:") + }) + + t.Run("investigationLink supplements the portal link, never replaces it", func(t *testing.T) { + body := strings.Replace(metricPayload("Fired"), + `"description": "Runbook: `+testRunbook+`"`, + `"description": "Runbook: `+testRunbook+`", "investigationLink": "https://portal.azure.com/investigate/abc"`, 1) + + a, _, _, err := buildAlert([]byte(body)) + require.NoError(t, err) + + assert.Contains(t, a.Details, "Portal: "+testPortalURL) + assert.Contains(t, a.Details, "Investigate: https://portal.azure.com/investigate/abc") + }) +} + +// Azure Managed Prometheus rule groups route to PagerDuty in this tenant (three +// "Azure Pod Health Degraded" groups). The shape has no conditionType and no +// condition.allOf, so before the dedicated branch it rendered essentials only. +func prometheusPayload() string { + return `{ + "schemaId": "azureMonitorCommonAlertSchema", + "data": { + "essentials": { + "alertId": "/subscriptions/sub-1/providers/Microsoft.AlertsManagement/alerts/prom-1", + "alertRule": "Azure Pod Health Degraded (microservices-prod)", + "severity": "Sev2", "signalType": "Metric", + "monitorCondition": "Fired", "monitoringService": "Prometheus", + "configurationItems": ["aks-prod"] + }, + "alertContext": { + "interval": "PT1M", + "expression": "kube_pod_status_ready{condition=\"false\"} > 0", + "expressionValue": "3", + "for": "PT5M", + "labels": {"cluster": "microservices-prod", "namespace": "default", "severity": "warning"}, + "annotations": { + "summary": "Pods are not ready in microservices-prod", + "description": "3 pods have been NotReady for more than 5 minutes", + "runbook_url": "https://runbook.example.com/pods" + }, + "ruleGroup": "/subscriptions/sub-1/resourceGroups/rg/providers/Microsoft.AlertsManagement/prometheusRuleGroups/pod-health" + } + } +}` +} + +func TestBuildAlert_Prometheus(t *testing.T) { + a, meta, _, err := buildAlert([]byte(prometheusPayload())) + require.NoError(t, err) + + assert.Equal(t, "Azure Pod Health Degraded (microservices-prod)", a.Summary) + assert.Equal(t, alert.StatusTriggered, a.Status) + require.NotNil(t, a.Dedup) + + // Annotations lead, as prose, because they are author-written. + assert.Contains(t, a.Details, "Pods are not ready in microservices-prod") + assert.Contains(t, a.Details, "3 pods have been NotReady for more than 5 minutes") + assert.Contains(t, a.Details, "runbook_url: https://runbook.example.com/pods") + + assert.Contains(t, a.Details, `Expression: kube_pod_status_ready{condition="false"} > 0`) + assert.Contains(t, a.Details, "Value: 3") + assert.Contains(t, a.Details, "For: PT5M") + assert.Contains(t, a.Details, "Interval: PT1M") + // Labels are Prometheus's dimensions, rendered in a stable order. + assert.Contains(t, a.Details, "Labels: cluster=microservices-prod, namespace=default, severity=warning") + + assert.Equal(t, "Prometheus", meta["monitoring_service"]) + assert.Contains(t, meta["rule_group"], "prometheusRuleGroups/pod-health") + + // Must not be mistaken for a metric or log alert. + assert.NotContains(t, a.Details, "Query:") + assert.NotContains(t, a.Details, "Namespace:") +} + +// Microsoft's documented Prometheus sample, verbatim, with no annotations set. +func TestBuildAlert_PrometheusDocSample(t *testing.T) { + body := `{ + "schemaId": "azureMonitorCommonAlertSchema", + "data": { + "essentials": {"alertId": "p1", "alertRule": "sql-availability", + "monitorCondition": "Fired", "signalType": "Metric", "monitoringService": "Prometheus"}, + "alertContext": { + "interval": "PT1M", "expression": "sql_up > 0", "expressionValue": "0", "for": "PT2M", + "labels": {"Environment": "Prod", "cluster": "myCluster1"}, + "annotations": {"summary": "alert on SQL availability"}, + "ruleGroup": "/subscriptions/s/resourceGroups/rg/providers/Microsoft.AlertsManagement/prometheusRuleGroups/g" + } + } +}` + a, _, _, err := buildAlert([]byte(body)) + require.NoError(t, err) + + assert.Contains(t, a.Details, "alert on SQL availability") + assert.Contains(t, a.Details, "Expression: sql_up > 0") + assert.Contains(t, a.Details, "Value: 0") + assert.Contains(t, a.Details, "Labels: Environment=Prod, cluster=myCluster1") +} + +// parseInfo is what makes an unrecognised payload visible in the logs. Without +// ContextRendered the only symptom of a newly-routed Azure alert type is a thin +// alert nobody notices. +func TestBuildAlert_ParseInfo(t *testing.T) { + tests := []struct { + name string + body string + wantService string + wantConditionType string + wantRendered bool + }{ + { + name: "static metric", body: metricPayload("Fired"), + wantService: "Platform", wantConditionType: "SingleResourceMultipleMetricCriteria", + wantRendered: true, + }, + { + name: "log alerts v2", body: logPayload("Heartbeat", "https://portal.azure.com#@x"), + wantConditionType: "LogQueryCriteria", wantRendered: true, + }, + { + // No conditionType at all, but still recognised via its own fields. + name: "prometheus", body: prometheusPayload(), + wantService: "Prometheus", wantConditionType: "", wantRendered: true, + }, + { + name: "service health", body: serviceHealthPayload(), + wantService: "ServiceHealth", wantConditionType: "", wantRendered: true, + }, + { + // The case the log line exists for. + name: "unrecognised shape", + body: `{"schemaId":"azureMonitorCommonAlertSchema","data":{ + "essentials":{"alertId":"b1","alertRule":"BackupJobFailed", + "signalType":"Log","monitoringService":"Azure Backup","monitorCondition":"Fired"}, + "alertContext":{"BackupItemName":"vm-1","JobFailureCode":"UserErrorX"}}}`, + wantService: "Azure Backup", wantConditionType: "", wantRendered: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + _, _, info, err := buildAlert([]byte(tt.body)) + require.NoError(t, err) + + assert.Equal(t, tt.wantRendered, info.ContextRendered, "ContextRendered") + assert.Equal(t, tt.wantConditionType, info.ConditionType, "ConditionType") + if tt.wantService != "" { + assert.Equal(t, tt.wantService, info.MonitorService, "MonitorService") + } + }) + } +} + +func TestBuildAlert_Fallback(t *testing.T) { + // An unknown conditionType that still carries condition.allOf is rendered, not + // dropped -- dispatch is on the envelope, not on a name allowlist. Only a + // payload with no condition at all falls back to essentials. + t.Run("unknown conditionType with a condition still renders", func(t *testing.T) { + body := strings.Replace(metricPayload("Fired"), + `"conditionType": "SingleResourceMultipleMetricCriteria"`, + `"conditionType": "SomeFutureAzureCriteria"`, 1) + a, _, _, err := buildAlert([]byte(body)) + require.NoError(t, err) + + assert.Equal(t, "Too Many Frontdoor Exceptions", a.Summary) + assert.Contains(t, a.Details, "Transactions GreaterThan 0 (Total, PT5M) = 100") + require.NotNil(t, a.Dedup) + }) + + // signalType Log with a non-log monitoringService must not go through the KQL + // parser -- this is why conditionType, not signalType, is the discriminator. + t.Run("azure backup under signalType Log", func(t *testing.T) { + body := `{ + "schemaId": "azureMonitorCommonAlertSchema", + "data": { + "essentials": { + "alertId": "/subscriptions/sub-1/providers/Microsoft.AlertsManagement/alerts/backup-1", + "alertRule": "BackupJobFailed", + "signalType": "Log", + "monitorCondition": "Fired", + "monitoringService": "Azure Backup" + }, + "alertContext": {"BackupItemName": "vm-1", "JobFailureCode": "UserErrorX"} + } +}` + a, meta, _, err := buildAlert([]byte(body)) + require.NoError(t, err) + + assert.Equal(t, "BackupJobFailed", a.Summary) + assert.NotContains(t, a.Details, "Query:") + assert.Equal(t, "Azure Backup", meta["monitoring_service"]) + }) + + t.Run("no alertContext at all", func(t *testing.T) { + body := `{ + "schemaId": "azureMonitorCommonAlertSchema", + "data": {"essentials": {"alertId": "a", "alertRule": "Bare", "monitorCondition": "Fired"}} +}` + a, _, _, err := buildAlert([]byte(body)) + require.NoError(t, err) + assert.Equal(t, "Bare", a.Summary) + }) + + // Neither a condition nor a Prometheus expression: nothing identifies the + // shape, so essentials alone must still produce a usable alert. + t.Run("no condition and no expression", func(t *testing.T) { + body := `{ + "schemaId": "azureMonitorCommonAlertSchema", + "data": { + "essentials": {"alertId": "p1", "alertRule": "PromRule", "signalType": "Metric", + "monitoringService": "Prometheus", "monitorCondition": "Fired"}, + "alertContext": {"labels": {"severity": "warning"}, "annotations": {}} + } +}` + a, _, _, err := buildAlert([]byte(body)) + require.NoError(t, err) + assert.Equal(t, "PromRule", a.Summary) + require.NotNil(t, a.Dedup) + }) + + t.Run("malformed json errors", func(t *testing.T) { + _, _, _, err := buildAlert([]byte(`{not json`)) + require.Error(t, err) + assert.NotErrorIs(t, err, errLegacySchema) + }) +} + +// Invariants that must hold for every accepted payload, on every branch. +func TestBuildAlert_Invariants(t *testing.T) { + cases := map[string]string{ + "metric fired": metricPayload("Fired"), + "metric resolved": metricPayload("Resolved"), + "dynamic": dynamicPayload(), + "log": logPayload("Heartbeat", "https://portal.azure.com#@x"), + "log long": logPayload(strings.Repeat("q ", 5000), "https://portal.azure.com#@"+strings.Repeat("z", 1000)), + "service health": serviceHealthPayload(), + "prometheus": prometheusPayload(), + "webtest": webtestPayload(), + "multi resource": strings.Replace(metricPayload("Fired"), + `"conditionType": "SingleResourceMultipleMetricCriteria"`, + `"conditionType": "MultipleResourceMultipleMetricCriteria"`, 1), + "numeric threshold": strings.Replace(metricPayload("Fired"), `"threshold": "0"`, `"threshold": 25`, 1), + "empty essentials": `{"schemaId":"azureMonitorCommonAlertSchema","data":{"essentials":{}}}`, + "empty data": `{"schemaId":"azureMonitorCommonAlertSchema","data":{}}`, + "only schema": `{"schemaId":"azureMonitorCommonAlertSchema"}`, + "null alertContext": `{"schemaId":"azureMonitorCommonAlertSchema","data":{"essentials":{"alertRule":"x"},"alertContext":null}}`, + } + + for name, body := range cases { + t.Run(name, func(t *testing.T) { + a, meta, _, err := buildAlert([]byte(body)) + require.NoError(t, err) + + // Empty summary does not error -- it creates a blank, unactionable + // alert -- so the fallback chain is the only guard. + assert.NotEmpty(t, a.Summary, "summary must never be empty") + assert.LessOrEqual(t, len([]rune(a.Summary)), alert.MaxSummaryLength) + assert.LessOrEqual(t, len([]rune(a.Details)), alert.MaxDetailsLength) + + // A nil dedup would fall back to a content hash that differs between + // the Fired and Resolved deliveries, breaking the close path. + require.NotNil(t, a.Dedup, "dedup must never be nil") + assert.Equal(t, alert.DedupTypeUser, a.Dedup.Type) + assert.Len(t, a.Dedup.Payload, 64) + assert.Same(t, a.Dedup, a.DedupKey()) + + assert.Equal(t, alert.SourceAzureMonitor, a.Source) + + // Proves the mapper can never produce a client error, and catches a + // missing SourceAzureMonitor entry in alert.Normalize's OneOf. + a.ServiceID = testServiceID + _, err = a.Normalize() + assert.NoError(t, err) + + total := 0 + for k, v := range meta { + assert.NotEmpty(t, v, "meta[%s] should have been dropped", k) + assert.LessOrEqual(t, len([]rune(v)), maxMetaValueLen) + total += len(k) + len(v) + } + assert.Less(t, total, 32*1024) + }) + } +} + +// Distinct alertIds must produce distinct dedup keys, or unrelated alerts would +// collapse onto one. +func TestBuildAlert_DistinctAlertIDs(t *testing.T) { + first, _, _, err := buildAlert([]byte(metricPayload("Fired"))) + require.NoError(t, err) + + other := strings.Replace(metricPayload("Fired"), "1db044ff-df8f-4064-a559-b9c9f5f4f000", "3a10e1f4-0000-0000-0000-000000000000", 1) + second, _, _, err := buildAlert([]byte(other)) + require.NoError(t, err) + + require.NotNil(t, first.Dedup) + require.NotNil(t, second.Dedup) + assert.NotEqual(t, first.Dedup.Payload, second.Dedup.Payload) +} + +// TestBuildAlert_MissingAlertIDStillDistinct pins the dedupKey fallback. Hashing a +// bare "" would give every alertId-less payload one shared key, so unrelated +// alerts would collapse onto a single alert and any one Resolved delivery would +// close all of them. +func TestBuildAlert_MissingAlertIDStillDistinct(t *testing.T) { + body := func(rule, ruleID, fired string) string { + return `{ + "schemaId": "azureMonitorCommonAlertSchema", + "data": {"essentials": { + "alertRule": "` + rule + `", + "alertRuleId": "` + ruleID + `", + "firedDateTime": "` + fired + `", + "monitorCondition": "Fired" + }} +}` + } + + dedupOf := func(t *testing.T, payload string) string { + t.Helper() + a, _, _, err := buildAlert([]byte(payload)) + require.NoError(t, err) + require.NotNil(t, a.Dedup, "dedup must never be nil: nil falls back to a content hash") + require.Len(t, a.Dedup.Payload, 64) + return a.Dedup.Payload + } + + var ( + cpuA = dedupOf(t, body("high-cpu", "/subscriptions/s/rules/cpu", "2026-08-05T10:00:00Z")) + memA = dedupOf(t, body("high-mem", "/subscriptions/s/rules/mem", "2026-08-05T10:00:00Z")) + cpuA2 = dedupOf(t, body("high-cpu", "/subscriptions/s/rules/cpu", "2026-08-05T11:30:00Z")) + ) + + assert.NotEqual(t, cpuA, memA, "different rules must not share a dedup key") + assert.NotEqual(t, cpuA, cpuA2, "separate firings of one rule must not share a dedup key") + + // The Resolved delivery of a firing must still match its own Fired delivery, + // or it can never close it. resolvedDateTime is deliberately excluded from the + // key for exactly this reason. + resolved := strings.Replace( + body("high-cpu", "/subscriptions/s/rules/cpu", "2026-08-05T10:00:00Z"), + `"monitorCondition": "Fired"`, + `"monitorCondition": "Resolved", "resolvedDateTime": "2026-08-05T10:45:00Z"`, 1) + assert.Equal(t, cpuA, dedupOf(t, resolved), "Resolved must share the Fired dedup key") +} + +func TestStringProps_StripsHTMLAndSkipsNonStrings(t *testing.T) { + body := `{ + "schemaId": "azureMonitorCommonAlertSchema", + "data": { + "essentials": {"alertRule": "x", "monitorCondition": "Fired"}, + "alertContext": {"properties": { + "title": "Bold title", + "channels": 1, + "nested": {"a": "b"}, + "nothing": null, + "blank": " " + }} + } +}` + a, _, _, err := buildAlert([]byte(body)) + require.NoError(t, err) + + assert.Contains(t, a.Details, "Title: Bold title") + assert.NotContains(t, a.Details, "") + // Non-string values are skipped rather than dumped as raw JSON. + assert.NotContains(t, a.Details, `{"a"`) +} + +// TestBuildAlert_ControlCharOnlyAlertRuleGetsFallback pins the same +// sanitize-before-testing-emptiness bug fixed in cloudwatch's buildAlarm: +// alertRule made only of non-printable control characters is non-blank under +// TrimSpace (which only strips whitespace) but sanitizes to "". Testing the raw +// value would let it through as the summary, only for the caller's sanitize pass +// to reduce it to "" -- an empty Summary passes validate.Text silently, so the +// alert would be created with no useful content instead of falling through to +// the next candidate. +func TestBuildAlert_ControlCharOnlyAlertRuleGetsFallback(t *testing.T) { + body, err := json.Marshal(map[string]any{ + "schemaId": "azureMonitorCommonAlertSchema", + "data": map[string]any{ + "essentials": map[string]any{ + "alertRule": "\x01\x02", + "monitorCondition": "Fired", + "signalType": "Metric", + }, + }, + }) + require.NoError(t, err) + + a, _, _, err := buildAlert(body) + require.NoError(t, err) + + assert.Equal(t, "Azure Monitor Metric alert", a.Summary) + assert.NotEmpty(t, a.Summary, "must never be blank: an empty Summary passes validate.Text silently") +} + +// TestCleanMeta_StaysUnderByteBudget pins the fix for the byte-vs-rune gap: +// maxMetaValueLen bounds each value in RUNES, but alert.ValidateMetadata sums +// BYTES. buildMeta's dozen keys, each near that rune cap with multi-byte +// content, add up to more bytes than the 32KiB total allows -- this test uses +// enough keys to actually cross it, so a regression that drops the byte-budget +// pass fails here regardless of buildMeta's current key count. +func TestCleanMeta_StaysUnderByteBudget(t *testing.T) { + wide := strings.Repeat("😀", maxMetaValueLen) // 1024 runes, 4 bytes each + + m := make(map[string]string, 20) + for i := 0; i < 20; i++ { + m[fmt.Sprintf("key_%d", i)] = wide + } + + out := cleanMeta(m) + + total := 0 + for k, v := range out { + total += len(k) + len(v) + require.True(t, utf8.ValidString(v), "truncation must not split a rune") + } + assert.Less(t, total, 32*1024) +} diff --git a/cloudwatch/allowlist.go b/cloudwatch/allowlist.go new file mode 100644 index 0000000000..2f4eac924d --- /dev/null +++ b/cloudwatch/allowlist.go @@ -0,0 +1,77 @@ +package cloudwatch + +import ( + "errors" + "fmt" + "net/url" + "regexp" + "strings" +) + +// snsHostRe matches an AWS SNS API hostname. The anchors are load-bearing: +// without them, `sns.us-west-2.amazonaws.com.evil.com` is accepted. +// +// The `[a-z0-9-]+` region segment deliberately forbids dots, so GovCloud +// (sns.us-gov-west-1.amazonaws.com) matches while ISO regions (c2s.ic.gov, +// sc2s.sgov.gov) do not. +var snsHostRe = regexp.MustCompile(`^sns\.[a-z0-9-]+\.amazonaws\.com(\.cn)?$`) + +// certPathRe matches a single *.pem path segment. Restricting the path bounds +// how many distinct cache keys an attacker-supplied URL can mint. +var certPathRe = regexp.MustCompile(`^/[A-Za-z0-9._-]+\.pem$`) + +var errNotAllowed = errors.New("cloudwatch: URL host is not an AWS SNS endpoint") + +// checkFetchURL validates raw as an AWS SNS endpoint and returns the parsed URL. +// +// Callers MUST build their request from the returned *url.URL and never re-parse +// raw: validating one string while dialing another is the classic bypass. +func checkFetchURL(raw string) (*url.URL, error) { + u, err := url.Parse(raw) + if err != nil { + return nil, fmt.Errorf("%w: %v", errNotAllowed, err) + } + if !strings.EqualFold(u.Scheme, "https") { + return nil, fmt.Errorf("%w: scheme %q", errNotAllowed, u.Scheme) + } + // Reject userinfo: `https://sns.us-west-2.amazonaws.com@evil.com/` has a host + // of evil.com, and we never want to leak credentials outbound either. + if u.User != nil { + return nil, fmt.Errorf("%w: URL contains userinfo", errNotAllowed) + } + // AWS never sends an explicit port. Rejecting it keeps the host check to a + // single auditable line and avoids hand-rolled port stripping. + if u.Port() != "" { + return nil, fmt.Errorf("%w: explicit port not allowed", errNotAllowed) + } + + // Hostname() strips the port and IPv6 brackets; u.Host does not. Lowercase + // before matching -- DNS is case-insensitive, so this is correctness. + host := strings.ToLower(u.Hostname()) + if !snsHostRe.MatchString(host) { + return nil, fmt.Errorf("%w: host %q", errNotAllowed, host) + } + + return u, nil +} + +// checkCertURL validates a SigningCertURL. Beyond checkFetchURL it requires a +// bare `/.pem` path with no query or fragment. +func checkCertURL(raw string) (*url.URL, error) { + u, err := checkFetchURL(raw) + if err != nil { + return nil, err + } + if u.RawQuery != "" || u.Fragment != "" { + return nil, fmt.Errorf("%w: cert URL must not have a query or fragment", errNotAllowed) + } + if !certPathRe.MatchString(u.Path) { + return nil, fmt.Errorf("%w: cert path %q is not a single .pem segment", errNotAllowed, u.Path) + } + + return u, nil +} + +// checkSubscribeURL validates a SubscribeURL. Unlike a cert URL it legitimately +// carries ?Action=ConfirmSubscription&TopicArn=...&Token=... +func checkSubscribeURL(raw string) (*url.URL, error) { return checkFetchURL(raw) } diff --git a/cloudwatch/allowlist_test.go b/cloudwatch/allowlist_test.go new file mode 100644 index 0000000000..a1acebbc2f --- /dev/null +++ b/cloudwatch/allowlist_test.go @@ -0,0 +1,81 @@ +package cloudwatch + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestCheckFetchURL(t *testing.T) { + tests := []struct { + name string + url string + ok bool + }{ + {name: "valid us-west-2", url: "https://sns.us-west-2.amazonaws.com/x.pem", ok: true}, + {name: "valid china partition", url: "https://sns.cn-north-1.amazonaws.com.cn/x.pem", ok: true}, + {name: "valid govcloud", url: "https://sns.us-gov-west-1.amazonaws.com/x.pem", ok: true}, + {name: "uppercase host is same DNS name", url: "https://SNS.US-WEST-2.AMAZONAWS.COM/x.pem", ok: true}, + + // The case an unanchored regex would accept. + {name: "suffix attack", url: "https://sns.us-west-2.amazonaws.com.evil.com/x.pem"}, + {name: "http scheme", url: "http://sns.us-west-2.amazonaws.com/x.pem"}, + {name: "prefix attack", url: "https://notsns.us-west-2.amazonaws.com/x.pem"}, + {name: "explicit port", url: "https://sns.us-west-2.amazonaws.com:8443/x.pem"}, + {name: "userinfo host is evil.com", url: "https://sns.us-west-2.amazonaws.com@evil.com/x.pem"}, + {name: "at sign in path", url: "https://evil.com/@sns.us-west-2.amazonaws.com/x.pem"}, + {name: "empty region segment", url: "https://sns..amazonaws.com/x.pem"}, + {name: "trailing dot", url: "https://sns.us-west-2.amazonaws.com./x.pem"}, + {name: "metadata endpoint", url: "https://169.254.169.254/latest/meta-data/"}, + {name: "percent encoded host", url: "https://%73ns.us-west-2.amazonaws.com/x.pem"}, + {name: "scheme relative", url: "//sns.us-west-2.amazonaws.com/x.pem"}, + {name: "empty", url: ""}, + {name: "dotted region", url: "https://sns.a.b.amazonaws.com/x.pem"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + u, err := checkFetchURL(tt.url) + if !tt.ok { + assert.Error(t, err, "expected %q to be refused", tt.url) + return + } + require.NoError(t, err) + require.NotNil(t, u) + }) + } +} + +func TestCheckCertURL(t *testing.T) { + tests := []struct { + name string + url string + ok bool + }{ + {name: "pem path", url: "https://sns.us-west-2.amazonaws.com/SimpleNotificationService-abc123.pem", ok: true}, + {name: "query string", url: "https://sns.us-west-2.amazonaws.com/x.pem?a=1"}, + {name: "fragment", url: "https://sns.us-west-2.amazonaws.com/x.pem#a"}, + {name: "nested path", url: "https://sns.us-west-2.amazonaws.com/a/b.pem"}, + {name: "not a pem", url: "https://sns.us-west-2.amazonaws.com/x.txt"}, + {name: "no extension", url: "https://sns.us-west-2.amazonaws.com/x"}, + {name: "bad host still refused", url: "https://sns.us-west-2.amazonaws.com.evil.com/x.pem"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + _, err := checkCertURL(tt.url) + if tt.ok { + assert.NoError(t, err) + return + } + assert.Error(t, err) + }) + } +} + +func TestCheckSubscribeURL_AllowsQuery(t *testing.T) { + // SubscribeURL legitimately carries the confirmation action and token. + _, err := checkSubscribeURL("https://sns.us-west-2.amazonaws.com/?Action=ConfirmSubscription&TopicArn=arn&Token=abc") + assert.NoError(t, err) +} diff --git a/cloudwatch/certcache.go b/cloudwatch/certcache.go new file mode 100644 index 0000000000..a2d88cc19f --- /dev/null +++ b/cloudwatch/certcache.go @@ -0,0 +1,219 @@ +package cloudwatch + +import ( + "context" + "crypto/rsa" + "fmt" + "io" + "net/http" + "net/url" + "sync" + "time" + + "golang.org/x/sync/singleflight" +) + +const ( + // maxCachedCerts bounds the cache. The cert URL path is attacker-supplied, + // so without a bound `https://sns..amazonaws.com/.pem` mints + // unlimited entries. SNS's real working set is one or two certs. + maxCachedCerts = 32 + + // maxCachedCertFailures bounds the negative cache, for the same reason as + // maxCachedCerts: its keys are attacker-supplied. Larger than the positive + // bound because failures are the higher-cardinality case by nature. + maxCachedCertFailures = 128 + + // certFailureTTL is how long a URL that did not yield a usable key is refused + // without a refetch. Deliberately short: a legitimate cert URL that fails for + // an unrelated reason must not stay poisoned, and the caller answers 503 so the + // delivery is retried well after this expires. + certFailureTTL = 30 * time.Second + + // maxCertBytes bounds the response read. The inbound request body limit does + // not apply to responses we fetch. + maxCertBytes = 16 * 1024 + + certFetchTimeout = 5 * time.Second + + // maxConcurrentCertFetches bounds how many outbound cert fetches may be in + // flight at once, across ALL urls -- see the fetchSem field doc. + maxConcurrentCertFetches = 8 +) + +// certCache maps a validated signing-cert URL to its RSA public key. +// +// Caching by URL is safe because AWS mints a new URL when it rotates a cert, so +// a stale entry becomes unreachable rather than wrong. If AWS ever reused a URL +// with new key material, verification would fail until restart. +// +// Eviction is FIFO rather than LRU: with a working set of one or two, insertion +// order is sufficient and far simpler. Only a successfully parsed key is ever +// inserted, so a URL that never yields one cannot evict a working entry. +// +// Failures are tracked separately and briefly, so a REPEATED bad URL is not +// refetched on every delivery. That alone does not bound a stream of DISTINCT +// forged `.pem` URLs -- each one is a first-time miss on both the positive and +// negative cache, and still costs a real outbound fetch. What actually bounds +// that is fetchSem: a semaphore capping how many fetches, across ALL urls, may +// be in flight at once, so a flood of distinct urls queues for a fetch slot +// rather than spawning an unbounded goroutine (each holding a connection open +// for up to certFetchTimeout) per request. fetchOnce additionally collapses +// concurrent requests for the SAME url into a single fetch -- the common +// legitimate case right after AWS rotates a cert, when many deliveries can +// arrive before the first fetch populates the cache. +type certCache struct { + mx sync.Mutex + keys map[string]*rsa.PublicKey + order []string + + failed map[string]time.Time + failedOrder []string + + // now is a field so the TTL is testable without sleeping. + now func() time.Time + + fetchSem chan struct{} + fetchOnce singleflight.Group +} + +func newCertCache() *certCache { + return &certCache{ + keys: make(map[string]*rsa.PublicKey, maxCachedCerts), + failed: make(map[string]time.Time, maxCachedCertFailures), + now: time.Now, + fetchSem: make(chan struct{}, maxConcurrentCertFetches), + } +} + +func (c *certCache) get(key string) (*rsa.PublicKey, bool) { + c.mx.Lock() + defer c.mx.Unlock() + pub, ok := c.keys[key] + return pub, ok +} + +func (c *certCache) put(key string, pub *rsa.PublicKey) { + c.mx.Lock() + defer c.mx.Unlock() + + if _, ok := c.keys[key]; ok { + return + } + for len(c.order) >= maxCachedCerts { + delete(c.keys, c.order[0]) + c.order = c.order[1:] + } + c.keys[key] = pub + c.order = append(c.order, key) +} + +// recentlyFailed reports whether key failed within certFailureTTL. +func (c *certCache) recentlyFailed(key string) bool { + c.mx.Lock() + defer c.mx.Unlock() + + at, ok := c.failed[key] + return ok && c.now().Sub(at) < certFailureTTL +} + +// putFailure records that key did not yield a usable key. +func (c *certCache) putFailure(key string) { + c.mx.Lock() + defer c.mx.Unlock() + + if _, ok := c.failed[key]; !ok { + for len(c.failedOrder) >= maxCachedCertFailures { + delete(c.failed, c.failedOrder[0]) + c.failedOrder = c.failedOrder[1:] + } + c.failedOrder = append(c.failedOrder, key) + } + c.failed[key] = c.now() +} + +// publicKey returns the RSA public key for the signing cert at rawURL, fetching +// and caching it if needed. rawURL is validated against the host allowlist +// before any request is made; dial maps the validated URL to the origin to +// actually request (identity in production). +func (c *certCache) publicKey(ctx context.Context, hc *http.Client, dial func(*url.URL) *url.URL, rawURL string) (*rsa.PublicKey, error) { + u, err := checkCertURL(rawURL) + if err != nil { + return nil, err + } + key := u.String() + + // Note the lock is not held across the fetch below: holding it would + // serialize every request behind a single AWS call. + if pub, ok := c.get(key); ok { + return pub, nil + } + if c.recentlyFailed(key) { + return nil, fmt.Errorf("cloudwatch: signing certificate %q failed recently", key) + } + + // fetchOnce.Do collapses concurrent callers with the SAME key onto one fetch; + // the semaphore acquired inside bounds how many fetches for DISTINCT keys run + // at once. Together they are what the certCache doc comment describes -- see + // there for why the negative cache above is not enough on its own. + v, err, _ := c.fetchOnce.Do(key, func() (any, error) { + select { + case c.fetchSem <- struct{}{}: + defer func() { <-c.fetchSem }() + case <-ctx.Done(): + return nil, ctx.Err() + } + + return c.fetchAndParse(ctx, hc, dial, u, key) + }) + if err != nil { + return nil, err + } + + return v.(*rsa.PublicKey), nil +} + +// fetchAndParse does the actual outbound request and parse for publicKey, run +// under fetchOnce so concurrent callers for the same key share one call. +func (c *certCache) fetchAndParse(ctx context.Context, hc *http.Client, dial func(*url.URL) *url.URL, u *url.URL, key string) (*rsa.PublicKey, error) { + ctx, cancel := context.WithTimeout(ctx, certFetchTimeout) + defer cancel() + + req, err := http.NewRequestWithContext(ctx, http.MethodGet, dial(u).String(), nil) + if err != nil { + return nil, fmt.Errorf("cloudwatch: build cert request: %w", err) + } + + resp, err := hc.Do(req) + if err != nil { + return nil, fmt.Errorf("cloudwatch: fetch signing certificate: %w", err) + } + defer resp.Body.Close() + + // A blocked redirect also lands here, since the client is configured to + // surface 3xx rather than follow it. + if resp.StatusCode != http.StatusOK { + // Negative-cached: the status is a property of this URL, so refetching it + // changes nothing until the TTL lapses. Transport errors above deliberately + // are not cached -- those are about connectivity, and caching them would + // poison a legitimate cert URL through a transient blip. + c.putFailure(key) + return nil, fmt.Errorf("cloudwatch: fetch signing certificate: %s", resp.Status) + } + + body, err := io.ReadAll(io.LimitReader(resp.Body, maxCertBytes)) + if err != nil { + return nil, fmt.Errorf("cloudwatch: read signing certificate: %w", err) + } + + // Only a successfully parsed key is cached, so a garbage body never becomes a + // usable entry -- and, since it also never joins order, never evicts one. + pub, err := parseCertPublicKey(body) + if err != nil { + c.putFailure(key) + return nil, err + } + c.put(key, pub) + + return pub, nil +} diff --git a/cloudwatch/certcache_test.go b/cloudwatch/certcache_test.go new file mode 100644 index 0000000000..1975875c9d --- /dev/null +++ b/cloudwatch/certcache_test.go @@ -0,0 +1,286 @@ +package cloudwatch + +import ( + "context" + "crypto/rand" + "crypto/rsa" + "fmt" + "net/http" + "net/http/httptest" + "net/url" + "strings" + "sync" + "sync/atomic" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// certURL builds an allowlisted signing-cert URL. The path matters: checkCertURL +// requires a single *.pem segment. +func certURL(name string) string { + return "https://sns.us-west-2.amazonaws.com/SimpleNotificationService-" + name + ".pem" +} + +// certServer serves body for every request and counts the hits it receives, so a +// test can assert that a fetch did or did not happen. dial redirects the cache at +// it while leaving the allowlist to run against the real URL. +func certServer(t *testing.T, handler http.HandlerFunc) (dial func(*url.URL) *url.URL, hits *atomic.Int32) { + t.Helper() + + hits = new(atomic.Int32) + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + hits.Add(1) + handler(w, r) + })) + t.Cleanup(srv.Close) + + base, err := url.Parse(srv.URL) + require.NoError(t, err) + + return func(u *url.URL) *url.URL { + out := *u + out.Scheme = base.Scheme + out.Host = base.Host + return &out + }, hits +} + +func TestCertCache_CachesOnSuccess(t *testing.T) { + key, err := rsa.GenerateKey(rand.Reader, 2048) + require.NoError(t, err) + pemData := selfSignedPEM(t, key) + + dial, hits := certServer(t, func(w http.ResponseWriter, r *http.Request) { + _, _ = w.Write(pemData) + }) + + c := newCertCache() + u := certURL("a") + + first, err := c.publicKey(context.Background(), &http.Client{}, dial, u) + require.NoError(t, err) + assert.Equal(t, key.N, first.N) + assert.EqualValues(t, 1, hits.Load()) + + second, err := c.publicKey(context.Background(), &http.Client{}, dial, u) + require.NoError(t, err) + assert.Equal(t, first, second) + assert.EqualValues(t, 1, hits.Load(), "a cached key must not refetch") +} + +// TestCertCache_GarbageBodyCachesNothing pins the invariant that only a parsed +// key is stored, so a URL that returns junk cannot become a usable entry. +func TestCertCache_GarbageBodyCachesNothing(t *testing.T) { + dial, _ := certServer(t, func(w http.ResponseWriter, r *http.Request) { + _, _ = w.Write([]byte("not a pem block")) + }) + + c := newCertCache() + _, err := c.publicKey(context.Background(), &http.Client{}, dial, certURL("junk")) + require.Error(t, err) + + _, ok := c.get(certURL("junk")) + assert.False(t, ok, "an unparseable body must not be cached as a key") +} + +// TestCertCache_NegativeCacheSuppressesRefetch covers the abuse case: a URL that +// cannot yield a key must not cost an outbound fetch on every delivery. +func TestCertCache_NegativeCacheSuppressesRefetch(t *testing.T) { + dial, hits := certServer(t, func(w http.ResponseWriter, r *http.Request) { + http.NotFound(w, r) + }) + + c := newCertCache() + u := certURL("missing") + + _, err := c.publicKey(context.Background(), &http.Client{}, dial, u) + require.Error(t, err) + assert.EqualValues(t, 1, hits.Load()) + + _, err = c.publicKey(context.Background(), &http.Client{}, dial, u) + require.Error(t, err) + assert.EqualValues(t, 1, hits.Load(), "a recently failed URL must not refetch") + + // Past the TTL the URL is retried, so a transient failure cannot poison a + // legitimate cert URL permanently. + c.mx.Lock() + c.failed[u] = c.failed[u].Add(-certFailureTTL - time.Second) + c.mx.Unlock() + + _, err = c.publicKey(context.Background(), &http.Client{}, dial, u) + require.Error(t, err) + assert.EqualValues(t, 2, hits.Load(), "the URL must be retried once the TTL lapses") +} + +// TestCertCache_FailuresDoNotEvictWorkingEntry pins that failure traffic cannot +// push a good cert out of the cache: only successful parses join the FIFO. +func TestCertCache_FailuresDoNotEvictWorkingEntry(t *testing.T) { + key, err := rsa.GenerateKey(rand.Reader, 2048) + require.NoError(t, err) + pemData := selfSignedPEM(t, key) + + good := certURL("good") + dial, _ := certServer(t, func(w http.ResponseWriter, r *http.Request) { + if strings.Contains(r.URL.Path, "good") { + _, _ = w.Write(pemData) + return + } + http.NotFound(w, r) + }) + + c := newCertCache() + _, err = c.publicKey(context.Background(), &http.Client{}, dial, good) + require.NoError(t, err) + + // Far more distinct failing URLs than maxCachedCerts. + for i := 0; i < maxCachedCerts*4; i++ { + _, err := c.publicKey(context.Background(), &http.Client{}, dial, certURL(fmt.Sprintf("bad-%d", i))) + require.Error(t, err) + } + + _, ok := c.get(good) + assert.True(t, ok, "failure traffic must not evict a working entry") + assert.LessOrEqual(t, len(c.failed), maxCachedCertFailures, "the negative cache must stay bounded") +} + +// TestCertCache_EvictsAtBound covers FIFO eviction once maxCachedCerts distinct +// certs have been cached successfully. +func TestCertCache_EvictsAtBound(t *testing.T) { + key, err := rsa.GenerateKey(rand.Reader, 2048) + require.NoError(t, err) + pemData := selfSignedPEM(t, key) + + dial, _ := certServer(t, func(w http.ResponseWriter, r *http.Request) { + _, _ = w.Write(pemData) + }) + + c := newCertCache() + for i := 0; i < maxCachedCerts+5; i++ { + _, err := c.publicKey(context.Background(), &http.Client{}, dial, certURL(fmt.Sprintf("c-%d", i))) + require.NoError(t, err) + } + + assert.Len(t, c.keys, maxCachedCerts, "cache must stay at its bound") + _, ok := c.get(certURL("c-0")) + assert.False(t, ok, "the oldest entry must be evicted first") + _, ok = c.get(certURL(fmt.Sprintf("c-%d", maxCachedCerts+4))) + assert.True(t, ok, "the newest entry must be retained") +} + +// TestCertCache_TruncatesOversizeBody pins maxCertBytes. A body past the limit is +// cut off, so it no longer parses -- an endpoint cannot stream unbounded data +// into this handler. +func TestCertCache_TruncatesOversizeBody(t *testing.T) { + key, err := rsa.GenerateKey(rand.Reader, 2048) + require.NoError(t, err) + pemData := selfSignedPEM(t, key) + require.Less(t, len(pemData), maxCertBytes, "a real cert must fit inside the limit") + + dial, _ := certServer(t, func(w http.ResponseWriter, r *http.Request) { + _, _ = w.Write([]byte(strings.Repeat("A", maxCertBytes))) + _, _ = w.Write(pemData) + }) + + c := newCertCache() + _, err = c.publicKey(context.Background(), &http.Client{}, dial, certURL("huge")) + require.Error(t, err, "a body pushed past maxCertBytes must not parse") +} + +// TestCertCache_RejectsBadURLBeforeFetch pins that the allowlist runs first: a +// disallowed host must cost no outbound request at all. +func TestCertCache_RejectsBadURLBeforeFetch(t *testing.T) { + dial, hits := certServer(t, func(w http.ResponseWriter, r *http.Request) { + _, _ = w.Write([]byte("should never be reached")) + }) + + c := newCertCache() + _, err := c.publicKey(context.Background(), &http.Client{}, dial, + "https://sns.us-west-2.amazonaws.com.evil.com/x.pem") + require.Error(t, err) + assert.EqualValues(t, 0, hits.Load(), "a disallowed host must not be fetched") +} + +// TestCertCache_BoundsConcurrentFetchesAcrossDistinctURLs is the test the +// certCache doc comment promises: the negative cache alone does nothing for a +// flood of DISTINCT urls (each is a first-time miss), so this walks 100 of them +// concurrently and asserts the server never sees more than +// maxConcurrentCertFetches requests in flight at once -- proving fetchSem, not +// the negative cache, is what bounds that case. +func TestCertCache_BoundsConcurrentFetchesAcrossDistinctURLs(t *testing.T) { + key, err := rsa.GenerateKey(rand.Reader, 2048) + require.NoError(t, err) + pemData := selfSignedPEM(t, key) + + var ( + inFlight, maxInFlight atomic.Int32 + hits atomic.Int32 + ) + dial, _ := certServer(t, func(w http.ResponseWriter, r *http.Request) { + hits.Add(1) + n := inFlight.Add(1) + defer inFlight.Add(-1) + for { + old := maxInFlight.Load() + if n <= old || maxInFlight.CompareAndSwap(old, n) { + break + } + } + // Hold the request open briefly so concurrent callers actually overlap; + // without this every request could complete before the next one starts. + time.Sleep(20 * time.Millisecond) + _, _ = w.Write(pemData) + }) + + c := newCertCache() + const nURLs = 100 + + var wg sync.WaitGroup + for i := 0; i < nURLs; i++ { + wg.Add(1) + go func(i int) { + defer wg.Done() + _, err := c.publicKey(context.Background(), &http.Client{}, dial, certURL(fmt.Sprintf("distinct-%d", i))) + assert.NoError(t, err) + }(i) + } + wg.Wait() + + assert.EqualValues(t, nURLs, hits.Load(), "every distinct url is still fetched eventually") + assert.LessOrEqual(t, maxInFlight.Load(), int32(maxConcurrentCertFetches), + "concurrent fetches across distinct urls must be bounded by fetchSem") +} + +// TestCertCache_SingleflightDedupesSameURL covers the case fetchOnce exists for: +// many deliveries racing to fetch the SAME url right after a cert rotation, none +// of which have populated the cache yet. +func TestCertCache_SingleflightDedupesSameURL(t *testing.T) { + key, err := rsa.GenerateKey(rand.Reader, 2048) + require.NoError(t, err) + pemData := selfSignedPEM(t, key) + + dial, hits := certServer(t, func(w http.ResponseWriter, r *http.Request) { + time.Sleep(20 * time.Millisecond) + _, _ = w.Write(pemData) + }) + + c := newCertCache() + u := certURL("rotated") + + const nCallers = 20 + var wg sync.WaitGroup + for i := 0; i < nCallers; i++ { + wg.Add(1) + go func() { + defer wg.Done() + pub, err := c.publicKey(context.Background(), &http.Client{}, dial, u) + assert.NoError(t, err) + assert.Equal(t, key.N, pub.N) + }() + } + wg.Wait() + + assert.EqualValues(t, 1, hits.Load(), "concurrent callers for the same url must share one fetch") +} diff --git a/cloudwatch/cloudwatch.go b/cloudwatch/cloudwatch.go new file mode 100644 index 0000000000..9f92f31595 --- /dev/null +++ b/cloudwatch/cloudwatch.go @@ -0,0 +1,342 @@ +// Package cloudwatch implements ingress for AWS CloudWatch alarms delivered over +// Amazon SNS. +// +// Unlike GoAlert's other ingress handlers this one performs the SNS subscription +// handshake (fetching SubscribeURL) and verifies the RSA message signature, so it +// can be subscribed to an SNS topic directly with no intermediate forwarder. +// +// # Trust model +// +// A valid SNS signature proves only that some SNS topic in some AWS account +// signed the message. It is NOT authorization -- the integration key is. The +// topic is not pinned to a key, so any AWS account holding the key token can +// deliver to it. +// +// The trust anchor for the signing certificate is TLS plus the host allowlist, +// not the certificate's contents: we cannot chain-validate it without pinning an +// AWS CA. That is why the allowlist in allowlist.go and the redirect blocking in +// this file are the load-bearing controls. +package cloudwatch + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "io" + "net/http" + "net/url" + "time" + + "github.com/target/goalert/alert" + "github.com/target/goalert/integrationkey" + "github.com/target/goalert/permission" + "github.com/target/goalert/retry" + "github.com/target/goalert/util/calllimiter" + "github.com/target/goalert/util/errutil" + "github.com/target/goalert/util/log" +) + +const ( + // maxBodyBytes bounds the request body independently of the global + // maxBodySizeMiddleware, which is disabled when MaxReqBodyBytes is 0. SNS + // caps messages at 256KB. + maxBodyBytes = 256 * 1024 + + confirmTimeout = 10 * time.Second + maxConfirmBytes = 8 * 1024 +) + +// Config configures a Handler. +type Config struct { + AlertStore *alert.Store + IntegrationKeyStore *integrationkey.Store + + // Client is used to fetch signing certificates and confirm subscriptions. If + // nil, a default is used. A supplied client must not follow redirects; + // NewHandler enforces that when CheckRedirect is unset. + Client *http.Client + + // BaseURL overrides the origin used for outbound requests. Testing only; the + // host allowlist still runs against the message-supplied URL first. + BaseURL string + + // MaxMessageAge bounds how old a signed Timestamp may be before the message is + // refused as a replay. Zero uses defaultMaxMessageAge. + // + // Widen this for a subscription with a custom delivery policy: SNS allows up to + // 100 retries with an hour of backoff, and a redelivery arriving past the + // window is refused permanently rather than retried. + MaxMessageAge time.Duration +} + +// Handler serves the CloudWatch/SNS ingress endpoint. +type Handler struct { + cfg Config + base *url.URL + hc *http.Client + certs *certCache + maxAge time.Duration +} + +// NewHandler returns a Handler for the given config. +func NewHandler(cfg Config) (*Handler, error) { + h := &Handler{cfg: cfg, hc: cfg.Client, certs: newCertCache(), maxAge: cfg.MaxMessageAge} + if h.maxAge <= 0 { + h.maxAge = defaultMaxMessageAge + } + if h.hc == nil { + h.hc = defaultClient() + } + if h.hc.CheckRedirect == nil { + // Enforced here rather than trusted to the caller: not following redirects + // is the load-bearing half of the SSRF control (the allowlist only covers + // the first hop), and the cert fetch happens before signature verification. + // A caller passing a plain &http.Client{} would silently reopen that hole. + h.hc = &http.Client{ + Transport: h.hc.Transport, + Timeout: h.hc.Timeout, + CheckRedirect: func(*http.Request, []*http.Request) error { return http.ErrUseLastResponse }, + } + } + if cfg.BaseURL != "" { + u, err := url.Parse(cfg.BaseURL) + if err != nil { + return nil, fmt.Errorf("cloudwatch: parse BaseURL: %w", err) + } + h.base = u + } + + return h, nil +} + +// defaultClient returns a client that never follows redirects. +// +// This is not optional: the host allowlist only covers the first hop, so a +// single 302 from an allowlisted host would otherwise reach an arbitrary +// address, including the EC2 instance metadata endpoint. +func defaultClient() *http.Client { + return &http.Client{ + Transport: calllimiter.RoundTripper(http.DefaultTransport), + CheckRedirect: func(req *http.Request, via []*http.Request) error { + return http.ErrUseLastResponse + }, + } +} + +// dialURL maps an already-validated URL to the origin to actually request. In +// production (BaseURL empty) it returns u unchanged. +func (h *Handler) dialURL(u *url.URL) *url.URL { + if h.base == nil { + return u + } + + // Scheme is replaced along with the host so a plain-HTTP test server works + // against an allowlist that requires HTTPS. Path and query are preserved. + out := *u + out.Scheme = h.base.Scheme + out.Host = h.base.Host + out.User = nil + out.Path = h.base.Path + u.Path + + return &out +} + +// ServeIncoming handles an SNS message. +// +// Invariant: no outbound request other than the signing-certificate fetch, and +// no alert write, happens before the signature is verified. The cert fetch +// necessarily precedes verification, which is why checkCertURL must be airtight. +func (h *Handler) ServeIncoming(w http.ResponseWriter, r *http.Request) { + ctx := r.Context() + + err := permission.LimitCheckAny(ctx, permission.Service) + if errutil.HTTPError(ctx, w, err) { + return + } + + // MaxBytesReader rather than io.LimitReader: errutil.HTTPError maps + // *http.MaxBytesError to a clean 413, whereas a silent truncation would + // surface as a confusing 400 or 403. + r.Body = http.MaxBytesReader(w, r.Body, maxBodyBytes) + data, err := io.ReadAll(r.Body) + if errutil.HTTPErrorRetry(ctx, w, err) { + return + } + + // Deliberately not gated on Content-Type: SNS always sends text/plain and + // offers no way to change it. Gating is the bug that makes the generic + // endpoint unusable for SNS. + var e envelope + if err := json.Unmarshal(data, &e); err != nil { + log.Debugf(ctx, "cloudwatch: bad request body: %v", err) + clientError(w, http.StatusBadRequest) + return + } + + ctx = log.WithFields(ctx, log.Fields{ + "SNSType": e.Type, + "TopicArn": e.TopicARN, + "SNSMessageID": e.MessageID, + }) + + // Must precede verification: the signed field list depends on the type. + if _, ok := signedFields[e.Type]; !ok { + log.Debugf(ctx, "cloudwatch: unknown message type %q", e.Type) + clientError(w, http.StatusBadRequest) + return + } + if e.SigningCertURL == "" || e.Signature == "" || e.SignatureVersion == "" || e.MessageID == "" || e.TopicARN == "" { + log.Debugf(ctx, "cloudwatch: envelope is missing required fields") + clientError(w, http.StatusBadRequest) + return + } + + u, err := checkCertURL(e.SigningCertURL) + if err != nil { + // Info level: a rejected cert URL is a security-relevant event, but it is + // caller-driven and must not fail the smoke harness. + log.Logf(ctx, "cloudwatch: rejected signing cert URL: %v", err) + clientError(w, http.StatusForbidden) + return + } + + pub, err := h.certs.publicKey(ctx, h.hc, h.dialURL, u.String()) + if err != nil { + // Transient: 503 so SNS retries rather than dropping the alarm. + log.Log(ctx, fmt.Errorf("cloudwatch: signing certificate: %w", err)) + serverError(w) + return + } + + if err := verifyMessage(time.Now(), h.maxAge, pub, &e); err != nil { + // The response is the same 403 either way -- the client must not learn which + // check failed -- but the log separates them, because only one is actionable: + // a stale message means the host clock is off or MaxMessageAge is too tight + // for this subscription's retry policy, whereas a forged one needs nothing. + if errors.Is(err, errStaleMessage) { + log.Log(ctx, fmt.Errorf("cloudwatch: refused stale message: %w", err)) + } else { + log.Logf(ctx, "cloudwatch: %v", err) + } + clientError(w, http.StatusForbidden) + return + } + + switch e.Type { + case typeSubscriptionConfirmation: + h.serveConfirmation(ctx, w, &e) + case typeUnsubscribeConfirmation: + // Log loudly: someone detached a live alarm feed. Deliberately do NOT + // fetch SubscribeURL -- re-confirming would resurrect a subscription + // somebody removed on purpose. + log.Log(ctx, fmt.Errorf("cloudwatch: subscription removed for topic %s", e.TopicARN)) + w.WriteHeader(http.StatusOK) + case typeNotification: + h.serveNotification(ctx, w, &e) + default: + clientError(w, http.StatusBadRequest) + } +} + +// serveConfirmation completes the SNS subscription handshake. The signature has +// already been verified, and SubscribeURL is inside the signed field set, so the +// URL is known to have come from AWS. +func (h *Handler) serveConfirmation(ctx context.Context, w http.ResponseWriter, e *envelope) { + u, err := checkSubscribeURL(e.SubscribeURL) + if err != nil { + // A valid signature over a non-AWS host should be impossible. + log.Log(ctx, fmt.Errorf("cloudwatch: rejected SubscribeURL: %w", err)) + clientError(w, http.StatusBadRequest) + return + } + + reqCtx, cancel := context.WithTimeout(ctx, confirmTimeout) + defer cancel() + + req, err := http.NewRequestWithContext(reqCtx, http.MethodGet, h.dialURL(u).String(), nil) + if err != nil { + log.Log(ctx, fmt.Errorf("cloudwatch: build confirmation request: %w", err)) + serverError(w) + return + } + + resp, err := h.hc.Do(req) + if err != nil { + log.Log(ctx, fmt.Errorf("cloudwatch: confirm subscription: %w", err)) + serverError(w) + return + } + // Drain and close for connection reuse. The body is never inspected and never + // echoed anywhere: reflecting it would turn a blind SSRF into a read oracle. + _, _ = io.Copy(io.Discard, io.LimitReader(resp.Body, maxConfirmBytes)) + resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + // 503 so SNS re-sends; the confirmation token stays valid for 3 days. + log.Log(ctx, fmt.Errorf("cloudwatch: confirm subscription: %s", resp.Status)) + serverError(w) + return + } + + // A new alarm feed attaching to a service is rare and worth recording. Note + // the Token and full SubscribeURL are deliberately not logged: the Token can + // confirm or cancel the subscription. + log.Logf(ctx, "cloudwatch: confirmed subscription for topic %s", e.TopicARN) + w.WriteHeader(http.StatusOK) +} + +func (h *Handler) serveNotification(ctx context.Context, w http.ResponseWriter, e *envelope) { + a, meta, ok := buildAlert(*e) + if !ok { + // INSUFFICIENT_DATA transitions are noise. 2xx so SNS does not retry. + w.WriteHeader(http.StatusNoContent) + return + } + a.ServiceID = permission.ServiceID(ctx) + if len(meta) == 0 { + meta = nil + } + + var created *alert.Alert + err := retry.DoTemporaryError(func(int) error { + var err error + created, _, err = h.cfg.AlertStore.CreateOrUpdateWithMeta(ctx, &a, meta) + return err + }, + retry.Log(ctx), + // Deliberately shorter than genericapi's Limit(10)+1s backoff: SNS aborts + // an HTTPS delivery at 15s, and under an alarm storm long in-handler + // retries just convert the storm into queue-full 429s. Let SNS handle the + // long-horizon retrying. + retry.Limit(5), + retry.FibBackoff(250*time.Millisecond), + ) + // HTTPErrorRetry, not HTTPError: the retries above are already exhausted, so an + // error here is an infrastructure failure and 503 asks SNS to keep trying. + if errutil.HTTPErrorRetry(ctx, w, err) { + return + } + + // created is nil with a nil error when the status was closed and no open + // alert held this dedup key. A stray OK is normal, so this is info, not an + // error, and not a 500. + if created == nil { + log.Logf(ctx, "cloudwatch: no open alert to close") + } + + w.WriteHeader(http.StatusNoContent) +} + +// clientError writes a bare status text, never the underlying error. +func clientError(w http.ResponseWriter, code int) { + http.Error(w, http.StatusText(code), code) +} + +// serverError reports a transient infrastructure failure as 503 rather than 500. +// SNS retries all 5xx so either would do here, but 503 keeps this handler's +// contract identical to the sibling azuremonitor one, where the distinction is +// load-bearing: Azure retries 503 and not 500. +func serverError(w http.ResponseWriter) { + http.Error(w, http.StatusText(http.StatusServiceUnavailable), http.StatusServiceUnavailable) +} diff --git a/cloudwatch/message.go b/cloudwatch/message.go new file mode 100644 index 0000000000..0b27d60d1d --- /dev/null +++ b/cloudwatch/message.go @@ -0,0 +1,109 @@ +package cloudwatch + +import ( + "fmt" + "strings" +) + +// SNS message types. +const ( + typeNotification = "Notification" + typeSubscriptionConfirmation = "SubscriptionConfirmation" + typeUnsubscribeConfirmation = "UnsubscribeConfirmation" +) + +// signedFields lists the fields AWS includes in the string-to-sign for each +// message type, in the alphabetical order required by the signing scheme. The +// lists are literals rather than sorted at runtime; canonicalOrderTest asserts +// they are in fact sorted. +// +// https://docs.aws.amazon.com/sns/latest/dg/sns-verify-signature-of-message.html +var signedFields = map[string][]string{ + typeNotification: {"Message", "MessageId", "Subject", "Timestamp", "TopicArn", "Type"}, + typeSubscriptionConfirmation: {"Message", "MessageId", "SubscribeURL", "Timestamp", "Token", "TopicArn", "Type"}, + typeUnsubscribeConfirmation: {"Message", "MessageId", "SubscribeURL", "Timestamp", "Token", "TopicArn", "Type"}, +} + +// envelope is the SNS HTTP/S message envelope. +// +// https://docs.aws.amazon.com/sns/latest/dg/sns-message-and-json-formats.html +type envelope struct { + Type string `json:"Type"` + MessageID string `json:"MessageId"` + TopicARN string `json:"TopicArn"` + Message string `json:"Message"` + + // Subject must stay a pointer. AWS omits the Subject block from the + // string-to-sign entirely when the field is absent, and a plain string + // cannot distinguish an absent key from `"Subject": ""`. + Subject *string `json:"Subject"` + + // Timestamp is signed as the raw string; never parse and re-format it for + // the canonical string. + Timestamp string `json:"Timestamp"` + + SignatureVersion string `json:"SignatureVersion"` + Signature string `json:"Signature"` + SigningCertURL string `json:"SigningCertURL"` + + // Present on the two *Confirmation types. Token is a bearer credential that + // can confirm or cancel a subscription: never log it above debug. + SubscribeURL string `json:"SubscribeURL"` + Token string `json:"Token"` +} + +// signedField returns the value of a field in the string-to-sign, and whether it +// is present at all. Absent fields are omitted from the canonical string rather +// than contributing an empty value. An unknown name is a bug in signedFields, +// not attacker input, but is still returned as an error rather than a panic so +// a mistake there degrades to a rejected message instead of a crashed request. +func (e *envelope) signedField(name string) (value string, present bool, err error) { + switch name { + case "Message": + return e.Message, true, nil + case "MessageId": + return e.MessageID, true, nil + case "Subject": + if e.Subject == nil { + return "", false, nil + } + return *e.Subject, true, nil + case "SubscribeURL": + return e.SubscribeURL, true, nil + case "Timestamp": + return e.Timestamp, true, nil + case "Token": + return e.Token, true, nil + case "TopicArn": + return e.TopicARN, true, nil + case "Type": + return e.Type, true, nil + } + return "", false, fmt.Errorf("cloudwatch: unknown signed field %q", name) +} + +// canonicalString builds the AWS SNS string-to-sign. It is pure: no I/O, no +// clock, no package state. +func canonicalString(e *envelope) (string, error) { + fields, ok := signedFields[e.Type] + if !ok { + return "", fmt.Errorf("cloudwatch: unknown message type %q", e.Type) + } + + var b strings.Builder + for _, f := range fields { + v, present, err := e.signedField(f) + if err != nil { + return "", err + } + if !present { + continue + } + b.WriteString(f) + b.WriteByte('\n') + b.WriteString(v) + b.WriteByte('\n') + } + + return b.String(), nil +} diff --git a/cloudwatch/message_test.go b/cloudwatch/message_test.go new file mode 100644 index 0000000000..2fa537a700 --- /dev/null +++ b/cloudwatch/message_test.go @@ -0,0 +1,124 @@ +package cloudwatch + +import ( + "sort" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// The signing scheme requires alphabetical field order, and the tables are +// hand-written literals rather than sorted at runtime. +func TestSignedFieldsAreSorted(t *testing.T) { + for msgType, fields := range signedFields { + assert.True(t, sort.StringsAreSorted(fields), "%s field list is not sorted: %v", msgType, fields) + } +} + +func strPtr(s string) *string { return &s } + +func TestCanonicalString(t *testing.T) { + tests := []struct { + name string + env envelope + want string + wantErr bool + }{ + { + name: "notification with subject", + env: envelope{ + Type: typeNotification, + MessageID: "mid", + TopicARN: "arn", + Subject: strPtr("subj"), + Message: "msg", + Timestamp: "ts", + }, + want: "Message\nmsg\nMessageId\nmid\nSubject\nsubj\nTimestamp\nts\nTopicArn\narn\nType\nNotification\n", + }, + { + // Absent Subject omits the whole block, it does not contribute an + // empty value. + name: "notification without subject", + env: envelope{ + Type: typeNotification, + MessageID: "mid", + TopicARN: "arn", + Message: "msg", + Timestamp: "ts", + }, + want: "Message\nmsg\nMessageId\nmid\nTimestamp\nts\nTopicArn\narn\nType\nNotification\n", + }, + { + // A forger cannot fake absence with an empty string: this produces a + // different canonical string, so verification fails. + name: "notification with empty subject differs from absent", + env: envelope{ + Type: typeNotification, + MessageID: "mid", + TopicARN: "arn", + Subject: strPtr(""), + Message: "msg", + Timestamp: "ts", + }, + want: "Message\nmsg\nMessageId\nmid\nSubject\n\nTimestamp\nts\nTopicArn\narn\nType\nNotification\n", + }, + { + name: "subscription confirmation", + env: envelope{ + Type: typeSubscriptionConfirmation, + MessageID: "mid", + TopicARN: "arn", + Message: "msg", + Timestamp: "ts", + SubscribeURL: "https://sub", + Token: "tok", + }, + want: "Message\nmsg\nMessageId\nmid\nSubscribeURL\nhttps://sub\nTimestamp\nts\nToken\ntok\nTopicArn\narn\nType\nSubscriptionConfirmation\n", + }, + { + name: "unsubscribe confirmation", + env: envelope{ + Type: typeUnsubscribeConfirmation, + MessageID: "mid", + TopicARN: "arn", + Message: "msg", + Timestamp: "ts", + SubscribeURL: "https://sub", + Token: "tok", + }, + want: "Message\nmsg\nMessageId\nmid\nSubscribeURL\nhttps://sub\nTimestamp\nts\nToken\ntok\nTopicArn\narn\nType\nUnsubscribeConfirmation\n", + }, + { + // Subject is not signed for confirmations, so it must not appear even + // when present. + name: "confirmation ignores subject", + env: envelope{ + Type: typeSubscriptionConfirmation, + MessageID: "mid", + TopicARN: "arn", + Subject: strPtr("ignored"), + Message: "msg", + Timestamp: "ts", + SubscribeURL: "https://sub", + Token: "tok", + }, + want: "Message\nmsg\nMessageId\nmid\nSubscribeURL\nhttps://sub\nTimestamp\nts\nToken\ntok\nTopicArn\narn\nType\nSubscriptionConfirmation\n", + }, + {name: "unknown type errors", env: envelope{Type: "Bogus"}, wantErr: true}, + {name: "empty type errors", env: envelope{}, wantErr: true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := canonicalString(&tt.env) + if tt.wantErr { + assert.Error(t, err) + return + } + require.NoError(t, err) + assert.Equal(t, tt.want, got) + }) + } +} diff --git a/cloudwatch/payload.go b/cloudwatch/payload.go new file mode 100644 index 0000000000..c04cf4aa0f --- /dev/null +++ b/cloudwatch/payload.go @@ -0,0 +1,321 @@ +package cloudwatch + +import ( + "crypto/sha256" + "encoding/hex" + "encoding/json" + "errors" + "fmt" + "net/url" + "strings" + + "github.com/target/goalert/alert" + "github.com/target/goalert/validation/validate" +) + +const ( + // maxReasonLen bounds NewStateReason, the one unbounded field in practice. + // Without it a verbose reason pushes AlarmDescription -- which carries the + // runbook URL -- past MaxDetailsLength and it gets truncated away. + maxReasonLen = 2048 + + // maxMetaValueLen is a first-pass bound on each value, in RUNES. Values come + // straight from the payload, and exceeding alert.ValidateMetadata's total cap + // is a client error, which SNS would retry forever. This alone is not + // sufficient to guarantee staying under that cap, which sums BYTES: with + // today's 7 keys, 7*1024 4-byte runes would be ~28KiB, under the 32KiB limit, + // but only arithmetically -- it breaks the moment another key is added. + // cleanMeta enforces the real, byte-based budget as a second pass. + maxMetaValueLen = 1024 + + // maxMetaTotalBytes leaves headroom under alert.ValidateMetadata's 32768-byte + // cap for the metadata keys themselves -- short ASCII constants, but sized off + // as a margin rather than their exact total so this doesn't need updating + // every time a key is added to alarmMeta. + maxMetaTotalBytes = 32000 +) + +// cloudWatchAlarm is the subset of a CloudWatch alarm notification we map. +type cloudWatchAlarm struct { + AlarmName string `json:"AlarmName"` + AlarmDescription string `json:"AlarmDescription"` + AWSAccountID string `json:"AWSAccountId"` + NewStateValue string `json:"NewStateValue"` + OldStateValue string `json:"OldStateValue"` + NewStateReason string `json:"NewStateReason"` + StateChangeTime string `json:"StateChangeTime"` + AlarmARN string `json:"AlarmArn"` + + // Region is CloudWatch's display name, e.g. "US West (Oregon)". Deliberately + // unused: region always comes from the topic ARN. + Region string `json:"Region"` + + Trigger struct { + Namespace string `json:"Namespace"` + MetricName string `json:"MetricName"` + } `json:"Trigger"` +} + +// buildAlert maps a verified SNS Notification onto an alert. +// +// The returned alert has ServiceID unset; the caller fills it in from the +// integration key. Source is always alert.SourceCloudwatch and Dedup is always +// non-nil -- a nil Dedup would silently fall back to a content hash that changes +// on every state transition, breaking both idempotency and the OK close. +// +// ok is false when the notification is intentionally ignored and nothing at all +// should be created (today: CloudWatch INSUFFICIENT_DATA). There is no error +// return: any body we cannot read as a CloudWatch alarm is mapped as a raw +// notification instead of failing, so this can never produce a 4xx. +func buildAlert(e envelope) (a alert.Alert, meta map[string]string, ok bool) { + region, topic := splitTopicARN(e.TopicARN) + + var alarm cloudWatchAlarm + err := json.Unmarshal([]byte(e.Message), &alarm) + + // Tolerate a type mismatch on some other field: encoding/json still fills + // everything it could decode, so a numeric AWSAccountId maps as an alarm + // minus that one value rather than losing the dedup contract entirely. + // Note the branch test is a bare non-empty check, matching the reference's + // truthiness gate: a whitespace-only AlarmName is still a CloudWatch alarm, + // and buildAlarm gives it the "unnamed alarm" fallback rather than letting it + // through as a raw notification with a blank summary. + var typeErr *json.UnmarshalTypeError + isAlarm := (err == nil || errors.As(err, &typeErr)) && alarm.AlarmName != "" + if isAlarm { + return buildAlarm(alarm, region, topic) + } + + return buildRaw(e, topic), cleanMeta(map[string]string{ + "topic": topic, + "region": region, + "source": "sns-raw", + }), true +} + +func buildAlarm(al cloudWatchAlarm, region, topic string) (alert.Alert, map[string]string, bool) { + // region is derived from the SNS topic's ARN, which is only the alarm's + // true region by coincidence: the deployed architecture is one topic + // receiving alarms from every region, so a topic's region and a given + // alarm's region routinely differ. Prefer the alarm's own ARN when it + // parses -- otherwise "Region:" here can silently disagree with the + // Console link below, which always uses the alarm's own ARN. + if alarmRegion, _, ok := parseAlarmARN(al.AlarmARN); ok { + region = alarmRegion + } + + if strings.EqualFold(al.NewStateValue, "INSUFFICIENT_DATA") { + return alert.Alert{}, nil, false + } + + // Sanitize before testing emptiness, not TrimSpace: SanitizeText also strips + // non-printable control characters, which TrimSpace leaves alone. Testing the + // raw value would let e.g. AlarmName == "\x01\x02" through as non-blank, only + // for SanitizeText to reduce it to "" a few lines down. That does NOT fail + // validation -- validate.Text treats an empty body as valid regardless of its + // minimum length -- so the fallback would be silently skipped and the alert + // created with a blank Summary: real, but useless to whoever is paged. + name := al.AlarmName + if sanitizeSummary(name) == "" { + name = "unnamed alarm on " + topic + } + + status := alert.StatusTriggered + if al.NewStateValue == "OK" { + status = alert.StatusClosed + } + + return alert.Alert{ + Summary: validate.SanitizeText(name, alert.MaxSummaryLength), + Details: validate.SanitizeText(alarmDetails(al, region, topic), alert.MaxDetailsLength), + Source: alert.SourceCloudwatch, + Status: status, + + // Cross-system contract: hex sha256 of the untruncated, unsanitized alarm + // name, matching the CloudWatch alarm Lambdas that post to PagerDuty. + // Changing it means one alarm produces two alerts. + Dedup: alert.NewUserDedup(sha256Hex(name)), + }, alarmMeta(al, region, topic), true +} + +func alarmDetails(al cloudWatchAlarm, region, topic string) string { + lines := []string{"State: " + al.OldStateValue + " -> " + al.NewStateValue} + + add := func(label, value string) { + if value != "" { + lines = append(lines, label+": "+value) + } + } + add("Reason", truncRunes(al.NewStateReason, maxReasonLen)) + add("Changed", al.StateChangeTime) + add("Region", region) + add("Account", al.AWSAccountID) + if al.Trigger.MetricName != "" { + add("Metric", al.Trigger.Namespace+"/"+al.Trigger.MetricName) + } + add("Topic", topic) + add("Alarm ARN", al.AlarmARN) + add("Console", alarmConsoleURL(al.AlarmARN)) + + if al.AlarmDescription != "" { + // Blank line, then the description verbatim: it carries the runbook URL. + lines = append(lines, "", al.AlarmDescription) + } + + return strings.Join(lines, "\n") +} + +// parseAlarmARN extracts the region and alarm name from a CloudWatch alarm +// ARN, e.g. arn:aws:cloudwatch:us-west-2:123456789012:alarm:AlarmName. +func parseAlarmARN(arn string) (region, name string, ok bool) { + // SplitN(...,7) so a name containing a literal colon (unusual, but not + // disallowed) stays whole in the last segment rather than being truncated. + parts := strings.SplitN(arn, ":", 7) + if len(parts) != 7 || parts[0] != "arn" || parts[2] != "cloudwatch" || parts[5] != "alarm" { + return "", "", false + } + region, name = parts[3], parts[6] + if region == "" || name == "" { + return "", "", false + } + return region, name, true +} + +// alarmConsoleURL builds a deep link to the alarm in the AWS console from its +// ARN, e.g. arn:aws:cloudwatch:us-west-2:123456789012:alarm:AlarmName. +// +// Returns "" for anything that doesn't parse as a CloudWatch alarm ARN, rather +// than a broken link -- ARN is attacker-influenced (it's a field in the signed +// payload, but not otherwise validated), and malformed input must degrade +// gracefully like every other field here. +func alarmConsoleURL(arn string) string { + region, name, ok := parseAlarmARN(arn) + if !ok { + return "" + } + + // PathEscape, not QueryEscape: this is a URL fragment that the console's own + // JS decodes with decodeURIComponent, which does NOT treat "+" as a space -- + // QueryEscape's "+" for spaces would silently mangle any alarm name + // containing one, and real alarm names commonly do (e.g. "[us-west-2] Too + // Many Write Errors"). + return fmt.Sprintf("https://%s.console.aws.amazon.com/cloudwatch/home?region=%s#alarmsV2:alarm/%s", + region, region, url.PathEscape(name)) +} + +func alarmMeta(al cloudWatchAlarm, region, topic string) map[string]string { + return cleanMeta(map[string]string{ + "topic": topic, + "region": region, + "state": al.NewStateValue, + "aws_account": al.AWSAccountID, + "namespace": al.Trigger.Namespace, + "metric": al.Trigger.MetricName, + "alarm_arn": al.AlarmARN, + }) +} + +// buildRaw maps a non-CloudWatch SNS notification. Raw notifications never close. +func buildRaw(e envelope, topic string) alert.Alert { + // Each candidate is sanitized before the emptiness test: a whitespace-only + // Subject is truthy but sanitizes to "". + summary := sanitizeSummary(derefStr(e.Subject)) + if summary == "" { + summary = sanitizeSummary(firstLine(e.Message)) + } + if summary == "" { + summary = sanitizeSummary("SNS notification on " + topic) + } + + return alert.Alert{ + Summary: summary, + Details: validate.SanitizeText(e.Message, alert.MaxDetailsLength), + Source: alert.SourceCloudwatch, + Status: alert.StatusTriggered, + Dedup: alert.NewUserDedup(sha256Hex(topic + "|" + summary)), + } +} + +// splitTopicARN returns the region and topic name from an SNS topic ARN of the +// form arn:aws:sns:::. A malformed or short ARN yields +// empty segments rather than panicking. +func splitTopicARN(arn string) (region, topic string) { + parts := strings.Split(arn, ":") + if len(parts) > 3 { + region = parts[3] + } + // strings.Split never returns an empty slice, so this index is always safe. + return region, parts[len(parts)-1] +} + +func sanitizeSummary(s string) string { return validate.SanitizeText(s, alert.MaxSummaryLength) } + +func derefStr(s *string) string { + if s == nil { + return "" + } + return *s +} + +func firstLine(s string) string { + s = strings.TrimSpace(s) + if i := strings.IndexAny(s, "\r\n"); i >= 0 { + s = s[:i] + } + return strings.TrimSpace(s) +} + +func truncRunes(s string, max int) string { + r := []rune(s) + if len(r) <= max { + return s + } + return string(r[:max]) +} + +func sha256Hex(s string) string { + sum := sha256.Sum256([]byte(s)) + return hex.EncodeToString(sum[:]) +} + +// cleanMeta drops empty values and bounds the rest so the map can never fail +// alert.ValidateMetadata's total-size check. Values are not sanitized: they are +// JSON-marshalled on write, so control characters are escaped rather than +// injected, and trimming would corrupt an ARN. +// +// The per-value rune cap alone is not enough: it bounds runes, but +// ValidateMetadata sums bytes, and enough multi-byte values at that cap can add +// up to more bytes than the total allows. The byte budget below is computed from +// however many keys are actually non-empty, rather than hardcoded to today's key +// count, so the total still fits if alarmMeta gains another key later. +func cleanMeta(m map[string]string) map[string]string { + for k, v := range m { + if v == "" { + delete(m, k) + } + } + + perValueBudget := maxMetaTotalBytes + if n := len(m); n > 0 { + perValueBudget = maxMetaTotalBytes / n + } + + for k, v := range m { + v = truncRunes(v, maxMetaValueLen) + m[k] = truncBytes(v, perValueBudget) + } + return m +} + +// truncBytes truncates s to at most n bytes without splitting a UTF-8 rune's +// encoding -- a plain byte-slice cut can leave a trailing partial rune, which +// ToValidUTF8 then scrubs rather than emit invalid UTF-8 into the metadata. +func truncBytes(s string, n int) string { + if n <= 0 { + return "" + } + if len(s) <= n { + return s + } + return strings.ToValidUTF8(s[:n], "") +} diff --git a/cloudwatch/payload_test.go b/cloudwatch/payload_test.go new file mode 100644 index 0000000000..2ed8194862 --- /dev/null +++ b/cloudwatch/payload_test.go @@ -0,0 +1,595 @@ +package cloudwatch + +import ( + "encoding/json" + "fmt" + "strings" + "testing" + "unicode/utf8" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/target/goalert/alert" +) + +const ( + testTopicARN = "arn:aws:sns:us-west-2:123456789012:PagerDuty-Data" + testAlarmName = "[us-west-2] Too Many Write Errors" + testRunbook = "Runbook: https://runbook.example.com/dp/ats-write-errors" + testServiceID = "3c1a1a44-8e7a-4d1b-9f4a-2b0e5c6d7f80" +) + +// goldenAlarm is the CloudWatch alarm body used by the golden-path cases. +const goldenAlarm = `{ + "AlarmName": "[us-west-2] Too Many Write Errors", + "AlarmDescription": "Runbook: https://runbook.example.com/dp/ats-write-errors", + "AWSAccountId": "123456789012", + "NewStateValue": "ALARM", + "OldStateValue": "OK", + "NewStateReason": "Threshold Crossed: 1 datapoint was greater than the threshold (1.0).", + "StateChangeTime": "2026-07-30T00:00:00.000+0000", + "AlarmArn": "arn:aws:cloudwatch:us-west-2:123456789012:alarm:x", + "Region": "US West (Oregon)", + "Trigger": {"Namespace": "Eightfold/DP", "MetricName": "WriteErrors"} +}` + +const goldenDetails = `State: OK -> ALARM +Reason: Threshold Crossed: 1 datapoint was greater than the threshold (1.0). +Changed: 2026-07-30T00:00:00.000+0000 +Region: us-west-2 +Account: 123456789012 +Metric: Eightfold/DP/WriteErrors +Topic: PagerDuty-Data +Alarm ARN: arn:aws:cloudwatch:us-west-2:123456789012:alarm:x +Console: https://us-west-2.console.aws.amazon.com/cloudwatch/home?region=us-west-2#alarmsV2:alarm/x + +Runbook: https://runbook.example.com/dp/ats-write-errors` + +func TestAlarmConsoleURL(t *testing.T) { + tests := []struct{ name, arn, want string }{ + { + name: "well-formed arn", + arn: "arn:aws:cloudwatch:us-west-2:123456789012:alarm:x", + want: "https://us-west-2.console.aws.amazon.com/cloudwatch/home?region=us-west-2#alarmsV2:alarm/x", + }, + { + // Spaces/brackets are common in real alarm names and must be + // percent-encoded, not "+"-encoded -- the console's SPA decodes the + // fragment with decodeURIComponent, which does not treat "+" as a space. + name: "name with spaces and brackets is percent-encoded", + arn: "arn:aws:cloudwatch:us-west-2:123456789012:alarm:[us-west-2] Too Many Write Errors", + want: "https://us-west-2.console.aws.amazon.com/cloudwatch/home?region=us-west-2#alarmsV2:alarm/%5Bus-west-2%5D%20Too%20Many%20Write%20Errors", + }, + { + // PathEscape leaves ":" unescaped -- it's valid unencoded in a URL + // path segment per RFC 3986 -- so this also confirms SplitN(...,7) + // kept the whole name (including its colon) rather than truncating it. + name: "name containing a literal colon stays whole", + arn: "arn:aws:cloudwatch:us-west-2:123456789012:alarm:svc:sub-alarm", + want: "https://us-west-2.console.aws.amazon.com/cloudwatch/home?region=us-west-2#alarmsV2:alarm/svc:sub-alarm", + }, + {name: "empty", arn: ""}, + {name: "short arn does not panic", arn: "arn:aws:cloudwatch"}, + {name: "wrong service", arn: "arn:aws:sns:us-west-2:123456789012:alarm:x"}, + {name: "wrong resource type", arn: "arn:aws:cloudwatch:us-west-2:123456789012:topic:x"}, + {name: "not an arn at all", arn: "not-an-arn"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.want, alarmConsoleURL(tt.arn)) + }) + } +} + +func notification(message string) envelope { + return envelope{Type: typeNotification, TopicARN: testTopicARN, Message: message} +} + +func TestSplitTopicARN(t *testing.T) { + tests := []struct { + name, arn, region, topic string + }{ + {name: "full arn", arn: testTopicARN, region: "us-west-2", topic: "PagerDuty-Data"}, + {name: "empty", arn: "", region: "", topic: ""}, + {name: "no colons", arn: "topic", region: "", topic: "topic"}, + {name: "short arn does not panic", arn: "arn:aws:sns", region: "", topic: "sns"}, + {name: "exactly four segments", arn: "arn:aws:sns:us-west-2", region: "us-west-2", topic: "us-west-2"}, + {name: "extra colons take last", arn: "arn:aws:sns:us-west-2:123:my:topic", region: "us-west-2", topic: "topic"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + region, topic := splitTopicARN(tt.arn) + assert.Equal(t, tt.region, region, "region") + assert.Equal(t, tt.topic, topic, "topic") + }) + } +} + +func TestFirstLine(t *testing.T) { + tests := []struct{ name, in, want string }{ + {name: "empty", in: "", want: ""}, + {name: "single line", in: "a", want: "a"}, + {name: "lf", in: "a\nb", want: "a"}, + {name: "crlf", in: "a\r\nb", want: "a"}, + {name: "leading and trailing blank lines", in: "\n\na\n", want: "a"}, + {name: "whitespace only", in: " \n ", want: ""}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.want, firstLine(tt.in)) + }) + } +} + +func TestBuildAlert_CloudWatch(t *testing.T) { + goldenDedup := sha256Hex(testAlarmName) + + t.Run("golden alarm", func(t *testing.T) { + a, meta, ok := buildAlert(notification(goldenAlarm)) + require.True(t, ok) + + assert.Equal(t, testAlarmName, a.Summary) + assert.Equal(t, goldenDetails, a.Details) + assert.Equal(t, alert.StatusTriggered, a.Status) + assert.Equal(t, alert.SourceCloudwatch, a.Source) + require.NotNil(t, a.Dedup) + assert.Equal(t, goldenDedup, a.Dedup.Payload) + assert.Equal(t, map[string]string{ + "topic": "PagerDuty-Data", + "region": "us-west-2", + "state": "ALARM", + "aws_account": "123456789012", + "namespace": "Eightfold/DP", + "metric": "WriteErrors", + "alarm_arn": "arn:aws:cloudwatch:us-west-2:123456789012:alarm:x", + }, meta) + }) + + // The dedup key must be identical to the ALARM case or the close never + // matches the open alert. + t.Run("OK closes with the same dedup", func(t *testing.T) { + body := strings.Replace(goldenAlarm, `"NewStateValue": "ALARM"`, `"NewStateValue": "OK"`, 1) + a, meta, ok := buildAlert(notification(body)) + require.True(t, ok) + + assert.Equal(t, alert.StatusClosed, a.Status) + require.NotNil(t, a.Dedup) + assert.Equal(t, goldenDedup, a.Dedup.Payload) + assert.Equal(t, "OK", meta["state"]) + }) + + t.Run("INSUFFICIENT_DATA creates nothing", func(t *testing.T) { + body := strings.Replace(goldenAlarm, `"NewStateValue": "ALARM"`, `"NewStateValue": "INSUFFICIENT_DATA"`, 1) + _, meta, ok := buildAlert(notification(body)) + assert.False(t, ok) + assert.Nil(t, meta) + }) + + t.Run("insufficient_data is case insensitive", func(t *testing.T) { + body := strings.Replace(goldenAlarm, `"NewStateValue": "ALARM"`, `"NewStateValue": "insufficient_data"`, 1) + _, _, ok := buildAlert(notification(body)) + assert.False(t, ok) + }) + + // The reference implementation compares state == "OK" exactly, so a + // lowercase "ok" triggers rather than closes. CloudWatch only ever sends + // uppercase; this pins the asymmetry with INSUFFICIENT_DATA above. + t.Run("lowercase ok triggers", func(t *testing.T) { + body := strings.Replace(goldenAlarm, `"NewStateValue": "ALARM"`, `"NewStateValue": "ok"`, 1) + a, _, ok := buildAlert(notification(body)) + require.True(t, ok) + assert.Equal(t, alert.StatusTriggered, a.Status) + }) + + t.Run("no description means no trailing blank line", func(t *testing.T) { + body := strings.Replace(goldenAlarm, `"AlarmDescription": "`+testRunbook+`",`, "", 1) + a, _, ok := buildAlert(notification(body)) + require.True(t, ok) + + assert.True(t, strings.HasSuffix(a.Details, "Console: https://us-west-2.console.aws.amazon.com/cloudwatch/home?region=us-west-2#alarmsV2:alarm/x"), a.Details) + assert.NotContains(t, a.Details, testRunbook) + }) + + t.Run("null description", func(t *testing.T) { + body := strings.Replace(goldenAlarm, `"AlarmDescription": "`+testRunbook+`"`, `"AlarmDescription": null`, 1) + a, _, ok := buildAlert(notification(body)) + require.True(t, ok) + assert.NotContains(t, a.Details, testRunbook) + }) + + t.Run("minimal fields", func(t *testing.T) { + e := envelope{Type: typeNotification, Message: `{"AlarmName":"x","NewStateValue":"ALARM"}`} + a, meta, ok := buildAlert(e) + require.True(t, ok) + + // The State line is unconditional even with an empty old state. + assert.Equal(t, "State: -> ALARM", a.Details) + assert.Equal(t, map[string]string{"state": "ALARM"}, meta) + }) + + t.Run("metric line requires a metric name", func(t *testing.T) { + body := strings.Replace(goldenAlarm, `"MetricName": "WriteErrors"`, `"MetricName": ""`, 1) + a, meta, ok := buildAlert(notification(body)) + require.True(t, ok) + + assert.NotContains(t, a.Details, "Metric:") + assert.Equal(t, "Eightfold/DP", meta["namespace"]) + assert.NotContains(t, meta, "metric") + }) + + t.Run("metric name without namespace keeps leading slash", func(t *testing.T) { + body := strings.Replace(goldenAlarm, `"Namespace": "Eightfold/DP"`, `"Namespace": ""`, 1) + a, _, ok := buildAlert(notification(body)) + require.True(t, ok) + assert.Contains(t, a.Details, "Metric: /WriteErrors") + }) + + // Summary is truncated but the dedup hashes the full name, so truncation can + // never collapse two distinct alarms onto one alert. + t.Run("summary truncated but dedup is not", func(t *testing.T) { + long := strings.Repeat("x", 1200) + a, _, ok := buildAlert(notification(`{"AlarmName":"` + long + `","NewStateValue":"ALARM"}`)) + require.True(t, ok) + + assert.Len(t, []rune(a.Summary), alert.MaxSummaryLength) + assert.True(t, strings.HasSuffix(a.Summary, "…")) + require.NotNil(t, a.Dedup) + assert.Equal(t, sha256Hex(long), a.Dedup.Payload) + }) + + // The whole point of capping NewStateReason: it is the one unbounded field, so + // without the cap it would crowd the runbook URL past the details limit. + // Capping it keeps the whole body comfortably under the limit instead. + t.Run("huge reason still keeps the runbook", func(t *testing.T) { + body := strings.Replace(goldenAlarm, + `"NewStateReason": "Threshold Crossed: 1 datapoint was greater than the threshold (1.0)."`, + `"NewStateReason": "`+strings.Repeat("r", 10000)+`"`, 1) + a, _, ok := buildAlert(notification(body)) + require.True(t, ok) + + assert.LessOrEqual(t, len([]rune(a.Details)), alert.MaxDetailsLength) + assert.NotContains(t, a.Details, "…", "details should fit without truncation") + assert.Contains(t, a.Details, testRunbook) + assert.Contains(t, a.Details, "Reason: "+strings.Repeat("r", maxReasonLen)+"\n") + assert.NotContains(t, a.Details, strings.Repeat("r", maxReasonLen+1)) + }) + + t.Run("region comes from the arn not the display name", func(t *testing.T) { + a, meta, ok := buildAlert(notification(goldenAlarm)) + require.True(t, ok) + + assert.Contains(t, a.Details, "Region: us-west-2") + assert.Equal(t, "us-west-2", meta["region"]) + assert.NotContains(t, a.Details, "Oregon") + for k, v := range meta { + assert.NotContains(t, v, "Oregon", "meta[%s]", k) + } + }) + + // The real deployment is one SNS topic (in one region) receiving alarms from + // every region, so a topic's own region and a given alarm's region routinely + // differ. Region must reflect the alarm, not the topic -- otherwise "Region:" + // and the Console link (which always uses the alarm's own ARN) could show + // two different regions on the same alert. + t.Run("cross-region: alarm's own region wins over the topic's", func(t *testing.T) { + body := strings.Replace(goldenAlarm, + `"AlarmArn": "arn:aws:cloudwatch:us-west-2:123456789012:alarm:x"`, + `"AlarmArn": "arn:aws:cloudwatch:us-east-1:123456789012:alarm:x"`, 1) + // testTopicARN is us-west-2; the alarm itself is us-east-1. + a, meta, ok := buildAlert(notification(body)) + require.True(t, ok) + + assert.Contains(t, a.Details, "Region: us-east-1") + assert.NotContains(t, a.Details, "Region: us-west-2") + assert.Equal(t, "us-east-1", meta["region"]) + + // The Console link must agree with the Region line, not the topic. + assert.Contains(t, a.Details, "Console: https://us-east-1.console.aws.amazon.com/") + assert.NotContains(t, a.Details, "console.aws.amazon.com/cloudwatch/home?region=us-west-2") + + // Topic still correctly reflects the topic ARN -- only region changes. + assert.Contains(t, a.Details, "Topic: PagerDuty-Data") + assert.Equal(t, "PagerDuty-Data", meta["topic"]) + }) + + t.Run("malformed topic arn still falls back to the alarm's own region", func(t *testing.T) { + e := envelope{Type: typeNotification, TopicARN: "arn:aws:sns", Message: goldenAlarm} + a, meta, ok := buildAlert(e) + require.True(t, ok) + + // A malformed topic ARN yields no region on its own, but AlarmArn (present + // in goldenAlarm) is independently valid, so region is still populated. + assert.Contains(t, a.Details, "Region: us-west-2") + assert.Equal(t, "us-west-2", meta["region"]) + + assert.Contains(t, a.Details, "Topic: sns") + assert.Equal(t, "sns", meta["topic"]) + }) + + t.Run("empty topic arn falls back to the alarm's own region", func(t *testing.T) { + e := envelope{Type: typeNotification, Message: goldenAlarm} + a, meta, ok := buildAlert(e) + require.True(t, ok) + + // Region has an alarm-level fallback (AlarmArn, present in goldenAlarm), + // so it's populated even with no topic ARN at all. + assert.Contains(t, a.Details, "Region: us-west-2") + assert.Equal(t, "us-west-2", meta["region"]) + + // Topic has no alarm-level equivalent to fall back to. + assert.NotContains(t, a.Details, "Topic:") + assert.NotContains(t, meta, "topic") + assert.Equal(t, testAlarmName, a.Summary) + }) + + // A blank-but-present AlarmName would otherwise sanitize to an empty summary, + // which alert.Normalize accepts, producing an unactionable alert. + t.Run("blank alarm name gets the fallback", func(t *testing.T) { + a, _, ok := buildAlert(notification(`{"AlarmName":" ","NewStateValue":"ALARM"}`)) + require.True(t, ok) + + assert.Equal(t, "unnamed alarm on PagerDuty-Data", a.Summary) + require.NotNil(t, a.Dedup) + assert.Equal(t, sha256Hex("unnamed alarm on PagerDuty-Data"), a.Dedup.Payload) + }) + + // AlarmName made only of non-printable control characters is non-blank under + // TrimSpace (which only strips whitespace) but sanitizes to "" -- the gate + // must test the sanitized value or this slips through as a blank summary + // instead of getting the fallback. + t.Run("control-character-only alarm name gets the fallback", func(t *testing.T) { + // json.Marshal, not a raw literal, so the control bytes are correctly + // JSON-escaped rather than producing invalid JSON that falls through to the + // raw-notification branch instead of exercising buildAlarm at all. + msg, err := json.Marshal(map[string]string{ + "AlarmName": "\x01\x02", + "NewStateValue": "ALARM", + }) + require.NoError(t, err) + + a, _, ok := buildAlert(notification(string(msg))) + require.True(t, ok) + + assert.Equal(t, "unnamed alarm on PagerDuty-Data", a.Summary) + assert.NotEmpty(t, a.Summary, "must never be blank: an empty Summary passes validate.Text silently") + }) + + // encoding/json fills what it can, so one wrong-typed field must not cost us + // the CloudWatch branch and its dedup contract. + t.Run("wrong typed field still maps as an alarm", func(t *testing.T) { + a, meta, ok := buildAlert(notification(`{"AlarmName":"x","AWSAccountId":12345,"NewStateValue":"ALARM"}`)) + require.True(t, ok) + + assert.Equal(t, "x", a.Summary) + assert.NotContains(t, a.Details, "Account:") + assert.NotContains(t, meta, "aws_account") + }) + + t.Run("non string alarm name falls through to raw", func(t *testing.T) { + _, meta, ok := buildAlert(notification(`{"AlarmName":123,"x":"y"}`)) + require.True(t, ok) + assert.Equal(t, "sns-raw", meta["source"]) + }) +} + +func TestBuildAlert_Raw(t *testing.T) { + t.Run("subject wins", func(t *testing.T) { + e := notification("line one\nline two") + e.Subject = strPtr("Backup failed") + a, meta, ok := buildAlert(e) + require.True(t, ok) + + assert.Equal(t, "Backup failed", a.Summary) + assert.Equal(t, "line one\nline two", a.Details) + assert.Equal(t, alert.StatusTriggered, a.Status) + require.NotNil(t, a.Dedup) + assert.Equal(t, sha256Hex("PagerDuty-Data|Backup failed"), a.Dedup.Payload) + assert.Equal(t, map[string]string{ + "topic": "PagerDuty-Data", + "region": "us-west-2", + "source": "sns-raw", + }, meta) + }) + + t.Run("nil subject uses first line", func(t *testing.T) { + a, _, ok := buildAlert(notification("first line\nsecond")) + require.True(t, ok) + assert.Equal(t, "first line", a.Summary) + }) + + t.Run("empty subject uses first line", func(t *testing.T) { + e := notification("hello") + e.Subject = strPtr("") + a, _, ok := buildAlert(e) + require.True(t, ok) + assert.Equal(t, "hello", a.Summary) + }) + + // Python's `subject or ...` treats " " as truthy, which would sanitize to + // an empty summary. + t.Run("whitespace subject falls through", func(t *testing.T) { + e := notification("hello") + e.Subject = strPtr(" ") + a, _, ok := buildAlert(e) + require.True(t, ok) + assert.Equal(t, "hello", a.Summary) + }) + + t.Run("no subject and no message", func(t *testing.T) { + a, _, ok := buildAlert(notification("")) + require.True(t, ok) + assert.Equal(t, "SNS notification on PagerDuty-Data", a.Summary) + assert.Empty(t, a.Details) + }) + + t.Run("no subject no message no arn is still non empty", func(t *testing.T) { + a, _, ok := buildAlert(envelope{Type: typeNotification}) + require.True(t, ok) + // Trailing space is trimmed by SanitizeText. + assert.Equal(t, "SNS notification on", a.Summary) + }) + + t.Run("plain text body", func(t *testing.T) { + a, _, ok := buildAlert(notification("just some text")) + require.True(t, ok) + assert.Equal(t, "just some text", a.Summary) + }) + + t.Run("json array body", func(t *testing.T) { + a, _, ok := buildAlert(notification("[1,2,3]")) + require.True(t, ok) + assert.Equal(t, "[1,2,3]", a.Summary) + }) + + t.Run("json object without alarm name", func(t *testing.T) { + a, _, ok := buildAlert(notification(`{"foo":"bar"}`)) + require.True(t, ok) + assert.Equal(t, `{"foo":"bar"}`, a.Summary) + }) + + // This is why details are sanitized rather than raw-sliced: alert.Normalize + // rejects text that begins with a space or holds non-printables, which would + // otherwise 400 forever while SNS retried. + t.Run("leading newlines sanitized", func(t *testing.T) { + a, _, ok := buildAlert(notification("\n\n hello \n")) + require.True(t, ok) + assert.Equal(t, "hello", a.Details) + assert.Equal(t, "hello", a.Summary) + }) + + t.Run("control chars stripped", func(t *testing.T) { + a, _, ok := buildAlert(notification("bad\x00char")) + require.True(t, ok) + assert.Equal(t, "badchar", a.Details) + }) + + t.Run("over limit details truncated", func(t *testing.T) { + a, _, ok := buildAlert(notification(strings.Repeat("d", 300000))) + require.True(t, ok) + assert.Len(t, []rune(a.Details), alert.MaxDetailsLength) + assert.True(t, strings.HasSuffix(a.Details, "…")) + }) + + t.Run("same subject and topic dedup identically", func(t *testing.T) { + a := notification("body one") + a.Subject = strPtr("same") + b := notification("body two") + b.Subject = strPtr("same") + + one, _, ok := buildAlert(a) + require.True(t, ok) + two, _, ok := buildAlert(b) + require.True(t, ok) + + require.NotNil(t, one.Dedup) + require.NotNil(t, two.Dedup) + assert.Equal(t, one.Dedup.Payload, two.Dedup.Payload) + }) +} + +// Invariants that must hold for every mapped notification, checked across all +// branches at once. +func TestBuildAlert_Invariants(t *testing.T) { + longName := strings.Repeat("x", 1200) + cases := map[string]envelope{ + "golden": notification(goldenAlarm), + "ok": notification(strings.Replace(goldenAlarm, `"NewStateValue": "ALARM"`, `"NewStateValue": "OK"`, 1)), + "minimal alarm": notification(`{"AlarmName":"x","NewStateValue":"ALARM"}`), + "blank alarm name": notification(`{"AlarmName":" ","NewStateValue":"ALARM"}`), + "long alarm name": notification(`{"AlarmName":"` + longName + `","NewStateValue":"ALARM"}`), + "malformed arn": {Type: typeNotification, TopicARN: "arn:aws:sns", Message: goldenAlarm}, + "empty arn": {Type: typeNotification, Message: goldenAlarm}, + "raw plain": notification("just some text"), + "raw empty": notification(""), + "raw nothing at all": {Type: typeNotification}, + "raw control chars": notification("bad\x00char"), + "raw leading newlines": notification("\n\n hello \n"), + "raw huge": notification(strings.Repeat("d", 300000)), + } + + for name, e := range cases { + t.Run(name, func(t *testing.T) { + a, meta, ok := buildAlert(e) + require.True(t, ok) + + // Never fall back to the auto content hash: it changes on every state + // transition, which would break idempotency and the OK close. + require.NotNil(t, a.Dedup, "dedup must never be nil") + assert.Equal(t, alert.DedupTypeUser, a.Dedup.Type) + assert.Equal(t, 1, a.Dedup.Version) + assert.Len(t, a.Dedup.Payload, 64, "dedup payload should be a hex sha256") + assert.Same(t, a.Dedup, a.DedupKey()) + + assert.Equal(t, alert.SourceCloudwatch, a.Source) + assert.NotEmpty(t, a.Summary, "summary must never be empty") + + // Proves the mapper can never produce a client error, and catches a + // missing SourceCloudwatch entry in alert.Normalize's OneOf. + a.ServiceID = testServiceID + _, err := a.Normalize() + assert.NoError(t, err) + + total := 0 + for k, v := range meta { + assert.NotEmpty(t, v, "meta[%s] should have been dropped", k) + assert.LessOrEqual(t, len([]rune(v)), maxMetaValueLen, "meta[%s]", k) + total += len(k) + len(v) + } + assert.Less(t, total, 32*1024, "metadata must stay inside the store cap") + }) + } +} + +// alert.Normalize collapses newlines in the summary and does a single pass of +// double-space replacement, so three spaces become two rather than one. +func TestNormalizeSummaryQuirks(t *testing.T) { + t.Run("three spaces collapse to two", func(t *testing.T) { + a, _, ok := buildAlert(notification(`{"AlarmName":"[us-west-2] Too Many Errors","NewStateValue":"ALARM"}`)) + require.True(t, ok) + assert.Equal(t, "[us-west-2] Too Many Errors", a.Summary) + + a.ServiceID = testServiceID + n, err := a.Normalize() + require.NoError(t, err) + assert.Equal(t, "[us-west-2] Too Many Errors", n.Summary) + }) + + t.Run("newlines become a single space", func(t *testing.T) { + a, _, ok := buildAlert(notification(`{"AlarmName":"a\n\n\nb","NewStateValue":"ALARM"}`)) + require.True(t, ok) + assert.Equal(t, "a\n\nb", a.Summary) + + a.ServiceID = testServiceID + n, err := a.Normalize() + require.NoError(t, err) + assert.Equal(t, "a b", n.Summary) + }) +} + +// TestCleanMeta_StaysUnderByteBudget pins the fix for the byte-vs-rune gap: the +// per-value cap (maxMetaValueLen) bounds RUNES, but alert.ValidateMetadata sums +// BYTES, so multi-byte values at that cap can add up to more bytes than the +// 32KiB total allows. cloudwatch's own 7 keys stay under the limit today only by +// arithmetic coincidence (7 * 1024 runes * 4 bytes/rune < 32KiB); this test uses +// enough keys to actually cross it, so a regression that drops the byte-budget +// pass fails here regardless of alarmMeta's current key count. +func TestCleanMeta_StaysUnderByteBudget(t *testing.T) { + wide := strings.Repeat("😀", maxMetaValueLen) // 1024 runes, 4 bytes each + + m := make(map[string]string, 20) + for i := 0; i < 20; i++ { + m[fmt.Sprintf("key_%d", i)] = wide + } + + out := cleanMeta(m) + + total := 0 + for k, v := range out { + total += len(k) + len(v) + require.True(t, utf8.ValidString(v), "truncation must not split a rune") + } + assert.Less(t, total, 32*1024) +} diff --git a/cloudwatch/signature.go b/cloudwatch/signature.go new file mode 100644 index 0000000000..c8fd1c4632 --- /dev/null +++ b/cloudwatch/signature.go @@ -0,0 +1,147 @@ +package cloudwatch + +import ( + "crypto" + "crypto/rsa" + "crypto/sha1" + "crypto/sha256" + "crypto/x509" + "encoding/base64" + "encoding/pem" + "errors" + "fmt" + "time" +) + +// Freshness window for the signed Timestamp. Timestamp is inside the signed +// field set but AWS does not check it for us, so without this a captured valid +// envelope replays forever. +// +// defaultMaxMessageAge is a tradeoff, and it can lose alarms: past the window +// every redelivery is refused, so a GoAlert outage longer than the window drops +// the alarms SNS is still retrying. An hour comfortably covers SNS's default +// HTTP/S retry policy (50 attempts, ~45 minutes), but a subscription with a +// custom policy can legitimately redeliver for far longer -- hence +// Config.MaxMessageAge. +const ( + defaultMaxMessageAge = time.Hour + maxMessageSkew = 5 * time.Minute + minCertKeyBits = 2048 + timestampFormat = "2006-01-02T15:04:05.000Z" +) + +// errBadSignature wraps every verification failure so the handler has exactly +// one branch to map to 403 and the client learns nothing about which step +// failed. Detail goes to the server log. +var errBadSignature = errors.New("cloudwatch: signature verification failed") + +// errStaleMessage marks a failure as "outside the freshness window" rather than +// "bad signature". It wraps errBadSignature so the handler's single 403 branch +// still covers it and the response stays indistinguishable, but the server log +// can separate the two: a forged message needs no operator action, while a stale +// one means the host clock is wrong or MaxMessageAge is too tight for this +// subscription's retry policy. +var errStaleMessage = fmt.Errorf("%w: outside freshness window", errBadSignature) + +// verifyMessage verifies e's signature against pub and checks that its Timestamp +// is no more than maxAge old. +// +// Pure: no I/O. now is a parameter rather than time.Now() so the freshness +// window is table-testable. +func verifyMessage(now time.Time, maxAge time.Duration, pub *rsa.PublicKey, e *envelope) error { + // Select the hash first: the canonical form could differ in a future + // version, so an unknown version must never fall back to SHA-1. + var hashID crypto.Hash + switch e.SignatureVersion { + case "1": + // SHA-1 is dictated by AWS -- it is their default SignatureVersion, not + // a choice we get to make here. + hashID = crypto.SHA1 + case "2": + hashID = crypto.SHA256 + default: + return fmt.Errorf("%w: unsupported SignatureVersion %q", errBadSignature, e.SignatureVersion) + } + + sig, err := base64.StdEncoding.DecodeString(e.Signature) + if err != nil { + return fmt.Errorf("%w: decode signature: %v", errBadSignature, err) + } + if len(sig) == 0 { + return fmt.Errorf("%w: empty signature", errBadSignature) + } + + str, err := canonicalString(e) + if err != nil { + return fmt.Errorf("%w: %v", errBadSignature, err) + } + + var digest []byte + if hashID == crypto.SHA1 { + sum := sha1.Sum([]byte(str)) + digest = sum[:] + } else { + sum := sha256.Sum256([]byte(str)) + digest = sum[:] + } + + if err := rsa.VerifyPKCS1v15(pub, hashID, digest, sig); err != nil { + return fmt.Errorf("%w: %v", errBadSignature, err) + } + + return checkFreshness(now, maxAge, e.Timestamp) +} + +func checkFreshness(now time.Time, maxAge time.Duration, timestamp string) error { + ts, err := time.Parse(timestampFormat, timestamp) + if err != nil { + // SNS also documents plain RFC3339. + ts, err = time.Parse(time.RFC3339, timestamp) + if err != nil { + return fmt.Errorf("%w: parse Timestamp %q", errBadSignature, timestamp) + } + } + + if age := now.Sub(ts); age > maxAge { + return fmt.Errorf("%w: message is %s old (limit %s)", errStaleMessage, age, maxAge) + } + if skew := ts.Sub(now); skew > maxMessageSkew { + return fmt.Errorf("%w: message is %s in the future", errStaleMessage, skew) + } + + return nil +} + +// parseCertPublicKey extracts the RSA public key from a PEM-encoded signing +// certificate. +// +// The certificate is deliberately not chain-validated and its expiry is not +// enforced: TLS to the allowlisted sns..amazonaws.com host is what +// authenticates these bytes, and enforcing NotAfter would only add a +// clock-skew alert-loss mode. +func parseCertPublicKey(pemData []byte) (*rsa.PublicKey, error) { + block, _ := pem.Decode(pemData) + if block == nil { + return nil, errors.New("cloudwatch: no PEM block in signing certificate") + } + if block.Type != "CERTIFICATE" { + return nil, fmt.Errorf("cloudwatch: PEM block is %q, want CERTIFICATE", block.Type) + } + + cert, err := x509.ParseCertificate(block.Bytes) + if err != nil { + return nil, fmt.Errorf("cloudwatch: parse signing certificate: %w", err) + } + + // Checked assertion: an ECDSA or Ed25519 cert would otherwise panic the + // request goroutine. + pub, ok := cert.PublicKey.(*rsa.PublicKey) + if !ok { + return nil, fmt.Errorf("cloudwatch: signing cert key type %T, want RSA", cert.PublicKey) + } + if pub.N.BitLen() < minCertKeyBits { + return nil, fmt.Errorf("cloudwatch: signing cert key is %d bits, want >= %d", pub.N.BitLen(), minCertKeyBits) + } + + return pub, nil +} diff --git a/cloudwatch/signature_test.go b/cloudwatch/signature_test.go new file mode 100644 index 0000000000..7469a12448 --- /dev/null +++ b/cloudwatch/signature_test.go @@ -0,0 +1,274 @@ +package cloudwatch + +import ( + "crypto" + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "crypto/rsa" + "crypto/sha1" + "crypto/sha256" + "crypto/x509" + "crypto/x509/pkix" + "encoding/base64" + "encoding/pem" + "math/big" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +const testTimestamp = "2026-07-30T12:00:00.000Z" + +var testNow = time.Date(2026, 7, 30, 12, 0, 0, 0, time.UTC) + +// selfSignedPEM returns a PEM-encoded self-signed cert for pub. Only the public +// key is ever read back, so no chain is needed. +func selfSignedPEM(t *testing.T, key *rsa.PrivateKey) []byte { + t.Helper() + + tmpl := &x509.Certificate{ + SerialNumber: big.NewInt(1), + Subject: pkix.Name{CommonName: "sns.amazonaws.com"}, + NotBefore: time.Now().Add(-time.Hour), + NotAfter: time.Now().Add(time.Hour), + } + der, err := x509.CreateCertificate(rand.Reader, tmpl, tmpl, &key.PublicKey, key) + require.NoError(t, err) + + return pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: der}) +} + +// signEnvelope signs e in place using the given version ("1" for SHA-1, AWS's +// default; "2" for SHA-256). +func signEnvelope(t *testing.T, key *rsa.PrivateKey, e *envelope, version string) { + t.Helper() + + e.SignatureVersion = version + str, err := canonicalString(e) + require.NoError(t, err) + + var ( + digest []byte + hashID crypto.Hash + ) + if version == "2" { + sum := sha256.Sum256([]byte(str)) + digest, hashID = sum[:], crypto.SHA256 + } else { + sum := sha1.Sum([]byte(str)) + digest, hashID = sum[:], crypto.SHA1 + } + + sig, err := rsa.SignPKCS1v15(rand.Reader, key, hashID, digest) + require.NoError(t, err) + e.Signature = base64.StdEncoding.EncodeToString(sig) +} + +func testEnvelope() envelope { + return envelope{ + Type: typeNotification, + MessageID: "mid", + TopicARN: "arn:aws:sns:us-west-2:123456789012:PagerDuty-Data", + Message: `{"AlarmName":"x","NewStateValue":"ALARM"}`, + Timestamp: testTimestamp, + } +} + +func TestVerifyMessage(t *testing.T) { + key, err := rsa.GenerateKey(rand.Reader, 2048) + require.NoError(t, err) + other, err := rsa.GenerateKey(rand.Reader, 2048) + require.NoError(t, err) + + t.Run("v1 sha1 round trip", func(t *testing.T) { + e := testEnvelope() + signEnvelope(t, key, &e, "1") + assert.NoError(t, verifyMessage(testNow, defaultMaxMessageAge, &key.PublicKey, &e)) + }) + + t.Run("v2 sha256 round trip", func(t *testing.T) { + e := testEnvelope() + signEnvelope(t, key, &e, "2") + assert.NoError(t, verifyMessage(testNow, defaultMaxMessageAge, &key.PublicKey, &e)) + }) + + t.Run("subscription confirmation round trip", func(t *testing.T) { + e := testEnvelope() + e.Type = typeSubscriptionConfirmation + e.SubscribeURL = "https://sns.us-west-2.amazonaws.com/?Action=ConfirmSubscription&Token=tok" + e.Token = "tok" + signEnvelope(t, key, &e, "1") + assert.NoError(t, verifyMessage(testNow, defaultMaxMessageAge, &key.PublicKey, &e)) + }) + + t.Run("wrong key rejected", func(t *testing.T) { + e := testEnvelope() + signEnvelope(t, key, &e, "1") + assert.ErrorIs(t, verifyMessage(testNow, defaultMaxMessageAge, &other.PublicKey, &e), errBadSignature) + }) + + t.Run("tampered message rejected", func(t *testing.T) { + e := testEnvelope() + signEnvelope(t, key, &e, "1") + e.Message = `{"AlarmName":"tampered","NewStateValue":"ALARM"}` + assert.ErrorIs(t, verifyMessage(testNow, defaultMaxMessageAge, &key.PublicKey, &e), errBadSignature) + }) + + t.Run("empty subject cannot forge absent subject", func(t *testing.T) { + e := testEnvelope() + signEnvelope(t, key, &e, "1") // signed with Subject absent + e.Subject = strPtr("") // now present-but-empty + assert.ErrorIs(t, verifyMessage(testNow, defaultMaxMessageAge, &key.PublicKey, &e), errBadSignature) + }) + + t.Run("unknown signature version rejected", func(t *testing.T) { + e := testEnvelope() + signEnvelope(t, key, &e, "1") + // Must not silently fall back to SHA-1: a future version could change the + // canonical form. + e.SignatureVersion = "3" + assert.ErrorIs(t, verifyMessage(testNow, defaultMaxMessageAge, &key.PublicKey, &e), errBadSignature) + }) + + t.Run("empty signature version rejected", func(t *testing.T) { + e := testEnvelope() + signEnvelope(t, key, &e, "1") + e.SignatureVersion = "" + assert.ErrorIs(t, verifyMessage(testNow, defaultMaxMessageAge, &key.PublicKey, &e), errBadSignature) + }) + + t.Run("non base64 signature rejected", func(t *testing.T) { + e := testEnvelope() + signEnvelope(t, key, &e, "1") + e.Signature = "!!!not base64!!!" + assert.ErrorIs(t, verifyMessage(testNow, defaultMaxMessageAge, &key.PublicKey, &e), errBadSignature) + }) + + t.Run("empty signature rejected", func(t *testing.T) { + e := testEnvelope() + signEnvelope(t, key, &e, "1") + e.Signature = "" + assert.ErrorIs(t, verifyMessage(testNow, defaultMaxMessageAge, &key.PublicKey, &e), errBadSignature) + }) + + t.Run("unknown type rejected", func(t *testing.T) { + e := testEnvelope() + signEnvelope(t, key, &e, "1") + e.Type = "Bogus" + assert.ErrorIs(t, verifyMessage(testNow, defaultMaxMessageAge, &key.PublicKey, &e), errBadSignature) + }) +} + +// Timestamp is inside the signed field set, so without a freshness window a +// captured valid envelope replays forever. +func TestVerifyMessage_Freshness(t *testing.T) { + key, err := rsa.GenerateKey(rand.Reader, 2048) + require.NoError(t, err) + + tests := []struct { + name string + now time.Time + ok bool + }{ + {name: "exactly now", now: testNow, ok: true}, + {name: "one minute old", now: testNow.Add(time.Minute), ok: true}, + {name: "59 minutes old", now: testNow.Add(59 * time.Minute), ok: true}, + {name: "two hours old", now: testNow.Add(2 * time.Hour)}, + {name: "one minute in future", now: testNow.Add(-time.Minute), ok: true}, + {name: "ten minutes in future", now: testNow.Add(-10 * time.Minute)}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + e := testEnvelope() + signEnvelope(t, key, &e, "1") + err := verifyMessage(tt.now, defaultMaxMessageAge, &key.PublicKey, &e) + if tt.ok { + assert.NoError(t, err) + return + } + assert.ErrorIs(t, err, errBadSignature) + }) + } +} + +func TestVerifyMessage_UnparsableTimestamp(t *testing.T) { + key, err := rsa.GenerateKey(rand.Reader, 2048) + require.NoError(t, err) + + e := testEnvelope() + e.Timestamp = "not a timestamp" + signEnvelope(t, key, &e, "1") + assert.ErrorIs(t, verifyMessage(testNow, defaultMaxMessageAge, &key.PublicKey, &e), errBadSignature) +} + +func TestVerifyMessage_RFC3339Timestamp(t *testing.T) { + key, err := rsa.GenerateKey(rand.Reader, 2048) + require.NoError(t, err) + + e := testEnvelope() + e.Timestamp = "2026-07-30T12:00:00Z" + signEnvelope(t, key, &e, "1") + assert.NoError(t, verifyMessage(testNow, defaultMaxMessageAge, &key.PublicKey, &e)) +} + +func TestParseCertPublicKey(t *testing.T) { + key, err := rsa.GenerateKey(rand.Reader, 2048) + require.NoError(t, err) + + t.Run("valid rsa cert", func(t *testing.T) { + pub, err := parseCertPublicKey(selfSignedPEM(t, key)) + require.NoError(t, err) + assert.Equal(t, key.N, pub.N) + }) + + t.Run("no pem block", func(t *testing.T) { + _, err := parseCertPublicKey([]byte("not a pem file")) + assert.Error(t, err) + }) + + t.Run("empty input", func(t *testing.T) { + _, err := parseCertPublicKey(nil) + assert.Error(t, err) + }) + + t.Run("wrong pem block type", func(t *testing.T) { + block := pem.EncodeToMemory(&pem.Block{Type: "PRIVATE KEY", Bytes: []byte("x")}) + _, err := parseCertPublicKey(block) + assert.Error(t, err) + }) + + t.Run("garbage der", func(t *testing.T) { + block := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: []byte("garbage")}) + _, err := parseCertPublicKey(block) + assert.Error(t, err) + }) + + // An unchecked type assertion here would panic the request goroutine. + t.Run("non rsa key is an error not a panic", func(t *testing.T) { + ecKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + require.NoError(t, err) + + tmpl := &x509.Certificate{ + SerialNumber: big.NewInt(1), + Subject: pkix.Name{CommonName: "sns.amazonaws.com"}, + NotBefore: time.Now().Add(-time.Hour), + NotAfter: time.Now().Add(time.Hour), + } + der, err := x509.CreateCertificate(rand.Reader, tmpl, tmpl, &ecKey.PublicKey, ecKey) + require.NoError(t, err) + + _, err = parseCertPublicKey(pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: der})) + assert.ErrorContains(t, err, "want RSA") + }) + + t.Run("undersized key rejected", func(t *testing.T) { + small, err := rsa.GenerateKey(rand.Reader, 1024) + require.NoError(t, err) + _, err = parseCertPublicKey(selfSignedPEM(t, small)) + assert.ErrorContains(t, err, "bits") + }) +} diff --git a/gadb/models.go b/gadb/models.go index 51378a4483..749de5cfc5 100644 --- a/gadb/models.go +++ b/gadb/models.go @@ -169,6 +169,8 @@ func (ns NullEnumAlertLogSubjectType) Value() (driver.Value, error) { type EnumAlertSource string const ( + EnumAlertSourceAzureMonitor EnumAlertSource = "azureMonitor" + EnumAlertSourceCloudwatch EnumAlertSource = "cloudwatch" EnumAlertSourceEmail EnumAlertSource = "email" EnumAlertSourceGeneric EnumAlertSource = "generic" EnumAlertSourceGrafana EnumAlertSource = "grafana" @@ -302,6 +304,8 @@ func (ns NullEnumHeartbeatState) Value() (driver.Value, error) { type EnumIntegrationKeysType string const ( + EnumIntegrationKeysTypeAzureMonitor EnumIntegrationKeysType = "azureMonitor" + EnumIntegrationKeysTypeCloudwatch EnumIntegrationKeysType = "cloudwatch" EnumIntegrationKeysTypeEmail EnumIntegrationKeysType = "email" EnumIntegrationKeysTypeGeneric EnumIntegrationKeysType = "generic" EnumIntegrationKeysTypeGrafana EnumIntegrationKeysType = "grafana" diff --git a/gadb/queries.sql.go b/gadb/queries.sql.go index fbde380de5..4bd3e21390 100644 --- a/gadb/queries.sql.go +++ b/gadb/queries.sql.go @@ -668,6 +668,56 @@ func (q *Queries) Alert_LockManyAlertServices(ctx context.Context, alertIds []in return err } +const alert_LockOneAlertDetails = `-- name: Alert_LockOneAlertDetails :one +SELECT + details +FROM + alerts +WHERE + id = $1 + -- ensure the alert is associated with the service, if coming from an integration + AND (service_id = $2 + OR $2 IS NULL) +FOR UPDATE +` + +type Alert_LockOneAlertDetailsParams struct { + ID int64 + ServiceID uuid.NullUUID +} + +// Returns the details for the alert and locks its row, so that a read-modify-write +// of details cannot interleave with a concurrent one. +func (q *Queries) Alert_LockOneAlertDetails(ctx context.Context, arg Alert_LockOneAlertDetailsParams) (string, error) { + row := q.db.QueryRowContext(ctx, alert_LockOneAlertDetails, arg.ID, arg.ServiceID) + var details string + err := row.Scan(&details) + return details, err +} + +const alert_LockOneAlertMetadata = `-- name: Alert_LockOneAlertMetadata :one +SELECT + id +FROM + alerts +WHERE + id = $1 +FOR UPDATE +` + +// Locks the alert's row for a metadata read-modify-write, so two concurrent +// writers cannot both read the same starting document and one silently +// overwrite the other. Locks alerts, not alert_data: alert_data has no row +// before an alert's first metadata write, and FOR UPDATE against a table with +// no matching row locks nothing, so it could not serialize the first-writer +// case, which is the common one for a brand new alert. +func (q *Queries) Alert_LockOneAlertMetadata(ctx context.Context, id int64) (int64, error) { + row := q.db.QueryRowContext(ctx, alert_LockOneAlertMetadata, id) + var id_2 int64 + err := row.Scan(&id_2) + return id_2, err +} + const alert_LockOneAlertService = `-- name: Alert_LockOneAlertService :one SELECT maintenance_expires_at NOTNULL::bool AS is_maint_mode, @@ -810,6 +860,34 @@ func (q *Queries) Alert_SetAlertMetadata(ctx context.Context, arg Alert_SetAlert return result.RowsAffected() } +const alert_SetDetails = `-- name: Alert_SetDetails :execrows +UPDATE + alerts +SET + details = $2 +WHERE + id = $1 + AND status != 'closed' + -- ensure the alert is associated with the service, if coming from an integration + AND (service_id = $3 + OR $3 IS NULL) +` + +type Alert_SetDetailsParams struct { + ID int64 + Details string + ServiceID uuid.NullUUID +} + +// Sets the details for the alert. +func (q *Queries) Alert_SetDetails(ctx context.Context, arg Alert_SetDetailsParams) (int64, error) { + result, err := q.db.ExecContext(ctx, alert_SetDetails, arg.ID, arg.Details, arg.ServiceID) + if err != nil { + return 0, err + } + return result.RowsAffected() +} + const alert_SetManyAlertFeedback = `-- name: Alert_SetManyAlertFeedback :many INSERT INTO alert_feedback(alert_id, noise_reason) VALUES (unnest($1::bigint[]), $2) diff --git a/go.mod b/go.mod index 977cbe9ab2..b9913be1aa 100644 --- a/go.mod +++ b/go.mod @@ -47,6 +47,7 @@ require ( github.com/vektah/gqlparser/v2 v2.5.34 golang.org/x/crypto v0.53.0 golang.org/x/oauth2 v0.36.0 + golang.org/x/sync v0.21.0 golang.org/x/sys v0.46.0 golang.org/x/term v0.44.0 google.golang.org/grpc v1.81.1 @@ -185,7 +186,6 @@ require ( golang.org/x/exp v0.0.0-20260410095643-746e56fc9e2f // indirect golang.org/x/mod v0.36.0 // indirect golang.org/x/net v0.55.0 // indirect - golang.org/x/sync v0.21.0 // indirect golang.org/x/telemetry v0.0.0-20260508192327-42602be52be6 // indirect golang.org/x/text v0.38.0 // indirect golang.org/x/tools v0.45.0 // indirect diff --git a/graphql2/generated.go b/graphql2/generated.go index 063972b3b4..f45ae94cdc 100644 --- a/graphql2/generated.go +++ b/graphql2/generated.go @@ -461,6 +461,7 @@ type ComplexityRoot struct { Mutation struct { AddAuthSubject func(childComplexity int, input user.AuthSubject) int + AppendAlertDetails func(childComplexity int, input AppendAlertDetailsInput) int ClearTemporarySchedules func(childComplexity int, input ClearTemporarySchedulesInput) int CloseMatchingAlert func(childComplexity int, input CloseMatchingAlertInput) int CreateAlert func(childComplexity int, input CreateAlertInput) int @@ -492,6 +493,7 @@ type ComplexityRoot struct { ReEncryptKeyringsAndConfig func(childComplexity int) int SendContactMethodVerification func(childComplexity int, input SendContactMethodVerificationInput) int SendSignal func(childComplexity int, input SendSignalInput) int + SetAlertMetadata func(childComplexity int, input SetAlertMetadataInput) int SetAlertNoiseReason func(childComplexity int, input SetAlertNoiseReasonInput) int SetConfig func(childComplexity int, input []ConfigValueInput) int SetFavorite func(childComplexity int, input SetFavoriteInput) int @@ -982,6 +984,8 @@ type MutationResolver interface { CreateAlert(ctx context.Context, input CreateAlertInput) (*alert.Alert, error) CloseMatchingAlert(ctx context.Context, input CloseMatchingAlertInput) (bool, error) SetAlertNoiseReason(ctx context.Context, input SetAlertNoiseReasonInput) (bool, error) + SetAlertMetadata(ctx context.Context, input SetAlertMetadataInput) (bool, error) + AppendAlertDetails(ctx context.Context, input AppendAlertDetailsInput) (bool, error) CreateService(ctx context.Context, input CreateServiceInput) (*service.Service, error) CreateEscalationPolicy(ctx context.Context, input CreateEscalationPolicyInput) (*escalation.Policy, error) CreateEscalationPolicyStep(ctx context.Context, input CreateEscalationPolicyStepInput) (*escalation.Step, error) @@ -2564,6 +2568,17 @@ func (e *executableSchema) Complexity(ctx context.Context, typeName, field strin } return e.ComplexityRoot.Mutation.AddAuthSubject(childComplexity, args["input"].(user.AuthSubject)), true + case "Mutation.appendAlertDetails": + if e.ComplexityRoot.Mutation.AppendAlertDetails == nil { + break + } + + args, err := ec.field_Mutation_appendAlertDetails_args(ctx, rawArgs) + if err != nil { + return 0, false + } + + return e.ComplexityRoot.Mutation.AppendAlertDetails(childComplexity, args["input"].(AppendAlertDetailsInput)), true case "Mutation.clearTemporarySchedules": if e.ComplexityRoot.Mutation.ClearTemporarySchedules == nil { break @@ -2895,6 +2910,17 @@ func (e *executableSchema) Complexity(ctx context.Context, typeName, field strin } return e.ComplexityRoot.Mutation.SendSignal(childComplexity, args["input"].(SendSignalInput)), true + case "Mutation.setAlertMetadata": + if e.ComplexityRoot.Mutation.SetAlertMetadata == nil { + break + } + + args, err := ec.field_Mutation_setAlertMetadata_args(ctx, rawArgs) + if err != nil { + return 0, false + } + + return e.ComplexityRoot.Mutation.SetAlertMetadata(childComplexity, args["input"].(SetAlertMetadataInput)), true case "Mutation.setAlertNoiseReason": if e.ComplexityRoot.Mutation.SetAlertNoiseReason == nil { break @@ -4924,6 +4950,7 @@ func (e *executableSchema) Exec(ctx context.Context) graphql.ResponseHandler { ec.unmarshalInputAlertMetricsOptions, ec.unmarshalInputAlertRecentEventsOptions, ec.unmarshalInputAlertSearchOptions, + ec.unmarshalInputAppendAlertDetailsInput, ec.unmarshalInputAuthSubjectInput, ec.unmarshalInputCalcRotationHandoffTimesInput, ec.unmarshalInputClauseInput, @@ -4973,6 +5000,7 @@ func (e *executableSchema) Exec(ctx context.Context) graphql.ResponseHandler { ec.unmarshalInputSendSignalInput, ec.unmarshalInputServiceAlertStatsOptions, ec.unmarshalInputServiceSearchOptions, + ec.unmarshalInputSetAlertMetadataInput, ec.unmarshalInputSetAlertNoiseReasonInput, ec.unmarshalInputSetFavoriteInput, ec.unmarshalInputSetLabelInput, @@ -6669,6 +6697,20 @@ func (ec *executionContext) field_Mutation_addAuthSubject_args(ctx context.Conte return args, nil } +func (ec *executionContext) field_Mutation_appendAlertDetails_args(ctx context.Context, rawArgs map[string]any) (map[string]any, error) { + var err error + args := map[string]any{} + arg0, err := graphql.ProcessArgField(ctx, rawArgs, "input", + func(ctx context.Context, v any) (AppendAlertDetailsInput, error) { + return ec.unmarshalNAppendAlertDetailsInput2githubᚗcomᚋtargetᚋgoalertᚋgraphql2ᚐAppendAlertDetailsInput(ctx, v) + }) + if err != nil { + return nil, err + } + args["input"] = arg0 + return args, nil +} + func (ec *executionContext) field_Mutation_clearTemporarySchedules_args(ctx context.Context, rawArgs map[string]any) (map[string]any, error) { var err error args := map[string]any{} @@ -7075,6 +7117,20 @@ func (ec *executionContext) field_Mutation_sendSignal_args(ctx context.Context, return args, nil } +func (ec *executionContext) field_Mutation_setAlertMetadata_args(ctx context.Context, rawArgs map[string]any) (map[string]any, error) { + var err error + args := map[string]any{} + arg0, err := graphql.ProcessArgField(ctx, rawArgs, "input", + func(ctx context.Context, v any) (SetAlertMetadataInput, error) { + return ec.unmarshalNSetAlertMetadataInput2githubᚗcomᚋtargetᚋgoalertᚋgraphql2ᚐSetAlertMetadataInput(ctx, v) + }) + if err != nil { + return nil, err + } + args["input"] = arg0 + return args, nil +} + func (ec *executionContext) field_Mutation_setAlertNoiseReason_args(ctx context.Context, rawArgs map[string]any) (map[string]any, error) { var err error args := map[string]any{} @@ -14734,6 +14790,94 @@ func (ec *executionContext) fieldContext_Mutation_setAlertNoiseReason(ctx contex return fc, nil } +func (ec *executionContext) _Mutation_setAlertMetadata(ctx context.Context, field graphql.CollectedField) (ret graphql.Marshaler) { + return graphql.ResolveField( + ctx, + ec.OperationContext, + field, + func(ctx context.Context, field graphql.CollectedField) (*graphql.FieldContext, error) { + return ec.fieldContext_Mutation_setAlertMetadata(ctx, field) + }, + func(ctx context.Context) (any, error) { + fc := graphql.GetFieldContext(ctx) + return ec.Resolvers.Mutation().SetAlertMetadata(ctx, fc.Args["input"].(SetAlertMetadataInput)) + }, + nil, + func(ctx context.Context, selections ast.SelectionSet, v bool) graphql.Marshaler { + return ec.marshalNBoolean2bool(ctx, selections, v) + }, + true, + true, + ) +} +func (ec *executionContext) fieldContext_Mutation_setAlertMetadata(ctx context.Context, field graphql.CollectedField) (fc *graphql.FieldContext, err error) { + fc = &graphql.FieldContext{ + Object: "Mutation", + Field: field, + IsMethod: true, + IsResolver: true, + Child: func(ctx context.Context, field graphql.CollectedField) (*graphql.FieldContext, error) { + return nil, errors.New("field of type Boolean does not have child fields") + }, + } + defer func() { + if r := recover(); r != nil { + err = ec.Recover(ctx, r) + ec.Error(ctx, err) + } + }() + ctx = graphql.WithFieldContext(ctx, fc) + if fc.Args, err = ec.field_Mutation_setAlertMetadata_args(ctx, field.ArgumentMap(ec.Variables)); err != nil { + ec.Error(ctx, err) + return fc, err + } + return fc, nil +} + +func (ec *executionContext) _Mutation_appendAlertDetails(ctx context.Context, field graphql.CollectedField) (ret graphql.Marshaler) { + return graphql.ResolveField( + ctx, + ec.OperationContext, + field, + func(ctx context.Context, field graphql.CollectedField) (*graphql.FieldContext, error) { + return ec.fieldContext_Mutation_appendAlertDetails(ctx, field) + }, + func(ctx context.Context) (any, error) { + fc := graphql.GetFieldContext(ctx) + return ec.Resolvers.Mutation().AppendAlertDetails(ctx, fc.Args["input"].(AppendAlertDetailsInput)) + }, + nil, + func(ctx context.Context, selections ast.SelectionSet, v bool) graphql.Marshaler { + return ec.marshalNBoolean2bool(ctx, selections, v) + }, + true, + true, + ) +} +func (ec *executionContext) fieldContext_Mutation_appendAlertDetails(ctx context.Context, field graphql.CollectedField) (fc *graphql.FieldContext, err error) { + fc = &graphql.FieldContext{ + Object: "Mutation", + Field: field, + IsMethod: true, + IsResolver: true, + Child: func(ctx context.Context, field graphql.CollectedField) (*graphql.FieldContext, error) { + return nil, errors.New("field of type Boolean does not have child fields") + }, + } + defer func() { + if r := recover(); r != nil { + err = ec.Recover(ctx, r) + ec.Error(ctx, err) + } + }() + ctx = graphql.WithFieldContext(ctx, fc) + if fc.Args, err = ec.field_Mutation_appendAlertDetails_args(ctx, field.ArgumentMap(ec.Variables)); err != nil { + ec.Error(ctx, err) + return fc, err + } + return fc, nil +} + func (ec *executionContext) _Mutation_createService(ctx context.Context, field graphql.CollectedField) (ret graphql.Marshaler) { return graphql.ResolveField( ctx, @@ -24885,6 +25029,43 @@ func (ec *executionContext) unmarshalInputAlertSearchOptions(ctx context.Context return it, nil } +func (ec *executionContext) unmarshalInputAppendAlertDetailsInput(ctx context.Context, obj any) (AppendAlertDetailsInput, error) { + var it AppendAlertDetailsInput + if obj == nil { + return it, nil + } + + asMap := map[string]any{} + for k, v := range obj.(map[string]any) { + asMap[k] = v + } + + fieldsInOrder := [...]string{"alertID", "text"} + for _, k := range fieldsInOrder { + v, ok := asMap[k] + if !ok { + continue + } + switch k { + case "alertID": + ctx := graphql.WithPathContext(ctx, graphql.NewPathWithField("alertID")) + data, err := ec.unmarshalNInt2int(ctx, v) + if err != nil { + return it, err + } + it.AlertID = data + case "text": + ctx := graphql.WithPathContext(ctx, graphql.NewPathWithField("text")) + data, err := ec.unmarshalNString2string(ctx, v) + if err != nil { + return it, err + } + it.Text = data + } + } + return it, nil +} + func (ec *executionContext) unmarshalInputAuthSubjectInput(ctx context.Context, obj any) (user.AuthSubject, error) { var it user.AuthSubject if obj == nil { @@ -27621,6 +27802,43 @@ func (ec *executionContext) unmarshalInputServiceSearchOptions(ctx context.Conte return it, nil } +func (ec *executionContext) unmarshalInputSetAlertMetadataInput(ctx context.Context, obj any) (SetAlertMetadataInput, error) { + var it SetAlertMetadataInput + if obj == nil { + return it, nil + } + + asMap := map[string]any{} + for k, v := range obj.(map[string]any) { + asMap[k] = v + } + + fieldsInOrder := [...]string{"alertID", "meta"} + for _, k := range fieldsInOrder { + v, ok := asMap[k] + if !ok { + continue + } + switch k { + case "alertID": + ctx := graphql.WithPathContext(ctx, graphql.NewPathWithField("alertID")) + data, err := ec.unmarshalNInt2int(ctx, v) + if err != nil { + return it, err + } + it.AlertID = data + case "meta": + ctx := graphql.WithPathContext(ctx, graphql.NewPathWithField("meta")) + data, err := ec.unmarshalNAlertMetadataInput2ᚕgithubᚗcomᚋtargetᚋgoalertᚋgraphql2ᚐAlertMetadataInputᚄ(ctx, v) + if err != nil { + return it, err + } + it.Meta = data + } + } + return it, nil +} + func (ec *executionContext) unmarshalInputSetAlertNoiseReasonInput(ctx context.Context, obj any) (SetAlertNoiseReasonInput, error) { var it SetAlertNoiseReasonInput if obj == nil { @@ -33409,6 +33627,20 @@ func (ec *executionContext) _Mutation(ctx context.Context, sel ast.SelectionSet) if out.Values[i] == graphql.Null { out.Invalids++ } + case "setAlertMetadata": + out.Values[i] = ec.OperationContext.RootResolverMiddleware(innerCtx, func(ctx context.Context) (res graphql.Marshaler) { + return ec._Mutation_setAlertMetadata(ctx, field) + }) + if out.Values[i] == graphql.Null { + out.Invalids++ + } + case "appendAlertDetails": + out.Values[i] = ec.OperationContext.RootResolverMiddleware(innerCtx, func(ctx context.Context) (res graphql.Marshaler) { + return ec._Mutation_appendAlertDetails(ctx, field) + }) + if out.Values[i] == graphql.Null { + out.Invalids++ + } case "createService": out.Values[i] = ec.OperationContext.RootResolverMiddleware(innerCtx, func(ctx context.Context) (res graphql.Marshaler) { return ec._Mutation_createService(ctx, field) @@ -39428,6 +39660,21 @@ func (ec *executionContext) unmarshalNAlertMetadataInput2githubᚗcomᚋtarget return res, graphql.ErrorOnPath(ctx, err) } +func (ec *executionContext) unmarshalNAlertMetadataInput2ᚕgithubᚗcomᚋtargetᚋgoalertᚋgraphql2ᚐAlertMetadataInputᚄ(ctx context.Context, v any) ([]AlertMetadataInput, error) { + var vSlice []any + vSlice = graphql.CoerceList(v) + var err error + res := make([]AlertMetadataInput, len(vSlice)) + for i := range vSlice { + ctx := graphql.WithPathContext(ctx, graphql.NewPathWithIndex(i)) + res[i], err = ec.unmarshalNAlertMetadataInput2githubᚗcomᚋtargetᚋgoalertᚋgraphql2ᚐAlertMetadataInput(ctx, vSlice[i]) + if err != nil { + return nil, err + } + } + return res, nil +} + func (ec *executionContext) marshalNAlertPendingNotification2githubᚗcomᚋtargetᚋgoalertᚋgraphql2ᚐAlertPendingNotification(ctx context.Context, sel ast.SelectionSet, v AlertPendingNotification) graphql.Marshaler { return ec._AlertPendingNotification(ctx, sel, &v) } @@ -39486,6 +39733,11 @@ func (ec *executionContext) marshalNAlertsByStatus2ᚖgithubᚗcomᚋtargetᚋgo return ec._AlertsByStatus(ctx, sel, v) } +func (ec *executionContext) unmarshalNAppendAlertDetailsInput2githubᚗcomᚋtargetᚋgoalertᚋgraphql2ᚐAppendAlertDetailsInput(ctx context.Context, v any) (AppendAlertDetailsInput, error) { + res, err := ec.unmarshalInputAppendAlertDetailsInput(ctx, v) + return res, graphql.ErrorOnPath(ctx, err) +} + func (ec *executionContext) marshalNAuthSubject2githubᚗcomᚋtargetᚋgoalertᚋuserᚐAuthSubject(ctx context.Context, sel ast.SelectionSet, v user.AuthSubject) graphql.Marshaler { return ec._AuthSubject(ctx, sel, &v) } @@ -41091,6 +41343,11 @@ func (ec *executionContext) marshalNServiceOnCallUser2ᚕgithubᚗcomᚋtarget return ret } +func (ec *executionContext) unmarshalNSetAlertMetadataInput2githubᚗcomᚋtargetᚋgoalertᚋgraphql2ᚐSetAlertMetadataInput(ctx context.Context, v any) (SetAlertMetadataInput, error) { + res, err := ec.unmarshalInputSetAlertMetadataInput(ctx, v) + return res, graphql.ErrorOnPath(ctx, err) +} + func (ec *executionContext) unmarshalNSetAlertNoiseReasonInput2githubᚗcomᚋtargetᚋgoalertᚋgraphql2ᚐSetAlertNoiseReasonInput(ctx context.Context, v any) (SetAlertNoiseReasonInput, error) { res, err := ec.unmarshalInputSetAlertNoiseReasonInput(ctx, v) return res, graphql.ErrorOnPath(ctx, err) diff --git a/graphql2/graph/_Mutation.graphqls b/graphql2/graph/_Mutation.graphqls index 0d614e3321..e0d5e3a605 100644 --- a/graphql2/graph/_Mutation.graphqls +++ b/graphql2/graph/_Mutation.graphqls @@ -63,6 +63,20 @@ type Mutation { setAlertNoiseReason(input: SetAlertNoiseReasonInput!): Boolean! @deprecated(reason: "Use updateAlerts instead with the noiseReason field.") + """ + setAlertMetadata merges the provided key/value pairs into an existing + alert's metadata, leaving any keys not listed untouched. Fails if the + alert is closed or does not exist. + """ + setAlertMetadata(input: SetAlertMetadataInput!): Boolean! + + """ + appendAlertDetails appends text to an existing alert's Details, separated + by a blank line from whatever is already there. Fails if the alert is + closed or does not exist. + """ + appendAlertDetails(input: AppendAlertDetailsInput!): Boolean! + createService(input: CreateServiceInput!): Service createEscalationPolicy(input: CreateEscalationPolicyInput!): EscalationPolicy createEscalationPolicyStep( diff --git a/graphql2/graphqlapp/alert.go b/graphql2/graphqlapp/alert.go index 800fd9e156..b11b9b58f7 100644 --- a/graphql2/graphqlapp/alert.go +++ b/graphql2/graphqlapp/alert.go @@ -453,6 +453,106 @@ func (m *Mutation) CreateAlert(ctx context.Context, input graphql2.CreateAlertIn return newAlert, nil } +// SetAlertMetadata merges the given key/value pairs into an existing alert's +// metadata. Keys not listed are left untouched -- this is a merge, not a +// replace, even though the underlying store call replaces the whole document; +// existing values are read first and folded in so a caller setting one key +// cannot wipe out metadata set by whatever created the alert. +// +// The read is preceded by LockMetadataTx, which locks the alert's row for the +// rest of the transaction. Without it, two concurrent calls setting different +// keys on the same alert (e.g. two automations, one writing jira_ticket and one +// writing pd_incident) can both read the same starting document and one commits +// last, silently discarding whatever the other added -- while still returning +// true to that caller. +func (m *Mutation) SetAlertMetadata(ctx context.Context, input graphql2.SetAlertMetadataInput) (bool, error) { + add := make(map[string]string, len(input.Meta)) + for _, kv := range input.Meta { + add[kv.Key] = kv.Value + } + + err := withContextTx(ctx, m.DB, func(ctx context.Context, tx *sql.Tx) error { + err := m.AlertStore.LockMetadataTx(ctx, tx, input.AlertID) + if errors.Is(err, sql.ErrNoRows) { + // Consistent with AppendAlertDetails: a missing AlertID is a client input + // error, not a server fault, so it is translated here rather than left to + // surface later as SetMetadataTx's access-denied fallback for a 0-row + // update, which conflates "no such alert" with "wrong service". + return validation.NewFieldError("AlertID", "not found") + } + if err != nil { + return err + } + + existing, err := m.AlertStore.Metadata(ctx, tx, input.AlertID) + if err != nil { + return err + } + + merged := make(map[string]string, len(existing)+len(add)) + for k, v := range existing { + merged[k] = v + } + for k, v := range add { + merged[k] = v + } + + return m.AlertStore.SetMetadataTx(ctx, tx, input.AlertID, merged) + }) + if err != nil { + return false, err + } + + return true, nil +} + +// AppendAlertDetails appends text to an existing alert's Details, separated by +// a blank line from whatever is already there. +// +// Unlike SetAlertMetadata there is no merge-friendly key/value store to fold into: +// Details is free text with no per-key structure, so preserving existing content +// means reading it and appending. The read therefore takes a row lock and happens +// inside the write's transaction -- without the lock, two concurrent appends on +// the same alert both read the same original and the second silently discards +// the first while still returning true. (An ingress CreateOrUpdate racing this is +// not a concern here: it only ever writes Details at creation, never on an +// existing dedup match, so there is no concurrent writer to race outside of +// AppendAlertDetails itself.) +// +// The append is deliberately not pushed down into SQL, which would remove the read +// entirely: SanitizeText's rune-aware truncation and "…" marker are applied to the +// combined text, and left() over bytes is not equivalent. +func (m *Mutation) AppendAlertDetails(ctx context.Context, input graphql2.AppendAlertDetailsInput) (bool, error) { + err := withContextTx(ctx, m.DB, func(ctx context.Context, tx *sql.Tx) error { + current, err := m.AlertStore.LockDetailsTx(ctx, tx, input.AlertID) + if errors.Is(err, sql.ErrNoRows) { + // The bare sql.ErrNoRows is an implementation detail; a missing AlertID is + // a client input error, not a server fault -- translating it keeps it out + // of the error-level server logs (the smoke harness treats any error-level + // log line as a test failure) and gives the caller an actionable message + // instead of a raw SQL error string. + return validation.NewFieldError("AlertID", "not found") + } + if err != nil { + return err + } + + details := current + if details != "" { + details += "\n\n" + } + details += input.Text + details = validate.SanitizeText(details, alert.MaxDetailsLength) + + return m.AlertStore.SetDetailsTx(ctx, tx, input.AlertID, details) + }) + if err != nil { + return false, err + } + + return true, nil +} + func (a *Alert) NoiseReason(ctx context.Context, raw *alert.Alert) (*string, error) { am, err := (*App)(a).FindOneAlertFeedback(ctx, raw.ID) if err != nil { diff --git a/graphql2/graphqlapp/config.go b/graphql2/graphqlapp/config.go index 8f6e02a177..1552c370d9 100644 --- a/graphql2/graphqlapp/config.go +++ b/graphql2/graphqlapp/config.go @@ -17,6 +17,8 @@ func (a *Query) IntegrationKeyTypes(ctx context.Context) ([]graphql2.Integration {ID: "grafana", Name: "Grafana", Label: "Grafana Webhook URL", Enabled: true}, {ID: "site24x7", Name: "Site 24x7", Label: "Site24x7 Webhook URL", Enabled: true}, {ID: "prometheusAlertmanager", Label: "Alertmanager Webhook URL", Name: "Prometheus Alertmanager", Enabled: true}, + {ID: "cloudwatch", Label: "CloudWatch Webhook URL", Name: "Amazon CloudWatch", Enabled: true}, + {ID: "azureMonitor", Label: "Azure Monitor Webhook URL", Name: "Azure Monitor", Enabled: true}, } if expflag.ContextHas(ctx, expflag.UnivKeys) { diff --git a/graphql2/graphqlapp/integrationkey.go b/graphql2/graphqlapp/integrationkey.go index 8a461ab5b0..12695067f6 100644 --- a/graphql2/graphqlapp/integrationkey.go +++ b/graphql2/graphqlapp/integrationkey.go @@ -229,6 +229,10 @@ func (key *IntegrationKey) Href(ctx context.Context, raw *integrationkey.Integra return cfg.CallbackURL("/api/v2/site24x7/incoming", q), nil case integrationkey.TypePrometheusAlertmanager: return cfg.CallbackURL("/api/v2/prometheusalertmanager/incoming", q), nil + case integrationkey.TypeCloudwatch: + return cfg.CallbackURL("/api/v2/cloudwatch/incoming", q), nil + case integrationkey.TypeAzureMonitor: + return cfg.CallbackURL("/api/v2/azuremonitor/incoming", q), nil case integrationkey.TypeEmail: if !cfg.EmailIngressEnabled() { return "", nil diff --git a/graphql2/models_gen.go b/graphql2/models_gen.go index 1a1f03e175..3c62d72735 100644 --- a/graphql2/models_gen.go +++ b/graphql2/models_gen.go @@ -103,6 +103,14 @@ type AlertsByStatus struct { Closed int `json:"closed"` } +type AppendAlertDetailsInput struct { + AlertID int `json:"alertID"` + // Text appended to the alert's existing Details, separated by a blank line. + // The combined text is truncated to fit if it exceeds the details length + // limit. + Text string `json:"text"` +} + type AuthSubjectConnection struct { Nodes []user.AuthSubject `json:"nodes"` PageInfo *PageInfo `json:"pageInfo"` @@ -719,6 +727,14 @@ type ServiceSearchOptions struct { FavoritesFirst *bool `json:"favoritesFirst,omitempty"` } +type SetAlertMetadataInput struct { + AlertID int `json:"alertID"` + // Meta keys provided here are merged into the alert's existing metadata; + // keys not listed are left untouched. There is no delete: an empty value is + // stored as an empty string and the key is still returned by Alert.meta. + Meta []AlertMetadataInput `json:"meta"` +} + type SetAlertNoiseReasonInput struct { AlertID int `json:"alertID"` NoiseReason string `json:"noiseReason"` @@ -1325,6 +1341,8 @@ const ( IntegrationKeyTypeGrafana IntegrationKeyType = "grafana" IntegrationKeyTypeSite24x7 IntegrationKeyType = "site24x7" IntegrationKeyTypePrometheusAlertmanager IntegrationKeyType = "prometheusAlertmanager" + IntegrationKeyTypeCloudwatch IntegrationKeyType = "cloudwatch" + IntegrationKeyTypeAzureMonitor IntegrationKeyType = "azureMonitor" IntegrationKeyTypeEmail IntegrationKeyType = "email" IntegrationKeyTypeUniversal IntegrationKeyType = "universal" ) @@ -1334,13 +1352,15 @@ var AllIntegrationKeyType = []IntegrationKeyType{ IntegrationKeyTypeGrafana, IntegrationKeyTypeSite24x7, IntegrationKeyTypePrometheusAlertmanager, + IntegrationKeyTypeCloudwatch, + IntegrationKeyTypeAzureMonitor, IntegrationKeyTypeEmail, IntegrationKeyTypeUniversal, } func (e IntegrationKeyType) IsValid() bool { switch e { - case IntegrationKeyTypeGeneric, IntegrationKeyTypeGrafana, IntegrationKeyTypeSite24x7, IntegrationKeyTypePrometheusAlertmanager, IntegrationKeyTypeEmail, IntegrationKeyTypeUniversal: + case IntegrationKeyTypeGeneric, IntegrationKeyTypeGrafana, IntegrationKeyTypeSite24x7, IntegrationKeyTypePrometheusAlertmanager, IntegrationKeyTypeCloudwatch, IntegrationKeyTypeAzureMonitor, IntegrationKeyTypeEmail, IntegrationKeyTypeUniversal: return true } return false diff --git a/graphql2/schema.graphql b/graphql2/schema.graphql index 412d1b1359..eaabd4c17c 100644 --- a/graphql2/schema.graphql +++ b/graphql2/schema.graphql @@ -421,6 +421,28 @@ input AlertMetadataInput { value: String! } +input SetAlertMetadataInput { + alertID: Int! + + """ + Meta keys provided here are merged into the alert's existing metadata; + keys not listed are left untouched. There is no delete: an empty value is + stored as an empty string and the key is still returned by Alert.meta. + """ + meta: [AlertMetadataInput!]! +} + +input AppendAlertDetailsInput { + alertID: Int! + + """ + Text appended to the alert's existing Details, separated by a blank line. + The combined text is truncated to fit if it exceeds the details length + limit. + """ + text: String! +} + input CreateAlertInput { summary: String! details: String @@ -1215,6 +1237,8 @@ enum IntegrationKeyType { grafana site24x7 prometheusAlertmanager + cloudwatch + azureMonitor email universal } diff --git a/integrationkey/integrationkey.go b/integrationkey/integrationkey.go index 0957f63daf..5d1825edae 100644 --- a/integrationkey/integrationkey.go +++ b/integrationkey/integrationkey.go @@ -17,7 +17,7 @@ func (i IntegrationKey) Normalize() (*IntegrationKey, error) { err := validate.Many( validate.IDName("Name", i.Name), validate.UUID("ServiceID", i.ServiceID), - validate.OneOf("Type", i.Type, TypeGrafana, TypeSite24x7, TypePrometheusAlertmanager, TypeGeneric, TypeEmail, TypeUniversal), + validate.OneOf("Type", i.Type, TypeGrafana, TypeSite24x7, TypePrometheusAlertmanager, TypeCloudwatch, TypeAzureMonitor, TypeGeneric, TypeEmail, TypeUniversal), validate.ASCII("ExternalSystemName", i.ExternalSystemName, 0, 255), ) if err != nil { diff --git a/integrationkey/store.go b/integrationkey/store.go index ade36f5d1c..98566a18fe 100644 --- a/integrationkey/store.go +++ b/integrationkey/store.go @@ -53,7 +53,7 @@ func (s *Store) GetServiceID(ctx context.Context, id string, t Type) (string, er keyUUID, err := validate.ParseUUID("IntegrationKeyID", id) err = validate.Many( err, - validate.OneOf("IntegrationType", t, TypeGrafana, TypeSite24x7, TypePrometheusAlertmanager, TypeGeneric, TypeEmail, TypeUniversal), + validate.OneOf("IntegrationType", t, TypeGrafana, TypeSite24x7, TypePrometheusAlertmanager, TypeCloudwatch, TypeAzureMonitor, TypeGeneric, TypeEmail, TypeUniversal), ) if err != nil { return "", err diff --git a/integrationkey/type.go b/integrationkey/type.go index 741833c6ea..70f4abd660 100644 --- a/integrationkey/type.go +++ b/integrationkey/type.go @@ -13,6 +13,8 @@ const ( TypeGrafana Type = "grafana" TypeSite24x7 Type = "site24x7" TypePrometheusAlertmanager Type = "prometheusAlertmanager" + TypeCloudwatch Type = "cloudwatch" + TypeAzureMonitor Type = "azureMonitor" TypeGeneric Type = "generic" TypeEmail Type = "email" TypeUniversal Type = "universal" diff --git a/migrate/migrations/20260731141638-cloudwatch-integration.sql b/migrate/migrations/20260731141638-cloudwatch-integration.sql new file mode 100644 index 0000000000..07782e8770 --- /dev/null +++ b/migrate/migrations/20260731141638-cloudwatch-integration.sql @@ -0,0 +1,7 @@ +-- +migrate Up notransaction +-- Add new integration key type 'cloudwatch' + +ALTER TYPE enum_integration_keys_type ADD VALUE IF NOT EXISTS 'cloudwatch'; +ALTER TYPE enum_alert_source ADD VALUE IF NOT EXISTS 'cloudwatch'; + +-- +migrate Down diff --git a/migrate/migrations/20260731201754-azuremonitor-integration.sql b/migrate/migrations/20260731201754-azuremonitor-integration.sql new file mode 100644 index 0000000000..85b2ab8243 --- /dev/null +++ b/migrate/migrations/20260731201754-azuremonitor-integration.sql @@ -0,0 +1,7 @@ +-- +migrate Up notransaction +-- Add new integration key type 'azureMonitor' + +ALTER TYPE enum_integration_keys_type ADD VALUE IF NOT EXISTS 'azureMonitor'; +ALTER TYPE enum_alert_source ADD VALUE IF NOT EXISTS 'azureMonitor'; + +-- +migrate Down diff --git a/migrate/schema.sql b/migrate/schema.sql index 4575451e7f..1c1264b059 100644 --- a/migrate/schema.sql +++ b/migrate/schema.sql @@ -1,7 +1,7 @@ -- This file is auto-generated by "make db-schema"; DO NOT EDIT --- DATA=48c69cc7d2bcafd088fd053a892530b1c79ccd856d4d69c173137a6d8dca58b6 - --- DISK=794c6d630bfe16fca4a32cfa500455bd9d49f6811f59b8dfcee2f603e5fe0da0 - --- PSQL=794c6d630bfe16fca4a32cfa500455bd9d49f6811f59b8dfcee2f603e5fe0da0 - +-- DATA=e8b7dd2545872522cb5268b5ee2799ccc5ddae8c6004e15447cb290e542d843c - +-- DISK=849e12a94e7e6bfe547fbbd0dd8470edf5e9c9e69a184998122858f7dd329260 - +-- PSQL=849e12a94e7e6bfe547fbbd0dd8470edf5e9c9e69a184998122858f7dd329260 - -- -- pgdump-lite database dump -- @@ -51,6 +51,8 @@ CREATE TYPE enum_alert_log_subject_type AS ENUM ( ); CREATE TYPE enum_alert_source AS ENUM ( + 'azureMonitor', + 'cloudwatch', 'email', 'generic', 'grafana', @@ -73,6 +75,8 @@ CREATE TYPE enum_heartbeat_state AS ENUM ( ); CREATE TYPE enum_integration_keys_type AS ENUM ( + 'azureMonitor', + 'cloudwatch', 'email', 'generic', 'grafana', diff --git a/notification/slack/channel.go b/notification/slack/channel.go index c2f33ed892..613fdc21e1 100644 --- a/notification/slack/channel.go +++ b/notification/slack/channel.go @@ -46,6 +46,13 @@ const ( colorAcked = "#867321" ) +// alertStateEmoji maps an AlertState to a status circle emoji, shown next to the alert title. +var alertStateEmoji = map[notification.AlertState]string{ + notification.AlertStateUnacknowledged: "🔴", + notification.AlertStateAcknowledged: "🟡", + notification.AlertStateClosed: "🟢", +} + var ( _ nfydest.MessageSender = &ChannelSender{} _ notification.ReceiverSetter = &ChannelSender{} @@ -358,10 +365,10 @@ func (s *ChannelSender) loadChannels(ctx context.Context) ([]Channel, error) { return channels, nil } -func alertLink(ctx context.Context, id int, summary string) string { +func alertLink(ctx context.Context, id int, summary string, state notification.AlertState) string { cfg := config.FromContext(ctx) path := fmt.Sprintf("/alerts/%d", id) - return fmt.Sprintf("<%s|Alert #%d: %s>", cfg.CallbackURL(path), id, slackutilsx.EscapeMessage(summary)) + return fmt.Sprintf("%s <%s|Alert #%d: %s>", alertStateEmoji[state], cfg.CallbackURL(path), id, slackutilsx.EscapeMessage(summary)) } const ( @@ -375,7 +382,7 @@ const ( func alertMsgOption(ctx context.Context, callbackID string, id int, summary, logEntry string, state notification.AlertState) slack.MsgOption { blocks := []slack.Block{ slack.NewSectionBlock( - slack.NewTextBlockObject("mrkdwn", alertLink(ctx, id, summary), false, false), nil, nil), + slack.NewTextBlockObject("mrkdwn", alertLink(ctx, id, summary, state), false, false), nil, nil), } var color string @@ -459,7 +466,7 @@ func (s *ChannelSender) SendMessage(ctx context.Context, msg notification.Messag // Reply in thread if we already sent a message for this alert. threadOpts := []slack.MsgOption{ slack.MsgOptionTS(ts), - slack.MsgOptionText(alertLink(ctx, t.AlertID, t.Summary), false), + slack.MsgOptionText(alertLink(ctx, t.AlertID, t.Summary, notification.AlertStateUnacknowledged), false), } // Conditionally add broadcast based on config (default: enabled) diff --git a/test/smoke/appendalertdetails_test.go b/test/smoke/appendalertdetails_test.go new file mode 100644 index 0000000000..b623fd8f28 --- /dev/null +++ b/test/smoke/appendalertdetails_test.go @@ -0,0 +1,104 @@ +package smoke + +import ( + "encoding/json" + "fmt" + "strings" + "testing" + + "github.com/stretchr/testify/require" + "github.com/target/goalert/test/smoke/harness" +) + +// TestAppendAlertDetails verifies appendAlertDetails appends to an alert's +// existing Details rather than replacing it, truncates rather than errors +// when the combined text is too long, and is rejected once the alert is +// closed. +func TestAppendAlertDetails(t *testing.T) { + const sql = ` + insert into escalation_policies (id, name) + values + ({{uuid "eid"}}, 'esc policy'); + insert into services (id, escalation_policy_id, name) + values + ({{uuid "sid"}}, {{uuid "eid"}}, 'service'); + ` + + h := harness.NewHarness(t, sql, "") + defer h.Close() + + getDetails := func(alertID int) string { + res := h.GraphQLQuery2(fmt.Sprintf(`query{alert(id:%d){status details}}`, alertID)) + require.Empty(t, res.Errors) + + var result struct { + Alert struct { + Status string + Details string + } + } + require.NoError(t, json.Unmarshal(res.Data, &result)) + return result.Alert.Details + } + + res := h.GraphQLQuery2(`mutation{createAlert(input:{serviceID:"` + h.UUID("sid") + `",summary:"test",details:"State: OK -> ALARM"}){alertID}}`) + require.Empty(t, res.Errors) + + var created struct { + CreateAlert struct{ AlertID int } + } + require.NoError(t, json.Unmarshal(res.Data, &created)) + alertID := created.CreateAlert.AlertID + + require.Equal(t, "State: OK -> ALARM", getDetails(alertID)) + + t.Run("appends without touching existing content", func(t *testing.T) { + res := h.GraphQLQuery2(fmt.Sprintf(`mutation{appendAlertDetails(input:{alertID:%d, text:"Ticket: CLOP-123"})}`, alertID)) + require.Empty(t, res.Errors) + + require.Equal(t, "State: OK -> ALARM\n\nTicket: CLOP-123", getDetails(alertID)) + }) + + t.Run("a second append stacks rather than overwrites", func(t *testing.T) { + res := h.GraphQLQuery2(fmt.Sprintf(`mutation{appendAlertDetails(input:{alertID:%d, text:"Also see CLOP-124"})}`, alertID)) + require.Empty(t, res.Errors) + + require.Equal(t, "State: OK -> ALARM\n\nTicket: CLOP-123\n\nAlso see CLOP-124", getDetails(alertID)) + }) + + t.Run("combined text over the length limit is truncated, not rejected", func(t *testing.T) { + // A resolver that errored here would be a worse outcome than truncating -- + // the alert already exists and is actionable; losing the append entirely + // over a length overflow would just discard useful information. + long := strings.Repeat("x", 10000) + res := h.GraphQLQuery2(fmt.Sprintf(`mutation{appendAlertDetails(input:{alertID:%d, text:%q})}`, alertID, long)) + require.Empty(t, res.Errors) + + details := getDetails(alertID) + require.LessOrEqual(t, len([]rune(details)), 6144) + require.True(t, strings.HasSuffix(details, "…"), "expected truncation marker, got: %s", details[len(details)-20:]) + }) + + t.Run("rejected once the alert is closed", func(t *testing.T) { + res := h.GraphQLQuery2(`mutation{createAlert(input:{serviceID:"` + h.UUID("sid") + `",summary:"closeme",details:"original"}){alertID}}`) + require.Empty(t, res.Errors) + var created struct { + CreateAlert struct{ AlertID int } + } + require.NoError(t, json.Unmarshal(res.Data, &created)) + closedID := created.CreateAlert.AlertID + + res = h.GraphQLQuery2(fmt.Sprintf(`mutation{updateAlerts(input:{alertIDs:[%d], newStatus: StatusClosed}){id}}`, closedID)) + require.Empty(t, res.Errors) + + res = h.GraphQLQuery2(fmt.Sprintf(`mutation{appendAlertDetails(input:{alertID:%d, text:"too late"})}`, closedID)) + require.NotEmpty(t, res.Errors, "expected an error appending details on a closed alert") + + require.Equal(t, "original", getDetails(closedID), "details must be unchanged after a rejected update") + }) + + t.Run("rejected for a nonexistent alert", func(t *testing.T) { + res := h.GraphQLQuery2(`mutation{appendAlertDetails(input:{alertID: 999999999, text:"x"})}`) + require.NotEmpty(t, res.Errors) + }) +} diff --git a/test/smoke/azuremonitor_test.go b/test/smoke/azuremonitor_test.go new file mode 100644 index 0000000000..9e207b4e5c --- /dev/null +++ b/test/smoke/azuremonitor_test.go @@ -0,0 +1,357 @@ +package smoke + +import ( + "encoding/json" + "fmt" + "net/http" + "sort" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/target/goalert/test/smoke/harness" +) + +const azAlertID = "/subscriptions/sub-1/providers/Microsoft.AlertsManagement/alerts/1db044ff-df8f-4064-a559-b9c9f5f4f000" + +// azMetric builds a SingleResourceMultipleMetricCriteria delivery. +func azMetric(alertID, condition string) string { + return fmt.Sprintf(`{ + "schemaId": "azureMonitorCommonAlertSchema", + "data": { + "essentials": { + "alertId": %q, + "alertRule": "Too Many Frontdoor Exceptions", + "severity": "Sev2", + "signalType": "Metric", + "monitorCondition": %q, + "monitoringService": "Platform", + "alertTargetIDs": ["/subscriptions/sub-1/resourceGroups/rg/providers/Microsoft.Storage/storageAccounts/acct"], + "configurationItems": ["stage-www-frontdoor"], + "firedDateTime": "2026-07-18T08:01:01.000Z", + "description": "Runbook: https://runbook.example.com/frontdoor" + }, + "alertContext": { + "conditionType": "SingleResourceMultipleMetricCriteria", + "condition": { + "windowSize": "PT5M", + "allOf": [{ + "metricName": "Transactions", + "operator": "GreaterThan", + "threshold": "0", + "timeAggregation": "Total", + "dimensions": [{"name": "ApiName", "value": "GetBlob"}], + "metricValue": 100 + }] + } + } + } +}`, alertID, condition) +} + +type azAlertNode struct { + AlertID int + Status string + Summary string + Details string + Meta []struct { + Key string + Value string + } +} + +// TestAzureMonitor exercises the Azure Monitor ingress end to end: alert +// creation, redelivery idempotency, the Resolved close and dedup-key release, +// the schema gate, token rejection, and body-size limits. +func TestAzureMonitor(t *testing.T) { + t.Parallel() + + const sql = ` + insert into escalation_policies (id, name) + values + ({{uuid "eid"}}, 'esc policy'); + insert into services (id, escalation_policy_id, name) + values + ({{uuid "sid"}}, {{uuid "eid"}}, 'service'); + insert into integration_keys (id, type, name, service_id) + values + ({{uuid "int_key"}}, 'azureMonitor', 'my key', {{uuid "sid"}}); + ` + + h := harness.NewHarness(t, sql, "azuremonitor-integration") + defer h.Close() + + url := h.URL() + "/api/v2/azuremonitor/incoming?token=" + h.UUID("int_key") + + post := func(t *testing.T, body string) int { + t.Helper() + resp, err := http.Post(url, "application/json", strings.NewReader(body)) + require.NoError(t, err) + resp.Body.Close() + return resp.StatusCode + } + + alerts := func(t *testing.T) []azAlertNode { + t.Helper() + res := h.GraphQLQuery2(`query{alerts(input:{includeNotified:true}){nodes{alertID status summary details meta{key value}}}}`) + require.Empty(t, res.Errors) + + var result struct { + Alerts struct{ Nodes []azAlertNode } + } + require.NoError(t, json.Unmarshal(res.Data, &result), "parse response: %s", string(res.Data)) + sort.Slice(result.Alerts.Nodes, func(i, j int) bool { + return result.Alerts.Nodes[i].AlertID < result.Alerts.Nodes[j].AlertID + }) + return result.Alerts.Nodes + } + + // The UI is server-driven: the dropdown comes from integrationKeyTypes and the + // copyable URL from IntegrationKey.href. If either misses the type, the + // operator is handed a URL that does not work. + t.Run("ui offers the type and the right url", func(t *testing.T) { + res := h.GraphQLQuery2(`query{integrationKeyTypes{id name label enabled}}`) + require.Empty(t, res.Errors) + + var types struct { + IntegrationKeyTypes []struct { + ID, Name, Label string + Enabled bool + } + } + require.NoError(t, json.Unmarshal(res.Data, &types)) + + var found bool + for _, kt := range types.IntegrationKeyTypes { + if kt.ID == "azureMonitor" { + found = true + assert.Equal(t, "Azure Monitor", kt.Name) + assert.True(t, kt.Enabled) + } + } + assert.True(t, found, "azureMonitor must appear in integrationKeyTypes") + + res = h.GraphQLQuery2(`query{service(id:"` + h.UUID("sid") + `"){integrationKeys{type href}}}`) + require.Empty(t, res.Errors) + + var svc struct { + Service struct { + IntegrationKeys []struct{ Type, Href string } + } + } + require.NoError(t, json.Unmarshal(res.Data, &svc)) + require.Len(t, svc.Service.IntegrationKeys, 1) + assert.Equal(t, "azureMonitor", svc.Service.IntegrationKeys[0].Type) + assert.Contains(t, svc.Service.IntegrationKeys[0].Href, "/api/v2/azuremonitor/incoming") + }) + + var firstID int + t.Run("metric alert creates an alert", func(t *testing.T) { + require.Equal(t, http.StatusNoContent, post(t, azMetric(azAlertID, "Fired"))) + + got := alerts(t) + require.Len(t, got, 1) + firstID = got[0].AlertID + + assert.Equal(t, "Too Many Frontdoor Exceptions", got[0].Summary) + assert.Equal(t, "StatusUnacknowledged", got[0].Status) + assert.Contains(t, got[0].Details, "Severity: Sev2") + assert.Contains(t, got[0].Details, "Transactions GreaterThan 0 (Total, PT5M) = 100") + assert.Contains(t, got[0].Details, "Runbook: https://runbook.example.com/frontdoor") + // configurationItems preferred over the full ARM path. + assert.Contains(t, got[0].Details, "Resource: stage-www-frontdoor") + + meta := map[string]string{} + for _, m := range got[0].Meta { + meta[m.Key] = m.Value + } + assert.Equal(t, "Metric", meta["signal_type"]) + assert.Equal(t, "Fired", meta["monitor_condition"]) + assert.Equal(t, azAlertID, meta["alert_id"]) + }) + + t.Run("redelivery is idempotent", func(t *testing.T) { + require.Equal(t, http.StatusNoContent, post(t, azMetric(azAlertID, "Fired"))) + + got := alerts(t) + require.Len(t, got, 1) + assert.Equal(t, firstID, got[0].AlertID) + }) + + // autoMitigate is on for essentially every Azure metric rule, so this path + // runs constantly in production. + t.Run("resolved closes the alert", func(t *testing.T) { + require.Equal(t, http.StatusNoContent, post(t, azMetric(azAlertID, "Resolved"))) + + got := alerts(t) + require.Len(t, got, 1) + assert.Equal(t, firstID, got[0].AlertID) + assert.Equal(t, "StatusClosed", got[0].Status) + }) + + // The close must free the dedup key, or every later firing of the same rule is + // silently suppressed forever. + t.Run("new firing after close creates a new alert", func(t *testing.T) { + const nextFiring = "/subscriptions/sub-1/providers/Microsoft.AlertsManagement/alerts/3a10e1f4-0000-0000-0000-000000000000" + require.Equal(t, http.StatusNoContent, post(t, azMetric(nextFiring, "Fired"))) + + got := alerts(t) + require.Len(t, got, 2) + assert.NotEqual(t, firstID, got[1].AlertID) + assert.Equal(t, "StatusUnacknowledged", got[1].Status) + }) + + // A Resolved delivery for an alert we never opened is normal, not an error. + t.Run("resolved with no open alert", func(t *testing.T) { + const unseen = "/subscriptions/sub-1/providers/Microsoft.AlertsManagement/alerts/never-seen" + require.Equal(t, http.StatusNoContent, post(t, azMetric(unseen, "Resolved"))) + assert.Len(t, alerts(t), 2) + }) + + // Rejected with an actionable message rather than degraded to a blank alert. + t.Run("legacy schema rejected with actionable message", func(t *testing.T) { + body := `{"schemaId":"AzureMonitorMetricAlert","data":{"essentials":{"alertRule":"legacy"}}}` + resp, err := http.Post(url, "application/json", strings.NewReader(body)) + require.NoError(t, err) + defer resp.Body.Close() + + assert.Equal(t, http.StatusBadRequest, resp.StatusCode) + msg := make([]byte, 512) + n, _ := resp.Body.Read(msg) + assert.Contains(t, string(msg[:n]), "common alert schema") + assert.Len(t, alerts(t), 2) + }) + + t.Run("service health payload is usable", func(t *testing.T) { + body := `{ + "schemaId": "azureMonitorCommonAlertSchema", + "data": { + "essentials": { + "alertId": "/subscriptions/sub-1/providers/Microsoft.AlertsManagement/alerts/sh-1", + "alertRule": "ServiceHealthRule", + "signalType": "Activity Log", + "monitorCondition": "Fired", + "monitoringService": "ServiceHealth" + }, + "alertContext": { + "properties": { + "title": "Azure Storage degraded", + "trackingId": "ABC-123", + "communication": "

We are investigating

", + "impactedServices": "[{\"ServiceName\":\"Storage\"}]" + }, + "status": "Resolved" + } + } +}` + require.Equal(t, http.StatusNoContent, post(t, body)) + + got := alerts(t) + require.Len(t, got, 3) + assert.Equal(t, "ServiceHealthRule", got[2].Summary) + assert.Contains(t, got[2].Details, "Tracking ID: ABC-123") + assert.NotContains(t, got[2].Details, "

") + // monitorCondition wins over alertContext.status. + assert.Equal(t, "StatusUnacknowledged", got[2].Status) + }) + + t.Run("log alert carries its query and exactly one link", func(t *testing.T) { + const link = "https://portal.azure.com/#blade/Microsoft_OperationalInsights/filtered-results" + body := fmt.Sprintf(`{ + "schemaId": "azureMonitorCommonAlertSchema", + "data": { + "essentials": { + "alertId": "/subscriptions/sub-1/providers/Microsoft.AlertsManagement/alerts/log-1", + "alertRule": "Heartbeat missing", + "severity": "Sev1", + "signalType": "Log", + "monitorCondition": "Fired", + "monitoringService": "Log Alerts V2", + "configurationItems": ["test-computer"] + }, + "alertContext": { + "conditionType": "LogQueryCriteria", + "condition": { + "windowSize": "PT10M", + "allOf": [{ + "searchQuery": "Heartbeat | summarize count() by Computer", + "metricMeasureColumn": null, + "targetResourceTypes": "['Microsoft.OperationalInsights/workspaces']", + "operator": "GreaterThan", + "threshold": "0", + "timeAggregation": "Count", + "dimensions": [{"name": "Computer", "value": "test-computer"}], + "metricValue": 3, + "failingPeriods": {"numberOfEvaluationPeriods": 1, "minFailingPeriodsToAlert": 1}, + "linkToSearchResultsUI": "https://portal.azure.com/#unfiltered", + "linkToFilteredSearchResultsUI": %q, + "linkToSearchResultsAPI": "https://api.loganalytics.io/v1/unfiltered", + "linkToFilteredSearchResultsAPI": "https://api.loganalytics.io/v1/filtered" + }] + } + } + } +}`, link) + require.Equal(t, http.StatusNoContent, post(t, body)) + + got := alerts(t) + require.Len(t, got, 4) + d := got[3].Details + + assert.Equal(t, "Heartbeat missing", got[3].Summary) + assert.Contains(t, d, "Query: Heartbeat | summarize count() by Computer") + assert.Contains(t, d, "GreaterThan 0 (Count, PT10M) = 3") + assert.Contains(t, d, link) + + // Exactly one link, and neither *API variant: all four together exceed + // the details limit on their own. + assert.Equal(t, 1, strings.Count(d, "Results: ")) + assert.NotContains(t, d, "api.loganalytics.io") + assert.NotContains(t, d, "#unfiltered") + }) + + t.Run("unrecognised conditionType still creates an alert", func(t *testing.T) { + body := strings.Replace( + azMetric("/subscriptions/sub-1/providers/Microsoft.AlertsManagement/alerts/webtest-1", "Fired"), + `"conditionType": "SingleResourceMultipleMetricCriteria"`, + `"conditionType": "WebtestLocationAvailabilityCriteria"`, 1) + require.Equal(t, http.StatusNoContent, post(t, body)) + + got := alerts(t) + require.Len(t, got, 5) + assert.NotEmpty(t, got[4].Summary) + }) + + t.Run("malformed body is a bad request", func(t *testing.T) { + assert.Equal(t, http.StatusBadRequest, post(t, `{not json`)) + assert.Len(t, alerts(t), 5) + }) + + t.Run("oversized body is rejected", func(t *testing.T) { + big := `{"schemaId":"azureMonitorCommonAlertSchema","data":{"essentials":{"alertRule":"` + + strings.Repeat("x", 300*1024) + `"}}}` + assert.Equal(t, http.StatusRequestEntityTooLarge, post(t, big)) + assert.Len(t, alerts(t), 5) + }) + + t.Run("wrong token rejected", func(t *testing.T) { + bad := h.URL() + "/api/v2/azuremonitor/incoming?token=" + h.UUID("sid") + resp, err := http.Post(bad, "application/json", strings.NewReader(azMetric(azAlertID, "Fired"))) + require.NoError(t, err) + resp.Body.Close() + + assert.NotEqual(t, http.StatusNoContent, resp.StatusCode) + assert.Len(t, alerts(t), 5) + }) + + t.Run("missing token rejected", func(t *testing.T) { + resp, err := http.Post(h.URL()+"/api/v2/azuremonitor/incoming", "application/json", + strings.NewReader(azMetric(azAlertID, "Fired"))) + require.NoError(t, err) + resp.Body.Close() + + assert.NotEqual(t, http.StatusNoContent, resp.StatusCode) + assert.Len(t, alerts(t), 5) + }) +} diff --git a/test/smoke/cloudwatch_test.go b/test/smoke/cloudwatch_test.go new file mode 100644 index 0000000000..6907ecc5cd --- /dev/null +++ b/test/smoke/cloudwatch_test.go @@ -0,0 +1,383 @@ +package smoke + +import ( + "crypto" + "crypto/rand" + "crypto/rsa" + "crypto/sha1" + "crypto/x509" + "crypto/x509/pkix" + "encoding/base64" + "encoding/json" + "encoding/pem" + "fmt" + "math/big" + "net/http" + "net/http/httptest" + "sort" + "strings" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/target/goalert/app" + "github.com/target/goalert/test/smoke/harness" +) + +const cwTopicARN = "arn:aws:sns:us-west-2:123456789012:PagerDuty-Data" + +// A genuinely allowlisted URL: only the transport is redirected to the test +// server, so the production host allowlist still runs against this string. +const cwCertURL = "https://sns.us-west-2.amazonaws.com/SimpleNotificationService-test.pem" + +const cwAlarm = `{ + "AlarmName": "[us-west-2] Too Many Write Errors", + "AlarmDescription": "Runbook: https://runbook.example.com/dp/ats-write-errors", + "AWSAccountId": "123456789012", + "NewStateValue": "%s", + "OldStateValue": "OK", + "NewStateReason": "Threshold Crossed", + "StateChangeTime": "2026-07-30T00:00:00.000+0000", + "AlarmArn": "arn:aws:cloudwatch:us-west-2:123456789012:alarm:x", + "Region": "US West (Oregon)", + "Trigger": {"Namespace": "Eightfold/DP", "MetricName": "WriteErrors"} +}` + +// cwSignEnvelope signs the envelope the way AWS does: the signed fields in +// alphabetical order as name+"\n"+value+"\n", omitting Subject entirely when +// absent, then RSA PKCS#1 v1.5 over SHA-1 (SignatureVersion 1, AWS's default). +// +// This is a deliberate independent reimplementation of the production canonical +// string -- if the two ever disagree, that is exactly what this test should +// catch. +func cwSignEnvelope(t *testing.T, key *rsa.PrivateKey, env map[string]string) []byte { + t.Helper() + + var fields []string + switch env["Type"] { + case "Notification": + fields = []string{"Message", "MessageId", "Subject", "Timestamp", "TopicArn", "Type"} + case "SubscriptionConfirmation", "UnsubscribeConfirmation": + fields = []string{"Message", "MessageId", "SubscribeURL", "Timestamp", "Token", "TopicArn", "Type"} + default: + fields = []string{"Message", "MessageId", "Timestamp", "TopicArn", "Type"} + } + + var sb strings.Builder + for _, f := range fields { + v, ok := env[f] + if !ok { + continue + } + sb.WriteString(f + "\n" + v + "\n") + } + + sum := sha1.Sum([]byte(sb.String())) + sig, err := rsa.SignPKCS1v15(rand.Reader, key, crypto.SHA1, sum[:]) + require.NoError(t, err) + + env["SignatureVersion"] = "1" + env["Signature"] = base64.StdEncoding.EncodeToString(sig) + env["SigningCertURL"] = cwCertURL + + body, err := json.Marshal(env) + require.NoError(t, err) + + return body +} + +func cwNotification(message string) map[string]string { + return map[string]string{ + "Type": "Notification", + "MessageId": "11111111-1111-1111-1111-111111111111", + "TopicArn": cwTopicARN, + "Message": message, + "Timestamp": time.Now().UTC().Format("2006-01-02T15:04:05.000Z"), + } +} + +type cwAlertNode struct { + AlertID int + Status string + Summary string + Details string + Meta []struct { + Key string + Value string + } +} + +// TestCloudWatch exercises the full CloudWatch/SNS ingress path in-process: the +// subscription handshake, real RSA signature verification, the host allowlist, +// alarm mapping, dedup, and the OK close. +func TestCloudWatch(t *testing.T) { + t.Parallel() + + const sql = ` + insert into escalation_policies (id, name) + values + ({{uuid "eid"}}, 'esc policy'); + insert into services (id, escalation_policy_id, name) + values + ({{uuid "sid"}}, {{uuid "eid"}}, 'service'); + insert into integration_keys (id, type, name, service_id) + values + ({{uuid "int_key"}}, 'cloudwatch', 'my key', {{uuid "sid"}}); + ` + + signKey, certPEM := cwGenCert(t) + forgedKey, _ := cwGenCert(t) + + certSrv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Query().Get("Action") == "ConfirmSubscription" { + w.WriteHeader(http.StatusOK) + return + } + w.Header().Set("Content-Type", "application/x-pem-file") + _, _ = w.Write(certPEM) + })) + defer certSrv.Close() + + h := harness.NewStoppedHarness(t, sql, nil, "cloudwatch-integration") + defer h.Close() + // The unsubscribe case logs at error level on purpose ("log loudly"), and the + // harness fails a test on any error log line. + h.IgnoreErrorsWith("subscription removed") + h.StartWithAppCfgHook(func(c *app.Config) { c.CloudwatchBaseURL = certSrv.URL }) + + url := h.URL() + "/api/v2/cloudwatch/incoming?token=" + h.UUID("int_key") + + // SNS always posts text/plain; the handler must parse regardless. + post := func(t *testing.T, body []byte) int { + t.Helper() + resp, err := http.Post(url, "text/plain; charset=UTF-8", strings.NewReader(string(body))) + require.NoError(t, err) + resp.Body.Close() + return resp.StatusCode + } + + alerts := func(t *testing.T) []cwAlertNode { + t.Helper() + res := h.GraphQLQuery2(`query{alerts(input:{includeNotified:true}){nodes{alertID status summary details meta{key value}}}}`) + require.Empty(t, res.Errors) + + var result struct { + Alerts struct{ Nodes []cwAlertNode } + } + require.NoError(t, json.Unmarshal(res.Data, &result), "parse response: %s", string(res.Data)) + sort.Slice(result.Alerts.Nodes, func(i, j int) bool { + return result.Alerts.Nodes[i].AlertID < result.Alerts.Nodes[j].AlertID + }) + return result.Alerts.Nodes + } + + // The UI is entirely server-driven: the dropdown comes from + // integrationKeyTypes and the copy-to-clipboard value comes from + // IntegrationKey.href. If either is missing the key type, the user is handed a + // URL that silently will not work with SNS. + t.Run("ui offers the type and the right url", func(t *testing.T) { + res := h.GraphQLQuery2(`query{integrationKeyTypes{id name label enabled}}`) + require.Empty(t, res.Errors) + + var types struct { + IntegrationKeyTypes []struct { + ID, Name, Label string + Enabled bool + } + } + require.NoError(t, json.Unmarshal(res.Data, &types)) + + var found bool + for _, kt := range types.IntegrationKeyTypes { + if kt.ID != "cloudwatch" { + continue + } + found = true + assert.Equal(t, "Amazon CloudWatch", kt.Name) + assert.Equal(t, "CloudWatch Webhook URL", kt.Label) + assert.True(t, kt.Enabled) + } + assert.True(t, found, "cloudwatch must appear in integrationKeyTypes") + + res = h.GraphQLQuery2(`query{service(id:"` + h.UUID("sid") + `"){integrationKeys{id type href}}}`) + require.Empty(t, res.Errors) + + var svc struct { + Service struct { + IntegrationKeys []struct{ ID, Type, Href string } + } + } + require.NoError(t, json.Unmarshal(res.Data, &svc)) + require.Len(t, svc.Service.IntegrationKeys, 1) + + key := svc.Service.IntegrationKeys[0] + assert.Equal(t, "cloudwatch", key.Type) + assert.Contains(t, key.Href, "/api/v2/cloudwatch/incoming") + assert.Contains(t, key.Href, "token="+h.UUID("int_key")) + }) + + t.Run("subscription confirmation", func(t *testing.T) { + env := map[string]string{ + "Type": "SubscriptionConfirmation", + "MessageId": "22222222-2222-2222-2222-222222222222", + "TopicArn": cwTopicARN, + "Message": "You have chosen to subscribe to the topic", + "Timestamp": time.Now().UTC().Format("2006-01-02T15:04:05.000Z"), + "Token": "confirm-token", + "SubscribeURL": "https://sns.us-west-2.amazonaws.com/?Action=ConfirmSubscription&TopicArn=" + cwTopicARN + "&Token=confirm-token", + } + assert.Equal(t, http.StatusOK, post(t, cwSignEnvelope(t, signKey, env))) + assert.Empty(t, alerts(t)) + }) + + var alarmID int + t.Run("alarm creates an alert", func(t *testing.T) { + body := cwSignEnvelope(t, signKey, cwNotification(fmt.Sprintf(cwAlarm, "ALARM"))) + require.Equal(t, http.StatusNoContent, post(t, body)) + + got := alerts(t) + require.Len(t, got, 1) + alarmID = got[0].AlertID + + assert.Equal(t, "[us-west-2] Too Many Write Errors", got[0].Summary) + assert.Contains(t, got[0].Details, "State: OK -> ALARM") + assert.Contains(t, got[0].Details, "Runbook: https://runbook.example.com/dp/ats-write-errors") + // Region must come from the topic ARN, not CloudWatch's display name. + assert.Contains(t, got[0].Details, "Region: us-west-2") + assert.NotContains(t, got[0].Details, "Oregon") + + meta := map[string]string{} + for _, m := range got[0].Meta { + meta[m.Key] = m.Value + } + assert.Equal(t, map[string]string{ + "topic": "PagerDuty-Data", + "region": "us-west-2", + "state": "ALARM", + "aws_account": "123456789012", + "namespace": "Eightfold/DP", + "metric": "WriteErrors", + "alarm_arn": "arn:aws:cloudwatch:us-west-2:123456789012:alarm:x", + }, meta) + }) + + t.Run("redelivery is idempotent", func(t *testing.T) { + body := cwSignEnvelope(t, signKey, cwNotification(fmt.Sprintf(cwAlarm, "ALARM"))) + require.Equal(t, http.StatusNoContent, post(t, body)) + + got := alerts(t) + require.Len(t, got, 1) + assert.Equal(t, alarmID, got[0].AlertID) + }) + + t.Run("insufficient data creates nothing", func(t *testing.T) { + body := cwSignEnvelope(t, signKey, cwNotification(fmt.Sprintf(cwAlarm, "INSUFFICIENT_DATA"))) + require.Equal(t, http.StatusNoContent, post(t, body)) + assert.Len(t, alerts(t), 1) + }) + + t.Run("ok closes the alert", func(t *testing.T) { + body := cwSignEnvelope(t, signKey, cwNotification(fmt.Sprintf(cwAlarm, "OK"))) + require.Equal(t, http.StatusNoContent, post(t, body)) + + got := alerts(t) + require.Len(t, got, 1) + assert.Equal(t, alarmID, got[0].AlertID) + assert.Equal(t, "StatusClosed", got[0].Status) + + // Metadata is written only when the alert is new, so state stays at the + // value it had at creation. Asserting otherwise would be wrong. + meta := map[string]string{} + for _, m := range got[0].Meta { + meta[m.Key] = m.Value + } + assert.Equal(t, "ALARM", meta["state"]) + }) + + // The created alert is nil with a nil error here; a 500 or an error log would + // fail this test twice over. + t.Run("stray ok with no open alert", func(t *testing.T) { + body := cwSignEnvelope(t, signKey, cwNotification(`{"AlarmName":"never-seen","NewStateValue":"OK"}`)) + require.Equal(t, http.StatusNoContent, post(t, body)) + assert.Len(t, alerts(t), 1) + }) + + t.Run("raw notification uses subject", func(t *testing.T) { + env := cwNotification("this is not json") + env["Subject"] = "Backup failed" + require.Equal(t, http.StatusNoContent, post(t, cwSignEnvelope(t, signKey, env))) + + got := alerts(t) + require.Len(t, got, 2) + assert.Equal(t, "Backup failed", got[1].Summary) + + meta := map[string]string{} + for _, m := range got[1].Meta { + meta[m.Key] = m.Value + } + assert.Equal(t, "sns-raw", meta["source"]) + }) + + t.Run("forged signature is rejected", func(t *testing.T) { + body := cwSignEnvelope(t, forgedKey, cwNotification(`{"AlarmName":"forged","NewStateValue":"ALARM"}`)) + assert.Equal(t, http.StatusForbidden, post(t, body)) + assert.Len(t, alerts(t), 2) + }) + + // Proves the allowlist is still live even with CloudwatchBaseURL set. + t.Run("cert host not allowlisted", func(t *testing.T) { + env := cwNotification(`{"AlarmName":"evil","NewStateValue":"ALARM"}`) + body := cwSignEnvelope(t, signKey, env) + + var raw map[string]string + require.NoError(t, json.Unmarshal(body, &raw)) + raw["SigningCertURL"] = "https://sns.us-west-2.amazonaws.com.evil.com/x.pem" + tampered, err := json.Marshal(raw) + require.NoError(t, err) + + assert.Equal(t, http.StatusForbidden, post(t, tampered)) + assert.Len(t, alerts(t), 2) + }) + + t.Run("unknown type is a bad request", func(t *testing.T) { + env := cwNotification(`{"AlarmName":"x","NewStateValue":"ALARM"}`) + env["Type"] = "Bogus" + assert.Equal(t, http.StatusBadRequest, post(t, cwSignEnvelope(t, signKey, env))) + assert.Len(t, alerts(t), 2) + }) + + t.Run("unsubscribe confirmation", func(t *testing.T) { + env := map[string]string{ + "Type": "UnsubscribeConfirmation", + "MessageId": "33333333-3333-3333-3333-333333333333", + "TopicArn": cwTopicARN, + "Message": "You have chosen to deactivate subscription", + "Timestamp": time.Now().UTC().Format("2006-01-02T15:04:05.000Z"), + "Token": "unsub-token", + "SubscribeURL": "https://sns.us-west-2.amazonaws.com/?Action=ConfirmSubscription&Token=unsub-token", + } + assert.Equal(t, http.StatusOK, post(t, cwSignEnvelope(t, signKey, env))) + assert.Len(t, alerts(t), 2) + }) +} + +// cwGenCert returns an RSA key and a self-signed PEM certificate for it. Only the +// public key is ever read back, so no chain is needed. +func cwGenCert(t *testing.T) (*rsa.PrivateKey, []byte) { + t.Helper() + + key, err := rsa.GenerateKey(rand.Reader, 2048) + require.NoError(t, err) + + tmpl := &x509.Certificate{ + SerialNumber: big.NewInt(1), + Subject: pkix.Name{CommonName: "sns.us-west-2.amazonaws.com"}, + NotBefore: time.Now().Add(-time.Hour), + NotAfter: time.Now().Add(time.Hour), + } + der, err := x509.CreateCertificate(rand.Reader, tmpl, tmpl, &key.PublicKey, key) + require.NoError(t, err) + + return key, pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: der}) +} diff --git a/test/smoke/setalertmetadata_test.go b/test/smoke/setalertmetadata_test.go new file mode 100644 index 0000000000..ba3746b5a6 --- /dev/null +++ b/test/smoke/setalertmetadata_test.go @@ -0,0 +1,153 @@ +package smoke + +import ( + "encoding/json" + "fmt" + "sync" + "testing" + + "github.com/stretchr/testify/require" + "github.com/target/goalert/test/smoke/harness" +) + +// TestSetAlertMetadata verifies setAlertMetadata merges into an alert's +// existing metadata rather than replacing it, and is rejected once the alert +// is closed. +func TestSetAlertMetadata(t *testing.T) { + const sql = ` + insert into escalation_policies (id, name) + values + ({{uuid "eid"}}, 'esc policy'); + insert into services (id, escalation_policy_id, name) + values + ({{uuid "sid"}}, {{uuid "eid"}}, 'service'); + ` + + h := harness.NewHarness(t, sql, "") + defer h.Close() + + type metaKV struct { + Key string + Value string + } + getMeta := func(alertID int) map[string]string { + res := h.GraphQLQuery2(fmt.Sprintf(`query{alert(id:%d){status meta{key value}}}`, alertID)) + require.Empty(t, res.Errors) + + var result struct { + Alert struct { + Status string + Meta []metaKV + } + } + require.NoError(t, json.Unmarshal(res.Data, &result)) + + out := make(map[string]string, len(result.Alert.Meta)) + for _, kv := range result.Alert.Meta { + out[kv.Key] = kv.Value + } + return out + } + + // Create with initial metadata, as an ingress integration would at + // alert-creation time. + res := h.GraphQLQuery2(`mutation{createAlert(input:{serviceID:"` + h.UUID("sid") + `",summary:"test",meta:[{key:"source", value:"cloudwatch"}]}){alertID}}`) + require.Empty(t, res.Errors) + + var created struct { + CreateAlert struct{ AlertID int } + } + require.NoError(t, json.Unmarshal(res.Data, &created)) + alertID := created.CreateAlert.AlertID + + require.Equal(t, map[string]string{"source": "cloudwatch"}, getMeta(alertID)) + + t.Run("adds a new key without touching existing ones", func(t *testing.T) { + res := h.GraphQLQuery2(fmt.Sprintf(`mutation{setAlertMetadata(input:{alertID:%d, meta:[{key:"jira_ticket", value:"CLOP-123"}]})}`, alertID)) + require.Empty(t, res.Errors) + + require.Equal(t, map[string]string{ + "source": "cloudwatch", + "jira_ticket": "CLOP-123", + }, getMeta(alertID)) + }) + + t.Run("overwrites an existing key, leaves others alone", func(t *testing.T) { + res := h.GraphQLQuery2(fmt.Sprintf(`mutation{setAlertMetadata(input:{alertID:%d, meta:[{key:"source", value:"cloudwatch-updated"}]})}`, alertID)) + require.Empty(t, res.Errors) + + require.Equal(t, map[string]string{ + "source": "cloudwatch-updated", + "jira_ticket": "CLOP-123", + }, getMeta(alertID)) + }) + + t.Run("an empty value is stored, not treated as a delete", func(t *testing.T) { + // Pins what the schema documents. There is no delete operation, so an empty + // value must round-trip as an empty string with the key still present -- + // otherwise callers storing a legitimately empty value would lose the key. + res := h.GraphQLQuery2(fmt.Sprintf(`mutation{setAlertMetadata(input:{alertID:%d, meta:[{key:"jira_ticket", value:""}]})}`, alertID)) + require.Empty(t, res.Errors) + + meta := getMeta(alertID) + require.Contains(t, meta, "jira_ticket", "key must survive an empty value") + require.Equal(t, "", meta["jira_ticket"]) + require.Equal(t, "cloudwatch-updated", meta["source"], "other keys must be untouched") + + // Restore, so the closed-alert subtest below still asserts against a + // meaningful value. + res = h.GraphQLQuery2(fmt.Sprintf(`mutation{setAlertMetadata(input:{alertID:%d, meta:[{key:"jira_ticket", value:"CLOP-123"}]})}`, alertID)) + require.Empty(t, res.Errors) + }) + + t.Run("concurrent sets of different keys both survive", func(t *testing.T) { + // Reproduces the realistic conflict: two automations, each writing a + // different key on the same alert at the same time (e.g. a Jira automation + // setting jira_ticket while a PagerDuty bridge sets pd_incident). Without + // LockMetadataTx serializing the read-modify-write, both read the same + // starting document and the later writer's INSERT ... ON CONFLICT DO UPDATE + // silently discards whatever the other added -- while still returning true. + const n = 8 + var wg sync.WaitGroup + errs := make([]bool, n) + for i := 0; i < n; i++ { + wg.Add(1) + go func(i int) { + defer wg.Done() + res := h.GraphQLQuery2(fmt.Sprintf(`mutation{setAlertMetadata(input:{alertID:%d, meta:[{key:"concurrent_%d", value:"v%d"}]})}`, alertID, i, i)) + errs[i] = len(res.Errors) > 0 + }(i) + } + wg.Wait() + + for i, hadErr := range errs { + require.False(t, hadErr, "concurrent setAlertMetadata call %d returned an error", i) + } + + meta := getMeta(alertID) + for i := 0; i < n; i++ { + require.Equal(t, fmt.Sprintf("v%d", i), meta[fmt.Sprintf("concurrent_%d", i)], + "key from concurrent call %d must not have been lost", i) + } + // Pre-existing keys from earlier subtests must also have survived. + require.Equal(t, "cloudwatch-updated", meta["source"]) + require.Equal(t, "CLOP-123", meta["jira_ticket"]) + }) + + t.Run("rejected once the alert is closed", func(t *testing.T) { + res := h.GraphQLQuery2(fmt.Sprintf(`mutation{updateAlerts(input:{alertIDs:[%d], newStatus: StatusClosed}){id}}`, alertID)) + require.Empty(t, res.Errors) + + before := getMeta(alertID) + + res = h.GraphQLQuery2(fmt.Sprintf(`mutation{setAlertMetadata(input:{alertID:%d, meta:[{key:"jira_ticket", value:"CLOP-999"}]})}`, alertID)) + require.NotEmpty(t, res.Errors, "expected an error setting metadata on a closed alert") + + require.Equal(t, before, getMeta(alertID), "metadata must be unchanged after a rejected update") + }) + + t.Run("rejected for a nonexistent alert", func(t *testing.T) { + res := h.GraphQLQuery2(`mutation{setAlertMetadata(input:{alertID: 999999999, meta:[{key:"x", value:"y"}]})}`) + require.NotEmpty(t, res.Errors) + }) +} diff --git a/util/errutil/httperror.go b/util/errutil/httperror.go index b19d444022..027103377d 100644 --- a/util/errutil/httperror.go +++ b/util/errutil/httperror.go @@ -45,6 +45,25 @@ func unwrapAll(err error) error { // HTTPError will respond in a standard way when err != nil. If // err is nil, false is returned, true otherwise. func HTTPError(ctx context.Context, w http.ResponseWriter, err error) bool { + return httpError(ctx, w, err, http.StatusInternalServerError) +} + +// HTTPErrorRetry behaves like HTTPError, except an unexpected server-side error +// responds 503 rather than 500. Client errors are unchanged: those are permanent +// and must not be retried. +// +// This is for webhook ingress, where the status code is the only back-channel to +// the sender's retry logic and a dropped delivery is a dropped page. The two +// providers disagree about 500: SNS retries all 5xx and 429, while Azure Monitor +// retries only 408, 429, 503 and 504 -- so a 500 during a database outage loses +// the alert with zero retries. 503 is retried by both. +func HTTPErrorRetry(ctx context.Context, w http.ResponseWriter, err error) bool { + return httpError(ctx, w, err, http.StatusServiceUnavailable) +} + +// httpError maps err onto a response. unexpectedCode is used for errors that +// match no known classification. +func httpError(ctx context.Context, w http.ResponseWriter, err error, unexpectedCode int) bool { if err == nil { return false } @@ -68,8 +87,17 @@ func HTTPError(ctx context.Context, w http.ResponseWriter, err error) bool { // even in the worst case scenario. http.Error(w, "Too many concurrent requests for this key or session", http.StatusTooManyRequests) case errors.Is(err, ctxlock.ErrTimeout): - // Similar to above, but that we timed out waiting in the queue. - http.Error(w, http.StatusText(http.StatusRequestTimeout), http.StatusRequestTimeout) + // Same status as ErrQueueFull above -- this is back-pressure, not a slow + // client, so 408 is wrong: it means the *client* failed to send a complete + // request in time (RFC 9110 15.5.9), the opposite of what happened, and + // webhook senders treat the two very differently -- Amazon SNS retries 429 + // and all 5xx but treats 408 as a permanent failure, so 408 here silently + // discards the delivery instead of backing off. This is an intentional, + // application-wide change to every HTTPError caller, not scoped to ingress; + // the body text differs from ErrQueueFull's so a client can still tell + // "rejected immediately, queue full" from "waited and timed out" even + // though the status code is now the same for both. + http.Error(w, "Too many concurrent requests for this key or session; timed out waiting", http.StatusTooManyRequests) case isCancel(err): // Client disconnected, send 499 back so logs reflect that this // was a client-side problem. @@ -86,9 +114,9 @@ func HTTPError(ctx context.Context, w http.ResponseWriter, err error) bool { // Timeout http.Error(w, http.StatusText(http.StatusGatewayTimeout), http.StatusGatewayTimeout) default: - // For all other unexpected errors, log the error and send a 500. + // For all other unexpected errors, log the error and send unexpectedCode. log.Log(ctx, err) - http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError) + http.Error(w, http.StatusText(unexpectedCode), unexpectedCode) } return true diff --git a/util/errutil/httperror_test.go b/util/errutil/httperror_test.go new file mode 100644 index 0000000000..c8d7436f8a --- /dev/null +++ b/util/errutil/httperror_test.go @@ -0,0 +1,120 @@ +package errutil + +import ( + "context" + "errors" + "net/http" + "net/http/httptest" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/target/goalert/ctxlock" + "github.com/target/goalert/permission" + "github.com/target/goalert/validation" +) + +func TestHTTPError_Nil(t *testing.T) { + rec := httptest.NewRecorder() + assert.False(t, HTTPError(context.Background(), rec, nil)) + assert.Equal(t, 200, rec.Code, "no response should be written for a nil error") +} + +// TestHTTPError_QueueFullAndTimeoutBothAnswer429 pins the app-wide 408->429 +// change: ctxlock.ErrTimeout is back-pressure, the same condition as +// ErrQueueFull, not a slow client, so both must be retryable (429) rather than +// 408 -- which SNS treats as a permanent failure. The bodies must still differ +// so a caller can tell "queue full" from "timed out waiting" apart even though +// the status code no longer does. +func TestHTTPError_QueueFullAndTimeoutBothAnswer429(t *testing.T) { + recQueueFull := httptest.NewRecorder() + assert.True(t, HTTPError(context.Background(), recQueueFull, ctxlock.ErrQueueFull)) + assert.Equal(t, http.StatusTooManyRequests, recQueueFull.Code) + + recTimeout := httptest.NewRecorder() + assert.True(t, HTTPError(context.Background(), recTimeout, ctxlock.ErrTimeout)) + assert.Equal(t, http.StatusTooManyRequests, recTimeout.Code) + + assert.NotEqual(t, recQueueFull.Body.String(), recTimeout.Body.String(), + "the two conditions must remain distinguishable by body even though the status code is now shared") +} + +func TestHTTPError_MaxBytesErrorIs413(t *testing.T) { + rec := httptest.NewRecorder() + err := &http.MaxBytesError{Limit: 1024} + assert.True(t, HTTPError(context.Background(), rec, err)) + assert.Equal(t, http.StatusRequestEntityTooLarge, rec.Code) + assert.Contains(t, rec.Body.String(), "1024") +} + +func TestHTTPError_PermissionErrors(t *testing.T) { + rec := httptest.NewRecorder() + assert.True(t, HTTPError(context.Background(), rec, permission.Unauthorized())) + assert.Equal(t, http.StatusUnauthorized, rec.Code) + + rec = httptest.NewRecorder() + assert.True(t, HTTPError(context.Background(), rec, permission.NewAccessDenied("nope"))) + assert.Equal(t, http.StatusForbidden, rec.Code) +} + +func TestHTTPError_ValidationErrorIs400(t *testing.T) { + rec := httptest.NewRecorder() + assert.True(t, HTTPError(context.Background(), rec, validation.NewFieldError("Foo", "bad"))) + assert.Equal(t, http.StatusBadRequest, rec.Code) +} + +func TestHTTPError_DeadlineExceededIs504(t *testing.T) { + rec := httptest.NewRecorder() + assert.True(t, HTTPError(context.Background(), rec, context.DeadlineExceeded)) + assert.Equal(t, http.StatusGatewayTimeout, rec.Code) +} + +// TestHTTPError_UnexpectedErrorIs500 and TestHTTPErrorRetry_UnexpectedErrorIs503 +// are the one behavioral difference between the two functions: an unclassified +// error is the only branch HTTPErrorRetry changes. +func TestHTTPError_UnexpectedErrorIs500(t *testing.T) { + rec := httptest.NewRecorder() + assert.True(t, HTTPError(context.Background(), rec, errors.New("boom"))) + assert.Equal(t, http.StatusInternalServerError, rec.Code) +} + +func TestHTTPErrorRetry_UnexpectedErrorIs503(t *testing.T) { + rec := httptest.NewRecorder() + assert.True(t, HTTPErrorRetry(context.Background(), rec, errors.New("boom"))) + assert.Equal(t, http.StatusServiceUnavailable, rec.Code) +} + +// TestHTTPErrorRetry_ClientErrorsUnchanged pins that HTTPErrorRetry only widens +// the unexpected-error branch -- every classified client error (permanent, by +// definition) must still answer exactly what HTTPError would, so a caller +// switching from HTTPError to HTTPErrorRetry cannot accidentally make SNS or +// Azure retry something that will never succeed. +func TestHTTPErrorRetry_ClientErrorsUnchanged(t *testing.T) { + cases := []struct { + name string + err error + code int + }{ + {"max bytes", &http.MaxBytesError{Limit: 10}, http.StatusRequestEntityTooLarge}, + {"queue full", ctxlock.ErrQueueFull, http.StatusTooManyRequests}, + {"ctxlock timeout", ctxlock.ErrTimeout, http.StatusTooManyRequests}, + {"unauthorized", permission.Unauthorized(), http.StatusUnauthorized}, + {"access denied", permission.NewAccessDenied("nope"), http.StatusForbidden}, + {"validation", validation.NewFieldError("Foo", "bad"), http.StatusBadRequest}, + {"deadline exceeded", context.DeadlineExceeded, http.StatusGatewayTimeout}, + } + + for _, tt := range cases { + t.Run(tt.name, func(t *testing.T) { + recA := httptest.NewRecorder() + require.True(t, HTTPError(context.Background(), recA, tt.err)) + + recB := httptest.NewRecorder() + require.True(t, HTTPErrorRetry(context.Background(), recB, tt.err)) + + assert.Equal(t, tt.code, recA.Code) + assert.Equal(t, recA.Code, recB.Code, "HTTPErrorRetry must not change a classified client error's status") + assert.Equal(t, recA.Body.String(), recB.Body.String(), "HTTPErrorRetry must not change a classified client error's body") + }) + } +} diff --git a/web/src/app/documentation/sections/IntegrationKeys.md b/web/src/app/documentation/sections/IntegrationKeys.md index d30efcfe75..182c473255 100644 --- a/web/src/app/documentation/sections/IntegrationKeys.md +++ b/web/src/app/documentation/sections/IntegrationKeys.md @@ -134,6 +134,69 @@ To trigger an alert using Prometheus Alertmanager, follow these steps: --- +## Amazon CloudWatch + +CloudWatch alarms reach GoAlert through an Amazon SNS topic. GoAlert confirms the SNS subscription itself and verifies the signature on every message, so no Lambda or forwarder is required. + +To trigger an alert from a CloudWatch alarm, follow these steps: + +1. Within GoAlert, on the Services page, select the service you want to process the alert. Under Integration Keys: + + - Key Name: Enter a name for the key. + - Key Type: Amazon CloudWatch + - Click Add Key. Copy the generated URL and keep it handy, as you'll need it in the next step. + +2. In the AWS console, open the SNS topic your alarms publish to and create a subscription: + + - Protocol: HTTPS + - Endpoint: Paste in the CloudWatch webhook URL you generated in step 1. + - Leave "Enable raw message delivery" **unchecked** — GoAlert needs the SNS envelope in order to verify the message signature. + - Click Create subscription. GoAlert completes the confirmation handshake automatically; the subscription should move from "Pending confirmation" to "Confirmed" within a few seconds. + +3. Ensure each CloudWatch alarm you want to page on has an alarm action publishing to that SNS topic. + +Notes: + +- The alarm name becomes the alert summary, and alerts are deduplicated on it, so a repeated alarm will not create a second alert while the first is still open. +- The `AlarmDescription` is appended to the alert details verbatim, which makes it a good place to put a runbook URL. +- An alarm moving to `OK` closes the matching alert. Transitions to `INSUFFICIENT_DATA` are ignored. +- Your GoAlert instance must be reachable from AWS over HTTPS for the subscription to confirm. + +--- + +## Azure Monitor + +Azure Monitor delivers alerts by having an **action group** POST a webhook. One GoAlert endpoint can serve any number of action groups across any number of subscriptions — the integration key in the URL selects which service the alert lands on. + +> **The webhook URL is a credential.** Unlike Amazon CloudWatch, Azure Monitor does not sign its webhook deliveries, so the integration key in the URL is the *only* thing protecting the endpoint. Anyone who obtains the URL can create arbitrary alerts on that service. Treat it with the same care as a password, and generate a new key if you believe it has been exposed. + +To trigger an alert from an Azure Monitor alert rule, follow these steps: + +1. Within GoAlert, on the Services page, select the service you want to process the alert. Under Integration Keys: + + - Key Name: Enter a name for the key. + - Key Type: Azure Monitor + - Click Add Key. Copy the generated URL and keep it handy, as you'll need it in the next step. + +2. In the Azure portal, open **Monitor → Alerts → Action groups** and select (or create) the action group your alert rules notify. Under **Actions**, add a **Webhook**: + + - Name: e.g. `GoAlert` + - URI: Paste in the Azure Monitor webhook URL you generated in step 1. + - **Enable the common alert schema: Yes** — this is required. GoAlert rejects deliveries that use Azure's legacy schema, with an error naming this setting. + - Click Save. + +3. Ensure the alert rules you want to page on reference that action group. + +Notes: + +- An action group can hold more than one webhook receiver, so you can add GoAlert alongside an existing destination and have both fire. That makes rollout additive, and rollback is deleting one receiver. +- The alert rule name becomes the alert summary, and alerts are deduplicated on Azure's alert ID, so a redelivery of the same firing will not create a second alert. +- Alert rules with **auto-mitigate** enabled (the default for metric alerts) send a `Resolved` notification when the condition clears, which closes the corresponding GoAlert alert automatically. +- Setting **custom properties** on an alert rule surfaces them in the alert details, which makes them a good place to put a runbook URL. +- Metric and log alerts render their condition, threshold and observed value. Log alerts also include the query and a link to the search results. Any other signal type still produces a usable alert built from the fields common to every Azure payload. + +--- + ## Email It is possible to create an Email integration key from the Service Details page. This will generate a unique email address that can be used for creating alerts. diff --git a/web/src/app/util/safeURL.test.ts b/web/src/app/util/safeURL.test.ts index d74f790aba..4b119ad15c 100644 --- a/web/src/app/util/safeURL.test.ts +++ b/web/src/app/util/safeURL.test.ts @@ -87,4 +87,12 @@ describe('safeURL', () => { '[https://example.com/query?foo=1&bar=2](https://example.com/query?foo=1&bar=3)', // bar doesn't match ], }) + + checkIt('should match when label and url share percent-encoding', { + true: [ + '[https://example.com/path%20with%20spaces](https://example.com/path%20with%20spaces)', + '[https://example.com/a%20b#frag](https://example.com/a%20b#frag)', + ], + false: [], + }) }) diff --git a/web/src/app/util/safeURL.ts b/web/src/app/util/safeURL.ts index d9841c4e62..09ed410460 100644 --- a/web/src/app/util/safeURL.ts +++ b/web/src/app/util/safeURL.ts @@ -5,7 +5,7 @@ import _ from 'lodash' // It tries to determine if the label is misleading. export function safeURL(_url: string, _label: string): boolean { const url = decodeURI(_.unescape(_url)) - const label = _.unescape(_label) + const label = decodeURI(_.unescape(_label)) if (url.startsWith('mailto:')) { const email = url.slice(7) diff --git a/web/src/schema.d.ts b/web/src/schema.d.ts index 16f77d46a9..06a3fe5500 100644 --- a/web/src/schema.d.ts +++ b/web/src/schema.d.ts @@ -126,6 +126,11 @@ export interface AlertsByStatus { unacked: number } +export interface AppendAlertDetailsInput { + alertID: number + text: string +} + export interface AuthSubject { providerID: string subjectID: string @@ -651,6 +656,8 @@ export interface IntegrationKeySearchOptions { } export type IntegrationKeyType = + | 'azureMonitor' + | 'cloudwatch' | 'email' | 'generic' | 'grafana' @@ -760,6 +767,7 @@ export interface MessageStatusHistory { export interface Mutation { addAuthSubject: boolean + appendAlertDetails: boolean clearTemporarySchedules: boolean closeMatchingAlert: boolean createAlert?: null | Alert @@ -791,6 +799,7 @@ export interface Mutation { reEncryptKeyringsAndConfig: boolean sendContactMethodVerification: boolean sendSignal: boolean + setAlertMetadata: boolean setAlertNoiseReason: boolean setConfig: boolean setFavorite: boolean @@ -1123,6 +1132,11 @@ export interface ServiceSearchOptions { search?: null | string } +export interface SetAlertMetadataInput { + alertID: number + meta: AlertMetadataInput[] +} + export interface SetAlertNoiseReasonInput { alertID: number noiseReason: string