Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug]: NpmPackageExtract extraction of malformed packages causes errors in RBE/Remote Cache #1637

Closed
Tracked by #1671
qtica opened this issue Apr 10, 2024 · 9 comments · Fixed by #1794
Closed
Tracked by #1671
Labels
bug Something isn't working

Comments

@qtica
Copy link

qtica commented Apr 10, 2024

What happened?

When depending on an npm package that has an archive with directories that do not contain the searchable permission (e.g.
drw-r--r--), the bazel cache fails to write the action output to the cache getting a permission denied on the files in the extracted non-searchable directory. If you are using remote execution, you may have the build exit due to the executors failing with the same permission denied. This started with the latest feature if extracting tars vis bsdtar. The prior method of extracting packages did not seem to have any issue.

I also think it would be nice if rules_js could gracefully handle this scenario considering npm and pnpm appear to handle extracting the package okay. However, at the end of the day, I don't really know how frequent this is, nor if it's really worth the effort of maintaining on your side. I mostly just wanted to report it for visibility.

Related issue on package where I noticed this: pngjs/pngjs#381

I've also reported this to the remote executions service I am using, as it crashes the remote executor.

Version

Development (host) and target OS/architectures:

Output of bazel --version: 7.1.1

Version of the Aspect rules, or other relevant rules from your
WORKSPACE or MODULE.bazel file: 1.4.0

Language(s) and/or frameworks involved:

How to reproduce

A full reproducible repo can be found here: https://github.com/qtica/bazel-rules-js-cache-error-with-bad-package?tab=readme-ov-file

In short, depend on pngjs, have RBE / remote cache (--disk_cache works too) setup and build your target. The remote cache will provide a warning with permission denied. The remote execution service I use results in a crashed executor with the error permission denied resulting in a failed build.

The full details are in the README.md of the reproducible repo.

Any other information?

My current workaround is to vendor the package and provide overrides in the package.json.

{
  // <redacted>
  "pnpm": {
    "overrides": {
      "pngjs": "file:./third_party/npm/pngjs-5.0.0"
    }
  }
}
@qtica qtica added the bug Something isn't working label Apr 10, 2024
@github-actions github-actions bot added the untriaged Requires traige label Apr 10, 2024
@gregmagolan gregmagolan removed the untriaged Requires traige label May 23, 2024
@gregmagolan
Copy link
Member

gregmagolan commented May 23, 2024

For future reference, one our of consulting clients hit something similar:

BUILD:8:22: Extracting npm package [email protected] failed: I/O exception during sandboxed execution: Could not move output artifacts from sandboxed execution

Here is the internal Slack thread link for that: https://aspect-build.slack.com/archives/C04PGV0U79P/p1715100472949349?thread_ts=1715093034.381059&cid=C04PGV0U79P

@gregmagolan
Copy link
Member

gregmagolan commented May 23, 2024

A Workflows customer may have also seen this with the [email protected] package:

21:31:30) WARNING: Remote Cache: /mnt/ephemeral/output/__main__/execroot/__main__/bazel-out/k8-fastbuild/bin/node_modules/.aspect_rules_js/[email protected]_react_18.1.0/node_modules/its-fine/src/index.tsx (Permission denied)
(21:32:42) WARNING: Remote Cache: /mnt/ephemeral/output/__main__/execroot/__main__/bazel-out/k8-opt-ST-d57f47055a04/bin/node_modules/.aspect_rules_js/[email protected]_react_18.1.0/node_modules/its-fine/src/index.tsx (Permission denied)

Internal slack link for future reference, https://aspect-build.slack.com/archives/C04BWU1519D/p1716500042335849

@AustinSchuhBRT
Copy link

https://github.com/frc971/971-Robot-Code/blob/master/third_party/rules_js/0001-Fix-package-permissions.patch is the workaround I applied to fix this. I don't know if that is good enough or performant enough to submit.

@gregmagolan
Copy link
Member

Fixed with #1794. Fix will be included in rules_js 2.0 final. It could be ported back to 1.x branch with a cherry-pick if needed. Please post here if you're on stuck on rules_js 1.x and you are hitting this and we'll backport the fix.

@fa93hws
Copy link

fa93hws commented Aug 7, 2024

Stucked on rules_js 1.x. Exactly the same issue, pngjs@5 with remote execution ✋

@gregmagolan
Copy link
Member

@fa93hws Noted. I'll backport to 1.x branch after the fix for https://bazelbuild.slack.com/archives/CEZUUKQ6P/p1723821643734399 lands.

@gregmagolan
Copy link
Member

@fa93hws Issues came up with the blanket chmod approach that landed on the 2.x branch so we're backing out of that change in favour of opting-in problematic packages to the chmod fix via custom_postinstall. For example,

npm_translate_lock(
    name = "npm",
    custom_postinstalls = {
        "pngjs": "chmod -R a+X *",
    },
    ...

You can use that approach with rules_js 1.x. Nothing to backport at this point and not needed to unblock you if you use custom_postinstalls.

See #1904 for more details.

@fa93hws
Copy link

fa93hws commented Aug 22, 2024

Many thanks, will add a patch

@rahul-roy-glean
Copy link

rahul-roy-glean commented Aug 28, 2024

we saw this happening sporadically for the following packages , putting this out in case more folks are seeing this -

custom_postinstalls = {
        "pngjs": "chmod -R a+X *",
        "websocket-extensions": "chmod -R a+X *",
        "pretty-format": "chmod -R a+X *",
        "readdirp": "chmod -R a+X *",
        "serve-static": "chmod -R a+X *",
        "serialize-javascript": "chmod -R a+X *",
        "supports-preserve-symlinks-flag": "chmod -R a+X *",
        "strip-final-newline": "chmod -R a+X *",
        "webpack-dev-server": "chmod -R a+X *",
        "tapable": "chmod -R a+X *",
        "statuses": "chmod -R a+X *",
        "@nivo/legends": "chmod -R a+X *",
        "@lingui/react": "chmod -R a+X *",
        "license-webpack-plugin": "chmod -R a+X *",
        "command-exists-promise": "chmod -R a+X *",
    },

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants