Skip to content

Add checks for files with read permissions too narrow #4187

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

Closed
villasv opened this issue Jul 10, 2020 · 4 comments
Closed

Add checks for files with read permissions too narrow #4187

villasv opened this issue Jul 10, 2020 · 4 comments
Labels
feature request Requesting a new feature p3-nice-to-have It should be done this or next sprint

Comments

@villasv
Copy link
Contributor

villasv commented Jul 10, 2020

This is not a problem I perceived directly, but got as feedback from the data science team.

Let's say someone on the team mistakenly ran dvc repro with sudo, generating output files that have strict read permissions. This person then pushes the hashes to git remote and the files to dvc remote. Now the rest of the team can't reproduce the pipeline without sudoeing out the wrong permissions first. To prevent this from happening, it would be cool if DVC issued a warning for files being pushed with weird permissions.

@ghost ghost added the triage Needs to be triaged label Jul 10, 2020
@pared pared added the feature request Requesting a new feature label Jul 10, 2020
@ghost ghost removed the triage Needs to be triaged label Jul 10, 2020
@pared pared added p2-medium Medium priority, should be done, but less important p3-nice-to-have It should be done this or next sprint and removed p2-medium Medium priority, should be done, but less important labels Jul 10, 2020
@efiop
Copy link
Contributor

efiop commented Jul 10, 2020

Hi @villasv !

Long time no see 😉

I suppose they are using local or ssh remote, right? Or are they using shared cache dir?

@efiop efiop added the awaiting response we are waiting for your reply, please respond! :) label Jul 10, 2020
@villasv
Copy link
Contributor Author

villasv commented Jul 13, 2020

IIRC they're using a combination of shared cache dir and s3 remotes, but mostly S3. Does pushing/pulling the file to to/from S3 remote unset those permissions? Didn't think about it, but makes sense. So they might be having trouble with the shared dir, or they're attempting to change protected hardlinks and misdiagnosed the problem.

@efiop
Copy link
Contributor

efiop commented Jul 13, 2020

@villasv Yeah, s3 doesn't preserve the regular fs permissions, so the issue is clearly on the cache dir side. Also, I wonder if they use dvc config cache.shared group setting?

@villasv
Copy link
Contributor Author

villasv commented Jul 13, 2020

If it's not default, probably not. My bet is on the "attempting to change protected files" side. I'm going to leave the feature suggestion if it can be useful for people using the shared cache settings.

@efiop efiop removed the awaiting response we are waiting for your reply, please respond! :) label Mar 1, 2021
@mattseddon mattseddon closed this as not planned Won't fix, can't repro, duplicate, stale Mar 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Requesting a new feature p3-nice-to-have It should be done this or next sprint
Projects
None yet
Development

No branches or pull requests

4 participants