Set initial config perms 0600, it holds secrets #120

Merged
merged 2 commits into from Apr 30, 2012

Conversation

Projects
None yet
3 participants
@bemurphy
Contributor

bemurphy commented Apr 28, 2012

Since the config file holds secret keys for oauth, I figured it was best written default the first time with 0600 perms.

@no6v

This comment has been minimized.

Show comment Hide comment
@no6v

no6v Apr 28, 2012

Collaborator

Thank you for noticing this. I agree with this patch even though I prefer using mode: "w", perm: 0600 style :).

I think this File instance could be closed immediately by giving (empty) block or close explicitly.
How do you feel? Is that GC job?

Collaborator

no6v commented Apr 28, 2012

Thank you for noticing this. I agree with this patch even though I prefer using mode: "w", perm: 0600 style :).

I think this File instance could be closed immediately by giving (empty) block or close explicitly.
How do you feel? Is that GC job?

@bemurphy

This comment has been minimized.

Show comment Hide comment
@bemurphy

bemurphy Apr 28, 2012

Contributor

Hmm, I wasn't aware File.open could be passed the perm in a hash. If so, that's probably more explicit.

As for closing it, I'd favor explicit .close over an empty block; it would confuse a reader less. I will drop both in an additional commit.

Contributor

bemurphy commented Apr 28, 2012

Hmm, I wasn't aware File.open could be passed the perm in a hash. If so, that's probably more explicit.

As for closing it, I'd favor explicit .close over an empty block; it would confuse a reader less. I will drop both in an additional commit.

@no6v

This comment has been minimized.

Show comment Hide comment
@no6v

no6v Apr 29, 2012

Collaborator

Does "w" leave alone? or {mode: "w"} ? But that is not essential.
Now, everything is reasonable for me, thanks.
@jugyo, can I merge this?

Collaborator

no6v commented Apr 29, 2012

Does "w" leave alone? or {mode: "w"} ? But that is not essential.
Now, everything is reasonable for me, thanks.
@jugyo, can I merge this?

@no6v

This comment has been minimized.

Show comment Hide comment
@no6v

no6v Apr 29, 2012

Collaborator

Sorry for my misreading. mode: is there exactly!

Collaborator

no6v commented Apr 29, 2012

Sorry for my misreading. mode: is there exactly!

jugyo added a commit that referenced this pull request Apr 30, 2012

Merge pull request #120 from bemurphy/fix_config_permissions
Set initial config perms 0600, it holds secrets

@jugyo jugyo merged commit 42f70ba into jugyo:master Apr 30, 2012

@jugyo

This comment has been minimized.

Show comment Hide comment
@jugyo

jugyo Apr 30, 2012

Owner

It was merged.
I think 0600 is best.

Thanks!

Owner

jugyo commented Apr 30, 2012

It was merged.
I think 0600 is best.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment