fix(docker): stage security-report script via tempfile, avoid ARG_MAX by Cre-eD · Pull Request #303 · simple-container-com/api

Cre-eD · 2026-05-30T08:17:12Z

Why

The Pulumi command:local:Command resource that emits SC's security-report invokes its Create field via /bin/sh -c "<Create>", which means the entire generated script body counts against the kernel's ARG_MAX (typically 128 KB on Linux).

buildSecurityReportScript composes the script from the merged scan-results.json (Trivy + Grype). On an image carrying a fresh Ubuntu base — e.g. chrome-base-derived images used by browser-automation services — the merged scan can exceed 5,000 findings, producing script bodies in the 150-200 KB range. Every deploy on those images fails with:

error: fork/exec /bin/sh: argument list too long
error: update failed

Observed in integrail/baas v2026.5.2 → test (run 26629920604) on 2026-05-29: 5,025 merged findings → 152 KB Create body → kernel rejected the fork before SC's report renderer ever ran.

Fix

Stage the generated script to a deterministic tempfile and invoke bash <path> instead of inlining. The Create argv stays under a few hundred bytes regardless of how many vulnerabilities the report enumerates.

New helper stageSecurityReportScript(resourceName, script):
- Writes to $TMPDIR/sc-security-report-<sanitized-name>-<hash>.sh.
- Path is deterministic on (resourceName, script) — same inputs produce the same path, so Pulumi sees no spurious drift between runs of an otherwise-unchanged resource.
- Filename sanitised — resource names may contain registry hostnames with : / /, which break tempfile paths.
Caller in build_and_push.go uses the staged path; if staging fails (any filesystem error), falls back to inlining the script — preserves prior behaviour for short reports where ARG_MAX is not a concern.

Tests

5 new in pkg/clouds/pulumi/docker/security_report_test.go:

Test	Asserts
`RoundTrip`	Content written + read back matches exactly
`DeterministicPath`	Same inputs ⇒ same path (no Pulumi drift)
`DifferentScriptDifferentPath`	Different content ⇒ different path (collision avoidance)
`SanitizesResourceName`	`:` and `/` removed from basename
`HandlesLargeScript`	256 KB script body roundtrips; path stays small (regression test for the ARG_MAX class of failure)

All 5 pass. Full go test ./pkg/clouds/pulumi/docker/... clean; no existing tests touched. go vet clean.

Out of scope (complementary)

VEX-based scan-result trimming at the scanner layer is being shipped in Integrail/baas#138 with a .vex/chrome-base.openvex.json doc following the canonical pattern from Integrail/everworker#2554. That reduces noise across the whole pipeline (Trivy/Grype output, DefectDojo uploads, etc).

This PR is a different fix at a different layer: it makes SC's report renderer robust to large inputs in the first place, so any image with a busy CVE surface can deploy cleanly even before VEX has been authored. The two fixes are complementary and worth shipping together.

Test plan

go test ./pkg/clouds/pulumi/docker/... — all green (5 new tests + existing)
go vet ./... — clean
CI on this PR
Validate end-to-end via baas deploy: bump SC pin in baas to a preview of this branch, re-trigger build-and-deploy.yaml, confirm renderer step completes with the 5K-finding scan body

