ci: switch to ubuntu-latest-large, likely root cause of stuck runs#1305
Open
abueide wants to merge 15 commits into
Open
ci: switch to ubuntu-latest-large, likely root cause of stuck runs#1305abueide wants to merge 15 commits into
abueide wants to merge 15 commits into
Conversation
Every run dispatched from this branch (release dry-run, e2e-tests, ci.yml) sat at runner_id: 0 - queued, never picked up - despite the code itself being fine. Found the likely actual cause: this repo's workflows still use plain ubuntu-latest, while segmentio/analytics-next (which works fine) uses ubuntu-latest-large everywhere. There's also an already-open, unrelated PR in this repo (#1296) whose entire diff is exactly this same ubuntu-latest -> ubuntu-latest-large migration, confirming the org has moved off plain ubuntu-latest. Switches the three files this PR touches. Will verify with a real dispatch before re-enabling e2e-tests.yml's push/pull_request triggers (left as workflow_dispatch-only in the previous commit, pending this fix's confirmation).
Artifactory's curation policy was returning 403 on these tarball fetches through virtual-npm-thirdparty: - flatted@3.2.9 -> bumped to ^3.4.2 (fixes two HIGH severity advisories: prototype pollution via parse(), unbounded recursion DoS) - lodash-es@4.17.21 -> bumped to ^4.18.0 (fixes HIGH severity code injection via _.template) - ip@2.0.0 -> bumped to ^2.0.1 (fixes the two LOW severity advisories that do have patches) ip's HIGH severity SSRF advisory (isPublic() misclassification) has no fix at all - GitHub's advisory data shows firstPatchedVersion: null, vulnerable range "<= 2.0.1" (every published version, including the one this bumps to). ip is purely a transitive dependency of build tooling (node-gyp, semantic-release, npm-registry-fetch, per node_modules) - never shipped in the published SDK packages. If Artifactory's curation still blocks 2.0.1 for that unfixed advisory, the remediation path is a curation waiver request for this specific package/advisory, not a further version bump - no code change can fix a vulnerability with no patched version. yarn.lock regenerated via devbox (yarn-berry, already declared in devbox.json) rather than the system yarn - this machine's global Homebrew yarn (3.4.1) conflicts with Corepack's need to own the same binary path for the packageManager-pinned yarn@4.1.0, and devbox's isolated Nix environment sidesteps that entirely.
The first real Artifactory-routed install surfaced far more blocked packages than the 3 originally reported - 22 distinct package@version pairs across 16 package names, all transitive (semantic-release/npm's own dependency chain: npm-the-package, pacote, tar, glob, sigstore, protobufjs, @grpc/grpc-js, etc.). Two categories: - Clear CVE fixes (babel-plugin-transform-modules-systemjs, grpc-js, diff, fast-xml-parser, glob, image-size, protobufjs, shell-quote, sigstore 4.x, ws, undici): bumped to each advisory's firstPatchedVersion per GitHub's Security Advisory API. - No covering CVE at the blocked version (browserslist, minimatch, npm, pacote, sigstore 2.x): these already exceeded their only known advisory's fix version, so the block is likely Artifactory's "immature version" cooldown policy rather than a real CVE - bumped forward with a modest floor and let yarn resolve to whatever's actually latest/mature. tar@^6.1.11/^6.1.2/^6.2.0 (resolving 6.2.1) has no available fix at all - `tar@^6.2.2` returned "No candidates found", confirming 6.2.1 is the last-ever 6.x release (same unfixed situation as ip's HIGH severity SSRF advisory from the previous commit). Left unresolved rather than forcing a major-version jump to 7.x, which risks breaking whatever declared the narrower ^6.x range deliberately - if curation still blocks this, the path is a waiver request, not a further version bump. yarn.lock regenerated via devbox (same as the previous commit). Verified: devbox run check passes clean (lint, format, build, typecheck, all 70 test suites/492 tests).
…outs Two real problems from the last CI run: 1. tar@6.2.1 (via node-gyp) and browserslist were still 403'd despite the previous batch fix. tar 6.x genuinely has no fix (confirmed last commit - ^6.2.2 returns "No candidates found", 6.2.1 is the last-ever 6.x release) - but node-gyp 13.0.1 depends on tar@^7.5.4 instead of 6.x (checked node-gyp's current package.json on GitHub). Overriding node-gyp itself sidesteps the unfixable tar line entirely rather than forcing a breaking major bump on tar directly. Needed two resolutions keys - node-gyp@^12.4.0 (what npm's package.json declared) and node-gyp@latest (a separate dist-tag declaration elsewhere in the tree, resolving independently to the same stale 10.0.1) - fixing only one left the other still pulling tar@6.2.1. browserslist was also still blocked at 4.28.6 despite already exceeding its only known CVE fix (4.16.5) by a wide margin - not a CVE match, more likely Artifactory's curation flagging brand-new releases pending an async scan. Bumped the floor to ^4.24.0 instead of chasing "latest" again. 2. Both failed jobs took ~9.5 minutes each to fail (not fast), per the last run's logs - yarn's default httpTimeout (60s) x httpRetry (3) let a single hung/blocked request eat most of the job's time before finally erroring. Added explicit httpTimeout: 30000 / httpRetry: 2 to .yarnrc.yml so a curation block or genuine network hang fails in under a minute instead of burning ~10 minutes per CI attempt. Verified: devbox run check passes clean (lint, format, build, typecheck, all 70 test suites/492 tests). tar@6.2.1 confirmed gone from yarn.lock entirely - only the already-fixed 7.5.20 line remains.
Confirmed via the actual CI log this didn't help: the ~7-9 minute hang is an ETIMEDOUT at the TCP connect layer (node:net internalConnectMultiple), which happens before any HTTP request is sent - httpTimeout/httpRetry only bound the HTTP request/response cycle after a connection is already established, so they were never going to touch this specific failure mode.
Every curation block so far has just shown a raw 403 with no policy reason, forcing manual guesswork (checked GitHub's advisory API, compared publish dates, etc.) to figure out why. IPD's curation.md documents jf ca (curation audit) as the actual authoritative way to see which policy blocked a package (48h age cooldown, critical CVE, malicious package) - this wires that in as an automatic, targeted diagnostic instead of leaving it to manual investigation next time. New .github/actions/curation-audit: - Parses the captured yarn install log for the specific packages that actually got a curation 403 (not a full-tree scan - jf ca doesn't support Yarn natively per IPD's own docs, and scanning the whole lockfile would be slow and could trigger unrelated checks) - Builds a throwaway npm-only project containing just those package@version pairs, authenticated with the same OIDC-exchanged token the Artifactory OIDC Auth step already obtained this job - jf ca --run-native reads registry config from .npmrc, sidestepping the missing Yarn support entirely - Runs jf ca and prints the actual policy violation per package Wired into every job in both ci.yml and release.yml as an if: failure() step right after "Install dependencies" - it only runs when the install actually fails, and only audits what failed. Split the release.yml jobs' combined "yarn install && yarn build && yarn release" run: blocks into a separate Install dependencies step so there's a clean point to attach the audit and capture its log, matching the pattern already used in the ci job.
![semgrep-code-segmentio-2[bot]](https://avatars.githubusercontent.com/u/819518?s=60&v=4){kind=small}
Comment on lines
+15
to
+32
| run: | | ||
| set -euo pipefail | ||
|
|
||
| # yarn logs each curation 403 as three lines: | ||
| # ➤ YN0035: │ <name>@npm:<version>: The remote server failed to provide... | ||
| # ➤ YN0035: │ Response Code: 403 (Forbidden) | ||
| # Strip ANSI color codes first, then pull name@version off the first line. | ||
| sed -E 's/\x1b\[[0-9;]*m//g' "${{ inputs.install-log }}" \ | ||
| | grep "npm:.*: The remote server failed" \ | ||
| | sed -E 's/.*YN0035: [│└] //' \ | ||
| | sed -E 's/(@npm:[^:]+): The remote.*/\1/' \ | ||
| | sort -u > /tmp/curation-blocked-packages.txt || true | ||
|
|
||
| count=$(wc -l < /tmp/curation-blocked-packages.txt | tr -d ' ') | ||
| echo "count=$count" >> "$GITHUB_OUTPUT" | ||
| echo "Found $count curation-blocked package(s):" | ||
| cat /tmp/curation-blocked-packages.txt | ||
|
|
There was a problem hiding this comment.
Using variable interpolation ${{...}} with github context data in a run: step could allow an attacker to inject their own code into the runner. This would allow them to steal secrets and code. github context data can have arbitrary user input and should be treated as untrusted. Instead, use an intermediate environment variable with env: to store the data and use the environment variable in the run: script. Be sure to use double-quotes the environment variable, like this: "$ENVVAR".
⭐ Fixed in commit 17305d2 ⭐
The last run proved this bug live: yarn install failed with real curation 403s (browserslist, npm, pacote, sigstore, ws) after 9m31s, but the step still reported success because GHA's default `bash -e` does NOT set pipefail - `| tee` made the step's exit code tee's (0), not yarn's. That silently skipped the new Curation Audit step (it's gated on if: failure()) and left eslint/commitlint to fail downstream with a confusing "missing node_modules" error instead of the real cause.
…fails Auditing only after yarn install already failed meant every curation block still cost the full ~9 minute install hang before we saw a reason. Read every resolved package straight out of yarn.lock (a committed file, not a log we scraped) and run jf ca against all of them before yarn install even starts, so a blocked package fails in seconds with a policy reason instead of minutes with a raw 403. This also removes the action's `install-log` input entirely, which was flagged by Semgrep as a shell-injection vector (untrusted input interpolated directly into a run: script). There's no longer an untrusted input to interpolate - the audit reads yarn.lock from the checkout, so the finding no longer applies.
The pre-flight audit builds a synthetic project pinning every package yarn.lock resolved as a flat top-level dependency. npm's own resolver enforces peer-dependency compatibility on that flat list and hit a real ERESOLVE conflict (@react-native-firebase/app@22.2.0 vs a peer requirement for 17.5.0 from @react-native-firebase/messaging) before ever reaching the curation check - yarn tolerates this shape of mismatch in its own resolver, npm does not. jf ca ships --legacy-peer-deps specifically to bypass this for audits like ours, where we only care whether a given version is curation-blocked, not whether the flattened tree is peer-valid.
… tree jf ca --run-native resolves the synthetic project as a single atomic npm operation. Confirmed live: with every yarn.lock package flattened into one project, npm's own resolution needs to actually fetch each tarball, and the first hard-blocked one (npm@10.9.8, denied by an Xray "download blocking policy" on remote-npm-thirdparty-filtered- cache) aborted the whole run before a single result could be reported - so a combined audit can only ever surface one blocked package per run, never the full picture. Give every resolved package its own throwaway project and .npmrc and run jf ca against it independently, 16 at a time via xargs. One package's hard block can no longer hide any of the others - every curation-blocked package gets its own reported reason. Verified the extraction, scoped-package name/version splitting, and the pass/fail aggregation locally against a stub jf before relying on real CI time to confirm it end-to-end.
Pre-flight auditing the whole resolved tree doesn't work with jf ca:
confirmed live that a single hard-curation-blocked package aborts the
entire native npm resolution atomically, so a full-tree audit can
only ever surface one blocked package per run. Auditing every
resolved package individually to work around that hits a different
problem: packages with their own peer dependencies (most babel/eslint
plugins) fail npm ls with ELSPROBLEMS in an isolated single-package
project - confirmed 556 of 557 "blocked" results just now were this
false positive, not real curation blocks.
The packages actually blocked in practice (browserslist, npm, pacote,
sigstore, ws) are plain utility packages with no peer deps of their
own, so isolating just them doesn't hit that problem - the fix is to
go back to running the audit after yarn install fails, scoped to
exactly the packages yarn's own resolver (which handles multi-version/
peer forking correctly) already confirmed as 403'd, rather than trying
to preflight the whole tree.
check_one.sh also no longer trusts jf's exit code alone - it only
reports BLOCKED when the output actually contains a real curation
signal ("download blocking policy" or a nonzero blocked-package
count), and downgrades any other failure to a non-fatal "inconclusive"
note, so unrelated resolution noise on some future package can't be
mistaken for a real block.
Restores the install-log input this reactive design needs, this time
threaded through via env: (as Semgrep's own suggested fix indicated)
instead of interpolated directly into the run: script, so the
shell-injection finding from before doesn't reappear.
…ines Confirmed against a real failed run: yarn prints each 403 twice - once in the resolution tree (prefixed with │/└) and again in the final summary (no prefix). The extraction's prefix-strip only handled the first form, so the second leaked "➤ YN0035: " into the parsed name and produced a bogus extra "package" per real block (7 real blocks showed up as 14 extracted lines). Made the │/└ prefix optional so both forms normalize to the same name@version and collapse via the existing sort -u.
The new curation audit step (see previous commits) finally gave real, authoritative reasons instead of raw 403s: - browserslist, sigstore, and pacote were never blocked themselves - they're blocked via transitive deps (baseline-browser-mapping, electron-to-chromium, tar) that publish new versions almost daily. Our existing resolutions used floating caret ranges as overrides, which still re-resolve to whatever's newest on every lockfile regen - for packages that publish faster than the 48h curation cooldown, "newest" is *always* less than 48h old, so every rebump just walked the pin forward into the next immature version. Pinned tar/baseline-browser-mapping/electron-to-chromium to exact (no caret) versions confirmed several days old, so they stop drifting forward on future installs. - npm@10.9.8 hit a separate, harder Xray "download blocking policy" (not the age cooldown) with no clear fixed version. Traced it to semantic-release-yarn depending on the old @semantic-release/npm@11 (which wants npm@^10.0.0) even though our own devDependencies already use @semantic-release/npm@13.1.5. Overriding that stale transitive consumer to 13.1.5 removes the npm@10.x subtree entirely instead of chasing a version pin - and turned out to also carry its own bundled (and separately blocked) copies of sigstore and pacote, so this one override cleared three of the seven blocked packages at once. devbox run check: 70 suites / 492 tests still pass.
ws@6.2.6 and ws@7.5.13 were still 403ing in real installs even though
the curation audit reports them clean (not a policy violation) -
both were published only 5 days ago, so this looks like a stale
cached block decision from when they really were brand-new rather
than a live re-check. Same underlying mistake as the earlier tar/npm/
pacote fixes: our resolutions values were floating caret ranges
("^6.2.4", "^7.5.11"), so every lockfile regen kept re-picking
whatever patch was newest at that moment.
Pinned to exact versions we have direct evidence already resolve
without issue: 7.5.10 (2+ years old, was already resolving cleanly
for the ws@^7.0.0 consumer with no override at all) for every 7.x
consumer, and 6.2.4 (2 months old) for the 6.x line. Also added an
explicit ws@^7.0.0 entry that had no override before and was quietly
relying on luck.
devbox run check: 70 suites / 492 tests still pass.
7.5.10 (used in the previous commit as a known-not-currently-blocked version) is actually vulnerable to a HIGH severity DoS advisory fixed in 7.5.11 (GHSA memory exhaustion from tiny fragments/data chunks, published 2026-06-15). Correctness here matters independently of whatever is blocking ws in CI - don't deliberately pin a vulnerable version just because it happens to currently pass Artifactory. devbox run check: 70 suites / 492 tests still pass.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Every run dispatched on the previous branch (release dry-run, e2e-tests, ci.yml) sat at
runner_id: 0- queued, never picked up - despite the workflow content itself being fine. Likely root cause: this repo's workflows still use plainubuntu-latest, whilesegmentio/analytics-next(which works fine) usesubuntu-latest-largeeverywhere. There's also an already-open, unrelated PR in this repo (#1296) whose entire diff is exactly this sameubuntu-latest->ubuntu-latest-largemigration, confirming the org has moved off plainubuntu-latest.Switches
ci.yml,release.yml, ande2e-tests.yml(the same three files touched in #1304, now merged) toubuntu-latest-large.Note
This commit was originally pushed to #1304's branch but landed ~3 minutes after that PR was merged, so it never made it into master - opening fresh here to carry it over.
Not verified end-to-end
Runs on the previous branch weren't even generating fresh queued jobs for new commits near the end (a scheduling-level symptom, not just runner-picking) - unclear if that was transient. Please confirm a real run on this branch gets picked up before relying on it.
Test plan