Subject: Re: [PATCH 2/2] setup: make bareRepository=explicit work in GIT_DIR of a secondary worktree

From: Junio C Hamano <[email-masked]>
Date: Fri, 08 Mar 2024 17:14:42 -0800

Kyle Lippincott <[email-masked]> writes:

>> > What loss of security do we have if we don't have as stringent of a
>> > check? i.e. if we just did `return !!strstr(path, "/.git/worktrees/)`?
>>
>> No loss of security.
>
> Then should we just do that?

I do not see what you mean.

> + /* Assumption: `path` points to the root of a $GIT_DIR. */
>  static int is_repo_with_working_tree(const char *path)
>  {
> -       return ends_with_path_components(path, ".git");
> +       /* $GIT_DIR immediately below the primary working tree */
> +       if (ends_with_path_components(path, ".git"))
> +               return 1;
> +
> +       /*
> +        * Also allow $GIT_DIRs in secondary worktrees.
> +        * These do not end in .git, but are still considered safe because
> +        * of the .git component in the path.
> +        */
> +       if (strstr(path, "/.git/worktrees/"))
> +               return 1;
> +
> +       return 0;
>  }

Ah, no.  I thought you were asking "goto out" vs "return", and my
answer was about these two.  Whether you leave with "goto out" or
"return", it does not change the security posture.  Direct return
will raise the risk of leaking resources after careless future
updates to the code.

I didn't get that you do not want to see the other two "sanity
check".

Losing these sanity checks may not lose "security" per-se, but it
may raise the risk of misidentification.  A healthy GIT_DIR of a
secondary worktree should satisfy these two extra conditions.