[PATCH] t5550: add netrc tests for http 401/403

[PATCH] t5550: add netrc tests for http 401/403

[Ashlesh Gawande]

Signed-off-by: Ashlesh Gawande <git@ashlesh.me>
---
Sending netrc test patches as suggested in: https://lore.kernel.org/git/aPAg3gYwzA9fHCC3@fruit.crustytoothpaste.net

 t/lib-httpd.sh             |  9 +++++++++
 t/lib-httpd/apache.conf    |  4 ++++
 t/lib-httpd/passwd         |  1 +
 t/t5550-http-fetch-dumb.sh | 25 +++++++++++++++++++++++++
 4 files changed, 39 insertions(+)

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..e002c7357d 100755
--- a/t/t5550-http-fetch-dumb.sh
+++ b/t/t5550-http-fetch-dumb.sh
@@ -102,6 +102,31 @@ 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 (fail on 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-netrc-403 2>err &&
+	expect_askpass none &&
+	grep "The requested URL returned error: 403" err
+'
+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 &&

base-commit: e0bfec3dfc356f7d808eb5ee546a54116b794397
-- 
2.43.0

Re: [PATCH] t5550: add netrc tests for http 401/403

[Junio C Hamano]

Ashlesh Gawande <git@ashlesh.me> writes:

> Signed-off-by: Ashlesh Gawande <git@ashlesh.me>
> ---
> Sending netrc test patches as suggested in: https://lore.kernel.org/git/aPAg3gYwzA9fHCC3@fruit.crustytoothpaste.net

At the conceptual level, I am happy to have tests for features that
we claim to support.  It is a different matter if we want to support
netrc, though ;-).

There are some nits.

> +set_netrc() {

Style.  SP on both sides of ().  I.e.

    set_netrc () {

> +	# $HOME=$TRASH_DIRECTORY
> +	echo "machine $1 login $2 password $3" > $TRASH_DIRECTORY/.netrc

Style.  No space between the redirection operator ">" and
redirection target.

Style.  Enclose the redirection target inside a pair of double
quotes if it involves variable interpolation.  I.e.

	echo ... >"$TRASH_DIRECTORY/.netrc"

> +}
> +
> +clear_netrc() {

Ditto.

> +	rm "$TRASH_DIRECTORY/.netrc"
> +}

Should this fail if .netrc did not exist in the first place, or is
the primary purpose of this helper to ensure the file does not exist
after it returns (in which case it would be desirable not to fail if
the file did not exist when it was called, with "rm -f")?

>  expect_askpass() {

Ditto.

> +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

We try not to run random shell functions outside the test_expect_*
blocks.  A clean-up function like this is better called at the end
of each piece, arranged with the test_when_finished helper.

	test_expect_success 'do random thing' '
		test_when_finished clear_netrc &&
		set_askpass wrong &&
		set_netrc ... &&
		...
	'

Re: [PATCH] t5550: add netrc tests for http 401/403

[Ashlesh Gawande]

On 1/6/26 15:50, Junio C Hamano wrote:
> Ashlesh Gawande <git@ashlesh.me> writes:
>
>> Signed-off-by: Ashlesh Gawande <git@ashlesh.me>
>> ---
>> Sending netrc test patches as suggested in: https://lore.kernel.org/git/aPAg3gYwzA9fHCC3@fruit.crustytoothpaste.net
> At the conceptual level, I am happy to have tests for features that
> we claim to support.  It is a different matter if we want to support
> netrc, though ;-).
>
> There are some nits.
Thanks for the quick review!
I think Brian also suggested getting rid of netrc in the future.
I have sent v2 to address your comments.
>
>> +set_netrc() {
> Style.  SP on both sides of ().  I.e.
>
>      set_netrc () {
>
>> +	# $HOME=$TRASH_DIRECTORY
>> +	echo "machine $1 login $2 password $3" > $TRASH_DIRECTORY/.netrc
> Style.  No space between the redirection operator ">" and
> redirection target.
>
> Style.  Enclose the redirection target inside a pair of double
> quotes if it involves variable interpolation.  I.e.
>
> 	echo ... >"$TRASH_DIRECTORY/.netrc"
>
>> +}
>> +
>> +clear_netrc() {
> Ditto.
>
>> +	rm "$TRASH_DIRECTORY/.netrc"
>> +}
> Should this fail if .netrc did not exist in the first place, or is
> the primary purpose of this helper to ensure the file does not exist
> after it returns (in which case it would be desirable not to fail if
> the file did not exist when it was called, with "rm -f")?
Yes, the primary purpose is to just clear the file as it might 
potentially break other tests (added -f in v2).
>>   expect_askpass() {
> Ditto.
>
>> +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
> We try not to run random shell functions outside the test_expect_*
> blocks.  A clean-up function like this is better called at the end
> of each piece, arranged with the test_when_finished helper.
>
> 	test_expect_success 'do random thing' '
> 		test_when_finished clear_netrc &&
> 		set_askpass wrong &&
> 		set_netrc ... &&
> 		...
> 	'
>
>

[PATCH v2] t5550: add netrc tests for http 401/403

[Ashlesh Gawande]

Signed-off-by: Ashlesh Gawande <git@ashlesh.me>
---
Range-diff against v1:
1:  27e112ea42 ! 1:  0b68f1d1af t5550: add netrc tests for http 401/403
    @@ Commit message
         Signed-off-by: Ashlesh Gawande <git@ashlesh.me>
     
      ## t/lib-httpd.sh ##
    -@@ t/lib-httpd.sh: set_askpass() {
    +@@ t/lib-httpd.sh: setup_askpass_helper() {
    + 	'
    + }
    + 
    +-set_askpass() {
    ++set_askpass () {
    + 	>"$TRASH_DIRECTORY/askpass-query" &&
    + 	echo "$1" >"$TRASH_DIRECTORY/askpass-user" &&
      	echo "$2" >"$TRASH_DIRECTORY/askpass-pass"
      }
      
    -+set_netrc() {
    +-expect_askpass() {
    ++set_netrc () {
     +	# $HOME=$TRASH_DIRECTORY
    -+	echo "machine $1 login $2 password $3" > $TRASH_DIRECTORY/.netrc
    ++	echo "machine $1 login $2 password $3" >"$TRASH_DIRECTORY/.netrc"
     +}
     +
    -+clear_netrc() {
    -+	rm "$TRASH_DIRECTORY/.netrc"
    ++clear_netrc () {
    ++	rm -f "$TRASH_DIRECTORY/.netrc"
     +}
     +
    - expect_askpass() {
    ++expect_askpass () {
      	dest=$HTTPD_DEST${3+/$3}
      
    + 	{
     
      ## t/lib-httpd/apache.conf ##
     @@ t/lib-httpd/apache.conf: SSLEngine On
    @@ t/t5550-http-fetch-dumb.sh: test_expect_success 'cloning password-protected repo
      '
      
     +test_expect_success 'using credentials from netrc to clone successfully' '
    ++	test_when_finished clear_netrc &&
     +	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)' '
    ++	test_when_finished clear_netrc &&
     +	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 (fail on 403)' '
    ++	test_when_finished clear_netrc &&
     +	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-netrc-403 2>err &&
     +	expect_askpass none &&
     +	grep "The requested URL returned error: 403" err
     +'
    -+clear_netrc
     +
      test_expect_success 'http auth can use user/pass in URL' '
      	set_askpass wrong &&

 t/lib-httpd.sh             | 13 +++++++++++--
 t/lib-httpd/apache.conf    |  4 ++++
 t/lib-httpd/passwd         |  1 +
 t/t5550-http-fetch-dumb.sh | 25 +++++++++++++++++++++++++
 4 files changed, 41 insertions(+), 2 deletions(-)

diff --git a/t/lib-httpd.sh b/t/lib-httpd.sh
index 5091db949b..5f42c311c2 100644
--- a/t/lib-httpd.sh
+++ b/t/lib-httpd.sh
@@ -319,13 +319,22 @@ setup_askpass_helper() {
 	'
 }
 
-set_askpass() {
+set_askpass () {
 	>"$TRASH_DIRECTORY/askpass-query" &&
 	echo "$1" >"$TRASH_DIRECTORY/askpass-user" &&
 	echo "$2" >"$TRASH_DIRECTORY/askpass-pass"
 }
 
-expect_askpass() {
+set_netrc () {
+	# $HOME=$TRASH_DIRECTORY
+	echo "machine $1 login $2 password $3" >"$TRASH_DIRECTORY/.netrc"
+}
+
+clear_netrc () {
+	rm -f "$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..9530f01b9e 100755
--- a/t/t5550-http-fetch-dumb.sh
+++ b/t/t5550-http-fetch-dumb.sh
@@ -102,6 +102,31 @@ test_expect_success 'cloning password-protected repository can fail' '
 	expect_askpass both wrong
 '
 
+test_expect_success 'using credentials from netrc to clone successfully' '
+	test_when_finished clear_netrc &&
+	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
+'
+
+test_expect_success 'netrc unauthorized credentials (prompt after 401)' '
+	test_when_finished clear_netrc &&
+	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
+'
+
+test_expect_success 'netrc authorized but forbidden credentials (fail on 403)' '
+	test_when_finished clear_netrc &&
+	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-netrc-403 2>err &&
+	expect_askpass none &&
+	grep "The requested URL returned error: 403" err
+'
+
 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 v2] t5550: add netrc tests for http 401/403

[Junio C Hamano]

Ashlesh Gawande <git@ashlesh.me> writes:

Here between the title and your sign-off is a space to explain why
it makes sense to add these new tests.  One way to do so may be to
explain that some cases were missing in the existing tests, and the
new ones are added to cover those cases, i.e., what the new tests
try to see under what situation, what behaviour do we expect out of
the system, and why do we expect that behaviour?

Thanks.

> Signed-off-by: Ashlesh Gawande <git@ashlesh.me>
> ---
> Range-diff against v1:
> 1:  27e112ea42 ! 1:  0b68f1d1af t5550: add netrc tests for http 401/403
>     @@ Commit message
>          Signed-off-by: Ashlesh Gawande <git@ashlesh.me>
>      
>       ## t/lib-httpd.sh ##
>     -@@ t/lib-httpd.sh: set_askpass() {
>     +@@ t/lib-httpd.sh: setup_askpass_helper() {
>     + 	'
>     + }
>     + 
>     +-set_askpass() {
>     ++set_askpass () {
>     + 	>"$TRASH_DIRECTORY/askpass-query" &&
>     + 	echo "$1" >"$TRASH_DIRECTORY/askpass-user" &&
>       	echo "$2" >"$TRASH_DIRECTORY/askpass-pass"
>       }
>       
>     -+set_netrc() {
>     +-expect_askpass() {
>     ++set_netrc () {
>      +	# $HOME=$TRASH_DIRECTORY
>     -+	echo "machine $1 login $2 password $3" > $TRASH_DIRECTORY/.netrc
>     ++	echo "machine $1 login $2 password $3" >"$TRASH_DIRECTORY/.netrc"
>      +}
>      +
>     -+clear_netrc() {
>     -+	rm "$TRASH_DIRECTORY/.netrc"
>     ++clear_netrc () {
>     ++	rm -f "$TRASH_DIRECTORY/.netrc"
>      +}
>      +
>     - expect_askpass() {
>     ++expect_askpass () {
>       	dest=$HTTPD_DEST${3+/$3}
>       
>     + 	{
>      
>       ## t/lib-httpd/apache.conf ##
>      @@ t/lib-httpd/apache.conf: SSLEngine On
>     @@ t/t5550-http-fetch-dumb.sh: test_expect_success 'cloning password-protected repo
>       '
>       
>      +test_expect_success 'using credentials from netrc to clone successfully' '
>     ++	test_when_finished clear_netrc &&
>      +	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)' '
>     ++	test_when_finished clear_netrc &&
>      +	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 (fail on 403)' '
>     ++	test_when_finished clear_netrc &&
>      +	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-netrc-403 2>err &&
>      +	expect_askpass none &&
>      +	grep "The requested URL returned error: 403" err
>      +'
>     -+clear_netrc
>      +
>       test_expect_success 'http auth can use user/pass in URL' '
>       	set_askpass wrong &&
>
>  t/lib-httpd.sh             | 13 +++++++++++--
>  t/lib-httpd/apache.conf    |  4 ++++
>  t/lib-httpd/passwd         |  1 +
>  t/t5550-http-fetch-dumb.sh | 25 +++++++++++++++++++++++++
>  4 files changed, 41 insertions(+), 2 deletions(-)
>
> diff --git a/t/lib-httpd.sh b/t/lib-httpd.sh
> index 5091db949b..5f42c311c2 100644
> --- a/t/lib-httpd.sh
> +++ b/t/lib-httpd.sh
> @@ -319,13 +319,22 @@ setup_askpass_helper() {
>  	'
>  }
>  
> -set_askpass() {
> +set_askpass () {
>  	>"$TRASH_DIRECTORY/askpass-query" &&
>  	echo "$1" >"$TRASH_DIRECTORY/askpass-user" &&
>  	echo "$2" >"$TRASH_DIRECTORY/askpass-pass"
>  }
>  
> -expect_askpass() {
> +set_netrc () {
> +	# $HOME=$TRASH_DIRECTORY
> +	echo "machine $1 login $2 password $3" >"$TRASH_DIRECTORY/.netrc"
> +}
> +
> +clear_netrc () {
> +	rm -f "$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..9530f01b9e 100755
> --- a/t/t5550-http-fetch-dumb.sh
> +++ b/t/t5550-http-fetch-dumb.sh
> @@ -102,6 +102,31 @@ test_expect_success 'cloning password-protected repository can fail' '
>  	expect_askpass both wrong
>  '
>  
> +test_expect_success 'using credentials from netrc to clone successfully' '
> +	test_when_finished clear_netrc &&
> +	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
> +'
> +
> +test_expect_success 'netrc unauthorized credentials (prompt after 401)' '
> +	test_when_finished clear_netrc &&
> +	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
> +'
> +
> +test_expect_success 'netrc authorized but forbidden credentials (fail on 403)' '
> +	test_when_finished clear_netrc &&
> +	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-netrc-403 2>err &&
> +	expect_askpass none &&
> +	grep "The requested URL returned error: 403" err
> +'
> +
>  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 &&

[PATCH v3] t5550: add netrc tests for http 401/403

[Ashlesh Gawande]

git allows using .netrc file to supply credentials for HTTP auth.
Three test cases are added in this patch to provide missing coverage
when cloning over HTTP using .netrc file:

  - First test case checks that the git clone is successful when credentials
    are provided via .netrc file
  - Second test case checks that the git clone fails when the .netrc file
    provides invalid credentials. The HTTP server is expected to return
    401 Unauthorized in such a case. The test checks that the user is
    provided with a prompt for username/password on 401 to provide
    the valid ones.
  - Third test case checks that the git clone fails when the .netrc file
    provides credentials that are valid but do not have permission for
    this user. For example one may have multiple tokens in GitHub
    and uses the one which was not authorized for cloning this repo.
    In such a case the HTTP server returns 403 Forbidden.
    For this test, the apache.conf is modified to return a 403
    on finding a forbidden-user. No prompt for username/password is
    expected after the 403 (unlike 401). This is because prompting may wipe
    out existing credentials or conflict with custom credential helpers.

Signed-off-by: Ashlesh Gawande <git@ashlesh.me>
---
Range-diff against v2:
1:  0b68f1d1af ! 1:  25ef751f28 t5550: add netrc tests for http 401/403
    @@ Metadata
      ## Commit message ##
         t5550: add netrc tests for http 401/403
     
    +    git allows using .netrc file to supply credentials for HTTP auth.
    +    Three test cases are added in this patch to provide missing coverage
    +    when cloning over HTTP using .netrc file:
    +
    +      - First test case checks that the git clone is successful when credentials
    +        are provided via .netrc file
    +      - Second test case checks that the git clone fails when the .netrc file
    +        provides invalid credentials. The HTTP server is expected to return
    +        401 Unauthorized in such a case. The test checks that the user is
    +        provided with a prompt for username/password on 401 to provide
    +        the valid ones.
    +      - Third test case checks that the git clone fails when the .netrc file
    +        provides credentials that are valid but do not have permission for
    +        this user. For example one may have multiple tokens in GitHub
    +        and uses the one which was not authorized for cloning this repo.
    +        In such a case the HTTP server returns 403 Forbidden.
    +        For this test, the apache.conf is modified to return a 403
    +        on finding a forbidden-user. No prompt for username/password is
    +        expected after the 403 (unlike 401). This is because prompting may wipe
    +        out existing credentials or conflict with custom credential helpers.
    +
         Signed-off-by: Ashlesh Gawande <git@ashlesh.me>
     
      ## t/lib-httpd.sh ##

 t/lib-httpd.sh             | 13 +++++++++++--
 t/lib-httpd/apache.conf    |  4 ++++
 t/lib-httpd/passwd         |  1 +
 t/t5550-http-fetch-dumb.sh | 25 +++++++++++++++++++++++++
 4 files changed, 41 insertions(+), 2 deletions(-)

diff --git a/t/lib-httpd.sh b/t/lib-httpd.sh
index 5091db949b..5f42c311c2 100644
--- a/t/lib-httpd.sh
+++ b/t/lib-httpd.sh
@@ -319,13 +319,22 @@ setup_askpass_helper() {
 	'
 }
 
-set_askpass() {
+set_askpass () {
 	>"$TRASH_DIRECTORY/askpass-query" &&
 	echo "$1" >"$TRASH_DIRECTORY/askpass-user" &&
 	echo "$2" >"$TRASH_DIRECTORY/askpass-pass"
 }
 
-expect_askpass() {
+set_netrc () {
+	# $HOME=$TRASH_DIRECTORY
+	echo "machine $1 login $2 password $3" >"$TRASH_DIRECTORY/.netrc"
+}
+
+clear_netrc () {
+	rm -f "$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..9530f01b9e 100755
--- a/t/t5550-http-fetch-dumb.sh
+++ b/t/t5550-http-fetch-dumb.sh
@@ -102,6 +102,31 @@ test_expect_success 'cloning password-protected repository can fail' '
 	expect_askpass both wrong
 '
 
+test_expect_success 'using credentials from netrc to clone successfully' '
+	test_when_finished clear_netrc &&
+	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
+'
+
+test_expect_success 'netrc unauthorized credentials (prompt after 401)' '
+	test_when_finished clear_netrc &&
+	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
+'
+
+test_expect_success 'netrc authorized but forbidden credentials (fail on 403)' '
+	test_when_finished clear_netrc &&
+	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-netrc-403 2>err &&
+	expect_askpass none &&
+	grep "The requested URL returned error: 403" err
+'
+
 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 v3] t5550: add netrc tests for http 401/403

[Ashlesh Gawande]

Any other comments or suggestions on this patch that I can address?
(re-sending because the mailing list rejected my previous email for not 
being plain text).

Thanks
Ashlesh

On 1/7/26 13:17, Ashlesh Gawande wrote:
> git allows using .netrc file to supply credentials for HTTP auth.
> Three test cases are added in this patch to provide missing coverage
> when cloning over HTTP using .netrc file:
>
>    - First test case checks that the git clone is successful when credentials
>      are provided via .netrc file
>    - Second test case checks that the git clone fails when the .netrc file
>      provides invalid credentials. The HTTP server is expected to return
>      401 Unauthorized in such a case. The test checks that the user is
>      provided with a prompt for username/password on 401 to provide
>      the valid ones.
>    - Third test case checks that the git clone fails when the .netrc file
>      provides credentials that are valid but do not have permission for
>      this user. For example one may have multiple tokens in GitHub
>      and uses the one which was not authorized for cloning this repo.
>      In such a case the HTTP server returns 403 Forbidden.
>      For this test, the apache.conf is modified to return a 403
>      on finding a forbidden-user. No prompt for username/password is
>      expected after the 403 (unlike 401). This is because prompting may wipe
>      out existing credentials or conflict with custom credential helpers.
>
> Signed-off-by: Ashlesh Gawande <git@ashlesh.me>
> ---
> Range-diff against v2:
> 1:  0b68f1d1af ! 1:  25ef751f28 t5550: add netrc tests for http 401/403
>      @@ Metadata
>        ## Commit message ##
>           t5550: add netrc tests for http 401/403
>       
>      +    git allows using .netrc file to supply credentials for HTTP auth.
>      +    Three test cases are added in this patch to provide missing coverage
>      +    when cloning over HTTP using .netrc file:
>      +
>      +      - First test case checks that the git clone is successful when credentials
>      +        are provided via .netrc file
>      +      - Second test case checks that the git clone fails when the .netrc file
>      +        provides invalid credentials. The HTTP server is expected to return
>      +        401 Unauthorized in such a case. The test checks that the user is
>      +        provided with a prompt for username/password on 401 to provide
>      +        the valid ones.
>      +      - Third test case checks that the git clone fails when the .netrc file
>      +        provides credentials that are valid but do not have permission for
>      +        this user. For example one may have multiple tokens in GitHub
>      +        and uses the one which was not authorized for cloning this repo.
>      +        In such a case the HTTP server returns 403 Forbidden.
>      +        For this test, the apache.conf is modified to return a 403
>      +        on finding a forbidden-user. No prompt for username/password is
>      +        expected after the 403 (unlike 401). This is because prompting may wipe
>      +        out existing credentials or conflict with custom credential helpers.
>      +
>           Signed-off-by: Ashlesh Gawande <git@ashlesh.me>
>       
>        ## t/lib-httpd.sh ##
>
>   t/lib-httpd.sh             | 13 +++++++++++--
>   t/lib-httpd/apache.conf    |  4 ++++
>   t/lib-httpd/passwd         |  1 +
>   t/t5550-http-fetch-dumb.sh | 25 +++++++++++++++++++++++++
>   4 files changed, 41 insertions(+), 2 deletions(-)
>
> diff --git a/t/lib-httpd.sh b/t/lib-httpd.sh
> index 5091db949b..5f42c311c2 100644
> --- a/t/lib-httpd.sh
> +++ b/t/lib-httpd.sh
> @@ -319,13 +319,22 @@ setup_askpass_helper() {
>   	'
>   }
>   
> -set_askpass() {
> +set_askpass () {
>   	>"$TRASH_DIRECTORY/askpass-query" &&
>   	echo "$1" >"$TRASH_DIRECTORY/askpass-user" &&
>   	echo "$2" >"$TRASH_DIRECTORY/askpass-pass"
>   }
>   
> -expect_askpass() {
> +set_netrc () {
> +	# $HOME=$TRASH_DIRECTORY
> +	echo "machine $1 login $2 password $3" >"$TRASH_DIRECTORY/.netrc"
> +}
> +
> +clear_netrc () {
> +	rm -f "$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..9530f01b9e 100755
> --- a/t/t5550-http-fetch-dumb.sh
> +++ b/t/t5550-http-fetch-dumb.sh
> @@ -102,6 +102,31 @@ test_expect_success 'cloning password-protected repository can fail' '
>   	expect_askpass both wrong
>   '
>   
> +test_expect_success 'using credentials from netrc to clone successfully' '
> +	test_when_finished clear_netrc &&
> +	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
> +'
> +
> +test_expect_success 'netrc unauthorized credentials (prompt after 401)' '
> +	test_when_finished clear_netrc &&
> +	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
> +'
> +
> +test_expect_success 'netrc authorized but forbidden credentials (fail on 403)' '
> +	test_when_finished clear_netrc &&
> +	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-netrc-403 2>err &&
> +	expect_askpass none &&
> +	grep "The requested URL returned error: 403" err
> +'
> +
>   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 &&

Re: [PATCH v3] t5550: add netrc tests for http 401/403

[Junio C Hamano]

Ashlesh Gawande <git@ashlesh.me> writes:

> git allows using .netrc file to supply credentials for HTTP auth.
> Three test cases are added in this patch to provide missing coverage
> when cloning over HTTP using .netrc file:
>
>   - First test case checks that the git clone is successful when credentials
>     are provided via .netrc file
>   - Second test case checks that the git clone fails when the .netrc file
>     provides invalid credentials. The HTTP server is expected to return
>     401 Unauthorized in such a case. The test checks that the user is
>     provided with a prompt for username/password on 401 to provide
>     the valid ones.
>   - Third test case checks that the git clone fails when the .netrc file
>     provides credentials that are valid but do not have permission for
>     this user. For example one may have multiple tokens in GitHub
>     and uses the one which was not authorized for cloning this repo.
>     In such a case the HTTP server returns 403 Forbidden.
>     For this test, the apache.conf is modified to return a 403
>     on finding a forbidden-user. No prompt for username/password is
>     expected after the 403 (unlike 401). This is because prompting may wipe
>     out existing credentials or conflict with custom credential helpers.

Nicely summarised.  So we say 401 when we do not know you, while we
say 403 when we know you and do not want you to be accessing the
resource.  We test for both.

Just out of curiosity, do we test for these codes with other
credential helpers or is this only relevant for .netrc users?

> +test_expect_success 'using credentials from netrc to clone successfully' '
> +	test_when_finished clear_netrc &&
> +	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
> +'
> +
> +test_expect_success 'netrc unauthorized credentials (prompt after 401)' '
> +	test_when_finished clear_netrc &&
> +	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
> +'
> +
> +test_expect_success 'netrc authorized but forbidden credentials (fail on 403)' '
> +	test_when_finished clear_netrc &&
> +	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-netrc-403 2>err &&
> +	expect_askpass none &&
> +	grep "The requested URL returned error: 403" err
> +'
> +
>  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 &&

Re: [PATCH v3] t5550: add netrc tests for http 401/403

[Jeff King]

On Thu, Feb 05, 2026 at 09:05:51PM -0800, Junio C Hamano wrote:

> >   - Third test case checks that the git clone fails when the .netrc file
> >     provides credentials that are valid but do not have permission for
> >     this user. For example one may have multiple tokens in GitHub
> >     and uses the one which was not authorized for cloning this repo.
> >     In such a case the HTTP server returns 403 Forbidden.
> >     For this test, the apache.conf is modified to return a 403
> >     on finding a forbidden-user. No prompt for username/password is
> >     expected after the 403 (unlike 401). This is because prompting may wipe
> >     out existing credentials or conflict with custom credential helpers.
> 
> Nicely summarised.  So we say 401 when we do not know you, while we
> say 403 when we know you and do not want you to be accessing the
> resource.  We test for both.

I think it is fine to check the 403 handling, but note that this _isn't_
how GitHub would respond. If you try to fetch from a repository you
don't have access to, it will return a 401 first (so you try to log in)
and then a 404. The idea being to avoid revealing the existence of the
repository to unauthorized users.

> Just out of curiosity, do we test for these codes with other
> credential helpers or is this only relevant for .netrc users?

The netrc support here should not involve credential helpers at all. It
is all being done internally by curl. So in this (third and final) test:

> > +test_expect_success 'netrc authorized but forbidden credentials (fail on 403)' '
> > +	test_when_finished clear_netrc &&
> > +	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-netrc-403 2>err &&
> > +	expect_askpass none &&
> > +	grep "The requested URL returned error: 403" err
> > +'

...what is happening is roughly:

  - curl sends the first request with no credentials, which gets a 401

  - curl internally, without returning a response to Git, looks up the
    netrc value and repeats the request with an Authorization header

  - curl returns the resulting 403 to Git

  - Git calls this an error (just like it would a 404) and bails

But from Git's perspective the use of netrc here is not really
interesting. We don't even know it happened! And if the server did
return a 401, we'd happily try to get credentials (from the user or from
a helper) in the usual way. And that's what happens in the second test:

> > +test_expect_success 'netrc unauthorized credentials (prompt after 401)' '
> > +	test_when_finished clear_netrc &&
> > +	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
> > +'

Curl tries the credential under the hood, but we have no idea, and we
process a 401 in the usual way.

And in the first one:

> > +test_expect_success 'using credentials from netrc to clone successfully' '
> > +	test_when_finished clear_netrc &&
> > +	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
> > +'

We do not ever even see the 401, and curl just magically handles it for
us. We see only the successful 200 code, just as if authentication was
not required in the first place.


So really, none of this is testing anything novel in Git at all that is
not covered elsewhere, except for the fact that we pass the flag to curl
that says "you may use netrc". And so there's some value in adding it in
that case. But trying to answer your question about other credential
helpers, no, they're not even entering the picture here.

-Peff

Re: [PATCH v3] t5550: add netrc tests for http 401/403

[Ashlesh Gawande]

On 2/6/26 15:08, Jeff King wrote:
> On Thu, Feb 05, 2026 at 09:05:51PM -0800, Junio C Hamano wrote:
>
>>>    - Third test case checks that the git clone fails when the .netrc file
>>>      provides credentials that are valid but do not have permission for
>>>      this user. For example one may have multiple tokens in GitHub
>>>      and uses the one which was not authorized for cloning this repo.
>>>      In such a case the HTTP server returns 403 Forbidden.
>>>      For this test, the apache.conf is modified to return a 403
>>>      on finding a forbidden-user. No prompt for username/password is
>>>      expected after the 403 (unlike 401). This is because prompting may wipe
>>>      out existing credentials or conflict with custom credential helpers.
>> Nicely summarised.  So we say 401 when we do not know you, while we
>> say 403 when we know you and do not want you to be accessing the
>> resource.  We test for both.
> I think it is fine to check the 403 handling, but note that this _isn't_
> how GitHub would respond. If you try to fetch from a repository you
> don't have access to, it will return a 401 first (so you try to log in)
> and then a 404. The idea being to avoid revealing the existence of the
> repository to unauthorized users.
In the case of fine-grained access token such that the token has read 
access to the repository
but not write access GitHub does return a 403.
(I think this is correct behavior as the token has read access so user 
is authorized/knows about the repository).
>> Just out of curiosity, do we test for these codes with other
>> credential helpers or is this only relevant for .netrc users?
> The netrc support here should not involve credential helpers at all. It
> is all being done internally by curl. So in this (third and final) test:
>
>>> +test_expect_success 'netrc authorized but forbidden credentials (fail on 403)' '
>>> +	test_when_finished clear_netrc &&
>>> +	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-netrc-403 2>err &&
>>> +	expect_askpass none &&
>>> +	grep "The requested URL returned error: 403" err
>>> +'
> ...what is happening is roughly:
>
>    - curl sends the first request with no credentials, which gets a 401
>
>    - curl internally, without returning a response to Git, looks up the
>      netrc value and repeats the request with an Authorization header
>
>    - curl returns the resulting 403 to Git
>
>    - Git calls this an error (just like it would a 404) and bails
>
> But from Git's perspective the use of netrc here is not really
> interesting. We don't even know it happened! And if the server did
> return a 401, we'd happily try to get credentials (from the user or from
> a helper) in the usual way. And that's what happens in the second test:
>
>>> +test_expect_success 'netrc unauthorized credentials (prompt after 401)' '
>>> +	test_when_finished clear_netrc &&
>>> +	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
>>> +'
> Curl tries the credential under the hood, but we have no idea, and we
> process a 401 in the usual way.
>
> And in the first one:
>
>>> +test_expect_success 'using credentials from netrc to clone successfully' '
>>> +	test_when_finished clear_netrc &&
>>> +	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
>>> +'
> We do not ever even see the 401, and curl just magically handles it for
> us. We see only the successful 200 code, just as if authentication was
> not required in the first place.
>
>
> So really, none of this is testing anything novel in Git at all that is
> not covered elsewhere, except for the fact that we pass the flag to curl
> that says "you may use netrc". And so there's some value in adding it in
> that case. But trying to answer your question about other credential
> helpers, no, they're not even entering the picture here.
>
> -Peff
>

Re: [PATCH v3] t5550: add netrc tests for http 401/403

[Ashlesh Gawande]

On 2/6/26 20:55, Ashlesh Gawande wrote:
>
> On 2/6/26 15:08, Jeff King wrote:
>> On Thu, Feb 05, 2026 at 09:05:51PM -0800, Junio C Hamano wrote:
>>
>>>>    - Third test case checks that the git clone fails when the 
>>>> .netrc file
>>>>      provides credentials that are valid but do not have permission 
>>>> for
>>>>      this user. For example one may have multiple tokens in GitHub
>>>>      and uses the one which was not authorized for cloning this repo.
>>>>      In such a case the HTTP server returns 403 Forbidden.
>>>>      For this test, the apache.conf is modified to return a 403
>>>>      on finding a forbidden-user. No prompt for username/password is
>>>>      expected after the 403 (unlike 401). This is because prompting 
>>>> may wipe
>>>>      out existing credentials or conflict with custom credential 
>>>> helpers.
>>> Nicely summarised.  So we say 401 when we do not know you, while we
>>> say 403 when we know you and do not want you to be accessing the
>>> resource.  We test for both.
>> I think it is fine to check the 403 handling, but note that this _isn't_
>> how GitHub would respond. If you try to fetch from a repository you
>> don't have access to, it will return a 401 first (so you try to log in)
>> and then a 404. The idea being to avoid revealing the existence of the
>> repository to unauthorized users.
> In the case of fine-grained access token such that the token has read 
> access to the repository
> but not write access GitHub does return a 403.
> (I think this is correct behavior as the token has read access so user 
> is authorized/knows about the repository).
So should I modify that test case to do a push instead for this specific 
scenario (and update the description)?
>>> Just out of curiosity, do we test for these codes with other
>>> credential helpers or is this only relevant for .netrc users?
>> The netrc support here should not involve credential helpers at all. It
>> is all being done internally by curl. So in this (third and final) test:
>>
>>>> +test_expect_success 'netrc authorized but forbidden credentials 
>>>> (fail on 403)' '
>>>> +    test_when_finished clear_netrc &&
>>>> +    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-netrc-403 2>err &&
>>>> +    expect_askpass none &&
>>>> +    grep "The requested URL returned error: 403" err
>>>> +'
>> ...what is happening is roughly:
>>
>>    - curl sends the first request with no credentials, which gets a 401
>>
>>    - curl internally, without returning a response to Git, looks up the
>>      netrc value and repeats the request with an Authorization header
>>
>>    - curl returns the resulting 403 to Git
>>
>>    - Git calls this an error (just like it would a 404) and bails
>>
>> But from Git's perspective the use of netrc here is not really
>> interesting. We don't even know it happened! And if the server did
>> return a 401, we'd happily try to get credentials (from the user or from
>> a helper) in the usual way. And that's what happens in the second test:
>>
>>>> +test_expect_success 'netrc unauthorized credentials (prompt after 
>>>> 401)' '
>>>> +    test_when_finished clear_netrc &&
>>>> +    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
>>>> +'
>> Curl tries the credential under the hood, but we have no idea, and we
>> process a 401 in the usual way.
>>
>> And in the first one:
>>
>>>> +test_expect_success 'using credentials from netrc to clone 
>>>> successfully' '
>>>> +    test_when_finished clear_netrc &&
>>>> +    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
>>>> +'
>> We do not ever even see the 401, and curl just magically handles it for
>> us. We see only the successful 200 code, just as if authentication was
>> not required in the first place.
>>
>>
>> So really, none of this is testing anything novel in Git at all that is
>> not covered elsewhere, except for the fact that we pass the flag to curl
>> that says "you may use netrc". And so there's some value in adding it in
>> that case. But trying to answer your question about other credential
>> helpers, no, they're not even entering the picture here.
>>
>> -Peff
>>
>

Re: [PATCH v3] t5550: add netrc tests for http 401/403

[Jeff King]

On Fri, Feb 06, 2026 at 09:23:18PM +0530, Ashlesh Gawande wrote:

> > > I think it is fine to check the 403 handling, but note that this _isn't_
> > > how GitHub would respond. If you try to fetch from a repository you
> > > don't have access to, it will return a 401 first (so you try to log in)
> > > and then a 404. The idea being to avoid revealing the existence of the
> > > repository to unauthorized users.
> > In the case of fine-grained access token such that the token has read
> > access to the repository
> > but not write access GitHub does return a 403.
> > (I think this is correct behavior as the token has read access so user
> > is authorized/knows about the repository).

Ah, that makes sense.

> So should I modify that test case to do a push instead for this specific
> scenario (and update the description)?

No, I think what you have is fine. From the client's perspective, they
know only that they got a 403 for some reason. So there's no need for
complex modeling of what the server thinks is going on.

-Peff

Re: [PATCH v3] t5550: add netrc tests for http 401/403

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> I think it is fine to check the 403 handling, but note that this _isn't_
> how GitHub would respond. If you try to fetch from a repository you
> don't have access to, it will return a 401 first (so you try to log in)
> and then a 404. The idea being to avoid revealing the existence of the
> repository to unauthorized users.

That is a sensible thing to do on the server side.  Presumably when
we talk with such a server we would report 404, right?  It is not
like we behave all that differently with either type of errors---as
long as we just give up and do not fall into an infinite loop of
asking "oops, that password did not work, try again", it would be
OK.

>> Just out of curiosity, do we test for these codes with other
>> credential helpers or is this only relevant for .netrc users?
>
> The netrc support here should not involve credential helpers at all. It
> is all being done internally by curl.

Yeah, I phrased my question in a wrong way.  As the code paths
involving credential helpers are separate, I wondered if we have
similar test coverage there as well.

> So really, none of this is testing anything novel in Git at all that is
> not covered elsewhere, except for the fact that we pass the flag to curl
> that says "you may use netrc". And so there's some value in adding it in
> that case. But trying to answer your question about other credential
> helpers, no, they're not even entering the picture here.

Re: [PATCH v3] t5550: add netrc tests for http 401/403

[Jeff King]

On Fri, Feb 06, 2026 at 09:39:54AM -0800, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > I think it is fine to check the 403 handling, but note that this _isn't_
> > how GitHub would respond. If you try to fetch from a repository you
> > don't have access to, it will return a 401 first (so you try to log in)
> > and then a 404. The idea being to avoid revealing the existence of the
> > repository to unauthorized users.
> 
> That is a sensible thing to do on the server side.  Presumably when
> we talk with such a server we would report 404, right?  It is not
> like we behave all that differently with either type of errors---as
> long as we just give up and do not fall into an infinite loop of
> asking "oops, that password did not work, try again", it would be
> OK.

Right, we'd report the 404. We never loop on trying to authenticate, but
do a maximum of two tries (and then only if we get a 401 on the first
request and did not already provide a credential ourselves to curl).
Curl might make multiple requests under the hood for each "try", but we
won't even know about them.

And all of that is independent of which HTTP error code was returned
(except for 401, obviously). We do eventually produce a different
message for 404 vs a 403, but that's at the top-level of remote-curl.c.

The interesting bits are in http_request_reauth(), though some of the
logic is in handle_curl_result().

> > The netrc support here should not involve credential helpers at all. It
> > is all being done internally by curl.
> 
> Yeah, I phrased my question in a wrong way.  As the code paths
> involving credential helpers are separate, I wondered if we have
> similar test coverage there as well.

The workings are hopefully covered by the explanation above. As far as
test coverage, I think t5550 covers this already. When we provide the
wrong password, we bail rather than asking repeatedly (e.g., in "cloning
password-protected repository can fail").

-Peff