Skip to content

Commit cb4d20f

Browse files
authored
Merge pull request from GHSA-mp38-vprc-7hf5
Ref GHSA-mp38-vprc-7hf5
1 parent f607db6 commit cb4d20f

File tree

5 files changed

+75
-5
lines changed

5 files changed

+75
-5
lines changed

readthedocs/projects/models.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1823,6 +1823,7 @@ def add_features(sender, **kwargs):
18231823
ALLOW_FORCED_REDIRECTS = "allow_forced_redirects"
18241824
DISABLE_PAGEVIEWS = "disable_pageviews"
18251825
DISABLE_SPHINX_DOMAINS = "disable_sphinx_domains"
1826+
RESOLVE_PROJECT_FROM_HEADER = "resolve_project_from_header"
18261827

18271828
# Versions sync related features
18281829
SKIP_SYNC_TAGS = 'skip_sync_tags'
@@ -1844,6 +1845,7 @@ def add_features(sender, **kwargs):
18441845
DEFAULT_TO_FUZZY_SEARCH = 'default_to_fuzzy_search'
18451846
INDEX_FROM_HTML_FILES = 'index_from_html_files'
18461847

1848+
# Build related features
18471849
LIST_PACKAGES_INSTALLED_ENV = "list_packages_installed_env"
18481850
VCS_REMOTE_LISTING = "vcs_remote_listing"
18491851
SPHINX_PARALLEL = "sphinx_parallel"
@@ -1933,6 +1935,10 @@ def add_features(sender, **kwargs):
19331935
DISABLE_SPHINX_DOMAINS,
19341936
_("Disable indexing of sphinx domains"),
19351937
),
1938+
(
1939+
RESOLVE_PROJECT_FROM_HEADER,
1940+
_("Allow usage of the X-RTD-Slug header"),
1941+
),
19361942

19371943
# Versions sync related features
19381944
(

readthedocs/proxito/middleware.py

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
import structlog
1313
from django.conf import settings
14+
from django.core.exceptions import SuspiciousOperation
1415
from django.http import Http404
1516
from django.shortcuts import redirect, render
1617
from django.urls import reverse
@@ -24,7 +25,7 @@
2425
unresolver,
2526
)
2627
from readthedocs.core.utils import get_cache_tag
27-
from readthedocs.projects.models import Domain, Project, ProjectRelationship
28+
from readthedocs.projects.models import Domain, Feature, Project, ProjectRelationship
2829
from readthedocs.proxito import constants
2930

3031
log = structlog.get_logger(__name__) # noqa
@@ -47,15 +48,26 @@ def map_host_to_project(request): # pylint: disable=too-many-return-statements
4748
"""
4849

4950
host = unresolver.get_domain_from_host(request.get_host())
51+
log.bind(host=host)
5052

5153
# Explicit Project slug being passed in.
5254
if "HTTP_X_RTD_SLUG" in request.META:
5355
project_slug = request.headers["X-RTD-Slug"].lower()
54-
project = Project.objects.filter(slug=project_slug).first()
56+
project = Project.objects.filter(
57+
slug=project_slug,
58+
feature__feature_id=Feature.RESOLVE_PROJECT_FROM_HEADER,
59+
).first()
5560
if project:
5661
request.rtdheader = True
57-
log.info('Setting project based on X_RTD_SLUG header.', project_slug=project_slug)
62+
log.info(
63+
"Setting project based on X_RTD_SLUG header.", project_slug=project_slug
64+
)
5865
return project
66+
log.warning(
67+
"X-RTD-Header passed for project without it enabled.",
68+
project_slug=project_slug,
69+
)
70+
raise SuspiciousOperation("Invalid X-RTD-Slug header.")
5971

6072
try:
6173
project, domain_object, external_version_slug = unresolver.unresolve_domain(
@@ -248,15 +260,24 @@ def add_cache_headers(self, request, response):
248260
249261
If privacy levels are enabled and the header isn't already present,
250262
set the cache level to private.
263+
Or if the request was from the `X-RTD-Slug` header,
264+
we don't cache the response, since we could be caching a response in another domain.
251265
252266
We use ``CDN-Cache-Control``, to control caching at the CDN level only.
253267
This doesn't affect caching at the browser level (``Cache-Control``).
254268
255269
See https://developers.cloudflare.com/cache/about/cdn-cache-control.
256270
"""
271+
header = "CDN-Cache-Control"
272+
# Never trust projects resolving from the X-RTD-Slug header,
273+
# we don't want to cache their content on domains from other
274+
# projects, see GHSA-mp38-vprc-7hf5.
275+
if hasattr(request, "rtdheader"):
276+
response.headers[header] = "private"
277+
257278
if settings.ALLOW_PRIVATE_REPOS:
258279
# Set the key to private only if it hasn't already been set by the view.
259-
response.headers.setdefault('CDN-Cache-Control', 'private')
280+
response.headers.setdefault(header, "private")
260281

261282
def process_request(self, request): # noqa
262283
skip = any(

readthedocs/proxito/tests/test_full.py

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1457,6 +1457,21 @@ def test_cache_public_versions_custom_domain(self):
14571457
self.assertEqual(resp.headers['CDN-Cache-Control'], 'public')
14581458
self.assertEqual(resp.headers['Cache-Tag'], 'subproject,subproject:latest')
14591459

1460+
def test_cache_disable_on_rtd_header_resolved_project(self):
1461+
get(
1462+
Feature,
1463+
feature_id=Feature.RESOLVE_PROJECT_FROM_HEADER,
1464+
projects=[self.project],
1465+
)
1466+
resp = self.client.get(
1467+
"/en/latest/index.html",
1468+
secure=True,
1469+
HTTP_HOST="docs.example.com",
1470+
HTTP_X_RTD_SLUG=self.project.slug,
1471+
)
1472+
self.assertEqual(resp.headers["CDN-Cache-Control"], "private")
1473+
self.assertEqual(resp.headers["Cache-Tag"], "project,project:latest")
1474+
14601475
def test_cache_on_plan(self):
14611476
self.organization = get(Organization)
14621477
self.plan = get(

readthedocs/proxito/tests/test_middleware.py

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,15 @@
22
from unittest import mock
33

44
import pytest
5+
from django.core.exceptions import SuspiciousOperation
56
from django.http import HttpRequest
67
from django.test import TestCase
78
from django.test.utils import override_settings
89
from django_dynamic_fixture import get
910

1011
from readthedocs.builds.models import Version
1112
from readthedocs.projects.constants import PUBLIC
12-
from readthedocs.projects.models import Domain, Project, ProjectRelationship
13+
from readthedocs.projects.models import Domain, Feature, Project, ProjectRelationship
1314
from readthedocs.proxito.middleware import ProxitoMiddleware
1415
from readthedocs.rtd_tests.base import RequestFactoryTestMixin
1516
from readthedocs.rtd_tests.storage import BuildMediaFileSystemStorageTest
@@ -163,6 +164,9 @@ def test_subdomain_different_length(self):
163164
self.assertEqual(request.host_project_slug, 'pip')
164165

165166
def test_request_header(self):
167+
get(
168+
Feature, feature_id=Feature.RESOLVE_PROJECT_FROM_HEADER, projects=[self.pip]
169+
)
166170
request = self.request(
167171
method='get', path=self.url, HTTP_HOST='some.random.com', HTTP_X_RTD_SLUG='pip'
168172
)
@@ -171,6 +175,9 @@ def test_request_header(self):
171175
self.assertEqual(request.host_project_slug, 'pip')
172176

173177
def test_request_header_uppercase(self):
178+
get(
179+
Feature, feature_id=Feature.RESOLVE_PROJECT_FROM_HEADER, projects=[self.pip]
180+
)
174181
request = self.request(
175182
method='get', path=self.url, HTTP_HOST='some.random.com', HTTP_X_RTD_SLUG='PIP'
176183
)
@@ -179,6 +186,16 @@ def test_request_header_uppercase(self):
179186
self.assertEqual(request.rtdheader, True)
180187
self.assertEqual(request.host_project_slug, 'pip')
181188

189+
def test_request_header_not_allowed(self):
190+
request = self.request(
191+
method="get",
192+
path=self.url,
193+
HTTP_HOST="docs.example.com",
194+
HTTP_X_RTD_SLUG="pip",
195+
)
196+
with pytest.raises(SuspiciousOperation):
197+
self.run_middleware(request)
198+
182199
def test_long_bad_subdomain(self):
183200
domain = 'www.pip.dev.readthedocs.io'
184201
request = self.request(method='get', path=self.url, HTTP_HOST=domain)

readthedocs/proxito/tests/test_old_redirects.py

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
from django.urls import Resolver404
1717

1818
from readthedocs.builds.models import Version
19+
from readthedocs.projects.models import Feature
1920
from readthedocs.redirects.models import Redirect
2021

2122
from .base import BaseDocServing
@@ -513,6 +514,11 @@ def test_redirect_recognizes_custom_cname(self):
513514
from_url='/install.html',
514515
to_url='/tutorial/install.html',
515516
)
517+
fixture.get(
518+
Feature,
519+
feature_id=Feature.RESOLVE_PROJECT_FROM_HEADER,
520+
projects=[self.project],
521+
)
516522
r = self.client.get(
517523
'/install.html',
518524
HTTP_HOST='docs.project.io',
@@ -850,6 +856,11 @@ def test_redirect_recognizes_custom_cname(self):
850856
to_url="/tutorial/install.html",
851857
force=True,
852858
)
859+
fixture.get(
860+
Feature,
861+
feature_id=Feature.RESOLVE_PROJECT_FROM_HEADER,
862+
projects=[self.project],
863+
)
853864
r = self.client.get(
854865
"/en/latest/install.html",
855866
HTTP_HOST="docs.project.io",

0 commit comments

Comments
 (0)