Skip to content

ci: triage chronic fork CI reds (zizmor, OSV, image scan, root-path tests) - #26

Merged
blackflame007 merged 1 commit into
litellm_internal_stagingfrom
litellm_ci_fork_red_triage
Jul 28, 2026
Merged

ci: triage chronic fork CI reds (zizmor, OSV, image scan, root-path tests)#26
blackflame007 merged 1 commit into
litellm_internal_stagingfrom
litellm_ci_fork_red_triage

Conversation

@blackflame007

Copy link
Copy Markdown

TLDR

Problem this solves:

  • Four checks failed on essentially every push or schedule against litellm_internal_staging, so nobody could tell a new red from the standing red; the chore: sync upstream/main → litellm_internal_staging (782 commits, v1.93.0 → main tip) #24 sync already cost us signal because one real failure had to be separated from the noise by hand
  • Two of them were reporting genuine vulnerable dependencies that we were merging past every day
  • test-server-root-path never asserted anything at all; it hung and got cancelled at the 30 minute job cap on every run since 2026-07-04, burning about an hour of runner time per PR across the two matrix legs

How it solves it:

  • Every finding here turned out to be real, so nothing is waived; each one is fixed at the source
  • The root-path job now bounds each step that can stall, so a failure names the step that timed out instead of surfacing as a bare cancelled job

Relevant issues

Linear ticket

Resolves NOL-81

Pre-Submission checklist

  • I have added meaningful tests
  • My PR passes all CI/CD checks (e.g., lint, format, unit tests)
  • My PR's scope is as isolated as possible; it only solves 1 specific problem
  • I have received a Greptile Confidence Score of at least 4/5 before requesting a maintainer review

There are no unit tests here because nothing in litellm/ changed; the checks themselves are the test, and the proof below is each scanner run to completion locally against this branch

Screenshots / Proof of Fix

zizmor, same version and severity floor the workflow uses, run against this branch:

$ uvx zizmor@1.24.1 --min-severity medium --no-progress .
...
No findings to report. Good job! (147 suppressed)
$ echo $?
0

Before this branch it exited 13 on:

##[warning]check-ui-api-types.yml:57: action's hash pin has mismatched or missing version comment: points to commit 249970729cb0
##[warning]test_server_root_path.yml:105: action's hash pin has mismatched or missing version comment: points to commit 249970729cb0
##[error]Process completed with exit code 13

osv-scanner, same pinned version and same invocation as the workflow:

$ ./osv-scanner scan source --config osv-scanner.toml -L uv.lock -L ui/litellm-dashboard/package-lock.json
Scanned uv.lock file and found 444 packages
Scanned ui/litellm-dashboard/package-lock.json file and found 969 packages
GHSA-w8v5-vhqr-4h9v and 2 aliases have been filtered out because: diskcache has no fixed release published; remove this entry once one exists
Filtered 2 vulnerabilities from output

No issues found
$ echo $?
0

On litellm_internal_staging the same command reports 7:

| https://osv.dev/GHSA-3rp5-jjmw-4wv2 | 7.0  | PyPI | gitpython             | 3.1.52 | 3.1.53  | uv.lock |
| https://osv.dev/GHSA-6p8h-3wgx-97gf | 7.5  | PyPI | gitpython             | 3.1.52 | 3.1.54  | uv.lock |
| https://osv.dev/GHSA-94p4-4cq8-9g67 | 7.5  | PyPI | gitpython             | 3.1.52 | 3.1.55  | uv.lock |
| https://osv.dev/GHSA-fjr4-x663-mwxc | 8.1  | PyPI | gitpython             | 3.1.52 | 3.1.54  | uv.lock |
| https://osv.dev/GHSA-r9mr-m37c-5fr3 | 8.8  | PyPI | gitpython             | 3.1.52 | 3.1.54  | uv.lock |
| https://osv.dev/GHSA-mh99-v99m-4gvg | 7.5  | npm  | brace-expansion (dev) | 5.0.7  | 5.0.8   | ui/litellm-dashboard/package-lock.json |
| https://osv.dev/GHSA-r28c-9q8g-f849 | 7.5  | npm  | postcss               | 8.5.13 | 8.5.18  | ui/litellm-dashboard/package-lock.json |

Lockfile still installs cleanly after the surgical bump:

$ cd ui/litellm-dashboard && npm ci --dry-run --ignore-scripts
added 827 packages in 290ms
$ echo $?
0

The Chrome-for-Testing object the root-path job stalls on is intact, which is what rules out a truncated download and points at the post-download handoff:

$ curl -sSIL https://cdn.playwright.dev/builds/cft/145.0.7632.6/linux64/chrome-linux64.zip
HTTP/2 307
location: https://storage.googleapis.com/chrome-for-testing-public/145.0.7632.6/linux64/chrome-linux64.zip
HTTP/2 200
content-length: 175440843

175440843 bytes is exactly the "167.3 MiB" the progress bar reaches before every run goes silent

Type

Infrastructure

Changes

zizmor was a real finding, though a small one. Two workflows pinned actions/setup-node to v6.4.0's SHA while commenting it # v6, and the moving v6 tag has since advanced to v6.5.0, so the comment named a different commit than the pin. Every other pin in the tree comments the exact tag whose commit it pins, which is why only these two were flagged. Both are repinned to v6.5.0 with an exact comment, which also picks up that release's own dependency updates

osv-scan was a real finding and is fixed rather than ignored, since every advisory had a published fix. gitpython goes to 3.1.56 through uv lock --upgrade-package gitpython, which touched four lines and nothing else. It reaches us through the optional mlflow extra, so it is not in the shipped image, but there is no reason to carry five advisories for a lockfile bump. On the dashboard, postcss goes to 8.5.18 and the brace-expansion override to 5.0.8. Those two are hand-applied to package-lock.json rather than regenerated, because npm install --package-lock-only on current npm also prunes three unrelated optional peer entries; that drift reproduces with zero package.json changes, so it does not belong in a security fix

image-scan was a real finding that is already resolved. Grype flagged pypdf and pyasn1 in the runtime image, and the upstream sync in #24 moved both to fixed releases. Only the daily schedule stayed red because it ran against the pre-merge commit; the image-scan run on #24's head passed every step including the grype gate. Nothing was needed here beyond confirming it

test-server-root-path was a real regression that the chronic red had been hiding. It last passed on 2026-06-13 and has been cancelled on all 28 runs since 2026-07-04, both matrix legs, always at the same place: npx playwright install prints the Chrome-for-Testing progress bar up to 100 percent and then never returns, and the job dies at its 30 minute cap. The retry wrapper added in BerriAI#32406 cannot help, because a hang means no attempt ever returns for the loop to retry. So each browser-install attempt is now wrapped in timeout, which is what actually lets that retry loop do its job. install-deps is split out and left outside timeout so a SIGTERM can never strand a half-applied dpkg state. The build, install and e2e steps each carry a timeout-minutes sized to their real cost, which means the job-level cap should never be what fires, and a failure points at a step instead of showing up as an unexplained cancellation. DEBUG=pw:install is set so the next occurrence records where it stopped

One unrelated bug fell out of reading that job: the npm cache was keyed on ui/litellm-dashboard/package-lock.json while npm ci runs in tests/e2e/ui, so it could never hit. It now points at the lockfile that is actually installed

I did not touch the osv-scanner.toml entry for diskcache. It is a correct waiver with a real reason and an ignoreUntil of 2026-09-09 that forces a re-look, which is exactly the shape a waiver should have

QA runbook

  1. Confirm GitHub Actions Security Analysis is green on this PR; it runs on every push, so it gates itself
  2. Confirm OSV Scan is green on this PR
  3. Confirm UI Build Check, UI Lint and UI Unit Tests are unchanged by the postcss bump
  4. Image Scan only runs on PRs that touch its paths list, and uv.lock is on it, so it should run here and pass
  5. For test-server-root-path, the honest expectation is that it now fails fast rather than passing. If the install still hangs, it should stop at 8 minutes with a named step timeout and pw:install output showing where, rather than a cancelled job at 30 minutes. If a retry does get through, the e2e assertion runs for the first time since June and its result is real signal either way

