Sitelet https://github.com/gpodder/gpodder/pull/1028
Skip to content

Create config file with group/world permissions revoked - #1028

Merged
elelay merged 1 commit into
gpodder:masterfrom
juergenhoetzel:config_permissions
May 10, 2021
Merged

elelay merged 1 commit into
gpodder:masterfrom
juergenhoetzel:config_permissions

Conversation

@juergenhoetzel

Copy link
Copy Markdown
Contributor

The config file contains sensible information (user/password).

Also remove unused import.

@auouymous

Copy link
Copy Markdown
Member

I run my entire system with 077 and fully agree with this change. The patch looks good and appears to work fine, but will it cause any issues on Windows?

@juergenhoetzel

Copy link
Copy Markdown
Contributor Author

I run my entire system with 077 and fully agree with this change. The patch looks good and appears to work fine, but will it cause any issues on Windows?

Posix umask is also awailable on Windows: https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/posix-umask?view=msvc-160

@elelay

elelay commented May 9, 2021

Copy link
Copy Markdown
Member

Thanks for the patch!

Tested on windows:

  • it doesn't crash
  • it doesn't change permissions.

withumask

I can't find information on python's os.umask, but testing results and this commit https://git.lclutz.de/ghstars/certbot-certbot/commit/50fa04ba0cb739eb0eafec2286cb08d9caae1b16 leads me to believe it's stubbed out on windows...

@elelay

elelay commented May 9, 2021

Copy link
Copy Markdown
Member

More direct test:

C:\Users\IEUser>C:\gpodder-3.10.17-portable\data\bin\python
Python 3.8.9 (default, Apr  3 2021, 09:29:25)  [GCC 10.2.0 32 bit] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> import os
>>> os.umask(0)
0
>>> os.umask(0o077)
0
>>> os.umask(0)
0

Also cpython tests don't assert modified persmissions on windows https://github.com/python/cpython/blob/bb3e0c240bc60fe08d332ff5955d54197f79751c/Lib/test/test_os.py#L1582

@juergenhoetzel please add a simple comment that it's a noop on windows and let's merge, to improve it on other platforms.

The config file contains sensible information (user/password).

Also remove unused import.
@juergenhoetzel

Copy link
Copy Markdown
Contributor Author

@juergenhoetzel please add a simple comment that it's a noop on windows and let's merge, to improve it on other platforms.

Thank you for your research. I just amended the commit and added your suggestion.

@elelay
elelay merged commit 5d4345b into gpodder:master May 10, 2021
@elelay

elelay commented May 10, 2021

Copy link
Copy Markdown
Member

Merged, thanks!

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