From e89bca00a0b33807e538ea884713b8b9a0806b3c Mon Sep 17 00:00:00 2001 From: "pulse-triage[bot]" <249995291+pulse-triage[bot]@users.noreply.github.com> Date: Thu, 27 Aug 2026 02:13:42 +0100 Subject: [PATCH] security(alerts): decouple event query allocation Change-source: pulse-maintainer --- .../v6/internal/subsystems/alerts.md | 6 ++++++ internal/alerts/alerts_test.go | 10 ++++++++++ internal/alerts/eventlog/eventlog.go | 7 ++++++- internal/alerts/eventlog/eventlog_test.go | 15 +++++++++++++++ 4 files changed, 37 insertions(+), 1 deletion(-) diff --git a/docs/release-control/v6/internal/subsystems/alerts.md b/docs/release-control/v6/internal/subsystems/alerts.md index 6649b6aa2..c6f1bccae 100644 --- a/docs/release-control/v6/internal/subsystems/alerts.md +++ b/docs/release-control/v6/internal/subsystems/alerts.md @@ -959,6 +959,12 @@ shared metric activation records one fired event when its pending incident becomes firing. Repeated firing observations and persisted-alert restore must not append another activation event, and the active-alert storage funnel must never invent one. +Event-log reads accept a caller limit but normalize it to the store's bounded +maximum before it reaches SQLite. Result-slice allocation is independent of +that caller value: a request-provided limit is a row-count preference, never a +memory-allocation hint. This remains defense in depth even when an authenticated +API handler validates the query parameter, because non-HTTP manager callers use +the same store boundary. The same dispatch policy owns firing-notification evidence on active alerts: any alert that passes notification suppression and enters the fired callback fan-out must carry `LastNotified` before the callback clone is emitted. Resolved diff --git a/internal/alerts/alerts_test.go b/internal/alerts/alerts_test.go index 0fb3e2b1c..a2b809b21 100644 --- a/internal/alerts/alerts_test.go +++ b/internal/alerts/alerts_test.go @@ -50,6 +50,16 @@ func TestAlertEventLogDoesNotInventFiringFromActiveStorage(t *testing.T) { } } +func TestAlertEventLogBoundsRequestDerivedLimit(t *testing.T) { + manager := newEventLogManager(t) + manager.recordAlertEvent(eventlog.TypeResolved, nil, "bounded-event", "resolved", "resolved", nil) + + events := queryAlertEvents(t, manager, eventlog.Filter{Limit: math.MaxInt}) + if len(events) != 1 || events[0].AlertID != "bounded-event" { + t.Fatalf("oversized-limit query = %+v, want the bounded event", events) + } +} + func TestGuestAlertIncludesTagsForNotificationRouting(t *testing.T) { m := newTestManager(t) m.ClearActiveAlerts() diff --git a/internal/alerts/eventlog/eventlog.go b/internal/alerts/eventlog/eventlog.go index 558d14a59..a3bb9779f 100644 --- a/internal/alerts/eventlog/eventlog.go +++ b/internal/alerts/eventlog/eventlog.go @@ -376,7 +376,12 @@ func (s *Store) Query(filter Filter) ([]Event, error) { } defer rows.Close() - events := make([]Event, 0, limit) + // Do not use the request-derived limit as an allocation hint. The SQL + // query is capped above, but keeping the result slice allocation independent + // of caller input makes that memory-safety boundary explicit and prevents a + // future query refactor from turning an oversized limit into an eager + // allocation. + events := make([]Event, 0, defaultQueryLimit) for rows.Next() { var event Event var occurredAt, details string diff --git a/internal/alerts/eventlog/eventlog_test.go b/internal/alerts/eventlog/eventlog_test.go index 91d9e226f..bac63687e 100644 --- a/internal/alerts/eventlog/eventlog_test.go +++ b/internal/alerts/eventlog/eventlog_test.go @@ -1,6 +1,7 @@ package eventlog import ( + "math" "testing" "time" ) @@ -99,6 +100,20 @@ func TestQueryLimitCaps(t *testing.T) { } } +func TestQueryOversizedLimitDoesNotControlAllocation(t *testing.T) { + store := newTestStore(t) + store.Append(Event{OccurredAt: time.Now(), Type: TypeResolved, AlertID: "a1"}) + store.Flush() + + events, err := store.Query(Filter{Limit: math.MaxInt}) + if err != nil { + t.Fatalf("query with oversized limit: %v", err) + } + if len(events) != 1 || events[0].AlertID != "a1" { + t.Fatalf("events = %+v, want the one stored event", events) + } +} + func TestNilStoreIsSafe(t *testing.T) { var store *Store store.Append(Event{Type: TypeResolved, AlertID: "a1"})