mbox series

[bitbake-devel,0/2] fetch/npm: fix security issue and re-enable fetcher

Message ID 20260610-dev-tprrt-fix-npm-v1-0-9bf501d4ee0e@bootlin.com
Headers show
Series fetch/npm: fix security issue and re-enable fetcher | expand

Message

Thomas Perrot June 10, 2026, 3:46 p.m. UTC 
The npm fetcher was disabled in 355cd226 (Jan 2026) because it retrieved
checksums from the remote registry rather than from the recipe.  A
compromised registry controls both the tarball and its advertised hash,
making checksum verification meaningless.

Fix the root cause by separating URL resolution from checksum handling:
- _resolve_proxy_url now stores only the bare tarball URL in the .resolved
  file; the registry-supplied dist.integrity / dist.shasum values are
  ignored entirely.
- _setup_proxy builds the proxy URL from that bare tarball URL and injects
  the checksum from the recipe's own SRC_URI parameters (sha512sum=,
  sha256sum=, etc.).  When no checksum is provided the proxy URL carries
  none, and BitBake's standard BB_STRICT_CHECKSUM machinery handles the
  missing-checksum case the same way the wget fetcher does.  A bb.warn()
  is emitted so recipe authors get a clear signal instead of a silent
  unsigned download.
- version=latest is now a hard ParameterError instead of a warning; it is
  inherently non-reproducible.
- Narrow the broad 'except Exception' in _npm_view to json.JSONDecodeError
  so that FetchError and ParameterError propagate typed to callers.  Also
  fall back to str(error) when 'summary' is absent in the registry error
  dict so the message is never silently None.
- Clear uri.params before rebuilding the proxy URL to prevent a
  .resolved file written by the npmsw fetcher (which stores the full URI
  with checksum params) from contaminating the npm proxy URL with a
  registry-sourced checksum, bypassing the security invariant that only
  recipe-provided checksums are trusted.
- Remove dead 'if ud.version == "latest": return True' branch from
  need_update(); version=latest is rejected at urldata_init time.

Migration note: 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")
Signed-off-by: Thomas Perrot <thomas.perrot@bootlin.com>
---
Thomas Perrot (2):
      fetch/npmsw: fix security issue and re-enable fetcher
      tests/fetch: restore and extend npm/npmsw test coverage

 lib/bb/fetch2/npmsw.py | 12 +++++-----
 lib/bb/tests/fetch.py  | 64 +++++++++++++++++++++++++++++++++++---------------
 2 files changed, 51 insertions(+), 25 deletions(-)
---
base-commit: d6bc0e5ec549a4f984cb3d470dd3c04d0ea46fde
change-id: 20260605-dev-tprrt-fix-npm-348c627ed2f2

Best regards,
--  
Thomas Perrot <thomas.perrot@bootlin.com>

Comments

Richard Purdie June 10, 2026, 4:27 p.m. UTC | #1 
On Wed, 2026-06-10 at 17:46 +0200, Thomas Perrot via lists.openembedded.org wrote:
> The npm fetcher was disabled in 355cd226 (Jan 2026) because it retrieved
> checksums from the remote registry rather than from the recipe.  A
> compromised registry controls both the tarball and its advertised hash,
> making checksum verification meaningless.
> 
> Fix the root cause by separating URL resolution from checksum handling:
> - _resolve_proxy_url now stores only the bare tarball URL in the .resolved
>   file; the registry-supplied dist.integrity / dist.shasum values are
>   ignored entirely.
> - _setup_proxy builds the proxy URL from that bare tarball URL and injects
>   the checksum from the recipe's own SRC_URI parameters (sha512sum=,
>   sha256sum=, etc.).  When no checksum is provided the proxy URL carries
>   none, and BitBake's standard BB_STRICT_CHECKSUM machinery handles the
>   missing-checksum case the same way the wget fetcher does.  A bb.warn()
>   is emitted so recipe authors get a clear signal instead of a silent
>   unsigned download.
> - version=latest is now a hard ParameterError instead of a warning; it is
>   inherently non-reproducible.
> - Narrow the broad 'except Exception' in _npm_view to json.JSONDecodeError
>   so that FetchError and ParameterError propagate typed to callers.  Also
>   fall back to str(error) when 'summary' is absent in the registry error
>   dict so the message is never silently None.
> - Clear uri.params before rebuilding the proxy URL to prevent a
>   .resolved file written by the npmsw fetcher (which stores the full URI
>   with checksum params) from contaminating the npm proxy URL with a
>   registry-sourced checksum, bypassing the security invariant that only
>   recipe-provided checksums are trusted.
> - Remove dead 'if ud.version == "latest": return True' branch from
>   need_update(); version=latest is rejected at urldata_init time.
> 
> Migration note: 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")
> Signed-off-by: Thomas Perrot <thomas.perrot@bootlin.com>
> ---
> Thomas Perrot (2):
>       fetch/npmsw: fix security issue and re-enable fetcher
>       tests/fetch: restore and extend npm/npmsw test coverage
> 
>  lib/bb/fetch2/npmsw.py | 12 +++++-----
>  lib/bb/tests/fetch.py  | 64 +++++++++++++++++++++++++++++++++++---------------
>  2 files changed, 51 insertions(+), 25 deletions(-)

