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.

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

[Unix] Fix temporary file permissions. #78347

Merged
merged 1 commit into from Jun 18, 2023

Conversation

bruvzg
Copy link
Member

@bruvzg bruvzg commented Jun 16, 2023

Fixes #78338

mkstemp default to 0600 instead of 0666 modified by the process's umask value, which is used by fopen.

@bruvzg bruvzg added this to the 4.x milestone Jun 16, 2023
@bruvzg bruvzg requested a review from a team as a code owner June 16, 2023 20:36
@akien-mga akien-mga modified the milestones: 4.x, 4.1 Jun 17, 2023
@lostminds
Copy link

Should permissions really be 666? For the final bundle files as mentioned in #78338 they seem to be 755, as in read and execute but not write permissions for group and everyone. As opposed to just read and write for all with 666?

@bruvzg
Copy link
Member Author

bruvzg commented Jun 17, 2023

In practice its 0644 (since it will apply umask), this is the default permissions, which were used before. 0755 should be used for executables only.

@lostminds
Copy link

Ok, I guess the issue was the lack of read permissions was the main issue. The 0755 was just something I got from looking into other app bundles and checking what they used.

For example here's was my Safari bundle content looks like:

drwxr-xr-x   3 root  wheel     96 Apr  1 08:53 Extensions
-rw-r--r--   1 root  wheel  21948 Apr  1 08:53 Info.plist
drwxr-xr-x   4 root  wheel    128 Apr  1 08:53 MacOS
-rw-r--r--   1 root  wheel      8 Apr  1 08:53 PkgInfo
drwxr-xr-x   5 root  wheel    160 Apr  1 08:53 PlugIns
drwxr-xr-x  93 root  wheel   2976 Apr  1 08:53 Resources
drwxr-xr-x   6 root  wheel    192 Apr  1 08:53 XPCServices
drwxr-xr-x   3 root  wheel     96 Apr  1 08:53 _CodeSignature
-rw-r--r--   3 root  wheel    468 Apr  1 08:53 version.plist

While not everything is set to 755 like I did in my blunt test. You can see that it's not just the executable that has 755 permissions set.

@AThousandShips
Copy link
Member

AThousandShips commented Jun 18, 2023

Those are all directories though, not files, none of the files there have execute permissions in your example, and for directories it's not execute but:

When set for a directory, the execute permission is interpreted as the search permission: it grants the ability to access file contents and meta-information if its name is known, but not list files inside the directory, unless read is set also.

@akien-mga akien-mga merged commit b7976f4 into godotengine:master Jun 18, 2023
13 checks passed
@akien-mga
Copy link
Member

Thanks!

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

Successfully merging this pull request may close these issues.

Incorrect permissions in macOS export bundle blocks upload to App Store
4 participants