Skip to content

The notification channel can never be approval-gated, and the changelog does not say so #14537

Open

Description

@mrveiss

What is true

Every notification channel passes require_approval=False to the egress governor, hardcoded:

  • services/notification_service.py — the email and webhook seams
  • the Telegram notification path, via TelegramBotService.send_message(require_approval=False)

The reasoning is sound and documented in place: these carry WORKFLOW_FAILED, SERVICE_FAILED and APPROVAL_NEEDED. Gating APPROVAL_NEEDED behind the approval system it exists to serve would deadlock it, and gating the outage alerts would silence them exactly when they matter.

Why it still needs recording

EgressGovernor._decide returns allowed=True unconditionally when require_approval=False. So arming AUTOBOT_GATEWAY_REQUIRE_OUTBOUND_APPROVAL platform-wide can never gate this channel family — not "not yet", but structurally never, for as long as the flag is hardcoded.

That is a real ceiling on the control, and nothing states it outside a code comment.

Two consequences worth deciding on:

  1. The changelog overstates the coverage. changelog/unreleased/14270-live-egress-seams.md says sends are "recorded and can be gated; operational alerts are audited but never blocked". After security(integrations): govern outbound sends on the live channel seams (#14270) #14334 the seams do honour an explicit denial, so "never blocked" is now imprecise in one direction while "can be gated" is imprecise in the other for this family. The entry should say plainly: recorded always, gateable by policy, never gateable by approval.
  2. A per-event-type decision may be the right shape. APPROVAL_NEEDED genuinely cannot be approval-gated. WORKFLOW_FAILED arguably could be, on a channel an operator has restricted. A blanket flag forecloses that distinction.

Scope

  • Decide whether the exemption stays blanket or becomes per-event-type
  • Correct the changelog entry to match what the code does
  • Whichever is chosen, assert it — a test that pins APPROVAL_NEEDED can never be approval-gated, so a later refactor cannot deadlock alerting

Provenance

Found by code review of #14334 (#14270), and independently flagged by a background security review of the same commit as a fail-open gate. The verdict-discarding half was fixed in #14334; this is the half that is a deliberate design ceiling rather than a defect.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions