Re: [PATCH v3 6/8] promisor-remote: trust known remotes matching acceptFromServerUrl
From: Kristoffer Haugsbakk <hidden>
Date: 2026-05-23 15:17:42
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 <redacted>
The rest of the commit message looks good to me.
quoted hunk ↗ jump to hunk
--- 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]