From 85a24a3203cd368bec9de30ae495dfd46e269b20 Mon Sep 17 00:00:00 2001 From: mesutoezdil Date: Thu, 11 Jun 2026 18:20:33 +0200 Subject: [PATCH 1/2] fix(controller): decouple skills from privileged sandbox skills init container handles loading, main container does not need privileged=true. only BashTool (cfg.GetExecuteCode) needs it. fixes #1997 Signed-off-by: mesutoezdil --- .../translator/agent/manifest_builder.go | 4 +--- .../translator/agent/security_context_test.go | 18 ++++++++---------- .../outputs/agent_with_git_skills.json | 3 --- .../testdata/outputs/agent_with_skills.json | 3 --- 4 files changed, 9 insertions(+), 19 deletions(-) diff --git a/go/core/internal/controller/translator/agent/manifest_builder.go b/go/core/internal/controller/translator/agent/manifest_builder.go index ebde454264..e8492581fb 100644 --- a/go/core/internal/controller/translator/agent/manifest_builder.go +++ b/go/core/internal/controller/translator/agent/manifest_builder.go @@ -297,7 +297,7 @@ func buildPodRuntime( volumeMounts = append(volumeMounts, manifestCtx.deployment.VolumeMounts...) needCodeExecIsolation := cfg != nil && cfg.GetExecuteCode() - initContainers, skillsInitCM, err := buildSkillsRuntime(manifestCtx, &sharedEnv, &volumes, &volumeMounts, &needCodeExecIsolation) + initContainers, skillsInitCM, err := buildSkillsRuntime(manifestCtx, &sharedEnv, &volumes, &volumeMounts) if err != nil { return nil, err } @@ -387,7 +387,6 @@ func buildSkillsRuntime( sharedEnv *[]corev1.EnvVar, volumes *[]corev1.Volume, volumeMounts *[]corev1.VolumeMount, - needCodeExecIsolation *bool, ) ([]corev1.Container, *corev1.ConfigMap, error) { spec := manifestCtx.agent.GetAgentSpec() if spec.Skills == nil { @@ -400,7 +399,6 @@ func buildSkillsRuntime( return nil, nil, nil } - *needCodeExecIsolation = true *sharedEnv = append(*sharedEnv, corev1.EnvVar{ Name: env.KagentSkillsFolder.Name(), Value: "/skills", diff --git a/go/core/internal/controller/translator/agent/security_context_test.go b/go/core/internal/controller/translator/agent/security_context_test.go index ae7149ea19..3f07ea1ec7 100644 --- a/go/core/internal/controller/translator/agent/security_context_test.go +++ b/go/core/internal/controller/translator/agent/security_context_test.go @@ -275,10 +275,10 @@ func TestSecurityContext_OnlyContainerSecurityContext(t *testing.T) { assert.Equal(t, int64(3000), *containerSecurityContext.RunAsGroup) } -// TestSecurityContext_SkillsDefaultPrivilegedSandbox verifies that when skills are -// configured and the user has NOT set any securityContext (i.e., no PSS restriction), -// the controller sets Privileged=true so that srt/bubblewrap can fully sandbox the BashTool. -func TestSecurityContext_SkillsDefaultPrivilegedSandbox(t *testing.T) { +// TestSecurityContext_SkillsNoPrivileged verifies that skills alone do NOT set Privileged=true. +// Skills are loaded by the init container; the main container does not need elevated privileges +// for skill loading. Only the BashTool sandbox (cfg.GetExecuteCode()) needs Privileged=true. +func TestSecurityContext_SkillsNoPrivileged(t *testing.T) { ctx := context.Background() agent := &v1alpha2.Agent{ @@ -294,7 +294,6 @@ func TestSecurityContext_SkillsDefaultPrivilegedSandbox(t *testing.T) { Declarative: &v1alpha2.DeclarativeAgentSpec{ SystemMessage: "Test agent", ModelConfig: "test-model", - // No Deployment.SecurityContext set — default behaviour }, }, } @@ -339,11 +338,10 @@ func TestSecurityContext_SkillsDefaultPrivilegedSandbox(t *testing.T) { podTemplate := &deployment.Spec.Template containerSecurityContext := podTemplate.Spec.Containers[0].SecurityContext - require.NotNil(t, containerSecurityContext, "SecurityContext should be created for sandbox") - // Without an explicit AllowPrivilegeEscalation=false constraint, skills trigger Privileged=true - // so that srt/bubblewrap can use kernel namespaces for full BashTool sandboxing. - require.NotNil(t, containerSecurityContext.Privileged, "Privileged should be set when no securityContext restriction") - assert.True(t, *containerSecurityContext.Privileged, "Privileged should be true for skills without PSS restrictions") + if containerSecurityContext != nil { + assert.True(t, containerSecurityContext.Privileged == nil || !*containerSecurityContext.Privileged, + "skills alone must not set Privileged=true") + } } // TestSecurityContext_SkillsPSSRestricted verifies that when a user explicitly sets diff --git a/go/core/internal/controller/translator/agent/testdata/outputs/agent_with_git_skills.json b/go/core/internal/controller/translator/agent/testdata/outputs/agent_with_git_skills.json index dd5efe2e78..c89d30c99e 100644 --- a/go/core/internal/controller/translator/agent/testdata/outputs/agent_with_git_skills.json +++ b/go/core/internal/controller/translator/agent/testdata/outputs/agent_with_git_skills.json @@ -243,9 +243,6 @@ "memory": "384Mi" } }, - "securityContext": { - "privileged": true - }, "volumeMounts": [ { "mountPath": "/config", diff --git a/go/core/internal/controller/translator/agent/testdata/outputs/agent_with_skills.json b/go/core/internal/controller/translator/agent/testdata/outputs/agent_with_skills.json index 400fa7fff1..cdab7b4d27 100644 --- a/go/core/internal/controller/translator/agent/testdata/outputs/agent_with_skills.json +++ b/go/core/internal/controller/translator/agent/testdata/outputs/agent_with_skills.json @@ -243,9 +243,6 @@ "memory": "684Mi" } }, - "securityContext": { - "privileged": true - }, "volumeMounts": [ { "mountPath": "/config", From 8a35f7f76ab8a1ba61d5db072a82de5c07eca0f1 Mon Sep 17 00:00:00 2001 From: mesutoezdil Date: Fri, 12 Jun 2026 23:41:16 +0200 Subject: [PATCH 2/2] test(translator): tighten skills security context assertions Reduce multi-line comments to single lines. Replace the weak if-not-nil guard with a direct assert.Nil so the test actually fails when the security context is unexpectedly set. Signed-off-by: mesutoezdil --- .../translator/agent/security_context_test.go | 16 +++------------- 1 file changed, 3 insertions(+), 13 deletions(-) diff --git a/go/core/internal/controller/translator/agent/security_context_test.go b/go/core/internal/controller/translator/agent/security_context_test.go index 3f07ea1ec7..c574745495 100644 --- a/go/core/internal/controller/translator/agent/security_context_test.go +++ b/go/core/internal/controller/translator/agent/security_context_test.go @@ -275,9 +275,7 @@ func TestSecurityContext_OnlyContainerSecurityContext(t *testing.T) { assert.Equal(t, int64(3000), *containerSecurityContext.RunAsGroup) } -// TestSecurityContext_SkillsNoPrivileged verifies that skills alone do NOT set Privileged=true. -// Skills are loaded by the init container; the main container does not need elevated privileges -// for skill loading. Only the BashTool sandbox (cfg.GetExecuteCode()) needs Privileged=true. +// TestSecurityContext_SkillsNoPrivileged verifies that skills alone do not produce any security context. func TestSecurityContext_SkillsNoPrivileged(t *testing.T) { ctx := context.Background() @@ -337,18 +335,10 @@ func TestSecurityContext_SkillsNoPrivileged(t *testing.T) { require.NotNil(t, deployment) podTemplate := &deployment.Spec.Template - containerSecurityContext := podTemplate.Spec.Containers[0].SecurityContext - if containerSecurityContext != nil { - assert.True(t, containerSecurityContext.Privileged == nil || !*containerSecurityContext.Privileged, - "skills alone must not set Privileged=true") - } + assert.Nil(t, podTemplate.Spec.Containers[0].SecurityContext, "skills must not set a security context") } -// TestSecurityContext_SkillsPSSRestricted verifies that when a user explicitly sets -// AllowPrivilegeEscalation=false (PSS Restricted profile), adding skills does NOT -// force Privileged=true — which Kubernetes rejects as an invalid combination. -// srt (Anthropic Sandbox Runtime) falls back to unprivileged user-namespace sandboxing -// on modern kernels (EKS, GKE) that have unprivileged_userns_clone enabled. +// TestSecurityContext_SkillsPSSRestricted verifies that AllowPrivilegeEscalation=false is preserved and skills do not override it. func TestSecurityContext_SkillsPSSRestricted(t *testing.T) { ctx := context.Background()