Skip to content

Fail PRs that pin packages JFrog will not serve - #5656

Merged
philip merged 4 commits into
mainfrom
ci/jfrog-deps-pr-gate
Aug 28, 2026
Merged

Fail PRs that pin packages JFrog will not serve#5656
philip merged 4 commits into
mainfrom
ci/jfrog-deps-pr-gate

Conversation

@andrelandgraf

@andrelandgraf andrelandgraf commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Problem

Protected-runner CI installs through the Databricks JFrog npm mirror. That mirror quarantines each package version for seven days. A lockfile that pins a version inside that window fails npm ci with 403 and {7d-cooldown}.

#5640 pinned next to 16.3.3. The existing check Dependencies available on Databricks proxy went red on that PR:

✗ next@16.3.3 — no publish date on the registry; cannot confirm it is available

The PR merged anyway. main then could not npm ci on this runner group. Scheduled jobs that install (agent-discovery, docs-api, format-markdown) fail at Install dependencies. Vercel stays green because it uses public npm.

scripts/deps-proxy-allowlist.json skipped named versions in that check. The JSON cannot make JFrog serve a quarantined tarball, so a PR could add its own exception and look green while npm ci still 403s.

This repo has no required status checks, so a red check does not block merge. The check also used path filters, so it never posted on content-only PRs and could not be marked required without deadlocking those PRs.

Diagnosis

The check already existed and already failed on the bump. Two things kept it from being a gate:

  1. Skip list, not install. The checker used packument publish time and treated deps-proxy-allowlist.json as a skip. JFrog 403s the tarball even when the packument is readable.
  2. Advisory check, path-filtered. main has no branch protection. The job only ran when package.json / package-lock.json changed, so the context is missing on most PRs.

What changed

Same-repo PRs probe the tarball with a 1-byte range GET against JFrog, then cancel the body (some registries ignore Range and would stream the whole file). HTTP 403 fails the check. A served tarball passes. scripts/deps-proxy-allowlist.json is deleted.

Fork PRs cannot mint JFrog OIDC. They run on ubuntu-latest and fail when public npm publish age is younger than seven days.

The workflow runs on every pull_request with no path filter. When the PR introduces no new lockfile versions vs the base, the script exits 0 so the check context still posts.

Same-repo blocked bump:

✗ next@16.3.3 — proxy returned 403; published 2.0d ago (< 7d cooldown).

The Databricks npm proxy quarantines package versions for their first 7 days.
Pin the offending dependency to an older, already-available version
(update the root dependency and refresh the lockfile).

Fork blocked bump (age check, no JFrog call):

✗ next@16.3.3 — published 2.0d ago (< 7d cooldown); too new for the Databricks npm mirror.

Content-only PR (no new lockfile versions):

Databricks proxy check — cooldown 7d, scope: changed (0 version(s) to verify).
Nothing to verify. ✓

npm run check:deps-proxy is unchanged as the entry point.

What this does not do

The merge button is unchanged until a repo admin requires the check context Dependencies available on Databricks proxy on main (strict required status checks).

It does not revert next@16.3.3 on main. PRs that do not introduce new lockfile versions still pass this check while other npm ci jobs stay red until that version clears the cooldown or is pinned back.

Verification

npx vitest run scripts/check-deps-proxy-allowlisted.test.js
# 16 passed, including a local server that ignores Range and streams a 50MB body

BASE_REF=origin/main node scripts/check-deps-proxy-allowlisted.mjs
# Databricks proxy check — cooldown 7d, scope: changed (0 version(s) to verify).
# Nothing to verify. ✓

This PR does not bump dependencies, so the always-on workflow should itself go green.

Risks

Until an admin requires the check, a red result can still be merged, same as #5640. Fork PRs approximate the cooldown with publish age rather than a JFrog 403.

A PR can already fail the proxy age check and still merge, and listing a version in deps-proxy-allowlist.json made that check green while npm ci still 403s. HEAD the tarball on same-repo CI and age-check forks, and post the check on every PR so it can be required.
HEAD is the wrong verb for what Artifactory actually answers; npm ci's 403 is on GET.
@vercel

vercel Bot commented Aug 27, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
neon-next Ready Ready Preview Aug 27, 2026 6:08pm

Request Review

Keep the why for the dual signal and the byte-range probe; drop restated how.
A Range-ignoring registry would stream the whole package and leave the required check hanging after it had already printed success. The JSON allowlist is unread and would still look like a way to pass.

@philip philip left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, thank you!

@philip philip left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@philip
philip merged commit 630c431 into main Aug 28, 2026
3 checks passed
@philip
philip deleted the ci/jfrog-deps-pr-gate branch August 28, 2026 15:25
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.

2 participants