feat(evaulator): store package_name and evr for all cve types + evr cache in evaluator

[jdobes]

Secure Coding Practices Checklist GitHub Link

• https://github.com/RedHatInsights/secure-coding-checklist

Secure Coding Checklist

• Input Validation
• Output Encoding
• Authentication and Password Management
• Session Management
• Access Control
• Cryptographic Practices
• Error Handling and Logging
• Data Protection
• Communication Security
• System Configuration
• Database Security
• File Management
• Memory Management
• General Coding Practices

Summary by Sourcery

Persist package and version-release metadata from VMAAS evaluations while improving evaluator caching and local database initialization.

New Features:

• Store package names and parsed EVR data for all affected CVE types returned by VMAAS.
• Add evaluator EVR caching and architecture-aware EVRA parsing.

Bug Fixes:

• Prevent local development fixtures from overwriting existing system data.

Enhancements:

• Add supported package architecture constants and persist EVR records alongside vulnerability evaluations.

Build:

• Update the minimum database schema version required by the container.

[sourcery-ai]

Reviewer's Guide

The evaluator now captures affected package names and parses, caches, and persists their EVR data from VMAAS responses for playbook, manually fixable, and unpatched CVEs, while local database initialization becomes idempotent and the container targets the newer schema version.

Sequence diagram for persisting VMAAS package and EVR data

sequenceDiagram
    participant Evaluator
    participant VMAAS
    participant PackageCache
    participant EVRCache
    participant Database

    Evaluator->>VMAAS: vmaas_request
    VMAAS-->>Evaluator: CVEs with affected package_name and evra
    loop affected packages
        Evaluator->>PackageCache: _get_or_upsert_package_name
        alt package name not cached
            PackageCache->>Database: _insert_package_name
        end
        Evaluator->>EVRCache: _get_or_upsert_evra
        EVRCache->>EVRCache: _parse_evra
        alt EVR not cached
            EVRCache->>Database: _insert_evr
        end
    end

Loading

File-Level Changes

Change	Details	Files
Persist VMAAS-reported affected package names and EVR records across all CVE evaluation paths.	Add EVRA parsing with epoch, version, release, and validated architecture handling. Load existing EVR rows into an in-memory cache and upsert newly encountered EVRs. Process affected package/EVRA mappings for playbook, manually fixable, and unpatched CVEs.	evaluator/common.py evaluator/logic.py
Add a shared architecture enum for validating database-supported package architectures.	Define supported RPM architecture values used during EVRA parsing.	common/constants.py
Prevent local development data from being reinserted into populated databases.	Check for existing system platforms before loading mock data.	database/schema/local_init_db.sh
Raise the container’s minimum required database schema version.	Update the schema wait gate from version 158 to 172.	Dockerfile

---

Tips and commands

Interacting with Sourcery

• Trigger a new review: Comment @sourcery-ai review on the pull request.
• Continue discussions: Reply directly to Sourcery's review comments.
• Generate a GitHub issue from a review comment: Ask Sourcery to create an
  issue from a review comment by replying to it. You can also reply to a
  review comment with @sourcery-ai issue to create an issue from it.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a title at any time. You can also comment
  @sourcery-ai title on the pull request to (re-)generate the title at any time.
• Generate a pull request summary: Write @sourcery-ai summary anywhere in
  the pull request body to generate a PR summary at any time exactly where you
  want it. You can also comment @sourcery-ai summary on the pull request to
  (re-)generate the summary at any time.
• Generate reviewer's guide: Comment @sourcery-ai guide on the pull
  request to (re-)generate the reviewer's guide at any time.
• Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
  pull request to resolve all Sourcery comments. Useful if you've already
  addressed all the comments and don't want to see them anymore.
• Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
  request to dismiss all existing Sourcery reviews. Especially useful if you
  want to start fresh with a new review - don't forget to comment
  @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

• Enable or disable review features such as the Sourcery-generated pull request
  summary, the reviewer's guide, and others.
• Change the review language.
• Add, remove or edit custom review instructions.
• Adjust other review settings.

Getting Help

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[sourcery-ai]

Hey - I've found 5 issues

Prompt for AI Agents

Please address the comments from this code review:

## Individual Comments

### Comment 1
<location path="evaluator/logic.py" line_range="399-402" />
<code_context>
+        if not parsed_evra:
+            return None
+
+        key = (parsed_evra.epoch, parsed_evra.version, parsed_evra.release)
+        if key not in self.evr_cache:
+            await self._insert_evr(*key)
+        return self.evr_cache[key]
+
+    @staticmethod
</code_context>
<issue_to_address>
**issue (broader_impact):** The parsed architecture is discarded before persistence: `_get_or_upsert_evra` builds a cache key containing only epoch, version, and release, and `_insert_evr` inserts only those three fields. Every affected architecture returned by VMAAS is therefore lost, so the new architecture information cannot populate the database's affected_arch data.

**Triggers:** When VMAAS returns affected packages with architecture information.

