Skip to content

fix(adk): write materialized secret files with 0600 permissions#2011

Open
mesutoezdil wants to merge 1 commit into
kagent-dev:mainfrom
mesutoezdil:fix/materialize-file-permissions
Open

fix(adk): write materialized secret files with 0600 permissions#2011
mesutoezdil wants to merge 1 commit into
kagent-dev:mainfrom
mesutoezdil:fix/materialize-file-permissions

Conversation

@mesutoezdil

@mesutoezdil mesutoezdil commented Jun 13, 2026

Copy link
Copy Markdown
Contributor
  • config.json contains model API keys (e.g. OpenAI api_key)
  • kagent-token is a k8s service account token injected via KAGENT_TOKEN in substrate mode
  • Both were written with 0644 (world-readable), changed to 0600 (owner-only)
  • Added a permission assertion to TestMaterializeFromEnv to catch regressions

Copilot AI review requested due to automatic review settings June 13, 2026 07:19
@github-actions github-actions Bot added bug Something isn't working and removed bug Something isn't working labels Jun 13, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

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

This PR tightens default security behavior by removing privileged container settings that were previously introduced by “skills”, and by restricting on-disk config materialization to owner-only permissions.

Changes:

  • Remove default securityContext.privileged=true behavior for skills and update associated golden test outputs.
  • Simplify skills runtime manifest building by no longer toggling a shared “code exec isolation” flag from the skills path.
  • Write materialized config files with 0600 permissions and add tests asserting file modes.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
go/core/internal/controller/translator/agent/testdata/outputs/agent_with_skills.json Updates golden output to remove privileged securityContext for skills containers.
go/core/internal/controller/translator/agent/testdata/outputs/agent_with_git_skills.json Updates golden output to remove privileged securityContext for git-skills containers.
go/core/internal/controller/translator/agent/security_context_test.go Updates expectations so skills do not add a container SecurityContext; clarifies PSS Restricted behavior.
go/core/internal/controller/translator/agent/manifest_builder.go Removes needCodeExecIsolation mutation from buildSkillsRuntime and updates call site.
go/adk/pkg/config/config_materialize.go Writes config files with 0600 instead of 0644.
go/adk/pkg/config/config_loader_test.go Adds assertions that materialized files are created with 0600 permissions.

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

Comment thread go/core/internal/controller/translator/agent/manifest_builder.go
Comment thread go/adk/pkg/config/config_materialize.go
Comment thread go/adk/pkg/config/config_loader_test.go Outdated
@github-actions github-actions Bot added bug Something isn't working and removed bug Something isn't working labels Jun 13, 2026
@mesutoezdil mesutoezdil force-pushed the fix/materialize-file-permissions branch from 8b2ab91 to 07dd6b6 Compare June 13, 2026 07:24
config.json contains model API keys and kagent-token is a k8s
service account token. Writing them world-readable (0644) exposes
credentials to other processes in the container.

Signed-off-by: mesutoezdil <mesudozdil@gmail.com>
@mesutoezdil mesutoezdil force-pushed the fix/materialize-file-permissions branch from 07dd6b6 to 3226d4c Compare June 13, 2026 07:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants