{"thread":{"id":"62678","subject":"[PATCH] credential-cache: respect request capabilities","startedAt":"2024-12-20T21:18:57Z","lastAt":"2025-01-18T20:14:56Z","messageCount":11,"participants":["M Hickford via GitGitGadget","brian m. carlson","M Hickford","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"509452","messageId":"pull.1842.git.1734729534213.gitgitgadget@gmail.com","threadId":"62678","inReplyTo":null,"subject":"[PATCH] credential-cache: respect request capabilities","fromName":"M Hickford via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-12-20T21:18:53Z","receivedAt":"2024-12-20T21:18:57Z","isPatch":true,"sender":{"key":"mirth.hickford@gmail.com","avatar":"https://avatars.githubusercontent.com/u/105314?v=4"},"body":"From: M Hickford <mirth.hickford@gmail.com>\n\nPreviously, credential-cache responded with capability[]=authtype\nregardless of request.\n\nThe capabilities in a credential helper response should be a subset of\nthe capabilities in the request.\n\nSigned-off-by: M Hickford <mirth.hickford@gmail.com>\n---\n    credential-cache: respect request capabilities\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1842%2Fhickford%2Fcache-capability-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1842/hickford/cache-capability-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/1842\n\n builtin/credential-cache--daemon.c | 11 +++++------\n t/lib-credential.sh                | 15 +++++++++++++++\n t/t0303-credential-external.sh     |  1 +\n 3 files changed, 21 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin/credential-cache--daemon.c b/builtin/credential-cache--daemon.c\nindex bc22f5c6d24..692216cf83c 100644\n--- a/builtin/credential-cache--daemon.c\n+++ b/builtin/credential-cache--daemon.c\n@@ -134,17 +134,16 @@ static void serve_one_client(FILE *in, FILE *out)\n \telse if (!strcmp(action.buf, \"get\")) {\n \t\tstruct credential_cache_entry *e = lookup_credential(&c);\n \t\tif (e) {\n-\t\t\te->item.capa_authtype.request_initial = 1;\n-\t\t\te->item.capa_authtype.request_helper = 1;\n-\n-\t\t\tfprintf(out, \"capability[]=authtype\\n\");\n+\t\t\tif (credential_has_capability(&c.capa_authtype, CREDENTIAL_OP_RESPONSE)) {\n+\t\t\t\tfprintf(out, \"capability[]=authtype\\n\");\n+\t\t\t}\n \t\t\tif (e->item.username)\n \t\t\t\tfprintf(out, \"username=%s\\n\", e->item.username);\n \t\t\tif (e->item.password)\n \t\t\t\tfprintf(out, \"password=%s\\n\", e->item.password);\n-\t\t\tif (credential_has_capability(&c.capa_authtype, CREDENTIAL_OP_HELPER) && e->item.authtype)\n+\t\t\tif (credential_has_capability(&c.capa_authtype, CREDENTIAL_OP_RESPONSE) && e->item.authtype)\n \t\t\t\tfprintf(out, \"authtype=%s\\n\", e->item.authtype);\n-\t\t\tif (credential_has_capability(&c.capa_authtype, CREDENTIAL_OP_HELPER) && e->item.credential)\n+\t\t\tif (credential_has_capability(&c.capa_authtype, CREDENTIAL_OP_RESPONSE) && e->item.credential)\n \t\t\t\tfprintf(out, \"credential=%s\\n\", e->item.credential);\n \t\t\tif (e->item.password_expiry_utc != TIME_MAX)\n \t\t\t\tfprintf(out, \"password_expiry_utc=%\"PRItime\"\\n\",\ndiff --git a/t/lib-credential.sh b/t/lib-credential.sh\nindex 58b9c740605..fe170b513fd 100644\n--- a/t/lib-credential.sh\n+++ b/t/lib-credential.sh\n@@ -566,6 +566,21 @@ helper_test_authtype() {\n \t\tEOF\n \t'\n \n+\ttest_expect_success \"helper ($HELPER) does not get authtype and credential without authtype capability\" '\n+\t\tcheck fill $HELPER <<-\\EOF\n+\t\tprotocol=https\n+\t\thost=git.example.com\n+\t\t--\n+\t\tprotocol=https\n+\t\thost=git.example.com\n+\t\tusername=askpass-username\n+\t\tpassword=askpass-password\n+\t\t--\n+\t\taskpass: Username for '\\''https://git.example.com'\\'':\n+\t\taskpass: Password for '\\''https://askpass-username@git.example.com'\\'':\n+\t\tEOF\n+\t'\n+\n \ttest_expect_success \"helper ($HELPER) stores authtype and credential with username\" '\n \t\tcheck approve $HELPER <<-\\EOF\n \t\tcapability[]=authtype\ndiff --git a/t/t0303-credential-external.sh b/t/t0303-credential-external.sh\nindex 8aadbe86c45..437eae5002a 100755\n--- a/t/t0303-credential-external.sh\n+++ b/t/t0303-credential-external.sh\n@@ -63,6 +63,7 @@ helper_test_clean \"$GIT_TEST_CREDENTIAL_HELPER\"\n helper_test \"$GIT_TEST_CREDENTIAL_HELPER\"\n helper_test_password_expiry_utc \"$GIT_TEST_CREDENTIAL_HELPER\"\n helper_test_oauth_refresh_token \"$GIT_TEST_CREDENTIAL_HELPER\"\n+helper_test_authtype \"$GIT_TEST_CREDENTIAL_HELPER\"\n \n if test -z \"$GIT_TEST_CREDENTIAL_HELPER_TIMEOUT\"; then\n \tsay \"# skipping timeout tests (GIT_TEST_CREDENTIAL_HELPER_TIMEOUT not set)\"\n\nbase-commit: 92999a42db1c5f43f330e4f2bca4026b5b81576f\n-- \ngitgitgadget\n"},{"id":"510013","messageId":"pull.1842.v2.git.1736193131798.gitgitgadget@gmail.com","threadId":"62678","inReplyTo":"pull.1842.git.1734729534213.gitgitgadget@gmail.com","subject":"[PATCH v2] credential-cache: respect request capabilities","fromName":"M Hickford via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-01-06T19:52:11Z","receivedAt":"2025-01-06T19:52:15Z","isPatch":true,"sender":{"key":"mirth.hickford@gmail.com","avatar":"https://avatars.githubusercontent.com/u/105314?v=4"},"body":"From: M Hickford <mirth.hickford@gmail.com>\n\nPreviously, credential-cache responded with capability[]=authtype\nregardless of request.\n\nThe capabilities in a credential helper response should be a subset of\nthe capabilities in the request.\n\nSigned-off-by: M Hickford <mirth.hickford@gmail.com>\n---\n    credential-cache: respect request capabilities\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1842%2Fhickford%2Fcache-capability-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1842/hickford/cache-capability-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/1842\n\nRange-diff vs v1:\n\n 1:  9197941029f ! 1:  696780d4782 credential-cache: respect request capabilities\n     @@ t/lib-credential.sh: helper_test_authtype() {\n       \t\tEOF\n       \t'\n       \n     -+\ttest_expect_success \"helper ($HELPER) does not get authtype and credential without authtype capability\" '\n     ++\ttest_expect_success \"helper ($HELPER) get authtype only if request has authtype capability\" '\n      +\t\tcheck fill $HELPER <<-\\EOF\n      +\t\tprotocol=https\n      +\t\thost=git.example.com\n     @@ t/lib-credential.sh: helper_test_authtype() {\n       \ttest_expect_success \"helper ($HELPER) stores authtype and credential with username\" '\n       \t\tcheck approve $HELPER <<-\\EOF\n       \t\tcapability[]=authtype\n     -\n     - ## t/t0303-credential-external.sh ##\n     -@@ t/t0303-credential-external.sh: helper_test_clean \"$GIT_TEST_CREDENTIAL_HELPER\"\n     - helper_test \"$GIT_TEST_CREDENTIAL_HELPER\"\n     - helper_test_password_expiry_utc \"$GIT_TEST_CREDENTIAL_HELPER\"\n     - helper_test_oauth_refresh_token \"$GIT_TEST_CREDENTIAL_HELPER\"\n     -+helper_test_authtype \"$GIT_TEST_CREDENTIAL_HELPER\"\n     - \n     - if test -z \"$GIT_TEST_CREDENTIAL_HELPER_TIMEOUT\"; then\n     - \tsay \"# skipping timeout tests (GIT_TEST_CREDENTIAL_HELPER_TIMEOUT not set)\"\n\n\n builtin/credential-cache--daemon.c | 11 +++++------\n t/lib-credential.sh                | 15 +++++++++++++++\n 2 files changed, 20 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin/credential-cache--daemon.c b/builtin/credential-cache--daemon.c\nindex bc22f5c6d24..692216cf83c 100644\n--- a/builtin/credential-cache--daemon.c\n+++ b/builtin/credential-cache--daemon.c\n@@ -134,17 +134,16 @@ static void serve_one_client(FILE *in, FILE *out)\n \telse if (!strcmp(action.buf, \"get\")) {\n \t\tstruct credential_cache_entry *e = lookup_credential(&c);\n \t\tif (e) {\n-\t\t\te->item.capa_authtype.request_initial = 1;\n-\t\t\te->item.capa_authtype.request_helper = 1;\n-\n-\t\t\tfprintf(out, \"capability[]=authtype\\n\");\n+\t\t\tif (credential_has_capability(&c.capa_authtype, CREDENTIAL_OP_RESPONSE)) {\n+\t\t\t\tfprintf(out, \"capability[]=authtype\\n\");\n+\t\t\t}\n \t\t\tif (e->item.username)\n \t\t\t\tfprintf(out, \"username=%s\\n\", e->item.username);\n \t\t\tif (e->item.password)\n \t\t\t\tfprintf(out, \"password=%s\\n\", e->item.password);\n-\t\t\tif (credential_has_capability(&c.capa_authtype, CREDENTIAL_OP_HELPER) && e->item.authtype)\n+\t\t\tif (credential_has_capability(&c.capa_authtype, CREDENTIAL_OP_RESPONSE) && e->item.authtype)\n \t\t\t\tfprintf(out, \"authtype=%s\\n\", e->item.authtype);\n-\t\t\tif (credential_has_capability(&c.capa_authtype, CREDENTIAL_OP_HELPER) && e->item.credential)\n+\t\t\tif (credential_has_capability(&c.capa_authtype, CREDENTIAL_OP_RESPONSE) && e->item.credential)\n \t\t\t\tfprintf(out, \"credential=%s\\n\", e->item.credential);\n \t\t\tif (e->item.password_expiry_utc != TIME_MAX)\n \t\t\t\tfprintf(out, \"password_expiry_utc=%\"PRItime\"\\n\",\ndiff --git a/t/lib-credential.sh b/t/lib-credential.sh\nindex 58b9c740605..324ecc792d5 100644\n--- a/t/lib-credential.sh\n+++ b/t/lib-credential.sh\n@@ -566,6 +566,21 @@ helper_test_authtype() {\n \t\tEOF\n \t'\n \n+\ttest_expect_success \"helper ($HELPER) get authtype only if request has authtype capability\" '\n+\t\tcheck fill $HELPER <<-\\EOF\n+\t\tprotocol=https\n+\t\thost=git.example.com\n+\t\t--\n+\t\tprotocol=https\n+\t\thost=git.example.com\n+\t\tusername=askpass-username\n+\t\tpassword=askpass-password\n+\t\t--\n+\t\taskpass: Username for '\\''https://git.example.com'\\'':\n+\t\taskpass: Password for '\\''https://askpass-username@git.example.com'\\'':\n+\t\tEOF\n+\t'\n+\n \ttest_expect_success \"helper ($HELPER) stores authtype and credential with username\" '\n \t\tcheck approve $HELPER <<-\\EOF\n \t\tcapability[]=authtype\n\nbase-commit: 92999a42db1c5f43f330e4f2bca4026b5b81576f\n-- \ngitgitgadget\n"},{"id":"510026","messageId":"Z3xaE_v45G447HQe@tapette.crustytoothpaste.net","threadId":"62678","inReplyTo":"pull.1842.v2.git.1736193131798.gitgitgadget@gmail.com","subject":"Re: [PATCH v2] credential-cache: respect request capabilities","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2025-01-06T22:32:51Z","receivedAt":"2025-01-06T22:32:53Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2025-01-06 at 19:52:11, M Hickford via GitGitGadget wrote:\n> From: M Hickford <mirth.hickford@gmail.com>\n> \n> Previously, credential-cache responded with capability[]=authtype\n> regardless of request.\n\nThat's the correct behaviour.\n\n> The capabilities in a credential helper response should be a subset of\n> the capabilities in the request.\n\nNo, it should not.  Otherwise, it's impossible for Git to know whether\nthe helper does or does not support the capability.  We rely on that\ninformation to correctly pass data back when saving data.\n\n> diff --git a/builtin/credential-cache--daemon.c b/builtin/credential-cache--daemon.c\n> index bc22f5c6d24..692216cf83c 100644\n> --- a/builtin/credential-cache--daemon.c\n> +++ b/builtin/credential-cache--daemon.c\n> @@ -134,17 +134,16 @@ static void serve_one_client(FILE *in, FILE *out)\n>  \telse if (!strcmp(action.buf, \"get\")) {\n>  \t\tstruct credential_cache_entry *e = lookup_credential(&c);\n>  \t\tif (e) {\n> -\t\t\te->item.capa_authtype.request_initial = 1;\n> -\t\t\te->item.capa_authtype.request_helper = 1;\n> -\n> -\t\t\tfprintf(out, \"capability[]=authtype\\n\");\n> +\t\t\tif (credential_has_capability(&c.capa_authtype, CREDENTIAL_OP_RESPONSE)) {\n> +\t\t\t\tfprintf(out, \"capability[]=authtype\\n\");\n> +\t\t\t}\n\nThis part is not correct.\n\n>  \t\t\tif (e->item.username)\n>  \t\t\t\tfprintf(out, \"username=%s\\n\", e->item.username);\n>  \t\t\tif (e->item.password)\n>  \t\t\t\tfprintf(out, \"password=%s\\n\", e->item.password);\n> -\t\t\tif (credential_has_capability(&c.capa_authtype, CREDENTIAL_OP_HELPER) && e->item.authtype)\n> +\t\t\tif (credential_has_capability(&c.capa_authtype, CREDENTIAL_OP_RESPONSE) && e->item.authtype)\n>  \t\t\t\tfprintf(out, \"authtype=%s\\n\", e->item.authtype);\n> -\t\t\tif (credential_has_capability(&c.capa_authtype, CREDENTIAL_OP_HELPER) && e->item.credential)\n> +\t\t\tif (credential_has_capability(&c.capa_authtype, CREDENTIAL_OP_RESPONSE) && e->item.credential)\n\nThis part may very well be correct.\n\n>  \t\t\t\tfprintf(out, \"credential=%s\\n\", e->item.credential);\n>  \t\t\tif (e->item.password_expiry_utc != TIME_MAX)\n>  \t\t\t\tfprintf(out, \"password_expiry_utc=%\"PRItime\"\\n\",\n> diff --git a/t/lib-credential.sh b/t/lib-credential.sh\n> index 58b9c740605..324ecc792d5 100644\n> --- a/t/lib-credential.sh\n> +++ b/t/lib-credential.sh\n> @@ -566,6 +566,21 @@ helper_test_authtype() {\n>  \t\tEOF\n>  \t'\n>  \n> +\ttest_expect_success \"helper ($HELPER) get authtype only if request has authtype capability\" '\n> +\t\tcheck fill $HELPER <<-\\EOF\n> +\t\tprotocol=https\n> +\t\thost=git.example.com\n> +\t\t--\n> +\t\tprotocol=https\n> +\t\thost=git.example.com\n> +\t\tusername=askpass-username\n> +\t\tpassword=askpass-password\n> +\t\t--\n> +\t\taskpass: Username for '\\''https://git.example.com'\\'':\n> +\t\taskpass: Password for '\\''https://askpass-username@git.example.com'\\'':\n> +\t\tEOF\n> +\t'\n> +\n>  \ttest_expect_success \"helper ($HELPER) stores authtype and credential with username\" '\n>  \t\tcheck approve $HELPER <<-\\EOF\n>  \t\tcapability[]=authtype\n> \n> base-commit: 92999a42db1c5f43f330e4f2bca4026b5b81576f\n> -- \n> gitgitgadget\n\n-- \nbrian m. carlson (they/them or he/him)\nToronto, Ontario, CA\n"},{"id":"510028","messageId":"CAGJzqsn6kV4LeYKk=qWu3GvbtNrJ5LG9LvbDVMQoHqibR9ieSA@mail.gmail.com","threadId":"62678","inReplyTo":"Z3xaE_v45G447HQe@tapette.crustytoothpaste.net","subject":"Re: [PATCH v2] credential-cache: respect request capabilities","fromName":"M Hickford","fromEmail":"mirth.hickford@gmail.com","sentAt":"2025-01-06T22:57:06Z","receivedAt":"2025-01-06T22:58:00Z","isPatch":true,"sender":{"key":"mirth.hickford@gmail.com","avatar":"https://avatars.githubusercontent.com/u/105314?v=4"},"body":"On Mon, 6 Jan 2025 at 22:32, brian m. carlson\n<sandals@crustytoothpaste.net> wrote:\n>\n> On 2025-01-06 at 19:52:11, M Hickford via GitGitGadget wrote:\n> > From: M Hickford <mirth.hickford@gmail.com>\n> >\n> > Previously, credential-cache responded with capability[]=authtype\n> > regardless of request.\n>\n> That's the correct behaviour.\n>\n> > The capabilities in a credential helper response should be a subset of\n> > the capabilities in the request.\n>\n> No, it should not.  Otherwise, it's impossible for Git to know whether\n> the helper does or does not support the capability.  We rely on that\n> information to correctly pass data back when saving data.\n>\n> > diff --git a/builtin/credential-cache--daemon.c b/builtin/credential-cache--daemon.c\n> > index bc22f5c6d24..692216cf83c 100644\n> > --- a/builtin/credential-cache--daemon.c\n> > +++ b/builtin/credential-cache--daemon.c\n> > @@ -134,17 +134,16 @@ static void serve_one_client(FILE *in, FILE *out)\n> >       else if (!strcmp(action.buf, \"get\")) {\n> >               struct credential_cache_entry *e = lookup_credential(&c);\n> >               if (e) {\n> > -                     e->item.capa_authtype.request_initial = 1;\n> > -                     e->item.capa_authtype.request_helper = 1;\n> > -\n> > -                     fprintf(out, \"capability[]=authtype\\n\");\n> > +                     if (credential_has_capability(&c.capa_authtype, CREDENTIAL_OP_RESPONSE)) {\n> > +                             fprintf(out, \"capability[]=authtype\\n\");\n> > +                     }\n>\n> This part is not correct.\n\nThanks for the review. I'll revert this part and amend the commit message.\n\n>\n> >                       if (e->item.username)\n> >                               fprintf(out, \"username=%s\\n\", e->item.username);\n> >                       if (e->item.password)\n> >                               fprintf(out, \"password=%s\\n\", e->item.password);\n> > -                     if (credential_has_capability(&c.capa_authtype, CREDENTIAL_OP_HELPER) && e->item.authtype)\n> > +                     if (credential_has_capability(&c.capa_authtype, CREDENTIAL_OP_RESPONSE) && e->item.authtype)\n> >                               fprintf(out, \"authtype=%s\\n\", e->item.authtype);\n> > -                     if (credential_has_capability(&c.capa_authtype, CREDENTIAL_OP_HELPER) && e->item.credential)\n> > +                     if (credential_has_capability(&c.capa_authtype, CREDENTIAL_OP_RESPONSE) && e->item.credential)\n>\n> This part may very well be correct.\n>\n> >                               fprintf(out, \"credential=%s\\n\", e->item.credential);\n> >                       if (e->item.password_expiry_utc != TIME_MAX)\n> >                               fprintf(out, \"password_expiry_utc=%\"PRItime\"\\n\",\n> > diff --git a/t/lib-credential.sh b/t/lib-credential.sh\n> > index 58b9c740605..324ecc792d5 100644\n> > --- a/t/lib-credential.sh\n> > +++ b/t/lib-credential.sh\n> > @@ -566,6 +566,21 @@ helper_test_authtype() {\n> >               EOF\n> >       '\n> >\n> > +     test_expect_success \"helper ($HELPER) get authtype only if request has authtype capability\" '\n> > +             check fill $HELPER <<-\\EOF\n> > +             protocol=https\n> > +             host=git.example.com\n> > +             --\n> > +             protocol=https\n> > +             host=git.example.com\n> > +             username=askpass-username\n> > +             password=askpass-password\n> > +             --\n> > +             askpass: Username for '\\''https://git.example.com'\\'':\n> > +             askpass: Password for '\\''https://askpass-username@git.example.com'\\'':\n> > +             EOF\n> > +     '\n> > +\n> >       test_expect_success \"helper ($HELPER) stores authtype and credential with username\" '\n> >               check approve $HELPER <<-\\EOF\n> >               capability[]=authtype\n> >\n> > base-commit: 92999a42db1c5f43f330e4f2bca4026b5b81576f\n> > --\n> > gitgitgadget\n>\n> --\n> brian m. carlson (they/them or he/him)\n> Toronto, Ontario, CA\n"},{"id":"510030","messageId":"Z3xhqCf7Gr74BHO4@tapette.crustytoothpaste.net","threadId":"62678","inReplyTo":"CAGJzqsn6kV4LeYKk=qWu3GvbtNrJ5LG9LvbDVMQoHqibR9ieSA@mail.gmail.com","subject":"Re: [PATCH v2] credential-cache: respect request capabilities","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2025-01-06T23:05:12Z","receivedAt":"2025-01-06T23:05:14Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2025-01-06 at 22:57:06, M Hickford wrote:\n> On Mon, 6 Jan 2025 at 22:32, brian m. carlson\n> <sandals@crustytoothpaste.net> wrote:\n> >\n> > On 2025-01-06 at 19:52:11, M Hickford via GitGitGadget wrote:\n> > > From: M Hickford <mirth.hickford@gmail.com>\n> > > diff --git a/builtin/credential-cache--daemon.c b/builtin/credential-cache--daemon.c\n> > > index bc22f5c6d24..692216cf83c 100644\n> > > --- a/builtin/credential-cache--daemon.c\n> > > +++ b/builtin/credential-cache--daemon.c\n> > > @@ -134,17 +134,16 @@ static void serve_one_client(FILE *in, FILE *out)\n> > >       else if (!strcmp(action.buf, \"get\")) {\n> > >               struct credential_cache_entry *e = lookup_credential(&c);\n> > >               if (e) {\n> > > -                     e->item.capa_authtype.request_initial = 1;\n> > > -                     e->item.capa_authtype.request_helper = 1;\n> > > -\n> > > -                     fprintf(out, \"capability[]=authtype\\n\");\n> > > +                     if (credential_has_capability(&c.capa_authtype, CREDENTIAL_OP_RESPONSE)) {\n> > > +                             fprintf(out, \"capability[]=authtype\\n\");\n> > > +                     }\n> >\n> > This part is not correct.\n> \n> Thanks for the review. I'll revert this part and amend the commit message.\n\nI applied this without that change and it does still pass the test,\nwhich I think is good and shows that can be omitted.  If I have some\ntime, I may send a follow-up patch to add some additional tests.\n-- \nbrian m. carlson (they/them or he/him)\nToronto, Ontario, CA\n"},{"id":"510031","messageId":"pull.1842.v3.git.1736204756030.gitgitgadget@gmail.com","threadId":"62678","inReplyTo":"pull.1842.v2.git.1736193131798.gitgitgadget@gmail.com","subject":"[PATCH v3] credential-cache: respect request capabilities","fromName":"M Hickford via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-01-06T23:05:55Z","receivedAt":"2025-01-06T23:05:59Z","isPatch":true,"sender":{"key":"mirth.hickford@gmail.com","avatar":"https://avatars.githubusercontent.com/u/105314?v=4"},"body":"From: M Hickford <mirth.hickford@gmail.com>\n\nPreviously, credential-cache populated authtype regardless of request.\n\nSigned-off-by: M Hickford <mirth.hickford@gmail.com>\n---\n    credential-cache: respect request capabilities\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1842%2Fhickford%2Fcache-capability-v3\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1842/hickford/cache-capability-v3\nPull-Request: https://github.com/gitgitgadget/git/pull/1842\n\nRange-diff vs v2:\n\n 1:  696780d4782 ! 1:  e9851c5c4ac credential-cache: respect request capabilities\n     @@ Metadata\n       ## Commit message ##\n          credential-cache: respect request capabilities\n      \n     -    Previously, credential-cache responded with capability[]=authtype\n     -    regardless of request.\n     -\n     -    The capabilities in a credential helper response should be a subset of\n     -    the capabilities in the request.\n     +    Previously, credential-cache populated authtype regardless of request.\n      \n          Signed-off-by: M Hickford <mirth.hickford@gmail.com>\n      \n       ## builtin/credential-cache--daemon.c ##\n      @@ builtin/credential-cache--daemon.c: static void serve_one_client(FILE *in, FILE *out)\n     - \telse if (!strcmp(action.buf, \"get\")) {\n     - \t\tstruct credential_cache_entry *e = lookup_credential(&c);\n     - \t\tif (e) {\n     --\t\t\te->item.capa_authtype.request_initial = 1;\n     --\t\t\te->item.capa_authtype.request_helper = 1;\n     --\n     --\t\t\tfprintf(out, \"capability[]=authtype\\n\");\n     -+\t\t\tif (credential_has_capability(&c.capa_authtype, CREDENTIAL_OP_RESPONSE)) {\n     -+\t\t\t\tfprintf(out, \"capability[]=authtype\\n\");\n     -+\t\t\t}\n     - \t\t\tif (e->item.username)\n       \t\t\t\tfprintf(out, \"username=%s\\n\", e->item.username);\n       \t\t\tif (e->item.password)\n       \t\t\t\tfprintf(out, \"password=%s\\n\", e->item.password);\n     @@ t/lib-credential.sh: helper_test_authtype() {\n       \t\tEOF\n       \t'\n       \n     -+\ttest_expect_success \"helper ($HELPER) get authtype only if request has authtype capability\" '\n     ++\ttest_expect_success \"helper ($HELPER) gets authtype and credential only if request has authtype capability\" '\n      +\t\tcheck fill $HELPER <<-\\EOF\n      +\t\tprotocol=https\n      +\t\thost=git.example.com\n      +\t\t--\n     ++\t\tcapability[]=authtype\n      +\t\tprotocol=https\n      +\t\thost=git.example.com\n      +\t\tusername=askpass-username\n\n\n builtin/credential-cache--daemon.c |  4 ++--\n t/lib-credential.sh                | 16 ++++++++++++++++\n 2 files changed, 18 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/credential-cache--daemon.c b/builtin/credential-cache--daemon.c\nindex bc22f5c6d24..e707618e743 100644\n--- a/builtin/credential-cache--daemon.c\n+++ b/builtin/credential-cache--daemon.c\n@@ -142,9 +142,9 @@ static void serve_one_client(FILE *in, FILE *out)\n \t\t\t\tfprintf(out, \"username=%s\\n\", e->item.username);\n \t\t\tif (e->item.password)\n \t\t\t\tfprintf(out, \"password=%s\\n\", e->item.password);\n-\t\t\tif (credential_has_capability(&c.capa_authtype, CREDENTIAL_OP_HELPER) && e->item.authtype)\n+\t\t\tif (credential_has_capability(&c.capa_authtype, CREDENTIAL_OP_RESPONSE) && e->item.authtype)\n \t\t\t\tfprintf(out, \"authtype=%s\\n\", e->item.authtype);\n-\t\t\tif (credential_has_capability(&c.capa_authtype, CREDENTIAL_OP_HELPER) && e->item.credential)\n+\t\t\tif (credential_has_capability(&c.capa_authtype, CREDENTIAL_OP_RESPONSE) && e->item.credential)\n \t\t\t\tfprintf(out, \"credential=%s\\n\", e->item.credential);\n \t\t\tif (e->item.password_expiry_utc != TIME_MAX)\n \t\t\t\tfprintf(out, \"password_expiry_utc=%\"PRItime\"\\n\",\ndiff --git a/t/lib-credential.sh b/t/lib-credential.sh\nindex 58b9c740605..8da0afe9395 100644\n--- a/t/lib-credential.sh\n+++ b/t/lib-credential.sh\n@@ -566,6 +566,22 @@ helper_test_authtype() {\n \t\tEOF\n \t'\n \n+\ttest_expect_success \"helper ($HELPER) gets authtype and credential only if request has authtype capability\" '\n+\t\tcheck fill $HELPER <<-\\EOF\n+\t\tprotocol=https\n+\t\thost=git.example.com\n+\t\t--\n+\t\tcapability[]=authtype\n+\t\tprotocol=https\n+\t\thost=git.example.com\n+\t\tusername=askpass-username\n+\t\tpassword=askpass-password\n+\t\t--\n+\t\taskpass: Username for '\\''https://git.example.com'\\'':\n+\t\taskpass: Password for '\\''https://askpass-username@git.example.com'\\'':\n+\t\tEOF\n+\t'\n+\n \ttest_expect_success \"helper ($HELPER) stores authtype and credential with username\" '\n \t\tcheck approve $HELPER <<-\\EOF\n \t\tcapability[]=authtype\n\nbase-commit: 92999a42db1c5f43f330e4f2bca4026b5b81576f\n-- \ngitgitgadget\n"},{"id":"510036","messageId":"pull.1842.v4.git.1736212760709.gitgitgadget@gmail.com","threadId":"62678","inReplyTo":"pull.1842.v3.git.1736204756030.gitgitgadget@gmail.com","subject":"[PATCH v4] credential-cache: respect request capabilities","fromName":"M Hickford via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-01-07T01:19:20Z","receivedAt":"2025-01-07T01:19:24Z","isPatch":true,"sender":{"key":"mirth.hickford@gmail.com","avatar":"https://avatars.githubusercontent.com/u/105314?v=4"},"body":"From: M Hickford <mirth.hickford@gmail.com>\n\nPreviously, credential-cache populated authtype regardless of request.\n\nSigned-off-by: M Hickford <mirth.hickford@gmail.com>\n---\n    credential-cache: respect request capabilities\n    \n    CC: sandals@crustytoothpaste.net\n    \n    Patch v4 fixes test\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1842%2Fhickford%2Fcache-capability-v4\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1842/hickford/cache-capability-v4\nPull-Request: https://github.com/gitgitgadget/git/pull/1842\n\nRange-diff vs v3:\n\n 1:  e9851c5c4ac ! 1:  23942f9fa47 credential-cache: respect request capabilities\n     @@ t/lib-credential.sh: helper_test_authtype() {\n      +\t\tprotocol=https\n      +\t\thost=git.example.com\n      +\t\t--\n     -+\t\tcapability[]=authtype\n      +\t\tprotocol=https\n      +\t\thost=git.example.com\n      +\t\tusername=askpass-username\n\n\n builtin/credential-cache--daemon.c |  4 ++--\n t/lib-credential.sh                | 15 +++++++++++++++\n 2 files changed, 17 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/credential-cache--daemon.c b/builtin/credential-cache--daemon.c\nindex bc22f5c6d24..e707618e743 100644\n--- a/builtin/credential-cache--daemon.c\n+++ b/builtin/credential-cache--daemon.c\n@@ -142,9 +142,9 @@ static void serve_one_client(FILE *in, FILE *out)\n \t\t\t\tfprintf(out, \"username=%s\\n\", e->item.username);\n \t\t\tif (e->item.password)\n \t\t\t\tfprintf(out, \"password=%s\\n\", e->item.password);\n-\t\t\tif (credential_has_capability(&c.capa_authtype, CREDENTIAL_OP_HELPER) && e->item.authtype)\n+\t\t\tif (credential_has_capability(&c.capa_authtype, CREDENTIAL_OP_RESPONSE) && e->item.authtype)\n \t\t\t\tfprintf(out, \"authtype=%s\\n\", e->item.authtype);\n-\t\t\tif (credential_has_capability(&c.capa_authtype, CREDENTIAL_OP_HELPER) && e->item.credential)\n+\t\t\tif (credential_has_capability(&c.capa_authtype, CREDENTIAL_OP_RESPONSE) && e->item.credential)\n \t\t\t\tfprintf(out, \"credential=%s\\n\", e->item.credential);\n \t\t\tif (e->item.password_expiry_utc != TIME_MAX)\n \t\t\t\tfprintf(out, \"password_expiry_utc=%\"PRItime\"\\n\",\ndiff --git a/t/lib-credential.sh b/t/lib-credential.sh\nindex 58b9c740605..cc6bf9aa5f3 100644\n--- a/t/lib-credential.sh\n+++ b/t/lib-credential.sh\n@@ -566,6 +566,21 @@ helper_test_authtype() {\n \t\tEOF\n \t'\n \n+\ttest_expect_success \"helper ($HELPER) gets authtype and credential only if request has authtype capability\" '\n+\t\tcheck fill $HELPER <<-\\EOF\n+\t\tprotocol=https\n+\t\thost=git.example.com\n+\t\t--\n+\t\tprotocol=https\n+\t\thost=git.example.com\n+\t\tusername=askpass-username\n+\t\tpassword=askpass-password\n+\t\t--\n+\t\taskpass: Username for '\\''https://git.example.com'\\'':\n+\t\taskpass: Password for '\\''https://askpass-username@git.example.com'\\'':\n+\t\tEOF\n+\t'\n+\n \ttest_expect_success \"helper ($HELPER) stores authtype and credential with username\" '\n \t\tcheck approve $HELPER <<-\\EOF\n \t\tcapability[]=authtype\n\nbase-commit: 92999a42db1c5f43f330e4f2bca4026b5b81576f\n-- \ngitgitgadget\n"},{"id":"510139","messageId":"xmqqttaaoyaz.fsf@gitster.g","threadId":"62678","inReplyTo":"pull.1842.v4.git.1736212760709.gitgitgadget@gmail.com","subject":"Re: [PATCH v4] credential-cache: respect request capabilities","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-01-08T02:05:08Z","receivedAt":"2025-01-08T02:05:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"M Hickford via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n\n> From: M Hickford <mirth.hickford@gmail.com>\n>\n> Previously, credential-cache populated authtype regardless of request.\n\nOK, that may be a correct statement of the fact, but it does not\ntell readers any of the following:\n\n - If it is a bad thing to populate authtype regardless of request,\n   and if so why?\n\n - What is the (negative) consequence of doing so, if any?  What\n   breaks because it populates authtype regardless of request?\n\n - What is the remedy?  Instead of unconditionally populating the\n   authtype, how does the new code decide when to populate it and\n   with what value?\n\n - Can there be downsides of fixing this?  Are there use cases where\n   this unconditional population of authtype was relied upon?\n\n - Where did the bug come from and what is its fix?  We used to look\n   at OP_HELPER to decide when to emit authtype, but the updated\n   code checks OP_RESPONSE, which readers can see in the patch.\n\n   It would be nice if the proposed log message helped them by\n   briefly explaining their differences, for example.\n\nwhich would help future \"git log\" readers what this fix was about.\n\nWill queue for now, but the log message would want to be a bit more\nhelpful to the readers.\n\nThanks.\n\n> Signed-off-by: M Hickford <mirth.hickford@gmail.com>\n> ---\n>     credential-cache: respect request capabilities\n>     \n>     CC: sandals@crustytoothpaste.net\n>     \n>     Patch v4 fixes test\n>\n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1842%2Fhickford%2Fcache-capability-v4\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1842/hickford/cache-capability-v4\n> Pull-Request: https://github.com/gitgitgadget/git/pull/1842\n>\n> Range-diff vs v3:\n>\n>  1:  e9851c5c4ac ! 1:  23942f9fa47 credential-cache: respect request capabilities\n>      @@ t/lib-credential.sh: helper_test_authtype() {\n>       +\t\tprotocol=https\n>       +\t\thost=git.example.com\n>       +\t\t--\n>      -+\t\tcapability[]=authtype\n>       +\t\tprotocol=https\n>       +\t\thost=git.example.com\n>       +\t\tusername=askpass-username\n>\n>\n>  builtin/credential-cache--daemon.c |  4 ++--\n>  t/lib-credential.sh                | 15 +++++++++++++++\n>  2 files changed, 17 insertions(+), 2 deletions(-)\n>\n> diff --git a/builtin/credential-cache--daemon.c b/builtin/credential-cache--daemon.c\n> index bc22f5c6d24..e707618e743 100644\n> --- a/builtin/credential-cache--daemon.c\n> +++ b/builtin/credential-cache--daemon.c\n> @@ -142,9 +142,9 @@ static void serve_one_client(FILE *in, FILE *out)\n>  \t\t\t\tfprintf(out, \"username=%s\\n\", e->item.username);\n>  \t\t\tif (e->item.password)\n>  \t\t\t\tfprintf(out, \"password=%s\\n\", e->item.password);\n> -\t\t\tif (credential_has_capability(&c.capa_authtype, CREDENTIAL_OP_HELPER) && e->item.authtype)\n> +\t\t\tif (credential_has_capability(&c.capa_authtype, CREDENTIAL_OP_RESPONSE) && e->item.authtype)\n>  \t\t\t\tfprintf(out, \"authtype=%s\\n\", e->item.authtype);\n> -\t\t\tif (credential_has_capability(&c.capa_authtype, CREDENTIAL_OP_HELPER) && e->item.credential)\n> +\t\t\tif (credential_has_capability(&c.capa_authtype, CREDENTIAL_OP_RESPONSE) && e->item.credential)\n>  \t\t\t\tfprintf(out, \"credential=%s\\n\", e->item.credential);\n>  \t\t\tif (e->item.password_expiry_utc != TIME_MAX)\n>  \t\t\t\tfprintf(out, \"password_expiry_utc=%\"PRItime\"\\n\",\n> diff --git a/t/lib-credential.sh b/t/lib-credential.sh\n> index 58b9c740605..cc6bf9aa5f3 100644\n> --- a/t/lib-credential.sh\n> +++ b/t/lib-credential.sh\n> @@ -566,6 +566,21 @@ helper_test_authtype() {\n>  \t\tEOF\n>  \t'\n>  \n> +\ttest_expect_success \"helper ($HELPER) gets authtype and credential only if request has authtype capability\" '\n> +\t\tcheck fill $HELPER <<-\\EOF\n> +\t\tprotocol=https\n> +\t\thost=git.example.com\n> +\t\t--\n> +\t\tprotocol=https\n> +\t\thost=git.example.com\n> +\t\tusername=askpass-username\n> +\t\tpassword=askpass-password\n> +\t\t--\n> +\t\taskpass: Username for '\\''https://git.example.com'\\'':\n> +\t\taskpass: Password for '\\''https://askpass-username@git.example.com'\\'':\n> +\t\tEOF\n> +\t'\n> +\n>  \ttest_expect_success \"helper ($HELPER) stores authtype and credential with username\" '\n>  \t\tcheck approve $HELPER <<-\\EOF\n>  \t\tcapability[]=authtype\n>\n> base-commit: 92999a42db1c5f43f330e4f2bca4026b5b81576f\n"},{"id":"510273","messageId":"pull.1842.v5.git.1736462721156.gitgitgadget@gmail.com","threadId":"62678","inReplyTo":"pull.1842.v4.git.1736212760709.gitgitgadget@gmail.com","subject":"[PATCH v5] credential-cache: respect authtype capability","fromName":"M Hickford via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-01-09T22:45:20Z","receivedAt":"2025-01-09T22:45:24Z","isPatch":true,"sender":{"key":"mirth.hickford@gmail.com","avatar":"https://avatars.githubusercontent.com/u/105314?v=4"},"body":"From: M Hickford <mirth.hickford@gmail.com>\n\nPreviously, credential-cache populated authtype regardless whether\n\"get\" request had authtype capability. As documented in\ngit-credential.txt, authtype \"should not be sent unless the appropriate\ncapability ... is provided\".\n\nAdd test. Without this change, the test failed because \"credential fill\"\nprinted an incomplete credential with only protocol and host attributes\n(the unexpected authtype attribute was discarded by credential.c).\n\nSigned-off-by: M Hickford <mirth.hickford@gmail.com>\n---\n    credential-cache: respect request capabilities\n    \n    CC: sandals@crustytoothpaste.net CC: gitster@pobox.com\n    \n    Patch v5 adds details to the commit message\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1842%2Fhickford%2Fcache-capability-v5\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1842/hickford/cache-capability-v5\nPull-Request: https://github.com/gitgitgadget/git/pull/1842\n\nRange-diff vs v4:\n\n 1:  23942f9fa47 ! 1:  db575d9d116 credential-cache: respect request capabilities\n     @@ Metadata\n      Author: M Hickford <mirth.hickford@gmail.com>\n      \n       ## Commit message ##\n     -    credential-cache: respect request capabilities\n     +    credential-cache: respect authtype capability\n      \n     -    Previously, credential-cache populated authtype regardless of request.\n     +    Previously, credential-cache populated authtype regardless whether\n     +    \"get\" request had authtype capability. As documented in\n     +    git-credential.txt, authtype \"should not be sent unless the appropriate\n     +    capability ... is provided\".\n     +\n     +    Add test. Without this change, the test failed because \"credential fill\"\n     +    printed an incomplete credential with only protocol and host attributes\n     +    (the unexpected authtype attribute was discarded by credential.c).\n      \n          Signed-off-by: M Hickford <mirth.hickford@gmail.com>\n      \n\n\n builtin/credential-cache--daemon.c |  4 ++--\n t/lib-credential.sh                | 15 +++++++++++++++\n 2 files changed, 17 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/credential-cache--daemon.c b/builtin/credential-cache--daemon.c\nindex bc22f5c6d24..e707618e743 100644\n--- a/builtin/credential-cache--daemon.c\n+++ b/builtin/credential-cache--daemon.c\n@@ -142,9 +142,9 @@ static void serve_one_client(FILE *in, FILE *out)\n \t\t\t\tfprintf(out, \"username=%s\\n\", e->item.username);\n \t\t\tif (e->item.password)\n \t\t\t\tfprintf(out, \"password=%s\\n\", e->item.password);\n-\t\t\tif (credential_has_capability(&c.capa_authtype, CREDENTIAL_OP_HELPER) && e->item.authtype)\n+\t\t\tif (credential_has_capability(&c.capa_authtype, CREDENTIAL_OP_RESPONSE) && e->item.authtype)\n \t\t\t\tfprintf(out, \"authtype=%s\\n\", e->item.authtype);\n-\t\t\tif (credential_has_capability(&c.capa_authtype, CREDENTIAL_OP_HELPER) && e->item.credential)\n+\t\t\tif (credential_has_capability(&c.capa_authtype, CREDENTIAL_OP_RESPONSE) && e->item.credential)\n \t\t\t\tfprintf(out, \"credential=%s\\n\", e->item.credential);\n \t\t\tif (e->item.password_expiry_utc != TIME_MAX)\n \t\t\t\tfprintf(out, \"password_expiry_utc=%\"PRItime\"\\n\",\ndiff --git a/t/lib-credential.sh b/t/lib-credential.sh\nindex 58b9c740605..cc6bf9aa5f3 100644\n--- a/t/lib-credential.sh\n+++ b/t/lib-credential.sh\n@@ -566,6 +566,21 @@ helper_test_authtype() {\n \t\tEOF\n \t'\n \n+\ttest_expect_success \"helper ($HELPER) gets authtype and credential only if request has authtype capability\" '\n+\t\tcheck fill $HELPER <<-\\EOF\n+\t\tprotocol=https\n+\t\thost=git.example.com\n+\t\t--\n+\t\tprotocol=https\n+\t\thost=git.example.com\n+\t\tusername=askpass-username\n+\t\tpassword=askpass-password\n+\t\t--\n+\t\taskpass: Username for '\\''https://git.example.com'\\'':\n+\t\taskpass: Password for '\\''https://askpass-username@git.example.com'\\'':\n+\t\tEOF\n+\t'\n+\n \ttest_expect_success \"helper ($HELPER) stores authtype and credential with username\" '\n \t\tcheck approve $HELPER <<-\\EOF\n \t\tcapability[]=authtype\n\nbase-commit: 92999a42db1c5f43f330e4f2bca4026b5b81576f\n-- \ngitgitgadget\n"},{"id":"510881","messageId":"8ef3bd22-d2e4-4361-93da-581d2f76204f@gmail.com","threadId":"62678","inReplyTo":"pull.1842.v5.git.1736462721156.gitgitgadget@gmail.com","subject":"Re: [PATCH v5] credential-cache: respect authtype capability","fromName":"M Hickford","fromEmail":"mirth.hickford@gmail.com","sentAt":"2025-01-18T20:09:50Z","receivedAt":"2025-01-18T20:09:53Z","isPatch":true,"sender":{"key":"mirth.hickford@gmail.com","avatar":"https://avatars.githubusercontent.com/u/105314?v=4"},"body":"On 2025-01-09 22:45, M Hickford via GitGitGadget wrote:\n> From: M Hickford <mirth.hickford@gmail.com>\n> \n> Previously, credential-cache populated authtype regardless whether\n> \"get\" request had authtype capability. As documented in\n> git-credential.txt, authtype \"should not be sent unless the appropriate\n> capability ... is provided\".\n> \n> Add test. Without this change, the test failed because \"credential fill\"\n> printed an incomplete credential with only protocol and host attributes\n> (the unexpected authtype attribute was discarded by credential.c).\n> \n> Signed-off-by: M Hickford <mirth.hickford@gmail.com>\n> ---\n>      credential-cache: respect request capabilities\n>      \n>      CC: sandals@crustytoothpaste.net CC: gitster@pobox.com\n>      \n>      Patch v5 adds details to the commit message\n> \n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1842%2Fhickford%2Fcache-capability-v5\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1842/hickford/cache-capability-v5\n> Pull-Request: https://github.com/gitgitgadget/git/pull/1842\n> \n> Range-diff vs v4:\n> \n>   1:  23942f9fa47 ! 1:  db575d9d116 credential-cache: respect request capabilities\n>       @@ Metadata\n>        Author: M Hickford <mirth.hickford@gmail.com>\n>        \n>         ## Commit message ##\n>       -    credential-cache: respect request capabilities\n>       +    credential-cache: respect authtype capability\n>        \n>       -    Previously, credential-cache populated authtype regardless of request.\n>       +    Previously, credential-cache populated authtype regardless whether\n>       +    \"get\" request had authtype capability. As documented in\n>       +    git-credential.txt, authtype \"should not be sent unless the appropriate\n>       +    capability ... is provided\".\n>       +\n>       +    Add test. Without this change, the test failed because \"credential fill\"\n>       +    printed an incomplete credential with only protocol and host attributes\n>       +    (the unexpected authtype attribute was discarded by credential.c).\n>        \n>            Signed-off-by: M Hickford <mirth.hickford@gmail.com>\n>        \n> \n> \n>   builtin/credential-cache--daemon.c |  4 ++--\n>   t/lib-credential.sh                | 15 +++++++++++++++\n>   2 files changed, 17 insertions(+), 2 deletions(-)\n> \n> diff --git a/builtin/credential-cache--daemon.c b/builtin/credential-cache--daemon.c\n> index bc22f5c6d24..e707618e743 100644\n> --- a/builtin/credential-cache--daemon.c\n> +++ b/builtin/credential-cache--daemon.c\n> @@ -142,9 +142,9 @@ static void serve_one_client(FILE *in, FILE *out)\n>   \t\t\t\tfprintf(out, \"username=%s\\n\", e->item.username);\n>   \t\t\tif (e->item.password)\n>   \t\t\t\tfprintf(out, \"password=%s\\n\", e->item.password);\n> -\t\t\tif (credential_has_capability(&c.capa_authtype, CREDENTIAL_OP_HELPER) && e->item.authtype)\n> +\t\t\tif (credential_has_capability(&c.capa_authtype, CREDENTIAL_OP_RESPONSE) && e->item.authtype)\n>   \t\t\t\tfprintf(out, \"authtype=%s\\n\", e->item.authtype);\n> -\t\t\tif (credential_has_capability(&c.capa_authtype, CREDENTIAL_OP_HELPER) && e->item.credential)\n> +\t\t\tif (credential_has_capability(&c.capa_authtype, CREDENTIAL_OP_RESPONSE) && e->item.credential)\n>   \t\t\t\tfprintf(out, \"credential=%s\\n\", e->item.credential);\n>   \t\t\tif (e->item.password_expiry_utc != TIME_MAX)\n>   \t\t\t\tfprintf(out, \"password_expiry_utc=%\"PRItime\"\\n\",\n> diff --git a/t/lib-credential.sh b/t/lib-credential.sh\n> index 58b9c740605..cc6bf9aa5f3 100644\n> --- a/t/lib-credential.sh\n> +++ b/t/lib-credential.sh\n> @@ -566,6 +566,21 @@ helper_test_authtype() {\n>   \t\tEOF\n>   \t'\n>   \n> +\ttest_expect_success \"helper ($HELPER) gets authtype and credential only if request has authtype capability\" '\n> +\t\tcheck fill $HELPER <<-\\EOF\n> +\t\tprotocol=https\n> +\t\thost=git.example.com\n> +\t\t--\n> +\t\tprotocol=https\n> +\t\thost=git.example.com\n> +\t\tusername=askpass-username\n> +\t\tpassword=askpass-password\n> +\t\t--\n> +\t\taskpass: Username for '\\''https://git.example.com'\\'':\n> +\t\taskpass: Password for '\\''https://askpass-username@git.example.com'\\'':\n> +\t\tEOF\n> +\t'\n> +\n>   \ttest_expect_success \"helper ($HELPER) stores authtype and credential with username\" '\n>   \t\tcheck approve $HELPER <<-\\EOF\n>   \t\tcapability[]=authtype\n> \n> base-commit: 92999a42db1c5f43f330e4f2bca4026b5b81576f\n\nHi Brian. Any further comments on patch v5? This addresses your comments \non v2 and expands the commit message as encouraged by Junio. (Thank you \nboth for the review so far.)\n\nhttps://lore.kernel.org/git/Z3xhqCf7Gr74BHO4@tapette.crustytoothpaste.net/\nhttps://lore.kernel.org/git/xmqqttaaoyaz.fsf@gitster.g/\n"},{"id":"510882","messageId":"Z4wLt3oPlR5p2_e5@tapette.crustytoothpaste.net","threadId":"62678","inReplyTo":"8ef3bd22-d2e4-4361-93da-581d2f76204f@gmail.com","subject":"Re: [PATCH v5] credential-cache: respect authtype capability","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2025-01-18T20:14:47Z","receivedAt":"2025-01-18T20:14:56Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2025-01-18 at 20:09:50, M Hickford wrote:\n> Hi Brian. Any further comments on patch v5? This addresses your comments on\n> v2 and expands the commit message as encouraged by Junio. (Thank you both\n> for the review so far.)\n\nI think this looks fine.\n-- \nbrian m. carlson (they/them or he/him)\nToronto, Ontario, CA\n"}]}