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

git_user_id should returnt he EUID #1577

Closed
b4ldr opened this issue Apr 26, 2023 · 1 comment
Closed

git_user_id should returnt he EUID #1577

b4ldr opened this issue Apr 26, 2023 · 1 comment

Comments

@b4ldr
Copy link

b4ldr commented Apr 26, 2023

i think that git.utils.get_user_id should return the Effective user and not the. currently the function uses getpass.getuser() which relies on environment variables and not getuid/geteuid functions, from the docs

checks the environment variables LOGNAME, USER, LNAME and USERNAME, in order, and returns the value of the first one which is set to a non-empty string. If none are set, the login name from the password database is returned on systems which support the pwd module, otherwise, an exception is raised.
with the following code

import git
import os
import pwd
import shutil
from git.util import get_user_id
from pathlib import Path

CHECKOUT = Path("/tmp/gist")
shutil.rmtree(CHECKOUT, ignore_errors=True)
ORIGIN_REPO = "https://gist.github.com/b4ldr/449d490b43fb3d2d01a74d28bc9a41e4"
os.seteuid(pwd.getpwnam('jbond').pw_uid)
euid = pwd.getpwuid(os.geteuid()).pw_name
print(f"git.utils.get_user_id: {get_user_id()}")
print(f"euid: {euid}")
repo = git.Repo.clone_from(ORIGIN_REPO, CHECKOUT)
print(f"directory owner: {CHECKOUT.owner()}")

When run as root we get the following output

$ sudo python -i git_test.py                                                                  
git user id: root@storage
euid: jbond
directory owner: jbond

i think it would be more useful if gitpython returned the user matching the EUID e.g. pwd.getpwuid(os.geteuid()).pw_name)

b4ldr added a commit to b4ldr/GitPython that referenced this issue Apr 26, 2023

Verified

This commit was signed with the committer’s verified signature. The key has expired.
b4ldr John Bond
@Byron
Copy link
Member

Byron commented Apr 26, 2023

No argument is made for why this should be preferred over the current implementation.

However, while checking what git does I noticed that it does also use getuid().

Thus there is nothing to fix, and changing it would be a breaking change as well which is another reason not to pursue this further. Thanks for your understanding.

@Byron Byron closed this as not planned Won't fix, can't repro, duplicate, stale Apr 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

2 participants