Thread (11 messages) 11 messages, 3 authors, 2025-12-11

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

From: Ashlesh Gawande <hidden>
Date: 2025-10-15 14:17:56

On 10/15/25 02:59, brian m. carlson wrote:
On 2025-10-14 at 14:43:52, Ashlesh Gawande wrote:
quoted
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 <redacted>
---
  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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help