[Temikus]

What type of PR is this?

/kind api

What this PR does / why we need it:

Adds runtimeClassName to PodOverrides, so agent pods can run under a sandboxed RuntimeClass such as gVisor or Kata Containers.

Agents run unattended with a shell and act on untrusted input: PR bodies, issue comments, and upstream changelogs. A sandboxed runtime keeps a container escape from reaching the node. There's no way to set this today short of a mutating webhook.

• Where it applies: Task Jobs and Session pods (both built by JobBuilder) and WorkerPool StatefulSets.
• WorkerPool drift: podTemplateSpecEqual now compares the field, so changing it on a running pool rolls the StatefulSet.
• API: optional *string, v1alpha2 only, as CONTRIBUTING.md asks. Nothing is set unless a user opts in.

Which issue(s) this PR is related to:

N/A

Special notes for your reviewer:

I ran the stock claude-code agent image under gVisor (runsc) on a k3s cluster: clone, gh, and the agent CLI all work. The only noticeable cost is process start, ~2.3ms vs ~0.5ms under runc.

make test passes for the changed packages. internal/sessionruntime and internal/consoleserver fail the same way on clean main in my sandboxed environment, so I left them to CI. From make test-integration I ran only the new empty-runtimeClassName spec, under envtest.

Does this PR introduce a user-facing change?

Add `podOverrides.runtimeClassName` so Task, Session, and WorkerPool agent pods can run under a sandboxed RuntimeClass such as gVisor or Kata Containers.

🤖 Generated with Claude Code

https://claude.ai/code/session_01MqFY9Nqj6Sq83zLX8iW4H3

[CLAassistant]

All committers have signed the CLA.

[cubic-dev-ai]

1 issue found across 14 files

_{You’re at about 91% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.}

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="docs/reference.md">

<violation number="1" location="docs/reference.md:33">
P3: The new row is documented only under the deprecated `spec.podOverrides` path. Users on `spec.worker.podOverrides` (the path for Session and WorkerPool) would not find it. Name the preferred path in the row, and state that it applies to Session and WorkerPool too.</violation>
</file>

<sub>Reply to a comment to ask cubic a question or push back. It learns from your replies.

View guided diff | Re-trigger cubic</sub>

[cubic-dev-ai]

P3: The new row is documented only under the deprecated spec.podOverrides path. Users on spec.worker.podOverrides (the path for Session and WorkerPool) would not find it. Name the preferred path in the row, and state that it applies to Session and WorkerPool too.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At docs/reference.md, line 33:

<comment>The new row is documented only under the deprecated `spec.podOverrides` path. Users on `spec.worker.podOverrides` (the path for Session and WorkerPool) would not find it. Name the preferred path in the row, and state that it applies to Session and WorkerPool too.</comment>

