search

mattock / automatic-cloud-backup

19 stars 19 forks source link

Fix #2 Change umask so all generated files are only readable by owner #5

Closed bgupta closed 7 years ago

bgupta commented 7 years ago

This will change the perms for all files, including cookie and backups themselves.

bgupta commented 7 years ago

@dzschille @mattock Is this ok? I went beyond the scope of the initial concern about the cookie file having the wrong perms.. and changed the umask so that everything the script writes should be 0600. (umask 0077)

dzschille commented 7 years ago

@bgupta why you want to set the permissions in the script? Normaly you define the umask for a user in /etc/profile or ~/.bashrc. All scripts executed by the user should set the file perms accordingly. Or did i miss the point of the PR here?

bgupta commented 7 years ago

@dzschillle See https://github.com/mattock/automatic-cloud-backup/issues/2 (The umask only scopes the commands in the script, and I thought it cleaner than running a chmod after the file(s) are created.)

mattock commented 7 years ago

I think setting umask makes sense if there would otherwise be several chmod invocations.

dzschille commented 7 years ago

It is also possible to define the umask in the crontab:

10 4 * * * umask 077 ; /path/to/script
bgupta commented 7 years ago

I don't really care. Please reject or accept the PR.

On Sun, Jul 23, 2017 at 8:44 AM, David Zschille notifications@github.com wrote:

It is also possible to define the umask in the crontab:

10 4 * umask 077 ; /path/to/script

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/mattock/automatic-cloud-backup/pull/5#issuecomment-317250478, or mute the thread https://github.com/notifications/unsubscribe-auth/AAwJcfUo8Tk_PS_D0PBCnDwtErxtDw-hks5sQ0AkgaJpZM4Ob7Pf .

-- http://www.aws-partner-directory.com/PartnerDirectory/PartnerDetail?Name=Brandorr+Group+LLC

dzschille commented 7 years ago

I don't see an advantage with this change, so i would reject the PR. @mattock are you okay with that decision?

bgupta commented 7 years ago

@mattock If you do reject you should probably close https://github.com/mattock/automatic-cloud-backup/issues/2

mattock commented 7 years ago

@dzschille in a script using chmod is more typical than changing the defaults (umask). So I'm fine with rejecting this PR to reduce the surprise factor. That said, I think https://github.com/mattock/automatic-cloud-backup/issues/2 can and should be fixed.