Final Attestation

  • The tests check the right things, including the edge cases, and regressions in the respective real-world customer use-cases are not possible after this PR

…ests)

Four checks had been failing on essentially every push or schedule against
litellm_internal_staging, so a genuine regression could no longer be told
apart from the standing noise.

zizmor exited 13 on two action pins whose version comment did not match the
commit they pinned: both used the moving `v6` tag as a comment while pinning
setup-node v6.4.0's SHA. Repinned to v6.5.0 with an exact comment.

osv-scan reported gitpython 3.1.52 (5 advisories, up to 8.8) plus postcss
8.5.13 and brace-expansion 5.0.7 on the dashboard. All three have fixed
releases, so they are bumped rather than ignored. gitpython arrives through
the optional mlflow extra and is not in the shipped image; postcss and
brace-expansion are build/dev only, but a fix exists so there is no reason
to carry them.

Grype's image-scan findings (pypdf, pyasn1) were already resolved by the
upstream sync in #24; only the daily schedule, which ran against the
pre-merge commit, was still red.

test-server-root-path was not asserting anything. Since 2026-07-04 every run
hung in `playwright install` immediately after the Chrome-for-Testing zip
reached 100%, then died at the 30 minute job cap as "cancelled". The retry
wrapper added in BerriAI#32406 cannot recover from that because no attempt ever
returns. Each browser-install attempt is now bounded by `timeout`, the apt
half is split out so a SIGTERM cannot strand dpkg, and every long step got
its own cap so a failure names the step instead of surfacing as a bare
cancelled job. DEBUG=pw:install is set so the next occurrence says where it
stopped. The npm cache key also pointed at the dashboard lockfile while
`npm ci` runs in tests/e2e/ui, so it never hit; it now points at the
lockfile actually installed.
@blackflame007
blackflame007 merged commit edf1586 into litellm_internal_staging Jul 28, 2026
73 of 77 checks passed
@blackflame007
blackflame007 deleted the litellm_ci_fork_red_triage branch July 28, 2026 21:53
blackflame007 added a commit that referenced this pull request Jul 28, 2026
… install hang (#28)

`test-server-root-path` last passed 2026-06-13 and was cancelled on every run
since 2026-07-04, on both matrix legs, always at the same point: the
Chrome-for-Testing zip reaches 100% and `npx playwright install` never returns,
burning the full 30m job cap.

With DEBUG=pw:install (added in #26) the first post-mitigation run named the
step exactly, identically on all four attempts and both legs:

    pw:install -- download complete, size: 175440843
    pw:install SUCCESS downloading Chrome for Testing 145.0.7632.6
    pw:install removing existing browser directory if any
    pw:install extracting archive
    <no further output; killed by `timeout 75`>

So the transfer was never the problem; extraction was. Node 24.16.0 regressed
stream destruction (nodejs/node#63487), which deadlocks the yauzl-based
extractor Playwright shells out to, and Playwright only worked around it in
1.60.0 (microsoft/playwright#41000). tests/e2e/ui pins @playwright/test 1.58.1,
so the pairing is unconditionally broken.

The Node bump came from this fork, not upstream: a1b68cf moved this job from
node-version "20" to "24.16.0" on 2026-06-16, three days after the last green
run, and upstream is still on "20". Moving to the 22 LTS line clears the
regression without dragging a shared upstream test suite through a four-minor
Playwright bump, and keeps us off an EOL Node.

Reproduced and confirmed locally with @playwright/test 1.58.1, same machine and
same archive, varying only the Node version: Node 26.5.0 hangs at `extracting
archive` indefinitely, Node 22.23.1 completes the install in 16s.

The timeout and retry guards from #26 stay in place so any recurrence fails fast
and names the step instead of going silent.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant