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

set file permissions on Unix #382

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

set file permissions on Unix #382

wants to merge 1 commit into from

Conversation

wfurt
Copy link
Member

@wfurt wfurt commented Nov 1, 2024

What's the problem?

fixes #357 Incorrect file permissions for ccache file

What's the solution?

When file is created it will have some default permissions on Unix. Depending on system configuration that can allow read access to anybody.

Describe the solution here.

There is no standard way how to set Unix permissions on File class - specialty when targeting Framework (not really applicable) or .NET Standard. .NET Core 7.0 introduced method to set permissions (essentially chmod) and this PR tries to use it on Unix via Reflection.

I was wondering where to put it @SteveSyfuhs and I put it to base file class instead of just the ccache. If there are files that should be readable or writable by others that may be the wrong place. The code also tries to create directory if needed and I'm not sure if we should be explicit there is well in case we end up creating it. (that is different from the example in /tmp where we should not touch the permissions)

  • Includes unit tests
  • Requires manual test

I'm not sure how to construct Unix specific test and how to verify the functionality in portable way. If you have sgesstions please let me know.

What issue is this related to, if any?

#357

@SteveSyfuhs
Copy link
Collaborator

I'm happy with this. It's the right spot and nothing else needs specific permissions. The only other writable thing is the configuration, which is generally going to be provided by an admin and also doesn't contain anything sensitive.

@SteveSyfuhs
Copy link
Collaborator

Build succeeded but failed a test. Its a flaky test so rerunning. Is probably fine.

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.

[security] Incorrect file permissions for ccache file
2 participants