/go/libraries/doltcore/gitremote: clean up errors

go/libraries/doltcore/gitremote/errors.go

@@ -160,12 +160,12 @@ func classifyError(err error) error {
 	errStr := strings.ToLower(err.Error())
 
 	// Authentication errors
+	// Note: "permission denied" can be auth (e.g. SSH publickey) or authz (repo access).
+	// We treat explicit SSH failures as auth, and everything else as permission denied below.
 	if strings.Contains(errStr, "authentication") ||
-		strings.Contains(errStr, "permission denied") ||
 		strings.Contains(errStr, "publickey") ||
 		strings.Contains(errStr, "invalid credentials") ||
-		strings.Contains(errStr, "401") ||
-		strings.Contains(errStr, "403") {
+		strings.Contains(errStr, "401") {
 		return fmt.Errorf("%w: %v", ErrAuthFailed, err)
 	}
 
@@ -199,7 +199,9 @@ func classifyError(err error) error {
 	}
 
 	// Permission denied
-	if strings.Contains(errStr, "access denied") {
+	if strings.Contains(errStr, "permission denied") ||
+		strings.Contains(errStr, "access denied") ||
+		strings.Contains(errStr, "403") {
 		return fmt.Errorf("%w: %v", ErrPermissionDenied, err)
 	}
 
@@ -208,7 +210,8 @@ func classifyError(err error) error {
 
 // IsAuthError returns true if the error is an authentication error.
 func IsAuthError(err error) bool {
-	return errors.Is(err, ErrAuthFailed)
+	// Treat both authentication failures and permission failures as "auth-ish" errors for callers.
+	return errors.Is(err, ErrAuthFailed) || errors.Is(err, ErrPermissionDenied)
 }
 
 // IsPushRejectedError returns true if the error is a push rejection.

go/libraries/doltcore/gitremote/errors_test.go

@@ -141,15 +141,25 @@ func TestClassifyError(t *testing.T) {
 			expectedError: ErrAuthFailed,
 		},
 		{
-			name:          "permission denied",
+			name:          "permission denied (publickey)",
 			inputErr:      errors.New("Permission denied (publickey)"),
 			expectedError: ErrAuthFailed,
 		},
+		{
+			name:          "access denied",
+			inputErr:      errors.New("access denied"),
+			expectedError: ErrPermissionDenied,
+		},
 		{
 			name:          "401 error",
 			inputErr:      errors.New("server returned 401"),
 			expectedError: ErrAuthFailed,
 		},
+		{
+			name:          "403 error",
+			inputErr:      errors.New("server returned 403"),
+			expectedError: ErrPermissionDenied,
+		},
 		{
 			name:          "repository not found",
 			inputErr:      errors.New("repository not found"),
@@ -217,6 +227,7 @@ func TestErrorHelpers(t *testing.T) {
 	t.Run("IsAuthError", func(t *testing.T) {
 		assert.True(t, IsAuthError(ErrAuthFailed))
 		assert.True(t, IsAuthError(classifyError(errors.New("authentication required"))))
+		assert.True(t, IsAuthError(classifyError(errors.New("access denied"))))
 		assert.False(t, IsAuthError(ErrPushRejected))
 	})