test(translator): tighten skills security context assertions

[mesutoezdil]

Skills mounting files via an init container do not require a privileged container. That requirement belongs to ExecuteCode (the code sandbox). The previous behavior set needCodeExecIsolation = true inside buildSkillsRuntime as a side effect, causing any agent with skills to get Privileged: true even when code execution was disabled. This change decouples the two: only cfg.GetExecuteCode() drives needCodeExecIsolation, and skills set up their own volumes without touching that flag.

Changes:

• Remove the needCodeExecIsolation pointer parameter from buildSkillsRuntime so skills no longer set the privileged flag as a side effect.
• Update golden outputs to remove the privileged security context from skills-based agent manifests.
• Replace the weak if containerSecurityContext != nil guard with assert.Nil so tests fail when a security context is unexpectedly present.
• Simplify multi-line comment blocks to single lines in TestSecurityContext_SkillsNoPrivileged and TestSecurityContext_SkillsPSSRestricted.

[Copilot]

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Updates the agent manifest translation and tests to avoid setting privileged container security context when skills are configured.

Changes:

• Removed securityContext.privileged=true from skills-related golden manifest outputs.
• Updated security context tests to assert that skills do not introduce a container security context by default.
• Refactored buildSkillsRuntime to no longer mutate needCodeExecIsolation via a pointer parameter.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
go/core/internal/controller/translator/agent/testdata/outputs/agent_with_skills.json	Updates golden output to remove privileged container security context.
go/core/internal/controller/translator/agent/testdata/outputs/agent_with_git_skills.json	Updates golden output to remove privileged container security context.
go/core/internal/controller/translator/agent/security_context_test.go	Adjusts test expectation: skills should not set a container security context.
go/core/internal/controller/translator/agent/manifest_builder.go	Removes needCodeExecIsolation mutation from buildSkillsRuntime API.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[EItanya]

Skills mounting files via an init container do not require a privileged container. That requirement belongs to ExecuteCode (the code sandbox). The previous behavior set needCodeExecIsolation = true inside buildSkillsRuntime as a side effect, causing any agent with skills to get Privileged: true even when code execution was disabled. This change decouples the two: only cfg.GetExecuteCode() drives needCodeExecIsolation, and skills set up their own volumes without touching that flag.

Skills can execute code though, that's part of the point

[mesutoezdil]

Skills mounting files via an init container do not require a privileged container. That requirement belongs to ExecuteCode (the code sandbox). The previous behavior set needCodeExecIsolation = true inside buildSkillsRuntime as a side effect, causing any agent with skills to get Privileged: true even when code execution was disabled. This change decouples the two: only cfg.GetExecuteCode() drives needCodeExecIsolation, and skills set up their own volumes without touching that flag.

Skills can execute code though, that's part of the point

hm, skills run trusted pythn packs installed via the init container, they execute inside the normal python runtime without needing a sandbox. and needcodeexecisolation drives the gvisor sandbox, which is for untrusted arbitrary code execution (executedode). the 2 are separate concerns?

[EItanya]

Skills mounting files via an init container do not require a privileged container. That requirement belongs to ExecuteCode (the code sandbox). The previous behavior set needCodeExecIsolation = true inside buildSkillsRuntime as a side effect, causing any agent with skills to get Privileged: true even when code execution was disabled. This change decouples the two: only cfg.GetExecuteCode() drives needCodeExecIsolation, and skills set up their own volumes without touching that flag.

Skills can execute code though, that's part of the point

hm, skills run trusted pythn packs installed via the init container, they execute inside the normal python runtime without needing a sandbox. and needcodeexecisolation drives the gvisor sandbox, which is for untrusted arbitrary code execution (executedode). the 2 are separate concerns?

Why don't they need a sandbox? Skills very often allow the agents to generate code files to run. They also use srt today. This will change with substrate, but that's how it works today.

[mesutoezdil]

Skills mounting files via an init container do not require a privileged container. That requirement belongs to ExecuteCode (the code sandbox). The previous behavior set needCodeExecIsolation = true inside buildSkillsRuntime as a side effect, causing any agent with skills to get Privileged: true even when code execution was disabled. This change decouples the two: only cfg.GetExecuteCode() drives needCodeExecIsolation, and skills set up their own volumes without touching that flag.

Skills can execute code though, that's part of the point

hm, skills run trusted pythn packs installed via the init container, they execute inside the normal python runtime without needing a sandbox. and needcodeexecisolation drives the gvisor sandbox, which is for untrusted arbitrary code execution (executedode). the 2 are separate concerns?

Why don't they need a sandbox? Skills very often allow the agents to generate code files to run. They also use srt today. This will change with substrate, but that's how it works today.

y r right, missed the srt dependency. should i close this and reopen after substrate replaces srt?