From 638236113d51618aaf5e409892ec8308c7a4079e Mon Sep 17 00:00:00 2001 From: anti Date: Fri, 24 Apr 2026 15:53:30 -0400 Subject: [PATCH] feat(webhooks): non-blocking http:// warning + WH-03 accepted risk MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit WebhookResponse now carries a `warnings: list[str]` field. When the subscription's URL starts with http://, an `insecure_url` advisory is surfaced on every GET/CREATE without blocking the request. HMAC still detects tampering regardless of transport — only read-confidentiality is lost over plaintext — and test/dev environments without TLS stay usable. Matches the operator-trust posture already established by DA-06 (admin-on-admin protection is out of scope). The alternative — hard rejection at admin time — was considered and declined; warning-plus- visibility is the right shape. THREAT_MODEL WH-03 accepted risk registered; revisit triggers are multi-admin delegation, a regulated customer, or an operator ticket asking for a DECNET_WEBHOOK_REQUIRE_HTTPS enforcement knob. --- decnet/web/db/models/webhooks.py | 34 ++++++++++++++++++++++++++--- development/THREAT_MODEL.md | 6 ++++-- tests/api/webhooks/test_crud.py | 37 ++++++++++++++++++++++++++++++++ 3 files changed, 72 insertions(+), 5 deletions(-) diff --git a/decnet/web/db/models/webhooks.py b/decnet/web/db/models/webhooks.py index 49480ef5..a3a7a1cc 100644 --- a/decnet/web/db/models/webhooks.py +++ b/decnet/web/db/models/webhooks.py @@ -83,7 +83,13 @@ class WebhookUpdateRequest(BaseModel): class WebhookResponse(BaseModel): - """Public shape — deliberately omits `secret`.""" + """Public shape — deliberately omits `secret`. + + The `warnings` field carries non-blocking advisories about the + subscription's configuration — e.g. an `http://` URL is fine but + surfaces a warning so the operator knows the event body is + plaintext on the wire. Empty list when nothing is worth flagging. + """ uuid: str name: str @@ -96,6 +102,7 @@ class WebhookResponse(BaseModel): last_error: Optional[str] = None created_at: datetime updated_at: datetime + warnings: List[str] = PydanticField(default_factory=list) class WebhookCreateResponse(WebhookResponse): @@ -110,11 +117,31 @@ class WebhookTestResponse(BaseModel): error: Optional[str] = None +def _compute_warnings(url: str) -> List[str]: + """Non-blocking advisories about a subscription's configuration. + + The HMAC signature detects tampering regardless of transport, but an + on-path attacker can still *read* the event body over plaintext HTTP. + We surface the warning and let the admin decide — matches DECNET's + operator-trust posture (see THREAT_MODEL WH-03). + """ + out: List[str] = [] + lower = (url or "").lower() + if lower.startswith("http://"): + out.append( + "insecure_url: URL uses http://. Event bodies (including " + "payload fields) traverse the wire in plaintext; HMAC still " + "detects tampering but anyone on-path can read the event. " + "Use https:// in production." + ) + return out + + def _row_to_response_dict(row: dict[str, Any]) -> dict[str, Any]: """Normalize a DB row into the WebhookResponse dict shape. - Used by the CRUD router to decode `topic_patterns` JSON and drop the - `secret` column before returning to the client. + Used by the CRUD router to decode `topic_patterns` JSON, drop the + `secret` column, and compute any configuration warnings. """ out = dict(row) raw = out.pop("topic_patterns", "[]") @@ -123,4 +150,5 @@ def _row_to_response_dict(row: dict[str, Any]) -> dict[str, Any]: except (ValueError, TypeError): out["topic_patterns"] = [] out.pop("secret", None) + out["warnings"] = _compute_warnings(out.get("url", "")) return out diff --git a/development/THREAT_MODEL.md b/development/THREAT_MODEL.md index 9e55cd01..0783e1f5 100644 --- a/development/THREAT_MODEL.md +++ b/development/THREAT_MODEL.md @@ -339,7 +339,7 @@ regardless of component: | Component | ID | Summary | |-----------|----|---------| | Dashboard↔API | DA-01..DA-09 | See component section. | -| DECNET↔Webhook destination | WH-01..WH-02 | See component section. | +| DECNET↔Webhook destination | WH-01..WH-03 | See component section. | --- @@ -400,6 +400,7 @@ the receiver (Shuffle→Slack, TheHive→Cortex, …). |----|--------|--------------|--------------| | WH-01 | Webhook secret + URL stored plaintext in the DB | Matches the existing pre-v1 posture (JWT secret is env-sourced; there's no operator expectation that DB-at-rest is encrypted). Encrypting one column in isolation invents a KEK lifecycle we don't have. | Comprehensive DB-at-rest encryption lands, OR regulated-industry customer engagement. Tracked in DEBT-037 §7. | | WH-02 | Half-dead receiver wastes the full retry budget (1+2+4 ≈ 7s with jitter) per delivery before the worker gives up | Admin role is trusted; this is operator-observable via `consecutive_failures` on the subscription row. A sticky-failure receiver disabled itself via operator action is fine pre-v1. | Circuit breaker lands (DEBT-037 §1) — auto-disable after N consecutive failures, require admin re-enable. | +| WH-03 | Admin configures an `http://` webhook URL; event body (incl. payload fields) travels plaintext on the wire | Operator-trust posture (same rationale as DA-06: protecting admin from self is out of scope). HMAC signature still detects tampering regardless of transport — only *read* confidentiality is lost. The API surfaces a non-blocking warning in `WebhookResponse.warnings` so the operator is informed on every GET/CREATE, and test/dev environments without TLS remain usable. | Multi-admin delegation lands, OR a regulated-industry customer engagement, OR an operator ticket asks for a `DECNET_WEBHOOK_REQUIRE_HTTPS=true` enforcement knob. | ### Needs-verification checklist (DECNET↔Webhook) @@ -409,7 +410,7 @@ the receiver (Shuffle→Slack, TheHive→Cortex, …). - [x] 4xx no-retry, 5xx/429/network retry — `tests/webhook/test_client.py::test_deliver_no_retry_on_4xx` + retry tests. - [x] Bounded concurrency + timeout per delivery — `Semaphore(10)` + 10s httpx timeout in `worker.py`. - [ ] Secret-field omission on the OpenAPI schema (not just the response body). Verify that `/openapi.json` shows `WebhookResponse` without `secret` so SDK consumers don't accidentally deserialize into a shape that expects it. -- [ ] Reject `http://` URLs at admin time (WH-01 adjunct). A viewer can't create subscriptions, but even an admin typo into a plaintext URL bypasses the HMAC-alone-detects-tampering assumption. Consider a router-level check warning on non-`https` + a `DECNET_WEBHOOK_ALLOW_INSECURE` opt-out for dev boxes. +- [x] ~~Reject `http://` URLs at admin time.~~ Resolved as **WH-03 accepted risk** — operator-trust posture, we warn rather than reject. `WebhookResponse.warnings` surfaces an `insecure_url` advisory on every GET/CREATE when the URL starts with `http://`. Tested at `tests/api/webhooks/test_crud.py::test_http_url_warns_but_accepts`. ### Out of scope (this component) @@ -445,3 +446,4 @@ In priority order: | 2026-04-24 | F5/I moved from **?** to **M** via `response_model=...` on every dict-returning mutation (`MessageResponse` + purpose-built models). F4/D "expensive `LIKE`" moved from **?** to **A** under new accepted risk DA-09 — admin-only surface, operator-scope rate limiting, `limit` cap. FTS5 kept as a performance TODO, not a security blocker. | ANTI | | 2026-04-24 | F6/I and F6/D both moved from **?** to **M**. F6/I: documented the viewer-safe-by-construction invariant for both SSE streams (every emitted event type wraps data already viewer-readable via REST). F6/D: added `decnet/web/sse_limits.py::sse_connection_slot` — per-user counter + async lock + 429 on overflow, wired into both SSE generators. `DECNET_SSE_MAX_PER_USER` env knob, default 5. | ANTI | | 2026-04-24 | Component 2 added — DECNET↔External webhook destination. Covers the new `decnet webhook` worker + `/api/v1/webhooks` admin CRUD. HMAC-SHA256 signing, 4xx no-retry + 5xx/429 retry with jittered backoff, admin-only CRUD, secret never leaks post-create. Two accepted risks registered (WH-01 secret at rest, WH-02 half-dead-receiver retry waste) paired with DEBT-037 pointers. | ANTI | +| 2026-04-24 | WH-03 accepted risk added — `http://` webhook URLs are allowed (operator-trust posture) but surface an `insecure_url` advisory in `WebhookResponse.warnings`. Checklist item "reject http://" resolved as "warn, not reject" per explicit operator decision. | ANTI | diff --git a/tests/api/webhooks/test_crud.py b/tests/api/webhooks/test_crud.py index acbdcc27..e643c8c7 100644 --- a/tests/api/webhooks/test_crud.py +++ b/tests/api/webhooks/test_crud.py @@ -173,6 +173,43 @@ async def test_delete_returns_message( assert res2.status_code == 404 +@pytest.mark.asyncio +async def test_http_url_warns_but_accepts( + client: httpx.AsyncClient, auth_token: str +): + """Plain http:// is allowed (operator-trust posture per WH-03) but + surfaces a non-blocking advisory in the response's warnings list.""" + res = await client.post( + PATH, + json={ + "name": "wh-http", + "url": "http://insecure.local/inbound", + "topic_patterns": ["system.>"], + }, + headers={"Authorization": f"Bearer {auth_token}"}, + ) + assert res.status_code == 201, res.text + body = res.json() + assert any("insecure_url" in w for w in body["warnings"]) + + +@pytest.mark.asyncio +async def test_https_url_has_no_warning( + client: httpx.AsyncClient, auth_token: str +): + res = await client.post( + PATH, + json={ + "name": "wh-https", + "url": "https://secure.example/inbound", + "topic_patterns": ["system.>"], + }, + headers={"Authorization": f"Bearer {auth_token}"}, + ) + assert res.status_code == 201 + assert res.json()["warnings"] == [] + + @pytest.mark.asyncio async def test_viewer_forbidden(client: httpx.AsyncClient, viewer_token: str): res = await client.get(