Skip to content

Move jq_filter in content_guard behind a feature flag - #8331

Open
mdellweg wants to merge 1 commit into
pulp:mainfrom
mdellweg:disable_jq_content_guard
Open

Move jq_filter in content_guard behind a feature flag#8331
mdellweg wants to merge 1 commit into
pulp:mainfrom
mdellweg:disable_jq_content_guard

Conversation

@mdellweg

@mdellweg mdellweg commented Nov 11, 2026

Copy link
Copy Markdown
Member

CVE-2026-123084

📜 Checklist

  • Commits are cleanly separated with meaningful messages (simple features and bug fixes should be squashed to one commit)
  • A changelog entry or entries has been added for any significant changes
  • Follows the Pulp policy on AI Usage
  • (For new features) - User documentation and test coverage has been added

See: Pull Request Walkthrough

@mdellweg
mdellweg requested a review from ggainey November 11, 2026 09:45
@mdellweg
mdellweg marked this pull request as ready for review November 11, 2026 12:49
jobselko
jobselko requested changes Nov 11, 2026
View reviewed changes
Comment thread pulpcore/app/models/publication.py
_logger = logging.getLogger(__name__)

if settings.ALLOW_INSECURE_HEADER_CONTENT_GUARD_JQ_PROCESSING_AND_FORFEIT_ALL_COMMUNITY_SUPPORT: # noqa
import jq

@jobselko jobselko Nov 11, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The 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?

@mdellweg mdellweg Nov 12, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Being sure that we really don't use it.

Comment thread pulpcore/app/serializers/publication.py Resolved
Comment thread pulpcore/app/serializers/publication.py Resolved
Comment thread pulpcore/app/settings.py Resolved
Comment thread pulpcore/app/settings.py Resolved
Comment thread pulpcore/app/settings.py

# JQ is a query language after all. And it shoudl be considered on the same level as SQL.
# Do not allow users to write arbitrary programmes to run on the content app.
ALLOW_INSECURE_HEADER_CONTENT_GUARD_JQ_PROCESSING_AND_FORFEIT_ALL_COMMUNITY_SUPPORT = False

@jobselko jobselko Nov 11, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The 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 ALLOW_INSECURE_HEADER_CONTENT_GUARD_JQ enough? :)

@mdellweg mdellweg Nov 12, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I disagree, as any of that would encourage using it.

@mdellweg mdellweg Nov 12, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Comment thread CHANGES/+CVE-2026-123084.bugfix Resolved
@mdellweg
mdellweg force-pushed the disable_jq_content_guard branch from 14122b5 to 524cde9 Compare November 12, 2026 06:34
@mdellweg
mdellweg force-pushed the disable_jq_content_guard branch 2 times, most recently from 9c99079 to 29f3242 Compare November 12, 2026 08:42
@mdellweg
mdellweg requested a review from jobselko November 12, 2026 08:42
@mdellweg
mdellweg force-pushed the disable_jq_content_guard branch from 29f3242 to 16025b6 Compare November 12, 2026 08:56
jobselko
jobselko reviewed Nov 12, 2026
View reviewed changes
Comment thread pulpcore/app/models/publication.py Resolved
Comment thread pulpcore/app/models/publication.py Resolved
Comment thread pulpcore/app/models/publication.py Resolved
Comment thread CHANGES/+CVE-2026-123084.bugfix Resolved
jobselko
jobselko reviewed Nov 12, 2026
View reviewed changes
Comment thread pulpcore/tests/unit/test_content_guard.py

@jobselko jobselko Nov 12, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe I overlooked it, but do these tests contain a regression test?

@mdellweg mdellweg Nov 13, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a test to make sure the feature flag is honored. And now a set of tests that the patched up jq path is not exposing the env as before.

@mdellweg
mdellweg force-pushed the disable_jq_content_guard branch 2 times, most recently from 6234f06 to bd1ec61 Compare November 12, 2026 12:54
jobselko
jobselko reviewed Nov 12, 2026
View reviewed changes
Comment thread pulpcore/app/models/publication.py
else:
# Match a dotted identifier path.
# e.g.: "tenant.auth.token"
json_path_re = re.compile(r"(?:\.\w+)+", flags=re.ASCII)

@jobselko jobselko Nov 12, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Comment thread pulpcore/app/serializers/fields.py Resolved
Comment thread pulpcore/app/serializers/publication.py Resolved
@mdellweg
mdellweg force-pushed the disable_jq_content_guard branch 2 times, most recently from 853246d to 37212c0 Compare November 12, 2026 13:52
@mdellweg
mdellweg force-pushed the disable_jq_content_guard branch from 37212c0 to 34c7ea5 Compare November 13, 2026 13:36
@mdellweg
mdellweg requested a review from jobselko November 13, 2026 13:46

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ggainey ggainey Awaiting requested review from ggainey
@jobselko jobselko Awaiting requested review from jobselko

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

backport-3.49 backport-3.63 backport-3.73 backport-3.85 backport-3.105 backport-3.118 backport-3.123 no-issue

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@mdellweg @jobselko