[PATCH] http.c: prompt for username on 403

[PATCH] http.c: prompt for username on 403

[Ashlesh Gawande]

Scenario:
- There are a few pre-production systems that a lot of testers and
  developers need to time share because of low availability
- Devops generates a GitHub token with pull only access
  and adds it to the netrc file on these systems
  (Pull only as we don't want testers/others to be able to push)
- Testers log in and do a git pull for the latest changes
  (via netrc credentials - though testers may not be aware)
- Developers login to debug issues and may make fixes to the test repo
- Now when developers try to push their changes they receive:
  fatal: unable to access 'https://github.com/<org>/<project>/':
  The requested URL returned error: 403
- The developer is not given the chance to supply an authorized token
  and either needs to comment the netrc file or copy the changes over
  to their own machine

Signed-off-by: Ashlesh Gawande <git@ashlesh.me>
---
 http.c                     |  2 +-
 t/lib-httpd.sh             |  9 +++++++++
 t/lib-httpd/apache.conf    |  4 ++++
 t/lib-httpd/passwd         |  1 +
 t/t5550-http-fetch-dumb.sh | 24 ++++++++++++++++++++++++
 5 files changed, 39 insertions(+), 1 deletion(-)

diff --git a/http.c b/http.c
index 7e3af1e72f..18959f63b9 100644
--- a/http.c
+++ b/http.c
@@ -1852,7 +1852,7 @@ static int handle_curl_result(struct slot_results *results)
 		return HTTP_NOMATCHPUBLICKEY;
 	} else if (missing_target(results))
 		return HTTP_MISSING_TARGET;
-	else if (results->http_code == 401) {
+	else if (results->http_code == 401 || results->http_code == 403) {
 		if ((http_auth.username && http_auth.password) ||\
 		    (http_auth.authtype && http_auth.credential)) {
 			if (http_auth.multistage) {
diff --git a/t/lib-httpd.sh b/t/lib-httpd.sh
index 5091db949b..cdc92b2916 100644
--- a/t/lib-httpd.sh
+++ b/t/lib-httpd.sh
@@ -325,6 +325,15 @@ set_askpass() {
 	echo "$2" >"$TRASH_DIRECTORY/askpass-pass"
 }
 
+set_netrc() {
+	# $HOME=$TRASH_DIRECTORY
+	echo "machine $1 login $2 password $3" > $TRASH_DIRECTORY/.netrc
+}
+
+clear_netrc() {
+	rm "$TRASH_DIRECTORY/.netrc"
+}
+
 expect_askpass() {
 	dest=$HTTPD_DEST${3+/$3}
 
diff --git a/t/lib-httpd/apache.conf b/t/lib-httpd/apache.conf
index e631ab0eb5..6b8c50a51a 100644
--- a/t/lib-httpd/apache.conf
+++ b/t/lib-httpd/apache.conf
@@ -238,6 +238,10 @@ SSLEngine On
 	AuthName "git-auth"
 	AuthUserFile passwd
 	Require valid-user
+
+	# return 403 for authenticated user: forbidden-user@host
+	RewriteCond "%{REMOTE_USER}" "^forbidden-user@host"
+	RewriteRule ^ - [F]
 </Location>
 
 <LocationMatch "^/auth-push/.*/git-receive-pack$">
diff --git a/t/lib-httpd/passwd b/t/lib-httpd/passwd
index d9c122f348..3bab7b6423 100644
--- a/t/lib-httpd/passwd
+++ b/t/lib-httpd/passwd
@@ -1 +1,2 @@
 user@host:$apr1$LGPmCZWj$9vxEwj5Z5GzQLBMxp3mCx1
+forbidden-user@host:$apr1$LGPmCZWj$9vxEwj5Z5GzQLBMxp3mCx1
diff --git a/t/t5550-http-fetch-dumb.sh b/t/t5550-http-fetch-dumb.sh
index ed0ad66fad..6c4c1cafb2 100755
--- a/t/t5550-http-fetch-dumb.sh
+++ b/t/t5550-http-fetch-dumb.sh
@@ -102,6 +102,30 @@ test_expect_success 'cloning password-protected repository can fail' '
 	expect_askpass both wrong
 '
 
+test_expect_success 'using credentials from netrc to clone successfully' '
+	set_askpass wrong &&
+	set_netrc 127.0.0.1 user@host pass@host &&
+	git clone "$HTTPD_URL/auth/dumb/repo.git" clone-auth-netrc &&
+	expect_askpass none
+'
+clear_netrc
+
+test_expect_success 'netrc unauthorized credentials (prompt after 401)' '
+	set_askpass wrong &&
+	set_netrc 127.0.0.1 user@host pass@wrong &&
+	test_must_fail git clone "$HTTPD_URL/auth/dumb/repo.git" clone-auth-netrc-401 &&
+	expect_askpass both wrong
+'
+clear_netrc
+
+test_expect_success 'netrc authorized but forbidden credentials (prompt after 403)' '
+	set_askpass wrong &&
+	set_netrc 127.0.0.1 forbidden-user@host pass@host &&
+	test_must_fail git clone "$HTTPD_URL/auth/dumb/repo.git" clone-auth-netc-403 &&
+	expect_askpass both wrong
+'
+clear_netrc
+
 test_expect_success 'http auth can use user/pass in URL' '
 	set_askpass wrong &&
 	git clone "$HTTPD_URL_USER_PASS/auth/dumb/repo.git" clone-auth-none &&
-- 
2.43.0

Re: [PATCH] http.c: prompt for username on 403

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 3034 bytes --]

On 2025-10-14 at 14:43:52, Ashlesh Gawande wrote:
> Scenario:
> - There are a few pre-production systems that a lot of testers and
>   developers need to time share because of low availability
> - Devops generates a GitHub token with pull only access
>   and adds it to the netrc file on these systems
>   (Pull only as we don't want testers/others to be able to push)
> - Testers log in and do a git pull for the latest changes
>   (via netrc credentials - though testers may not be aware)
> - Developers login to debug issues and may make fixes to the test repo
> - Now when developers try to push their changes they receive:
>   fatal: unable to access 'https://github.com/<org>/<project>/':
>   The requested URL returned error: 403
> - The developer is not given the chance to supply an authorized token
>   and either needs to comment the netrc file or copy the changes over
>   to their own machine
> 
> Signed-off-by: Ashlesh Gawande <git@ashlesh.me>
> ---
>  http.c                     |  2 +-
>  t/lib-httpd.sh             |  9 +++++++++
>  t/lib-httpd/apache.conf    |  4 ++++
>  t/lib-httpd/passwd         |  1 +
>  t/t5550-http-fetch-dumb.sh | 24 ++++++++++++++++++++++++
>  5 files changed, 39 insertions(+), 1 deletion(-)
> 
> diff --git a/http.c b/http.c
> index 7e3af1e72f..18959f63b9 100644
> --- a/http.c
> +++ b/http.c
> @@ -1852,7 +1852,7 @@ static int handle_curl_result(struct slot_results *results)
>  		return HTTP_NOMATCHPUBLICKEY;
>  	} else if (missing_target(results))
>  		return HTTP_MISSING_TARGET;
> -	else if (results->http_code == 401) {
> +	else if (results->http_code == 401 || results->http_code == 403) {

I don't think this is a good idea.  Existing servers send a 401 when no
credentials are available and 403 if credentials are sent but are not
valid for a repository.  The former case causes credentials to be
erased, but the latter does not.

Your proposal will cause someone's credentials to be erased just because
they don't have access to a repository, which would be bad because it's
not that the credentials are invalid (that would be a 401) but that the
credentials are not usable for that repository or for that operation.

So if I attempt to push to https://github.com/git/git.git, then my
credentials will be erased even though there are no valid credentials
that could possibly grant me access to that repository (because I'm not
Junio).  Then _none_ of my pushes work because my token is gone.

I agree that it's inconvenient that netrc credential override other
credentials, but the proper thing to do would be to (a) not share
working trees among users (since Git's security model doesn't allow for
that), (b) not use netrc for this purpose and use a credential helper,
(c) add functionality to disable netrc via config, or (d) use an SSH
deploy key for automated systems with `GIT_SSH_COMMAND` and `ssh -i` and
have developers forward their SSH agent to push.
-- 
brian m. carlson (they/them)
Toronto, Ontario, CA

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

Re: [PATCH] http.c: prompt for username on 403

[Ashlesh Gawande]

On 10/15/25 02:59, brian m. carlson wrote:
> On 2025-10-14 at 14:43:52, Ashlesh Gawande wrote:
>> Scenario:
>> - There are a few pre-production systems that a lot of testers and
>>    developers need to time share because of low availability
>> - Devops generates a GitHub token with pull only access
>>    and adds it to the netrc file on these systems
>>    (Pull only as we don't want testers/others to be able to push)
>> - Testers log in and do a git pull for the latest changes
>>    (via netrc credentials - though testers may not be aware)
>> - Developers login to debug issues and may make fixes to the test repo
>> - Now when developers try to push their changes they receive:
>>    fatal: unable to access 'https://github.com/<org>/<project>/':
>>    The requested URL returned error: 403
>> - The developer is not given the chance to supply an authorized token
>>    and either needs to comment the netrc file or copy the changes over
>>    to their own machine
>>
>> Signed-off-by: Ashlesh Gawande <git@ashlesh.me>
>> ---
>>   http.c                     |  2 +-
>>   t/lib-httpd.sh             |  9 +++++++++
>>   t/lib-httpd/apache.conf    |  4 ++++
>>   t/lib-httpd/passwd         |  1 +
>>   t/t5550-http-fetch-dumb.sh | 24 ++++++++++++++++++++++++
>>   5 files changed, 39 insertions(+), 1 deletion(-)
>>
>> diff --git a/http.c b/http.c
>> index 7e3af1e72f..18959f63b9 100644
>> --- a/http.c
>> +++ b/http.c
>> @@ -1852,7 +1852,7 @@ static int handle_curl_result(struct slot_results *results)
>>   		return HTTP_NOMATCHPUBLICKEY;
>>   	} else if (missing_target(results))
>>   		return HTTP_MISSING_TARGET;
>> -	else if (results->http_code == 401) {
>> +	else if (results->http_code == 401 || results->http_code == 403) {
> I don't think this is a good idea.  Existing servers send a 401 when no
> credentials are available and 403 if credentials are sent but are not
> valid for a repository.  The former case causes credentials to be
> erased, but the latter does not.
>
> Your proposal will cause someone's credentials to be erased just because
> they don't have access to a repository, which would be bad because it's
> not that the credentials are invalid (that would be a 401) but that the
> credentials are not usable for that repository or for that operation.
>
> So if I attempt to push to https://github.com/git/git.git, then my
> credentials will be erased even though there are no valid credentials
> that could possibly grant me access to that repository (because I'm not
> Junio).  Then _none_ of my pushes work because my token is gone.
>
> I agree that it's inconvenient that netrc credential override other
> credentials, but the proper thing to do would be to (a) not share
> working trees among users (since Git's security model doesn't allow for
> that), (b) not use netrc for this purpose and use a credential helper,
> (c) add functionality to disable netrc via config, or (d) use an SSH
> deploy key for automated systems with `GIT_SSH_COMMAND` and `ssh -i` and
> have developers forward their SSH agent to push.
Oh I see - yeah don't want to erase the credentials.
Was trying to figure why 403 was happening instead of a prompt (as I was 
not aware of netrc file being used).
Thanks for the detailed explanation and suggestions Brian!

Is it worth it to include the netrc tests in git that I wrote as part of 
this

(if so I can email them as a separate patch)?

Thanks
Ashlesh

Re: [PATCH] http.c: prompt for username on 403

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 753 bytes --]

On 2025-10-15 at 14:12:09, Ashlesh Gawande wrote:
> Oh I see - yeah don't want to erase the credentials.
> Was trying to figure why 403 was happening instead of a prompt (as I was not
> aware of netrc file being used).
> Thanks for the detailed explanation and suggestions Brian!
> 
> Is it worth it to include the netrc tests in git that I wrote as part of
> this

Yes, I think if you have patches to test our netrc handling, those would
be very welcome.  I was complaining a couple months ago about how we had
no tests for netrc after I accidentally broke the code that makes it
work, so I would very much appreciate any tests we could add to make
that less likely in the future.
-- 
brian m. carlson (they/them)
Toronto, Ontario, CA

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

Re: [PATCH] http.c: prompt for username on 403

[Ashlesh Gawande]

On 10/16/25 04:01, brian m. carlson wrote:
> On 2025-10-15 at 14:12:09, Ashlesh Gawande wrote:
>> Oh I see - yeah don't want to erase the credentials.
>> Was trying to figure why 403 was happening instead of a prompt (as I was not
>> aware of netrc file being used).
>> Thanks for the detailed explanation and suggestions Brian!
>>
>> Is it worth it to include the netrc tests in git that I wrote as part of
>> this
> Yes, I think if you have patches to test our netrc handling, those would
> be very welcome.  I was complaining a couple months ago about how we had
> no tests for netrc after I accidentally broke the code that makes it
> work, so I would very much appreciate any tests we could add to make
> that less likely in the future.

I was working on separating the tests and thought about the original 
proposal a bit more.
To stop the credentials from being erased on 403 could something like 
the following be acceptable?

         else if (results->http_code == 401 || results->http_code == 403) {
                 if ((http_auth.username && http_auth.password) ||\
                     (http_auth.authtype && http_auth.credential)) {
+                       // Do not override existing credentials on 403
+                       if (results->http_code == 403) {
+                               return HTTP_ERROR;
+                       }
+
                         if (http_auth.multistage) {

So then we would prompt on 403 only if credentials are not configured.

Re: [PATCH] http.c: prompt for username on 403

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 1217 bytes --]

On 2025-12-09 at 08:22:49, Ashlesh Gawande wrote:
> I was working on separating the tests and thought about the original
> proposal a bit more.
> To stop the credentials from being erased on 403 could something like the
> following be acceptable?
> 
>         else if (results->http_code == 401 || results->http_code == 403) {
>                 if ((http_auth.username && http_auth.password) ||\
>                     (http_auth.authtype && http_auth.credential)) {
> +                       // Do not override existing credentials on 403
> +                       if (results->http_code == 403) {
> +                               return HTTP_ERROR;
> +                       }
> +
>                         if (http_auth.multistage) {
> 
> So then we would prompt on 403 only if credentials are not configured.

Can you tell me what file you see this in?  I don't actually see any
place in the code that has "http_code == 403" in the latest version of
the main branch.

I wonder if your issue may already be fixed in a newer version than you
have.
-- 
brian m. carlson (they/them)
Toronto, Ontario, CA

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

Re: [PATCH] http.c: prompt for username on 403

[Ashlesh Gawande]

On 12/10/25 07:35, brian m. carlson wrote:
> On 2025-12-09 at 08:22:49, Ashlesh Gawande wrote:
>> I was working on separating the tests and thought about the original
>> proposal a bit more.
>> To stop the credentials from being erased on 403 could something like the
>> following be acceptable?
>>
>>          else if (results->http_code == 401 || results->http_code == 403) {
>>                  if ((http_auth.username && http_auth.password) ||\
>>                      (http_auth.authtype && http_auth.credential)) {
>> +                       // Do not override existing credentials on 403
>> +                       if (results->http_code == 403) {
>> +                               return HTTP_ERROR;
>> +                       }
>> +
>>                          if (http_auth.multistage) {
>>
>> So then we would prompt on 403 only if credentials are not configured.
> Can you tell me what file you see this in?  I don't actually see any
> place in the code that has "http_code == 403" in the latest version of
> the main branch.
>
> I wonder if your issue may already be fixed in a newer version than you
> have.
Oh, that http_code == 403 is my original proposal to prompt for 
username/password on 403 (I did the diff on top of that instead of base).
But you pointed out that it would wipe out existing credentials. This is 
an attempt to fix that by not prompting on 403 if git credentials are set.
So when credentials are provided through default netrc file (such that 
http_auth.* are not set; git credential helper is not set) then we can 
still get the prompt on 403.

RE: [PATCH] http.c: prompt for username on 403

[rsbecker]

On December 10, 2025 7:30 AM, Ashlesh Gawande wrote:
>On 12/10/25 07:35, brian m. carlson wrote:
>> On 2025-12-09 at 08:22:49, Ashlesh Gawande wrote:
>>> I was working on separating the tests and thought about the original
>>> proposal a bit more.
>>> To stop the credentials from being erased on 403 could something like
>>> the following be acceptable?
>>>
>>>          else if (results->http_code == 401 || results->http_code ==
>>> 403) {
>>>                  if ((http_auth.username && http_auth.password) ||\
>>>                      (http_auth.authtype && http_auth.credential)) {
>>> +                       // Do not override existing credentials on
>>> +403
>>> +                       if (results->http_code == 403) {
>>> +                               return HTTP_ERROR;
>>> +                       }
>>> +
>>>                          if (http_auth.multistage) {
>>>
>>> So then we would prompt on 403 only if credentials are not configured.
>> Can you tell me what file you see this in?  I don't actually see any
>> place in the code that has "http_code == 403" in the latest version of
>> the main branch.
>>
>> I wonder if your issue may already be fixed in a newer version than
>> you have.
>Oh, that http_code == 403 is my original proposal to prompt for
>username/password on 403 (I did the diff on top of that instead of base).
>But you pointed out that it would wipe out existing credentials. This is an attempt to
>fix that by not prompting on 403 if git credentials are set.
>So when credentials are provided through default netrc file (such that
>http_auth.* are not set; git credential helper is not set) then we can still get the
>prompt on 403.

Please make sure that any existing git credential helpers, including custom helpers
are not impacted by this change. This would have serious negative consequences
and would be a blocker for many in my community who use both the standard
git credential helpers and custom ones. If you are going to force a credential wipe
this should cause an update to the credential protocol to inform the helper that a
wipe has occurred or is requested. The .netrc approach is most limited to Linux
implementations and is not available on or applicable to some other platforms.

Thank you for your attention.
Randall

Re: [PATCH] http.c: prompt for username on 403

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 1138 bytes --]

On 2025-12-10 at 12:30:27, Ashlesh Gawande wrote:
> Oh, that http_code == 403 is my original proposal to prompt for
> username/password on 403 (I did the diff on top of that instead of base).
> But you pointed out that it would wipe out existing credentials. This is an
> attempt to fix that by not prompting on 403 if git credentials are set.
> So when credentials are provided through default netrc file (such that
> http_auth.* are not set; git credential helper is not set) then we can still
> get the prompt on 403.

As Randall said, I don't think it's a good idea to do this.  It's a
major change in how functionality works and it will probably break
users.

I did mention before that a better approach is to add a config to decide
whether to honour the netrc and I think that would be the right choice
here.  That lets people opt into different behaviour if they want it
(and, to be honest, I _do_ very much want to skip netrc for Git
credentials since I have similar problems as the ones you're describing)
and avoids breaking things for existing users.
-- 
brian m. carlson (they/them)
Toronto, Ontario, CA

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

Re: [PATCH] http.c: prompt for username on 403

[Ashlesh Gawande]

On 12/11/25 03:58, brian m. carlson wrote:
> On 2025-12-10 at 12:30:27, Ashlesh Gawande wrote:
>> Oh, that http_code == 403 is my original proposal to prompt for
>> username/password on 403 (I did the diff on top of that instead of base).
>> But you pointed out that it would wipe out existing credentials. This is an
>> attempt to fix that by not prompting on 403 if git credentials are set.
>> So when credentials are provided through default netrc file (such that
>> http_auth.* are not set; git credential helper is not set) then we can still
>> get the prompt on 403.
> As Randall said, I don't think it's a good idea to do this.  It's a
> major change in how functionality works and it will probably break
> users.
>
> I did mention before that a better approach is to add a config to decide
> whether to honour the netrc and I think that would be the right choice
> here.  That lets people opt into different behaviour if they want it
> (and, to be honest, I _do_ very much want to skip netrc for Git
> credentials since I have similar problems as the ones you're describing)
> and avoids breaking things for existing users.

Hmm, okay I understand. Yes probably good idea to skip netrc for Git 
credentials.
Thank you for your input Brian and Randall!

Re: [PATCH] http.c: prompt for username on 403

[Ashlesh Gawande]

On 12/10/25 07:35, brian m. carlson wrote:
> On 2025-12-09 at 08:22:49, Ashlesh Gawande wrote:
>> I was working on separating the tests and thought about the original
>> proposal a bit more.
>> To stop the credentials from being erased on 403 could something like the
>> following be acceptable?
>>
>>          else if (results->http_code == 401 || results->http_code == 403) {
>>                  if ((http_auth.username && http_auth.password) ||\
>>                      (http_auth.authtype && http_auth.credential)) {
>> +                       // Do not override existing credentials on 403
>> +                       if (results->http_code == 403) {
>> +                               return HTTP_ERROR;
>> +                       }
>> +
>>                          if (http_auth.multistage) {
>>
>> So then we would prompt on 403 only if credentials are not configured.
> Can you tell me what file you see this in?  I don't actually see any
> place in the code that has "http_code == 403" in the latest version of
> the main branch.
>
> I wonder if your issue may already be fixed in a newer version than you
> have.
Oh, that http_code == 403 is my original proposal to prompt for 
username/password on 403 (I did the diff on top of that instead of base).
But you pointed out that it would wipe out existing credentials. This is 
an attempt to fix that by not prompting on 403 if git credentials are set.
So when credentials are provided through default netrc file (such that 
http_auth.* are not set; git credential helper is not set) then we can 
still get the prompt on 403.