Skip to content

BUILD-12160 Fetch SaaS-pinned npm lockfile tarballs through the Edge - #355

Closed
hedinasr wants to merge 2 commits into
masterfrom
feat/hnasr/BUILD-12160-npmReplaceRegistryHost
Closed

hedinasr wants to merge 2 commits into
masterfrom
feat/hnasr/BUILD-12160-npmReplaceRegistryHost

Conversation

@hedinasr

@hedinasr hedinasr commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

Part of BUILD-12160

Summary

npm lockfiles record absolute resolved URLs on https://repox.jfrog.io. With an Edge repox-url, config-npm only authenticates the Edge host, so npm ci still downloads every tarball from SaaS without a token and gets a 401.

  • With a repox-url other than https://repox.jfrog.io, config-npm rewrites every https://repox.jfrog.io/artifactory/api/npm/ URL in the workspace package-lock.json and npm-shrinkwrap.json files (outside node_modules) to repox-url. The rewrite is not committed. The SaaS default is unchanged.
  • replace-registry-host stays at npm's default, so lockfile URLs on registry.npmjs.org still go through the configured registry. Setting it to repox.jfrog.io instead would double /artifactory/api/npm/npm on npm 10, whose Arborist builds ${registry}${resolved.pathname}, and would turn off the npmjs rewrite.
  • README: documented in the config-npm section.

Test plan

  • shellspec spec/config-npm_spec.sh: 16 examples, 0 failures. The new rewrite_lockfiles() cases cover root, nested and npm-shrinkwrap.json lockfiles, skip node_modules, keep registry.npmjs.org URLs, and leave SaaS untouched.
  • npm 11.19.0, lockfile with one https://repox.jfrog.io/artifactory/api/npm/npm/is-number/-/is-number-7.0.0.tgz entry and one https://registry.npmjs.org/is-odd/-/is-odd-3.0.1.tgz entry, rewritten by rewrite_lockfiles with ARTIFACTORY_URL=http://127.0.0.1:9/artifactory, registry http://127.0.0.1:9/artifactory/api/npm/npm/: npm ci requests http://127.0.0.1:9/artifactory/api/npm/npm/is-number/-/is-number-7.0.0.tgz and http://127.0.0.1:9/artifactory/api/npm/npm/is-odd/-/is-odd-3.0.1.tgz.
  • SonarJS populate_npm_cache on config-npm from this branch with the Edge repox-url.

With a repox-url other than https://repox.jfrog.io, config-npm also sets
replace-registry-host=repox.jfrog.io. Lockfiles generated against SaaS Repox
then download their tarballs from repox-url instead of hitting SaaS
without a token for that host.
@hashicorp-vault-sonar-prod

hashicorp-vault-sonar-prod Bot commented Oct 2, 2026 •

Copy link
Copy Markdown

BUILD-12160

@gitar-bot

gitar-bot Bot commented Oct 2, 2026 •

Copy link
Copy Markdown
Code Review ✅ Approved 2 closed / 2 findings

🔴 High risk · Risk could not be assessed, so high risk was applied as a precaution.

Rewrites SaaS-pinned npm lockfile URLs to the Edge host instead of setting replace-registry-host, avoiding path doubling on npm 10+ and preserving the npmjs registry rewrite. Lockfile rewriting is thoroughly tested across root, nested, and shrinkwrap files with proper node_modules exclusion.

✅ 2 closed
✅ Bug: Bare-host replace-registry-host doubles the /artifactory/api/npm/npm path

📄 config-npm/npm_config.sh:18 📄 config-npm/npm_config.sh:21-23 📄 README.md:968-969
When replace-registry-host is a bare hostname that matches the lockfile URL's host, Arborist's #registryResolved builds the new URL as ${registry.slice(0,-1)}${resolvedURL.pathname}. That is the full configured registry, path included, followed by the full original path. It does not just swap the origin. With registry=https://repox-internal.dev.sonar.build/artifactory/api/npm/npm and resolved=https://repox.jfrog.io/artifactory/api/npm/npm/is-number/-/is-number-7.0.0.tgz, npm ci would request …/artifactory/api/npm/npm/artifactory/api/npm/npm/is-number/-/…, which should 404. This is the known path-duplication bug npm/cli#6110. Its fix in v11.18.0 added URL-prefix matching and left bare-hostname behaviour as it was. The PR's manual test probably missed this because its registry (http://127.0.0.1:9) had no path. Fix: re-test with a registry URL that has the real /artifactory/api/npm/npm path. Then either use the prefix form (replace-registry-host=https://repox.jfrog.io/artifactory/api/npm/npm, npm ≥ 11.18 per the docs) or rewrite the lockfile URLs another way on older npm versions.

✅ Edge Case: Setting a hostname turns off the default registry.npmjs.org rewrite

📄 config-npm/npm_config.sh:21-23
The default replace-registry-host is npmjs. It rewrites lockfile URLs on registry.npmjs.org to the configured registry, and because the npmjs path is only /pkg/-/pkg.tgz, they resolve correctly through Repox. The npm docs say a bare hostname replaces URLs "only from that host". So writing replace-registry-host=repox.jfrog.io turns off the npmjs rewrite for Edge users. Any lockfile entries that still point at registry.npmjs.org would then be downloaded straight from public npm, skipping Repox. This may fail on runners with restricted egress, and it changes behaviour that worked before this PR. Fix: make sure the chosen setting still covers registry.npmjs.org entries, or at least document the change in the README.

Review coverage

🧪 Functional validation 0 of 4 objectives covered

📋 Rules No rules evaluated

Cross-repo coverage 2 repositories selected

🤖 Auto-approval Not enabled · Set up

Implementation Status ◻️ 0 of 4 objectives covered
◻️ BUILD-12160 - 0 of 4 objectives covered

This PR adds NuGet configuration to resolve packages through Repox, which is unrelated to the pilot objectives for sonar-dummy or SaaS-only deploys.

Other objectives on this issue, possibly covered elsewhere:

  • ◻️ Confirm deploy remains SaaS-only
  • ◻️ Enable Edge resolve for sonar-dummy against prod Edge
  • ◻️ Pick a real consumer repo with EngXP and product DRI
  • ◻️ Monitor 401/403 and federation errors
Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Counting what did not apply, without listing it.

Comment with these commands to change the behavior for this request:

Auto-apply Compact
gitar auto-apply:on         
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar

Comment thread config-npm/npm_config.sh
Comment thread config-npm/npm_config.sh Outdated
…istry-host

npm 10 builds a host-only replace-registry-host URL from the full registry
URL plus the original path, which doubles /artifactory/api/npm/npm. A
hostname also turns off the default registry.npmjs.org rewrite.

With an Edge repox-url, config-npm now rewrites the SaaS Repox URLs of the
workspace lockfiles outside node_modules to repox-url, and leaves
replace-registry-host at its default.
@sonarqubecloud

sonarqubecloud Bot commented Oct 2, 2026

Copy link
Copy Markdown

@hedinasr

hedinasr commented Oct 2, 2026

Copy link
Copy Markdown
Contributor Author

Closing: the lockfile is fixed at the source instead. SonarJS now records its tarballs on registry.npmjs.org, which npm maps onto the configured registry by default, so config-npm 2.2.0 needs no change (SonarJS#8062).

@hedinasr hedinasr closed this Oct 2, 2026
@hedinasr
hedinasr deleted the feat/hnasr/BUILD-12160-npmReplaceRegistryHost branch October 2, 2026 08:57
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