bug: github.com packages not supported

[vpanta]

We have a transitive dependency on aframe which requires other packages via a github reference. In our pnpm-lock.json that looks like:

  /aframe/1.3.0:
    resolution: {integrity: sha512-f6OQaDf49SzdSxVskJPvPWldOwHpYMsGttmQHhU6FRiASsnyULKG1nqZom9pDuS3fFYEzQVqRMakQ2AhXeLkFg==}
    engines: {node: '>= 4.6.0', npm: '>= 2.15.9'}
    dependencies:
      custom-event-polyfill: 1.0.7
      debug: github.com/ngokevin/debug/ef5f8e66d49ce8bc64c6f282c15f8b7164409e3a
      deep-assign: 2.0.0
      document-register-element: github.com/dmarcos/document-register-element/8ccc532b7f3744be954574caf3072a5fd260ca90
      ...

(caveat: we don't use pnpm ourselves, but this comes from pnpm import on our Node package-lock.json)

With rules_js@0.11.0, bazel sync gets the following error:

Repository rule npm_translate_lock defined at:
  /private/var/tmp/_bazel_vasilios/96c9699731d9b99e83af4f7d36096194/external/aspect_rules_js/npm/npm_import.bzl:32:37: in <toplevel>
ERROR: An error occurred during the fetch of repository 'npm':
   Traceback (most recent call last):
        File "/private/var/tmp/_bazel_vasilios/96c9699731d9b99e83af4f7d36096194/external/aspect_rules_js/npm/private/npm_translate_lock.bzl", line 319, column 33, in _impl
                lockfile = _process_lockfile(rctx)
        File "/private/var/tmp/_bazel_vasilios/96c9699731d9b99e83af4f7d36096194/external/aspect_rules_js/npm/private/npm_translate_lock.bzl", line 145, column 43, in _process_lockfile
                return translate_to_transitive_closure(lockfile, rctx.attr.prod, rctx.attr.dev, rctx.attr.no_optional)
        File "/private/var/tmp/_bazel_vasilios/96c9699731d9b99e83af4f7d36096194/external/aspect_rules_js/npm/private/transitive_closure.bzl", line 86, column 17, in translate_to_transitive_closure
                fail("unsupported package path " + package_path)
Error in fail: unsupported package path github.com/dmarcos/document-register-element/8ccc532b7f3744be954574caf3072a5fd260ca90

This error seems to actually occur when processing the list of packages themselves, which expects first that the package path starts with /, then that it has a resolution.integrity field (these packages have a resolution.tarball field instead).

Presumably, it needs to recognize general paths for packages, and pull them down via the tarball link.

[gregmagolan]

Thanks for the issue. I ran into this as well and we should resolve for 1.0.

[gregmagolan]

Hmm. That is unfortunate that there is no integrity SHA provided for these URLs.

+  github.com/dmarcos/document-register-element/8ccc532b7f3744be954574caf3072a5fd260ca90:
+    resolution: {tarball: https://codeload.github.com/dmarcos/document-register-element/tar.gz/8ccc532b7f3744be954574caf3072a5fd260ca90}
+    name: document-register-element
+    version: 0.5.4
+    dev: true

yarn doesn't either in its lockfile

"document-register-element@github:dmarcos/document-register-element#8ccc532b7f3744be954574caf3072a5fd260ca90":
  version "0.5.4"
  resolved "https://codeload.github.com/dmarcos/document-register-element/tar.gz/8ccc532b7f3744be954574caf3072a5fd260ca90"

Presumably this is because a consistent SHA is not guaranteed over time. Without a SHA, the downloaded archives won't go into the external repository cache but they shouldn't anyway if their SHA is not guaranteed to be consistent.

[vpanta]

Ah yeah, makes sense. So short term just (unfortunately) not caching it makes sense, but long term maybe it's worthwhile to add a dictionary for sha's to the npm_translate_lock call similar to patches?

I appreciate the call-out though, as I'd like to raise this with our teams as it feels like a big code-audit hole.

[gregmagolan]

Cut a release which includes the fix https://github.com/aspect-build/rules_js/releases/tag/v0.11.1

[gregmagolan]

A dictionary of SHAs sounds reasonable to fill in gaps where there is no SHA provided in the lock file.

[alexeagle]

let's use 90% of our energy to push for an upstream fix, they are breaking supply chain security for everyone and it's not a bazel-specific problem...

[vpanta]

let's use 90% of our energy to push for an upstream fix, they are breaking supply chain security for everyone and it's not a bazel-specific problem...

100% agreed here, if anything (at all) is done on this side it should be minimal effort.

[vpanta]

Interesting side note: there actually is a SHA in the package-lock.json for these, it's just not getting propagated to pnpm-lock.yaml I'd guess?

"node_modules/document-register-element": {
      "version": "0.5.4",
      "resolved": "git+ssh://git@github.com/dmarcos/document-register-element.git#8ccc532b7f3744be954574caf3072a5fd260ca90",
      "integrity": "sha512-dwvGei9I/m1pYQ/9aNODyVmvSWBtlncfIROn5Sbi4MVnIcZKre5QaWx+AGLI/j6VH9sp8jwLyeuWP1micANT0g==",
      "license": "MIT"
    },

[alexeagle]

Must be a pnpm bug then