Skip to content

docs: Document GenericWebhook TaskSpawner source#1035

Merged
gjkim42 merged 2 commits intomainfrom
kelos-task-1033
Apr 29, 2026
Merged

docs: Document GenericWebhook TaskSpawner source#1035
gjkim42 merged 2 commits intomainfrom
kelos-task-1033

Conversation

@kelos-bot
Copy link
Copy Markdown

@kelos-bot kelos-bot Bot commented Apr 28, 2026

What type of PR is this?

/kind docs

What this PR does / why we need it:

The spec.when.webhook (GenericWebhook) TaskSpawner source has been merged and tested but is entirely undocumented, which forces new users to write custom glue code instead of finding the feature. This PR adds user-facing documentation for it.

Specifically:

  • README.md — list Generic Webhooks (and the previously-omitted Linear Webhooks) in the TaskSpawner integration summary.
  • docs/integration.md — add a Generic Webhooks section covering source, fieldMapping, filters, setup, and template variables; add a Generic Webhook column to the source-vs-template-variable table; document that arbitrary fieldMapping keys also become template variables.
  • docs/reference.md — add spec.when.webhook.{source,fieldMapping,filters[].field,filters[].value,filters[].pattern} rows to the TaskSpawner reference table; add a Generic Webhook column to the promptTemplate variables table; mark workspaceRef.name as required for the webhook source too.
  • examples/13-taskspawner-generic-webhook/ — new example illustrating a Sentry-error-driven TaskSpawner with annotated YAML and a README walkthrough (setup, field mapping, filters, sample payload, troubleshooting).
  • examples/README.md — also list 12-taskspawner-file-patterns/ so the index no longer skips from 11 to 13.

Security note

The generic webhook endpoint does not currently validate request signatures — the API doc-comment, the Helm chart's webhookServer.sources.generic.secretName field, and the <SOURCE>_WEBHOOK_SECRET envFrom mount all imply HMAC protection that the handler does not implement. The docs describe this state honestly, recommend restricting access at the network layer (NetworkPolicy / Ingress IP allowlists / mTLS), and link to the follow-up issue tracking the missing validation.

Follow-up: #1040

Which issue(s) this PR is related to:

Fixes #1033

Special notes for your reviewer:

Docs-only — no behavior changes. make verify and make test pass.

Does this PR introduce a user-facing change?

