Replace deprecated `readthedocs/actions/preview`

[ezio-melotti]

As I was reviewing the python/cpython workflows for security issues, I noticed that documentation-links.yml uses pull_request_target, which is potentially dangerous:

• https://securitylab.github.com/resources/github-actions-preventing-pwn-requests/
• Analyze and document the security implications of pull_request_target trigger readthedocs/actions#45

This is apparently required by readthedocs/actions/preview in order to edit the first PR message and add the link to the doc preview.

However I also noticed that readthedocs/actions/preview is now deprecated and that its README states:

Warning

This action is deprecated and it shouldn't be used.
This feature was included in the Read the Docs application itself.
For more information, check our documentation.

As an alternative they suggest to connect their GitHub app and use that instead. Instead of editing the first comment, the app will add a comment which will list and link to changed/added/deleted files.
Note that the app is still in beta.

If we switch to the app we can stop using the deprecated action get rid of the documentation-links.yml workflow and pull_request_target use.

I brought this up to @hugovk attention, and he suggested to try the app on one of the other (smaller) repos first:

• devguide: devguide#1713
• peps
• python-docs-theme
• docs-community

If it works fine and we are happy with it, we can then update all the other repos (including cpython).

• cpython

cc @humitos

[hugovk]

The links to the changed files looks handy:

[StanFromIreland]

I’m happy to do this if no one else is doing it.

[AA-Turner]

Auto-bot-comment spam on PRs is very annoying. Can we keep the feature of only adding a couple of lines to the PR body itself?

A

[hugovk]

Given that:

• pull_request_target can be insecure
• pull_request_target was exploited in the (ongoing?) Shai-Hulud 2 attack
  • For example, see "Why did it happen?" at https://posthog.com/blog/nov-24-shai-hulud-attack-post-mortem
• despite GitHub making it simpler soon, it still wouldn't have prevented all of the attack, and GitHub admits there's still an elevated risk
• reasoning about these edge cases is complex, it's easier to replace pull_request_target
• we already have plenty of bots commenting (Bedevere, Miss Islington, CLA bot) on cpython PRs

I'm in favour replacing the workflows with the app, to at least test it out.

Otherwise, let's just remove the workflows. We can still access the docs preview via the status checks, although less convenient.

cc @sethmlarson re: security stuff

[sethmlarson]

@hugovk @ezio-melotti Thanks for the analysis, yes let's remove this deprecated workflow and replace it with the application if that's a suitable replacement.

[hugovk]

PR for the devguide: python/devguide#1713

[hugovk]

Let's continue with this.

@JacobCoffee I've invited you to https://app.readthedocs.org/projects/pep-previews/ and https://app.readthedocs.org/projects/python-docs-theme-previews/

No rush, please could you also migrate those?

---

I don't have access to https://app.readthedocs.org/projects/docs-community/

@AA-Turner Please could you add JacobCoffee and hugovk?

[encukou]

Please could you add JacobCoffee and hugovk?

I've added you.

[AA-Turner]

Sorry for the delay! Thanks Petr, looks like Hugo has accepted and the invitation has been sent to Jacob.

A

[hugovk]

Let's also do CPython now as well, we don't need to wait longer. This one can be first.

@JacobCoffee I've invited you to https://app.readthedocs.org/projects/cpython-previews/