The Pulumi `command:local:Command` security-report resource invokes its `Create` field via `/bin/sh -c "<Create>"`, which means the entire generated script body counts against the kernel's ARG_MAX (typically 128 KB on Linux). `buildSecurityReportScript` composes the script from the merged scan-results.json (Trivy + Grype). On an image carrying a fresh Ubuntu base — e.g. chrome-base-derived images used by browser-automation services — the merged scan can exceed 5,000 findings, producing script bodies in the 150-200 KB range. Every deploy on those images fails with: error: fork/exec /bin/sh: argument list too long error: update failed Observed in integrail/baas v2026.5.2 → test (run 26629920604) on 2026-05-29: 5,025 merged findings → 152 KB Create body → kernel rejected the fork before SC's report renderer ever ran. ## Fix Stage the generated script to a deterministic tempfile and invoke `bash <path>` instead of inlining. The Create argv stays under a few hundred bytes regardless of how many vulnerabilities the report enumerates. - New helper `stageSecurityReportScript(resourceName, script)`: - Writes to `$TMPDIR/sc-security-report-<sanitized-name>-<hash>.sh`. - Path is deterministic on `(resourceName, script)` so Pulumi sees no spurious drift between runs of an otherwise-unchanged resource. - Filename sanitised — resource names may contain registry hostnames with `:` / `/`, which break tempfile paths. - Caller in `build_and_push.go` uses the staged path; if staging fails (any filesystem error), falls back to inlining the script — preserves prior behaviour for short reports where ARG_MAX is not a concern. ## Tests 5 new in `pkg/clouds/pulumi/docker/security_report_test.go`: - `RoundTrip` — content written + read back matches exactly. - `DeterministicPath` — same inputs => same path (no Pulumi drift). - `DifferentScriptDifferentPath` — different content => different path (collision avoidance). - `SanitizesResourceName` — `:` and `/` removed from basename. - `HandlesLargeScript` — 256 KB script body roundtrips; path stays small. Direct regression test for the ARG_MAX class of failure. All 5 pass. Full `go test ./pkg/clouds/pulumi/docker/...` clean; no existing tests touched. `go vet` clean. ## Out of scope - VEX-based scan-result trimming (consumed-side fix at the scanner layer): tracked in Integrail/baas#138 with a `.vex/` document produced by Integrail/everworker#2554's canonical pattern. That shrinks the input to this renderer; this commit makes the renderer robust to large inputs in the first place. The two fixes are complementary and both worth shipping — VEX reduces noise across the pipeline (DefectDojo, etc), tempfile-staging hardens this specific step. Signed-off-by: Dmitrii Creed <creeed22@gmail.com>

github-actions · 2026-05-30T08:18:08Z

Semgrep Scan Results

Repository: api | Commit: 8eb8b17

Check	Status	Details
✅ Semgrep	Pass	0 total findings (no error/warning)

Scanned at 2026-05-30 12:39 UTC

github-actions · 2026-05-30T08:18:27Z

Security Scan Results

Repository: api | Commit: 8eb8b17

Check	Status	Details
✅ Secret Scan	Pass	No secrets detected
✅ Dependencies (Trivy)	Pass	0 total (no critical/high)
✅ Dependencies (Grype)	Pass	0 total (no critical/high)
📦 SBOM	Generated	527 components (CycloneDX)

Scanned at 2026-05-30 12:39 UTC

Four follow-ups from independent code reviews on this PR (codex + claude opus, identical findings on the bash issue, opus caught the atomicity gap, codex caught the filename-length gap). ## 1. `bash` → `sh` (codex + opus, both P0) `build_and_push.go:241` invoked the staged script via `bash`. SC can land on Alpine-based Pulumi runners where `bash` isn't installed — exit 127 before the report script ever runs. The generated script uses only POSIX constructs (`printf`, `jq`, parameter expansion) so `sh` works equally well. Switched. ## 2. Atomic write via `os.CreateTemp` + `os.Rename` (opus P0) `security_report.go`: previous `os.WriteFile` truncates-then-streams. Concurrent deploys with *different* script content but the *same* sanitised prefix could interleave a write with bash reading the file. Same-content collisions were already safe (idempotent overwrite), but different-content collisions weren't. Now writes to a unique `os.CreateTemp` file in `$TMPDIR` and `os.Rename`s to the deterministic final path. Rename is atomic on the same filesystem (both files in `$TMPDIR`), so readers never observe a partial file from another writer. ## 3. Cap resource-name component of the basename (codex P0) `security_report.go`: a long resource name (registry hostname + slash-separated path + tag, easily 200+ chars from SC's naming convention) could push the sanitised basename past NAME_MAX (255 bytes on Linux). `os.WriteFile` returned ENAMETOOLONG → caller fell back to inlining the full script → reintroduced the exact ARG_MAX failure this helper exists to fix. Added `maxSafeNameLen = 64` cap on the resource-name component. Hash (16 hex chars) carries collision avoidance; the human-readable prefix is purely for debuggability. ## 4. Log fallback errors via `ctx.Log.Warn` (opus P0) `build_and_push.go`: previously the staging error was silently discarded — operators wouldn't see *why* the deploy fell back to inlining (and potentially hit ARG_MAX). Now logs a warn through the Pulumi context with the underlying error, while still falling back gracefully (preserves prior behaviour for short reports). ## Test additions - `TestStageSecurityReportScript_CapsLongResourceName` — 640-char resource name produces a basename < 255 bytes. - `TestStageSecurityReportScript_ConcurrentWriters` — 24 goroutines hammering the same final path; every observed read sees complete, correct content (regression guard for the atomicity fix). All 7 tests pass. Full `go test ./pkg/clouds/pulumi/docker/...` clean. `go vet` clean. Signed-off-by: Dmitrii Creed <creeed22@gmail.com>

