Webhook: Add HMAC signature validation for the generic webhook source

[kelos-bot]

🤖 Kelos Agent @gjkim42

Background

The spec.when.webhook (GenericWebhook) TaskSpawner source currently exposes an unauthenticated endpoint at `/webhook/`.

The Helm chart (`internal/manifests/charts/kelos/templates/webhook-server.yaml`) mounts `webhookServer.sources.generic.secretName` via `envFrom`, but the handler in `internal/webhook/handler.go` (`GenericSource` case, ~lines 211–231) does not:

• read `_WEBHOOK_SECRET`
• inspect `X-Hub-Signature-256`
• call any HMAC validation helper

`signature.go` only exposes `ValidateGitHubSignature` and `ValidateLinearSignature`. Tests like `TestGenericServeHTTP_CreatesTaskForMatchingSpawner` confirm the current behavior — they POST without any signature header and expect HTTP 200.

This was caught during review of #1035 (docs PR), which originally documented HMAC validation that does not exist. The docs there have been corrected to describe the current unauthenticated state.

Proposal

Wire up per-source HMAC-SHA256 validation for the generic webhook source:

1. In the `GenericSource` branch of `WebhookHandler.ServeHTTP`, look up `_WEBHOOK_SECRET` from the environment (uppercased `source` name).
2. Read the `X-Hub-Signature-256` header (`sha256=`).
3. Call `validateHMACSignature(body, signature, secret)`. Return `401 Unauthorized` on failure.
4. If `_WEBHOOK_SECRET` is missing, fail-closed (refuse the request) so misconfiguration is loud.
5. Add `TestGenericServeHTTP_RejectsInvalidSignature` and `TestGenericServeHTTP_RejectsMissingSecret` to exercise the new code paths; update existing tests to send a valid signature.
6. Once landed, restore the HMAC docs in `docs/integration.md` and `examples/13-taskspawner-generic-webhook/README.md` (the "Webhook security" sections).

/kind feature

[kelos-bot]

🤖 Kelos Agent @gjkim42

Kelos Triage Report

Kind: kind/api (reclassified from kind/feature — this changes the user-facing webhook surface: adds a required <SOURCE>_WEBHOOK_SECRET env var, introduces a new X-Hub-Signature-256 request requirement, and adds a 401 response path. Per the triage rules, webhooks are explicitly an API surface)
Priority: priority/important-soon — the generic webhook endpoint is currently unauthenticated, so any caller who can reach /webhook/<source> can trigger Task creation; this is a security gap, not a nice-to-have
Actor: actor/human — kind/api issues require maintainer review of the user-facing behavior change before a PR is opened. The fail-closed decision in step 4 in particular is operationally breaking for any existing user of the generic webhook source and warrants explicit sign-off

Status: STILL VALID

Evidence:

• internal/webhook/handler.go lines 211–231 (the GenericSource branch in ServeHTTP) reads the URL path, gathers spawners, and computes a delivery ID, but does not read any secret or call any signature validator. Compare to the GitHubSource (lines 179–190) and LinearSource (lines 192–209) branches which both call ValidateGitHubSignature / ValidateLinearSignature and return 401 on failure.
• internal/webhook/handler.go lines 117–126 (NewWebhookHandler) explicitly skips loading WEBHOOK_SECRET for GenericSource with the comment that the secret is meant to be looked up "per-request from <SOURCE>_WEBHOOK_SECRET env vars" — but no such per-request lookup is implemented anywhere.
• internal/webhook/signature.go exposes only ValidateGitHubSignature and ValidateLinearSignature; the shared validateHMACSignature helper is unexported and has no generic-source caller.
• No merged PRs reference Webhook: Add HMAC signature validation for the generic webhook source #1039 (gh pr list --state merged --search 1039 is empty); no commits on main mention this issue or the words "generic webhook HMAC".
• PR docs: Document GenericWebhook TaskSpawner source #1035 (open docs PR) corroborates the report — it was rewritten to remove HMAC documentation because the validation does not exist.

Duplicates found: None

• Docs: GenericWebhook source (spec.when.webhook) is fully implemented but entirely undocumented #1033 (Docs: GenericWebhook source ... entirely undocumented) is docs-only and complementary
• API: Add eventPolicy to webhook-based TaskSpawner for debouncing and work-item deduplication to prevent storm-driven task duplication #962 (API: Add eventPolicy to webhook-based TaskSpawner for debouncing) is about deduplication/throttling, not authentication
• Integration: Add CloudEvents support to kelos-webhook-server for universal CNCF-ecosystem event-driven task creation #914 (CloudEvents support) is a different webhook protocol entirely
• Configuration Alignment: Webhook filter asymmetries and shared-config identity collisions across TaskSpawners #979 (Webhook filter asymmetries) covers filter parity, not signature validation

Recommendation: KEEP OPEN — concrete, actionable security gap with a well-specified proposal. Maintainer review needed on (1) the fail-closed-on-missing-secret decision, since it is a breaking change for existing generic-webhook deployments, and (2) whether to support a header name other than X-Hub-Signature-256 for non-GitHub-style senders.