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

install: don't preserve owner when extracting files #3065

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

behrmann
Copy link

Checklist
  • npm install && npm run lint && npm test passes
  • tests are included
  • documentation is changed or added
  • commit message follows commit guidelines
Description of change

tar defaults to true for preserveOwner when run as root.

In a user namespace one can be root, but not have the capability to chown files, e.g. because of a seccomp filter.

It doesn't make sense to preserve the owner anyway, root or not, since the owner in a tar is chosen by the side providing the tar file, so the user name might not be present in the system. Also, all other files written, would also have a different owner than what was extracted from the tar file.

This will result in errors when run as root in a user namespace without the
capability to chown files. Since the owner of a file in a tar can be arbitrary
and not be present in the system, the owner is uninteresting anyway.
@behrmann
Copy link
Author

My bad, the sandbox that made this necessary actually didn't catch the chown calls, so this explains why the errors from chown bubbled up. That being said, even if gyp could untar things with their uid from the tar, it shouldn't since it doesn't control the input and the uid might not exist on the system. Chowning to an effectively random number doesn't make a lot of sense.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant