From 8e807ca3ccce78c5fc912341f99432acf717b4c1 Mon Sep 17 00:00:00 2001 From: Andre Landgraf Date: Thu, 27 Aug 2026 10:51:20 -0700 Subject: [PATCH 1/4] Gate website CI on JFrog tarball availability, not a skip list. 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. --- .github/workflows/deps-proxy-allowlist.yml | 24 +-- scripts/check-deps-proxy-allowlisted.mjs | 198 ++++++++++++------- scripts/check-deps-proxy-allowlisted.test.js | 157 +++++++++++++++ 3 files changed, 291 insertions(+), 88 deletions(-) create mode 100644 scripts/check-deps-proxy-allowlisted.test.js diff --git a/.github/workflows/deps-proxy-allowlist.yml b/.github/workflows/deps-proxy-allowlist.yml index cc9f0e0c884..c7ebe439acd 100644 --- a/.github/workflows/deps-proxy-allowlist.yml +++ b/.github/workflows/deps-proxy-allowlist.yml @@ -9,15 +9,14 @@ name: Dependencies ↔ Databricks proxy # dependency, so a contributor who bumps a package before it clears the cooldown # learns exactly what to do (pin an older version / refresh the lockfile) instead # of chasing a 403 in an unrelated job. See scripts/check-deps-proxy-allowlisted.mjs. +# +# No path filter: the check context must post on every PR so it can be a required +# status check without deadlocking content-only PRs. The script is a no-op when +# the PR introduces no new lockfile versions. Forks cannot mint JFrog OIDC, so +# they run on ubuntu-latest and age-check against public npm instead. on: pull_request: - paths: - - 'package.json' - - 'package-lock.json' - - 'scripts/check-deps-proxy-allowlisted.mjs' - - 'scripts/deps-proxy-allowlist.json' - - '.github/workflows/deps-proxy-allowlist.yml' workflow_dispatch: permissions: @@ -27,9 +26,7 @@ permissions: jobs: check: name: 'Dependencies available on Databricks proxy' - runs-on: - group: neondatabase-protected-runner-group - labels: linux-ubuntu-latest + runs-on: ${{ github.event.pull_request.head.repo.fork && 'ubuntu-latest' || fromJSON('{"group":"neondatabase-protected-runner-group","labels":"linux-ubuntu-latest"}') }} steps: - name: Harden the runner (Audit all outbound calls) uses: step-security/harden-runner@b09bb98e06d4d774595224525879c09bc6e98c40 # v2.20.1 @@ -48,8 +45,9 @@ jobs: # The protected runner group has no egress to the public npm registry — it # can only reach the Databricks JFrog mirror. Mint a short-lived token via # OIDC and point npm at the mirror so the packument (with publish dates) - # is readable. + # is readable. Forks cannot mint OIDC; they age-check on public npm. - name: Setup JFrog CLI with OIDC + if: ${{ !github.event.pull_request.head.repo.fork }} id: setup-jfrog uses: jfrog/setup-jfrog-cli@279b1f629f43dd5bc658d8361ac4802a7ef8d2d5 # v4.9.1 env: @@ -57,6 +55,7 @@ jobs: with: oidc-provider-name: github-actions - name: Configure npm registry (Databricks JFrog mirror) + if: ${{ !github.event.pull_request.head.repo.fork }} run: | cat > ~/.npmrc < Set of locked versions. Skips the root project and any * git/file/workspace/link deps (they don't go through the npm mirror). */ -function collectRegistryDeps(lock) { +export function collectRegistryDeps(lock) { const out = new Map(); const packages = lock.packages ?? {}; for (const [key, entry] of Object.entries(packages)) { @@ -90,14 +87,14 @@ function collectRegistryDeps(lock) { } /** Derive the package name from a lockfile `packages` key like a/node_modules/b. */ -function nameFromPackageKey(key) { +export function nameFromPackageKey(key) { const marker = 'node_modules/'; const idx = key.lastIndexOf(marker); return idx === -1 ? key : key.slice(idx + marker.length); } /** Versions present in head but not in base -> the set a PR newly introduces. */ -function newlyIntroduced(headMap, baseMap) { +export function newlyIntroduced(headMap, baseMap) { const candidates = []; for (const [name, versions] of headMap) { const baseVersions = baseMap.get(name) ?? new Set(); @@ -108,7 +105,7 @@ function newlyIntroduced(headMap, baseMap) { return candidates; } -function flatten(headMap) { +export function flatten(headMap) { const all = []; for (const [name, versions] of headMap) { for (const version of versions) all.push({ name, version }); @@ -116,17 +113,11 @@ function flatten(headMap) { return all; } -/** Turn an allowlist file's entries into a predicate. */ -function makeAllowlist(entries) { - const set = new Set(entries); - return ({ name, version }) => set.has(name) || set.has(`${name}@${version}`); -} - /** * Classify a single candidate given the packument `time` map. Pure. * Returns { name, version, status: 'ok'|'immature'|'unknown', ageDays }. */ -function classify({ name, version }, timeMap, now) { +export function classify({ name, version }, timeMap, now, cooldownMs = COOLDOWN_MS) { const published = timeMap?.[version]; if (!published) return { name, version, status: 'unknown', ageDays: null }; const ageMs = now - Date.parse(published); @@ -134,11 +125,48 @@ function classify({ name, version }, timeMap, now) { return { name, version, - status: ageMs < COOLDOWN_MS ? 'immature' : 'ok', + status: ageMs < cooldownMs ? 'immature' : 'ok', ageDays, }; } +/** npm tarball path: unscoped `foo/-/foo-1.0.0.tgz`, scoped `@scope/foo/-/foo-1.0.0.tgz`. */ +export function fallbackTarballUrl(registry, name, version) { + const encoded = name.startsWith('@') + ? `@${encodeURIComponent(name.slice(1))}` + : encodeURIComponent(name); + const filename = name.slice(name.lastIndexOf('/') + 1); + return new URL(`${encoded}/-/${filename}-${version}.tgz`, registry).toString(); +} + +export function tarballStatusFromHttp(status) { + if (status === 200 || status === 204 || status === 206) return 'ok'; + if (status === 401 || status === 403 || status === 404) return 'blocked'; + return 'unknown'; +} + +/** + * Same-repo CI talks to JFrog: the tarball status is whether `npm ci` will + * succeed. Forks only see public npm, which serves new tarballs, so age is + * the cooldown signal. + */ +export function decideAvailability({ tarballStatus, ageStatus, useTarballGate }) { + if (useTarballGate) { + if (tarballStatus === 'ok') return 'ok'; + if (tarballStatus === 'blocked') return 'blocked'; + return 'unknown'; + } + return ageStatus; +} + +export function usesTarballGate(registry) { + try { + return new URL(registry).host !== PUBLIC_REGISTRY_HOST; + } catch { + return false; + } +} + // --------------------------------------------------------------------------- // Imperative shell: git, npm config, network, process exit. // --------------------------------------------------------------------------- @@ -147,18 +175,6 @@ function readJson(file) { return JSON.parse(fs.readFileSync(file, 'utf8')); } -function loadAllowlist() { - if (!fs.existsSync(ALLOWLIST_PATH)) return () => false; - try { - const entries = readJson(ALLOWLIST_PATH); - if (!Array.isArray(entries)) throw new Error('allowlist must be a JSON array'); - return makeAllowlist(entries); - } catch (err) { - console.error(`Warning: could not read ${path.relative(ROOT, ALLOWLIST_PATH)}: ${err.message}`); - return () => false; - } -} - function npmConfig(key) { try { const value = execFileSync('npm', ['config', 'get', key], { @@ -219,13 +235,21 @@ function packumentUrl(registry, name) { return new URL(encoded, registry).toString(); } -async function fetchTimeMap(registry, headers, name) { +async function fetchPackument(registry, headers, name) { const res = await fetch(packumentUrl(registry, name), { headers }); if (!res.ok) { throw new Error(`HTTP ${res.status} fetching packument for ${name}`); } - const doc = await res.json(); - return doc.time ?? {}; + return res.json(); +} + +async function probeTarball(url, headers) { + const res = await fetch(url, { + method: 'GET', + headers: { ...headers, Range: 'bytes=0-0' }, + redirect: 'follow', + }); + return tarballStatusFromHttp(res.status); } /** Resolve tasks with bounded concurrency, preserving input order. */ @@ -261,6 +285,20 @@ function loadBaseDepMap() { return null; } +function formatFailure(v) { + const id = `${v.name}@${v.version}`; + if (v.status === 'blocked' && v.ageDays != null) { + return ` ✗ ${id} — proxy returned 403; published ${v.ageDays.toFixed(1)}d ago (< ${COOLDOWN_DAYS}d cooldown).`; + } + if (v.status === 'blocked') { + return ` ✗ ${id} — tarball is forbidden on the Databricks proxy.`; + } + if (v.status === 'immature') { + return ` ✗ ${id} — published ${v.ageDays.toFixed(1)}d ago (< ${COOLDOWN_DAYS}d cooldown); blocked by the Databricks proxy.`; + } + return ` ✗ ${id} — cannot confirm it is available on the Databricks proxy.`; +} + async function main() { if (!fs.existsSync(LOCK_PATH)) { console.error('No package-lock.json found — nothing to check.'); @@ -269,16 +307,13 @@ async function main() { const headMap = collectRegistryDeps(readJson(LOCK_PATH)); const baseMap = loadBaseDepMap(); - const isAllowed = loadAllowlist(); const scope = baseMap ? 'changed' : 'full'; - const raw = baseMap ? newlyIntroduced(headMap, baseMap) : flatten(headMap); - const candidates = raw.filter((c) => !isAllowed(c)); + const candidates = baseMap ? newlyIntroduced(headMap, baseMap) : flatten(headMap); console.log( - `Databricks proxy allowlist check — cooldown ${COOLDOWN_DAYS}d, ` + - `scope: ${scope} (${candidates.length} version(s) to verify` + - `${raw.length !== candidates.length ? `, ${raw.length - candidates.length} allowlisted` : ''}).` + `Databricks proxy check — cooldown ${COOLDOWN_DAYS}d, ` + + `scope: ${scope} (${candidates.length} version(s) to verify).` ); if (candidates.length === 0) { @@ -287,51 +322,57 @@ async function main() { } const { registry, headers } = registryInfo(); + const useTarballGate = usesTarballGate(registry); - // Fetch each packument once per unique package name. const names = [...new Set(candidates.map((c) => c.name))]; - const timeMaps = new Map(); + const packuments = new Map(); const fetchErrors = []; await mapWithConcurrency(names, FETCH_CONCURRENCY, async (name) => { try { - timeMaps.set(name, await fetchTimeMap(registry, headers, name)); + packuments.set(name, await fetchPackument(registry, headers, name)); } catch (err) { fetchErrors.push({ name, message: err.message }); - timeMaps.set(name, {}); + packuments.set(name, null); } }); const now = Date.now(); - const verdicts = candidates.map((c) => classify(c, timeMaps.get(c.name), now)); + const verdicts = await mapWithConcurrency(candidates, FETCH_CONCURRENCY, async (c) => { + const doc = packuments.get(c.name); + const age = classify(c, doc?.time ?? {}, now); + let tarballStatus = 'unknown'; + if (useTarballGate) { + const url = + doc?.versions?.[c.version]?.dist?.tarball ?? + fallbackTarballUrl(registry, c.name, c.version); + try { + tarballStatus = await probeTarball(url, headers); + } catch (err) { + fetchErrors.push({ name: `${c.name}@${c.version}`, message: err.message }); + } + } + return { + ...age, + status: decideAvailability({ tarballStatus, ageStatus: age.status, useTarballGate }), + }; + }); - const immature = verdicts.filter((v) => v.status === 'immature'); - const unknown = verdicts.filter((v) => v.status === 'unknown'); + const failed = verdicts.filter((v) => v.status !== 'ok'); - for (const v of immature) { - console.error( - ` ✗ ${v.name}@${v.version} — published ${v.ageDays.toFixed(1)}d ago ` + - `(< ${COOLDOWN_DAYS}d cooldown); blocked by the Databricks proxy.` - ); - } - for (const v of unknown) { - console.error( - ` ✗ ${v.name}@${v.version} — no publish date on the registry; ` + - `cannot confirm it is available on the Databricks proxy.` - ); + for (const v of failed) { + console.error(formatFailure(v)); } if (fetchErrors.length) { - console.error('\nCould not fetch some packuments:'); + console.error('\nCould not fetch some registry metadata:'); for (const e of fetchErrors) console.error(` ! ${e.name}: ${e.message}`); } - if (immature.length || unknown.length) { + if (failed.length) { console.error( '\nThe Databricks npm proxy quarantines package versions for their first ' + `${COOLDOWN_DAYS} days. Pin the offending dependency to an older, ` + - 'already-available version (update the root dependency and refresh the ' + - 'lockfile), or, if the version has been explicitly allowlisted on the ' + - `proxy, add it to ${path.relative(ROOT, ALLOWLIST_PATH)}.` + 'already-available version (update the root dependency and refresh the lockfile).' ); process.exit(1); } @@ -341,7 +382,12 @@ async function main() { ); } -main().catch((err) => { - console.error(err?.stack || String(err)); - process.exit(1); -}); +const isDirectRun = + Boolean(process.argv[1]) && import.meta.url === pathToFileURL(path.resolve(process.argv[1])).href; + +if (isDirectRun) { + main().catch((err) => { + console.error(err?.stack || String(err)); + process.exit(1); + }); +} diff --git a/scripts/check-deps-proxy-allowlisted.test.js b/scripts/check-deps-proxy-allowlisted.test.js new file mode 100644 index 00000000000..fcf77858869 --- /dev/null +++ b/scripts/check-deps-proxy-allowlisted.test.js @@ -0,0 +1,157 @@ +import { describe, expect, it } from 'vitest'; + +import { + classify, + collectRegistryDeps, + decideAvailability, + fallbackTarballUrl, + nameFromPackageKey, + newlyIntroduced, + tarballStatusFromHttp, + usesTarballGate, +} from './check-deps-proxy-allowlisted.mjs'; + +const DAY = 24 * 60 * 60 * 1000; +const COOLDOWN = 7 * DAY; +const NOW = Date.parse('2026-08-27T12:00:00Z'); + +describe('nameFromPackageKey', () => { + it('reads a nested node_modules key', () => { + expect(nameFromPackageKey('node_modules/next')).toBe('next'); + expect(nameFromPackageKey('node_modules/@next/env')).toBe('@next/env'); + expect(nameFromPackageKey('node_modules/foo/node_modules/@scope/bar')).toBe('@scope/bar'); + }); +}); + +describe('collectRegistryDeps / newlyIntroduced', () => { + const lock = (packages) => ({ packages }); + + it('collects public-registry versions and skips the root and links', () => { + const map = collectRegistryDeps( + lock({ + '': { version: '0.1.0' }, + 'node_modules/next': { + version: '16.3.3', + resolved: 'https://registry.npmjs.org/next/-/next-16.3.3.tgz', + }, + 'node_modules/local': { version: '1.0.0', link: true }, + 'node_modules/other': { version: '1.0.0', resolved: 'https://example.com/other.tgz' }, + }) + ); + expect([...map.keys()]).toEqual(['next']); + expect([...map.get('next')]).toEqual(['16.3.3']); + }); + + it('returns only versions the head lockfile newly introduces', () => { + const base = collectRegistryDeps( + lock({ + 'node_modules/next': { + version: '16.1.6', + resolved: 'https://registry.npmjs.org/next/-/next-16.1.6.tgz', + }, + }) + ); + const head = collectRegistryDeps( + lock({ + 'node_modules/next': { + version: '16.3.3', + resolved: 'https://registry.npmjs.org/next/-/next-16.3.3.tgz', + }, + 'node_modules/left-pad': { + version: '1.3.0', + resolved: 'https://registry.npmjs.org/left-pad/-/left-pad-1.3.0.tgz', + }, + }) + ); + expect(newlyIntroduced(head, base)).toEqual([ + { name: 'next', version: '16.3.3' }, + { name: 'left-pad', version: '1.3.0' }, + ]); + }); +}); + +describe('classify', () => { + it('marks a version inside the cooldown as immature', () => { + const time = { '16.3.3': '2026-08-25T00:00:00Z' }; + expect(classify({ name: 'next', version: '16.3.3' }, time, NOW, COOLDOWN)).toMatchObject({ + status: 'immature', + ageDays: 2.5, + }); + }); + + it('marks a missing publish time as unknown', () => { + expect(classify({ name: 'next', version: '16.3.3' }, {}, NOW, COOLDOWN).status).toBe('unknown'); + }); + + it('marks a version older than the cooldown as ok', () => { + const time = { '16.1.6': '2026-01-01T00:00:00Z' }; + expect(classify({ name: 'next', version: '16.1.6' }, time, NOW, COOLDOWN).status).toBe('ok'); + }); +}); + +describe('fallbackTarballUrl', () => { + const registry = 'https://databricks.jfrog.io/artifactory/api/npm/db-npm/'; + + it('builds unscoped and scoped tarball URLs', () => { + expect(fallbackTarballUrl(registry, 'next', '16.3.3')).toBe( + `${registry}next/-/next-16.3.3.tgz` + ); + expect(fallbackTarballUrl(registry, '@next/env', '16.3.3')).toBe( + `${registry}@next%2Fenv/-/env-16.3.3.tgz` + ); + }); +}); + +describe('tarballStatusFromHttp', () => { + it('treats success and partial-content as available', () => { + expect(tarballStatusFromHttp(200)).toBe('ok'); + expect(tarballStatusFromHttp(206)).toBe('ok'); + }); + + it('treats auth and missing as blocked', () => { + expect(tarballStatusFromHttp(403)).toBe('blocked'); + expect(tarballStatusFromHttp(401)).toBe('blocked'); + expect(tarballStatusFromHttp(404)).toBe('blocked'); + }); +}); + +describe('decideAvailability', () => { + it('on JFrog, a 403 fails even when the packument age would pass or an allowlist would skip', () => { + expect( + decideAvailability({ tarballStatus: 'blocked', ageStatus: 'ok', useTarballGate: true }) + ).toBe('blocked'); + expect( + decideAvailability({ + tarballStatus: 'blocked', + ageStatus: 'immature', + useTarballGate: true, + }) + ).toBe('blocked'); + }); + + it('on JFrog, a served tarball passes even when the packument has no publish time', () => { + expect( + decideAvailability({ tarballStatus: 'ok', ageStatus: 'unknown', useTarballGate: true }) + ).toBe('ok'); + }); + + it('on public npm, age is the gate because new tarballs are served', () => { + expect( + decideAvailability({ + tarballStatus: 'ok', + ageStatus: 'immature', + useTarballGate: false, + }) + ).toBe('immature'); + expect( + decideAvailability({ tarballStatus: 'ok', ageStatus: 'ok', useTarballGate: false }) + ).toBe('ok'); + }); +}); + +describe('usesTarballGate', () => { + it('is true for the Databricks JFrog npm API and false for public npm', () => { + expect(usesTarballGate('https://databricks.jfrog.io/artifactory/api/npm/db-npm/')).toBe(true); + expect(usesTarballGate('https://registry.npmjs.org/')).toBe(false); + }); +}); From ee74e6aa020fb25b8ef19b2176d8520951b7c2b8 Mon Sep 17 00:00:00 2001 From: Andre Landgraf Date: Thu, 27 Aug 2026 10:52:23 -0700 Subject: [PATCH 2/4] Probe JFrog tarballs with a byte-range GET. HEAD is the wrong verb for what Artifactory actually answers; npm ci's 403 is on GET. --- .github/workflows/deps-proxy-allowlist.yml | 2 +- scripts/check-deps-proxy-allowlisted.mjs | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/.github/workflows/deps-proxy-allowlist.yml b/.github/workflows/deps-proxy-allowlist.yml index c7ebe439acd..56889b0f0a9 100644 --- a/.github/workflows/deps-proxy-allowlist.yml +++ b/.github/workflows/deps-proxy-allowlist.yml @@ -69,7 +69,7 @@ jobs: node-version: '22' # No `npm ci` here on purpose: the check only reads registry metadata and - # HEADs tarballs, and a too-new dependency is exactly what would make + # probes tarballs, and a too-new dependency is exactly what would make # `npm ci` fail with a 403. - name: Check dependencies are available on the Databricks proxy env: diff --git a/scripts/check-deps-proxy-allowlisted.mjs b/scripts/check-deps-proxy-allowlisted.mjs index 48eac754e81..99ec314bba8 100644 --- a/scripts/check-deps-proxy-allowlisted.mjs +++ b/scripts/check-deps-proxy-allowlisted.mjs @@ -18,7 +18,7 @@ * * How it decides * -------------- - * On the JFrog mirror (same-repo CI): HEAD the tarball. 403 is a failure even + * On the JFrog mirror (same-repo CI): request one byte of the tarball. 403 is a failure even * if a local JSON allowlist names the version — that file cannot make the * mirror serve a quarantined tarball. * On public npm (fork PRs, which cannot mint JFrog OIDC): fail when the @@ -146,7 +146,7 @@ export function tarballStatusFromHttp(status) { } /** - * Same-repo CI talks to JFrog: the tarball status is whether `npm ci` will + * Same-repo CI talks to JFrog: tarball status is whether `npm ci` will * succeed. Forks only see public npm, which serves new tarballs, so age is * the cooldown signal. */ @@ -244,6 +244,7 @@ async function fetchPackument(registry, headers, name) { } async function probeTarball(url, headers) { + // Range GET returns the same 403 npm ci sees, without downloading the tarball. const res = await fetch(url, { method: 'GET', headers: { ...headers, Range: 'bytes=0-0' }, From 92cd028f8c92eff6eaad7bf906ec688ed3ae7079 Mon Sep 17 00:00:00 2001 From: Andre Landgraf Date: Thu, 27 Aug 2026 10:55:01 -0700 Subject: [PATCH 3/4] Tighten comments on the JFrog proxy check. Keep the why for the dual signal and the byte-range probe; drop restated how. --- .github/workflows/deps-proxy-allowlist.yml | 13 +++++-------- scripts/check-deps-proxy-allowlisted.mjs | 20 ++++++-------------- 2 files changed, 11 insertions(+), 22 deletions(-) diff --git a/.github/workflows/deps-proxy-allowlist.yml b/.github/workflows/deps-proxy-allowlist.yml index 56889b0f0a9..3cc8d4f1f33 100644 --- a/.github/workflows/deps-proxy-allowlist.yml +++ b/.github/workflows/deps-proxy-allowlist.yml @@ -10,10 +10,8 @@ name: Dependencies ↔ Databricks proxy # learns exactly what to do (pin an older version / refresh the lockfile) instead # of chasing a 403 in an unrelated job. See scripts/check-deps-proxy-allowlisted.mjs. # -# No path filter: the check context must post on every PR so it can be a required -# status check without deadlocking content-only PRs. The script is a no-op when -# the PR introduces no new lockfile versions. Forks cannot mint JFrog OIDC, so -# they run on ubuntu-latest and age-check against public npm instead. +# Run on every PR so the required check also reports on content-only changes. +# Forks cannot mint JFrog OIDC, so they age-check against public npm instead. on: pull_request: @@ -45,7 +43,7 @@ jobs: # The protected runner group has no egress to the public npm registry — it # can only reach the Databricks JFrog mirror. Mint a short-lived token via # OIDC and point npm at the mirror so the packument (with publish dates) - # is readable. Forks cannot mint OIDC; they age-check on public npm. + # is readable. - name: Setup JFrog CLI with OIDC if: ${{ !github.event.pull_request.head.repo.fork }} id: setup-jfrog @@ -68,9 +66,8 @@ jobs: with: node-version: '22' - # No `npm ci` here on purpose: the check only reads registry metadata and - # probes tarballs, and a too-new dependency is exactly what would make - # `npm ci` fail with a 403. + # Probe tarballs directly so a quarantined dependency cannot prevent this + # check from naming the package behind `npm ci`'s 403. - name: Check dependencies are available on the Databricks proxy env: BASE_REF: ${{ github.base_ref != '' && format('origin/{0}', github.base_ref) || '' }} diff --git a/scripts/check-deps-proxy-allowlisted.mjs b/scripts/check-deps-proxy-allowlisted.mjs index 99ec314bba8..0edfead9006 100644 --- a/scripts/check-deps-proxy-allowlisted.mjs +++ b/scripts/check-deps-proxy-allowlisted.mjs @@ -16,14 +16,11 @@ * one: it names the offending dependency and tells the contributor to pin an * older version. * - * How it decides - * -------------- - * On the JFrog mirror (same-repo CI): request one byte of the tarball. 403 is a failure even - * if a local JSON allowlist names the version — that file cannot make the - * mirror serve a quarantined tarball. - * On public npm (fork PRs, which cannot mint JFrog OIDC): fail when the - * packument publish time is missing or younger than COOLDOWN_DAYS. Public npm - * serves new tarballs, so age is the only cooldown signal a fork can see. + * Why the signals differ + * ---------------------- + * Same-repo CI can probe JFrog tarballs directly. Forks cannot mint JFrog + * credentials, so they use public npm publish age as the closest available + * cooldown signal. * * Scope: to stay fast and to target the "bumped a dep too aggressively" case, * only versions newly introduced relative to the base branch are checked. When @@ -130,7 +127,6 @@ export function classify({ name, version }, timeMap, now, cooldownMs = COOLDOWN_ }; } -/** npm tarball path: unscoped `foo/-/foo-1.0.0.tgz`, scoped `@scope/foo/-/foo-1.0.0.tgz`. */ export function fallbackTarballUrl(registry, name, version) { const encoded = name.startsWith('@') ? `@${encodeURIComponent(name.slice(1))}` @@ -145,11 +141,7 @@ export function tarballStatusFromHttp(status) { return 'unknown'; } -/** - * Same-repo CI talks to JFrog: tarball status is whether `npm ci` will - * succeed. Forks only see public npm, which serves new tarballs, so age is - * the cooldown signal. - */ +/** Forks use publish age because they cannot access JFrog's tarball gate. */ export function decideAvailability({ tarballStatus, ageStatus, useTarballGate }) { if (useTarballGate) { if (tarballStatus === 'ok') return 'ok'; From 6c9a99b0e859a992d7fd0aae5d87520b9c5e8d06 Mon Sep 17 00:00:00 2001 From: Andre Landgraf Date: Thu, 27 Aug 2026 11:06:22 -0700 Subject: [PATCH 4/4] Cancel tarball probe bodies and drop the skip-list file. 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. --- .github/workflows/deps-proxy-allowlist.yml | 6 +-- scripts/check-deps-proxy-allowlisted.mjs | 22 +++++--- scripts/check-deps-proxy-allowlisted.test.js | 53 ++++++++++++++++++++ scripts/deps-proxy-allowlist.json | 1 - 4 files changed, 70 insertions(+), 12 deletions(-) delete mode 100644 scripts/deps-proxy-allowlist.json diff --git a/.github/workflows/deps-proxy-allowlist.yml b/.github/workflows/deps-proxy-allowlist.yml index 3cc8d4f1f33..2ae9e0df3e6 100644 --- a/.github/workflows/deps-proxy-allowlist.yml +++ b/.github/workflows/deps-proxy-allowlist.yml @@ -5,10 +5,8 @@ name: Dependencies ↔ Databricks proxy # ("immature package" cooldown). A lockfile that pins a too-new version fails # `npm ci` deep in every build with an opaque 403. # -# This gate runs first and fails fast with a clear message naming the offending -# dependency, so a contributor who bumps a package before it clears the cooldown -# learns exactly what to do (pin an older version / refresh the lockfile) instead -# of chasing a 403 in an unrelated job. See scripts/check-deps-proxy-allowlisted.mjs. +# Names the quarantined package here instead of failing later inside an unrelated +# `npm ci`. See scripts/check-deps-proxy-allowlisted.mjs. # # Run on every PR so the required check also reports on content-only changes. # Forks cannot mint JFrog OIDC, so they age-check against public npm instead. diff --git a/scripts/check-deps-proxy-allowlisted.mjs b/scripts/check-deps-proxy-allowlisted.mjs index 0edfead9006..5f5acdb98ee 100644 --- a/scripts/check-deps-proxy-allowlisted.mjs +++ b/scripts/check-deps-proxy-allowlisted.mjs @@ -235,14 +235,22 @@ async function fetchPackument(registry, headers, name) { return res.json(); } -async function probeTarball(url, headers) { - // Range GET returns the same 403 npm ci sees, without downloading the tarball. +export async function probeTarball(url, headers) { + // Range GET returns the same 403 npm ci sees. Cancel the body: some registries + // ignore Range and would otherwise stream the whole tarball until the job hangs. + const controller = new AbortController(); const res = await fetch(url, { method: 'GET', headers: { ...headers, Range: 'bytes=0-0' }, redirect: 'follow', + signal: controller.signal, }); - return tarballStatusFromHttp(res.status); + const status = tarballStatusFromHttp(res.status); + controller.abort(); + if (res.body) { + await res.body.cancel().catch(() => {}); + } + return status; } /** Resolve tasks with bounded concurrency, preserving input order. */ @@ -278,18 +286,18 @@ function loadBaseDepMap() { return null; } -function formatFailure(v) { +export function formatFailure(v, cooldownDays = COOLDOWN_DAYS) { const id = `${v.name}@${v.version}`; if (v.status === 'blocked' && v.ageDays != null) { - return ` ✗ ${id} — proxy returned 403; published ${v.ageDays.toFixed(1)}d ago (< ${COOLDOWN_DAYS}d cooldown).`; + return ` ✗ ${id} — proxy returned 403; published ${v.ageDays.toFixed(1)}d ago (< ${cooldownDays}d cooldown).`; } if (v.status === 'blocked') { return ` ✗ ${id} — tarball is forbidden on the Databricks proxy.`; } if (v.status === 'immature') { - return ` ✗ ${id} — published ${v.ageDays.toFixed(1)}d ago (< ${COOLDOWN_DAYS}d cooldown); blocked by the Databricks proxy.`; + return ` ✗ ${id} — published ${v.ageDays.toFixed(1)}d ago (< ${cooldownDays}d cooldown); too new for the Databricks npm mirror.`; } - return ` ✗ ${id} — cannot confirm it is available on the Databricks proxy.`; + return ` ✗ ${id} — cannot confirm it is available on the Databricks npm mirror.`; } async function main() { diff --git a/scripts/check-deps-proxy-allowlisted.test.js b/scripts/check-deps-proxy-allowlisted.test.js index fcf77858869..d1dc0ed4096 100644 --- a/scripts/check-deps-proxy-allowlisted.test.js +++ b/scripts/check-deps-proxy-allowlisted.test.js @@ -1,3 +1,6 @@ +// @vitest-environment node +import http from 'node:http'; + import { describe, expect, it } from 'vitest'; import { @@ -5,8 +8,10 @@ import { collectRegistryDeps, decideAvailability, fallbackTarballUrl, + formatFailure, nameFromPackageKey, newlyIntroduced, + probeTarball, tarballStatusFromHttp, usesTarballGate, } from './check-deps-proxy-allowlisted.mjs'; @@ -155,3 +160,51 @@ describe('usesTarballGate', () => { expect(usesTarballGate('https://registry.npmjs.org/')).toBe(false); }); }); + +describe('formatFailure', () => { + it('does not claim a fork age check contacted the proxy', () => { + expect( + formatFailure({ name: 'next', version: '16.3.3', status: 'immature', ageDays: 2 }, 7) + ).toBe( + ' ✗ next@16.3.3 — published 2.0d ago (< 7d cooldown); too new for the Databricks npm mirror.' + ); + }); + + it('names a 403 when the tarball probe is the signal', () => { + expect( + formatFailure({ name: 'next', version: '16.3.3', status: 'blocked', ageDays: 2 }, 7) + ).toBe(' ✗ next@16.3.3 — proxy returned 403; published 2.0d ago (< 7d cooldown).'); + }); +}); + +describe('probeTarball', () => { + it('returns after status when the server ignores Range and streams a large body', async () => { + const server = http.createServer((req, res) => { + res.writeHead(200, { + 'Content-Type': 'application/octet-stream', + 'Content-Length': 50 * 1024 * 1024, + }); + const chunk = Buffer.alloc(64 * 1024, 0x78); + const write = () => { + if (!res.writableEnded && res.writable) res.write(chunk); + }; + write(); + const timer = setInterval(write, 20); + const stop = () => clearInterval(timer); + req.on('close', stop); + res.on('close', stop); + }); + await new Promise((resolve) => { + server.listen(0, '127.0.0.1', resolve); + }); + const { port } = server.address(); + const started = Date.now(); + try { + const status = await probeTarball(`http://127.0.0.1:${port}/next-16.3.3.tgz`, {}); + expect(status).toBe('ok'); + expect(Date.now() - started).toBeLessThan(2000); + } finally { + await new Promise((resolve) => server.close(resolve)); + } + }); +}); diff --git a/scripts/deps-proxy-allowlist.json b/scripts/deps-proxy-allowlist.json deleted file mode 100644 index 98d43616355..00000000000 --- a/scripts/deps-proxy-allowlist.json +++ /dev/null @@ -1 +0,0 @@ -["@neon/sdk@2.0.0"]