Repository navigation
Move jq_filter in content_guard behind a feature flag #8331
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| Disabled complex `jq_filter` programs in `HeaderContentGuard` by default. Simple dotted paths continue to work. Use `settings.ALLOW_INSECURE_HEADER_CONTENT_GUARD_JQ_PROCESSING_AND_FORFEIT_ALL_COMMUNITY_SUPPORT` if you __need__ to use it. CVE-2026-123084 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,10 +7,10 @@ | |
| from base64 import b64decode | ||
| from binascii import Error as Base64DecodeError | ||
| from datetime import timedelta | ||
| from functools import reduce | ||
| from gettext import gettext as _ | ||
| from urllib.parse import urljoin, urlparse | ||
|
|
||
| import jq | ||
| from aiohttp.web_exceptions import HTTPNotFound | ||
| from django.conf import settings | ||
| from django.contrib.postgres.fields import HStoreField | ||
|
|
@@ -35,6 +35,13 @@ | |
|
|
||
| _logger = logging.getLogger(__name__) | ||
|
|
||
| if settings.ALLOW_INSECURE_HEADER_CONTENT_GUARD_JQ_PROCESSING_AND_FORFEIT_ALL_COMMUNITY_SUPPORT: # noqa | ||
| import jq | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we keep the import as it was before? What is the benefit of this change?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Being sure that we really don't use it. |
||
| else: | ||
| # Match a dotted identifier path. | ||
| # e.g.: "tenant.auth.token" | ||
| json_path_re = re.compile(r"(?:\.\w+)+", flags=re.ASCII) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we define the regex outside if/else, we would not need to duplicate it again in the serializer... |
||
|
|
||
|
|
||
| class PublicationQuerySet(models.QuerySet): | ||
| """A queryset that provides publication filtering methods.""" | ||
|
|
@@ -544,12 +551,32 @@ def permit(self, request): | |
|
|
||
| if self.jq_filter: | ||
| try: | ||
| header_value = json.loads(header_decoded_content) | ||
| json_path = jq.compile(self.jq_filter) | ||
| header_json = json.loads(header_decoded_content) | ||
| if settings.ALLOW_INSECURE_HEADER_CONTENT_GUARD_JQ_PROCESSING_AND_FORFEIT_ALL_COMMUNITY_SUPPORT: # noqa | ||
| # DISCLAIMER: | ||
| # The prelude attempts to strip the environment from the jq srcipt. | ||
| # That does not mean it is safe to allow users to define a script to run on | ||
| # the content app. | ||
| JQ_PRELUDE = r"def env: {}; {} as $ENV |" | ||
| json_path = jq.compile( | ||
| f"{JQ_PRELUDE} {self.jq_filter}", | ||
| args={ | ||
| "JQ_BUILD_CONFIGURATION": "", | ||
| "ARGS": {"positional": [], "named": {}}, | ||
| }, | ||
| ) | ||
|
|
||
| header_value = json_path.input_value(header_value).first() | ||
| header_value = json_path.input_value(header_json).first() | ||
|
|
||
| except json.JSONDecodeError: | ||
| elif json_path_re.fullmatch(self.jq_filter): | ||
| path = self.jq_filter.lstrip(".").split(".") | ||
| header_value = reduce(lambda x, y: x[y], path, header_json) | ||
| else: | ||
| _logger.warning( | ||
| "Using 'jq_filter' is only allowed with simple paths on this installation." | ||
| ) | ||
| raise PermissionError(_("ContentGuard misconfigured.")) | ||
| except (TypeError, KeyError, json.JSONDecodeError): | ||
| _logger.debug("Access not allowed - Invalid JSON or Path not found.") | ||
| raise PermissionError(_("Access denied.")) | ||
| else: | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -456,6 +456,11 @@ | |
| # Replaces every non PulpException error msg with "An internal error occured" | ||
| REDACT_UNSAFE_EXCEPTIONS = False | ||
|
|
||
| # JQ is a query language after all. And it should be considered on the same level as SQL. | ||
| # Do not allow users to write arbitrary programs to run on the content app. | ||
| # Exposes to at least CVE-2026-123084. | ||
| ALLOW_INSECURE_HEADER_CONTENT_GUARD_JQ_PROCESSING_AND_FORFEIT_ALL_COMMUNITY_SUPPORT = False | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should be documented in docs/admin/reference/settings.md. And maybe also in docs/user/guides/protect-content.md? Also, would not be
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I disagree, as any of that would encourage using it.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let me say it like this: The name is purposefully making it really icky to use. |
||
|
|
||
| # HERE STARTS DYNACONF EXTENSION LOAD (Keep at the very bottom of settings.py) | ||
| # Read more at https://www.dynaconf.com/django/ | ||
|
|
||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.