Skip to content

Commit e8c9636

Browse files
authored
Merge commit from fork
1 parent d0b3908 commit e8c9636

File tree

10 files changed

+219
-4
lines changed

10 files changed

+219
-4
lines changed

readthedocs/api/v2/serializers.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,7 @@ class Meta(ProjectSerializer.Meta):
9494
"max_concurrent_builds",
9595
"readthedocs_yaml_path",
9696
"clone_token",
97+
"has_ssh_key_with_write_access",
9798
)
9899

99100

readthedocs/doc_builder/director.py

Lines changed: 33 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
from readthedocs.projects.constants import BUILD_COMMANDS_OUTPUT_PATH_HTML
2929
from readthedocs.projects.constants import GENERIC
3030
from readthedocs.projects.exceptions import RepositoryError
31+
from readthedocs.projects.notifications import MESSAGE_PROJECT_SSH_KEY_WITH_WRITE_ACCESS
3132
from readthedocs.projects.signals import after_build
3233
from readthedocs.projects.signals import before_build
3334
from readthedocs.projects.signals import before_vcs
@@ -205,6 +206,23 @@ def checkout(self):
205206
log.info("Cloning and fetching.")
206207
self.vcs_repository.update()
207208

209+
# Check if the key has write access to the repository (RTD Business only).
210+
# This check is done immediately after clone step, and before running any
211+
# commands that make use of user given input (like the post_checkout job).
212+
has_ssh_key_with_write_access = False
213+
if settings.ALLOW_PRIVATE_REPOS:
214+
has_ssh_key_with_write_access = self.vcs_repository.has_ssh_key_with_write_access()
215+
if has_ssh_key_with_write_access != self.data.project.has_ssh_key_with_write_access:
216+
self.data.api_client.project(self.data.project.pk).patch(
217+
{"ssh_key_with_write_access": has_ssh_key_with_write_access}
218+
)
219+
if has_ssh_key_with_write_access:
220+
self.attach_notification(
221+
attached_to=f"project/{self.data.project.pk}",
222+
message_id=MESSAGE_PROJECT_SSH_KEY_WITH_WRITE_ACCESS,
223+
dismissable=True,
224+
)
225+
208226
identifier = self.data.build_commit or self.data.version.identifier
209227
log.info("Checking out.", identifier=identifier)
210228
self.vcs_repository.checkout(identifier)
@@ -262,6 +280,13 @@ def checkout(self):
262280

263281
self.vcs_repository.update_submodules(self.data.config)
264282

283+
# If the config has a post_checkout job, we stop the build,
284+
# as it could be abused to write to the repository.
285+
if has_ssh_key_with_write_access and get_dotted_attribute(
286+
self.data.config, "build.jobs.post_checkout", None
287+
):
288+
raise BuildUserError(BuildUserError.SSH_KEY_WITH_WRITE_ACCESS)
289+
265290
# System dependencies (``build.apt_packages``)
266291
# NOTE: `system_dependencies` should not be possible to override by the
267292
# user because it's executed as ``RTD_DOCKER_USER`` (e.g. ``root``) user.
@@ -794,20 +819,26 @@ def store_readthedocs_build_yaml(self):
794819

795820
def attach_notification(
796821
self,
822+
attached_to,
797823
message_id,
798824
format_values=None,
799825
state="unread",
800826
dismissable=False,
801827
news=False,
802828
):
803-
"""Attach a notification to build in progress using the APIv2."""
829+
"""
830+
Attach a notification to build in progress using the APIv2.
831+
832+
:param attached_to: The object to which the notification is attached.
833+
It should have the form of `project/{project_id}` or `build/{build_id}`.
834+
"""
804835

