From cc43878cee08f7d05ad8e751eb4de3c5c5d8d160 Mon Sep 17 00:00:00 2001 From: Dmitrii Creed Date: Wed, 17 Jun 2026 10:54:00 +0400 Subject: [PATCH 1/2] fix(aws/alerts): kill residual cloud-helpers image phantom diff (deterministic build dir) PR #298 narrowed the helpers-image build context (killing the contextDigest churn) and IgnoreChanges'd skipPush, but the resource still shows `build: update` on every provision preview. Root cause: pulumi-docker's Image Diff is a structural diff of the build inputs, which include build.context and build.dockerfile as literal path strings. The Dockerfile is generated under os.MkdirTemp, which appends a random suffix every run, so both strings differ from the stored state on every provision -> permanent phantom `build: update`. (contextDigest is content-based and stable; args/skipPush are already handled.) Fix: write the generated Dockerfile to a DETERMINISTIC dir keyed on imageName instead of os.MkdirTemp. build.context/build.dockerfile are now byte-identical across runs and the Dockerfile content is constant, so the whole build object is stable -> no phantom diff. A genuine SOURCE_IMAGE or VERSION change still diffs via build.args, so legitimate rebuilds are unaffected; deliberately not using IgnoreChanges(['build']). Affects every aws-cloudtrail-security-alerts and ECS/ALB alert stack that provisions a helpers image. Note: one transitional `build: update` is expected on the first preview after this ships (old random path in state vs new deterministic path); it clears on the next real provision and stays clean thereafter. Signed-off-by: Dmitrii Creed --- pkg/clouds/pulumi/aws/alerts.go | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/pkg/clouds/pulumi/aws/alerts.go b/pkg/clouds/pulumi/aws/alerts.go index c1f29e69..fdcade55 100644 --- a/pkg/clouds/pulumi/aws/alerts.go +++ b/pkg/clouds/pulumi/aws/alerts.go @@ -5,6 +5,7 @@ import ( "fmt" "os" "path/filepath" + "strings" "github.com/pkg/errors" "github.com/samber/lo" @@ -79,13 +80,22 @@ func pushHelpersImageToECR(ctx *sdk.Context, cfg helperCfg) (*docker.Image, erro cfg.provisionParams.Log.Info(ctx.Context(), "creating temporary Dockerfile for cloud-helpers...") // hack taken from here https://github.com/pulumi/pulumi-docker/issues/54#issuecomment-772250411 - var dockerFilePath string - if depDir, err := os.MkdirTemp(os.TempDir(), cfg.imageName); err != nil { - return nil, errors.Wrapf(err, "failed to create tempDir") - } else if err = os.WriteFile(filepath.Join(depDir, "Dockerfile"), []byte("ARG SOURCE_IMAGE\n\nFROM ${SOURCE_IMAGE}\nARG VERSION\nLABEL VERSION=${VERSION}"), os.ModePerm); err != nil { + // + // Use a DETERMINISTIC dir (not os.MkdirTemp, which appends a random suffix) + // keyed on imageName. pulumi-docker's Diff is a structural diff of the build + // inputs, and it stores build.context / build.dockerfile as the literal path + // strings — a fresh random path every provision makes both differ from state + // and the image reports a permanent `build: update` phantom diff on every + // PR preview. The Dockerfile content is constant and imageName already encodes + // the stack+env, so reusing/overwriting a stable per-image dir is safe and + // never collides across stacks. + depDir := filepath.Join(os.TempDir(), "sc-helpers-"+strings.NewReplacer("/", "-", "\\", "-", ":", "-").Replace(cfg.imageName)) + if err := os.MkdirAll(depDir, 0o700); err != nil { + return nil, errors.Wrapf(err, "failed to create helpers build dir") + } + dockerFilePath := filepath.Join(depDir, "Dockerfile") + if err := os.WriteFile(dockerFilePath, []byte("ARG SOURCE_IMAGE\n\nFROM ${SOURCE_IMAGE}\nARG VERSION\nLABEL VERSION=${VERSION}"), 0o600); err != nil { return nil, errors.Wrapf(err, "failed to write temporary Dockerfile") - } else { - dockerFilePath = filepath.Join(depDir, "Dockerfile") } version := lo.If(cfg.deployParams.Version == "", "latest").Else(cfg.deployParams.Version) From 48e1fc87c02dea4c2f0a55fd6a0f1db9a2ef7864 Mon Sep 17 00:00:00 2001 From: Dmitrii Creed Date: Wed, 17 Jun 2026 10:59:17 +0400 Subject: [PATCH 2/2] test(aws/alerts): regression-guard deterministic helpers build dir Extract the build-dir computation into a pure helpersBuildDir() and add tests asserting it is deterministic across calls, distinct per image name, and sanitises path separators (no traversal out of TempDir). A test like this would have caught that #298 left an os.MkdirTemp random suffix in the path, which is what kept the `build: update` phantom diff alive. Signed-off-by: Dmitrii Creed --- pkg/clouds/pulumi/aws/alerts.go | 28 +++++++++----- pkg/clouds/pulumi/aws/alerts_test.go | 58 ++++++++++++++++++++++++++++ 2 files changed, 76 insertions(+), 10 deletions(-) create mode 100644 pkg/clouds/pulumi/aws/alerts_test.go diff --git a/pkg/clouds/pulumi/aws/alerts.go b/pkg/clouds/pulumi/aws/alerts.go index fdcade55..c6441a0f 100644 --- a/pkg/clouds/pulumi/aws/alerts.go +++ b/pkg/clouds/pulumi/aws/alerts.go @@ -58,6 +58,23 @@ type helperCfg struct { deployParams api.StackParams } +// helpersBuildDir returns a DETERMINISTIC build-context directory for the +// generated helpers Dockerfile, keyed on imageName. +// +// pulumi-docker's Image Diff is a structural diff of the build inputs, and it +// stores build.context / build.dockerfile as the literal path strings. Using +// os.MkdirTemp (random suffix every run) made both differ from the stored state +// on every provision, so the image reported a permanent `build: update` phantom +// diff on every PR preview. A stable path makes those fields byte-identical +// across runs; the Dockerfile content is constant and imageName already encodes +// the stack+env, so reusing/overwriting one dir per image is safe and never +// collides across stacks. Separators are replaced so imageName stays a single +// path segment (no traversal out of TempDir). +func helpersBuildDir(imageName string) string { + safe := strings.NewReplacer("/", "-", "\\", "-", ":", "-").Replace(imageName) + return filepath.Join(os.TempDir(), "sc-helpers-"+safe) +} + func pushHelpersImageToECR(ctx *sdk.Context, cfg helperCfg) (*docker.Image, error) { ecrRepoName := cfg.ecrRepoName if ecrRepoName == "" { @@ -80,16 +97,7 @@ func pushHelpersImageToECR(ctx *sdk.Context, cfg helperCfg) (*docker.Image, erro cfg.provisionParams.Log.Info(ctx.Context(), "creating temporary Dockerfile for cloud-helpers...") // hack taken from here https://github.com/pulumi/pulumi-docker/issues/54#issuecomment-772250411 - // - // Use a DETERMINISTIC dir (not os.MkdirTemp, which appends a random suffix) - // keyed on imageName. pulumi-docker's Diff is a structural diff of the build - // inputs, and it stores build.context / build.dockerfile as the literal path - // strings — a fresh random path every provision makes both differ from state - // and the image reports a permanent `build: update` phantom diff on every - // PR preview. The Dockerfile content is constant and imageName already encodes - // the stack+env, so reusing/overwriting a stable per-image dir is safe and - // never collides across stacks. - depDir := filepath.Join(os.TempDir(), "sc-helpers-"+strings.NewReplacer("/", "-", "\\", "-", ":", "-").Replace(cfg.imageName)) + depDir := helpersBuildDir(cfg.imageName) if err := os.MkdirAll(depDir, 0o700); err != nil { return nil, errors.Wrapf(err, "failed to create helpers build dir") } diff --git a/pkg/clouds/pulumi/aws/alerts_test.go b/pkg/clouds/pulumi/aws/alerts_test.go new file mode 100644 index 00000000..39f13f1f --- /dev/null +++ b/pkg/clouds/pulumi/aws/alerts_test.go @@ -0,0 +1,58 @@ +package aws + +import ( + "os" + "path/filepath" + "strings" + "testing" + + . "github.com/onsi/gomega" +) + +// helpersBuildDir must be DETERMINISTIC across calls for a given image name — +// this is the property that kills the pulumi-docker `build: update` phantom +// diff. A regression here (e.g. reverting to os.MkdirTemp's random suffix) +// reintroduces a per-preview diff on every helpers image, so guard it directly. +func TestHelpersBuildDir_DeterministicAcrossCalls(t *testing.T) { + RegisterTestingT(t) + + const name = "cloudtrail-security-audit--prod-security-helpers" + first := helpersBuildDir(name) + second := helpersBuildDir(name) + + Expect(first).To(Equal(second), "build dir must be identical across calls for the same image name") + Expect(first).To(HavePrefix(os.TempDir()), "build dir must live under the OS temp dir") + Expect(filepath.Dir(first)).To(Equal(filepath.Clean(os.TempDir())), + "build dir must be a single segment under TempDir, not nested/escaped") +} + +// Distinct image names (e.g. the audit vs critical cloudtrail stacks, or the +// ECS/ALB cloud-helpers image) must map to distinct dirs so concurrent stacks +// in one provision process never clobber each other's Dockerfile. +func TestHelpersBuildDir_DistinctPerImageName(t *testing.T) { + RegisterTestingT(t) + + audit := helpersBuildDir("cloudtrail-security-audit--prod-security-helpers") + critical := helpersBuildDir("cloudtrail-security-critical--prod-security-helpers") + ecs := helpersBuildDir("sc-cloud-helpers") + + Expect(audit).NotTo(Equal(critical)) + Expect(audit).NotTo(Equal(ecs)) + Expect(critical).NotTo(Equal(ecs)) +} + +// Path separators in the image name must be neutralised so the result stays a +// single path segment under TempDir (no directory traversal / escape). +func TestHelpersBuildDir_SanitisesSeparators(t *testing.T) { + RegisterTestingT(t) + + dir := helpersBuildDir("../../etc/evil:tag") + base := filepath.Base(dir) + + Expect(filepath.Dir(dir)).To(Equal(filepath.Clean(os.TempDir())), + "a malicious image name must not escape TempDir") + Expect(base).NotTo(ContainSubstring("/")) + Expect(base).NotTo(ContainSubstring("\\")) + Expect(base).NotTo(ContainSubstring(":")) + Expect(strings.HasPrefix(base, "sc-helpers-")).To(BeTrue()) +}