Readthedocs.org: Invalid submodule URL on public repositories

Created on 1 May 2018 · 3Comments · Source: readthedocs/readthedocs.org

Some time ago, we added a submodule URL validator as a security step in this commit: https://github.com/rtfd/readthedocs.org/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) [email protected] 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

One or more submodule URLs are not valid

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)

https://github.com/rtfd/readthedocs-build/pull/47
#3215
#3857
#2829

Sentry issue: https://sentry.io/read-the-docs/readthedocs-org/issues/502672091/events/latest/

design decision

Source

humitos

Most helpful comment

Hi, I have just had a similar problem, where RTD was reporting:

One or more submodule URLs are not valid.

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.)

jcarrano on 7 Dec 2018

👍3

All 3 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.

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)

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.

stsewd on 26 Oct 2018

👍1

Hi, I have just had a similar problem, where RTD was reporting:

One or more submodule URLs are not valid.

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.)

jcarrano on 7 Dec 2018

👍3

working with https as scheme for submodules isn't ideal. could the URLs be transformed (like the referencing commits in this issue) before the submodules are clones?