Document the `spec.when.webhook` (GenericWebhook) TaskSpawner source: integration guide, field reference, template-variable table, and an end-to-end Sentry example under `examples/13-taskspawner-generic-webhook/`. The generic webhook endpoint is currently unauthenticated — restrict access at the network layer until per-source HMAC validation lands (tracked in #1040).

@github-actions github-actions Bot added kind/docs needs-triage needs-kind Indicates an issue or PR lacks a kind/* label needs-priority needs-actor release-note and removed needs-kind Indicates an issue or PR lacks a kind/* label labels Apr 28, 2026
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 6 files

@gjkim42
Copy link
Copy Markdown
Collaborator

gjkim42 commented Apr 28, 2026

/kelos review

Copy link
Copy Markdown
Author

@kelos-bot kelos-bot Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Kelos Reviewer Agent @gjkim42

Review Summary

Verdict: COMMENT (would-block; cannot REQUEST_CHANGES on own bot-authored PR)
Overall correctness: patch is incorrect
Scope: Documents the existing spec.when.webhook (GenericWebhook) TaskSpawner source — README list, integration guide section, reference table rows, template-variable column, and a Sentry-driven example.

Findings Overview

Priority Count File:Line Summary
P0 0 none
P1 2 docs/integration.md:332, examples/13-taskspawner-generic-webhook/README.md:159 Docs claim per-source HMAC signature validation that the handler does not actually perform
P2 0 none
P3 1 examples/README.md:25 Example index jumps from 11 to 13 (pre-existing gap, but easy to fix while the table is being edited)

Findings

Documentation accuracy / Security

  • [P1] docs/integration.md:332 and examples/13-taskspawner-generic-webhook/README.md:35-55,159 — The Setup/Helm-secret instructions and the "The server validates each delivery against the matching <SOURCE>_WEBHOOK_SECRET using X-Hub-Signature-256" / "Invalid signatures return HTTP 401" claims are not backed by the implementation. In internal/webhook/handler.go the GenericSource switch branch (~lines 211–231) only extracts the source path, lists spawners, and computes a delivery ID — there is no signature-header read, no env-var lookup of <SOURCE>_WEBHOOK_SECRET, and no call to any validateHMACSignature helper. signature.go exposes only ValidateGitHubSignature and ValidateLinearSignature. The handler tests corroborate this: TestGenericServeHTTP_CreatesTaskForMatchingSpawner and TestGenericServeHTTP_AcceptsUnknownSource both succeed with no signature header set, and there is no TestGenericServeHTTP_RejectsInvalidSignature counterpart. The Helm chart's envFrom: secretRef mounts the secret into the pod, but no code reads it. As written, the docs lead operators to deploy a webhook endpoint they believe is HMAC-protected when it actually accepts any unauthenticated POST; this is a meaningful security misrepresentation, not a stylistic gap. Recommended fix: describe the current state honestly (the /webhook/<source> endpoint is unauthenticated, recommend restricting ingress / network policy) and file a follow-up issue to add <SOURCE>_WEBHOOK_SECRET-based HMAC validation. Alternatively, hold this PR until the validation is implemented in the handler.

Suggestions

  • [P3] examples/README.md:25 — While editing the example index, consider also adding the existing examples/12-taskspawner-file-patterns/ row so the table doesn't skip from 11 to 13.

Key takeaways

  • The docs make a security guarantee (HMAC signature validation) that the current generic webhook handler does not implement; following the Setup steps yields an unauthenticated webhook endpoint. Resolve before merge by either correcting the docs or wiring up the missing validation.

Comment thread docs/integration.md Outdated
Comment thread examples/13-taskspawner-generic-webhook/README.md Outdated
Comment thread examples/README.md
@gjkim42
Copy link
Copy Markdown
Collaborator

gjkim42 commented Apr 28, 2026

/kelos pick-up

1 similar comment
@gjkim42
Copy link
Copy Markdown
Collaborator

gjkim42 commented Apr 28, 2026

/kelos pick-up

Add user-facing docs for the spec.when.webhook (GenericWebhook) source
type that has been merged but undocumented:

- README.md: list Generic Webhooks (and Linear Webhooks) in the
  TaskSpawner integration summary.
- docs/integration.md: add a Generic Webhooks section covering source,
  fieldMapping, filters, setup, and template variables; add a Generic
  Webhook column to the source-vs-variable table.
- docs/reference.md: add spec.when.webhook.* rows to the TaskSpawner
  field reference and a Generic Webhook column to the promptTemplate
  variables table; mark workspaceRef as required for webhook sources.
- examples/13-taskspawner-generic-webhook: add a Sentry-driven example
  with annotated YAML and a README walkthrough.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The previous docs claimed the generic webhook endpoint validates each
delivery against a per-source <SOURCE>_WEBHOOK_SECRET via
X-Hub-Signature-256, but the handler does not currently read or verify
any signature header — operators following the docs would deploy an
endpoint they believed was HMAC-protected but that actually accepts any
unauthenticated POST.

Replace the HMAC-validation language with a security warning that
describes the current state honestly, recommends restricting access at
the network layer (NetworkPolicy / Ingress / Gateway IP allowlists),
and links to the follow-up issue tracking the validation work. Note
the chart's webhookServer.sources.generic.secretName field as reserved
for future use.

Also list examples/12-taskspawner-file-patterns/ in the examples index
so it no longer skips from 11 to 13.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@kelos-bot kelos-bot Bot force-pushed the kelos-task-1033 branch from 6a08ffe to 92104a0 Compare April 28, 2026 13:50
@gjkim42
Copy link
Copy Markdown
Collaborator

gjkim42 commented Apr 28, 2026

/kelos review

@kelos-bot
Copy link
Copy Markdown
Author

kelos-bot Bot commented Apr 28, 2026

🤖 Kelos Agent @gjkim42

Addressed the prior review's P1 finding (security misrepresentation about HMAC validation) and the P3 example-index gap in commit 92104a0:

  • Removed the inaccurate <SOURCE>_WEBHOOK_SECRET / X-Hub-Signature-256 validation claims from docs/integration.md, docs/reference.md, the example README, and the example YAML comments.
  • Added a [!WARNING] callout in both docs/integration.md and examples/13-taskspawner-generic-webhook/README.md stating the endpoint is currently unauthenticated, recommending network-layer mitigations (NetworkPolicy, Ingress IP allowlisting, mTLS, private network), and noting the chart's webhookServer.sources.generic.secretName field is reserved for future use.
  • Filed Generic webhook endpoint is unauthenticated — implement <SOURCE>_WEBHOOK_SECRET HMAC validation #1040 to track implementing per-source HMAC validation in the handler; the docs link to it.
  • Added examples/12-taskspawner-file-patterns/ to the examples index so it no longer skips from 11 to 13.

make verify and make test pass; all CI checks are green. Ready for re-review.

Copy link
Copy Markdown
Author

@kelos-bot kelos-bot Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Kelos Reviewer Agent @gjkim42

Review Summary

Verdict: COMMENT (would-approve; cannot approve own bot-authored PR)
Overall correctness: patch is correct
Scope: Documents the existing spec.when.webhook (GenericWebhook) TaskSpawner source — README list, integration guide section, reference table rows, template-variable column, and a Sentry-driven example under examples/13-taskspawner-generic-webhook/.

Findings Overview

Priority Count File:Line Summary
P0 0 none
P1 0 none
P2 0 none
P3 0 none

Findings

None — the prior P1 finding (security misrepresentation around HMAC validation) and the P3 finding (examples/12-taskspawner-file-patterns/ missing from the index) were both addressed in commit 92104a0. Spot-checked claims against the implementation:

  • docs/integration.md and examples/13-taskspawner-generic-webhook/README.md now describe the endpoint as unauthenticated, recommend network-layer mitigations (NetworkPolicy / Ingress IP allowlist / mTLS), note the chart's webhookServer.sources.generic.secretName env-var mount is unread, and link the follow-up tracking issue #1040. This matches internal/webhook/handler.go (the GenericSource branch at lines 211–231 reads no signature header and calls no validator) and internal/webhook/signature.go (only ValidateGitHubSignature / ValidateLinearSignature exist).
  • The fieldMapping/filters semantics — id required, JSONPath against the parsed body, missing fields → empty strings, malformed JSONPath → error, AND-across-filters, missing filter field → silent skip, lowercase id/title/body/url aliased to uppercase, {{.Kind}} = "GenericWebhook", {{.Payload}} is the full body — all line up with internal/webhook/generic_filter.go (ExtractFields, MatchesGenericFilters, ExtractGenericWorkItem, extractGenericDeliveryID) and the API-level XValidation rule in api/v1alpha1/taskspawner_types.go:471.
  • docs/reference.md correctly extends workspaceRef.name's required-when clause to webhook and adds the five spec.when.webhook.* rows; the Generic Webhook column in the template-variable table matches what ExtractGenericWorkItem populates (Number / Labels / Comments / Event / Action / Sender / Branch / Ref / Repository* / Review* / Type / State / IssueID / Time / Schedule are all empty for this source).
  • examples/README.md no longer skips from 11 to 13.

Key takeaways

  • Docs-only change, accurate against the current implementation. Earlier review threads from kelos-bot have been resolved as obsolete; this PR is ready to merge from a docs-correctness standpoint.

@gjkim42 gjkim42 added this pull request to the merge queue Apr 29, 2026
Merged via the queue into main with commit 71d57d9 Apr 29, 2026
25 checks passed
@gjkim42 gjkim42 deleted the kelos-task-1033 branch April 29, 2026 12:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Docs: GenericWebhook source (spec.when.webhook) is fully implemented but entirely undocumented

1 participant