Re: a problem with git describe

From: Taylor Blau <me@ttaylorr.com>
Date: Apr 26, 2022, 01:52 UTC
Message-ID: <YmdQcrl/uu7HFBlc@nand.local>
In-Reply-To: <xmqqczh73hns.fsf@gitster.g>

On Sat, Apr 23, 2022 at 09:09:59AM -0700, Junio C Hamano wrote:

Show 18 quoted lines

> Philip Oakley <philipoakley@iee.email> writes:
>
> >> guy@renard ~/Software/uncrustify $ sudo git describe --always --dirty
> > ...
> > There has also been added an escape hatch of allowing "*" for the
> > permitted safe directories. but do check the updated manuals, and the
> > git mailing list archive (update the search in the above link).
>
> In this particular case, I do not think '*' is needed, but you need
> to be careful here.  Whose configuration are you suggesting to add
> such an entry?  Yourself?  ~root/.gitconfig?
>
> I wonder if we should loosen "the same owner" check somewhat to
> cover this situation better.  I expect people also run the
> installation in repositories they own with "sudo make install",
> and complaining "euid does not own that repository" when it is
> merely because they are running as root (and their real identity
> is still in ruid) feels a bit too strict to be useful.

I was thinking about this today and realized that my original train of thought along the lines of "ignore the new safety check when the current user has higher permissions than the user who owns the repository we're working in" was misguided.

What about loosening the check in a different way? Instead of causing Git to abort early, what if we:

  - skipped reading the repository's configuration and hooks (unless
    safe.directory includes $CWD)
  - emit a warning (which goes away when safe.directory contains $CWD)
  - otherwise continue executing as normal

That would unbreak the case of $(git describe ...) in our Makefile in this instance, without opening ourselves up to execution-via-config.

Though I think we'd have to be slightly more careful than that, since we definitely _do_ want to read repository format extensions.

I'm hesitant to recommend reading some parts of the configuration without others, though this is slightly different than that. Instead of saying "read every entry of config except core.editor, core.pager, core.alternateRefsCommand, core.fsmonitor" and so on, this says "when operating in a repository not owned by the current user (or the repository is in our global safe.directory list) only read repository format extensions, but ignore everything else in config and hooks".

> Dscho, what do you think?

I'd be curious to hear what Johannes thinks of your original mail as well as my own.

Thanks, Taylor