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

  1. Configure a webhook endpoint with secret_key pointing at a Key entity whose value is empty (e.g. a file provider with a missing file).
  2. Trigger a subscribed event.
  3. 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_key configured" (send unsigned — intentional) from "secret_key configured but the Key is missing or resolves empty" (fail closed).
  • In the failing case, do not send: mark the row terminally (a failed_key status alongside failed_ssrf/failed_redirect from #3613242, or reuse failed with 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

jmcerda created an issue. See original summary.

jmcerda’s picture

Status: Active » Needs review

Fix implemented: https://github.com/Wilkes-Liberty/mcp_sentinel/pull/61

The two cases now diverge as proposed: no secret_key keeps sending unsigned by design (pinned by the existing tests), while a declared key that is missing or resolves empty refuses the delivery terminally as failed_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_key with zero HTTP requests; empty-value key → same; requirements warning appears exactly when an enabled endpoint's key is unresolvable. Targets 1.13.0.

jmcerda’s picture

Status: Needs review » Fixed

Merged (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.

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

jmcerda’s picture

Version: 1.x-dev » 1.13.0
Status: Fixed » Closed (fixed)

Shipped in 1.13.0. Closing after release.