Skip to content

feat(pm): add private npm registry auth#2758

Draft
elrrrrrrr wants to merge 3 commits into
nextfrom
feat/pm-private-registry-auth
Draft

feat(pm): add private npm registry auth#2758
elrrrrrrr wants to merge 3 commits into
nextfrom
feat/pm-private-registry-auth

Conversation

@elrrrrrrr
Copy link
Copy Markdown
Contributor

Summary

  • Support Bearer token authentication for private npm registries
  • Token resolved from NPM_TOKEN / NODE_AUTH_TOKEN env vars or ~/.utoo/config.toml (utoo login)
  • ruborist HTTP client made injectable (set_http_client) — stays auth-unaware
  • Tarball downloads use hostname guard to prevent token leakage to third-party CDNs

Design

Layer Responsibility
ruborist Zero auth awareness. HTTP client settable from outside via set_http_client, otherwise auto-built
pm/user_config Resolves token, builds client with Authorization: Bearer default header, injects into ruborist
pm/downloader Holds token locally, attaches Bearer only when tarball URL host matches registry host

Test plan

  • NPM_TOKEN=<token> ut install with a private scoped package
  • utoo login + ut install reads token from ~/.utoo/config.toml
  • Public registry install still works without token
  • Tarball from a CDN URL does not receive the auth header

🤖 Generated with Claude Code

elrrrrrrr and others added 2 commits April 2, 2026 18:12
The old auto-update used in-memory global state (AtomicBool) that reset
on every process start, so updates could never actually trigger. The
background version check was also killed by runtime shutdown before
completing.

New design:
- Read cached version from ~/.utoo/remote-version.json on startup
- If cache is fresh and version differs, try update immediately
- If cache is missing/expired, background fetch + update if needed
- 24h cooldown after failed attempts to avoid nagging
- UTOO_FORCE_UPDATE=1|true to bypass dev/CI guards for testing
- Suppress install output during auto-update, show colored status

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…wnload

Support Bearer token authentication for private npm registries. Token is
resolved from NPM_TOKEN / NODE_AUTH_TOKEN env vars or ~/.utoo/config.toml
(via `utoo login`).

- ruborist: make HTTP client injectable (`set_http_client`) so pm can
  configure it with auth headers — ruborist itself stays auth-unaware
- pm: build pre-configured HTTP client with Bearer default header and
  inject into ruborist at startup
- downloader: add per-request Bearer auth with hostname guard to prevent
  token leakage to third-party CDNs

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request refactors the auto-update mechanism to include a 24-hour retry cooldown and introduces private registry authentication support by injecting Bearer tokens into HTTP clients. Key changes include moving the update check to a non-blocking background task and adding a global authentication token resolver. Feedback focuses on improving the robustness of host extraction for token attachment, handling potential token parsing failures, and addressing thread-safety issues in tests that modify environment variables.

// Set CI environment variable
#[test]
fn test_skip_ci_environment() {
unsafe { std::env::set_var("CI", "1") };
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.

medium

Using std::env::set_var in tests is unsafe and can cause race conditions when tests are run in parallel, as the environment is shared across the process. Additionally, this test no longer calls init_auto_update(), so it doesn't verify the CI-skipping logic it intends to test.

Comment on lines +39 to +45
fn extract_host(url: &str) -> &str {
let without_scheme = url
.strip_prefix("https://")
.or_else(|| url.strip_prefix("http://"))
.unwrap_or(url);
without_scheme.split('/').next().unwrap_or(without_scheme)
}
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.

medium

The manual string parsing for host extraction is fragile and doesn't handle authentication info in the URL (e.g., https://user:pass@host). Since the url crate is already a dependency, it's recommended to use it for robust host extraction, or at least improve the string manipulation to handle the auth part.

Suggested change
fn extract_host(url: &str) -> &str {
let without_scheme = url
.strip_prefix("https://")
.or_else(|| url.strip_prefix("http://"))
.unwrap_or(url);
without_scheme.split('/').next().unwrap_or(without_scheme)
}
fn extract_host(url: &str) -> &str {
let without_scheme = url
.strip_prefix("https://")
.or_else(|| url.strip_prefix("http://"))
.unwrap_or(url);
let host_part = without_scheme.split('/').next().unwrap_or(without_scheme);
host_part.rsplit('@').next().unwrap_or(host_part)
}

Comment thread crates/pm/src/util/user_config.rs Outdated
Comment on lines +84 to +86
if let Ok(val) = format!("Bearer {token}").parse() {
headers.insert(AUTHORIZATION, val);
}
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.

medium

If the token contains characters that are invalid for an HTTP header value (like newlines), parse() will fail silently, and the client will be configured without authentication. This can lead to confusing 401 errors later. Consider logging a warning or returning early if the token is invalid.

        let Ok(val) = format!("Bearer {token}").parse() else {
            tracing::warn!("Failed to parse auth token for registry {}", registry);
            return;
        };
        headers.insert(AUTHORIZATION, val);

- extract_host: handle user:pass@host URLs by stripping auth prefix
- init_auth_token: warn and return early on invalid token header value

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant