mirror of
https://github.com/pnpm/pnpm.git
synced 2026-06-27 17:35:30 -04:00
fix(npm-resolver): surface registry fetch errors in tarball URL check (#12519)
Lockfile verification fetches registry metadata to bind each entry's tarball URL (and to apply the minimumReleaseAge / trust-downgrade policies). On a fetch failure the abbreviated-metadata fetch collapsed every failure mode — 403, 401, network error, 5xx — into the same "missing" value as a version genuinely absent from the metadata, so a transport failure was reported as a tampering-style ERR_PNPM_TARBALL_URL_MISMATCH. Rather than mint a dedicated violation code/message for the transport case, the verifier now propagates the registry's own fetch error and the install aborts with it: - runTarballUrlCheck / runAgeCheck / runTrustCheck rethrow the underlying fetch error rather than folding it into a policy violation. The abbreviated-metadata age shortcut still swallows the error and falls back to per-version lookups. - The verification gate captures a thrown error per entry and rethrows the first after the fan-out settles, so concurrent siblings hitting the same dead registry do not raise unhandled rejections. A transport failure takes precedence over collected policy violations: the run never finished, so the batch is incomplete and the actionable failure is the transport error; a re-run surfaces any remaining violations once the registry is reachable. - Credential leak fixed at the source: `@pnpm/error`'s redactUrlCredentials strips user:pass@ userinfo from a URL in FetchError messages, and the npm resolver redacts the message, stack, and cause of META_FETCH_FAIL. It is a single forward scan, not a regex (the input is uncontrolled, so a backtracking pattern is a ReDoS vector), and it strips up to the last @ in the authority so a raw @ inside the password cannot leak its tail. pacquet parity: ResolutionVerification::FetchFailed is added (the runner aborts with it rather than collecting it as a violation); the gate aborts via VerifyError::RegistryMetaFetchFailed (ERR_PNPM_META_FETCH_FAIL), collect_resolution_policy_violations returns a Result, and the pnpr server maps the abort to a 502. The surfaced fetch error is credential-redacted. The TARBALL_URL_FETCH_FAILED code, its VerifyError variant and hint, and the pnpr-client interning are removed. Note: pacquet surfaces its existing fetch error under ERR_PNPM_META_FETCH_FAIL rather than pnpm's literal ERR_PNPM_FETCH_403 — pacquet's fetch errors do not use the per-status code scheme today, so aligning that globally is a separate effort. The behavior matches: abort with the registry's error, never a tampering label, no credential leak. Fixes pnpm/pnpm#12489.
This commit is contained in:
kyungseopk1m
8
.changeset/tarball-verifier-surface-fetch-error.md
Normal file
8
.changeset/tarball-verifier-surface-fetch-error.md
Normal file
@@ -0,0 +1,8 @@
|
||||
---
|
||||
"pnpm": patch
|
||||
"@pnpm/resolving.npm-resolver": patch
|
||||
"@pnpm/installing.deps-installer": patch
|
||||
"@pnpm/error": patch
|
||||
---
|
||||
|
||||
Lockfile verification no longer reports a registry metadata fetch failure (for example a `403`/`401` on a private registry, or a network error) as `ERR_PNPM_TARBALL_URL_MISMATCH`. When the registry can't be reached to verify an entry, the install now aborts with the registry's own fetch error (such as `ERR_PNPM_FETCH_403`, which already explains the authentication situation) instead of mislabeling a transport failure as lockfile tampering. Registry fetch errors no longer leak basic-auth credentials embedded in the registry URL (`https://user:pass@host/`) into their message.
|
||||
@@ -57,6 +57,18 @@ pub enum VerifyError {
|
||||
breakdown: String,
|
||||
},
|
||||
|
||||
/// The registry couldn't be reached to verify an entry
|
||||
/// (auth/network/5xx). Surfaces the registry's own fetch error — which
|
||||
/// already explains the auth situation — rather than a tampering-style
|
||||
/// mismatch or a lockfile-policy batch. The message is credential-redacted
|
||||
/// at the verifier before it reaches here.
|
||||
#[display("{message}")]
|
||||
#[diagnostic(code(ERR_PNPM_META_FETCH_FAIL))]
|
||||
RegistryMetaFetchFailed {
|
||||
#[error(not(source))]
|
||||
message: String,
|
||||
},
|
||||
|
||||
#[display("{count} lockfile entries failed verification:\n{breakdown}")]
|
||||
#[diagnostic(code(ERR_PNPM_RESOLUTION_SHAPE_MISMATCH), help("{HINT}"))]
|
||||
ResolutionShapeMismatch {
|
||||
|
||||
@@ -170,7 +170,13 @@ pub async fn verify_lockfile_resolutions<Reporter: self::Reporter>(
|
||||
let mut emit_guard =
|
||||
TerminalEmitGuard::<Reporter>::failed(entries, started_at, lockfile_path_str.clone());
|
||||
|
||||
let violations = run_fan_out(candidates, verifiers, opts.concurrency).await;
|
||||
let violations = match run_fan_out(candidates, verifiers, opts.concurrency).await {
|
||||
Ok(violations) => violations,
|
||||
// The registry couldn't be reached to verify an entry: abort with its
|
||||
// own error (already credential-redacted) instead of a policy batch.
|
||||
// `emit_guard` is still armed to emit `failed` on drop.
|
||||
Err(message) => return Err(VerifyError::RegistryMetaFetchFailed { message }),
|
||||
};
|
||||
if violations.is_empty() {
|
||||
emit_guard.cancel(LockfileVerificationMessage::Done {
|
||||
entries,
|
||||
@@ -200,14 +206,16 @@ pub async fn collect_resolution_policy_violations(
|
||||
lockfile: &Lockfile,
|
||||
verifiers: &[Arc<dyn ResolutionVerifier>],
|
||||
concurrency: Option<usize>,
|
||||
) -> Vec<ResolutionPolicyViolation> {
|
||||
) -> Result<Vec<ResolutionPolicyViolation>, String> {
|
||||
if verifiers.is_empty() || lockfile.packages.is_none() {
|
||||
return Vec::new();
|
||||
return Ok(Vec::new());
|
||||
}
|
||||
// Shape violations and invalid aliases are deliberately not
|
||||
// collected here: they are hard tampering failures, not policy
|
||||
// picks a caller may auto-exclude.
|
||||
let (candidates, _shape_violations, _invalid_aliases) = collect_candidates(lockfile);
|
||||
// `Err(message)` is a transport failure the caller must surface rather than
|
||||
// treat as "no violations" — see [`run_fan_out`].
|
||||
run_fan_out(candidates, verifiers, concurrency).await
|
||||
}
|
||||
|
||||
@@ -424,7 +432,7 @@ async fn run_fan_out(
|
||||
candidates: Vec<Candidate>,
|
||||
verifiers: &[Arc<dyn ResolutionVerifier>],
|
||||
concurrency: Option<usize>,
|
||||
) -> Vec<ResolutionPolicyViolation> {
|
||||
) -> Result<Vec<ResolutionPolicyViolation>, String> {
|
||||
let limit = concurrency.unwrap_or(DEFAULT_CONCURRENCY).max(1);
|
||||
let semaphore = Arc::new(Semaphore::new(limit));
|
||||
let mut futures = FuturesUnordered::new();
|
||||
@@ -450,35 +458,57 @@ async fn run_fan_out(
|
||||
evaluate_candidate(candidate, &verifiers).await
|
||||
});
|
||||
}
|
||||
// A transport failure (the registry couldn't be reached to verify an
|
||||
// entry) aborts the whole pass with the registry's own error rather than
|
||||
// collecting it as a policy violation. Drain the rest of the fan-out so no
|
||||
// in-flight task is dropped mid-await, but keep only the first abort.
|
||||
let mut violations = Vec::new();
|
||||
let mut fetch_error: Option<String> = None;
|
||||
while let Some(result) = futures.next().await {
|
||||
if let Some(violation) = result {
|
||||
violations.push(violation);
|
||||
match result {
|
||||
Ok(Some(violation)) => violations.push(violation),
|
||||
Ok(None) => {}
|
||||
Err(message) => {
|
||||
if fetch_error.is_none() {
|
||||
fetch_error = Some(message);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
violations
|
||||
// A registry that couldn't be reached takes precedence over collected
|
||||
// violations: the pass never finished, so the batch is incomplete and the
|
||||
// actionable failure is the transport error.
|
||||
match fetch_error {
|
||||
Some(message) => Err(message),
|
||||
None => Ok(violations),
|
||||
}
|
||||
}
|
||||
|
||||
/// Outcome of evaluating one candidate against the active verifiers.
|
||||
/// `Err(message)` is a transport failure (the registry couldn't be
|
||||
/// reached to verify the entry) — the runner aborts the whole pass with
|
||||
/// it rather than collecting it as a policy violation.
|
||||
async fn evaluate_candidate(
|
||||
candidate: Candidate,
|
||||
verifiers: &[Arc<dyn ResolutionVerifier>],
|
||||
) -> Option<ResolutionPolicyViolation> {
|
||||
) -> Result<Option<ResolutionPolicyViolation>, String> {
|
||||
for verifier in verifiers {
|
||||
let ctx = VerifyCtx { name: &candidate.name, version: &candidate.version };
|
||||
match verifier.verify(&candidate.resolution, ctx).await {
|
||||
ResolutionVerification::Ok => continue,
|
||||
ResolutionVerification::Err { code, reason } => {
|
||||
return Some(ResolutionPolicyViolation {
|
||||
return Ok(Some(ResolutionPolicyViolation {
|
||||
name: candidate.name,
|
||||
version: candidate.version,
|
||||
resolution: candidate.resolution,
|
||||
code,
|
||||
reason,
|
||||
});
|
||||
}));
|
||||
}
|
||||
ResolutionVerification::FetchFailed { message } => return Err(message),
|
||||
}
|
||||
}
|
||||
None
|
||||
Ok(None)
|
||||
}
|
||||
|
||||
/// Sort violations by `name@version` and build the matching
|
||||
|
||||
@@ -145,6 +145,57 @@ impl ResolutionVerifier for FailFor {
|
||||
}
|
||||
}
|
||||
|
||||
/// Abort every candidate with a transport failure (the registry couldn't be
|
||||
/// reached to verify it).
|
||||
struct FetchFails {
|
||||
message: &'static str,
|
||||
policy: serde_json::Map<String, serde_json::Value>,
|
||||
}
|
||||
|
||||
impl FetchFails {
|
||||
fn new(message: &'static str) -> Arc<Self> {
|
||||
Arc::new(Self { message, policy: serde_json::Map::new() })
|
||||
}
|
||||
}
|
||||
|
||||
impl ResolutionVerifier for FetchFails {
|
||||
fn verify<'a>(
|
||||
&'a self,
|
||||
_resolution: &'a LockfileResolution,
|
||||
_ctx: VerifyCtx<'a>,
|
||||
) -> VerifyFuture<'a> {
|
||||
let message = self.message.to_string();
|
||||
Box::pin(async move { ResolutionVerification::FetchFailed { message } })
|
||||
}
|
||||
|
||||
fn policy(&self) -> &serde_json::Map<String, serde_json::Value> {
|
||||
&self.policy
|
||||
}
|
||||
|
||||
fn can_trust_past_check(&self, _cached: &serde_json::Map<String, serde_json::Value>) -> bool {
|
||||
true
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn a_fetch_failure_aborts_with_the_registry_error() {
|
||||
let lockfile = parse(SINGLE_PKG_LOCKFILE);
|
||||
let verifier =
|
||||
FetchFails::new("Failed to fetch metadata from https://registry.example/acme: 403");
|
||||
let err = verify_lockfile_resolutions::<SilentReporter>(
|
||||
&lockfile,
|
||||
&[verifier as Arc<dyn ResolutionVerifier>],
|
||||
&VerifyLockfileResolutionsOptions::default(),
|
||||
)
|
||||
.await
|
||||
.expect_err("a transport failure must abort verification");
|
||||
// It surfaces the registry's own error, not a lockfile-policy batch.
|
||||
let VerifyError::RegistryMetaFetchFailed { message } = err else {
|
||||
panic!("expected RegistryMetaFetchFailed, got: {err:?}");
|
||||
};
|
||||
assert!(message.contains("403"), "message: {message}");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn no_verifiers_is_a_noop() {
|
||||
static EVENTS: Mutex<Vec<LogEvent>> = Mutex::new(Vec::new());
|
||||
@@ -280,7 +331,8 @@ async fn per_candidate_fan_out_stops_at_first_failure() {
|
||||
&[first as Arc<dyn ResolutionVerifier>, second as Arc<dyn ResolutionVerifier>],
|
||||
None,
|
||||
)
|
||||
.await;
|
||||
.await
|
||||
.expect("no fetch failure");
|
||||
assert_eq!(violations.len(), 1, "stop at first failing verifier");
|
||||
assert_eq!(violations[0].code, "MINIMUM_RELEASE_AGE_VIOLATION");
|
||||
}
|
||||
@@ -294,7 +346,8 @@ async fn collect_returns_data_for_all_violations() {
|
||||
&[verifier as Arc<dyn ResolutionVerifier>],
|
||||
None,
|
||||
)
|
||||
.await;
|
||||
.await
|
||||
.expect("no fetch failure");
|
||||
assert_eq!(violations.len(), 2);
|
||||
}
|
||||
|
||||
|
||||
@@ -449,7 +449,7 @@ impl NpmResolutionVerifier {
|
||||
lockfile_tarball: &str,
|
||||
) -> Option<ResolutionVerification> {
|
||||
let registry_tarball = match self.fetch_abbreviated_meta(registry, name).await {
|
||||
Ok(Some(meta)) => {
|
||||
Ok(meta) => {
|
||||
if let Some(sink) = self.observed_dist_stats.as_ref()
|
||||
&& let Some(stats) =
|
||||
meta.version_dist_stats.as_ref().and_then(|stats| stats.get(version))
|
||||
@@ -458,7 +458,15 @@ impl NpmResolutionVerifier {
|
||||
}
|
||||
meta.version_tarballs.and_then(|tarballs| tarballs.get(version).cloned())
|
||||
}
|
||||
Ok(None) | Err(_) => None,
|
||||
Err(message) => {
|
||||
// Couldn't reach the registry to verify (auth/network/5xx).
|
||||
// Propagate the registry's own fetch error (already
|
||||
// credential-redacted) so the install aborts with it rather
|
||||
// than mislabeling a transport failure as a tampering-style URL
|
||||
// mismatch. Still fail-closed: the entry never reaches the
|
||||
// filesystem because the install never proceeds.
|
||||
return Some(ResolutionVerification::FetchFailed { message });
|
||||
}
|
||||
};
|
||||
match registry_tarball {
|
||||
Some(url) if same_tarball_url(lockfile_tarball, &url) => None,
|
||||
@@ -485,12 +493,10 @@ impl NpmResolutionVerifier {
|
||||
let cutoff = self.cutoff.expect("cutoff is Some when age check is active");
|
||||
let published = match self.fetch_published_at(registry, name, version).await {
|
||||
Ok(value) => value,
|
||||
Err(reason) => {
|
||||
return Some(ResolutionVerification::Err {
|
||||
code: MINIMUM_RELEASE_AGE_VIOLATION_CODE,
|
||||
reason: uncheckable("minimumReleaseAge", &reason),
|
||||
});
|
||||
}
|
||||
// A transport failure propagates the registry's own fetch error so
|
||||
// the install aborts with it; a successful fetch that merely lacks a
|
||||
// timestamp is handled below.
|
||||
Err(message) => return Some(ResolutionVerification::FetchFailed { message }),
|
||||
};
|
||||
let Some(published) = published else {
|
||||
// No source surfaced a publish timestamp; mirror the
|
||||
@@ -535,12 +541,10 @@ impl NpmResolutionVerifier {
|
||||
) -> Option<ResolutionVerification> {
|
||||
let meta = match self.fetch_full_meta_for_trust(registry, name).await {
|
||||
Ok(meta) => meta,
|
||||
Err(reason) => {
|
||||
return Some(ResolutionVerification::Err {
|
||||
code: TRUST_DOWNGRADE_VIOLATION_CODE,
|
||||
reason: uncheckable("trustPolicy", &reason),
|
||||
});
|
||||
}
|
||||
// A transport failure propagates the registry's own fetch error so
|
||||
// the install aborts with it rather than folding it into a policy
|
||||
// violation.
|
||||
Err(message) => return Some(ResolutionVerification::FetchFailed { message }),
|
||||
};
|
||||
let trust_opts = TrustCheckOptions {
|
||||
trust_policy_exclude: self.trust_policy_exclude.as_ref(),
|
||||
@@ -600,8 +604,7 @@ impl NpmResolutionVerifier {
|
||||
name: &PkgName,
|
||||
version: &str,
|
||||
) -> Result<Option<String>, String> {
|
||||
if let Some(value) = self.try_abbreviated_modified_shortcut(registry, name, version).await?
|
||||
{
|
||||
if let Some(value) = self.try_abbreviated_modified_shortcut(registry, name, version).await {
|
||||
return Ok(Some(value));
|
||||
}
|
||||
if let Some(map) = self.read_local_meta_time(registry, name).await
|
||||
@@ -632,20 +635,23 @@ impl NpmResolutionVerifier {
|
||||
registry: &str,
|
||||
name: &PkgName,
|
||||
version: &str,
|
||||
) -> Result<Option<String>, String> {
|
||||
) -> Option<String> {
|
||||
let cutoff = self.cutoff.expect("cutoff is Some when age check is active");
|
||||
let Some(meta) = self.fetch_abbreviated_meta(registry, name).await? else {
|
||||
return Ok(None);
|
||||
// A fetch failure here is fine: ignore the error and fall back to
|
||||
// per-version lookups, the same as a successful-but-uninformative
|
||||
// metadata response.
|
||||
let Ok(meta) = self.fetch_abbreviated_meta(registry, name).await else {
|
||||
return None;
|
||||
};
|
||||
let Some(modified) = meta.modified else { return Ok(None) };
|
||||
let Some(parsed) = parse_packument_timestamp(&modified) else { return Ok(None) };
|
||||
let modified = meta.modified?;
|
||||
let parsed = parse_packument_timestamp(&modified)?;
|
||||
if parsed >= cutoff {
|
||||
return Ok(None);
|
||||
return None;
|
||||
}
|
||||
if !meta.version_tarballs.as_ref().is_some_and(|map| map.contains_key(version)) {
|
||||
return Ok(None);
|
||||
return None;
|
||||
}
|
||||
Ok(Some(modified))
|
||||
Some(modified)
|
||||
}
|
||||
|
||||
/// Per-`(registry, name)` abbreviated-meta lookup. The result is
|
||||
@@ -662,9 +668,11 @@ impl NpmResolutionVerifier {
|
||||
/// 2. The on-disk + network cached fetcher
|
||||
/// ([`fetch_full_metadata_cached()`] with `full_metadata: false`)
|
||||
/// when no shared entry is available.
|
||||
/// 3. A failure (decode / network / cache-write IO) caches
|
||||
/// `None` so subsequent calls fall through to the next layer
|
||||
/// of [`Self::resolve_published_at`] without retrying.
|
||||
/// 3. A failure (decode / network / cache-write IO) caches a
|
||||
/// credential-safe `Err(reason)` so subsequent calls reuse the
|
||||
/// same verdict without retrying. The tarball-URL check surfaces
|
||||
/// this error; the age shortcut ignores it and falls through to
|
||||
/// the next layer of [`Self::resolve_published_at`].
|
||||
///
|
||||
/// Mirrors upstream's
|
||||
/// [`fetchAbbreviatedMeta`](https://github.com/pnpm/pnpm/blob/2a9bd897bf/resolving/npm-resolver/src/createNpmResolutionVerifier.ts#L626-L653).
|
||||
@@ -672,7 +680,7 @@ impl NpmResolutionVerifier {
|
||||
&self,
|
||||
registry: &str,
|
||||
name: &PkgName,
|
||||
) -> Result<Option<crate::lookup_context::AbbreviatedMetaProjection>, String> {
|
||||
) -> Result<crate::lookup_context::AbbreviatedMetaProjection, String> {
|
||||
let key = package_key(registry, &name.to_string());
|
||||
let cell = {
|
||||
let mut cache = self.lookup_context.abbreviated_meta.lock().await;
|
||||
@@ -681,7 +689,7 @@ impl NpmResolutionVerifier {
|
||||
let value = cell
|
||||
.get_or_init(|| async {
|
||||
if let Some(shared) = self.read_shared_meta(name) {
|
||||
return Some(project_abbreviated_meta(&shared));
|
||||
return Ok(project_abbreviated_meta(&shared));
|
||||
}
|
||||
let opts = FetchFullMetadataCachedOptions {
|
||||
registry,
|
||||
@@ -692,13 +700,18 @@ impl NpmResolutionVerifier {
|
||||
filter_metadata: false,
|
||||
retry_opts: self.retry_opts,
|
||||
};
|
||||
// Carry a fetch failure (auth/network/5xx) as the `Err` value
|
||||
// instead of collapsing it to a missing projection: the
|
||||
// tarball-URL check needs to tell a transport failure apart
|
||||
// from a version genuinely absent from the metadata, otherwise
|
||||
// it reports a 403 as a tampering-style mismatch.
|
||||
match fetch_full_metadata_cached(&name.to_string(), &opts).await {
|
||||
Ok(meta) => Some(project_abbreviated_meta(&meta)),
|
||||
Err(_) => None,
|
||||
Ok(meta) => Ok(project_abbreviated_meta(&meta)),
|
||||
Err(error) => Err(redact_url_credentials(&error.to_string())),
|
||||
}
|
||||
})
|
||||
.await;
|
||||
Ok(value.clone())
|
||||
value.clone()
|
||||
}
|
||||
|
||||
/// Try the resolver's shared [`PackageMetaCache`] for a packument
|
||||
@@ -771,7 +784,7 @@ impl NpmResolutionVerifier {
|
||||
};
|
||||
fetch_attestation_published_at(&name.to_string(), version, &opts)
|
||||
.await
|
||||
.map_err(|err| err.to_string())
|
||||
.map_err(|err| redact_url_credentials(&err.to_string()))
|
||||
}
|
||||
|
||||
async fn fetch_full_meta_time(
|
||||
@@ -852,7 +865,9 @@ impl NpmResolutionVerifier {
|
||||
filter_metadata: false,
|
||||
retry_opts: self.retry_opts,
|
||||
};
|
||||
fetch_full_metadata_cached(&name.to_string(), &opts).await.map_err(|err| err.to_string())
|
||||
fetch_full_metadata_cached(&name.to_string(), &opts)
|
||||
.await
|
||||
.map_err(|err| redact_url_credentials(&err.to_string()))
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1075,6 +1090,44 @@ fn project_abbreviated_meta(meta: &Package) -> crate::lookup_context::Abbreviate
|
||||
}
|
||||
}
|
||||
|
||||
/// Strip `user:pass@` (or `user@`) that appears right after a URL scheme in
|
||||
/// any message text, e.g. `… https://user:pass@host/pkg …` →
|
||||
/// `… https://host/pkg …`. A registry configured as `https://user:pass@host/`
|
||||
/// would otherwise leak its embedded basic-auth into the fetch error the
|
||||
/// verifier surfaces when it aborts on a transport failure.
|
||||
fn redact_url_credentials(text: &str) -> String {
|
||||
let mut out = String::with_capacity(text.len());
|
||||
let mut rest = text;
|
||||
while let Some(pos) = rest.find("://") {
|
||||
let (before, after) = rest.split_at(pos + "://".len());
|
||||
out.push_str(before);
|
||||
// Only treat "://" as a URL authority boundary when a scheme character
|
||||
// (schemes end in an ASCII alphanumeric) precedes it, so an unrelated
|
||||
// "://" in the message isn't mangled.
|
||||
let has_scheme = pos > 0 && rest.as_bytes()[pos - 1].is_ascii_alphanumeric();
|
||||
rest = strip_leading_userinfo(after).filter(|_| has_scheme).unwrap_or(after);
|
||||
}
|
||||
out.push_str(rest);
|
||||
out
|
||||
}
|
||||
|
||||
/// If the authority leading `text` contains `userinfo@`, return the slice after
|
||||
/// the **last** `@` within it; otherwise `None`. The authority ends at the first
|
||||
/// `/`, `?`, `#`, or whitespace. Stripping to the last `@` keeps a raw `@` inside
|
||||
/// the password (`user:p@ss@host`) from leaking its tail.
|
||||
fn strip_leading_userinfo(authority: &str) -> Option<&str> {
|
||||
let mut last_at = None;
|
||||
for (idx, ch) in authority.char_indices() {
|
||||
match ch {
|
||||
'@' => last_at = Some(idx + ch.len_utf8()),
|
||||
'/' | '?' | '#' => break,
|
||||
c if c.is_whitespace() => break,
|
||||
_ => {}
|
||||
}
|
||||
}
|
||||
last_at.map(|end| &authority[end..])
|
||||
}
|
||||
|
||||
fn same_tarball_url(left: &str, right: &str) -> bool {
|
||||
canonical_tarball_url(left) == canonical_tarball_url(right)
|
||||
}
|
||||
|
||||
@@ -10,6 +10,7 @@ use ssri::Integrity;
|
||||
|
||||
use super::{
|
||||
CreateNpmResolutionVerifierOptions, create_npm_resolution_verifier, observed_dist_stats_sink,
|
||||
redact_url_credentials,
|
||||
};
|
||||
|
||||
const FAKE_INTEGRITY: &str = "sha512-AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA==";
|
||||
@@ -1073,3 +1074,119 @@ async fn binding_check_records_dist_stats_into_the_sink() {
|
||||
assert_eq!(recorded.unpacked_size, Some(123_456));
|
||||
assert_eq!(recorded.file_count, Some(42));
|
||||
}
|
||||
|
||||
/// A 403 on the metadata fetch (e.g. a CI token that is authenticated but not
|
||||
/// authorized to read a private package) must not be reported as a lockfile
|
||||
/// tarball-URL mismatch: the lockfile is correct, the fetch is the problem. The
|
||||
/// verifier propagates the registry's own fetch error so the install aborts.
|
||||
#[tokio::test]
|
||||
async fn propagates_metadata_fetch_failure_instead_of_a_tampering_mismatch() {
|
||||
let mut server = mockito::Server::new_async().await;
|
||||
let registry = format!("{}/", server.url());
|
||||
let server_url = server.url();
|
||||
let _meta_mock = server
|
||||
.mock("GET", "/private-pkg")
|
||||
.with_status(403)
|
||||
.with_body(r#"{"error":"Forbidden"}"#)
|
||||
.create_async()
|
||||
.await;
|
||||
|
||||
let opts = default_opts(®istry);
|
||||
let verifier = create_npm_resolution_verifier(opts);
|
||||
let resolution = LockfileResolution::Tarball(TarballResolution {
|
||||
tarball: format!("{server_url}/private-pkg/-/private-pkg-1.0.0.tgz"),
|
||||
integrity: Some(fake_integrity()),
|
||||
git_hosted: None,
|
||||
path: None,
|
||||
});
|
||||
let name: PkgName = "private-pkg".parse().expect("parse");
|
||||
let result = verifier.verify(&resolution, ctx(&name, "1.0.0")).await;
|
||||
|
||||
// A transport failure aborts via FetchFailed, never a tampering-style
|
||||
// TARBALL_URL_MISMATCH.
|
||||
let ResolutionVerification::FetchFailed { message } = result else {
|
||||
panic!("expected FetchFailed, got {result:?}");
|
||||
};
|
||||
assert!(message.contains("403"), "message: {message}");
|
||||
}
|
||||
|
||||
/// The metadata fetch succeeds but does not list the pinned version. That is a
|
||||
/// genuine verification failure (not a transport error), so it stays
|
||||
/// `TARBALL_URL_MISMATCH` rather than aborting via `FetchFailed`.
|
||||
#[tokio::test]
|
||||
async fn version_absent_from_fetched_metadata_stays_tarball_url_mismatch() {
|
||||
let mut server = mockito::Server::new_async().await;
|
||||
let registry = format!("{}/", server.url());
|
||||
let server_url = server.url();
|
||||
let packument = serde_json::json!({
|
||||
"name": "present-pkg",
|
||||
"dist-tags": { "latest": "1.0.0" },
|
||||
"versions": {
|
||||
"1.0.0": {
|
||||
"name": "present-pkg",
|
||||
"version": "1.0.0",
|
||||
"dist": {
|
||||
"integrity": FAKE_INTEGRITY,
|
||||
"shasum": "0000000000000000000000000000000000000000",
|
||||
"tarball": format!("{server_url}/present-pkg/-/present-pkg-1.0.0.tgz"),
|
||||
}
|
||||
}
|
||||
}
|
||||
});
|
||||
let _meta_mock = server
|
||||
.mock("GET", "/present-pkg")
|
||||
.with_status(200)
|
||||
.with_body(packument.to_string())
|
||||
.create_async()
|
||||
.await;
|
||||
|
||||
let opts = default_opts(®istry);
|
||||
let verifier = create_npm_resolution_verifier(opts);
|
||||
let resolution = LockfileResolution::Tarball(TarballResolution {
|
||||
tarball: format!("{server_url}/present-pkg/-/present-pkg-2.0.0.tgz"),
|
||||
integrity: Some(fake_integrity()),
|
||||
git_hosted: None,
|
||||
path: None,
|
||||
});
|
||||
let name: PkgName = "present-pkg".parse().expect("parse");
|
||||
let result = verifier.verify(&resolution, ctx(&name, "2.0.0")).await;
|
||||
|
||||
let ResolutionVerification::Err { code, .. } = result else {
|
||||
panic!("expected Err, got {result:?}");
|
||||
};
|
||||
assert_eq!(code, "TARBALL_URL_MISMATCH");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn redact_url_credentials_strips_embedded_basic_auth() {
|
||||
assert_eq!(
|
||||
redact_url_credentials(
|
||||
"Failed to fetch metadata from https://user:pass@host/pkg: timed out"
|
||||
),
|
||||
"Failed to fetch metadata from https://host/pkg: timed out",
|
||||
);
|
||||
// user-only userinfo (no password) is stripped too.
|
||||
assert_eq!(
|
||||
redact_url_credentials("got https://token@registry.example/foo"),
|
||||
"got https://registry.example/foo",
|
||||
);
|
||||
// A raw "@" inside the password is stripped up to the last "@" in the
|
||||
// authority, so the password tail can't leak.
|
||||
assert_eq!(
|
||||
redact_url_credentials("Failed to fetch metadata from https://user:p@ss@host/pkg: 403"),
|
||||
"Failed to fetch metadata from https://host/pkg: 403",
|
||||
);
|
||||
// An "@" in the path/query (after the authority) is preserved.
|
||||
assert_eq!(
|
||||
redact_url_credentials("got https://host/path?to=a@b"),
|
||||
"got https://host/path?to=a@b",
|
||||
);
|
||||
// A credential-free URL is left untouched.
|
||||
assert_eq!(
|
||||
redact_url_credentials("Failed to fetch metadata from https://host/pkg: timed out"),
|
||||
"Failed to fetch metadata from https://host/pkg: timed out",
|
||||
);
|
||||
// A bare "://" with no preceding scheme character is not treated as a URL
|
||||
// authority, so an "@" further along is preserved.
|
||||
assert_eq!(redact_url_credentials("a :// b@c"), "a :// b@c");
|
||||
}
|
||||
|
||||
@@ -75,7 +75,12 @@ pub(crate) struct PublishedAtLookupContext {
|
||||
pub published_at: SingleflightMap<Result<Option<String>, String>>,
|
||||
pub full_meta: SingleflightMap<Result<Option<Arc<PublishedAtTimeMap>>, String>>,
|
||||
pub full_meta_for_trust: SingleflightMap<Result<Arc<Package>, String>>,
|
||||
pub abbreviated_meta: SingleflightMap<Option<AbbreviatedMetaProjection>>,
|
||||
/// `Ok(projection)` on a successful fetch, `Err(reason)` on a fetch
|
||||
/// failure (auth/network/5xx). The error is carried as a value rather than
|
||||
/// discarded so the tarball-URL check can tell a transport failure apart
|
||||
/// from a version genuinely absent from the metadata; the age shortcut
|
||||
/// ignores it and falls back to per-version lookups.
|
||||
pub abbreviated_meta: SingleflightMap<Result<AbbreviatedMetaProjection, String>>,
|
||||
pub local_meta: SingleflightMap<Option<Arc<PublishedAtTimeMap>>>,
|
||||
}
|
||||
|
||||
|
||||
@@ -30,7 +30,9 @@ fn resolution_verification_err_round_trip() {
|
||||
assert_eq!(code, "MINIMUM_RELEASE_AGE_VIOLATION");
|
||||
assert_eq!(reason, "was published yesterday");
|
||||
}
|
||||
ResolutionVerification::Ok => panic!("expected Err"),
|
||||
ResolutionVerification::Ok | ResolutionVerification::FetchFailed { .. } => {
|
||||
panic!("expected Err")
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -29,6 +29,18 @@ pub enum ResolutionVerification {
|
||||
/// breakdown. Allowed to allocate.
|
||||
reason: String,
|
||||
},
|
||||
/// The registry couldn't be reached to verify the entry
|
||||
/// (auth/network/5xx). Unlike [`ResolutionVerification::Err`], this
|
||||
/// is not a per-entry policy pick to collect into the batch — the
|
||||
/// verification never completed, so the runner aborts the install
|
||||
/// with the registry's own fetch error rather than mislabeling a
|
||||
/// transport failure as lockfile tampering. Mirrors pnpm, where the
|
||||
/// verifier rethrows the underlying `FetchError`.
|
||||
FetchFailed {
|
||||
/// The registry fetch error, already rendered and stripped of
|
||||
/// any credentials embedded in the URL.
|
||||
message: String,
|
||||
},
|
||||
}
|
||||
|
||||
/// A [`ResolutionVerifier`]'s rejection materialized for one
|
||||
|
||||
@@ -45,7 +45,7 @@ export class FetchError extends PnpmError {
|
||||
if (request.authHeaderValue) {
|
||||
_request.authHeaderValue = hideAuthInformation(request.authHeaderValue)
|
||||
}
|
||||
const message = `GET ${request.url}: ${response.statusText} - ${response.status}`
|
||||
const message = `GET ${redactUrlCredentials(request.url)}: ${response.statusText} - ${response.status}`
|
||||
// NOTE: For security reasons, some registries respond with 404 on authentication errors as well.
|
||||
// So we print authorization info on 404 errors as well.
|
||||
if (response.status === 401 || response.status === 403 || response.status === 404) {
|
||||
@@ -62,6 +62,53 @@ export class FetchError extends PnpmError {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Strip `user:pass@` (or `user@`) userinfo that follows a URL scheme in any
|
||||
* text, e.g. `GET https://user:pass@host/pkg: …` → `GET https://host/pkg: …`.
|
||||
* A registry configured as `https://user:pass@host/` would otherwise leak its
|
||||
* embedded basic-auth credentials into every error message that interpolates
|
||||
* the request URL (terminal output, CI logs). `FetchError` already hides the
|
||||
* auth *header*; this covers credentials carried in the URL itself.
|
||||
*
|
||||
* Implemented as a single forward scan rather than a regex: `text` is
|
||||
* uncontrolled (it interpolates the request URL), so a backtracking pattern is
|
||||
* a ReDoS vector, and the scan strips up to the **last** `@` in the authority
|
||||
* so a raw `@` inside the password (`user:p@ss@host`) doesn't leak its tail.
|
||||
*/
|
||||
export function redactUrlCredentials (text: string): string {
|
||||
let result = ''
|
||||
let cursor = 0
|
||||
while (cursor < text.length) {
|
||||
const schemeSep = text.indexOf('://', cursor)
|
||||
if (schemeSep === -1) return result + text.slice(cursor)
|
||||
const authorityStart = schemeSep + 3
|
||||
result += text.slice(cursor, authorityStart)
|
||||
cursor = authorityStart
|
||||
// Only treat `://` as a URL authority boundary when a scheme character
|
||||
// (schemes end in an ASCII alphanumeric) sits right before it; otherwise a
|
||||
// bare `://` in the text is left untouched.
|
||||
if (schemeSep === 0 || !isSchemeTailChar(text.charCodeAt(schemeSep - 1))) continue
|
||||
// Userinfo runs to the last `@` within the authority, which itself ends at
|
||||
// the first `/`, `?`, `#`, or whitespace.
|
||||
let lastAt = -1
|
||||
for (let i = authorityStart; i < text.length; i++) {
|
||||
const code = text.charCodeAt(i)
|
||||
if (code === 0x2f || code === 0x3f || code === 0x23 || isAsciiWhitespace(code)) break
|
||||
if (code === 0x40) lastAt = i
|
||||
}
|
||||
if (lastAt !== -1) cursor = lastAt + 1
|
||||
}
|
||||
return result
|
||||
}
|
||||
|
||||
function isSchemeTailChar (code: number): boolean {
|
||||
return (code >= 0x30 && code <= 0x39) || (code >= 0x41 && code <= 0x5a) || (code >= 0x61 && code <= 0x7a)
|
||||
}
|
||||
|
||||
function isAsciiWhitespace (code: number): boolean {
|
||||
return code === 0x20 || code === 0x09 || code === 0x0a || code === 0x0b || code === 0x0c || code === 0x0d
|
||||
}
|
||||
|
||||
function hideAuthInformation (authHeaderValue: string): string {
|
||||
const [authType, token] = authHeaderValue.split(' ')
|
||||
if (token == null) return '[hidden]'
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import { expect, test } from '@jest/globals'
|
||||
import { FetchError, PnpmError } from '@pnpm/error'
|
||||
import { FetchError, PnpmError, redactUrlCredentials } from '@pnpm/error'
|
||||
|
||||
test('PnpmError exposes cause when provided', () => {
|
||||
const cause = new Error('original failure')
|
||||
@@ -48,3 +48,35 @@ test('FetchError escapes non-standard auth header', () => {
|
||||
expect(error.hint).toBe('An authorization header was used: [hidden]')
|
||||
expect(error.request.authHeaderValue).toBe('[hidden]')
|
||||
})
|
||||
|
||||
test('FetchError strips basic-auth credentials embedded in the request URL', () => {
|
||||
const error = new FetchError(
|
||||
{ url: 'https://user:pass@registry.example/@scope%2fpkg' },
|
||||
{ status: 403, statusText: 'Forbidden' }
|
||||
)
|
||||
expect(error.message).toBe('GET https://registry.example/@scope%2fpkg: Forbidden - 403')
|
||||
})
|
||||
|
||||
test('redactUrlCredentials', () => {
|
||||
// user:pass@ and user@ userinfo are stripped, regardless of scheme.
|
||||
expect(redactUrlCredentials('GET https://user:pass@host/pkg: timed out'))
|
||||
.toBe('GET https://host/pkg: timed out')
|
||||
expect(redactUrlCredentials('git+ssh://token@host/repo.git'))
|
||||
.toBe('git+ssh://host/repo.git')
|
||||
// A raw "@" inside the password is stripped up to the last "@" in the
|
||||
// authority, so the password tail can't leak.
|
||||
expect(redactUrlCredentials('GET https://user:p@ss@host/pkg: 403'))
|
||||
.toBe('GET https://host/pkg: 403')
|
||||
// An "@" in the path/query (after the authority) is preserved.
|
||||
expect(redactUrlCredentials('https://host/path?to=a@b'))
|
||||
.toBe('https://host/path?to=a@b')
|
||||
// A credential-free URL is left untouched.
|
||||
expect(redactUrlCredentials('GET https://host/pkg: timed out'))
|
||||
.toBe('GET https://host/pkg: timed out')
|
||||
// A bare "://" with no preceding scheme character is not a URL authority, so
|
||||
// a later "@" is preserved.
|
||||
expect(redactUrlCredentials('a :// b@c')).toBe('a :// b@c')
|
||||
// Multiple credentialed URLs in one message are all redacted.
|
||||
expect(redactUrlCredentials('a https://u:p@h1/x and b https://t@h2/y'))
|
||||
.toBe('a https://h1/x and b https://h2/y')
|
||||
})
|
||||
|
||||
@@ -262,6 +262,10 @@ function buildVerificationError (violations: ResolutionPolicyViolation[]): PnpmE
|
||||
const details = omitted > 0
|
||||
? `${breakdown}\n …and ${omitted} more`
|
||||
: breakdown
|
||||
// Registry fetch failures (auth/network/5xx) don't reach this batch — the
|
||||
// verifier throws the registry's own error and the gate aborts with it. So
|
||||
// every violation here is a genuine policy rejection, and the hint points at
|
||||
// the lockfile rather than at connectivity.
|
||||
return new PnpmError(
|
||||
errorCode,
|
||||
`${violations.length} lockfile entries failed verification:\n${details}`,
|
||||
@@ -422,23 +426,40 @@ async function iterateLockfileViolations (
|
||||
concurrency: number | undefined
|
||||
): Promise<ResolutionPolicyViolation[]> {
|
||||
const violations: ResolutionPolicyViolation[] = []
|
||||
// A verifier may throw rather than return a violation when it can't reach the
|
||||
// registry to verify an entry (auth/network/5xx) — that's not a per-entry
|
||||
// policy pick, it's an incomplete verification, so the registry's own error
|
||||
// should abort the install. Capture the first such error and rethrow it after
|
||||
// the fan-out settles: rethrowing straight into Promise.all would leave the
|
||||
// sibling tasks (all failing against the same dead registry) as unhandled
|
||||
// rejections once Promise.all rejects on the first.
|
||||
let fetchError: unknown
|
||||
const limit = pLimit(concurrency ?? DEFAULT_CONCURRENCY)
|
||||
await Promise.all(
|
||||
Array.from(candidates.values(), ({ name, version, nonSemverVersion, resolution }) => limit(async () => {
|
||||
// Fan out across every active verifier; each handles its own
|
||||
// protocol short-circuit (e.g. the npm verifier returns ok:true for
|
||||
// git resolutions). We stop at the first failure per entry so a
|
||||
// multi-verifier setup doesn't produce duplicate violations for the
|
||||
// same (name, version).
|
||||
for (const verifier of verifiers) {
|
||||
// eslint-disable-next-line no-await-in-loop
|
||||
const result = await verifier.verify(resolution, { name, version, nonSemverVersion })
|
||||
if (!result.ok) {
|
||||
violations.push({ name, version, resolution, code: result.code, reason: result.reason })
|
||||
break
|
||||
try {
|
||||
// Fan out across every active verifier; each handles its own
|
||||
// protocol short-circuit (e.g. the npm verifier returns ok:true for
|
||||
// git resolutions). We stop at the first failure per entry so a
|
||||
// multi-verifier setup doesn't produce duplicate violations for the
|
||||
// same (name, version).
|
||||
for (const verifier of verifiers) {
|
||||
// eslint-disable-next-line no-await-in-loop
|
||||
const result = await verifier.verify(resolution, { name, version, nonSemverVersion })
|
||||
if (!result.ok) {
|
||||
violations.push({ name, version, resolution, code: result.code, reason: result.reason })
|
||||
break
|
||||
}
|
||||
}
|
||||
} catch (err) {
|
||||
fetchError ??= err
|
||||
}
|
||||
}))
|
||||
)
|
||||
// A registry that couldn't be reached takes precedence over collected
|
||||
// violations: the run never finished verifying, so the batch is incomplete
|
||||
// and the actionable failure is the transport error. Once it's resolved the
|
||||
// re-run surfaces any remaining violations.
|
||||
if (fetchError != null) throw fetchError
|
||||
return violations
|
||||
}
|
||||
|
||||
@@ -71,6 +71,27 @@ test('throws with the verifier-supplied code and reason on a single failure', as
|
||||
})
|
||||
})
|
||||
|
||||
test('propagates a verifier throw (registry fetch failure) instead of folding it into a batch', async () => {
|
||||
// A verifier throws — rather than returning a violation — when it can't reach
|
||||
// the registry to verify an entry. That transport error must surface as-is
|
||||
// (the install aborts with the registry's own error), not be turned into a
|
||||
// lockfile-verification batch.
|
||||
const lockfile = makeLockfile({
|
||||
'is-odd@0.1.2': { resolution: tarballResolution('sha512-a') },
|
||||
'private-pkg@1.0.0': { resolution: tarballResolution('sha512-b') },
|
||||
})
|
||||
const fetchError = Object.assign(new Error('GET https://registry.example/private-pkg: Forbidden - 403'), {
|
||||
code: 'ERR_PNPM_FETCH_403',
|
||||
})
|
||||
const verifier = wrap(async (_, { name }) => {
|
||||
if (name === 'private-pkg') throw fetchError
|
||||
return { ok: false, code: 'MINIMUM_RELEASE_AGE_VIOLATION', reason: 'too fresh' }
|
||||
})
|
||||
|
||||
// The thrown transport error wins over the collected policy violation.
|
||||
await expect(verifyLockfileResolutions(lockfile, [verifier])).rejects.toBe(fetchError)
|
||||
})
|
||||
|
||||
test('throws a generic code with per-entry codes in the breakdown when violations span policies', async () => {
|
||||
const lockfile = makeLockfile({
|
||||
'is-odd@0.1.2': { resolution: tarballResolution('sha512-a') },
|
||||
|
||||
@@ -335,16 +335,11 @@ async function runAgeCheck (
|
||||
cutoff: number,
|
||||
ignoreMissingTimeField: boolean
|
||||
): Promise<{ ok: false, code: string, reason: string } | undefined> {
|
||||
let published: string | undefined
|
||||
try {
|
||||
published = await fetchPublishedAt(context, registry, name, version)
|
||||
} catch (err) {
|
||||
return {
|
||||
ok: false,
|
||||
code: MINIMUM_RELEASE_AGE_VIOLATION_CODE,
|
||||
reason: uncheckable('minimumReleaseAge', err instanceof Error ? err.message : String(err)),
|
||||
}
|
||||
}
|
||||
// A transport failure (auth/network/5xx) propagates the registry's own fetch
|
||||
// error (e.g. ERR_PNPM_FETCH_403); the gate aborts the install with it rather
|
||||
// than folding it into a policy violation. A successful fetch that simply
|
||||
// lacks a publish timestamp for this version is handled below.
|
||||
const published = await fetchPublishedAt(context, registry, name, version)
|
||||
if (!published) {
|
||||
// No source — attestation, local mirror, or full metadata —
|
||||
// surfaced a publish timestamp for this version. The resolver's
|
||||
@@ -400,7 +395,15 @@ async function runTarballUrlCheck (
|
||||
version: string,
|
||||
lockfileTarball: string
|
||||
): Promise<{ ok: false, code: string, reason: string } | undefined> {
|
||||
const meta = await fetchAbbreviatedMeta(context, registry, name)
|
||||
const { meta, error } = await fetchAbbreviatedMeta(context, registry, name)
|
||||
if (error != null) {
|
||||
// Couldn't reach the registry to verify (auth/network/5xx). Propagate the
|
||||
// registry's own fetch error (e.g. ERR_PNPM_FETCH_403, which already
|
||||
// explains the auth situation) instead of mislabeling a transport failure
|
||||
// as a tampering-style URL mismatch. The gate aborts the install with that
|
||||
// error — still fail-closed, the entry never reaches the filesystem.
|
||||
throw error
|
||||
}
|
||||
const registryTarball = meta?.versionTarballs?.get(version)
|
||||
if (registryTarball != null && sameTarballUrl(lockfileTarball, registryTarball)) {
|
||||
return undefined
|
||||
@@ -454,19 +457,11 @@ async function runTrustCheck (
|
||||
trustPolicyIgnoreAfter?: number
|
||||
}
|
||||
): Promise<{ ok: false, code: string, reason: string } | undefined> {
|
||||
let meta: PackageMeta
|
||||
try {
|
||||
meta = await fetchFullMetaForTrust(context, registry, name)
|
||||
} catch (err) {
|
||||
// `fetchFullMetadataCached` rejects (network error, 404, etc.); the
|
||||
// verifier fails closed so a missing manifest can't be mistaken
|
||||
// for a passing trust check.
|
||||
return {
|
||||
ok: false,
|
||||
code: TRUST_DOWNGRADE_VIOLATION_CODE,
|
||||
reason: uncheckable('trustPolicy', err instanceof Error ? err.message : String(err)),
|
||||
}
|
||||
}
|
||||
// A transport failure (auth/network/5xx) propagates the registry's own fetch
|
||||
// error; the gate aborts the install with it rather than folding it into a
|
||||
// policy violation. Still fail-closed: a missing manifest can't be mistaken
|
||||
// for a passing trust check because the install never proceeds.
|
||||
const meta = await fetchFullMetaForTrust(context, registry, name)
|
||||
|
||||
try {
|
||||
failIfTrustDowngraded(meta, version, opts)
|
||||
@@ -611,10 +606,11 @@ interface PublishedAtLookupContext {
|
||||
* package-level `modified` plus the set of currently-listed version
|
||||
* names — so the multi-hundred-KB packument can be GC'd as soon as
|
||||
* the fetch returns (the cache only needs to dedupe network/disk
|
||||
* round-trips, not full document storage). Resolves to `undefined`
|
||||
* on failure.
|
||||
* round-trips, not full document storage). Resolves to `{ meta }` on
|
||||
* success or `{ error }` on a fetch failure — it never rejects, so the
|
||||
* cached promise is safe to share between callers.
|
||||
*/
|
||||
abbreviatedMetaCache: Map<string, Promise<AbbreviatedMetaProjection | undefined>>
|
||||
abbreviatedMetaCache: Map<string, Promise<AbbreviatedMetaResult>>
|
||||
/**
|
||||
* Per-(registry+name+version) memo of the final published-at answer
|
||||
* the verifier hands to the policy check. One install verifies each
|
||||
@@ -724,7 +720,9 @@ async function tryAbbreviatedModifiedShortcut (
|
||||
name: string,
|
||||
version: string
|
||||
): Promise<string | undefined> {
|
||||
const meta = await fetchAbbreviatedMeta(context, registry, name)
|
||||
// A fetch failure here is fine: ignore `error` and fall back to per-version
|
||||
// lookups, the same as a successful-but-uninformative metadata response.
|
||||
const { meta } = await fetchAbbreviatedMeta(context, registry, name)
|
||||
const modified = meta?.modified
|
||||
if (typeof modified !== 'string') return undefined
|
||||
const modifiedMs = Date.parse(modified)
|
||||
@@ -742,7 +740,7 @@ function fetchAbbreviatedMeta (
|
||||
context: PublishedAtLookupContext,
|
||||
registry: string,
|
||||
name: string
|
||||
): Promise<AbbreviatedMetaProjection | undefined> {
|
||||
): Promise<AbbreviatedMetaResult> {
|
||||
const cacheKey = `${registry}\x00${name}`
|
||||
let cachedPromise = context.abbreviatedMetaCache.get(cacheKey)
|
||||
if (cachedPromise == null) {
|
||||
@@ -753,13 +751,22 @@ function fetchAbbreviatedMeta (
|
||||
// can only return this registry's own packument.
|
||||
const shared = readSharedMeta(context.sharedMetaCache, registry, name)
|
||||
if (shared != null) {
|
||||
cachedPromise = Promise.resolve(projectAbbreviatedMeta(shared))
|
||||
cachedPromise = Promise.resolve({ meta: projectAbbreviatedMeta(shared) })
|
||||
} else {
|
||||
// Carry a fetch failure (auth/network/5xx) as `error` instead of
|
||||
// collapsing it to `undefined`: the tarball-URL check rethrows it (so the
|
||||
// registry's own error surfaces, not a tampering-style mismatch) while
|
||||
// the age shortcut ignores it and falls back to per-version lookups.
|
||||
// Keeping it a resolved value — not a rejected promise — lets the two
|
||||
// callers share one cached promise without an unhandled rejection.
|
||||
cachedPromise = fetchAbbreviatedMetadataCached(context.fetchOpts, name, {
|
||||
registry,
|
||||
authHeaderValue: context.getAuthHeaderValueByURI(registry, { pkgName: name }),
|
||||
cacheDir: context.cacheDir,
|
||||
}).then(projectAbbreviatedMeta, () => undefined)
|
||||
}).then(
|
||||
(meta) => ({ meta: projectAbbreviatedMeta(meta) }),
|
||||
(error: unknown) => ({ error })
|
||||
)
|
||||
}
|
||||
context.abbreviatedMetaCache.set(cacheKey, cachedPromise)
|
||||
}
|
||||
@@ -846,6 +853,21 @@ interface AbbreviatedMetaProjection {
|
||||
versionTarballs?: Map<string, string | undefined>
|
||||
}
|
||||
|
||||
/**
|
||||
* Result of an abbreviated-metadata fetch. The fetch error is carried as a
|
||||
* value rather than rejected so the per-install cache can hold a single
|
||||
* resolved promise — a cached rejection would surface as an unhandled
|
||||
* rejection and be shared across every caller of the same key. The
|
||||
* tarball-URL check rethrows this error; the age shortcut ignores it and
|
||||
* falls back to per-version lookups.
|
||||
*
|
||||
* Modeled as a discriminated union so a result carries exactly one of `meta`
|
||||
* or `error` — never both, never neither.
|
||||
*/
|
||||
type AbbreviatedMetaResult =
|
||||
| { meta: AbbreviatedMetaProjection, error?: undefined }
|
||||
| { error: unknown, meta?: undefined }
|
||||
|
||||
function readLocalMetaTime (
|
||||
context: PublishedAtLookupContext,
|
||||
registry: string,
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import url from 'node:url'
|
||||
import util from 'node:util'
|
||||
|
||||
import { requestRetryLogger } from '@pnpm/core-loggers'
|
||||
import {
|
||||
@@ -6,6 +7,7 @@ import {
|
||||
type FetchErrorRequest,
|
||||
type FetchErrorResponse,
|
||||
PnpmError,
|
||||
redactUrlCredentials,
|
||||
} from '@pnpm/error'
|
||||
import type { FetchFromRegistry, RetryTimeoutOptions } from '@pnpm/fetching.types'
|
||||
import { globalWarn } from '@pnpm/logger'
|
||||
@@ -143,7 +145,16 @@ export async function fetchMetadataFromFromRegistry (
|
||||
timeout: fetchOpts.timeout,
|
||||
}) as RegistryResponse
|
||||
} catch (error: any) { // eslint-disable-line
|
||||
reject(new PnpmError('META_FETCH_FAIL', `GET ${uri}: ${error.message as string}`, { attempts: attempt, cause: error }))
|
||||
// Redact credentials embedded in the URL from the cause as well, not
|
||||
// just the top-level message: a reporter or debugger that renders
|
||||
// `error.cause` would otherwise print the raw URL-bearing message. The
|
||||
// `stack` string embeds the original (pre-mutation) message, so redact
|
||||
// it too — mutating `message` alone leaves the credentials in `stack`.
|
||||
if (util.types.isNativeError(error)) {
|
||||
if (typeof error.message === 'string') error.message = redactUrlCredentials(error.message)
|
||||
if (typeof error.stack === 'string') error.stack = redactUrlCredentials(error.stack)
|
||||
}
|
||||
reject(new PnpmError('META_FETCH_FAIL', redactUrlCredentials(`GET ${uri}: ${error.message as string}`), { attempts: attempt, cause: error }))
|
||||
return
|
||||
}
|
||||
if (response.status === 304) {
|
||||
|
||||
@@ -563,3 +563,47 @@ test('createNpmResolutionVerifier() canTrustPastCheck rejects when the trust-exc
|
||||
trustPolicyIgnoreAfter: null,
|
||||
})).toBe(false)
|
||||
})
|
||||
|
||||
test('createNpmResolutionVerifier() propagates the registry fetch error instead of reporting a tampering-style mismatch', async () => {
|
||||
// A 403 on the metadata fetch (e.g. a CI token that is authenticated but not
|
||||
// authorized to read a private package) must not be reported as a lockfile
|
||||
// tarball-URL mismatch: the lockfile is correct, the fetch is the problem.
|
||||
// The verifier rethrows the registry's own error so the install aborts with
|
||||
// ERR_PNPM_FETCH_403 (which already explains the auth situation).
|
||||
const pool = getMockAgent().get('https://registry.npmjs.org')
|
||||
pool.intercept({ path: '/private-pkg', method: 'GET' }).reply(403, { error: 'Forbidden' }).persist()
|
||||
|
||||
const verifier = createNpmResolutionVerifier(makeVerifierOpts())
|
||||
await expect(verifier.verify(
|
||||
makeTarballResolution('private-pkg', '1.0.0'),
|
||||
{ name: 'private-pkg', version: '1.0.0' }
|
||||
)).rejects.toMatchObject({ code: 'ERR_PNPM_FETCH_403' })
|
||||
})
|
||||
|
||||
test('createNpmResolutionVerifier() still flags a version absent from fetched metadata as TARBALL_URL_MISMATCH', async () => {
|
||||
// The metadata fetch succeeds but does not list the pinned version. That is a
|
||||
// genuine verification failure (not a transport error), so it must stay
|
||||
// TARBALL_URL_MISMATCH — distinct from the new TARBALL_URL_FETCH_FAILED.
|
||||
const meta = {
|
||||
name: 'present-pkg',
|
||||
'dist-tags': { latest: '1.0.0' },
|
||||
versions: {
|
||||
'1.0.0': {
|
||||
name: 'present-pkg',
|
||||
version: '1.0.0',
|
||||
dist: { tarball: 'https://registry.npmjs.org/present-pkg/-/present-pkg-1.0.0.tgz', shasum: 'aa' },
|
||||
},
|
||||
},
|
||||
modified: '2020-01-01T00:00:00.000Z',
|
||||
}
|
||||
const pool = getMockAgent().get('https://registry.npmjs.org')
|
||||
pool.intercept({ path: '/present-pkg', method: 'GET' }).reply(200, meta).persist()
|
||||
|
||||
const verifier = createNpmResolutionVerifier(makeVerifierOpts())
|
||||
const result = await verifier.verify(
|
||||
makeTarballResolution('present-pkg', '2.0.0'),
|
||||
{ name: 'present-pkg', version: '2.0.0' }
|
||||
)
|
||||
|
||||
expect(result).toMatchObject({ ok: false, code: 'TARBALL_URL_MISMATCH' })
|
||||
})
|
||||
|
||||
@@ -795,7 +795,15 @@ async fn verify_input_lockfile(
|
||||
return Ok(None);
|
||||
}
|
||||
|
||||
let violations = collect_resolution_policy_violations(lockfile, &verifiers, None).await;
|
||||
// A transport failure verifying an entry (the upstream registry couldn't be
|
||||
// reached/authorized) is a gateway error, not a policy violation — surface
|
||||
// the registry's own (credential-redacted) message to the client.
|
||||
let violations = match collect_resolution_policy_violations(lockfile, &verifiers, None).await {
|
||||
Ok(violations) => violations,
|
||||
Err(message) => {
|
||||
return Err(VerifyFailure::Internal(json_error(StatusCode::BAD_GATEWAY, &message)));
|
||||
}
|
||||
};
|
||||
let osv_violations = runtime
|
||||
.osv_index
|
||||
.as_ref()
|
||||
|
||||
Reference in New Issue
Block a user