adrg/xdg

Should not change permissions without consent

KalleDK opened this issue · 2 comments

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 commented

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 commented

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.