From 682ad745300ef8566625ba3faf777537dc90f92d Mon Sep 17 00:00:00 2001 From: antoinetoussaint-byte <312536060+antoinetoussaint-byte@users.noreply.github.com> Date: Sun, 11 Oct 2026 15:46:36 -0400 Subject: [PATCH 1/3] =?UTF-8?q?release:=20v0.18.3=20=E2=80=94=20a=20servic?= =?UTF-8?q?e=20account=20projection=20runs=20entirely=20on=20a=20caller-su?= =?UTF-8?q?pplied=20filesystem?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A renderer that confines every read and write to its render root (an os.Root handle) could not keep that guarantee across core's ServiceAccount projection, which read and wrote the overlay — and ran its Kustomize build — straight from the disk path, through any symlink a concurrent change put there. ProjectServiceAccountOnFS projects and validates an identity using only the supplied filesystem, including the Kustomize build; the caller owns the transaction and publishes a staged tree only after it returns nil. ProjectServiceAccount keeps that transaction for ordinary disk callers. Validation identifies the emitted account after name and namespace transforms and never rewrites the caller's kustomization. Co-Authored-By: Claude Opus 5.5 --- agents/services/pod_overlay.go | 254 +++++++++++------- .../services/pod_overlay_filesystem_test.go | 43 +++ version/info.codefly.yaml | 2 +- 3 files changed, 196 insertions(+), 103 deletions(-) create mode 100644 agents/services/pod_overlay_filesystem_test.go diff --git a/agents/services/pod_overlay.go b/agents/services/pod_overlay.go index d5a87ca36..9a9f009b9 100644 --- a/agents/services/pod_overlay.go +++ b/agents/services/pod_overlay.go @@ -16,6 +16,7 @@ import ( "github.com/gofrs/flock" "gopkg.in/yaml.v3" "sigs.k8s.io/kustomize/api/krusty" + "sigs.k8s.io/kustomize/api/resmap" "sigs.k8s.io/kustomize/api/types" "sigs.k8s.io/kustomize/kyaml/filesys" ) @@ -130,11 +131,7 @@ func ProjectServiceAccount(ctx context.Context, baseDir, namespace, serviceAccou if overlay == nil { return nil } - if overlay.ServiceAccount == nil { - return fmt.Errorf("service account projection requires a service account") - } - overlay.DefaultServiceAccountName(serviceAccountName) - if err := overlay.Validate(); err != nil { + if err := prepareServiceAccountOverlay(serviceAccountName, overlay); err != nil { return err } baseDir, err := filepath.Abs(baseDir) @@ -194,92 +191,9 @@ func ProjectServiceAccount(ctx context.Context, baseDir, namespace, serviceAccou if err != nil { return err } - if err := emitWorkloadServiceAccount(ctx, staging, namespace, overlay.ServiceAccount); err != nil { - return err - } - result, err := applyPodOverlay(ctx, staging, overlay) - if err != nil { - return err - } - if !result.boundServiceAccount { - return fmt.Errorf("service account %q bound no workload in %s: no manifest there carries a pod template", overlay.ServiceAccount.Name, baseDir) - } - kustomizationPath := filepath.Join(staging, "kustomization.yaml") - kustomizationData, err := os.ReadFile(kustomizationPath) - if err != nil { - return err - } - var kustomization map[string]any - if err := yaml.Unmarshal(kustomizationData, &kustomization); err != nil { - return err - } - // Origin tracking identifies the emitted account after name/namespace transforms. - // This option is validation-only; the caller's kustomization is preserved. - kustomization["buildMetadata"] = []string{types.OriginAnnotations} - validationData, err := yaml.Marshal(kustomization) - if err != nil { - return err - } - if err := os.WriteFile(kustomizationPath, validationData, 0o600); err != nil { + if err = ProjectServiceAccountOnFS(ctx, filesys.MakeFsOnDisk(), staging, namespace, serviceAccountName, overlay); err != nil { return err } - built, err := krusty.MakeKustomizer(krusty.MakeDefaultOptions()).Run(filesys.MakeFsOnDisk(), staging) - if err != nil { - return fmt.Errorf("render workload identity: %w", err) - } - if err := os.WriteFile(kustomizationPath, kustomizationData, 0o600); err != nil { - return err - } - resolvedAccount := "" - resolvedNamespace := "" - for _, resource := range built.Resources() { - origin, err := resource.GetOrigin() - if err != nil { - return err - } - if resource.GetKind() != "ServiceAccount" || origin == nil || origin.Path != "serviceaccount.yaml" || origin.Repo != "" { - continue - } - for key, value := range overlay.ServiceAccount.Annotations { - if resource.GetAnnotations()[key] != value { - return fmt.Errorf("workload identity conflicts with rendered service account annotation %q", key) - } - } - resolvedAccount = resource.GetName() - resolvedNamespace = resource.GetNamespace() - } - if resolvedAccount == "" { - return fmt.Errorf("workload identity service account is absent from the rendered kustomization") - } - bound := false - for _, resource := range built.Resources() { - data, err := resource.AsYAML() - if err != nil { - return err - } - var document yaml.Node - if err := yaml.Unmarshal(data, &document); err != nil { - return err - } - root := document.Content[0] - template := workloadPodTemplate(root) - if template == nil { - continue - } - if resource.GetNamespace() != resolvedNamespace || mappingScalar(mappingChild(template, "spec"), "serviceAccountName") != resolvedAccount { - return fmt.Errorf("workload %q does not bind declared service account %q", mappingScalar(mappingChild(root, "metadata"), "name"), resolvedAccount) - } - labels := mappingChild(mappingChild(template, "metadata"), "labels") - for key, value := range overlay.PodLabels { - if mappingScalar(labels, key) != value { - return fmt.Errorf("workload identity conflicts with rendered pod label %q", key) - } - } - bound = true - } - if !bound { - return fmt.Errorf("service account %q bound no workload in kustomization", overlay.ServiceAccount.Name) - } err = filepath.WalkDir(staging, func(path string, entry os.DirEntry, walkErr error) error { if walkErr != nil || entry.IsDir() { return walkErr @@ -359,6 +273,124 @@ func ProjectServiceAccount(ctx context.Context, baseDir, namespace, serviceAccou return exchangeWorkloadTree(staging, baseDir) } +func prepareServiceAccountOverlay(name string, overlay *PodTemplateOverlay) error { + if overlay.ServiceAccount == nil { + return fmt.Errorf("service account projection requires a service account") + } + overlay.DefaultServiceAccountName(name) + return overlay.Validate() +} + +// ProjectServiceAccountOnFS projects and validates an identity using only the +// supplied filesystem, including the Kustomize build. The caller owns the +// transaction: use a staged tree and publish it only after this returns nil. +// ProjectServiceAccount supplies that transaction for ordinary disk callers. +func ProjectServiceAccountOnFS(ctx context.Context, filesystem filesys.FileSystem, baseDir, namespace, serviceAccountName string, overlay *PodTemplateOverlay) error { + if overlay == nil { + return nil + } + if err := prepareServiceAccountOverlay(serviceAccountName, overlay); err != nil { + return err + } + if err := ctx.Err(); err != nil { + return err + } + if err := emitWorkloadServiceAccountOnFS(ctx, filesystem, baseDir, namespace, overlay.ServiceAccount); err != nil { + return err + } + result, err := applyPodOverlayOnFS(ctx, filesystem, baseDir, overlay) + if err != nil { + return err + } + if !result.boundServiceAccount { + return fmt.Errorf("service account %q bound no workload in %s: no manifest there carries a pod template", overlay.ServiceAccount.Name, baseDir) + } + kustomizationPath := filepath.Join(baseDir, "kustomization.yaml") + kustomizationData, err := filesystem.ReadFile(kustomizationPath) + if err != nil { + return err + } + var kustomization map[string]any + if err = yaml.Unmarshal(kustomizationData, &kustomization); err != nil { + return err + } + // Origin tracking identifies the emitted account after name/namespace transforms. + // This option is validation-only; the caller's kustomization is preserved. + kustomization["buildMetadata"] = []string{types.OriginAnnotations} + validationData, err := yaml.Marshal(kustomization) + if err != nil { + return err + } + if err = filesystem.WriteFile(kustomizationPath, validationData); err != nil { + return err + } + built, err := krusty.MakeKustomizer(krusty.MakeDefaultOptions()).Run(filesystem, baseDir) + if err != nil { + return fmt.Errorf("render workload identity: %w", err) + } + if err = filesystem.WriteFile(kustomizationPath, kustomizationData); err != nil { + return err + } + if err = validateServiceAccountResources(built, overlay); err != nil { + return err + } + return ctx.Err() +} + +func validateServiceAccountResources(built resmap.ResMap, overlay *PodTemplateOverlay) error { + resolvedAccount := "" + resolvedNamespace := "" + for _, resource := range built.Resources() { + origin, err := resource.GetOrigin() + if err != nil { + return err + } + if resource.GetKind() != "ServiceAccount" || origin == nil || origin.Path != "serviceaccount.yaml" || origin.Repo != "" { + continue + } + for key, value := range overlay.ServiceAccount.Annotations { + if resource.GetAnnotations()[key] != value { + return fmt.Errorf("workload identity conflicts with rendered service account annotation %q", key) + } + } + resolvedAccount = resource.GetName() + resolvedNamespace = resource.GetNamespace() + } + if resolvedAccount == "" { + return fmt.Errorf("workload identity service account is absent from the rendered kustomization") + } + bound := false + for _, resource := range built.Resources() { + data, err := resource.AsYAML() + if err != nil { + return err + } + var document yaml.Node + if err = yaml.Unmarshal(data, &document); err != nil { + return err + } + root := document.Content[0] + template := workloadPodTemplate(root) + if template == nil { + continue + } + if resource.GetNamespace() != resolvedNamespace || mappingScalar(mappingChild(template, "spec"), "serviceAccountName") != resolvedAccount { + return fmt.Errorf("workload %q does not bind declared service account %q", mappingScalar(mappingChild(root, "metadata"), "name"), resolvedAccount) + } + labels := mappingChild(mappingChild(template, "metadata"), "labels") + for key, value := range overlay.PodLabels { + if mappingScalar(labels, key) != value { + return fmt.Errorf("workload identity conflicts with rendered pod label %q", key) + } + } + bound = true + } + if !bound { + return fmt.Errorf("service account %q bound no workload in kustomization", overlay.ServiceAccount.Name) + } + return nil +} + // dns1123Subdomain matches a Kubernetes ServiceAccount name. var dns1123Subdomain = regexp.MustCompile(`^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$`) @@ -510,7 +542,14 @@ func uniqueVolumeName(base string, used map[string]bool) string { // app.kubernetes.io/managed-by: codefly ownership label is baked in here so the // SA is conformant by construction — no agent re-implements the label and none // can forget it (ValidateKubernetesManifestTree requires it on ServiceAccounts). -func emitWorkloadServiceAccount(_ context.Context, baseDir, namespace string, sa *WorkloadServiceAccount) error { +func emitWorkloadServiceAccount(ctx context.Context, baseDir, namespace string, sa *WorkloadServiceAccount) error { + return emitWorkloadServiceAccountOnFS(ctx, filesys.MakeFsOnDisk(), baseDir, namespace, sa) +} + +func emitWorkloadServiceAccountOnFS(ctx context.Context, filesystem filesys.FileSystem, baseDir, namespace string, sa *WorkloadServiceAccount) error { + if err := ctx.Err(); err != nil { + return err + } metadata := map[string]any{ "name": sa.Name, "namespace": namespace, @@ -529,7 +568,7 @@ func emitWorkloadServiceAccount(_ context.Context, baseDir, namespace string, sa "metadata": metadata, } path := filepath.Join(baseDir, "serviceaccount.yaml") - if existing, err := os.ReadFile(path); err == nil { + if existing, err := filesystem.ReadFile(path); err == nil { var current map[string]any decoder := yaml.NewDecoder(bytes.NewReader(existing)) if err := decoder.Decode(¤t); err != nil { @@ -573,10 +612,10 @@ func emitWorkloadServiceAccount(_ context.Context, baseDir, namespace string, sa if err != nil { return fmt.Errorf("marshal service account: %w", err) } - if err = os.WriteFile(filepath.Join(baseDir, "serviceaccount.yaml"), document, 0o644); err != nil { + if err = filesystem.WriteFile(filepath.Join(baseDir, "serviceaccount.yaml"), document); err != nil { return fmt.Errorf("write service account: %w", err) } - return addKustomizeResource(filepath.Join(baseDir, "kustomization.yaml"), "serviceaccount.yaml") + return addKustomizeResourceOnFS(filesystem, filepath.Join(baseDir, "kustomization.yaml"), "serviceaccount.yaml") } // podTemplateWorkloadKinds have a pod template at spec.template. @@ -624,7 +663,14 @@ type podOverlayResult struct { // in its own template. The write is idempotent: a field the template already // rendered is left untouched, so an agent mid-migration that still carries the // binding does not double-render it. -func applyPodOverlay(_ context.Context, baseDir string, overlay *PodTemplateOverlay) (podOverlayResult, error) { +func applyPodOverlay(ctx context.Context, baseDir string, overlay *PodTemplateOverlay) (podOverlayResult, error) { + return applyPodOverlayOnFS(ctx, filesys.MakeFsOnDisk(), baseDir, overlay) +} + +func applyPodOverlayOnFS(ctx context.Context, filesystem filesys.FileSystem, baseDir string, overlay *PodTemplateOverlay) (podOverlayResult, error) { + if err := ctx.Err(); err != nil { + return podOverlayResult{}, err + } result := podOverlayResult{renderedConfigVolumes: map[string]bool{}} if overlay == nil { return result, nil @@ -632,15 +678,15 @@ func applyPodOverlay(_ context.Context, baseDir string, overlay *PodTemplateOver if !overlay.HasServiceAccount() && len(overlay.PodLabels) == 0 && len(overlay.PodAnnotations) == 0 && !overlay.HasConfigMounts() { return result, nil } - entries, err := os.ReadDir(baseDir) + entries, err := filesystem.ReadDir(baseDir) if err != nil { return result, fmt.Errorf("read base dir: %w", err) } for _, entry := range entries { - if entry.IsDir() || !strings.HasSuffix(entry.Name(), ".yaml") { + if filesystem.IsDir(filepath.Join(baseDir, entry)) || !strings.HasSuffix(entry, ".yaml") { continue } - bound, volumes, err := applyPodOverlayToFile(filepath.Join(baseDir, entry.Name()), overlay) + bound, volumes, err := applyPodOverlayToFile(filesystem, filepath.Join(baseDir, entry), overlay) if err != nil { return result, err } @@ -654,8 +700,8 @@ func applyPodOverlay(_ context.Context, baseDir string, overlay *PodTemplateOver return result, nil } -func applyPodOverlayToFile(path string, overlay *PodTemplateOverlay) (bool, []string, error) { - content, err := os.ReadFile(path) +func applyPodOverlayToFile(filesystem filesys.FileSystem, path string, overlay *PodTemplateOverlay) (bool, []string, error) { + content, err := filesystem.ReadFile(path) if err != nil { return false, nil, fmt.Errorf("read manifest: %w", err) } @@ -696,7 +742,7 @@ func applyPodOverlayToFile(path string, overlay *PodTemplateOverlay) (bool, []st if err = encoder.Close(); err != nil { return false, nil, fmt.Errorf("render manifest %s: %w", path, err) } - if err = os.WriteFile(path, out.Bytes(), 0o644); err != nil { + if err = filesystem.WriteFile(path, out.Bytes()); err != nil { return false, nil, err } return bound, volumes, nil @@ -825,7 +871,11 @@ func mergeScalarsIfAbsent(node *yaml.Node, values map[string]string) bool { // idempotently. It round-trips the rendered YAML rather than editing the // plugin template so the wiring stays owned by core. func addKustomizeResource(kustomizationPath, resource string) error { - content, err := os.ReadFile(kustomizationPath) + return addKustomizeResourceOnFS(filesys.MakeFsOnDisk(), kustomizationPath, resource) +} + +func addKustomizeResourceOnFS(filesystem filesys.FileSystem, kustomizationPath, resource string) error { + content, err := filesystem.ReadFile(kustomizationPath) if err != nil { return fmt.Errorf("read kustomization: %w", err) } @@ -847,5 +897,5 @@ func addKustomizeResource(kustomizationPath, resource string) error { if err != nil { return fmt.Errorf("marshal kustomization: %w", err) } - return os.WriteFile(kustomizationPath, out, 0o644) + return filesystem.WriteFile(kustomizationPath, out) } diff --git a/agents/services/pod_overlay_filesystem_test.go b/agents/services/pod_overlay_filesystem_test.go new file mode 100644 index 000000000..46b6ee5fb --- /dev/null +++ b/agents/services/pod_overlay_filesystem_test.go @@ -0,0 +1,43 @@ +package services + +import ( + "context" + "path/filepath" + "testing" + + "github.com/stretchr/testify/require" + "sigs.k8s.io/kustomize/kyaml/filesys" +) + +func TestProjectServiceAccountOnFSUsesOnlySuppliedFilesystem(t *testing.T) { + filesystem := filesys.MakeFsInMemory() + base := "/render/base" + require.NoError(t, filesystem.MkdirAll(base)) + require.NoError(t, filesystem.WriteFile(filepath.Join(base, "kustomization.yaml"), []byte("resources: [deployment.yaml]\n"))) + require.NoError(t, filesystem.WriteFile(filepath.Join(base, "deployment.yaml"), []byte(`apiVersion: apps/v1 +kind: Deployment +metadata: + name: api + namespace: target +spec: + template: + spec: + containers: + - name: api + image: example/api +`))) + overlay := &PodTemplateOverlay{} + require.NoError(t, overlay.AttachServiceAccount(&WorkloadServiceAccount{Annotations: map[string]string{"example.test/identity": "principal"}}, map[string]string{"example.test/workload": "true"})) + require.NoError(t, ProjectServiceAccountOnFS(context.Background(), filesystem, base, "target", "api", overlay)) + account, err := filesystem.ReadFile(filepath.Join(base, "serviceaccount.yaml")) + require.NoError(t, err) + require.Contains(t, string(account), "example.test/identity: principal") + deployment, err := filesystem.ReadFile(filepath.Join(base, "deployment.yaml")) + require.NoError(t, err) + require.Contains(t, string(deployment), "serviceAccountName: api") + require.Contains(t, string(deployment), `example.test/workload: "true"`) + index, err := filesystem.ReadFile(filepath.Join(base, "kustomization.yaml")) + require.NoError(t, err) + require.Contains(t, string(index), "serviceaccount.yaml") + require.NotContains(t, string(index), "buildMetadata") +} diff --git a/version/info.codefly.yaml b/version/info.codefly.yaml index 11a230dbe..518a04ae8 100644 --- a/version/info.codefly.yaml +++ b/version/info.codefly.yaml @@ -1 +1 @@ -version: 0.18.2 +version: 0.18.3 From 76f9e0f11cb24881cdb24104e252446392a69376 Mon Sep 17 00:00:00 2001 From: antoinetoussaint-byte <312536060+antoinetoussaint-byte@users.noreply.github.com> Date: Sun, 11 Oct 2026 16:08:40 -0400 Subject: [PATCH 2/3] fix(services): a projection refuses kustomize references that would leave its filesystem Kustomize clones a reference it reads as a git repository by running git in a host temporary directory, and fetches an http(s) file over the network; neither stays on the supplied filesystem. Every kustomization reachable from the base is read through that filesystem first, and a remote reference refuses the projection before the build. The confinement test now also covers an existing account that exists only in the supplied filesystem. Co-Authored-By: Claude Opus 5.5 --- agents/services/pod_overlay.go | 3 + .../services/pod_overlay_filesystem_test.go | 96 ++++++++++++ agents/services/pod_overlay_remote.go | 140 ++++++++++++++++++ 3 files changed, 239 insertions(+) create mode 100644 agents/services/pod_overlay_remote.go diff --git a/agents/services/pod_overlay.go b/agents/services/pod_overlay.go index 9a9f009b9..a7489a344 100644 --- a/agents/services/pod_overlay.go +++ b/agents/services/pod_overlay.go @@ -295,6 +295,9 @@ func ProjectServiceAccountOnFS(ctx context.Context, filesystem filesys.FileSyste if err := ctx.Err(); err != nil { return err } + if err := refuseRemoteKustomizeReferences(filesystem, baseDir); err != nil { + return err + } if err := emitWorkloadServiceAccountOnFS(ctx, filesystem, baseDir, namespace, overlay.ServiceAccount); err != nil { return err } diff --git a/agents/services/pod_overlay_filesystem_test.go b/agents/services/pod_overlay_filesystem_test.go index 46b6ee5fb..da594dbab 100644 --- a/agents/services/pod_overlay_filesystem_test.go +++ b/agents/services/pod_overlay_filesystem_test.go @@ -3,12 +3,108 @@ package services import ( "context" "path/filepath" + "strings" "testing" "github.com/stretchr/testify/require" "sigs.k8s.io/kustomize/kyaml/filesys" ) +const confinedDeployment = `apiVersion: apps/v1 +kind: Deployment +metadata: + name: api + namespace: target +spec: + template: + spec: + containers: + - name: api + image: example/api +` + +// confinedBase writes a base whose files exist only in memory, so a read or +// write that leaves the supplied filesystem cannot find them. +func confinedBase(t *testing.T, kustomization string, files map[string]string) (filesys.FileSystem, string) { + t.Helper() + filesystem := filesys.MakeFsInMemory() + base := "/render/base" + require.NoError(t, filesystem.MkdirAll(base)) + require.NoError(t, filesystem.WriteFile(filepath.Join(base, "kustomization.yaml"), []byte(kustomization))) + require.NoError(t, filesystem.WriteFile(filepath.Join(base, "deployment.yaml"), []byte(confinedDeployment))) + for name, body := range files { + require.NoError(t, filesystem.MkdirAll(filepath.Dir(filepath.Join(base, name)))) + require.NoError(t, filesystem.WriteFile(filepath.Join(base, name), []byte(body))) + } + return filesystem, base +} + +func confinedOverlay(t *testing.T) *PodTemplateOverlay { + t.Helper() + overlay := &PodTemplateOverlay{} + require.NoError(t, overlay.AttachServiceAccount(&WorkloadServiceAccount{Annotations: map[string]string{"example.test/identity": "principal"}}, map[string]string{"example.test/workload": "true"})) + return overlay +} + +// A reference Kustomize would clone or fetch leaves any filesystem: it is +// refused before the build, and nothing is written. +func TestProjectServiceAccountOnFSRefusesRemoteReferences(t *testing.T) { + for name, kustomization := range map[string]string{ + "file repository": "resources: [deployment.yaml, 'file:///outside/repo.git']\n", + "https repository": "resources: [deployment.yaml, 'https://example.test/org/repo//base?ref=v1']\n", + "github shorthand": "resources: [deployment.yaml, 'github.com/org/repo/base']\n", + "scp repository": "resources: [deployment.yaml, 'git@example.test:org/repo.git']\n", + "forced git": "resources: [deployment.yaml, 'git::example.test/org/repo']\n", + "remote component": "resources: [deployment.yaml]\ncomponents: ['https://example.test/c']\n", + "remote generator": "resources: [deployment.yaml]\ngenerators: ['github.com/org/repo/gen']\n", + "remote patch file": "resources: [deployment.yaml]\npatches:\n- path: https://example.test/patch.yaml\n", + "remote config file": "resources: [deployment.yaml]\nconfigMapGenerator:\n- name: c\n files: [k=https://example.test/v]\n", + "nested remote": "resources: [deployment.yaml, nested]\n", + } { + t.Run(name, func(t *testing.T) { + filesystem, base := confinedBase(t, kustomization, map[string]string{ + "nested/kustomization.yaml": "resources: ['https://example.test/r.yaml']\n", + }) + err := ProjectServiceAccountOnFS(context.Background(), filesystem, base, "target", "api", confinedOverlay(t)) + require.ErrorContains(t, err, "remote") + require.False(t, filesystem.Exists(filepath.Join(base, "serviceaccount.yaml"))) + }) + } +} + +// A URL in a field Kustomize never loads is data, and an image digest or a +// registry port is not a repository. +func TestProjectServiceAccountOnFSAcceptsURLsThatAreData(t *testing.T) { + filesystem, base := confinedBase(t, `resources: [deployment.yaml] +commonAnnotations: + example.test/docs: https://example.test/docs +images: +- name: example/api + newName: registry.example.test:5000/api + digest: sha256:0000000000000000000000000000000000000000000000000000000000000000 +configMapGenerator: +- name: c + literals: [URL=https://example.test/v] +`, nil) + require.NoError(t, ProjectServiceAccountOnFS(context.Background(), filesystem, base, "target", "api", confinedOverlay(t))) +} + +// An account that exists only in the supplied filesystem is the one merged +// and checked: a read that went to the disk would miss it. +func TestProjectServiceAccountOnFSReadsTheExistingAccountFromTheSuppliedFilesystem(t *testing.T) { + existing := "apiVersion: v1\nkind: ServiceAccount\nmetadata:\n name: api\n namespace: target\n annotations:\n example.test/kept: \"yes\"\n" + filesystem, base := confinedBase(t, "resources: [deployment.yaml, serviceaccount.yaml]\n", map[string]string{"serviceaccount.yaml": existing}) + require.NoError(t, ProjectServiceAccountOnFS(context.Background(), filesystem, base, "target", "api", confinedOverlay(t))) + account, err := filesystem.ReadFile(filepath.Join(base, "serviceaccount.yaml")) + require.NoError(t, err) + require.Contains(t, string(account), "example.test/kept") + require.Contains(t, string(account), "example.test/identity: principal") + + conflicting := strings.Replace(existing, "name: api", "name: other", 1) + filesystem, base = confinedBase(t, "resources: [deployment.yaml, serviceaccount.yaml]\n", map[string]string{"serviceaccount.yaml": conflicting}) + require.ErrorContains(t, ProjectServiceAccountOnFS(context.Background(), filesystem, base, "target", "api", confinedOverlay(t)), "conflicts") +} + func TestProjectServiceAccountOnFSUsesOnlySuppliedFilesystem(t *testing.T) { filesystem := filesys.MakeFsInMemory() base := "/render/base" diff --git a/agents/services/pod_overlay_remote.go b/agents/services/pod_overlay_remote.go new file mode 100644 index 000000000..5963d0030 --- /dev/null +++ b/agents/services/pod_overlay_remote.go @@ -0,0 +1,140 @@ +package services + +import ( + "fmt" + "path" + "strings" + + "gopkg.in/yaml.v3" + "sigs.k8s.io/kustomize/kyaml/filesys" +) + +// The Kustomize build that validates a projection runs on the supplied +// filesystem, but Kustomize does not keep every load there: a reference it +// reads as a git repository is cloned by running git into a host temporary +// directory, and an http(s) file reference is fetched over the network. A +// filesystem confined to a render root says nothing about either. So every +// kustomization reachable from the base is read through the supplied +// filesystem first, and a remote reference refuses the projection before +// Kustomize sees it. + +// kustomizationFileNames are the names Kustomize accepts for a kustomization. +var kustomizationFileNames = []string{"kustomization.yaml", "kustomization.yml", "Kustomization"} + +// rootFields name entries Kustomize may open as new roots — the loads it +// clones when the reference parses as a repository. +var rootFields = []string{"resources", "bases", "components", "generators", "transformers", "validators"} + +// textFields hold data Kustomize never loads, so a URL there is a value, not +// a reference. +var textFields = map[string]bool{ + "commonAnnotations": true, "commonLabels": true, "labels": true, "metadata": true, + "patch": true, "buildMetadata": true, "images": true, "literals": true, +} + +func refuseRemoteKustomizeReferences(filesystem filesys.FileSystem, baseDir string) error { + seen := map[string]bool{} + var walk func(dir string) error + walk = func(dir string) error { + dir = path.Clean(dir) + if seen[dir] { + return nil + } + seen[dir] = true + file := "" + for _, name := range kustomizationFileNames { + if candidate := path.Join(dir, name); filesystem.Exists(candidate) { + file = candidate + break + } + } + if file == "" { + return nil + } + data, err := filesystem.ReadFile(file) + if err != nil { + return err + } + var kustomization map[string]any + if err := yaml.Unmarshal(data, &kustomization); err != nil { + return fmt.Errorf("read %s: %w", file, err) + } + if err := refuseRemoteValues(file, "", kustomization); err != nil { + return err + } + for _, field := range rootFields { + entries, _ := kustomization[field].([]any) + for _, entry := range entries { + reference, ok := entry.(string) + if !ok || strings.Contains(reference, "\n") { + continue // an inline configuration, not a reference + } + if remoteRepository(reference) { + return fmt.Errorf("%s: %s entry %q is a remote repository; a projection builds only from its render root", file, field, reference) + } + if child := path.Join(dir, reference); filesystem.IsDir(child) { + if err := walk(child); err != nil { + return err + } + } + } + } + return nil + } + return walk(baseDir) +} + +// refuseRemoteValues refuses any string a kustomization could fetch over +// http(s), outside the fields that only carry data. +func refuseRemoteValues(file, field string, value any) error { + switch typed := value.(type) { + case map[string]any: + for key, child := range typed { + if textFields[key] { + continue + } + if err := refuseRemoteValues(file, key, child); err != nil { + return err + } + } + case []any: + for _, child := range typed { + if err := refuseRemoteValues(file, field, child); err != nil { + return err + } + } + case string: + // A generator source may be "key=path". + reference := typed + if _, after, found := strings.Cut(reference, "="); found && (field == "files" || field == "envs") { + reference = after + } + if remoteFile(reference) { + return fmt.Errorf("%s: %s value %q is a remote reference; a projection builds only from its render root", file, field, typed) + } + } + return nil +} + +// remoteFile matches what Kustomize fetches over http(s). +func remoteFile(reference string) bool { + lower := strings.ToLower(strings.TrimSpace(reference)) + return strings.HasPrefix(lower, "http://") || strings.HasPrefix(lower, "https://") +} + +// remoteRepository is a superset of what Kustomize parses as a repository: a +// URL scheme, the forced git:: prefix, an SCP-style user@host:path, or the +// scheme-less github.com form. A plain relative path matches none of them. +func remoteRepository(reference string) bool { + trimmed := strings.TrimSpace(reference) + lower := strings.ToLower(trimmed) + if strings.Contains(lower, "://") || strings.HasPrefix(lower, "git::") || + strings.HasPrefix(lower, "github.com/") || strings.HasPrefix(lower, "github.com:") { + return true + } + first := trimmed + if index := strings.IndexAny(first, "/"); index >= 0 { + first = first[:index] + } + return strings.Contains(first, "@") || strings.Contains(first, ":") +} From ba2562bbb37158afe591d20551696ad657667ebf Mon Sep 17 00:00:00 2001 From: antoinetoussaint-byte <312536060+antoinetoussaint-byte@users.noreply.github.com> Date: Sun, 11 Oct 2026 16:19:26 -0400 Subject: [PATCH 3/3] fix(services): read kustomizations with Kustomize's own decoder before refusing remote loads Field names are matched as Kustomize matches them (case-insensitively), a plugin entry is inline only when it parses as configuration, plugin configurations in files and inline are checked for the loads they drive, env files are whole paths, and annotation data is never read as a reference. Co-Authored-By: Claude Opus 5.5 --- .../services/pod_overlay_filesystem_test.go | 19 +- agents/services/pod_overlay_remote.go | 269 ++++++++++++------ 2 files changed, 206 insertions(+), 82 deletions(-) diff --git a/agents/services/pod_overlay_filesystem_test.go b/agents/services/pod_overlay_filesystem_test.go index da594dbab..24d0118a6 100644 --- a/agents/services/pod_overlay_filesystem_test.go +++ b/agents/services/pod_overlay_filesystem_test.go @@ -60,10 +60,19 @@ func TestProjectServiceAccountOnFSRefusesRemoteReferences(t *testing.T) { "remote patch file": "resources: [deployment.yaml]\npatches:\n- path: https://example.test/patch.yaml\n", "remote config file": "resources: [deployment.yaml]\nconfigMapGenerator:\n- name: c\n files: [k=https://example.test/v]\n", "nested remote": "resources: [deployment.yaml, nested]\n", + "capitalised field": "resources: [deployment.yaml, capital]\n", + "capitalised file": "resources: [deployment.yaml, named]\n", + "newline repository": "resources: [deployment.yaml, \"file:///outside/repo.git\\n\"]\n", + "plugin file": "resources: [deployment.yaml]\ntransformers: [plugin.yaml]\n", + "inline plugin": "resources: [deployment.yaml]\ntransformers:\n- |\n apiVersion: builtin\n kind: PatchTransformer\n metadata: {name: p}\n path: http://example.test/patch.yaml\n", + "env file query": "resources: [deployment.yaml]\nconfigMapGenerator:\n- name: c\n envs: ['http://example.test/env?x=y']\n", } { t.Run(name, func(t *testing.T) { filesystem, base := confinedBase(t, kustomization, map[string]string{ - "nested/kustomization.yaml": "resources: ['https://example.test/r.yaml']\n", + "nested/kustomization.yaml": "resources: ['https://example.test/r.yaml']\n", + "capital/kustomization.yaml": "Resources: ['file:///outside/repo.git']\n", + "named/Kustomization": "resources: ['https://example.test/r.yaml']\n", + "plugin.yaml": "apiVersion: builtin\nkind: PatchTransformer\nmetadata: {name: p}\npath: http://example.test/patch.yaml\n", }) err := ProjectServiceAccountOnFS(context.Background(), filesystem, base, "target", "api", confinedOverlay(t)) require.ErrorContains(t, err, "remote") @@ -85,6 +94,14 @@ images: configMapGenerator: - name: c literals: [URL=https://example.test/v] + options: + annotations: + example.test/source: https://example.test/source +generatorOptions: + annotations: + example.test/docs: https://example.test/docs +transformers: +- '{apiVersion: builtin, kind: LabelTransformer, metadata: {name: label}, labels: {hello: world}, fieldSpecs: [{path: metadata/labels, create: true}]}' `, nil) require.NoError(t, ProjectServiceAccountOnFS(context.Background(), filesystem, base, "target", "api", confinedOverlay(t))) } diff --git a/agents/services/pod_overlay_remote.go b/agents/services/pod_overlay_remote.go index 5963d0030..89a03780e 100644 --- a/agents/services/pod_overlay_remote.go +++ b/agents/services/pod_overlay_remote.go @@ -1,11 +1,16 @@ package services import ( + "bytes" + "errors" "fmt" + "io" "path" "strings" "gopkg.in/yaml.v3" + "sigs.k8s.io/kustomize/api/konfig" + "sigs.k8s.io/kustomize/api/types" "sigs.k8s.io/kustomize/kyaml/filesys" ) @@ -15,102 +20,207 @@ import ( // directory, and an http(s) file reference is fetched over the network. A // filesystem confined to a render root says nothing about either. So every // kustomization reachable from the base is read through the supplied -// filesystem first, and a remote reference refuses the projection before -// Kustomize sees it. +// filesystem first — decoded by Kustomize's own type, so a field is read under +// every spelling Kustomize accepts — and a remote reference refuses the +// projection before Kustomize sees it. -// kustomizationFileNames are the names Kustomize accepts for a kustomization. -var kustomizationFileNames = []string{"kustomization.yaml", "kustomization.yml", "Kustomization"} - -// rootFields name entries Kustomize may open as new roots — the loads it -// clones when the reference parses as a repository. -var rootFields = []string{"resources", "bases", "components", "generators", "transformers", "validators"} +func refuseRemoteKustomizeReferences(filesystem filesys.FileSystem, baseDir string) error { + walker := remoteReferenceWalker{filesystem: filesystem, seen: map[string]bool{}} + return walker.kustomization(baseDir) +} -// textFields hold data Kustomize never loads, so a URL there is a value, not -// a reference. -var textFields = map[string]bool{ - "commonAnnotations": true, "commonLabels": true, "labels": true, "metadata": true, - "patch": true, "buildMetadata": true, "images": true, "literals": true, +type remoteReferenceWalker struct { + filesystem filesys.FileSystem + seen map[string]bool } -func refuseRemoteKustomizeReferences(filesystem filesys.FileSystem, baseDir string) error { - seen := map[string]bool{} - var walk func(dir string) error - walk = func(dir string) error { - dir = path.Clean(dir) - if seen[dir] { - return nil +func (w remoteReferenceWalker) kustomization(dir string) error { + dir = path.Clean(dir) + if w.seen[dir] { + return nil + } + w.seen[dir] = true + file := "" + for _, name := range konfig.RecognizedKustomizationFileNames() { + if candidate := path.Join(dir, name); w.filesystem.Exists(candidate) { + file = candidate + break } - seen[dir] = true - file := "" - for _, name := range kustomizationFileNames { - if candidate := path.Join(dir, name); filesystem.Exists(candidate) { - file = candidate - break + } + if file == "" { + return nil + } + data, err := w.filesystem.ReadFile(file) + if err != nil { + return err + } + var k types.Kustomization + if err := k.Unmarshal(data); err != nil { + return fmt.Errorf("read %s: %w", file, err) + } + k.FixKustomization() + refuse := func(field, value string) error { + return fmt.Errorf("%s: %s entry %q is a remote reference; a projection builds only from its render root", file, field, value) + } + // Resources are tried as a file (fetched when http) and then as a root + // (cloned when it reads as a repository); components and bases are roots. + for field, entries := range map[string][]string{"resources": k.Resources, "components": k.Components, "bases": k.Bases} { + for _, entry := range entries { + if remoteFile(entry) || remoteRepository(entry) { + return refuse(field, entry) + } + if err := w.local(dir, entry); err != nil { + return err } } - if file == "" { - return nil - } - data, err := filesystem.ReadFile(file) - if err != nil { - return err - } - var kustomization map[string]any - if err := yaml.Unmarshal(data, &kustomization); err != nil { - return fmt.Errorf("read %s: %w", file, err) + } + // A plugin entry that parses as configuration is inline; otherwise it is + // a path, loaded as a resource is. + for field, entries := range map[string][]string{"generators": k.Generators, "transformers": k.Transformers, "validators": k.Validators} { + for _, entry := range entries { + if documents, inline := inlineConfiguration(entry); inline { + if err := refusePluginLoads(file, field, documents); err != nil { + return err + } + continue + } + if remoteFile(entry) || remoteRepository(entry) { + return refuse(field, entry) + } + if err := w.local(dir, entry); err != nil { + return err + } } - if err := refuseRemoteValues(file, "", kustomization); err != nil { - return err + } + files := map[string][]string{ + "crds": k.Crds, "configurations": k.Configurations, + } + for _, patch := range k.PatchesStrategicMerge { + files["patchesStrategicMerge"] = append(files["patchesStrategicMerge"], string(patch)) + } + for _, patch := range append(append([]types.Patch{}, k.Patches...), k.PatchesJson6902...) { + files["patches"] = append(files["patches"], patch.Path) + } + for _, replacement := range k.Replacements { + files["replacements"] = append(files["replacements"], replacement.Path) + } + files["openapi"] = append(files["openapi"], k.OpenAPI["path"]) + sources := func(args types.GeneratorArgs) { + for _, source := range args.FileSources { + // A file source is [{key}=]{path}. + if _, after, found := strings.Cut(source, "="); found { + source = after + } + files["files"] = append(files["files"], source) } - for _, field := range rootFields { - entries, _ := kustomization[field].([]any) - for _, entry := range entries { - reference, ok := entry.(string) - if !ok || strings.Contains(reference, "\n") { - continue // an inline configuration, not a reference - } - if remoteRepository(reference) { - return fmt.Errorf("%s: %s entry %q is a remote repository; a projection builds only from its render root", file, field, reference) - } - if child := path.Join(dir, reference); filesystem.IsDir(child) { - if err := walk(child); err != nil { - return err - } - } + files["envs"] = append(append(files["envs"], args.EnvSources...), args.EnvSource) + } + for _, generator := range k.ConfigMapGenerator { + sources(generator.GeneratorArgs) + } + for _, generator := range k.SecretGenerator { + sources(generator.GeneratorArgs) + } + for field, entries := range files { + for _, entry := range entries { + if remoteFile(entry) { + return refuse(field, entry) } } + } + return nil +} + +// local follows a reference inside the supplied filesystem: a directory is a +// kustomization to walk, a file may hold plugin configurations whose own +// loads must stay local. +func (w remoteReferenceWalker) local(dir, entry string) error { + target := path.Join(dir, entry) + if w.filesystem.IsDir(target) { + return w.kustomization(target) + } + if !w.filesystem.Exists(target) { return nil } - return walk(baseDir) + data, err := w.filesystem.ReadFile(target) + if err != nil { + return err + } + documents, _ := decodeDocuments(data) + return refusePluginLoads(target, "configuration", documents) } -// refuseRemoteValues refuses any string a kustomization could fetch over -// http(s), outside the fields that only carry data. -func refuseRemoteValues(file, field string, value any) error { - switch typed := value.(type) { - case map[string]any: - for key, child := range typed { - if textFields[key] { - continue +// inlineConfiguration reports whether a plugin entry is configuration rather +// than a path, as Kustomize decides it: it parses as resources. +func inlineConfiguration(entry string) ([]any, bool) { + documents, err := decodeDocuments([]byte(entry)) + if err != nil || len(documents) == 0 { + return nil, false + } + for _, document := range documents { + mapping, ok := document.(map[string]any) + if !ok || mapping["kind"] == nil { + return nil, false + } + } + return documents, true +} + +func decodeDocuments(data []byte) ([]any, error) { + decoder := yaml.NewDecoder(bytes.NewReader(data)) + var documents []any + for { + var document any + err := decoder.Decode(&document) + if errors.Is(err, io.EOF) { + return documents, nil + } + if err != nil { + return documents, err + } + if document != nil { + documents = append(documents, document) + } + } +} + +// refusePluginLoads refuses an http(s) string in a plugin configuration — a +// built-in generator or transformer loads its path, files and env files +// through the same loader — outside its metadata and annotation data. +func refusePluginLoads(file, field string, documents []any) error { + var walk func(key string, value any) error + walk = func(key string, value any) error { + switch typed := value.(type) { + case map[string]any: + for child, nested := range typed { + if strings.EqualFold(child, "metadata") || strings.EqualFold(child, "annotations") || + strings.EqualFold(child, "labels") || strings.EqualFold(child, "literals") { + continue + } + if err := walk(child, nested); err != nil { + return err + } } - if err := refuseRemoteValues(file, key, child); err != nil { - return err + case []any: + for _, nested := range typed { + if err := walk(key, nested); err != nil { + return err + } } - } - case []any: - for _, child := range typed { - if err := refuseRemoteValues(file, field, child); err != nil { - return err + case string: + reference := typed + if _, after, found := strings.Cut(reference, "="); found && strings.EqualFold(key, "files") { + reference = after + } + if remoteFile(reference) { + return fmt.Errorf("%s: %s value %q is a remote reference; a projection builds only from its render root", file, field, typed) } } - case string: - // A generator source may be "key=path". - reference := typed - if _, after, found := strings.Cut(reference, "="); found && (field == "files" || field == "envs") { - reference = after - } - if remoteFile(reference) { - return fmt.Errorf("%s: %s value %q is a remote reference; a projection builds only from its render root", file, field, typed) + return nil + } + for _, document := range documents { + if err := walk("", document); err != nil { + return err } } return nil @@ -132,9 +242,6 @@ func remoteRepository(reference string) bool { strings.HasPrefix(lower, "github.com/") || strings.HasPrefix(lower, "github.com:") { return true } - first := trimmed - if index := strings.IndexAny(first, "/"); index >= 0 { - first = first[:index] - } + first, _, _ := strings.Cut(trimmed, "/") return strings.Contains(first, "@") || strings.Contains(first, ":") }