fix: handle malformed npm packages gracefully in extract action (#1794)

* fix: handle malformed npm packages gracefully in extract action

* review comments

.aspect/rules/external_repository_action_cache/npm_translate_lock_LTE4Nzc1MDcwNjU=

@@ -2,26 +2,26 @@
 # Input hashes for repository rule npm_translate_lock(name = "npm", pnpm_lock = "@@//:pnpm-lock.yaml").
 # This file should be checked into version control along with the pnpm-lock.yaml file.
 .npmrc=-2065072158
-pnpm-lock.yaml=-481966245
+pnpm-lock.yaml=-1309835144
 examples/npm_deps/patches/meaning-of-life@1.0.0-pnpm.patch=-442666336
 package.json=-275319675
 pnpm-workspace.yaml=-1178830835
 examples/js_binary/package.json=-41174383
+examples/linked_empty_node_modules/package.json=-1039372825
 examples/macro/package.json=857146175
 examples/npm_deps/package.json=-1377141392
 examples/npm_package/libs/lib_a/package.json=-1377103079
 examples/npm_package/packages/pkg_a/package.json=1006424040
 examples/npm_package/packages/pkg_b/package.json=1041247977
 examples/webpack_cli/package.json=1911342006
 js/private/coverage/bundle/package.json=-1543718929
+js/private/image/package.json=-1260474848
+js/private/test/image/package.json=-687546763
+js/private/test/js_run_devserver/package.json=-260856079
 js/private/worker/src/package.json=1608383745
-npm/private/test/package.json=1756993924
+npm/private/test/package.json=600650131
 npm/private/test/vendored/lodash-4.17.21.tgz=-1206623349
 npm/private/test/npm_package/package.json=-1991705133
 npm/private/test/vendored/is-odd/package.json=1041695223
 npm/private/test/vendored/semver-max/package.json=578664053
-examples/linked_empty_node_modules/package.json=-1039372825
 examples/npm_package/packages/pkg_d/package.json=1110895851
-js/private/image/package.json=-1260474848
-js/private/test/image/package.json=-687546763
-js/private/test/js_run_devserver/package.json=-260856079

npm/private/npm_package_store.bzl

@@ -205,24 +205,22 @@ def _npm_package_store_impl(ctx):
         else:
             package_store_directory = ctx.actions.declare_directory(package_store_directory_path)
             if utils.is_tarball_extension(src.extension):
-                # npm packages are always published with one top-level directory inside the tarball, tho the name is not predictable
-                # we can use the --strip-components 1 argument with tar to strip one directory level
-                args = ctx.actions.args()
-                args.add("--extract")
-                args.add("--no-same-owner")
-                args.add("--no-same-permissions")
-                args.add("--strip-components")
-                args.add(str(1))
-                args.add("--file")
-                args.add(src.path)
-                args.add("--directory")
-                args.add(package_store_directory.path)
-
+                # npm packages are always published with one top-level directory inside the tarball,
+                # tho the name is not predictable we can use the --strip-components 1 argument with
+                # tar to strip one directory level. Some packages have directory permissions missing
+                # executable which make the directories not listable (pngjs@5.0.0 for example). Run
+                # `chmod -R a+X` to fix up these packages (https://stackoverflow.com/a/14634721).
+                # See https://github.com/aspect-build/rules_js/issues/1637 for more info.
                 bsdtar = ctx.toolchains["@aspect_bazel_lib//lib:tar_toolchain_type"]
-                ctx.actions.run(
-                    executable = bsdtar.tarinfo.binary,
+                args = ctx.actions.args()
+                args.add(bsdtar.tarinfo.binary)
+                args.add(src)
+                args.add(package_store_directory.path)  # Need to use `.path` due to: Error in add: Cannot add directories to Args#add since they may expand to multiple values. Either use Args#add_all (if you want expansion) or args.add(directory.path).
+                ctx.actions.run_shell(
+                    tools = [bsdtar.tarinfo.binary],
                     inputs = depset(direct = [src], transitive = [bsdtar.default.files]),
                     outputs = [package_store_directory],
+                    command = "$1 --extract --no-same-owner --no-same-permissions --strip-components 1 --file $2 --directory $3 && chmod -R a+X $3",
                     arguments = [args],
                     mnemonic = "NpmPackageExtract",
                     progress_message = "Extracting npm package {}@{}".format(package, version),

npm/private/test/package.json

@@ -25,6 +25,7 @@
         "typescript": "*",
         "unused": "latest",
         "test-npm_package": "workspace:*",
-        "webpack-bundle-analyzer": "4.5.0"
+        "webpack-bundle-analyzer": "4.5.0",
+        "pngjs": "5.0.0"
     }
 }