[knechtionscoding]

What type of PR is this?

/kind feature

What this PR does / why we need it:

Adds a generic webhook source type (spec.when.webhook) to TaskSpawner that accepts arbitrary HTTP POST payloads with JSON bodies and maps them to task template variables via configurable JSONPath expressions. This enables integration with any webhook-capable system (Notion, Sentry, Drata, Salesforce, Aikido, etc.) without requiring Kelos-side code changes for each new integration.

Key design decisions:

• Path-based routing: Each source gets its own path (/webhook/<source>) and derives its HMAC secret from the <SOURCE>_WEBHOOK_SECRET environment variable (e.g., /webhook/notion → NOTION_WEBHOOK_SECRET)
• JSONPath field extraction: fieldMapping maps JSONPath expressions to template variables available in promptTemplate and branch
• AND-semantics filters: Filters support exact match and regex, all must match for a delivery to trigger a task
• Plugs into existing infrastructure: Reuses the kelos-webhook-server with --source generic rather than creating per-spawner Deployments
• No workspaceRef required: Generic webhook spawners can optionally omit workspaceRef since they may not be git-repo-driven
• Helm chart support: Adds webhookServer.sources.generic with envFrom-based multi-secret injection so all per-source secrets come from a single Secret resource

Which issue(s) this PR is related to:

Fixes #687

Special notes for your reviewer:

• The generic source uses envFrom in the helm chart (not valueFrom) since users will have multiple <SOURCE>_WEBHOOK_SECRET keys in one Secret
• The SignaturePrefix field defaults to "sha256=" but can be set to "" for providers that send raw hex digests (e.g., Linear-style)
• Filter semantics are AND (all must match), unlike GitHub/Linear webhook filters which use OR. AND is more intuitive for generic payloads since there are no event-type scopes. OR can be achieved by creating multiple TaskSpawners
• The ingress/gateway routes use /webhook/ as a catch-all prefix for generic, which is placed after the more specific /webhook/github and /webhook/linear rules
• Added github.com/PaesslerAG/jsonpath dependency for JSONPath evaluation

Does this PR introduce a user-facing change?

feat: Add generic webhook source type for TaskSpawner that accepts arbitrary HTTP POST payloads and maps JSON fields to task template variables via JSONPath expressions, enabling integration with any webhook-capable system without Kelos-side code changes.

---

Summary by cubic

Adds a generic webhook source to TaskSpawner to spawn tasks from any JSON POST at /webhook/<source> using JSONPath field mapping and AND-filters, with id-based idempotency when fieldMapping.id is set (fallback to body‑hash). Implements #687.

• New Features

  • Generic webhooks: POST /webhook/<source> routes to the generic handler; delivery IDs use fieldMapping.id when present, else body‑hash.
  • JSONPath mapping: fieldMapping populates template vars (id required); ID, Title, Body, URL default to empty; .Payload always available.
  • Filters: AND semantics with exact or regex matches; spawners match on source.
  • Server/Charts: kelos-webhook-server --source=generic serves under /webhook/; adds generic Deployment/Service and /webhook/ routes to ingress/gateway; controller treats generic as webhook‑driven; CRD validations added.
  • Dependencies: add github.com/PaesslerAG/jsonpath.

• Migration

  • Helm: enable webhookServer.sources.generic.enabled, optionally set webhookServer.sources.generic.secretName for env injection.
  • Define a TaskSpawner with spec.when.webhook (source, fieldMapping with id, optional filters), then send POSTs to /webhook/<source>.

^{Written for commit f960432. Summary will update on new commits.}

[cubic-dev-ai]

2 issues found across 17 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="internal/manifests/charts/kelos/templates/crds/taskspawner-crd.yaml">

<violation number="1" location="internal/manifests/charts/kelos/templates/crds/taskspawner-crd.yaml:1011">
P2: CRD says fieldMapping must include an `id` key, but schema doesn’t enforce it; generic webhook handling will accept missing `id` and leave parsed.ID empty, contradicting the documented requirement.</violation>
</file>

<file name="api/v1alpha1/taskspawner_types.go">

<violation number="1" location="api/v1alpha1/taskspawner_types.go:488">
P2: XValidation does not enforce the documented “set exactly one” requirement for value/pattern; filters with neither field set pass validation.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

[gjkim42]

/kelos review

[kelos-bot]

🤖 Kelos Task Status

Task kelos-reviewer-902 has failed. ❌

[kelos-bot]

🤖 Kelos Reviewer Agent @gjkim42

Review Summary

Verdict: REQUEST CHANGES
Scope: Adds generic webhook source type to TaskSpawner with path-based routing, JSONPath field extraction, HMAC signature validation, and Helm chart support

Findings

Correctness

• internal/webhook/generic_filter.go:39 / internal/webhook/handler.go:482 — Shared Fields map leaks state across spawner iterations (blocking)
  ExtractFields adds keys to parsed.Generic.Fields but never clears the map between spawner iterations. In processWebhook (line 357–414), the loop calls matchesSpawner → ExtractFields for each spawner. If Spawner A has fieldMapping: {id: "$.a.id", custom_a: "$.a.val"} and Spawner B has fieldMapping: {id: "$.b.id"}, after processing Spawner A the Fields map contains {id: "...", custom_a: "..."}. When Spawner B runs, it overwrites id but leaves custom_a from Spawner A. Then createTask → ExtractGenericWorkItem for Spawner B includes the stale custom_a as a template variable.
  Fix: Reset Fields to a fresh map at the start of ExtractFields.

• internal/webhook/handler.go:563-581 — getGenericWebhookConfig lists ALL TaskSpawners on every request (non-blocking)
  Every incoming generic webhook triggers a client.List of all TaskSpawners to determine signature/delivery headers, in addition to the existing getMatchingSpawners List call further down. This doubles the API server load per webhook delivery. Consider combining these into a single List, or caching the config per source with a short TTL.

• internal/webhook/handler.go:576 — getGenericWebhookConfig returns first-match only
  If multiple TaskSpawners listen to the same source with different SignatureHeader or SignaturePrefix values, only the first match's config is used for validation. The second spawner's signature config is silently ignored. Worth documenting in the CRD field comments.

Tests

• Test coverage is comprehensive: parsing, field extraction, filters (AND semantics, exact/regex, nested fields, numeric/boolean coercion), path extraction, signature validation (with/without prefix), HTTP handler tests for matching, non-matching, wrong source, idempotency, and hyphenated source names. Well done.
• make verify passes — generated files (deepcopy, CRD YAML, install-crd.yaml) are up to date.
• make test was still compiling from scratch in this container environment; no failures observed at the time of review.

Cubic's prior review — rebuttal

The two P2 violations flagged by cubic are both incorrect:

1. "CRD says fieldMapping must include an id key, but schema doesn't enforce it" — Wrong. The CRD YAML (taskspawner-crd.yaml:1084) contains an x-kubernetes-validation rule: 'id' in self.fieldMapping. This enforces the requirement at the schema level.

2. "XValidation does not enforce the 'set exactly one' requirement for value/pattern" — Wrong. The rule has(self.value) != (has(self.pattern) && size(self.pattern) > 0) uses XOR semantics that correctly rejects both-omitted and both-set cases.

Security

• No hardcoded secrets — per-source secrets loaded from env vars at runtime
• HMAC-SHA256 signature validation with timing-safe comparison via existing validateHMACSignature
• Payload size limit (10 MB) inherited from existing handler
• Source name from URL path is safe: os.Getenv treats the string literally, no injection risk
• Optional no-secret mode (skip validation) is clearly logged — appropriate for sources that don't support signing

Conventions

• Logging messages start with capital letters, no trailing punctuation ✓
• Generated files included and match make update output ✓
• PR description follows template with all sections filled ✓
• workspaceRef validation rule intentionally excludes webhook source — correct per design (generic webhooks may not be git-repo-driven)

Suggestions (optional)

• internal/webhook/generic_filter.go:68 — regexp.MatchString recompiles the regex on every filter evaluation. For high-throughput sources, consider compiling once. Not blocking for v1 but worth a TODO.
• Consider adding validation that prevents two spawners from claiming the same source name with conflicting SignatureHeader/SignaturePrefix configurations.
• Minor: extractSourceFromPath rejects paths with >3 segments but returns the generic "Missing source" message. A more descriptive error could help debugging.

/kelos needs-input

[knechtionscoding]

@gjkim42 I have implemented review notes and suggestions. Let me know if there's anything else :)

[gjkim42]

Will take a look. Thanks for the PR!

[knechtionscoding]

@gjkim42 bump :)

[gjkim42]

I am generally positive about the feature. My only concern is API is hard to fix, so I'd like to find the best way to design it.

[gjkim42]

The source should be unique in cluster scope, right?

[knechtionscoding]

Yes. The source is unique in the cluster, but we aren't spawning a new webhook server per source (at least not currently).

The current implementation spawns a single GenericWebhook taskSpawner that then listens for everything that isn't routed to one of our specific handlers. This means on the taskSpawner you have to declare which source you care to listen too.

For example, if you are listening to both notion and salesforce on the taskspawners for those you need to declare what source you care about.

[knechtionscoding]

@gjkim42 Any concerns here? Otherwise I'll rework the other thread and push for final review

[gjkim42]

Is there a validation that prevents duplicate sources?

[knechtionscoding]

No, because it isn't declared anywhere as a universal.

You are allowed to have multiple task spawners listening to the same source.

And on the webhook server side it just sees the path and reads for the secret. There's no ability for duplication anywhere.

Because once deployed you can send to webhook/test, webhook/notion, webhook/drata without any further configuration on the server side.

Once the generic source is deployed it then just checks for an existing taskSpawner with source that matches what it received.

So there's nothing to ensure isn't duplicated.

[knechtionscoding]

@gjkim42 Updated based on review comments and ensured verify passes.