Skip to content

Commit 476a03c

Browse files
skhedimEItanya
andauthored
fix: don't set Privileged when AllowPrivilegeEscalation is false for skills (#1551)
## Problem When an `Agent` spec includes a container `securityContext` with `allowPrivilegeEscalation: false` (PSS Restricted profile) **and** the agent has `spec.skills.refs` configured, the controller generates an invalid pod spec: ``` cannot set allowPrivilegeEscalation to false and privileged to true ``` Kubernetes rejects this combination at admission time, leaving the agent stuck in a reconciliation loop with all pods failing to start. ## Root cause `buildManifest()` sets `needSandbox = true` when skills are present (because skills use `BashTool` which calls `srt` → bubblewrap for sandboxing), then blindly sets `Privileged: true` on whatever `securityContext` is provided — including one that already has `AllowPrivilegeEscalation: false`. The securityContext merge did not check for this conflict before setting `Privileged: true`. ## Fix Add a helper `allowPrivilegeEscalationExplicitlyFalse()` that detects when the user has opted out of privilege escalation, and skip `Privileged: true` in that case. When `Privileged` is withheld, `srt` degrades gracefully: on modern kernels (EKS, GKE, AL2023 ≥ 5.10) that have unprivileged user namespaces enabled (`user.max_user_namespaces > 0`), bubblewrap can still create sandboxes using `clone(CLONE_NEWUSER)` without requiring a privileged seccomp profile. > **Note on seccomp**: The containerd `RuntimeDefault` seccomp profile blocks > `clone(CLONE_NEWUSER)` without `CAP_SYS_ADMIN`. Users who need full bwrap > sandbox execution (running bash scripts inside skills) must additionally > provide a custom `seccompProfile: Localhost` with a profile that allows > user namespace syscalls. This PR makes agent deployment possible for > PSS-Restricted namespaces; the seccomp tuning is an operational concern > separate from this bug. ## Behaviour matrix | Agent spec | needSandbox | Result | |---|---|---| | skills or executeCode, **no** custom securityContext | `true` | `Privileged: true` — full bwrap sandbox (unchanged) | | skills + `allowPrivilegeEscalation: false` | `true` | No `Privileged` — srt uses user-namespace mode | | executeCode + `allowPrivilegeEscalation: false` | `true` | No `Privileged` — srt uses user-namespace mode | | No skills, no executeCode | `false` | No securityContext created (unchanged) | ## Changes ### `go/core/internal/controller/translator/agent/adk_api_translator.go` - Add comment explaining *why* `needSandbox = true` for skills (BashTool → srt → bwrap) - Fix securityContext merge: guard `Privileged: true` with `allowPrivilegeEscalationExplicitlyFalse()` - Add `allowPrivilegeEscalationExplicitlyFalse()` helper ### `go/core/internal/controller/translator/agent/security_context_test.go` - Replace `TestSecurityContext_WithSandbox` (which tested an internally contradictory state) with two focused tests: - `TestSecurityContext_SkillsDefaultPrivilegedSandbox`: no custom securityContext → `Privileged: true` (default sandbox path) - `TestSecurityContext_SkillsPSSRestricted`: `allowPrivilegeEscalation: false` → no `Privileged`, original securityContext fields preserved intact ## Testing ```bash cd go go test -race -skip 'TestE2E.*' -v ./core/internal/controller/translator/agent/... ``` All existing tests pass. Two new tests cover both code paths. ## Related - PSS Restricted policy: https://kubernetes.io/docs/concepts/security/pod-security-standards/ - Bubblewrap user namespaces: https://github.com/containers/bubblewrap#usage Signed-off-by: skhedim <sebastien.khedim@gmail.com> Co-authored-by: Eitan Yarmush <eitan.yarmush@solo.io>
1 parent 213be60 commit 476a03c

2 files changed

Lines changed: 106 additions & 9 deletions

File tree

go/core/internal/controller/translator/agent/adk_api_translator.go

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -452,6 +452,10 @@ func (a *adkApiTranslator) buildManifest(
452452
Name: env.KagentSkillsFolder.Name(),
453453
Value: "/skills",
454454
}
455+
// Skills use the BashTool which calls srt (Anthropic Sandbox Runtime) → bubblewrap.
456+
// Mark that a sandbox is needed so Privileged is set when possible.
457+
// Exception: if the user explicitly set AllowPrivilegeEscalation=false (PSS Restricted),
458+
// we respect their security context and let srt fall back to user-namespace sandboxing.
455459
needSandbox = true
456460
volumes = append(volumes, corev1.Volume{
457461
Name: "kagent-skills",
@@ -515,12 +519,16 @@ func (a *adkApiTranslator) buildManifest(
515519
if dep.SecurityContext != nil {
516520
// Deep copy the user-provided security context
517521
securityContext = dep.SecurityContext.DeepCopy()
518-
// If sandbox is needed, ensure Privileged is set (may override user setting)
519-
if needSandbox {
522+
// Set Privileged for sandbox ONLY if it won't create an invalid securityContext.
523+
// Kubernetes rejects {Privileged:true, AllowPrivilegeEscalation:false} as contradictory.
524+
// When the user explicitly sets AllowPrivilegeEscalation=false (PSS Restricted namespace),
525+
// we respect their choice: srt will use unprivileged user-namespace sandboxing instead.
526+
// On modern kernels (EKS, GKE) unprivileged_userns_clone is enabled by default.
527+
if needSandbox && !allowPrivilegeEscalationExplicitlyFalse(securityContext) {
520528
securityContext.Privileged = new(true)
521529
}
522530
} else if needSandbox {
523-
// Only create security context if sandbox is needed
531+
// No user-provided securityContext: create one with Privileged for full sandbox
524532
securityContext = &corev1.SecurityContext{
525533
Privileged: new(true),
526534
}
@@ -1800,3 +1808,11 @@ func (a *adkApiTranslator) runPlugins(ctx context.Context, agent *v1alpha2.Agent
18001808
}
18011809
return errs
18021810
}
1811+
1812+
// allowPrivilegeEscalationExplicitlyFalse reports whether the security context
1813+
// has AllowPrivilegeEscalation explicitly set to false (PSS Restricted profile).
1814+
// This is used to detect when adding Privileged:true would create an invalid
1815+
// securityContext that Kubernetes refuses to admit.
1816+
func allowPrivilegeEscalationExplicitlyFalse(sc *corev1.SecurityContext) bool {
1817+
return sc != nil && sc.AllowPrivilegeEscalation != nil && !*sc.AllowPrivilegeEscalation
1818+
}

go/core/internal/controller/translator/agent/security_context_test.go

Lines changed: 87 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -275,7 +275,83 @@ func TestSecurityContext_OnlyContainerSecurityContext(t *testing.T) {
275275
assert.Equal(t, int64(3000), *containerSecurityContext.RunAsGroup)
276276
}
277277

278-
func TestSecurityContext_WithSandbox(t *testing.T) {
278+
// TestSecurityContext_SkillsDefaultPrivilegedSandbox verifies that when skills are
279+
// configured and the user has NOT set any securityContext (i.e., no PSS restriction),
280+
// the controller sets Privileged=true so that srt/bubblewrap can fully sandbox the BashTool.
281+
func TestSecurityContext_SkillsDefaultPrivilegedSandbox(t *testing.T) {
282+
ctx := context.Background()
283+
284+
agent := &v1alpha2.Agent{
285+
ObjectMeta: metav1.ObjectMeta{
286+
Name: "test-agent",
287+
Namespace: "test",
288+
},
289+
Spec: v1alpha2.AgentSpec{
290+
Type: v1alpha2.AgentType_Declarative,
291+
Skills: &v1alpha2.SkillForAgent{
292+
Refs: []string{"test-skill:latest"},
293+
},
294+
Declarative: &v1alpha2.DeclarativeAgentSpec{
295+
SystemMessage: "Test agent",
296+
ModelConfig: "test-model",
297+
// No Deployment.SecurityContext set — default behaviour
298+
},
299+
},
300+
}
301+
302+
modelConfig := &v1alpha2.ModelConfig{
303+
ObjectMeta: metav1.ObjectMeta{
304+
Name: "test-model",
305+
Namespace: "test",
306+
},
307+
Spec: v1alpha2.ModelConfigSpec{
308+
Provider: "OpenAI",
309+
Model: "gpt-4o",
310+
},
311+
}
312+
313+
scheme := schemev1.Scheme
314+
err := v1alpha2.AddToScheme(scheme)
315+
require.NoError(t, err)
316+
317+
kubeClient := fake.NewClientBuilder().
318+
WithScheme(scheme).
319+
WithObjects(agent, modelConfig).
320+
Build()
321+
322+
defaultModel := types.NamespacedName{
323+
Namespace: "test",
324+
Name: "test-model",
325+
}
326+
translatorInstance := translator.NewAdkApiTranslator(kubeClient, defaultModel, nil, "")
327+
328+
result, err := translatorInstance.TranslateAgent(ctx, agent)
329+
require.NoError(t, err)
330+
331+
var deployment *appsv1.Deployment
332+
for _, obj := range result.Manifest {
333+
if dep, ok := obj.(*appsv1.Deployment); ok {
334+
deployment = dep
335+
break
336+
}
337+
}
338+
require.NotNil(t, deployment)
339+
podTemplate := &deployment.Spec.Template
340+
341+
containerSecurityContext := podTemplate.Spec.Containers[0].SecurityContext
342+
require.NotNil(t, containerSecurityContext, "SecurityContext should be created for sandbox")
343+
// Without an explicit AllowPrivilegeEscalation=false constraint, skills trigger Privileged=true
344+
// so that srt/bubblewrap can use kernel namespaces for full BashTool sandboxing.
345+
require.NotNil(t, containerSecurityContext.Privileged, "Privileged should be set when no securityContext restriction")
346+
assert.True(t, *containerSecurityContext.Privileged, "Privileged should be true for skills without PSS restrictions")
347+
}
348+
349+
// TestSecurityContext_SkillsPSSRestricted verifies that when a user explicitly sets
350+
// AllowPrivilegeEscalation=false (PSS Restricted profile), adding skills does NOT
351+
// force Privileged=true — which Kubernetes rejects as an invalid combination.
352+
// srt (Anthropic Sandbox Runtime) falls back to unprivileged user-namespace sandboxing
353+
// on modern kernels (EKS, GKE) that have unprivileged_userns_clone enabled.
354+
func TestSecurityContext_SkillsPSSRestricted(t *testing.T) {
279355
ctx := context.Background()
280356

281357
agent := &v1alpha2.Agent{
@@ -294,8 +370,12 @@ func TestSecurityContext_WithSandbox(t *testing.T) {
294370
Deployment: &v1alpha2.DeclarativeDeploymentSpec{
295371
SharedDeploymentSpec: v1alpha2.SharedDeploymentSpec{
296372
SecurityContext: &corev1.SecurityContext{
297-
RunAsUser: new(int64(1000)),
298-
RunAsGroup: new(int64(1000)),
373+
RunAsUser: new(int64(1000)),
374+
RunAsGroup: new(int64(1000)),
375+
AllowPrivilegeEscalation: new(false),
376+
Capabilities: &corev1.Capabilities{
377+
Drop: []corev1.Capability{"ALL"},
378+
},
299379
},
300380
},
301381
},
@@ -342,9 +422,10 @@ func TestSecurityContext_WithSandbox(t *testing.T) {
342422
require.NotNil(t, deployment)
343423
podTemplate := &deployment.Spec.Template
344424

345-
// When sandbox is needed, Privileged should be set even if user provided securityContext
346425
containerSecurityContext := podTemplate.Spec.Containers[0].SecurityContext
347426
require.NotNil(t, containerSecurityContext)
348-
assert.True(t, *containerSecurityContext.Privileged, "Privileged should be true when sandbox is needed")
349-
assert.Equal(t, int64(1000), *containerSecurityContext.RunAsUser, "User-provided runAsUser should still be set")
427+
// AllowPrivilegeEscalation=false prevents Privileged=true (invalid Kubernetes combination)
428+
assert.Nil(t, containerSecurityContext.Privileged, "Privileged must not be set when AllowPrivilegeEscalation=false")
429+
assert.Equal(t, int64(1000), *containerSecurityContext.RunAsUser, "User-provided runAsUser should be preserved")
430+
assert.False(t, *containerSecurityContext.AllowPrivilegeEscalation, "AllowPrivilegeEscalation should be preserved as false")
350431
}

0 commit comments

Comments
 (0)