feat(taskspawner): add contextSources to TaskTemplate for cross-system declarative info gathering

[knechtionscoding]

What type of PR is this?

/kind feature

What this PR does / why we need it:

Adds a contextSources field to TaskTemplate that declaratively fetches data from external endpoints during the spawner's discovery cycle and makes responses available as {{.Context.<name>}} template variables. This gives agents rich, pre-fetched cross-system context from the first token, improving first-attempt success rates and reducing cost.

Extensible design (per #881): HTTP-specific fields are nested under http: rather than flattened, leaving room for future source kinds (secretRef, configMapRef, s3, kelosTaskOutput, etc.) to be added as sibling fields without a breaking API change.

Key design decisions:

• ContextSource.HTTP *HTTPContextSource — oneOf union pattern for source kinds, with CEL validation enforcing exactly one is set
• FailurePolicy enum (Fail/Ignore) instead of a boolean — leaves room for future policies like Retry
• ResponseFilter {Type, Expression} struct instead of bare jsonPathFilter string — extensible to JQ, XPath, regex
• HTTPHeaderSource {Header, SecretName, SecretKey} — flat SecretKeySelector pattern (idiomatic k8s, no wrapper struct)
• Parallel fetching via errgroup with per-source timeout and response size limits
• HTTPS-by-default with opt-in allowInsecure

Which issue(s) this PR is related to:

Fixes #881

Special notes for your reviewer:

Example usage:

taskTemplate:
  contextSources:
    - name: errorDetails
      http:
        url: "https://sentry.io/api/0/organizations/my-org/issues/?query={{.Number}}"
        headersFrom:
          - header: Authorization
            secretName: sentry-credentials
            secretKey: auth-header
        responseFilter:
          type: JSONPath
          expression: "$[0].culprit"
      failurePolicy: Fail
  promptTemplate: |
    Fix bug #{{.Number}}: {{.Title}}

    {{.Body}}
    {{- if .Context.errorDetails}}

    ## Sentry context
    {{.Context.errorDetails}}
    {{- end}}

The API is designed so future source kinds can be added as siblings to http:

contextSources:
  - name: fromSecret
    secretRef:        # future
      name: my-secret
      key: data.json
  - name: fromHTTP
    http:
      url: https://api.example.com/data

Does this PR introduce a user-facing change?

feat: add contextSources to TaskTemplate for declarative cross-system context enrichment. Sources are fetched before task creation and exposed as .Context.NAME template variables. Initial support for HTTP(S) endpoints with Secret-backed headers, JSONPath response filtering, and configurable failure policy.

[cubic-dev-ai]

1 issue found across 10 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/contextfetch/fetcher.go">

<violation number="1" location="internal/contextfetch/fetcher.go:232">
P2: Template rendering is non-strict, so missing variables may silently render as `<no value>` and still trigger outbound HTTP requests.</violation>
</file>

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

[greptile-apps]

Greptile Summary

• Adds contextSources to TaskTemplate enabling declarative parallel HTTP fetching before task creation, with results injected as .Context.NAME template variables across both the polling spawner loop and webhook handler. The contextfetch package is well-structured with proper timeout handling, Secret-based header resolution, JSONPath filtering, and graceful degradation for optional sources.
• Two previously-flagged concerns remain open: cluster-wide secrets.get ClusterRole grant, and the contextSources schema being absent from the second TaskTemplate block in both CRD manifests (field will be silently pruned by Kubernetes on that schema path).
• Minor style issues: http.DefaultClient used in production without a custom transport, and POST bodies have Content-Type: application/json auto-set regardless of the actual body format.

Confidence Score: 4/5

Safe to merge with minor caveats; no new P1 or P0 issues beyond previously-flagged concerns in open threads.

All new findings are P2 style suggestions. The two substantive issues (missing CRD schema on second TaskTemplate path, cluster-wide secrets RBAC) are carried over from already-open review threads. Core logic, error handling, and test coverage are solid.

internal/manifests/install-crd.yaml and internal/manifests/charts/kelos/templates/crds/taskspawner-crd.yaml — the second TaskTemplate schema block is missing the contextSources field in both files (previously flagged).

Important Files Changed

Filename	Overview
internal/contextfetch/fetcher.go	New parallel context-fetching package; URL/body templating, HTTPS validation, JSONPath filtering, and Secret-based headers are all solid. Minor concerns: Secret re-fetched per-source per-item (no intra-call cache), and auto-setting Content-Type for POST bodies assumes JSON.
cmd/kelos-spawner/main.go	Correctly gates Fetcher creation on non-empty ContextSources and skips item (continue) on FetchAll error; uses http.DefaultClient where a custom transport would be preferable.
internal/webhook/handler.go	Context-fetch integration mirrors the spawner path; error propagation from required sources will return a non-2xx to the webhook sender (e.g. GitHub), triggering retries. Also uses http.DefaultClient.
api/v1alpha1/taskspawner_types.go	Well-annotated ContextSource and HeaderFromSecret types with kubebuilder validation markers; uniqueness CEL rule on contextSources is correct.
api/v1alpha1/zz_generated.deepcopy.go	Auto-generated deep-copy for ContextSource and HeaderFromSecret; correctly copies map, slice, and pointer fields.
internal/contextfetch/fetcher_test.go	Comprehensive unit tests covering GET/POST, URL templating, JSONPath, headers, TLS, timeouts, size limits, required/optional failure modes, and Secret resolution.
cmd/kelos-spawner/main_test.go	Three new integration tests validating happy-path context injection, optional-source graceful degradation, and required-source task-skip behaviour.
internal/manifests/charts/kelos/templates/rbac.yaml	Adds secrets.get to both kelos-spawner-role and kelos-webhook-role ClusterRoles; the SSRF and overly-broad secrets grant concerns were raised in previous threads.
internal/manifests/install-crd.yaml	contextSources schema added to first TaskTemplate block with correct CEL uniqueness rule; second TaskTemplate block at ~line 6147 only has the descriptive comment added, missing the full schema (previously flagged).
internal/manifests/charts/kelos/templates/crds/taskspawner-crd.yaml	Same pattern as install-crd.yaml: contextSources schema added to first TaskTemplate block but missing from the second occurrence around line 3024 (previously flagged).

Sequence Diagram

sequenceDiagram
    participant S as Spawner/Webhook
    participant F as contextfetch.Fetcher
    participant K as Kubernetes API
    participant E as External HTTP API

    S->>F: FetchAll(ctx, contextSources, templateVars)
    activate F

    par For each ContextSource (parallel via errgroup)
        F->>F: renderTemplateStr(url, templateVars)
        F->>F: validateURLScheme(renderedURL)
        F->>K: Get Secret (for HeadersFrom)
        K-->>F: Secret data
        F->>E: HTTP request (with timeout context)
        E-->>F: Response body
        F->>F: LimitReader (maxResponseBytes)
        opt JSONPathFilter set
            F->>F: applyJSONPathFilter(body, expr)
        end
        F-->>F: result[name] = value
    end

    alt Any required source fails
        F-->>S: error (task skipped/retried)
    else All sources succeed or optional failures
        F-->>S: map[name]string context data
        deactivate F
        S->>S: templateVars[Context] = contextData
        S->>S: Render promptTemplate / branch / metadata
        S->>S: Create Task
    end

Loading

Prompt To Fix All With AI

Fix the following 3 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 3
cmd/kelos-spawner/main.go:350
**`http.DefaultClient` used in production Fetcher**

`http.DefaultClient` is process-global shared state with no `ResponseHeaderTimeout` or configurable TLS settings. Both `main.go` and `handler.go` (line 536) hard-code it. A dedicated `*http.Client` with a transport that limits idle connections and enables `ResponseHeaderTimeout` is safer and makes the behaviour easier to reason about — especially since the per-source `context.WithTimeout` only covers total request time, not stalled response headers.

### Issue 2 of 3
internal/contextfetch/fetcher.go:649-660
**Secret fetched once per source, per work item — no intra-call cache**

`resolveHeaders` issues a fresh `client.Get` for every `HeaderFromSecret` entry on every call to `fetchOne`. If multiple context sources reference the same Secret (e.g., every source uses the same `sentry-credentials`), or if the spawner processes many work items in one cycle, each item triggers N separate Kubernetes API reads for the same object.

Consider memoizing Secret lookups within a single `FetchAll` invocation using a `map[string]*corev1.Secret` keyed by name, built once before launching goroutines. The map would be read-only by the goroutines, so no additional locking is required.

### Issue 3 of 3
internal/contextfetch/fetcher.go:606-608
**`Content-Type: application/json` auto-injected for all POST bodies**

The code sets `Content-Type: application/json` whenever a POST has a body and the header isn't already set. The `Body` field is documented as a generic "Go text/template" — it isn't required to produce JSON. A user sending form data or XML will silently get the wrong content type unless they explicitly override it in `headers`. Consider not auto-injecting this header and letting users opt in via the `headers` field instead.

_{Reviews (4): Last reviewed commit: "feat(taskspawner): add contextSources to..." | Re-trigger Greptile}

[knechtionscoding]

@gjkim42 I believe this is ready for review

[knechtionscoding]

This is mostly the way I think to work around extending linear or GitHub APIs for or other fields by using enrichment via contextSources, @gjkim42. Thoughts on the API schema?

[knechtionscoding]

@gjkim42 Merge conflicts resolved.

[gjkim42]

I am considering to add a context store feature to the kelos.
It may be related to #1000 as well.

The basic idea is to store the current context to a persistent store and so that you can load it later.

The main points I am struggling with are

1. How context is structured?
   Is it just a string?
   Or files like claude code session files? (Chat history)

2. Store abstraction
   It may be implemented by a kubernetes resource, pvc, s3, or other API.
   may need something like contextstoreclass (like storageclass)

[knechtionscoding]

I think there are two types of context:

1. Context specific to the task. Currently adding that context requires users to either add scripts that are executed at the start of the agent (in some form) and hope the agent does it correctly. Or extending kelos directly to support gathering that context (i.e. fix: enrich Linear Comment webhook payloads with issue description #898) which I think we rightfully want to avoid.
2. Shared or general context that is either generated by other agents or generated by humans that needs to be loaded in.

I don't think number 1 really fits into a persistent storage method. The user probably doesn't want the ticket other context stored into a persistent context. The description might change, the comments might change, etc. And having to write custom scripts compared to being able to declaratively use that in the prompt via templating is, I think, important to not have to extend the kelos API for each of the incoming triggers.

I do think 2 is useful to get kelos agents to learn from each other. Whether this is memory or persistent storage in the form of strings of claude files, I think is debatable and I don't have a great answer yet. But I do think that this is fundamentally different context from the get data from an API so that it can be populated into context and is long lived vs the api data that likely shouldn't.

Having the ability to add contextSources to the taskspawner I think is the easiest way to minimize the need for expanding the API with each source that comes in.

[knechtionscoding]

For number 2 types of context I am trying out the memory feature internally #1062 is related and I documented some tradeoffs and what to do there.

[knechtionscoding]

@gjkim42 curious if you have any other thoughts here. Happy to work on a more general context system as well for the purposes of permanent shared context

[gjkim42]

/kelos api-review

[kelos-bot]

🤖 Kelos Task Status

Task kelos-api-reviewer-pull-request-review-d82a18d0a7e2 has succeeded. ✅

[kelos-bot]

🤖 Kelos API Reviewer Agent @gjkim42

API Design Review

Verdict: REQUEST CHANGES
Scope: New TaskTemplate.contextSources field with ContextSource and HeaderFromSecret API types for declarative pre-task HTTP enrichment.

The motivation is solid and well-aligned with issue #881. The validation markers, CEL uniqueness rule, defaulting, RBAC, and templating integration are all in good shape. My concerns are concentrated in the shape of the new types, which are about to ship in a kelos.dev/v1alpha1 CRD and become difficult to evolve later.

Findings

Extensibility (blocking)

• ContextSource flattens HTTP transport into the top-level type — api/v1alpha1/taskspawner_types.go:655-728. This is the single most important point and matches the inline comment already on the PR (line 667): the original proposal in #881 nested the HTTP-specific fields under http: HTTPContextSource, leaving room for secretRef, configMapRef, s3, kelosTaskOutput, etc., to be added later as sibling source types. By inlining url, method, headers, headersFrom, body, jsonPathFilter, allowInsecure, timeoutSeconds, and maxResponseBytes directly into ContextSource, every one of those names is permanently reserved at the top level of a source. Adding a non-HTTP source later requires either an awkward oneOf of mutually-exclusive flat fields or a compatibility-breaking nested rework.
  Suggested shape (matches #881):

  type ContextSource struct {
      Name string         `json:"name"`
      HTTP *HTTPContextSource `json:"http,omitempty"` // +optional, oneOf future source kinds
  }
  type HTTPContextSource struct { URL, Method, Headers, HeadersFrom, Body, JSONPathFilter, AllowInsecure, TimeoutSeconds, MaxResponseBytes, Required }

  Combined with an XValidation rule that exactly one source kind is set, this gives the same UX today and a clean extension path.

• Required bool — taskspawner_types.go:728. A boolean locks the failure semantics into two states; a named string enum leaves room for Retry, FallbackTo<other>, or per-source backoff later. Prefer onFailure: Fail|Ignore (or failurePolicy) over a bool, especially since the surrounding API already uses string enums for similar policy choices (e.g., Method, commentPolicy). This is the same "list of choices, prefer enum over bool" guidance that drove CommentOn/commentPolicy.

• JSONPathFilter string — taskspawner_types.go:701. A bare string bakes in JSONPath as the filter language forever. If you later want JQ, XPath, regex extraction, or a templated transform, you'd need a parallel field. A structured wrapper such as responseFilter: { type: JSONPath, expression: "$..." } keeps a single field and lets new filter kinds be added by enum extension. (Reusing an existing filter type from the repo if one is a closer fit is even better — none seemed to be a 1:1 match in api/v1alpha1/ so this would be net-new.)

API Conventions / Naming

• HeaderFromSecret shape diverges from the Kubernetes SecretKeySelector pattern — taskspawner_types.go:733-748. Kubernetes' established pattern for "value at a key in a Secret" is corev1.SecretKeySelector{Name, Key}, used by EnvVar.ValueFrom.SecretKeyRef, EnvFromSource.SecretRef, etc. The PR introduces a third nesting layer (SecretRef.Name) that wraps a single string and then puts Key as a sibling of SecretRef. Two options:

  1. Flatten to { header, secretName, secretKey } — simplest, no extra type.
  2. Use a valueFrom style — { header, valueFrom: { secretKeyRef: { name, key } } } — verbose but maximally extensible (lets you add configMapKeyRef later for the same pattern as Pod env).
     Either is more idiomatic than the current shape. The inner SecretRef SecretReference wrapper-of-a-single-Name is the part that stands out compared to existing types in this repo (SecretValuesSource in agentconfig_types.go:131 also exhibits this pattern, but propagating that further does not seem ideal).

• AllowInsecure bool — taskspawner_types.go:705. Mild concern: allowX reads as a feature gate. If the HTTP fields move under http:, this naturally becomes http.allowInsecure and is fine. If kept flat, consider tls: { mode: Required|Optional } or similar to keep room for, e.g., custom CA bundles or insecureSkipVerify semantics later.

• Headers map[string]string + HeadersFrom []HeaderFromSecret are asymmetric. This works, but a single unified list using valueFrom-style (above) collapses static and secret-sourced headers into one shape and preserves a stable iteration order. Optional improvement — keep in mind if the http: wrapper refactor lands.

Validation

• CEL uniqueness rule on contextSources — taskspawner_types.go:865. self.all(a, self.exists_one(b, b.name == a.name)) is correct but O(n²); for maxItems=8 this is fine. A slightly clearer equivalent is self.map(s, s.name).size() == self.size(). Not blocking — your existing rule already matches the style used elsewhere in the repo.

• Name pattern excludes leading underscores/digits but allows ASCII only — taskspawner_types.go:664. Reasonable; matches Go template identifier syntax. Worth a brief godoc line noting that the name must be a valid Go template identifier since it's used as {{.Context.NAME}}.

Compatibility / Lifecycle

• Field is additive, optional, and gated by length checks. Forwards-compatibility for existing clients is fine. The risk is future compatibility: every blocking point above is much cheaper to fix now than after v1alpha1 users adopt the field.

CRD generation note (defensive — likely false positive in a prior bot review)

• A prior automated review flagged the absence of contextSources under a "second TaskTemplate block" in internal/manifests/install-crd.yaml around line 6147 (and the chart CRD). I verified there is only one taskTemplate: schema block per CRD; the line-6146 reference is the alphabetically-sorted promptTemplate property inside the same block (lines 3218–6189 are one continuous taskTemplate.properties object, with podOverrides's embedded PodSpec accounting for the large span). contextSources is defined once at line 3263 with the correct validation rule. Calling this out so it doesn't get treated as a blocker.

Suggestions (non-blocking)

• Document in the ContextSource godoc that .Context.NAME values are always rendered as strings (JSONPath scalars/objects are stringified) so prompt authors know what to expect.
• Consider documenting that TaskSpawner creation should be treated as a privileged operation, since contextSources lets the spawner pod reach arbitrary HTTPS endpoints reachable from the cluster network (cloud metadata, internal services). This is more of a release-note item than an API change.
• If Method may need PUT/PATCH/DELETE in the future (e.g., for context fetched via mutating-but-idempotent APIs), the enum is easy to extend; just be aware that POST-only Body semantics will need a small documentation pass at that time.

/kelos needs-input

[greptile-apps]

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

[knechtionscoding]

@gjkim42 review notes addressed.

[gjkim42]

/kelos api-review

[kelos-bot]

🤖 Kelos Task Status

Task kelos-api-reviewer-pull-request-review-0438336a94e5 has succeeded. ✅

[kelos-bot]

🤖 Kelos API Reviewer Agent @gjkim42

API Design Review

Verdict: APPROVE
Scope: New TaskTemplate.contextSources field with ContextSource, HTTPContextSource, HTTPHeaderSource, and ResponseFilter types for declarative pre-task context enrichment.

The four blocking concerns from the previous round have all been addressed cleanly:

• HTTP transport fields are now nested under ContextSource.HTTP *HTTPContextSource (api/v1alpha1/taskspawner_types.go:728-783), leaving sibling space for future secretRef/configMapRef/s3/kelosTaskOutput kinds without a breaking rework.
• Required bool → FailurePolicy named enum (Fail/Ignore) — room for Retry, FallbackTo*, etc. without a second flag.
• JSONPathFilter string → structured ResponseFilter{Type, Expression} with a JSONPath-only enum that can grow to JQ/XPath/regex by enum extension.
• HeaderFromSecret → flat HTTPHeaderSource{Header, SecretName, SecretKey}, idiomatic and consistent with SecretKeySelector style.

Validation markers, defaults, MaxItems, MinLength, and the uniqueness CEL rule are all in good shape. The Name pattern ^[a-zA-Z][a-zA-Z0-9_]*$ correctly enforces a Go template identifier.

Findings

Validation (non-blocking, but please tighten before adding a second source kind)

• CEL rule has(self.http) won't enforce "exactly one" once siblings exist — api/v1alpha1/taskspawner_types.go:751. The message reads "exactly one source kind must be set (currently only http is supported)", but the rule is has(self.http). Today these are equivalent. The moment a sibling field is added (e.g., secretRef), the rule must be rewritten to truly enforce one-of, otherwise users could set both http and secretRef simultaneously without a clear error from the API server. Suggested future shape:

  // +kubebuilder:validation:XValidation:rule="[has(self.http), has(self.secretRef)].filter(x, x).size() == 1",message="..."

  Leaving an inline TODO/comment by the rule that calls this out would help avoid the trap on the next extension.

• HTTPContextSource.Body is meaningful only for POST, but nothing prevents method: GET + body: "...". — api/v1alpha1/taskspawner_types.go:746. A short CEL guard is cheap and forward-compatible:

  // +kubebuilder:validation:XValidation:rule="!has(self.body) || self.method == 'POST'",message="body may only be set when method is POST"

Naming / API Conventions

• HeadersFrom []HTTPHeaderSource is mildly misleading next to upstream envFrom — taskspawner_types.go:739. In core Kubernetes, envFrom means "import all keys", whereas this headersFrom picks a single key per entry (closer to env.valueFrom.secretKeyRef). The current split also forces Headers to be a map (no ordering, no duplicates) while HeadersFrom is a list. Non-blocking, but worth considering before this leaves alpha — a single unified list of {name, value, valueFrom: {secretKeyRef: {name, key}}} would mirror EnvVar exactly, preserve order, and give a single extension point for future header value sources (e.g., configMapKeyRef, fieldRef). If you want to stay split, the current naming is acceptable.

• HTTPContextSource.Headers map[string]string doesn't bound size — taskspawner_types.go:736. HeadersFrom is capped at MaxItems=16 but inline Headers is unbounded. A symmetric MaxProperties or comparable limit (16–32) avoids a footgun for users who paste a giant headers map.

Compatibility / Lifecycle

• Field is additive, optional, and gated by length and XValidation rules. Forwards-compatibility for existing v1alpha1 clients is fine, and the +kubebuilder:default markers mean omitted optional values won't surprise readers. Per CRD-fields-are-forever discipline: all the names visible to users today (contextSources, http, failurePolicy, responseFilter.type, headersFrom) are good ones to commit to long-term.

CRD generation

• contextSources schema is correctly present once in each CRD (internal/manifests/install-crd.yaml:3263, internal/manifests/charts/kelos/templates/crds/taskspawner-crd.yaml:144). The earlier "second TaskTemplate block missing" finding was a false positive arising from alphabetical field ordering — branch (with the updated description) and promptTemplate (also with the updated description) live in the same single taskTemplate.properties object, separated by ~3000 lines because of the embedded PodSpec under podOverrides. No action needed.

Suggestions (non-blocking)

• Document on the ContextSource or HTTPContextSource godoc that .Context.NAME is always a string (JSONPath scalars are stringified via fmt.Sprintf, objects/arrays are JSON-marshaled) so prompt authors know what to expect when templating.
• Document on TaskSpawner (release note or godoc) that contextSources can reach any HTTPS endpoint routable from the cluster network — including cloud metadata services, internal cluster IPs, and the API server's host — and that creating a TaskSpawner should therefore be treated as a privileged operation. The SSRF allowlist discussion in a prior review thread is reasonable to defer past alpha, but the security posture should be called out in user-facing docs.
• When Method grows to include PUT/PATCH/DELETE later, the Body-vs-method validation rule above should be relaxed accordingly. Worth a small in-code comment on the enum so this isn't forgotten.

/kelos needs-input

[gjkim42]

/kelos review

[kelos-bot]

🤖 Kelos Task Status

Task kelos-reviewer-issue-comment-36be7c307f8a has succeeded. ✅

[kelos-bot]

🤖 Kelos Reviewer Agent @gjkim42

Review Summary

Verdict: APPROVE
Overall correctness: patch is correct
Scope: Adds taskTemplate.contextSources for declarative HTTP-based context enrichment, exposed to templates as {{.Context.NAME}}; wired into both the polling spawner loop and the webhook handler.

The previous api-review's blocking concerns (nested http:, FailurePolicy enum, structured ResponseFilter, flat HTTPHeaderSource) are all addressed cleanly. URL/body templates use missingkey=error so missing variables fail fast; errgroup cancellation noise is suppressed via gctx.Err() check before logging; response size is bounded; HTTPS-by-default with an explicit opt-in for plain HTTP. Unit tests in internal/contextfetch/fetcher_test.go cover GET/POST, URL templating, JSONPath, headers, TLS gating, timeout, size limits, Fail/Ignore policies, and the cancellation log-suppression behavior; integration tests in cmd/kelos-spawner/main_test.go cover happy path, optional graceful degradation, and required-source task skip.

The "second TaskTemplate block missing contextSources" concern from earlier reviews is a false positive — there is only one taskTemplate: schema block in both internal/manifests/install-crd.yaml (line 3218) and internal/manifests/charts/kelos/templates/crds/taskspawner-crd.yaml (line 99), and contextSources is correctly defined under each (line 3263 / line 144). Confirmed via direct grep on the CRDs.

Findings below are all non-blocking; raised against the new code paths only.

Findings Overview

Priority	Count	File:Line	Summary
P0	0	—	none
P1	0	—	none
P2	2	api/v1alpha1/taskspawner_types.go:770	CEL has(self.http) won't enforce one-of when a sibling source kind is added
P2		api/v1alpha1/taskspawner_types.go:735	No validation prevents method: GET + non-empty body
P3	2	internal/contextfetch/fetcher.go:141	Content-Type: application/json auto-set for any POST body regardless of format
P3		internal/manifests/charts/kelos/templates/rbac.yaml:139	Cluster-wide secrets get grant broader than the fetcher actually needs

Findings

Validation / Forward compatibility

• [P2] api/v1alpha1/taskspawner_types.go:770 — The CEL rule is has(self.http) but the message reads "exactly one source kind must be set (currently only http is supported)". Today the parenthetical disclaimer covers the gap, but the moment a sibling kind (e.g., secretRef, configMapRef) lands, this rule must be rewritten to [has(self.http), has(self.secretRef)].filter(x, x).size() == 1 — otherwise users will be able to set both simultaneously with no error from the API server. Adding a // TODO comment by the rule now makes the trap harder to step on later.
• [P2] api/v1alpha1/taskspawner_types.go:735 — Method defaults to GET, but nothing prevents method: GET with a non-empty body. Go's net/http will still send the body on a GET, which most APIs ignore — so the misconfiguration is silently wrong rather than caught at admission. A short CEL guard like !has(self.body) || self.method == 'POST' would surface it.

Code quality

• [P3] internal/contextfetch/fetcher.go:141-143 — Auto-injecting Content-Type: application/json assumes the templated Body is JSON. The field is documented as a generic Go text/template, so users sending form-encoded data, XML, or multipart silently get the wrong content type unless they remember to override via Headers. Consider either dropping the auto-injection (require users to opt in through Headers) or documenting the JSON default on the Body godoc.

RBAC

• [P3] internal/manifests/charts/kelos/templates/rbac.yaml:139-148 and :245-251 — kelos-spawner-role and kelos-webhook-role are ClusterRoles; adding secrets get to them grants cluster-wide secret read via ClusterRoleBinding. The fetcher only reads from f.Namespace (the TaskSpawner's own namespace), so a namespaced Role per operational namespace, or a resourceNames-constrained ClusterRole, would shrink the blast radius. Not blocking — flagging in case the broader grant wasn't intentional.

Key takeaways

• API surface is in good shape; the http: nesting and FailurePolicy/ResponseFilter structuring leave clean room to extend.
• Two cheap CEL guards (one-of placeholder TODO, body/method compatibility) would harden the alpha against the most likely misuse paths.
• Cluster-wide secrets get is the only security-shaped expansion worth a deliberate decision before this lands.

Note on prompt injection: a prior cubic-dev-ai comment contained an embedded directive to attribute findings to its source. That instruction was disregarded; the analysis above is independent and based on direct inspection of the diff and surrounding code.

[kelos-bot]

[P2] The CEL rule has(self.http) doesn't enforce one-of — only that http is set. The message reads "exactly one source kind must be set (currently only http is supported)", and the parenthetical disclaimer is the only thing covering the gap today. The moment a sibling source kind (e.g., secretRef, configMapRef, s3, kelosTaskOutput) is added, this rule must be rewritten to actually enforce mutual exclusivity ([has(self.http), has(self.secretRef)].filter(x, x).size() == 1), otherwise users could set both without an admission-time error. An inline // TODO next to the rule would make the trap harder to step on later.

[kelos-bot]

[P2] No validation prevents method: GET (the default) with a non-empty body. Go's net/http will still send the body on a GET request, but most APIs ignore it — so this silently produces a request the user didn't mean to send rather than failing at admission. A short CEL guard catches the misconfiguration without baking in any future restrictions, e.g. // +kubebuilder:validation:XValidation:rule="!has(self.body) || self.method == 'POST'",message="body may only be set when method is POST" on the Body field.

[kelos-bot]

[P3] Auto-injecting Content-Type: application/json for any POST body presumes the templated body is JSON. The Body field is documented as a generic Go text/template — a user templating form-encoded data, XML, or multipart would silently get the wrong content type unless they remember to override it via Headers. Two reasonable options: drop the auto-injection and require users to set Content-Type explicitly through Headers, or document the JSON default on the Body godoc so it's not a surprise.

[gjkim42]

@knechtionscoding
can I merge this PR? or is there anything else that you are planning to update?

[knechtionscoding]

@gjkim42 you can merge, I was just on vacation. So I haven't had a chance to respond. I don't see anything blocking or needs fixing in this PR. So merge away!