fix(npm-resolver): minimumReleaseAge handling for cached abbreviated metadata (#11622)

* fix(npm-resolver): dont rethrow ERR_PNPM_MISSING_TIME from version-spec cache * fix(npm-resolver): upgrade cached abbreviated metadata on 304 for minimumReleaseAge * fix(npm-resolver): expand abbreviated-meta upgrade to in-memory cache and preferOffline paths * fix(npm-resolver): address Copilot review feedback on pickPackage - Extract `persistUpgradedMeta` helper and call it from all three sites (in-memory cache hit, preferOffline disk-cache hit, 304 path) so a fresh process doesn't repeat the upgrade fetch. - Forward `etag`/`modified` to the upgrade fetch in `maybeUpgradeAbbreviatedMetaForReleaseAge` so the registry can answer 304. - Extract `shouldRethrowFromFastPathCache` so the two fast-path catch sites can't drift on the MISSING_TIME-vs-strict invariant. - Document the deliberate choice to upgrade-fetch when `meta.modified` is absent or unparseable (correctness over saving a network call). - Add a companion test that exercises the catch fix with the default `ignoreMissingTimeField` so the invariant holds regardless of that flag. - Fix the existing `bareSpecifier: '3.1.0'` test setup: 3.1.0 was published 2016-01-11, after the test's `publishedBy` of 2015-08-17, so strict mode correctly rejected it. Switch to 3.0.0 (released 2015-07-10). * chore(npm-resolver): replace 'unparseable' with 'malformed' for cspell * style(npm-resolver): declare pickPackage helpers after their caller --------- Co-authored-by: Zoltan Kochan <z@kochan.io>
2026-06-01 12:41:16 -04:00 · 2026-05-14 12:49:45 +02:00
parent 180aee9ba5
commit e526f89650
3 changed files with 315 additions and 17 deletions
--- a/.changeset/fix-minimum-release-age-cached-abbreviated-metadata.md
+++ b/.changeset/fix-minimum-release-age-cached-abbreviated-metadata.md
@@ -0,0 +1,12 @@
+---
+"@pnpm/resolving.npm-resolver": patch
+"pnpm": patch
+---
+
+Fix `minimumReleaseAge` handling for cached abbreviated metadata.
+
+The version-spec cache fast path no longer rethrows `ERR_PNPM_MISSING_TIME` under `strictPublishedByCheck`; it now falls through to the registry-fetch path, consistent with the adjacent mtime-gated cache block.
+
+When the registry returns 304 Not Modified for a package whose cached metadata is abbreviated (no per-version `time`), pnpm now re-fetches with `fullMetadata: true` if `minimumReleaseAge` is active and the package was modified after the cutoff. The upgraded metadata is persisted to disk so subsequent installs don't repeat the fetch. Previously the abbreviated meta was used as-is and the maturity check fell back to its warn-and-skip path, silently bypassing the quarantine and emitting a misleading "metadata is missing the time field" warning.
+
+Closes #11619.
--- a/resolving/npm-resolver/src/pickPackage.ts
+++ b/resolving/npm-resolver/src/pickPackage.ts
@@ -206,16 +206,30 @@ export async function pickPackage (
    : ABBREVIATED_META_DIR
  // Cache key includes fullMetadata to avoid returning abbreviated metadata when full metadata is requested.
  const cacheKey = fullMetadata ? `${spec.name}:full` : spec.name
-  const cachedMeta = ctx.metaCache.get(cacheKey)
-  if (cachedMeta != null) {
-    return {
-      meta: cachedMeta,
-      pickedPackage: pickMatchingVersionFinal(pickerOpts, spec, cachedMeta),
-    }
-  }
-
  const registryName = getRegistryName(opts.registry)
  const pkgMirror = path.join(ctx.cacheDir, metaDir, registryName, `${encodePkgName(spec.name)}.jsonl`)
+  const cachedMeta = ctx.metaCache.get(cacheKey)
+  if (cachedMeta != null) {
+    // The in-memory cache may hold abbreviated metadata from an earlier call
+    // that didn't need `time` (no publishedBy then). If this call has
+    // publishedBy and the package was modified recently, upgrade to full
+    // metadata so the maturity check runs properly.
+    const upgrade = await maybeUpgradeAbbreviatedMetaForReleaseAge(ctx, spec, opts, cachedMeta)
+    let metaForCache = upgrade.meta
+    if (upgrade.upgradedFrom != null) {
+      // Persist the upgraded meta to disk too: the on-disk mirror still holds
+      // the abbreviated form, so without this a fresh process would re-trigger
+      // the upgrade fetch on its next install.
+      metaForCache = opts.dryRun
+        ? upgrade.meta
+        : persistUpgradedMeta(ctx, pkgMirror, upgrade.upgradedFrom)
+      ctx.metaCache.set(cacheKey, metaForCache)
+    }
+    return {
+      meta: metaForCache,
+      pickedPackage: pickMatchingVersionFinal(pickerOpts, spec, metaForCache),
+    }
+  }

  return runLimited(pkgMirror, async (limit) => {
    let metaCachedInStore: PackageMeta | null | undefined
@@ -232,6 +246,17 @@ export async function pickPackage (
      }

      if (metaCachedInStore != null) {
+        // Disk-cached meta may be abbreviated; upgrade for the maturity check
+        // before letting pickMatchingVersionFinal warn-and-skip on missing time.
+        const upgrade = await maybeUpgradeAbbreviatedMetaForReleaseAge(ctx, spec, opts, metaCachedInStore)
+        metaCachedInStore = upgrade.meta
+        if (upgrade.upgradedFrom != null) {
+          // Persist so the next install skips this upgrade fetch entirely.
+          if (!opts.dryRun) {
+            metaCachedInStore = persistUpgradedMeta(ctx, pkgMirror, upgrade.upgradedFrom)
+          }
+          ctx.metaCache.set(cacheKey, metaCachedInStore)
+        }
        const pickedPackage = pickMatchingVersionFinal(pickerOpts, spec, metaCachedInStore)
        if (pickedPackage) {
          return {
@@ -255,8 +280,8 @@ export async function pickPackage (
              pickedPackage,
            }
          }
-        } catch (err) {
-          if (ctx.strictPublishedByCheck) {
+        } catch (err: unknown) {
+          if (shouldRethrowFromFastPathCache(err, ctx.strictPublishedByCheck)) {
            throw err
          }
        }
@@ -276,12 +301,7 @@ export async function pickPackage (
              }
            }
          } catch (err: unknown) {
-            // Don't rethrow ERR_PNPM_MISSING_TIME from cached abbreviated metadata —
-            // let the code fall through to the network fetch path which will get full metadata.
-            if (
-              ctx.strictPublishedByCheck &&
-              !(isMissingTimeError(err))
-            ) {
+            if (shouldRethrowFromFastPathCache(err, ctx.strictPublishedByCheck)) {
              throw err
            }
          }
@@ -309,6 +329,21 @@ export async function pickPackage (
      if (fetchResult.notModified) {
        metaCachedInStore = metaCachedInStore ?? await limit(async () => loadMeta(pkgMirror))
        if (metaCachedInStore != null) {
+          // The cached metadata may be abbreviated (no per-version `time`).
+          // When minimumReleaseAge is active we need `time` for the maturity check,
+          // so upgrade to full metadata via a follow-up fetch when warranted.
+          // Without this, repeat installs of recently-modified packages would
+          // silently bypass the maturity check via the warn-and-skip fallback.
+          const upgrade = await maybeUpgradeAbbreviatedMetaForReleaseAge(
+            ctx, spec, opts, metaCachedInStore
+          )
+          metaCachedInStore = upgrade.meta
+          if (upgrade.upgradedFrom != null && !opts.dryRun) {
+            // Persist the upgraded full metadata to disk so subsequent installs
+            // skip this upgrade fetch entirely (the cached meta will then have
+            // `time` populated, so the upgrade condition won't trigger).
+            metaCachedInStore = persistUpgradedMeta(ctx, pkgMirror, upgrade.upgradedFrom)
+          }
          ctx.metaCache.set(cacheKey, metaCachedInStore)
          return {
            meta: metaCachedInStore,
@@ -400,6 +435,98 @@ export async function pickPackage (
  })
 }

+// When `minimumReleaseAge` is active and we have abbreviated metadata (which
+// the npm registry serves by default and which omits per-version `time`),
+// the maturity check can't run on the data we have. If the package has been
+// modified since the maturity cutoff, re-fetch with `fullMetadata: true` so
+// `time` is populated and the check can proceed properly. Without this,
+// `pickMatchingVersionFinal` would fall back to its warn-and-skip path,
+// silently bypassing the minimumReleaseAge guarantee for affected packages.
+//
+// Returns the original meta when no upgrade is needed. When an upgrade
+// happens, returns both the upgraded meta and the underlying fetch result
+// so callers can persist it to disk and avoid re-fetching on next install.
+async function maybeUpgradeAbbreviatedMetaForReleaseAge (
+  ctx: {
+    fetch: (pkgName: string, opts: { registry: string, authHeaderValue?: string, fullMetadata?: boolean, etag?: string, modified?: string }) => Promise<FetchMetadataResult | FetchMetadataNotModifiedResult>
+    offline?: boolean
+  },
+  spec: RegistryPackageSpec,
+  opts: {
+    publishedBy?: Date
+    publishedByExclude?: PickPackageFromMetaOptions['publishedByExclude']
+    authHeaderValue?: string
+    registry: string
+  },
+  meta: PackageMeta
+): Promise<{ meta: PackageMeta, upgradedFrom?: FetchMetadataResult }> {
+  if (
+    ctx.offline === true ||
+    !opts.publishedBy ||
+    meta.time != null ||
+    opts.publishedByExclude?.(spec.name) === true
+  ) {
+    return { meta }
+  }
+  const modifiedDate = meta.modified ? new Date(meta.modified) : null
+  const isModifiedValid = modifiedDate != null && !Number.isNaN(modifiedDate.getTime())
+  if (isModifiedValid && modifiedDate < opts.publishedBy) {
+    // The package was last modified before the maturity cutoff. No individual
+    // version can be newer than the cutoff, so the abbreviated form is fine.
+    return { meta }
+  }
+  // When `modified` is missing or malformed we fall through to the upgrade
+  // fetch: prefer correctness (run the maturity check on real `time` data)
+  // over saving a network call when our cached freshness signal is unusable.
+  // Forward etag/modified so the registry can answer 304 if the upgraded
+  // representation hasn't actually changed (rare on the npm registry where
+  // full and abbreviated have distinct etags, but cheap to support).
+  const fullFetchResult = await ctx.fetch(spec.name, {
+    authHeaderValue: opts.authHeaderValue,
+    fullMetadata: true,
+    etag: meta.etag,
+    modified: meta.modified,
+    registry: opts.registry,
+  })
+  if (fullFetchResult.notModified) {
+    // Upgrade fetch came back 304: keep the abbreviated meta. The downstream
+    // `pickMatchingVersionFinal` will fall through to its warn-and-skip path.
+    return { meta }
+  }
+  return { meta: fullFetchResult.meta, upgradedFrom: fullFetchResult }
+}
+
+// Returns true when a fast-path cache catch should rethrow under
+// strictPublishedByCheck. ERR_PNPM_MISSING_TIME is excluded so callers fall
+// through to the network fetch path, which can upgrade abbreviated cached
+// metadata to full and run the maturity check on real `time` data.
+function shouldRethrowFromFastPathCache (err: unknown, strictPublishedByCheck: boolean | undefined): boolean {
+  return strictPublishedByCheck === true && !isMissingTimeError(err)
+}
+
+// Persists upgraded full metadata to the on-disk cache mirror and returns
+// the meta to store in the in-memory cache. When `filterMetadata` is on, the
+// in-memory and on-disk forms are both stripped via `clearMeta`; otherwise
+// the original raw response body is written and the unstripped meta is kept.
+function persistUpgradedMeta (
+  ctx: { filterMetadata?: boolean },
+  pkgMirror: string,
+  upgradedFrom: FetchMetadataResult
+): PackageMeta {
+  const metaForCache = ctx.filterMetadata ? clearMeta(upgradedFrom.meta) : upgradedFrom.meta
+  const jsonForDisk = ctx.filterMetadata
+    ? prepareJsonForDisk(metaForCache, upgradedFrom.etag)
+    : prepareJsonForDisk(upgradedFrom.meta, upgradedFrom.etag, upgradedFrom.jsonText)
+  runLimited(pkgMirror, (l) => l(async () => {
+    try {
+      await saveMeta(pkgMirror, jsonForDisk)
+    } catch (err: any) { // eslint-disable-line
+      // We don't care if this file was not written to the cache
+    }
+  }))
+  return metaForCache
+}
+
 function clearMeta (pkg: PackageMeta): PackageMeta {
  const versions: PackageMeta['versions'] = {}
  for (const [version, info] of Object.entries(pkg.versions)) {
--- a/resolving/npm-resolver/test/publishedBy.test.ts
+++ b/resolving/npm-resolver/test/publishedBy.test.ts
@@ -10,7 +10,7 @@ import type { Registries } from '@pnpm/types'
 import { loadJsonFileSync } from 'load-json-file'
 import { temporaryDirectory } from 'tempy'

-import { getMockAgent, setupMockAgent, teardownMockAgent } from './utils/index.js'
+import { getMockAgent, retryLoadJsonFile, setupMockAgent, teardownMockAgent } from './utils/index.js'

 const f = fixtures(import.meta.dirname)

@@ -320,6 +320,165 @@ test('ignoreMissingTimeField=true skips maturity check from disk-cached metadata
  expect(resolveResult!.id).toBe('is-positive@3.1.0')
 })

+test('strictPublishedByCheck=true does not rethrow ERR_PNPM_MISSING_TIME from the version-spec cache path', async () => {
+  // Regression test for the bug where the version-spec cache fast path
+  // (`!opts.includeLatestTag && spec.type === 'version'`) in pickPackage
+  // would rethrow ERR_PNPM_MISSING_TIME under strictPublishedByCheck, instead
+  // of falling through to the registry-fetch path like the adjacent mtime-gated
+  // cache block does. The fix makes the two catch blocks consistent.
+  //
+  // Setup: cache abbreviated metadata (no per-version `time` field) for the
+  // package, then request an exact-version pin that IS present in the cached
+  // meta.versions. The version-spec fast path will try pickMatchingVersionFast
+  // against the cached meta, which throws MISSING_TIME because the abbreviated
+  // form lacks `time`. Before the fix this would rethrow. After the fix it
+  // falls through to the registry fetch, which returns full metadata with time,
+  // and resolution succeeds.
+  const cacheDir = temporaryDirectory()
+  const abbrevCacheDir = path.join(cacheDir, `${ABBREVIATED_META_DIR}/registry.npmjs.org`)
+  fs.mkdirSync(abbrevCacheDir, { recursive: true })
+  const cachePath = path.join(abbrevCacheDir, 'is-positive.jsonl')
+  // Strip `time` from the cached abbreviated metadata to simulate the
+  // real-world "registry returned abbreviated form without per-version times" case.
+  // Set `modified` to something recent so the mtime-gated block above doesn't
+  // short-circuit the resolution before we hit the version-spec cache path.
+  const { time: _time, ...abbreviatedWithoutTime } = isPositiveAbbreviatedMeta
+  const cachedMeta = {
+    ...abbreviatedWithoutTime,
+    modified: new Date().toISOString(),
+  }
+  fs.writeFileSync(
+    cachePath,
+    `${JSON.stringify({ modified: cachedMeta.modified })}\n${JSON.stringify(cachedMeta)}`,
+    'utf8'
+  )
+
+  // Mock the network fetch (the path the fix lets us fall through to). Returns
+  // full metadata with the `time` field present.
+  getMockAgent().get(registries.default.replace(/\/$/, ''))
+    .intercept({ path: '/is-positive', method: 'GET' })
+    .reply(200, isPositiveMeta)
+
+  const { resolveFromNpm } = createResolveFromNpm({
+    storeDir: temporaryDirectory(),
+    cacheDir,
+    registries,
+    strictPublishedByCheck: true,
+    ignoreMissingTimeField: true,
+  })
+
+  // Exact-version bareSpecifier → spec.type === 'version' → hits the cache path
+  // we are testing. 3.0.0 was published 2015-07-10, mature relative to publishedBy.
+  const resolveResult = await resolveFromNpm({ alias: 'is-positive', bareSpecifier: '3.0.0' }, {
+    publishedBy: new Date('2015-08-17T19:26:00.508Z'),
+  })
+
+  expect(resolveResult!.resolvedVia).toBe('npm-registry')
+  expect(resolveResult!.id).toBe('is-positive@3.0.0')
+})
+
+test('strictPublishedByCheck=true with default ignoreMissingTimeField does not rethrow ERR_PNPM_MISSING_TIME from the version-spec cache path', async () => {
+  // Companion to the test above: same scenario but with the default
+  // ignoreMissingTimeField (false). The catch-block fix must hold regardless
+  // of the ignore flag — MISSING_TIME from cached abbreviated meta should
+  // never escape the catch under strict mode, so resolution falls through to
+  // the registry fetch and succeeds with full (time-bearing) metadata.
+  const cacheDir = temporaryDirectory()
+  const abbrevCacheDir = path.join(cacheDir, `${ABBREVIATED_META_DIR}/registry.npmjs.org`)
+  fs.mkdirSync(abbrevCacheDir, { recursive: true })
+  const cachePath = path.join(abbrevCacheDir, 'is-positive.jsonl')
+  const { time: _time, ...abbreviatedWithoutTime } = isPositiveAbbreviatedMeta
+  const cachedMeta = {
+    ...abbreviatedWithoutTime,
+    modified: new Date().toISOString(),
+  }
+  fs.writeFileSync(
+    cachePath,
+    `${JSON.stringify({ modified: cachedMeta.modified })}\n${JSON.stringify(cachedMeta)}`,
+    'utf8'
+  )
+
+  getMockAgent().get(registries.default.replace(/\/$/, ''))
+    .intercept({ path: '/is-positive', method: 'GET' })
+    .reply(200, isPositiveMeta)
+
+  const { resolveFromNpm } = createResolveFromNpm({
+    storeDir: temporaryDirectory(),
+    cacheDir,
+    registries,
+    strictPublishedByCheck: true,
+  })
+
+  const resolveResult = await resolveFromNpm({ alias: 'is-positive', bareSpecifier: '3.0.0' }, {
+    publishedBy: new Date('2015-08-17T19:26:00.508Z'),
+  })
+
+  expect(resolveResult!.resolvedVia).toBe('npm-registry')
+  expect(resolveResult!.id).toBe('is-positive@3.0.0')
+})
+
+test('upgrades cached abbreviated metadata to full when 304 Not Modified and publishedBy is set', async () => {
+  // Regression test for the misleading "missing the time field" warning.
+  //
+  // When pnpm has abbreviated metadata in its disk cache and the registry
+  // returns 304 Not Modified on the conditional fetch, pnpm reuses the
+  // cached abbreviated metadata. Abbreviated metadata has no per-version
+  // `time` field by registry spec, so the minimumReleaseAge check would
+  // fall back to its warn-and-skip path — silently bypassing the maturity
+  // guarantee even though the registry HAS the time data in full metadata.
+  //
+  // The fix: after a 304, if the cached metadata is abbreviated and the
+  // package was recently modified, re-fetch with `fullMetadata: true` to
+  // get per-version times and run the check properly.
+  const cacheDir = temporaryDirectory()
+  const abbrevCacheDir = path.join(cacheDir, `${ABBREVIATED_META_DIR}/registry.npmjs.org`)
+  fs.mkdirSync(abbrevCacheDir, { recursive: true })
+  const cachePath = path.join(abbrevCacheDir, 'is-positive.jsonl')
+
+  // Cache abbreviated metadata: no `time`, recent `modified` (so the upgrade
+  // condition triggers), with an etag so the conditional fetch returns 304.
+  const { time: _time, ...abbreviatedWithoutTime } = isPositiveAbbreviatedMeta
+  const cachedMeta = {
+    ...abbreviatedWithoutTime,
+    modified: '2015-06-10T00:00:00.000Z',
+  }
+  const cacheHeaders = JSON.stringify({ etag: '"abc123"', modified: cachedMeta.modified })
+  fs.writeFileSync(cachePath, `${cacheHeaders}\n${JSON.stringify(cachedMeta)}`, 'utf8')
+
+  // First fetch: conditional request with the cached etag → 304 Not Modified.
+  // Second fetch: upgrade request with `fullMetadata: true` → 200 with time-bearing full metadata.
+  const agent = getMockAgent().get(registries.default.replace(/\/$/, ''))
+  agent.intercept({
+    path: '/is-positive',
+    method: 'GET',
+    headers: { 'if-none-match': '"abc123"' },
+  }).reply(304, '')
+  agent.intercept({ path: '/is-positive', method: 'GET' })
+    .reply(200, isPositiveMeta)
+
+  const { resolveFromNpm } = createResolveFromNpm({
+    storeDir: temporaryDirectory(),
+    cacheDir,
+    registries,
+  })
+  // publishedBy is 2015-06-05, cached meta `modified` is 2015-06-10 → upgrade triggers.
+  // is-positive@1.0.0 (2015-06-02) is mature; @3.1.0 (2015-08-21) is not.
+  const resolveResult = await resolveFromNpm({ alias: 'is-positive', bareSpecifier: '^1.0.0' }, {
+    publishedBy: new Date('2015-06-05T00:00:00.000Z'),
+  })
+
+  expect(resolveResult!.resolvedVia).toBe('npm-registry')
+  // The maturity check ran properly thanks to the upgrade — picked 1.0.0, not 3.x.
+  expect(resolveResult!.id).toBe('is-positive@1.0.0')
+
+  // The upgraded full metadata should be persisted to disk so the next
+  // install doesn't re-trigger the upgrade fetch.
+  /* eslint-disable @typescript-eslint/no-explicit-any */
+  const persistedMeta = await retryLoadJsonFile<any>(cachePath)
+  /* eslint-enable @typescript-eslint/no-explicit-any */
+  expect(persistedMeta?.time).toBeDefined()
+})
+
 test('use cached metadata based on file mtime when publishedBy is set', async () => {
  const cacheDir = temporaryDirectory()
  // Write abbreviated metadata to the abbreviated cache dir