{"thread":{"id":"58011","subject":"[PATCH 0/2] For v2.37.0: rename fetch.credentialsInUrl to transfer.credentialsInUrl","startedAt":"2022-06-15T10:44:24Z","lastAt":"2022-06-26T13:16:13Z","messageCount":6,"participants":["Ævar Arnfjörð Bjarmason","Derrick Stolee","Junio C Hamano","René Scharfe"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"457259","messageId":"cover-0.2-00000000000-20220615T103852Z-avarab@gmail.com","threadId":"58011","inReplyTo":null,"subject":"[PATCH 0/2] For v2.37.0: rename fetch.credentialsInUrl to transfer.credentialsInUrl","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-06-15T10:44:10Z","receivedAt":"2022-06-15T10:44:24Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"The $subject is a case where nothing is broken except potential user\nexpectations, but as argued in 2/2 I think we really should rename\nthis before we kick it out the door.\n\nHaving a \"fetch\" configuration configure \"git push\" is really\nconfusing, and once we need to support it backing out of it would be a\nhassle.\n\nThen 1/2 clarifies how it really works, and what it's aimed to do, and\nnotes the major caveat that the check doesn't cover \"pushurl\"\nconfiguration, which in some setups is the only place you'll put\npasswords in URLs. I think it likewise makes sense to have that in\nv2.37.0 so we won't lull users into a false sense of security.\n\nThis is a relatively large change for an \"rc\", but the only code\nchanges are s/fetch\\.credentialsInUrl/transfer.credentialsInUrl/g, the\nrest is all docs.\n\nThere's a trivial conflict with my just-submitted\nhttps://lore.kernel.org/git/cover-0.1-00000000000-20220615T103609Z-avarab@gmail.com:\nThe relevant \"test_expect_success\" line needs a \"LIBCURL\" prerequisite\nadded in the merge conflict.\n\nÆvar Arnfjörð Bjarmason (2):\n  fetch doc: note \"pushurl\" caveat about \"credentialsInUrl\", elaborate\n  transfer doc: move fetch.credentialsInUrl to \"transfer\" config\n    namespace\n\n Documentation/RelNotes/2.37.0.txt |  6 +++--\n Documentation/config/fetch.txt    | 14 ------------\n Documentation/config/transfer.txt | 38 +++++++++++++++++++++++++++++++\n remote.c                          |  4 ++--\n t/t5516-fetch-push.sh             | 14 ++++++------\n t/t5601-clone.sh                  | 10 ++++----\n 6 files changed, 56 insertions(+), 30 deletions(-)\n\n-- \n2.36.1.1239.gfba91521d90\n\n"},{"id":"457260","messageId":"patch-1.2-b6631cd839c-20220615T103852Z-avarab@gmail.com","threadId":"58011","inReplyTo":"cover-0.2-00000000000-20220615T103852Z-avarab@gmail.com","subject":"[PATCH 1/2] fetch doc: note \"pushurl\" caveat about \"credentialsInUrl\", elaborate","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-06-15T10:44:11Z","receivedAt":"2022-06-15T10:44:25Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Amend the documentation and release notes entry for the\n\"fetch.credentialsInUrl\" feature added in 6dcbdc0d661 (remote: create\nfetch.credentialsInUrl config, 2022-06-06), it currently doesn't\ndetect passwords in `remote.<name>.pushurl` configuration. We\nshouldn't lull users into a false sense of security, so we need to\nmention that prominently.\n\nThis also elaborates and clarifies the \"exposes the password in\nmultiple ways\" part of the documentation. As noted in [1] a user\nunfamiliar with git's implementation won't know what to make of that\nscary claim, e.g. git hypothetically have novel git-specific ways of\nexposing configured credentials.\n\nThe reality is that this configuration is intended as an aid for users\nwho can't fully trust their OS's or system's security model, so lets\nsay that's what this is intended for, and mention the most common ways\npasswords stored in configuration might inadvertently get exposed.\n\n1. https://lore.kernel.org/git/220524.86ilpuvcqh.gmgdl@evledraar.gmail.com/\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n Documentation/RelNotes/2.37.0.txt |  4 +++-\n Documentation/config/fetch.txt    | 34 +++++++++++++++++++++++++------\n 2 files changed, 31 insertions(+), 7 deletions(-)\n\ndiff --git a/Documentation/RelNotes/2.37.0.txt b/Documentation/RelNotes/2.37.0.txt\nindex 8f1ff3a5961..39ca3606dec 100644\n--- a/Documentation/RelNotes/2.37.0.txt\n+++ b/Documentation/RelNotes/2.37.0.txt\n@@ -55,7 +55,9 @@ UI, Workflows & Features\n  * Update the doctype written in gitweb output to xhtml5.\n \n  * The \"fetch.credentialsInUrl\" configuration variable controls what\n-   happens when a URL with embedded login credential is used.\n+   happens when a URL with embedded login credential is used on either\n+   \"fetch\" or \"push\". Credentials are currently only detected in\n+   `remote.<name>.url` config, not `remote.<name>.pushurl`.\n \n \n Performance, Internal Implementation, Development Support etc.\ndiff --git a/Documentation/config/fetch.txt b/Documentation/config/fetch.txt\nindex 0db7fe85bb8..827961059f8 100644\n--- a/Documentation/config/fetch.txt\n+++ b/Documentation/config/fetch.txt\n@@ -98,12 +98,34 @@ fetch.writeCommitGraph::\n \t`git push -f`, and `git log --graph`. Defaults to false.\n \n fetch.credentialsInUrl::\n-\tA URL can contain plaintext credentials in the form\n-\t`<protocol>://<user>:<password>@<domain>/<path>`. Using such URLs\n-\tis not recommended as it exposes the password in multiple ways,\n-\tincluding Git storing the URL as plaintext in the repository config.\n-\tThe `fetch.credentialsInUrl` option provides instruction for how Git\n-\tshould react to seeing such a URL, with these values:\n+\tA configured URL can contain plaintext credentials in the form\n+\t`<protocol>://<user>:<password>@<domain>/<path>`. You may want\n+\tto warn or forbid the use of such configuration (in favor of\n+\tusing linkgit:git-credential[1]).\n++\n+Note that this is currently limited to detecting credentials in\n+`remote.<name>.url` configuration, it won't detect credentials in\n+`remote.<name>.pushurl` configuration.\n++\n+You might want to enable this to prevent inadvertent credentials\n+exposure, e.g. because:\n++\n+* The OS or system where you're running git may not provide way way or\n+  otherwise allow you to configure the permissions of the\n+  configuration file where the username and/or password are stored.\n+* Even if it does, having such data stored \"at rest\" might expose you\n+  in other ways, e.g. a backup process might copy the data to another\n+  system.\n+* The git programs will pass the full URL to one another as arguments\n+  on the command-line, meaning the credentials will be exposed to oher\n+  users on OS's or systems that allow other users to see the full\n+  process list of other users. On linux the \"hidepid\" setting\n+  documented in procfs(5) allows for configuring this behavior.\n++\n+If such concerns don't apply to you then you probably don't need to be\n+concerned about credentials exposure due to storing that sensitive\n+data in git's configuration files. If you do want to use this, set\n+`fetch.credentialsInUrl` to one of these values:\n +\n * `allow` (default): Git will proceed with its activity without warning.\n * `warn`: Git will write a warning message to `stderr` when parsing a URL\n-- \n2.36.1.1239.gfba91521d90\n\n"},{"id":"457261","messageId":"patch-2.2-9eb1ff28e80-20220615T103852Z-avarab@gmail.com","threadId":"58011","inReplyTo":"cover-0.2-00000000000-20220615T103852Z-avarab@gmail.com","subject":"[PATCH 2/2] transfer doc: move fetch.credentialsInUrl to \"transfer\" config namespace","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-06-15T10:44:12Z","receivedAt":"2022-06-15T10:44:28Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Rename the \"fetch.credentialsInUrl\" configuration variable introduced\nin 6dcbdc0d661 (remote: create fetch.credentialsInUrl config,\n2022-06-06) to \"transfer\".\n\nThere are existing exceptions, but generally speaking the\n\"<namespace>.<var>\" configuration should only apply to command\ndescribed in the \"namespace\" (and its sub-commands, so e.g. \"clone.*\"\nor \"fetch.*\" might also configure \"git-remote-https\").\n\nBut in the case of \"fetch.credentialsInUrl\" we've got a configuration\nvariable that configures the behavior of all of \"clone\", \"push\" and\n\"fetch\", someone adjusting \"fetch.*\" configuration won't expect to\nhave the behavior of \"git push\" altered, especially as we have the\npre-existing \"{transfer,fetch,receive}.fsckObjects\", which configures\ndifferent parts of the transfer dialog.\n\nSo let's move this configuration variable to the \"transfer\" namespace\nbefore it's exposed in a release. We could add all of\n\"{transfer,fetch,pull}.credentialsInUrl\" at some other time, but once\nwe have \"fetch\" configure \"pull\" such an arrangement would would be a\nconfusing mess, as we'd at least need to have \"fetch\" configure\n\"push\" (but not the other way around), or change existing behavior.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n Documentation/RelNotes/2.37.0.txt |  2 +-\n Documentation/config/fetch.txt    | 36 -----------------------------\n Documentation/config/transfer.txt | 38 +++++++++++++++++++++++++++++++\n remote.c                          |  4 ++--\n t/t5516-fetch-push.sh             | 14 ++++++------\n t/t5601-clone.sh                  | 10 ++++----\n 6 files changed, 53 insertions(+), 51 deletions(-)\n\ndiff --git a/Documentation/RelNotes/2.37.0.txt b/Documentation/RelNotes/2.37.0.txt\nindex 39ca3606dec..9902a74f34a 100644\n--- a/Documentation/RelNotes/2.37.0.txt\n+++ b/Documentation/RelNotes/2.37.0.txt\n@@ -54,7 +54,7 @@ UI, Workflows & Features\n \n  * Update the doctype written in gitweb output to xhtml5.\n \n- * The \"fetch.credentialsInUrl\" configuration variable controls what\n+ * The \"transfer.credentialsInUrl\" configuration variable controls what\n    happens when a URL with embedded login credential is used on either\n    \"fetch\" or \"push\". Credentials are currently only detected in\n    `remote.<name>.url` config, not `remote.<name>.pushurl`.\ndiff --git a/Documentation/config/fetch.txt b/Documentation/config/fetch.txt\nindex 827961059f8..cd65d236b43 100644\n--- a/Documentation/config/fetch.txt\n+++ b/Documentation/config/fetch.txt\n@@ -96,39 +96,3 @@ fetch.writeCommitGraph::\n \tmerge and the write may take longer. Having an updated commit-graph\n \tfile helps performance of many Git commands, including `git merge-base`,\n \t`git push -f`, and `git log --graph`. Defaults to false.\n-\n-fetch.credentialsInUrl::\n-\tA configured URL can contain plaintext credentials in the form\n-\t`<protocol>://<user>:<password>@<domain>/<path>`. You may want\n-\tto warn or forbid the use of such configuration (in favor of\n-\tusing linkgit:git-credential[1]).\n-+\n-Note that this is currently limited to detecting credentials in\n-`remote.<name>.url` configuration, it won't detect credentials in\n-`remote.<name>.pushurl` configuration.\n-+\n-You might want to enable this to prevent inadvertent credentials\n-exposure, e.g. because:\n-+\n-* The OS or system where you're running git may not provide way way or\n-  otherwise allow you to configure the permissions of the\n-  configuration file where the username and/or password are stored.\n-* Even if it does, having such data stored \"at rest\" might expose you\n-  in other ways, e.g. a backup process might copy the data to another\n-  system.\n-* The git programs will pass the full URL to one another as arguments\n-  on the command-line, meaning the credentials will be exposed to oher\n-  users on OS's or systems that allow other users to see the full\n-  process list of other users. On linux the \"hidepid\" setting\n-  documented in procfs(5) allows for configuring this behavior.\n-+\n-If such concerns don't apply to you then you probably don't need to be\n-concerned about credentials exposure due to storing that sensitive\n-data in git's configuration files. If you do want to use this, set\n-`fetch.credentialsInUrl` to one of these values:\n-+\n-* `allow` (default): Git will proceed with its activity without warning.\n-* `warn`: Git will write a warning message to `stderr` when parsing a URL\n-  with a plaintext credential.\n-* `die`: Git will write a failure message to `stderr` when parsing a URL\n-  with a plaintext credential.\ndiff --git a/Documentation/config/transfer.txt b/Documentation/config/transfer.txt\nindex b49429eb4db..b4475c06900 100644\n--- a/Documentation/config/transfer.txt\n+++ b/Documentation/config/transfer.txt\n@@ -1,3 +1,41 @@\n+transfer.credentialsInUrl::\n+\tA configured URL can contain plaintext credentials in the form\n+\t`<protocol>://<user>:<password>@<domain>/<path>`. You may want\n+\tto warn or forbid the use of such configuration (in favor of\n+\tusing linkgit:git-credential[1]). This will be used on\n+\tlinkgit:git-clone[1], linkgit:git-fetch[1], linkgit:git-push[1],\n+\tand any other direct use of the configured URL.\n++\n+Note that this is currently limited to detecting credentials in\n+`remote.<name>.url` configuration, it won't detect credentials in\n+`remote.<name>.pushurl` configuration.\n++\n+You might want to enable this to prevent inadvertent credentials\n+exposure, e.g. because:\n++\n+* The OS or system where you're running git may not provide way way or\n+  otherwise allow you to configure the permissions of the\n+  configuration file where the username and/or password are stored.\n+* Even if it does, having such data stored \"at rest\" might expose you\n+  in other ways, e.g. a backup process might copy the data to another\n+  system.\n+* The git programs will pass the full URL to one another as arguments\n+  on the command-line, meaning the credentials will be exposed to oher\n+  users on OS's or systems that allow other users to see the full\n+  process list of other users. On linux the \"hidepid\" setting\n+  documented in procfs(5) allows for configuring this behavior.\n++\n+If such concerns don't apply to you then you probably don't need to be\n+concerned about credentials exposure due to storing that sensitive\n+data in git's configuration files. If you do want to use this, set\n+`transfer.credentialsInUrl` to one of these values:\n++\n+* `allow` (default): Git will proceed with its activity without warning.\n+* `warn`: Git will write a warning message to `stderr` when parsing a URL\n+  with a plaintext credential.\n+* `die`: Git will write a failure message to `stderr` when parsing a URL\n+  with a plaintext credential.\n+\n transfer.fsckObjects::\n \tWhen `fetch.fsckObjects` or `receive.fsckObjects` are\n \tnot set, the value of this variable is used instead.\ndiff --git a/remote.c b/remote.c\nindex 9b9bbfe80ec..42c891d44fd 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -623,7 +623,7 @@ static void validate_remote_url(struct remote *remote)\n \tstruct strbuf redacted = STRBUF_INIT;\n \tint warn_not_die;\n \n-\tif (git_config_get_string_tmp(\"fetch.credentialsinurl\", &value))\n+\tif (git_config_get_string_tmp(\"transfer.credentialsinurl\", &value))\n \t\treturn;\n \n \tif (!strcmp(\"warn\", value))\n@@ -633,7 +633,7 @@ static void validate_remote_url(struct remote *remote)\n \telse if (!strcmp(\"allow\", value))\n \t\treturn;\n \telse\n-\t\tdie(_(\"unrecognized value fetch.credentialsInURL: '%s'\"), value);\n+\t\tdie(_(\"unrecognized value transfer.credentialsInURL: '%s'\"), value);\n \n \tfor (i = 0; i < remote->url_nr; i++) {\n \t\tstruct url_info url_info = { 0 };\ndiff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh\nindex dedca106a7a..79d8a7b3675 100755\n--- a/t/t5516-fetch-push.sh\n+++ b/t/t5516-fetch-push.sh\n@@ -1836,18 +1836,18 @@ test_expect_success 'refuse to push a hidden ref, and make sure do not pollute t\n \n test_expect_success 'fetch warns or fails when using username:password' '\n \tmessage=\"URL '\\''https://username:<redacted>@localhost/'\\'' uses plaintext credentials\" &&\n-\ttest_must_fail git -c fetch.credentialsInUrl=allow fetch https://username:password@localhost 2>err &&\n+\ttest_must_fail git -c transfer.credentialsInUrl=allow fetch https://username:password@localhost 2>err &&\n \t! grep \"$message\" err &&\n \n-\ttest_must_fail git -c fetch.credentialsInUrl=warn fetch https://username:password@localhost 2>err &&\n+\ttest_must_fail git -c transfer.credentialsInUrl=warn fetch https://username:password@localhost 2>err &&\n \tgrep \"warning: $message\" err >warnings &&\n \ttest_line_count = 3 warnings &&\n \n-\ttest_must_fail git -c fetch.credentialsInUrl=die fetch https://username:password@localhost 2>err &&\n+\ttest_must_fail git -c transfer.credentialsInUrl=die fetch https://username:password@localhost 2>err &&\n \tgrep \"fatal: $message\" err >warnings &&\n \ttest_line_count = 1 warnings &&\n \n-\ttest_must_fail git -c fetch.credentialsInUrl=die fetch https://username:@localhost 2>err &&\n+\ttest_must_fail git -c transfer.credentialsInUrl=die fetch https://username:@localhost 2>err &&\n \tgrep \"fatal: $message\" err >warnings &&\n \ttest_line_count = 1 warnings\n '\n@@ -1855,12 +1855,12 @@ test_expect_success 'fetch warns or fails when using username:password' '\n \n test_expect_success 'push warns or fails when using username:password' '\n \tmessage=\"URL '\\''https://username:<redacted>@localhost/'\\'' uses plaintext credentials\" &&\n-\ttest_must_fail git -c fetch.credentialsInUrl=allow push https://username:password@localhost 2>err &&\n+\ttest_must_fail git -c transfer.credentialsInUrl=allow push https://username:password@localhost 2>err &&\n \t! grep \"$message\" err &&\n \n-\ttest_must_fail git -c fetch.credentialsInUrl=warn push https://username:password@localhost 2>err &&\n+\ttest_must_fail git -c transfer.credentialsInUrl=warn push https://username:password@localhost 2>err &&\n \tgrep \"warning: $message\" err >warnings &&\n-\ttest_must_fail git -c fetch.credentialsInUrl=die push https://username:password@localhost 2>err &&\n+\ttest_must_fail git -c transfer.credentialsInUrl=die push https://username:password@localhost 2>err &&\n \tgrep \"fatal: $message\" err >warnings &&\n \ttest_line_count = 1 warnings\n '\ndiff --git a/t/t5601-clone.sh b/t/t5601-clone.sh\nindex d2f046b4b92..e6a248bbdcc 100755\n--- a/t/t5601-clone.sh\n+++ b/t/t5601-clone.sh\n@@ -73,24 +73,24 @@ test_expect_success 'clone respects GIT_WORK_TREE' '\n \n test_expect_success 'clone warns or fails when using username:password' '\n \tmessage=\"URL '\\''https://username:<redacted>@localhost/'\\'' uses plaintext credentials\" &&\n-\ttest_must_fail git -c fetch.credentialsInUrl=allow clone https://username:password@localhost attempt1 2>err &&\n+\ttest_must_fail git -c transfer.credentialsInUrl=allow clone https://username:password@localhost attempt1 2>err &&\n \t! grep \"$message\" err &&\n \n-\ttest_must_fail git -c fetch.credentialsInUrl=warn clone https://username:password@localhost attempt2 2>err &&\n+\ttest_must_fail git -c transfer.credentialsInUrl=warn clone https://username:password@localhost attempt2 2>err &&\n \tgrep \"warning: $message\" err >warnings &&\n \ttest_line_count = 2 warnings &&\n \n-\ttest_must_fail git -c fetch.credentialsInUrl=die clone https://username:password@localhost attempt3 2>err &&\n+\ttest_must_fail git -c transfer.credentialsInUrl=die clone https://username:password@localhost attempt3 2>err &&\n \tgrep \"fatal: $message\" err >warnings &&\n \ttest_line_count = 1 warnings &&\n \n-\ttest_must_fail git -c fetch.credentialsInUrl=die clone https://username:@localhost attempt3 2>err &&\n+\ttest_must_fail git -c transfer.credentialsInUrl=die clone https://username:@localhost attempt3 2>err &&\n \tgrep \"fatal: $message\" err >warnings &&\n \ttest_line_count = 1 warnings\n '\n \n test_expect_success 'clone does not detect username:password when it is https://username@domain:port/' '\n-\ttest_must_fail git -c fetch.credentialsInUrl=warn clone https://username@localhost:8080 attempt3 2>err &&\n+\ttest_must_fail git -c transfer.credentialsInUrl=warn clone https://username@localhost:8080 attempt3 2>err &&\n \t! grep \"uses plaintext credentials\" err\n '\n \n-- \n2.36.1.1239.gfba91521d90\n\n"},{"id":"457277","messageId":"e53a247d-1d12-e34c-6cc8-3813a2fa34dc@github.com","threadId":"58011","inReplyTo":"cover-0.2-00000000000-20220615T103852Z-avarab@gmail.com","subject":"Re: [PATCH 0/2] For v2.37.0: rename fetch.credentialsInUrl to transfer.credentialsInUrl","fromName":"Derrick Stolee","fromEmail":"derrickstolee@github.com","sentAt":"2022-06-15T13:12:25Z","receivedAt":"2022-06-15T13:12:42Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 6/15/22 6:44 AM, Ævar Arnfjörð Bjarmason wrote:\n> The $subject is a case where nothing is broken except potential user\n> expectations, but as argued in 2/2 I think we really should rename\n> this before we kick it out the door.\n> \n> Having a \"fetch\" configuration configure \"git push\" is really\n> confusing, and once we need to support it backing out of it would be a\n> hassle.\n> \n> Then 1/2 clarifies how it really works, and what it's aimed to do, and\n> notes the major caveat that the check doesn't cover \"pushurl\"\n> configuration, which in some setups is the only place you'll put\n> passwords in URLs. I think it likewise makes sense to have that in\n> v2.37.0 so we won't lull users into a false sense of security.\n> \n> This is a relatively large change for an \"rc\", but the only code\n> changes are s/fetch\\.credentialsInUrl/transfer.credentialsInUrl/g, the\n> rest is all docs.\n\nSorry that I didn't see this before looking at your other RFC.\n(That RFC still has some references to \"fetch.credentialsInUrl\"\nin your commit messages, btw.)\n\nThis rename makes sense and is good to handle before the release.\nThank you for updating the release notes.\n\nThe expanded details in the docs are good.\n\nThanks,\n-Stolee\n"},{"id":"457303","messageId":"xmqqy1xxzsdw.fsf@gitster.g","threadId":"58011","inReplyTo":"e53a247d-1d12-e34c-6cc8-3813a2fa34dc@github.com","subject":"Re: [PATCH 0/2] For v2.37.0: rename fetch.credentialsInUrl to transfer.credentialsInUrl","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-06-15T18:35:55Z","receivedAt":"2022-06-15T18:36:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Derrick Stolee <derrickstolee@github.com> writes:\n\n> This rename makes sense and is good to handle before the release.\n> Thank you for updating the release notes.\n>\n> The expanded details in the docs are good.\n\nGenerally, transfer.$name hierarchy is there to give defaults to\nfetch.$name and push.$name so that the knob can be set separately\nfor different directions of the transfer, but for this particular\none, I do not see a reason why we want to redact fetch requests\nwithout redacting push requests.\n\nOne thing that worries me is that this will probably burn our early\nadopters, those who are willing to help our development by running\nversions based on 'next' and already have fetch.* configuration.\nBut the breakage this change brings on them hopefully will not be\nlarge enough to discourage them from being guinea pigs ;-)\n\nThanks, all.\n"},{"id":"457884","messageId":"3bada133-3dec-8b59-96a6-567258660f6b@web.de","threadId":"58011","inReplyTo":"patch-1.2-b6631cd839c-20220615T103852Z-avarab@gmail.com","subject":"Re: [PATCH 1/2] fetch doc: note \"pushurl\" caveat about \"credentialsInUrl\", elaborate","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2022-06-26T13:15:47Z","receivedAt":"2022-06-26T13:16:13Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 15.06.22 um 12:44 schrieb Ævar Arnfjörð Bjarmason:\n> Amend the documentation and release notes entry for the\n> \"fetch.credentialsInUrl\" feature added in 6dcbdc0d661 (remote: create\n> fetch.credentialsInUrl config, 2022-06-06), it currently doesn't\n> detect passwords in `remote.<name>.pushurl` configuration. We\n> shouldn't lull users into a false sense of security, so we need to\n> mention that prominently.\n>\n> This also elaborates and clarifies the \"exposes the password in\n> multiple ways\" part of the documentation. As noted in [1] a user\n> unfamiliar with git's implementation won't know what to make of that\n> scary claim, e.g. git hypothetically have novel git-specific ways of\n> exposing configured credentials.\n>\n> The reality is that this configuration is intended as an aid for users\n> who can't fully trust their OS's or system's security model, so lets\n> say that's what this is intended for, and mention the most common ways\n> passwords stored in configuration might inadvertently get exposed.\n>\n> 1. https://lore.kernel.org/git/220524.86ilpuvcqh.gmgdl@evledraar.gmail.com/\n>\n> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n> ---\n>  Documentation/RelNotes/2.37.0.txt |  4 +++-\n>  Documentation/config/fetch.txt    | 34 +++++++++++++++++++++++++------\n>  2 files changed, 31 insertions(+), 7 deletions(-)\n>\n> diff --git a/Documentation/RelNotes/2.37.0.txt b/Documentation/RelNotes/2.37.0.txt\n> index 8f1ff3a5961..39ca3606dec 100644\n> --- a/Documentation/RelNotes/2.37.0.txt\n> +++ b/Documentation/RelNotes/2.37.0.txt\n> @@ -55,7 +55,9 @@ UI, Workflows & Features\n>   * Update the doctype written in gitweb output to xhtml5.\n>\n>   * The \"fetch.credentialsInUrl\" configuration variable controls what\n> -   happens when a URL with embedded login credential is used.\n> +   happens when a URL with embedded login credential is used on either\n> +   \"fetch\" or \"push\". Credentials are currently only detected in\n> +   `remote.<name>.url` config, not `remote.<name>.pushurl`.\n>\n>\n>  Performance, Internal Implementation, Development Support etc.\n> diff --git a/Documentation/config/fetch.txt b/Documentation/config/fetch.txt\n> index 0db7fe85bb8..827961059f8 100644\n> --- a/Documentation/config/fetch.txt\n> +++ b/Documentation/config/fetch.txt\n> @@ -98,12 +98,34 @@ fetch.writeCommitGraph::\n>  \t`git push -f`, and `git log --graph`. Defaults to false.\n>\n>  fetch.credentialsInUrl::\n> -\tA URL can contain plaintext credentials in the form\n> -\t`<protocol>://<user>:<password>@<domain>/<path>`. Using such URLs\n> -\tis not recommended as it exposes the password in multiple ways,\n> -\tincluding Git storing the URL as plaintext in the repository config.\n> -\tThe `fetch.credentialsInUrl` option provides instruction for how Git\n> -\tshould react to seeing such a URL, with these values:\n> +\tA configured URL can contain plaintext credentials in the form\n> +\t`<protocol>://<user>:<password>@<domain>/<path>`. You may want\n> +\tto warn or forbid the use of such configuration (in favor of\n> +\tusing linkgit:git-credential[1]).\n\nWhat I want is one thing, but the more interesting point here is that\nthis option actually allows me to achieve these goals, no?  E.g. \"This\nconfiguration variable can let git warn about or reject such URLs.\".\nOtherwise this reads more as a general kind of advise along the lines\nof \"Passwords are insecure; you may want to use multi-factor\nauthentication.\".\n\n> ++\n> +Note that this is currently limited to detecting credentials in\n> +`remote.<name>.url` configuration, it won't detect credentials in\n> +`remote.<name>.pushurl` configuration.\n\nThat's a puzzling restriction.\n\n> ++\n> +You might want to enable this to prevent inadvertent credentials\n> +exposure, e.g. because:\n\nAt this point in the text \"this\" is undefined; the values and their\nmeaning are explained two paragraphs down.  Perhaps reverse the order?\n\n> ++\n> +* The OS or system where you're running git may not provide way way or\n> +  otherwise allow you to configure the permissions of the\n> +  configuration file where the username and/or password are stored.\n\nPerhaps s/way way/a way/?  In that case: What's the difference between\n\"provide a way\" and \"allow you\" in this context?  The point here is that\n\"you are unable to restrict access to the configuration file storing the\ncredentials\", right?\n\n> +* Even if it does, having such data stored \"at rest\" might expose you\n> +  in other ways, e.g. a backup process might copy the data to another\n> +  system.\n\nNo need for scare quotes here; this is a legit term.\n\n> +* The git programs will pass the full URL to one another as arguments\n> +  on the command-line, meaning the credentials will be exposed to oher\n> +  users on OS's or systems that allow other users to see the full\n> +  process list of other users. On linux the \"hidepid\" setting\n> +  documented in procfs(5) allows for configuring this behavior.\n\n\"oher\" has been fixed already.  Linux should get a capital L.\n\n> ++\n> +If such concerns don't apply to you then you probably don't need to be\n> +concerned about credentials exposure due to storing that sensitive\n> +data in git's configuration files. If you do want to use this, set\n> +`fetch.credentialsInUrl` to one of these values:\n\nSo I'm safe as long as I don't make backups, right? ;-)\n\nCleartext passwords are a security risk and advising not to be concerned\nabout storing them in a config file is a bit scary.  A better default\nwould probably be to reject URLs with credentials and make credential\nhelpers at least as convenient to use as a plaintext file (if they\naren't already) -- but that's just a tangent from a slightly paranoid\nadmin type.  My point is that the tone seems a bit off.\n\n>  +\n>  * `allow` (default): Git will proceed with its activity without warning.\n>  * `warn`: Git will write a warning message to `stderr` when parsing a URL\n\n"}]}