Re: [RFC] setup: fail if .git is not a file or directory

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 11, 2026, 19:47 UTC
Message-ID: <xmqq3437t643.fsf@gitster.g>
In-Reply-To: <20260211182122.35352-1-a3205153416@gmail.com>

Tian Yuchen <a3205153416@gmail.com> writes:

Show 5 quoted lines

> Currently, `setup_git_directory_gently_1()` checks if `.git` is a
> regular file (handling submodules/worktrees) or a directory. If it is
> neither (e.g., a FIFO), the code hits a NEEDSWORK comment and simply
> ignores the entity, continuing the discovery process in the parent
> directory.

Thanks for noticing the needswork comment.

> This behavior can be very dangerous. If a user is inside a subdirectory
> containing a melformed/broken `.git` entity, the Git will traverse up,
> attach to a parent repository and might execute destructive commands.

Is it? If a malicious party can replace your .git file in your submodule with a device node or a fifo, it means they can write into your working tree of your top level superproject, so they have other and better things to use to attack you if they wanted to. I would say 'very dangerous' is an overstatement. If "git add" in a place you thought was in your submodule ends up adding the file to the superproject because of filesystem corruption making ".git" in your submodule to something strange, you'd have larger problem anyway--- would you trust your commits and trees recorded in such a corrupt filesystem?

Having said all that.

Failing instead of ignoring may make it easier for the end-users to notice anomalies and take corrective action, if this non-file non-directory filesystem entity was created by mistake or by file-system corruption. It would be an unnecessary breakage to those who deliberately named a fifo they use ".git", fully knowing well that it is a reserved name that "git add" and friends happily ignore, but I somehow find it unlikely.

Show 6 quoted lines

> But I still have questions:
> 1. Is failing hard the desired behavior here? Should skipping it and
>    continuing discovery be an option for the user, which might seem
>    more fault-tolerant?
> 2. Should we die() immediately here, or return GIT_DIR_INVALID_GITFILE
>    and let the caller decide?

Determining if it makes sense to do so is 80% of the work needed to resolve a NEEDSWORK comment. The actual implementation is often much simpler than the work needed for it. ;-)

Show 24 quoted lines

> Signed-off-by: Tian Yuchen <a3205153416@gmail.com>
> ---
>  setup.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/setup.c b/setup.c
> index 3a6a048620..a1b56de67a 100644
> --- a/setup.c
> +++ b/setup.c
> @@ -1581,7 +1581,17 @@ static enum discovery_result setup_git_directory_gently_1(struct strbuf *dir,
>  		if (!gitdirenv) {
>  			if (die_on_error ||
>  			    error_code == READ_GITFILE_ERR_NOT_A_FILE) {
> -				/* NEEDSWORK: fail if .git is not file nor dir */
> +				struct stat st;
> +				if (!lstat(dir->buf, &st) &&
> +					!S_ISREG(st.st_mode) &&
> +					!S_ISDIR(st.st_mode)){
> +
> +					if (die_on_error)
> +						die(_("Invalid %s: not a regular file or directory"), dir->buf);
> +					else
> +						return GIT_DIR_INVALID_GITFILE;
> +				}

I would have expected that the new code would come after the existing "if the thing is a .git directory, it is a happy outcome" code. In this code path, where we found ".git" that is not a file, the most likely reason is because we found a healthy git directory, so it is a sane thing to avoid extra lstat() in that normal codepath. Only after we see that the ".git" we have is neither a "gitdir:" file nor a git directory, we know we are dealing with a rare anomaly that we can afford to waste extra cycles.

>  				if (is_git_directory(dir->buf)) {
>  					gitdirenv = DEFAULT_GIT_DIR_ENVIRONMENT;
>  					gitdir_path = xstrdup(dir->buf);