Two should-fix items from the codex + opus review pass, folded into this PR. ## 1. TTL sweep (opus #2 should-fix) On a long-lived self-hosted CI runner, `/tmp/sc-security-report-*.sh` files accumulate indefinitely — every deploy stages a new file, nothing cleans them up. Hundreds per day × months → directory growth unbounded. Added `sweepStaleStagedScripts(maxAge)` that scans `$TMPDIR` for prefix-matching files and removes anything older than `maxAge`. Called once per process via `sync.Once` from `stageSecurityReportScript` — first stage in a deploy sweeps prior runs, subsequent stages in the same `pulumi up` no-op. Threshold: 24h. Long enough that a Pulumi resource's Delete (which inspects the original Create text) can still find the file during normal teardown windows. Best-effort: any FS error during sweep is silently ignored — failure here would be cleanup of prior runs, not the critical path of this deploy. ## 2. Stat short-circuit (opus #4 nit) Pulumi's ApplyT callback re-fires on every `preview`/`up`. The previous implementation rewrote the file (via atomic rename) every time, even when nothing had changed — mtime churn on no-op refreshes. Added an `os.Stat(finalPath)` guard: if the deterministic-path file already exists, return it without rewriting. Refreshes mtime via `os.Chtimes` so the TTL sweep doesn't garbage-collect actively- referenced files between preview and apply. The hash-derived path guarantees content matches when the path exists (collision probability ~2^-64), so the short-circuit is safe without a content read. ## Tests - `TestStageSecurityReportScript_StatShortCircuit` — backdates the staged file, re-stages with identical content, asserts mtime was refreshed AND file size unchanged (no rewrite happened). - `TestSweepStaleStagedScripts` — creates a young + old file with the staged prefix, runs sweep with 1h cutoff, asserts young survives and old is reaped. All 9 tests in the file pass. Full package + `go vet` clean. ## Not in scope - `bash` → `sh` fallback path for the *inlined* script (the inline-script case only runs when staging fails — already exceptional, doesn't compound the Alpine concern). - Shared `integrail-deployer-bot` IAM-key blast radius — operational concern, not an SC code change. Signed-off-by: Dmitrii Creed <creeed22@gmail.com>

