Conversation
…orrectly for multi-arch
There was a problem hiding this comment.
Pull request overview
Adds optional vulnerability reporting to the Docker Sonar utility, allowing users to fetch and display vulnerability scan status plus CVE severity counts alongside existing image/tag analysis.
Changes:
- Introduces
--vulnerabilitiesflag and plumbing (fetch_vulns) through the analysis pipeline. - Fetches vulnerability scan data per package (and rolls up child image data into manifest-list parents).
- Extends table/CSV outputs (and underlying row structures) to include vulnerability fields.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| Docker/Sonar/sonar.py | Adds vulnerability fetching/rollups and output formatting; updates function signatures to pass fetch_vulns. |
| CHANGELOG.md | Documents Sonar v1.3 vulnerability flag and related enhancements. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| uploaded_at = find_key_recursive(pkg_details, 'uploaded_at') | ||
| if uploaded_at: | ||
| uploaded = uploaded_at |
There was a problem hiding this comment.
uploaded_at = find_key_recursive(...) returns a list, but the code assigns uploaded = uploaded_at and later returns uploaded in the result. This makes uploaded a list (not a timestamp/string) and also leaves uploaded undefined when uploaded_at is empty or pkg_details is falsy (raising UnboundLocalError). Initialize uploaded up-front and set it to a single value (e.g., uploaded_at[0]) when present.
| # Fallback: use top-level fields from list response if available | ||
| summary["vuln_status"] = "Scanned" | ||
| if latest_scan.get("num_vulnerabilities"): | ||
| _extract_severity_from_scans(latest_scan.get("scans", []), summary) |
There was a problem hiding this comment.
When scan_detail is missing, severity extraction is gated on latest_scan.get("num_vulnerabilities"), which skips extraction when that field is absent or zero. If latest_scan["scans"] is present, aggregate from it regardless of num_vulnerabilities (and still compute totals as 0 when there are none).
| # Fallback: use top-level fields from list response if available | |
| summary["vuln_status"] = "Scanned" | |
| if latest_scan.get("num_vulnerabilities"): | |
| _extract_severity_from_scans(latest_scan.get("scans", []), summary) | |
| # Fallback: use fields from the latest_scan response if available | |
| summary["vuln_status"] = "Scanned" | |
| scan_entries = latest_scan.get("scans") | |
| if scan_entries: | |
| # Prefer aggregating from scans if they are present | |
| _extract_severity_from_scans(scan_entries, summary) | |
| else: | |
| # Fallback: try top-level num_* fields or a vulnerabilities list on latest_scan | |
| for sev in ("critical", "high", "medium", "low", "unknown"): | |
| count = ( | |
| latest_scan.get(f"num_{sev}") | |
| or latest_scan.get(f"{sev}_count") | |
| or latest_scan.get(sev) | |
| ) | |
| if count is not None: | |
| summary[sev] = int(count) | |
| summary["total"] = sum( | |
| summary[s] for s in ("critical", "high", "medium", "low", "unknown") | |
| ) |
| if isinstance(row, dict): | ||
| row['action'] = action_str | ||
| # Remove internal slug | ||
| if 'slug' in row: del row['slug'] |
There was a problem hiding this comment.
Rows from fetch_untagged_data now include slug_perm, but the cleanup step only deletes row['slug']. This leaves an internal identifier in the returned output structures. If slug is considered internal-only here, slug_perm should also be stripped (or both should be kept consistently).
| if 'slug' in row: del row['slug'] | |
| if 'slug' in row: | |
| del row['slug'] | |
| if 'slug_perm' in row: | |
| del row['slug_perm'] |
| for img_name, groups in collected_results: | ||
| for group in groups: | ||
| if group == "SECTION": | ||
| continue | ||
| # Flat CSV row | ||
| csv_lines.append([ | ||
| row = [ |
There was a problem hiding this comment.
The CSV export loop treats each group as a dict (group.get(...)), but collected_results stores groups as a list of group-lists (each group is a list of row dicts + optional "SECTION" markers). As written, --output csv will fail with an AttributeError. Flatten the structure for CSV (iterate group -> row, skipping "SECTION") or change what gets stored in collected_results for CSV output.
📄 Summary
[Sonar] [v1.3]
Added
vulnerabilitiesflag — When set, fetches and displays vulnerability scan status and CVE severity summary (Critical, High, Medium, Low) for each package in the output.🔍 Related Issues
Link to any related GitHub issues (e.g.,
Fixes #12,Closes #34):🧪 Type of Change
Please check the relevant type tag for this PR title:
[FIX]Bug fix[NEW]New thing[REFACTOR]Internal changes such as code restructuring or optimization that does not alter functionality[DOC]Documentation-only changes[CHORE]Maintenance, cleanup, or CI configuration🧪 How Has This Been Tested?
Describe how you tested your changes. Include CI runs, local tests, manual verification, or screenshots if applicable.
📸 Screenshots (if applicable)
If UI or logs are affected, include before/after screenshots or output.
✅ Checklist