Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions crates/aube-codes/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ pub const ERR_AUBE_SECURITY_SCANNER_FAILED: &str = "ERR_AUBE_SECURITY_SCANNER_FA
pub const ERR_AUBE_TARBALL_INTEGRITY: &str = "ERR_AUBE_TARBALL_INTEGRITY";
pub const ERR_AUBE_TARBALL_EXTRACT: &str = "ERR_AUBE_TARBALL_EXTRACT";
pub const ERR_AUBE_PKG_CONTENT_MISMATCH: &str = "ERR_AUBE_PKG_CONTENT_MISMATCH";
pub const ERR_AUBE_TARBALL_URL_MISMATCH: &str = "ERR_AUBE_TARBALL_URL_MISMATCH";
pub const ERR_AUBE_NO_HOME: &str = "ERR_AUBE_NO_HOME";
pub const ERR_AUBE_GIT_ERROR: &str = "ERR_AUBE_GIT_ERROR";

Expand Down Expand Up @@ -225,6 +226,12 @@ pub const ALL: &[CodeMeta] = &[
description: "Git operation failed during a `git:` dep prepare or checkout.",
exit_code: Some(33),
},
CodeMeta {
name: ERR_AUBE_TARBALL_URL_MISMATCH,
category: category::TARBALL_STORE,
description: "A registry lockfile entry's explicit tarball URL didn't match the registry metadata for that `(name, version)`.",
exit_code: Some(34),
},
CodeMeta {
name: ERR_AUBE_NO_HOME,
category: category::TARBALL_STORE,
Expand Down
111 changes: 111 additions & 0 deletions crates/aube/src/commands/install/fetch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -787,6 +787,10 @@ where
let url = tarball_url_override
.clone()
.unwrap_or_else(|| client.tarball_url(&registry_name, &version));
if let Some(lockfile_url) = tarball_url_override.as_deref() {
verify_lockfile_tarball_url(&client, &registry_name, &version, lockfile_url)
.await?;
Comment thread
cursor[bot] marked this conversation as resolved.
Resolved
}

let dl_start = std::time::Instant::now();

Expand Down Expand Up @@ -973,6 +977,69 @@ where
Ok((indices, cached_count, fetch_count))
}

async fn verify_lockfile_tarball_url(
client: &aube_registry::client::RegistryClient,
registry_name: &str,
version: &str,
lockfile_url: &str,
) -> miette::Result<()> {
let meta = client
.fetch_single_version_metadata(registry_name, version)
.await
.map_err(|e| {
miette!(
code = aube_codes::errors::ERR_AUBE_TARBALL_URL_MISMATCH,
"{}@{}: failed to fetch registry metadata to verify lockfile tarball URL: {}",
registry_name,
version,
e
)
})?;
Comment thread
cursor[bot] marked this conversation as resolved.
Resolved
let Some(expected_url) = meta.dist.as_ref().map(|dist| dist.tarball.as_str()) else {
return Err(miette!(
Comment on lines +990 to +999

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Network failures misattributed to ERR_AUBE_TARBALL_URL_MISMATCH

When fetch_single_version_metadata fails for reasons unrelated to a URL mismatch — network timeout, DNS failure, HTTP 5xx, or even a 404 for the specific version — the error is wrapped and emitted as ERR_AUBE_TARBALL_URL_MISMATCH. The code's own description ("A registry lockfile entry's explicit tarball URL didn't match the registry metadata") only covers the true mismatch case; wrapping network errors under it tells the user their lockfile was tampered with when the real cause is a registry connectivity problem. This is particularly confusing in a security-sensitive code path where distinguishing "tampered lockfile" from "registry unreachable" matters. The map_err on the metadata fetch should use a network/registry error code (e.g. ERR_AUBE_REGISTRY_REQUEST_FAILED) or just propagate the upstream error unchanged.

Fix in Claude Code

code = aube_codes::errors::ERR_AUBE_TARBALL_URL_MISMATCH,
"{}@{}: registry metadata did not include dist.tarball while lockfile pinned {}",
registry_name,
version,
aube_util::url::redact_url(lockfile_url)
));
};
if !lockfile_tarball_url_matches_metadata(lockfile_url, expected_url) {
return Err(miette!(
code = aube_codes::errors::ERR_AUBE_TARBALL_URL_MISMATCH,
"{}@{}: lockfile tarball URL {} does not match registry metadata {}",
registry_name,
version,
aube_util::url::redact_url(lockfile_url),
aube_util::url::redact_url(expected_url)
));
}
Ok(())
}