Root-cause fix for the post-v2026.5.43 E2BIG failure observed in [integrail/baas run 26690935574](https://github.com/Integrail/baas/actions/runs/26690935574). ## What we saw SC v2026.5.43 deploy retry on baas — tempfile staging from #303 was in effect (Create was the short `sh '/tmp/sc-security-report-…sh'`, ~80 bytes), yet kernel still returned E2BIG. Pulumi diff showed an in-place update on `command:local:Command::security-report-baas--test/baas` and a coincidental provider URN upgrade (`default → default_1_2_1`). My first instinct was the migration was carrying old state somewhere — wrong. ## Actual root cause Reading `pulumi-command/provider/pkg/provider/local/commandOutputs.go::run()`: ```go if in.AddPreviousOutputInEnv == nil || *in.AddPreviousOutputInEnv { if out.Stdout != "" { cmd.Env = append(cmd.Env, fmt.Sprintf("%s=%s", util.PulumiCommandStdout, out.Stdout)) } if out.Stderr != "" { ... } } ``` **`AddPreviousOutputInEnv` defaults to true.** On every Update, the provider injects the previous run's full stdout as a single `PULUMI_COMMAND_STDOUT=<…>` env var. The security-report script writes the rendered vulnerability table (thousands of CVE rows from Trivy + Grype merged) to stdout for inline build-log visibility — on a chrome-base-derived image, ~150-200 KB. pulumi-command captures that into the resource's `Stdout` output. On the next Update, kernel ARG_MAX (~128 KB on Linux, covering argv + envp combined) trips at fork/exec — even though our argv is the ~80-byte `sh '/tmp/...'`. The earlier suspect (provider migration) was a red herring. Update path reads `news.Create`, not `olds.Create`: ```go // commandController.go func (c *Command) Update(ctx, req) { cmd := news.Create // not olds.Create err := run(ctx, *cmd, state.BaseInputs, ...) } ``` ## Empirical signature this matches - `scan-baas--test/baas-merged` Update **succeeded** before security-report failed. Both used the same provider migration; the difference is scan-merged's stdout is a short summary (~few KB). - `security-report-baas--test/baas` Update **failed** with E2BIG. Its stdout is the full ~150 KB table. - Argv shown in the error is short. The bloat is envp, not argv. ## Fix `AddPreviousOutputInEnv: sdk.Bool(false)` on the security-report `CommandArgs`. The security script doesn't read `PULUMI_COMMAND_STDOUT`, so disabling the env injection is a no-op for script behaviour. Stdout is still captured into Pulumi state for `pulumi stack output` and downstream references — just not re-injected into envp. ## Why this is better than `ReplaceOnChanges + DeleteBeforeReplace` The first draft of this PR used those options. They happen to work (replace creates a fresh resource with empty `out.Stdout`, so the env injection has nothing to pass) — but for the wrong reason: 1. Replace cycle every time report content changes (every deploy with new CVEs) is wasteful churn. 2. Doesn't address the actual root cause; if pulumi-command ever passes state by other means we'd regress. 3. Replace-vs-update in operator-visible Pulumi diffs is noise. `AddPreviousOutputInEnv: false` is surgical and pinpoints the real issue. ## Blast radius Audited every `local.NewCommand` site in SC's docker package: | Resource | Stdout size | Risk | Action | |---|---|---|---| | `security-report-<image>` | Unbounded (rendered CVE table) | **HIGH** | **Fix in this PR** | | `scan-<image>-merged` / `scan-<image>-<tool>` | Bounded (~few KB summary) | Low | Worth defense-in-depth follow-up | | `registry-login-<image>` | ~zero | Safe | — | | `sign-<image>` / `verify-<image>` / `sbom-<image>` / `provenance-<image>` (via `newSecurityCommand`) | Bounded (cosign/syft output) | Low | Worth defense-in-depth follow-up | Defense-in-depth across all security commands is a follow-up; this PR addresses the bug that's currently breaking deploys. ## Test plan - [x] `go build ./pkg/clouds/pulumi/docker/...` clean - [x] `go test ./pkg/clouds/pulumi/docker/...` — all 9 existing tests pass - [x] `go vet ./pkg/clouds/pulumi/docker/...` clean - [x] Read pulumi-command source to confirm root cause - [x] Cross-reference scan-merged success vs security-report failure to validate the stdout-size hypothesis - [ ] CI on this PR - [ ] Release + install-sc bump + retry baas deploy 26690935574 → confirm security-report Pulumi resource completes Signed-off-by: Dmitrii Creed <creeed22@gmail.com>

Cre-eD requested review from Laboratory, smecsia and universe-ops as code owners May 30, 2026 08:17

Cre-eD added 2 commits May 30, 2026 15:55

smecsia approved these changes May 30, 2026

View reviewed changes

Merge branch 'main' into fix/scan-report-arg-max

76e049d

Cre-eD merged commit 42d3374 into main May 30, 2026
8 checks passed

Cre-eD mentioned this pull request May 30, 2026

fix(docker): disable AddPreviousOutputInEnv on security-report #305

Merged

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(docker): stage security-report script via tempfile, avoid ARG_MAX#303

fix(docker): stage security-report script via tempfile, avoid ARG_MAX#303
Cre-eD merged 4 commits into
mainfrom
fix/scan-report-arg-max

Cre-eD commented May 30, 2026

Uh oh!

github-actions Bot commented May 30, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented May 30, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

Cre-eD commented May 30, 2026

Why

Fix

Tests

Out of scope (complementary)

Test plan

Uh oh!

github-actions Bot commented May 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Semgrep Scan Results

Uh oh!

github-actions Bot commented May 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Security Scan Results

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

github-actions Bot commented May 30, 2026 •

edited

Loading

github-actions Bot commented May 30, 2026 •

edited

Loading