Re: [PATCH] http.c: prompt for username on 403
- From
Ashlesh Gawande <git@ashlesh.me>
- Date
- Oct 15, 2025, 14:12 UTC
- 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:
Show 58 quoted lines
> 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