diff --git a/pkg/clouds/pulumi/aws/alerts.go b/pkg/clouds/pulumi/aws/alerts.go index c1f29e69..c6441a0f 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" @@ -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) 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()) +}