Fixes#1681Fixes#1682Fixes#1683
Contract-Neutral: Notification grouping initialization and alert-config propagation do not alter the broadly referenced agent-lifecycle or storage-recovery contracts; primary alerts, notifications, API, and monitoring contracts and regression proofs are updated.
golangci-lint had accumulated 12 findings since 5abb2d8f4. All fixed with
real dedup (no nolint suppressions) and the repo's existing errcheck idioms:
- dupl internal/monitoring: docker/host identity-conflict trackers were
structural clones; extracted a shared identityFlapTracker core with a
domain-neutral identityConflict result. Per-domain files now hold only
the window const and the model translation. Tracker-behavior tests
consolidated into identity_flap_tracker_test.go; Monitor-level
translation and Apply*Report integration tests remain per domain.
- dupl internal/api/router.go: VM/container workload chart loops shared a
16-line live-fallback block; extracted guestChartSeriesWithLiveFallback
over a guestLiveMetricsView interface both views satisfy.
- dupl internal/storagehealth/risk.go: SMART attribute copying extracted
into applySMARTAttributes shared by both assessors (same
*models.SMARTAttributes type on both inputs).
- errcheck pkg/audit/sqlite_logger.go: three defer tx.Rollback() sites
now use the repo-wide defer func() { _ = tx.Rollback() }() idiom.
- errcheck telemetry/notifications tests: send() errors now fail the
test; queue.Stop() uses the package's _ = idiom.
Full test suites pass for all six touched packages.
Contract-Neutral: lint-hygiene restoration: dupl dedup (identical logic extracted to shared helpers) and errcheck idiom fixes; no public-contract or behavioral delta
A third pass on partially covered functions, led by the alert evaluation
predicates where a wrong arm means a missed or spurious alert.
- internal/alerts/specs: matches 47.9 to 100, and all six matches helpers
(severity threshold, change threshold, baseline anomaly, health assessment,
posture threshold, and the severity latch) from 50 to 75 percent up to 100.
Each threshold is pinned at, just below and just above, and the latch arm is
exercised both latched and unlatched with concrete verdicts.
- pkg/audit: exportCSV 76 to 88 with commas, quotes and newlines in the detail
field asserted through a parsed round-trip; NewSigner error arms, both
IsPersistent predicates and VerifySignature against a tampered payload and a
wrong key.
- internal/agentupdate: retryBackoffDelay, sleepWithContext, Snapshot and
writeSelfTestTokenFile to 100, the token file exercised under t.TempDir
including the unwritable-directory arm.
- internal/notifications: writeMultipartBodyPart and alertNodeDisplay to 100,
attachment handling to 69, all asserted on the produced MIME text. No test
opens a network or SMTP connection.
- internal/unifiedresources: the three pure action-dispatch helpers to 100.
- internal/alerts/config: CanonicalResourceTypeKeys 34.3 to 78.4.
Five targets deliberately did not move and are recorded rather than faked:
the error arms of writeEmailThreadingHeaders, buildMultipartEmailMessage and
copyWebhookConfig are unreachable because those functions write only into a
local bytes.Buffer, which never errors; exportJSON's only gap is a
json.MarshalIndent failure that its event struct cannot produce; and
verifyBinaryMagic's remaining gap is a deferred close-error handler.
No source file is modified. Adversarial review returned no rejects and flagged
seven near-duplicate subtests; all were removed and every target function
re-measured at an identical percentage.
PULSE_ALLOW_CONTRACT_NEUTRAL_COMMIT=test-only branch coverage, no source or contract change
f744e0700 moved webhook URL redaction onto the canonical
RedactWebhookURLSecrets helper and applied it at five log sites, but missed
two. checkWebhookRateLimit logged the raw webhookURL, and the enhanced
sender logged the raw webhook.URL on the same event. The enhanced path is
the one Gotify and other token-in-URL destinations actually take, and a
rate-limit drop is the event most likely to repeat for a misconfigured
destination, so the token reached the logs on every retry.
Both sites now redact. The reason the sweep missed them is that no test
asserted log content, so the new proof captures zerolog output and fails on
the raw token rather than reading the call sites, which a source scan cannot
do reliably.
Regression coverage: TestWebhookRateLimitLogsRedactURLSecrets, verified to
fail against the unredacted call site with the token visible in the captured
log line.
Same defect class as 64fb3d198 (pkg/securityutil): the SSRF-pinned webhook
dialer resolved the host and dialed only the first permitted IP, so a host
resolving to ::1 ahead of 127.0.0.1 while the receiver listens on one
loopback family — or a multi-A-record host with a dead leading address —
got a hard connection failure even though curl works. The dialer now tries
each permitted resolved IP in resolution order; every candidate still
passes the private-IP/allowlist validation, so rebinding protection is
unchanged.
Evaluated migrating onto securityutil.NewRestrictedOutboundHTTPClient and
rejected a straight swap: the webhook client's isPrivateIP blocks a wider
range set (CGNAT, benchmarking, TEST-NET), its redirect policy allows
re-validated cross-origin hops where securityutil enforces same-origin,
Proxy is deliberately nil, and the private-IP allowlist is a live per-CIDR
runtime hook — a swap would loosen the SSRF policy and change behavior.
Adds a resolver test seam on NotificationManager and a regression test
pinning the ::1-first fallback; notifications contract records the
multi-IP dial obligation.
Closes chip task_5e91afd9.
Alert start times are stored in UTC and the email templates rendered
them with no zone conversion or label, so the Started line read as a
local clock while showing UTC (#1582). Convert to the server's local
zone and include the zone name.
An alert that resolved while its firing notification was still in the
grouping window or waiting in the persistent queue (alert delay pushes
activation close to resolution; quiet-hours replay defers delivery)
produced a recovery-only notification: CancelAlert dropped the queued
firing, but LastNotified had been set optimistically at dispatch, so
the resolved-notification gate believed the firing had been sent.
CancelAlert now reports whether it cancelled a firing notification that
had not been delivered (grouping window entries and pending queue rows;
mid-send rows are excluded because their delivery may still complete),
and handleAlertResolved suppresses the recovery in that case. This also
covers the quiet-hours replay bypass: a recovery only follows a deferred
firing if the replay was actually delivered.
Addresses #1553
Every email covering a single alert now carries In-Reply-To and
References headers set to a deterministic incident thread ID derived
from the alert ID and firing start time, so mail clients thread the
firing, re-notification, and resolved emails of one incident together.
Message-ID stays unique per send because re-notified incidents send
multiple emails and some providers de-duplicate on Message-ID. Grouped
emails skip threading since firing and resolved batches rarely contain
the same alert set.
Addresses #1543 (discussion)
The ntfy branch of prepareEnhancedWebhookExecution parsed the payload
template without templateFuncMap(), so the built-in ntfy preset (which
uses {{.Type | title}}) failed to parse and Test sends returned HTTP
400 with "function \"title\" not defined". Register the func map on
that parse, matching the generic webhook path, and add a regression
test that renders the actual built-in ntfy preset template.
Addresses #1549
sendViaProviderWithAddresses mutated the shared e.config.Username for
provider-specific defaults (SendGrid, Postmark, SparkPost, Resend).
If concurrent goroutines sent email simultaneously, this was a data
race on the config struct.
Move the resolution into negotiateAuth via a local variable
(resolveProviderUsername helper) so the shared config is never mutated.
Adds TestContract_MetadataGetPayloadsUseZeroRecordsInsteadOf404: empty
guest/docker metadata maps must serialize as {} (never null) and a
missing resource must return a 200 zero record echoing the requested ID
(never a 404). This is the proof companion to the
metadata_handlers_shared.go consolidation in the previous commit — it
was authored with that change but lost to a shared-index race at commit
time.
Alert webhook payloads now carry the tenant that fired them, so MSP/PSA
receivers (ConnectWise and similar) can route tickets by client without
inferring the tenant from which webhook endpoint fired.
- WebhookPayloadData gains TenantID/TenantName, exposed to custom
templates as {{.TenantID}}/{{.TenantName}}.
- Defaults come from PULSE_TENANT_ID/PULSE_TENANT_NAME (already injected
into provider-hosted client runtimes; name falls back to ID).
- Shared-process multi-tenant orgs override via a lazy org-backed
resolver wired in MultiTenantMonitor.GetMonitor, so display-name
renames are picked up without restart.
- Generic service template emits a tenant block when identity is set,
omits it otherwise; single-tenant payloads are unchanged.
- Notifications and monitoring subsystem contracts updated with the
tenant-identity ownership boundary; guardrail test pins the org
wiring.
Back-port v5 fixes 5997fd81f and 0a7b93a84 to v6:
- GetWebhooks list response now includes the configured mention so the UI
shows it after reload instead of blanking it.
- sendResolvedWebhook now assigns data.Mention (v6 set it for grouped/
firing webhooks but dropped it on resolved), and the Discord/Slack/Teams/
Mattermost ResolvedPayloadTemplate strings gained {{if .Mention}} guards.
Without these, a configured @everyone/@channel was silently omitted from
resolved/cleared notifications. Adds list-API and per-service resolved
mention regression tests.
Dead-code sweep. Functions flagged unreachable by golang.org/x/tools/cmd/deadcode
and confirmed unused across pulse, pulse-enterprise, pulse-pro and pulse-mobile by
adversarial cross-repo verification. Cross-module reachability was checked
explicitly (only pkg/ exported symbols are importable by other modules; internal/
packages and _test.go files are not). go build, go vet and test-compile all pass.
Two fixes for missing recovery/resolved notifications:
1. API config PUT handler now preserves notifyOnResolve when the client
omits it from the request body. Go decodes a missing bool as false,
which silently disabled recovery notifications on older clients.
2. CancelAlert now always cleans up the cooldown record even when the
alert has already left the pending buffer, preventing stale cooldown
entries from suppressing future alert cycles.
rcourtman