mbox series

[0/2] fetch/{npm,npmsw}: fix security issue and re-enable fetchers

Message ID 20260616-dev-tprrt-fix-npm-v1-0-6fde95bf0a8b@bootlin.com
Headers show
Series fetch/{npm,npmsw}: fix security issue and re-enable fetchers | expand

Message

Thomas Perrot June 16, 2026, 1:37 p.m. UTC
The npm and npmsw fetchers were disabled in 355cd226 (Jan 2026) because
the npm fetcher retrieved checksums from the remote registry rather than
from the recipe. This series fixes the root cause and re-enables both
fetchers.

Root cause analysis
-------------------

The npm fetcher called `npm view` to resolve the tarball URL and then
embedded the registry-supplied dist.integrity / dist.shasum directly into
the proxy URL. Since the same party controls both the tarball and the
checksum, a compromised registry could serve a modified tarball with a
matching hash and bypass BitBake's tamper detection completely.

The npmsw (shrinkwrap) fetcher was caught in the same disable sweep, but
its security model is already correct: checksums come from a locally
committed npm-shrinkwrap.json file, not from the network. It needed only
a missing FetchError import fix and re-enabling.

What changes
------------

On the fetcher side, npmsw is re-enabled after adding the missing
FetchError import and fixing two correctness bugs: an empty packages dict
no longer raises FetchError ('if packages is None' replaces 'if not
packages'), and a missing 'resolved' field now raises ParameterError
instead of AttributeError.

The npm fetcher is fixed at its root: _resolve_proxy_url now stores only
the bare tarball URL in .resolved, discarding any registry-provided
checksum. _setup_proxy injects the checksum from the recipe's own SRC_URI
parameters (sha512sum= or sha256sum=) instead; uri.params is cleared
before rebuilding the proxy URL to prevent an npmsw-written .resolved
file from contaminating the npm fetcher with a registry-sourced checksum.
When no checksum is present in ud.parm, bb.warn() is emitted so recipe
authors get a clear signal instead of a silent unsigned download.
version=latest is now a hard ParameterError rather than a warning, and
the dead 'if ud.version == "latest": return True' branch in need_update()
is removed since version=latest is rejected at urldata_init time. The
broad 'except Exception' in _npm_view is narrowed to json.JSONDecodeError
so that FetchError and ParameterError propagate typed to callers; the
error message falls back to str(error) when 'summary' is absent from the
registry error dict so it is never silently None.

On the test side, skipIfNoNpm() dead code is fixed, all unconditional
return-skip guards are removed from test_npmsw_* tests, and new tests
cover version=latest rejection, recipe-checksum injection, and
wrong-checksum rejection.

Migration note for recipe authors
----------------------------------

Recipes using npm:// must now supply a checksum in SRC_URI, e.g.:

  SRC_URI = "npm://registry.npmjs.org;package=lodash;version=4.17.21;sha512sum=<hex>"

On the first build without a checksum BitBake will download the
tarball, compute the hash, and instruct the author to add it. Any
stale .resolved files under DL_DIR/npm2/ written by the old fetcher
(which embedded registry-sourced checksums) must be deleted before
rebuilding.

[YOCTO #16105]

Fixes: 355cd226e072 ("fetch2/npm: Disable npm/npmsw fetchers due to security issues")
Bugzilla: https://bugzilla.yoctoproject.org/show_bug.cgi?id=16105
Signed-off-by: Thomas Perrot <thomas.perrot@bootlin.com>
---
Thomas Perrot (2):
      fetch/{npm,npmsw}: fix security issue and re-enable fetchers
      tests/fetch: restore and extend npm/npmsw test coverage

 lib/bb/fetch2/npm.py   | 94 ++++++++++++++++++++++++++------------------------
 lib/bb/fetch2/npmsw.py | 12 +++----
 lib/bb/tests/fetch.py  | 64 ++++++++++++++++++++++++----------
 3 files changed, 100 insertions(+), 70 deletions(-)
---
base-commit: 7e6466f48191c1e4ab9b91705deb237eff2c7f01
change-id: 20260605-dev-tprrt-fix-npm-348c627ed2f2

Best regards,