**Suggested fix:** Preserve the parsed `arch` when creating the vulnerability-detail rows, and populate the corresponding `affected_arch` field along with the EVR ID.
</issue_to_address>

### Comment 2
<location path="evaluator/logic.py" line_range="635-637" />
<code_context>
             # system is potentially vulnerable to cves returned from vmaas
             for cve_adv in playbook_cves:
                 cve = await self._get_or_upsert_cve(cve_adv.name)
+                for package_name, evra in cve_adv.affected_packages.items():
+                    await self._get_or_upsert_package_name(package_name)
+                    await self._get_or_upsert_evra(evra)
                 sys_vuln_rows[cve_adv.name] = SystemVulnerabilitiesRow(
                     VulnerabilityState.VULNERABLE_BY_PACKAGE,
</code_context>
<issue_to_address>
**issue (bug_risk):** The IDs returned by the package-name and EVR upserts are ignored, and no affected package or EVR IDs are assigned to the `SystemVulnerabilitiesRow` or vulnerable-package records. The evaluator consequently creates orphaned package_name/evr rows while vulnerability records retain NULL package-detail references.

**Triggers:** For CVEs processed through the playbook, manually-fixable, or unpatched paths.

**Suggested fix:** Store the returned package-name and EVR cache IDs in the corresponding vulnerability-detail fields when constructing or updating the vulnerability records.
</issue_to_address>

### Comment 3
<location path="evaluator/logic.py" line_range="459" />
<code_context>
+                CveAdvisories(
+                    cve["cve"],
+                    ",".join(sorted(cve["errata"] or [])) or None,
+                    {affected_package["package_name"]: affected_package["evra"] for affected_package in cve.get("affected", [])},
+                )
+                for cve in vmaas_response.get("cve_list", [])
</code_context>
<issue_to_address>
**issue (bug_risk):** The dictionary comprehension uses package name as the sole key, so multiple affected entries with the same package name overwrite one another. When VMAAS returns the same package for multiple architectures or EVR values, all but the last entry are dropped and are never persisted.

**Triggers:** When a CVE's affected list contains more than one entry for the same package name.

**Suggested fix:** Represent affected packages as a list or key the mapping by a distinct package/architecture/EVR combination instead of package name alone.
</issue_to_address>

### Comment 4
<location path="evaluator/logic.py" line_range="407" />
<code_context>
+    @staticmethod
+    def _parse_evra(evra: str) -> Optional[Evra]:
+        """Parse EVRA data returned by VMAAS and validate its architecture"""
+        match = EVRA_RE.match(evra)
+        if not match:
+            LOGGER.warning("unable to parse EVRA from VMAAS: %s", evra)
+            return None
+
+        try:
</code_context>
<issue_to_address>
**issue (bug_risk):** `_parse_evra` uses `EVRA_RE.match` without requiring the match to consume the entire string, so malformed EVRA values with a valid prefix and trailing characters are accepted and persisted as if they were valid.

**Triggers:** When VMAAS supplies an EVRA value with unexpected trailing text.

**Suggested fix:** Anchor the expression with `$` or use a full-match operation before persisting the parsed value.

```suggestion
        match = EVRA_RE.fullmatch(evra)
```
</issue_to_address>

### Comment 5
<location path="database/schema/local_init_db.sh" line_range="16-23" />
<code_context>
-echo "Inserting mock data."
-psql_exec ./database/schema/ve_db_dev_data.sql
+# Try to initialize local schema, if there are no data
+EXISTING_DATA=$(echo "select 1 from system_platform limit 1" | psql_exec - | sed 's/[[:space:]]//g')
+RETVAL=$?
+if [[ "$RETVAL" == "0" && "$EXISTING_DATA" != "1" ]]; then
+    echo "Inserting mock data."
+    psql_exec ./database/schema/ve_db_dev_data.sql
+else
+    echo "Skipping mock data insert, some systems already present."
+fi
</code_context>
<issue_to_address>
**issue (bug_risk):** If the existence query fails, the script enters the `else` branch and reports that systems already exist while skipping development-data insertion. A failed query caused by an unavailable or incomplete schema is therefore silently treated as a populated database.

**Triggers:** When `select 1 from system_platform limit 1` returns a nonzero status.

**Suggested fix:** Handle query failure separately and exit with an error; only skip insertion when the query succeeds and returns `1`.

```suggestion
EXISTING_DATA=$(echo "select 1 from system_platform limit 1" | psql_exec -)
RETVAL=$?
EXISTING_DATA=$(echo "$EXISTING_DATA" | sed 's/[[:space:]]//g')
if [[ "$RETVAL" != "0" ]]; then
    echo "Failed to query existing systems." >&2
    exit "$RETVAL"
elif [[ "$EXISTING_DATA" != "1" ]]; then
    echo "Inserting mock data."
    psql_exec ./database/schema/ve_db_dev_data.sql
else
    echo "Skipping mock data insert, some systems already present."
fi
```
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook