Keep GHES asset links that redirect to the login page

What does this MR do and why?

AttachmentsDownloader#perform returns the attachment URL as a String instead of a File when a GitHub Enterprise server answers the download with a redirect to its login page. NoteAttachmentsImporter#download_attachment only recognised that signal for URLs matching <web_endpoint>/.*/files/, so a /user-attachments/assets/<uuid> URL fell through to update_ghe_video_path, which called .path on a String. ObjectImporter rescues the NoMethodError, so the import looked successful while the description kept the unrewritten GHES link and nothing was reported.

This MR treats any String result as the skip signal, since the downloader returns one in exactly that case, and removes the now unused github_file_url_regex.

Why the web_endpoint scope is dropped rather than kept

Removing it means the check now applies to github.com imports as well, so it is worth saying why that is uniform rather than wider.

perform returns a String at one place only, return file_url if download_url.include?("login?return_to="). download_url comes from get_assets_download_redirection_url, which returns one of three things:

• file_url unchanged, when the URL does not match #{web_endpoint}/.*/(assets|files)/
• the location header, when the request redirects and redirects are not followed
• file_url again, when the response is not a redirect

The first and third only carry login?return_to= if the attachment URL already did, which GitHub does not generate. The second is the GHE case this MR is about. So on github.com the String branch is not reached in practice.

The more important point is that the guard was never really about the host. It keys off the response being a login redirect. If github.com ever did answer an attachment fetch that way, returning the original URL is still the outcome we want, because the alternative is handing UploadService the HTML of a login page and storing that as the attachment. Dropping the web_endpoint scope therefore makes the behaviour uniform rather than widening the risk.

That is pinned by a spec on a plain github.com endpoint, in the when the downloader returns a URL instead of a file context, so it is covered rather than only reasoned about.

References

• Resolves #621704 (closed)

Differences

Before: Importing an issue whose description contains a GHES /user-attachments/assets/ URL that redirects to /login?return_to= raises NoMethodError: undefined method 'path' for String, which is swallowed; the description keeps the GHES URL and no failure is recorded.

After: The attachment is left as the original URL (the same behaviour /files/ URLs already had) and the importer continues without raising.

How to set up and validate locally

Run the importer spec:

bundle exec rspec spec/lib/gitlab/github_import/importer/note_attachments_importer_spec.rb

The context when an asset URL redirects to the GHE login page fails on master with the NoMethodError and passes with this MR. The context when the downloader returns a URL instead of a file pins the same handling on a github.com endpoint.

Against a real GHES instance:

1. Import an issue whose description contains ![img](https://<ghes>/user-attachments/assets/<uuid>), where the asset request answers with a 302 to /login?return_to=.
2. On master the import records a NoMethodError in import_failures and keeps the raw URL.
3. With this MR the link is left as it is and nothing raises.

MR acceptance checklist

Evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.