diff --git a/docs/docs/repositoryTypes/npm/index.md b/docs/docs/repositoryTypes/npm/index.md index bb3d093..3e7486c 100644 --- a/docs/docs/repositoryTypes/npm/index.md +++ b/docs/docs/repositoryTypes/npm/index.md @@ -23,3 +23,16 @@ in front of public registries. Any user with read access can install packages using the standard NPM tooling by pointing to the repository endpoint. Cached assets are returned immediately; proxy mode automatically refreshes missing packages from the configured upstreams. + +### Yarn Classic + +Yarn 1.x is supported for proxy repositories, including scoped packages. Configure the same +registry URL with a trailing slash: + +```bash +yarn config set registry https://your-pkgly.example.com/repositories/storage/npm-proxy/ +``` + +Yarn Classic requests scoped metadata using an encoded slash (for example, +`@babel%2Fcode-frame`). Pkgly rewrites the returned tarball URLs to the proxy, so both metadata +and archives are cached locally. diff --git a/pkgly/src/repository/npm/proxy.rs b/pkgly/src/repository/npm/proxy.rs index 69ca6bf..e36f439 100644 --- a/pkgly/src/repository/npm/proxy.rs +++ b/pkgly/src/repository/npm/proxy.rs @@ -26,7 +26,7 @@ use nr_core::{ }, storage::StoragePath, }; -use nr_storage::{DynStorage, FileContent, Storage, StorageFile}; +use nr_storage::{DynStorage, FileContent, FileType, Storage, StorageFile, StorageFileMeta}; use parking_lot::{RwLock, RwLockReadGuard}; use serde::Deserialize; use serde_json::Value; @@ -100,31 +100,76 @@ async fn serve_cached_response( storage: &DynStorage, repository_id: Uuid, path: &StoragePath, - cache_path: Option<&StoragePath>, + cache_path: &StoragePath, + is_metadata: bool, ) -> Result, NPMRegistryError> { - if let Some(file) = storage.open_file(repository_id, path).await? { - if cache_path.is_none() { - if let Some(response) = rewrite_metadata_tarballs(parts, path, file).await? { - return Ok(Some(response)); - } - if let Some(file) = storage.open_file(repository_id, path).await? { - return Ok(Some(file.into())); - } - return Ok(Some(RepoResponse::basic_text_response( - StatusCode::NOT_FOUND, - "File not found", - ))); - } + let legacy_metadata_path = is_metadata.then_some(path); + let Some(file) = + open_cached_file(storage, repository_id, cache_path, legacy_metadata_path).await? + else { + return Ok(None); + }; + if !is_metadata { return Ok(Some(file.into())); } + if let Some(response) = rewrite_metadata_tarballs(parts, path, file).await? { + return Ok(Some(response)); + } - if let Some(cache_path) = cache_path { - if let Some(file) = storage.open_file(repository_id, cache_path).await? { - return Ok(Some(file.into())); - } + Ok( + open_cached_file(storage, repository_id, cache_path, legacy_metadata_path) + .await? + .map(Into::into), + ) +} + +async fn open_cached_file( + storage: &DynStorage, + repository_id: Uuid, + cache_path: &StoragePath, + legacy_metadata_path: Option<&StoragePath>, +) -> Result, NPMRegistryError> { + if let Some(file @ StorageFile::File { .. }) = + storage.open_file(repository_id, cache_path).await? + { + return Ok(Some(file)); } - Ok(None) + let Some(legacy_metadata_path) = legacy_metadata_path else { + return Ok(None); + }; + Ok(storage + .open_file(repository_id, legacy_metadata_path) + .await? + .and_then(|file| match file { + file @ StorageFile::File { .. } => Some(file), + StorageFile::Directory { .. } => None, + })) +} + +async fn cached_file_information( + storage: &DynStorage, + repository_id: Uuid, + cache_path: &StoragePath, + legacy_metadata_path: Option<&StoragePath>, +) -> Result>, NPMRegistryError> { + let metadata = storage + .get_file_information(repository_id, cache_path) + .await?; + if metadata + .as_ref() + .is_some_and(|metadata| matches!(metadata.file_type(), FileType::File(_))) + { + return Ok(metadata); + } + + let Some(legacy_metadata_path) = legacy_metadata_path else { + return Ok(None); + }; + let metadata = storage + .get_file_information(repository_id, legacy_metadata_path) + .await?; + Ok(metadata.filter(|metadata| matches!(metadata.file_type(), FileType::File(_)))) } impl NpmProxyRegistry { @@ -212,57 +257,26 @@ impl NpmProxyRegistry { url: url.to_string(), error: err.to_string(), })?; + let tarball_cache_path = cache_path_for_npm_proxy(path); + let cache_path = tarball_cache_path + .clone() + .unwrap_or_else(|| metadata_cache_path_for_npm_proxy(path)); match self .storage() - .save_file(self.0.id, FileContent::Bytes(bytes.clone()), path) + .save_file(self.0.id, FileContent::Bytes(bytes.clone()), &cache_path) .await { Ok(_) => {} Err(nr_storage::StorageError::PathCollision(_)) => { - debug!(%url, "Skipping cache write for existing npm metadata file"); + debug!(%url, ?cache_path, "Skipping cache write for existing npm resource"); } Err(other) => return Err(other.into()), } - let cache_path = cache_path_for_npm_proxy(path); - let canonical_path = if let Some(cache_path) = &cache_path { - if cache_path != path { - if let Err(err) = self - .storage() - .save_file( - self.0.id, - FileContent::Bytes(bytes.clone()), - cache_path, - ) - .await - { - match err { - nr_storage::StorageError::PathCollision(_) => { - debug!( - ?cache_path, - "Cache file already exists, skipping overwrite" - ); - } - other => { - warn!( - ?other, - ?cache_path, - "Failed to persist npm proxy cache entry" - ); - return Err(other.into()); - } - } - } - } - cache_path.clone() - } else { - path.clone() - }; - - if cache_path.is_some() { + if tarball_cache_path.is_some() { record_npm_proxy_cache_hit( self.indexer().as_ref(), - &canonical_path, + &cache_path, bytes.len() as u64, Some(&url), ) @@ -455,7 +469,11 @@ impl Repository for NpmProxyRegistry { let path = request.path.clone(); - let cache_path = cache_path_for_npm_proxy(&path); + let tarball_cache_path = cache_path_for_npm_proxy(&path); + let cache_path = tarball_cache_path + .clone() + .unwrap_or_else(|| metadata_cache_path_for_npm_proxy(&path)); + let is_metadata = tarball_cache_path.is_none(); let storage = this.storage(); if path.is_directory() { @@ -476,7 +494,8 @@ impl Repository for NpmProxyRegistry { &storage, this.id(), &path, - cache_path.as_ref(), + &cache_path, + is_metadata, ) .await? { @@ -489,7 +508,8 @@ impl Repository for NpmProxyRegistry { &storage, this.id(), &path, - cache_path.as_ref(), + &cache_path, + is_metadata, ) .await? { @@ -535,7 +555,10 @@ impl Repository for NpmProxyRegistry { let path = request.path; - let cache_path = cache_path_for_npm_proxy(&path); + let tarball_cache_path = cache_path_for_npm_proxy(&path); + let is_metadata = tarball_cache_path.is_none(); + let cache_path = + tarball_cache_path.unwrap_or_else(|| metadata_cache_path_for_npm_proxy(&path)); if path.is_directory() { if let Some(response) = this @@ -550,41 +573,24 @@ impl Repository for NpmProxyRegistry { )); } - if let Some(meta) = this - .storage() - .get_file_information(this.id(), &path) - .await? + let storage = this.storage(); + if let Some(meta) = cached_file_information( + &storage, + this.id(), + &cache_path, + is_metadata.then_some(&path), + ) + .await? { return Ok(meta.into()); } - if let Some(cache_path) = &cache_path { - if let Some(meta) = this - .storage() - .get_file_information(this.id(), cache_path) - .await? - { - return Ok(meta.into()); - } - } - if this.download_and_cache(&path, query.as_deref()).await? { - if let Some(meta) = this - .storage() - .get_file_information(this.id(), &path) - .await? + if let Some(meta) = + cached_file_information(&storage, this.id(), &cache_path, None).await? { return Ok(meta.into()); } - if let Some(cache_path) = &cache_path { - if let Some(meta) = this - .storage() - .get_file_information(this.id(), cache_path) - .await? - { - return Ok(meta.into()); - } - } } if let Some(response) = this @@ -635,6 +641,15 @@ fn cache_path_for_npm_proxy(path: &StoragePath) -> Option { ))) } +fn metadata_cache_path_for_npm_proxy(path: &StoragePath) -> StoragePath { + let path = path.to_string(); + if path.is_empty() { + StoragePath::from("metadata/root.json") + } else { + StoragePath::from(format!("metadata/{path}")) + } +} + #[cfg(test)] mod tests; @@ -825,15 +840,11 @@ async fn rewrite_metadata_tarballs( }; let scheme = parts.uri.scheme_str().unwrap_or("http"); - // Compute repository base: full request path minus the requested package path. - let full_path = parts.uri.path(); - let suffix = format!("/{}", requested_path.to_string()); - let base_path = if let Some(stripped) = full_path.strip_suffix(&suffix) { - stripped - } else { + // Yarn Classic percent-encodes the slash in scoped package metadata paths. + let Some(base_path) = repository_base_path(parts.uri.path(), requested_path) else { return Ok(None); }; - let mut base_path = base_path.to_string(); + let mut base_path = base_path; if !base_path.ends_with('/') { base_path.push('/'); } @@ -879,6 +890,48 @@ async fn rewrite_metadata_tarballs( Ok(Some(RepoResponse::Other(builder.body(bytes)))) } +fn repository_base_path(full_path: &str, requested_path: &StoragePath) -> Option { + let mut suffixes = vec![format!("/{requested_path}")]; + let components: Vec = requested_path + .clone() + .into_iter() + .map(String::from) + .collect(); + if let [scope, package, rest @ ..] = components.as_slice() + && scope.starts_with('@') + { + let rest = rest + .iter() + .map(|part| format!("/{part}")) + .collect::(); + suffixes.push(format!("/{scope}%2f{package}{rest}")); + } + + let base_path = suffixes + .into_iter() + .find_map(|suffix| strip_suffix_ignore_ascii_case(full_path, &suffix))?; + + // Axum strips `/repositories` before invoking the nested repository router. + // Return the canonical public endpoint so clients receive the same base they configure. + if base_path == "/repositories" || base_path.starts_with("/repositories/") { + Some(base_path.to_string()) + } else { + Some(format!( + "/repositories/{}", + base_path.trim_start_matches('/') + )) + } +} + +fn strip_suffix_ignore_ascii_case<'a>(value: &'a str, suffix: &str) -> Option<&'a str> { + let start = value.len().checked_sub(suffix.len())?; + let prefix = value.get(..start)?; + value + .get(start..)? + .eq_ignore_ascii_case(suffix) + .then_some(prefix) +} + fn build_head_response(response: reqwest::Response) -> RepoResponse { use http::header::{CONTENT_LENGTH, CONTENT_TYPE}; diff --git a/pkgly/src/repository/npm/proxy/tests.rs b/pkgly/src/repository/npm/proxy/tests.rs index e21f664..05e0bca 100644 --- a/pkgly/src/repository/npm/proxy/tests.rs +++ b/pkgly/src/repository/npm/proxy/tests.rs @@ -36,6 +36,26 @@ fn cache_path_requires_tarball_segment() { assert!(cache_path_for_npm_proxy(&path).is_none()); } +#[test] +fn metadata_cache_path_is_separate_from_scoped_package_paths() { + let path = StoragePath::from("@babel/code-frame"); + let cache = metadata_cache_path_for_npm_proxy(&path); + + assert_eq!(cache.to_string(), "metadata/@babel/code-frame"); +} + +#[test] +fn metadata_cache_path_uses_a_file_for_the_registry_root() { + let cache = metadata_cache_path_for_npm_proxy(&StoragePath::default()); + + assert_eq!(cache.to_string(), "metadata/root.json"); +} + +#[test] +fn strip_suffix_ignore_ascii_case_rejects_non_utf8_boundaries() { + assert_eq!(strip_suffix_ignore_ascii_case("é", "x"), None); +} + #[test] fn normalize_routes_adds_default_when_empty() { let routes = normalize_routes(Vec::new()); @@ -211,11 +231,60 @@ async fn metadata_tarball_urls_rewritten_to_proxy_base() { ); } +#[tokio::test] +async fn metadata_tarball_urls_rewritten_for_yarn_escaped_scoped_request() { + let (parts, _) = Request::builder() + .uri("https://pkgly.test/abc/npm-proxy/@scope%2fpkg") + .header(http::header::HOST, "pkgly.test") + .body(()) + .unwrap() + .into_parts(); + + let body = r#"{ + "name": "@scope/pkg", + "versions": { + "1.0.0": { "dist": { "tarball": "https://registry.npmjs.org/@scope/pkg/-/pkg-1.0.0.tgz" } } + } + }"#; + + let tempdir = tempdir().unwrap(); + let meta_path = tempdir.path().join("package.json"); + std::fs::write(&meta_path, body).unwrap(); + let meta = nr_storage::StorageFileMeta::read_from_file(&meta_path).unwrap(); + + let file = StorageFile::File { + meta, + content: nr_storage::StorageFileReader::Bytes(nr_storage::FileContentBytes::Bytes( + Bytes::from(body), + )), + }; + + let path = StoragePath::from("@scope/pkg"); + let response = super::rewrite_metadata_tarballs(&parts, &path, file) + .await + .expect("rewrite works") + .expect("metadata response"); + + let RepoResponse::Other(response) = response else { + panic!("expected Other response"); + }; + let bytes = to_bytes(response.into_body(), usize::MAX) + .await + .expect("read body bytes"); + let rewritten: serde_json::Value = serde_json::from_slice(&bytes).unwrap(); + + assert_eq!( + rewritten["versions"]["1.0.0"]["dist"]["tarball"].as_str(), + Some("https://pkgly.test/repositories/abc/npm-proxy/@scope/pkg/-/pkg-1.0.0.tgz") + ); +} + #[tokio::test] async fn serve_cached_response_rewrites_metadata_tarballs() { let repository_id = Uuid::new_v4(); let storage = test_storage().await; let path = StoragePath::from("@scope/pkg"); + let cache_path = metadata_cache_path_for_npm_proxy(&path); let metadata = br#"{ "name": "@scope/pkg", "versions": { @@ -227,7 +296,7 @@ async fn serve_cached_response_rewrites_metadata_tarballs() { .save_file( repository_id, FileContent::Bytes(Bytes::from_static(metadata)), - &path, + &cache_path, ) .await .expect("write metadata"); @@ -239,10 +308,11 @@ async fn serve_cached_response_rewrites_metadata_tarballs() { .unwrap() .into_parts(); - let response = super::serve_cached_response(&parts, &storage, repository_id, &path, None) - .await - .expect("rewrite succeeds") - .expect("response exists"); + let response = + super::serve_cached_response(&parts, &storage, repository_id, &path, &cache_path, true) + .await + .expect("rewrite succeeds") + .expect("response exists"); let RepoResponse::Other(resp) = response else { panic!("expected Other response"); @@ -262,6 +332,111 @@ async fn serve_cached_response_rewrites_metadata_tarballs() { ); } +#[tokio::test] +async fn serve_cached_response_falls_back_to_legacy_metadata_cache() { + let repository_id = Uuid::new_v4(); + let storage = test_storage().await; + let path = StoragePath::from("@scope/pkg"); + let cache_path = metadata_cache_path_for_npm_proxy(&path); + let metadata = br#"{ + "name": "@scope/pkg", + "versions": { + "1.0.0": { "dist": { "tarball": "https://registry.npmjs.org/@scope/pkg/-/pkg-1.0.0.tgz" } } + } + }"#; + + storage + .save_file( + repository_id, + FileContent::Bytes(Bytes::from_static(metadata)), + &path, + ) + .await + .expect("write legacy metadata"); + + let (parts, _) = Request::builder() + .uri("https://pkgly.test/repositories/abc/npm-proxy/@scope/pkg") + .header(http::header::HOST, "pkgly.test") + .body(()) + .unwrap() + .into_parts(); + + let response = + super::serve_cached_response(&parts, &storage, repository_id, &path, &cache_path, true) + .await + .expect("cache lookup succeeds") + .expect("legacy cache response exists"); + + let RepoResponse::Other(response) = response else { + panic!("expected rewritten metadata response"); + }; + let body = to_bytes(response.into_body(), usize::MAX) + .await + .expect("read rewritten body"); + let rewritten: serde_json::Value = serde_json::from_slice(&body).unwrap(); + + assert_eq!( + rewritten["versions"]["1.0.0"]["dist"]["tarball"].as_str(), + Some("https://pkgly.test/repositories/abc/npm-proxy/@scope/pkg/-/pkg-1.0.0.tgz") + ); +} + +#[tokio::test] +async fn cached_file_information_falls_back_to_legacy_metadata_cache() { + let repository_id = Uuid::new_v4(); + let storage = test_storage().await; + let path = StoragePath::from("@scope/pkg"); + let cache_path = metadata_cache_path_for_npm_proxy(&path); + + storage + .save_file( + repository_id, + FileContent::Bytes(Bytes::from_static(b"legacy metadata")), + &path, + ) + .await + .expect("write legacy metadata"); + + let metadata = + super::cached_file_information(&storage, repository_id, &cache_path, Some(&path)) + .await + .expect("cache lookup succeeds") + .expect("legacy metadata exists"); + + assert_eq!(metadata.name(), "pkg"); +} + +#[tokio::test] +async fn serve_cached_response_ignores_a_directory_at_cache_path() { + let repository_id = Uuid::new_v4(); + let storage = test_storage().await; + let path = StoragePath::from("@scope/pkg"); + let cache_path = metadata_cache_path_for_npm_proxy(&path); + let nested_path = cache_path.clone().push("unexpected.json"); + + storage + .save_file( + repository_id, + FileContent::Bytes(Bytes::from_static(b"unexpected")), + &nested_path, + ) + .await + .expect("seed malformed cache directory"); + + let (parts, _) = Request::builder() + .uri("https://pkgly.test/repositories/abc/npm-proxy/@scope%2fpkg") + .body(()) + .unwrap() + .into_parts(); + + let response = + super::serve_cached_response(&parts, &storage, repository_id, &path, &cache_path, true) + .await + .expect("cache lookup succeeds"); + + assert!(response.is_none()); +} + #[derive(Clone, Default)] struct RecordingIndexer { recorded: Arc>>, diff --git a/tests/README.md b/tests/README.md index c118fd2..3f06c2f 100644 --- a/tests/README.md +++ b/tests/README.md @@ -46,7 +46,7 @@ tests/ | Package Type | Hosted | Proxy | Tests | |-------------|--------|-------|-------| | Maven | ✅ | ✅ | 12 | -| NPM | ✅ | ✅ | 14 | +| NPM | ✅ | ✅ | 18 | | Docker | ⚠️ | ✅ | 8 | | Python | ✅ | ✅ | 11 | | Python (Virtual) | ✅ | ✅ | 10 | @@ -159,7 +159,7 @@ All containers communicate on the `test-network` bridge network. Tests run insid 11. ✅ Proxy caching verification 12. ✅ Authentication and error handling -### NPM Tests (14 tests) +### NPM Tests (18 tests) 1. ✅ Create NPM package tarball 2. ✅ Publish package to hosted repository @@ -172,9 +172,13 @@ All containers communicate on the `test-network` bridge network. Tests run insid 9. ✅ Install latest version 10. ✅ Proxy package from npmjs.org 11. ✅ Proxy caching verification -12. ✅ Authentication required for publish -13. ✅ 404 for non-existent package -14. ✅ Scoped package support +12. ✅ Yarn Classic scoped-package proxy install +13. ✅ Configure npm virtual repository members +14. ✅ Install hosted package through npm virtual repository +15. ✅ Install proxied package through npm virtual repository +16. ✅ Authentication required for publish +17. ✅ 404 for non-existent package +18. ✅ Scoped package support ### Docker Tests (8 tests) diff --git a/tests/docker/Dockerfile.test-runner b/tests/docker/Dockerfile.test-runner index 4ae01e3..34de03a 100644 --- a/tests/docker/Dockerfile.test-runner +++ b/tests/docker/Dockerfile.test-runner @@ -46,6 +46,9 @@ RUN curl -fsSL https://deb.nodesource.com/setup_20.x | bash - \ && apt-get install -y nodejs \ && rm -rf /var/lib/apt/lists/* +# Yarn Classic is required to verify NPM registry compatibility. +RUN npm install --global yarn@1.22.22 + # Install Docker CLI RUN install -m 0755 -d /etc/apt/keyrings \ && curl -fsSL https://download.docker.com/linux/ubuntu/gpg | gpg --dearmor -o /etc/apt/keyrings/docker.gpg \ diff --git a/tests/integration/test_npm.sh b/tests/integration/test_npm.sh index eacc700..18a72db 100755 --- a/tests/integration/test_npm.sh +++ b/tests/integration/test_npm.sh @@ -230,6 +230,28 @@ else fail "Failed to retrieve cached package" fi +# Test 12: Yarn Classic requests scoped package metadata using an encoded slash. +print_test "Proxy: install scoped package with Yarn Classic" +YARN_PROXY_DIR="$WORKSPACE/yarn-proxy-test" +mkdir -p "$YARN_PROXY_DIR" +cd "$YARN_PROXY_DIR" + +cat > "$YARN_PROXY_DIR/.npmrc" <