From 220a880d747f6f5ebb2b800a76a3377c52637ffd Mon Sep 17 00:00:00 2001 From: Justin Chung Date: Sun, 6 Sep 2026 12:51:17 -0400 Subject: [PATCH 1/7] feat(check): name the registries that declined to answer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `ManifestCheck::registry_unreachable` was one boolean per manifest, so a `--fail-on` gate could refuse to certify a run but never say which registry had declined. A manifest is not the routing unit either: `route_item` sends a `deno.json` to npm and to JSR, and a `Cargo.toml` to crates.io and to every alternate registry its dependencies name, so a per-manifest — or even a per-ecosystem — answer collapses routes that failed independently. `fetch_all` now records the route behind each failed lookup, keyed by the cache key that already distinguishes those routes, and returns them sorted so `buffer_unordered`'s arrival order cannot reach the message. `ManifestCheck` gains `unreachable_registries`; the boolean stays, derived from the collection at the one site that builds both, because this crate is published and the field is part of its surface. The refusal now names them: "the Go registry did not answer", "the Go and npm registries did not answer". A root that is not the ecosystem's own default is named beside it — `npm (jsr.io)` — reduced to host and port, because a configured root may carry credentials (`.npmrc` interpolates `${VAR}`) and the refusal is printed on stderr, which CI captures as job output. The 404 carve-out is untouched: the predicate that decides what counts as a registry declining to answer is character-identical, and an empty collection pushes no reason at all. Refs #112 --- crates/dependable-fetch/src/check.rs | 378 ++++++++++++++++++++++++++- crates/dependable-fetch/src/lib.rs | 4 +- crates/dependable/src/output/mod.rs | 16 +- crates/dependable/src/runner.rs | 194 +++++++++++++- 4 files changed, 571 insertions(+), 21 deletions(-) diff --git a/crates/dependable-fetch/src/check.rs b/crates/dependable-fetch/src/check.rs index e18eaac..de713ac 100644 --- a/crates/dependable-fetch/src/check.rs +++ b/crates/dependable-fetch/src/check.rs @@ -84,6 +84,112 @@ pub enum CheckError { Fetch(#[from] FetchError), } +/// One registry that declined to answer during a check, named by where the requests +/// were routed. +/// +/// A manifest is not the routing unit. A `Cargo.toml` can name crates.io and a private +/// alternate registry; a `deno.json` routes to npm and to JSR both. A single +/// per-manifest flag could therefore say only *that* something did not answer, never +/// which — so a run against two registries could not tell the reader whether its results +/// were half unfounded or wholly unfounded. +/// +/// `#[non_exhaustive]`: build one with [`UnreachableRegistry::new`]; future fields are +/// additive. +/// +/// # Limitation +/// [`root`](Self::root) is the fetcher's *default* root. `NpmFetcher` resolves an +/// `@scope` package to a per-scope registry read from `.npmrc`, which +/// [`RegistryFetcher::registry_root`] does not name, so a scoped package that failed +/// against a private host is reported under the fetcher's default root instead. Naming +/// the host that was actually asked needs a per-package root on the trait, which every +/// fetcher would have to implement. +#[non_exhaustive] +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct UnreachableRegistry { + /// The ecosystem whose lookups were routed here. + pub ecosystem: Ecosystem, + /// The registry root the fetcher talks to, when it names one. + /// + /// `None` when the fetcher opts out of [`RegistryFetcher::registry_root`], whose + /// trait default returns nothing. The ecosystem is then the whole of what can be + /// said, and [`label`](Self::label) says exactly that rather than an empty string. + pub root: Option, +} + +impl UnreachableRegistry { + /// Name a registry by the ecosystem routed to it and, when known, its root. + #[must_use] + pub fn new(ecosystem: Ecosystem, root: Option) -> Self { + Self { ecosystem, root } + } + + /// Whether [`root`](Self::root) is the ecosystem's own default registry — `true` + /// also when no root is known, since nothing then distinguishes it. + /// + /// The same discrimination `default_cache_key` makes when deciding whether a cache + /// key needs scoping: only a non-default root says anything the ecosystem name does + /// not already say. + #[must_use] + pub fn is_default_root(&self) -> bool { + match &self.root { + Some(root) => { + root.trim_end_matches('/') + == self.ecosystem.default_registry().trim_end_matches('/') + } + None => true, + } + } + + /// A short name for a message: the ecosystem, plus the root's host when that root is + /// not the ecosystem's default one — `npm`, or `npm (jsr.io)`. + /// + /// The root is reduced to its authority, host and port, and never printed whole. A + /// configured root may legitimately carry credentials — `.npmrc` interpolates + /// `${VAR}` into its `registry` lines, and `.dependable.toml` roots are written by + /// hand — and this string is printed on stderr, which CI captures as job output. + #[must_use] + pub fn label(&self) -> String { + let name = self.ecosystem.display_name(); + if self.is_default_root() { + return name.to_owned(); + } + match self.root.as_deref().and_then(authority_of) { + Some(authority) => format!("{name} ({authority})"), + None => name.to_owned(), + } + } +} + +/// The order the unreachable registries are reported in: ecosystem name, then root. +/// +/// `Ecosystem` derives `Hash` but not `Ord`, so this is an explicit key rather than a +/// `BTreeSet`. The raw root orders the key — two mirrors that reduce to the same +/// printed authority still sort deterministically. +fn unreachable_sort_key(registry: &UnreachableRegistry) -> (&str, &str) { + ( + registry.ecosystem.display_name(), + registry.root.as_deref().unwrap_or(""), + ) +} + +/// The scheme-less authority of a URL-ish string: host and port, with userinfo, path, +/// query and fragment dropped. `None` when nothing is left. +/// +/// Hand-rolled over `&str` rather than taken from a URL crate: this workspace depends on +/// none, and the input is not guaranteed to parse as a URL anyway — a configured root +/// may carry an unexpanded `${VAR}`, or no scheme at all. +fn authority_of(root: &str) -> Option { + let after_scheme = root.split_once("://").map_or(root, |(_, rest)| rest); + let authority = after_scheme.split(['/', '?', '#']).next().unwrap_or(""); + // Userinfo is credentials. The *last* `@` wins, because a password may legally + // contain one and only the tail is the host. + let host = authority + .rsplit_once('@') + .map_or(authority, |(_, h)| h) + .trim(); + (!host.is_empty()).then(|| host.to_owned()) +} + /// The outcome of checking one manifest. /// /// `#[non_exhaustive]`: future fields are additive. @@ -114,7 +220,29 @@ pub struct ManifestCheck { /// timeout, a refused connection, a 5xx, or an undecodable response is the registry /// declining to answer, and a `--fail-on` promise cannot be kept from answers that /// were never given. + /// + /// Exactly `!`[`unreachable_registries`](Self::unreachable_registries)`.is_empty()`, + /// derived at construction from that one collection so the two cannot disagree. + /// Prefer the collection: it says *which* registry, which a caller reporting the + /// failure needs and this cannot supply. pub registry_unreachable: bool, + /// Which registries declined to answer, in a stable order. + /// + /// Empty is the affirmative statement that **every registry this manifest routed to + /// answered** — not "unknown". A manifest routes to more than one registry often + /// enough that "the registry did not answer" names nothing a reader can act on: a + /// `deno.json` reaches npm and JSR, and a `Cargo.toml` reaches crates.io and any + /// alternate registry its dependencies name. + /// + /// Deduplicated per registry, not per failed lookup — twenty timeouts against one + /// mirror are one registry that did not answer — and sorted by ecosystem name then + /// root, because the lookups complete out of order and a message built from an + /// arrival-ordered list would change between runs. + /// + /// Empty on a fully cached run even where the registry is down: a cache hit issues + /// no request, so nothing declines to answer. That is the same reach the boolean + /// always had. + pub unreachable_registries: Vec, /// The manifest whose `[workspace.dependencies]` govern this one — itself, when it /// declares its own `[workspace]`, else the nearest ancestor that does. /// @@ -651,7 +779,7 @@ impl Checker { } } - let (fetched, registry_unreachable) = self.fetch_all(tasks).await; + let (fetched, unreachable_registries) = self.fetch_all(tasks, ecosystem).await; let mut results: Vec = parsed .items .iter() @@ -685,7 +813,10 @@ impl Checker { results, warnings, vulnerability_scan_failed, - registry_unreachable, + // Derived here, in the one place both are set, so the boolean can never + // contradict the collection it summarises. + registry_unreachable: !unreachable_registries.is_empty(), + unreachable_registries, workspace_root: workspace.map(|(root, _)| root), }; @@ -760,17 +891,42 @@ impl Checker { /// request. The concurrency inside a single manifest is safe — its tasks are /// already deduplicated by `(cache_key, name)` before they get here. Anyone /// parallelising the *manifest* loop must add coalescing here first. - /// Fetch every task's version list, returning the results and whether any lookup - /// failed for a reason other than the package not existing. + /// Fetch every task's version list, returning the results and the registries that + /// declined to answer. /// /// A 404 is an answer: the package is private, internal, deleted, or served by a /// registry this run does not route to. Anything else — a timeout, a refused /// connection, a 5xx, a response that would not decode — is the registry declining /// to answer, and a gate cannot be honoured from answers that were never given. - async fn fetch_all(&self, tasks: Vec) -> (FetchedMap, bool) { + /// + /// The registry is named per *route*, not per manifest, because a manifest is not + /// the routing unit: `route_item` sends a `deno.json` to npm and to JSR, + /// and a `Cargo.toml` to crates.io and to any alternate registry it names. Each + /// route has its own cache key, so the key is what deduplicates the report. + async fn fetch_all( + &self, + tasks: Vec, + ecosystem: Ecosystem, + ) -> (FetchedMap, Vec) { let total = tasks.len(); self.emit(ProgressEvent::Started { total }); + // Built before the tasks are consumed: an outcome carries only + // `(name, cache_key, result)`, and the `Arc` that knows the + // root is dropped when the task moves into its future. + let routes: HashMap = tasks + .iter() + .map(|task| { + ( + task.cache_key.clone(), + UnreachableRegistry::new( + ecosystem, + task.fetcher.registry_root().map(str::to_owned), + ), + ) + }) + .collect(); + let mut out: FetchedMap = HashMap::new(); let mut to_fetch: Vec = Vec::new(); for task in tasks { @@ -815,7 +971,7 @@ impl Checker { .collect() .await; - let mut registry_unreachable = false; + let mut unreachable: HashMap = HashMap::new(); for (name, cache_key, result) in fetched { if let Ok(versions) = &result { self.versions_cache @@ -828,7 +984,14 @@ impl Checker { if let Err(e) = &result && !matches!(e, FetchError::NotFound(_)) { - registry_unreachable = true; + // Every task contributed a route, so the lookup always hits; the + // fallback is there because losing an entry must degrade to a + // less precise report, never to a certified run. + let route = routes + .get(&cache_key) + .cloned() + .unwrap_or_else(|| UnreachableRegistry::new(ecosystem, None)); + unreachable.insert(cache_key.clone(), route); } out.insert( (cache_key, name), @@ -837,7 +1000,12 @@ impl Checker { } self.emit(ProgressEvent::Finished); - (out, registry_unreachable) + // `buffer_unordered` completes in arrival order, so the map's iteration order is + // whatever the network did. Sorting here is what stops the gate's sentence + // reordering itself between two runs of the same repository. + let mut unreachable: Vec = unreachable.into_values().collect(); + unreachable.sort_by(|a, b| unreachable_sort_key(a).cmp(&unreachable_sort_key(b))); + (out, unreachable) } fn emit(&self, event: ProgressEvent) { @@ -2070,4 +2238,198 @@ mod tests { ); assert_eq!(result.latest_available.as_deref(), Some("7.1.0.M1")); } + + // ----------------------------------------------------------------------- + // Naming the registry that declined to answer + // ----------------------------------------------------------------------- + + /// A fetcher that opts out of `registry_root` — the trait's default, and what any + /// third-party implementation gets without writing a line. + struct RootlessFetcher; + + impl RegistryFetcher for RootlessFetcher { + fn fetch_versions<'a>( + &'a self, + name: &'a str, + ) -> futures::future::BoxFuture<'a, Result> + { + let name = name.to_owned(); + Box::pin(async move { Err(FetchError::NotFound(name)) }) + } + } + + /// A manifest is not the routing unit, which is the whole of #112: one `deno.json` + /// reaches npm and JSR, and one `Cargo.toml` reaches crates.io and every alternate + /// registry its dependencies name. Each route has to be able to name itself, or a + /// refusal can say only that *something* declined. + #[test] + fn each_route_names_its_own_registry() { + use crate::registries::{JsrFetcher, NpmFetcher}; + + const NPM: &str = "https://nexus.corp.example/repository/npm"; + const JSR: &str = "https://jsr.corp.example"; + const ALT: &str = "https://alt.corp.example/index"; + + let client = reqwest::Client::new(); + let checker = Checker::builder() + .http_client(client.clone()) + .registry( + Ecosystem::Npm, + Arc::new(NpmFetcher::with_registry(client.clone(), NPM)), + ) + .jsr_registry(Arc::new(JsrFetcher::with_registry(client, JSR))) + .rust_alt_registry("internal", ALT, None) + .vulnerabilities(false) + .build() + .expect("a checker"); + + let deno = parse( + ManifestKind::DenoJson, + r#"{ "imports": { "chalk": "npm:chalk@^5.0.0", "p": "jsr:@std/path@^1.0.0" } }"#, + ) + .expect("the deno fixture parses"); + let npm_item = deno + .items + .iter() + .find(|i| i.source == PackageSource::Registry) + .expect("an npm-sourced import"); + let jsr_item = deno + .items + .iter() + .find(|i| i.source == PackageSource::Jsr) + .expect("a jsr-sourced import"); + let alt_item = + item("[dependencies]\nthing = { version = \"1\", registry = \"internal\" }\n"); + + // Exactly what `fetch_all` records: the route's cache key, and the registry the + // route's fetcher talks to. + let route = |item: &Item, default: &Arc, ecosystem| { + let (fetcher, cache_key) = checker.route_item(item, default, ecosystem); + ( + cache_key, + UnreachableRegistry::new(ecosystem, fetcher.registry_root().map(str::to_owned)), + ) + }; + let npm_default = checker + .registries + .get(&Ecosystem::Npm) + .expect("the npm fetcher") + .clone(); + let rust_default = checker + .registries + .get(&Ecosystem::Rust) + .expect("the crates.io fetcher") + .clone(); + + let (npm_key, npm_registry) = route(npm_item, &npm_default, Ecosystem::Npm); + let (jsr_key, jsr_registry) = route(jsr_item, &npm_default, Ecosystem::Npm); + let (alt_key, alt_registry) = route(&alt_item, &rust_default, Ecosystem::Rust); + + // Three routes, three cache keys — which is what makes the key the right thing + // to deduplicate the report by. + assert_ne!(npm_key, jsr_key); + assert_ne!(npm_key, alt_key); + assert_ne!(jsr_key, alt_key); + + // Three routes, three labels. Two of them are the same ecosystem, which is the + // granularity a per-ecosystem answer would still have collapsed. + assert_eq!(npm_registry.label(), "npm (nexus.corp.example)"); + assert_eq!(jsr_registry.label(), "npm (jsr.corp.example)"); + assert_eq!(alt_registry.label(), "Rust (alt.corp.example)"); + + // A fetcher that names no root degrades to the ecosystem, never to a blank. + assert!(RootlessFetcher.registry_root().is_none()); + let rootless = UnreachableRegistry::new( + Ecosystem::Go, + RootlessFetcher.registry_root().map(str::to_owned), + ); + assert_eq!(rootless.label(), "Go"); + assert_eq!(rootless.root, None); + } + + /// Only a root that is not the ecosystem's own says anything the ecosystem name does + /// not already say — the same discrimination `default_cache_key` makes. + #[test] + fn only_a_non_default_root_is_named_beside_the_ecosystem() { + let npm_default = Ecosystem::Npm.default_registry(); + assert_eq!( + UnreachableRegistry::new(Ecosystem::Npm, Some(npm_default.to_owned())).label(), + "npm" + ); + // A trailing slash is the same registry. + assert_eq!( + UnreachableRegistry::new(Ecosystem::Npm, Some(format!("{npm_default}/"))).label(), + "npm" + ); + // JSR is not npm's default registry, and a `deno.json` reaches both. + assert_eq!( + UnreachableRegistry::new(Ecosystem::Npm, Some("https://jsr.io".to_owned())).label(), + "npm (jsr.io)" + ); + assert_eq!( + UnreachableRegistry::new(Ecosystem::Rust, None).label(), + "Rust" + ); + } + + /// The security-relevant half. A registry root comes from `.dependable.toml` or + /// `.npmrc` — where `expand_env` interpolates `${VAR}` — and the refusal built from + /// it is printed on stderr, which CI captures as job output. Only host and port may + /// survive. + #[test] + fn a_registry_root_is_reduced_to_its_authority() { + let authority = |root| authority_of(root).unwrap_or_default(); + + // Userinfo is credentials, and never reaches the message. + assert_eq!( + authority("https://ci:secret@nexus.internal/repo/npm"), + "nexus.internal" + ); + assert_eq!(authority("https://token@nexus.internal"), "nexus.internal"); + // A password may legally contain an `@`; only the tail is the host. + assert_eq!( + authority("https://ci:p@ss@nexus.internal/x"), + "nexus.internal" + ); + // Port kept; path, query and fragment dropped. + assert_eq!( + authority("http://127.0.0.1:8080/repository/npm?x=1#frag"), + "127.0.0.1:8080" + ); + // An IPv6 literal keeps its brackets, and its port. + assert_eq!(authority("http://[::1]:8080/v1"), "[::1]:8080"); + assert_eq!(authority("https://[2001:db8::1]"), "[2001:db8::1]"); + // A root written with no scheme at all, which a hand-edited config may well be. + assert_eq!(authority("nexus.internal:8081/repo"), "nexus.internal:8081"); + // A trailing slash is not part of the host. + assert_eq!( + authority("https://registry.npmjs.org/"), + "registry.npmjs.org" + ); + + // Nothing to say is `None`, so the label falls back to the ecosystem rather than + // rendering an empty pair of brackets. + assert_eq!(authority_of(""), None); + assert_eq!(authority_of("https://"), None); + assert_eq!(authority_of("https:///path"), None); + assert_eq!( + UnreachableRegistry::new(Ecosystem::Go, Some(String::new())).label(), + "Go" + ); + } + + /// `buffer_unordered` completes in arrival order, so the collection is sorted before + /// it leaves `fetch_all`. Unsorted, the gate's sentence reorders itself between two + /// runs of the same repository. + #[test] + fn unreachable_registries_sort_by_ecosystem_then_root() { + let mut registries = [ + UnreachableRegistry::new(Ecosystem::Npm, Some("https://jsr.io".to_owned())), + UnreachableRegistry::new(Ecosystem::Go, None), + UnreachableRegistry::new(Ecosystem::Npm, None), + ]; + registries.sort_by(|a, b| unreachable_sort_key(a).cmp(&unreachable_sort_key(b))); + let labels: Vec = registries.iter().map(UnreachableRegistry::label).collect(); + assert_eq!(labels, ["Go", "npm", "npm (jsr.io)"]); + } } diff --git a/crates/dependable-fetch/src/lib.rs b/crates/dependable-fetch/src/lib.rs index 0ba6bc5..30e12a3 100644 --- a/crates/dependable-fetch/src/lib.rs +++ b/crates/dependable-fetch/src/lib.rs @@ -48,7 +48,9 @@ mod retry; pub mod tree; // High-level entry point (recommended for embedding). -pub use check::{CheckError, Checker, CheckerBuilder, ManifestCheck, ProgressEvent}; +pub use check::{ + CheckError, Checker, CheckerBuilder, ManifestCheck, ProgressEvent, UnreachableRegistry, +}; // Manifest discovery (filesystem; shared by every frontend). pub use discover::{ diff --git a/crates/dependable/src/output/mod.rs b/crates/dependable/src/output/mod.rs index 47df136..1e17c0e 100644 --- a/crates/dependable/src/output/mod.rs +++ b/crates/dependable/src/output/mod.rs @@ -34,16 +34,26 @@ pub struct ManifestReport { } /// How much of what a gate needs was actually established for one manifest. -#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] +/// +/// Not `Copy`: [`registry_unreachable`](Self::registry_unreachable) names the registries +/// that declined, and naming them costs an allocation. The code only ever cloned it. +#[derive(Debug, Clone, PartialEq, Eq, Default)] pub struct ScanIntegrity { /// The vulnerability scan was asked for and did not complete. pub vulnerability_scan_failed: bool, - /// A registry lookup failed for a reason other than the package not existing. + /// The registries that declined to answer — a lookup failed for a reason other than + /// the package not existing. /// /// This, and not the count below, is what a `--fail-on` gate cannot be honoured /// through: the registry declined to answer, so the run has no facts about the /// dependencies it asked about. - pub registry_unreachable: bool, + /// + /// Empty means **every registry this manifest routed to answered**, affirmatively — + /// not "nothing is known". Per registry rather than per manifest because a manifest + /// is not the routing unit: a `deno.json` reaches npm and JSR, a `Cargo.toml` + /// crates.io and any alternate registry its dependencies name. A bare flag could say + /// only that *something* declined, which names nothing the reader can go and fix. + pub registry_unreachable: Vec, /// How many dependencies a registry answered about by name: no such package. /// /// Reported, never gated on. Each is a permanent fact about one dependency — a diff --git a/crates/dependable/src/runner.rs b/crates/dependable/src/runner.rs index 1318f98..e7cba49 100644 --- a/crates/dependable/src/runner.rs +++ b/crates/dependable/src/runner.rs @@ -272,7 +272,7 @@ impl Engine { }; let integrity = ScanIntegrity { vulnerability_scan_failed: check.vulnerability_scan_failed, - registry_unreachable: check.registry_unreachable, + registry_unreachable: check.unreachable_registries.clone(), unresolved: count(ErrorOrigin::NotFound), unevaluated: count(ErrorOrigin::Local), }; @@ -1551,8 +1551,11 @@ fn gate_is_answerable(reports: &[ManifestReport], fail_on: FailOn) -> Result<(), // settings match specific statuses and skip errors entirely, which is where a run // that established nothing could still be reported as clean. if fail_on != FailOn::Any { - if reports.iter().any(|r| r.integrity.registry_unreachable) { - reasons.push("the registry did not answer".to_owned()); + let unanswered = unanswered_registries(reports); + // Empty pushes nothing at all — which is what keeps the 404 carve-out and the + // locally-unevaluable dependency below reaching the gate on their own terms. + if !unanswered.is_empty() { + reasons.push(name_unanswered(&unanswered)); } let unevaluated: usize = reports.iter().map(|r| r.integrity.unevaluated).sum(); if unevaluated > 0 { @@ -1568,6 +1571,50 @@ fn gate_is_answerable(reports: &[ManifestReport], fail_on: FailOn) -> Result<(), Err(join_reasons(&reasons)) } +/// Every registry that declined to answer anywhere in the run, deduplicated, in a stable +/// order, rendered as labels. +/// +/// The union across manifests, because the sentence the gate prints is about the run: a +/// polyglot repository is many [`ManifestReport`]s, and the same unreachable registry is +/// one fact however many manifests routed to it. Each check already sorts its own list, +/// but a union of sorted lists is not sorted, so this re-sorts on the same key. +fn unanswered_registries(reports: &[ManifestReport]) -> Vec { + let mut all: Vec<&dependable_fetch::UnreachableRegistry> = reports + .iter() + .flat_map(|r| r.integrity.registry_unreachable.iter()) + .collect(); + all.sort_by_key(|r| { + ( + r.ecosystem.display_name(), + r.root.clone().unwrap_or_default(), + ) + }); + all.dedup(); + let mut labels: Vec = all.iter().map(|r| r.label()).collect(); + // Two roots can reduce to one printed label — an unnamed root and the ecosystem's + // own default both print the bare ecosystem name — and the sentence must not say the + // same registry twice. + labels.dedup(); + labels +} + +/// `the a registry did not answer`, `the a and b registries did not answer`, +/// `the a, b and c registries did not answer`. +/// +/// The same connective grammar as [`join_reasons`], deliberately not the same function: +/// this string is one *element* of that list, and composing them would leave a sentence +/// whose commas belong to two different lists at once. +fn name_unanswered(labels: &[String]) -> String { + match labels { + [] => String::new(), + [only] => format!("the {only} registry did not answer"), + [rest @ .., last] => format!( + "the {} and {last} registries did not answer", + rest.join(", ") + ), + } +} + /// `a`, `a and b`, `a, b and c` — the gate's reasons read as a sentence. fn join_reasons(reasons: &[String]) -> String { match reasons { @@ -1822,6 +1869,12 @@ mod tests { ) } + /// A registry named only by its ecosystem — what a fetcher that opts out of + /// `registry_root` yields, and what the gate prints as a bare ecosystem name. + fn unreachable_default(ecosystem: Ecosystem) -> dependable_fetch::UnreachableRegistry { + dependable_fetch::UnreachableRegistry::new(ecosystem, None) + } + fn report_of( integrity: ScanIntegrity, results: Vec, @@ -1853,7 +1906,7 @@ mod tests { let reports = vec![report_with( ScanIntegrity { vulnerability_scan_failed: true, - registry_unreachable: false, + registry_unreachable: Vec::new(), unresolved: 0, unevaluated: 0, }, @@ -1874,7 +1927,7 @@ mod tests { let reports = vec![report_with( ScanIntegrity { vulnerability_scan_failed: false, - registry_unreachable: true, + registry_unreachable: vec![unreachable_default(Ecosystem::Rust)], unresolved: 0, unevaluated: 0, }, @@ -1914,7 +1967,7 @@ mod tests { let reports = vec![report_of( ScanIntegrity { vulnerability_scan_failed: false, - registry_unreachable: false, + registry_unreachable: Vec::new(), unresolved: 1, unevaluated: 0, }, @@ -1958,7 +2011,7 @@ mod tests { let reports = vec![report_of( ScanIntegrity { vulnerability_scan_failed: false, - registry_unreachable: false, + registry_unreachable: Vec::new(), unresolved: 0, unevaluated: 1, }, @@ -1990,7 +2043,7 @@ mod tests { let reports = vec![report_of( ScanIntegrity { vulnerability_scan_failed: true, - registry_unreachable: true, + registry_unreachable: vec![unreachable_default(Ecosystem::Rust)], unresolved: 0, unevaluated: 2, }, @@ -1998,11 +2051,134 @@ mod tests { )]; assert_eq!( gate_is_answerable(&reports, FailOn::Vulnerable).unwrap_err(), - "the vulnerability scan did not complete, the registry did not answer and 2 \ + "the vulnerability scan did not complete, the Rust registry did not answer and 2 \ dependencies could not be evaluated" ); } + /// The defect #112 was filed for. A run reaching two registries could say only that + /// "the registry" did not answer, so a polyglot repository whose Go proxy was down + /// while npm answered every request read as a run that had established nothing — + /// and named no host anyone could go and check. + #[test] + fn the_refusal_names_each_registry_that_declined() { + // A fresh report per call: `ManifestReport` is not `Clone`, and a shared fixture + // would say nothing about how a union across *separate* manifests behaves. + let report = |ecosystem| { + report_of( + ScanIntegrity { + vulnerability_scan_failed: false, + registry_unreachable: vec![unreachable_default(ecosystem)], + unresolved: 0, + unevaluated: 0, + }, + vec![], + ) + }; + + // One registry, one name. + assert_eq!( + gate_is_answerable(&[report(Ecosystem::Go)], FailOn::Vulnerable).unwrap_err(), + "the Go registry did not answer" + ); + + // The union across manifests, in sorted order rather than in arrival order — + // and reversing the reports must not reverse the sentence. + let both = [report(Ecosystem::Npm), report(Ecosystem::Go)]; + let reversed = [report(Ecosystem::Go), report(Ecosystem::Npm)]; + assert_eq!( + gate_is_answerable(&both, FailOn::Vulnerable).unwrap_err(), + "the Go and npm registries did not answer" + ); + assert_eq!( + gate_is_answerable(&reversed, FailOn::Vulnerable).unwrap_err(), + gate_is_answerable(&both, FailOn::Vulnerable).unwrap_err() + ); + + // The same registry reached from two manifests is one fact, not two. + let twice = [report(Ecosystem::Go), report(Ecosystem::Go)]; + assert_eq!( + gate_is_answerable(&twice, FailOn::Vulnerable).unwrap_err(), + "the Go registry did not answer" + ); + + // Three read as a list. + let three = [ + report(Ecosystem::Go), + report(Ecosystem::Npm), + report(Ecosystem::Jvm), + ]; + assert_eq!( + gate_is_answerable(&three, FailOn::Vulnerable).unwrap_err(), + "the Go, JVM and npm registries did not answer" + ); + + // The settings that were never blocked by this reason still are not. + assert!(gate_is_answerable(&both, FailOn::Any).is_ok()); + assert!(gate_is_answerable(&both, FailOn::None).is_ok()); + } + + /// A non-default root is named by its host, so two registries inside one ecosystem + /// are told apart — the `deno.json` case, where npm answers and JSR does not. + #[test] + fn a_registry_that_is_not_the_ecosystems_default_is_named_by_its_host() { + let reports = vec![report_of( + ScanIntegrity { + vulnerability_scan_failed: false, + registry_unreachable: vec![dependable_fetch::UnreachableRegistry::new( + Ecosystem::Npm, + Some("https://jsr.io".to_owned()), + )], + unresolved: 0, + unevaluated: 0, + }, + vec![], + )]; + assert_eq!( + gate_is_answerable(&reports, FailOn::Vulnerable).unwrap_err(), + "the npm (jsr.io) registry did not answer" + ); + } + + /// An empty collection pushes no reason at all. Without that the 404 carve-out and + /// the locally-unevaluable dependency would both be swallowed by a sentence about a + /// registry nothing had gone wrong with. + #[test] + fn no_unreachable_registry_contributes_no_reason() { + let reports = vec![report_of( + ScanIntegrity { + vulnerability_scan_failed: false, + registry_unreachable: Vec::new(), + unresolved: 3, + unevaluated: 1, + }, + vec![], + )]; + assert_eq!( + gate_is_answerable(&reports, FailOn::Vulnerable).unwrap_err(), + "1 dependency could not be evaluated" + ); + } + + /// The sentence fragments, in isolation, so the grammar is pinned without a gate + /// around it. + #[test] + fn the_unanswered_registries_read_as_a_sentence() { + let name = |labels: &[&str]| { + name_unanswered(&labels.iter().map(|l| (*l).to_owned()).collect::>()) + }; + assert_eq!(name(&[]), ""); + assert_eq!(name(&["Go"]), "the Go registry did not answer"); + assert_eq!( + name(&["Go", "npm"]), + "the Go and npm registries did not answer" + ); + assert_eq!( + name(&["Go", "JVM", "npm"]), + "the Go, JVM and npm registries did not answer" + ); + } + /// A complete run still gates on what it found, and still passes when it finds /// nothing — the guard must not turn every check into a failure. #[test] From 49649f25af4e7262d49d1e6a966651c39dca01dd Mon Sep 17 00:00:00 2001 From: Justin Chung Date: Sun, 6 Sep 2026 12:55:03 -0400 Subject: [PATCH 2/7] test: pin the refusal to the registry that actually declined MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two end-to-end tests the old boolean could not express. `checker.rs` drives a `deno.json` against two servers: npm answers the packument, JSR answers 500. Both routes are `Ecosystem::Npm`, so this is the case that separates a per-registry answer from a per-ecosystem one — the unreachable set names JSR alone, by JSR's own host, while the npm import is evaluated in full. `cli_gate.rs` runs the real binary with npm pointed at one loopback registry and Go at another, and asserts exit 2, that the refusal names the Go proxy, and — the assertion the issue is actually about — that it does not name npm, which answered every request. The 404/410 carve-out tests are unchanged apart from the pinned wording they assert against. Refs #112 --- crates/dependable-fetch/tests/checker.rs | 76 +++++++++++++++++++++ crates/dependable/tests/cli_gate.rs | 87 +++++++++++++++++++++++- 2 files changed, 161 insertions(+), 2 deletions(-) diff --git a/crates/dependable-fetch/tests/checker.rs b/crates/dependable-fetch/tests/checker.rs index 624765d..8071923 100644 --- a/crates/dependable-fetch/tests/checker.rs +++ b/crates/dependable-fetch/tests/checker.rs @@ -1284,3 +1284,79 @@ fn a_default_checker_writes_to_no_shared_cache() { "the disk cache must be opted into, not inherited" ); } + +/// #112: a manifest is not the routing unit, so "the registry did not answer" could not +/// say which one. A `deno.json` reaches npm *and* JSR, and this is the case that +/// distinguishes a per-registry answer from a per-ecosystem one end to end — both routes +/// are `Ecosystem::Npm`, one answered and one did not, and only the routing key can tell +/// them apart. +#[tokio::test] +async fn a_deno_manifest_names_the_jsr_registry_alone_when_only_jsr_declines() { + let npm = MockServer::start().await; + let jsr = MockServer::start().await; + + Mock::given(method("GET")) + .and(path("/chalk")) + .respond_with( + ResponseTemplate::new(200).set_body_string(r#"{"versions":{"5.0.0":{},"5.3.0":{}}}"#), + ) + .mount(&npm) + .await; + // JSR declines to answer at all. `any()` rather than a path matcher because the + // failure must not depend on how the fetcher spells `@std/path` in a URL. + Mock::given(wiremock::matchers::any()) + .respond_with(ResponseTemplate::new(500)) + .mount(&jsr) + .await; + + let client = build_client().unwrap(); + let checker = Checker::builder() + .http_client(client.clone()) + .registry( + Ecosystem::Npm, + Arc::new(NpmFetcher::with_registry(client.clone(), npm.uri())), + ) + .jsr_registry(Arc::new(JsrFetcher::with_registry(client, jsr.uri()))) + .vulnerabilities(false) + .build() + .unwrap(); + + let manifest = r#"{ "imports": { "chalk": "npm:chalk@^5.0.0", "p": "jsr:@std/path@^1.0.0" } }"#; + let check = checker + .check_manifest(ManifestKind::DenoJson, manifest, None) + .await + .unwrap(); + + // The npm half is fully evaluated. Nothing about JSR's outage reaches it. + let chalk = check + .results + .iter() + .find(|r| r.item.name == "chalk") + .expect("the npm import"); + assert_eq!(chalk.latest_available.as_deref(), Some("5.3.0")); + assert!( + !matches!(chalk.status, DependencyStatus::Error(_)), + "the npm route answered, so its result must not be an error: {:?}", + chalk.status + ); + + // Exactly one registry declined, and it is JSR — named by its own root, not by npm's. + assert_eq!(check.unreachable_registries.len(), 1, "{check:?}"); + let declined = &check.unreachable_registries[0]; + assert_eq!(declined.ecosystem, Ecosystem::Npm); + assert_eq!(declined.root.as_deref(), Some(jsr.uri().as_str())); + assert!( + !declined.is_default_root(), + "JSR is not npm's default registry" + ); + + // The label a refusal prints names the JSR host, and never the npm one. + let label = declined.label(); + let jsr_authority = jsr.uri().trim_start_matches("http://").to_owned(); + let npm_authority = npm.uri().trim_start_matches("http://").to_owned(); + assert!(label.contains(&jsr_authority), "label was {label}"); + assert!(!label.contains(&npm_authority), "label was {label}"); + + // The boolean still summarises the collection, and cannot disagree with it. + assert!(check.registry_unreachable); +} diff --git a/crates/dependable/tests/cli_gate.rs b/crates/dependable/tests/cli_gate.rs index 8812533..2e93a2b 100644 --- a/crates/dependable/tests/cli_gate.rs +++ b/crates/dependable/tests/cli_gate.rs @@ -127,6 +127,22 @@ fn write_config(dir: &Path, base: &str) -> PathBuf { config } +/// Point npm at one base and Go at another, so a run reaches two registries that can +/// fail independently. The single-base [`write_config`] cannot express that, and it is +/// exactly the shape #112 is about. +fn write_split_config(dir: &Path, npm: &str, go: &str) -> PathBuf { + let config = dir.join(".dependable.toml"); + fs::write( + &config, + format!( + "[npm]\nregistry = \"{npm}\"\n\n[go]\nregistry = \"{go}\"\n\n\ + [vulnerability]\nenabled = false\n" + ), + ) + .unwrap(); + config +} + fn check(dir: &Path, config: &Path, args: &[&str]) -> Output { let mut command = Command::new(env!("CARGO_BIN_EXE_dependable")); command @@ -192,7 +208,7 @@ fn a_go_module_the_proxy_answers_410_for_does_not_break_the_gate() { "a 410 was not read as an absent module:\n{stdout}" ); assert!( - !stderr.contains("the registry did not answer"), + !stderr.contains("did not answer"), "one private module marked the whole registry unreachable:\n{stderr}" ); assert!( @@ -444,11 +460,78 @@ fn a_metadata_document_listing_no_versions_is_not_exempt_from_the_gate() { assert_eq!(code, 2, "stdout: {stdout}\nstderr: {stderr}"); assert!( - stderr.contains("error: cannot honour --fail-on: the registry did not answer"), + stderr.contains("error: cannot honour --fail-on: the JVM ("), "stderr: {stderr}" ); + assert!( + stderr.contains(") registry did not answer"), + "the refusal did not name the registry it could not read:\nstderr: {stderr}" + ); assert!( !stderr.contains("not found in its registry"), "an answered-but-empty document was reported as a 404:\n{stderr}" ); } + +// --------------------------------------------------------------------------- +// One registry declines; the other answers +// --------------------------------------------------------------------------- + +/// #112. `registry_unreachable` was one boolean per manifest, so a run whose Go proxy +/// timed out while npm answered every request could say only "the registry did not +/// answer" — naming no host, and reading as though nothing at all had been established. +/// +/// The negative assertion is the whole issue: npm answered, and the refusal must not +/// implicate it. +#[test] +fn the_refusal_names_the_registry_that_declined_and_not_the_one_that_answered() { + let dir = workdir("gate_split_registries"); + let npm_base = registry(vec![( + "/express".to_string(), + packument("express", &["4.19.2"], "4.19.2"), + )]); + // A proxy that answers 500 has not said "no such module"; it has declined to answer. + let go_base = registry(vec![ + ("/github.com/acme/thing/@v/list".to_string(), status(500)), + ("/github.com/acme/thing/@latest".to_string(), status(500)), + ]); + let config = write_split_config(&dir, &npm_base, &go_base); + fs::write( + dir.join("package.json"), + "{\"name\":\"app\",\"dependencies\":{\"express\":\"^4.19.0\"}}\n", + ) + .unwrap(); + fs::write( + dir.join("go.mod"), + "module example.com/app\n\ngo 1.22\n\nrequire github.com/acme/thing v0.1.0\n", + ) + .unwrap(); + + let output = check(&dir, &config, &["--fail-on", "vulnerable"]); + let (stdout, stderr, code) = outcome(&output); + + assert_eq!(code, 2, "stdout: {stdout}\nstderr: {stderr}"); + let refusal = stderr + .lines() + .find(|line| line.contains("cannot honour --fail-on")) + .unwrap_or_else(|| panic!("no refusal on stderr:\n{stderr}")); + + // The Go proxy is named, by its own host. + let go_authority = go_base.trim_start_matches("http://"); + assert!( + refusal.contains(&format!("the Go ({go_authority}) registry did not answer")), + "the refusal did not name the Go proxy:\n{refusal}" + ); + // npm answered every request, and must not be implicated in the refusal. + let npm_authority = npm_base.trim_start_matches("http://"); + assert!( + !refusal.contains("npm"), + "npm answered, but the refusal blamed it:\n{refusal}" + ); + assert!( + !refusal.contains(npm_authority), + "the refusal named the registry that answered:\n{refusal}" + ); + // The manifest whose registry answered is still evaluated in full. + assert!(stdout.contains("express"), "stdout: {stdout}"); +} From 51e7f26715c84721ffb697f9ca47a761dbc659c5 Mon Sep 17 00:00:00 2001 From: Justin Chung Date: Sun, 6 Sep 2026 12:55:03 -0400 Subject: [PATCH 3/7] docs: show which registry a gate refusal names The exit-code section said a run that could not reach a registry exits 2, but not what it prints. It now shows the one- and two-registry sentences, says that a non-default root is named beside its ecosystem and reduced to host and port so a credential in a configured root cannot reach CI output, and repeats that a 404 or 410 is an answer rather than an outage. Refs #112 --- README.md | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/README.md b/README.md index 6453570..ff15581 100644 --- a/README.md +++ b/README.md @@ -635,6 +635,29 @@ success on the run it could not perform is worse than no gate at all. With no ga armed, an unreachable registry is still reported per dependency and the run exits `0`, because nothing was promised. +The refusal names the registries that declined, not just the fact that one did: + +```console +$ dependable check --fail-on vulnerable +error: cannot honour --fail-on: the Go registry did not answer +``` + +```console +$ dependable check --fail-on vulnerable +error: cannot honour --fail-on: the Go and npm registries did not answer +``` + +A run reaches more than one registry — a polyglot repository has one per ecosystem, +a `deno.json` reaches npm and JSR, and a `Cargo.toml` reaches crates.io alongside any +alternate registry its dependencies name — so a registry that is not the ecosystem's +default one is named beside it (`the npm (jsr.io) registry did not answer`). Only the +host and port are printed, never the configured URL, because a registry root may carry +credentials and this line lands in CI job output. + +A registry that answered `404` (or, for a Go proxy, `410`) *answered*: a private, +internal or deleted package is a per-dependency fact, reported in the table and noted +on stderr, and it does not make a gate unanswerable. + `.dependable.toml` is validated: an unknown key or a wrong-typed value is an error, not a silent fallback to defaults. One mistyped character used to reset `[global] fail_on` to `none` and disarm the gate with nothing on stderr. From df57da511e235521d833d42b9489994aa199e598 Mon Sep 17 00:00:00 2001 From: Justin Chung Date: Sun, 6 Sep 2026 13:04:52 -0400 Subject: [PATCH 4/7] fix(check): stop a registry root's credentials reaching the refusal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four defects raised against the previous commit before review. `authority_of` split the path off before stripping userinfo, so a password containing `/` — a base64 token contains one about 40% of the time at 32 characters — survived as the "host": `https://ci:AbC/dEf@nexus.internal/npm` reduced to `ci:AbC`, and the gate printed a username and a token prefix on stderr, where CI captures it as job output. The same held for `?` and `#`. This is a redaction, so it now fails towards saying less: the authority candidate is taken first, and an `@` surviving outside it means the string cannot be split into userinfo and host with any confidence, so the root is dropped and the ecosystem named alone. `\` joins the separators because WHATWG resolves it as `/` for special schemes, so reading it as part of the authority named a host the run never contacted. `unanswered_registries` deduplicated labels by adjacency under a sort keyed on the raw root, but several roots reduce to one label and need not be adjacent: three Rust registries at `http://nexus.corp/a`, `http://other.host/x` and `https://nexus.corp/b` sort in exactly that order, and the sentence named `nexus.corp` twice. Labels are now sorted before they are deduplicated, which also makes the printed order the order a reader sees. `ManifestCheck::unreachable_registries` promised deduplication per registry but deduplicated per route, so two alternate-registry aliases naming one index URL reached a library consumer as two identical entries. The CLI masked it. `label()` now says what an authority-only name cannot distinguish — two paths on one host — and `name_unanswered` no longer claims to have avoided a nesting it in fact produces. Refs #112 --- README.md | 7 ++- crates/dependable-fetch/src/check.rs | 85 +++++++++++++++++++++++++--- crates/dependable/src/runner.rs | 52 +++++++++++++++-- 3 files changed, 127 insertions(+), 17 deletions(-) diff --git a/README.md b/README.md index ff15581..349d6d9 100644 --- a/README.md +++ b/README.md @@ -650,9 +650,10 @@ error: cannot honour --fail-on: the Go and npm registries did not answer A run reaches more than one registry — a polyglot repository has one per ecosystem, a `deno.json` reaches npm and JSR, and a `Cargo.toml` reaches crates.io alongside any alternate registry its dependencies name — so a registry that is not the ecosystem's -default one is named beside it (`the npm (jsr.io) registry did not answer`). Only the -host and port are printed, never the configured URL, because a registry root may carry -credentials and this line lands in CI job output. +default one is named beside it (`the npm (jsr.io) registry did not answer`). This line +prints only the host and port, never the configured URL, because a registry root may +carry credentials and the line lands in CI job output. (The per-dependency error text in +the table is a separate matter: it carries whatever the HTTP client put in its message.) A registry that answered `404` (or, for a Go proxy, `410`) *answered*: a private, internal or deleted package is a per-dependency fact, reported in the table and noted diff --git a/crates/dependable-fetch/src/check.rs b/crates/dependable-fetch/src/check.rs index de713ac..23e7bdb 100644 --- a/crates/dependable-fetch/src/check.rs +++ b/crates/dependable-fetch/src/check.rs @@ -147,6 +147,11 @@ impl UnreachableRegistry { /// configured root may legitimately carry credentials — `.npmrc` interpolates /// `${VAR}` into its `registry` lines, and `.dependable.toml` roots are written by /// hand — and this string is printed on stderr, which CI captures as job output. + /// + /// The price of that reduction is that two registries on one host — a Nexus serving + /// `/repository/crates-a` and `/repository/crates-b` — share a label. They stay + /// distinct in the data; a caller counting registries must count + /// [`ManifestCheck::unreachable_registries`], not labels. #[must_use] pub fn label(&self) -> String { let name = self.ecosystem.display_name(); @@ -173,20 +178,41 @@ fn unreachable_sort_key(registry: &UnreachableRegistry) -> (&str, &str) { } /// The scheme-less authority of a URL-ish string: host and port, with userinfo, path, -/// query and fragment dropped. `None` when nothing is left. +/// query and fragment dropped. `None` when nothing usable is left. /// /// Hand-rolled over `&str` rather than taken from a URL crate: this workspace depends on /// none, and the input is not guaranteed to parse as a URL anyway — a configured root /// may carry an unexpanded `${VAR}`, or no scheme at all. +/// +/// This is a **redaction**, so it fails towards saying less. Splitting the path off first +/// and stripping userinfo second leaks a credential whenever a hand-written password +/// contains `/`, which base64 tokens routinely do: +/// `https://ci:se/cret@nexus.internal/repo` would have reduced to `ci:se`. So the +/// authority candidate is taken first, and an `@` surviving *outside* it means the string +/// cannot be split into userinfo and host with any confidence — the root is then dropped +/// rather than guessed at. fn authority_of(root: &str) -> Option { let after_scheme = root.split_once("://").map_or(root, |(_, rest)| rest); - let authority = after_scheme.split(['/', '?', '#']).next().unwrap_or(""); - // Userinfo is credentials. The *last* `@` wins, because a password may legally - // contain one and only the tail is the host. - let host = authority - .rsplit_once('@') - .map_or(authority, |(_, h)| h) - .trim(); + // The authority is everything before the path, query or fragment. + // `\` is included because WHATWG resolves it as `/` for special schemes, so + // `http://evil.example\@real.internal/x` reaches `evil.example` — reading it as part + // of the authority would print a host the run never contacted. + let end = after_scheme + .find(['/', '?', '#', '\\']) + .unwrap_or(after_scheme.len()); + let (candidate, rest) = after_scheme.split_at(end); + + let host = match candidate.rsplit_once('@') { + // Userinfo, correctly delimited. The *last* `@` wins: a password may legally + // contain one, and only the tail is the host. + Some((_, host)) => host, + // No `@` in the authority, but one further on. Either that is a path containing + // `@`, or it is a password containing `/`, and nothing here can tell them apart. + // Say the ecosystem's name alone rather than risk saying a secret. + None if rest.contains('@') => return None, + None => candidate, + }; + let host = host.trim(); (!host.is_empty()).then(|| host.to_owned()) } @@ -1005,6 +1031,11 @@ impl Checker { // reordering itself between two runs of the same repository. let mut unreachable: Vec = unreachable.into_values().collect(); unreachable.sort_by(|a, b| unreachable_sort_key(a).cmp(&unreachable_sort_key(b))); + // Per *registry*, which is what the field promises — the map is keyed by cache + // key, and `route_item` gives each alternate-registry alias its own key, so two + // aliases naming one index URL arrive here as two identical entries. Equal values + // sort adjacently under the key above, so an adjacent dedup is total. + unreachable.dedup(); (out, unreachable) } @@ -2407,6 +2438,38 @@ mod tests { "registry.npmjs.org" ); + // The reduction fails towards saying less. A hand-written password containing + // `/` — base64 tokens routinely do — cannot be told apart from a path, so the + // root is dropped entirely rather than reduced to the credential fragment + // `ci:se`. + assert_eq!(authority_of("https://ci:se/cret@nexus.internal/repo"), None); + assert_eq!( + authority_of("https://ci:AbC/dEf+gh=@nexus.internal/npm"), + None + ); + assert_eq!( + authority_of("sparse+https://ci:tok/en@nexus.internal/index"), + None + ); + assert_eq!(authority_of("https://a/b@c"), None); + // `?` and `#` end the authority too, so a token containing either takes the same + // route. + assert_eq!(authority_of("https://ci:p?ss@nexus.internal/x"), None); + assert_eq!(authority_of("https://ci:p#ss@nexus.internal/x"), None); + // WHATWG resolves `\\` as `/` for special schemes, so this URL reaches + // `evil.example`. Reading the backslash as part of the authority named + // `real.internal` — a host the run never contacted. + assert_eq!(authority_of("http://evil.example\\@real.internal/x"), None); + assert_eq!( + UnreachableRegistry::new( + Ecosystem::Npm, + Some("https://ci:se/cret@nexus.internal/repo".to_owned()) + ) + .label(), + "npm", + "a credential fragment must never reach the message" + ); + // Nothing to say is `None`, so the label falls back to the ecosystem rather than // rendering an empty pair of brackets. assert_eq!(authority_of(""), None); @@ -2423,12 +2486,16 @@ mod tests { /// runs of the same repository. #[test] fn unreachable_registries_sort_by_ecosystem_then_root() { - let mut registries = [ + let mut registries = vec![ UnreachableRegistry::new(Ecosystem::Npm, Some("https://jsr.io".to_owned())), UnreachableRegistry::new(Ecosystem::Go, None), UnreachableRegistry::new(Ecosystem::Npm, None), + // A second alternate-registry alias naming the same index: one registry, + // two routes, two cache keys — and one entry in what is published. + UnreachableRegistry::new(Ecosystem::Go, None), ]; registries.sort_by(|a, b| unreachable_sort_key(a).cmp(&unreachable_sort_key(b))); + registries.dedup(); let labels: Vec = registries.iter().map(UnreachableRegistry::label).collect(); assert_eq!(labels, ["Go", "npm", "npm (jsr.io)"]); } diff --git a/crates/dependable/src/runner.rs b/crates/dependable/src/runner.rs index e7cba49..8d7c0d7 100644 --- a/crates/dependable/src/runner.rs +++ b/crates/dependable/src/runner.rs @@ -1591,9 +1591,17 @@ fn unanswered_registries(reports: &[ManifestReport]) -> Vec { }); all.dedup(); let mut labels: Vec = all.iter().map(|r| r.label()).collect(); - // Two roots can reduce to one printed label — an unnamed root and the ecosystem's - // own default both print the bare ecosystem name — and the sentence must not say the - // same registry twice. + // Sorted *again*, by label, before deduplicating. Several roots reduce to one printed + // label — an unnamed root and the ecosystem's own default both print the bare + // ecosystem name, and two paths on one host share an authority — and those roots need + // not be adjacent under the root-ordered sort above. Three Rust registries at + // `http://nexus.corp/a`, `http://other.host/x` and `https://nexus.corp/b` sort in + // exactly that order (`:` sorts before `s`, so `http://` precedes `https://`), and an + // adjacency-only dedup left the sentence naming `nexus.corp` twice. + // + // It also makes the printed order the order a reader sees, rather than the order of + // roots they are never shown. + labels.sort(); labels.dedup(); labels } @@ -1602,8 +1610,14 @@ fn unanswered_registries(reports: &[ManifestReport]) -> Vec { /// `the a, b and c registries did not answer`. /// /// The same connective grammar as [`join_reasons`], deliberately not the same function: -/// this string is one *element* of that list, and composing them would leave a sentence -/// whose commas belong to two different lists at once. +/// this string is one *element* of that list, and `join_reasons` over the registry labels +/// alone would render three of them as a bare `Go, JVM and npm` with no sentence round it. +/// +/// The nesting is accepted rather than avoided. With three registries and another reason +/// the commas do belong to two lists at once — `the vulnerability scan did not complete, +/// the Go, JVM and npm registries did not answer and 2 dependencies could not be +/// evaluated` — which reads worse than either list alone, and better than a run that +/// cannot say which registry declined. fn name_unanswered(labels: &[String]) -> String { match labels { [] => String::new(), @@ -2118,6 +2132,34 @@ mod tests { assert!(gate_is_answerable(&both, FailOn::None).is_ok()); } + /// Several roots reduce to one printed label, and under a root-ordered sort they need + /// not be adjacent: `http://nexus.corp/a`, `http://other.host/x` and + /// `https://nexus.corp/b` sort in exactly that order, because `:` sorts before `s`. + /// An adjacency-only dedup named `nexus.corp` twice in one sentence. + #[test] + fn one_registry_is_never_named_twice_however_its_roots_sort() { + let rust = |root: &str| { + dependable_fetch::UnreachableRegistry::new(Ecosystem::Rust, Some(root.to_owned())) + }; + let reports = [report_of( + ScanIntegrity { + vulnerability_scan_failed: false, + registry_unreachable: vec![ + rust("http://nexus.corp/a"), + rust("http://other.host/x"), + rust("https://nexus.corp/b"), + ], + unresolved: 0, + unevaluated: 0, + }, + vec![], + )]; + assert_eq!( + gate_is_answerable(&reports, FailOn::Vulnerable).unwrap_err(), + "the Rust (nexus.corp) and Rust (other.host) registries did not answer" + ); + } + /// A non-default root is named by its host, so two registries inside one ecosystem /// are told apart — the `deno.json` case, where npm answers and JSR does not. #[test] From 16a0ffb2196714ad2d6ec4223bef2094c78bb583 Mon Sep 17 00:00:00 2001 From: Justin Chung Date: Sun, 6 Sep 2026 13:55:34 -0400 Subject: [PATCH 5/7] fix(check): stop an `@` in both userinfo and path leaking a credential MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `authority_of` asked whether an `@` survived past the authority as a match guard on the `None` arm of the userinfo split, so the question was put only when the authority candidate held no `@` of its own. A root whose userinfo and whose path both contain one took the `Some` arm instead and reduced to the text between the embedded `@` and the first delimiter — a fragment of the token, and a host the run never contacted: https://ci:AbC@dEf/ghi@nexus.internal/npm -> dEf Such a root makes every request to it fail, so the refusal printed it on every gated run, into CI job output. Seven shapes leaked, across `/`, `?`, `#` and `\\` as the delimiter. Hoist the check above the split so it applies unconditionally. Whenever `authority_of` now returns `Some`, every `@` in the string lies inside the authority, so what survives is a host and port and never userinfo. Verified over a 40-input corpus: the seven leaking rows become `None` and no other row moves, including every row this branch already asserted on. The seven are added as regression cases, and `authority_of`'s doc comment and the README line describing the refusal are corrected — the README now also says that the line degrades to the bare ecosystem name rather than guessing. --- README.md | 8 +++- crates/dependable-fetch/src/check.rs | 57 +++++++++++++++++++++++++--- 2 files changed, 58 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index e3d0bbd..35d748f 100644 --- a/README.md +++ b/README.md @@ -667,8 +667,12 @@ a `deno.json` reaches npm and JSR, and a `Cargo.toml` reaches crates.io alongsid alternate registry its dependencies name — so a registry that is not the ecosystem's default one is named beside it (`the npm (jsr.io) registry did not answer`). This line prints only the host and port, never the configured URL, because a registry root may -carry credentials and the line lands in CI job output. (The per-dependency error text in -the table is a separate matter: it carries whatever the HTTP client put in its message.) +carry credentials and the line lands in CI job output. Where host and port cannot be +recovered with confidence — a root whose path contains an `@`, which an ordinary Nexus +npm proxy path does — the reduction says *less* rather than guessing, and the line falls +back to the bare ecosystem name (`the npm registry did not answer`), indistinguishable +from a fetcher that names no root at all. (The per-dependency error text in the table is +a separate matter: it carries whatever the HTTP client put in its message.) A registry that answered `404` (or, for a Go proxy, `410`) *answered*: a private, internal or deleted package is a per-dependency fact, reported in the table and noted diff --git a/crates/dependable-fetch/src/check.rs b/crates/dependable-fetch/src/check.rs index 23e7bdb..9cf943a 100644 --- a/crates/dependable-fetch/src/check.rs +++ b/crates/dependable-fetch/src/check.rs @@ -191,6 +191,14 @@ fn unreachable_sort_key(registry: &UnreachableRegistry) -> (&str, &str) { /// authority candidate is taken first, and an `@` surviving *outside* it means the string /// cannot be split into userinfo and host with any confidence — the root is then dropped /// rather than guessed at. +/// +/// That question is asked **unconditionally**, before the candidate is split at all, and +/// not only when the candidate holds no `@` of its own. Both halves can hold one: +/// `https://ci:AbC@dEf/ghi@nexus.internal/npm` is a token containing `@` *and* a path +/// containing `@`, and splitting its candidate at the last `@` yields `dEf` — part of the +/// token, and a host the run never contacted. Whenever this returns `Some`, the whole +/// string's `@`s lie inside the authority, so what survives is a host and port and never +/// a fragment of userinfo. fn authority_of(root: &str) -> Option { let after_scheme = root.split_once("://").map_or(root, |(_, rest)| rest); // The authority is everything before the path, query or fragment. @@ -202,14 +210,21 @@ fn authority_of(root: &str) -> Option { .unwrap_or(after_scheme.len()); let (candidate, rest) = after_scheme.split_at(end); + // An `@` surviving past the authority is checked *before* the split, not as a guard + // on its `None` arm. Either it is a path containing `@`, or it is userinfo containing + // a delimiter, and nothing here can tell them apart — so the root is dropped whether + // or not the candidate also holds an `@`. Asking only when the candidate holds none + // would let `https://ci:AbC@dEf/ghi@nexus.internal/npm` split at the embedded `@` and + // print `dEf`, a fragment of the token. Say the ecosystem's name alone instead. + if rest.contains('@') { + return None; + } + let host = match candidate.rsplit_once('@') { // Userinfo, correctly delimited. The *last* `@` wins: a password may legally - // contain one, and only the tail is the host. + // contain one, and only the tail is the host — but only once the check above has + // established that the authority holds every `@` in the string. Some((_, host)) => host, - // No `@` in the authority, but one further on. Either that is a path containing - // `@`, or it is a password containing `/`, and nothing here can tell them apart. - // Say the ecosystem's name alone rather than risk saying a secret. - None if rest.contains('@') => return None, None => candidate, }; let host = host.trim(); @@ -2460,6 +2475,38 @@ mod tests { // `evil.example`. Reading the backslash as part of the authority named // `real.internal` — a host the run never contacted. assert_eq!(authority_of("http://evil.example\\@real.internal/x"), None); + + // An `@` past the authority drops the root *whether or not* the authority holds + // one of its own. Asked only when the authority holds none — as a guard on the + // `None` arm of the split below — every one of these reduced to a fragment of + // the credential instead: `ss`, `dEf`, `s`, `s`, `s`, `en`, `host.example:pw`. + assert_eq!( + authority_of("https://user:p@ss/word@nexus.internal/x"), + None + ); + assert_eq!( + authority_of("https://ci:AbC@dEf/ghi@nexus.internal/npm"), + None + ); + assert_eq!(authority_of("https://ci:p@s?s@nexus.internal/x"), None); + assert_eq!(authority_of("https://ci:p@s#s@nexus.internal/x"), None); + assert_eq!(authority_of("https://ci:p@s\\s@nexus.internal/x"), None); + assert_eq!(authority_of("https://ci:tok@en/@nexus.internal/npm"), None); + // Not only a fragment: `host.example:pw` is a host the run never contacted, with + // the password glued to it as a port. + assert_eq!( + authority_of("https://user@host.example:pw/x@real.internal/y"), + None + ); + assert_eq!( + UnreachableRegistry::new( + Ecosystem::Npm, + Some("https://ci:AbC@dEf/ghi@nexus.internal/npm".to_owned()) + ) + .label(), + "npm", + "an `@` in both the userinfo and the path must not print `dEf`" + ); assert_eq!( UnreachableRegistry::new( Ecosystem::Npm, From 3457a3f506f3b33309b1dfcb1b7b8816df9c5a96 Mon Sep 17 00:00:00 2001 From: Justin Chung Date: Sun, 6 Sep 2026 13:55:44 -0400 Subject: [PATCH 6/7] docs(check): warn on `UnreachableRegistry::root` that it may carry credentials MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `label()`'s doc explains at length that a root may carry credentials and must never be printed whole, but a consumer reading the field it is handed got no signal at all — its doc said only where the root came from. The argument that a second consumer must not have to reimplement the redaction needs the field itself to say so, and to name `label()` as the only safe rendering. --- crates/dependable-fetch/src/check.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/crates/dependable-fetch/src/check.rs b/crates/dependable-fetch/src/check.rs index 9cf943a..9dfbea3 100644 --- a/crates/dependable-fetch/src/check.rs +++ b/crates/dependable-fetch/src/check.rs @@ -113,6 +113,13 @@ pub struct UnreachableRegistry { /// `None` when the fetcher opts out of [`RegistryFetcher::registry_root`], whose /// trait default returns nothing. The ecosystem is then the whole of what can be /// said, and [`label`](Self::label) says exactly that rather than an empty string. + /// + /// **This is the configured root, verbatim, and it may carry credentials** — an + /// `.npmrc` `registry` line interpolates `${VAR}`, and `.dependable.toml` roots are + /// written by hand. Never render it, whole or in part, anywhere a person or a CI job + /// log can see: [`label`](Self::label) is the only safe rendering, and it exists so + /// that a second consumer does not have to reimplement the redaction. Use the field + /// for identity — comparison, keying, counting — not for display. pub root: Option, } From 4dc00586865371aa6954de40f43e927d965aa6c9 Mon Sep 17 00:00:00 2001 From: Justin Chung Date: Sun, 6 Sep 2026 13:55:59 -0400 Subject: [PATCH 7/7] test(check): bind `fetch_all`'s published ordering at the site that produces it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `unreachable_registries_sort_by_ecosystem_then_root` re-applied `sort_by(unreachable_sort_key)` and `dedup()` in its own body over a hand-built vector, so it exercised the key and not `fetch_all`'s use of it. The one integration test that reached the production site yielded a single entry, so it passed either way: deleting both lines from `fetch_all` left the whole suite green, because the CLI re-sorts and re-dedups downstream and that half is covered. The library's published ordering — which an embedding consumer reads straight off the value — was untested. Add an integration test in which both of a `deno.json`'s registries decline, and assert the returned `unreachable_registries` order directly. Both routes are `Ecosystem::Npm`, so only the root can order them. The outcomes are gathered in a `HashMap`, whose iteration order is seeded per map, so one check would agree by luck half the time; the check is repeated concurrently through fresh `Checker`s, which costs one round of retry backoff and drives an unsorted `fetch_all`'s chance of passing to 1 in 256. Measured: 30/30 runs pass as written, 30/30 fail with the sort deleted from `fetch_all`. The unit test keeps its coverage of the key and the dedup, with its doc comment corrected to claim only that. --- crates/dependable-fetch/src/check.rs | 11 ++- crates/dependable-fetch/tests/checker.rs | 106 +++++++++++++++++++++++ 2 files changed, 114 insertions(+), 3 deletions(-) diff --git a/crates/dependable-fetch/src/check.rs b/crates/dependable-fetch/src/check.rs index 9dfbea3..5815419 100644 --- a/crates/dependable-fetch/src/check.rs +++ b/crates/dependable-fetch/src/check.rs @@ -2535,9 +2535,14 @@ mod tests { ); } - /// `buffer_unordered` completes in arrival order, so the collection is sorted before - /// it leaves `fetch_all`. Unsorted, the gate's sentence reorders itself between two - /// runs of the same repository. + /// The ordering key itself: ecosystem name, then root, with equal entries adjacent so + /// a plain `dedup` is total. + /// + /// This exercises `unreachable_sort_key`, not `fetch_all`'s use of it — the sort and + /// the dedup are applied here, in the test's own body, over a hand-built vector. That + /// the collection actually leaves `fetch_all` in this order is a separate claim, and + /// `a_deno_manifest_orders_both_declining_registries_by_root` in `tests/checker.rs` + /// is what holds it. #[test] fn unreachable_registries_sort_by_ecosystem_then_root() { let mut registries = vec![ diff --git a/crates/dependable-fetch/tests/checker.rs b/crates/dependable-fetch/tests/checker.rs index 8071923..691b04e 100644 --- a/crates/dependable-fetch/tests/checker.rs +++ b/crates/dependable-fetch/tests/checker.rs @@ -1360,3 +1360,109 @@ async fn a_deno_manifest_names_the_jsr_registry_alone_when_only_jsr_declines() { // The boolean still summarises the collection, and cannot disagree with it. assert!(check.registry_unreachable); } + +/// #112: the order `ManifestCheck::unreachable_registries` is published in is a contract +/// an embedding consumer reads straight off the value, so it is asserted where +/// `fetch_all` produces it rather than over a vector a test sorted for itself. +/// +/// `buffer_unordered` completes in arrival order and the outcomes are gathered in a +/// `HashMap`, so without the sort in `fetch_all` the published order is whatever the +/// network and the hasher did — and the sentence a `--fail-on` refusal prints would +/// reorder itself between two runs of the same repository. Both routes here are +/// `Ecosystem::Npm`, so it is the root that has to order them, and only `fetch_all` can +/// do it. +#[tokio::test] +async fn a_deno_manifest_orders_both_declining_registries_by_root() { + /// Each `HashMap` draws its own seed, so one check would agree with the contract + /// half the time by luck. Repeating drives an unsorted `fetch_all`'s chance of + /// passing to 1 in 2^ATTEMPTS. The attempts run concurrently against the same two + /// servers, so the test still costs one round of retry backoff. + const ATTEMPTS: usize = 8; + const MANIFEST: &str = + r#"{ "imports": { "chalk": "npm:chalk@^5.0.0", "p": "jsr:@std/path@^1.0.0" } }"#; + + let first = MockServer::start().await; + let second = MockServer::start().await; + // `any()` rather than a path matcher: the failure must not depend on how either + // fetcher spells a package name in a URL. + for server in [&first, &second] { + Mock::given(wiremock::matchers::any()) + .respond_with(ResponseTemplate::new(500)) + .mount(server) + .await; + } + // The two roots differ only in an ephemeral port the OS chose, so the roles go to + // whichever server sorts first. That fixes the expected order without the test + // asserting anything about port allocation — and the servers are interchangeable, + // since both decline every request. + let (npm, jsr) = if first.uri() < second.uri() { + (first, second) + } else { + (second, first) + }; + let npm_root = npm.uri(); + let jsr_root = jsr.uri(); + + let mut attempts = Vec::with_capacity(ATTEMPTS); + for _ in 0..ATTEMPTS { + let client = build_client().unwrap(); + // A fresh `Checker` per attempt: a warm versions cache issues no request, so a + // reused one would decline nothing and prove nothing. + let checker = Checker::builder() + .http_client(client.clone()) + .registry( + Ecosystem::Npm, + Arc::new(NpmFetcher::with_registry(client.clone(), npm_root.clone())), + ) + .jsr_registry(Arc::new(JsrFetcher::with_registry( + client, + jsr_root.clone(), + ))) + .vulnerabilities(false) + .build() + .unwrap(); + attempts.push(tokio::spawn(async move { + checker + .check_manifest(ManifestKind::DenoJson, MANIFEST, None) + .await + .unwrap() + })); + } + + for attempt in attempts { + let check = attempt.await.unwrap(); + + // Both registries declined, and they arrive in root order — asserted against the + // returned value itself, not against a vector this test sorted. + let roots: Vec<&str> = check + .unreachable_registries + .iter() + .map(|r| r.root.as_deref().unwrap_or_default()) + .collect(); + assert_eq!( + roots, + [npm_root.as_str(), jsr_root.as_str()], + "unreachable_registries must arrive sorted by ecosystem then root: {check:?}" + ); + assert!( + check + .unreachable_registries + .iter() + .all(|r| r.ecosystem == Ecosystem::Npm), + "both routes belong to the npm ecosystem: {check:?}" + ); + + // Neither route answered, so neither dependency could be evaluated, and the + // boolean still summarises the collection. + assert!(check.registry_unreachable); + assert_eq!(check.results.len(), 2, "{check:?}"); + assert!( + check + .results + .iter() + .all(|r| matches!(r.status, DependencyStatus::Error(_))), + "{:?}", + check.results.iter().map(|r| &r.status).collect::>() + ); + } +}