-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Invalid submodule URL on public repositories #4043
Comments
We could return the invalid url here https://github.com/rtfd/readthedocs.org/blob/dfc8fc9eba8dc9caae171ca0b3e8f6a71594e088/readthedocs/vcs_support/backends/git.py#L117-L121 and shown that to the use, at least is a better hint.
I think the same, is there any way that users can trespass that? I mean, if we don't have their private keys, then we can't pull their code. |
Hi, I have just had a similar problem, where RTD was reporting:
It took me a while to figure out that it was because the submodule had a SSH URL- in fact I was about to submit an issue. I changed it to HTTPS and the problem went away. I think the message is not very clear. It would be nice also if the RTD manual described this limitation (I tried searching for "submodules" and nothing comes up.) |
working with |
We improved the message shown to the user and I think we can close this issue now. The error message now looks like this:
This is a failed build showing this message https://readthedocs.org/projects/ocdsdeploy/builds/9705262/ |
Some time ago, we added a submodule URL validator as a security step in this commit: 43de909b5. We allow different URLs scheme, and we protect ourself for those that we don't want to support.
Taking a look at Sentry error reports, I foundt that there are a couple of projects that have invalid (for our rules)
git@github.com
submodules URLs for public repositories that are failing because our rules.These projects are, for example:
We marked this as invalid repos since we can't be sure that they will be public, but maybe we will need an extra step before marking them as invalid. I'm not sure, but I'm opening this issue to start the discussion at least.
On the other hand, the only feedback that we are providing to the user is
which, without knowing the rules, it's very hard to realize that you need the HTTPS version of the submodule URL to make it work.
Also, I don't think we want to force our users to use the HTTPS version of the submodule URL (depending on how the module is used, it's not a good workflow for the developers)
Related:
Sentry issue: https://sentry.io/read-the-docs/readthedocs-org/issues/502672091/events/latest/
The text was updated successfully, but these errors were encountered: