Skip to content

Disallow root script run #209

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

Merged
merged 4 commits into from
Oct 22, 2023
Merged

Disallow root script run #209

merged 4 commits into from
Oct 22, 2023

Conversation

DE0CH
Copy link
Contributor

@DE0CH DE0CH commented Oct 22, 2023

I added a check to stop the script if the user is root. Also, I hid an environment variable ALLOWROOT which if set to 1 will allow the script to run if the user wants to run it as root for whatever reason.

This would avoid numerous issues with ~/DoChat becoming owned by root causing permission denided issues (such as in #165 and #178).

Also, I got rid of the warning message of lshw -C display which, if not run with sudo, confusingly prints out WARNING: you should run this program as super-user., which can mislead people (including me) to run the script with sudo

@CLAassistant
Copy link

CLAassistant commented Oct 22, 2023

CLA assistant check
All committers have signed the CLA.

Copy link
Owner

@huan huan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for great improvement, it definitely can help users to prevent the related issues you mentioned.

it will good to be merged after you added the message I mentioned in the comment.

@DE0CH
Copy link
Contributor Author

DE0CH commented Oct 22, 2023

Thanks. I just made the changes requested.

huan
huan previously approved these changes Oct 22, 2023
Copy link
Owner

@huan huan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

fix shell return code
Copy link
Owner

@huan huan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed shellcheck

@huan huan merged commit c176c4a into huan:main Oct 22, 2023
@huan
Copy link
Owner

huan commented Oct 22, 2023

It seems that there's a problem when publishing the image to the docker hub: https://github.com/huan/docker-wechat/actions/runs/6606361107/job/17942485588#step:5:294

@DE0CH Could you please take a look at it?

Thank you very much!

@DE0CH
Copy link
Contributor Author

DE0CH commented Oct 23, 2023

Yep. #210.

I actually ran into this problem when debugging, but seeing that the docker image somehow was built and published to the docker hub I thought it didn't need fixing.

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.

3 participants