Add dynamic .npmrc authentication #2550
Conversation
Fixes static authentication token caching issue where tokens from .npmrc were read once during module extension evaluation and cached, causing 401 errors when short-lived credentials expired (e.g., AWS CodeArtifact 12-hour tokens). Changes: - Add _read_npmrc_auth() function to read .npmrc files dynamically on each download instead of caching tokens statically - Add _get_auth_from_url() helper to extract auth for specific registry URLs - Add npmrc and use_home_npmrc attributes to npm_import_rule - Modify _download_and_extract_archive() to read auth dynamically first, with fallback to static auth attributes for backward compatibility - Pass npmrc/use_home_npmrc from npm_translate_lock extension instead of static npm_auth* attributes Testing: - Add unit tests for _get_auth_from_url() helper function - Add e2e integration test that verifies tokens are read dynamically: * Fetches succeed with valid tokens * Fetches fail with 401 when tokens are broken (proving fresh reads) * Fetches succeed when tokens are restored - Test uses --repository_cache= to force fresh downloads Fixes: aspect-build#2547
|
|
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
rules_js/npm/private/npm_import.bzl
Lines 1366 to 1370 in 70a0a59
Allow npm_import macro to accept npmrc/use_home_npmrc
Although the repository rule now declares npmrc and use_home_npmrc, the npm_import macro still pops only generate_bzl_library_targets/extract_full_archive and fails on any remaining kwargs, so callers cannot pass the new auth-related parameters and the repository rule never sees them (dynamic .npmrc auth remains disabled and passing npmrc will raise "Invalid npm_import parameter"). The macro should accept and forward the new attributes to npm_import_rule.
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| def _get_auth_from_url(url, npm_auth_dict): | ||
| for registry, auth_info in npm_auth_dict.items(): | ||
| if registry in url: | ||
| if "bearer" in auth_info: | ||
| return { |
There was a problem hiding this comment.
The new _get_auth_from_url helper returns the first registry entry whose key is a substring of the URL, so a global _authToken entry (key "") in ~/.npmrc will always match and short‑circuit even when a scoped registry token is present. In that common setup, downloads for scoped registries will use the global token and get 401s instead of the registry‑specific credentials, which regresses the longest‑prefix selection previously used when static auth was passed to the rule. Consider skipping the empty registry until no specific match exists or applying the same longest‑match logic as _select_npm_auth.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
This seems correct and an easy test+fix?
ravi-pplx
force-pushed
the
fix-dynamic-npmrc-auth-v2
branch
from
6042fa5 to
f0f606e
Compare
|
Don't we need to change |
|
I think your e2e test needs to be added to CI still? |
| npm_auth_basic = "", | ||
| npm_auth_username = "", | ||
| npm_auth_password = "", | ||
| npmrc = None, |
There was a problem hiding this comment.
I don't see these being specified anywhere?
|
I'm also curious if #2554 solves this? Can you try that? That is for |
3 participants
Fixes static authentication token caching issue where tokens from .npmrc were read once during module extension evaluation and cached, causing 401 errors when short-lived credentials expired (e.g., AWS CodeArtifact 12-hour tokens).
Changes:
Changes are visible to end-users: no
Test plan
Fixes: #2547