jdaugherty commented on PR #16530: URL: https://github.com/apache/grails-core/pull/16530#issuecomment-6018918926
Thanks for the thorough review. Everything is addressed in b9b13ef2fc, and the description is rewritten; CI for that head is still running. **1. Failed scans without a report.** The task now deletes the previous run's report first, resolves the graphs inside the `try`, and catches `Exception`. Cache writes are best-effort: an unwritable cache directory is warned about once and the answers are still used (there's a spec that puts a file where the cache directory should be). While writing the resolution-failure spec I found that `rootComponent.get()` is lenient: a missing module, and even a `failOnVersionConflict()` conflict, never throw, they show up as `UnresolvedDependencyResult`s. So the "fails outside the try" path doesn't actually exist, but the related nit does, and a warning alone would have let the scan pass with the unresolved (and in the spec, the only vulnerable) dependency silently dropped. An unresolved dependency now fails the scan with an error report that names it, its configurations and the first line of the reason, so the summary shows ⚠️ instead of ✅. **2. `reportMaxBytes`.** The limit now bounds the whole summary. The sections are assembled in priority order (header, findings, excluded, unused exclusions); the first one that doesn't fit is cut at a line boundary and the ones after it are left out, so a long exclusions list can't push the findings out or the comment past the limit. The workflow comment is corrected, and there are specs for both the "exclusions are what gets cut" and the "findings are cut" cases, plus one for multi-byte characters at the cut. **3. Specs in CI.** `:build-logic-root:vulnerability-scan:check` is added to the `build` and `buildRerunTasks` jobs of `gradle.yml`, so they also run on the Windows and macOS matrix entries now. **4. Sonatype Guide matching.** Reports are matched on a normalized package URL (percent-decoded, lower-cased, with a literal `+` kept as one); the index fallback is gone and an unmatched component fails the lookup. Specs cover an echo in another case/encoding and order, and both ways of not getting a report (fewer reports, and as many but on something else). I can't exercise this against the real API locally, so the labeled scan job on this PR is what verifies it. **5. Scope.** I kept the dependency commit and rewrote the description: the bumps have their own section with a from/to/why table, and the counts are now 28 exclusions and 78 specs. **Nits.** Exclusion ids compare case-insensitively (`VulnerabilityExclusionSpec`); the extension's configurations are mapped into the task through a provider, so a build script that names them after the task is realized is honoured (spec added); OSV records that only carry a CVSS v3 vector are rated by its base score (`CvssV3`, specs against the published example scores), and a record with only a v4 vector shows `CVSS_V4`; the cache growth and the one-at-a-time scans are documented in the plugin Javadoc and the build-logic README. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
