From b4623951f4f6fa5c56dd0494fa3c900fd9b88f8b Mon Sep 17 00:00:00 2001 From: acherifi Date: Thu, 8 Oct 2026 10:18:22 +0200 Subject: [PATCH] feat(skills): private Git skill sources via egress gateway credentials Add an optional `authorizationFrom` Secret key reference to AgentTemplate `skills[].source.git`. The compiler binds the key to the repository hostname as a Substrate egress gateway credential and the runtime sends a placeholder `Authorization` header for the gateway to replace, so the token never enters the Actor, its configuration, or a snapshot. A gateway credential only applies once an Actor has egress, which Substrate activates after readiness and never grants the golden Actor (#2604). Runtimes therefore fetch standalone skills before the first task of a session Actor instead of at startup: the Go ADK, Claude and Codex runtimes call Materialization.EnsureSkills per task, and a fetch failure fails that task with a `skills_unavailable` message instead of crashing the golden Actor. Plugins keep their startup fetch and reject `authorizationFrom`. Refs #2604, #2815 Signed-off-by: acherifi --- docs/architecture/credential-injection.md | 58 +++++++++- go/adk/cmd/main.go | 6 +- go/adk/pkg/a2a/executor.go | 17 +++ go/adk/pkg/a2a/executor_test.go | 45 ++++++++ go/adk/pkg/agent/agent.go | 5 +- go/adk/pkg/config/agentplugins.go | 72 +++++++----- go/adk/pkg/config/agentplugins_test.go | 11 +- go/api/agentplugin/types.go | 8 +- .../crd/bases/api.kagent.dev_agents.yaml | 57 ++++++++++ .../bases/api.kagent.dev_agenttemplates.yaml | 57 ++++++++++ go/api/v1alpha3/agenttemplate_types.go | 11 ++ go/api/v1alpha3/common_types.go | 15 +++ go/api/v1alpha3/configuration_crd_cel_test.go | 41 +++++++ go/api/v1alpha3/zz_generated.deepcopy.go | 22 +++- go/core/internal/skillsinit/git.go | 24 +++- go/core/internal/skillsinit/git_auth_test.go | 104 ++++++++++++++++++ go/core/internal/skillsinit/git_test.go | 2 +- go/core/internal/translator/credentials.go | 13 +++ .../internal/translator/credentials_test.go | 72 ++++++++++++ go/core/internal/translator/skills.go | 7 +- go/core/pkg/agentplugins/materialize.go | 62 ++++++++--- go/core/pkg/agentplugins/materialize_test.go | 50 +++++++++ go/harness/claude/internal/adapter/adapter.go | 3 + .../claude/internal/adapter/adapter_test.go | 11 +- go/harness/claude/internal/driver/process.go | 8 ++ .../claude/internal/driver/skills_test.go | 32 ++++++ go/harness/codex/internal/adapter/adapter.go | 4 +- go/harness/codex/internal/driver/process.go | 8 ++ .../codex/internal/driver/skills_test.go | 32 ++++++ .../templates/api.kagent.dev_agents.yaml | 57 ++++++++++ .../api.kagent.dev_agenttemplates.yaml | 57 ++++++++++ 31 files changed, 910 insertions(+), 61 deletions(-) create mode 100644 go/core/internal/skillsinit/git_auth_test.go create mode 100644 go/harness/claude/internal/driver/skills_test.go create mode 100644 go/harness/codex/internal/driver/skills_test.go diff --git a/docs/architecture/credential-injection.md b/docs/architecture/credential-injection.md index 3e29ad1f5c..1d9bc712bc 100644 --- a/docs/architecture/credential-injection.md +++ b/docs/architecture/credential-injection.md @@ -56,12 +56,13 @@ overrides of these variables are rejected. | Gemini API key | `x-goog-api-key: ` | | Bedrock bearer token | `authorization: Bearer ` | | RemoteMCPServer Secret-backed header | Configured header; Secret contains its full value | +| AgentTemplate `skills[].source.git.authorizationFrom` | `authorization`; Secret contains its full value | Provider endpoint overrides determine the allowed HTTP(S) origin. Egress rules match its scheme, DNS name, and port; credential bindings remain scoped to the hostname and header. Different credentials for the same hostname and header -are rejected, including conflicts between models, memory embeddings, and MCP -servers. Use distinct DNS names for +are rejected, including conflicts between models, memory embeddings, MCP +servers, and Git skill sources. Use distinct DNS names for origins requiring different credentials. IP-address destinations are unsupported by Substrate egress policies. @@ -76,6 +77,59 @@ Caller-token passthrough retains its existing behavior. A passthrough model cannot share a hostname with static gateway credentials, which would override the caller's authentication. +## Private Git skill sources + +A standalone skill may come from a private HTTPS Git repository. The Secret, +in the AgentTemplate's namespace, holds the complete `Authorization` header +value for the repository host: + +```yaml +apiVersion: v1 +kind: Secret +metadata: + name: skills-auth +stringData: + # GitHub: base64 of "x-access-token:". Other forges may accept "Bearer ". + authorization: Basic eC1hY2Nlc3MtdG9rZW46PHRva2VuPg== +--- +apiVersion: api.kagent.dev/v1alpha3 +kind: AgentTemplate +metadata: + name: reviewer +spec: + skills: + - name: review + source: + git: + url: https://github.com/acme/private-skills + commit: 0123456789abcdef0123456789abcdef01234567 + authorizationFrom: + name: skills-auth + key: authorization +``` + +The compiler allows the repository origin and binds the Secret key to its +hostname. The runtime fetches with a placeholder `Authorization` header, which +the gateway replaces; it never holds the token. The binding covers every +request the Actor makes to that hostname, including requests made by its tools, +so use a read-only token scoped to the skill repositories. `authorizationFrom` +requires an `https` URL and is rejected on `plugins[]`, which are still +fetched while the Actor starts. + +### When skills are fetched + +Actor networking starts only after the runtime is ready, and the golden Actor +that produces the template snapshot runs without an egress policy. Runtimes +therefore fetch standalone skills (Git, OCI, or S3, public or private) before +the first task of each session Actor, not at startup. Consequences: + +- The golden snapshot contains neither a credential nor any standalone skill + content. Session snapshots contain the fetched files, never the credential. +- The first task of a session waits for the fetch. Later tasks reuse the files. +- A fetch failure fails that task with a `skills_unavailable: …` status message + and an error log line in the runtime; the next task retries the remaining + skills. It no longer prevents the golden snapshot. + ## Deferred API fields The source retains commented declarations for Harness and SandboxTemplate diff --git a/go/adk/cmd/main.go b/go/adk/cmd/main.go index dd1e6a730d..2121e52e37 100644 --- a/go/adk/cmd/main.go +++ b/go/adk/cmd/main.go @@ -68,14 +68,15 @@ func run(logger *slog.Logger, host, port, configDir string) error { if err != nil { return fmt.Errorf("load agent config from %s (model configuration is required): %w", configDir, err) } - if err := config.MaterializeAgentPlugins( + ensureSkills, err := config.MaterializeAgentPlugins( logging.IntoContext(context.Background(), logger), agentConfig, config.AgentPluginPaths{ Packages: defaultPluginPackagesRoot, Skills: defaultSkillsRoot, Data: defaultPluginDataRoot, }, - ); err != nil { + ) + if err != nil { return fmt.Errorf("materialize Agent Plugins: %w", err) } logger.Info("loaded agent config", "config_dir", configDir) @@ -187,6 +188,7 @@ func run(logger *slog.Logger, host, port, configDir string) error { Logger: logger, Output: agentConfig.Output, Flush: providers.ForceFlush, + EnsureSkills: ensureSkills, }) if err != nil { return fmt.Errorf("create A2A executor: %w", err) diff --git a/go/adk/pkg/a2a/executor.go b/go/adk/pkg/a2a/executor.go index 51d4e7f641..becc69d62a 100644 --- a/go/adk/pkg/a2a/executor.go +++ b/go/adk/pkg/a2a/executor.go @@ -41,6 +41,9 @@ type KAgentExecutorConfig struct { Logger *slog.Logger Output *apiadk.OutputConfig Flush func(context.Context) error + // EnsureSkills fetches deferred standalone skills before a task runs. + // A failure fails the task and is retried by the next one. + EnsureSkills func(context.Context) error } // KAgentExecutor keeps kagent's request/session glue around the upstream ADK @@ -52,6 +55,7 @@ type KAgentExecutor struct { logger *slog.Logger structuredOutputEnabled bool flush func(context.Context) error + ensureSkills func(context.Context) error } type structuredOutput struct { @@ -109,6 +113,7 @@ func NewKAgentExecutor(cfg KAgentExecutorConfig) (*KAgentExecutor, error) { logger: cfg.Logger.With("component", "kagent-executor"), structuredOutputEnabled: output != nil, flush: cfg.Flush, + ensureSkills: cfg.EnsureSkills, }, nil } @@ -264,6 +269,18 @@ func (e *KAgentExecutor) Execute(ctx context.Context, reqCtx *a2asrv.ExecutorCon "user_id", userID, ) + // Standalone skills need Actor egress, which only exists once a session + // Actor runs, so they are fetched here instead of at startup. + if e.ensureSkills != nil { + if err := e.ensureSkills(ctx); err != nil { + e.logger.ErrorContext(ctx, "standalone skills are unavailable", "task_id", reqCtx.TaskID, "error", err) + message := a2atype.NewMessageForTask(a2atype.MessageRoleAgent, reqCtx, a2atype.NewTextPart("skills_unavailable: "+err.Error())) + apia2a.SetTimelinePosition(message, time.Now()) + yield(a2atype.NewStatusUpdateEvent(reqCtx, a2atype.TaskStateFailed, message), nil) + return + } + } + // Run our own session management before upstream executor runs its prepareSession function. // This ensures that we create a session that contains metadata like x-kagent-source, // and the upstream executor will find this session already exists and skip creation. diff --git a/go/adk/pkg/a2a/executor_test.go b/go/adk/pkg/a2a/executor_test.go index 37e80f492f..5c0b837d0b 100644 --- a/go/adk/pkg/a2a/executor_test.go +++ b/go/adk/pkg/a2a/executor_test.go @@ -2,6 +2,7 @@ package a2a import ( "context" + "errors" "iter" "maps" "slices" @@ -249,6 +250,50 @@ func TestKAgentExecutor_StampsStructuredOutputFailureMessage(t *testing.T) { } } +func TestKAgentExecutor_FailsTaskWhenSkillsAreUnavailable(t *testing.T) { + reqCtx := &a2asrv.ExecutorContext{ + TaskID: "task-1", ContextID: "ctx-1", + Message: a2atype.NewMessage(a2atype.MessageRoleUser, a2atype.NewTextPart("hello")), + } + builtin := &recordingExecutor{} + fetchErr := errors.New(`materialize skill "review": exit status 128`) + executor := &KAgentExecutor{ + builtin: builtin, logger: slog.New(slog.DiscardHandler), + ensureSkills: func(context.Context) error { return fetchErr }, + } + var events []a2atype.Event + for event, err := range executor.Execute(context.Background(), reqCtx) { + if err != nil { + t.Fatalf("Execute() error = %v", err) + } + events = append(events, event) + } + if builtin.message != nil { + t.Fatal("the agent ran without its skills") + } + if len(events) != 1 { + t.Fatalf("events = %#v, want one failed status", events) + } + got, ok := events[0].(*a2atype.TaskStatusUpdateEvent) + if !ok || got.Status.State != a2atype.TaskStateFailed || got.Status.Message == nil { + t.Fatalf("status update = %#v, want failed status", events[0]) + } + text, _ := got.Status.Message.Parts[0].Content.(a2atype.Text) + if want := "skills_unavailable: " + fetchErr.Error(); string(text) != want { + t.Fatalf("failure message = %q, want %q", text, want) + } + + executor.ensureSkills = func(context.Context) error { return nil } + for _, err := range executor.Execute(context.Background(), reqCtx) { + if err != nil { + t.Fatalf("Execute() error = %v", err) + } + } + if builtin.message == nil { + t.Fatal("the agent did not run once its skills were present") + } +} + func TestKAgentExecutor_PreservesContentBearingLastChunk(t *testing.T) { reqCtx := &a2asrv.ExecutorContext{TaskID: "task-1", ContextID: "ctx-1"} reqCtx.Message = a2atype.NewMessage(a2atype.MessageRoleUser, a2atype.NewTextPart("hi")) diff --git a/go/adk/pkg/agent/agent.go b/go/adk/pkg/agent/agent.go index c1c4768dc0..c048f3e14c 100644 --- a/go/adk/pkg/agent/agent.go +++ b/go/adk/pkg/agent/agent.go @@ -71,7 +71,10 @@ func createGoogleADKAgent(ctx context.Context, agentConfig *adk.AgentConfig, age if err != nil { return nil, fmt.Errorf("failed to load skills: %w", err) } - if len(skills) > 0 { + // Standalone skills are fetched before the first task, after the agent + // is built; the skill toolset lists the directory on every request. + deferredSkills := agentConfig.AgentPlugins != nil && len(agentConfig.AgentPlugins.Skills) > 0 + if len(skills) > 0 || deferredSkills { executionTools, err := tools.NewSkillExecutionTools(skillsDirectory) if err != nil { return nil, fmt.Errorf("failed to create skill execution tools: %w", err) diff --git a/go/adk/pkg/config/agentplugins.go b/go/adk/pkg/config/agentplugins.go index 578f5fe803..3c38d5b21e 100644 --- a/go/adk/pkg/config/agentplugins.go +++ b/go/adk/pkg/config/agentplugins.go @@ -18,37 +18,55 @@ type AgentPluginPaths struct { } // MaterializeAgentPlugins materializes plugins independently for each agent -// and adds their skills and MCP servers to the ADK runtime configuration. -func MaterializeAgentPlugins(ctx context.Context, agentConfig *adk.AgentConfig, paths AgentPluginPaths) error { - if agentConfig.AgentPlugins != nil { - materialization, err := agentplugins.Materialize(ctx, *agentConfig.AgentPlugins, agentplugins.Paths{ - Packages: paths.Packages, - Skills: paths.Skills, - }) - if err != nil { - return fmt.Errorf("materialize agent plugins: %w", err) - } - if len(materialization.ClaudeFormatPluginRoots()) > 0 { - return fmt.Errorf("plugin is Claude-format; only the Agent Plugins format (plugin.json at the plugin root) is supported here") +// and adds their skills and MCP servers to the ADK runtime configuration. The +// returned function fetches every agent's standalone skills; the caller runs it +// before each task because the fetch needs Actor egress, which startup lacks. +func MaterializeAgentPlugins(ctx context.Context, agentConfig *adk.AgentConfig, paths AgentPluginPaths) (func(context.Context) error, error) { + var materializations []agentplugins.Materialization + var visit func(*adk.AgentConfig, AgentPluginPaths) error + visit = func(agentConfig *adk.AgentConfig, paths AgentPluginPaths) error { + if agentConfig.AgentPlugins != nil { + materialization, err := agentplugins.Materialize(ctx, *agentConfig.AgentPlugins, agentplugins.Paths{ + Packages: paths.Packages, + Skills: paths.Skills, + }) + if err != nil { + return fmt.Errorf("materialize agent plugins: %w", err) + } + if len(materialization.ClaudeFormatPluginRoots()) > 0 { + return fmt.Errorf("plugin is Claude-format; only the Agent Plugins format (plugin.json at the plugin root) is supported here") + } + mcpConfig, err := agentplugins.LoadMCP(ctx, materialization, paths.Data) + if err != nil { + return fmt.Errorf("load agent plugin MCP configuration: %w", err) + } + addMCPConfig(agentConfig, mcpConfig) + agentConfig.SkillsDirectory = materialization.SkillsDirectory + materializations = append(materializations, materialization) } - mcpConfig, err := agentplugins.LoadMCP(ctx, materialization, paths.Data) - if err != nil { - return fmt.Errorf("load agent plugin MCP configuration: %w", err) + for i, child := range agentConfig.SubAgents { + childRoot := filepath.Join("subagents", fmt.Sprintf("%d", i)) + if err := visit(child, AgentPluginPaths{ + Packages: filepath.Join(paths.Packages, childRoot), + Skills: filepath.Join(paths.Skills, childRoot), + Data: filepath.Join(paths.Data, childRoot), + }); err != nil { + return fmt.Errorf("materialize sub-agent %q: %w", child.Name, err) + } } - addMCPConfig(agentConfig, mcpConfig) - agentConfig.SkillsDirectory = materialization.SkillsDirectory + return nil } - for i, child := range agentConfig.SubAgents { - childRoot := filepath.Join("subagents", fmt.Sprintf("%d", i)) - if err := MaterializeAgentPlugins(ctx, child, AgentPluginPaths{ - Packages: filepath.Join(paths.Packages, childRoot), - Skills: filepath.Join(paths.Skills, childRoot), - Data: filepath.Join(paths.Data, childRoot), - }); err != nil { - return fmt.Errorf("materialize sub-agent %q: %w", child.Name, err) - } + if err := visit(agentConfig, paths); err != nil { + return nil, err } - return nil + return func(ctx context.Context) error { + for _, materialization := range materializations { + if err := materialization.EnsureSkills(ctx); err != nil { + return err + } + } + return nil + }, nil } func addMCPConfig(agentConfig *adk.AgentConfig, mcpConfig agentplugins.MCPConfig) { diff --git a/go/adk/pkg/config/agentplugins_test.go b/go/adk/pkg/config/agentplugins_test.go index c7e660111d..215b4a8e79 100644 --- a/go/adk/pkg/config/agentplugins_test.go +++ b/go/adk/pkg/config/agentplugins_test.go @@ -35,7 +35,14 @@ func TestMaterializeAgentPluginsIsolatesSubagentSkills(t *testing.T) { t.Fatal(err) } } - if err := MaterializeAgentPlugins(context.Background(), agentConfig, paths); err != nil { + ensureSkills, err := MaterializeAgentPlugins(context.Background(), agentConfig, paths) + if err != nil { + t.Fatal(err) + } + if _, err := os.Stat(filepath.Join(agentConfig.SkillsDirectory, "root")); !os.IsNotExist(err) { + t.Fatalf("standalone skill materialized during startup: %v", err) + } + if err := ensureSkills(context.Background()); err != nil { t.Fatal(err) } if agentConfig.SkillsDirectory == agentConfig.SubAgents[0].SkillsDirectory { @@ -64,7 +71,7 @@ func TestMaterializeAgentPluginsRejectsClaudeFormatPlugin(t *testing.T) { } source := agentplugin.Source{Git: &agentplugin.GitSource{URL: "unused", Commit: strings.Repeat("a", 40)}} agentConfig := &adk.AgentConfig{AgentPlugins: &agentplugin.Resources{Plugins: []agentplugin.Bundle{{Source: source}}}} - if err := MaterializeAgentPlugins(context.Background(), agentConfig, paths); err == nil { + if _, err := MaterializeAgentPlugins(context.Background(), agentConfig, paths); err == nil { t.Fatal("MaterializeAgentPlugins() accepted a Claude-format plugin") } } diff --git a/go/api/agentplugin/types.go b/go/api/agentplugin/types.go index 87d7f1cb73..2259fe5440 100644 --- a/go/api/agentplugin/types.go +++ b/go/api/agentplugin/types.go @@ -2,8 +2,8 @@ // standalone skills and Agent Plugin packages. package agentplugin -// Resources describes the skill resources a Harness must materialize before -// starting an agent. +// Resources describes the skill resources a Harness materializes: plugins +// before starting an agent, standalone skills before its first task. type Resources struct { Skills []Skill `json:"skills,omitempty"` Plugins []Bundle `json:"plugins,omitempty"` @@ -33,6 +33,10 @@ type Source struct { type GitSource struct { URL string `json:"url"` Commit string `json:"commit"` + // GatewayAuthorization reports that the egress gateway owns this host's + // Authorization header. The runtime sends a placeholder value for the + // gateway to replace; it never holds the credential. + GatewayAuthorization bool `json:"gateway_authorization,omitempty"` } // S3Source identifies one immutable S3 object version. diff --git a/go/api/config/crd/bases/api.kagent.dev_agents.yaml b/go/api/config/crd/bases/api.kagent.dev_agents.yaml index 84023a3624..a8cde5991c 100644 --- a/go/api/config/crd/bases/api.kagent.dev_agents.yaml +++ b/go/api/config/crd/bases/api.kagent.dev_agents.yaml @@ -386,6 +386,30 @@ spec: description: GitArtifact identifies immutable content at a full Git commit ID. properties: + authorizationFrom: + description: |- + AuthorizationFrom references a key in a Secret in the AgentTemplate's + namespace whose value is the complete HTTP Authorization header for the + repository host, for example + `Basic ` for GitHub or `Bearer TOKEN`. + The egress gateway adds the header to the runtime's requests to that + host; the value never enters the runtime, its configuration, or a + snapshot. Supported for skills only. + properties: + key: + maxLength: 253 + minLength: 1 + pattern: ^[-._a-zA-Z0-9]+$ + type: string + name: + maxLength: 253 + minLength: 1 + pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$ + type: string + required: + - key + - name + type: object commit: pattern: ^([0-9a-fA-F]{40}|[0-9a-fA-F]{64})$ type: string @@ -397,6 +421,9 @@ spec: - commit - url type: object + x-kubernetes-validations: + - message: authorizationFrom requires an https URL + rule: '!has(self.authorizationFrom) || self.url.startsWith(''https://'')' oci: description: OCI is a digest-pinned image reference. pattern: ^[^[:space:]@]+@sha256:[0-9a-f]{64}$ @@ -418,6 +445,9 @@ spec: required: - source type: object + x-kubernetes-validations: + - message: authorizationFrom is supported only for skills[].source.git + rule: '!has(self.source.git) || !has(self.source.git.authorizationFrom)' maxItems: 20 type: array promptTemplate: @@ -499,6 +529,30 @@ spec: description: GitArtifact identifies immutable content at a full Git commit ID. properties: + authorizationFrom: + description: |- + AuthorizationFrom references a key in a Secret in the AgentTemplate's + namespace whose value is the complete HTTP Authorization header for the + repository host, for example + `Basic ` for GitHub or `Bearer TOKEN`. + The egress gateway adds the header to the runtime's requests to that + host; the value never enters the runtime, its configuration, or a + snapshot. Supported for skills only. + properties: + key: + maxLength: 253 + minLength: 1 + pattern: ^[-._a-zA-Z0-9]+$ + type: string + name: + maxLength: 253 + minLength: 1 + pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$ + type: string + required: + - key + - name + type: object commit: pattern: ^([0-9a-fA-F]{40}|[0-9a-fA-F]{64})$ type: string @@ -510,6 +564,9 @@ spec: - commit - url type: object + x-kubernetes-validations: + - message: authorizationFrom requires an https URL + rule: '!has(self.authorizationFrom) || self.url.startsWith(''https://'')' oci: description: OCI is a digest-pinned image reference. pattern: ^[^[:space:]@]+@sha256:[0-9a-f]{64}$ diff --git a/go/api/config/crd/bases/api.kagent.dev_agenttemplates.yaml b/go/api/config/crd/bases/api.kagent.dev_agenttemplates.yaml index ebe57de679..46ee123e75 100644 --- a/go/api/config/crd/bases/api.kagent.dev_agenttemplates.yaml +++ b/go/api/config/crd/bases/api.kagent.dev_agenttemplates.yaml @@ -141,6 +141,30 @@ spec: description: GitArtifact identifies immutable content at a full Git commit ID. properties: + authorizationFrom: + description: |- + AuthorizationFrom references a key in a Secret in the AgentTemplate's + namespace whose value is the complete HTTP Authorization header for the + repository host, for example + `Basic ` for GitHub or `Bearer TOKEN`. + The egress gateway adds the header to the runtime's requests to that + host; the value never enters the runtime, its configuration, or a + snapshot. Supported for skills only. + properties: + key: + maxLength: 253 + minLength: 1 + pattern: ^[-._a-zA-Z0-9]+$ + type: string + name: + maxLength: 253 + minLength: 1 + pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$ + type: string + required: + - key + - name + type: object commit: pattern: ^([0-9a-fA-F]{40}|[0-9a-fA-F]{64})$ type: string @@ -152,6 +176,9 @@ spec: - commit - url type: object + x-kubernetes-validations: + - message: authorizationFrom requires an https URL + rule: '!has(self.authorizationFrom) || self.url.startsWith(''https://'')' oci: description: OCI is a digest-pinned image reference. pattern: ^[^[:space:]@]+@sha256:[0-9a-f]{64}$ @@ -172,6 +199,9 @@ spec: required: - source type: object + x-kubernetes-validations: + - message: authorizationFrom is supported only for skills[].source.git + rule: '!has(self.source.git) || !has(self.source.git.authorizationFrom)' maxItems: 20 type: array promptTemplate: @@ -252,6 +282,30 @@ spec: description: GitArtifact identifies immutable content at a full Git commit ID. properties: + authorizationFrom: + description: |- + AuthorizationFrom references a key in a Secret in the AgentTemplate's + namespace whose value is the complete HTTP Authorization header for the + repository host, for example + `Basic ` for GitHub or `Bearer TOKEN`. + The egress gateway adds the header to the runtime's requests to that + host; the value never enters the runtime, its configuration, or a + snapshot. Supported for skills only. + properties: + key: + maxLength: 253 + minLength: 1 + pattern: ^[-._a-zA-Z0-9]+$ + type: string + name: + maxLength: 253 + minLength: 1 + pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$ + type: string + required: + - key + - name + type: object commit: pattern: ^([0-9a-fA-F]{40}|[0-9a-fA-F]{64})$ type: string @@ -263,6 +317,9 @@ spec: - commit - url type: object + x-kubernetes-validations: + - message: authorizationFrom requires an https URL + rule: '!has(self.authorizationFrom) || self.url.startsWith(''https://'')' oci: description: OCI is a digest-pinned image reference. pattern: ^[^[:space:]@]+@sha256:[0-9a-f]{64}$ diff --git a/go/api/v1alpha3/agenttemplate_types.go b/go/api/v1alpha3/agenttemplate_types.go index b306126311..fee2cbc3ef 100644 --- a/go/api/v1alpha3/agenttemplate_types.go +++ b/go/api/v1alpha3/agenttemplate_types.go @@ -102,6 +102,7 @@ type AgentTemplateSkill struct { } // GitArtifact identifies immutable content at a full Git commit ID. +// +kubebuilder:validation:XValidation:rule="!has(self.authorizationFrom) || self.url.startsWith('https://')",message="authorizationFrom requires an https URL" type GitArtifact struct { // +kubebuilder:validation:Pattern=`^https?://[^[:space:]]+$` // +kubebuilder:validation:MinLength=1 @@ -110,6 +111,15 @@ type GitArtifact struct { // +kubebuilder:validation:Pattern=`^([0-9a-fA-F]{40}|[0-9a-fA-F]{64})$` // +required Commit string `json:"commit"` + // AuthorizationFrom references a key in a Secret in the AgentTemplate's + // namespace whose value is the complete HTTP Authorization header for the + // repository host, for example + // `Basic ` for GitHub or `Bearer TOKEN`. + // The egress gateway adds the header to the runtime's requests to that + // host; the value never enters the runtime, its configuration, or a + // snapshot. Supported for skills only. + // +optional + AuthorizationFrom *SecretKeyReference `json:"authorizationFrom,omitempty"` } // S3Object identifies one immutable S3 object version. @@ -157,6 +167,7 @@ type ArtifactSource struct { } // PluginBundle selects Agent Skills from one immutable Agent Plugins package. +// +kubebuilder:validation:XValidation:rule="!has(self.source.git) || !has(self.source.git.authorizationFrom)",message="authorizationFrom is supported only for skills[].source.git" type PluginBundle struct { // +required Source ArtifactSource `json:"source"` diff --git a/go/api/v1alpha3/common_types.go b/go/api/v1alpha3/common_types.go index aa2bb49a1b..0b61b3eba6 100644 --- a/go/api/v1alpha3/common_types.go +++ b/go/api/v1alpha3/common_types.go @@ -39,6 +39,21 @@ type ConfigMapKeyReference struct { Key string `json:"key"` } +// SecretKeyReference selects a key of a Secret in the referencing object's +// namespace. +type SecretKeyReference struct { + // +kubebuilder:validation:MinLength=1 + // +kubebuilder:validation:MaxLength=253 + // +kubebuilder:validation:Pattern=`^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$` + // +required + Name string `json:"name"` + // +kubebuilder:validation:MinLength=1 + // +kubebuilder:validation:MaxLength=253 + // +kubebuilder:validation:Pattern=`^[-._a-zA-Z0-9]+$` + // +required + Key string `json:"key"` +} + // FromNamespaces specifies namespace from which references to this resource are allowed. // This follows the same pattern as Gateway API's cross-namespace route attachment. // See: https://gateway-api.sigs.k8s.io/guides/multiple-ns/#cross-namespace-route-attachment diff --git a/go/api/v1alpha3/configuration_crd_cel_test.go b/go/api/v1alpha3/configuration_crd_cel_test.go index be23e3a308..e6ec9c3717 100644 --- a/go/api/v1alpha3/configuration_crd_cel_test.go +++ b/go/api/v1alpha3/configuration_crd_cel_test.go @@ -404,6 +404,47 @@ func TestConfigurationCRDValidation(t *testing.T) { } } }) + + t.Run("git skill authorizationFrom", func(t *testing.T) { + commit := strings.Repeat("a", 40) + git := func(url string, authorization map[string]any) map[string]any { + source := map[string]any{"url": url, "commit": commit} + if authorization != nil { + source["authorizationFrom"] = authorization + } + return map[string]any{"git": source} + } + secretKey := map[string]any{"name": "skills-auth", "key": "authorization"} + for _, tc := range []struct { + name string + path []string + entry map[string]any + wantReject string + }{ + {name: "skill", path: []string{"spec", "skills"}, entry: map[string]any{"name": "review", "source": git("https://github.com/acme/skills", secretKey)}}, + {name: "public-skill", path: []string{"spec", "skills"}, entry: map[string]any{"name": "review", "source": git("http://git.example.com/acme/skills", nil)}}, + {name: "plain-http", path: []string{"spec", "skills"}, entry: map[string]any{"name": "review", "source": git("http://git.example.com/acme/skills", secretKey)}, wantReject: "authorizationFrom requires an https URL"}, + {name: "missing-key", path: []string{"spec", "skills"}, entry: map[string]any{"name": "review", "source": git("https://github.com/acme/skills", map[string]any{"name": "skills-auth"})}, wantReject: "key: Required value"}, + {name: "empty-key", path: []string{"spec", "skills"}, entry: map[string]any{"name": "review", "source": git("https://github.com/acme/skills", map[string]any{"name": "skills-auth", "key": ""})}, wantReject: "authorizationFrom.key"}, + {name: "namespace", path: []string{"spec", "skills"}, entry: map[string]any{"name": "review", "source": git("https://github.com/acme/skills", map[string]any{"name": "skills-auth", "key": "authorization", "namespace": "other"})}, wantReject: "unknown field"}, + {name: "plugin", path: []string{"spec", "plugins"}, entry: map[string]any{"source": git("https://github.com/acme/plugin", secretKey)}, wantReject: "authorizationFrom is supported only for skills[].source.git"}, + } { + t.Run(tc.name, func(t *testing.T) { + data, err := runtime.DefaultUnstructuredConverter.ToUnstructured(validAgentTemplate(namespace, "git-auth-"+tc.name, nil)) + require.NoError(t, err) + object := &unstructured.Unstructured{Object: data} + object.SetAPIVersion(GroupVersion.String()) + object.SetKind("AgentTemplate") + require.NoError(t, unstructured.SetNestedSlice(data, []any{tc.entry}, tc.path...)) + err = cl.Create(ctx, object, &ctrlclient.CreateOptions{FieldValidation: metav1.FieldValidationStrict}) + if tc.wantReject != "" { + require.ErrorContains(t, err, tc.wantReject) + return + } + require.NoError(t, err) + }) + } + }) } func sandboxTemplateForValidation(namespace, name string, mutate func(*SandboxTemplateSpec)) *SandboxTemplate { diff --git a/go/api/v1alpha3/zz_generated.deepcopy.go b/go/api/v1alpha3/zz_generated.deepcopy.go index 59a3d9ddf2..e8e916f944 100644 --- a/go/api/v1alpha3/zz_generated.deepcopy.go +++ b/go/api/v1alpha3/zz_generated.deepcopy.go @@ -375,7 +375,7 @@ func (in *ArtifactSource) DeepCopyInto(out *ArtifactSource) { if in.Git != nil { in, out := &in.Git, &out.Git *out = new(GitArtifact) - **out = **in + (*in).DeepCopyInto(*out) } if in.Bucket != nil { in, out := &in.Bucket, &out.Bucket @@ -614,6 +614,11 @@ func (in *GeminiVertexAIConfig) DeepCopy() *GeminiVertexAIConfig { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *GitArtifact) DeepCopyInto(out *GitArtifact) { *out = *in + if in.AuthorizationFrom != nil { + in, out := &in.AuthorizationFrom, &out.AuthorizationFrom + *out = new(SecretKeyReference) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new GitArtifact. @@ -1676,6 +1681,21 @@ func (in *SandboxTemplateWorkload) DeepCopy() *SandboxTemplateWorkload { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *SecretKeyReference) DeepCopyInto(out *SecretKeyReference) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new SecretKeyReference. +func (in *SecretKeyReference) DeepCopy() *SecretKeyReference { + if in == nil { + return nil + } + out := new(SecretKeyReference) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *SecretReference) DeepCopyInto(out *SecretReference) { *out = *in diff --git a/go/core/internal/skillsinit/git.go b/go/core/internal/skillsinit/git.go index 35ddca9278..2aa8aba607 100644 --- a/go/core/internal/skillsinit/git.go +++ b/go/core/internal/skillsinit/git.go @@ -48,9 +48,17 @@ func CloneGit(ref GitRef) error { return nil } +// GatewayAuthorizationPlaceholder is the Authorization header value sent for +// gateway-authorized sources. Substrate's egress gateway replaces only headers +// a request already carries, so the runtime must send one; the gateway +// overwrites the whole value with the referenced Secret. +const GatewayAuthorizationPlaceholder = "kagent-credential-injected" + // CloneGitCommit fetches only one immutable commit instead of cloning the -// repository's complete history. -func CloneGitCommit(url, commit, destination string) error { +// repository's complete history. When gatewayAuthorization is set, every +// request carries a placeholder Authorization header for the egress gateway to +// replace; the credential itself never reaches this process. +func CloneGitCommit(url, commit, destination string, gatewayAuthorization bool) error { if !immutableGitCommit.MatchString(commit) { return fmt.Errorf("git commit must be a full SHA") } @@ -63,7 +71,13 @@ func CloneGitCommit(url, commit, destination string) error { if err := runGitIn(destination, "remote", "add", "origin", url); err != nil { return err } - if err := runGitIn(destination, "fetch", "--depth", "1", "origin", commit); err != nil { + fetch := []string{"fetch", "--depth", "1", "origin", commit} + if gatewayAuthorization { + // Passed per command rather than written to .git/config so the + // checkout records nothing about how it was authorized. + fetch = append([]string{"-c", "http.extraHeader=Authorization: " + GatewayAuthorizationPlaceholder}, fetch...) + } + if err := runGitIn(destination, fetch...); err != nil { return err } return runGitIn(destination, "checkout", "--detach", "FETCH_HEAD") @@ -80,7 +94,9 @@ func runGitIn(dir string, args ...string) error { } cmd.Stdout = os.Stdout cmd.Stderr = os.Stderr - cmd.Env = os.Environ() + // A runtime has no terminal: an authentication failure must fail the + // fetch instead of waiting for a username prompt. + cmd.Env = append(os.Environ(), "GIT_TERMINAL_PROMPT=0") return cmd.Run() } diff --git a/go/core/internal/skillsinit/git_auth_test.go b/go/core/internal/skillsinit/git_auth_test.go new file mode 100644 index 0000000000..d04cea2328 --- /dev/null +++ b/go/core/internal/skillsinit/git_auth_test.go @@ -0,0 +1,104 @@ +package skillsinit + +import ( + "net/http" + "net/http/cgi" + "net/http/httptest" + "net/http/httputil" + "net/url" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +const testAuthorization = "Basic eC1hY2Nlc3MtdG9rZW46c2VjcmV0" // x-access-token:secret + +// privateGitServer serves one repository over Git smart HTTP and rejects every +// request without the expected Authorization header, like a private forge. +func privateGitServer(t *testing.T) (serverURL, commit string) { + t.Helper() + root := t.TempDir() + repository := filepath.Join(root, "skills.git") + require.NoError(t, os.MkdirAll(filepath.Join(repository, "review"), 0o755)) + gitIn(t, repository, "init", "-q") + gitIn(t, repository, "config", "user.email", "t@example.com") + gitIn(t, repository, "config", "user.name", "t") + require.NoError(t, os.WriteFile(filepath.Join(repository, "review", "SKILL.md"), []byte("# Review"), 0o644)) + gitIn(t, repository, "add", ".") + gitIn(t, repository, "commit", "-qm", "skill") + commit = gitOut(t, repository, "rev-parse", "HEAD") + gitIn(t, repository, "commit", "-qm", "later", "--allow-empty") + + execPath, err := exec.Command("git", "--exec-path").Output() + require.NoError(t, err) + backend := &cgi.Handler{ + Path: filepath.Join(strings.TrimSpace(string(execPath)), "git-http-backend"), + Env: []string{"GIT_PROJECT_ROOT=" + root, "GIT_HTTP_EXPORT_ALL=1"}, + } + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Header.Get("Authorization") != testAuthorization { + w.Header().Set("WWW-Authenticate", `Basic realm="private"`) + http.Error(w, "authentication required", http.StatusUnauthorized) + return + } + backend.ServeHTTP(w, r) + })) + t.Cleanup(server.Close) + return server.URL + "/skills.git", commit +} + +// fakeGateway forwards to target and, like Substrate's egress gateway, +// overwrites the Authorization header only on requests that already carry one. +func fakeGateway(t *testing.T, target string) string { + t.Helper() + upstream, err := url.Parse(target) + require.NoError(t, err) + proxy := httputil.NewSingleHostReverseProxy(&url.URL{Scheme: upstream.Scheme, Host: upstream.Host}) + gateway := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Header.Get("Authorization") != "" { + r.Header.Set("Authorization", testAuthorization) + } + proxy.ServeHTTP(w, r) + })) + t.Cleanup(gateway.Close) + gatewayURL, err := url.Parse(gateway.URL) + require.NoError(t, err) + upstream.Host = gatewayURL.Host + return upstream.String() +} + +func TestCloneGitCommitThroughCredentialGateway(t *testing.T) { + // Isolate from any developer credential helper or global configuration. + t.Setenv("GIT_CONFIG_GLOBAL", os.DevNull) + t.Setenv("GIT_CONFIG_NOSYSTEM", "1") + repository, commit := privateGitServer(t) + throughGateway := fakeGateway(t, repository) + + t.Run("placeholder is replaced by the gateway", func(t *testing.T) { + destination := filepath.Join(t.TempDir(), "skill") + require.NoError(t, CloneGitCommit(throughGateway, commit, destination, true)) + content, err := os.ReadFile(filepath.Join(destination, "review", "SKILL.md")) + require.NoError(t, err) + assert.Equal(t, "# Review", string(content)) + assert.Equal(t, commit, gitOut(t, destination, "rev-parse", "HEAD")) + config, err := os.ReadFile(filepath.Join(destination, ".git", "config")) + require.NoError(t, err) + assert.NotContains(t, string(config), GatewayAuthorizationPlaceholder) + assert.NotContains(t, string(config), "extraHeader") + }) + + t.Run("without a placeholder the gateway injects nothing", func(t *testing.T) { + err := CloneGitCommit(throughGateway, commit, filepath.Join(t.TempDir(), "skill"), false) + require.Error(t, err) + }) + + t.Run("the placeholder is not a credential", func(t *testing.T) { + err := CloneGitCommit(repository, commit, filepath.Join(t.TempDir(), "skill"), true) + require.Error(t, err) + }) +} diff --git a/go/core/internal/skillsinit/git_test.go b/go/core/internal/skillsinit/git_test.go index f1b486645d..cff92edeb8 100644 --- a/go/core/internal/skillsinit/git_test.go +++ b/go/core/internal/skillsinit/git_test.go @@ -46,7 +46,7 @@ func Test_applySubPath_rejectsNonDir(t *testing.T) { } func TestCloneGitCommitRejectsMutableRef(t *testing.T) { - err := CloneGitCommit("https://example.com/repository.git", "main", t.TempDir()) + err := CloneGitCommit("https://example.com/repository.git", "main", t.TempDir(), false) require.ErrorContains(t, err, "full SHA") } diff --git a/go/core/internal/translator/credentials.go b/go/core/internal/translator/credentials.go index 6872047f52..dcf762fa93 100644 --- a/go/core/internal/translator/credentials.go +++ b/go/core/internal/translator/credentials.go @@ -48,6 +48,19 @@ func CompileCredentials(input *HarnessInput, extraModels []*ResolvedModelConfig, } } } + if agent.Template != nil { + for _, skill := range agent.Template.Spec.Skills { + git := skill.Source.Git + if git == nil || git.AuthorizationFrom == nil { + continue + } + // The runtime sends a placeholder Authorization header that the + // gateway replaces with the Secret value. + if err := bind(git.URL, "authorization", "", agent.Template.Namespace, git.AuthorizationFrom.Name, git.AuthorizationFrom.Key); err != nil { + return err + } + } + } for _, child := range agent.Shared { if err := visit(child.Agent); err != nil { return err diff --git a/go/core/internal/translator/credentials_test.go b/go/core/internal/translator/credentials_test.go index fb52d53aa4..24a2c8b9af 100644 --- a/go/core/internal/translator/credentials_test.go +++ b/go/core/internal/translator/credentials_test.go @@ -1,9 +1,12 @@ package translator import ( + "encoding/json" + "strings" "testing" "github.com/kagent-dev/kagent/go/api/v1alpha3" + "github.com/kagent-dev/kagent/go/core/internal/egress" "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -79,6 +82,75 @@ func TestCompileCredentialsPreservesPassthrough(t *testing.T) { require.ErrorContains(t, err, "cannot combine caller-token passthrough") } +func privateSkillTemplate(skills ...v1alpha3.AgentTemplateSkill) *TemplateConfiguration { + return &TemplateConfiguration{Name: "reviewer", Namespace: "team", Spec: v1alpha3.AgentTemplateSpec{Skills: skills}} +} + +func privateGitSkill(name, url, secret string) v1alpha3.AgentTemplateSkill { + return v1alpha3.AgentTemplateSkill{Name: name, Source: v1alpha3.ArtifactSource{Git: &v1alpha3.GitArtifact{ + URL: url, Commit: strings.Repeat("a", 40), + AuthorizationFrom: &v1alpha3.SecretKeyReference{Name: secret, Key: "authorization"}, + }}} +} + +func TestCompilePrivateGitSkillBindsGatewayCredentialAndEgress(t *testing.T) { + template := privateSkillTemplate( + privateGitSkill("review", "https://github.com/acme/private-skills", "skills-auth"), + v1alpha3.AgentTemplateSkill{Name: "public", Source: v1alpha3.ArtifactSource{Git: &v1alpha3.GitArtifact{ + URL: "https://gitlab.example.com/acme/public-skills", Commit: strings.Repeat("b", 40), + }}}, + ) + resources, destinations, err := CompileSkillResources(template) + require.NoError(t, err) + require.Equal(t, []string{"https://github.com:443", "https://gitlab.example.com:443"}, destinations) + require.True(t, resources.Skills[0].Source.Git.GatewayAuthorization) + require.False(t, resources.Skills[1].Source.Git.GatewayAuthorization) + raw, err := json.Marshal(resources) + require.NoError(t, err) + require.NotContains(t, string(raw), "skills-auth", "the runtime contract must not name the Secret") + + input := credentialInput(v1alpha3.ModelConfigSpec{}) + input.Root.Template = template + environment, bindings, err := CompileCredentials(input, nil, nil) + require.NoError(t, err) + require.Empty(t, environment, "a git credential needs no runtime environment") + require.Equal(t, []egress.Credential{{ + Hostname: "github.com", Header: "authorization", URI: "ate-secret://k8s.io/default/team/skills-auth/authorization", + }}, bindings) +} + +func TestCompilePrivateGitSkillCredentialsFromSharedChild(t *testing.T) { + input := credentialInput(v1alpha3.ModelConfigSpec{}) + child := credentialInput(v1alpha3.ModelConfigSpec{}).Root + child.Template = privateSkillTemplate(privateGitSkill("review", "https://git.example.com/acme/skills", "child-auth")) + child.Template.Namespace = "child-team" + input.Root.Shared = []AgentInputBinding{{Agent: child}} + _, bindings, err := CompileCredentials(input, nil, nil) + require.NoError(t, err) + require.Len(t, bindings, 1) + require.Equal(t, "ate-secret://k8s.io/default/child-team/child-auth/authorization", bindings[0].URI) +} + +func TestCompilePrivateGitSkillRejectsConflictsAndPlugins(t *testing.T) { + input := credentialInput(v1alpha3.ModelConfigSpec{}) + input.Root.Template = privateSkillTemplate( + privateGitSkill("one", "https://github.com/acme/one", "first"), + privateGitSkill("two", "https://github.com/acme/two", "second"), + ) + _, _, err := CompileCredentials(input, nil, nil) + require.ErrorContains(t, err, "conflicting credentials") + + input.Root.Template = privateSkillTemplate(privateGitSkill("one", "https://user@github.com/acme/one", "first")) + _, _, err = CompileCredentials(input, nil, nil) + require.ErrorContains(t, err, "without user information") + + template := &TemplateConfiguration{Namespace: "team", Spec: v1alpha3.AgentTemplateSpec{Plugins: []v1alpha3.PluginBundle{{ + Source: privateGitSkill("unused", "https://github.com/acme/plugin", "auth").Source, + }}}} + _, _, err = CompileSkillResources(template) + require.ErrorContains(t, err, "supported only for skills") +} + func credentialInput(spec v1alpha3.ModelConfigSpec) *HarnessInput { resolved := &ResolvedModelConfig{Config: &v1alpha3.ModelConfig{ObjectMeta: metav1.ObjectMeta{Namespace: "team"}, Spec: spec}} if spec.Foundry != nil { diff --git a/go/core/internal/translator/skills.go b/go/core/internal/translator/skills.go index 39b6d8fd8b..72b91e9a87 100644 --- a/go/core/internal/translator/skills.go +++ b/go/core/internal/translator/skills.go @@ -28,6 +28,11 @@ func CompileSkillResources(template *TemplateConfiguration) (agentplugin.Resourc egress = appendArtifactSourceDestination(egress, source) } for _, plugin := range template.Spec.Plugins { + // Plugins are materialized while the Actor starts, before it has + // egress, so a gateway credential could never apply to them. + if plugin.Source.Git != nil && plugin.Source.Git.AuthorizationFrom != nil { + return agentplugin.Resources{}, nil, NewValidationError("authorizationFrom is supported only for skills[].source.git") + } for _, name := range plugin.Skills { if _, exists := selected[name]; exists { return agentplugin.Resources{}, nil, NewValidationError("duplicate skill name %q", name) @@ -47,7 +52,7 @@ func CompileSkillResources(template *TemplateConfiguration) (agentplugin.Resourc func compileArtifactSource(source v1alpha3.ArtifactSource) agentplugin.Source { result := agentplugin.Source{OCI: source.OCI, Path: source.Path} if source.Git != nil { - result.Git = &agentplugin.GitSource{URL: source.Git.URL, Commit: source.Git.Commit} + result.Git = &agentplugin.GitSource{URL: source.Git.URL, Commit: source.Git.Commit, GatewayAuthorization: source.Git.AuthorizationFrom != nil} } if source.Bucket != nil { result.S3 = &agentplugin.S3Source{ diff --git a/go/core/pkg/agentplugins/materialize.go b/go/core/pkg/agentplugins/materialize.go index 89e4a9db10..420d86450e 100644 --- a/go/core/pkg/agentplugins/materialize.go +++ b/go/core/pkg/agentplugins/materialize.go @@ -12,6 +12,7 @@ import ( "regexp" "slices" "strings" + "sync" "github.com/kagent-dev/kagent/go/api/agentplugin" "github.com/kagent-dev/kagent/go/core/internal/skillsinit" @@ -38,6 +39,7 @@ type Paths struct { type Materialization struct { SkillsDirectory string plugins []materializedPlugin + skills *standaloneSkills } // MCPConfig contains MCP servers resolved from materialized plugins. @@ -80,15 +82,54 @@ func (m Materialization) ClaudeFormatPluginRoots() []string { return roots } -// Materialize fetches Agent Plugin resources and copies explicitly selected -// skills into their runtime directory; Claude-format plugins are kept whole. -// It does not load runtime configuration. +// Materialize fetches Agent Plugin packages and copies their selected skills +// into the runtime directory; Claude-format plugins are kept whole. Standalone +// skills are only validated here and fetched by EnsureSkills. It does not load +// runtime configuration. func Materialize(ctx context.Context, resources agentplugin.Resources, paths Paths) (Materialization, error) { plugins, err := materializeResources(ctx, resources, paths) if err != nil { return Materialization{}, err } - return Materialization{SkillsDirectory: paths.Skills, plugins: plugins}, nil + return Materialization{ + SkillsDirectory: paths.Skills, plugins: plugins, + skills: &standaloneSkills{skills: slices.Clone(resources.Skills), paths: paths}, + }, nil +} + +// standaloneSkills fetches standalone skills once. A failed fetch keeps the +// skills already copied and is resumed by the next call. +type standaloneSkills struct { + mu sync.Mutex + skills []agentplugin.Skill + fetched int + paths Paths +} + +// EnsureSkills fetches standalone skills into SkillsDirectory and returns once +// all of them are present. Call it before each task, never during startup: a +// Substrate Actor has no egress until it is ready, and a golden snapshot is +// taken before any session's egress policy and gateway credentials apply. It +// is safe for concurrent use and does no work after the first success. +func (m Materialization) EnsureSkills(ctx context.Context) error { + if m.skills == nil { + return nil + } + s := m.skills + s.mu.Lock() + defer s.mu.Unlock() + for ; s.fetched < len(s.skills); s.fetched++ { + skill := s.skills[s.fetched] + root := filepath.Join(s.paths.Packages, fmt.Sprintf("standalone-%d", s.fetched)) + sourceRoot, err := fetchSource(ctx, skill.Source, root, "SKILL.md") + if err != nil { + return fmt.Errorf("materialize skill %q: %w", skill.Name, err) + } + if err := copySkill(sourceRoot, filepath.Join(s.paths.Skills, skill.Name)); err != nil { + return fmt.Errorf("materialize skill %q: %w", skill.Name, err) + } + } + return nil } // LoadMCP resolves standard MCP configuration from a materialization. The @@ -126,17 +167,6 @@ func materializeResources(ctx context.Context, resources agentplugin.Resources, return nil, fmt.Errorf("create package directory: %w", err) } - for i, skill := range resources.Skills { - root := filepath.Join(paths.Packages, fmt.Sprintf("standalone-%d", i)) - sourceRoot, err := fetchSource(ctx, skill.Source, root, "SKILL.md") - if err != nil { - return nil, fmt.Errorf("materialize skill %q: %w", skill.Name, err) - } - if err := copySkill(sourceRoot, filepath.Join(paths.Skills, skill.Name)); err != nil { - return nil, fmt.Errorf("materialize skill %q: %w", skill.Name, err) - } - } - pluginNames := make(map[string]struct{}) plugins := make([]materializedPlugin, 0, len(resources.Plugins)) for i, plugin := range resources.Plugins { @@ -231,7 +261,7 @@ func fetchSource(ctx context.Context, source agentplugin.Source, destination str return "", err } case source.Git != nil: - if err := skillsinit.CloneGitCommit(source.Git.URL, source.Git.Commit, destination); err != nil { + if err := skillsinit.CloneGitCommit(source.Git.URL, source.Git.Commit, destination, source.Git.GatewayAuthorization); err != nil { return "", err } case source.S3 != nil: diff --git a/go/core/pkg/agentplugins/materialize_test.go b/go/core/pkg/agentplugins/materialize_test.go index 3697a737fb..89be159a2e 100644 --- a/go/core/pkg/agentplugins/materialize_test.go +++ b/go/core/pkg/agentplugins/materialize_test.go @@ -98,6 +98,56 @@ func TestFetchSourceDoesNotReuseIncompleteMaterialization(t *testing.T) { } } +func TestMaterializeDefersStandaloneSkillsUntilEnsureSkills(t *testing.T) { + repository, commit := commitFiles(t, map[string]string{"SKILL.md": "# Review"}) + root := t.TempDir() + paths := Paths{Packages: filepath.Join(root, "packages"), Skills: filepath.Join(root, "skills")} + resources := agentplugin.Resources{Skills: []agentplugin.Skill{ + {Name: "review", Source: agentplugin.Source{Git: &agentplugin.GitSource{URL: repository, Commit: commit}}}, + {Name: "later", Source: agentplugin.Source{Git: &agentplugin.GitSource{URL: filepath.Join(root, "missing"), Commit: commit}}}, + }} + materialization, err := Materialize(context.Background(), resources, paths) + if err != nil { + t.Fatalf("Materialize() fetched a standalone skill during startup: %v", err) + } + if _, err := os.Stat(filepath.Join(paths.Skills, "review")); !os.IsNotExist(err) { + t.Fatalf("standalone skill materialized before EnsureSkills: %v", err) + } + + if err := materialization.EnsureSkills(context.Background()); err == nil || !strings.Contains(err.Error(), `skill "later"`) { + t.Fatalf("EnsureSkills() error = %v, want the unreachable skill", err) + } + if content, err := os.ReadFile(filepath.Join(paths.Skills, "review", "SKILL.md")); err != nil || string(content) != "# Review" { + t.Fatalf("first skill = %q, %v", content, err) + } + // The next task resumes at the failed skill without refetching the first. + if err := os.RemoveAll(filepath.Join(paths.Packages, "standalone-0")); err != nil { + t.Fatal(err) + } + later := filepath.Join(paths.Packages, "standalone-1") + if err := os.MkdirAll(later, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(later, "SKILL.md"), []byte("# Later"), 0o644); err != nil { + t.Fatal(err) + } + if err := materialization.EnsureSkills(context.Background()); err != nil { + t.Fatal(err) + } + if _, err := os.Stat(filepath.Join(paths.Packages, "standalone-0")); !os.IsNotExist(err) { + t.Fatalf("EnsureSkills() refetched a skill it already copied: %v", err) + } + if content, err := os.ReadFile(filepath.Join(paths.Skills, "later", "SKILL.md")); err != nil || string(content) != "# Later" { + t.Fatalf("second skill = %q, %v", content, err) + } +} + +func TestEnsureSkillsOnZeroMaterialization(t *testing.T) { + if err := (Materialization{}).EnsureSkills(context.Background()); err != nil { + t.Fatal(err) + } +} + func TestMaterializeCopiesSelectionsWithoutLoadingPluginMCP(t *testing.T) { root := t.TempDir() pluginRoot := filepath.Join(root, "plugins", "plugin-0") diff --git a/go/harness/claude/internal/adapter/adapter.go b/go/harness/claude/internal/adapter/adapter.go index d332007030..949f0b4516 100644 --- a/go/harness/claude/internal/adapter/adapter.go +++ b/go/harness/claude/internal/adapter/adapter.go @@ -58,6 +58,7 @@ func New(ctx context.Context, input Input) (*driver.ProcessDriver, error) { } } var pluginDirs []string + var ensureSkills func(context.Context) error if cfg.SkillResources != nil { skillsDir := filepath.Join(skillRoot, ".claude", "skills") if err := utils.EnsurePrivateDir(skillsDir); err != nil { @@ -71,6 +72,7 @@ func New(ctx context.Context, input Input) (*driver.ProcessDriver, error) { return nil, fmt.Errorf("materialize Claude skills: %w", err) } pluginDirs = materialized.ClaudeFormatPluginRoots() + ensureSkills = materialized.EnsureSkills } environment := setEnvironment(input.Environment, config.ClaudeConfigDirEnvName, claudeDir) // The native runtime inherits the compiled identity through the standard @@ -147,6 +149,7 @@ func New(ctx context.Context, input Input) (*driver.ProcessDriver, error) { SkillRoot: skillRoot, PluginDirs: pluginDirs, Environment: environment, MaxEventBytes: cfg.MaxEventBytes, MaxStderrBytes: cfg.MaxStderrBytes, InterruptGrace: cfg.InterruptGrace(), AwaitTelemetry: awaitTelemetry, + EnsureSkills: ensureSkills, }), nil } diff --git a/go/harness/claude/internal/adapter/adapter_test.go b/go/harness/claude/internal/adapter/adapter_test.go index 571febfba0..c7138b2413 100644 --- a/go/harness/claude/internal/adapter/adapter_test.go +++ b/go/harness/claude/internal/adapter/adapter_test.go @@ -55,6 +55,7 @@ func TestNewMaterializesSkillsAndMCPConfig(t *testing.T) { } cfg := config.Production("claude-test", "help") cfg.StrictVersion = false + cfg.ClaudeExecutable = filepath.Join(t.TempDir(), "missing-claude") cfg.SkillResources = &agentplugin.Resources{Skills: []agentplugin.Skill{{ Name: "review", Source: agentplugin.Source{Git: &agentplugin.GitSource{URL: "unused", Commit: strings.Repeat("a", 40)}}, }}} @@ -71,8 +72,16 @@ func TestNewMaterializesSkillsAndMCPConfig(t *testing.T) { if err != nil { t.Fatal(err) } + skillPath := filepath.Join(skillRoot, ".claude", "skills", "review", "SKILL.md") + if _, err := os.Stat(skillPath); !os.IsNotExist(err) { + t.Fatalf("standalone skill materialized during startup: %v", err) + } + // The first turn fetches skills before it starts Claude, which is absent here. + if _, err := runner.Run(context.Background(), runtime.Turn{Prompt: "test"}, nil); err == nil { + t.Fatal("Run() started a missing Claude executable") + } for path, want := range map[string]string{ - filepath.Join(skillRoot, ".claude", "skills", "review", "SKILL.md"): "# Review", + skillPath: "# Review", filepath.Join(ephemeralDir, "mcp.json"): `{"mcpServers":{"tools":{"type":"http","url":"https://mcp.example.com/mcp"}}}`, } { contents, err := os.ReadFile(path) diff --git a/go/harness/claude/internal/driver/process.go b/go/harness/claude/internal/driver/process.go index 4c65fc41d9..d4673331c8 100644 --- a/go/harness/claude/internal/driver/process.go +++ b/go/harness/claude/internal/driver/process.go @@ -41,6 +41,9 @@ type ProcessConfig struct { // AwaitTelemetry holds each prompt until Claude Code telemetry has // initialized. AwaitTelemetry bool + // EnsureSkills fetches deferred standalone skills before a turn. They + // need Actor egress, which does not exist while the Actor starts. + EnsureSkills func(context.Context) error } // ProcessDriver supervises one Claude Code process per ordinary runtime turn @@ -165,6 +168,11 @@ func (d *ProcessDriver) Args(turn runtime.Turn) []string { // Run supervises one Claude Code process and emits its ordered runtime events. func (d *ProcessDriver) Run(ctx context.Context, turn runtime.Turn, sink runtime.EventSink) (runtime.Outcome, error) { + if d.config.EnsureSkills != nil { + if err := d.config.EnsureSkills(ctx); err != nil { + return runtime.Outcome{Failure: &runtime.Failure{Message: "skills_unavailable: " + err.Error()}}, nil + } + } if strings.TrimSpace(turn.Prompt) == "" { return runtime.Outcome{}, fmt.Errorf("Claude prompt is required") } diff --git a/go/harness/claude/internal/driver/skills_test.go b/go/harness/claude/internal/driver/skills_test.go new file mode 100644 index 0000000000..3456103612 --- /dev/null +++ b/go/harness/claude/internal/driver/skills_test.go @@ -0,0 +1,32 @@ +package driver + +import ( + "context" + "errors" + "path/filepath" + "testing" + + "github.com/kagent-dev/kagent/go/harness/runtime" +) + +func TestRunFailsTurnWhenSkillsAreUnavailable(t *testing.T) { + calls := 0 + d := NewProcessDriver(ProcessConfig{ + // Never started: the turn fails before the native process runs. + Executable: filepath.Join(t.TempDir(), "missing"), Workspace: t.TempDir(), + EnsureSkills: func(context.Context) error { + calls++ + return errors.New("materialize skill \"review\": exit status 128") + }, + }) + outcome, err := d.Run(context.Background(), runtime.Turn{Prompt: "hello"}, nil) + if err != nil { + t.Fatalf("Run() error = %v, want a task failure", err) + } + if outcome.Failure == nil || outcome.Failure.Message != "skills_unavailable: materialize skill \"review\": exit status 128" { + t.Fatalf("Run() outcome = %#v", outcome) + } + if calls != 1 { + t.Fatalf("EnsureSkills called %d times", calls) + } +} diff --git a/go/harness/codex/internal/adapter/adapter.go b/go/harness/codex/internal/adapter/adapter.go index 879371a1db..838b791eb7 100644 --- a/go/harness/codex/internal/adapter/adapter.go +++ b/go/harness/codex/internal/adapter/adapter.go @@ -52,6 +52,7 @@ func New(ctx context.Context, input Input) (*driver.ProcessDriver, error) { if err := reconcileGeneratedDir(filepath.Join(codexHome, "skills"), nil); err != nil { return nil, fmt.Errorf("reconcile Codex skills: %w", err) } + var ensureSkills func(context.Context) error if cfg.SkillResources != nil { materialized, err := agentplugins.Materialize(ctx, *cfg.SkillResources, agentplugins.Paths{ Packages: filepath.Join(codexHome, "packages"), @@ -63,6 +64,7 @@ func New(ctx context.Context, input Input) (*driver.ProcessDriver, error) { if len(materialized.ClaudeFormatPluginRoots()) > 0 { return nil, fmt.Errorf("plugin is Claude-format; only the Agent Plugins format (plugin.json at the plugin root) is supported here") } + ensureSkills = materialized.EnsureSkills } if err := materializeAgents(codexHome, cfg.Agents); err != nil { return nil, err @@ -86,7 +88,7 @@ func New(ctx context.Context, input Input) (*driver.ProcessDriver, error) { Workspace: input.Workspace, Model: cfg.Model, Provider: nativeProviderName(cfg.Provider.Name), DeveloperInstruction: cfg.DeveloperInstruction, Environment: environment, MaxFrameBytes: cfg.MaxFrameBytes, MaxStderrBytes: cfg.MaxStderrBytes, InterruptGrace: cfg.InterruptGrace(), - ApprovalServers: approvalServers, + ApprovalServers: approvalServers, EnsureSkills: ensureSkills, }), nil } diff --git a/go/harness/codex/internal/driver/process.go b/go/harness/codex/internal/driver/process.go index 368bdbc9b4..8651763185 100644 --- a/go/harness/codex/internal/driver/process.go +++ b/go/harness/codex/internal/driver/process.go @@ -32,6 +32,9 @@ type ProcessConfig struct { MaxStderrBytes int InterruptGrace time.Duration ApprovalServers map[string]struct{} + // EnsureSkills fetches deferred standalone skills before a turn. They + // need Actor egress, which does not exist while the Actor starts. + EnsureSkills func(context.Context) error } // ProcessDriver supervises Codex App Server and retains it while a protected @@ -81,6 +84,11 @@ func (d *ProcessDriver) Validate(ctx context.Context) error { // Run initializes App Server, starts or resumes the Actor's native thread, and // emits the turn's ordered runtime events. func (d *ProcessDriver) Run(ctx context.Context, turn runtime.Turn, sink runtime.EventSink) (runtime.Outcome, error) { + if d.config.EnsureSkills != nil { + if err := d.config.EnsureSkills(ctx); err != nil { + return runtime.Outcome{Failure: &runtime.Failure{Message: "skills_unavailable: " + err.Error()}}, nil + } + } if strings.TrimSpace(turn.Prompt) == "" { return runtime.Outcome{}, fmt.Errorf("codex prompt is required") } diff --git a/go/harness/codex/internal/driver/skills_test.go b/go/harness/codex/internal/driver/skills_test.go new file mode 100644 index 0000000000..3456103612 --- /dev/null +++ b/go/harness/codex/internal/driver/skills_test.go @@ -0,0 +1,32 @@ +package driver + +import ( + "context" + "errors" + "path/filepath" + "testing" + + "github.com/kagent-dev/kagent/go/harness/runtime" +) + +func TestRunFailsTurnWhenSkillsAreUnavailable(t *testing.T) { + calls := 0 + d := NewProcessDriver(ProcessConfig{ + // Never started: the turn fails before the native process runs. + Executable: filepath.Join(t.TempDir(), "missing"), Workspace: t.TempDir(), + EnsureSkills: func(context.Context) error { + calls++ + return errors.New("materialize skill \"review\": exit status 128") + }, + }) + outcome, err := d.Run(context.Background(), runtime.Turn{Prompt: "hello"}, nil) + if err != nil { + t.Fatalf("Run() error = %v, want a task failure", err) + } + if outcome.Failure == nil || outcome.Failure.Message != "skills_unavailable: materialize skill \"review\": exit status 128" { + t.Fatalf("Run() outcome = %#v", outcome) + } + if calls != 1 { + t.Fatalf("EnsureSkills called %d times", calls) + } +} diff --git a/helm/kagent-crds/templates/api.kagent.dev_agents.yaml b/helm/kagent-crds/templates/api.kagent.dev_agents.yaml index 84023a3624..a8cde5991c 100644 --- a/helm/kagent-crds/templates/api.kagent.dev_agents.yaml +++ b/helm/kagent-crds/templates/api.kagent.dev_agents.yaml @@ -386,6 +386,30 @@ spec: description: GitArtifact identifies immutable content at a full Git commit ID. properties: + authorizationFrom: + description: |- + AuthorizationFrom references a key in a Secret in the AgentTemplate's + namespace whose value is the complete HTTP Authorization header for the + repository host, for example + `Basic ` for GitHub or `Bearer TOKEN`. + The egress gateway adds the header to the runtime's requests to that + host; the value never enters the runtime, its configuration, or a + snapshot. Supported for skills only. + properties: + key: + maxLength: 253 + minLength: 1 + pattern: ^[-._a-zA-Z0-9]+$ + type: string + name: + maxLength: 253 + minLength: 1 + pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$ + type: string + required: + - key + - name + type: object commit: pattern: ^([0-9a-fA-F]{40}|[0-9a-fA-F]{64})$ type: string @@ -397,6 +421,9 @@ spec: - commit - url type: object + x-kubernetes-validations: + - message: authorizationFrom requires an https URL + rule: '!has(self.authorizationFrom) || self.url.startsWith(''https://'')' oci: description: OCI is a digest-pinned image reference. pattern: ^[^[:space:]@]+@sha256:[0-9a-f]{64}$ @@ -418,6 +445,9 @@ spec: required: - source type: object + x-kubernetes-validations: + - message: authorizationFrom is supported only for skills[].source.git + rule: '!has(self.source.git) || !has(self.source.git.authorizationFrom)' maxItems: 20 type: array promptTemplate: @@ -499,6 +529,30 @@ spec: description: GitArtifact identifies immutable content at a full Git commit ID. properties: + authorizationFrom: + description: |- + AuthorizationFrom references a key in a Secret in the AgentTemplate's + namespace whose value is the complete HTTP Authorization header for the + repository host, for example + `Basic ` for GitHub or `Bearer TOKEN`. + The egress gateway adds the header to the runtime's requests to that + host; the value never enters the runtime, its configuration, or a + snapshot. Supported for skills only. + properties: + key: + maxLength: 253 + minLength: 1 + pattern: ^[-._a-zA-Z0-9]+$ + type: string + name: + maxLength: 253 + minLength: 1 + pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$ + type: string + required: + - key + - name + type: object commit: pattern: ^([0-9a-fA-F]{40}|[0-9a-fA-F]{64})$ type: string @@ -510,6 +564,9 @@ spec: - commit - url type: object + x-kubernetes-validations: + - message: authorizationFrom requires an https URL + rule: '!has(self.authorizationFrom) || self.url.startsWith(''https://'')' oci: description: OCI is a digest-pinned image reference. pattern: ^[^[:space:]@]+@sha256:[0-9a-f]{64}$ diff --git a/helm/kagent-crds/templates/api.kagent.dev_agenttemplates.yaml b/helm/kagent-crds/templates/api.kagent.dev_agenttemplates.yaml index ebe57de679..46ee123e75 100644 --- a/helm/kagent-crds/templates/api.kagent.dev_agenttemplates.yaml +++ b/helm/kagent-crds/templates/api.kagent.dev_agenttemplates.yaml @@ -141,6 +141,30 @@ spec: description: GitArtifact identifies immutable content at a full Git commit ID. properties: + authorizationFrom: + description: |- + AuthorizationFrom references a key in a Secret in the AgentTemplate's + namespace whose value is the complete HTTP Authorization header for the + repository host, for example + `Basic ` for GitHub or `Bearer TOKEN`. + The egress gateway adds the header to the runtime's requests to that + host; the value never enters the runtime, its configuration, or a + snapshot. Supported for skills only. + properties: + key: + maxLength: 253 + minLength: 1 + pattern: ^[-._a-zA-Z0-9]+$ + type: string + name: + maxLength: 253 + minLength: 1 + pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$ + type: string + required: + - key + - name + type: object commit: pattern: ^([0-9a-fA-F]{40}|[0-9a-fA-F]{64})$ type: string @@ -152,6 +176,9 @@ spec: - commit - url type: object + x-kubernetes-validations: + - message: authorizationFrom requires an https URL + rule: '!has(self.authorizationFrom) || self.url.startsWith(''https://'')' oci: description: OCI is a digest-pinned image reference. pattern: ^[^[:space:]@]+@sha256:[0-9a-f]{64}$ @@ -172,6 +199,9 @@ spec: required: - source type: object + x-kubernetes-validations: + - message: authorizationFrom is supported only for skills[].source.git + rule: '!has(self.source.git) || !has(self.source.git.authorizationFrom)' maxItems: 20 type: array promptTemplate: @@ -252,6 +282,30 @@ spec: description: GitArtifact identifies immutable content at a full Git commit ID. properties: + authorizationFrom: + description: |- + AuthorizationFrom references a key in a Secret in the AgentTemplate's + namespace whose value is the complete HTTP Authorization header for the + repository host, for example + `Basic ` for GitHub or `Bearer TOKEN`. + The egress gateway adds the header to the runtime's requests to that + host; the value never enters the runtime, its configuration, or a + snapshot. Supported for skills only. + properties: + key: + maxLength: 253 + minLength: 1 + pattern: ^[-._a-zA-Z0-9]+$ + type: string + name: + maxLength: 253 + minLength: 1 + pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$ + type: string + required: + - key + - name + type: object commit: pattern: ^([0-9a-fA-F]{40}|[0-9a-fA-F]{64})$ type: string @@ -263,6 +317,9 @@ spec: - commit - url type: object + x-kubernetes-validations: + - message: authorizationFrom requires an https URL + rule: '!has(self.authorizationFrom) || self.url.startsWith(''https://'')' oci: description: OCI is a digest-pinned image reference. pattern: ^[^[:space:]@]+@sha256:[0-9a-f]{64}$