{"thread":{"id":"65335","subject":"[PATCH 00/16] Auto-configure advertised remotes via URL whitelist","startedAt":"2026-03-23T08:05:42Z","lastAt":"2026-04-27T12:45:54Z","messageCount":47,"participants":["Christian Couder","Junio C Hamano","Patrick Steinhardt"],"isPatch":true,"patchVersion":1,"patchTotal":16},"messages":[{"id":"539702","messageId":"20260323080520.887550-1-christian.couder@gmail.com","threadId":"65335","inReplyTo":null,"subject":"[PATCH 00/16] Auto-configure advertised remotes via URL whitelist","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2026-03-23T08:05:03Z","receivedAt":"2026-03-23T08:05:42Z","isPatch":true,"body":"Currently, the \"promisor-remote\" protocol capability allows a server\nto advertise promisor remotes (and their tokens/filters), but the\nclient's `promisor.acceptFromServer` mechanism requires these remotes\nto already exist in the config.\n\nThis is a significant burden for users and administrators who have to\npre-configure remotes.\n\nThis patch series improves on this by introducing a new\n`promisor.acceptFromServerUrl` config option, which provides an\nadditive, URL-based security whitelist.\n\nMultiple `promisor.acceptFromServerUrl` config options can be provided\nin different config files. Each one should contain a URL glob pattern\nwhich can optionally be prefixed with a remote name in the\n\"[=]\" format.\n\nWith this new config option:\n\n - The server can update fields (like tokens) for known remotes,\n provided their URL matches the whitelist, even if\n `acceptFromServer` is set to `None`.\n\n - Unknown remotes advertised by the server can be automatically\n configured on the client if their URL matches the whitelist.\n\n - If there is no `` prefix before the glob pattern matched, the\n auto-configured remote is named using the\n \"promisor-auto-\" format. So the same auto-configured\n remote config entry will be reused for the same URL.\n\n - If a `` prefix is provided, it will be used for the\n auto-configured remote config entry.\n\n - If the chosen name (auto-generated or prefixed) already exists but\n points to a different URL, overwriting the existing config is\n prevented by appending a numeric suffix (e.g., -1, -2) to the name\n and auto-configuring using that name.\n\n - The server's originally advertised name is always saved in the\n `remote..advertisedAs` config variable of the auto-configured\n remote for tracing and debugging.\n\n - To honor the server's recommendation, promisor_remote_get_direct()\n is updated to try accepted remotes first before falling back to\n other configured promisor remotes. This ensures auto-configured\n remotes are preferred over other remotes especially the\n partial-clone origin.\n\nSecurity considerations:\n\n - Advertised URLs are routed through url_normalize() before matching\n against the user's glob patterns to prevent percent-encoding, case\n variation, or path-traversal (../) bypasses.\n\n - Auto-generated remote names are sanitized (non-alphanumeric\n characters are replaced with '-' and prefixed with\n 'promisor-auto-'). This guarantees safe config section names and\n prevents a server from maliciously overwriting standard remotes\n (like origin).\n\n - The documentation explains in detail how to use secure glob\n patterns in `promisor.acceptFromServerUrl`.\n\nHigh level description of the patches\n=====================================\n\n - Patch 1/16 (\"promisor-remote: try accepted remotes before others in\n get_direct()\"):\n\n Fixes promisor_remote_get_direct() to prioritize accepted\n remotes. This could be a separate fix, but is needed towards the\n end of the series.\n\n - Patches 2-3/16 (\"urlmatch:*\"):\n\n Exposes and adapts helpers in the urlmatch API.\n\n - Patches 4-11/16 (\"promisor-remote:*\"):\n\n Big refactoring of filter_promisor_remote() and\n should_accept_remote(). This keeps `struct promisor_info` instances\n alive longer to anticipate possible state-desync bugs, decouples\n the server's advertised name from the local config name, and\n sanitizes control flow without changing the existing behavior.\n\n - Patch 12/16 (\"t5710:*\"):\n\n Cleans up how \"file://\" URIs are managed in the test script to\n prepare for URI normalization later in the series and avoid issues\n on Windows.\n\n - Patches 13-15/16 (\"promisor-remote:*\"):\n\n The core feature. Introduces the parsing machinery, adds the\n additive whitelist for known remotes (with url_normalize()\n security), and finally implements the auto-creation and collision\n resolution for unknown remotes.\n\n - Patch 16/16 (\"doc: promisor: improve acceptFromServer entry\"):\n\n Cleans up and modernizes the existing `promisor.acceptFromServer`\n documentation.\n\nCI tests\n========\n\nThey all pass, see:\n\nhttps://github.com/chriscool/git/actions/runs/23350745268\n\n\nChristian Couder (16):\n promisor-remote: try accepted remotes before others in get_direct()\n urlmatch: change 'allow_globs' arg to bool\n urlmatch: add url_is_valid_pattern() helper\n promisor-remote: clarify that a remote is ignored\n promisor-remote: refactor has_control_char()\n promisor-remote: refactor accept_from_server()\n promisor-remote: keep accepted promisor_info structs alive\n promisor-remote: remove the 'accepted' strvec\n promisor-remote: add 'local_name' to 'struct promisor_info'\n promisor-remote: pass config entry to all_fields_match() directly\n promisor-remote: refactor should_accept_remote() control flow\n t5710: use proper file:// URIs for absolute paths\n promisor-remote: introduce promisor.acceptFromServerUrl\n promisor-remote: trust known remotes matching acceptFromServerUrl\n promisor-remote: auto-configure unknown remotes\n doc: promisor: improve acceptFromServer entry\n\n Documentation/config/promisor.adoc | 118 +++++-\n Documentation/config/remote.adoc | 9 +\n Documentation/gitprotocol-v2.adoc | 9 +-\n promisor-remote.c | 532 +++++++++++++++++++++-----\n t/t5710-promisor-remote-capability.sh | 250 ++++++++++--\n urlmatch.c | 18 +-\n urlmatch.h | 11 +\n 7 files changed, 802 insertions(+), 145 deletions(-)\n\n-- \n2.53.0.625.g20f70b52bb\n\n"},{"id":"539703","messageId":"20260323080520.887550-2-christian.couder@gmail.com","threadId":"65335","inReplyTo":"20260323080520.887550-1-christian.couder@gmail.com","subject":"[PATCH 01/16] promisor-remote: try accepted remotes before others in get_direct()","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2026-03-23T08:05:04Z","receivedAt":"2026-03-23T08:05:43Z","isPatch":true,"body":"When a server advertises promisor remotes and the client accepts some\nof them, those remotes carry the server's intent: 'fetch missing\nobjects preferably from here', and the client agrees with that for the\nremotes it accepts.\n\nHowever promisor_remote_get_direct() actually iterates over all\npromisor remotes in list order, which is the order they appear in the\nconfig files (except perhaps for the one appearing in the\n`extensions.partialClone` config variable which is tried last).\n\nThis means an existing, but not accepted, promisor remote, could be\ntried before the accepted ones, which does not reflect the intent of\nthe agreement between client and server.\n\nIf the client doesn't care about what the server suggests, it should\naccept nothing and rely on its remotes as they are already configured.\n\nTo better reflect the agreement between client and server, let's make\npromisor_remote_get_direct() try the accepted promisor remotes before\nthe non-accepted ones.\n\nConcretely, let's extract a try_promisor_remotes() helper and call it\ntwice from promisor_remote_get_direct():\n\n- first with an `accepted_only=true` argument to try only the accepted\n remotes,\n- then with `accepted_only=false` to fall back to any remaining remote.\n\nEnsuring that accepted remotes are preferred will be even more\nimportant if in the future a mechanism is developed to allow the\nclient to auto-configure remotes that the server advertises. This will\nin particular avoid fetching from the server (which is already\nconfigured as a promisor remote) before trying the auto-configured\nremotes, as these new remotes would likely appear at the end of the\nconfig file, and as the server might not appear in the\n`extensions.partialClone` config variable.\n\nSigned-off-by: Christian Couder \n---\n promisor-remote.c | 46 +++++++++++++++++++++++++++++++++-------------\n 1 file changed, 33 insertions(+), 13 deletions(-)\n\ndiff --git a/promisor-remote.c b/promisor-remote.c\nindex 96fa215b06..3f8aeee787 100644\n--- a/promisor-remote.c\n+++ b/promisor-remote.c\n@@ -268,11 +268,37 @@ static int remove_fetched_oids(struct repository *repo,\n \treturn remaining_nr;\n }\n \n+static int try_promisor_remotes(struct repository *repo,\n+\t\t\t\tstruct object_id **remaining_oids,\n+\t\t\t\tint *remaining_nr, int *to_free,\n+\t\t\t\tbool accepted_only)\n+{\n+\tstruct promisor_remote *r = repo->promisor_remote_config->promisors;\n+\n+\tfor (; r; r = r->next) {\n+\t\tif (accepted_only && !r->accepted)\n+\t\t\tcontinue;\n+\t\tif (!accepted_only && r->accepted)\n+\t\t\tcontinue;\n+\t\tif (fetch_objects(repo, r->name, *remaining_oids, *remaining_nr) < 0) {\n+\t\t\tif (*remaining_nr == 1)\n+\t\t\t\tcontinue;\n+\t\t\t*remaining_nr = remove_fetched_oids(repo, remaining_oids,\n+\t\t\t\t\t\t\t *remaining_nr, *to_free);\n+\t\t\tif (*remaining_nr) {\n+\t\t\t\t*to_free = 1;\n+\t\t\t\tcontinue;\n+\t\t\t}\n+\t\t}\n+\t\treturn 1; /* all fetched */\n+\t}\n+\treturn 0;\n+}\n+\n void promisor_remote_get_direct(struct repository *repo,\n \t\t\t\tconst struct object_id *oids,\n \t\t\t\tint oid_nr)\n {\n-\tstruct promisor_remote *r;\n \tstruct object_id *remaining_oids = (struct object_id *)oids;\n \tint remaining_nr = oid_nr;\n \tint to_free = 0;\n@@ -283,19 +309,13 @@ void promisor_remote_get_direct(struct repository *repo,\n \n \tpromisor_remote_init(repo);\n \n-\tfor (r = repo->promisor_remote_config->promisors; r; r = r->next) {\n-\t\tif (fetch_objects(repo, r->name, remaining_oids, remaining_nr) < 0) {\n-\t\t\tif (remaining_nr == 1)\n-\t\t\t\tcontinue;\n-\t\t\tremaining_nr = remove_fetched_oids(repo, &remaining_oids,\n-\t\t\t\t\t\t\t remaining_nr, to_free);\n-\t\t\tif (remaining_nr) {\n-\t\t\t\tto_free = 1;\n-\t\t\t\tcontinue;\n-\t\t\t}\n-\t\t}\n+\t/* Try accepted remotes first (those the server told us to use) */\n+\tif (try_promisor_remotes(repo, &remaining_oids, &remaining_nr,\n+\t\t\t\t &to_free, true))\n+\t\tgoto all_fetched;\n+\tif (try_promisor_remotes(repo, &remaining_oids, &remaining_nr,\n+\t\t\t\t &to_free, false))\n \t\tgoto all_fetched;\n-\t}\n \n \tfor (i = 0; i < remaining_nr; i++) {\n \t\tif (is_promisor_object(repo, &remaining_oids[i]))\n-- \n2.53.0.625.g20f70b52bb\n\n"},{"id":"539704","messageId":"20260323080520.887550-3-christian.couder@gmail.com","threadId":"65335","inReplyTo":"20260323080520.887550-1-christian.couder@gmail.com","subject":"[PATCH 02/16] urlmatch: change 'allow_globs' arg to bool","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2026-03-23T08:05:05Z","receivedAt":"2026-03-23T08:05:44Z","isPatch":true,"body":"The last argument of url_normalize_1() is `char allow_globs` but it is\nused as a boolean, not as a char.\n\nLet's convert it to a `bool`, and while at it convert the two calls to\nurl_normalize_1() so they pass 'true' or 'false' instead of '1' or '0'.\n\nSigned-off-by: Christian Couder \n---\n urlmatch.c | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/urlmatch.c b/urlmatch.c\nindex eea8300489..989bc7eb8b 100644\n--- a/urlmatch.c\n+++ b/urlmatch.c\n@@ -111,7 +111,7 @@ static int match_host(const struct url_info *url_info,\n \treturn (!url_len && !pat_len);\n }\n \n-static char *url_normalize_1(const char *url, struct url_info *out_info, char allow_globs)\n+static char *url_normalize_1(const char *url, struct url_info *out_info, bool allow_globs)\n {\n \t/*\n \t * Normalize NUL-terminated url using the following rules:\n@@ -437,7 +437,7 @@ static char *url_normalize_1(const char *url, struct url_info *out_info, char al\n \n char *url_normalize(const char *url, struct url_info *out_info)\n {\n-\treturn url_normalize_1(url, out_info, 0);\n+\treturn url_normalize_1(url, out_info, false);\n }\n \n static size_t url_match_prefix(const char *url,\n@@ -577,7 +577,7 @@ int urlmatch_config_entry(const char *var, const char *value,\n \t\tstruct url_info norm_info;\n \n \t\tconfig_url = xmemdupz(key, dot - key);\n-\t\tnorm_url = url_normalize_1(config_url, &norm_info, 1);\n+\t\tnorm_url = url_normalize_1(config_url, &norm_info, true);\n \t\tif (norm_url)\n \t\t\tretval = match_urls(url, &norm_info, &matched);\n \t\telse if (collect->fallback_match_fn)\n-- \n2.53.0.625.g20f70b52bb\n\n"},{"id":"539705","messageId":"20260323080520.887550-4-christian.couder@gmail.com","threadId":"65335","inReplyTo":"20260323080520.887550-1-christian.couder@gmail.com","subject":"[PATCH 03/16] urlmatch: add url_is_valid_pattern() helper","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2026-03-23T08:05:06Z","receivedAt":"2026-03-23T08:05:45Z","isPatch":true,"body":"In a following commit, we will need to check if a URL that might have\nglob patterns looks valid, so let's export a dedicated helper function\nfor that purpose.\n\nSigned-off-by: Christian Couder \n---\n urlmatch.c | 12 ++++++++++++\n urlmatch.h | 11 +++++++++++\n 2 files changed, 23 insertions(+)\n\ndiff --git a/urlmatch.c b/urlmatch.c\nindex 989bc7eb8b..a8cb6c3bee 100644\n--- a/urlmatch.c\n+++ b/urlmatch.c\n@@ -440,6 +440,18 @@ char *url_normalize(const char *url, struct url_info *out_info)\n \treturn url_normalize_1(url, out_info, false);\n }\n \n+bool url_is_valid_pattern(const char *url)\n+{\n+\tchar *normalized = url_normalize_1(url, NULL, true);\n+\n+\tif (normalized) {\n+\t\tfree(normalized);\n+\t\treturn true;\n+\t}\n+\n+\treturn false;\n+}\n+\n static size_t url_match_prefix(const char *url,\n \t\t\t const char *url_prefix,\n \t\t\t size_t url_prefix_len)\ndiff --git a/urlmatch.h b/urlmatch.h\nindex 5ba85cea13..4e01422a02 100644\n--- a/urlmatch.h\n+++ b/urlmatch.h\n@@ -36,6 +36,17 @@ struct url_info {\n \n char *url_normalize(const char *, struct url_info *);\n \n+/*\n+ * Return 'true' if the string looks like a valid URL or a valid URL pattern\n+ * (allowing '*' globs), 'false' otherwise.\n+ *\n+ * This is NOT a URL validation function. Full URL validation is NOT\n+ * performed. Some invalid host names are passed through this function\n+ * undetected. However, most all other problems that make a URL invalid\n+ * will be detected (including a missing host for non file: URLs).\n+ */\n+bool url_is_valid_pattern(const char *url);\n+\n struct urlmatch_item {\n \tsize_t hostmatch_len;\n \tsize_t pathmatch_len;\n-- \n2.53.0.625.g20f70b52bb\n\n"},{"id":"539706","messageId":"20260323080520.887550-5-christian.couder@gmail.com","threadId":"65335","inReplyTo":"20260323080520.887550-1-christian.couder@gmail.com","subject":"[PATCH 04/16] promisor-remote: clarify that a remote is ignored","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2026-03-23T08:05:07Z","receivedAt":"2026-03-23T08:05:47Z","isPatch":true,"body":"In should_accept_remote() when a remote is ignored, we might tell users\nwhy it is ignored in a warning, but we don't tell them that the remote\nis actually ignored.\n\nLet's clarify that, so users have a better idea of what's actually\nhappening.\n\nSigned-off-by: Christian Couder \n---\n promisor-remote.c | 7 ++++---\n 1 file changed, 4 insertions(+), 3 deletions(-)\n\ndiff --git a/promisor-remote.c b/promisor-remote.c\nindex 3f8aeee787..f5c4d41155 100644\n--- a/promisor-remote.c\n+++ b/promisor-remote.c\n@@ -660,15 +660,16 @@ static int should_accept_remote(enum accept_promisor accept,\n \t\tBUG(\"Unhandled 'enum accept_promisor' value '%d'\", accept);\n \n \tif (!remote_url || !*remote_url) {\n-\t\twarning(_(\"no or empty URL advertised for remote '%s'\"), remote_name);\n+\t\twarning(_(\"no or empty URL advertised for remote '%s', \"\n+\t\t\t \"ignoring this remote\"), remote_name);\n \t\treturn 0;\n \t}\n \n \tif (!strcmp(p->url, remote_url))\n \t\treturn all_fields_match(advertised, config_info, 0);\n \n-\twarning(_(\"known remote named '%s' but with URL '%s' instead of '%s'\"),\n-\t\tremote_name, p->url, remote_url);\n+\twarning(_(\"known remote named '%s' but with URL '%s' instead of '%s', \"\n+\t\t \"ignoring this remote\"), remote_name, p->url, remote_url);\n \n \treturn 0;\n }\n-- \n2.53.0.625.g20f70b52bb\n\n"},{"id":"539707","messageId":"20260323080520.887550-6-christian.couder@gmail.com","threadId":"65335","inReplyTo":"20260323080520.887550-1-christian.couder@gmail.com","subject":"[PATCH 05/16] promisor-remote: refactor has_control_char()","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2026-03-23T08:05:08Z","receivedAt":"2026-03-23T08:05:47Z","isPatch":true,"body":"In a following commit we are going to check if some strings contain\ncontrol character, so let's refactor the logic to do that in a new\nhas_control_char() helper function.\n\nSigned-off-by: Christian Couder \n---\n promisor-remote.c | 24 ++++++++++++++----------\n 1 file changed, 14 insertions(+), 10 deletions(-)\n\ndiff --git a/promisor-remote.c b/promisor-remote.c\nindex f5c4d41155..eda38223b9 100644\n--- a/promisor-remote.c\n+++ b/promisor-remote.c\n@@ -632,6 +632,14 @@ static int all_fields_match(struct promisor_info *advertised,\n \treturn 1;\n }\n \n+static bool has_control_char(const char *s)\n+{\n+\tfor (const char *c = s; *c; c++)\n+\t\tif (iscntrl(*c))\n+\t\t\treturn true;\n+\treturn false;\n+}\n+\n static int should_accept_remote(enum accept_promisor accept,\n \t\t\t\tstruct promisor_info *advertised,\n \t\t\t\tstruct string_list *config_info)\n@@ -762,18 +770,14 @@ static bool valid_filter(const char *filter, const char *remote_name)\n \treturn !res;\n }\n \n-/* Check that a token doesn't contain any control character */\n static bool valid_token(const char *token, const char *remote_name)\n {\n-\tconst char *c = token;\n-\n-\tfor (; *c; c++)\n-\t\tif (iscntrl(*c)) {\n-\t\t\twarning(_(\"invalid token '%s' for remote '%s' \"\n-\t\t\t\t \"will not be stored\"),\n-\t\t\t\ttoken, remote_name);\n-\t\t\treturn false;\n-\t\t}\n+\tif (has_control_char(token)) {\n+\t\twarning(_(\"invalid token '%s' for remote '%s' \"\n+\t\t\t \"will not be stored\"),\n+\t\t\ttoken, remote_name);\n+\t\treturn false;\n+\t}\n \n \treturn true;\n }\n-- \n2.53.0.625.g20f70b52bb\n\n"},{"id":"539708","messageId":"20260323080520.887550-7-christian.couder@gmail.com","threadId":"65335","inReplyTo":"20260323080520.887550-1-christian.couder@gmail.com","subject":"[PATCH 06/16] promisor-remote: refactor accept_from_server()","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2026-03-23T08:05:09Z","receivedAt":"2026-03-23T08:05:48Z","isPatch":true,"body":"In a following commit, we are going to add more logic to\nfilter_promisor_remote() which is already doing a lot of things.\n\nLet's alleviate that by moving the logic that checks and validates the\nvalue of the `promisor.acceptFromServer` config variable into its own\naccept_from_server() helper function.\n\nSigned-off-by: Christian Couder \n---\n promisor-remote.c | 27 +++++++++++++++++----------\n 1 file changed, 17 insertions(+), 10 deletions(-)\n\ndiff --git a/promisor-remote.c b/promisor-remote.c\nindex eda38223b9..3116d14d14 100644\n--- a/promisor-remote.c\n+++ b/promisor-remote.c\n@@ -852,20 +852,12 @@ static bool promisor_store_advertised_fields(struct promisor_info *advertised,\n \treturn reload_config;\n }\n \n-static void filter_promisor_remote(struct repository *repo,\n-\t\t\t\t struct strvec *accepted,\n-\t\t\t\t const char *info)\n+static enum accept_promisor accept_from_server(struct repository *repo)\n {\n \tconst char *accept_str;\n \tenum accept_promisor accept = ACCEPT_NONE;\n-\tstruct string_list config_info = STRING_LIST_INIT_NODUP;\n-\tstruct string_list remote_info = STRING_LIST_INIT_DUP;\n-\tstruct store_info *store_info = NULL;\n-\tstruct string_list_item *item;\n-\tbool reload_config = false;\n-\tstruct string_list accepted_filters = STRING_LIST_INIT_DUP;\n \n-\tif (!repo_config_get_string_tmp(the_repository, \"promisor.acceptfromserver\", &accept_str)) {\n+\tif (!repo_config_get_string_tmp(repo, \"promisor.acceptfromserver\", &accept_str)) {\n \t\tif (!*accept_str || !strcasecmp(\"None\", accept_str))\n \t\t\taccept = ACCEPT_NONE;\n \t\telse if (!strcasecmp(\"KnownUrl\", accept_str))\n@@ -879,6 +871,21 @@ static void filter_promisor_remote(struct repository *repo,\n \t\t\t\taccept_str, \"promisor.acceptfromserver\");\n \t}\n \n+\treturn accept;\n+}\n+\n+static void filter_promisor_remote(struct repository *repo,\n+\t\t\t\t struct strvec *accepted,\n+\t\t\t\t const char *info)\n+{\n+\tstruct string_list config_info = STRING_LIST_INIT_NODUP;\n+\tstruct string_list remote_info = STRING_LIST_INIT_DUP;\n+\tstruct store_info *store_info = NULL;\n+\tstruct string_list_item *item;\n+\tbool reload_config = false;\n+\tstruct string_list accepted_filters = STRING_LIST_INIT_DUP;\n+\tenum accept_promisor accept = accept_from_server(repo);\n+\n \tif (accept == ACCEPT_NONE)\n \t\treturn;\n \n-- \n2.53.0.625.g20f70b52bb\n\n"},{"id":"539709","messageId":"20260323080520.887550-8-christian.couder@gmail.com","threadId":"65335","inReplyTo":"20260323080520.887550-1-christian.couder@gmail.com","subject":"[PATCH 07/16] promisor-remote: keep accepted promisor_info structs alive","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2026-03-23T08:05:10Z","receivedAt":"2026-03-23T08:05:49Z","isPatch":true,"body":"In filter_promisor_remote(), the instances of `struct promisor_info`\nfor accepted remotes are dismantled into separate parallel data\nstructures (the 'accepted' strvec for server names, and\n'accepted_filters' for filter strings) and then immediately freed.\n\nInstead, let's keep these instances on an 'accepted_remotes' list.\n\nThis way the post-loop phase can iterate a single list to build the\nprotocol reply, apply advertised filters, and mark remotes as\naccepted, rather than iterating three separate structures.\n\nThis refactoring also prepares for a following commit that will add a\n'local_name' member to 'struct promisor_info'. Since struct instances\nstay alive, downstream code will be able to simply read both names\nfrom them rather than needing yet another parallel strvec.\n\nSigned-off-by: Christian Couder \n---\n promisor-remote.c | 42 +++++++++++++++++-------------------------\n 1 file changed, 17 insertions(+), 25 deletions(-)\n\ndiff --git a/promisor-remote.c b/promisor-remote.c\nindex 3116d14d14..34b4ca5806 100644\n--- a/promisor-remote.c\n+++ b/promisor-remote.c\n@@ -880,10 +880,10 @@ static void filter_promisor_remote(struct repository *repo,\n {\n \tstruct string_list config_info = STRING_LIST_INIT_NODUP;\n \tstruct string_list remote_info = STRING_LIST_INIT_DUP;\n+\tstruct string_list accepted_remotes = STRING_LIST_INIT_NODUP;\n \tstruct store_info *store_info = NULL;\n \tstruct string_list_item *item;\n \tbool reload_config = false;\n-\tstruct string_list accepted_filters = STRING_LIST_INIT_DUP;\n \tenum accept_promisor accept = accept_from_server(repo);\n \n \tif (accept == ACCEPT_NONE)\n@@ -912,17 +912,10 @@ static void filter_promisor_remote(struct repository *repo,\n \t\t\tif (promisor_store_advertised_fields(advertised, store_info))\n \t\t\t\treload_config = true;\n \n-\t\t\tstrvec_push(accepted, advertised->name);\n-\n-\t\t\t/* Capture advertised filters for accepted remotes */\n-\t\t\tif (advertised->filter) {\n-\t\t\t\tstruct string_list_item *i;\n-\t\t\t\ti = string_list_append(&accepted_filters, advertised->name);\n-\t\t\t\ti->util = xstrdup(advertised->filter);\n-\t\t\t}\n+\t\t\tstring_list_append(&accepted_remotes, advertised->name)->util = advertised;\n+\t\t} else {\n+\t\t\tpromisor_info_free(advertised);\n \t\t}\n-\n-\t\tpromisor_info_free(advertised);\n \t}\n \n \tpromisor_info_list_clear(&config_info);\n@@ -932,24 +925,23 @@ static void filter_promisor_remote(struct repository *repo,\n \tif (reload_config)\n \t\trepo_promisor_remote_reinit(repo);\n \n-\t/* Apply accepted remote filters to the stable repo state */\n-\tfor_each_string_list_item(item, &accepted_filters) {\n-\t\tstruct promisor_remote *r = repo_promisor_remote_find(repo, item->string);\n-\t\tif (r) {\n-\t\t\tfree(r->advertised_filter);\n-\t\t\tr->advertised_filter = item->util;\n-\t\t\titem->util = NULL;\n-\t\t}\n-\t}\n+\t/* Apply accepted remotes to the stable repo state */\n+\tfor_each_string_list_item(item, &accepted_remotes) {\n+\t\tstruct promisor_info *info = item->util;\n+\t\tstruct promisor_remote *r = repo_promisor_remote_find(repo, info->name);\n \n-\tstring_list_clear(&accepted_filters, 1);\n+\t\tstrvec_push(accepted, info->name);\n \n-\t/* Mark the remotes as accepted in the repository state */\n-\tfor (size_t i = 0; i < accepted->nr; i++) {\n-\t\tstruct promisor_remote *r = repo_promisor_remote_find(repo, accepted->v[i]);\n-\t\tif (r)\n+\t\tif (r) {\n \t\t\tr->accepted = 1;\n+\t\t\tif (info->filter) {\n+\t\t\t\tfree(r->advertised_filter);\n+\t\t\t\tr->advertised_filter = xstrdup(info->filter);\n+\t\t\t}\n+\t\t}\n \t}\n+\n+\tpromisor_info_list_clear(&accepted_remotes);\n }\n \n void promisor_remote_reply(const char *info, char **accepted_out)\n-- \n2.53.0.625.g20f70b52bb\n\n"},{"id":"539710","messageId":"20260323080520.887550-9-christian.couder@gmail.com","threadId":"65335","inReplyTo":"20260323080520.887550-1-christian.couder@gmail.com","subject":"[PATCH 08/16] promisor-remote: remove the 'accepted' strvec","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2026-03-23T08:05:11Z","receivedAt":"2026-03-23T08:05:51Z","isPatch":true,"body":"In a previous commit, filter_promisor_remote() was refactored to keep\naccepted 'struct promisor_info' instances alive instead of dismantling\nthem into separate parallel data structures.\n\nLet's go one step further and replace the 'struct strvec *accepted'\nargument passed to filter_promisor_remote() with a\n'struct string_list *accepted_remotes' argument.\n\nSigned-off-by: Christian Couder \n---\n promisor-remote.c | 27 ++++++++++++---------------\n 1 file changed, 12 insertions(+), 15 deletions(-)\n\ndiff --git a/promisor-remote.c b/promisor-remote.c\nindex 34b4ca5806..bdfc5e7608 100644\n--- a/promisor-remote.c\n+++ b/promisor-remote.c\n@@ -875,12 +875,11 @@ static enum accept_promisor accept_from_server(struct repository *repo)\n }\n \n static void filter_promisor_remote(struct repository *repo,\n-\t\t\t\t struct strvec *accepted,\n+\t\t\t\t struct string_list *accepted_remotes,\n \t\t\t\t const char *info)\n {\n \tstruct string_list config_info = STRING_LIST_INIT_NODUP;\n \tstruct string_list remote_info = STRING_LIST_INIT_DUP;\n-\tstruct string_list accepted_remotes = STRING_LIST_INIT_NODUP;\n \tstruct store_info *store_info = NULL;\n \tstruct string_list_item *item;\n \tbool reload_config = false;\n@@ -912,7 +911,7 @@ static void filter_promisor_remote(struct repository *repo,\n \t\t\tif (promisor_store_advertised_fields(advertised, store_info))\n \t\t\t\treload_config = true;\n \n-\t\t\tstring_list_append(&accepted_remotes, advertised->name)->util = advertised;\n+\t\t\tstring_list_append(accepted_remotes, advertised->name)->util = advertised;\n \t\t} else {\n \t\t\tpromisor_info_free(advertised);\n \t\t}\n@@ -926,12 +925,10 @@ static void filter_promisor_remote(struct repository *repo,\n \t\trepo_promisor_remote_reinit(repo);\n \n \t/* Apply accepted remotes to the stable repo state */\n-\tfor_each_string_list_item(item, &accepted_remotes) {\n+\tfor_each_string_list_item(item, accepted_remotes) {\n \t\tstruct promisor_info *info = item->util;\n \t\tstruct promisor_remote *r = repo_promisor_remote_find(repo, info->name);\n \n-\t\tstrvec_push(accepted, info->name);\n-\n \t\tif (r) {\n \t\t\tr->accepted = 1;\n \t\t\tif (info->filter) {\n@@ -940,23 +937,23 @@ static void filter_promisor_remote(struct repository *repo,\n \t\t\t}\n \t\t}\n \t}\n-\n-\tpromisor_info_list_clear(&accepted_remotes);\n }\n \n void promisor_remote_reply(const char *info, char **accepted_out)\n {\n-\tstruct strvec accepted = STRVEC_INIT;\n+\tstruct string_list accepted_remotes = STRING_LIST_INIT_NODUP;\n \n-\tfilter_promisor_remote(the_repository, &accepted, info);\n+\tfilter_promisor_remote(the_repository, &accepted_remotes, info);\n \n \tif (accepted_out) {\n-\t\tif (accepted.nr) {\n+\t\tif (accepted_remotes.nr) {\n \t\t\tstruct strbuf reply = STRBUF_INIT;\n-\t\t\tfor (size_t i = 0; i < accepted.nr; i++) {\n-\t\t\t\tif (i)\n+\t\t\tstruct string_list_item *item;\n+\n+\t\t\tfor_each_string_list_item(item, &accepted_remotes) {\n+\t\t\t\tif (reply.len)\n \t\t\t\t\tstrbuf_addch(&reply, ';');\n-\t\t\t\tstrbuf_addstr_urlencode(&reply, accepted.v[i], allow_unsanitized);\n+\t\t\t\tstrbuf_addstr_urlencode(&reply, item->string, allow_unsanitized);\n \t\t\t}\n \t\t\t*accepted_out = strbuf_detach(&reply, NULL);\n \t\t} else {\n@@ -964,7 +961,7 @@ void promisor_remote_reply(const char *info, char **accepted_out)\n \t\t}\n \t}\n \n-\tstrvec_clear(&accepted);\n+\tpromisor_info_list_clear(&accepted_remotes);\n }\n \n void mark_promisor_remotes_as_accepted(struct repository *r, const char *remotes)\n-- \n2.53.0.625.g20f70b52bb\n\n"},{"id":"539711","messageId":"20260323080520.887550-10-christian.couder@gmail.com","threadId":"65335","inReplyTo":"20260323080520.887550-1-christian.couder@gmail.com","subject":"[PATCH 09/16] promisor-remote: add 'local_name' to 'struct promisor_info'","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2026-03-23T08:05:12Z","receivedAt":"2026-03-23T08:05:52Z","isPatch":true,"body":"In a following commit, we will store promisor remote information under\na remote name different than the one the server advertised.\n\nTo prepare for this change, let's add a new 'char* local_name' member\nto 'struct promisor_info', and let's update the related functions.\n\nWhile at it, let's also add a small promisor_info_internal_name()\nhelper that returns `local_name` when set, `name` otherwise, and let's\nuse this small helper in promisor_store_advertised_fields() and in the\npost-loop of filter_promisor_remote() so that lookups against the local\nrepo configuration use the right name.\n\nSigned-off-by: Christian Couder \n---\n promisor-remote.c | 25 ++++++++++++++++++-------\n 1 file changed, 18 insertions(+), 7 deletions(-)\n\ndiff --git a/promisor-remote.c b/promisor-remote.c\nindex bdfc5e7608..da347fa2dc 100644\n--- a/promisor-remote.c\n+++ b/promisor-remote.c\n@@ -434,15 +434,19 @@ static struct string_list *fields_stored(void)\n \n /*\n * Struct for promisor remotes involved in the \"promisor-remote\"\n- * protocol capability.\n+ * protocol capability:\n *\n- * Except for \"name\", each in this struct and its \n- * should correspond (either on the client side or on the server side)\n- * to a \"remote..\" config variable set to where\n- * \"\" is a promisor remote name.\n+ * - \"name\" is the name the server advertised.\n+ * - \"local_name\" is the name we use locally (may be auto-generated).\n+ *\n+ * Except for \"name\" and \"local_name\", each in this struct\n+ * and its should correspond (either on the client side or on\n+ * the server side) to a \"remote..\" config variable set\n+ * to where \"\" is a promisor remote name.\n */\n struct promisor_info {\n \tconst char *name;\n+\tconst char *local_name;\n \tconst char *url;\n \tconst char *filter;\n \tconst char *token;\n@@ -451,6 +455,7 @@ struct promisor_info {\n static void promisor_info_free(struct promisor_info *p)\n {\n \tfree((char *)p->name);\n+\tfree((char *)p->local_name);\n \tfree((char *)p->url);\n \tfree((char *)p->filter);\n \tfree((char *)p->token);\n@@ -464,6 +469,11 @@ static void promisor_info_list_clear(struct string_list *list)\n \tstring_list_clear(list, 0);\n }\n \n+static const char *promisor_info_internal_name(struct promisor_info *p)\n+{\n+\treturn p->local_name ? p->local_name : p->name;\n+}\n+\n static void set_one_field(struct promisor_info *p,\n \t\t\t const char *field, const char *value)\n {\n@@ -819,7 +829,7 @@ static bool promisor_store_advertised_fields(struct promisor_info *advertised,\n {\n \tstruct promisor_info *p;\n \tstruct string_list_item *item;\n-\tconst char *remote_name = advertised->name;\n+\tconst char *remote_name = promisor_info_internal_name(advertised);\n \tbool reload_config = false;\n \n \tif (!(store_info->store_filter || store_info->store_token))\n@@ -927,7 +937,8 @@ static void filter_promisor_remote(struct repository *repo,\n \t/* Apply accepted remotes to the stable repo state */\n \tfor_each_string_list_item(item, accepted_remotes) {\n \t\tstruct promisor_info *info = item->util;\n-\t\tstruct promisor_remote *r = repo_promisor_remote_find(repo, info->name);\n+\t\tconst char *local = promisor_info_internal_name(info);\n+\t\tstruct promisor_remote *r = repo_promisor_remote_find(repo, local);\n \n \t\tif (r) {\n \t\t\tr->accepted = 1;\n-- \n2.53.0.625.g20f70b52bb\n\n"},{"id":"539712","messageId":"20260323080520.887550-11-christian.couder@gmail.com","threadId":"65335","inReplyTo":"20260323080520.887550-1-christian.couder@gmail.com","subject":"[PATCH 10/16] promisor-remote: pass config entry to all_fields_match() directly","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2026-03-23T08:05:13Z","receivedAt":"2026-03-23T08:05:53Z","isPatch":true,"body":"The `in_list == 0` path of all_fields_match() re-looks up the\nremote in config_info by advertised->name, even though every\ncaller in should_accept_remote() has already performed this\nlookup and holds the result in 'p'.\n\nTo avoid this useless work, let's replace the `int in_list`\nparameter with a `struct promisor_info *config_entry` pointer:\n\n - When NULL (ACCEPT_ALL mode): scan the whole `config_info` list, as\n the old `in_list == 1` path did.\n\n - When non-NULL: match against that single config entry directly,\n avoiding the redundant string_list_lookup() call.\n\nThis removes the hidden dependency on `advertised->name` inside\nall_fields_match(), which would be wrong in the following commits when\nauto-configured remotes will be implemented as the local config name\nmay differ from the server's advertised name.\n\nSigned-off-by: Christian Couder \n---\n promisor-remote.c | 30 ++++++++++++++++++------------\n 1 file changed, 18 insertions(+), 12 deletions(-)\n\ndiff --git a/promisor-remote.c b/promisor-remote.c\nindex da347fa2dc..8f2c1280c3 100644\n--- a/promisor-remote.c\n+++ b/promisor-remote.c\n@@ -598,9 +598,18 @@ static int match_field_against_config(const char *field, const char *value,\n \treturn 0;\n }\n \n+/*\n+ * Check that the advertised fields match the local configuration.\n+ *\n+ * When 'config_entry' is NULL (ACCEPT_ALL mode), every checked field\n+ * must match at least one remote in 'config_info'.\n+ *\n+ * When 'config_entry' points to a specific remote's config, the\n+ * checked fields are compared against that single remote only.\n+ */\n static int all_fields_match(struct promisor_info *advertised,\n \t\t\t struct string_list *config_info,\n-\t\t\t int in_list)\n+\t\t\t struct promisor_info *config_entry)\n {\n \tstruct string_list *fields = fields_checked();\n \tstruct string_list_item *item_checked;\n@@ -609,7 +618,6 @@ static int all_fields_match(struct promisor_info *advertised,\n \t\tint match = 0;\n \t\tconst char *field = item_checked->string;\n \t\tconst char *value = NULL;\n-\t\tstruct string_list_item *item;\n \n \t\tif (!strcasecmp(field, promisor_field_filter))\n \t\t\tvalue = advertised->filter;\n@@ -619,7 +627,11 @@ static int all_fields_match(struct promisor_info *advertised,\n \t\tif (!value)\n \t\t\treturn 0;\n \n-\t\tif (in_list) {\n+\t\tif (config_entry) {\n+\t\t\tmatch = match_field_against_config(field, value,\n+\t\t\t\t\t\t\t config_entry);\n+\t\t} else {\n+\t\t\tstruct string_list_item *item;\n \t\t\tfor_each_string_list_item(item, config_info) {\n \t\t\t\tstruct promisor_info *p = item->util;\n \t\t\t\tif (match_field_against_config(field, value, p)) {\n@@ -627,12 +639,6 @@ static int all_fields_match(struct promisor_info *advertised,\n \t\t\t\t\tbreak;\n \t\t\t\t}\n \t\t\t}\n-\t\t} else {\n-\t\t\titem = string_list_lookup(config_info, advertised->name);\n-\t\t\tif (item) {\n-\t\t\t\tstruct promisor_info *p = item->util;\n-\t\t\t\tmatch = match_field_against_config(field, value, p);\n-\t\t\t}\n \t\t}\n \n \t\tif (!match)\n@@ -660,7 +666,7 @@ static int should_accept_remote(enum accept_promisor accept,\n \tconst char *remote_url = advertised->url;\n \n \tif (accept == ACCEPT_ALL)\n-\t\treturn all_fields_match(advertised, config_info, 1);\n+\t\treturn all_fields_match(advertised, config_info, NULL);\n \n \t/* Get config info for that promisor remote */\n \titem = string_list_lookup(config_info, remote_name);\n@@ -672,7 +678,7 @@ static int should_accept_remote(enum accept_promisor accept,\n \tp = item->util;\n \n \tif (accept == ACCEPT_KNOWN_NAME)\n-\t\treturn all_fields_match(advertised, config_info, 0);\n+\t\treturn all_fields_match(advertised, config_info, p);\n \n \tif (accept != ACCEPT_KNOWN_URL)\n \t\tBUG(\"Unhandled 'enum accept_promisor' value '%d'\", accept);\n@@ -684,7 +690,7 @@ static int should_accept_remote(enum accept_promisor accept,\n \t}\n \n \tif (!strcmp(p->url, remote_url))\n-\t\treturn all_fields_match(advertised, config_info, 0);\n+\t\treturn all_fields_match(advertised, config_info, p);\n \n \twarning(_(\"known remote named '%s' but with URL '%s' instead of '%s', \"\n \t\t \"ignoring this remote\"), remote_name, p->url, remote_url);\n-- \n2.53.0.625.g20f70b52bb\n\n"},{"id":"539713","messageId":"20260323080520.887550-13-christian.couder@gmail.com","threadId":"65335","inReplyTo":"20260323080520.887550-1-christian.couder@gmail.com","subject":"[PATCH 12/16] t5710: use proper file:// URIs for absolute paths","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2026-03-23T08:05:15Z","receivedAt":"2026-03-23T08:05:56Z","isPatch":true,"body":"In t5710, we frequently construct local file URIs using `file://$(pwd)`.\nOn Unix-like systems, $(pwd) returns an absolute path starting with a\nslash (e.g., `/tmp/repo`), resulting in a valid 3-slash URI with an\nempty host (`file:///tmp/repo`).\n\nHowever, on Windows, $(pwd) returns a path starting with a drive\nletter (e.g., `D:/a/repo`). This results in a 2-slash URI\n(`file://D:/a/repo`). Standard URI parsers misinterpret this format,\ntreating `D:` as the host rather than part of the absolute path.\n\nWhile this hasn't broken the existing tests (because the old\n`promisor.acceptFromServer` logic relies entirely on strict `strcmp`\nwithout normalizing the URLs), it will break future commits that pass\nthese URLs through `url_normalize()`.\n\nTo future-proof the tests and ensure cross-platform URI compliance,\nlet's introduce a $PWD_URL helper that explicitly guarantees a leading\nslash for the path component, ensuring valid 3-slash `file:///` URIs on\nall operating systems.\n\nWhile at it, let's also introduce $ENCODED_PWD_URL to handle spaces in\ndirectory paths (which is needed for URL glob pattern matching).\n\nThen let's replace all instances of `file://$(pwd)` with $PWD_URL across\nthe test script, and let's simplify the `ENCODED_URL` constructions in\nthe `sendFields` and `checkFields` tests to use $ENCODED_PWD_URL\ndirectly.\n\nSigned-off-by: Christian Couder \n---\n t/t5710-promisor-remote-capability.sh | 55 ++++++++++++++++-----------\n 1 file changed, 32 insertions(+), 23 deletions(-)\n\ndiff --git a/t/t5710-promisor-remote-capability.sh b/t/t5710-promisor-remote-capability.sh\nindex 357822c01a..c7a484228f 100755\n--- a/t/t5710-promisor-remote-capability.sh\n+++ b/t/t5710-promisor-remote-capability.sh\n@@ -76,6 +76,17 @@ copy_to_lop () {\n \tcp \"$path\" \"$path2\"\n }\n \n+# On Windows, 'pwd' returns a path like 'D:/foo/bar'. Prepend '/' to turn\n+# it into '/D:/foo/bar', which is what git expects in file:// URLs on Windows.\n+# On Unix, the path already starts with '/', so this is a no-op.\n+pwd_path=$(pwd)\n+case \"$pwd_path\" in\n+[a-zA-Z]:*) pwd_path=\"/$pwd_path\" ;;\n+esac\n+PWD_URL=\"file://$pwd_path\"\n+# Same as PWD_URL but with spaces percent-encoded, for use in URL patterns.\n+ENCODED_PWD_URL=\"file://$(echo \"$pwd_path\" | sed \"s/ /%20/g\")\"\n+\n test_expect_success \"setup for testing promisor remote advertisement\" '\n \t# Create another bare repo called \"lop\" (for Large Object Promisor)\n \tgit init --bare lop &&\n@@ -88,7 +99,7 @@ test_expect_success \"setup for testing promisor remote advertisement\" '\n \tinitialize_server 1 \"$oid\" &&\n \n \t# Configure lop as promisor remote for server\n-\tgit -C server remote add lop \"file://$(pwd)/lop\" &&\n+\tgit -C server remote add lop \"$PWD_URL/lop\" &&\n \tgit -C server config remote.lop.promisor true &&\n \n \tgit -C lop config uploadpack.allowFilter true &&\n@@ -104,7 +115,7 @@ test_expect_success \"clone with promisor.advertise set to 'true'\" '\n \t# Clone from server to create a client\n \tGIT_NO_LAZY_FETCH=0 git clone -c remote.lop.promisor=true \\\n \t\t-c remote.lop.fetch=\"+refs/heads/*:refs/remotes/lop/*\" \\\n-\t\t-c remote.lop.url=\"file://$(pwd)/lop\" \\\n+\t\t-c remote.lop.url=\"$PWD_URL/lop\" \\\n \t\t-c promisor.acceptfromserver=All \\\n \t\t--no-local --filter=\"blob:limit=5k\" server client &&\n \n@@ -119,7 +130,7 @@ test_expect_success \"clone with promisor.advertise set to 'false'\" '\n \t# Clone from server to create a client\n \tGIT_NO_LAZY_FETCH=0 git clone -c remote.lop.promisor=true \\\n \t\t-c remote.lop.fetch=\"+refs/heads/*:refs/remotes/lop/*\" \\\n-\t\t-c remote.lop.url=\"file://$(pwd)/lop\" \\\n+\t\t-c remote.lop.url=\"$PWD_URL/lop\" \\\n \t\t-c promisor.acceptfromserver=All \\\n \t\t--no-local --filter=\"blob:limit=5k\" server client &&\n \n@@ -137,7 +148,7 @@ test_expect_success \"clone with promisor.acceptfromserver set to 'None'\" '\n \t# Clone from server to create a client\n \tGIT_NO_LAZY_FETCH=0 git clone -c remote.lop.promisor=true \\\n \t\t-c remote.lop.fetch=\"+refs/heads/*:refs/remotes/lop/*\" \\\n-\t\t-c remote.lop.url=\"file://$(pwd)/lop\" \\\n+\t\t-c remote.lop.url=\"$PWD_URL/lop\" \\\n \t\t-c promisor.acceptfromserver=None \\\n \t\t--no-local --filter=\"blob:limit=5k\" server client &&\n \n@@ -156,8 +167,8 @@ test_expect_success \"init + fetch with promisor.advertise set to 'true'\" '\n \tgit -C client init &&\n \tgit -C client config remote.lop.promisor true &&\n \tgit -C client config remote.lop.fetch \"+refs/heads/*:refs/remotes/lop/*\" &&\n-\tgit -C client config remote.lop.url \"file://$(pwd)/lop\" &&\n-\tgit -C client config remote.server.url \"file://$(pwd)/server\" &&\n+\tgit -C client config remote.lop.url \"$PWD_URL/lop\" &&\n+\tgit -C client config remote.server.url \"$PWD_URL/server\" &&\n \tgit -C client config remote.server.fetch \"+refs/heads/*:refs/remotes/server/*\" &&\n \tgit -C client config promisor.acceptfromserver All &&\n \tGIT_NO_LAZY_FETCH=0 git -C client fetch --filter=\"blob:limit=5k\" server &&\n@@ -173,7 +184,7 @@ test_expect_success \"clone with promisor.acceptfromserver set to 'KnownName'\" '\n \t# Clone from server to create a client\n \tGIT_NO_LAZY_FETCH=0 git clone -c remote.lop.promisor=true \\\n \t\t-c remote.lop.fetch=\"+refs/heads/*:refs/remotes/lop/*\" \\\n-\t\t-c remote.lop.url=\"file://$(pwd)/lop\" \\\n+\t\t-c remote.lop.url=\"$PWD_URL/lop\" \\\n \t\t-c promisor.acceptfromserver=KnownName \\\n \t\t--no-local --filter=\"blob:limit=5k\" server client &&\n \n@@ -188,7 +199,7 @@ test_expect_success \"clone with 'KnownName' and different remote names\" '\n \t# Clone from server to create a client\n \tGIT_NO_LAZY_FETCH=0 git clone -c remote.serverTwo.promisor=true \\\n \t\t-c remote.serverTwo.fetch=\"+refs/heads/*:refs/remotes/lop/*\" \\\n-\t\t-c remote.serverTwo.url=\"file://$(pwd)/lop\" \\\n+\t\t-c remote.serverTwo.url=\"$PWD_URL/lop\" \\\n \t\t-c promisor.acceptfromserver=KnownName \\\n \t\t--no-local --filter=\"blob:limit=5k\" server client &&\n \n@@ -225,7 +236,7 @@ test_expect_success \"clone with promisor.acceptfromserver set to 'KnownUrl'\" '\n \t# Clone from server to create a client\n \tGIT_NO_LAZY_FETCH=0 git clone -c remote.lop.promisor=true \\\n \t\t-c remote.lop.fetch=\"+refs/heads/*:refs/remotes/lop/*\" \\\n-\t\t-c remote.lop.url=\"file://$(pwd)/lop\" \\\n+\t\t-c remote.lop.url=\"$PWD_URL/lop\" \\\n \t\t-c promisor.acceptfromserver=KnownUrl \\\n \t\t--no-local --filter=\"blob:limit=5k\" server client &&\n \n@@ -242,7 +253,7 @@ test_expect_success \"clone with 'KnownUrl' and different remote urls\" '\n \t# Clone from server to create a client\n \tGIT_NO_LAZY_FETCH=0 git clone -c remote.lop.promisor=true \\\n \t\t-c remote.lop.fetch=\"+refs/heads/*:refs/remotes/lop/*\" \\\n-\t\t-c remote.lop.url=\"file://$(pwd)/serverTwo\" \\\n+\t\t-c remote.lop.url=\"$PWD_URL/serverTwo\" \\\n \t\t-c promisor.acceptfromserver=KnownUrl \\\n \t\t--no-local --filter=\"blob:limit=5k\" server client &&\n \n@@ -257,7 +268,7 @@ test_expect_success \"clone with 'KnownUrl' and url not configured on the server\"\n \tgit -C server config promisor.advertise true &&\n \ttest_when_finished \"rm -rf client\" &&\n \n-\ttest_when_finished \"git -C server config set remote.lop.url \\\"file://$(pwd)/lop\\\"\" &&\n+\ttest_when_finished \"git -C server config set remote.lop.url \\\"$PWD_URL/lop\\\"\" &&\n \tgit -C server config unset remote.lop.url &&\n \n \t# Clone from server to create a client\n@@ -266,7 +277,7 @@ test_expect_success \"clone with 'KnownUrl' and url not configured on the server\"\n \t# missing, so the remote name will be used instead which will fail.\n \ttest_must_fail env GIT_NO_LAZY_FETCH=0 git clone -c remote.lop.promisor=true \\\n \t\t-c remote.lop.fetch=\"+refs/heads/*:refs/remotes/lop/*\" \\\n-\t\t-c remote.lop.url=\"file://$(pwd)/lop\" \\\n+\t\t-c remote.lop.url=\"$PWD_URL/lop\" \\\n \t\t-c promisor.acceptfromserver=KnownUrl \\\n \t\t--no-local --filter=\"blob:limit=5k\" server client &&\n \n@@ -278,7 +289,7 @@ test_expect_success \"clone with 'KnownUrl' and empty url, so not advertised\" '\n \tgit -C server config promisor.advertise true &&\n \ttest_when_finished \"rm -rf client\" &&\n \n-\ttest_when_finished \"git -C server config set remote.lop.url \\\"file://$(pwd)/lop\\\"\" &&\n+\ttest_when_finished \"git -C server config set remote.lop.url \\\"$PWD_URL/lop\\\"\" &&\n \tgit -C server config set remote.lop.url \"\" &&\n \n \t# Clone from server to create a client\n@@ -287,7 +298,7 @@ test_expect_success \"clone with 'KnownUrl' and empty url, so not advertised\" '\n \t# so the remote name will be used instead which will fail.\n \ttest_must_fail env GIT_NO_LAZY_FETCH=0 git clone -c remote.lop.promisor=true \\\n \t\t-c remote.lop.fetch=\"+refs/heads/*:refs/remotes/lop/*\" \\\n-\t\t-c remote.lop.url=\"file://$(pwd)/lop\" \\\n+\t\t-c remote.lop.url=\"$PWD_URL/lop\" \\\n \t\t-c promisor.acceptfromserver=KnownUrl \\\n \t\t--no-local --filter=\"blob:limit=5k\" server client &&\n \n@@ -311,13 +322,12 @@ test_expect_success \"clone with promisor.sendFields\" '\n \tGIT_TRACE_PACKET=\"$(pwd)/trace\" GIT_NO_LAZY_FETCH=0 git clone \\\n \t\t-c remote.lop.promisor=true \\\n \t\t-c remote.lop.fetch=\"+refs/heads/*:refs/remotes/lop/*\" \\\n-\t\t-c remote.lop.url=\"file://$(pwd)/lop\" \\\n+\t\t-c remote.lop.url=\"$PWD_URL/lop\" \\\n \t\t-c promisor.acceptfromserver=All \\\n \t\t--no-local --filter=\"blob:limit=5k\" server client &&\n \n \t# Check that fields are properly transmitted\n-\tENCODED_URL=$(echo \"file://$(pwd)/lop\" | sed -e \"s/ /%20/g\") &&\n-\tPR1=\"name=lop,url=$ENCODED_URL,partialCloneFilter=blob:none\" &&\n+\tPR1=\"name=lop,url=$ENCODED_PWD_URL/lop,partialCloneFilter=blob:none\" &&\n \tPR2=\"name=otherLop,url=https://invalid.invalid,partialCloneFilter=blob:limit=10k,token=fooBar\" &&\n \ttest_grep \"clone< promisor-remote=$PR1;$PR2\" trace &&\n \ttest_grep \"clone> promisor-remote=lop;otherLop\" trace &&\n@@ -342,15 +352,14 @@ test_expect_success \"clone with promisor.checkFields\" '\n \tGIT_TRACE_PACKET=\"$(pwd)/trace\" GIT_NO_LAZY_FETCH=0 git clone \\\n \t\t-c remote.lop.promisor=true \\\n \t\t-c remote.lop.fetch=\"+refs/heads/*:refs/remotes/lop/*\" \\\n-\t\t-c remote.lop.url=\"file://$(pwd)/lop\" \\\n+\t\t-c remote.lop.url=\"$PWD_URL/lop\" \\\n \t\t-c remote.lop.partialCloneFilter=\"blob:none\" \\\n \t\t-c promisor.acceptfromserver=All \\\n \t\t-c promisor.checkFields=partialcloneFilter \\\n \t\t--no-local --filter=\"blob:limit=5k\" server client &&\n \n \t# Check that fields are properly transmitted\n-\tENCODED_URL=$(echo \"file://$(pwd)/lop\" | sed -e \"s/ /%20/g\") &&\n-\tPR1=\"name=lop,url=$ENCODED_URL,partialCloneFilter=blob:none\" &&\n+\tPR1=\"name=lop,url=$ENCODED_PWD_URL/lop,partialCloneFilter=blob:none\" &&\n \tPR2=\"name=otherLop,url=https://invalid.invalid,partialCloneFilter=blob:limit=10k,token=fooBar\" &&\n \ttest_grep \"clone< promisor-remote=$PR1;$PR2\" trace &&\n \ttest_grep \"clone> promisor-remote=lop\" trace &&\n@@ -380,7 +389,7 @@ test_expect_success \"clone with promisor.storeFields=partialCloneFilter\" '\n \tGIT_TRACE_PACKET=\"$(pwd)/trace\" GIT_NO_LAZY_FETCH=0 git clone \\\n \t\t-c remote.lop.promisor=true \\\n \t\t-c remote.lop.fetch=\"+refs/heads/*:refs/remotes/lop/*\" \\\n-\t\t-c remote.lop.url=\"file://$(pwd)/lop\" \\\n+\t\t-c remote.lop.url=\"$PWD_URL/lop\" \\\n \t\t-c remote.lop.token=\"fooYYY\" \\\n \t\t-c remote.lop.partialCloneFilter=\"blob:none\" \\\n \t\t-c promisor.acceptfromserver=All \\\n@@ -432,7 +441,7 @@ test_expect_success \"clone and fetch with --filter=auto\" '\n \n \tGIT_TRACE_PACKET=\"$(pwd)/trace\" GIT_NO_LAZY_FETCH=0 git clone \\\n \t\t-c remote.lop.promisor=true \\\n-\t\t-c remote.lop.url=\"file://$(pwd)/lop\" \\\n+\t\t-c remote.lop.url=\"$PWD_URL/lop\" \\\n \t\t-c promisor.acceptfromserver=All \\\n \t\t--no-local --filter=auto server client 2>err &&\n \n@@ -489,7 +498,7 @@ test_expect_success \"clone with promisor.advertise set to 'true' but don't delet\n \t# Clone from server to create a client\n \tGIT_NO_LAZY_FETCH=0 git clone -c remote.lop.promisor=true \\\n \t\t-c remote.lop.fetch=\"+refs/heads/*:refs/remotes/lop/*\" \\\n-\t\t-c remote.lop.url=\"file://$(pwd)/lop\" \\\n+\t\t-c remote.lop.url=\"$PWD_URL/lop\" \\\n \t\t-c promisor.acceptfromserver=All \\\n \t\t--no-local --filter=\"blob:limit=5k\" server client &&\n \n-- \n2.53.0.625.g20f70b52bb\n\n"},{"id":"539714","messageId":"20260323080520.887550-14-christian.couder@gmail.com","threadId":"65335","inReplyTo":"20260323080520.887550-1-christian.couder@gmail.com","subject":"[PATCH 13/16] promisor-remote: introduce promisor.acceptFromServerUrl","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2026-03-23T08:05:16Z","receivedAt":"2026-03-23T08:05:56Z","isPatch":true,"body":"The \"promisor-remote\" protocol capability allows servers to advertise\npromisor remotes, but doesn't allow these remotes to be automatically\nconfigured on the client.\n\nLet's introduce a new `promisor.acceptFromServerUrl` config variable\nwhich contains a glob pattern, so that advertised remotes with a URL\nmatching that pattern will be automatically configured.\n\nThe glob pattern can optionally be prefixed with a remote name which\nwill be used as the name of the new local remote.\n\nFor now though, let's only introduce the functions to read and validate\nthe glob patterns and the optional prefixes.\n\nChecking if the URLs of the advertised remotes match the glob patterns\nand taking the appropriate action is left for a following commit.\n\nSigned-off-by: Christian Couder \n---\n promisor-remote.c | 73 +++++++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 73 insertions(+)\n\ndiff --git a/promisor-remote.c b/promisor-remote.c\nindex c2f0eb7223..4cb18e1a6a 100644\n--- a/promisor-remote.c\n+++ b/promisor-remote.c\n@@ -12,6 +12,7 @@\n #include \"packfile.h\"\n #include \"environment.h\"\n #include \"url.h\"\n+#include \"urlmatch.h\"\n #include \"version.h\"\n \n struct promisor_remote_config {\n@@ -656,6 +657,76 @@ static bool has_control_char(const char *s)\n \treturn false;\n }\n \n+struct allowed_url {\n+\tchar *remote_name;\n+\tchar *url_pattern;\n+};\n+\n+static struct allowed_url *valid_accept_url(const char *url)\n+{\n+\tchar *dup, *p;\n+\tstruct allowed_url *allowed;\n+\n+\tif (!url)\n+\t\treturn NULL;\n+\n+\tdup = xstrdup(url);\n+\tp = strchr(dup, '=');\n+\tif (p) {\n+\t\t*p = '\\0';\n+\t\tif (!valid_remote_name(dup)) {\n+\t\t\twarning(_(\"invalid remote name '%s' before '=' sign \"\n+\t\t\t\t \"in '%s' from promisor.acceptFromServerUrl config\"),\n+\t\t\t\tdup, url);\n+\t\t\tfree(dup);\n+\t\t\treturn NULL;\n+\t\t}\n+\t\tp++;\n+\t} else {\n+\t\tp = dup;\n+\t}\n+\n+\tif (has_control_char(p) || !url_is_valid_pattern(p)) {\n+\t\twarning(_(\"invalid url pattern '%s' \"\n+\t\t\t \"in '%s' from promisor.acceptFromServerUrl config\"), p, url);\n+\t\tfree(dup);\n+\t\treturn NULL;\n+\t}\n+\n+\tallowed = xmalloc(sizeof(*allowed));\n+\tallowed->remote_name = (p == dup) ? NULL : dup;\n+\tallowed->url_pattern = p;\n+\n+\treturn allowed;\n+}\n+\n+static struct string_list *accept_from_server_url(struct repository *repo)\n+{\n+\tstatic struct string_list accept_urls = STRING_LIST_INIT_DUP;\n+\tstatic int initialized;\n+\tconst struct string_list *config_urls;\n+\n+\tif (initialized)\n+\t\treturn &accept_urls;\n+\n+\tinitialized = 1;\n+\n+\tif (!repo_config_get_string_multi(repo, \"promisor.acceptfromserverurl\", &config_urls)) {\n+\t\tstruct string_list_item *item;\n+\n+\t\tfor_each_string_list_item(item, config_urls) {\n+\t\t\tstruct allowed_url *allowed = valid_accept_url(item->string);\n+\t\t\tif (allowed) {\n+\t\t\t\tstruct string_list_item *new;\n+\t\t\t\tnew = string_list_append(&accept_urls, item->string);\n+\t\t\t\tnew->util = allowed;\n+\t\t\t}\n+\t\t}\n+\t}\n+\n+\treturn &accept_urls;\n+}\n+\n static int should_accept_remote(enum accept_promisor accept,\n \t\t\t\tstruct promisor_info *advertised,\n \t\t\t\tstruct string_list *config_info)\n@@ -901,6 +972,8 @@ static void filter_promisor_remote(struct repository *repo,\n \tstruct string_list_item *item;\n \tbool reload_config = false;\n \tenum accept_promisor accept = accept_from_server(repo);\n+\t/* Pre-load and validate the acceptFromServerUrl config */\n+\t(void)accept_from_server_url(repo);\n \n \tif (accept == ACCEPT_NONE)\n \t\treturn;\n-- \n2.53.0.625.g20f70b52bb\n\n"},{"id":"539715","messageId":"20260323080520.887550-12-christian.couder@gmail.com","threadId":"65335","inReplyTo":"20260323080520.887550-1-christian.couder@gmail.com","subject":"[PATCH 11/16] promisor-remote: refactor should_accept_remote() control flow","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2026-03-23T08:05:14Z","receivedAt":"2026-03-23T08:05:56Z","isPatch":true,"body":"In following commits, we are going to add URL-based acceptance logic\ninto should_accept_remote().\n\nTo prepare for the upcoming changes, let's restructure the control flow\nin should_accept_remote().\n\nConcretely, let's:\n\n - Move the empty-URL check to the very top of the function, so that\n every acceptance mode, instead of only ACCEPT_KNOWN_URL, rejects\n remotes with a missing or blank URL early.\n\n - Invert the URL comparison: instead of returning on match and\n warning on mismatch, return early on mismatch and let the match\n case fall through. This opens a single exit path at the bottom of\n the function for following commits to extend.\n\nAnyway the server shouldn't send any empty URL in the first place, so\nthis shouldn't change any behavior in practice.\n\nSigned-off-by: Christian Couder \n---\n promisor-remote.c | 21 +++++++++++----------\n 1 file changed, 11 insertions(+), 10 deletions(-)\n\ndiff --git a/promisor-remote.c b/promisor-remote.c\nindex 8f2c1280c3..c2f0eb7223 100644\n--- a/promisor-remote.c\n+++ b/promisor-remote.c\n@@ -665,6 +665,12 @@ static int should_accept_remote(enum accept_promisor accept,\n \tconst char *remote_name = advertised->name;\n \tconst char *remote_url = advertised->url;\n \n+\tif (!remote_url || !*remote_url) {\n+\t\twarning(_(\"no or empty URL advertised for remote '%s', \"\n+\t\t\t \"ignoring this remote\"), remote_name);\n+\t\treturn 0;\n+\t}\n+\n \tif (accept == ACCEPT_ALL)\n \t\treturn all_fields_match(advertised, config_info, NULL);\n \n@@ -683,19 +689,14 @@ static int should_accept_remote(enum accept_promisor accept,\n \tif (accept != ACCEPT_KNOWN_URL)\n \t\tBUG(\"Unhandled 'enum accept_promisor' value '%d'\", accept);\n \n-\tif (!remote_url || !*remote_url) {\n-\t\twarning(_(\"no or empty URL advertised for remote '%s', \"\n-\t\t\t \"ignoring this remote\"), remote_name);\n+\tif (strcmp(p->url, remote_url)) {\n+\t\twarning(_(\"known remote named '%s' but with URL '%s' instead of '%s', \"\n+\t\t\t \"ignoring this remote\"),\n+\t\t\tremote_name, p->url, remote_url);\n \t\treturn 0;\n \t}\n \n-\tif (!strcmp(p->url, remote_url))\n-\t\treturn all_fields_match(advertised, config_info, p);\n-\n-\twarning(_(\"known remote named '%s' but with URL '%s' instead of '%s', \"\n-\t\t \"ignoring this remote\"), remote_name, p->url, remote_url);\n-\n-\treturn 0;\n+\treturn all_fields_match(advertised, config_info, p);\n }\n \n static int skip_field_name_prefix(const char *elem, const char *field_name, const char **value)\n-- \n2.53.0.625.g20f70b52bb\n\n"},{"id":"539716","messageId":"20260323080520.887550-15-christian.couder@gmail.com","threadId":"65335","inReplyTo":"20260323080520.887550-1-christian.couder@gmail.com","subject":"[PATCH 14/16] promisor-remote: trust known remotes matching acceptFromServerUrl","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2026-03-23T08:05:17Z","receivedAt":"2026-03-23T08:05:58Z","isPatch":true,"body":"A previous commit introduced the `promisor.acceptFromServerUrl` config\nvariable along with the machinery to parse and validate the URL glob\npatterns and optional remote name prefixes it contains. However, these\nURL patterns are not yet tied into the client's acceptance logic.\n\nWhen a promisor remote is already configured locally, its fields (like\nauthentication tokens) may occasionally need to be refreshed by the\nserver. If `promisor.acceptFromServer` is set to the secure default\n(\"None\"), these updates are rejected, potentially causing future\nfetches to fail.\n\nTo enable such targeted updates for trusted URLs, let's use the URL\npatterns from `promisor.acceptFromServerUrl` as an additional URL\nbased whitelist.\n\nConcretely, let's check the advertised URLs against the URL glob\npatterns by introducing a new small helper function called\nurl_matches_accept_list(), which iterates over the glob patterns and\nreturns the first matching allowed_url entry (or NULL).\n\n(Before matching, the advertised URL is passed through url_normalize()\nso that case variations in the scheme/host, percent-encoding tricks,\nand \"..\" path segments cannot bypass the whitelist.)\n\nLet's then use this helper at the tail of should_accept_remote() so\nthat, when `accept == ACCEPT_NONE`, a known remote whose URL matches\nthe whitelist is still accepted.\n\nTo prepare for this new logic, let's also:\n\n - Add an 'accept_urls' parameter to should_accept_remote().\n\n - Replace the BUG() guard in the ACCEPT_KNOWN_URL case with an\n explicit 'if (accept == ACCEPT_KNOWN_URL) return' and a new\n BUG() guard in the ACCEPT_NONE case, so url_matches_accept_list()\n is only called in the ACCEPT_NONE case.\n\n - Call accept_from_server_url() from filter_promisor_remote()\n and relax its early return so that the function is entered when\n `accept_urls` has entries even if `accept == ACCEPT_NONE`.\n\nLet's then properly document `promisor.acceptFromServerUrl` in\n\"promisor.adoc\" as an additive security whitelist for known remotes,\nincluding the URL normalization behavior, and let's mention it in\n\"gitprotocol-v2.adoc\".\n\nSigned-off-by: Christian Couder \n---\n Documentation/config/promisor.adoc | 46 +++++++++++++++++\n Documentation/gitprotocol-v2.adoc | 9 ++--\n promisor-remote.c | 50 +++++++++++++++---\n t/t5710-promisor-remote-capability.sh | 73 +++++++++++++++++++++++++++\n 4 files changed, 166 insertions(+), 12 deletions(-)\n\ndiff --git a/Documentation/config/promisor.adoc b/Documentation/config/promisor.adoc\nindex b0fa43b839..6f5442cd65 100644\n--- a/Documentation/config/promisor.adoc\n+++ b/Documentation/config/promisor.adoc\n@@ -51,6 +51,52 @@ promisor.acceptFromServer::\n \tto \"fetch\" and \"clone\" requests from the client. Name and URL\n \tcomparisons are case sensitive. See linkgit:gitprotocol-v2[5].\n \n+promisor.acceptFromServerUrl::\n+\tA glob pattern to specify which URLs advertised by a server\n+\tare considered trusted by the client. This option acts as an\n+\tadditive security whitelist that works in conjunction with\n+\t`promisor.acceptFromServer`.\n++\n+This option can appear multiple times in config files. An advertised\n+URL will be accepted if it matches _ANY_ glob pattern specified by\n+this option in _ANY_ config file read by Git.\n++\n+Be _VERY_ careful with these glob patterns, as it can be a big\n+security hole to allow any advertised remote to be auto-configured!\n+To minimize security risks, follow these guidelines:\n++\n+1. Start with a secure protocol scheme, like `https://` or `ssh://`.\n++\n+2. Only allow domain names or paths where you control and trust _ALL_\n+ the content. Be especially careful with shared hosting platforms\n+ like `github.com` or `gitlab.com`. A broad pattern like\n+ `https://gitlab.com/*` is dangerous because it trusts every\n+ repository on the entire platform. Always restrict such patterns to\n+ your specific organization or namespace (e.g.,\n+ `https://gitlab.com/your-org/*`).\n++\n+3. Don't use globs (`*`) in the domain name. For example\n+ `https://cdn.example.com/*` is much safer than\n+ `https://*.example.com/*`, because the latter matches\n+ `https://evil-hacker.net/fake.example.com/repo`.\n++\n+4. Make sure to have a `/` at the end of the domain name (or the end\n+ of specific directories). For example `https://cdn.example.com/*`\n+ is much safer than `https://cdn.example.com*`, because the latter\n+ matches `https://cdn.example.com.hacker.net/repo`.\n++\n+Before matching, the advertised URL is normalized: the scheme and\n+host are lowercased, percent-encoded characters are decoded where\n+possible, and path segments like `..` are resolved. Glob patterns\n+are matched against this normalized URL as-is, so patterns should\n+be written in normalized form (e.g., lowercase scheme and host).\n++\n+Even if `promisor.acceptFromServer` is set to `None` (the default),\n+Git will still accept field updates (like tokens) for known remotes,\n+provided their URLs match a pattern in\n+`promisor.acceptFromServerUrl`. See linkgit:gitprotocol-v2[5] for\n+details on the protocol.\n+\n promisor.checkFields::\n \tA comma or space separated list of additional remote related\n \tfield names. A client checks if the values of these fields\ndiff --git a/Documentation/gitprotocol-v2.adoc b/Documentation/gitprotocol-v2.adoc\nindex f985cb4c47..175358bba2 100644\n--- a/Documentation/gitprotocol-v2.adoc\n+++ b/Documentation/gitprotocol-v2.adoc\n@@ -862,10 +862,11 @@ the server advertised, the client shouldn't advertise the\n \n On the server side, the \"promisor.advertise\" and \"promisor.sendFields\"\n configuration options can be used to control what it advertises. On\n-the client side, the \"promisor.acceptFromServer\" configuration option\n-can be used to control what it accepts, and the \"promisor.storeFields\"\n-option, to control what it stores. See the documentation of these\n-configuration options in linkgit:git-config[1] for more information.\n+the client side, the \"promisor.acceptFromServer\" and\n+\"promisor.acceptFromServerUrl\" configuration options can be used to\n+control what it accepts, and the \"promisor.storeFields\" option, to\n+control what it stores. See the documentation of these configuration\n+options in linkgit:git-config[1] for more information.\n \n Note that in the future it would be nice if the \"promisor-remote\"\n protocol capability could be used by the server, when responding to\ndiff --git a/promisor-remote.c b/promisor-remote.c\nindex 4cb18e1a6a..210e6950af 100644\n--- a/promisor-remote.c\n+++ b/promisor-remote.c\n@@ -14,6 +14,7 @@\n #include \"url.h\"\n #include \"urlmatch.h\"\n #include \"version.h\"\n+#include \"wildmatch.h\"\n \n struct promisor_remote_config {\n \tstruct promisor_remote *promisors;\n@@ -727,8 +728,31 @@ static struct string_list *accept_from_server_url(struct repository *repo)\n \treturn &accept_urls;\n }\n \n+static struct allowed_url *url_matches_accept_list(\n+\t\tstruct string_list *accept_urls, const char *url)\n+{\n+\tstruct string_list_item *item;\n+\tchar *normalized = url_normalize(url, NULL);\n+\n+\tif (!normalized)\n+\t\treturn NULL;\n+\n+\tfor_each_string_list_item(item, accept_urls) {\n+\t\tstruct allowed_url *allowed = item->util;\n+\n+\t\tif (!wildmatch(allowed->url_pattern, normalized, 0)) {\n+\t\t\tfree(normalized);\n+\t\t\treturn allowed;\n+\t\t}\n+\t}\n+\n+\tfree(normalized);\n+\treturn NULL;\n+}\n+\n static int should_accept_remote(enum accept_promisor accept,\n \t\t\t\tstruct promisor_info *advertised,\n+\t\t\t\tstruct string_list *accept_urls,\n \t\t\t\tstruct string_list *config_info)\n {\n \tstruct promisor_info *p;\n@@ -757,9 +781,6 @@ static int should_accept_remote(enum accept_promisor accept,\n \tif (accept == ACCEPT_KNOWN_NAME)\n \t\treturn all_fields_match(advertised, config_info, p);\n \n-\tif (accept != ACCEPT_KNOWN_URL)\n-\t\tBUG(\"Unhandled 'enum accept_promisor' value '%d'\", accept);\n-\n \tif (strcmp(p->url, remote_url)) {\n \t\twarning(_(\"known remote named '%s' but with URL '%s' instead of '%s', \"\n \t\t\t \"ignoring this remote\"),\n@@ -767,7 +788,21 @@ static int should_accept_remote(enum accept_promisor accept,\n \t\treturn 0;\n \t}\n \n-\treturn all_fields_match(advertised, config_info, p);\n+\tif (accept == ACCEPT_KNOWN_URL)\n+\t\treturn all_fields_match(advertised, config_info, p);\n+\n+\tif (accept != ACCEPT_NONE)\n+\t\tBUG(\"Unhandled 'enum accept_promisor' value '%d'\", accept);\n+\n+\t/*\n+\t * Even if accept == ACCEPT_NONE, we MUST trust this known\n+\t * remote to update its token or other such fields if its URL\n+\t * matches the acceptFromServerUrl whitelist!\n+\t */\n+\tif (url_matches_accept_list(accept_urls, remote_url))\n+\t\treturn all_fields_match(advertised, config_info, p);\n+\n+\treturn 0;\n }\n \n static int skip_field_name_prefix(const char *elem, const char *field_name, const char **value)\n@@ -972,10 +1007,9 @@ static void filter_promisor_remote(struct repository *repo,\n \tstruct string_list_item *item;\n \tbool reload_config = false;\n \tenum accept_promisor accept = accept_from_server(repo);\n-\t/* Pre-load and validate the acceptFromServerUrl config */\n-\t(void)accept_from_server_url(repo);\n+\tstruct string_list *accept_urls = accept_from_server_url(repo);\n \n-\tif (accept == ACCEPT_NONE)\n+\tif (accept == ACCEPT_NONE && !accept_urls->nr)\n \t\treturn;\n \n \t/* Parse remote info received */\n@@ -995,7 +1029,7 @@ static void filter_promisor_remote(struct repository *repo,\n \t\t\tstring_list_sort(&config_info);\n \t\t}\n \n-\t\tif (should_accept_remote(accept, advertised, &config_info)) {\n+\t\tif (should_accept_remote(accept, advertised, accept_urls, &config_info)) {\n \t\t\tif (!store_info)\n \t\t\t\tstore_info = store_info_new(repo);\n \t\t\tif (promisor_store_advertised_fields(advertised, store_info))\ndiff --git a/t/t5710-promisor-remote-capability.sh b/t/t5710-promisor-remote-capability.sh\nindex c7a484228f..4ebad14af5 100755\n--- a/t/t5710-promisor-remote-capability.sh\n+++ b/t/t5710-promisor-remote-capability.sh\n@@ -306,6 +306,77 @@ test_expect_success \"clone with 'KnownUrl' and empty url, so not advertised\" '\n \tcheck_missing_objects server 1 \"$oid\"\n '\n \n+test_expect_success \"clone with 'None' but URL whitelisted\" '\n+\tgit -C server config promisor.advertise true &&\n+\ttest_when_finished \"rm -rf client\" &&\n+\n+\tGIT_NO_LAZY_FETCH=0 git clone -c remote.lop.promisor=true \\\n+\t\t-c remote.lop.fetch=\"+refs/heads/*:refs/remotes/lop/*\" \\\n+\t\t-c remote.lop.url=\"$PWD_URL/lop\" \\\n+\t\t-c promisor.acceptfromserver=None \\\n+\t\t-c promisor.acceptFromServerUrl=\"$ENCODED_PWD_URL/*\" \\\n+\t\t--no-local --filter=\"blob:limit=5k\" server client &&\n+\n+\t# Check that the largest object is still missing on the server\n+\tcheck_missing_objects server 1 \"$oid\"\n+'\n+\n+test_expect_success \"clone with 'None' but URL not in whitelist\" '\n+\tgit -C server config promisor.advertise true &&\n+\ttest_when_finished \"rm -rf client\" &&\n+\n+\tGIT_NO_LAZY_FETCH=0 git clone -c remote.lop.promisor=true \\\n+\t\t-c remote.lop.fetch=\"+refs/heads/*:refs/remotes/lop/*\" \\\n+\t\t-c remote.lop.url=\"$PWD_URL/lop\" \\\n+\t\t-c promisor.acceptfromserver=None \\\n+\t\t-c promisor.acceptFromServerUrl=\"https://example.com/*\" \\\n+\t\t--no-local --filter=\"blob:limit=5k\" server client &&\n+\n+\t# Check that the largest object is not missing on the server\n+\tcheck_missing_objects server 0 \"\" &&\n+\n+\t# Reinitialize server so that the largest object is missing again\n+\tinitialize_server 1 \"$oid\"\n+'\n+\n+test_expect_success \"clone with 'None' but URL whitelisted in one pattern out of two\" '\n+\tgit -C server config promisor.advertise true &&\n+\ttest_when_finished \"rm -rf client\" &&\n+\n+\tGIT_NO_LAZY_FETCH=0 git clone -c remote.lop.promisor=true \\\n+\t\t-c remote.lop.fetch=\"+refs/heads/*:refs/remotes/lop/*\" \\\n+\t\t-c remote.lop.url=\"$PWD_URL/lop\" \\\n+\t\t-c promisor.acceptfromserver=None \\\n+\t\t-c promisor.acceptFromServerUrl=\"https://example.com/*\" \\\n+\t\t-c promisor.acceptFromServerUrl=\"$ENCODED_PWD_URL/*\" \\\n+\t\t--no-local --filter=\"blob:limit=5k\" server client &&\n+\n+\t# Check that the largest object is still missing on the server\n+\tcheck_missing_objects server 1 \"$oid\"\n+'\n+\n+test_expect_success \"clone with 'None', URL whitelisted, but client has different URL\" '\n+\tgit -C server config promisor.advertise true &&\n+\ttest_when_finished \"rm -rf client\" &&\n+\n+\t# The client configures \"lop\" with a different URL (serverTwo) than\n+\t# what the server advertises (lop). Even though the advertised URL\n+\t# matches the whitelist, the remote is rejected because the\n+\t# configured URL does not match the advertised one.\n+\tGIT_NO_LAZY_FETCH=0 git clone -c remote.lop.promisor=true \\\n+\t\t-c remote.lop.fetch=\"+refs/heads/*:refs/remotes/lop/*\" \\\n+\t\t-c remote.lop.url=\"$PWD_URL/serverTwo\" \\\n+\t\t-c promisor.acceptfromserver=None \\\n+\t\t-c promisor.acceptFromServerUrl=\"$ENCODED_PWD_URL/*\" \\\n+\t\t--no-local --filter=\"blob:limit=5k\" server client &&\n+\n+\t# Check that the largest object is not missing on the server\n+\tcheck_missing_objects server 0 \"\" &&\n+\n+\t# Reinitialize server so that the largest object is missing again\n+\tinitialize_server 1 \"$oid\"\n+'\n+\n test_expect_success \"clone with promisor.sendFields\" '\n \tgit -C server config promisor.advertise true &&\n \ttest_when_finished \"rm -rf client\" &&\n@@ -573,4 +644,6 @@ test_expect_success \"subsequent fetch from a client when promisor.advertise is f\n \tcheck_missing_objects server 1 \"$oid\"\n '\n \n+\n+\n test_done\n-- \n2.53.0.625.g20f70b52bb\n\n"},{"id":"539717","messageId":"20260323080520.887550-17-christian.couder@gmail.com","threadId":"65335","inReplyTo":"20260323080520.887550-1-christian.couder@gmail.com","subject":"[PATCH 16/16] doc: promisor: improve acceptFromServer entry","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2026-03-23T08:05:19Z","receivedAt":"2026-03-23T08:05:59Z","isPatch":true,"body":"The entry for the `promisor.acceptFromServer` in\n\"Documentation/config/promisor.adoc\" has a number of issues:\n\n- it's not clear if new remotes and URLs can be created,\n- it looks like a big block of text,\n- it's not easy to see all the options,\n- it's not easy to see which option is the default one,\n- for \"knownName\", it says \"advertised by the client\" instead of\n \"advertised by the server\",\n- it doesn't refer to the new related `acceptFromServerUrl`\n option.\n\nLet's address all these issues by rewording large parts of it\nand using bullet points for the different options.\n\nSigned-off-by: Christian Couder \n---\n Documentation/config/promisor.adoc | 53 ++++++++++++++++++++----------\n 1 file changed, 35 insertions(+), 18 deletions(-)\n\ndiff --git a/Documentation/config/promisor.adoc b/Documentation/config/promisor.adoc\nindex d8e5f4a6dc..1d64e7f1d4 100644\n--- a/Documentation/config/promisor.adoc\n+++ b/Documentation/config/promisor.adoc\n@@ -32,24 +32,41 @@ variable is set to \"true\", and the \"name\" and \"url\" fields are always\n advertised regardless of this setting.\n \n promisor.acceptFromServer::\n-\tIf set to \"all\", a client will accept all the promisor remotes\n-\ta server might advertise using the \"promisor-remote\"\n-\tcapability. If set to \"knownName\" the client will accept\n-\tpromisor remotes which are already configured on the client\n-\tand have the same name as those advertised by the client. This\n-\tis not very secure, but could be used in a corporate setup\n-\twhere servers and clients are trusted to not switch name and\n-\tURLs. If set to \"knownUrl\", the client will accept promisor\n-\tremotes which have both the same name and the same URL\n-\tconfigured on the client as the name and URL advertised by the\n-\tserver. This is more secure than \"all\" or \"knownName\", so it\n-\tshould be used if possible instead of those options. Default\n-\tis \"none\", which means no promisor remote advertised by a\n-\tserver will be accepted. By accepting a promisor remote, the\n-\tclient agrees that the server might omit objects that are\n-\tlazily fetchable from this promisor remote from its responses\n-\tto \"fetch\" and \"clone\" requests from the client. Name and URL\n-\tcomparisons are case sensitive. See linkgit:gitprotocol-v2[5].\n+\tControls which promisor remotes advertised by a server (using the\n+\t\"promisor-remote\" protocol capability) a client will accept. By\n+\taccepting a promisor remote, the client agrees that the server\n+\tmight omit objects that are lazily fetchable from this promisor\n+\tremote from its responses to \"fetch\" and \"clone\" requests.\n++\n+Note that this option does not cause new remotes to be automatically\n+created in the client's configuration. It only allows remotes which\n+are somehow already configured to be trusted for the current\n+operation, or their fields to be updated (if `promisor.storeFields` is\n+set and the remote already exists locally). To allow Git to\n+automatically create and persist new remotes from server\n+advertisements, use `promisor.acceptFromServerUrl`.\n++\n+The available options are:\n++\n+* `none` (default): No promisor remote advertised by a server will be\n+ accepted.\n++\n+* `knownUrl`: The client will accept promisor remotes that are already\n+ configured on the client and have both the same name and the same URL\n+ as advertised by the server. This is more secure than `all` or\n+ `knownName`, and should be used if possible instead of those options.\n++\n+* `knownName`: The client will accept promisor remotes that are already\n+ configured on the client and have the same name as those advertised\n+ by the server. This is not very secure, but could be used in a corporate\n+ setup where servers and clients are trusted to not switch names and URLs.\n++\n+* `all`: The client will accept all the promisor remotes a server might\n+ advertise. This is the least secure option and should only be used in\n+ fully trusted environments.\n++\n+Name and URL comparisons are case-sensitive. See linkgit:gitprotocol-v2[5]\n+for protocol details.\n \n promisor.acceptFromServerUrl::\n \tA glob pattern to specify which URLs advertised by a server\n-- \n2.53.0.625.g20f70b52bb\n\n"},{"id":"539718","messageId":"20260323080520.887550-16-christian.couder@gmail.com","threadId":"65335","inReplyTo":"20260323080520.887550-1-christian.couder@gmail.com","subject":"[PATCH 15/16] promisor-remote: auto-configure unknown remotes","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2026-03-23T08:05:18Z","receivedAt":"2026-03-23T08:05:59Z","isPatch":true,"body":"Previous commits have introduced the `promisor.acceptFromServerUrl`\nconfig variable to whitelist some URLs advertised by a server through\nthe \"promisor-remote\" protocol capability.\n\nHowever the new `promisor.acceptFromServerUrl` mechanism, like the old\n`promisor.acceptFromServer` mechanism, still requires a remote to\nalready exist in the client's local configuration before it can be\naccepted. This places a significant manual burden on users to\npre-configure these remotes, and creates friction for administrators\nwho have to troubleshoot or manually provision these setups for their\nteams.\n\nTo eliminate this burden, let's automatically create a new `[remote]`\nsection in the client's config when a server advertises an unknown\nremote whose URL matches a `promisor.acceptFromServerUrl` glob pattern.\n\nConcretely, let's add four helpers:\n\n - sanitize_remote_name(): turn an arbitrary URL-derived string into a\n valid remote name by replacing non-alphanumeric characters,\n collapsing runs of '-', and prepending \"promisor-auto-\".\n\n - promisor_remote_name_from_url(): normalize the URL and extract\n host+port+path to build a human-readable base name, then pass it\n through sanitize_remote_name().\n\n - configure_auto_promisor_remote(): write the remote.*.url,\n remote.*.promisor and remote.*.advertisedAs keys to the repo\n config.\n\n - handle_matching_allowed_url(): pick the final name (user-supplied\n alias or auto-generated), handle collisions by appending \"-1\",\n \"-2\", etc., then call configure_auto_promisor_remote().\n\nLet's also add should_accept_new_remote_url() which reuses the\nurl_matches_accept_list() helper introduced in a previous commit to\nfind a matching pattern, then delegates to handle_matching_allowed_url()\nto create the remote.\n\nAnd then let's call should_accept_new_remote_url() from the '!item'\n(unknown remote) branch of should_accept_remote(), setting\n`reload_config` so that the newly-written config is picked up.\n\nFinally let's document all that by:\n\n - expanding the `promisor.acceptFromServerUrl` entry to describe\n auto-creation, the optional \"name=\" prefix syntax, the\n \"promisor-auto-*\" generation rules, and numeric-suffix collision\n handling, and by\n - adding a \"remote..advertisedAs\" entry to \"remote.adoc\".\n\nSigned-off-by: Christian Couder \n---\n Documentation/config/promisor.adoc | 35 ++++-\n Documentation/config/remote.adoc | 9 ++\n promisor-remote.c | 202 +++++++++++++++++++++++++-\n t/t5710-promisor-remote-capability.sh | 122 ++++++++++++++++\n 4 files changed, 355 insertions(+), 13 deletions(-)\n\ndiff --git a/Documentation/config/promisor.adoc b/Documentation/config/promisor.adoc\nindex 6f5442cd65..d8e5f4a6dc 100644\n--- a/Documentation/config/promisor.adoc\n+++ b/Documentation/config/promisor.adoc\n@@ -53,9 +53,11 @@ promisor.acceptFromServer::\n \n promisor.acceptFromServerUrl::\n \tA glob pattern to specify which URLs advertised by a server\n-\tare considered trusted by the client. This option acts as an\n-\tadditive security whitelist that works in conjunction with\n-\t`promisor.acceptFromServer`.\n+\tare allowed to be auto-configured (created and persisted) on\n+\tthe client side. Unlike `promisor.acceptFromServer`, which\n+\tonly accepts already configured remotes, a match against this\n+\toption instructs Git to write a new `[remote \"\"]`\n+\tsection to the client's configuration.\n +\n This option can appear multiple times in config files. An advertised\n URL will be accepted if it matches _ANY_ glob pattern specified by\n@@ -91,11 +93,28 @@ possible, and path segments like `..` are resolved. Glob patterns\n are matched against this normalized URL as-is, so patterns should\n be written in normalized form (e.g., lowercase scheme and host).\n +\n-Even if `promisor.acceptFromServer` is set to `None` (the default),\n-Git will still accept field updates (like tokens) for known remotes,\n-provided their URLs match a pattern in\n-`promisor.acceptFromServerUrl`. See linkgit:gitprotocol-v2[5] for\n-details on the protocol.\n+The glob pattern can optionally be prefixed with a remote name and an\n+equals sign (e.g., `cdn=https://cdn.example.com/*`). If such a prefix\n+is provided, accepted remotes will be saved under that name. If no\n+such prefix is provided, a safe remote name will be automatically\n+generated by sanitizing the URL and prefixing it with\n+`promisor-auto-`. If a remote with the chosen name already exists but\n+points to a different URL, Git will append a numeric suffix (e.g.,\n+`-1`, `-2`) to the name to prevent overwriting existing\n+configurations. You should make sure that this doesn't happen often\n+though, as remotes will be rejected if the numeric suffix increases\n+too much. In all cases, the original name advertised by the server is\n+recorded in the `remote..advertisedAs` configuration variable\n+for tracing and debugging purposes.\n++\n+Note that this option acts as an additive security whitelist. It works\n+in conjunction with `promisor.acceptFromServer` (see the documentation\n+of that option for the implications of accepting a promisor\n+remote). Even if `promisor.acceptFromServer` is set to `None` (the\n+default), Git will still automatically configure new remotes, and\n+accept field updates (like tokens) for known remotes, provided their\n+URLs match a pattern in `promisor.acceptFromServerUrl`. See\n+linkgit:gitprotocol-v2[5] for details on the protocol.\n \n promisor.checkFields::\n \tA comma or space separated list of additional remote related\ndiff --git a/Documentation/config/remote.adoc b/Documentation/config/remote.adoc\nindex 91e46f66f5..6e2bbdf457 100644\n--- a/Documentation/config/remote.adoc\n+++ b/Documentation/config/remote.adoc\n@@ -91,6 +91,15 @@ remote..promisor::\n \tWhen set to true, this remote will be used to fetch promisor\n \tobjects.\n \n+remote..advertisedAs::\n+\tWhen a promisor remote is automatically configured using\n+\tinformation advertised by a server through the\n+\t`promisor-remote` protocol capability (see\n+\t`promisor.acceptFromServerUrl`), the server's originally\n+\tadvertised name is saved in this variable. This is for\n+\tinformation, tracing and debugging purposes. Users should not\n+\ttypically modify or create such configuration entries.\n+\n remote..partialclonefilter::\n \tThe filter that will be applied when fetching from this\tpromisor remote.\n \tChanging or clearing this value will only affect fetches for new commits.\ndiff --git a/promisor-remote.c b/promisor-remote.c\nindex 210e6950af..5321a9c4bf 100644\n--- a/promisor-remote.c\n+++ b/promisor-remote.c\n@@ -750,10 +750,197 @@ static struct allowed_url *url_matches_accept_list(\n \treturn NULL;\n }\n \n-static int should_accept_remote(enum accept_promisor accept,\n+/*\n+ * Sanitize the buffer to make it a valid remote name coming from the\n+ * server by:\n+ *\n+ * - replacing any non alphanumeric character with a '-'\n+ * - stripping any leading '-',\n+ * - condensing multiple '-' into one,\n+ * - prepending \"promisor-auto-\",\n+ * - validating the result.\n+ */\n+static int sanitize_remote_name(struct strbuf *buf, const char *url)\n+{\n+\tchar prev = '-';\n+\tfor (size_t i = 0; i < buf->len; ) {\n+\t\tif (!isalnum(buf->buf[i]))\n+\t\t\tbuf->buf[i] = '-';\n+\t\tif (prev == '-' && buf->buf[i] == '-') {\n+\t\t\tstrbuf_remove(buf, i, 1);\n+\t\t} else {\n+\t\t\tprev = buf->buf[i];\n+\t\t\ti++;\n+\t\t}\n+\t}\n+\n+\tstrbuf_strip_suffix(buf, \"-\");\n+\n+\tif (!buf->len) {\n+\t\twarning(_(\"couldn't generate a valid remote name from \"\n+\t\t\t \"advertised url '%s', ignoring this remote\"), url);\n+\t\treturn -1;\n+\t}\n+\n+\tstrbuf_insertstr(buf, 0, \"promisor-auto-\");\n+\n+\tif (!valid_remote_name(buf->buf)) {\n+\t\twarning(_(\"generated remote name '%s' from advertised url '%s' \"\n+\t\t\t \"is invalid, ignoring this remote\"), buf->buf, url);\n+\t\treturn -1;\n+\t}\n+\n+\treturn 0;\n+}\n+\n+static char *promisor_remote_name_from_url(const char *url)\n+{\n+\tstruct url_info url_info = { 0 };\n+\tchar *normalized = url_normalize(url, &url_info);\n+\tstruct strbuf buf = STRBUF_INIT;\n+\n+\tif (!normalized) {\n+\t\twarning(_(\"couldn't normalize advertised url '%s', \"\n+\t\t\t \"ignoring this remote\"), url);\n+\t\treturn NULL;\n+\t}\n+\n+\tif (url_info.host_len) {\n+\t\tstrbuf_add(&buf, normalized + url_info.host_off, url_info.host_len);\n+\t\tstrbuf_addch(&buf, '-');\n+\t}\n+\n+\tif (url_info.port_len) {\n+\t\tstrbuf_add(&buf, normalized + url_info.port_off, url_info.port_len);\n+\t\tstrbuf_addch(&buf, '-');\n+\t}\n+\n+\tif (url_info.path_len) {\n+\t\tstrbuf_add(&buf, normalized + url_info.path_off, url_info.path_len);\n+\t\tstrbuf_trim_trailing_dir_sep(&buf);\n+\t\tstrbuf_strip_suffix(&buf, \".git\");\n+\t}\n+\n+\tfree(normalized);\n+\n+\tif (sanitize_remote_name(&buf, url)) {\n+\t\tstrbuf_release(&buf);\n+\t\treturn NULL;\n+\t}\n+\n+\treturn strbuf_detach(&buf, NULL);\n+}\n+\n+static void configure_auto_promisor_remote(struct repository *repo,\n+\t\t\t\t\t const char *name,\n+\t\t\t\t\t const char *url,\n+\t\t\t\t\t const char *advertised_as,\n+\t\t\t\t\t bool reuse)\n+{\n+\tchar *key;\n+\n+\tif (!reuse) {\n+\t\tfprintf(stderr, _(\"Auto-creating promisor remote '%s' for URL '%s'\\n\"),\n+\t\t\tname, url);\n+\n+\t\tkey = xstrfmt(\"remote.%s.url\", name);\n+\t\trepo_config_set_gently(repo, key, url);\n+\t\tfree(key);\n+\t}\n+\n+\t/* NB: when reusing, this promotes an existing non-promisor remote */\n+\tkey = xstrfmt(\"remote.%s.promisor\", name);\n+\trepo_config_set_gently(repo, key, \"true\");\n+\tfree(key);\n+\n+\tif (advertised_as) {\n+\t\tkey = xstrfmt(\"remote.%s.advertisedAs\", name);\n+\t\trepo_config_set_gently(repo, key, advertised_as);\n+\t\tfree(key);\n+\t}\n+}\n+\n+#define MAX_REMOTES_WITH_SIMILAR_NAMES 20\n+\n+/* Return the allocated local name, or NULL on failure */\n+static char *handle_matching_allowed_url(struct repository *repo,\n+\t\t\t\t\t char *allowed_name,\n+\t\t\t\t\t const char *remote_url,\n+\t\t\t\t\t const char *remote_name)\n+{\n+\tchar *name;\n+\tchar *basename = allowed_name ?\n+\t\txstrdup(allowed_name) :\n+\t\tpromisor_remote_name_from_url(remote_url);\n+\tint i = 0;\n+\tbool reuse = false;\n+\n+\tif (!basename)\n+\t\treturn NULL;\n+\n+\tname = xstrdup(basename);\n+\n+\twhile (i < MAX_REMOTES_WITH_SIMILAR_NAMES) {\n+\t\tchar *url_key = xstrfmt(\"remote.%s.url\", name);\n+\t\tconst char *existing_url;\n+\t\tint exists = !repo_config_get_string_tmp(repo, url_key, &existing_url);\n+\n+\t\tfree(url_key);\n+\n+\t\tif (!exists)\n+\t\t\tbreak; /* Free to use */\n+\n+\t\tif (!strcmp(existing_url, remote_url)) {\n+\t\t\treuse = true;\n+\t\t\tbreak; /* Same URL, so safe to reuse */\n+\t\t}\n+\n+\t\ti++;\n+\t\tfree(name);\n+\t\tname = xstrfmt(\"%s-%d\", basename, i);\n+\t}\n+\n+\tif (i < MAX_REMOTES_WITH_SIMILAR_NAMES) {\n+\t\tconfigure_auto_promisor_remote(repo, name,\n+\t\t\t\t\t remote_url, remote_name,\n+\t\t\t\t\t reuse);\n+\t} else {\n+\t\twarning(_(\"too many remotes accepted with name like '%s-X', \"\n+\t\t\t \"ignoring this remote\"), basename);\n+\t\tFREE_AND_NULL(name);\n+\t}\n+\n+\tfree(basename);\n+\treturn name;\n+}\n+\n+static int should_accept_new_remote_url(struct repository *repo,\n+\t\t\t\t\tstruct string_list *accept_urls,\n+\t\t\t\t\tstruct promisor_info *advertised)\n+{\n+\tstruct allowed_url *allowed = url_matches_accept_list(accept_urls,\n+\t\t\t\t\t\t\t advertised->url);\n+\tif (allowed) {\n+\t\tchar *name = handle_matching_allowed_url(repo,\n+\t\t\t\t\t\t\t allowed->remote_name,\n+\t\t\t\t\t\t\t advertised->url,\n+\t\t\t\t\t\t\t advertised->name);\n+\t\tif (name) {\n+\t\t\tfree((char *)advertised->local_name);\n+\t\t\tadvertised->local_name = name;\n+\t\t\treturn 1;\n+\t\t}\n+\t}\n+\n+\treturn 0;\n+}\n+\n+static int should_accept_remote(struct repository *repo,\n+\t\t\t\tenum accept_promisor accept,\n \t\t\t\tstruct promisor_info *advertised,\n \t\t\t\tstruct string_list *accept_urls,\n-\t\t\t\tstruct string_list *config_info)\n+\t\t\t\tstruct string_list *config_info,\n+\t\t\t\tbool *reload_config)\n {\n \tstruct promisor_info *p;\n \tstruct string_list_item *item;\n@@ -772,9 +959,13 @@ static int should_accept_remote(enum accept_promisor accept,\n \t/* Get config info for that promisor remote */\n \titem = string_list_lookup(config_info, remote_name);\n \n-\tif (!item)\n+\tif (!item) {\n \t\t/* We don't know about that remote */\n-\t\treturn 0;\n+\t\tint res = should_accept_new_remote_url(repo, accept_urls, advertised);\n+\t\tif (res)\n+\t\t\t*reload_config = true;\n+\t\treturn res;\n+\t}\n \n \tp = item->util;\n \n@@ -1029,7 +1220,8 @@ static void filter_promisor_remote(struct repository *repo,\n \t\t\tstring_list_sort(&config_info);\n \t\t}\n \n-\t\tif (should_accept_remote(accept, advertised, accept_urls, &config_info)) {\n+\t\tif (should_accept_remote(repo, accept, advertised, accept_urls,\n+\t\t\t\t\t &config_info, &reload_config)) {\n \t\t\tif (!store_info)\n \t\t\t\tstore_info = store_info_new(repo);\n \t\t\tif (promisor_store_advertised_fields(advertised, store_info))\ndiff --git a/t/t5710-promisor-remote-capability.sh b/t/t5710-promisor-remote-capability.sh\nindex 4ebad14af5..7d8df05fc7 100755\n--- a/t/t5710-promisor-remote-capability.sh\n+++ b/t/t5710-promisor-remote-capability.sh\n@@ -377,6 +377,128 @@ test_expect_success \"clone with 'None', URL whitelisted, but client has differen\n \tinitialize_server 1 \"$oid\"\n '\n \n+test_expect_success \"clone with URL whitelisted and no remote already configured\" '\n+\tgit -C server config promisor.advertise true &&\n+\ttest_when_finished \"rm -rf client\" &&\n+\n+\tGIT_NO_LAZY_FETCH=0 git clone \\\n+\t\t-c promisor.acceptfromserver=None \\\n+\t\t-c promisor.acceptFromServerUrl=\"$ENCODED_PWD_URL/*\" \\\n+\t\t--no-local --filter=\"blob:limit=5k\" server client &&\n+\n+\t# Check that a remote has been auto-created with the right fields.\n+\t# The remote is identified by \"remote..advertisedAs\" == \"lop\".\n+\tFULL_NAME=$(git -C client config --name-only --get-regexp \"remote\\..*\\.advertisedas\" \"^lop$\") &&\n+\tREMOTE_NAME=$(echo \"$FULL_NAME\" | sed \"s/remote\\.\\(.*\\)\\.advertisedas/\\1/\") &&\n+\n+\t# Check \".url\" and \".promisor\" values\n+\tprintf \"%s\\n\" \"$PWD_URL/lop\" \"true\" >expect &&\n+\tgit -C client config \"remote.$REMOTE_NAME.url\" >actual &&\n+\tgit -C client config \"remote.$REMOTE_NAME.promisor\" >>actual &&\n+\ttest_cmp expect actual &&\n+\n+\t# Check that the largest object is still missing on the server\n+\tcheck_missing_objects server 1 \"$oid\"\n+'\n+\n+test_expect_success \"clone with named URL whitelisted and no pre-configured remote\" '\n+\tgit -C server config promisor.advertise true &&\n+\ttest_when_finished \"rm -rf client\" &&\n+\n+\tGIT_NO_LAZY_FETCH=0 git clone \\\n+\t\t-c promisor.acceptfromserver=None \\\n+\t\t-c promisor.acceptFromServerUrl=\"cdn=$ENCODED_PWD_URL/*\" \\\n+\t\t--no-local --filter=\"blob:limit=5k\" server client &&\n+\n+\t# Check that a remote has been auto-created with the right \"cdn\" name and fields.\n+\tprintf \"%s\\n\" \"$PWD_URL/lop\" \"true\" \"lop\" >expect &&\n+\tgit -C client config \"remote.cdn.url\" >actual &&\n+\tgit -C client config \"remote.cdn.promisor\" >>actual &&\n+\tgit -C client config \"remote.cdn.advertisedAs\" >>actual &&\n+\ttest_cmp expect actual &&\n+\n+\t# Check that the largest object is still missing on the server\n+\tcheck_missing_objects server 1 \"$oid\"\n+'\n+\n+test_expect_success \"clone with URL whitelisted but colliding name\" '\n+\tgit -C server config promisor.advertise true &&\n+\ttest_when_finished \"rm -rf client\" &&\n+\n+\tGIT_NO_LAZY_FETCH=0 git clone -c remote.cdn.promisor=true \\\n+\t\t-c remote.cdn.fetch=\"+refs/heads/*:refs/remotes/lop/*\" \\\n+\t\t-c remote.cdn.url=\"https://example.com/cdn\" \\\n+\t\t-c promisor.acceptfromserver=None \\\n+\t\t-c promisor.acceptFromServerUrl=\"cdn=$ENCODED_PWD_URL/*\" \\\n+\t\t--no-local --filter=\"blob:limit=5k\" server client &&\n+\n+\t# Check that a remote has been auto-created with the right \"cdn-1\" name and fields.\n+\tprintf \"%s\\n\" \"$PWD_URL/lop\" \"true\" \"lop\" >expect &&\n+\tgit -C client config \"remote.cdn-1.url\" >actual &&\n+\tgit -C client config \"remote.cdn-1.promisor\" >>actual &&\n+\tgit -C client config \"remote.cdn-1.advertisedAs\" >>actual &&\n+\ttest_cmp expect actual &&\n+\n+\t# Check that the original \"cdn\" remote was not overwritten.\n+\tprintf \"%s\\n\" \"https://example.com/cdn\" \"true\" >expect &&\n+\tgit -C client config \"remote.cdn.url\" >actual &&\n+\tgit -C client config \"remote.cdn.promisor\" >>actual &&\n+\ttest_cmp expect actual &&\n+\n+\t# Check that the largest object is still missing on the server\n+\tcheck_missing_objects server 1 \"$oid\"\n+'\n+\n+test_expect_success \"clone with URL whitelisted and reusable remote\" '\n+\tgit -C server config promisor.advertise true &&\n+\ttest_when_finished \"rm -rf client\" &&\n+\n+\tGIT_NO_LAZY_FETCH=0 git clone \\\n+\t\t-c remote.cdn.fetch=\"+refs/heads/*:refs/remotes/lop/*\" \\\n+\t\t-c remote.cdn.url=\"$PWD_URL/lop\" \\\n+\t\t-c promisor.acceptfromserver=None \\\n+\t\t-c promisor.acceptFromServerUrl=\"cdn=$ENCODED_PWD_URL/*\" \\\n+\t\t--no-local --filter=\"blob:limit=5k\" server client &&\n+\n+\t# Check that the existing \"cdn\" remote has been properly updated.\n+\tprintf \"%s\\n\" \"$PWD_URL/lop\" \"true\" \"lop\" \"+refs/heads/*:refs/remotes/lop/*\" >expect &&\n+\tgit -C client config \"remote.cdn.url\" >actual &&\n+\tgit -C client config \"remote.cdn.promisor\" >>actual &&\n+\tgit -C client config \"remote.cdn.advertisedAs\" >>actual &&\n+\tgit -C client config \"remote.cdn.fetch\" >>actual &&\n+\ttest_cmp expect actual &&\n+\n+\t# Check that no new \"cdn-1\" remote has been created.\n+\ttest_must_fail git -C client config \"remote.cdn-1.url\" &&\n+\n+\t# Check that the largest object is still missing on the server\n+\tcheck_missing_objects server 1 \"$oid\"\n+'\n+\n+test_expect_success \"clone with invalid promisor.acceptFromServerUrl\" '\n+\tgit -C server config promisor.advertise true &&\n+\ttest_when_finished \"rm -rf client\" &&\n+\n+\t# As \"bad name\" contains a space, which is not a valid remote name,\n+\t# the pattern should be rejected with a warning and no remote created.\n+\tGIT_NO_LAZY_FETCH=0 git clone \\\n+\t\t-c promisor.acceptfromserver=None \\\n+\t\t-c \"promisor.acceptFromServerUrl=bad name=https://example.com/*\" \\\n+\t\t--no-local --filter=\"blob:limit=5k\" server client 2>err &&\n+\n+\t# Check that a warning was emitted\n+\ttest_grep \"invalid remote name '\\''bad name'\\''\" err &&\n+\n+\t# Check that no remote was auto-created\n+\ttest_must_fail git -C client config --get-regexp \"remote\\..*\\.advertisedas\" &&\n+\n+\t# Check that the largest object is not missing on the server\n+\tcheck_missing_objects server 0 \"\" &&\n+\n+\t# Reinitialize server so that the largest object is missing again\n+\tinitialize_server 1 \"$oid\"\n+'\n+\n test_expect_success \"clone with promisor.sendFields\" '\n \tgit -C server config promisor.advertise true &&\n \ttest_when_finished \"rm -rf client\" &&\n-- \n2.53.0.625.g20f70b52bb\n\n"},{"id":"539773","messageId":"xmqqzf3y4bsg.fsf@gitster.g","threadId":"65335","inReplyTo":"20260323080520.887550-15-christian.couder@gmail.com","subject":"Re: [PATCH 14/16] promisor-remote: trust known remotes matching acceptFromServerUrl","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-03-23T18:54:07Z","receivedAt":"2026-03-23T18:54:11Z","isPatch":true,"body":"Christian Couder writes:\n\n> diff --git a/Documentation/config/promisor.adoc b/Documentation/config/promisor.adoc\n> index b0fa43b839..6f5442cd65 100644\n> --- a/Documentation/config/promisor.adoc\n> +++ b/Documentation/config/promisor.adoc\n> @@ -51,6 +51,52 @@ promisor.acceptFromServer::\n> \tto \"fetch\" and \"clone\" requests from the client. Name and URL\n> \tcomparisons are case sensitive. See linkgit:gitprotocol-v2[5].\n> \n> +promisor.acceptFromServerUrl::\n> +\tA glob pattern to specify which URLs advertised by a server\n> +\tare considered trusted by the client. This option acts as an\n> +\tadditive security whitelist that works in conjunction with\n> +\t`promisor.acceptFromServer`.\n\nBetween the first sentence and the second one, I think there needs\nto be an explanation on what \"trusted\" means in this context. Is it\ntrusted so that the URL can feed random configuration variable=value\npairs for the client to blindly apply? Or is it trusted to do very\nlimited things that other remotes can do, and if so what are these\nlimited things? Without knowing that, the end-users cannot assess\nthe security implications of setting this option.\n\nI am guessing that the client would behave as if the existing\npromisor.acceptFromServer configuration variable were set to \"all\"\nwhen talking with a remote whose URL matches one of the patterns\nlisted?\n\nBy the way, some people may suggest \"white\" -> \"allow\".\n\n\n> +1. Start with a secure protocol scheme, like `https://` or `ssh://`.\n\nIs there a practical reason why people would want to use schemes\nother than the above two? This sounds like something a small amount\nof code can easily enforce.\n\n> +2. Only allow domain names or paths where you control and trust _ALL_\n> + the content.\n\nObviously.\n\n> +3. Don't use globs (`*`) in the domain name. For example\n> + `https://cdn.example.com/*` is much safer than\n> + `https://*.example.com/*`, because the latter matches\n> + `https://evil-hacker.net/fake.example.com/repo`.\n\nIs there a practical use case where allowing '*' to match anything\nthat contains a slash '/' is useful?\n\n> +4. Make sure to have a `/` at the end of the domain name (or the end\n> + of specific directories). For example `https://cdn.example.com/*`\n> + is much safer than `https://cdn.example.com*`, because the latter\n> + matches `https://cdn.example.com.hacker.net/repo`.\n\nDitto. The above two points sound like excuses to keep sloppy\nasterisk matching logic. Yes, retroactively tightening rules always\nhave risk to break existing deployments, but if existing code paths\nof urlmatch do not have any good reason to allow '*' to match a\nstring that contains a slash '/', perhaps there is no fallout.\n\n"},{"id":"539792","messageId":"xmqqo6ke2jn4.fsf@gitster.g","threadId":"65335","inReplyTo":"xmqqzf3y4bsg.fsf@gitster.g","subject":"Re: [PATCH 14/16] promisor-remote: trust known remotes matching acceptFromServerUrl","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-03-23T23:47:27Z","receivedAt":"2026-03-23T23:47:30Z","isPatch":true,"body":"Junio C Hamano writes:\n\n> Obviously.\n>\n>> +3. Don't use globs (`*`) in the domain name. For example\n>> + `https://cdn.example.com/*` is much safer than\n>> + `https://*.example.com/*`, because the latter matches\n>> + `https://evil-hacker.net/fake.example.com/repo`.\n>\n> Is there a practical use case where allowing '*' to match anything\n> that contains a slash '/' is useful?\n>\n>> +4. Make sure to have a `/` at the end of the domain name (or the end\n>> + of specific directories). For example `https://cdn.example.com/*`\n>> + is much safer than `https://cdn.example.com*`, because the latter\n>> + matches `https://cdn.example.com.hacker.net/repo`.\n>\n> Ditto. The above two points sound like excuses to keep sloppy\n> asterisk matching logic. Yes, retroactively tightening rules always\n> have risk to break existing deployments, but if existing code paths\n> of urlmatch do not have any good reason to allow '*' to match a\n> string that contains a slash '/', perhaps there is no fallout.\n\nI probably should caution the readers not to take the above too\nliterally. Forbidding an asterisk '*' glob not to match '/'\nanywhere in urlmatch will obviusly break existing deployments that\ndoes this\n\n [http \"https://example.com/*\"]\n\tvar = val\n\nand expects it to catch any URL pointing into the site.\n\nBut I still do think the matcher should be more intelligent than the\ncurrent implementation to avoid pitfalls like #3 and #4 above.\n\nPerhaps if an additional rule says that '*' after the scheme:// part\nbefore the first '/' in the pattern, e.g.,\n\n https://*.example.com/\n https://*.example.com\n https://example.com*\n\nunlike '*' that appear anywhere else, never matches a substring that\ncontains a slash '/' in it, it would cover plausible mistakes that\nthe above #3 and #4 are trying to catch, without hurting any real\nworld use case?\n\n\n\n"},{"id":"540056","messageId":"acUklLd07f04wOYi@pks.im","threadId":"65335","inReplyTo":"20260323080520.887550-2-christian.couder@gmail.com","subject":"Re: [PATCH 01/16] promisor-remote: try accepted remotes before others in get_direct()","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-03-26T12:20:41Z","receivedAt":"2026-03-26T12:20:53Z","isPatch":true,"body":"On Mon, Mar 23, 2026 at 09:05:04AM +0100, Christian Couder wrote:\n> When a server advertises promisor remotes and the client accepts some\n> of them, those remotes carry the server's intent: 'fetch missing\n> objects preferably from here', and the client agrees with that for the\n> remotes it accepts.\n> \n> However promisor_remote_get_direct() actually iterates over all\n> promisor remotes in list order, which is the order they appear in the\n> config files (except perhaps for the one appearing in the\n> `extensions.partialClone` config variable which is tried last).\n> \n> This means an existing, but not accepted, promisor remote, could be\n> tried before the accepted ones, which does not reflect the intent of\n> the agreement between client and server.\n> \n> If the client doesn't care about what the server suggests, it should\n> accept nothing and rely on its remotes as they are already configured.\n> \n> To better reflect the agreement between client and server, let's make\n> promisor_remote_get_direct() try the accepted promisor remotes before\n> the non-accepted ones.\n\nInteresting, and it feels sensible to me. Is it documented anywhere that\nthe ordering of announced remotes is actually important?\n\n> Concretely, let's extract a try_promisor_remotes() helper and call it\n> twice from promisor_remote_get_direct():\n> \n> - first with an `accepted_only=true` argument to try only the accepted\n> remotes,\n> - then with `accepted_only=false` to fall back to any remaining remote.\n> \n> Ensuring that accepted remotes are preferred will be even more\n> important if in the future a mechanism is developed to allow the\n> client to auto-configure remotes that the server advertises. This will\n> in particular avoid fetching from the server (which is already\n> configured as a promisor remote) before trying the auto-configured\n> remotes, as these new remotes would likely appear at the end of the\n> config file, and as the server might not appear in the\n> `extensions.partialClone` config variable.\n\nNot quite sure I correctly understand this paragraph. Is the idea that\nin the future, we might not even store announced promisors in the config\nat all but simply use whatever the server announces on any given fetch?\n\n> diff --git a/promisor-remote.c b/promisor-remote.c\n> index 96fa215b06..3f8aeee787 100644\n> --- a/promisor-remote.c\n> +++ b/promisor-remote.c\n> @@ -268,11 +268,37 @@ static int remove_fetched_oids(struct repository *repo,\n> \treturn remaining_nr;\n> }\n> \n> +static int try_promisor_remotes(struct repository *repo,\n> +\t\t\t\tstruct object_id **remaining_oids,\n> +\t\t\t\tint *remaining_nr, int *to_free,\n> +\t\t\t\tbool accepted_only)\n> +{\n> +\tstruct promisor_remote *r = repo->promisor_remote_config->promisors;\n> +\n> +\tfor (; r; r = r->next) {\n> +\t\tif (accepted_only && !r->accepted)\n> +\t\t\tcontinue;\n> +\t\tif (!accepted_only && r->accepted)\n> +\t\t\tcontinue;\n\nThis can be simplified to `if (!accepted_only != !r->accepted)`.\n\nAlso, can we maybe add a test for this to verify that we use the correct\nordering now?\n\nPatrick\n"},{"id":"540057","messageId":"acUkpJjHgYs0jqX4@pks.im","threadId":"65335","inReplyTo":"20260323080520.887550-4-christian.couder@gmail.com","subject":"Re: [PATCH 03/16] urlmatch: add url_is_valid_pattern() helper","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-03-26T12:20:52Z","receivedAt":"2026-03-26T12:20:57Z","isPatch":true,"body":"On Mon, Mar 23, 2026 at 09:05:06AM +0100, Christian Couder wrote:\n> diff --git a/urlmatch.c b/urlmatch.c\n> index 989bc7eb8b..a8cb6c3bee 100644\n> --- a/urlmatch.c\n> +++ b/urlmatch.c\n> @@ -440,6 +440,18 @@ char *url_normalize(const char *url, struct url_info *out_info)\n> \treturn url_normalize_1(url, out_info, false);\n> }\n> \n> +bool url_is_valid_pattern(const char *url)\n> +{\n> +\tchar *normalized = url_normalize_1(url, NULL, true);\n> +\n> +\tif (normalized) {\n> +\t\tfree(normalized);\n> +\t\treturn true;\n> +\t}\n> +\n> +\treturn false;\n> +}\n\nWe could simplify this implementation to:\n\n\tbool url_is_valid_pattern(const char *url)\n\t{\n\t\tchar *normalized = url_normalize_1(url, NULL, true);\n\t\tfree(normalized);\n\t\treturn !!normalized;\n\t}\n\n> diff --git a/urlmatch.h b/urlmatch.h\n> index 5ba85cea13..4e01422a02 100644\n> --- a/urlmatch.h\n> +++ b/urlmatch.h\n> @@ -36,6 +36,17 @@ struct url_info {\n> \n> char *url_normalize(const char *, struct url_info *);\n> \n> +/*\n> + * Return 'true' if the string looks like a valid URL or a valid URL pattern\n> + * (allowing '*' globs), 'false' otherwise.\n> + *\n> + * This is NOT a URL validation function. Full URL validation is NOT\n> + * performed. Some invalid host names are passed through this function\n> + * undetected. However, most all other problems that make a URL invalid\n> + * will be detected (including a missing host for non file: URLs).\n> + */\n> +bool url_is_valid_pattern(const char *url);\n\nOkay, this comment is basically copied from `url_normalize_1()`, which\nmakes sense.\n\nPatrick\n"},{"id":"540058","messageId":"acUkqRUSyRUysf-I@pks.im","threadId":"65335","inReplyTo":"20260323080520.887550-5-christian.couder@gmail.com","subject":"Re: [PATCH 04/16] promisor-remote: clarify that a remote is ignored","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-03-26T12:20:57Z","receivedAt":"2026-03-26T12:21:02Z","isPatch":true,"body":"On Mon, Mar 23, 2026 at 09:05:07AM +0100, Christian Couder wrote:\n> In should_accept_remote() when a remote is ignored, we might tell users\n> why it is ignored in a warning, but we don't tell them that the remote\n> is actually ignored.\n> \n> Let's clarify that, so users have a better idea of what's actually\n> happening.\n\nInteresting that this doesn't result in any test changes. Don't we have\ntest coverage for `should_accept_remote()`?\n\n> diff --git a/promisor-remote.c b/promisor-remote.c\n> index 3f8aeee787..f5c4d41155 100644\n> --- a/promisor-remote.c\n> +++ b/promisor-remote.c\n> @@ -660,15 +660,16 @@ static int should_accept_remote(enum accept_promisor accept,\n> \t\tBUG(\"Unhandled 'enum accept_promisor' value '%d'\", accept);\n> \n> \tif (!remote_url || !*remote_url) {\n> -\t\twarning(_(\"no or empty URL advertised for remote '%s'\"), remote_name);\n> +\t\twarning(_(\"no or empty URL advertised for remote '%s', \"\n> +\t\t\t \"ignoring this remote\"), remote_name);\n> \t\treturn 0;\n> \t}\n> \n> \tif (!strcmp(p->url, remote_url))\n> \t\treturn all_fields_match(advertised, config_info, 0);\n> \n> -\twarning(_(\"known remote named '%s' but with URL '%s' instead of '%s'\"),\n> -\t\tremote_name, p->url, remote_url);\n> +\twarning(_(\"known remote named '%s' but with URL '%s' instead of '%s', \"\n> +\t\t \"ignoring this remote\"), remote_name, p->url, remote_url);\n> \n> \treturn 0;\n> }\n\nThe change itself seems sensible to me.\n\nPatrick\n"},{"id":"540059","messageId":"acUkr2xxQZSJU2lL@pks.im","threadId":"65335","inReplyTo":"20260323080520.887550-8-christian.couder@gmail.com","subject":"Re: [PATCH 07/16] promisor-remote: keep accepted promisor_info structs alive","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-03-26T12:21:03Z","receivedAt":"2026-03-26T12:21:08Z","isPatch":true,"body":"On Mon, Mar 23, 2026 at 09:05:10AM +0100, Christian Couder wrote:\n> diff --git a/promisor-remote.c b/promisor-remote.c\n> index 3116d14d14..34b4ca5806 100644\n> --- a/promisor-remote.c\n> +++ b/promisor-remote.c\n> @@ -912,17 +912,10 @@ static void filter_promisor_remote(struct repository *repo,\n> \t\t\tif (promisor_store_advertised_fields(advertised, store_info))\n> \t\t\t\treload_config = true;\n> \n> -\t\t\tstrvec_push(accepted, advertised->name);\n\nOkay, here we used to store the name of the accepted remote...\n\n> -\t\t\t/* Capture advertised filters for accepted remotes */\n> -\t\t\tif (advertised->filter) {\n> -\t\t\t\tstruct string_list_item *i;\n> -\t\t\t\ti = string_list_append(&accepted_filters, advertised->name);\n> -\t\t\t\ti->util = xstrdup(advertised->filter);\n> -\t\t\t}\n\n... and here we used to store the filter, if any.\n\n> +\t\t\tstring_list_append(&accepted_remotes, advertised->name)->util = advertised;\n\nInstead, we now store the whole remote, ...\n\n> +\t\t} else {\n> +\t\t\tpromisor_info_free(advertised);\n\n... unless we don't want to accept it, in which case we free it now.\nMakes sense.\n\n> @@ -932,24 +925,23 @@ static void filter_promisor_remote(struct repository *repo,\n> \tif (reload_config)\n> \t\trepo_promisor_remote_reinit(repo);\n> \n> -\t/* Apply accepted remote filters to the stable repo state */\n> -\tfor_each_string_list_item(item, &accepted_filters) {\n> -\t\tstruct promisor_remote *r = repo_promisor_remote_find(repo, item->string);\n> -\t\tif (r) {\n> -\t\t\tfree(r->advertised_filter);\n> -\t\t\tr->advertised_filter = item->util;\n> -\t\t\titem->util = NULL;\n> -\t\t}\n> -\t}\n> +\t/* Apply accepted remotes to the stable repo state */\n> +\tfor_each_string_list_item(item, &accepted_remotes) {\n> +\t\tstruct promisor_info *info = item->util;\n> +\t\tstruct promisor_remote *r = repo_promisor_remote_find(repo, info->name);\n\nAnd instead of iterating through the accepted filters we now iterate\nthrough the accepted remotes and take their respective filters from the\npromisor remote info.\n\n> -\tstring_list_clear(&accepted_filters, 1);\n> +\t\tstrvec_push(accepted, info->name);\n> \n> -\t/* Mark the remotes as accepted in the repository state */\n> -\tfor (size_t i = 0; i < accepted->nr; i++) {\n> -\t\tstruct promisor_remote *r = repo_promisor_remote_find(repo, accepted->v[i]);\n> -\t\tif (r)\n> +\t\tif (r) {\n> \t\t\tr->accepted = 1;\n> +\t\t\tif (info->filter) {\n> +\t\t\t\tfree(r->advertised_filter);\n> +\t\t\t\tr->advertised_filter = xstrdup(info->filter);\n> +\t\t\t}\n> +\t\t}\n> \t}\n> +\n> +\tpromisor_info_list_clear(&accepted_remotes);\n> }\n\nPatrick\n"},{"id":"540060","messageId":"acUks6pBmjgzN4M3@pks.im","threadId":"65335","inReplyTo":"20260323080520.887550-9-christian.couder@gmail.com","subject":"Re: [PATCH 08/16] promisor-remote: remove the 'accepted' strvec","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-03-26T12:21:07Z","receivedAt":"2026-03-26T12:21:12Z","isPatch":true,"body":"On Mon, Mar 23, 2026 at 09:05:11AM +0100, Christian Couder wrote:\n> In a previous commit, filter_promisor_remote() was refactored to keep\n> accepted 'struct promisor_info' instances alive instead of dismantling\n> them into separate parallel data structures.\n> \n> Let's go one step further and replace the 'struct strvec *accepted'\n> argument passed to filter_promisor_remote() with a\n> 'struct string_list *accepted_remotes' argument.\n\nRight, this is indeed something I was wondering about given that we now\neffectively stored the remote name twice.\n\nPatrick\n"},{"id":"540061","messageId":"acUkuD6iuq6nTeHn@pks.im","threadId":"65335","inReplyTo":"20260323080520.887550-10-christian.couder@gmail.com","subject":"Re: [PATCH 09/16] promisor-remote: add 'local_name' to 'struct promisor_info'","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-03-26T12:21:12Z","receivedAt":"2026-03-26T12:21:18Z","isPatch":true,"body":"On Mon, Mar 23, 2026 at 09:05:12AM +0100, Christian Couder wrote:\n> In a following commit, we will store promisor remote information under\n> a remote name different than the one the server advertised.\n> \n> To prepare for this change, let's add a new 'char* local_name' member\n\nMicronit: s/char* local_name/char *local_name/\n\n> diff --git a/promisor-remote.c b/promisor-remote.c\n> index bdfc5e7608..da347fa2dc 100644\n> --- a/promisor-remote.c\n> +++ b/promisor-remote.c\n> @@ -434,15 +434,19 @@ static struct string_list *fields_stored(void)\n> \n> /*\n> * Struct for promisor remotes involved in the \"promisor-remote\"\n> - * protocol capability.\n> + * protocol capability:\n> *\n> - * Except for \"name\", each in this struct and its \n> - * should correspond (either on the client side or on the server side)\n> - * to a \"remote..\" config variable set to where\n> - * \"\" is a promisor remote name.\n> + * - \"name\" is the name the server advertised.\n> + * - \"local_name\" is the name we use locally (may be auto-generated).\n> + *\n> + * Except for \"name\" and \"local_name\", each in this struct\n> + * and its should correspond (either on the client side or on\n> + * the server side) to a \"remote..\" config variable set\n> + * to where \"\" is a promisor remote name.\n> */\n> struct promisor_info {\n> \tconst char *name;\n> +\tconst char *local_name;\n> \tconst char *url;\n> \tconst char *filter;\n> \tconst char *token;\n\nI think it would be easier to follow if the struct-level comment applied\nto the general description of the struct, and individual members would\nthen have their own comments describing their intent.\n\n> @@ -464,6 +469,11 @@ static void promisor_info_list_clear(struct string_list *list)\n> \tstring_list_clear(list, 0);\n> }\n> \n> +static const char *promisor_info_internal_name(struct promisor_info *p)\n> +{\n> +\treturn p->local_name ? p->local_name : p->name;\n> +}\n> +\n> static void set_one_field(struct promisor_info *p,\n> \t\t\t const char *field, const char *value)\n> {\n> @@ -819,7 +829,7 @@ static bool promisor_store_advertised_fields(struct promisor_info *advertised,\n> {\n> \tstruct promisor_info *p;\n> \tstruct string_list_item *item;\n> -\tconst char *remote_name = advertised->name;\n> +\tconst char *remote_name = promisor_info_internal_name(advertised);\n> \tbool reload_config = false;\n> \n> \tif (!(store_info->store_filter || store_info->store_token))\n> @@ -927,7 +937,8 @@ static void filter_promisor_remote(struct repository *repo,\n> \t/* Apply accepted remotes to the stable repo state */\n> \tfor_each_string_list_item(item, accepted_remotes) {\n> \t\tstruct promisor_info *info = item->util;\n> -\t\tstruct promisor_remote *r = repo_promisor_remote_find(repo, info->name);\n> +\t\tconst char *local = promisor_info_internal_name(info);\n> +\t\tstruct promisor_remote *r = repo_promisor_remote_find(repo, local);\n> \n> \t\tif (r) {\n> \t\t\tr->accepted = 1;\n\nOkay. These hunks are essentially a no-op for now given that we don't\nyet store a local name.\n\nPatrick\n"},{"id":"540062","messageId":"acUkvkLYiO0wkCfm@pks.im","threadId":"65335","inReplyTo":"20260323080520.887550-11-christian.couder@gmail.com","subject":"Re: [PATCH 10/16] promisor-remote: pass config entry to all_fields_match() directly","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-03-26T12:21:18Z","receivedAt":"2026-03-26T12:21:24Z","isPatch":true,"body":"On Mon, Mar 23, 2026 at 09:05:13AM +0100, Christian Couder wrote:\n> The `in_list == 0` path of all_fields_match() re-looks up the\n\nThis reads a bit weird. How about \"looks up the remote in config_info by\nadvertised->name repeatedly\" instead?\n\n> diff --git a/promisor-remote.c b/promisor-remote.c\n> index da347fa2dc..8f2c1280c3 100644\n> --- a/promisor-remote.c\n> +++ b/promisor-remote.c\n> @@ -619,7 +627,11 @@ static int all_fields_match(struct promisor_info *advertised,\n> \t\tif (!value)\n> \t\t\treturn 0;\n> \n> -\t\tif (in_list) {\n> +\t\tif (config_entry) {\n> +\t\t\tmatch = match_field_against_config(field, value,\n> +\t\t\t\t\t\t\t config_entry);\n> +\t\t} else {\n> +\t\t\tstruct string_list_item *item;\n> \t\t\tfor_each_string_list_item(item, config_info) {\n> \t\t\t\tstruct promisor_info *p = item->util;\n> \t\t\t\tif (match_field_against_config(field, value, p)) {\n> @@ -627,12 +639,6 @@ static int all_fields_match(struct promisor_info *advertised,\n> \t\t\t\t\tbreak;\n> \t\t\t\t}\n> \t\t\t}\n> -\t\t} else {\n> -\t\t\titem = string_list_lookup(config_info, advertised->name);\n> -\t\t\tif (item) {\n> -\t\t\t\tstruct promisor_info *p = item->util;\n> -\t\t\t\tmatch = match_field_against_config(field, value, p);\n> -\t\t\t}\n> \t\t}\n> \n> \t\tif (!match)\n\nOkay, the logic is reversed now, which makes sense as we now pass `NULL`\ninstead of `1`, and the promisor info instead of `0`.\n\nThe change itself makes sense, but other than that I have a very hard\ntime understanding these two functions. I think they would strongly\nbenefit from some comments explaining what's going on, what the input is\nand what we're trying to do. Of course that doesn't have to be part of\nthis commit here, but I would appreciate a preparatory commit that helps\nguide the reader a bit.\n\nPatrick\n"},{"id":"540063","messageId":"acUkwrm1rN4l4qgP@pks.im","threadId":"65335","inReplyTo":"20260323080520.887550-12-christian.couder@gmail.com","subject":"Re: [PATCH 11/16] promisor-remote: refactor should_accept_remote() control flow","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-03-26T12:21:22Z","receivedAt":"2026-03-26T12:21:28Z","isPatch":true,"body":"On Mon, Mar 23, 2026 at 09:05:14AM +0100, Christian Couder wrote:\n> diff --git a/promisor-remote.c b/promisor-remote.c\n> index 8f2c1280c3..c2f0eb7223 100644\n> --- a/promisor-remote.c\n> +++ b/promisor-remote.c\n> @@ -665,6 +665,12 @@ static int should_accept_remote(enum accept_promisor accept,\n> \tconst char *remote_name = advertised->name;\n> \tconst char *remote_url = advertised->url;\n> \n> +\tif (!remote_url || !*remote_url) {\n> +\t\twarning(_(\"no or empty URL advertised for remote '%s', \"\n> +\t\t\t \"ignoring this remote\"), remote_name);\n> +\t\treturn 0;\n> +\t}\n> +\n> \tif (accept == ACCEPT_ALL)\n> \t\treturn all_fields_match(advertised, config_info, NULL);\n> \n\nYou mention that it shouldn't change behaviour in well-defined cases\nwhere the remote sends non-empty URLs. But does it change behaviour in\nill-defined cases where the remote sends empty ones?\n\nIn other words, does this fix a bug that can be hit in the real world?\n\nPatrick\n"},{"id":"540064","messageId":"acUkx8KwusUzYqne@pks.im","threadId":"65335","inReplyTo":"20260323080520.887550-13-christian.couder@gmail.com","subject":"Re: [PATCH 12/16] t5710: use proper file:// URIs for absolute paths","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-03-26T12:21:27Z","receivedAt":"2026-03-26T12:21:33Z","isPatch":true,"body":"On Mon, Mar 23, 2026 at 09:05:15AM +0100, Christian Couder wrote:\n> diff --git a/t/t5710-promisor-remote-capability.sh b/t/t5710-promisor-remote-capability.sh\n> index 357822c01a..c7a484228f 100755\n> --- a/t/t5710-promisor-remote-capability.sh\n> +++ b/t/t5710-promisor-remote-capability.sh\n> @@ -76,6 +76,17 @@ copy_to_lop () {\n> \tcp \"$path\" \"$path2\"\n> }\n> \n> +# On Windows, 'pwd' returns a path like 'D:/foo/bar'. Prepend '/' to turn\n> +# it into '/D:/foo/bar', which is what git expects in file:// URLs on Windows.\n> +# On Unix, the path already starts with '/', so this is a no-op.\n> +pwd_path=$(pwd)\n> +case \"$pwd_path\" in\n> +[a-zA-Z]:*) pwd_path=\"/$pwd_path\" ;;\n> +esac\n> +PWD_URL=\"file://$pwd_path\"\n> +# Same as PWD_URL but with spaces percent-encoded, for use in URL patterns.\n> +ENCODED_PWD_URL=\"file://$(echo \"$pwd_path\" | sed \"s/ /%20/g\")\"\n\nCan't we do this unconditionally on all platforms? \"file:////foobar\"\nshould be valid on Unix systems, too, shouldn't it?\n\nPatrick\n"},{"id":"540065","messageId":"acUkzY7f5302uWD8@pks.im","threadId":"65335","inReplyTo":"20260323080520.887550-14-christian.couder@gmail.com","subject":"Re: [PATCH 13/16] promisor-remote: introduce promisor.acceptFromServerUrl","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-03-26T12:21:33Z","receivedAt":"2026-03-26T12:21:38Z","isPatch":true,"body":"On Mon, Mar 23, 2026 at 09:05:16AM +0100, Christian Couder wrote:\n> diff --git a/promisor-remote.c b/promisor-remote.c\n> index c2f0eb7223..4cb18e1a6a 100644\n> --- a/promisor-remote.c\n> +++ b/promisor-remote.c\n[snip]\n> +static struct string_list *accept_from_server_url(struct repository *repo)\n> +{\n> +\tstatic struct string_list accept_urls = STRING_LIST_INIT_DUP;\n> +\tstatic int initialized;\n> +\tconst struct string_list *config_urls;\n> +\n> +\tif (initialized)\n> +\t\treturn &accept_urls;\n> +\n> +\tinitialized = 1;\n> +\n> +\tif (!repo_config_get_string_multi(repo, \"promisor.acceptfromserverurl\", &config_urls)) {\n> +\t\tstruct string_list_item *item;\n> +\n> +\t\tfor_each_string_list_item(item, config_urls) {\n> +\t\t\tstruct allowed_url *allowed = valid_accept_url(item->string);\n> +\t\t\tif (allowed) {\n> +\t\t\t\tstruct string_list_item *new;\n> +\t\t\t\tnew = string_list_append(&accept_urls, item->string);\n> +\t\t\t\tnew->util = allowed;\n> +\t\t\t}\n> +\t\t}\n> +\t}\n> +\n> +\treturn &accept_urls;\n> +}\n\nI'm still not much of a fan of us getting more and more function-local\nstatic variables. It just feels wrong to me, and like we're accruing\ntechnical debt. I also doubt that the performance overhead of storing\nthis on the stack with proper lifecycle management will matter at all\ngiven that we're in a context where we talk with a remote anyway. The\nhandful of allocations really shouldn't matter in that context.\n\nPatrick\n"},{"id":"540066","messageId":"acUk0vTuj8COlvgf@pks.im","threadId":"65335","inReplyTo":"20260323080520.887550-15-christian.couder@gmail.com","subject":"Re: [PATCH 14/16] promisor-remote: trust known remotes matching acceptFromServerUrl","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-03-26T12:21:38Z","receivedAt":"2026-03-26T12:21:42Z","isPatch":true,"body":"On Mon, Mar 23, 2026 at 09:05:17AM +0100, Christian Couder wrote:\n> A previous commit introduced the `promisor.acceptFromServerUrl` config\n> variable along with the machinery to parse and validate the URL glob\n> patterns and optional remote name prefixes it contains. However, these\n> URL patterns are not yet tied into the client's acceptance logic.\n> \n> When a promisor remote is already configured locally, its fields (like\n> authentication tokens) may occasionally need to be refreshed by the\n> server. If `promisor.acceptFromServer` is set to the secure default\n> (\"None\"), these updates are rejected, potentially causing future\n> fetches to fail.\n\nThis only talks about already-configured remotes, and so I was naturally\nwondering what about not-yet-configured remotes. But looking ahead a bit\nshows that this is implemented in the next commit. Good.\n\n> To enable such targeted updates for trusted URLs, let's use the URL\n> patterns from `promisor.acceptFromServerUrl` as an additional URL\n> based whitelist.\n> \n> Concretely, let's check the advertised URLs against the URL glob\n> patterns by introducing a new small helper function called\n> url_matches_accept_list(), which iterates over the glob patterns and\n> returns the first matching allowed_url entry (or NULL).\n> \n> (Before matching, the advertised URL is passed through url_normalize()\n> so that case variations in the scheme/host, percent-encoding tricks,\n> and \"..\" path segments cannot bypass the whitelist.)\n> \n> Let's then use this helper at the tail of should_accept_remote() so\n> that, when `accept == ACCEPT_NONE`, a known remote whose URL matches\n> the whitelist is still accepted.\n> \n> To prepare for this new logic, let's also:\n> \n> - Add an 'accept_urls' parameter to should_accept_remote().\n> \n> - Replace the BUG() guard in the ACCEPT_KNOWN_URL case with an\n> explicit 'if (accept == ACCEPT_KNOWN_URL) return' and a new\n> BUG() guard in the ACCEPT_NONE case, so url_matches_accept_list()\n> is only called in the ACCEPT_NONE case.\n> \n> - Call accept_from_server_url() from filter_promisor_remote()\n> and relax its early return so that the function is entered when\n> `accept_urls` has entries even if `accept == ACCEPT_NONE`.\n> \n> Let's then properly document `promisor.acceptFromServerUrl` in\n> \"promisor.adoc\" as an additive security whitelist for known remotes,\n> including the URL normalization behavior, and let's mention it in\n> \"gitprotocol-v2.adoc\".\n\nI feel like the description is steering a bit too strongly into the\ndirection of a step-by-step instruction of how to implement the change\nrather than explaining what's done and why it's done this way.\n\n> Signed-off-by: Christian Couder \n> ---\n> Documentation/config/promisor.adoc | 46 +++++++++++++++++\n> Documentation/gitprotocol-v2.adoc | 9 ++--\n> promisor-remote.c | 50 +++++++++++++++---\n> t/t5710-promisor-remote-capability.sh | 73 +++++++++++++++++++++++++++\n> 4 files changed, 166 insertions(+), 12 deletions(-)\n> \n> diff --git a/Documentation/config/promisor.adoc b/Documentation/config/promisor.adoc\n> index b0fa43b839..6f5442cd65 100644\n> --- a/Documentation/config/promisor.adoc\n> +++ b/Documentation/config/promisor.adoc\n> @@ -51,6 +51,52 @@ promisor.acceptFromServer::\n> \tto \"fetch\" and \"clone\" requests from the client. Name and URL\n> \tcomparisons are case sensitive. See linkgit:gitprotocol-v2[5].\n> \n> +promisor.acceptFromServerUrl::\n> +\tA glob pattern to specify which URLs advertised by a server\n> +\tare considered trusted by the client. This option acts as an\n> +\tadditive security whitelist that works in conjunction with\n> +\t`promisor.acceptFromServer`.\n> ++\n> +This option can appear multiple times in config files. An advertised\n> +URL will be accepted if it matches _ANY_ glob pattern specified by\n> +this option in _ANY_ config file read by Git.\n> ++\n> +Be _VERY_ careful with these glob patterns, as it can be a big\n> +security hole to allow any advertised remote to be auto-configured!\n> +To minimize security risks, follow these guidelines:\n> ++\n> +1. Start with a secure protocol scheme, like `https://` or `ssh://`.\n> ++\n> +2. Only allow domain names or paths where you control and trust _ALL_\n> + the content. Be especially careful with shared hosting platforms\n> + like `github.com` or `gitlab.com`. A broad pattern like\n> + `https://gitlab.com/*` is dangerous because it trusts every\n> + repository on the entire platform. Always restrict such patterns to\n> + your specific organization or namespace (e.g.,\n> + `https://gitlab.com/your-org/*`).\n> ++\n> +3. Don't use globs (`*`) in the domain name. For example\n> + `https://cdn.example.com/*` is much safer than\n> + `https://*.example.com/*`, because the latter matches\n> + `https://evil-hacker.net/fake.example.com/repo`.\n> ++\n> +4. Make sure to have a `/` at the end of the domain name (or the end\n> + of specific directories). For example `https://cdn.example.com/*`\n> + is much safer than `https://cdn.example.com*`, because the latter\n> + matches `https://cdn.example.com.hacker.net/repo`.\n> ++\n> +Before matching, the advertised URL is normalized: the scheme and\n> +host are lowercased, percent-encoded characters are decoded where\n> +possible, and path segments like `..` are resolved. Glob patterns\n> +are matched against this normalized URL as-is, so patterns should\n> +be written in normalized form (e.g., lowercase scheme and host).\n> ++\n> +Even if `promisor.acceptFromServer` is set to `None` (the default),\n> +Git will still accept field updates (like tokens) for known remotes,\n> +provided their URLs match a pattern in\n> +`promisor.acceptFromServerUrl`. See linkgit:gitprotocol-v2[5] for\n> +details on the protocol.\n\nGiven that there's a bunch to process here, would it make sense to give\nusers an example for how to do it properly?\n\nI also wonder why we require a new config entry instead of extending\n`promisor.acceptFromRemote` to have for example a new \"url:https://...\"\nsetting. Are there cases where you would ever want to use the new\nURL-based schema with a different setting than \"all\"?\n\nI guess the case with \"none\" is exactly that, where you may auto-update\nconfigured remotes. But I wonder whether it would be more sensible to\nsplit out behaviour of accepting and updating promisors into separate\nconfiguration variables. These are ultimately different concerns, and\nthe interaction as layed out in this commit is somewhat non-obvious to\nme.\n\nPatrick\n"},{"id":"540067","messageId":"acUk11mt06GJZaur@pks.im","threadId":"65335","inReplyTo":"20260323080520.887550-16-christian.couder@gmail.com","subject":"Re: [PATCH 15/16] promisor-remote: auto-configure unknown remotes","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-03-26T12:21:43Z","receivedAt":"2026-03-26T12:21:48Z","isPatch":true,"body":"On Mon, Mar 23, 2026 at 09:05:18AM +0100, Christian Couder wrote:\n> Previous commits have introduced the `promisor.acceptFromServerUrl`\n> config variable to whitelist some URLs advertised by a server through\n> the \"promisor-remote\" protocol capability.\n> \n> However the new `promisor.acceptFromServerUrl` mechanism, like the old\n> `promisor.acceptFromServer` mechanism, still requires a remote to\n> already exist in the client's local configuration before it can be\n> accepted. This places a significant manual burden on users to\n> pre-configure these remotes, and creates friction for administrators\n> who have to troubleshoot or manually provision these setups for their\n> teams.\n> \n> To eliminate this burden, let's automatically create a new `[remote]`\n> section in the client's config when a server advertises an unknown\n> remote whose URL matches a `promisor.acceptFromServerUrl` glob pattern.\n\nWould it make sense to extend git-clone(1) to have a command line option\nthat basically does this as a one-shot? Something like `git clone\n--accept-promisors=url:https://gitlab.com/*`? I assume that many users\nmay not want to keep on updating their configured promisors all the\ntime.\n\nFurthermore, this here reconfirms my thought on the previous commit that\nit would make sense to detangle accepting promisors, storing them in the\nconfiguration and updating them automatically. These are all different\nthings:\n\n - Accepting promisors is basically an ongoing runtime thing where you\n start to use announced promisors even though they are not configured\n at all.\n\n - Storing promisors is typically a one-time thing that you'd want to\n do when creating a new repository.\n\n - Updating promisors automatically is probably something you want to\n do on an ongoing basis when you have stored promisors.\n\nWe're currently putting all of these use cases into the same bag, but\nthey have very different characteristics. I guess the most common use\ncase will eventually be to never auto-accept promisors, store them at\nclone time, and keep them updated whenever they change. This cannot be\nexpressed with \"promisor.acceptFromServerUrl\" as far as I understand.\n\nSo ideally, we'd have:\n\n - \"acceptFromServer\" to configure ongoing runtime behaviour.\n\n - \"storeFromServer\" and a one-time command-line option for\n git-clone(1) to configure which remotes to persist.\n\n - \"updateFromServer\" to update stored promisors.\n\nPatrick\n"},{"id":"540068","messageId":"acUk3EAcL8-xM4VK@pks.im","threadId":"65335","inReplyTo":"20260323080520.887550-1-christian.couder@gmail.com","subject":"Re: [PATCH 00/16] Auto-configure advertised remotes via URL whitelist","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-03-26T12:21:48Z","receivedAt":"2026-03-26T12:21:53Z","isPatch":true,"body":"On Mon, Mar 23, 2026 at 09:05:03AM +0100, Christian Couder wrote:\n> High level description of the patches\n> =====================================\n> \n> - Patch 1/16 (\"promisor-remote: try accepted remotes before others in\n> get_direct()\"):\n> \n> Fixes promisor_remote_get_direct() to prioritize accepted\n> remotes. This could be a separate fix, but is needed towards the\n> end of the series.\n> \n> - Patches 2-3/16 (\"urlmatch:*\"):\n> \n> Exposes and adapts helpers in the urlmatch API.\n> \n> - Patches 4-11/16 (\"promisor-remote:*\"):\n> \n> Big refactoring of filter_promisor_remote() and\n> should_accept_remote(). This keeps `struct promisor_info` instances\n> alive longer to anticipate possible state-desync bugs, decouples\n> the server's advertised name from the local config name, and\n> sanitizes control flow without changing the existing behavior.\n> \n> - Patch 12/16 (\"t5710:*\"):\n> \n> Cleans up how \"file://\" URIs are managed in the test script to\n> prepare for URI normalization later in the series and avoid issues\n> on Windows.\n> \n> - Patches 13-15/16 (\"promisor-remote:*\"):\n> \n> The core feature. Introduces the parsing machinery, adds the\n> additive whitelist for known remotes (with url_normalize()\n> security), and finally implements the auto-creation and collision\n> resolution for unknown remotes.\n> \n> - Patch 16/16 (\"doc: promisor: improve acceptFromServer entry\"):\n> \n> Cleans up and modernizes the existing `promisor.acceptFromServer`\n> documentation.\n\nI wonder whether it would make sense to split up this series into two.\nThe first 12 patches and parts of 16 are all sensible improvements that\ncan land independent of the patches that introduce the new logic. And\ngiven that I expect some discussion around the new logic itself, I\nexpect that these refactorings can land way faster on their own.\n\nIt would also help reduce the review load a bit if one then ultimately\nonly has to review three patches for the new feature.\n\nPatrick\n"},{"id":"540176","messageId":"CAP8UFD2vAK_khTkJMP4QBfhYA5iYVW5sfB3i-vnzhf71BvwQ=w@mail.gmail.com","threadId":"65335","inReplyTo":"xmqqzf3y4bsg.fsf@gitster.g","subject":"Re: [PATCH 14/16] promisor-remote: trust known remotes matching acceptFromServerUrl","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2026-03-27T12:17:59Z","receivedAt":"2026-03-27T12:18:11Z","isPatch":true,"body":"On Mon, Mar 23, 2026 at 7:54 PM Junio C Hamano wrote:\n>\n> Christian Couder writes:\n>\n> > diff --git a/Documentation/config/promisor.adoc b/Documentation/config/promisor.adoc\n> > index b0fa43b839..6f5442cd65 100644\n> > --- a/Documentation/config/promisor.adoc\n> > +++ b/Documentation/config/promisor.adoc\n> > @@ -51,6 +51,52 @@ promisor.acceptFromServer::\n> > to \"fetch\" and \"clone\" requests from the client. Name and URL\n> > comparisons are case sensitive. See linkgit:gitprotocol-v2[5].\n> >\n> > +promisor.acceptFromServerUrl::\n> > + A glob pattern to specify which URLs advertised by a server\n> > + are considered trusted by the client. This option acts as an\n> > + additive security whitelist that works in conjunction with\n> > + `promisor.acceptFromServer`.\n>\n> Between the first sentence and the second one, I think there needs\n> to be an explanation on what \"trusted\" means in this context. Is it\n> trusted so that the URL can feed random configuration variable=value\n> pairs for the client to blindly apply? Or is it trusted to do very\n> limited things that other remotes can do, and if so what are these\n> limited things? Without knowing that, the end-users cannot assess\n> the security implications of setting this option.\n\nYeah, in the current version, the following is used, which is more explicit:\n\n A glob pattern to specify which server-advertised URLs a\n client is allowed to act on. When a URL matches, the client\n will accept the advertised remote as a promisor remote and may\n automatically accept field updates (such as authentication\n tokens) from the server, even if `promisor.acceptFromServer`\n is set to `none` (the default).\n\n> I am guessing that the client would behave as if the existing\n> promisor.acceptFromServer configuration variable were set to \"all\"\n> when talking with a remote whose URL matches one of the patterns\n> listed?\n\nYes, that's the idea.\n\n> By the way, some people may suggest \"white\" -> \"allow\".\n\nRight, I have changed all the \"whitelist\" instances with \"allowlist\".\n\n> > +1. Start with a secure protocol scheme, like `https://` or `ssh://`.\n>\n> Is there a practical reason why people would want to use schemes\n> other than the above two? This sounds like something a small amount\n> of code can easily enforce.\n\nThe main issue is that some remote helper schemes might be secure,\nwhile it might be difficult to maintain a hardcoded allowlist of them.\n(Different implementations might exist out there with the same scheme\nname but different security properties.)\n\nAlso file:// and http:// for example might be OK in some corporate setups.\n\nWe could add yet another config variable (or env variable) for an\nallowlist of schemes (on top of `https://` and `ssh://` which would be\nthe only ones accepted by default), but maybe we can do that in a\nfuture patch series.\n\n[...]\n\n> >> +3. Don't use globs (`*`) in the domain name. For example\n> >> + `https://cdn.example.com/*` is much safer than\n> >> + `https://*.example.com/*`, because the latter matches\n> >> + `https://evil-hacker.net/fake.example.com/repo`.\n> >\n> > Is there a practical use case where allowing '*' to match anything\n> > that contains a slash '/' is useful?\n> >\n> >> +4. Make sure to have a `/` at the end of the domain name (or the end\n> >> + of specific directories). For example `https://cdn.example.com/*`\n> >> + is much safer than `https://cdn.example.com*`, because the latter\n> >> + matches `https://cdn.example.com.hacker.net/repo`.\n> >\n> > Ditto. The above two points sound like excuses to keep sloppy\n> > asterisk matching logic. Yes, retroactively tightening rules always\n> > have risk to break existing deployments, but if existing code paths\n> > of urlmatch do not have any good reason to allow '*' to match a\n> > string that contains a slash '/', perhaps there is no fallout.\n>\n> I probably should caution the readers not to take the above too\n> literally. Forbidding an asterisk '*' glob not to match '/'\n> anywhere in urlmatch will obviusly break existing deployments that\n> does this\n>\n> [http \"https://example.com/*\"]\n> var = val\n>\n> and expects it to catch any URL pointing into the site.\n\nYeah, the main reason for allowing an asterisk '*' glob to match '/'\nis to allow something like:\n\ngit config set --global promisor.acceptFromServerUrl \"https://my-org.com/*\"\n\nto be all what is needed for most internal work in many random orgs.\n\n> But I still do think the matcher should be more intelligent than the\n> current implementation to avoid pitfalls like #3 and #4 above.\n>\n> Perhaps if an additional rule says that '*' after the scheme:// part\n> before the first '/' in the pattern, e.g.,\n>\n> https://*.example.com/\n> https://*.example.com\n> https://example.com*\n>\n> unlike '*' that appear anywhere else, never matches a substring that\n> contains a slash '/' in it, it would cover plausible mistakes that\n> the above #3 and #4 are trying to catch, without hurting any real\n> world use case?\n\nI agree that it is better security wise, so I have implemented it in\nthe current version. Now the scheme and port parts must match exactly\nwhile * match any sequence of characters within the host and path\nparts but cannot cross part boundaries.\n\nIt's not a panacea though. Users still have to be very careful.\n\nThe current documentation looks like this:\n\n--------------------\n\npromisor.acceptFromServerUrl::\n A glob pattern to specify which server-advertised URLs a\n client is allowed to act on. When a URL matches, the client\n will accept the advertised remote as a promisor remote and may\n automatically accept field updates (such as authentication\n tokens) from the server, even if `promisor.acceptFromServer`\n is set to `none` (the default).\n+\nThis option can appear multiple times in config files. An advertised\nURL will be accepted if it matches _ANY_ glob pattern specified by\nthis option in _ANY_ config file read by Git.\n+\nBe _VERY_ careful with these patterns: `*` matches any sequence of\ncharacters within the 'host' and 'path' parts of a URL (but cannot\ncross part boundaries). An overly broad pattern is a major security\nrisk, as a matching URL allows a server to update fields (such as\nauthentication tokens) on known remotes without further confirmation.\nTo minimize security risks, follow these guidelines:\n+\n1. Start with a secure protocol scheme, like `https://` or `ssh://`.\n+\n2. Only allow domain names or paths where you control and trust _ALL_\n the content. Be especially careful with shared hosting platforms\n like `github.com` or `gitlab.com`. A broad pattern like\n `https://gitlab.com/*` is dangerous because it trusts every\n repository on the entire platform. Always restrict such patterns to\n your specific organization or namespace (e.g.,\n `https://gitlab.com/your-org/*`).\n+\n3. Never use globs at the end of domain names. For example,\n `https://cdn.your-org.com/*` might be safe, but\n `https://cdn.your-org.com*/*` is a major security risk because\n the latter matches `https://cdn.your-org.com.hacker.net/repo`.\n+\n4. Be careful using globs at the beginning of domain names. While the\n code ensures a `*` in the host cannot cross into the path, a\n pattern like `https://*.example.com/*` will still match any\n subdomain. This is extremely dangerous on shared hosting platforms\n (e.g., `https://*.github.io/*` trusts every user's site on the\n entire platform).\n+\nBefore matching, both the advertised URL and the pattern are\nnormalized: the scheme and host are lowercased, percent-encoded\ncharacters are decoded where possible, and path segments like `..`\nare resolved. The port must also match exactly (e.g.,\n`https://example.com:8080/*` will not match a URL advertised on\nport 9999).\n+\nFor the security implications of accepting a promisor remote, see the\ndocumentation of `promisor.acceptFromServer`. For details on the\nprotocol, see linkgit:gitprotocol-v2[5].\n\n--------------------\n\nI will work on the suggestions Patrick made before sending the v2\n(which I may split into 2 patch series as Patrick suggested).\n\nThanks.\n"},{"id":"540569","messageId":"xmqq341fy7v4.fsf@gitster.g","threadId":"65335","inReplyTo":"CAP8UFD2vAK_khTkJMP4QBfhYA5iYVW5sfB3i-vnzhf71BvwQ=w@mail.gmail.com","subject":"Re: [PATCH 14/16] promisor-remote: trust known remotes matching acceptFromServerUrl","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-03-31T22:03:27Z","receivedAt":"2026-03-31T22:03:30Z","isPatch":true,"body":"Christian Couder writes:\n\n>> Between the first sentence and the second one, I think there needs\n>> to be an explanation on what \"trusted\" means in this context. Is it\n>> trusted so that the URL can feed random configuration variable=value\n>> pairs for the client to blindly apply? Or is it trusted to do very\n>> limited things that other remotes can do, and if so what are these\n>> limited things? Without knowing that, the end-users cannot assess\n>> the security implications of setting this option.\n>\n> Yeah, in the current version, the following is used, which is more explicit:\n> ...\n\nDo you mean by \"the current version\", the one you are preparing as\nan updated iteration?\n\nIf so, let me mark the topic to be expecting a reroll. From the\nreviews by Patrick, I am not sure if I should also add the usual\n\"(hopefully small and final)\" in this case, not just yet, though.\n\nThanks.\n"},{"id":"540617","messageId":"CAP8UFD2HsfNGX6LrthBX0SqXUpgwiGyT3R2X1zwHN9SribAqgw@mail.gmail.com","threadId":"65335","inReplyTo":"xmqq341fy7v4.fsf@gitster.g","subject":"Re: [PATCH 14/16] promisor-remote: trust known remotes matching acceptFromServerUrl","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2026-04-01T05:41:36Z","receivedAt":"2026-04-01T05:41:49Z","isPatch":true,"body":"On Wed, Apr 1, 2026 at 12:03 AM Junio C Hamano wrote:\n>\n> Christian Couder writes:\n>\n> >> Between the first sentence and the second one, I think there needs\n> >> to be an explanation on what \"trusted\" means in this context. Is it\n> >> trusted so that the URL can feed random configuration variable=value\n> >> pairs for the client to blindly apply? Or is it trusted to do very\n> >> limited things that other remotes can do, and if so what are these\n> >> limited things? Without knowing that, the end-users cannot assess\n> >> the security implications of setting this option.\n> >\n> > Yeah, in the current version, the following is used, which is more explicit:\n> > ...\n>\n> Do you mean by \"the current version\", the one you are preparing as\n> an updated iteration?\n\nYes, but I am going to split the series as Patrick suggested into:\n\n1) a preparatory series which adds fixes, refactorings and cleanups,\n2) a series which adds the new features related to the new\n`acceptFromServerUrl` config variable.\n\nI will send 1) soon, and 2) later when it looks like 1) has graduated\nor will graduate soon.\n\n> If so, let me mark the topic to be expecting a reroll. From the\n> reviews by Patrick, I am not sure if I should also add the usual\n> \"(hopefully small and final)\" in this case, not just yet, though.\n\nYou can also just drop it or mark it as superseded by 1) when I have sent it.\n\nThanks.\n"},{"id":"540706","messageId":"CAP8UFD1C-+0Nwpj-2ZVvpHVj91ogj3s8dHKz_6MoHR0VJeSXxQ@mail.gmail.com","threadId":"65335","inReplyTo":"acUklLd07f04wOYi@pks.im","subject":"Re: [PATCH 01/16] promisor-remote: try accepted remotes before others in get_direct()","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2026-04-02T06:34:59Z","receivedAt":"2026-04-02T06:35:11Z","isPatch":true,"body":"On Thu, Mar 26, 2026 at 1:20 PM Patrick Steinhardt wrote:\n\n> > To better reflect the agreement between client and server, let's make\n> > promisor_remote_get_direct() try the accepted promisor remotes before\n> > the non-accepted ones.\n>\n> Interesting, and it feels sensible to me. Is it documented anywhere that\n> the ordering of announced remotes is actually important?\n\nI have documented it in the next version of this patch (which I will\nsend soon) by adding the following to\n\"Documentation/gitprotocol-v2.adoc\":\n\n\"The promisor remotes that the client accepted will be tried before the\nother configured promisor remotes when the client will attempt to\nfetch missing objects.\"\n\n> > Ensuring that accepted remotes are preferred will be even more\n> > important if in the future a mechanism is developed to allow the\n> > client to auto-configure remotes that the server advertises. This will\n> > in particular avoid fetching from the server (which is already\n> > configured as a promisor remote) before trying the auto-configured\n> > remotes, as these new remotes would likely appear at the end of the\n> > config file, and as the server might not appear in the\n> > `extensions.partialClone` config variable.\n>\n> Not quite sure I correctly understand this paragraph. Is the idea that\n> in the future, we might not even store announced promisors in the config\n> at all but simply use whatever the server announces on any given fetch?\n\nCurrently the promisor remotes are fetched from in the same order as\ntheir order in the config file, except that a remote whose name\nappears in the `extensions.partialClone` config variable is fetched\nfrom last.\n\nThe idea is that with `promisor.acceptFromServerUrl` it will be\npossible to store accepted promisors in the config, but the resulting\nconfig order will not be the best for fetching. Especially if the main\nremote doesn't appear in the `extensions.partialClone` config variable\nit will likely be tried before the accepted promisors which we do not\nwant.\n\nSo fetching first from the accepted promisors will also make things\nwork better when `promisor.acceptFromServerUrl` will be implemented.\n\n> > diff --git a/promisor-remote.c b/promisor-remote.c\n> > index 96fa215b06..3f8aeee787 100644\n> > --- a/promisor-remote.c\n> > +++ b/promisor-remote.c\n> > @@ -268,11 +268,37 @@ static int remove_fetched_oids(struct repository *repo,\n> > return remaining_nr;\n> > }\n> >\n> > +static int try_promisor_remotes(struct repository *repo,\n> > + struct object_id **remaining_oids,\n> > + int *remaining_nr, int *to_free,\n> > + bool accepted_only)\n> > +{\n> > + struct promisor_remote *r = repo->promisor_remote_config->promisors;\n> > +\n> > + for (; r; r = r->next) {\n> > + if (accepted_only && !r->accepted)\n> > + continue;\n> > + if (!accepted_only && r->accepted)\n> > + continue;\n>\n> This can be simplified to `if (!accepted_only != !r->accepted)`.\n\nRight, but I think we should go for `if (accepted_only !=\nr->accepted)`. It's even simpler and works because `r->accepted` is a\n1 bit bitfield and `accepted_only` a bool.\n\n> Also, can we maybe add a test for this to verify that we use the correct\n> ordering now?\n\nYeah, I have added tests for this in the new version.\n\nThanks.\n"},{"id":"540707","messageId":"CAP8UFD2Pp3vg=NtfYrpn=UEzpiPvu5F8nYiFrv+94tXh2LABPw@mail.gmail.com","threadId":"65335","inReplyTo":"acUkvkLYiO0wkCfm@pks.im","subject":"Re: [PATCH 10/16] promisor-remote: pass config entry to all_fields_match() directly","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2026-04-02T06:35:35Z","receivedAt":"2026-04-02T06:35:49Z","isPatch":true,"body":"On Thu, Mar 26, 2026 at 1:21 PM Patrick Steinhardt wrote:\n>\n> On Mon, Mar 23, 2026 at 09:05:13AM +0100, Christian Couder wrote:\n> > The `in_list == 0` path of all_fields_match() re-looks up the\n>\n> This reads a bit weird. How about \"looks up the remote in config_info by\n> advertised->name repeatedly\" instead?\n\nYeah, this is better. It's now used in the next version.\n\n> > diff --git a/promisor-remote.c b/promisor-remote.c\n> > index da347fa2dc..8f2c1280c3 100644\n> > --- a/promisor-remote.c\n> > +++ b/promisor-remote.c\n> > @@ -619,7 +627,11 @@ static int all_fields_match(struct promisor_info *advertised,\n> > if (!value)\n> > return 0;\n> >\n> > - if (in_list) {\n> > + if (config_entry) {\n> > + match = match_field_against_config(field, value,\n> > + config_entry);\n> > + } else {\n> > + struct string_list_item *item;\n> > for_each_string_list_item(item, config_info) {\n> > struct promisor_info *p = item->util;\n> > if (match_field_against_config(field, value, p)) {\n> > @@ -627,12 +639,6 @@ static int all_fields_match(struct promisor_info *advertised,\n> > break;\n> > }\n> > }\n> > - } else {\n> > - item = string_list_lookup(config_info, advertised->name);\n> > - if (item) {\n> > - struct promisor_info *p = item->util;\n> > - match = match_field_against_config(field, value, p);\n> > - }\n> > }\n> >\n> > if (!match)\n>\n> Okay, the logic is reversed now, which makes sense as we now pass `NULL`\n> instead of `1`, and the promisor info instead of `0`.\n>\n> The change itself makes sense, but other than that I have a very hard\n> time understanding these two functions. I think they would strongly\n> benefit from some comments explaining what's going on, what the input is\n> and what we're trying to do. Of course that doesn't have to be part of\n> this commit here, but I would appreciate a preparatory commit that helps\n> guide the reader a bit.\n\nThe patch already added a comment in front of all_fields_match() but\nnot in front of match_field_against_config(). Now it adds comments in\nfront of both.\n\nThanks.\n"},{"id":"540708","messageId":"CAP8UFD25LFuUX4m0EUooGQimb+h9K0Vn1AGisyPyocbqj6qTug@mail.gmail.com","threadId":"65335","inReplyTo":"acUkx8KwusUzYqne@pks.im","subject":"Re: [PATCH 12/16] t5710: use proper file:// URIs for absolute paths","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2026-04-02T06:36:26Z","receivedAt":"2026-04-02T06:36:38Z","isPatch":true,"body":"On Thu, Mar 26, 2026 at 1:21 PM Patrick Steinhardt wrote:\n>\n> On Mon, Mar 23, 2026 at 09:05:15AM +0100, Christian Couder wrote:\n> > diff --git a/t/t5710-promisor-remote-capability.sh b/t/t5710-promisor-remote-capability.sh\n> > index 357822c01a..c7a484228f 100755\n> > --- a/t/t5710-promisor-remote-capability.sh\n> > +++ b/t/t5710-promisor-remote-capability.sh\n> > @@ -76,6 +76,17 @@ copy_to_lop () {\n> > cp \"$path\" \"$path2\"\n> > }\n> >\n> > +# On Windows, 'pwd' returns a path like 'D:/foo/bar'. Prepend '/' to turn\n> > +# it into '/D:/foo/bar', which is what git expects in file:// URLs on Windows.\n> > +# On Unix, the path already starts with '/', so this is a no-op.\n> > +pwd_path=$(pwd)\n> > +case \"$pwd_path\" in\n> > +[a-zA-Z]:*) pwd_path=\"/$pwd_path\" ;;\n> > +esac\n> > +PWD_URL=\"file://$pwd_path\"\n> > +# Same as PWD_URL but with spaces percent-encoded, for use in URL patterns.\n> > +ENCODED_PWD_URL=\"file://$(echo \"$pwd_path\" | sed \"s/ /%20/g\")\"\n>\n> Can't we do this unconditionally on all platforms? \"file:////foobar\"\n> should be valid on Unix systems, too, shouldn't it?\n\nIt depends what you mean by \"valid\". Perhaps it works and would\nsimplify this code a bit, but I think the standard is 'RFC 8089: The\n\"file\" URI Scheme' which seems to says that the `//` prefix with an\nempty local host must be followed by an absolute path starting with a\nslash, resulting in exactly 3 slashes (file:///foobar). (See:\nhttps://www.rfc-editor.org/rfc/rfc8089#section-2)\n\nAppendix E.2 of RFC 8089 also seems to say that\n`file:///c:/path/to/file` is the standard representation for Windows\ndrive letters.\n\nSo it seems safer to follow that standard in our parsers and tests, as\nwe could then refer users to the standard and possibly make our\nparsers strictly follow it and test them against it in the future.\n"},{"id":"540709","messageId":"CAP8UFD05qM0WtD+csThQ3gZZ8dCqguzgs0bxT2HAHGHMELLWxg@mail.gmail.com","threadId":"65335","inReplyTo":"acUk3EAcL8-xM4VK@pks.im","subject":"Re: [PATCH 00/16] Auto-configure advertised remotes via URL whitelist","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2026-04-02T06:41:00Z","receivedAt":"2026-04-02T06:41:13Z","isPatch":true,"body":"On Thu, Mar 26, 2026 at 1:21 PM Patrick Steinhardt wrote:\n\n> I wonder whether it would make sense to split up this series into two.\n> The first 12 patches and parts of 16 are all sensible improvements that\n> can land independent of the patches that introduce the new logic. And\n> given that I expect some discussion around the new logic itself, I\n> expect that these refactorings can land way faster on their own.\n>\n> It would also help reduce the review load a bit if one then ultimately\n> only has to review three patches for the new feature.\n\nFine with me. I will send a preparatory series with 10 patches; one\nnew and 9 from the 16 patch series. Then I expect that the series with\nthe new logic will only be around 7 patches.\n"},{"id":"540713","messageId":"CAP8UFD1G2np6dJX_J6-5-Pxn=j_GgJ2BVdDkaxVw34EU6DDCLQ@mail.gmail.com","threadId":"65335","inReplyTo":"acUkwrm1rN4l4qgP@pks.im","subject":"Re: [PATCH 11/16] promisor-remote: refactor should_accept_remote() control flow","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2026-04-02T06:55:09Z","receivedAt":"2026-04-02T06:55:23Z","isPatch":true,"body":"On Thu, Mar 26, 2026 at 1:21 PM Patrick Steinhardt wrote:\n>\n> On Mon, Mar 23, 2026 at 09:05:14AM +0100, Christian Couder wrote:\n> > diff --git a/promisor-remote.c b/promisor-remote.c\n> > index 8f2c1280c3..c2f0eb7223 100644\n> > --- a/promisor-remote.c\n> > +++ b/promisor-remote.c\n> > @@ -665,6 +665,12 @@ static int should_accept_remote(enum accept_promisor accept,\n> > const char *remote_name = advertised->name;\n> > const char *remote_url = advertised->url;\n> >\n> > + if (!remote_url || !*remote_url) {\n> > + warning(_(\"no or empty URL advertised for remote '%s', \"\n> > + \"ignoring this remote\"), remote_name);\n> > + return 0;\n> > + }\n> > +\n> > if (accept == ACCEPT_ALL)\n> > return all_fields_match(advertised, config_info, NULL);\n>\n> You mention that it shouldn't change behaviour in well-defined cases\n> where the remote sends non-empty URLs. But does it change behaviour in\n> ill-defined cases where the remote sends empty ones?\n\nYes, it could change the behavior in the ill-defined case where the\nremote sends empty URLs.\n\nSo I have added a new patch to catch advertised empty URLs and empty\nremote names at parsing time. It is patch 4/10 (\"promisor-remote:\nreject empty name or URL in advertised remote\") in the preparatory\nseries I will send very soon now.\n\n> In other words, does this fix a bug that can be hit in the real world?\n\nNo, because I think Git has the only implementation of the\n\"promisor-remote\" capability, and a Git server doesn't advertise empty\nURLs. (promisor_config_info_list() ignores empty URLs.)\n\nSo the ill-defined case could only happen if people use their own fork\nof either Git (with a hacked the \"promisor-remote\" capability) or an\nimplementation of Git (that implements the \"promisor-remote\"\ncapability differently). So not likely in practice.\n\nThanks.\n"},{"id":"540715","messageId":"CAP8UFD2B1wHcfi_MOO6iy5bmZ2ULg1H9J2YuL-EaebAk0N302g@mail.gmail.com","threadId":"65335","inReplyTo":"acUks6pBmjgzN4M3@pks.im","subject":"Re: [PATCH 08/16] promisor-remote: remove the 'accepted' strvec","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2026-04-02T06:59:49Z","receivedAt":"2026-04-02T07:00:01Z","isPatch":true,"body":"On Thu, Mar 26, 2026 at 1:21 PM Patrick Steinhardt wrote:\n>\n> On Mon, Mar 23, 2026 at 09:05:11AM +0100, Christian Couder wrote:\n> > In a previous commit, filter_promisor_remote() was refactored to keep\n> > accepted 'struct promisor_info' instances alive instead of dismantling\n> > them into separate parallel data structures.\n> >\n> > Let's go one step further and replace the 'struct strvec *accepted'\n> > argument passed to filter_promisor_remote() with a\n> > 'struct string_list *accepted_remotes' argument.\n>\n> Right, this is indeed something I was wondering about given that we now\n> effectively stored the remote name twice.\n\nYeah, I think both patches together result in a nice code simplification.\n\nThanks for taking a close look at them.\n"},{"id":"540716","messageId":"CAP8UFD12BQUZOq4wfkthUE2No_ozrzhETKRWZgfgOha8UviuKg@mail.gmail.com","threadId":"65335","inReplyTo":"acUkqRUSyRUysf-I@pks.im","subject":"Re: [PATCH 04/16] promisor-remote: clarify that a remote is ignored","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2026-04-02T07:03:00Z","receivedAt":"2026-04-02T07:03:13Z","isPatch":true,"body":"On Thu, Mar 26, 2026 at 1:21 PM Patrick Steinhardt wrote:\n>\n> On Mon, Mar 23, 2026 at 09:05:07AM +0100, Christian Couder wrote:\n> > In should_accept_remote() when a remote is ignored, we might tell users\n> > why it is ignored in a warning, but we don't tell them that the remote\n> > is actually ignored.\n> >\n> > Let's clarify that, so users have a better idea of what's actually\n> > happening.\n>\n> Interesting that this doesn't result in any test changes. Don't we have\n> test coverage for `should_accept_remote()`?\n\nWe have test coverage but we don't test the emitted warning, we test\nwhether the remote is actually accepted and used.\n\nLet me know if you think we should test the warnings too.\n"},{"id":"542378","messageId":"CAP8UFD2AwxJCNdgdL4KSRdRdeSdapTkAbFj6pzcy8w4gOczM2A@mail.gmail.com","threadId":"65335","inReplyTo":"acUkpJjHgYs0jqX4@pks.im","subject":"Re: [PATCH 03/16] urlmatch: add url_is_valid_pattern() helper","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2026-04-27T12:42:10Z","receivedAt":"2026-04-27T12:42:24Z","isPatch":true,"body":"On Thu, Mar 26, 2026 at 1:20 PM Patrick Steinhardt wrote:\n>\n> On Mon, Mar 23, 2026 at 09:05:06AM +0100, Christian Couder wrote:\n\n> > +bool url_is_valid_pattern(const char *url)\n> > +{\n> > + char *normalized = url_normalize_1(url, NULL, true);\n> > +\n> > + if (normalized) {\n> > + free(normalized);\n> > + return true;\n> > + }\n> > +\n> > + return false;\n> > +}\n>\n> We could simplify this implementation to:\n>\n> bool url_is_valid_pattern(const char *url)\n> {\n> char *normalized = url_normalize_1(url, NULL, true);\n> free(normalized);\n> return !!normalized;\n> }\n\nThanks for the suggestion but this patch has been replaced by a patch\nadding url_normalize_pattern() in the v2 I just sent.\n"},{"id":"542379","messageId":"CAP8UFD0BMWJZx95pQFsj5aDgGLEq+R4OuaxB2Hcv3ZNwAw7QpA@mail.gmail.com","threadId":"65335","inReplyTo":"acUkuD6iuq6nTeHn@pks.im","subject":"Re: [PATCH 09/16] promisor-remote: add 'local_name' to 'struct promisor_info'","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2026-04-27T12:42:38Z","receivedAt":"2026-04-27T12:42:50Z","isPatch":true,"body":"On Thu, Mar 26, 2026 at 1:21 PM Patrick Steinhardt wrote:\n>\n> On Mon, Mar 23, 2026 at 09:05:12AM +0100, Christian Couder wrote:\n> > In a following commit, we will store promisor remote information under\n> > a remote name different than the one the server advertised.\n> >\n> > To prepare for this change, let's add a new 'char* local_name' member\n>\n> Micronit: s/char* local_name/char *local_name/\n\nFixed in the v2 I just sent.\n\n> > diff --git a/promisor-remote.c b/promisor-remote.c\n> > index bdfc5e7608..da347fa2dc 100644\n> > --- a/promisor-remote.c\n> > +++ b/promisor-remote.c\n> > @@ -434,15 +434,19 @@ static struct string_list *fields_stored(void)\n> >\n> > /*\n> > * Struct for promisor remotes involved in the \"promisor-remote\"\n> > - * protocol capability.\n> > + * protocol capability:\n> > *\n> > - * Except for \"name\", each in this struct and its \n> > - * should correspond (either on the client side or on the server side)\n> > - * to a \"remote..\" config variable set to where\n> > - * \"\" is a promisor remote name.\n> > + * - \"name\" is the name the server advertised.\n> > + * - \"local_name\" is the name we use locally (may be auto-generated).\n> > + *\n> > + * Except for \"name\" and \"local_name\", each in this struct\n> > + * and its should correspond (either on the client side or on\n> > + * the server side) to a \"remote..\" config variable set\n> > + * to where \"\" is a promisor remote name.\n> > */\n> > struct promisor_info {\n> > const char *name;\n> > + const char *local_name;\n> > const char *url;\n> > const char *filter;\n> > const char *token;\n>\n> I think it would be easier to follow if the struct-level comment applied\n> to the general description of the struct, and individual members would\n> then have their own comments describing their intent.\n\nRight, the diff looks like the following in the v2:\n\n@@ -434,13 +434,14 @@ static struct string_list *fields_stored(void)\n * Struct for promisor remotes involved in the \"promisor-remote\"\n * protocol capability.\n *\n- * Except for \"name\", each in this struct and its \n- * should correspond (either on the client side or on the server side)\n- * to a \"remote..\" config variable set to where\n- * \"\" is a promisor remote name.\n+ * Except for \"name\" and \"local_name\", each in this struct\n+ * and its should correspond (either on the client side or on\n+ * the server side) to a \"remote..\" config variable set\n+ * to where \"\" is a promisor remote name.\n */\n struct promisor_info {\n- const char *name;\n+ const char *name; /* name the server advertised */\n+ const char *local_name; /* name used locally (may be auto-generated) */\n const char *url;\n const char *filter;\n const char *token;\n\nThanks.\n"},{"id":"542380","messageId":"CAP8UFD20=ArLUDbD36=02_i2io8+uMYhm5LJVGayzKYOkqmMBg@mail.gmail.com","threadId":"65335","inReplyTo":"acUk11mt06GJZaur@pks.im","subject":"Re: [PATCH 15/16] promisor-remote: auto-configure unknown remotes","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2026-04-27T12:44:02Z","receivedAt":"2026-04-27T12:44:14Z","isPatch":true,"body":"On Thu, Mar 26, 2026 at 1:21 PM Patrick Steinhardt wrote:\n>\n> On Mon, Mar 23, 2026 at 09:05:18AM +0100, Christian Couder wrote:\n> > Previous commits have introduced the `promisor.acceptFromServerUrl`\n> > config variable to whitelist some URLs advertised by a server through\n> > the \"promisor-remote\" protocol capability.\n> >\n> > However the new `promisor.acceptFromServerUrl` mechanism, like the old\n> > `promisor.acceptFromServer` mechanism, still requires a remote to\n> > already exist in the client's local configuration before it can be\n> > accepted. This places a significant manual burden on users to\n> > pre-configure these remotes, and creates friction for administrators\n> > who have to troubleshoot or manually provision these setups for their\n> > teams.\n> >\n> > To eliminate this burden, let's automatically create a new `[remote]`\n> > section in the client's config when a server advertises an unknown\n> > remote whose URL matches a `promisor.acceptFromServerUrl` glob pattern.\n>\n> Would it make sense to extend git-clone(1) to have a command line option\n> that basically does this as a one-shot? Something like `git clone\n> --accept-promisors=url:https://gitlab.com/*`? I assume that many users\n> may not want to keep on updating their configured promisors all the\n> time.\n\nI don't understand why you say \"many users may not want to keep on\nupdating their configured promisors all the time\". It seems to me that\nwhat I propose requires even less effort from users than what you\nsuggest.\n\nIf users set up something like:\n\n git config set --global promisor.acceptFromServerUrl \"https://my-org.com/*\"\n\nor:\n\n git config set --global promisor.acceptFromServerUrl\n\"https://gitlab.com/my-org/*\"\n\nthey would then automatically accept the promisor remotes with an URL\nmatching the pattern when they make a partial clone.\n\nSo it's a one time setup instead of having to use\n`--accept-promisors=url:...` each time they clone.\n\nAlso `git -c promisor.acceptFromServerUrl=\"...\" clone` can basically\nbe used to get the same thing as the `--accept-promisors=url:...` flag\nyou suggest.\n\n> Furthermore, this here reconfirms my thought on the previous commit that\n> it would make sense to detangle accepting promisors, storing them in the\n> configuration and updating them automatically. These are all different\n> things:\n>\n> - Accepting promisors is basically an ongoing runtime thing where you\n> start to use announced promisors even though they are not configured\n> at all.\n\nWhy an \"ongoing runtime thing\"? If users think it's fine to accept\npromisors from their own domain, why should they have to confirm that\nevery time they clone?\n\n> - Storing promisors is typically a one-time thing that you'd want to\n> do when creating a new repository.\n\nExcept that some fields and maybe sometimes URLs might change on the\nserver side and it would be nice if this didn't require manual updates\non the client side.\n\n> - Updating promisors automatically is probably something you want to\n> do on an ongoing basis when you have stored promisors.\n\nYeah, so it's similar in many ways to storing promisors.\n\n> We're currently putting all of these use cases into the same bag, but\n> they have very different characteristics.\n\nI don't think all use cases are put in the same bag. There are a\nnumber of config options already that allow a lot of customization.\nAdding \"promisor.acceptFromServerUrl\" as a separate option from\n\"promisor.acceptFromServer\" also only increases the possibilities for\nusers.\n\n> I guess the most common use\n> case will eventually be to never auto-accept promisors, store them at\n> clone time, and keep them updated whenever they change.\n\nI am not sure at all this is the most common use case.\n\nIf you require a `--accept-promisors=url:https://my-org.com/*` each\ntime, people might just copy-paste it or create an alias for that and\nthen use it all the time and you won't get much more security than\nsomething like:\n\n git config set --global promisor.acceptFromServerUrl \"https://my-org.com/*\"\n\nonce and then regular `git clone ...`\n\nWhen users have to often pass parameters manually, the typos and\nmisconfiguration risks also increase compared to admins setting things\nup globally for everyone, or even users doing it once for themselves.\n\n> This cannot be\n> expressed with \"promisor.acceptFromServerUrl\" as far as I understand.\n\n`git -c promisor.acceptFromServerUrl=\"...\" clone` is basically the\nsame as the option you suggest.\n"},{"id":"542381","messageId":"CAP8UFD3AWRjgKnwdppS7=Q7WL5pO3r1T_e61ZPqm15W8aY3mxg@mail.gmail.com","threadId":"65335","inReplyTo":"acUkzY7f5302uWD8@pks.im","subject":"Re: [PATCH 13/16] promisor-remote: introduce promisor.acceptFromServerUrl","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2026-04-27T12:44:45Z","receivedAt":"2026-04-27T12:44:58Z","isPatch":true,"body":"On Thu, Mar 26, 2026 at 1:21 PM Patrick Steinhardt wrote:\n>\n> On Mon, Mar 23, 2026 at 09:05:16AM +0100, Christian Couder wrote:\n> > diff --git a/promisor-remote.c b/promisor-remote.c\n> > index c2f0eb7223..4cb18e1a6a 100644\n> > --- a/promisor-remote.c\n> > +++ b/promisor-remote.c\n> [snip]\n> > +static struct string_list *accept_from_server_url(struct repository *repo)\n> > +{\n> > + static struct string_list accept_urls = STRING_LIST_INIT_DUP;\n> > + static int initialized;\n> > + const struct string_list *config_urls;\n> > +\n> > + if (initialized)\n> > + return &accept_urls;\n> > +\n> > + initialized = 1;\n> > +\n> > + if (!repo_config_get_string_multi(repo, \"promisor.acceptfromserverurl\", &config_urls)) {\n> > + struct string_list_item *item;\n> > +\n> > + for_each_string_list_item(item, config_urls) {\n> > + struct allowed_url *allowed = valid_accept_url(item->string);\n> > + if (allowed) {\n> > + struct string_list_item *new;\n> > + new = string_list_append(&accept_urls, item->string);\n> > + new->util = allowed;\n> > + }\n> > + }\n> > + }\n> > +\n> > + return &accept_urls;\n> > +}\n>\n> I'm still not much of a fan of us getting more and more function-local\n> static variables. It just feels wrong to me, and like we're accruing\n> technical debt. I also doubt that the performance overhead of storing\n> this on the stack with proper lifecycle management will matter at all\n> given that we're in a context where we talk with a remote anyway. The\n> handful of allocations really shouldn't matter in that context.\n\nOK, I have removed the static variables and it looks like the following in v2:\n\n+static void load_accept_from_server_url(struct repository *repo,\n+ struct string_list *accept_urls)\n+{\n+ const struct string_list *config_urls;\n+\n+ if (!repo_config_get_string_multi(repo,\n\"promisor.acceptfromserverurl\", &config_urls)) {\n+ struct string_list_item *item;\n+\n+ for_each_string_list_item(item, config_urls) {\n+ struct allowed_url *allowed =\nvalid_accept_url(item->string);\n+ if (allowed) {\n+ struct string_list_item *new;\n+ new = string_list_append(accept_urls,\nitem->string);\n+ new->util = allowed;\n+ }\n+ }\n+ }\n+}\n+\n static int should_accept_remote(enum accept_promisor accept,\n struct promisor_info *advertised,\n struct string_list *config_info)\n@@ -901,6 +986,10 @@ static void filter_promisor_remote(struct repository *repo,\n struct string_list_item *item;\n bool reload_config = false;\n enum accept_promisor accept = accept_from_server(repo);\n+ struct string_list accept_urls = STRING_LIST_INIT_DUP;\n+\n+ /* Load and validate the acceptFromServerUrl config */\n+ load_accept_from_server_url(repo, &accept_urls);\n\n\n if (accept == ACCEPT_NONE)\n return;\n"},{"id":"542382","messageId":"CAP8UFD1dvys9nEF6tRudWaeHmmEFH0NGPqK5YM_mk5RQa2=ujw@mail.gmail.com","threadId":"65335","inReplyTo":"acUk0vTuj8COlvgf@pks.im","subject":"Re: [PATCH 14/16] promisor-remote: trust known remotes matching acceptFromServerUrl","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2026-04-27T12:45:41Z","receivedAt":"2026-04-27T12:45:54Z","isPatch":true,"body":"On Thu, Mar 26, 2026 at 1:21 PM Patrick Steinhardt wrote:\n>\n> On Mon, Mar 23, 2026 at 09:05:17AM +0100, Christian Couder wrote:\n\n> > To enable such targeted updates for trusted URLs, let's use the URL\n> > patterns from `promisor.acceptFromServerUrl` as an additional URL\n> > based whitelist.\n> >\n> > Concretely, let's check the advertised URLs against the URL glob\n> > patterns by introducing a new small helper function called\n> > url_matches_accept_list(), which iterates over the glob patterns and\n> > returns the first matching allowed_url entry (or NULL).\n> >\n> > (Before matching, the advertised URL is passed through url_normalize()\n> > so that case variations in the scheme/host, percent-encoding tricks,\n> > and \"..\" path segments cannot bypass the whitelist.)\n> >\n> > Let's then use this helper at the tail of should_accept_remote() so\n> > that, when `accept == ACCEPT_NONE`, a known remote whose URL matches\n> > the whitelist is still accepted.\n> >\n> > To prepare for this new logic, let's also:\n> >\n> > - Add an 'accept_urls' parameter to should_accept_remote().\n> >\n> > - Replace the BUG() guard in the ACCEPT_KNOWN_URL case with an\n> > explicit 'if (accept == ACCEPT_KNOWN_URL) return' and a new\n> > BUG() guard in the ACCEPT_NONE case, so url_matches_accept_list()\n> > is only called in the ACCEPT_NONE case.\n> >\n> > - Call accept_from_server_url() from filter_promisor_remote()\n> > and relax its early return so that the function is entered when\n> > `accept_urls` has entries even if `accept == ACCEPT_NONE`.\n> >\n> > Let's then properly document `promisor.acceptFromServerUrl` in\n> > \"promisor.adoc\" as an additive security whitelist for known remotes,\n> > including the URL normalization behavior, and let's mention it in\n> > \"gitprotocol-v2.adoc\".\n>\n> I feel like the description is steering a bit too strongly into the\n> direction of a step-by-step instruction of how to implement the change\n> rather than explaining what's done and why it's done this way.\n\nI have added the following:\n\n\"With this, many organizations may only need something like:\n\n git config set --global \\\n promisor.acceptFromServerUrl \"https://my-org.com/*\"\n\nto accept only their own remotes. And if they need to accept additional\nremotes in some specific repos, they can also set:\n\n git config set promisor.acceptFromServer knownUrl\n\nand configure the additional remote manually only in the repos where\nthey are needed.\"\n\n> > +promisor.acceptFromServerUrl::\n> > + A glob pattern to specify which URLs advertised by a server\n> > + are considered trusted by the client. This option acts as an\n> > + additive security whitelist that works in conjunction with\n> > + `promisor.acceptFromServer`.\n> > ++\n> > +This option can appear multiple times in config files. An advertised\n> > +URL will be accepted if it matches _ANY_ glob pattern specified by\n> > +this option in _ANY_ config file read by Git.\n> > ++\n> > +Be _VERY_ careful with these glob patterns, as it can be a big\n> > +security hole to allow any advertised remote to be auto-configured!\n> > +To minimize security risks, follow these guidelines:\n> > ++\n> > +1. Start with a secure protocol scheme, like `https://` or `ssh://`.\n> > ++\n> > +2. Only allow domain names or paths where you control and trust _ALL_\n> > + the content. Be especially careful with shared hosting platforms\n> > + like `github.com` or `gitlab.com`. A broad pattern like\n> > + `https://gitlab.com/*` is dangerous because it trusts every\n> > + repository on the entire platform. Always restrict such patterns to\n> > + your specific organization or namespace (e.g.,\n> > + `https://gitlab.com/your-org/*`).\n> > ++\n> > +3. Don't use globs (`*`) in the domain name. For example\n> > + `https://cdn.example.com/*` is much safer than\n> > + `https://*.example.com/*`, because the latter matches\n> > + `https://evil-hacker.net/fake.example.com/repo`.\n> > ++\n> > +4. Make sure to have a `/` at the end of the domain name (or the end\n> > + of specific directories). For example `https://cdn.example.com/*`\n> > + is much safer than `https://cdn.example.com*`, because the latter\n> > + matches `https://cdn.example.com.hacker.net/repo`.\n> > ++\n> > +Before matching, the advertised URL is normalized: the scheme and\n> > +host are lowercased, percent-encoded characters are decoded where\n> > +possible, and path segments like `..` are resolved. Glob patterns\n> > +are matched against this normalized URL as-is, so patterns should\n> > +be written in normalized form (e.g., lowercase scheme and host).\n> > ++\n> > +Even if `promisor.acceptFromServer` is set to `None` (the default),\n> > +Git will still accept field updates (like tokens) for known remotes,\n> > +provided their URLs match a pattern in\n> > +`promisor.acceptFromServerUrl`. See linkgit:gitprotocol-v2[5] for\n> > +details on the protocol.\n>\n> Given that there's a bunch to process here, would it make sense to give\n> users an example for how to do it properly?\n\nI can give an example like the one I added to the commit message (see\nabove), but it might be too lax for some use cases. Perhaps in many\norganizations only a single repo will ever require to accept promisor\nremotes, so giving an example with the `--global` flag like:\n\n git config set --global promisor.acceptFromServerUrl \"https://my-org.com/*\"\n\ncould make everyone's config a bit more vulnerable than necessary.\n\nI think it's better to nudge people to think through the four steps\nabove, rather than to encourage them to copy-paste something that\nmight not be very well suited to their needs.\n\n> I also wonder why we require a new config entry instead of extending\n> `promisor.acceptFromRemote` to have for example a new \"url:https://...\"\n> setting. Are there cases where you would ever want to use the new\n> URL-based schema with a different setting than \"all\"?\n\nYes, I think the example in the commit message shows why having both:\n\n- `promisor.acceptFromRemote` that you can set for example to\n\"knownUrl\" only in some specific repos where you accept external\npromisor remotes that you configure manually, and\n\n- `promisor.acceptFromRemoteUrl` that you can set for example to\n\"https://my-org.com/*\" globally\n\ncan be relatively simple and quite powerful:\n\n- all internal remotes (with URLs in https://my-org.com/) are\nautomatically accepted in all the repos,\n- in certain specific repos, some external remotes (with names and\nURLs that are manually configured in the repos) are also accepted.\n\n> I guess the case with \"none\" is exactly that, where you may auto-update\n> configured remotes. But I wonder whether it would be more sensible to\n> split out behaviour of accepting and updating promisors into separate\n> configuration variables. These are ultimately different concerns, and\n> the interaction as layed out in this commit is somewhat non-obvious to\n> me.\n\nLet me know if there are things I could clarify more in the above\nexplanations. I am also open to concrete suggestions.\n\nThanks.\n"}]}