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

Should not change permissions without consent #9

Closed
KalleDK opened this issue Jan 7, 2021 · 2 comments
Closed

Should not change permissions without consent #9

KalleDK opened this issue Jan 7, 2021 · 2 comments

Comments

@KalleDK
Copy link
Contributor

KalleDK commented Jan 7, 2021

xdg/base_dirs.go

Lines 52 to 55 in aad56ae

// The runtime directory must be owned by the user.
if err = chown(bd.runtime, os.Getuid(), os.Getgid()); err != nil {
return "", err
}

I think this should give an error with wrong permissions, and not try to set permissions on it own. This path could already contain files from other programs, that "by error" rely on these permissions, and by changing them maybe break them.

@adrg
Copy link
Owner

adrg commented Jan 7, 2021

Yes, the verification should probably not be done at all. It's not really the responsibility of the library to check if the user owns the directory, if it already exists. If users don't have write access to the defined runtime directory, they will receive an error when attempting to write the runtime file, or when adding sub-directories to the path.

If the directory does not exist, it is created for the current user with the appropriate permissions.
Should also probably drop the directory verification as it's the same scenario basically. If the defined runtime directory is not really a directory, users will get an error either at write time or when trying to add sub-directories to the path. Either way, an error will be reported.

I think removing all checks is the way to go here. That would address this issue and #8 as well.

@adrg
Copy link
Owner

adrg commented Jan 7, 2021

Removed the ownership change code. If the user does not have access to the directory, an error will be reported either by createPath or when attempting to write the runtime file. Closing this issue.

@adrg adrg closed this as completed Jan 7, 2021
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

No branches or pull requests

2 participants