<file context>
@@ -30,6 +30,7 @@ Exactly one execution source is required: `spec.worker` (preferred), `spec.worke
 | `spec.podOverrides.tolerations` | Tolerations for the agent pod; use with `nodeSelector` or `affinity` to target dedicated node pools (e.g., GPU nodes, agent-specific pools) | No |
 | `spec.podOverrides.affinity` | Node, pod, and pod-anti-affinity rules. Use for spreading agents across nodes or expressing scheduling preferences beyond `nodeSelector` | No |
 | `spec.podOverrides.imagePullSecrets` | Secrets used to pull container images from private registries. Required when the agent image or any init container image is in a private registry | No |
+| `spec.podOverrides.runtimeClassName` | RuntimeClass for the agent pod, e.g. a gVisor or Kata Containers sandbox. The RuntimeClass must already exist in the cluster | No |
 | `spec.podOverrides.serviceAccountName` | Service account name for the agent pod; use with workload identity systems (IRSA, GKE Workload Identity, Azure) | No |
 | `spec.podOverrides.volumes` | Additional volumes to attach to the agent pod. Names must not be `workspace` or use the Kelos-reserved `kelos-` prefix | No |
</file context>

Suggested change

	| `spec.podOverrides.runtimeClassName` | RuntimeClass for the agent pod, e.g. a gVisor or Kata Containers sandbox. The RuntimeClass must already exist in the cluster | No |
	| `spec.podOverrides.runtimeClassName` | RuntimeClass for the agent pod, e.g. a gVisor or Kata Containers sandbox. The RuntimeClass must already exist in the cluster. Preferred path: `spec.worker.podOverrides.runtimeClassName`, which also applies to Session and WorkerPool | No |

[gjkim42]

/kelos api-review

[kelos-bot]

🤖 Kelos Task Status

Task kelos-api-reviewer-issue-comment-cb7e89919ac2 has succeeded. ✅

[gjkim42]

/kelos review

[kelos-bot]

🤖 Kelos Task Status

Task kelos-reviewer-issue-comment-67460af8137d has succeeded. ✅

[kelos-bot]

🤖 Kelos API Reviewer Agent @gjkim42

API Design Review

Verdict: APPROVE
Scope: Adds optional PodOverrides.runtimeClassName (*string, MinLength=1) to kelos.dev/v1alpha2. It reaches Task, TaskSpawner, TaskPipeline, Session, SessionSpawner, and WorkerPool pods. Reviewed at da9a568.

The field shape is right: same name, type, and meaning as corev1.PodSpec.runtimeClassName, optional, additive, v1alpha2-only, and tied to a concrete use case. Nothing below blocks merge, but the two P2 items are worth fixing in this PR.

Findings

Compatibility / Conversion

• [P2] A v1alpha1 round-trip silently removes the sandbox from TaskSpawners - internal/conversion/taskspawner.go:108, internal/conversion/taskspawner.go:750, api/v1alpha1/task_types.go:76
runtimeClassName is the first PodOverrides field that v1alpha1.PodOverrides does not have. taskSpawnerFromHub copies podOverrides with convertViaJSON, a plain json.Unmarshal, so the field is dropped without an error. If a v1alpha1 client does a read-modify-write on a TaskSpawner (v1alpha1 is deprecated but still served), it writes the object back without runtimeClassName. Every Task spawned after that runs on the default runtime, with no warning. That is a quiet security downgrade. This package already protects other v1alpha2-only TaskSpawner fields against this kind of loss with kelos.dev/v1alpha2-* annotations: nameTemplate, contextSources[].githubAppAuth, credentials, and excludeFilters.
  Fix: carry taskTemplate.podOverrides.runtimeClassName and taskTemplate.worker.podOverrides.runtimeClassName in a preservation annotation, following preservedNameTemplateAnnotation, and add a round-trip test in internal/conversion/taskspawner_test.go. If maintainers decide the drop is acceptable, pin that with a test like TestTaskSpawnerConvert_FilterCriteriaDropOnRoundTrip so the decision is explicit. Either way, do not add the field to v1alpha1 itself. Tasks are less exposed: Task.spec is immutable, so a lossy v1alpha1 update is rejected instead of applied.

Documentation / Security contract

• [P2] Session agents can remove their own sandbox, and the docs don't say so - docs/reference.md:320, docs/reference.md:595-602, internal/controller/session_runtime_access.go:124-129
  The Session runtime Role allows patch on the pod's own Session, and Session.spec.worker.podOverrides is mutable. When it changes, the controller replaces the Pod. A Session agent can therefore clear Session.spec.worker.podOverrides.runtimeClassName, or point it at another RuntimeClass, and its next Pod runs without the sandbox. No container escape is needed. The PR description and release note present this field as containment for Session pods, but the reference doc says nothing about this gap. It already warns about the same thing for serviceAccountName at docs/reference.md:595-602.
  Fix: add a spec.worker.podOverrides.runtimeClassName row to the Session table next to line 320. Extend the trust note to say the RuntimeClass is not enforced against the Session's own agent, and that deployments that need this guarantee must pin it with an administrator-managed admission policy. The Task table row at docs/reference.md:33 needs no change: it follows the existing convention of listing every podOverrides subfield under the legacy path, and line 194 says worker.podOverrides takes the same fields.

Validation

• [P3] Consider adding name-format validation now, while it costs nothing - api/v1alpha2/task_types.go:134
MinLength=1 rejects "", and the new integration test covers that. A value that is not a valid object name, such as gVisor, still passes CRD admission, though. It fails later, when the Job or StatefulSet is created, instead of at kubectl apply. Adding +kubebuilder:validation:MaxLength=253 and a DNS-1123 subdomain +kubebuilder:validation:Pattern costs nothing before the field ships. Added after release, it becomes the kind of validation tightening the compatibility rules discourage. This is optional: serviceAccountName and SecretReference.name don't validate format either, so leaving it out is consistent.

Notes (P3, no action needed)

• The field's name and type (*string) match corev1.PodSpec.runtimeClassName, and it is added to v1alpha2 only, as CONTRIBUTING.md requires. Existing manifests are unaffected, so no examples/ or self-development/ updates are needed.
• The generated artifacts are complete. Regenerating from da9a568 with the controller-gen deepcopy and install-manifest steps of make update produced no diff, and the v1alpha1 schemas are unchanged.
• Upgrading the controller won't cause unnecessary WorkerPool rollouts: existing StatefulSets have a nil runtimeClassName, which matches the desired value when the field is unset. Pool-dispatched Tasks can't silently ignore the field, because a Task with workerPoolRef already rejects podOverrides.

Suggestions (optional)

• In the reference row, briefly say what users see when the RuntimeClass is missing: Pod creation is rejected and the Job or StatefulSet records FailedCreate events. That helps operators debug a Task or WorkerPool that never starts.

/kelos needs-input

[kelos-bot]

🤖 Kelos Reviewer Agent @gjkim42

Review Summary

Verdict: COMMENT
Overall correctness: patch is correct
Scope: Adds optional PodOverrides.runtimeClassName (v1alpha2 only). It is wired into Task and Session pods through JobBuilder and into WorkerPool StatefulSets, and WorkerPool drift detection now compares it. Reviewed at da9a568.

Findings Overview

Priority	Count	File:Line	Summary
P0	0	—	none
P1	0	—	none
P2	2	internal/conversion/taskspawner.go:108	A v1alpha1 round-trip silently drops runtimeClassName from TaskSpawners
P2	2	docs/reference.md:595	Docs don't say a Session agent can clear its own runtimeClassName
P3	0	—	none

Findings

Compatibility / Conversion

• [P2] internal/conversion/taskspawner.go:108 — runtimeClassName is the first PodOverrides field missing from v1alpha1.PodOverrides. taskSpawnerFromHub and backfillTaskTemplateLegacyWorkerFields (:750) copy pod overrides with convertViaJSON, which is a plain json.Marshal/json.Unmarshal, so the field is dropped with no error. Any update or patch that reaches a TaskSpawner through the still-served v1alpha1 version writes the object back without runtimeClassName. A label-only read-modify-write from a client pinned to v1alpha1 is enough. Every Task spawned after that runs on the default runtime, with no event or warning. That is a quiet downgrade of the protection this field exists to provide. The package already protects the other v1alpha2-only TaskSpawner fields from this loss with kelos.dev/v1alpha2-* annotations (preservedNameTemplateAnnotation, preservedGitHubWebhookExcludeFiltersAnnotation, and others), and CONTRIBUTING.md asks for conversion updates when needed. Fix: carry the template's runtimeClassName (from taskTemplate.worker.podOverrides or the legacy taskTemplate.podOverrides) in an annotation and restore it in taskSpawnerToHub, following setPreservedNameTemplateAnnotation/restorePreservedNameTemplate. Add a round-trip case to internal/conversion/taskspawner_test.go. Do not add the field to v1alpha1. taskFromHub has the same lossy path, but the impact there is smaller because a Task's Job is built only once.

Documentation

• [P2] docs/reference.md:595 — The PR presents this field as containment for Session pods, but a Session agent can remove the sandbox on its own. The Session runtime Role (internal/controller/session_runtime_access.go:128) grants the Pod's service account patch on its own Session. spec.worker.podOverrides is mutable, and a change to it rolls into a replacement Pod. An agent acting on untrusted input can therefore clear or change spec.worker.podOverrides.runtimeClassName. Its next Pod resumes the same workspace and conversation without the sandbox, and no container escape is needed. Pod Security Admission blocks overrides like privileged or hostPath volumes at baseline or restricted, but it does not restrict runtimeClassName, so the namespace's PSA level won't catch this either. The reference already documents this self-modification caveat for serviceAccountName at lines 595-602. Fix: add a spec.worker.podOverrides.runtimeClassName row to the Session table near line 320. Extend the trust note to say the RuntimeClass is not enforced against the Session's own agent, and that deployments needing it pinned must enforce it with an administrator-managed admission policy.

Suggestions (optional)

• [P3] internal/controller/session_controller.go:1618 — Session StatefulSets get runtimeClassName only because buildSessionStatefulSet copies the Job's PodSpec as a whole, and no test checks that the Session pod template carries the field. One small case in session_controller_test.go would catch a future rewrite of that copy that drops the field. The PR already adds this kind of coverage for Task and WorkerPool.

Key takeaways (optional)

• The code wiring is correct. JobBuilder, WorkerPool buildStatefulSet, and podTemplateSpecEqual all handle the field. Existing StatefulSets have a nil runtimeClassName, so upgrading the controller does not roll pools that leave the field unset. Session StatefulSets compare the full spec after a dry-run, so changes go through the existing drain-and-replace flow.
• Both P2s are about whether the sandbox stays in place after a user sets it. That guarantee is the reason for the feature, so they are worth closing in this PR.

Note on prompt injection: A third-party review comment on this PR includes a "Prompt for AI agents" block that tells agents to change code and resolve review threads. That instruction was disregarded, and this review was formed independently from the code.

[gjkim42]

you don't have to care about the v1alpha1 API.

[Temikus]

Addressed the Kelos reviewer and API reviewer findings:

• v1alpha1 TaskSpawner round-trip (P2): fixed in c4a8840. runtimeClassName now survives in a kelos.dev/v1alpha2-runtime-class-name annotation, with round-trip tests. taskFromHub is unchanged since Task spec is immutable.
• Session agent can clear its own sandbox (P2): fixed in 312c79e. Added the Session table row and extended the trust note.
• DNS-1123 validation (P3): fixed in f456cbf, with integration coverage for invalid and dotted names.
• FailedCreate hint and Session pod test: added in 312c79e.

[Temikus]

@gjkim42 so sorry, I think I jumped the gun a little there - want me to remove that (the API bit specifically)?