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

sudo composer self-update changes owner to root for current user composer cache dir #6602

Closed
Koc opened this issue Aug 11, 2017 · 16 comments
Closed

Comments

@Koc
Copy link
Contributor

Koc commented Aug 11, 2017

When I run this command:

# install composer as global
sudo mv composer.phar /usr/bin/local/composer
# wait for new composer version, then update
sudo composer self-update
ls -l ~/.composer/

I get the following output:

total 1820
drwxr-xr-x  3 root root    4096 Aug 11 13:26 ./
drwxr-xr-x 16 dev  dev     4096 Aug 11 13:26 ../
-rw-r--r--  1 root root 1838958 Aug 11 13:26 2017-05-17_08-17-52-1.4.2-old.phar
drwxr-xr-x  4 root root    4096 Aug 11 13:26 cache/
-rw-r--r--  1 root root      13 Aug 11 13:26 .htaccess
-rw-r--r--  1 root root     799 Aug 11 13:26 keys.dev.pub
-rw-r--r--  1 root root     799 Aug 11 13:26 keys.tags.pub

And I expected this to happen:

drwxr-xr-x  3 dev dev    4096 Aug 11 13:26 ./
drwxr-xr-x 16 dev dev    4096 Aug 11 13:26 ../
-rw-r--r--  1 dev dev 1838958 Aug 11 13:26 2017-05-17_08-17-52-1.4.2-old.phar
drwxr-xr-x  4 dev dev    4096 Aug 11 13:26 cache/
-rw-r--r--  1 dev dev      13 Aug 11 13:26 .htaccess
-rw-r--r--  1 dev dev     799 Aug 11 13:26 keys.dev.pub
-rw-r--r--  1 dev dev     799 Aug 11 13:26 keys.tags.pub

sudo chown dev:dev -R ~/.composer fixes the problem but I need call it after each sudo composer self-update

@curry684
Copy link
Contributor

Agreed that this should never occur.

@alcohol
Copy link
Member

alcohol commented Aug 14, 2017

You run composer with sudo, so I am not sure why you expect any different results?

@alcohol alcohol closed this as completed Aug 14, 2017
@curry684
Copy link
Contributor

@alcohol it reassigned cache ownership for the current user to root, with mode 755. He cannot delete his own cache anymore.

@alcohol
Copy link
Member

alcohol commented Aug 14, 2017

Then he should not use sudo. Depending on your local setup, sudo can inherit the $HOME value of the user that is invoking sudo, so it is pretty straightforward why his permissions are screwed up. Maybe he never noticed the big warning we throw about not running composer as root?

@Koc
Copy link
Contributor Author

Koc commented Aug 14, 2017

  1. It was work ok on previous versions
  2. I'm not doing composer update with sudo. I'm doing composer self-update. https://getcomposer.org/doc/03-cli.md#self-update as I see - docs was updated some time ago and correct upgrade process is set flag -H for sudo command - sudo -H composer self-update

@Seldaek
Copy link
Member

Seldaek commented Aug 14, 2017

Yup, without -H the HOME dir is not set as root's, so composer will behave as usual but with your user's home dir. I agree that for self-update and well in general it should not change the ownership though. Are you sure that if the dir already exists it changes it to be root owned? If it does not exist yet I can see that it would create it as root, and I don't know if we wanna fix that, but an existing one should not be changed.

@curry684
Copy link
Contributor

curry684 commented Aug 14, 2017

for self-update and well in general it should not change the ownership though

That's the core point, all the rest is logical.

edit: looking at TS again it seems to be first run though. In that case it makes sense that it's happening. We may however want to 777 the caches and/or detect the SUDO_USER env variable.

@Koc
Copy link
Contributor Author

Koc commented Aug 14, 2017

Are you sure that if the dir already exists it changes

Will recheck it again, but afraid that it changes owner when directory already exists.

@Koc
Copy link
Contributor Author

Koc commented Nov 10, 2017

Can we reopen this issue with DX label and low priority?

We can improve DX a little bit by adding additional checks. For example pip inform user

The directory 'XXX' or its parent directory is not owned by the current user and the debug log has been disabled. Please check the permissions and owner of that directory. If executing pip with sudo, you may want the -H flag.

We can inform user and ask prompt for confirm possible destructive actions.

@Seldaek Seldaek reopened this Nov 18, 2017
@Seldaek
Copy link
Member

Seldaek commented Nov 18, 2017

Right a more verbose warning sounds reasonable.

@omoustaouda
Copy link

hello, I'd like to contribute to Composer and go deeper in my knowledge of PHP,
would this be a good first issue?

if not, please propose me an alternative one,
thank you!

@curry684
Copy link
Contributor

curry684 commented Nov 7, 2018

Assuming you have proper knowledge of Unix file permissions and a clear idea on how to solve the issue. If in doubt I'd recommend sharing your plans first and asking for feedback, I'll be happy to comment if pinged as I wrote some sudo-related parts of Composer.

In all honesty though the Composer codebase is not exactly "simple". I'm not sure where you are now if you wish to "go deeper" but contributing to this project is not something I'd recommend right away to someone without 3+ years of solid PHP knowledge. This issue may be a "Good First Issue" but Composer is not really a "Good First Project" ;)

@omoustaouda
Copy link

I'll do that! To propose an implementation before starting to do it.

I work with PHP since 5 years and I've been using Linux daily since 8+ years,
as first step I'll dive into the source code to start mapping it into my mind, and then after, if things are clear, I can propose an implementation here.

@omoustaouda
Copy link

There is already a warning message when the home directory owner is different from the user caller of composer self-update

the check is done here:
https://github.com/composer/composer/blob/master/src/Composer/Command/SelfUpdateCommand.php#L111-L118

I tried many different combinations (sudo, sudo -H, composer self-update --{stable, preview,snapshot}) to reproduce the error but I can't get the owner of ~/.composer changed to root.

@Koc can you please try to see if you still get the error in the current version? Thank you!

@artem-prozorov
Copy link

Is this still an active issue?

@Seldaek
Copy link
Member

Seldaek commented Oct 26, 2020

No idea, but closing as IMO not really a big issue. We warn on sudo usage already.

@Seldaek Seldaek closed this as completed Oct 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants