freshlybakedca.ke / git
Git fork
fork atom

promisor-remote: allow a client to check fields

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>

authored by

Christian Couder and committed by
Junio C Hamano c213820c bcb08c83

+154 -8
unified split
+39
Documentation/config/promisor.adoc
··· 50 50 lazily fetchable from this promisor remote from its responses 51 51 to "fetch" and "clone" requests from the client. Name and URL 52 52 comparisons are case sensitive. See linkgit:gitprotocol-v2[5]. 53 + 54 + promisor.checkFields:: 55 + A comma or space separated list of additional remote related 56 + field names. A client checks if the values of these fields 57 + transmitted by a server correspond to the values of these 58 + fields in its own configuration before accepting a promisor 59 + remote. Currently, "partialCloneFilter" and "token" are the 60 + only supported field names. 61 + + 62 + If one of these field names (e.g., "token") is being checked for an 63 + advertised promisor remote (e.g., "foo"), three conditions must be met 64 + for the check of this specific field to pass: 65 + + 66 + 1. The corresponding local configuration (e.g., `remote.foo.token`) 67 + must be set. 68 + 2. The server must advertise the "token" field for remote "foo". 69 + 3. The value of the locally configured `remote.foo.token` must exactly 70 + match the value advertised by the server for the "token" field. 71 + + 72 + If any of these conditions is not met for any field name listed in 73 + `promisor.checkFields`, the advertised remote "foo" is rejected. 74 + + 75 + For the "partialCloneFilter" field, this allows the client to ensure 76 + that the server's filter matches what it expects locally, preventing 77 + inconsistencies in filtering behavior. For the "token" field, this can 78 + be used to verify that authentication credentials match expected 79 + values. 80 + + 81 + Field values are compared case-sensitively. 82 + + 83 + The "name" and "url" fields are always checked according to the 84 + `promisor.acceptFromServer` policy, independently of this setting. 85 + + 86 + The field names and values should be passed by the server through the 87 + "promisor-remote" capability by using the `promisor.sendFields` config 88 + variable. The fields are checked only if the 89 + `promisor.acceptFromServer` config variable is not set to "None". If 90 + set to "None", this config variable has no effect. See 91 + linkgit:gitprotocol-v2[5].
+81 -8
promisor-remote.c
··· 389 389 return &fields_list; 390 390 } 391 391 392 + static struct string_list *fields_checked(void) 393 + { 394 + static struct string_list fields_list = STRING_LIST_INIT_NODUP; 395 + static int initialized; 396 + 397 + if (!initialized) { 398 + fields_list.cmp = strcasecmp; 399 + fields_from_config(&fields_list, "promisor.checkFields"); 400 + initialized = 1; 401 + } 402 + 403 + return &fields_list; 404 + } 405 + 392 406 /* 393 407 * Struct for promisor remotes involved in the "promisor-remote" 394 408 * protocol capability. ··· 534 548 ACCEPT_ALL 535 549 }; 536 550 551 + static int match_field_against_config(const char *field, const char *value, 552 + struct promisor_info *config_info) 553 + { 554 + if (config_info->filter && !strcasecmp(field, promisor_field_filter)) 555 + return !strcmp(config_info->filter, value); 556 + else if (config_info->token && !strcasecmp(field, promisor_field_token)) 557 + return !strcmp(config_info->token, value); 558 + 559 + return 0; 560 + } 561 + 562 + static int all_fields_match(struct promisor_info *advertised, 563 + struct string_list *config_info, 564 + int in_list) 565 + { 566 + struct string_list *fields = fields_checked(); 567 + struct string_list_item *item_checked; 568 + 569 + for_each_string_list_item(item_checked, fields) { 570 + int match = 0; 571 + const char *field = item_checked->string; 572 + const char *value = NULL; 573 + struct string_list_item *item; 574 + 575 + if (!strcasecmp(field, promisor_field_filter)) 576 + value = advertised->filter; 577 + else if (!strcasecmp(field, promisor_field_token)) 578 + value = advertised->token; 579 + 580 + if (!value) 581 + return 0; 582 + 583 + if (in_list) { 584 + for_each_string_list_item(item, config_info) { 585 + struct promisor_info *p = item->util; 586 + if (match_field_against_config(field, value, p)) { 587 + match = 1; 588 + break; 589 + } 590 + } 591 + } else { 592 + item = string_list_lookup(config_info, advertised->name); 593 + if (item) { 594 + struct promisor_info *p = item->util; 595 + match = match_field_against_config(field, value, p); 596 + } 597 + } 598 + 599 + if (!match) 600 + return 0; 601 + } 602 + 603 + return 1; 604 + } 605 + 537 606 static int should_accept_remote(enum accept_promisor accept, 538 607 struct promisor_info *advertised, 539 608 struct string_list *config_info) ··· 544 613 const char *remote_url = advertised->url; 545 614 546 615 if (accept == ACCEPT_ALL) 547 - return 1; 616 + return all_fields_match(advertised, config_info, 1); 548 617 549 618 /* Get config info for that promisor remote */ 550 619 item = string_list_lookup(config_info, remote_name); ··· 556 625 p = item->util; 557 626 558 627 if (accept == ACCEPT_KNOWN_NAME) 559 - return 1; 628 + return all_fields_match(advertised, config_info, 0); 560 629 561 630 if (accept != ACCEPT_KNOWN_URL) 562 631 BUG("Unhandled 'enum accept_promisor' value '%d'", accept); ··· 567 636 } 568 637 569 638 if (!strcmp(p->url, remote_url)) 570 - return 1; 639 + return all_fields_match(advertised, config_info, 0); 571 640 572 641 warning(_("known remote named '%s' but with URL '%s' instead of '%s'"), 573 642 remote_name, p->url, remote_url); ··· 605 674 info->name = url_percent_decode(p); 606 675 else if (skip_field_name_prefix(elem, promisor_field_url, &p)) 607 676 info->url = url_percent_decode(p); 677 + else if (skip_field_name_prefix(elem, promisor_field_filter, &p)) 678 + info->filter = url_percent_decode(p); 679 + else if (skip_field_name_prefix(elem, promisor_field_token, &p)) 680 + info->token = url_percent_decode(p); 608 681 } 609 682 610 683 string_list_clear(&elem_list, 0); ··· 646 719 if (accept == ACCEPT_NONE) 647 720 return; 648 721 649 - if (accept != ACCEPT_ALL) { 650 - promisor_config_info_list(repo, &config_info, NULL); 651 - string_list_sort(&config_info); 652 - } 653 - 654 722 /* Parse remote info received */ 655 723 656 724 string_list_split(&remote_info, info, ";", -1); ··· 662 730 663 731 if (!advertised) 664 732 continue; 733 + 734 + if (!config_info.nr) { 735 + promisor_config_info_list(repo, &config_info, fields_checked()); 736 + string_list_sort(&config_info); 737 + } 665 738 666 739 if (should_accept_remote(accept, advertised, &config_info)) 667 740 strvec_push(accepted, advertised->name);
+34
t/t5710-promisor-remote-capability.sh
··· 326 326 check_missing_objects server 1 "$oid" 327 327 ' 328 328 329 + test_expect_success "clone with promisor.checkFields" ' 330 + git -C server config promisor.advertise true && 331 + test_when_finished "rm -rf client" && 332 + 333 + git -C server remote add otherLop "https://invalid.invalid" && 334 + git -C server config remote.otherLop.token "fooBar" && 335 + git -C server config remote.otherLop.stuff "baz" && 336 + git -C server config remote.otherLop.partialCloneFilter "blob:limit=10k" && 337 + test_when_finished "git -C server remote remove otherLop" && 338 + test_config -C server promisor.sendFields "partialCloneFilter, token" && 339 + test_when_finished "rm trace" && 340 + 341 + # Clone from server to create a client 342 + GIT_TRACE_PACKET="$(pwd)/trace" GIT_NO_LAZY_FETCH=0 git clone \ 343 + -c remote.lop.promisor=true \ 344 + -c remote.lop.fetch="+refs/heads/*:refs/remotes/lop/*" \ 345 + -c remote.lop.url="file://$(pwd)/lop" \ 346 + -c remote.lop.partialCloneFilter="blob:none" \ 347 + -c promisor.acceptfromserver=All \ 348 + -c promisor.checkFields=partialcloneFilter \ 349 + --no-local --filter="blob:limit=5k" server client && 350 + 351 + # Check that fields are properly transmitted 352 + ENCODED_URL=$(echo "file://$(pwd)/lop" | sed -e "s/ /%20/g") && 353 + PR1="name=lop,url=$ENCODED_URL,partialCloneFilter=blob:none" && 354 + PR2="name=otherLop,url=https://invalid.invalid,partialCloneFilter=blob:limit=10k,token=fooBar" && 355 + test_grep "clone< promisor-remote=$PR1;$PR2" trace && 356 + test_grep "clone> promisor-remote=lop" trace && 357 + test_grep ! "clone> promisor-remote=lop;otherLop" trace && 358 + 359 + # Check that the largest object is still missing on the server 360 + check_missing_objects server 1 "$oid" 361 + ' 362 + 329 363 test_expect_success "clone with promisor.advertise set to 'true' but don't delete the client" ' 330 364 git -C server config promisor.advertise true && 331 365