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

gitconfig: set safe.directory = * #19870

Merged
merged 9 commits into from Jun 17, 2022
Merged

Conversation

singuliere
Copy link
Contributor

@singuliere singuliere commented Jun 2, 2022

Allow git push to work when networked file systems with mixed
ownership are used with Gitea docker images >= 1.16.6 or Gitea
binaries running alongside git versions published after 04/2022.

There are circumstances independent of Gitea (networked file systems
with various permission systems) by which the git repositories managed
by Gitea may have mixed owners. It is not a behavior that Gitea have
control over nor is it a problem as long as the permissions for Gitea to
operate are correct. Gitea instances have been operating under these
conditions for a number of years.

It is detected as a potential security risk ( see
GHSA-vw2c-22j4-2fh2
) by the most recent git versions. However, Gitea always runs git
commands with a current directory matching the repository on
which it operates. That makes Gitea immune from this security problem
and it is safe to ignore the mixed owner permission check.

This gitconfig modification is done on a file dedicated to the user
exclusively used by Gitea.

Fixes: #19455

@singuliere
Copy link
Contributor Author

singuliere commented Jun 2, 2022

Now that #19797 is merged and git 2.36.1 is included in the Gitea docker images, this can be backported to 1.16.9 and fix the issue for good.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jun 2, 2022
@lunny
Copy link
Member

lunny commented Jun 2, 2022

I would like to merge #19732 because then you will not change the global configuration.

Copy link
Contributor

@zeripath zeripath left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is the right answer and I think that users who are being affected by this problem need to sort out their configuration but... Honestly I'm bored of seeing complaints about this.

I don't think anything bad can happen from this so LGTM

@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Jun 3, 2022
@singuliere singuliere closed this Jun 4, 2022
@zeripath
Copy link
Contributor

zeripath commented Jun 4, 2022

Hmm @singuliere why'd you close?

I think we should merge this simply because it is going to solve the issue in a very minimal way.

@zeripath zeripath reopened this Jun 4, 2022
@lunny
Copy link
Member

lunny commented Jun 4, 2022

Hmm @singuliere why'd you close?

I think we should merge this simply because it is going to solve the issue in a very minimal way.

I respect @singuliere 's effort to resolve the problem. But I would against to merge this directly. My point is we should not change the git security setting globally and silence in user's machine when Gitea running and users may complaint that. Users may run Gitea in his developer machines, docker or a public rasp berry. It may results in an unsafe state of that machine. A Gitea special setting is a more suitable resolution.

@zeripath
Copy link
Contributor

zeripath commented Jun 5, 2022

Perhaps we should just add this variable to the docker builds?

Unfortunately docker has made it almost completely impossible to get ANY ownership mapping of files on docker for Windows. (This is frankly ridiculous.) It is possible as I've written on the issue but it's pretty opaque.

Users of docker on other systems can also face similar ownership problems due to filesystem misconfiguration with smbfs shares etc.

It appears too complicated to expect people to get this right and it probably doesn't really matter.

At least in this case, we don't need to rely on ownership being right if we just set safe.directory - so we should just set it but remember we can never rely on ownership because of this dumb mechanism. If we want to make configuration of Gitea simple we're forced to do this.

Docker is forcing us in to setting this because of its absolutely useless mechanism of volume mounting on Windows.

@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Jun 9, 2022
@lunny
Copy link
Member

lunny commented Jun 10, 2022

Please wait #19732

@wxiaoguang wxiaoguang added status/blocked This PR cannot be merged yet, i.e. because it depends on another unmerged PR and removed backport/v1.16 labels Jun 10, 2022
docs/content/doc/installation/from-binary.en-us.md Outdated Show resolved Hide resolved
modules/git/git.go Outdated Show resolved Hide resolved
@delvh delvh removed the status/blocked This PR cannot be merged yet, i.e. because it depends on another unmerged PR label Jun 12, 2022
singuliere and others added 4 commits June 17, 2022 11:18
Allow git push to work when networked file systems with mixed
ownership are used with Gitea docker images >= 1.16.6 or Gitea
binaries running alongside git versions published after 04/2022.

There are circumstances independent of Gitea (networked file systems
with various permission systems) by which the git repositories managed
by Gitea may have mixed owners. It is not a behavior that Gitea have
control over nor is it a problem as long as the permissions for Gitea to
operate are correct. Gitea instances have been operating under these
conditions for a number of years.

It is detected as a potential security risk ( see
GHSA-vw2c-22j4-2fh2
) by the most recent git versions. However, Gitea always runs git
commands with a current directory matching the repository on
which it operates. That makes Gitea immune from this security problem
and it is safe to ignore the mixed owner permission check.

This gitconfig modification is done on a file dedicated to the user
exclusively used by Gitea.

Fixes: go-gitea#19455
Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
modules/git/git.go Outdated Show resolved Hide resolved
modules/git/git.go Outdated Show resolved Hide resolved
modules/git/git.go Outdated Show resolved Hide resolved
modules/git/git.go Outdated Show resolved Hide resolved
modules/git/git.go Outdated Show resolved Hide resolved
@zeripath zeripath merged commit a036507 into go-gitea:main Jun 17, 2022
vsysoev pushed a commit to IntegraSDL/gitea that referenced this pull request Aug 10, 2022
Allow git push to work when networked file systems with mixed
ownership are used with Gitea docker images >= 1.16.6 or Gitea
binaries running alongside git versions published after 04/2022.

There are circumstances independent of Gitea (networked file systems
with various permission systems) by which the git repositories managed
by Gitea may have mixed owners. It is not a behavior that Gitea have
control over nor is it a problem as long as the permissions for Gitea to
operate are correct. Gitea instances have been operating under these
conditions for a number of years.

It is detected as a potential security risk ( see
GHSA-vw2c-22j4-2fh2
) by the most recent git versions. However, Gitea always runs git
commands with a current directory matching the repository on
which it operates. That makes Gitea immune from this security problem
and it is safe to ignore the mixed owner permission check.

This gitconfig modification is done on a file dedicated to the user
exclusively used by Gitea.

Fixes: go-gitea#19455

Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
Co-authored-by: zeripath <art27@cantab.net>
AbdulrhmnGhanem pushed a commit to kitspace/gitea that referenced this pull request Aug 24, 2022
Allow git push to work when networked file systems with mixed
ownership are used with Gitea docker images >= 1.16.6 or Gitea
binaries running alongside git versions published after 04/2022.

There are circumstances independent of Gitea (networked file systems
with various permission systems) by which the git repositories managed
by Gitea may have mixed owners. It is not a behavior that Gitea have
control over nor is it a problem as long as the permissions for Gitea to
operate are correct. Gitea instances have been operating under these
conditions for a number of years.

It is detected as a potential security risk ( see
GHSA-vw2c-22j4-2fh2
) by the most recent git versions. However, Gitea always runs git
commands with a current directory matching the repository on
which it operates. That makes Gitea immune from this security problem
and it is safe to ignore the mixed owner permission check.

This gitconfig modification is done on a file dedicated to the user
exclusively used by Gitea.

Fixes: go-gitea#19455

Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
Co-authored-by: zeripath <art27@cantab.net>
@go-gitea go-gitea locked and limited conversation to collaborators May 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

New git security policy safe.directory (eg: 1.16.6 docker image) makes pushing fails
7 participants