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

Gitea can overwrite users gitconfig when using XDG spec #33039

Closed
TheFox0x7 opened this issue Dec 29, 2024 · 17 comments · Fixed by #33067
Closed

Gitea can overwrite users gitconfig when using XDG spec #33039

TheFox0x7 opened this issue Dec 29, 2024 · 17 comments · Fixed by #33067
Labels

Comments

@TheFox0x7
Copy link
Contributor

Description

Finally tracked that one, I was wondering why git tests always used my local config and messed with it.

If user has $XDG_CONFIG_HOME/git/config file and the XDG_CONFIG_HOME variable is set, gitea uses it both for run and tests.

I see two solutions for this:

  1. Set XDG_CONFIG_HOME in commonBaseEnvs
  2. Unset XDG_CONFIG_HOME in gitea

First option has the downside of hardcoding the value of the variable.

Git config manual page for reference

Gitea Version

a92f505

Can you reproduce the bug on the Gitea demo site?

No

Log Gist

No response

Screenshots

No response

Git Version

No response

Operating System

No response

How are you running Gitea?

self-built

Database

SQLite

@eeyrjmr
Copy link
Contributor

eeyrjmr commented Dec 29, 2024

What user is your gitea being run as? A dedicated user would have the XDG set correctly for the dedicated user (git on Gentoo, gitea on alpine)

@techknowlogick
Copy link
Member

@eeyrjmr I believe this issue is reporting the case when tests are run, that it'll use the xdg path as well, which in the case of tests is run as the user testing rather than that of a dedicated user.

@TheFox0x7
Copy link
Contributor Author

Correct, either tests or a quick ./gitea instance for testing changes.

