Subject: Re: [PATCH v3 6/8] promisor-remote: trust known remotes matching acceptFromServerUrl

From: "Kristoffer Haugsbakk" <[email-masked]>
Date: Sat, 23 May 2026 17:17:20 +0200

On Tue, May 19, 2026, at 17:38, Christian Couder wrote:
>[snip]
>
> Let's then use this helper in should_accept_remote() so that, a known
> remote whose URL matches the allowlist is accepted.

I don’t understand this comma break?

>
> To prepare for this new logic, let's also:
>
>[snip]
>
> Signed-off-by: Christian Couder <[email-masked]>

The rest of the commit message looks good to me.

> ---
>  Documentation/config/promisor.adoc    |  74 +++++++++++++++++++
>  Documentation/gitprotocol-v2.adoc     |   9 ++-
>  promisor-remote.c                     | 102 +++++++++++++++++++++++---
>  t/t5710-promisor-remote-capability.sh |  71 ++++++++++++++++++
>  4 files changed, 242 insertions(+), 14 deletions(-)
>
> diff --git a/Documentation/config/promisor.adoc
>[snip]
> ++
> +Be _VERY_ careful with these patterns: `*` matches any sequence of
> +characters within the 'host' and 'path' parts of a URL (but cannot
> +cross part boundaries). An overly broad pattern is a major security
> +risk, as a matching URL allows a server to update fields (such as
> +authentication tokens) on known remotes without further confirmation.
> +To minimize security risks, follow these guidelines:
> ++

So this introduces a list of precautions to take.

> +1. Start with a secure protocol scheme, like `https://` or `ssh://`.
> ++
> +2. Only allow domain names or paths where you control and trust _ALL_
> +   the content. Be especially careful with shared hosting platforms
> +   like `github.com` or `gitlab.com`. A broad pattern like
> +   `https://gitlab.com/*` is dangerous because it trusts every
> +   repository on the entire platform. Always restrict such patterns to
> +   your specific organization or namespace (e.g.,
> +   `https://gitlab.com/your-org/*`).
> ++
> +3. Never use globs at the end of domain names. For example,
> +   `https://cdn.your-org.com/*` might be safe, but
> +   `https://cdn.your-org.com*/*` is a major security risk because
> +   the latter matches `https://cdn.your-org.com.hacker.net/repo`.
> ++
> +4. Be careful using globs at the beginning of domain names. While the
> +   code ensures a `*` in the host cannot cross into the path, a
> +   pattern like `https://*.example.com/*` will still match any
> +   subdomain. This is extremely dangerous on shared hosting platforms
> +   (e.g., `https://*.github.io/*` trusts every user's site on the
> +   entire platform).

The list seems to end here, because...

> ++
> +Before matching, both the advertised URL and the pattern are
> +normalized: the scheme and host are lowercased, percent-encoded

This next paragraph seems to go back to describing how things work. But
this paragraph as well as all of the following ones belong to this list
item:

      4.   Be careful using globs [...]

           Before matching, [...]

           The glob pattern can [...]

           If a remote with the [...]

           For the security implications [...]

    promisor.checkFields
    [...]

I don’t know what the intent is. But using an open block will delimit
the ordered list.

    diff --git Documentation/config/promisor.adoc Documentation/config/promisor.adoc
    index cc728bb0b5e..f07a2e883bd 100644
    --- Documentation/config/promisor.adoc
    +++ Documentation/config/promisor.adoc
    @@ -109,6 +109,7 @@ and to update fields (such as authentication tokens) on known remotes
     without further confirmation. To minimize security risks, follow these
     guidelines:
     +
    +--
     1. Start with a secure protocol scheme, like `https://` or `ssh://`.
     +
     2. Only allow domain names or paths where you control and trust _ALL_
    @@ -130,6 +131,7 @@ guidelines:
        subdomain. This is extremely dangerous on shared hosting platforms
        (e.g., `https://*.github.io/*` trusts every user's site on the
        entire platform).
    +--
     +
     Before matching, both the advertised URL and the pattern are
     normalized: the scheme and host are lowercased, percent-encoded

>[snip]