]> git.ipfire.org Git - thirdparty/git.git/commitdiff
promisor-remote: allow a client to check fields
authorChristian Couder <christian.couder@gmail.com>
Wed, 11 Jun 2025 13:45:05 +0000 (15:45 +0200)
committerJunio C Hamano <gitster@pobox.com>
Thu, 12 Jun 2025 15:07:43 +0000 (08:07 -0700)
A previous commit allowed a server to pass additional fields through
the "promisor-remote" protocol capability after the "name" and "url"
fields, specifically the "partialCloneFilter" and "token" fields.

Let's make it possible for a client to check if these fields match
what it expects before accepting a promisor remote.

We allow this by introducing a new "promisor.checkFields"
configuration variable. It should contain a comma or space separated
list of fields that will be checked.

By limiting the protocol to specific well-defined fields, we ensure
both server and client have a shared understanding of field
semantics and usage.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Documentation/config/promisor.adoc
promisor-remote.c
t/t5710-promisor-remote-capability.sh

index beb8f65518dd03b4da9520fd175eff8f3bc61081..9682ada23cafb4a0c5b7ac71fab20d6a509cfe4b 100644 (file)
@@ -50,3 +50,43 @@ promisor.acceptFromServer::
        lazily fetchable from this promisor remote from its responses
        to "fetch" and "clone" requests from the client. Name and URL
        comparisons are case sensitive. See linkgit:gitprotocol-v2[5].
+
+promisor.checkFields::
+       A comma or space separated list of additional remote related
+       field names. A client will check if the values of these fields
+       transmitted by a server correspond to the values of these
+       fields in its own configuration before accepting a promisor
+       remote. Currently, "partialCloneFilter" and "token" are the
+       only supported field names.
++
+If one of these field names (e.g., "token") is being checked for an
+advertised promisor remote (e.g., "foo"), three conditions must be met
+for the check of this specific field to pass:
++
+1. The corresponding local configuration (e.g., `remote.foo.token`)
+   must be set.
+2. The server must advertise the "token" field for remote "foo".
+3. The value of the locally configured `remote.foo.token` must exactly
+   match the value advertised by the server for the "token" field.
++
+If any of these conditions is not met for any field name listed in
+`promisor.checkFields`, the advertised remote "foo" will be rejected.
++
+For the "partialCloneFilter" field, this allows the client to ensure
+that the server's filter matches what it expects locally, preventing
+inconsistencies in filtering behavior. For the "token" field, this can
+be used to verify that authentication credentials match expected
+values.
++
+Field names are compared case-insensitively. Field values are compared
+case-sensitively.
++
+The "name" and "url" fields are always checked according to the
+`promisor.acceptFromServer` policy, independently of this setting.
++
+The field names and values should be passed by the server through the
+"promisor-remote" capability by using the `promisor.sendFields` config
+variable. The fields will be checked only if the
+`promisor.acceptFromServer` config variable is not set to "None". If
+set to "None", this config variable will have no effect. See
+linkgit:gitprotocol-v2[5].
index e291a00a73edb246a204d09d77ec6addc20779c0..939cc78a7da61cd94ca2f693ea8eb7842a461c86 100644 (file)
@@ -382,6 +382,20 @@ static struct string_list *fields_sent(void)
        return &fields_list;
 }
 
+static struct string_list *fields_checked(void)
+{
+       static struct string_list fields_list = STRING_LIST_INIT_NODUP;
+       static int initialized = 0;
+
+       if (!initialized) {
+               fields_list.cmp = strcasecmp;
+               fields_from_config(&fields_list, "promisor.checkFields");
+               initialized = 1;
+       }
+
+       return &fields_list;
+}
+
 /*
  * Struct for promisor remotes involved in the "promisor-remote"
  * protocol capability.
@@ -527,6 +541,61 @@ enum accept_promisor {
        ACCEPT_ALL
 };
 
+static int match_field_against_config(const char *field, const char *value,
+                                     struct promisor_info *config_info)
+{
+       if (config_info->filter && !strcasecmp(field, promisor_field_filter))
+               return !strcmp(config_info->filter, value);
+       else if (config_info->token && !strcasecmp(field, promisor_field_token))
+               return !strcmp(config_info->token, value);
+
+       return 0;
+}
+
+static int all_fields_match(struct promisor_info *advertised,
+                           struct string_list *config_info,
+                           int in_list)
+{
+       struct string_list* fields = fields_checked();
+       struct string_list_item *item_checked;
+
+       for_each_string_list_item(item_checked, fields) {
+               int match = 0;
+               const char *field = item_checked->string;
+               const char *value = NULL;
+               struct string_list_item *item;
+
+               if (!strcasecmp(field, promisor_field_filter))
+                       value = advertised->filter;
+               else if (!strcasecmp(field, promisor_field_token))
+                       value = advertised->token;
+
+               if (!value)
+                       return 0;
+
+               if (in_list) {
+                       for_each_string_list_item(item, config_info) {
+                               struct promisor_info *p = item->util;
+                               if (match_field_against_config(field, value, p)) {
+                                       match = 1;
+                                       break;
+                               }
+                       }
+               } else {
+                       item = string_list_lookup(config_info, advertised->name);
+                       if (item) {
+                               struct promisor_info *p = item->util;
+                               match = match_field_against_config(field, value, p);
+                       }
+               }
+
+               if (!match)
+                       return 0;
+       }
+
+       return 1;
+}
+
 static int should_accept_remote(enum accept_promisor accept,
                                struct promisor_info *advertised,
                                struct string_list *config_info)
@@ -537,7 +606,7 @@ static int should_accept_remote(enum accept_promisor accept,
        const char *remote_url = advertised->url;
 
        if (accept == ACCEPT_ALL)
-               return 1;
+               return all_fields_match(advertised, config_info, 1);
 
        /* Get config info for that promisor remote */
        item = string_list_lookup(config_info, remote_name);