805836
format_values = format_values or {}
806837
# NOTE: we are using APIv2 here because it uses BuildAPIKey authentication,
807838
# which is not currently supported by APIv3.
808839
self.data.api_client.notifications.post(
809840
{
810-
"attached_to": f"build/{self.data.build['id']}",
841+
"attached_to": attached_to,
811842
"message_id": message_id,
812843
"state": state, # Optional
813844
"dismissable": dismissable,

readthedocs/doc_builder/exceptions.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,8 @@ class BuildUserError(BuildBaseException):
5252
BUILD_EXCESSIVE_MEMORY = "build:user:excessive-memory"
5353
VCS_DEPRECATED = "build:vcs:deprecated"
5454

55+
SSH_KEY_WITH_WRITE_ACCESS = "build:user:ssh-key-with-write-access"
56+
5557

5658
class BuildMaxConcurrencyError(BuildUserError):
5759
LIMIT_REACHED = "build:user:concurrency-limit-reached"

readthedocs/notifications/messages.py

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -218,6 +218,21 @@ def get_rendered_body(self):
218218
),
219219
type=ERROR,
220220
),
221+
Message(
222+
id=BuildUserError.SSH_KEY_WITH_WRITE_ACCESS,
223+
header=_("Build aborted due to SSH key with write access."),
224+
body=_(
225+
textwrap.dedent(
226+
"""
227+
This build has failed because the current deploy key on the repository was created with write permission.
228+
For protection against abuse we've restricted use of these deploy keys.
229+
A read-only deploy key will need to be set up <b>before July 31, 2025</b> to continue building this project.
230+
Read more about this in our <a href="https://about.readthedocs.com/blog/2025/05/ssh-keys-with-write-access/">blog post</a>.
231+
"""
232+
).strip(),
233+
),
234+
type=ERROR,
235+
),
221236
Message(
222237
id=BuildAppError.BUILD_DOCKER_UNKNOWN_ERROR,
223238
header=_("Build terminated due to unknown error."),
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
# Generated by Django 4.2.20 on 2025-05-06 22:55
2+
3+
from django.db import migrations
4+
from django.db import models
5+
from django_safemigrate import Safe
6+
7+
8+
class Migration(migrations.Migration):
9+
safe = Safe.before_deploy()
10+
11+
dependencies = [
12+
("projects", "0148_remove_unused_indexes"),
13+
]
14+
15+
operations = [
16+
migrations.AddField(
17+
model_name="historicalproject",
18+
name="has_ssh_key_with_write_access",
19+
field=models.BooleanField(
20+
default=False, help_text="Project has an SSH key with write access", null=True
21+
),
22+
),
23+
migrations.AddField(
24+
model_name="project",
25+
name="has_ssh_key_with_write_access",
26+
field=models.BooleanField(
27+
default=False, help_text="Project has an SSH key with write access", null=True
28+
),
29+
),
30+
]

readthedocs/projects/models.py

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import hashlib
55
import hmac
66
import os
7+
import re
78
from shlex import quote
89
from urllib.parse import urlparse
910

@@ -619,6 +620,14 @@ class Project(models.Model):
619620
),
620621
)
621622

623+
# Keep track if the SSH key has write access or not (RTD Business),
624+
# so we can take further actions if needed.
625+
has_ssh_key_with_write_access = models.BooleanField(
626+
help_text=_("Project has an SSH key with write access"),
627+
default=False,
628+
null=True,
629+
)
630+
622631
# Property used for storing the latest build for a project when prefetching
623632
LATEST_BUILD_CACHE = "_latest_build"
624633

@@ -897,6 +906,17 @@ def clean_repo(self):
897906
return self.repo.replace("http://github.com", "https://github.com")
898907
return self.repo
899908

909+
@property
910+
def repository_html_url(self):
911+
if self.remote_repository:
912+
return self.remote_repository.html_url
913+
914+
ssh_url_pattern = re.compile(r"^(?P<user>.+)@(?P<host>.+):(?<repo>.+)$")
915+
match = ssh_url_pattern.match(self.repo)
916+
if match:
917+
return f"https://{match.group('host')}/{match.group('repo')}"
918+
return self.repo
919+
900920
# Doc PATH:
901921
# MEDIA_ROOT/slug/checkouts/version/<repo>
902922

readthedocs/projects/notifications.py

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@
1717

1818
MESSAGE_PROJECT_SKIP_BUILDS = "project:invalid:skip-builds"
1919
MESSAGE_PROJECT_ADDONS_BY_DEFAULT = "project:addons:by-default"
20+
MESSAGE_PROJECT_SSH_KEY_WITH_WRITE_ACCESS = "project:ssh-key-with-write-access"
21+
2022
messages = [
2123
Message(
2224
id=MESSAGE_PROJECT_SKIP_BUILDS,
@@ -176,5 +178,20 @@
176178
),
177179
type=INFO,
178180
),
181+
Message(
182+
id=MESSAGE_PROJECT_SSH_KEY_WITH_WRITE_ACCESS,
183+
header=_("SSH key with read-only access is required"),
184+
body=_(
185+
textwrap.dedent(
186+
"""
187+
This project has a deploy key with write access to the repository.
188+
For protection against abuse we've restricted use of these deploy keys.
189+
A read-only deploy key will need to be set up <b>before July 31, 2025</b> to continue building this project.
190+
Read more about this in our <a href="https://about.readthedocs.com/blog/2025/05/ssh-keys-with-write-access/">blog post</a>.
191+
"""
192+
).strip(),
193+
),
194+
type=WARNING,
195+
),
179196
]
180197
registry.add(messages)

readthedocs/projects/tasks/builds.py

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -496,10 +496,15 @@ def on_failure(self, exc, task_id, args, kwargs, einfo):
496496
# It may happens the director is not created because the API failed to retrieve
497497
# required data to initialize it on ``before_start``.
498498
if self.data.build_director:
499+
self.data.build_director.attach_notification(
500+
attached_to=f"build/{self.data.build['id']}",
501+
message_id=message_id,
502+
format_values=format_values,
503+
)
504+
else:
499505
log.warning(
500506
"We couldn't attach a notification to the build since it failed on an early stage."
501507
)
502-
self.data.build_director.attach_notification(message_id, format_values)
503508

