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.

Sign up for GitHub

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

Jump to bottom

chmod keystores and deposit_data.json to 440 #176

Merged
merged 2 commits into from
Jan 29, 2021
Merged

chmod keystores and deposit_data.json to 440 #176

merged 2 commits into from
Jan 29, 2021

Conversation

CarlBeek
Copy link
Collaborator

@CarlBeek CarlBeek commented Dec 18, 2020 •
edited
Loading

File permissions seem different between users and are usually globally readable. This PR address this by chmoding the keystores and deposit_data.json to 440 owner and group read only.

@CarlBeek CarlBeek added the enhancement New feature or request label Dec 18, 2020
@CarlBeek CarlBeek requested a review from hwwhww December 18, 2020 15:57
@Grubson33421
Copy link

File permissions seem different between users and are usually globally readable. This PR address this by chmoding the keystores and deposit_data.json to 440 owner and group read only.

hwwhww
hwwhww approved these changes Jan 26, 2021
View reviewed changes
Copy link
Contributor

@hwwhww hwwhww left a comment

Choose a reason for hiding this comment

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

The PR looks good.

I guess some users may see new permission denied errors per their environment configurations. What do you think about adding a note about the 440 permission to the README.md?

@CarlBeek
Copy link
Collaborator Author

Definitly worth explaining in the README.md, thanks for the recommendation.

@CarlBeek CarlBeek changed the base branch from master to dev January 27, 2021 14:07
@CarlBeek CarlBeek merged commit d45d894 into dev Jan 29, 2021
@CarlBeek CarlBeek mentioned this pull request Jan 29, 2021
Closed
@CarlBeek CarlBeek mentioned this pull request Mar 22, 2021
Merged
CarlBeek added a commit that referenced this pull request Mar 22, 2021
@CarlBeek
Merge pull request #190 from ethereum/dev
7bac110 
## Release v1.1.1:

* Ensure keystore permissions are 440 #176
* Adds Prater testnet support #189
* Removes support for old testnets (Witti, Altona, Medalla, Spadina, Zinken) #189
everhusk pushed a commit to earthwallet/earth-wallet-cli that referenced this pull request Aug 3, 2023
@CarlBeek
Merge pull request ethereum#176 from ethereum/carl_chmod
d95d787 
chmod keystores and deposit_data.json to `440`
everhusk pushed a commit to earthwallet/earth-wallet-cli that referenced this pull request Aug 3, 2023
@CarlBeek
Merge pull request ethereum#190 from ethereum/dev
0f7ed22 
## Release v1.1.1:

* Ensure keystore permissions are 440 ethereum#176
* Adds Prater testnet support ethereum#189
* Removes support for old testnets (Witti, Altona, Medalla, Spadina, Zinken) ethereum#189
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hwwhww hwwhww hwwhww approved these changes

Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@CarlBeek @Grubson33421 @hwwhww