fn lockfile_tarball_url_matches_metadata(lockfile_url: &str, expected_url: &str) -> bool {
lockfile_url == expected_url
|| (is_public_npm_registry_tarball(lockfile_url)
&& tarball_url_path(lockfile_url)
.zip(tarball_url_path(expected_url))
.is_some_and(|(lockfile_path, expected_path)| lockfile_path == expected_path))
}

fn is_public_npm_registry_tarball(url: &str) -> bool {
reqwest::Url::parse(url)
.ok()
.and_then(|url| url.host_str().map(str::to_ascii_lowercase))
.as_deref()
== Some("registry.npmjs.org")
}

fn tarball_url_path(url: &str) -> Option<String> {
let url = reqwest::Url::parse(url).ok()?;
let path = url.path();
path.contains("/-/")
.then_some(path.trim_start_matches('/').to_string())
}

/// Pull the canonical version off a dep_path for display purposes. The
/// dep_path looks like `name@1.2.3(peer@x)` — we strip the `name@` prefix
/// and any peer suffix so the warning shows `1.2.3` not `1.2.3(peer@x)`.
Expand Down Expand Up @@ -1004,3 +1071,47 @@ pub(super) fn remap_indices_to_contextualized(
}
out
}

#[cfg(test)]
mod tests {
use super::lockfile_tarball_url_matches_metadata;

#[test]
fn tarball_url_match_accepts_exact_url() {
let url = "https://private.example.com/is-odd/-/is-odd-3.0.1.tgz";

assert!(lockfile_tarball_url_matches_metadata(url, url));
}

#[test]
fn tarball_url_match_accepts_public_npm_lockfile_against_mirror() {
assert!(lockfile_tarball_url_matches_metadata(
"https://registry.npmjs.org/is-odd/-/is-odd-3.0.1.tgz",
"http://localhost:4873/is-odd/-/is-odd-3.0.1.tgz",
));
}

#[test]
fn tarball_url_match_accepts_scoped_public_npm_lockfile_against_mirror() {
assert!(lockfile_tarball_url_matches_metadata(
"https://registry.npmjs.org/@isaacs/fs-minipass/-/fs-minipass-4.0.1.tgz",
"http://localhost:4873/@isaacs/fs-minipass/-/fs-minipass-4.0.1.tgz",
));
}

#[test]
fn tarball_url_match_rejects_tampered_path() {
assert!(!lockfile_tarball_url_matches_metadata(
"https://registry.npmjs.org/not-is-odd/-/not-is-odd-3.0.1.tgz",
"http://localhost:4873/is-odd/-/is-odd-3.0.1.tgz",
));
}

#[test]
fn tarball_url_match_rejects_mirror_match_from_arbitrary_host() {
assert!(!lockfile_tarball_url_matches_metadata(
"https://example.com/is-odd/-/is-odd-3.0.1.tgz",
"http://localhost:4873/is-odd/-/is-odd-3.0.1.tgz",
));
}
}
6 changes: 6 additions & 0 deletions docs/error-codes.data.json
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,12 @@
"description": "Git operation failed during a `git:` dep prepare or checkout.",
"exit_code": 33
},
{
"name": "ERR_AUBE_TARBALL_URL_MISMATCH",
"category": "Tarball / store",
"description": "A registry lockfile entry's explicit tarball URL didn't match the registry metadata for that `(name, version)`.",
"exit_code": 34
},
{
"name": "ERR_AUBE_NO_HOME",
"category": "Tarball / store",
Expand Down
24 changes: 24 additions & 0 deletions test/lockfile_settings.bats
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,30 @@ teardown() {
assert_success
}

@test "lockfile tarball URLs are verified against registry metadata before fetch" {
cat >package.json <<-'EOF'
{
"name": "test-tarball-url-mismatch",
"version": "1.0.0",
"dependencies": { "is-odd": "3.0.1" }
}
EOF
cat >>.npmrc <<-'EOF'

lockfile-include-tarball-url=true
EOF
run aube install --no-frozen-lockfile
assert_success

perl -0pi -e 's#tarball: .*/is-odd/-/is-odd-3\.0\.1\.tgz#tarball: https://example.com/not-is-odd.tgz#' aube-lock.yaml
rm -rf node_modules "$XDG_DATA_HOME/aube/store"

run aube install --frozen-lockfile
assert_failure
assert_output --partial "ERR_AUBE_TARBALL_URL_MISMATCH"
assert_output --partial "not match registry metadata"
}

@test "lockfileIncludeTarballUrl=false (default) omits tarball URLs" {
cat >package.json <<-'EOF'
{
Expand Down
Loading