From 1df5012f4d0aae7bd39d6ea63527757710d89163 Mon Sep 17 00:00:00 2001 From: Jose Armesto Date: Fri, 9 Oct 2026 12:08:22 -0700 Subject: [PATCH 1/2] =?UTF-8?q?feat(api):=20Agent.spec.egress=20=E2=80=94?= =?UTF-8?q?=20HTTP(S)=20origins=20an=20Agent=20may=20reach?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit kagent-dev/kagent#3019, squashed. Co-authored-by: QuentinBisson Signed-off-by: Jose Armesto Signed-off-by: QuentinBisson --- .../configuration-and-compilation.md | 12 ++++ .../crd/bases/api.kagent.dev_agents.yaml | 15 +++++ go/api/v1alpha3/agent_types.go | 12 ++++ go/api/v1alpha3/configuration_crd_cel_test.go | 15 +++++ go/api/v1alpha3/zz_generated.deepcopy.go | 5 ++ go/core/internal/egress/destination.go | 42 ++++++++++++ go/core/internal/egress/destination_test.go | 28 ++++++++ .../internal/service/session/workflow_test.go | 6 +- go/core/internal/substrate/policy.go | 35 +++++----- go/core/internal/translator/compiler.go | 26 +++++++ go/core/internal/translator/compiler_test.go | 67 +++++++++++++++++++ .../templates/api.kagent.dev_agents.yaml | 15 +++++ ui/playwright/DEFERRED.md | 9 +++ ui/src/api/domain/agents.ts | 5 +- ui/src/pages/agents/AgentFormPage.tsx | 2 + 15 files changed, 272 insertions(+), 22 deletions(-) diff --git a/docs/architecture/configuration-and-compilation.md b/docs/architecture/configuration-and-compilation.md index 684c0f8f5b..6e554f6e17 100644 --- a/docs/architecture/configuration-and-compilation.md +++ b/docs/architecture/configuration-and-compilation.md @@ -20,6 +20,18 @@ with `tools[].subAgent.templateRef` and compile under the parent Agent's Harness Each subagent requires `templateRef` and shares the parent's runtime and Harness. Dedicated `agentRef` bindings are deferred and are not part of the served API. +`Agent.spec.egress` lists HTTP(S) origins the Agent may reach besides the +destinations its revision compiles, such as `https://proxy.golang.org`. A host +may have `*` as its leftmost label (`https://*.githubusercontent.com` matches +one label), with at least two labels under it, so nothing opens a top-level +domain or every host. The field is on the Agent, not the +template or the Harness: the hosts are a property of the workload in the place +where it runs (an internal git host, a registry mirror), so a template stays +portable and a Harness stays about the runtime. The compiler adds the origins, +in canonical form, after the runtime compiler has run, so every runtime and +every Shared subagent treats them the same, and they are part of the revision +identity. + All three are `api.kagent.dev/v1alpha3` Kubernetes resources. Infrastructure-derived values such as runtime addresses and inferred egress do not belong in the public API. 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..3a4845f7b5 100644 --- a/go/api/config/crd/bases/api.kagent.dev_agents.yaml +++ b/go/api/config/crd/bases/api.kagent.dev_agents.yaml @@ -51,6 +51,21 @@ spec: AgentSpec pairs portable behavior with a runner. References are local to the Agent's namespace, including references nested in inline specs. properties: + egress: + description: |- + Egress lists HTTP(S) origins the Agent may reach besides the destinations + its revision compiles (the model, its MCP servers, its skill and plugin + sources, telemetry), such as "https://proxy.golang.org". The host may + have "*" as its leftmost label: "https://*.githubusercontent.com" matches + one label, such as raw.githubusercontent.com. A wildcard needs two labels + under it, so nothing opens a whole top-level domain or every host. + items: + maxLength: 270 + pattern: ^https?://(\*\.([a-z0-9]([-a-z0-9]{0,61}[a-z0-9])?\.)+|([a-z0-9]([-a-z0-9]{0,61}[a-z0-9])?\.)*)[a-z]([-a-z0-9]{0,61}[a-z0-9])?(:([1-9][0-9]{0,3}|[1-5][0-9]{4}|6[0-4][0-9]{3}|65[0-4][0-9]{2}|655[0-2][0-9]|6553[0-5]))?$ + type: string + maxItems: 64 + type: array + x-kubernetes-list-type: set harness: description: HarnessSpec defines a reusable runtime and its infrastructure policy. diff --git a/go/api/v1alpha3/agent_types.go b/go/api/v1alpha3/agent_types.go index 95fd057d5a..20e9879e9c 100644 --- a/go/api/v1alpha3/agent_types.go +++ b/go/api/v1alpha3/agent_types.go @@ -37,6 +37,18 @@ type AgentSpec struct { // +kubebuilder:validation:XValidation:rule="has(self.name) && self.name != ''",message="harnessRef.name must not be empty" // +optional HarnessRef *corev1.LocalObjectReference `json:"harnessRef,omitempty"` + // Egress lists HTTP(S) origins the Agent may reach besides the destinations + // its revision compiles (the model, its MCP servers, its skill and plugin + // sources, telemetry), such as "https://proxy.golang.org". The host may + // have "*" as its leftmost label: "https://*.githubusercontent.com" matches + // one label, such as raw.githubusercontent.com. A wildcard needs two labels + // under it, so nothing opens a whole top-level domain or every host. + // +optional + // +listType=set + // +kubebuilder:validation:MaxItems=64 + // +kubebuilder:validation:items:MaxLength=270 + // +kubebuilder:validation:items:Pattern=`^https?://(\*\.([a-z0-9]([-a-z0-9]{0,61}[a-z0-9])?\.)+|([a-z0-9]([-a-z0-9]{0,61}[a-z0-9])?\.)*)[a-z]([-a-z0-9]{0,61}[a-z0-9])?(:([1-9][0-9]{0,3}|[1-5][0-9]{4}|6[0-4][0-9]{3}|65[0-4][0-9]{2}|655[0-2][0-9]|6553[0-5]))?$` + Egress []string `json:"egress,omitempty"` } const ( diff --git a/go/api/v1alpha3/configuration_crd_cel_test.go b/go/api/v1alpha3/configuration_crd_cel_test.go index be23e3a308..204602b844 100644 --- a/go/api/v1alpha3/configuration_crd_cel_test.go +++ b/go/api/v1alpha3/configuration_crd_cel_test.go @@ -341,6 +341,21 @@ func TestConfigurationCRDValidation(t *testing.T) { } require.ErrorContains(t, cl.Create(ctx, &Agent{ObjectMeta: metav1.ObjectMeta{Namespace: namespace, Name: "empty-" + strings.ToLower(field)}, Spec: spec}), field+".name must not be empty") } + t.Run("Agent egress accepts HTTP(S) origins and leftmost-label wildcards", func(t *testing.T) { + spec := AgentSpec{TemplateRef: &corev1.LocalObjectReference{Name: "behavior"}, HarnessRef: &corev1.LocalObjectReference{Name: "runner"}, + Egress: []string{"https://proxy.golang.org", "http://mirror.internal:8080", "https://git.internal:65535", "https://*.githubusercontent.com"}} + require.NoError(t, cl.Create(ctx, &Agent{ObjectMeta: metav1.ObjectMeta{Namespace: namespace, Name: "egress"}, Spec: spec})) + }) + for i, origin := range []string{ + "proxy.golang.org", "ftp://proxy.golang.org", "https://*", "https://*.com", "https://a.*.example.com", "https://192.0.2.1", "https://[2001:db8::1]", + "https://proxy.golang.org/path", "https://Proxy.Golang.org", "https://user@proxy.golang.org", "https://proxy.golang.org?x=1", + "https://proxy.golang.org#f", "https://proxy.golang.org:0", "https://proxy.golang.org:70000", + } { + t.Run("Agent egress rejects "+origin, func(t *testing.T) { + spec := AgentSpec{TemplateRef: &corev1.LocalObjectReference{Name: "behavior"}, HarnessRef: &corev1.LocalObjectReference{Name: "runner"}, Egress: []string{origin}} + require.ErrorContains(t, cl.Create(ctx, &Agent{ObjectMeta: metav1.ObjectMeta{Namespace: namespace, Name: fmt.Sprintf("egress-invalid-%d", i)}, Spec: spec}), "spec.egress[0]") + }) + } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { diff --git a/go/api/v1alpha3/zz_generated.deepcopy.go b/go/api/v1alpha3/zz_generated.deepcopy.go index 59a3d9ddf2..d5426ad471 100644 --- a/go/api/v1alpha3/zz_generated.deepcopy.go +++ b/go/api/v1alpha3/zz_generated.deepcopy.go @@ -109,6 +109,11 @@ func (in *AgentSpec) DeepCopyInto(out *AgentSpec) { *out = new(v1.LocalObjectReference) **out = **in } + if in.Egress != nil { + in, out := &in.Egress, &out.Egress + *out = make([]string, len(*in)) + copy(*out, *in) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AgentSpec. diff --git a/go/core/internal/egress/destination.go b/go/core/internal/egress/destination.go index ade6f203ed..6171155f25 100644 --- a/go/core/internal/egress/destination.go +++ b/go/core/internal/egress/destination.go @@ -1,9 +1,14 @@ package egress import ( + "fmt" "net" + "net/netip" "net/url" + "strconv" "strings" + + "k8s.io/apimachinery/pkg/util/validation" ) // Origin retains the protocol and destination port needed by Substrate's @@ -24,3 +29,40 @@ func Origin(u *url.URL) string { } return (&url.URL{Scheme: u.Scheme, Host: host}).String() } + +// ParseOrigin validates a declared HTTP(S) origin, such as +// "https://proxy.golang.org" or "https://*.githubusercontent.com", and returns +// its canonical form. Unlike Origin, it refuses anything beyond scheme, host +// and port, because a declared path or credential would silently widen to the +// whole origin. +func ParseOrigin(value string) (string, error) { + u, err := url.Parse(value) + if err != nil || (u.Scheme != "http" && u.Scheme != "https") || u.Opaque != "" || u.User != nil || u.Hostname() == "" || + u.Path != "" || u.ForceQuery || u.RawQuery != "" || u.Fragment != "" { + return "", fmt.Errorf("invalid egress origin %q: expected http(s)://host[:port]", value) + } + if !ValidHostPattern(strings.TrimSuffix(strings.ToLower(u.Hostname()), ".")) { + return "", fmt.Errorf("invalid egress origin %q: the host must be a DNS name, optionally with \"*\" as its leftmost label", value) + } + if port := u.Port(); port != "" { + if number, err := strconv.ParseUint(port, 10, 16); err != nil || number == 0 { + return "", fmt.Errorf("invalid egress origin %q: invalid port", value) + } + } + return Origin(u), nil +} + +// ValidHostPattern reports whether a normalized host is a DNS name, or a DNS +// name with "*" as its leftmost label, which is what Substrate's hostname +// rules match. An IP address and "*" alone are not patterns, and a wildcard +// needs two labels under it: "*.com" would open a whole top-level domain. +func ValidHostPattern(host string) bool { + name, wildcard := strings.CutPrefix(host, "*.") + if _, err := netip.ParseAddr(name); err == nil { + return false + } + if wildcard && !strings.Contains(name, ".") { + return false + } + return len(validation.IsDNS1123Subdomain(name)) == 0 +} diff --git a/go/core/internal/egress/destination_test.go b/go/core/internal/egress/destination_test.go index dc9fa6d812..1bcebcd606 100644 --- a/go/core/internal/egress/destination_test.go +++ b/go/core/internal/egress/destination_test.go @@ -22,3 +22,31 @@ func TestOrigin(t *testing.T) { }) } } + +func TestParseOrigin(t *testing.T) { + for _, tt := range []struct{ raw, want string }{ + {"https://proxy.golang.org", "https://proxy.golang.org:443"}, + {"https://Proxy.Golang.org.", "https://proxy.golang.org:443"}, + {"http://mirror.internal", "http://mirror.internal:80"}, + {"https://git.internal:8443", "https://git.internal:8443"}, + {"https://*.githubusercontent.com", "https://*.githubusercontent.com:443"}, + } { + t.Run(tt.raw, func(t *testing.T) { + got, err := ParseOrigin(tt.raw) + require.NoError(t, err) + require.Equal(t, tt.want, got) + }) + } + for _, raw := range []string{ + "proxy.golang.org", "ftp://proxy.golang.org", "https://", "https://*", "https://*.", + "https://a.*.example.com", "https://*.*.example.com", "https://*github.com", "https://*.com", "https://-bad.example.com", + "https://192.0.2.1", "https://[2001:db8::1]", "https://proxy.golang.org/", "https://proxy.golang.org/path", + "https://proxy.golang.org?x=1", "https://proxy.golang.org?", "https://proxy.golang.org#f", + "https://user@proxy.golang.org", "https://proxy.golang.org:0", "https://proxy.golang.org:70000", + } { + t.Run(raw, func(t *testing.T) { + _, err := ParseOrigin(raw) + require.Error(t, err) + }) + } +} diff --git a/go/core/internal/service/session/workflow_test.go b/go/core/internal/service/session/workflow_test.go index 47387e939c..a49c0958bc 100644 --- a/go/core/internal/service/session/workflow_test.go +++ b/go/core/internal/service/session/workflow_test.go @@ -364,7 +364,11 @@ func TestActorEgressPolicy(t *testing.T) { policy, err = substrate.ActorEgressPolicy("team-a", nil, nil) require.NoError(t, err) require.Empty(t, policy.Rules, "no destinations must deny all egress") - for _, destination := range []string{"", "*", "api.example.com", "https://api.example.com/path", "http://api.example.com:0", "http://api.example.com:65536", "http://user@api.example.com", "http://api.example.com?key=value", "http://*.example.com"} { + policy, err = substrate.ActorEgressPolicy("team-a", []string{"https://*.example.com:443"}, nil) + require.NoError(t, err, "a leftmost-label wildcard is a Substrate hostname pattern") + require.Equal(t, []string{"*.example.com"}, policy.Rules[0].GetHttps().GetHostnames()) + for _, destination := range []string{"", "*", "api.example.com", "https://api.example.com/path", "http://api.example.com:0", "http://api.example.com:65536", "http://user@api.example.com", "http://api.example.com?key=value", + "https://*", "https://*.com", "https://a.*.example.com", "https://*.*.example.com", "https://*example.com"} { t.Run(destination, func(t *testing.T) { _, err := substrate.ActorEgressPolicy("team-a", []string{destination}, nil) require.Error(t, err) diff --git a/go/core/internal/substrate/policy.go b/go/core/internal/substrate/policy.go index 64bcb848fa..67dd233f8d 100644 --- a/go/core/internal/substrate/policy.go +++ b/go/core/internal/substrate/policy.go @@ -11,7 +11,6 @@ import ( "github.com/agent-substrate/substrate/pkg/proto/ateapipb" "github.com/kagent-dev/kagent/go/core/internal/egress" - "k8s.io/apimachinery/pkg/util/validation" ) // ActorEgressPolicy compiles HTTP(S) origins into an actor's default allowlist. @@ -25,26 +24,20 @@ func ActorEgressPolicy(atespace string, destinations []string, credentials []egr protocols := map[string]string{} hosts := map[string]bool{} for _, destination := range destinations { - u, err := url.Parse(destination) - if err != nil || (u.Scheme != "http" && u.Scheme != "https") || u.User != nil || u.Hostname() == "" || u.ForceQuery || u.RawQuery != "" || u.Fragment != "" || u.Path != "" { - return nil, fmt.Errorf("invalid egress destination %q: expected an HTTP(S) origin", destination) + if u, err := url.Parse(destination); err == nil { + if _, err := netip.ParseAddr(strings.TrimSuffix(u.Hostname(), ".")); err == nil { + return nil, fmt.Errorf("egress destination %q requires a DNS name: Substrate does not support IP allowlists", destination) + } } - host := strings.TrimSuffix(strings.ToLower(u.Hostname()), ".") - if _, err := netip.ParseAddr(host); err == nil { - return nil, fmt.Errorf("egress destination %q requires a DNS name: Substrate does not support IP allowlists", destination) - } - if len(validation.IsDNS1123Subdomain(host)) != 0 { - return nil, fmt.Errorf("invalid egress destination %q", destination) - } - origin := egress.Origin(u) - u, err = url.Parse(origin) + origin, err := egress.ParseOrigin(destination) if err != nil { - return nil, fmt.Errorf("invalid egress destination %q: %w", destination, err) - } - port, err := strconv.ParseUint(u.Port(), 10, 16) - if err != nil || port == 0 { - return nil, fmt.Errorf("invalid egress port in %q", destination) + return nil, fmt.Errorf("invalid egress destination: %w", err) } + // ParseOrigin returns scheme://host:port with a valid port; the port is + // re-encoded so "0443" and "443" are one origin. + u, _ := url.Parse(origin) + host := u.Hostname() + port, _ := strconv.ParseUint(u.Port(), 10, 16) u.Host = net.JoinHostPort(host, strconv.FormatUint(port, 10)) if protocol, found := protocols[u.Host]; found && protocol != u.Scheme { return nil, fmt.Errorf("egress destination %q uses both HTTP and HTTPS on the same port", u.Host) @@ -72,8 +65,10 @@ func ActorEgressPolicy(atespace string, destinations []string, credentials []egr for _, origin := range names { target := origins[origin] ports := &ateapipb.Ports{Numbers: []int32{target.port}} - // Keep exactly one rule per host and port so an overlapping rule cannot - // bypass credential replacement. + // Keep exactly one rule per host pattern and port. A wildcard rule can + // overlap an exact one, but Substrate decides on the exact hostname + // first (EgressPolicy in proto/ateapi.proto), and credentials bind only + // to exact hostnames, so a wildcard cannot bypass credential replacement. if target.scheme == "https" { policy.Rules = append(policy.Rules, &ateapipb.EgressRule{Https: &ateapipb.HTTPSRule{Hostnames: []string{target.hostname}, Ports: ports, Effects: effects[target.hostname]}}) } else { diff --git a/go/core/internal/translator/compiler.go b/go/core/internal/translator/compiler.go index 6eaaaef2c0..b2e9db8f21 100644 --- a/go/core/internal/translator/compiler.go +++ b/go/core/internal/translator/compiler.go @@ -5,8 +5,10 @@ import ( "encoding/json" "fmt" "maps" + "slices" "github.com/kagent-dev/kagent/go/api/v1alpha3" + "github.com/kagent-dev/kagent/go/core/internal/egress" "istio.io/istio/pkg/kube/krt" "k8s.io/apimachinery/pkg/types" ) @@ -125,6 +127,10 @@ func (c *Compiler) CompileAgent(ctx context.Context, agent *v1alpha3.Agent) (*Co return nil, err } result.AgentUID = string(agent.UID) + result.EgressDestinations, err = withAgentEgress(result.EgressDestinations, agent.Spec.Egress) + if err != nil { + return nil, err + } result.Provenance, err = json.Marshal(struct { AgentName string `json:"agentName"` AgentUID string `json:"agentUID"` @@ -137,6 +143,26 @@ func (c *Compiler) CompileAgent(ctx context.Context, agent *v1alpha3.Agent) (*Co return result, nil } +// withAgentEgress adds the Agent's declared origins to the destinations its +// runtime compiled, in canonical form and once each. Every runtime gets the +// same treatment, so no runtime compiler reads the Agent's egress. +func withAgentEgress(compiled, declared []string) ([]string, error) { + if len(declared) == 0 { + return compiled, nil + } + destinations := slices.Clone(compiled) + for _, value := range declared { + origin, err := egress.ParseOrigin(value) + if err != nil { + return nil, NewValidationError("Agent egress: %v", err) + } + if !slices.Contains(destinations, origin) { + destinations = append(destinations, origin) + } + } + return destinations, nil +} + // compileConfiguration compiles resolved configuration for the named Agent. // The Agent name owns runtime identity; template and Harness names are provenance. func (c *Compiler) compileConfiguration(ctx context.Context, agentName string, harness *HarnessConfiguration, template *TemplateConfiguration) (*CompileResult, error) { diff --git a/go/core/internal/translator/compiler_test.go b/go/core/internal/translator/compiler_test.go index 6427eaa5e4..e23e6aedf9 100644 --- a/go/core/internal/translator/compiler_test.go +++ b/go/core/internal/translator/compiler_test.go @@ -978,3 +978,70 @@ func TestResolveModelConfigMistral(t *testing.T) { resolved = mockCollections(t, model).ResolvedModelConfigs.List()[0] require.Equal(t, "APIKeySecretNotFound", resolved.Failure().Reason) } + +func TestCompileAgentAddsTheAgentEgress(t *testing.T) { + template := &v1alpha3.AgentTemplate{ + ObjectMeta: metav1.ObjectMeta{Name: "go-coder", Namespace: "test"}, + Spec: v1alpha3.AgentTemplateSpec{ModelConfig: &corev1.LocalObjectReference{Name: "default-model"}, SystemPrompt: "help"}, + } + harness := &v1alpha3.Harness{ + ObjectMeta: metav1.ObjectMeta{Name: "kagent", Namespace: "test"}, + Spec: v1alpha3.HarnessSpec{ + Kagent: &v1alpha3.KagentHarness{}, + Workload: v1alpha3.HarnessWorkload{Image: "example.com/runtime@sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"}, + Substrate: v1alpha3.RuntimeSubstratePolicy{WorkerPoolRef: corev1.LocalObjectReference{Name: "default"}, SnapshotPolicy: v1alpha3.RuntimeSnapshotPolicy{Location: "snapshots"}}, + }, + } + declared := []string{"https://proxy.golang.org", "https://*.githubusercontent.com", "https://Proxy.Golang.org.", "https://git.internal:8443"} + want := []string{"https://proxy.golang.org:443", "https://*.githubusercontent.com:443", "https://git.internal:8443"} + c := compiler(t, modelConfig(), template, harness) + + referenced := &v1alpha3.Agent{ObjectMeta: metav1.ObjectMeta{Name: "coder", Namespace: "test"}, Spec: v1alpha3.AgentSpec{ + TemplateRef: &corev1.LocalObjectReference{Name: template.Name}, HarnessRef: &corev1.LocalObjectReference{Name: harness.Name}, Egress: declared, + }} + inline := inlineAgent(harness, template) + inline.Spec.Egress = declared + for name, agent := range map[string]*v1alpha3.Agent{"references": referenced, "inline": inline} { + t.Run(name, func(t *testing.T) { + result, err := c.CompileAgent(t.Context(), agent) + require.NoError(t, err) + require.Subset(t, result.EgressDestinations, want) + require.Equal(t, 1, countOf(result.EgressDestinations, "https://proxy.golang.org:443"), "an origin is listed once") + policy, err := substrate.ActorEgressPolicy("test", result.EgressDestinations, result.Credentials) + require.NoError(t, err) + var hostnames []string + for _, rule := range policy.Rules { + hostnames = append(hostnames, rule.GetHttps().GetHostnames()...) + } + require.Subset(t, hostnames, []string{"proxy.golang.org", "*.githubusercontent.com", "git.internal"}) + }) + } + + withEgress, err := c.CompileAgent(t.Context(), referenced) + require.NoError(t, err) + plain := referenced.DeepCopy() + plain.Spec.Egress = nil + withoutEgress, err := c.CompileAgent(t.Context(), plain) + require.NoError(t, err) + // The destinations are a digest input, so the revision identity follows them. + require.Subset(t, withEgress.EgressDestinations, want) + for _, origin := range want { + require.NotContains(t, withoutEgress.EgressDestinations, origin) + } + + invalid := referenced.DeepCopy() + invalid.Spec.Egress = []string{"https://proxy.golang.org/path"} + _, err = c.CompileAgent(t.Context(), invalid) + var validation *v2translator.ValidationError + require.ErrorAs(t, err, &validation) +} + +func countOf(values []string, want string) int { + n := 0 + for _, value := range values { + if value == want { + n++ + } + } + return n +} diff --git a/helm/kagent-crds/templates/api.kagent.dev_agents.yaml b/helm/kagent-crds/templates/api.kagent.dev_agents.yaml index 84023a3624..3a4845f7b5 100644 --- a/helm/kagent-crds/templates/api.kagent.dev_agents.yaml +++ b/helm/kagent-crds/templates/api.kagent.dev_agents.yaml @@ -51,6 +51,21 @@ spec: AgentSpec pairs portable behavior with a runner. References are local to the Agent's namespace, including references nested in inline specs. properties: + egress: + description: |- + Egress lists HTTP(S) origins the Agent may reach besides the destinations + its revision compiles (the model, its MCP servers, its skill and plugin + sources, telemetry), such as "https://proxy.golang.org". The host may + have "*" as its leftmost label: "https://*.githubusercontent.com" matches + one label, such as raw.githubusercontent.com. A wildcard needs two labels + under it, so nothing opens a whole top-level domain or every host. + items: + maxLength: 270 + pattern: ^https?://(\*\.([a-z0-9]([-a-z0-9]{0,61}[a-z0-9])?\.)+|([a-z0-9]([-a-z0-9]{0,61}[a-z0-9])?\.)*)[a-z]([-a-z0-9]{0,61}[a-z0-9])?(:([1-9][0-9]{0,3}|[1-5][0-9]{4}|6[0-4][0-9]{3}|65[0-4][0-9]{2}|655[0-2][0-9]|6553[0-5]))?$ + type: string + maxItems: 64 + type: array + x-kubernetes-list-type: set harness: description: HarnessSpec defines a reusable runtime and its infrastructure policy. diff --git a/ui/playwright/DEFERRED.md b/ui/playwright/DEFERRED.md index be664c4f12..53e148bc42 100644 --- a/ui/playwright/DEFERRED.md +++ b/ui/playwright/DEFERRED.md @@ -332,3 +332,12 @@ navigation had just reset. What actually removes the coupling is a second `test` same file — one recording for the lifecycle, one for the states, neither able to abort the other. That costs `conventions.test.ts` its "one spec, one test" rule, so it is an amendment to the convention rather than a reshuffle, and belongs in its own change. + +## An Agent edit keeps the egress the form does not show + +`AgentFormPage` does not author `spec.egress`, and the update sends the whole spec, so +the save carries the Agent's existing `egress` over. Nothing asserts it: no spec opens +an Agent that declares egress, saves it, and reads the request back. The case it guards +is silent — a save that dropped the line would widen nothing and narrow the Agent's +egress to what its revision compiles, with no message on the page. It belongs in the +agent lifecycle spec, with a fixture Agent whose `egress` matches the CRD shape. diff --git a/ui/src/api/domain/agents.ts b/ui/src/api/domain/agents.ts index 537ff3af5c..ceb5c7c8b6 100644 --- a/ui/src/api/domain/agents.ts +++ b/ui/src/api/domain/agents.ts @@ -5,7 +5,10 @@ import type { HarnessSpec } from "./harnesses"; /** Exactly one of each pair; refs resolve in the Agent's namespace. */ export type AgentSpec = ({ template: AgentTemplateSpec; templateRef?: never } | { templateRef: { name: string }; template?: never }) & - ({ harness: HarnessSpec; harnessRef?: never } | { harnessRef: { name: string }; harness?: never }); + ({ harness: HarnessSpec; harnessRef?: never } | { harnessRef: { name: string }; harness?: never }) & { + /** HTTP(S) origins the Agent may reach besides what its revision compiles. */ + egress?: string[]; + }; /** One condition the controller recorded for an Agent. */ export interface AgentCondition { type: string; diff --git a/ui/src/pages/agents/AgentFormPage.tsx b/ui/src/pages/agents/AgentFormPage.tsx index 75693eb0e3..d1d615229a 100644 --- a/ui/src/pages/agents/AgentFormPage.tsx +++ b/ui/src/pages/agents/AgentFormPage.tsx @@ -137,6 +137,8 @@ function AgentForm({ agent }: { agent?: Agent }) { ...(harnessSource === "reference" ? { harnessRef: { name: harnessRef! } } : { harness: harnessSpecFromDraft(harnessDraft, spec?.harness) }), + // The form does not author egress; an edit keeps what the Agent declares. + ...(spec?.egress ? { egress: spec.egress } : {}), } as AgentSpec; const input = { namespace, From 1fc5f7b2ca5222ebfcef2f40263335c6effbe4b5 Mon Sep 17 00:00:00 2001 From: Thomas Barrett Date: Fri, 9 Oct 2026 12:08:36 -0700 Subject: [PATCH 2/2] feat(api): Secret-backed egress headers for Agents, and egress for SandboxTemplates Agent.spec.egress entries become objects with optional headers, each from a key of a Secret in the Agent's namespace. Headers compile to Substrate replaceHeaders bindings, checked with the runtime's own, and are allowed only on exact https origins the Agent reaches on one port. A missing Secret or key is reported on ResolvedRefs. Sandboxes could reach nothing: creation gave Substrate no destinations. SandboxTemplate now takes the same egress entries as an Agent, so a sandbox can reach the origins it lists, with Secret-backed headers the gateway sets. A sandbox keeps the egress it was created with. AgentEgress becomes EgressEntry, shared by both; the JSON is unchanged. Part of #3093 Co-Authored-By: Claude Opus 5.5 (1M context) Signed-off-by: Thomas Barrett --- .../configuration-and-compilation.md | 5 + docs/architecture/credential-injection.md | 7 +- docs/architecture/sandboxes.md | 10 +- .../crd/bases/api.kagent.dev_agents.yaml | 96 ++++++++++- .../api.kagent.dev_sandboxtemplates.yaml | 99 ++++++++++++ go/api/v1alpha3/agent_types.go | 54 ++++++- go/api/v1alpha3/configuration_crd_cel_test.go | 67 +++++++- go/api/v1alpha3/sandboxtemplate_types.go | 8 + go/api/v1alpha3/zz_generated.deepcopy.go | 71 ++++++++- .../internal/controller/collections_test.go | 60 +++++++ go/core/internal/controller/reconciler.go | 5 + .../controller/sandbox_collections.go | 48 ++++++ .../internal/controller/sandbox_reconciler.go | 2 +- .../controller/sandbox_reconciler_test.go | 64 +++++++- .../internal/database/sandbox_revisions.go | 13 +- go/core/internal/database/sandboxes_test.go | 28 ++++ go/core/internal/mcp/prompts.go | 2 +- go/core/internal/mcp/sandboxes.go | 2 +- go/core/internal/service/sandbox/workflow.go | 2 +- go/core/internal/translator/compiler.go | 39 ++--- go/core/internal/translator/compiler_test.go | 150 +++++++++++++++++- go/core/internal/translator/credentials.go | 1 + go/core/internal/translator/egress.go | 123 ++++++++++++++ go/core/internal/translator/validation.go | 13 ++ .../templates/api.kagent.dev_agents.yaml | 96 ++++++++++- .../api.kagent.dev_sandboxtemplates.yaml | 99 ++++++++++++ ui/src/api/domain/agents.ts | 15 +- 27 files changed, 1126 insertions(+), 53 deletions(-) create mode 100644 go/core/internal/translator/egress.go diff --git a/docs/architecture/configuration-and-compilation.md b/docs/architecture/configuration-and-compilation.md index 6e554f6e17..0dd0dc4d60 100644 --- a/docs/architecture/configuration-and-compilation.md +++ b/docs/architecture/configuration-and-compilation.md @@ -32,6 +32,11 @@ in canonical form, after the runtime compiler has run, so every runtime and every Shared subagent treats them the same, and they are part of the revision identity. +An entry on an exact `https` origin may list `headers`, each from a key of a +Secret in the Agent's namespace. They compile to gateway credential bindings +like ModelConfig keys (see [credential injection](credential-injection.md)). +A missing Secret or key reports `SecretNotFound` or `SecretKeyNotFound`. + All three are `api.kagent.dev/v1alpha3` Kubernetes resources. Infrastructure-derived values such as runtime addresses and inferred egress do not belong in the public API. diff --git a/docs/architecture/credential-injection.md b/docs/architecture/credential-injection.md index 3e29ad1f5c..d4e114dbfc 100644 --- a/docs/architecture/credential-injection.md +++ b/docs/architecture/credential-injection.md @@ -56,6 +56,7 @@ 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 | +| Agent or SandboxTemplate egress header (`spec.egress[].headers`) | Configured header and optional prefix; Secret contains the rest of the 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 @@ -67,7 +68,11 @@ by Substrate egress policies. Harness and SandboxTemplate environment entries accept only literal `value` strings, including empty strings. Configure Secret-backed credentials on -ModelConfig or RemoteMCPServer for gateway injection. +ModelConfig, RemoteMCPServer, or Agent or SandboxTemplate egress headers for +gateway injection. Egress headers are also rejected on a host the runtime +reaches over HTTPS on another port. A sandbox's headers resolve Secrets in its +SandboxTemplate's namespace, under the same Substrate credential policy as +Agents there. AWS IAM signing keys, Google service-account keys, and OAuth client credentials require mechanisms beyond static header injection and are rejected rather than diff --git a/docs/architecture/sandboxes.md b/docs/architecture/sandboxes.md index 2848ee41d8..73444bd3cb 100644 --- a/docs/architecture/sandboxes.md +++ b/docs/architecture/sandboxes.md @@ -115,8 +115,14 @@ TTL. Helm supplies these through the controller ConfigMap. Defaults are one CPU, 1 GiB memory, a one-hour TTL, and a 24-hour maximum TTL. Activity does not extend expiration. -Sandboxes have no allowed egress destinations. A follow-up will add destination -configuration to `SandboxTemplate` and pin it with each prepared revision. +`SandboxTemplate.spec.egress` lists the HTTP(S) origins a sandbox may reach, in +the same form as `Agent.spec.egress`: origins, with optional Secret-backed +headers the egress gateway sets (see [credential injection](credential-injection.md)). +A template without it reaches nothing. Egress is part of the prepared revision, +and a sandbox gets its revision's policy when it is created, so a template +change applies to new sandboxes; existing ones keep the egress they started with. +A header whose Secret or key is missing holds preparation on `SecretNotFound` or +`SecretKeyNotFound` until it appears. ## Lifecycle and recovery 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 3a4845f7b5..4d95bacb65 100644 --- a/go/api/config/crd/bases/api.kagent.dev_agents.yaml +++ b/go/api/config/crd/bases/api.kagent.dev_agents.yaml @@ -60,12 +60,100 @@ spec: one label, such as raw.githubusercontent.com. A wildcard needs two labels under it, so nothing opens a whole top-level domain or every host. items: - maxLength: 270 - pattern: ^https?://(\*\.([a-z0-9]([-a-z0-9]{0,61}[a-z0-9])?\.)+|([a-z0-9]([-a-z0-9]{0,61}[a-z0-9])?\.)*)[a-z]([-a-z0-9]{0,61}[a-z0-9])?(:([1-9][0-9]{0,3}|[1-5][0-9]{4}|6[0-4][0-9]{3}|65[0-4][0-9]{2}|655[0-2][0-9]|6553[0-5]))?$ - type: string + description: EgressEntry is one origin a runtime may reach. + properties: + headers: + description: |- + Headers the egress gateway sets, so the runtime never holds their + values. The gateway only replaces a header the request sends, so the + runtime sends each with a placeholder. Credentials bind per host, not + per port. Whoever can write the resource can send any Secret in its + namespace to the origin. + items: + description: EgressHeader is a request header the egress gateway + sets. + properties: + name: + maxLength: 256 + pattern: ^[!#$%&'*+.^_|~0-9A-Za-z-]+$ + type: string + prefix: + description: Prefix goes before the value, such as "Bearer + ". + maxLength: 64 + pattern: ^[^\x00-\x08\x0a-\x1f\x7f]*$ + type: string + valueFrom: + description: EgressHeaderSource is where a header's value + comes from. + properties: + secretKeyRef: + description: SecretKeyRef is a key of a Secret in + the resource's namespace. + properties: + key: + description: The key of the secret to select from. Must + be a valid secret key. + type: string + name: + default: "" + description: |- + Name of the referent. + This field is effectively required, but due to backwards compatibility is + allowed to be empty. Instances of this type with an empty value here are + almost certainly wrong. + More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + type: string + optional: + description: Specify whether the Secret or its + key must be defined + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + x-kubernetes-validations: + - message: secretKeyRef needs a name and a key + rule: self.name != '' && self.key != '' + - message: a header's Secret cannot be optional + rule: '!has(self.optional) || !self.optional' + type: object + x-kubernetes-validations: + - message: secretKeyRef is required + rule: has(self.secretKeyRef) + required: + - name + - valueFrom + type: object + x-kubernetes-validations: + - message: the gateway cannot set the Host header + rule: self.name.lowerAscii() != 'host' + maxItems: 16 + type: array + x-kubernetes-list-map-keys: + - name + x-kubernetes-list-type: map + x-kubernetes-validations: + - message: header names must be unique, ignoring case + rule: self.all(a, self.exists_one(b, a.name.lowerAscii() == + b.name.lowerAscii())) + origin: + description: Origin is an HTTP(S) origin, such as "https://proxy.golang.org". + maxLength: 270 + pattern: ^https?://(\*\.([a-z0-9]([-a-z0-9]{0,61}[a-z0-9])?\.)+|([a-z0-9]([-a-z0-9]{0,61}[a-z0-9])?\.)*)[a-z]([-a-z0-9]{0,61}[a-z0-9])?(:([1-9][0-9]{0,3}|[1-5][0-9]{4}|6[0-4][0-9]{3}|65[0-4][0-9]{2}|655[0-2][0-9]|6553[0-5]))?$ + type: string + required: + - origin + type: object + x-kubernetes-validations: + - message: headers need an exact https origin + rule: '!has(self.headers) || (self.origin.startsWith(''https://'') + && !self.origin.contains(''*''))' maxItems: 64 type: array - x-kubernetes-list-type: set + x-kubernetes-list-map-keys: + - origin + x-kubernetes-list-type: map harness: description: HarnessSpec defines a reusable runtime and its infrastructure policy. diff --git a/go/api/config/crd/bases/api.kagent.dev_sandboxtemplates.yaml b/go/api/config/crd/bases/api.kagent.dev_sandboxtemplates.yaml index 80b1a4b595..c4b17b9eae 100644 --- a/go/api/config/crd/bases/api.kagent.dev_sandboxtemplates.yaml +++ b/go/api/config/crd/bases/api.kagent.dev_sandboxtemplates.yaml @@ -51,6 +51,105 @@ spec: description: SandboxTemplateSpec defines standalone sandbox environment configuration. properties: + egress: + description: |- + Egress lists the HTTP(S) origins a sandbox may reach, in the same form as + an Agent's. A sandbox keeps the egress it was created with. + items: + description: EgressEntry is one origin a runtime may reach. + properties: + headers: + description: |- + Headers the egress gateway sets, so the runtime never holds their + values. The gateway only replaces a header the request sends, so the + runtime sends each with a placeholder. Credentials bind per host, not + per port. Whoever can write the resource can send any Secret in its + namespace to the origin. + items: + description: EgressHeader is a request header the egress gateway + sets. + properties: + name: + maxLength: 256 + pattern: ^[!#$%&'*+.^_|~0-9A-Za-z-]+$ + type: string + prefix: + description: Prefix goes before the value, such as "Bearer + ". + maxLength: 64 + pattern: ^[^\x00-\x08\x0a-\x1f\x7f]*$ + type: string + valueFrom: + description: EgressHeaderSource is where a header's value + comes from. + properties: + secretKeyRef: + description: SecretKeyRef is a key of a Secret in + the resource's namespace. + properties: + key: + description: The key of the secret to select from. Must + be a valid secret key. + type: string + name: + default: "" + description: |- + Name of the referent. + This field is effectively required, but due to backwards compatibility is + allowed to be empty. Instances of this type with an empty value here are + almost certainly wrong. + More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + type: string + optional: + description: Specify whether the Secret or its + key must be defined + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + x-kubernetes-validations: + - message: secretKeyRef needs a name and a key + rule: self.name != '' && self.key != '' + - message: a header's Secret cannot be optional + rule: '!has(self.optional) || !self.optional' + type: object + x-kubernetes-validations: + - message: secretKeyRef is required + rule: has(self.secretKeyRef) + required: + - name + - valueFrom + type: object + x-kubernetes-validations: + - message: the gateway cannot set the Host header + rule: self.name.lowerAscii() != 'host' + maxItems: 16 + type: array + x-kubernetes-list-map-keys: + - name + x-kubernetes-list-type: map + x-kubernetes-validations: + - message: header names must be unique, ignoring case + rule: self.all(a, self.exists_one(b, a.name.lowerAscii() == + b.name.lowerAscii())) + origin: + description: Origin is an HTTP(S) origin, such as "https://proxy.golang.org". + maxLength: 270 + pattern: ^https?://(\*\.([a-z0-9]([-a-z0-9]{0,61}[a-z0-9])?\.)+|([a-z0-9]([-a-z0-9]{0,61}[a-z0-9])?\.)*)[a-z]([-a-z0-9]{0,61}[a-z0-9])?(:([1-9][0-9]{0,3}|[1-5][0-9]{4}|6[0-4][0-9]{3}|65[0-4][0-9]{2}|655[0-2][0-9]|6553[0-5]))?$ + type: string + required: + - origin + type: object + x-kubernetes-validations: + - message: headers need an exact https origin + rule: '!has(self.headers) || (self.origin.startsWith(''https://'') + && !self.origin.contains(''*''))' + maxItems: 64 + type: array + x-kubernetes-list-map-keys: + - origin + x-kubernetes-list-type: map env: description: |- Env supplies runtime environment defaults. diff --git a/go/api/v1alpha3/agent_types.go b/go/api/v1alpha3/agent_types.go index 20e9879e9c..62dc76f9d3 100644 --- a/go/api/v1alpha3/agent_types.go +++ b/go/api/v1alpha3/agent_types.go @@ -44,11 +44,57 @@ type AgentSpec struct { // one label, such as raw.githubusercontent.com. A wildcard needs two labels // under it, so nothing opens a whole top-level domain or every host. // +optional - // +listType=set + // +listType=map + // +listMapKey=origin // +kubebuilder:validation:MaxItems=64 - // +kubebuilder:validation:items:MaxLength=270 - // +kubebuilder:validation:items:Pattern=`^https?://(\*\.([a-z0-9]([-a-z0-9]{0,61}[a-z0-9])?\.)+|([a-z0-9]([-a-z0-9]{0,61}[a-z0-9])?\.)*)[a-z]([-a-z0-9]{0,61}[a-z0-9])?(:([1-9][0-9]{0,3}|[1-5][0-9]{4}|6[0-4][0-9]{3}|65[0-4][0-9]{2}|655[0-2][0-9]|6553[0-5]))?$` - Egress []string `json:"egress,omitempty"` + Egress []EgressEntry `json:"egress,omitempty"` +} + +// EgressEntry is one origin a runtime may reach. +// +kubebuilder:validation:XValidation:rule="!has(self.headers) || (self.origin.startsWith('https://') && !self.origin.contains('*'))",message="headers need an exact https origin" +type EgressEntry struct { + // Origin is an HTTP(S) origin, such as "https://proxy.golang.org". + // +required + // +kubebuilder:validation:MaxLength=270 + // +kubebuilder:validation:Pattern=`^https?://(\*\.([a-z0-9]([-a-z0-9]{0,61}[a-z0-9])?\.)+|([a-z0-9]([-a-z0-9]{0,61}[a-z0-9])?\.)*)[a-z]([-a-z0-9]{0,61}[a-z0-9])?(:([1-9][0-9]{0,3}|[1-5][0-9]{4}|6[0-4][0-9]{3}|65[0-4][0-9]{2}|655[0-2][0-9]|6553[0-5]))?$` + Origin string `json:"origin"` + // Headers the egress gateway sets, so the runtime never holds their + // values. The gateway only replaces a header the request sends, so the + // runtime sends each with a placeholder. Credentials bind per host, not + // per port. Whoever can write the resource can send any Secret in its + // namespace to the origin. + // +optional + // +listType=map + // +listMapKey=name + // +kubebuilder:validation:MaxItems=16 + // +kubebuilder:validation:XValidation:rule="self.all(a, self.exists_one(b, a.name.lowerAscii() == b.name.lowerAscii()))",message="header names must be unique, ignoring case" + Headers []EgressHeader `json:"headers,omitempty"` +} + +// EgressHeader is a request header the egress gateway sets. +// +kubebuilder:validation:XValidation:rule="self.name.lowerAscii() != 'host'",message="the gateway cannot set the Host header" +type EgressHeader struct { + // +required + // +kubebuilder:validation:MaxLength=256 + // +kubebuilder:validation:Pattern=`^[!#$%&'*+.^_|~0-9A-Za-z-]+$` + Name string `json:"name"` + // Prefix goes before the value, such as "Bearer ". + // +optional + // +kubebuilder:validation:MaxLength=64 + // +kubebuilder:validation:Pattern=`^[^\x00-\x08\x0a-\x1f\x7f]*$` + Prefix string `json:"prefix,omitempty"` + // +required + ValueFrom EgressHeaderSource `json:"valueFrom"` +} + +// EgressHeaderSource is where a header's value comes from. +// +kubebuilder:validation:XValidation:rule="has(self.secretKeyRef)",message="secretKeyRef is required" +type EgressHeaderSource struct { + // SecretKeyRef is a key of a Secret in the resource's namespace. + // +optional + // +kubebuilder:validation:XValidation:rule="self.name != '' && self.key != ''",message="secretKeyRef needs a name and a key" + // +kubebuilder:validation:XValidation:rule="!has(self.optional) || !self.optional",message="a header's Secret cannot be optional" + SecretKeyRef *corev1.SecretKeySelector `json:"secretKeyRef,omitempty"` } const ( diff --git a/go/api/v1alpha3/configuration_crd_cel_test.go b/go/api/v1alpha3/configuration_crd_cel_test.go index 204602b844..25be6caed3 100644 --- a/go/api/v1alpha3/configuration_crd_cel_test.go +++ b/go/api/v1alpha3/configuration_crd_cel_test.go @@ -250,6 +250,29 @@ func TestConfigurationCRDValidation(t *testing.T) { }), wantReject: "Duplicate value", }, + { + name: "SandboxTemplate allows egress with headers on an exact https origin", + object: sandboxTemplateForValidation(namespace, "sandbox-egress", func(spec *SandboxTemplateSpec) { + spec.Egress = []EgressEntry{ + {Origin: "https://*.githubusercontent.com"}, + {Origin: "https://api.internal.example", Headers: []EgressHeader{{Name: "Authorization", Prefix: "Bearer ", ValueFrom: EgressHeaderSource{SecretKeyRef: &corev1.SecretKeySelector{LocalObjectReference: corev1.LocalObjectReference{Name: "api"}, Key: "token"}}}}}, + } + }), + }, + { + name: "SandboxTemplate rejects egress headers on a wildcard origin", + object: sandboxTemplateForValidation(namespace, "sandbox-egress-wildcard", func(spec *SandboxTemplateSpec) { + spec.Egress = []EgressEntry{{Origin: "https://*.example.com", Headers: []EgressHeader{{Name: "Authorization", ValueFrom: EgressHeaderSource{SecretKeyRef: &corev1.SecretKeySelector{LocalObjectReference: corev1.LocalObjectReference{Name: "api"}, Key: "token"}}}}}} + }), + wantReject: "headers need an exact https origin", + }, + { + name: "SandboxTemplate rejects a duplicate egress origin", + object: sandboxTemplateForValidation(namespace, "sandbox-egress-duplicate", func(spec *SandboxTemplateSpec) { + spec.Egress = []EgressEntry{{Origin: "https://api.internal.example"}, {Origin: "https://api.internal.example"}} + }), + wantReject: "Duplicate value", + }, } for _, tc := range []struct { @@ -343,7 +366,7 @@ func TestConfigurationCRDValidation(t *testing.T) { } t.Run("Agent egress accepts HTTP(S) origins and leftmost-label wildcards", func(t *testing.T) { spec := AgentSpec{TemplateRef: &corev1.LocalObjectReference{Name: "behavior"}, HarnessRef: &corev1.LocalObjectReference{Name: "runner"}, - Egress: []string{"https://proxy.golang.org", "http://mirror.internal:8080", "https://git.internal:65535", "https://*.githubusercontent.com"}} + Egress: []EgressEntry{{Origin: "https://proxy.golang.org"}, {Origin: "http://mirror.internal:8080"}, {Origin: "https://git.internal:65535"}, {Origin: "https://*.githubusercontent.com"}}} require.NoError(t, cl.Create(ctx, &Agent{ObjectMeta: metav1.ObjectMeta{Namespace: namespace, Name: "egress"}, Spec: spec})) }) for i, origin := range []string{ @@ -352,10 +375,50 @@ func TestConfigurationCRDValidation(t *testing.T) { "https://proxy.golang.org#f", "https://proxy.golang.org:0", "https://proxy.golang.org:70000", } { t.Run("Agent egress rejects "+origin, func(t *testing.T) { - spec := AgentSpec{TemplateRef: &corev1.LocalObjectReference{Name: "behavior"}, HarnessRef: &corev1.LocalObjectReference{Name: "runner"}, Egress: []string{origin}} + spec := AgentSpec{TemplateRef: &corev1.LocalObjectReference{Name: "behavior"}, HarnessRef: &corev1.LocalObjectReference{Name: "runner"}, Egress: []EgressEntry{{Origin: origin}}} require.ErrorContains(t, cl.Create(ctx, &Agent{ObjectMeta: metav1.ObjectMeta{Namespace: namespace, Name: fmt.Sprintf("egress-invalid-%d", i)}, Spec: spec}), "spec.egress[0]") }) } + secretHeader := func(name, secretName string) EgressHeader { + return EgressHeader{Name: name, Prefix: "Bearer ", ValueFrom: EgressHeaderSource{SecretKeyRef: &corev1.SecretKeySelector{LocalObjectReference: corev1.LocalObjectReference{Name: secretName}, Key: "token"}}} + } + withHeaders := func(origin string, headers ...EgressHeader) AgentSpec { + return AgentSpec{TemplateRef: &corev1.LocalObjectReference{Name: "behavior"}, HarnessRef: &corev1.LocalObjectReference{Name: "runner"}, + Egress: []EgressEntry{{Origin: origin, Headers: headers}}} + } + t.Run("Agent egress rejects a duplicate origin", func(t *testing.T) { + spec := AgentSpec{TemplateRef: &corev1.LocalObjectReference{Name: "behavior"}, HarnessRef: &corev1.LocalObjectReference{Name: "runner"}, + Egress: []EgressEntry{{Origin: "https://api.internal.example"}, {Origin: "https://api.internal.example"}}} + require.ErrorContains(t, cl.Create(ctx, &Agent{ObjectMeta: metav1.ObjectMeta{Namespace: namespace, Name: "egress-duplicate"}, Spec: spec}), "Duplicate value") + }) + t.Run("Agent egress accepts headers on an exact https origin", func(t *testing.T) { + spec := withHeaders("https://api.internal.example", secretHeader("Authorization", "internal-api"), secretHeader("X-App-Id", "internal-api")) + require.NoError(t, cl.Create(ctx, &Agent{ObjectMeta: metav1.ObjectMeta{Namespace: namespace, Name: "egress-headers"}, Spec: spec})) + }) + optional := true + for i, tc := range []struct { + name string + spec AgentSpec + want string + }{ + {"an http origin", withHeaders("http://api.internal.example", secretHeader("Authorization", "internal-api")), "headers need an exact https origin"}, + {"a wildcard origin", withHeaders("https://*.internal.example", secretHeader("Authorization", "internal-api")), "headers need an exact https origin"}, + {"names differing only in case", withHeaders("https://api.internal.example", secretHeader("Authorization", "a"), secretHeader("authorization", "b")), "header names must be unique"}, + {"an invalid header name", withHeaders("https://api.internal.example", secretHeader("Bad Header", "a")), "spec.egress[0].headers[0].name"}, + {"no source", withHeaders("https://api.internal.example", EgressHeader{Name: "Authorization"}), "secretKeyRef is required"}, + {"an empty Secret name", withHeaders("https://api.internal.example", secretHeader("Authorization", "")), "secretKeyRef needs a name and a key"}, + {"an empty key", withHeaders("https://api.internal.example", EgressHeader{Name: "Authorization", ValueFrom: EgressHeaderSource{SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{Name: "a"}}}}), "secretKeyRef needs a name and a key"}, + {"the Host header", withHeaders("https://api.internal.example", secretHeader("host", "a")), "cannot set the Host header"}, + {"a control character in the prefix", withHeaders("https://api.internal.example", EgressHeader{Name: "Authorization", Prefix: "Bearer\x00", ValueFrom: EgressHeaderSource{SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{Name: "a"}, Key: "token"}}}), "spec.egress[0].headers[0].prefix"}, + {"an optional Secret", withHeaders("https://api.internal.example", EgressHeader{Name: "Authorization", ValueFrom: EgressHeaderSource{SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{Name: "a"}, Key: "token", Optional: &optional}}}), "cannot be optional"}, + } { + t.Run("Agent egress rejects headers with "+tc.name, func(t *testing.T) { + require.ErrorContains(t, cl.Create(ctx, &Agent{ObjectMeta: metav1.ObjectMeta{Namespace: namespace, Name: fmt.Sprintf("egress-headers-invalid-%d", i)}, Spec: tc.spec}), tc.want) + }) + } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { diff --git a/go/api/v1alpha3/sandboxtemplate_types.go b/go/api/v1alpha3/sandboxtemplate_types.go index 9687b688a9..d5765837db 100644 --- a/go/api/v1alpha3/sandboxtemplate_types.go +++ b/go/api/v1alpha3/sandboxtemplate_types.go @@ -37,6 +37,14 @@ type SandboxTemplateSpec struct { // resolved in this template's namespace. // +required Substrate RuntimeSubstratePolicy `json:"substrate"` + + // Egress lists the HTTP(S) origins a sandbox may reach, in the same form as + // an Agent's. A sandbox keeps the egress it was created with. + // +optional + // +listType=map + // +listMapKey=origin + // +kubebuilder:validation:MaxItems=64 + Egress []EgressEntry `json:"egress,omitempty"` } // SandboxTemplateStatus reports preparation of the current template inputs. diff --git a/go/api/v1alpha3/zz_generated.deepcopy.go b/go/api/v1alpha3/zz_generated.deepcopy.go index d5426ad471..efd5e98db3 100644 --- a/go/api/v1alpha3/zz_generated.deepcopy.go +++ b/go/api/v1alpha3/zz_generated.deepcopy.go @@ -111,8 +111,10 @@ func (in *AgentSpec) DeepCopyInto(out *AgentSpec) { } if in.Egress != nil { in, out := &in.Egress, &out.Egress - *out = make([]string, len(*in)) - copy(*out, *in) + *out = make([]EgressEntry, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } } } @@ -565,6 +567,64 @@ func (in *ConfigMapKeyReference) DeepCopy() *ConfigMapKeyReference { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *EgressEntry) DeepCopyInto(out *EgressEntry) { + *out = *in + if in.Headers != nil { + in, out := &in.Headers, &out.Headers + *out = make([]EgressHeader, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new EgressEntry. +func (in *EgressEntry) DeepCopy() *EgressEntry { + if in == nil { + return nil + } + out := new(EgressEntry) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *EgressHeader) DeepCopyInto(out *EgressHeader) { + *out = *in + in.ValueFrom.DeepCopyInto(&out.ValueFrom) +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new EgressHeader. +func (in *EgressHeader) DeepCopy() *EgressHeader { + if in == nil { + return nil + } + out := new(EgressHeader) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *EgressHeaderSource) DeepCopyInto(out *EgressHeaderSource) { + *out = *in + if in.SecretKeyRef != nil { + in, out := &in.SecretKeyRef, &out.SecretKeyRef + *out = new(v1.SecretKeySelector) + (*in).DeepCopyInto(*out) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new EgressHeaderSource. +func (in *EgressHeaderSource) DeepCopy() *EgressHeaderSource { + if in == nil { + return nil + } + out := new(EgressHeaderSource) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *FoundryConfig) DeepCopyInto(out *FoundryConfig) { *out = *in @@ -1632,6 +1692,13 @@ func (in *SandboxTemplateSpec) DeepCopyInto(out *SandboxTemplateSpec) { copy(*out, *in) } out.Substrate = in.Substrate + if in.Egress != nil { + in, out := &in.Egress, &out.Egress + *out = make([]EgressEntry, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new SandboxTemplateSpec. diff --git a/go/core/internal/controller/collections_test.go b/go/core/internal/controller/collections_test.go index 6d8cd169d0..e3db2e344f 100644 --- a/go/core/internal/controller/collections_test.go +++ b/go/core/internal/controller/collections_test.go @@ -7,6 +7,7 @@ import ( atev1alpha1 "github.com/agent-substrate/substrate/pkg/api/v1alpha1" "github.com/agent-substrate/substrate/pkg/proto/ateapipb" kagentv1alpha3 "github.com/kagent-dev/kagent/go/api/v1alpha3" + "github.com/kagent-dev/kagent/go/core/internal/egress" "github.com/kagent-dev/kagent/go/core/internal/translator" "github.com/stretchr/testify/require" "google.golang.org/protobuf/proto" @@ -453,6 +454,65 @@ func harness(namespace, name string, matchLabels map[string]string) *kagentv1alp } } +func TestReconciliationWaitsForEgressHeaderSecret(t *testing.T) { + stop := make(chan struct{}) + t.Cleanup(func() { close(stop) }) + opts := krt.NewOptionsBuilder(stop, "test-egress", nil) + template := &kagentv1alpha3.AgentTemplate{ + ObjectMeta: metav1.ObjectMeta{Namespace: "team-a", Name: "assistant", UID: "template-uid"}, + Spec: kagentv1alpha3.AgentTemplateSpec{ModelConfig: &corev1.LocalObjectReference{Name: "model"}, SystemPrompt: "help"}, + } + runtimeHarness := harness("team-a", "kagent", nil) + runtimeHarness.Spec.Kagent = &kagentv1alpha3.KagentHarness{} + runtimeHarness.Spec.Workload.Image = "example.com/agent@sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + runtimeHarness.Spec.Substrate = kagentv1alpha3.RuntimeSubstratePolicy{ + WorkerPoolRef: corev1.LocalObjectReference{Name: "default"}, SnapshotPolicy: kagentv1alpha3.RuntimeSnapshotPolicy{Location: "snapshots"}, + } + model := &kagentv1alpha3.ModelConfig{ + ObjectMeta: metav1.ObjectMeta{Namespace: "team-a", Name: "model"}, + Spec: kagentv1alpha3.ModelConfigSpec{Provider: kagentv1alpha3.ModelProviderOpenAI, Model: "gpt-5", APIKeySecret: "model-auth", APIKeySecretKey: "api-key"}, + } + agent := testAgent(template, runtimeHarness) + agent.Spec.Egress = []kagentv1alpha3.EgressEntry{{Origin: "https://api.internal.example", Headers: []kagentv1alpha3.EgressHeader{{ + Name: "Authorization", Prefix: "Bearer ", + ValueFrom: kagentv1alpha3.EgressHeaderSource{SecretKeyRef: &corev1.SecretKeySelector{LocalObjectReference: corev1.LocalObjectReference{Name: "internal-api"}, Key: "token"}}, + }}}} + mock := krttest.NewMock(t, []any{template, runtimeHarness, model}) + configMaps := krttest.GetMockCollection[*corev1.ConfigMap](mock) + secrets := krt.NewStaticCollection(nil, []*corev1.Secret{ + {ObjectMeta: metav1.ObjectMeta{Namespace: "team-a", Name: "model-auth"}, Data: map[string][]byte{"api-key": []byte("secret")}}, + }, opts.WithName("Secrets")...) + _, resolvedModels := newModelConfigReconciliations(krttest.GetMockCollection[*kagentv1alpha3.ModelConfig](mock), configMaps, secrets, opts) + workerPools := krt.NewStaticCollection(nil, []*atev1alpha1.WorkerPool{{ObjectMeta: metav1.ObjectMeta{Namespace: "team-a", Name: "default"}}}, opts.WithName("WorkerPools")...) + observations := krt.NewStaticCollection[AgentRuntimeObservation](nil, nil, opts.WithName("AgentRuntimeObservations")...) + reconciliations := newAgentReconciliations(krt.NewStaticCollection(nil, []*kagentv1alpha3.Agent{agent}, opts.WithName("Agents")...), translator.Collections{ + Harnesses: krttest.GetMockCollection[*kagentv1alpha3.Harness](mock), AgentTemplates: krttest.GetMockCollection[*kagentv1alpha3.AgentTemplate](mock), + ResolvedModelConfigs: resolvedModels, RemoteMCPServers: krttest.GetMockCollection[*kagentv1alpha3.RemoteMCPServer](mock), + ConfigMaps: configMaps, Secrets: secrets, WorkerPools: workerPools, + }, observations, opts) + key := "team-a/assistant" + failureReason := func() string { + state := reconciliations.GetKey(key) + if state == nil || state.CompilationFailure == nil { + return "" + } + require.Equal(t, kagentv1alpha3.AgentConditionResolvedRefs, state.CompilationFailure.Condition) + return state.CompilationFailure.Reason + } + + waitFor(t, func() bool { return failureReason() == "SecretNotFound" }) + secrets.UpdateObject(&corev1.Secret{ObjectMeta: metav1.ObjectMeta{Namespace: "team-a", Name: "internal-api"}, Data: map[string][]byte{"other": []byte("t")}}) + waitFor(t, func() bool { return failureReason() == "SecretKeyNotFound" }) + secrets.UpdateObject(&corev1.Secret{ObjectMeta: metav1.ObjectMeta{Namespace: "team-a", Name: "internal-api"}, Data: map[string][]byte{"token": []byte("t")}}) + waitFor(t, func() bool { + state := reconciliations.GetKey(key) + return state != nil && state.CompilationFailure == nil && state.Target != nil + }) + require.Contains(t, reconciliations.GetKey(key).Target.Revision.Credentials, egress.Credential{ + Hostname: "api.internal.example", Header: "authorization", Prefix: "Bearer ", URI: "ate-secret://k8s.io/default/team-a/internal-api/token", + }) +} + func waitFor(t *testing.T, condition func() bool) { t.Helper() deadline := time.Now().Add(time.Second) diff --git a/go/core/internal/controller/reconciler.go b/go/core/internal/controller/reconciler.go index 82970da08f..b90207c386 100644 --- a/go/core/internal/controller/reconciler.go +++ b/go/core/internal/controller/reconciler.go @@ -108,11 +108,16 @@ func newAgentReconciliations( condition, reason := kagentv1alpha3.AgentConditionResolvedRefs, "ReferenceResolutionFailed" var validation *translator.ValidationError var missingPool *translator.WorkerPoolNotFoundError + var missingSecret *translator.SecretNotFoundError switch { case errors.As(err, &validation): condition, reason = kagentv1alpha3.AgentConditionCompatible, "UnsupportedConfiguration" case errors.As(err, &missingPool): reason = "WorkerPoolNotFound" + case errors.As(err, &missingSecret) && missingSecret.Key != "": + reason = "SecretKeyNotFound" + case errors.As(err, &missingSecret): + reason = "SecretNotFound" } state.CompilationFailure = &ReconciliationFailure{Condition: condition, Reason: reason, Message: err.Error()} return state diff --git a/go/core/internal/controller/sandbox_collections.go b/go/core/internal/controller/sandbox_collections.go index 01b01babb9..8ed8fc36ba 100644 --- a/go/core/internal/controller/sandbox_collections.go +++ b/go/core/internal/controller/sandbox_collections.go @@ -2,12 +2,15 @@ package controller import ( "encoding/json" + "errors" "fmt" "reflect" "github.com/agent-substrate/substrate/pkg/proto/ateapipb" kagentv1alpha3 "github.com/kagent-dev/kagent/go/api/v1alpha3" + "github.com/kagent-dev/kagent/go/core/internal/egress" "github.com/kagent-dev/kagent/go/core/internal/substrate" + "github.com/kagent-dev/kagent/go/core/internal/translator" "google.golang.org/protobuf/proto" "istio.io/istio/pkg/kube/krt" ) @@ -23,6 +26,8 @@ type sandboxReconciliation struct { Template *kagentv1alpha3.SandboxTemplate RevisionID string SourceSnapshot json.RawMessage + EgressDestinations []string + Credentials []egress.Credential DesiredActorTemplate *ateapipb.ActorTemplate ObservedActorTemplate *ateapipb.ActorTemplate CompilationError string @@ -86,6 +91,11 @@ func newSandboxCollections(inputs Collections, policy substrate.SandboxPolicy, o pool := krt.FetchOne(ctx, inputs.WorkerPools, krt.FilterKey(template.Namespace+"/"+template.Spec.Substrate.WorkerPoolRef.Name)) if pool == nil { state.Failure = &ReconciliationFailure{Reason: "WorkerPoolNotFound", Message: "The referenced sandbox WorkerPool does not exist"} + } else if err := compileSandboxEgress(ctx, inputs, template, state); err != nil { + state.Failure = sandboxEgressFailure(err) + if state.Failure.Reason == sandboxPreparationFailed { + state.CompilationError = err.Error() + } } else { var err error state.DesiredActorTemplate, state.RevisionID, state.SourceSnapshot, err = substrate.SandboxActorTemplate(template, (*pool).Spec.SandboxClass, policy) @@ -107,3 +117,41 @@ func newSandboxCollections(inputs Collections, policy substrate.SandboxPolicy, o }, opts.WithName("SandboxReconciliations")...) return sandboxCollections{states: states, observations: observations} } + +// The policy is checked at preparation but applied only when a sandbox is created. +func compileSandboxEgress(ctx krt.HandlerContext, inputs Collections, template *kagentv1alpha3.SandboxTemplate, state *sandboxReconciliation) error { + declared := template.Spec.Egress + destinations, err := translator.WithEgress(nil, declared) + if err != nil { + return err + } + if err := translator.CheckEgressHeaderPorts(destinations, declared); err != nil { + return err + } + credentials, err := translator.EgressCredentials(template.Namespace, declared, func(namespace, name, key string) error { + return translator.RequireSecretKey(ctx, inputs.Secrets, namespace, name, key) + }) + if err != nil { + return err + } + if credentials, err = egress.CanonicalCredentials(credentials); err != nil { + return translator.NewValidationError("%v", err) + } + if _, err := substrate.ActorEgressPolicy(template.Namespace, destinations, credentials); err != nil { + return err + } + state.EgressDestinations, state.Credentials = destinations, credentials + return nil +} + +func sandboxEgressFailure(err error) *ReconciliationFailure { + var missing *translator.SecretNotFoundError + switch { + case errors.As(err, &missing) && missing.Key != "": + return &ReconciliationFailure{Reason: "SecretKeyNotFound", Message: err.Error()} + case errors.As(err, &missing): + return &ReconciliationFailure{Reason: "SecretNotFound", Message: err.Error()} + default: + return &ReconciliationFailure{Reason: sandboxPreparationFailed, Message: sandboxPreparationFailureMessage} + } +} diff --git a/go/core/internal/controller/sandbox_reconciler.go b/go/core/internal/controller/sandbox_reconciler.go index fe910b85c6..1e274cb566 100644 --- a/go/core/internal/controller/sandbox_reconciler.go +++ b/go/core/internal/controller/sandbox_reconciler.go @@ -172,7 +172,7 @@ func (s *SandboxReconciler) reconcile(ctx context.Context, key string) error { RuntimeArtifact: database.RuntimeArtifact{Revision: state.RevisionID, Kind: "sandbox", Namespace: state.Template.Namespace, ActorTemplateAtespace: ref.GetAtespace(), ActorTemplateName: ref.GetName(), ActorTemplateUID: observed.GetMetadata().GetUid()}, SandboxTemplateName: state.Template.Name, SandboxTemplateUID: string(state.Template.UID), - SourceSnapshot: state.SourceSnapshot, + SourceSnapshot: state.SourceSnapshot, EgressDestinations: state.EgressDestinations, Credentials: state.Credentials, } if err := s.store.RecordSandboxRevision(ctx, revision, ready); err != nil { return s.observeError(*state, fmt.Errorf("store sandbox revision %s: %w", state.RevisionID, err), true) diff --git a/go/core/internal/controller/sandbox_reconciler_test.go b/go/core/internal/controller/sandbox_reconciler_test.go index 500ac371f6..f9a16b9252 100644 --- a/go/core/internal/controller/sandbox_reconciler_test.go +++ b/go/core/internal/controller/sandbox_reconciler_test.go @@ -115,20 +115,27 @@ func sandboxTestTemplate() *kagentv1alpha3.SandboxTemplate { } func newSandboxTestReconciler(t *testing.T, guestImage string) (*SandboxReconciler, krt.StaticCollection[*kagentv1alpha3.SandboxTemplate], krt.StaticCollection[*atev1alpha1.WorkerPool]) { + t.Helper() + reconciler, templates, pools, _ := newSandboxTestReconcilerWithSecrets(t, guestImage) + return reconciler, templates, pools +} + +func newSandboxTestReconcilerWithSecrets(t *testing.T, guestImage string) (*SandboxReconciler, krt.StaticCollection[*kagentv1alpha3.SandboxTemplate], krt.StaticCollection[*atev1alpha1.WorkerPool], krt.StaticCollection[*corev1.Secret]) { t.Helper() opts := krt.NewOptionsBuilder(t.Context().Done(), "test-sandbox", nil) template := sandboxTestTemplate() pool := &atev1alpha1.WorkerPool{ObjectMeta: metav1.ObjectMeta{Namespace: template.Namespace, Name: "default"}} templates := krt.NewStaticCollection(nil, []*kagentv1alpha3.SandboxTemplate{template}, opts.WithName("SandboxTemplates")...) pools := krt.NewStaticCollection(nil, []*atev1alpha1.WorkerPool{pool}, opts.WithName("WorkerPools")...) + secrets := krt.NewStaticCollection[*corev1.Secret](nil, nil, opts.WithName("Secrets")...) store := &sandboxTestStore{} actors := &sandboxTestActors{store: store, templates: map[string]*ateapipb.ActorTemplate{}} reconciler := &SandboxReconciler{ - collections: newSandboxCollections(Collections{SandboxTemplates: templates, WorkerPools: pools}, substrate.SandboxPolicy{GuestImage: guestImage, CPU: "1", Memory: "1Gi"}, opts), + collections: newSandboxCollections(Collections{SandboxTemplates: templates, WorkerPools: pools, Secrets: secrets}, substrate.SandboxPolicy{GuestImage: guestImage, CPU: "1", Memory: "1Gi"}, opts), store: store, actors: actors, client: kagentfake.NewSimpleClientset(template.DeepCopy()).ApiV1alpha3(), } waitFor(t, func() bool { return reconciler.collections.states.GetKey("team-a/scratch") != nil }) - return reconciler, templates, pools + return reconciler, templates, pools, secrets } func syncSandboxTemplate(t *testing.T, reconciler *SandboxReconciler, templates krt.StaticCollection[*kagentv1alpha3.SandboxTemplate]) *kagentv1alpha3.SandboxTemplate { @@ -294,3 +301,56 @@ func TestSandboxPendingStatusRecoversWithoutGraphEvent(t *testing.T) { return err == nil && apimeta.IsStatusConditionTrue(current.Status.Conditions, "Ready") }, 5*time.Second, 10*time.Millisecond) } + +func TestSandboxEgress(t *testing.T) { + s, templates, _, secrets := newSandboxTestReconcilerWithSecrets(t, "guest@sha256:"+strings.Repeat("b", 64)) + const key = "team-a/scratch" + before := s.collections.states.GetKey(key).RevisionID + + t.Run("compiles origins and waits for a header's Secret", func(t *testing.T) { + template := (*templates.GetKey(key)).DeepCopy() + template.Generation++ + template.Spec.Egress = []kagentv1alpha3.EgressEntry{ + {Origin: "https://*.githubusercontent.com"}, + {Origin: "https://api.internal.example", Headers: []kagentv1alpha3.EgressHeader{{ + Name: "Authorization", Prefix: "Bearer ", + ValueFrom: kagentv1alpha3.EgressHeaderSource{SecretKeyRef: &corev1.SecretKeySelector{LocalObjectReference: corev1.LocalObjectReference{Name: "internal-api"}, Key: "token"}}, + }}}, + } + templates.UpdateObject(template) + waitFor(t, func() bool { return s.collections.states.GetKey(key).Template.Generation == template.Generation }) + missing := s.collections.states.GetKey(key) + require.Equal(t, "SecretNotFound", missing.Failure.Reason) + require.Contains(t, missing.Failure.Message, "internal-api") + require.Nil(t, missing.DesiredActorTemplate) + + secrets.UpdateObject(&corev1.Secret{ObjectMeta: metav1.ObjectMeta{Namespace: "team-a", Name: "internal-api"}, Data: map[string][]byte{"token": []byte("t")}}) + waitFor(t, func() bool { return s.collections.states.GetKey(key).Failure == nil }) + state := s.collections.states.GetKey(key) + require.NotEqual(t, before, state.RevisionID, "egress is part of the revision") + require.Equal(t, []string{"https://*.githubusercontent.com:443", "https://api.internal.example:443"}, state.EgressDestinations) + require.Len(t, state.Credentials, 1) + require.Equal(t, "ate-secret://k8s.io/default/team-a/internal-api/token", state.Credentials[0].URI) + + require.NoError(t, s.reconcile(t.Context(), key)) + syncSandboxTemplate(t, s, templates) + require.NoError(t, s.reconcile(t.Context(), key)) + recorded := s.store.(*sandboxTestStore).revision + require.Equal(t, state.EgressDestinations, recorded.EgressDestinations) + require.Equal(t, state.Credentials, recorded.Credentials) + }) + + t.Run("rejects headers on a wildcard origin", func(t *testing.T) { + template := (*templates.GetKey(key)).DeepCopy() + template.Generation++ + template.Spec.Egress = []kagentv1alpha3.EgressEntry{{Origin: "https://*.example.com", Headers: []kagentv1alpha3.EgressHeader{{ + Name: "Authorization", + ValueFrom: kagentv1alpha3.EgressHeaderSource{SecretKeyRef: &corev1.SecretKeySelector{LocalObjectReference: corev1.LocalObjectReference{Name: "internal-api"}, Key: "token"}}, + }}}} + templates.UpdateObject(template) + waitFor(t, func() bool { return s.collections.states.GetKey(key).Template.Generation == template.Generation }) + state := s.collections.states.GetKey(key) + require.Equal(t, sandboxPreparationFailed, state.Failure.Reason) + require.Contains(t, state.CompilationError, "exact https origin") + }) +} diff --git a/go/core/internal/database/sandbox_revisions.go b/go/core/internal/database/sandbox_revisions.go index 5bee645014..d648a229a7 100644 --- a/go/core/internal/database/sandbox_revisions.go +++ b/go/core/internal/database/sandbox_revisions.go @@ -6,6 +6,7 @@ import ( "fmt" "github.com/jackc/pgx/v5" + "github.com/kagent-dev/kagent/go/core/internal/egress" ) type SandboxRevision struct { @@ -13,6 +14,8 @@ type SandboxRevision struct { SandboxTemplateName string SandboxTemplateUID string SourceSnapshot json.RawMessage + EgressDestinations []string + Credentials []egress.Credential } // SandboxTemplateDefinition tracks the desired and last successful runtime for a SandboxTemplate UID. @@ -79,7 +82,10 @@ func (c *Client) RecordSandboxRevision(ctx context.Context, revision SandboxRevi return err } revision.Kind = runtimeKindSandbox - if err := recordRuntimeRevision(ctx, tx, runtimeRevisionRecord{RuntimeArtifact: revision.RuntimeArtifact, SourceSnapshot: revision.SourceSnapshot}); err != nil { + if err := recordRuntimeRevision(ctx, tx, runtimeRevisionRecord{ + RuntimeArtifact: revision.RuntimeArtifact, SourceSnapshot: revision.SourceSnapshot, + EgressDestinations: revision.EgressDestinations, Credentials: revision.Credentials, + }); err != nil { return err } if err := execSQL(ctx, tx, ` @@ -103,11 +109,14 @@ func (c *Client) RecordSandboxRevision(ctx context.Context, revision SandboxRevi func (c *Client) GetSandboxRevision(ctx context.Context, revision string) (*SandboxRevision, error) { row, err := queryOne(ctx, c.db, ` SELECT r.revision, r.kind, r.namespace, r.actor_template_atespace, r.actor_template_name, r.actor_template_uid, - r.deleted_at, r.source_snapshot, s.sandbox_template_name, s.sandbox_template_uid + r.deleted_at, r.source_snapshot, r.egress_destinations, r.credentials, s.sandbox_template_name, s.sandbox_template_uid FROM runtime_revision r JOIN sandbox_revision s USING (revision) WHERE r.revision = $1 AND r.kind = 'sandbox' `, pgx.RowToStructByName[SandboxRevision], revision) if err != nil { return nil, fmt.Errorf("get sandbox revision: %w", notFoundOr(err)) } + if row.Credentials, err = egress.CanonicalCredentials(row.Credentials); err != nil { + return nil, fmt.Errorf("decode sandbox revision %s credentials: %w", revision, err) + } return &row, nil } diff --git a/go/core/internal/database/sandboxes_test.go b/go/core/internal/database/sandboxes_test.go index f224f04d52..39b81cbd54 100644 --- a/go/core/internal/database/sandboxes_test.go +++ b/go/core/internal/database/sandboxes_test.go @@ -8,6 +8,7 @@ import ( "github.com/google/uuid" apiv1alpha1 "github.com/kagent-dev/kagent/go/api/gen/kagent/api/v1alpha1" + "github.com/kagent-dev/kagent/go/core/internal/egress" "github.com/stretchr/testify/require" "google.golang.org/protobuf/proto" ) @@ -231,3 +232,30 @@ func TestSandboxLifecycleSupersedesEndedAttempts(t *testing.T) { require.ErrorIs(t, err, ErrFailedPrecondition, "deletion must not be canceled into leaked compute") finishSandbox(t, client, created.Id, apiv1alpha1.RuntimeOperation_RUNTIME_OPERATION_DELETE) } + +func TestSandboxRevisionKeepsItsEgress(t *testing.T) { + client := NewClient(setupTestDB(t), "public") + ctx := t.Context() + const revision = "egress-revision" + require.NoError(t, client.UpsertSandboxTemplateDefinition(ctx, SandboxTemplateDefinition{Namespace: "team-a", SandboxTemplateName: "scratch", SandboxTemplateUID: "uid", DesiredRevision: revision})) + destinations := []string{"https://api.internal.example:443", "https://*.githubusercontent.com:443"} + credentials := []egress.Credential{{Hostname: "api.internal.example", Header: "authorization", Prefix: "Bearer ", URI: "ate-secret://k8s.io/default/team-a/internal-api/token"}} + require.NoError(t, client.RecordSandboxRevision(ctx, SandboxRevision{ + RuntimeArtifact: RuntimeArtifact{Revision: revision, Kind: "sandbox", Namespace: "team-a", ActorTemplateAtespace: "team-a", ActorTemplateName: revision, ActorTemplateUID: "actor-uid"}, + SandboxTemplateName: "scratch", SandboxTemplateUID: "uid", SourceSnapshot: []byte(`{}`), + EgressDestinations: destinations, Credentials: credentials, + }, true)) + + got, err := client.GetSandboxRevision(ctx, revision) + require.NoError(t, err) + require.Equal(t, destinations, got.EgressDestinations) + require.Equal(t, credentials, got.Credentials) + + t.Run("a template without egress records none", func(t *testing.T) { + sandboxFixture(t, client, "plain-revision") + plain, err := client.GetSandboxRevision(ctx, "plain-revision") + require.NoError(t, err) + require.Empty(t, plain.EgressDestinations) + require.Empty(t, plain.Credentials) + }) +} diff --git a/go/core/internal/mcp/prompts.go b/go/core/internal/mcp/prompts.go index e5ee7dfe66..da2b963b94 100644 --- a/go/core/internal/mcp/prompts.go +++ b/go/core/internal/mcp/prompts.go @@ -12,7 +12,7 @@ import ( ) const sandboxInstructions = `Use standalone Sandboxes for commands and files; their lifetime is independent of Sessions. -Discover a prepared SandboxTemplate with the programs needed for the task. Sandboxes currently have no allowed network egress; upload inputs and use dependencies already in the image. Work in /data/workspace. +Discover a prepared SandboxTemplate with the programs needed for the task. A sandbox reaches only the origins its SandboxTemplate allows; upload other inputs and use dependencies already in the image. Work in /data/workspace. Create with a unique request_id retained for retries, or reuse a sandbox belonging to the task. Check state, operation, and expires_at before work. Activity does not extend expiration. Start a process once, retain its process_id, and use get_sandbox_process to check terminal status and exit_code. A lost start response may mean the process is running: never blindly retry a start. Read output with read_sandbox_outputs, decode base64, and continue with both returned byte offsets. Empty output does not mean completion. MCP file transfers and output reads are limited to 1 MiB; file writes replace the destination. diff --git a/go/core/internal/mcp/sandboxes.go b/go/core/internal/mcp/sandboxes.go index 541d29c518..25162694a2 100644 --- a/go/core/internal/mcp/sandboxes.go +++ b/go/core/internal/mcp/sandboxes.go @@ -147,7 +147,7 @@ func registerSandboxTools(server *mcp.Server, service *sandbox.Service, template if service == nil { return } - addSandboxTool(server, "create_sandbox", "Create a temporary workspace from a prepared SandboxTemplate. Retain request_id and identical inputs for lifecycle retries. Check state, operation, and expires_at before guest calls. No network egress is currently allowed; files belong under /data/workspace.", func(ctx context.Context, in sandboxCreateInput) (sandboxSummary, error) { + addSandboxTool(server, "create_sandbox", "Create a temporary workspace from a prepared SandboxTemplate. Retain request_id and identical inputs for lifecycle retries. Check state, operation, and expires_at before guest calls. The network is limited to the origins its SandboxTemplate allows; files belong under /data/workspace.", func(ctx context.Context, in sandboxCreateInput) (sandboxSummary, error) { request := &apiv1alpha1.CreateSandboxRequest{SandboxTemplate: &apiv1alpha1.ResourceReference{Namespace: in.Namespace, Name: in.Template}, RequestId: in.RequestID, Name: in.Name} if in.TTLSeconds != 0 { request.Ttl = &durationpb.Duration{Seconds: in.TTLSeconds} diff --git a/go/core/internal/service/sandbox/workflow.go b/go/core/internal/service/sandbox/workflow.go index 4e275a3af2..78648d05b0 100644 --- a/go/core/internal/service/sandbox/workflow.go +++ b/go/core/internal/service/sandbox/workflow.go @@ -42,7 +42,7 @@ func (s *Service) run(ctx context.Context, id string, kind apiv1alpha1.RuntimeOp TemplateAtespace: revision.ActorTemplateAtespace, TemplateName: revision.ActorTemplateName} var creation *substrate.ActorCreation if kind == apiv1alpha1.RuntimeOperation_RUNTIME_OPERATION_CREATE { - policy, err := substrate.ActorEgressPolicy(binding.Atespace, nil, nil) + policy, err := substrate.ActorEgressPolicy(binding.Atespace, revision.EgressDestinations, revision.Credentials) if err != nil { return nil, sandboxError(err) } diff --git a/go/core/internal/translator/compiler.go b/go/core/internal/translator/compiler.go index b2e9db8f21..95f911b6c7 100644 --- a/go/core/internal/translator/compiler.go +++ b/go/core/internal/translator/compiler.go @@ -5,7 +5,6 @@ import ( "encoding/json" "fmt" "maps" - "slices" "github.com/kagent-dev/kagent/go/api/v1alpha3" "github.com/kagent-dev/kagent/go/core/internal/egress" @@ -64,6 +63,9 @@ type HarnessInput struct { Harness *HarnessConfiguration Root *AgentInput OutputSchema *ResolvedOutputSchema + // EgressCredentials are the Agent's egress header bindings, checked with + // the runtime's own. + EgressCredentials []egress.Credential } // AgentInput contains resolved Kubernetes inputs for one agent. @@ -122,15 +124,22 @@ func (c *Compiler) CompileAgent(ctx context.Context, agent *v1alpha3.Agent) (*Co } harness = harnessConfiguration(*found) } - result, err := c.compileConfiguration(ctx, agent.Name, harness, template) + credentials, err := EgressCredentials(agent.Namespace, agent.Spec.Egress, c.requireSecretKey) + if err != nil { + return nil, err + } + result, err := c.compileConfiguration(ctx, agent.Name, harness, template, credentials) if err != nil { return nil, err } result.AgentUID = string(agent.UID) - result.EgressDestinations, err = withAgentEgress(result.EgressDestinations, agent.Spec.Egress) + result.EgressDestinations, err = WithEgress(result.EgressDestinations, agent.Spec.Egress) if err != nil { return nil, err } + if err := CheckEgressHeaderPorts(result.EgressDestinations, agent.Spec.Egress); err != nil { + return nil, err + } result.Provenance, err = json.Marshal(struct { AgentName string `json:"agentName"` AgentUID string `json:"agentUID"` @@ -143,29 +152,14 @@ func (c *Compiler) CompileAgent(ctx context.Context, agent *v1alpha3.Agent) (*Co return result, nil } -// withAgentEgress adds the Agent's declared origins to the destinations its -// runtime compiled, in canonical form and once each. Every runtime gets the -// same treatment, so no runtime compiler reads the Agent's egress. -func withAgentEgress(compiled, declared []string) ([]string, error) { - if len(declared) == 0 { - return compiled, nil - } - destinations := slices.Clone(compiled) - for _, value := range declared { - origin, err := egress.ParseOrigin(value) - if err != nil { - return nil, NewValidationError("Agent egress: %v", err) - } - if !slices.Contains(destinations, origin) { - destinations = append(destinations, origin) - } - } - return destinations, nil +// requireSecretKey returns a reference failure, which the controller retries. +func (c *Compiler) requireSecretKey(namespace, name, key string) error { + return RequireSecretKey(c.ctx, c.collections.Secrets, namespace, name, key) } // compileConfiguration compiles resolved configuration for the named Agent. // The Agent name owns runtime identity; template and Harness names are provenance. -func (c *Compiler) compileConfiguration(ctx context.Context, agentName string, harness *HarnessConfiguration, template *TemplateConfiguration) (*CompileResult, error) { +func (c *Compiler) compileConfiguration(ctx context.Context, agentName string, harness *HarnessConfiguration, template *TemplateConfiguration, credentials []egress.Credential) (*CompileResult, error) { harnessCompiler := c.harnessCompilers[harnessType(harness)] if harnessCompiler == nil { return nil, NewValidationError("Harness runtime is not supported by any compiler") @@ -179,6 +173,7 @@ func (c *Compiler) compileConfiguration(ctx context.Context, agentName string, h return nil, err } input.AgentName = agentName + input.EgressCredentials = credentials result, err := harnessCompiler.Compile(ctx, input) if err != nil { return nil, err diff --git a/go/core/internal/translator/compiler_test.go b/go/core/internal/translator/compiler_test.go index e23e6aedf9..50b0f1f68a 100644 --- a/go/core/internal/translator/compiler_test.go +++ b/go/core/internal/translator/compiler_test.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "encoding/json" + "errors" "slices" "strings" "testing" @@ -11,6 +12,7 @@ import ( atev1alpha1 "github.com/agent-substrate/substrate/pkg/api/v1alpha1" "github.com/kagent-dev/kagent/go/api/adk" "github.com/kagent-dev/kagent/go/api/v1alpha3" + "github.com/kagent-dev/kagent/go/core/internal/egress" "github.com/kagent-dev/kagent/go/core/internal/substrate" "github.com/kagent-dev/kagent/go/core/internal/translator" byotranslator "github.com/kagent-dev/kagent/go/core/internal/translator/byo" @@ -992,7 +994,7 @@ func TestCompileAgentAddsTheAgentEgress(t *testing.T) { Substrate: v1alpha3.RuntimeSubstratePolicy{WorkerPoolRef: corev1.LocalObjectReference{Name: "default"}, SnapshotPolicy: v1alpha3.RuntimeSnapshotPolicy{Location: "snapshots"}}, }, } - declared := []string{"https://proxy.golang.org", "https://*.githubusercontent.com", "https://Proxy.Golang.org.", "https://git.internal:8443"} + declared := egressOrigins("https://proxy.golang.org", "https://*.githubusercontent.com", "https://Proxy.Golang.org.", "https://git.internal:8443") want := []string{"https://proxy.golang.org:443", "https://*.githubusercontent.com:443", "https://git.internal:8443"} c := compiler(t, modelConfig(), template, harness) @@ -1030,12 +1032,154 @@ func TestCompileAgentAddsTheAgentEgress(t *testing.T) { } invalid := referenced.DeepCopy() - invalid.Spec.Egress = []string{"https://proxy.golang.org/path"} + invalid.Spec.Egress = egressOrigins("https://proxy.golang.org/path") _, err = c.CompileAgent(t.Context(), invalid) - var validation *v2translator.ValidationError + var validation *translator.ValidationError require.ErrorAs(t, err, &validation) } +func egressOrigins(origins ...string) []v1alpha3.EgressEntry { + entries := make([]v1alpha3.EgressEntry, len(origins)) + for i, origin := range origins { + entries[i] = v1alpha3.EgressEntry{Origin: origin} + } + return entries +} + +func TestCompileAgentBindsTheAgentEgressHeaders(t *testing.T) { + template := &v1alpha3.AgentTemplate{ + ObjectMeta: metav1.ObjectMeta{Name: "api-reader", Namespace: "test"}, + Spec: v1alpha3.AgentTemplateSpec{ModelConfig: &corev1.LocalObjectReference{Name: "default-model"}, SystemPrompt: "help"}, + } + harness := &v1alpha3.Harness{ + ObjectMeta: metav1.ObjectMeta{Name: "kagent", Namespace: "test"}, + Spec: v1alpha3.HarnessSpec{ + Kagent: &v1alpha3.KagentHarness{}, + Workload: v1alpha3.HarnessWorkload{Image: "example.com/runtime@sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"}, + Substrate: v1alpha3.RuntimeSubstratePolicy{WorkerPoolRef: corev1.LocalObjectReference{Name: "default"}, SnapshotPolicy: v1alpha3.RuntimeSnapshotPolicy{Location: "snapshots"}}, + }, + } + secret := &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: "internal-api", Namespace: "test"}, Data: map[string][]byte{"token": []byte("t"), "other": []byte("o")}} + header := func(name, prefix, secretName, key string) v1alpha3.EgressHeader { + return v1alpha3.EgressHeader{Name: name, Prefix: prefix, ValueFrom: v1alpha3.EgressHeaderSource{SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{Name: secretName}, Key: key, + }}} + } + agent := &v1alpha3.Agent{ObjectMeta: metav1.ObjectMeta{Name: "reader", Namespace: "test"}, Spec: v1alpha3.AgentSpec{ + TemplateRef: &corev1.LocalObjectReference{Name: template.Name}, HarnessRef: &corev1.LocalObjectReference{Name: harness.Name}, + Egress: []v1alpha3.EgressEntry{{Origin: "https://API.internal.example", Headers: []v1alpha3.EgressHeader{header("Authorization", "Bearer ", "internal-api", "token")}}}, + }} + uri := "ate-secret://k8s.io/default/test/internal-api/token" + + t.Run("binds the Secret to the origin's host", func(t *testing.T) { + result, err := compiler(t, modelConfig(), template, harness, secret).CompileAgent(t.Context(), agent) + require.NoError(t, err) + require.Contains(t, result.EgressDestinations, "https://api.internal.example:443") + require.Contains(t, result.Credentials, egress.Credential{Hostname: "api.internal.example", Header: "authorization", Prefix: "Bearer ", URI: uri}) + policy, err := substrate.ActorEgressPolicy("test", result.EgressDestinations, result.Credentials) + require.NoError(t, err) + var injected []string + for _, rule := range policy.Rules { + if slices.Contains(rule.GetHttps().GetHostnames(), "api.internal.example") { + for _, replace := range rule.GetHttps().GetEffects().GetReplaceHeaders() { + injected = append(injected, replace.GetCredentialUri()) + } + } + } + require.Equal(t, []string{uri}, injected) + }) + + t.Run("a missing Secret or key is a reference failure", func(t *testing.T) { + for name, objects := range map[string][]any{ + "secret": {modelConfig(), template, harness}, + "key": {modelConfig(), template, harness, &corev1.Secret{ObjectMeta: secret.ObjectMeta, Data: map[string][]byte{"other": []byte("t")}}}, + } { + t.Run(name, func(t *testing.T) { + _, err := compiler(t, objects...).CompileAgent(t.Context(), agent) + var missing *translator.SecretNotFoundError + require.ErrorAs(t, err, &missing) + var validation *translator.ValidationError + require.False(t, errors.As(err, &validation)) + }) + } + }) + + t.Run("an identical runtime binding on the host is shared", func(t *testing.T) { + model := modelConfig() + model.Spec.APIKeySecret, model.Spec.APIKeySecretKey = "internal-api", "token" + shared := agent.DeepCopy() + shared.Spec.Egress[0].Origin = "https://api.openai.com" + result, err := compiler(t, model, template, harness, secret).CompileAgent(t.Context(), shared) + require.NoError(t, err) + require.Equal(t, 1, countCredentials(result.Credentials, "api.openai.com")) + }) + + for name, change := range map[string]func(*v1alpha3.Agent){ + "http origin": func(a *v1alpha3.Agent) { a.Spec.Egress[0].Origin = "http://api.internal.example" }, + "wildcard origin": func(a *v1alpha3.Agent) { a.Spec.Egress[0].Origin = "https://*.internal.example" }, + "Host header": func(a *v1alpha3.Agent) { a.Spec.Egress[0].Headers[0].Name = "Host" }, + "invalid key": func(a *v1alpha3.Agent) { a.Spec.Egress[0].Headers[0].ValueFrom.SecretKeyRef.Key = "a/b c" }, + "invalid name": func(a *v1alpha3.Agent) { a.Spec.Egress[0].Headers[0].ValueFrom.SecretKeyRef.Name = "Bad/Name" }, + "conflicting credentials": func(a *v1alpha3.Agent) { + a.Spec.Egress[0].Headers = append(a.Spec.Egress[0].Headers, header("authorization", "", "internal-api", "other")) + }, + "the host on another port": func(a *v1alpha3.Agent) { + a.Spec.Egress = append(a.Spec.Egress, v1alpha3.EgressEntry{Origin: "https://api.internal.example:8443"}) + }, + "a runtime binding on the host": func(a *v1alpha3.Agent) { a.Spec.Egress[0].Origin = "https://api.openai.com" }, + } { + t.Run(name, func(t *testing.T) { + invalid := agent.DeepCopy() + change(invalid) + model := modelConfig() + model.Spec.APIKeySecret, model.Spec.APIKeySecretKey = "model-auth", "api-key" + modelSecret := &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: "model-auth", Namespace: "test"}, Data: map[string][]byte{"api-key": []byte("k")}} + _, err := compiler(t, model, template, harness, secret, modelSecret).CompileAgent(t.Context(), invalid) + var validation *translator.ValidationError + require.ErrorAs(t, err, &validation) + }) + } +} + +func TestCompileAgentKeepsCallerTokenPassthrough(t *testing.T) { + model := modelConfig() + model.Spec.APIKeyPassthrough = true + template := &v1alpha3.AgentTemplate{ + ObjectMeta: metav1.ObjectMeta{Name: "api-reader", Namespace: "test"}, + Spec: v1alpha3.AgentTemplateSpec{ModelConfig: &corev1.LocalObjectReference{Name: "default-model"}, SystemPrompt: "help"}, + } + harness := &v1alpha3.Harness{ + ObjectMeta: metav1.ObjectMeta{Name: "kagent", Namespace: "test"}, + Spec: v1alpha3.HarnessSpec{ + Kagent: &v1alpha3.KagentHarness{}, + Workload: v1alpha3.HarnessWorkload{Image: "example.com/runtime@sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"}, + Substrate: v1alpha3.RuntimeSubstratePolicy{WorkerPoolRef: corev1.LocalObjectReference{Name: "default"}, SnapshotPolicy: v1alpha3.RuntimeSnapshotPolicy{Location: "snapshots"}}, + }, + } + secret := &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: "shared", Namespace: "test"}, Data: map[string][]byte{"token": []byte("t")}} + agent := &v1alpha3.Agent{ObjectMeta: metav1.ObjectMeta{Name: "reader", Namespace: "test"}, Spec: v1alpha3.AgentSpec{ + TemplateRef: &corev1.LocalObjectReference{Name: template.Name}, HarnessRef: &corev1.LocalObjectReference{Name: harness.Name}, + Egress: []v1alpha3.EgressEntry{{Origin: "https://api.openai.com", Headers: []v1alpha3.EgressHeader{{ + Name: "Authorization", Prefix: "Bearer ", + ValueFrom: v1alpha3.EgressHeaderSource{SecretKeyRef: &corev1.SecretKeySelector{LocalObjectReference: corev1.LocalObjectReference{Name: "shared"}, Key: "token"}}, + }}}}, + }} + _, err := compiler(t, model, template, harness, secret).CompileAgent(t.Context(), agent) + var validation *translator.ValidationError + require.ErrorAs(t, err, &validation) + require.ErrorContains(t, err, "passthrough") +} + +func countCredentials(credentials []egress.Credential, hostname string) int { + n := 0 + for _, c := range credentials { + if c.Hostname == hostname { + n++ + } + } + return n +} + func countOf(values []string, want string) int { n := 0 for _, value := range values { diff --git a/go/core/internal/translator/credentials.go b/go/core/internal/translator/credentials.go index 6872047f52..fdc91ef3f1 100644 --- a/go/core/internal/translator/credentials.go +++ b/go/core/internal/translator/credentials.go @@ -85,6 +85,7 @@ func CompileCredentials(input *HarnessInput, extraModels []*ResolvedModelConfig, } boundModels[name+"\x00"+model.Spec.APIKeySecret+"\x00"+key] = true } + bindings = append(bindings, input.EgressCredentials...) bindings, err := egress.CanonicalCredentials(bindings) if err != nil { return nil, nil, NewValidationError("%v", err) diff --git a/go/core/internal/translator/egress.go b/go/core/internal/translator/egress.go new file mode 100644 index 0000000000..ff7cd41b3c --- /dev/null +++ b/go/core/internal/translator/egress.go @@ -0,0 +1,123 @@ +package translator + +import ( + "fmt" + "net/url" + "slices" + "strings" + + "github.com/kagent-dev/kagent/go/api/v1alpha3" + "github.com/kagent-dev/kagent/go/core/internal/egress" + "istio.io/istio/pkg/kube/krt" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/types" +) + +// Agents and SandboxTemplates declare egress alike and compile it here. + +// SecretKeyLookup returns a SecretNotFoundError when a Secret key is missing. +// Fetching through krt recompiles the caller when the Secret changes. +type SecretKeyLookup func(namespace, name, key string) error + +// RequireSecretKey is the SecretKeyLookup over a Secret collection. +func RequireSecretKey(ctx krt.HandlerContext, secrets krt.Collection[*corev1.Secret], namespace, name, key string) error { + secret := krt.FetchOne(ctx, secrets, krt.FilterObjectName(types.NamespacedName{Namespace: namespace, Name: name})) + if secret == nil { + return &SecretNotFoundError{Secret: types.NamespacedName{Namespace: namespace, Name: name}} + } + if _, ok := (*secret).Data[key]; !ok { + return &SecretNotFoundError{Secret: types.NamespacedName{Namespace: namespace, Name: name}, Key: key} + } + return nil +} + +// WithEgress adds declared origins to a runtime's destinations, once each. +func WithEgress(compiled []string, declared []v1alpha3.EgressEntry) ([]string, error) { + if len(declared) == 0 { + return compiled, nil + } + destinations := slices.Clone(compiled) + for _, entry := range declared { + origin, err := egress.ParseOrigin(entry.Origin) + if err != nil { + return nil, NewValidationError("egress: %v", err) + } + if !slices.Contains(destinations, origin) { + destinations = append(destinations, origin) + } + } + return destinations, nil +} + +// EgressCredentials compiles declared headers to gateway bindings. +func EgressCredentials(namespace string, declared []v1alpha3.EgressEntry, lookup SecretKeyLookup) ([]egress.Credential, error) { + var bindings []egress.Credential + for _, entry := range declared { + if len(entry.Headers) == 0 { + continue + } + host, _, err := headerOrigin(entry.Origin) + if err != nil { + return nil, err + } + for _, header := range entry.Headers { + ref := header.ValueFrom.SecretKeyRef + if ref == nil { + return nil, NewValidationError("egress %q header %q: secretKeyRef is required", entry.Origin, header.Name) + } + binding := egress.Credential{ + Hostname: host, + Header: header.Name, + Prefix: header.Prefix, + URI: "ate-secret://k8s.io/default/" + namespace + "/" + ref.Name + "/" + ref.Key, + } + // A malformed reference can never resolve, so it is invalid, not missing. + if _, err := egress.CanonicalCredentials([]egress.Credential{binding}); err != nil { + return nil, NewValidationError("egress %q header %q: %v", entry.Origin, header.Name, err) + } + if strings.EqualFold(header.Name, "host") { + return nil, NewValidationError("egress %q: the gateway cannot set the Host header", entry.Origin) + } + if err := lookup(namespace, ref.Name, ref.Key); err != nil { + return nil, fmt.Errorf("egress %q header %q: %w", entry.Origin, header.Name, err) + } + bindings = append(bindings, binding) + } + } + return bindings, nil +} + +// headerOrigin returns the host and port of an origin that may carry headers: +// exact and https, since Substrate injects only into intercepted HTTPS. +func headerOrigin(value string) (host, port string, err error) { + origin, err := egress.ParseOrigin(value) + if err != nil { + return "", "", NewValidationError("egress: %v", err) + } + u, err := url.Parse(origin) + if err != nil || u.Scheme != "https" || strings.HasPrefix(u.Hostname(), "*.") { + return "", "", NewValidationError("egress %q: headers need an exact https origin", value) + } + return u.Hostname(), u.Port(), nil +} + +// CheckEgressHeaderPorts refuses headers on a host also reached over HTTPS on +// another port, since Substrate binds credentials per host. +func CheckEgressHeaderPorts(destinations []string, declared []v1alpha3.EgressEntry) error { + for _, entry := range declared { + if len(entry.Headers) == 0 { + continue + } + host, port, err := headerOrigin(entry.Origin) + if err != nil { + return err + } + for _, destination := range destinations { + u, err := url.Parse(destination) + if err == nil && u.Scheme == "https" && u.Hostname() == host && u.Port() != port { + return NewValidationError("egress %q: headers would also be sent to %s, since the gateway binds them per host", entry.Origin, destination) + } + } + } + return nil +} diff --git a/go/core/internal/translator/validation.go b/go/core/internal/translator/validation.go index 2e0b00854f..8fbf69ddf6 100644 --- a/go/core/internal/translator/validation.go +++ b/go/core/internal/translator/validation.go @@ -15,6 +15,19 @@ func (e *WorkerPoolNotFoundError) Error() string { return fmt.Sprintf("WorkerPool %q not found", e.WorkerPool.String()) } +// SecretNotFoundError identifies a missing Secret, or a missing key when Key is set. +type SecretNotFoundError struct { + Secret types.NamespacedName + Key string +} + +func (e *SecretNotFoundError) Error() string { + if e.Key != "" { + return fmt.Sprintf("Secret %q has no key %q", e.Secret.String(), e.Key) + } + return fmt.Sprintf("Secret %q not found", e.Secret.String()) +} + // ValidationError marks a resolved but unsupported public configuration. The // controller reports these as a compatibility condition instead of retrying // them like transient Kubernetes lookup failures. diff --git a/helm/kagent-crds/templates/api.kagent.dev_agents.yaml b/helm/kagent-crds/templates/api.kagent.dev_agents.yaml index 3a4845f7b5..4d95bacb65 100644 --- a/helm/kagent-crds/templates/api.kagent.dev_agents.yaml +++ b/helm/kagent-crds/templates/api.kagent.dev_agents.yaml @@ -60,12 +60,100 @@ spec: one label, such as raw.githubusercontent.com. A wildcard needs two labels under it, so nothing opens a whole top-level domain or every host. items: - maxLength: 270 - pattern: ^https?://(\*\.([a-z0-9]([-a-z0-9]{0,61}[a-z0-9])?\.)+|([a-z0-9]([-a-z0-9]{0,61}[a-z0-9])?\.)*)[a-z]([-a-z0-9]{0,61}[a-z0-9])?(:([1-9][0-9]{0,3}|[1-5][0-9]{4}|6[0-4][0-9]{3}|65[0-4][0-9]{2}|655[0-2][0-9]|6553[0-5]))?$ - type: string + description: EgressEntry is one origin a runtime may reach. + properties: + headers: + description: |- + Headers the egress gateway sets, so the runtime never holds their + values. The gateway only replaces a header the request sends, so the + runtime sends each with a placeholder. Credentials bind per host, not + per port. Whoever can write the resource can send any Secret in its + namespace to the origin. + items: + description: EgressHeader is a request header the egress gateway + sets. + properties: + name: + maxLength: 256 + pattern: ^[!#$%&'*+.^_|~0-9A-Za-z-]+$ + type: string + prefix: + description: Prefix goes before the value, such as "Bearer + ". + maxLength: 64 + pattern: ^[^\x00-\x08\x0a-\x1f\x7f]*$ + type: string + valueFrom: + description: EgressHeaderSource is where a header's value + comes from. + properties: + secretKeyRef: + description: SecretKeyRef is a key of a Secret in + the resource's namespace. + properties: + key: + description: The key of the secret to select from. Must + be a valid secret key. + type: string + name: + default: "" + description: |- + Name of the referent. + This field is effectively required, but due to backwards compatibility is + allowed to be empty. Instances of this type with an empty value here are + almost certainly wrong. + More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + type: string + optional: + description: Specify whether the Secret or its + key must be defined + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + x-kubernetes-validations: + - message: secretKeyRef needs a name and a key + rule: self.name != '' && self.key != '' + - message: a header's Secret cannot be optional + rule: '!has(self.optional) || !self.optional' + type: object + x-kubernetes-validations: + - message: secretKeyRef is required + rule: has(self.secretKeyRef) + required: + - name + - valueFrom + type: object + x-kubernetes-validations: + - message: the gateway cannot set the Host header + rule: self.name.lowerAscii() != 'host' + maxItems: 16 + type: array + x-kubernetes-list-map-keys: + - name + x-kubernetes-list-type: map + x-kubernetes-validations: + - message: header names must be unique, ignoring case + rule: self.all(a, self.exists_one(b, a.name.lowerAscii() == + b.name.lowerAscii())) + origin: + description: Origin is an HTTP(S) origin, such as "https://proxy.golang.org". + maxLength: 270 + pattern: ^https?://(\*\.([a-z0-9]([-a-z0-9]{0,61}[a-z0-9])?\.)+|([a-z0-9]([-a-z0-9]{0,61}[a-z0-9])?\.)*)[a-z]([-a-z0-9]{0,61}[a-z0-9])?(:([1-9][0-9]{0,3}|[1-5][0-9]{4}|6[0-4][0-9]{3}|65[0-4][0-9]{2}|655[0-2][0-9]|6553[0-5]))?$ + type: string + required: + - origin + type: object + x-kubernetes-validations: + - message: headers need an exact https origin + rule: '!has(self.headers) || (self.origin.startsWith(''https://'') + && !self.origin.contains(''*''))' maxItems: 64 type: array - x-kubernetes-list-type: set + x-kubernetes-list-map-keys: + - origin + x-kubernetes-list-type: map harness: description: HarnessSpec defines a reusable runtime and its infrastructure policy. diff --git a/helm/kagent-crds/templates/api.kagent.dev_sandboxtemplates.yaml b/helm/kagent-crds/templates/api.kagent.dev_sandboxtemplates.yaml index 80b1a4b595..c4b17b9eae 100644 --- a/helm/kagent-crds/templates/api.kagent.dev_sandboxtemplates.yaml +++ b/helm/kagent-crds/templates/api.kagent.dev_sandboxtemplates.yaml @@ -51,6 +51,105 @@ spec: description: SandboxTemplateSpec defines standalone sandbox environment configuration. properties: + egress: + description: |- + Egress lists the HTTP(S) origins a sandbox may reach, in the same form as + an Agent's. A sandbox keeps the egress it was created with. + items: + description: EgressEntry is one origin a runtime may reach. + properties: + headers: + description: |- + Headers the egress gateway sets, so the runtime never holds their + values. The gateway only replaces a header the request sends, so the + runtime sends each with a placeholder. Credentials bind per host, not + per port. Whoever can write the resource can send any Secret in its + namespace to the origin. + items: + description: EgressHeader is a request header the egress gateway + sets. + properties: + name: + maxLength: 256 + pattern: ^[!#$%&'*+.^_|~0-9A-Za-z-]+$ + type: string + prefix: + description: Prefix goes before the value, such as "Bearer + ". + maxLength: 64 + pattern: ^[^\x00-\x08\x0a-\x1f\x7f]*$ + type: string + valueFrom: + description: EgressHeaderSource is where a header's value + comes from. + properties: + secretKeyRef: + description: SecretKeyRef is a key of a Secret in + the resource's namespace. + properties: + key: + description: The key of the secret to select from. Must + be a valid secret key. + type: string + name: + default: "" + description: |- + Name of the referent. + This field is effectively required, but due to backwards compatibility is + allowed to be empty. Instances of this type with an empty value here are + almost certainly wrong. + More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + type: string + optional: + description: Specify whether the Secret or its + key must be defined + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + x-kubernetes-validations: + - message: secretKeyRef needs a name and a key + rule: self.name != '' && self.key != '' + - message: a header's Secret cannot be optional + rule: '!has(self.optional) || !self.optional' + type: object + x-kubernetes-validations: + - message: secretKeyRef is required + rule: has(self.secretKeyRef) + required: + - name + - valueFrom + type: object + x-kubernetes-validations: + - message: the gateway cannot set the Host header + rule: self.name.lowerAscii() != 'host' + maxItems: 16 + type: array + x-kubernetes-list-map-keys: + - name + x-kubernetes-list-type: map + x-kubernetes-validations: + - message: header names must be unique, ignoring case + rule: self.all(a, self.exists_one(b, a.name.lowerAscii() == + b.name.lowerAscii())) + origin: + description: Origin is an HTTP(S) origin, such as "https://proxy.golang.org". + maxLength: 270 + pattern: ^https?://(\*\.([a-z0-9]([-a-z0-9]{0,61}[a-z0-9])?\.)+|([a-z0-9]([-a-z0-9]{0,61}[a-z0-9])?\.)*)[a-z]([-a-z0-9]{0,61}[a-z0-9])?(:([1-9][0-9]{0,3}|[1-5][0-9]{4}|6[0-4][0-9]{3}|65[0-4][0-9]{2}|655[0-2][0-9]|6553[0-5]))?$ + type: string + required: + - origin + type: object + x-kubernetes-validations: + - message: headers need an exact https origin + rule: '!has(self.headers) || (self.origin.startsWith(''https://'') + && !self.origin.contains(''*''))' + maxItems: 64 + type: array + x-kubernetes-list-map-keys: + - origin + x-kubernetes-list-type: map env: description: |- Env supplies runtime environment defaults. diff --git a/ui/src/api/domain/agents.ts b/ui/src/api/domain/agents.ts index ceb5c7c8b6..edaae7e8de 100644 --- a/ui/src/api/domain/agents.ts +++ b/ui/src/api/domain/agents.ts @@ -7,8 +7,21 @@ export type AgentSpec = ({ template: AgentTemplateSpec; templateRef?: never } | { templateRef: { name: string }; template?: never }) & ({ harness: HarnessSpec; harnessRef?: never } | { harnessRef: { name: string }; harness?: never }) & { /** HTTP(S) origins the Agent may reach besides what its revision compiles. */ - egress?: string[]; + egress?: AgentEgress[]; }; + +/** An origin the Agent may reach, with headers the egress gateway sets. */ +export interface AgentEgress { + origin: string; + headers?: EgressHeader[]; +} + +/** A header whose value comes from a Secret in the Agent's namespace. */ +export interface EgressHeader { + name: string; + prefix?: string; + valueFrom: { secretKeyRef: { name: string; key: string } }; +} /** One condition the controller recorded for an Agent. */ export interface AgentCondition { type: string;