savoirfairelinux / sous-chef

Sous-Chef is a web application to help organizations to plan and deliver meals, and to manage clients files.
GNU Affero General Public License v3.0
67 stars 45 forks source link

--unsafe-perm #759

Closed kousu closed 7 years ago

kousu commented 7 years ago

INSTALL.md doesn't explain why --unsafe-perm is necessary. I'm having trouble understanding the official docs but it sounds like --unsafe-perm means not only that it skips setuid() --- so it runs as your current user --- but that not passing it implies it always setuid()s, which would fail when run unprivileged? It all seems a little bonkers to me.

Anyway, it would help if this was cleared up. My instinct is to remove --unsafe-perm. Is it really necessary? Can we get around it without having to type the scary flag? If not, can we explain better just why we need this in the docs? Thank you.

lingxiaoyang commented 7 years ago

@kousu Thank you for reporting. I tested again with a fresh installation, and when I was inside the container, I was the root user and npm install worked fine without --unsafe-perm.

Then I went outside the container and it worked again without --unsafe-perm.

According to the doc, we don't have to explicitly set this option. The default value is false when the user is root, otherwise true.

Then I consulted previous IRC/email communications. I added it myself because we were using a few outdated Node.js packages at that time and --unsafe-perm was in fact a temporary solution. So I'm sure that we can remove this in INSTALL.md now. It's not needed anymore as we have already updated all the dependencies.

Could you please include this issue in your PR #762? Thank you!

kousu commented 7 years ago

Putting it in #762 is a great idea. I'll do that.

erozqba commented 7 years ago

I'm closing this because it's already in place with PR #775