Skip to content

Notifications: escalation tiers notify by default via webhook (ADR-0059 Amendment 1) - #100

Merged
gterdem merged 2 commits into
mainfrom
issue-74-notify-v2
Jul 17, 2026
Merged

gterdem merged 2 commits into
mainfrom
issue-74-notify-v2

Conversation

@gterdem

@gterdem gterdem commented Jul 17, 2026

Copy link
Copy Markdown
Owner

Summary

Implements issue #74 / ADR-0059 Amendment 1: escalation-tier ("HIGH ALERT") actors now
notify by default via webhook, and the notifier fires only on a genuine
alert-worthiness state transition instead of on every ingest-triggered
re-evaluation.

  • RuntimeConfig.notify_on_auto_escalate default flips False -> True (A1.1).
    Existing persisted configs keep their stored value — only the default for
    absent values changed (no migration).
  • New: escalation/transition.py — NotifyTransitionTracker, a small
    in-memory per-actor cadence gate. webhook_notifier.check_and_alert now
    fires only when an actor's state transitions (enters the escalation tiers
    from no-tier, moves to a louder tier, or first crosses the severity band) —
    never on repeated re-evaluation of an unchanged state. This closes the
    stock-vs-flow gap called out in the ADR: without it, an actor that stays in
    the triage queue would re-notify on every POST /logs-triggered analysis
    (the Maintainer's 50-events/min brute-force case would be ~50 webhook posts
    a minute instead of one at the crossing).
  • Fixes the pre-existing band-axis repeat-fire bug too (not just the new
    tier path) — a CRITICAL-band actor no longer re-notifies on every analysis.
  • The notification model stays deliberately simple, per ADR-0059 A1.2:
    on/off (webhook configured or not) + the existing threat-level selector
    (alert_threshold). No per-state (CRITICAL/HIGH ALERT/INFORM) notification
    preferences and no OS/desktop push were added — both explicitly deferred.
  • tier=None (observed stratum) still never fires via the tier axis, at any
    config — but the band axis stays live for an observed actor that reaches
    the operator's threshold (A1.2 correction; not a defect).
  • Webhook safety: reuses the existing _assert_webhook_url_safe SSRF/
    egress validator on RuntimeConfig.webhook_url — no new URL validator was
    added.
  • Updates NotifyOnAutoEscalateToggle.tsx help copy and
    docs/escalation-and-triage-model.md to cite ADR-0059 Amendment 1 (default
    ON; quiet chat is preserved by the ADR-0067 assertion gate, not by the
    toggle) instead of the superseded D3 default text.
  • tests/golden/fixtures/expected_scores.json is byte-identical — sha256
    fe4787643955c920e934e3789c79f741cd8c8cde6b2adbc6540b66ff3743f31f. This
    is a notification/config change only; no scoring/escalation-decision logic
    was touched.

Security review note: this PR touches an egress/webhook path (default
posture flip + payload cadence). It reuses the existing SSRF validator and
does not change what the payload contains, but please give it a security
pass given the egress-by-default change.

Scope note (found during implementation, not fixed here): WebhookNotifier
is not currently wired into the live Pipeline in either firewatch serve or
firewatch run (_pipeline_factory.py never constructs one) — so in today's
main, no webhook notification is dispatched end-to-end regardless of config.
This is a pre-existing gap outside #74's stated scope ("this issue changes one
default and its copy, no new machinery"); flagging for a follow-up issue since
the default flip has no live effect until that wiring exists.

Supersedes PR #97

This PR is a clean-history rebuild of #97 (branch
issue-74-notify-by-default), which is being closed. #97's content is
identical except for one fix: a test fixture in
test_issue_74_notify_transition.py used the real, routable IP 1.1.1.1
(and 2.2.2.2) as arbitrary per-actor identifiers, which the public-ipv4
gitleaks rule correctly flagged. Because the violation was in an already-
pushed historical commit, a fix-on-top could not clear a full-history scan
(git log-scoped gitleaks still re-flags the old commit), and force-push is
blocked in this workflow — so the branch was rebuilt from scratch off current
main with the final file contents, one clean commit, and the identifiers
swapped to RFC 5737 TEST-NET-3 addresses (203.0.113.10, 203.0.113.11).
This is a pure identifier rename with no behavior change (verified: all 98
backend tests touching this code pass unchanged).

Acceptance criteria checklist (issue #74)

  • RuntimeConfig.notify_on_auto_escalate defaults to True; existing
    persisted configs keep their stored value (no migration).
  • Transition semantics: fires on entering Tier 1/2 from no-tier, moving to
    a louder tier, or first crossing the band threshold; not on unchanged state.
  • Must-NOT: an actor continuously in the queue / continuously above the
    band threshold does not re-notify on every ingest-triggered analysis
    (50/min brute-force test: exactly one notification).
  • Transition semantics cover both axes (band repeat-fire bug fixed too).
  • Falling then re-transitioning into a tier fires again ("left and came
    back" is a new transition, not a duplicate) — both axes.
  • Must-NOT: tier=None never notifies via the tier axis, under any
    config; band axis stays live for an observed actor (test asserts this, not
    a blanket "never notifies").
  • Must-NOT: no OS/desktop push path added.
  • Must-NOT: alert_threshold default (CRITICAL) and the band-axis gate
    unchanged.
  • UI help copy updated, citing ADR-0059 Amendment 1.
  • docs/escalation-and-triage-model.md notification text updated.
  • Golden untouched (sha confirmed above).

Test plan

  • packages/firewatch-core/tests/test_issue_74_notify_transition.py (new,
    32 tests): NotifyTransitionTracker unit tests (both axes, louder/quieter
    moves, leave-and-return, reset), check_and_alert cadence integration
    (brute-force falsifier, band-axis repeat-fire fix, left-and-came-back,
    observed-stratum tier/band split, threshold selector gating), and the
    webhook SSRF-safety reuse assertions. Actor identifiers use RFC 5737
    TEST-NET-3 (203.0.113.0/24), never real IPs.
  • packages/firewatch-core/tests/test_issue_661_worthiness_and_notify.py
    (edited): default-True assertions replace the old default-False ones;
    toggle-explicit-off / toggle-on paths unchanged.
  • packages/firewatch-api/tests/test_config_routes.py (edited): GET default
    is now true; added an explicit opt-out-to-false round-trip test
    alongside the existing opt-in-to-true one.
  • Frontend: NotificationsPanel.test.tsx / SettingsRoute.test.tsx /
    AlertingPolicyPanel.test.tsx updated for the new default; full frontend
    suite green (4305 tests, npm run typecheck / lint / test).

Gates

bash scripts/gates-backend.sh — green:

==> tree:   /home/galip/projects/firewatch/.claude/worktrees/agent-ab1619f73ff00e45d
==> branch: issue-74-notify-v2
==> HEAD:   872eb4e
================ 4417 passed, 1 skipped, 36 warnings in 22.18s =================
✅ backend gates passed

Golden sha256: fe4787643955c920e934e3789c79f741cd8c8cde6b2adbc6540b66ff3743f31f
(matches the required value byte-for-byte).

gitleaks git -c .gitleaks.toml --no-banner --log-opts="origin/main..HEAD"
on this branch: no leaks found (1 commit scanned).

Frontend gates: npm run typecheck / npm run lint / npm run test -- --run
— all green (177 test files, 4305 tests passed).

Closes #74

…59 Amendment 1) (#74)

Implements issue #74 / ADR-0059 Amendment 1: escalation-tier (HIGH ALERT)
actors now notify by default via webhook, and the notifier fires only on a
genuine alert-worthiness state transition instead of on every
ingest-triggered re-evaluation.

- RuntimeConfig.notify_on_auto_escalate default flips False -> True (A1.1).
  Existing persisted configs keep their stored value -- only the default for
  absent values changed (no migration).
- New: escalation/transition.py -- NotifyTransitionTracker, a small
  in-memory per-actor cadence gate. webhook_notifier.check_and_alert now
  fires only when an actor's state transitions (enters the escalation tiers
  from no-tier, moves to a louder tier, or first crosses the severity band)
  -- never on repeated re-evaluation of an unchanged state.
- Fixes the pre-existing band-axis repeat-fire bug too (not just the new
  tier path).
- tier=None (observed stratum) still never fires via the tier axis, at any
  config -- but the band axis stays live for an observed actor that reaches
  the operator's threshold (A1.2 correction; not a defect).
- Webhook safety: reuses the existing _assert_webhook_url_safe SSRF/egress
  validator -- no new URL validator was added.
- Updates NotifyOnAutoEscalateToggle.tsx help copy and
  docs/escalation-and-triage-model.md to cite ADR-0059 Amendment 1.

Scope note (found during implementation, not fixed here): WebhookNotifier
is not currently wired into the live Pipeline in either 'firewatch serve' or
'firewatch run' (_pipeline_factory.py never constructs one) -- so in today's
main, no webhook notification is dispatched end-to-end regardless of config.
This is a pre-existing gap outside #74's stated scope; flagging for a
follow-up issue since the default flip has no live effect until that wiring
exists.

This is a clean-history rebuild of PR #97 (branch issue-74-notify-by-default),
which contained a real routable IP (1.1.1.1) in a test fixture flagged by the
gitleaks public-ipv4 rule. All actor identifiers in
test_issue_74_notify_transition.py now use RFC 5737 TEST-NET-3
(203.0.113.0/24) documentation addresses.

Closes #74
# Conflicts:
#	packages/firewatch-core/src/firewatch_core/escalation/__init__.py
@gterdem
gterdem merged commit 3ae71d4 into main Jul 17, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Notifications: escalation tiers notify by default via webhook (ADR-0059 Amendment 1)

1 participant