fix(aws/alerts): kill residual cloud-helpers image phantom diff (deterministic build dir)

pkg/clouds/pulumi/aws/alerts.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
+	"strings"
 
 	"github.com/pkg/errors"
 	"github.com/samber/lo"
@@ -57,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 == "" {
@@ -79,13 +97,13 @@ 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 {
+	depDir := helpersBuildDir(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)

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())
+}