No real production use is affected by this as there isn't a clash of HOME=/tmp/testing for gitea with regular users XDG_CONFIG_HOME=/home/user1/.config and you have to actively try to cause it:

  1. XDG_CONFIG_HOME needs to be set
  2. $XDG_CONFIG_HOME/git/config needs to exist
  3. $HOME/.gitconfig has to be absent (HOME being gitea's), which is the default case in tests and on fresh start

I can always unset it manually now that I know it's an issue, but I figured reporting it won't hurt. It might save someone some time wondering why tests want to sign commits at worst.

@wxiaoguang
Copy link
Contributor

wxiaoguang commented Dec 30, 2024

Haven't looked into it, just FYI, some backgrounds are here

-> Proposal: make Gitea use its own customized global config #19709
-> Refactor git module, make Gitea use internal git config #19732
-> Use git.HOME_PATH for Git HOME directory #20114

We should have set up the $HOME/.gitconfig by git.InitFull and write something into it IIRC? (update: see below)

Need to figure out why/how XDG_CONFIG_HOME affects git's home-behavior and/or how to disable it.


Oops, I could somewhat understand the problem now ....

On a fresh startup, there is no $HOME/.gitconfig but $XDG_CONFIG_HOME/git/config, then git.InitFull makes git writes its config into $XDG_CONFIG_HOME/git/config 🤣

So I prefer to use method 2 (Unset XDG_CONFIG_HOME in gitea), since XDG_CONFIG_HOME is mainly for desktop users, it shouldn't appear in a server process.

@eeyrjmr
Copy link
Contributor

eeyrjmr commented Dec 30, 2024

Correct, either tests or a quick ./gitea instance for testing changes.

No real production use is affected by this as there isn't a clash of HOME=/tmp/testing for gitea with regular users XDG_CONFIG_HOME=/home/user1/.config and you have to actively try to cause it:

Just to be clear, you are mangling the env $HOME that is automatically set via /usr/bin/login (or ssh) to facilitate the testing? I ask because @wxiaoguang is advocating unsetting XDG_CONFIG_HOME, a variable that is set via PAM when triggered via a login session manager (systemd-logind, elogind, turnstile, or via a script for compatibility reason)

I ask because this looks like your use-case is a valid user and thus has

  1. $HOME (as set by the login application)
  2. XDG variables (as set by the session manager)

Thus for gitea to unset XDG_CONFIG_HOME might cause other issues...

Also it isn't enough to unset XDG_CONFIG_HOME since git (as this is fundamentally a git feature) actually checks for

  1. $XDG_CONFIG_HOME/git/config
  2. $HOME/.config/git/config
  3. $HOME/.gitconfig https://github.com/git/git/blob/306ab352f4e98f6809ce52fc4e5d63fb947d0635/path.h#L214-L216
    in this order.

Thus unsetting XDG_CONFIG_HOME isn't enough and HOME would also need to be overridden, which you have done in your testcase BUT isn't a normal thing todo ... be it either via writing into env or via updating

GitExecutable = "git" // the command name of git, will be updated to an absolute path during initialization
to say...
GitExecutable = "git" -> GitExecutable = "XDG_CONFIG_HOME=""git"

unsetting HOME and XDG_CONFIG_HOME needs to be thought about IF gitea is to manage this edgecase.

@wxiaoguang
Copy link
Contributor

wxiaoguang commented Dec 30, 2024

Thus for gitea to unset XDG_CONFIG_HOME might cause other issues...

I don't see "other issues" at the moment. Could you elaborate and show some real examples?

Also it isn't enough to unset XDG_CONFIG_HOME since git (as this is fundamentally a git feature) actually checks for

I think it is enough ..... because Gitea's git's HOME environment is managed by us:

https://github.com/go-gitea/gitea/pull/19732/files#diff-649ccdc85c6aef1cb94bbd733876aa807ed3790070217669d76e03239b7f2b51R115


ps: although there is another solution: setting "GIT_CONFIG_GLOBAL", but we can't use it because it requires git >= 2.32 .......

@eeyrjmr
Copy link
Contributor

eeyrjmr commented Dec 30, 2024

I don't see "other issues" at the moment. Could you elaborate and show some real examples?

It comes down to how XDG_CONFIG_HOME (from the perspective of /usr/bin/git) is unset and what the login session is also used for. My crude test (which I know was going to break things) was done to find that git looks for $HOME/.config/git/config as well. Thus unset XDG_CONFIG_HOME has wider system ramifications

HOWEVER... within a golang application,

err := os.Unsetenv("XDG_CONFIG_HOME")

does not bleed outside of the application, so yes this should be ok

@wxiaoguang
Copy link
Contributor

HOWEVER... within a golang application,

err := os.Unsetenv("XDG_CONFIG_HOME")

does not bleed outside of the application, so yes this should be ok

That's what we are discussing about method 2: 2. Unset XDG_CONFIG_HOME in gitea

@eeyrjmr
Copy link
Contributor

eeyrjmr commented Dec 30, 2024

The only real downside to this is ... gitea would only support a gitconfig in $HOME not where the trend for the last decade has been (server or desktop...) ... move dot files into $HOME/.config (hence git's hard-coded 2nd check area being $HOME/.config/git).

When you then consider what triggered this was a non-standard modification of a login session and technically LINUX breaking configuration where $HOME is being overridden but the rest of an XDG setup isn't... how much should an application support this? especially since git doesn't and this is actually occuring due to git (not gitea)

@TheFox0x7
Copy link
Contributor Author

This can be left without fix - it's non-trivial to get in the first place and requires actively messing with the configuration. It's just it is really annoying when I run gitea for quick experiments and it messes with my gitconfigs or wants to sign every webui commit.

One other thing that I thought of is a sanity check (is $HOME in $XDG_CONFIG_HOME so strings.Contain("/home/tmp/.config,"/home/tmp")) which would also work as it is highly unlikely that XDG_CONFIG_HOME will be in any other place which does not start with user directory:

if os.getenv("HOME") not in os.getenv("XDG_CONFIG_HOME"):
    os.unsetenv("XDG_CONFIG_HOME")

Gitea itself also doesn't really use XDG base dir standard in any way so that itself might be interesting to explore, but probably not relevant to the program.

@lunny
Copy link
Member

lunny commented Dec 30, 2024

@eeyrjmr The env vars override/unset will only affect Gitea itself or sub-progress, not other applications. So that I think it's safe.

@eeyrjmr
Copy link
Contributor

eeyrjmr commented Dec 31, 2024

@eeyrjmr The env vars override/unset will only affect Gitea itself or sub-progress, not other applications. So that I think it's safe.

yup, there was a part of me that was cautious about how this was to be done and then should it be done.

now whether os.Unsetenv('XDG_HOME_DIR") should be called is the question because it will work, but should aspects of git be worked around for an invalid setup, a setup that was created for testing.
As I mentioned, if this was to be done, it is implying that gitea will only support a gitconfig in the root of $HOME and at some point in the future someone will open a ticket stating that gitea doesn't support git config in $HOME/.config/git, as per git's own specification.

@TheFox0x7
Copy link
Contributor Author

If we go with unset route it would support $HOME/.config/git and $HOME/.gitconfig but not $XDG_CONFIG_HOME/git/config. $HOME is already "hijacked" by gitea. If we go with my 3rd idea, that lets users use $XDG* for git but it's conditional to it being in $HOME, which I think is a sane limitation.

Also consider that git by default doesn't use {$XDG_CONFIG_HOME,$HOME/.config}/git/config location but it defaults to ~/.gitconfig. The xdg version is used only if it exists and .gitconfig doesn't so unless someone went out of their way to create it for gitea, it doesn't exist in the first place.

@eeyrjmr
Copy link
Contributor

eeyrjmr commented Jan 1, 2025

Also consider that git by default doesn't use {$XDG_CONFIG_HOME,$HOME/.config}/git/config location but it defaults to ~/.gitconfig. The xdg version is used only if it exists and .gitconfig doesn't so unless someone went out of their way to create it for gitea, it doesn't exist in the first place.

Unfortunately that isn't the case. It can check 3 locations (it assumes two should be the same for a VALID XDG configured system) and it reads them in the order: $XDG_CONFIG_HOME/.config/git/config -> $HOME/.config/git/config -> $HOME/.gitconfig and thus ~/.gitconfig takes preceidence in overwriting any values previously loaded from any of the other config files BUT they are all loaded and parsed.

To confirm this I did

  1. set XDG_CONFIG_HOME to /tmp/foobar
    1. made a /tmp/foobar/git/config file and added a couple of [core] options
  2. edited my ~/.gitconfig and left only [color] options
  3. added $HOME/.config/git/config and added [alias option]
git config --list --show-origin
file:/etc/gitconfig     safe.directory=/var/db/repos/gentoo
file:/etc/gitconfig     safe.directory=/var/db/repos/steam-overlay
file:/etc/gitconfig     safe.directory=/var/db/repos/guru
file:/tmp/foobar/git/config     core.quotepath=false
file:/tmp/foobar/git/config     core.commitgraph=true
file:/home/jrb/.gitconfig       color.branch=auto
file:/home/jrb/.gitconfig       color.diff=auto
file:/home/jrb/.gitconfig       color.status=auto

As you can see 1 and 2 were read (3 wasn't because the git code checks if XDG_CONFIG_HOME is set)

There are weird and wacky ways that people can have their system setup and not realise what is being loaded but its all to support legacy systems which is exactly what triggered the scenario in the this issue report.

I am just trying to point out the potential pitfalls in trying to mitigate a technically inconsistent user environment (from the perspective of git) as it assumes that $HOME is set by /usr/bin/login (or ssh) and XDG_* is set by systemd-logind/elogind/turnstile/$script and they are all consistent with each other.

Options I see are

  1. do nothing
  2. as @wxiaoguang advocates, unsetting $XDG_CONFIG_HOME
  3. as you advocated crosscheck if $HOME is part of $XDG_CONFIG_HOME

I honestly see an issue being opened in a few years where someone states that gitea won't load the git config from ~/.config/git/config as per git's manual

@TheFox0x7
Copy link
Contributor Author

Unfortunately that isn't the case. It checks 2 locations (it assumes two should be the same for a VALID XDG configured system) and it reads them in the order: $XDG_CONFIG_HOME/.config/git/config -> $HOME/.config/git/config -> $HOME/.gitconfig and thus ~/.gitconfig takes preceidence in overwriting any values previously loaded from any of the other config files BUT they are all loaded and parsed.

Huh... Admittedly I did check the behavior on two keys (user.name and user.email) and when I had name and email in git/config and name in .gitconfig, then even though list showed both configs loaded, get global returned name from .gitconfig and empty email.

I guess I need to run more checks and see if the behavior is consistent when section and key aren't defined... I don't remember if I did that.

I honestly see an issue being opened in a few years where someone states that gitea won't load the git config from ~/.config/git/config as per git's manual

It would though? Just .gitconfig would take priority.1

As I mentioned before, I see nothing wrong with saying it's not a bug. It's obscure and misconfiguration to a degree (to a degree because I haven't actually misconfigured it, Gitea just set a new HOME which caused this). I mainly wanted to have it somewhat documented that it is a thing that might happen - and it results in git tests misbehaving and failing.

Footnotes

  1. Something that popped in my head as I was writing this is that we could use this to an advantage, set XDG_CONFIG_HOME to HOME/config (dot or no dot), to make git config file more naturally discoverable for users. Just throwing it out there as it might lead somewhere, it might be an awful idea.

@wxiaoguang
Copy link
Contributor

wxiaoguang commented Jan 1, 2025

I honestly see an issue being opened in a few years where someone states that gitea won't load the git config from ~/.config/git/config as per git's manual

Gitea should never do that, see the background in my comment above: #33039 (comment)

-> Proposal: make Gitea use its own customized global config #19709
-> Refactor git module, make Gitea use internal git config #19732
-> Use git.HOME_PATH for Git HOME directory #20114


To be honest ..... let's keep it simple: Gitea should be fully self-managed, it is not a desktop program, so let's just unset the XDG_CONFIG_HOME in Gitea, welcome to propose PRs (I could also propose one some days later when I get some time).

(Update: ideally Gitea should only accept the environments which it clearly knows instead of unsetting the ones it doesn't want, but this would be a breaking change and it seems not bringing enough benefits to end users, so at the moment we could still keep "unsetting the unnecessary environments")

@eeyrjmr
Copy link
Contributor

eeyrjmr commented Jan 1, 2025

welcome to propose PRs (I could also propose one some days later when I get some time).

done

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants