fix(distribute): stop bloating FLY_DISTRIBUTE_RESULTS env var (E2BIG)#70
Conversation
The distribute action persisted the full DistributeResponse — including the unbounded per-file `files[]` breakdown — into $GITHUB_ENV as FLY_DISTRIBUTE_RESULTS so the post step could render the job-summary table. Across distribute steps this accumulated past the Linux single-env-var limit (MAX_ARG_STRLEN, 128 KB), after which the kernel rejects execve for every later step and post-cleanup with "Argument list too long" (E2BIG). The job summary only reads package_name, package_version, package_type, public_url and download_url. Persist just those fields via a new DistributeSummaryEntry projection, bounding per-entry size. The full response is still exposed verbatim through the step `results` output, which is not subject to the env-var limit. Fixes #69
Add a createJobSummary test that feeds the exact slim FLY_DISTRIBUTE_RESULTS shape (no download_count, no files[]) and asserts the Distributed Artifacts table — including the docker pull command — still renders, faithfully covering the new env-var contract from #69.
|
📗 Scan Summary
📦 Vulnerable Dependencies
🔖 Details[ CVE-2026-12151 ] undici 6.24.1Vulnerability Details
Resource exhaustion in undici's WebSocket client leads to denial of service when when connecting to untrusted servers. [ CVE-2026-9679 ] undici 6.24.1Vulnerability Details
Impact: Applications that parse a Set-Cookie header and then forward the parsed value into a response header (proxies, middleware, SSR frameworks) become vulnerable to HTTP response header injection: an attacker-controlled upstream can inject arbitrary Set-Cookie, Location, or Cache-Control headers into the application's downstream response, enabling session fixation, open redirect, or cache poisoning. Affected applications are those that use undici's cookie parsing (parseSetCookie, parseCookie, getSetCookies) and forward the parsed cookie value into a response header. This was introduced in undici 7.0.0 via PR #3789. Patches: Workarounds: |
Transitive dependency via @actions/http-client (range ^6.23.0). Also syncs the lockfile's stale root version (1.6.10 -> 1.7.1) to match package.json.
The lib/*.js bundles embed undici via @actions/http-client (esbuild). Bumping undici requires regenerating them so the committed bundles match the lockfile; fixes the "Verify lib/ is compiled and up to date" CI check.
omerzi
left a comment
omerzi
left a comment
There was a problem hiding this comment.
Solid, correctly-targeted fix — the projection is the right call, the type-narrowing of buildDockerPullCommand is valid (it only reads package_type/public_url/package_version, all present in the slim entry), and the regression test pins the exact failure mode. A few things worth a conversation before merge; details are inline. Two cross-cutting points that don't land on changed lines:
1. Same latent bug in the transfer path (most important). appendTransferResults in src/transfer.ts:210 persists the full TransferSummaryEntry { ..., results: FlyClientResult[] } to FLY_TRANSFER_RESULTS with the identical newline-accumulate pattern. FlyClientResult is small per item, but results[] scales with file count, so a large upload/download accumulated across steps can hit the same E2BIG wall. This fix addresses one of two instances of the same class of bug — please at least flag the transfer path as the next domino, ideally apply the same projection there.
2. Root cause vs. mitigation. The projection bounds per-entry size but not total accumulation — the env var is still newline-appended across every distribute step, so with enough steps you march toward 128 KB more slowly. The truly root-cause fix is to stop using $GITHUB_ENV as the accumulator (write to a temp file or $GITHUB_STATE, which the post step can already read and which isn't subject to MAX_ARG_STRLEN). Scoping that out here is reasonable since slim entries are tiny, but "Fixes #69" slightly oversells it — consider softening to "mitigates" or filing a follow-up for the file-based accumulator.
The code change itself is clean on KISS/YAGNI — explicit field-listing over Omit/destructure-rest is the right choice since it fails safe when a new unbounded field is added to DistributeResponse.
- Apply the same slim-projection to the transfer path: appendTransferResults
now persists only {name,status} per file to FLY_TRANSFER_RESULTS, dropping
the unused `message`. results[] scales with file count, so this closes the
second instance of the #69 E2BIG class of bug. Full results (with message)
remain on the step `results` output.
- DRY: export toSummaryEntry from distribute.ts and reuse it in the test
instead of a byte-identical local copy.
- Docs: keep the full #69 rationale on the DistributeSummaryEntry type
(canonical home) and reduce the toSummaryEntry / appendDistributeResults
JSDocs to one-line pointers.
|
Thanks @omerzi — all points addressed in 18b11f4:
296 tests pass, lint clean, |
2 participants
Summary
Mitigates #69 — the
distributeaction breaks all later steps withArgument list too long(E2BIG) onceFLY_DISTRIBUTE_RESULTSexceeds the Linux single-env-var limit (MAX_ARG_STRLEN, 128 KB).Root cause:
distributepersisted the entireDistributeResponse— including the unbounded per-filefiles[]breakdown — into$GITHUB_ENVso the post step could render the job-summary "Distributed Artifacts" table. Accumulated across distribute steps, the value crossed 128 KB, after which the kernel rejectsexecvefor every later step and post-cleanup step.Fix: The job summary only ever reads 5 fields (
package_name,package_version,package_type,public_url,download_url). Persist just those via a newDistributeSummaryEntryprojection, bounding per-entry size. The full response is still exposed verbatim through the stepresultsoutput, which is not subject to the env-var limit.Changes
src/types.ts— addDistributeSummaryEntry(canonical distribute action breaks all later steps with "Argument list too long" (E2BIG) — FLY_DISTRIBUTE_RESULTS exported to $GITHUB_ENV exceeds 128 KB #69 rationale lives here) andTransferSummaryResult.src/distribute.ts— exporttoSummaryEntry; project results before exporting to$GITHUB_ENV; keep full response on theresultsoutput.src/transfer.ts— same projection applied to the transfer path (appendTransferResults): persist only{name,status}per file toFLY_TRANSFER_RESULTS, dropping the unusedmessage.results[]scales with file count, so this closes the second instance of the same E2BIG class. Full results (withmessage) remain on theresultsoutput.src/job-summary.ts— parse/render the slim types.src/distribute-core.ts— narrowbuildDockerPullCommandparam (full responses stay compatible).lib/*.js— rebuilt bundles.Dependency bump (security, not scope creep)
@actions/http-client). Frogbot flagged that 6.24.1 carries CVE-2026-12151 (High, WebSocket DoS) and CVE-2026-9679 (Medium, cookie-parser header injection), both fixed in 6.27.0. Both are Not Applicable by contextual analysis (we don't use undici's WebSocket client or cookie parser), so the bump is remediation/hygiene with no behavior change. The rebuiltlib/*.jsundici internals follow from this bump. Also synced the lockfile's stale root version (1.6.10 → 1.7.1) to matchpackage.json.Test plan
npm test— 296 tests pass, incl. regressions asserting the distribute env var contains nofiles/manifest.jsonand that the job summary renders from the slim shape (nodownload_count/files).npm run build— tsc--noEmittypecheck + esbuild bundles regenerated.npm run lint— clean.unit-tests(incl. "Verify lib/ is compiled and up to date") green.Follow-up
$GITHUB_ENV(temp file) for the fully root-cause fix.