Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Fix default permissions in UnixFileStream#6797

Merged
stephentoub merged 1 commit intodotnet:masterfrom
stephentoub:fix_file_perms
Mar 10, 2016
Merged

Fix default permissions in UnixFileStream#6797
stephentoub merged 1 commit intodotnet:masterfrom
stephentoub:fix_file_perms

Conversation

@stephentoub
Copy link
Member

We currently have UnixFileStream create new files with the permissions 700. This doesn't map to what's commonly expected on Unix, where most utilities default instead to at least 666.

This commit changes the default to be 766, which attempts to map to what's commonly expected on Unix, while also factoring in the default on Windows, where a new FileStream's output can be executed by default. All of these permissions are of course subject to the system's umask, such that the actual permissions applied will likely be less than what's specified.

The commit also fixes up some code in X509Certificates, which was coded to rely on the more limited permissions being applied.

cc: @bartonjs, @ianhays

We currently have UnixFileStream create new files with the permissions 700.  This doesn't map to what's commonly expected on Unix, where most utilities default instead to 666.

This commit changes the default to be 766, which attempts to map to what's commonly expected on Unix, while also factoring in the default on Windows, where a new file can be executed by default.  All of these permissions are of course subject to the system's umask, such that the actual permissions applied will likely be less than what's specified.

The commit also fixes up some code in X509Certificates, which was coded to rely on the more limited permissions being applied.
@bartonjs
Copy link
Member

@blowdart does this make you happy, sad, or indifferent? 😄

@blowdart
Copy link
Contributor

When am I ever happy? :D

So what are you writing that needs execute permission on them? I suggest making the FileStream on windows more restrictive by default, to match 666

@bartonjs
Copy link
Member

When am I ever happy? :D

#6672 (comment) 😄

I suggest making the FileStream on windows more restrictive by default, to match 666

I didn't know that NTFS had a no-execute mode... but that'd probably come as quite a shock to people if we used it :).

@bartonjs
Copy link
Member

@stephentoub The change LGTM, in that it matches our offline discussion. Since it's the same as the default for redirection, mkdir, etc, I think we're probably good here; given the X509 lockdown you did for me.

@blowdart
Copy link
Contributor

Damnit I was happy.

And yea, I figured Traverse Folder / Execute File would be it, but it doesn't do what I expect. It only disables running in explorer for bats, exes and other things. Darn it.

@stephentoub
Copy link
Member Author

Thanks, guys.

stephentoub added a commit that referenced this pull request Mar 10, 2016
Fix default permissions in UnixFileStream
@stephentoub stephentoub merged commit 529bc34 into dotnet:master Mar 10, 2016
@stephentoub stephentoub deleted the fix_file_perms branch March 10, 2016 05:14

// If the file gets created a new, we'll select the permissions for it. Most utilities by default use 666 (read and
// write for all). However, on Windows it's possible to write out a file and then execute it. To maintain that similarity,
// we use 766, so that in additoin the user has execute privileges. No matter what we choose, it'll be subject to the umask
Copy link
Contributor

Choose a reason for hiding this comment

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

NIT: "additoin" (Sorry, but when I catch a spelling error, I feel obligated to point it out...)

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks :) Fixed.

@karelz karelz modified the milestone: 1.0.0-rtm Dec 3, 2016
picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
Fix default permissions in UnixFileStream

Commit migrated from dotnet/corefx@529bc34
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants