Problem/Motivation
McpWebhookWorker::resolveSecret() returns an empty string both when an endpoint declares no secret_key (a legitimate secretless endpoint) and when it declares one whose Key entity is missing or resolves to an empty value (a misconfiguration). The worker only adds X-MCP-Signature when the secret is non-empty — so in the misconfigured case every delivery is sent unsigned, silently voiding an explicitly configured security control.
Observed in production: a file-based Key whose backing file was never created resolved to '' for a month. Every delivery went out with no signature header; nothing logged the degradation. A receiver that verifies signatures rejects them (visible only as 401s in the delivery log); a receiver that treats the signature as optional accepts unsigned posts while believing them governed.
Steps to reproduce
- Configure a webhook endpoint with
secret_keypointing at a Key entity whose value is empty (e.g. a file provider with a missing file). - Trigger a subscribed event.
- The delivery is sent without
X-MCP-Signature; no warning or error anywhere distinguishes this from an intentionally secretless endpoint.
Proposed resolution
- Distinguish "no
secret_keyconfigured" (send unsigned — intentional) from "secret_keyconfigured but the Key is missing or resolves empty" (fail closed). - In the failing case, do not send: mark the row terminally (a
failed_keystatus alongsidefailed_ssrf/failed_redirectfrom #3613242, or reusefailedwith an explicit message naming the key id) and log loudly. - The #3613242 status-report warning then surfaces the accumulation automatically; optionally hook_requirements() could also pre-flight every enabled endpoint's key resolution so the misconfiguration is visible before any delivery is attempted.
Comments
Comment #2
jmcerdaFix implemented: https://github.com/Wilkes-Liberty/mcp_sentinel/pull/61
The two cases now diverge as proposed: no
secret_keykeeps sending unsigned by design (pinned by the existing tests), while a declared key that is missing or resolves empty refuses the delivery terminally asfailed_key— before the atomic claim, alongside the other configuration-class refusals — with the key id in the row and a loud log naming the endpoint. All status surfaces (filter, delivery log, dashboard badge, metrics, the permanent-failure warning) learn the new state, and hook_requirements() pre-flights every enabled endpoint's key so the misconfiguration shows on the status report before any delivery is attempted.Kernel coverage: missing key →
failed_keywith zero HTTP requests; empty-value key → same; requirements warning appears exactly when an enabled endpoint's key is unresolvable. Targets 1.13.0.Comment #3
jmcerdaMerged (PR #61, with review follow-ups folded in) and released in 1.13.0. Verified on the full drupalcode pipeline for the release commit (518 tests, functional leg included): pipeline #902448.
Comment #5
jmcerdaShipped in 1.13.0. Closing after release.