Skip to content

fix: decouple skills from the privileged sandbox and default to restricted PSS #2334

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
QuentinBisson wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

fix: decouple skills from the privileged sandbox and default to restricted PSS #2334

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 19 additions & 5 deletions go/core/internal/controller/translator/agent/manifest_builder.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,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
}
Expand Down Expand Up @@ -399,7 +399,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 {
Expand All @@ -412,7 +411,6 @@ func buildSkillsRuntime(
return nil, nil, nil
}

*needCodeExecIsolation = true
*sharedEnv = append(*sharedEnv, corev1.EnvVar{
Name: env.KagentSkillsFolder.Name(),
Value: "/skills",
Expand Down Expand Up @@ -445,7 +443,7 @@ func buildSkillsRuntime(
spec.Skills.GitAuthSecretRef,
skills,
spec.Skills.InsecureSkipVerify,
manifestCtx.deployment.SecurityContext,
buildContainerSecurityContext(manifestCtx.deployment.SecurityContext, false),
initEnv,
getDefaultResources(initResources),
spec.Skills.ImagePullSecrets,
Expand Down Expand Up @@ -488,12 +486,28 @@ func buildContainerSecurityContext(
}

if !needCodeExecIsolation {
return nil
return restrictedSecurityContext()
}

return &corev1.SecurityContext{Privileged: new(true)}
}

// restrictedSecurityContext satisfies the Pod Security Standards "restricted"
// profile, so agents are admitted on clusters enforcing it without a
// per-agent securityContext override.
func restrictedSecurityContext() *corev1.SecurityContext {
return &corev1.SecurityContext{
AllowPrivilegeEscalation: new(false),
RunAsNonRoot: new(true),
Capabilities: &corev1.Capabilities{
Drop: []corev1.Capability{"ALL"},
},
SeccompProfile: &corev1.SeccompProfile{
Type: corev1.SeccompProfileTypeRuntimeDefault,
},
}
}

func buildPodTemplate(
manifestCtx manifestContext,
runtimeInputs *podRuntimeInputs,
Expand Down
131 changes: 123 additions & 8 deletions go/core/internal/controller/translator/agent/security_context_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,9 +195,11 @@ func TestSecurityContext_OnlyPodSecurityContext(t *testing.T) {
assert.Equal(t, int64(2000), *podSecurityContext.RunAsUser)
assert.Equal(t, int64(2000), *podSecurityContext.RunAsGroup)

// Container security context should be nil if not specified
// Container security context defaults to the restricted-PSS profile when
// not specified; the pod-level settings are preserved alongside.
containerSecurityContext := podTemplate.Spec.Containers[0].SecurityContext
assert.Nil(t, containerSecurityContext, "Container securityContext should be nil when not specified")
require.NotNil(t, containerSecurityContext)
assertRestrictedSecurityContext(t, containerSecurityContext)
}

func TestSecurityContext_OnlyContainerSecurityContext(t *testing.T) {
Expand Down Expand Up @@ -278,7 +280,11 @@ func TestSecurityContext_OnlyContainerSecurityContext(t *testing.T) {
// 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_SkillsDefaultNotPrivileged verifies that skills alone do
// not trigger Privileged=true: skills loading needs no elevated privileges, and
// only an explicit executeCodeBlocks opt-in requests the srt sandbox. Without a
// user-provided securityContext the containers get the restricted-PSS defaults.
func TestSecurityContext_SkillsDefaultNotPrivileged(t *testing.T) {
ctx := context.Background()

agent := &v1alpha2.Agent{
Expand Down Expand Up @@ -339,11 +345,120 @@ 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")
require.NotNil(t, containerSecurityContext)
assert.Nil(t, containerSecurityContext.Privileged, "skills alone must not trigger Privileged")
assertRestrictedSecurityContext(t, containerSecurityContext)

require.NotEmpty(t, podTemplate.Spec.InitContainers, "skills agent must have the skills-init container")
initSecurityContext := podTemplate.Spec.InitContainers[0].SecurityContext
require.NotNil(t, initSecurityContext, "skills-init must get the restricted defaults too")
assert.Nil(t, initSecurityContext.Privileged)
assertRestrictedSecurityContext(t, initSecurityContext)
}

func assertRestrictedSecurityContext(t *testing.T, securityContext *corev1.SecurityContext) {
t.Helper()
require.NotNil(t, securityContext.AllowPrivilegeEscalation)
assert.False(t, *securityContext.AllowPrivilegeEscalation)
require.NotNil(t, securityContext.RunAsNonRoot)
assert.True(t, *securityContext.RunAsNonRoot)
require.NotNil(t, securityContext.Capabilities)
assert.Equal(t, []corev1.Capability{"ALL"}, securityContext.Capabilities.Drop)
require.NotNil(t, securityContext.SeccompProfile)
assert.Equal(t, corev1.SeccompProfileTypeRuntimeDefault, securityContext.SeccompProfile.Type)
}

// TestSecurityContext_ExecuteCodeBlocksKeepsPrivileged verifies that the
// explicit executeCodeBlocks opt-in still requests the privileged srt sandbox
// when the user did not restrict the securityContext.
func TestSecurityContext_ExecuteCodeBlocksKeepsPrivileged(t *testing.T) {
ctx := context.Background()

executeCode := true
agent := &v1alpha2.Agent{
ObjectMeta: metav1.ObjectMeta{
Name: "test-agent",
Namespace: "test",
},
Spec: v1alpha2.AgentSpec{
Type: v1alpha2.AgentType_Declarative,
Declarative: &v1alpha2.DeclarativeAgentSpec{
SystemMessage: "Test agent",
ModelConfig: "test-model",
ExecuteCodeBlocks: &executeCode,
},
},
}

deployment := translateToDeployment(ctx, t, agent)
containerSecurityContext := deployment.Spec.Template.Spec.Containers[0].SecurityContext
require.NotNil(t, containerSecurityContext)
require.NotNil(t, containerSecurityContext.Privileged)
assert.True(t, *containerSecurityContext.Privileged, "executeCodeBlocks is the explicit sandbox opt-in")
}

// TestSecurityContext_DefaultIsRestricted verifies that a plain agent (no
// skills, no code execution, no user securityContext) renders restricted-PSS
// compliant container security contexts.
func TestSecurityContext_DefaultIsRestricted(t *testing.T) {
ctx := context.Background()

agent := &v1alpha2.Agent{
ObjectMeta: metav1.ObjectMeta{
Name: "test-agent",
Namespace: "test",
},
Spec: v1alpha2.AgentSpec{
Type: v1alpha2.AgentType_Declarative,
Declarative: &v1alpha2.DeclarativeAgentSpec{
SystemMessage: "Test agent",
ModelConfig: "test-model",
},
},
}

deployment := translateToDeployment(ctx, t, agent)
containerSecurityContext := deployment.Spec.Template.Spec.Containers[0].SecurityContext
require.NotNil(t, containerSecurityContext, "default securityContext must be rendered")
assert.Nil(t, containerSecurityContext.Privileged)
assertRestrictedSecurityContext(t, containerSecurityContext)
}

func translateToDeployment(ctx context.Context, t *testing.T, agent *v1alpha2.Agent) *appsv1.Deployment {
t.Helper()

modelConfig := &v1alpha2.ModelConfig{
ObjectMeta: metav1.ObjectMeta{
Name: "test-model",
Namespace: "test",
},
Spec: v1alpha2.ModelConfigSpec{
Provider: "OpenAI",
Model: "gpt-4o",
},
}

scheme := schemev1.Scheme
require.NoError(t, v1alpha2.AddToScheme(scheme))

kubeClient := fake.NewClientBuilder().
WithScheme(scheme).
WithObjects(agent, modelConfig).
Build()

translatorInstance := translator.NewAdkApiTranslator(kubeClient,
types.NamespacedName{Namespace: "test", Name: "test-model"}, nil, "", nil)

result, err := translator.TranslateAgent(ctx, translatorInstance, agent)
require.NoError(t, err)

for _, obj := range result.Manifest {
if dep, ok := obj.(*appsv1.Deployment); ok {
return dep
}
}
t.Fatal("no Deployment in translated manifest")
return nil
}

// TestSecurityContext_SkillsPSSRestricted verifies that when a user explicitly sets
Expand Down
12 changes: 12 additions & 0 deletions go/core/internal/controller/translator/agent/testdata/outputs/agent_with_a2a_config.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,18 @@
"memory": "384Mi"
}
},
"securityContext": {
"allowPrivilegeEscalation": false,
"capabilities": {
"drop": [
"ALL"
]
},
"runAsNonRoot": true,
"seccompProfile": {
"type": "RuntimeDefault"
}
},
"volumeMounts": [
{
"mountPath": "/config",
Expand Down
12 changes: 12 additions & 0 deletions ...ore/internal/controller/translator/agent/testdata/outputs/agent_with_allowed_headers.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,18 @@
"memory": "384Mi"
}
},
"securityContext": {
"allowPrivilegeEscalation": false,
"capabilities": {
"drop": [
"ALL"
]
},
"runAsNonRoot": true,
"seccompProfile": {
"type": "RuntimeDefault"
}
},
"volumeMounts": [
{
"mountPath": "/config",
Expand Down
12 changes: 12 additions & 0 deletions go/core/internal/controller/translator/agent/testdata/outputs/agent_with_context_config.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,18 @@
"memory": "684Mi"
}
},
"securityContext": {
"allowPrivilegeEscalation": false,
"capabilities": {
"drop": [
"ALL"
]
},
"runAsNonRoot": true,
"seccompProfile": {
"type": "RuntimeDefault"
}
},
"volumeMounts": [
{
"mountPath": "/config",
Expand Down
12 changes: 12 additions & 0 deletions ...ternal/controller/translator/agent/testdata/outputs/agent_with_cross_namespace_tools.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,18 @@
"memory": "384Mi"
}
},
"securityContext": {
"allowPrivilegeEscalation": false,
"capabilities": {
"drop": [
"ALL"
]
},
"runAsNonRoot": true,
"seccompProfile": {
"type": "RuntimeDefault"
}
},
"volumeMounts": [
{
"mountPath": "/config",
Expand Down
12 changes: 12 additions & 0 deletions go/core/internal/controller/translator/agent/testdata/outputs/agent_with_custom_sa.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,18 @@
"memory": "384Mi"
}
},
"securityContext": {
"allowPrivilegeEscalation": false,
"capabilities": {
"drop": [
"ALL"
]
},
"runAsNonRoot": true,
"seccompProfile": {
"type": "RuntimeDefault"
}
},
"volumeMounts": [
{
"mountPath": "/config",
Expand Down
12 changes: 12 additions & 0 deletions go/core/internal/controller/translator/agent/testdata/outputs/agent_with_default_sa.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,18 @@
"memory": "384Mi"
}
},
"securityContext": {
"allowPrivilegeEscalation": false,
"capabilities": {
"drop": [
"ALL"
]
},
"runAsNonRoot": true,
"seccompProfile": {
"type": "RuntimeDefault"
}
},
"volumeMounts": [
{
"mountPath": "/config",
Expand Down
12 changes: 12 additions & 0 deletions .../internal/controller/translator/agent/testdata/outputs/agent_with_embedding_provider.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,18 @@
"memory": "384Mi"
}
},
"securityContext": {
"allowPrivilegeEscalation": false,
"capabilities": {
"drop": [
"ALL"
]
},
"runAsNonRoot": true,
"seccompProfile": {
"type": "RuntimeDefault"
}
},
"volumeMounts": [
{
"mountPath": "/config",
Expand Down
12 changes: 12 additions & 0 deletions ...re/internal/controller/translator/agent/testdata/outputs/agent_with_extra_containers.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,18 @@
"memory": "384Mi"
}
},
"securityContext": {
"allowPrivilegeEscalation": false,
"capabilities": {
"drop": [
"ALL"
]
},
"runAsNonRoot": true,
"seccompProfile": {
"type": "RuntimeDefault"
}
},
"volumeMounts": [
{
"mountPath": "/config",
Expand Down
Loading
Loading