@@ -549,7 +618,7 @@ static int should_accept_remote(enum accept_promisor accept,
        p = item->util;
 
        if (accept == ACCEPT_KNOWN_NAME)
-               return 1;
+               return all_fields_match(advertised, config_info, 0);
 
        if (accept != ACCEPT_KNOWN_URL)
                BUG("Unhandled 'enum accept_promisor' value '%d'", accept);
@@ -564,7 +633,7 @@ static int should_accept_remote(enum accept_promisor accept,
                    remote_name);
 
        if (!strcmp(p->url, remote_url))
-               return 1;
+               return all_fields_match(advertised, config_info, 0);
 
        warning(_("known remote named '%s' but with URL '%s' instead of '%s'"),
                remote_name, p->url, remote_url);
@@ -596,6 +665,10 @@ static struct promisor_info *parse_one_advertised_remote(struct strbuf *remote_i
                        info->name = value;
                else if (!strcmp(elem, "url"))
                        info->url = value;
+               else if (!strcasecmp(elem, promisor_field_filter))
+                       info->filter = value;
+               else if (!strcasecmp(elem, promisor_field_token))
+                       info->token = value;
                else
                        free(value);
        }
@@ -638,11 +711,6 @@ static void filter_promisor_remote(struct repository *repo,
        if (accept == ACCEPT_NONE)
                return;
 
-       if (accept != ACCEPT_ALL) {
-               promisor_config_info_list(repo, &config_info, NULL);
-               string_list_sort(&config_info);
-       }
-
        /* Parse remote info received */
 
        remotes = strbuf_split_str(info, ';', 0);
@@ -657,6 +725,11 @@ static void filter_promisor_remote(struct repository *repo,
                if (!advertised)
                        continue;
 
+               if (!config_info.nr) {
+                       promisor_config_info_list(repo, &config_info, fields_checked());
+                       string_list_sort(&config_info);
+               }
+
                if (should_accept_remote(accept, advertised, &config_info))
                        strvec_push(accepted, advertised->name);
 
index 204528b2e0cc698c093b87b4081547ee5d954126..023735d6a84ea8158896dafc132c43af6f4c38eb 100755 (executable)
@@ -326,6 +326,40 @@ test_expect_success "clone with promisor.sendFields" '
        check_missing_objects server 1 "$oid"
 '
 
+test_expect_success "clone with promisor.checkFields" '
+       git -C server config promisor.advertise true &&
+       test_when_finished "rm -rf client" &&
+
+       git -C server remote add otherLop "https://invalid.invalid"  &&
+       git -C server config remote.otherLop.token "fooBar" &&
+       git -C server config remote.otherLop.stuff "baz" &&
+       git -C server config remote.otherLop.partialCloneFilter "blob:limit=10k" &&
+       test_when_finished "git -C server remote remove otherLop" &&
+       test_config -C server promisor.sendFields "partialCloneFilter, token" &&
+       test_when_finished "rm trace" &&
+
+       # Clone from server to create a client
+       GIT_TRACE_PACKET="$(pwd)/trace" GIT_NO_LAZY_FETCH=0 git clone \
+               -c remote.lop.promisor=true \
+               -c remote.lop.fetch="+refs/heads/*:refs/remotes/lop/*" \
+               -c remote.lop.url="file://$(pwd)/lop" \
+               -c remote.lop.partialCloneFilter="blob:none" \
+               -c promisor.acceptfromserver=All \
+               -c promisor.checkFields=partialcloneFilter \
+               --no-local --filter="blob:limit=5k" server client &&
+
+       # Check that fields are properly transmitted
+       ENCODED_URL=$(echo "file://$(pwd)/lop" | sed -e "s/ /%20/g") &&
+       PR1="name=lop,url=$ENCODED_URL,partialCloneFilter=blob:none" &&
+       PR2="name=otherLop,url=https://invalid.invalid,partialCloneFilter=blob:limit=10k,token=fooBar" &&
+       test_grep "clone< promisor-remote=$PR1;$PR2" trace &&
+       test_grep "clone> promisor-remote=lop" trace &&
+       test_grep ! "clone> promisor-remote=lop;otherLop" trace &&
+
+       # Check that the largest object is still missing on the server
+       check_missing_objects server 1 "$oid"
+'
+
 test_expect_success "clone with promisor.advertise set to 'true' but don't delete the client" '
        git -C server config promisor.advertise true &&