I suspect there is a patch missing or something here as it doesn't
touch npm.py and half the changes above appear missing...

Cheers,

Richard
Thomas Perrot June 11, 2026, 7:16 a.m. UTC | #2 
Hello Richard,

On Wed, 2026-06-10 at 17:27 +0100, Richard Purdie via
lists.openembedded.org wrote:
> On Wed, 2026-06-10 at 17:46 +0200, Thomas Perrot via
> lists.openembedded.org wrote:
> > The npm fetcher was disabled in 355cd226 (Jan 2026) because it
> > retrieved
> > checksums from the remote registry rather than from the recipe.  A
> > compromised registry controls both the tarball and its advertised
> > hash,
> > making checksum verification meaningless.
> > 
> > Fix the root cause by separating URL resolution from checksum
> > handling:
> > - _resolve_proxy_url now stores only the bare tarball URL in the
> > .resolved
> >   file; the registry-supplied dist.integrity / dist.shasum values
> > are
> >   ignored entirely.
> > - _setup_proxy builds the proxy URL from that bare tarball URL and
> > injects
> >   the checksum from the recipe's own SRC_URI parameters
> > (sha512sum=,
> >   sha256sum=, etc.).  When no checksum is provided the proxy URL
> > carries
> >   none, and BitBake's standard BB_STRICT_CHECKSUM machinery handles
> > the
> >   missing-checksum case the same way the wget fetcher does.  A
> > bb.warn()
> >   is emitted so recipe authors get a clear signal instead of a
> > silent
> >   unsigned download.
> > - version=latest is now a hard ParameterError instead of a warning;
> > it is
> >   inherently non-reproducible.
> > - Narrow the broad 'except Exception' in _npm_view to
> > json.JSONDecodeError
> >   so that FetchError and ParameterError propagate typed to
> > callers.  Also
> >   fall back to str(error) when 'summary' is absent in the registry
> > error
> >   dict so the message is never silently None.
> > - Clear uri.params before rebuilding the proxy URL to prevent a
> >   .resolved file written by the npmsw fetcher (which stores the
> > full URI
> >   with checksum params) from contaminating the npm proxy URL with a
> >   registry-sourced checksum, bypassing the security invariant that
> > only
> >   recipe-provided checksums are trusted.
> > - Remove dead 'if ud.version == "latest": return True' branch from
> >   need_update(); version=latest is rejected at urldata_init time.
> > 
> > Migration note: 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")
> > Signed-off-by: Thomas Perrot <thomas.perrot@bootlin.com>
> > ---
> > Thomas Perrot (2):
> >       fetch/npmsw: fix security issue and re-enable fetcher
> >       tests/fetch: restore and extend npm/npmsw test coverage
> > 
> >  lib/bb/fetch2/npmsw.py | 12 +++++-----
> >  lib/bb/tests/fetch.py  | 64 +++++++++++++++++++++++++++++++++++---
> > ------------
> >  2 files changed, 51 insertions(+), 25 deletions(-)
> 
> I suspect there is a patch missing or something here as it doesn't
> touch npm.py and half the changes above appear missing...

Sorry, that’s strange, it seems I made a mistake with b4. I’ll resend
them.

Kind regards,
Thomas 

> 
> Cheers,
> 
> Richard
> 
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#19646):
> https://lists.openembedded.org/g/bitbake-devel/message/19646
> Mute This Topic: https://lists.openembedded.org/mt/119741992/5443093
> Group Owner: bitbake-devel+owner@lists.openembedded.org
> Unsubscribe:
> https://lists.openembedded.org/g/bitbake-devel/unsub [thomas.perrot@bootlin.com
> ]
> -=-=-=-=-=-=-=-=-=-=-=-