From: Ashlesh Gawande <git@ashlesh.me>
Date: Wed, 15 Oct 2025 14:12:09 GMT
Subject: Re: [PATCH] http.c: prompt for username on 403
Message-ID: <30639771-4999-45f4-a8d7-1ed4774ffd8e@ashlesh.me>
In-Reply-To: <aO7Aqooz-0ppbcMP@fruit.crustytoothpaste.net>


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