Skip to content

Fix CVE details page 400 error by respecting no-tags patterns #1164

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.

Sign up for GitHub

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

Open
nofaralfasi wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

Fix CVE details page 400 error by respecting no-tags patterns #1164

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 12 additions & 5 deletions app/services/foreman_rh_cloud/insights_api_forwarder.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,14 +224,21 @@ def original_headers(original_request)
def scope_request?(original_request, path)
return nil unless original_request.get?

# Only consider patterns that define tag_name - this ensures patterns without
# tag_name (permission-only entries) cannot override tag-supporting patterns
matching_patterns = SCOPED_REQUESTS.select { |pattern| pattern[:tag_name] && pattern[:test].match?(path) }
# Find patterns that match this path AND are relevant for GET requests.
# A pattern is relevant if it either:
# - Has tag_name defined (supports scoping)
# - Has GET permissions defined (explicitly handles GET, even without tags)
# This ensures patterns like api/vulnerability/v1/cves/[^/]+$ (GET without tags)
# take precedence over general patterns, while POST-only patterns are ignored.
matching_patterns = SCOPED_REQUESTS.select do |pattern|
pattern[:test].match?(path) && (pattern[:tag_name] || pattern.dig(:permissions, 'GET'))
Copy link
Contributor

@vkrizan vkrizan Mar 4, 2026

Choose a reason for hiding this comment

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

I see that this combines two unrelated things: 1) that it needs to have a tag definition; or 2) it needs to have a GET permissions.

Could you elaborate on the requirement for testing the existence of a tag definition? Could there be just one requirement for GET permissions definition? Or is this kept for a backwards compatibility?

Copy link
Collaborator Author

@nofaralfasi nofaralfasi Mar 5, 2026

Choose a reason for hiding this comment

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

Let me explain why I beleieve both conditions are needed.
Looking at the SCOPED_REQUESTS patterns, there are different categories:
Category A: Patterns with tag_name but NO permissions defined:
These patterns exist purely for tagging - they don't enforce permissions but do add tags to GET requests. Without the tag_name check, these would be excluded.

Category B: Patterns with GET permissions but NO tag_name:
These patterns explicitly say "this GET endpoint does NOT support tags". Without the GET permissions check, these would be excluded (the bug we're fixing).

If we only used pattern.dig(:permissions, 'GET'):

  • api/inventory/.* and api/tasks/.* would be excluded (breaking change)
  • GET requests to those endpoints would no longer get tags

So yes, it combines two concepts, but both are required to:

  • Fix the CVE details bug (needs GET check)
  • Maintain backwards compatibility for tagging-only patterns (needs tag_name check)

Copy link
Contributor

@vkrizan vkrizan Mar 5, 2026

Choose a reason for hiding this comment

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

Thanks for the explanation!

end
return nil if matching_patterns.empty?

# Choose the most specific pattern by regex source length for consistency
# with required_permission_for behavior
# Choose the most specific pattern by regex source length
request_pattern = matching_patterns.max_by { |pattern| pattern[:test].source.length }

# Return the tag_name (may be nil if the most specific pattern doesn't support tags)
request_pattern[:tag_name]
end

Expand Down