Skip to content
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

Closed
humitos opened this issue Apr 30, 2018 · 4 comments
Closed

Invalid submodule URL on public repositories #4043

humitos opened this issue Apr 30, 2018 · 4 comments
Labels
Needed: design decision A core team decision is required

Comments

@humitos
Copy link
Member

humitos commented Apr 30, 2018

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

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)

Related:

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

@stsewd
Copy link
Member

stsewd commented Oct 25, 2018

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.

@humitos humitos added this to the Build error reporting milestone Nov 13, 2018
@jcarrano
Copy link

jcarrano commented Dec 7, 2018

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

lau-jay added a commit to lau-jay/Tornado-doc-cn that referenced this issue Feb 18, 2019
rkkautsar added a commit to rkkautsar/benchmark-tool that referenced this issue Feb 21, 2019
rkkautsar added a commit to rkkautsar/benchmark-tool that referenced this issue Feb 21, 2019
rkkautsar added a commit to daajoe/benchmark-tool that referenced this issue Feb 21, 2019
@funkyfuture
Copy link

funkyfuture commented Mar 16, 2019

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?

@humitos
Copy link
Member Author

humitos commented Mar 15, 2022

We improved the message shown to the user and I think we can close this issue now. The error message now looks like this:

One or more submodule URLs are not valid: ['salt/private', 'pillar/private'], git/ssh URL schemas for submodules are not supported.

This is a failed build showing this message https://readthedocs.org/projects/ocdsdeploy/builds/9705262/

@humitos humitos closed this as completed Mar 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needed: design decision A core team decision is required
Projects
None yet
Development

No branches or pull requests

4 participants