-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Ensure file-based program artifacts are restricted to the current user #48813
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
base: main
Are you sure you want to change the base?
Conversation
// The directory might have been created by someone else, set its permissions again to be sure. | ||
new DirectoryInfo(directory).UnixFileMode = mode; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So this is a change from what we had previously said right? I thought we'd said we would ensure the permissions were set on creation but not if the directory already existed, given the directory path is derived from the entry-point file path and under our own sub-directory. If this is effectively free perf-wise and we're not concerned about some odd edge case potentially causing this to flip owners on each run, then it's likely fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I forgot we said that, makes sense, I will remove this, thanks.
@dotnet/run-file for reviews, thanks |
@@ -807,6 +807,47 @@ public void NoBuild_02() | |||
.And.HaveStdOut("Changed"); | |||
} | |||
|
|||
[Fact] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: We have a LinuxOnlyFact
if you want to use that instead. Or a PlatformSpecificFact
that takes an or'ed set of platform flags.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Didn't find LinuxOnlyFact
actually, but PlatformSpecificFact(TestPlatforms.AnyUnix)
looks good, thanks.
No description provided.