On 27/06/2024 16:28, Junio C Hamano wrote:
Phillip Wood <phillip.wood123@xxxxxxxxx> writes:
On 26/06/2024 19:11, Junio C Hamano wrote:
Phillip Wood <phillip.wood123@xxxxxxxxx> writes:
To expand an this a little - a couple of times I've wanted to checkout
a bare repository that is owned by a different user. It is a pain to
have to add a new config setting just for a one-off checkout. Being
able to adjust the config on the command line would be very useful in
that case.
True. As long as it is deemed safe to honor the one-off "git -c
safe.directory=..." from the command line, for the purpose of this
"I who am running this 'git' process hereby declare that I trust
this and that repository", I think it would be the best solution
for the "git daemon" use case.
This actually works already, the behavior was changed in 6061601d9f
(safe.directory: use git_protected_config(), 2022-07-14). The reason I
thought it didn't work was that I remember it failing on Debian
bullseye a few months ago but that used an older version of git. There
is some more rationale for the change in 779ea9303a7 (Documentation:
define protected configuration, 2022-07-14)
Thanks.
So, does this more or less conclude the episode about how best to
deal with the 2.45.1 regression that Florian's patch in this thread
started?
I think so yes
It seems that we already have enough mechanisms to help
users tweak their existing set-up, so we may not need code changes,
but I am wondering if we want to add a bit of documentation around
safe.directory to tell them when it makes sense to set it, what
value(s) they would want to set it to, etc.
* For "git daemon" invocations, because we know the command is run
after chdir to a directory with '.' specified as the repository,
we recommend to have safe.directory=., either on the command line
with "-c var=val" or in daemon user's ~/.gitconfig, in the
"git-daemon" help page? We could recommend safe.directory=*, but
they would mean the same thing in the context of running "git
daemon".
I think we'd be better to fix the safe.directory check as you suggest
below if we can but failing that updating the documentation would
certainly help.
We may want to discuss who protects from whom with the
safe.directory mechanism and git-daemon-export-ok mechanism. The
former is "the daemon trusts that repositories won't harm the
daemon user", while the latter is "the repository owner is OK for
it to be published".
Yes that would be helpful
Also optionally, we may update the code to take the absolute path
of the repository before passing it to the safe.directory check.
I think doing this would be more helpful than updating the documentation
to recommend adding "safe.directory=.". If we do this we would also want
to convert "//" -> "/" in the config keys as we've been forcing users to
add paths like "/srv/git//my-repo" if the --base-path argument to
git-daemon ended with a "/"
* For "http-backend" invocations, we should think about potential
additions that would help users, similar to what I listed above
for "git daemon".
That sounds sensible.
Having said all that, I do not think I mind GIT_SAFE_DIRECTORIES
that is a ":" separated list of paths that is honored just like the
multi-valued configuration variable safe.directory. Once an
attacker can influence your environment variables, it already is
game over, so trusting it does not make the attack surface any
worse.
Indeed in that case the attacker can influence the path that we read the
protected config from by setting $HOME (and do far worse by setting $PATH)
As Peff explained, we can trigger the more general "git -c
var=val" mechanism by exporting a set of environment variables, so
such a specialized environment variable is not strictly needed, but
it would make writing the "SetEnv" directive in apache configuration
(and similar ones for other HTTP server implementations) slighly
simpler and a lot more straight-forward.
Yes having to set all the GIT_CONFIG_* variables can be rather confusing
Best Wishes
Phillip
