Skip to content

Commit 5161639

Browse files
committed
refactor: comprehensive codebase improvements for v2.8.0
- Extract CheckOptions struct with clap::Args to eliminate parameter explosion - Split cmd_check (310 lines) into 5 focused functions - Split scanner.rs (1148 lines) into scanner/{mod,report,network}.rs - Create shared lockfile/platform.rs to unify duplicated platform parsing - Encapsulate parser state into SourceState struct with methods - Migrate deprecated serde_yaml to serde_yml - Fix Database::open().unwrap() panic with proper error propagation - Extract check_staleness/build_ignore_comments to src/check.rs for testability - Replace .ok() with io::Result<()> in format output (text.rs, json.rs) - Handle broken pipe gracefully in CLI output - Add tempfile crate for reliable test cleanup - Add Clone derive to FixSuggestion - Add 13 new unit tests (307 → 320)
1 parent 11a9554 commit 5161639

19 files changed

Lines changed: 1454 additions & 1353 deletions

Cargo.lock

Lines changed: 25 additions & 12 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[package]
22
name = "gem-audit"
3-
version = "2.7.0"
3+
version = "2.8.0"
44
edition = "2024"
55
description = "Ultra-fast, standalone security auditor for Gemfile.lock"
66
license = "MIT"
@@ -14,9 +14,10 @@ clap = { version = "4", features = ["derive"] }
1414
gix = { version = "0.80", default-features = false, features = ["blocking-network-client", "blocking-http-transport-reqwest-rust-tls", "worktree-mutation"] }
1515
serde = { version = "1", features = ["derive"] }
1616
serde_json = "1"
17-
serde_yaml = "0.9"
17+
serde_yml = "0.0.12"
1818
thiserror = "2"
1919

2020
[dev-dependencies]
2121
assert_cmd = "2"
2222
predicates = "3"
23+
tempfile = "3"

codecov.yml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,8 @@
1+
comment:
2+
layout: "reach,diff,flags,files"
3+
behavior: default
4+
require_changes: false
5+
16
coverage:
27
status:
38
project:

src/advisory/database.rs

Lines changed: 18 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -464,9 +464,9 @@ mod tests {
464464

465465
#[test]
466466
fn open_fixture_advisory_dir() {
467-
let (db_dir, _) = temp_mock_db("fixture");
467+
let (tmp, _) = temp_mock_db("fixture");
468468

469-
let db = Database::open(&db_dir).unwrap();
469+
let db = Database::open(tmp.path()).unwrap();
470470
assert!(!db.is_git());
471471

472472
let advisories = db.advisories_for("test");
@@ -480,8 +480,6 @@ mod tests {
480480
// Check patched version
481481
let (vulns, _errors) = db.check_gem("test", &Version::parse("1.0.0").unwrap());
482482
assert!(vulns.is_empty());
483-
484-
std::fs::remove_dir_all(&db_dir).unwrap();
485483
}
486484

487485
// ========== Error Cases ==========
@@ -505,60 +503,55 @@ mod tests {
505503

506504
// Helper: create an isolated temporary mock DB for tests that don't
507505
// share state with `mock_database()` in scanner tests.
508-
fn temp_mock_db(suffix: &str) -> (PathBuf, PathBuf) {
506+
fn temp_mock_db(_suffix: &str) -> (tempfile::TempDir, PathBuf) {
509507
let fixture_dir = PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("tests/fixtures");
510-
let db_dir = std::env::temp_dir().join(format!("gem_audit_db_test_{}", suffix));
511-
let _ = std::fs::remove_dir_all(&db_dir);
512-
let gem_dir = db_dir.join("gems").join("test");
508+
let tmp = tempfile::tempdir().unwrap();
509+
let gem_dir = tmp.path().join("gems").join("test");
513510
std::fs::create_dir_all(&gem_dir).unwrap();
514511
std::fs::copy(
515512
fixture_dir.join("advisory/CVE-2020-1234.yml"),
516513
gem_dir.join("CVE-2020-1234.yml"),
517514
)
518515
.unwrap();
519-
(db_dir, fixture_dir)
516+
(tmp, fixture_dir)
520517
}
521518

522519
// ========== Database Display ==========
523520

524521
#[test]
525522
fn database_display() {
526-
let (db_dir, _) = temp_mock_db("display");
527-
let db = Database::open(&db_dir).unwrap();
523+
let (tmp, _) = temp_mock_db("display");
524+
let db = Database::open(tmp.path()).unwrap();
528525
let display = db.to_string();
529-
assert!(display.contains("gem_audit_db_test_display"));
530-
std::fs::remove_dir_all(&db_dir).unwrap();
526+
assert_eq!(display, tmp.path().to_string_lossy());
531527
}
532528

533529
// ========== Database exists/path ==========
534530

535531
#[test]
536532
fn database_exists_with_gems() {
537-
let (db_dir, _) = temp_mock_db("exists");
538-
let db = Database::open(&db_dir).unwrap();
533+
let (tmp, _) = temp_mock_db("exists");
534+
let db = Database::open(tmp.path()).unwrap();
539535
assert!(db.exists());
540-
assert!(db.path() == db_dir.as_path());
541-
std::fs::remove_dir_all(&db_dir).unwrap();
536+
assert!(db.path() == tmp.path());
542537
}
543538

544539
// ========== Database advisories/size with mock ==========
545540

546541
#[test]
547542
fn database_advisories_with_mock() {
548-
let (db_dir, _) = temp_mock_db("advisories");
549-
let db = Database::open(&db_dir).unwrap();
543+
let (tmp, _) = temp_mock_db("advisories");
544+
let db = Database::open(tmp.path()).unwrap();
550545
let all = db.advisories();
551546
assert_eq!(all.len(), 1);
552547
assert_eq!(all[0].id, "CVE-2020-1234");
553-
std::fs::remove_dir_all(&db_dir).unwrap();
554548
}
555549

556550
#[test]
557551
fn database_size_with_mock() {
558-
let (db_dir, _) = temp_mock_db("size");
559-
let db = Database::open(&db_dir).unwrap();
552+
let (tmp, _) = temp_mock_db("size");
553+
let db = Database::open(tmp.path()).unwrap();
560554
assert_eq!(db.size(), 1);
561-
std::fs::remove_dir_all(&db_dir).unwrap();
562555
}
563556

564557
// ========== Ruby advisory methods ==========
@@ -613,11 +606,10 @@ mod tests {
613606

614607
#[test]
615608
fn commit_id_none_for_non_git() {
616-
let (db_dir, _) = temp_mock_db("nongit");
617-
let db = Database::open(&db_dir).unwrap();
609+
let (tmp, _) = temp_mock_db("nongit");
610+
let db = Database::open(tmp.path()).unwrap();
618611
assert_eq!(db.commit_id(), None);
619612
assert_eq!(db.last_updated_at(), None);
620-
std::fs::remove_dir_all(&db_dir).unwrap();
621613
}
622614

623615
// ========== DatabaseError Display ==========

src/advisory/model.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ pub enum AdvisoryError {
111111
#[error("IO error: {0}")]
112112
Io(#[from] std::io::Error),
113113
#[error("YAML parse error: {0}")]
114-
Yaml(#[from] serde_yaml::Error),
114+
Yaml(#[from] serde_yml::Error),
115115
#[error("invalid requirement '{version_str}': {error}")]
116116
InvalidRequirement { version_str: String, error: String },
117117
#[error("advisory {path} is missing both 'gem' and 'engine' fields")]
@@ -133,7 +133,7 @@ impl Advisory {
133133
.unwrap_or("unknown")
134134
.to_string();
135135

136-
let raw: AdvisoryYaml = serde_yaml::from_str(yaml)?;
136+
let raw: AdvisoryYaml = serde_yml::from_str(yaml)?;
137137

138138
let (name, kind) = match (raw.gem, raw.engine) {
139139
(Some(gem), _) => (gem, AdvisoryKind::Gem),
@@ -511,7 +511,7 @@ mod tests {
511511

512512
#[test]
513513
fn advisory_error_yaml_display() {
514-
let yaml_err = serde_yaml::from_str::<AdvisoryYaml>("not valid yaml {{{{").unwrap_err();
514+
let yaml_err = serde_yml::from_str::<AdvisoryYaml>("not valid yaml {{{{").unwrap_err();
515515
let err = AdvisoryError::Yaml(yaml_err);
516516
assert!(err.to_string().contains("YAML parse error"));
517517
}

0 commit comments

Comments
 (0)