fix(s52): reject scheme-less webhook URL fail-fast in NotifierConfig

This commit is contained in:
Kjell Tore Guttormsen 2026-07-17 03:14:36 +02:00
commit 8c252c1064
3 changed files with 25 additions and 2 deletions

View file

@ -125,7 +125,8 @@ territory for a deployer, with the seam named:
point and is fail-closed behind an explicit per-run `allow_egress=True` opt-in (a code kwarg,
never a config field — mirroring the ingest layer's `allow_network`). SSRF guards, HMAC
signing, and auth headers remain deployer-owned extension points on the injectable
`WebhookPost` transport seam.
`WebhookPost` transport seam. Webhook URLs must start with `http://` or `https://` — a
scheme-less URL is rejected fail-fast at config construction.
- **U12 — checkpointing / crash-survival of a run.** A run either completes or is re-run; the
async verdict inbox (step 7) is the resumable boundary, not intra-run state.
- **U14 — OpenTelemetry / observability.** Provenance stamping is the audit trail the core

View file

@ -23,7 +23,7 @@ from typing import TYPE_CHECKING, Any, Protocol, TextIO, runtime_checkable
from urllib.error import URLError
from urllib.request import Request, urlopen
from pydantic import BaseModel, model_validator
from pydantic import BaseModel, ConfigDict, model_validator
if TYPE_CHECKING: # verdicts imports agent_framework — keep it out of the runtime import graph
from portfolio_optimiser.verdicts import Verdict
@ -141,6 +141,10 @@ class NotifierConfig(BaseModel):
``allow_egress`` field egress opt-in is a code-level factory kwarg, so config alone can
never grant it (mirrors ingest's ``allow_network`` discipline)."""
# the url field is secret-bearing (webhook URLs embed the receiver secret), so
# validation errors must not echo input (pydantic would render input_value='…secret')
model_config = ConfigDict(hide_input_in_errors=True)
type: str
url: str | None = None
path: str | None = None
@ -149,6 +153,8 @@ class NotifierConfig(BaseModel):
def _required_fields_by_type(self) -> NotifierConfig:
if self.type == "webhook" and not self.url:
raise ValueError("webhook notifier config requires a non-empty url")
if self.type == "webhook" and self.url and not self.url.startswith(("http://", "https://")):
raise ValueError("webhook notifier url must start with http:// or https://")
if self.type == "file" and not self.path:
raise ValueError("file notifier config requires a non-empty path")
return self

View file

@ -131,6 +131,22 @@ def test_config_fail_fast() -> None:
assert isinstance(notifier, WebhookNotifier)
def test_webhook_config_rejects_schemeless_url() -> None:
"""Fail-fast scheme gate (S5.2 remediation): a scheme-less webhook url is rejected at config
construction BEFORE it can reach ``_urllib_post``, whose bare ``ValueError`` would carry the
secret-bearing URL verbatim. The str/repr absence assertions guard ``hide_input_in_errors``:
without that flag pydantic v2 echoes ``input_value='…secret-token'`` in the error display
(remove the flag RED here)."""
with pytest.raises(ValidationError) as excinfo:
NotifierConfig(type="webhook", url="hooks.example.test/T000/B000/secret-token")
assert "secret-token" not in str(excinfo.value)
assert "secret-token" not in repr(excinfo.value)
# http:// and https:// stay accepted (the gate rejects ONLY scheme-less pastes)
NotifierConfig(type="webhook", url="https://hooks.example.test/x")
NotifierConfig(type="webhook", url="http://hooks.example.test/x")
def test_public_contract_exported() -> None:
"""The declared B11 contract is public authoring API: importable from the package top and
listed in ``__all__`` (the brief-permitted export path run.py's seam stays byte-intact)."""