diff --git a/pkg/clouds/pulumi/docker/build_and_push.go b/pkg/clouds/pulumi/docker/build_and_push.go index 22255aa9..01343f3a 100644 --- a/pkg/clouds/pulumi/docker/build_and_push.go +++ b/pkg/clouds/pulumi/docker/build_and_push.go @@ -223,10 +223,34 @@ func executeSecurityOperations(ctx *sdk.Context, stack api.Stack, dockerImage *d if scanGate != nil { reportDeps = append(reportDeps, scanGate) } - _, err = local.NewCommand(ctx, fmt.Sprintf("security-report-%s", imageName), &local.CommandArgs{ + reportResourceName := fmt.Sprintf("security-report-%s", imageName) + _, err = local.NewCommand(ctx, reportResourceName, &local.CommandArgs{ + // Stage the report script to a tempfile and invoke it by path — + // keeps the Create argv under ARG_MAX regardless of how many + // vulnerabilities the merged scan-results.json enumerates. Invoked + // via `sh` (not `bash`) because the generated script only uses + // POSIX constructs (`printf`, `jq`, parameter expansion) and SC + // can land on Alpine-based runners where `bash` is absent. + // + // On any filesystem error the helper returns "" and we fall back + // to inlining the script (preserves prior behaviour for short + // reports). The staging error is logged via ctx.Log.Warn so + // operators can investigate — silent fallback would re-introduce + // the exact ARG_MAX failure this helper exists to fix. Create: securityImageRef.ApplyT(func(img string) string { commentOutput := resolveCommentOutputPath(security, imageName) - return buildSecurityReportScript(img, imageName, security, commentOutput) + script := buildSecurityReportScript(img, imageName, security, commentOutput) + path, stageErr := stageSecurityReportScript(reportResourceName, script) + if stageErr != nil || path == "" { + if stageErr != nil { + _ = ctx.Log.Warn( + fmt.Sprintf("security-report: failed to stage script for %q (falling back to inline, large reports may hit ARG_MAX): %v", imageName, stageErr), + &sdk.LogArgs{}, + ) + } + return script + } + return fmt.Sprintf("sh %s", shellQuote(path)) }).(sdk.StringOutput), }, sdk.DependsOn(reportDeps)) if err != nil { diff --git a/pkg/clouds/pulumi/docker/security_report.go b/pkg/clouds/pulumi/docker/security_report.go index 38b3ac73..5bf87e26 100644 --- a/pkg/clouds/pulumi/docker/security_report.go +++ b/pkg/clouds/pulumi/docker/security_report.go @@ -1,13 +1,166 @@ package docker import ( + "crypto/sha256" + "encoding/hex" "fmt" + "os" "path/filepath" + "regexp" "strings" + "sync" + "time" "github.com/simple-container-com/api/pkg/api" ) +// sanitizeForFilenameRe replaces any char outside the safelist with `_`, used +// when composing a tempfile name from a Pulumi resource name (which may +// contain registry hostnames, `:`, `/`, etc.). +var sanitizeForFilenameRe = regexp.MustCompile(`[^A-Za-z0-9._-]`) + +// maxSafeNameLen caps the resource-name component of the staged-script +// basename so the full filename stays under common NAME_MAX limits (255 +// bytes on Linux, 255 chars on macOS). A long ECR-derived image name + +// stack + service can easily push the unsanitised resource name past 200 +// chars — at which point os.WriteFile returns ENAMETOOLONG and the caller +// falls back to inlining the full script, reintroducing the ARG_MAX +// failure this helper exists to avoid. +const maxSafeNameLen = 64 +const stagedScriptPrefix = "sc-security-report-" + +// stagedScriptMaxAge is the lifetime ceiling for staged scripts in TMPDIR. +// On a long-lived CI runner with hundreds of deploys per day, the staged +// files would accumulate indefinitely otherwise. 24h is long enough that +// a Pulumi resource's Delete (which inspects Create text) can still find +// the file during normal teardown windows. +const stagedScriptMaxAge = 24 * time.Hour + +// sweepOnce coalesces the TTL sweep so multiple stages in a single +// process (e.g., a stack with several images) don't each scan TMPDIR. +var sweepOnce sync.Once + +// sweepStaleStagedScripts removes stagedScriptPrefix-named files in TMPDIR +// older than `maxAge`. Best-effort — any error is silently ignored, since +// failure here would be cleanup of prior runs, not the critical path. +func sweepStaleStagedScripts(maxAge time.Duration) { + dir := os.TempDir() + entries, err := os.ReadDir(dir) + if err != nil { + return + } + cutoff := time.Now().Add(-maxAge) + for _, entry := range entries { + name := entry.Name() + if !strings.HasPrefix(name, stagedScriptPrefix) { + continue + } + info, err := entry.Info() + if err != nil { + continue + } + if info.ModTime().Before(cutoff) { + _ = os.Remove(filepath.Join(dir, name)) + } + } +} + +// stageSecurityReportScript writes the dynamically-built report script to a +// deterministic tempfile under $TMPDIR and returns the path. +// +// Why: the script is composed from the merged scan-results.json (Trivy + +// Grype) — which on an image carrying a fresh Ubuntu base can run into +// thousands of CVEs. The Pulumi `command:local:Command` resource invokes its +// `Create` field via `/bin/sh -c ""`, which means the script content +// counts against the kernel's ARG_MAX (typically 128 KB on Linux). On a +// chrome-base-derived image we observed 5,025 merged findings producing a +// >150 KB script body, and every deploy failed with: +// +// error: fork/exec /bin/sh: argument list too long +// error: update failed +// +// Staging the script to a tempfile and returning a short `sh ` +// invocation keeps the Create argv well under ARG_MAX regardless of how many +// vulnerabilities the report enumerates. +// +// ## Atomicity +// +// Writes through `os.CreateTemp + os.Rename` so concurrent deploys with +// different script content but colliding final paths (extremely unlikely +// given the 64-bit hash, but possible) can't observe a half-written file +// from another writer. `os.Rename` is atomic on the same filesystem; both +// the unique stage file and the final path live in `$TMPDIR`. +// +// ## Path stability +// +// Path is deterministic on (resourceName, script-content) — same inputs +// produce the same path, so Pulumi doesn't see spurious "drift" between +// runs that would otherwise re-trigger the resource on no-op refreshes. +// +// ## Fallback contract +// +// On any filesystem error, returns the empty string and the error; callers +// should fall back to inlining the script (preserves prior behaviour for +// short reports where ARG_MAX is not a concern) AND log the staging error +// so operators can investigate — silent fallback would re-introduce the +// exact failure mode this helper exists to fix. +// +// ## Idempotence +// +// Pulumi's ApplyT callback re-fires on every `up` / `preview`, even when +// the script content hasn't changed. The hash-derived path is stable, so +// we `Stat` first and skip the write if a file already exists at that +// path — both cheaper (no rewrite, no mtime churn) and a no-op for +// readers concurrent with our re-entry. +func stageSecurityReportScript(resourceName, script string) (string, error) { + // First call per process: clear out stale stages from prior deploys. + sweepOnce.Do(func() { sweepStaleStagedScripts(stagedScriptMaxAge) }) + + sum := sha256.Sum256([]byte(resourceName + "\x00" + script)) + safeName := sanitizeForFilenameRe.ReplaceAllString(resourceName, "_") + if len(safeName) > maxSafeNameLen { + safeName = safeName[:maxSafeNameLen] + } + finalPath := filepath.Join(os.TempDir(), fmt.Sprintf("%s%s-%s.sh", stagedScriptPrefix, safeName, hex.EncodeToString(sum[:8]))) + + // Hash-derived path uniquely identifies the (resourceName, script) + // tuple. If the file already exists, it was written by an earlier + // stage of this process or a prior `pulumi up` — trust the hash and + // skip the rewrite. Refresh mtime so the TTL sweep doesn't garbage- + // collect the file while it's still actively referenced. + if _, err := os.Stat(finalPath); err == nil { + now := time.Now() + _ = os.Chtimes(finalPath, now, now) + return finalPath, nil + } + + tmpFile, err := os.CreateTemp(os.TempDir(), stagedScriptPrefix+"stage-*.sh.tmp") + if err != nil { + return "", fmt.Errorf("create stage tempfile: %w", err) + } + tmpPath := tmpFile.Name() + cleanup := func() { _ = os.Remove(tmpPath) } + if _, err := tmpFile.Write([]byte(script)); err != nil { + _ = tmpFile.Close() + cleanup() + return "", fmt.Errorf("write stage tempfile: %w", err) + } + if err := tmpFile.Chmod(0o600); err != nil { + _ = tmpFile.Close() + cleanup() + return "", fmt.Errorf("chmod stage tempfile: %w", err) + } + if err := tmpFile.Close(); err != nil { + cleanup() + return "", fmt.Errorf("close stage tempfile: %w", err) + } + if err := os.Rename(tmpPath, finalPath); err != nil { + cleanup() + return "", fmt.Errorf("rename stage tempfile to %s: %w", finalPath, err) + } + return finalPath, nil +} + // buildSecurityReportScript generates a shell script that reads scan results and // outputs a unified security summary to the console, $GITHUB_STEP_SUMMARY, and // an optional markdown file for PR comments. diff --git a/pkg/clouds/pulumi/docker/security_report_test.go b/pkg/clouds/pulumi/docker/security_report_test.go new file mode 100644 index 00000000..b66f394f --- /dev/null +++ b/pkg/clouds/pulumi/docker/security_report_test.go @@ -0,0 +1,221 @@ +package docker + +import ( + "os" + "path/filepath" + "strings" + "sync" + "testing" + "time" + + . "github.com/onsi/gomega" +) + +func TestStageSecurityReportScript_RoundTrip(t *testing.T) { + RegisterTestingT(t) + + script := "echo hello\nprintf 'world\\n'\n" + path, err := stageSecurityReportScript("security-report-baas", script) + defer os.Remove(path) + + Expect(err).NotTo(HaveOccurred()) + Expect(path).NotTo(BeEmpty()) + Expect(path).To(HavePrefix(os.TempDir())) + Expect(path).To(HaveSuffix(".sh")) + + got, err := os.ReadFile(path) + Expect(err).NotTo(HaveOccurred()) + Expect(string(got)).To(Equal(script)) +} + +// stageSecurityReportScript must produce the same path on identical inputs so +// Pulumi sees no drift between runs of an otherwise-unchanged resource. +func TestStageSecurityReportScript_DeterministicPath(t *testing.T) { + RegisterTestingT(t) + + script := "REPORT=\"\"\nprintf '%b' \"$REPORT\"\n" + p1, err := stageSecurityReportScript("security-report-api", script) + Expect(err).NotTo(HaveOccurred()) + defer os.Remove(p1) + + p2, err := stageSecurityReportScript("security-report-api", script) + Expect(err).NotTo(HaveOccurred()) + defer os.Remove(p2) + + Expect(p1).To(Equal(p2)) +} + +// Same resource name + different script content => different paths. +func TestStageSecurityReportScript_DifferentScriptDifferentPath(t *testing.T) { + RegisterTestingT(t) + + p1, err := stageSecurityReportScript("security-report-api", "echo one\n") + Expect(err).NotTo(HaveOccurred()) + defer os.Remove(p1) + + p2, err := stageSecurityReportScript("security-report-api", "echo two\n") + Expect(err).NotTo(HaveOccurred()) + defer os.Remove(p2) + + Expect(p1).NotTo(Equal(p2)) +} + +// Resource names may contain `:` or `/` (registry URLs, etc.). The tempfile +// path must be filesystem-safe. +func TestStageSecurityReportScript_SanitizesResourceName(t *testing.T) { + RegisterTestingT(t) + + path, err := stageSecurityReportScript( + "security-report-471112843480.dkr.ecr.us-west-2.amazonaws.com/baas-ecr:tag", + "echo ok\n", + ) + Expect(err).NotTo(HaveOccurred()) + defer os.Remove(path) + + // No raw `:` or `/` should leak into the basename. + base := path[strings.LastIndex(path, "/")+1:] + Expect(base).NotTo(ContainSubstring(":")) +} + +// Pulumi's ApplyT callback re-fires on every preview/up. The helper must +// short-circuit when the deterministic path already exists — no rewrite, +// no mtime churn beyond the explicit refresh. Asserts: +// 1. Second call returns identical path. +// 2. mtime was refreshed by the second call (so TTL sweep won't garbage- +// collect actively-referenced files between preview and apply). +// 3. File size unchanged (no rewrite happened — content was already correct). +func TestStageSecurityReportScript_StatShortCircuit(t *testing.T) { + RegisterTestingT(t) + + const script = "echo first\n" + first, err := stageSecurityReportScript("security-report-shortcircuit", script) + Expect(err).NotTo(HaveOccurred()) + defer os.Remove(first) + + firstInfo, err := os.Stat(first) + Expect(err).NotTo(HaveOccurred()) + firstSize := firstInfo.Size() + + // Backdate so we can detect whether the second call refreshed mtime. + old := time.Now().Add(-12 * time.Hour) + Expect(os.Chtimes(first, old, old)).To(Succeed()) + + second, err := stageSecurityReportScript("security-report-shortcircuit", script) + Expect(err).NotTo(HaveOccurred()) + Expect(second).To(Equal(first)) + + secondInfo, err := os.Stat(second) + Expect(err).NotTo(HaveOccurred()) + Expect(secondInfo.Size()).To(Equal(firstSize), "stat short-circuit should not rewrite file") + Expect(secondInfo.ModTime()).To(BeTemporally(">", old.Add(time.Hour)), "mtime should have been refreshed by stat short-circuit") +} + +// The TTL sweep removes prefix-matching files older than the threshold. +// Asserts a young file survives + an old file gets reaped. +func TestSweepStaleStagedScripts(t *testing.T) { + RegisterTestingT(t) + + dir := os.TempDir() + young := filepath.Join(dir, stagedScriptPrefix+"sweep-young-"+strings.Repeat("a", 8)+".sh") + old := filepath.Join(dir, stagedScriptPrefix+"sweep-old-"+strings.Repeat("b", 8)+".sh") + Expect(os.WriteFile(young, []byte("echo young\n"), 0o600)).To(Succeed()) + Expect(os.WriteFile(old, []byte("echo old\n"), 0o600)).To(Succeed()) + defer os.Remove(young) + defer os.Remove(old) + + // Backdate the old one well past the sweep threshold (1h). + pastCutoff := time.Now().Add(-2 * time.Hour) + Expect(os.Chtimes(old, pastCutoff, pastCutoff)).To(Succeed()) + + sweepStaleStagedScripts(1 * time.Hour) + + Expect(young).To(BeAnExistingFile(), "young file should survive sweep") + Expect(old).NotTo(BeAnExistingFile(), "old file should be reaped by sweep") +} + +// Long resource names (registry hostname + slash-separated path + tag +// produced by SC's naming convention) can push the unsanitised resource +// name well past NAME_MAX (255 bytes on Linux). The helper must cap the +// resource-name component so the final basename stays under the limit — +// otherwise ENAMETOOLONG would trip the fallback path and reintroduce +// the ARG_MAX failure for the large reports this helper exists to fix. +func TestStageSecurityReportScript_CapsLongResourceName(t *testing.T) { + RegisterTestingT(t) + + long := strings.Repeat("very-long-resource-name-segment.", 20) // ~640 chars + path, err := stageSecurityReportScript(long, "echo ok\n") + Expect(err).NotTo(HaveOccurred()) + defer os.Remove(path) + + base := filepath.Base(path) + Expect(len(base)).To(BeNumerically("<", 255)) +} + +// `os.Rename` is atomic on the same filesystem. Concurrent writers +// targeting the same final path (same resource name + same script +// content => same path) must not produce a partially-written file +// observable by a reader. We can't directly assert atomicity, but we +// can run many writers in parallel and assert every observed final +// file is complete + correct. +func TestStageSecurityReportScript_ConcurrentWriters(t *testing.T) { + RegisterTestingT(t) + + const script = "REPORT=\"value\"\nprintf '%s\\n' \"$REPORT\"\n" + const n = 24 + + var wg sync.WaitGroup + wg.Add(n) + paths := make([]string, n) + errs := make([]error, n) + for i := 0; i < n; i++ { + go func(i int) { + defer wg.Done() + paths[i], errs[i] = stageSecurityReportScript("security-report-concurrent", script) + }(i) + } + wg.Wait() + + for i, p := range paths { + Expect(errs[i]).NotTo(HaveOccurred()) + Expect(p).NotTo(BeEmpty()) + got, err := os.ReadFile(p) + Expect(err).NotTo(HaveOccurred()) + Expect(string(got)).To(Equal(script), "writer %d observed truncated/interleaved content", i) + } + // All paths identical (deterministic). + for i := 1; i < n; i++ { + Expect(paths[i]).To(Equal(paths[0])) + } + defer os.Remove(paths[0]) +} + +// Reality check: the original ARG_MAX failure was a ~150 KB inlined script. +// A staged invocation `sh ` is well under the kernel limit +// (typically 128 KB on Linux). This test asserts the contract — that the +// helper produces a short path that the caller can compose into a short +// Create command, regardless of the script body size. +func TestStageSecurityReportScript_HandlesLargeScript(t *testing.T) { + RegisterTestingT(t) + + // Build a script body that exceeds typical Linux ARG_MAX (128 KB) by + // roughly 2× — the size class the original failure landed at on a + // chrome-base-derived image (5,025 merged CVEs producing ~150 KB). + const targetBytes = 256 * 1024 + var sb strings.Builder + for sb.Len() < targetBytes { + sb.WriteString("REPORT=\"${REPORT}| HIGH | CVE-XXXX-YYYY | pkg | x.y | - |\\n\"\n") + } + largeScript := sb.String() + Expect(len(largeScript)).To(BeNumerically(">", 256*1024)) + + path, err := stageSecurityReportScript("security-report-large", largeScript) + Expect(err).NotTo(HaveOccurred()) + defer os.Remove(path) + + // Path itself stays small. + Expect(len(path)).To(BeNumerically("<", 256)) + + got, err := os.ReadFile(path) + Expect(err).NotTo(HaveOccurred()) + Expect(len(got)).To(Equal(len(largeScript))) +}