Add CVE scan workflow with automatic base-verified rebuild dispatch - #74
Add CVE scan workflow with automatic base-verified rebuild dispatch#74roshkhatri wants to merge 5 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughChangesAdds a scheduled and manually triggered CVE scan workflow. It resolves image matrices, scans multiple platforms with Trivy, classifies and verifies findings, publishes outputs and summaries, and conditionally dispatches targeted container rebuilds. ChangesCVE scan workflow
Sequence Diagram(s)sequenceDiagram
participant CVEWorkflow
participant Sweep
participant Trivy
participant BasePrecheck
participant ContainerCI
CVEWorkflow->>Sweep: start configured CVE scan
Sweep->>Trivy: scan images across platforms
Trivy-->>Sweep: return findings
Sweep->>BasePrecheck: verify rebuild-fixable findings
BasePrecheck-->>Sweep: return confirmed versions
Sweep-->>CVEWorkflow: emit fixable and versions outputs
CVEWorkflow->>ContainerCI: dispatch targeted rebuild workflow
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
tests/test_image_matrix.py (1)
105-113: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueNit:
resp.read = resp.read(line 110) is a no-op self-assignment.Harmless leftover; can be removed for clarity.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_image_matrix.py` around lines 105 - 113, Remove the no-op resp.read self-assignment from _mock_urlopen, leaving the mock response behavior and context-manager setup unchanged.tests/test_cve_issue_manager.py (1)
492-510: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueNit: prefer a direct import over the
__import__(...).TRIAGE_ISSUE_TITLEchain (line 499).Other tests already
from scripts.cve_scan.issue_manager import TRIAGE_ISSUE_TITLE; using that here is clearer.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_cve_issue_manager.py` around lines 492 - 510, Replace the dynamic __import__(...).TRIAGE_ISSUE_TITLE access in test_open_issue_commented_and_closed with a direct TRIAGE_ISSUE_TITLE import from scripts.cve_scan.issue_manager, reusing the existing close_triage_issue_all_clear import or consolidating the imports.scripts/cve_scan/image_matrix.py (1)
67-132: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffConsolidate duplicated derivation logic —
_derive_imagesis unused in production.
resolve_matrixderives images fromsorted(base_map.keys())and never calls_derive_images. That leaves two parallel derivation functions (_derive_imagesand_derive_base_map) with the same version-iteration/include_unstable/isinstancerules that must be kept in sync by hand. The direct_derive_imagesunit tests (TestDeriveImages,TestRcEraDerivation) exercise code that never runs in the real path.Consider deriving the image list from
_derive_base_mapoutput (or having one function delegate to the other) and dropping the redundant implementation.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/cve_scan/image_matrix.py` around lines 67 - 132, Consolidate image derivation around _derive_base_map, which is used by resolve_matrix, and remove or delegate _derive_images so version filtering and variant handling have one implementation. Update the direct _derive_images tests, including TestDeriveImages and TestRcEraDerivation, to exercise the production path or the retained shared helper without preserving duplicate logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/architecture.md`:
- Line 390: Update the fenced code block in the architecture documentation flow
diagram to specify the text language, using a text fence while preserving the
diagram content unchanged.
In `@README.md`:
- Line 531: Update the “Emit outputs” documentation to state only that fixable
is written to GITHUB_OUTPUT, removing the unsupported images claim. Keep the
description consistent with _emit_outputs and the downstream job’s
rebuild_all=true behavior.
---
Nitpick comments:
In `@scripts/cve_scan/image_matrix.py`:
- Around line 67-132: Consolidate image derivation around _derive_base_map,
which is used by resolve_matrix, and remove or delegate _derive_images so
version filtering and variant handling have one implementation. Update the
direct _derive_images tests, including TestDeriveImages and TestRcEraDerivation,
to exercise the production path or the retained shared helper without preserving
duplicate logic.
In `@tests/test_cve_issue_manager.py`:
- Around line 492-510: Replace the dynamic __import__(...).TRIAGE_ISSUE_TITLE
access in test_open_issue_commented_and_closed with a direct TRIAGE_ISSUE_TITLE
import from scripts.cve_scan.issue_manager, reusing the existing
close_triage_issue_all_clear import or consolidating the imports.
In `@tests/test_image_matrix.py`:
- Around line 105-113: Remove the no-op resp.read self-assignment from
_mock_urlopen, leaving the mock response behavior and context-manager setup
unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e697fe7e-eb85-452e-a37b-6bb95c770c50
📒 Files selected for processing (21)
.github/workflows/cve-scan.ymlREADME.mddocs/architecture.mdscripts/cve_scan/__init__.pyscripts/cve_scan/base_precheck.pyscripts/cve_scan/config.pyscripts/cve_scan/image_matrix.pyscripts/cve_scan/issue_manager.pyscripts/cve_scan/models.pyscripts/cve_scan/rebuild_decider.pyscripts/cve_scan/scanner.pyscripts/cve_scan/sweep.pyscripts/parsers/__init__.pyscripts/parsers/cve_findings_parser.pytests/test_base_precheck.pytests/test_cve_config.pytests/test_cve_issue_manager.pytests/test_cve_parser.pytests/test_cve_rebuild_decider.pytests/test_cve_sweep_integration.pytests/test_image_matrix.py
There was a problem hiding this comment.
AI Review
Thanks for putting this together—the overall scan → verify → rebuild direction makes sense, but I don’t think this is ready to merge yet.
A few blockers:
- The rebuild command sends
rebuild_all=true, butvalkey-container’s currentci.ymlonly accepts the requiredversioninput. The dispatch will fail as written. - The scan job closes the triage issue before the downstream rebuild job starts. If dispatch, build, or publishing fails, the vulnerability remains while its tracking issue is already closed.
- The custom version comparator does not implement Debian package ordering correctly. For example, it orders
1.0-1above1.0+deb12u1, contrary to Debian semantics. Since this is the safety gate, we should use nativedpkg/apkcomparison. - Existing triage issues are deliberately left untouched, so CVEs discovered on later scans never get added to the issue.
- A manual
CRITICALscan with no results can close an issue previously opened for unresolvedHIGHfindings because all thresholds share the same issue.
I’d also like the architecture coverage clarified: scanning and base verification currently inspect only the runner’s native platform, while the downstream workflow publishes four architectures.
|
Thanks for the review @sarthakaggarwal97 |
b6c383d to
cb3c938
Compare
|
All six blockers are addressed, and I simplified the design: issue tracking is removed. Findings now go to the workflow run summary instead of a GitHub issue, as we discussed offline.
Out of scope: valkey's CVEs itself and vendored libs (jemalloc, libbacktrace), covered by the release/versions.json flow. |
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/cve-scan.yml (1)
127-133: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winPass
versionsthroughenv:instead of interpolating it directly into the script.
${{ needs.scan.outputs.versions }}is expanded directly into therun:script body here, unlike the dispatch step above which correctly passes the same value viaenv: VERSIONS. Sinceversionsultimately derives from parsing an externally-fetchedversions.json, direct expression interpolation into a shell script is a script-injection footgun if that value ever contains shell metacharacters.🔒 Proposed fix
- name: Job summary if: always() + env: + VERSIONS: ${{ needs.scan.outputs.versions }} run: | echo "## Rebuild Dispatch" >> "$GITHUB_STEP_SUMMARY" echo "" >> "$GITHUB_STEP_SUMMARY" - echo "Dispatched \`ci.yml\` on \`valkey-io/valkey-container\` for versions: \`${{ needs.scan.outputs.versions }}\`" >> "$GITHUB_STEP_SUMMARY" + echo "Dispatched \`ci.yml\` on \`valkey-io/valkey-container\` for versions: \`${VERSIONS}\`" >> "$GITHUB_STEP_SUMMARY"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/cve-scan.yml around lines 127 - 133, Update the “Job summary” step to pass the `needs.scan.outputs.versions` value through an `env` entry, such as `VERSIONS`, and reference that environment variable in the shell commands instead of interpolating the GitHub expression directly in `run:`. Preserve the existing summary content and formatting.Source: Linters/SAST tools
🧹 Nitpick comments (3)
tests/test_cve_versions_and_platforms.py (1)
305-319: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueWrong type annotation on
tmp_pathfixture parameter.
tmp_pathreturnspathlib.Path, notpytest.TempdirFactory(that's the type of the legacytmpdir_factoryfixture). The body immediately doestmp_path / "github_output", which is valid forPathbut not forTempdirFactory. If mypy checks this test file, this annotation mismatch would likely fail the "clean mypy" claim.🔧 Suggested fix
- def test_versions_output_written_to_github_output( - self, tmp_path: pytest.TempdirFactory, monkeypatch - ) -> None: + def test_versions_output_written_to_github_output( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None:🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_cve_versions_and_platforms.py` around lines 305 - 319, Correct the tmp_path parameter annotation in test_versions_output_written_to_github_output to use pathlib.Path, matching the fixture’s actual return type and its path-joining usage. Remove the incorrect pytest.TempdirFactory annotation and add the necessary Path reference if it is not already imported.scripts/cve_scan/scanner.py (1)
173-197: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winConsider bounding total scan wall-clock time as the image/platform matrix grows.
At ~180s worst-case timeout per invocation,
len(images) * len(platforms)sequential scans could exceed the stated 30 min job timeout well before hitting the per-scan timeout (e.g., 10 images × 4 platforms × even 45s = 30 min). Current testing shows this isn't an issue today (cached DB scans run in tens of seconds), but a boundedThreadPoolExecutorfor platform scans within an image would remove the dependency on scans staying fast as the matrix grows.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/cve_scan/scanner.py` around lines 173 - 197, Bound the image/platform scan duration in the loop using a bounded ThreadPoolExecutor for the platform calls to scan_image, while preserving the existing per-platform logging, finding aggregation, deduplication, threshold counting, and final filter_by_threshold behavior. Limit concurrency to a controlled worker count and ensure executor tasks are collected so failures propagate consistently.scripts/cve_scan/config.py (1)
40-44: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDefault platform list hardcoded independently in two files. Both
config.py's_DEFAULT_PLATFORMSandscanner.py'sDEFAULT_PLATFORMSencode the identical "verified published platforms" set from the same manual verification, with no shared source of truth and no test asserting they stay in sync.
scripts/cve_scan/config.py#L40-L44: derive_DEFAULT_PLATFORMSfromscanner.DEFAULT_PLATFORMS(e.g.",".join(scanner.DEFAULT_PLATFORMS)) instead of a separately maintained literal.scripts/cve_scan/scanner.py#L34-L43: keepDEFAULT_PLATFORMSas the single canonical list thatconfig.pyimports from, so future platform additions/removals only need one edit.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/cve_scan/config.py` around lines 40 - 44, Use scanner.DEFAULT_PLATFORMS as the single source of truth: in scripts/cve_scan/config.py lines 40-44, derive _DEFAULT_PLATFORMS by joining scanner.DEFAULT_PLATFORMS instead of maintaining a duplicate literal; retain scanner.DEFAULT_PLATFORMS unchanged as the canonical platform list in scripts/cve_scan/scanner.py lines 34-43.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/cve_scan/sweep.py`:
- Around line 161-169: Update the summary generation in the scan flow around
fixable findings so it does not claim that a rebuild was dispatched; report only
that versions were identified or are eligible for rebuilding, while retaining
the existing dry-run distinction and findings table.
In `@scripts/cve_scan/version_compare.py`:
- Around line 128-142: Update _compare_alpine to invoke apk version -t directly
as Docker argv arguments, removing the sh -c wrapper, shell interpolation, and
2>/dev/null redirection; preserve the existing _run_docker result handling and
warning behavior.
- Around line 94-125: Update the Debian comparison flow in compare_versions to
treat any unexpected nonzero _run_docker exit code as a failure, returning None
rather than interpreting it as a valid comparison result. Validate rc_lt and
rc_eq explicitly against the expected dpkg outcomes (0 or 1), preserve the
existing run-failure handling and warnings as appropriate, and add coverage for
docker/dpkg exit codes such as 125, 126, or 127.
In `@tests/test_base_precheck.py`:
- Line 9: Update the test-description comment in tests/test_base_precheck.py to
state that images missing from base_map are conservatively downgraded
fail-closed, with fixable set to false, matching TestMissingBaseMap and its
asserted rationale.
---
Outside diff comments:
In @.github/workflows/cve-scan.yml:
- Around line 127-133: Update the “Job summary” step to pass the
`needs.scan.outputs.versions` value through an `env` entry, such as `VERSIONS`,
and reference that environment variable in the shell commands instead of
interpolating the GitHub expression directly in `run:`. Preserve the existing
summary content and formatting.
---
Nitpick comments:
In `@scripts/cve_scan/config.py`:
- Around line 40-44: Use scanner.DEFAULT_PLATFORMS as the single source of
truth: in scripts/cve_scan/config.py lines 40-44, derive _DEFAULT_PLATFORMS by
joining scanner.DEFAULT_PLATFORMS instead of maintaining a duplicate literal;
retain scanner.DEFAULT_PLATFORMS unchanged as the canonical platform list in
scripts/cve_scan/scanner.py lines 34-43.
In `@scripts/cve_scan/scanner.py`:
- Around line 173-197: Bound the image/platform scan duration in the loop using
a bounded ThreadPoolExecutor for the platform calls to scan_image, while
preserving the existing per-platform logging, finding aggregation,
deduplication, threshold counting, and final filter_by_threshold behavior. Limit
concurrency to a controlled worker count and ensure executor tasks are collected
so failures propagate consistently.
In `@tests/test_cve_versions_and_platforms.py`:
- Around line 305-319: Correct the tmp_path parameter annotation in
test_versions_output_written_to_github_output to use pathlib.Path, matching the
fixture’s actual return type and its path-joining usage. Remove the incorrect
pytest.TempdirFactory annotation and add the necessary Path reference if it is
not already imported.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 205789a6-6702-4ad0-a2fe-d7aab335bb5e
📒 Files selected for processing (24)
.github/workflows/cve-scan.ymlREADME.mddocs/architecture.mdscripts/cve_scan/__init__.pyscripts/cve_scan/base_precheck.pyscripts/cve_scan/config.pyscripts/cve_scan/image_matrix.pyscripts/cve_scan/models.pyscripts/cve_scan/rebuild_decider.pyscripts/cve_scan/scanner.pyscripts/cve_scan/summary.pyscripts/cve_scan/sweep.pyscripts/cve_scan/version_compare.pyscripts/parsers/__init__.pyscripts/parsers/cve_findings_parser.pytests/test_base_precheck.pytests/test_cve_config.pytests/test_cve_parser.pytests/test_cve_rebuild_decider.pytests/test_cve_summary.pytests/test_cve_sweep_integration.pytests/test_cve_versions_and_platforms.pytests/test_image_matrix.pytests/test_version_compare.py
🚧 Files skipped from review as they are similar to previous changes (5)
- scripts/parsers/init.py
- scripts/cve_scan/models.py
- scripts/parsers/cve_findings_parser.py
- tests/test_cve_parser.py
- tests/test_cve_rebuild_decider.py
| if fixable: | ||
| versions = dispatched_versions or [] | ||
| if dry_run: | ||
| lines.append(f"### Confirmed fixable (rebuild would be dispatched for versions: {' '.join(versions) or '(none)'})") | ||
| else: | ||
| lines.append(f"### Rebuild dispatched for versions: {' '.join(versions) or '(none)'}") | ||
| lines.append("") | ||
| lines.append(render_findings_table(fixable)) | ||
| lines.append("") |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
"Rebuild dispatched" is asserted before the dispatch actually runs.
This summary is written by the scan job, but the actual gh workflow run ci.yml call happens in the separate downstream rebuild job (.github/workflows/cve-scan.yml, gated on needs.scan.outputs.fixable). If that job fails or is skipped for any reason, this job's summary will have already claimed the rebuild was dispatched.
✏️ Proposed wording fix
if dry_run:
lines.append(f"### Confirmed fixable (rebuild would be dispatched for versions: {' '.join(versions) or '(none)'})")
else:
- lines.append(f"### Rebuild dispatched for versions: {' '.join(versions) or '(none)'}")
+ lines.append(f"### Confirmed fixable (rebuild will be dispatched for versions: {' '.join(versions) or '(none)'})")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if fixable: | |
| versions = dispatched_versions or [] | |
| if dry_run: | |
| lines.append(f"### Confirmed fixable (rebuild would be dispatched for versions: {' '.join(versions) or '(none)'})") | |
| else: | |
| lines.append(f"### Rebuild dispatched for versions: {' '.join(versions) or '(none)'}") | |
| lines.append("") | |
| lines.append(render_findings_table(fixable)) | |
| lines.append("") | |
| if fixable: | |
| versions = dispatched_versions or [] | |
| if dry_run: | |
| lines.append(f"### Confirmed fixable (rebuild would be dispatched for versions: {' '.join(versions) or '(none)'})") | |
| else: | |
| lines.append(f"### Confirmed fixable (rebuild will be dispatched for versions: {' '.join(versions) or '(none)'})") | |
| lines.append("") | |
| lines.append(render_findings_table(fixable)) | |
| lines.append("") |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/cve_scan/sweep.py` around lines 161 - 169, Update the summary
generation in the scan flow around fixable findings so it does not claim that a
rebuild was dispatched; report only that versions were identified or are
eligible for rebuilding, while retaining the existing dry-run distinction and
findings table.
…atch Weekly Trivy scan of published valkey-container images (all published architectures) that rebuilds affected version lines automatically when a CVE fix is verifiably available: - resolves the image matrix dynamically from valkey-container's versions.json (RC windows included) - classifies each CVE as rebuild-fixable or not; the safety gate verifies the patched package is present in the base image package database using native dpkg/apk comparison, fail-closed - dispatches ci.yml for the affected version lines only, via a scoped Valkeyrie Bot App token - reports all findings in the Actions run summary; no GitHub issues are created Valkey core and statically-linked vendored libraries are out of scope (covered by the valkey security release and versions.json flow). Signed-off-by: Roshan Khatri <rvkhatri@amazon.com>
cb3c938 to
c4e0185
Compare
|
@coderabbitai review this PR again |
|
✅ Action performedFull review finished. Your included review limit is currently reached under our Fair Usage Limits Policy. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 26 minutes. |
ae79568 to
c699ef1
Compare
…against each platform's base image Signed-off-by: Roshan Khatri <rvkhatri@amazon.com>
c699ef1 to
b8d48d2
Compare
Signed-off-by: Roshan Khatri <rvkhatri@amazon.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
scripts/cve_scan/version_compare.py (1)
22-23: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winPin the dpkg/apk comparator images instead of floating tags.
debian:stable-slimandalpine:latestare mutable tags — the registry can silently point them at a different image over time. This module is the "actual gate before dispatch" for automatic rebuilds and is documented as deterministic; a floating comparator image undermines that guarantee and audit trail.🔒️ Suggested fix: pin to a digest or fixed release tag
-_DEBIAN_COMPARATOR_IMAGE = "public.ecr.aws/docker/library/debian:stable-slim" -_ALPINE_COMPARATOR_IMAGE = "public.ecr.aws/docker/library/alpine:latest" +_DEBIAN_COMPARATOR_IMAGE = "public.ecr.aws/docker/library/debian:stable-slim@sha256:<digest>" +_ALPINE_COMPARATOR_IMAGE = "public.ecr.aws/docker/library/alpine:3.21@sha256:<digest>"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/cve_scan/version_compare.py` around lines 22 - 23, Replace the mutable tags in _DEBIAN_COMPARATOR_IMAGE and _ALPINE_COMPARATOR_IMAGE with immutable image references, preferably pinned digests or fixed release tags, so comparator behavior remains deterministic and auditable.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@README.md`:
- Line 527: Update the README “Scan (multi-arch)” description to accurately
state that findings remain distinct per platform because deduplication includes
the platform, rather than claiming identical cross-architecture CVEs are counted
once. Preserve the listed platforms and explain that the report reflects
per-platform findings.
---
Nitpick comments:
In `@scripts/cve_scan/version_compare.py`:
- Around line 22-23: Replace the mutable tags in _DEBIAN_COMPARATOR_IMAGE and
_ALPINE_COMPARATOR_IMAGE with immutable image references, preferably pinned
digests or fixed release tags, so comparator behavior remains deterministic and
auditable.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e731580b-eea0-4125-a922-735109e21518
📒 Files selected for processing (24)
.github/workflows/cve-scan.ymlREADME.mddocs/architecture.mdscripts/cve_scan/__init__.pyscripts/cve_scan/base_precheck.pyscripts/cve_scan/config.pyscripts/cve_scan/image_matrix.pyscripts/cve_scan/models.pyscripts/cve_scan/rebuild_decider.pyscripts/cve_scan/scanner.pyscripts/cve_scan/summary.pyscripts/cve_scan/sweep.pyscripts/cve_scan/version_compare.pyscripts/parsers/__init__.pyscripts/parsers/cve_findings_parser.pytests/test_base_precheck.pytests/test_cve_config.pytests/test_cve_parser.pytests/test_cve_rebuild_decider.pytests/test_cve_summary.pytests/test_cve_sweep_integration.pytests/test_cve_versions_and_platforms.pytests/test_image_matrix.pytests/test_version_compare.py
🚧 Files skipped from review as they are similar to previous changes (10)
- scripts/parsers/init.py
- scripts/cve_scan/config.py
- scripts/parsers/cve_findings_parser.py
- tests/test_cve_parser.py
- tests/test_cve_summary.py
- tests/test_cve_config.py
- scripts/cve_scan/summary.py
- tests/test_version_compare.py
- tests/test_base_precheck.py
- tests/test_cve_rebuild_decider.py
…n comparator images Signed-off-by: Roshan Khatri <rvkhatri@amazon.com>
… the sole version gate Signed-off-by: Roshan Khatri <rvkhatri@amazon.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/test_base_precheck.py (1)
27-53: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winMake the comparator stub validate routing parameters.
_stub_compareignores bothflavorandbase_image, so these precheck tests would still pass ifverify_fixable_in_baserouted an Alpine package through the Debian path or passed the wrong comparator image. Record the calls and assert the expected flavor/image for at least one Debian and one Alpine verification case.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_base_precheck.py` around lines 27 - 53, Update _stub_compare and the precheck tests to record each comparator invocation and validate both routing parameters. Add assertions covering at least one Debian and one Alpine verify_fixable_in_base case, confirming the expected flavor and comparator base image are passed while preserving the existing version-comparison behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@tests/test_base_precheck.py`:
- Around line 27-53: Update _stub_compare and the precheck tests to record each
comparator invocation and validate both routing parameters. Add assertions
covering at least one Debian and one Alpine verify_fixable_in_base case,
confirming the expected flavor and comparator base image are passed while
preserving the existing version-comparison behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: f0d25766-e14a-49cc-b77b-7015d460aa4a
📒 Files selected for processing (7)
README.mddocs/architecture.mdscripts/cve_scan/rebuild_decider.pytests/test_base_precheck.pytests/test_cve_rebuild_decider.pytests/test_cve_sweep_integration.pytests/test_version_compare.py
🚧 Files skipped from review as they are similar to previous changes (3)
- docs/architecture.md
- tests/test_cve_sweep_integration.py
- README.md
2 participants
fixes: valkey-io/valkey-container#146
Summary
Weekly scan of published valkey-container images that auto-rebuilds them when a CVE fix is verifiably available, closing the gap where stable images carry vulnerable base packages until someone notices (see #117/#120, #136, #146).
How it works
Safety
Rebuilds only on verified evidence (fix published + present in base + missing from image). Deterministic, no AI, fail-loud config, least-privilege tokens (only the rebuild job holds actions:write).
Testing
1,481 tests (unit + unmocked integration), ruff/mypy clean, real Trivy scans locally, and a green GitHub Actions dry-run.