504509
# Send notifications for unhandled errors
505510
if message_id not in self.exceptions_without_notifications:
@@ -709,7 +714,8 @@ def on_retry(self, exc, task_id, args, kwargs, einfo):
709714
# Grab the format values from the exception in case it contains
710715
format_values = exc.format_values if hasattr(exc, "format_values") else None
711716
self.data.build_director.attach_notification(
712-
BuildMaxConcurrencyError.LIMIT_REACHED,
717+
attached_to=f"build/{self.data.build['id']}",
718+
message_id=BuildMaxConcurrencyError.LIMIT_REACHED,
713719
format_values=format_values,
714720
)
715721
self.update_build(state=BUILD_STATE_TRIGGERED)

readthedocs/rtd_tests/tests/test_api.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3441,6 +3441,7 @@ def test_get_version_by_id(self):
34413441
"users": [1],
34423442
"custom_prefix": None,
34433443
"clone_token": None,
3444+
"has_ssh_key_with_write_access": False,
34443445
},
34453446
"privacy_level": "public",
34463447
"downloads": {},

readthedocs/vcs_support/backends/git.py

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,98 @@ def clone(self):
155155
message_id = RepositoryError.CLONE_ERROR_WITH_PRIVATE_REPO_ALLOWED
156156
raise RepositoryError(message_id=message_id) from exc
157157

158+
def has_ssh_key_with_write_access(self) -> bool:
159+
"""
160+
Check if the SSH key has write access to the repository.
161+
162+
This is done by trying to push to the repository in dry-run mode.
163+
164+
We create a temporary remote to test the SSH key,
165+
since we need the remote to be in the SSH format,
166+
which isn't always the case for projects that are public,
167+
and changing the URL of the default remote may be a breaking
168+
change for some projects.
169+
170+
.. note::
171+
172+
This check is better done just after the clone step,
173+
to ensure that no commands controled by the user are run.
174+
"""
175+
remote_name = "rtd-test-ssh-key"
176+
ssh_url = re.sub("https?://github.com/", "[email protected]:", self.project.repo, 1)
177+
178+
try:
179+
cmd = ["git", "remote", "add", remote_name, ssh_url]
180+
self.run(*cmd, record=False)
181+
182+
cmd = ["git", "push", "--dry-run", remote_name]
183+
code, stdout, stderr = self.run(*cmd, record=False, demux=True)
184+
185+
if code == 0:
186+
return True
187+
188+
# NOTE: if the command fails, it doesn't necessarily mean the key
189+
# doesn't have write access, there are a couple of other reasons
190+
# why this may fail, so we check for the error message.
191+
192+
# This error is shown when the repo is archived, but the key has write access.
193+
if "ERROR: This repository was archived so it is read-only" in stderr:
194+
return True
195+
196+
# This error is shown when the repo is empty, but we don't know if the key has write access or not.
197+
# We assume it has write access just to be safe, future steps will fail after the repo is cloned anyway.
198+
# Example: error: src refspec refs/heads/master does not match any
199+
pattern = r"error: src refspec refs/heads/\w does not match any"
200+
if re.search(pattern, stderr):
201+
return True
202+
203+
# Example: ERROR: Permission to user/repo denied to deploy key
204+
pattern = r"ERROR: Permission to .* denied to deploy key"
205+
if re.search(pattern, stderr):
206+
return False
207+
208+
errors_read_access_only = [
209+
"ERROR: The key you are authenticating with has been marked as read only",
210+
"ERROR: Write access to repository not granted",
211+
# This error is shown when the key isn't registered in GH at all.
212+
"[email protected]: Permission denied (publickey).",
213+
# We don't know if the key has write access or not,
214+
# but since the key can't be used from the builders,
215+
# it should be safe to return False.
216+
"ERROR: The repository owner has an IP allow list enabled",
217+
]
218+
for pattern in errors_read_access_only:
219+
if pattern in stderr:
220+
return False
221+
222+
# This is an error when the repo URL is not a git+ssh URL,
223+
# we don't know if the key has write access or not, as the URL is invalid.
224+
# Log and return False for now.
225+
if "fatal: could not read Username for" in stderr:
226+
log.error(
227+
"Invalid repo URL for SSH key check.",
228+
project_slug=self.project.slug,
229+
repo_url=self.project.repo,
230+
ssh_url=ssh_url,
231+
exit_code=code,
232+
stdout=stdout,
233+
stderr=stderr,
234+
)
235+
return False
236+
237+
# Log any other errors that we don't know about.
238+
log.error(
239+
"Unknown error when checking SSH key access.",
240+
project_slug=self.project.slug,
241+
exit_code=code,
242+
stdout=stdout,
243+
stderr=stderr,
244+
)
245+
return False
246+
finally:
247+
# Always remove the temporary remote.
248+
self.run("git", "remote", "remove", remote_name, record=False)
249+
158250
def fetch(self):
159251
# --force: Likely legacy, it seems to be irrelevant to this usage
160252
# --prune: Likely legacy, we don't expect a previous fetch command to have run

0 commit comments

Comments
 (0)