mirror of
https://github.com/imjasonh/rustvulncheck
synced 2026-07-08 04:08:07 +00:00
Improve vuln enrichment: retry incomplete entries, fix PR parent SHA, restore symbols
Three improvements to the enrichment pipeline: 1. Entries with commit_sha but empty vulnerable_symbols are now treated as incomplete and retried automatically (previously they were skipped forever once added to the DB). 2. PR diff fetching now resolves the merge commit's first parent for the "before" file contents, instead of using base.sha which reflects the current branch tip and drifts for old PRs. 3. Restored 141 entries' symbols that were lost across CI merge conflicts (141 → 6 → 0 regression). Also improved is_test_file to filter out proptests.rs, *_tests.rs, and -tests/ path components. https://claude.ai/code/session_017qkaszgfEhmS9ifowCV8YX
This commit is contained in:
parent
a4b117f373
commit
32b7302b00
4 changed files with 8624 additions and 145 deletions
|
|
@ -153,12 +153,20 @@ pub(crate) fn is_test_file(path: &str) -> bool {
|
|||
}) {
|
||||
return true;
|
||||
}
|
||||
// Files named *_test.rs or test_*.rs
|
||||
// Files named *_test.rs, test_*.rs, or *_tests.rs, or proptests.rs
|
||||
if let Some(filename) = parts.last() {
|
||||
if filename.ends_with("_test.rs") || filename.starts_with("test_") {
|
||||
if filename.ends_with("_test.rs")
|
||||
|| filename.ends_with("_tests.rs")
|
||||
|| filename.starts_with("test_")
|
||||
|| *filename == "proptests.rs"
|
||||
{
|
||||
return true;
|
||||
}
|
||||
}
|
||||
// Path components ending in -tests (e.g. lucet-runtime-tests/src/...)
|
||||
if parts.iter().any(|p| p.ends_with("-tests") || p.ends_with("_tests")) {
|
||||
return true;
|
||||
}
|
||||
// Top-level perf/ directories (e.g. quinn's perf/src/server.rs)
|
||||
if parts.first() == Some(&"perf") {
|
||||
return true;
|
||||
|
|
@ -196,7 +204,12 @@ mod tests {
|
|||
assert!(is_test_file("fuzz/fuzz_targets/params.rs"));
|
||||
assert!(is_test_file("perf/src/server.rs"));
|
||||
assert!(is_test_file("benches/bench.rs"));
|
||||
assert!(is_test_file("src/btreemap/proptests.rs"));
|
||||
assert!(is_test_file("lucet-runtime/lucet-runtime-tests/src/guest_fault.rs"));
|
||||
assert!(is_test_file("crate/foo_tests/src/bar.rs"));
|
||||
assert!(is_test_file("src/integration_tests.rs"));
|
||||
assert!(!is_test_file("src/lib.rs"));
|
||||
assert!(!is_test_file("src/http/request.rs"));
|
||||
assert!(!is_test_file("src/testing/trace.rs"));
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -119,7 +119,14 @@ impl GithubClient {
|
|||
.merge_commit_sha
|
||||
.unwrap_or_else(|| format!("pr-{}", number));
|
||||
|
||||
let parent_sha = pr_resp.base.map(|b| b.sha);
|
||||
// For the parent SHA, prefer the merge commit's first parent (the base
|
||||
// branch at merge time) over `base.sha` (which reflects the *current*
|
||||
// branch tip and may have drifted for old PRs).
|
||||
let parent_sha = self
|
||||
.fetch_commit_parent(owner, repo, &commit_sha)
|
||||
.ok()
|
||||
.flatten()
|
||||
.or_else(|| pr_resp.base.map(|b| b.sha));
|
||||
|
||||
// Fetch all files changed in the PR (paginated, up to 300 files)
|
||||
let files_url = format!(
|
||||
|
|
@ -155,6 +162,36 @@ impl GithubClient {
|
|||
})
|
||||
}
|
||||
|
||||
/// Fetch just the first parent SHA of a commit (lightweight, no file data).
|
||||
fn fetch_commit_parent(&self, owner: &str, repo: &str, sha: &str) -> Result<Option<String>> {
|
||||
let url = format!(
|
||||
"https://api.github.com/repos/{}/{}/commits/{}",
|
||||
owner, repo, sha
|
||||
);
|
||||
let resp = self
|
||||
.get(&url)
|
||||
.send()
|
||||
.context("fetching commit for parent")?
|
||||
.error_for_status()
|
||||
.context("commit API error")?;
|
||||
|
||||
#[derive(Deserialize)]
|
||||
struct ParentOnly {
|
||||
parents: Option<Vec<CommitParentRef>>,
|
||||
}
|
||||
#[derive(Deserialize)]
|
||||
struct CommitParentRef {
|
||||
sha: String,
|
||||
}
|
||||
|
||||
let commit: ParentOnly = resp.json().context("parsing commit for parent")?;
|
||||
Ok(commit
|
||||
.parents
|
||||
.as_ref()
|
||||
.and_then(|p| p.first())
|
||||
.map(|p| p.sha.clone()))
|
||||
}
|
||||
|
||||
fn fetch_commit_diff(&self, owner: &str, repo: &str, sha: &str) -> Result<PatchDiff> {
|
||||
let url = format!(
|
||||
"https://api.github.com/repos/{}/{}/commits/{}",
|
||||
|
|
|
|||
30
src/main.rs
30
src/main.rs
|
|
@ -281,11 +281,27 @@ fn run_enrich(args: EnrichArgs) -> Result<()> {
|
|||
}
|
||||
|
||||
// Step 5: Process new (unenriched) advisories
|
||||
// An entry is considered "enriched" only if it has symbols OR has no commit
|
||||
// to fetch from. Entries with a commit_sha but empty symbols are treated as
|
||||
// incomplete and will be retried.
|
||||
let already_enriched: HashSet<String> = vuln_db
|
||||
.entries
|
||||
.iter()
|
||||
.filter(|e| !e.vulnerable_symbols.is_empty() || e.commit_sha.is_none())
|
||||
.map(|e| e.advisory_id.clone())
|
||||
.collect();
|
||||
let incomplete_ids: HashSet<String> = vuln_db
|
||||
.entries
|
||||
.iter()
|
||||
.filter(|e| e.commit_sha.is_some() && e.vulnerable_symbols.is_empty())
|
||||
.map(|e| e.advisory_id.clone())
|
||||
.collect();
|
||||
if !incomplete_ids.is_empty() {
|
||||
println!(
|
||||
"{} entries have commit SHAs but no symbols (will retry)",
|
||||
incomplete_ids.len()
|
||||
);
|
||||
}
|
||||
|
||||
// Filter to advisories with GitHub refs unless --include-all
|
||||
let candidates: Vec<&Advisory> = if args.include_all {
|
||||
|
|
@ -390,7 +406,19 @@ fn run_enrich(args: EnrichArgs) -> Result<()> {
|
|||
}
|
||||
}
|
||||
|
||||
vuln_db.entries.push(entry);
|
||||
// If this is an incomplete entry being retried, update it in-place
|
||||
if incomplete_ids.contains(&adv.id) {
|
||||
if let Some(existing) = vuln_db
|
||||
.entries
|
||||
.iter_mut()
|
||||
.find(|e| e.advisory_id == adv.id)
|
||||
{
|
||||
existing.commit_sha = entry.commit_sha;
|
||||
existing.vulnerable_symbols = entry.vulnerable_symbols;
|
||||
}
|
||||
} else {
|
||||
vuln_db.entries.push(entry);
|
||||
}
|
||||
new_count += 1;
|
||||
println!();
|
||||
}
|
||||
|
|
|
|||
8683
vuln_db.json
8683
vuln_db.json
File diff suppressed because it is too large
Load diff
Loading…
Add table
Add a link
Reference in a new issue