Merged
Conversation
|
Referenced Jiras: |
Reviewer's GuideAdds per-host vulnerability evaluation summaries enriched with CVE impact and exploitability to a new Kafka topic for inventory views, and extends messaging/configuration to support headers and the new topic across evaluator, common utilities, and deployment artifacts. Sequence diagram for sending inventory host vulnerability summary to KafkasequenceDiagram
participant Evaluator
participant EvaluatorProcessor
participant EvaluatorLogic
participant CveCache
participant MQWriter_inventoryViews
participant Kafka_inventoryViews
Evaluator->>EvaluatorProcessor: evaluate_system(msg, inventory_id, org_id, request_id, request_timestamp)
EvaluatorProcessor->>EvaluatorProcessor: _evaluate_system(inventory_id, org_id, request_id, request_timestamp, recalc_event_id)
loop over system_vulnerabilities
EvaluatorProcessor->>EvaluatorLogic: evaluator_logic.cve_cache.get(cve)
EvaluatorLogic->>CveCache: lookup(cve)
CveCache-->>EvaluatorLogic: CveCache(id, impact_id, exploitable)
EvaluatorLogic-->>EvaluatorProcessor: CveCache
EvaluatorProcessor->>EvaluatorProcessor: update total_cves, critical_cves, high_severity_cves, cves_with_security_rules, cves_with_known_exploits
end
EvaluatorProcessor->>MQWriter_inventoryViews: send_one(summary_msg, key=None, headers=[application=vulnerability, request_id])
MQWriter_inventoryViews->>Kafka_inventoryViews: send_and_wait(topic=platform.inventory.host-apps, value=summary_msg, headers)
Kafka_inventoryViews-->>MQWriter_inventoryViews: ack
MQWriter_inventoryViews-->>EvaluatorProcessor: result
EvaluatorProcessor-->>Evaluator: evaluation complete
ER diagram for updated cve_metadata usage in CVE cacheerDiagram
cve_metadata {
bigint id PK
varchar cve
smallint impact_id
jsonb exploit_data
}
cve_metadata ||--o{ cve_cache_entry : populates
cve_cache_entry {
bigint id
smallint impact_id
boolean exploitable
}
Updated class diagram for evaluator messaging and CVE cacheclassDiagram
class MQWriter {
-topic
-client
+__init__(topic, bootstrap_servers, loop, kwargs)
+send_one(msg, key, headers)
+send_many(msg_list, key, headers)
+send_raw(msg, key, headers)
-_serialize_key(key)
}
class CveCache {
+id
+impact_id
+exploitable
}
class EvaluatorLogic {
-db_pool
+cve_cache: Dict~str, CveCache~
+_load_cve_cache() Dict~str, CveCache~
}
class Evaluator {
-loop: BaseEventLoop
-db_pool: AsyncConnectionPool
+payload_tracker: MQWriter
+remediations_results: MQWriter
+evaluator_results: MQWriter
+inventory_views_results: MQWriter
+processor: EvaluatorProcessor
+__init__(db_pool, loop)
}
class EvaluatorProcessor {
+remediations_results: MQWriter
+evaluator_results: MQWriter
+inventory_views_results: MQWriter
+payload_tracker: MQWriter
+db_pool: AsyncConnectionPool
+evaluator_logic: EvaluatorLogic
+recent_recalc_events: Dict~int, RecalcEvent~
+__init__(remediations_results, evaluator_results, inventory_views_results, payload_tracker, db_pool, loop)
+evaluate_system(msg, inventory_id, org_id, request_id, request_timestamp, recalc_event_id)
-_evaluate_system(inventory_id, org_id, request_id, request_timestamp, recalc_event_id)
}
EvaluatorLogic --> CveCache : uses
Evaluator --> EvaluatorProcessor : composes
Evaluator --> MQWriter : creates
EvaluatorProcessor --> MQWriter : sends messages via
EvaluatorProcessor --> EvaluatorLogic : uses CVE cache
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
MichaelMraka
force-pushed
the
pr1
branch
2 times, most recently
from
dc684e5 to
d2ed2b1
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2195 +/- ##
==========================================
- Coverage 65.55% 62.41% -3.15%
==========================================
Files 56 58 +2
Lines 6640 6994 +354
==========================================
+ Hits 4353 4365 +12
- Misses 2287 2629 +342 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
MichaelMraka
force-pushed
the
pr1
branch
2 times, most recently
from
05fb781 to
9ae16e3
Compare
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- In
_evaluate_system,cve = await self.evaluator_logic.cve_cache.get(cve)is awaiting a dict lookup and overwriting thecvekey variable;cve_cacheentries are loaded synchronously, so this should be a plain lookup (e.g.cve_data = self.evaluator_logic.cve_cache[cve]) using a different variable name. - In
send_inventory_views, the log messages still refer to 'notification' / 'notificator' even though this function publishes to inventory views; consider updating the log text to reflect the new target to avoid confusion when debugging. send_inventory_viewsassumesrequest_idis always a non-empty ASCII string (bytes(request_id, 'ascii')); if there is any chance it can beNoneor contain non-ASCII characters, consider guarding or normalizing it before constructing the header.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `_evaluate_system`, `cve = await self.evaluator_logic.cve_cache.get(cve)` is awaiting a dict lookup and overwriting the `cve` key variable; `cve_cache` entries are loaded synchronously, so this should be a plain lookup (e.g. `cve_data = self.evaluator_logic.cve_cache[cve]`) using a different variable name.
- In `send_inventory_views`, the log messages still refer to 'notification' / 'notificator' even though this function publishes to inventory views; consider updating the log text to reflect the new target to avoid confusion when debugging.
- `send_inventory_views` assumes `request_id` is always a non-empty ASCII string (`bytes(request_id, 'ascii')`); if there is any chance it can be `None` or contain non-ASCII characters, consider guarding or normalizing it before constructing the header.
## Individual Comments
### Comment 1
<location> `evaluator/processor.py:363` </location>
<code_context>
+ total_cves += 1
+ if row.rule_id is not None:
+ cves_with_security_rules += 1
+ cve = await self.evaluator_logic.cve_cache.get(cve)
+ if cve.impact_id == 7:
+ critical_cves += 1
</code_context>
<issue_to_address>
**issue (bug_risk):** Using `await` on `cve_cache.get` is likely incorrect and will fail if `cve_cache` is a plain dict.
`_load_cve_cache` builds `cve_cache` as a dict of `CveCache` instances, so `.get` is a normal dict lookup, not async. `await` on this will raise `TypeError` at runtime. This should be a plain lookup, e.g. `cve = self.evaluator_logic.cve_cache.get(cve)`, with appropriate handling for a missing key.
</issue_to_address>
### Comment 2
<location> `common/utils.py:372-374` </location>
<code_context>
+ cves_with_known_exploits,
+):
+ """Sends kafka message to inventory views"""
+ if not org_id:
+ LOGGER.info("Attempt to send notification to unknown org_id: %s", rh_account_id)
+ return
+
</code_context>
<issue_to_address>
**suggestion:** Log message for missing `org_id` logs `rh_account_id`, which is potentially misleading.
Since this branch handles a missing `org_id`, the log should highlight `org_id` as the unknown value, and optionally include `rh_account_id` as context. For example: `"Attempt to send inventory views message with unknown org_id: %s (account: %s)"`.
```suggestion
if not org_id:
LOGGER.info(
"Attempt to send inventory views message with unknown org_id: %s (account: %s)",
org_id,
rh_account_id,
)
return
```
</issue_to_address>
### Comment 3
<location> `evaluator/processor.py:370` </location>
<code_context>
+ high_severity_cves += 1
+ if cve.exploitable:
+ cves_with_known_exploits += 1
+ await self._mark_system_evaluated(total_cves, system_platform, conn)
send_remediations_update(self.remediations_results, inventory_id, fixable_sys_vuln_rows)
</code_context>
<issue_to_address>
**nitpick:** `total_cves` only counts fixable CVEs, which may be confusing given the name and usage.
Given it’s only incremented when `row.remediation_type_id > 1`, this value is effectively `fixable_cves_count` and matches `len(fixable_sys_vuln_rows)`. Renaming to something like `fixable_cves` (and/or directly using `len(fixable_sys_vuln_rows)`) would better reflect its meaning and reduce confusion in metrics/logs.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
RHINENG-23481
RHINENG-23481
jdobes
approved these changes
Feb 12, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Secure Coding Practices Checklist GitHub Link
Secure Coding Checklist
Summary by Sourcery
Publish host vulnerability evaluation summaries to the inventory views Kafka topic and enrich CVE caching and messaging to support severity and exploitability metadata.
New Features:
Enhancements: