Check credentials for configured registry before falling back to public npm registry
#12798
Conversation
thavaahariharangit
changed the title
|
Can you write a test so we can see it's working and make sure it doesn't fail in the future? |
Thanks @jakecoffman , RSpec tests are added as per the discussion. |
thavaahariharangit
changed the title
npm registry
| # Look for a credential that replaces the base registry (global registry replacement) | ||
| replaces_base_cred = credentials.find { |cred| cred["type"] == "npm_registry" && cred.replaces_base? } | ||
| return normalize_registry_url(replaces_base_cred["registry"]) if replaces_base_cred | ||
|
|
||
| # Look for any npm_registry credential as fallback | ||
| npm_cred = credentials.find { |cred| cred["type"] == "npm_registry" && cred["registry"] } | ||
| return normalize_registry_url(npm_cred["registry"]) if npm_cred |
There was a problem hiding this comment.
When we have a config with two npm ecosystems, one using a private registry and one that doesn't, will we always attempt to fetch metadata from the private registry?
There was a problem hiding this comment.
It might be worth writing a test for this scenario
There was a problem hiding this comment.
This change enables Dependabot to work seamlessly in enterprise environments where public registry access is blocked by security policies.
Before this fix: Even with private registry credentials configured, Dependabot would still attempt to fetch metadata from the public registry, causing failures for firewall-restricted environments.
After this fix: Dependabot respects the configured private registry credentials and avoids unnecessary public registry calls.
Test scenario added: The "with multiple credentials" test ensures we have predictable behavior when multiple private registries are configured - always using the first available credential as fallback.
There was a problem hiding this comment.
Registry Selection Priority:
When lockfile source info is available: Uses the resolved registry URL from package-lock.json - credential-based selection is bypassed entirely.
When no lockfile source info exists: Falls back to credential-based registry selection in this order:
- First credential with
replaces-base: true - First regular
npm_registrycredential - Public registry
registry.npmjs.orgas final fallback
Key Point: The credential selection logic only activates when package-lock.json doesn't provide registry resolution information for a dependency.
There was a problem hiding this comment.
@thavaahariharangit I feel two ecosystem test case testing is not needed as we are not writing this from scratch.
There was a problem hiding this comment.
@thavaahariharangit Thanks for the detailed explanation! That helps clarify the behavior.
To directly answer my original question: if we have a monorepo where both ecosystems (one with a private registry and one without) are npm and both ecosystems have lockfiles that don't provide registry information for a dependency, are we going to try and get the info from the private registry?
There was a problem hiding this comment.
@randhircs I disagree, the two ecosystem test case is valuable here. We might not be writing from scratch, but we're changing behavior, and mixed configs are common enough that we should have explicit coverage
There was a problem hiding this comment.
We gave 2 option.
- Set
replace_base=true - When
replace_base=trueis not set then use.npmrc
Focus of this implementation is to ensure that if one of this option is set then it's not accessing the public registry.
If it is complex as mono repo (Having multiple projects in single repo) we definitely recommend them to use the lock file. Getting the registry url from global files like .npmrc or dependabot.yml will not be an option there.
But in a simple cases such as choosing between private or public, this option will be handy.
Are you suggesting to have a fallback logic only if replace_base=true is set?
There was a problem hiding this comment.
@robaiken Following our call discussion, please go ahead and test or share the failure scenarios for the specific case you mentioned. From my understanding, when there's a conflict between public and private registries, if the system defaults to the private registry and encounters a failure, that should be acceptable since private registries take precedence over public ones for customers.
…ic `npm` registry (#12798) * Check credentials for registry url before falling back to public registry * Lint fix * Lint fixes * RSpec added. * Focusing only on registry url. * updating the comments. * updating the comments. * Update npm_and_yarn/lib/dependabot/npm_and_yarn/metadata_finder.rb Co-authored-by: Rob Aiken <[email protected]> * Test added for multi credentials scenario * fallback only if replaces true is * Lint fixes --------- Co-authored-by: Rob Aiken <[email protected]>
What are you trying to accomplish?
If the lockfile is missing, then it will collect the information from
.npmrcor thedependabot.ymlprivate registry credential configuration.Anything you want to highlight for special attention from reviewers?
Maintains existing behavior when lockfile source information is available
Normalizes registry URLs by removing trailing slashes for consistent formatting
How will you know you've accomplished your goal?
Executed the Dependabot CLI and confirmed that it does not fall back to the public registry URL, and that there are no trailing backslashes causing 404 errors.
Checklist