Skip to content

Commit 6e73a28

Browse files
Address CodeRabbit review feedback
- main.rs: Use clap ValueEnum for --format to reject invalid values at parse time instead of silently defaulting to cyclonedx - scanner.rs: Dedup keeps highest-confidence finding per location using HashMap instead of first-seen HashSet - scanner.rs: pattern_applies now checks extensionless manifest filenames (Gemfile, Podfile, etc.) not just file extensions - Analyze.hs: Distinguish scanner failure from empty results in CBOM output and FIPS report (log warning on failure instead of misleading "no findings" message) - ScanSummary.hs: Pass warnings through on successful crypto scan instead of always showing plain success - FipsReport.hs: Dedupe by (name, parameter_set) instead of name only, so RSA-2048 and RSA-1024 are counted separately Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent ed3c466 commit 6e73a28

5 files changed

Lines changed: 71 additions & 29 deletions

File tree

extlib/cryptoscan/src/main.rs

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,15 @@ mod fips;
44
mod patterns;
55
mod scanner;
66

7-
use clap::Parser;
7+
use clap::{Parser, ValueEnum};
88
use std::path::PathBuf;
99

10+
#[derive(Copy, Clone, Debug, ValueEnum)]
11+
enum OutputFormat {
12+
Json,
13+
Cyclonedx,
14+
}
15+
1016
#[derive(Parser, Debug)]
1117
#[command(name = "cryptoscan", about = "Detect cryptographic algorithm usage in source code")]
1218
struct Cli {
@@ -19,8 +25,8 @@ struct Cli {
1925
ecosystem: String,
2026

2127
/// Output format (json, cyclonedx)
22-
#[arg(short, long, default_value = "cyclonedx")]
23-
format: String,
28+
#[arg(short, long, value_enum, default_value_t = OutputFormat::Cyclonedx)]
29+
format: OutputFormat,
2430

2531
/// Only show non-FIPS-compliant findings
2632
#[arg(long, default_value_t = false)]
@@ -47,9 +53,9 @@ fn main() {
4753
findings
4854
};
4955

50-
let output = match cli.format.as_str() {
51-
"json" => serde_json::to_string_pretty(&findings).expect("Failed to serialize findings"),
52-
"cyclonedx" | _ => {
56+
let output = match cli.format {
57+
OutputFormat::Json => serde_json::to_string_pretty(&findings).expect("Failed to serialize findings"),
58+
OutputFormat::Cyclonedx => {
5359
let bom = cyclonedx::to_cyclonedx_bom(&findings);
5460
serde_json::to_string_pretty(&bom).expect("Failed to serialize CBOM")
5561
}

extlib/cryptoscan/src/scanner.rs

Lines changed: 38 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
1-
use std::collections::HashSet;
1+
use std::collections::{HashMap, HashSet};
22
use std::fs;
33
use std::path::Path;
44

55
use walkdir::WalkDir;
66

7-
use crate::crypto_algorithm::{CryptoAlgorithm, CryptoFinding, Primitive};
7+
use crate::crypto_algorithm::{Confidence, CryptoAlgorithm, CryptoFinding, Primitive};
88
use crate::fips;
99
use crate::patterns::{self, CryptoPattern};
1010

@@ -106,6 +106,11 @@ pub fn scan_project(project_path: &Path, ecosystems: &[String]) -> Vec<CryptoFin
106106
.and_then(|e| e.to_str())
107107
.unwrap_or("");
108108

109+
let file_name = path
110+
.file_name()
111+
.and_then(|n| n.to_str())
112+
.unwrap_or("");
113+
109114
// Read file content
110115
let content = match fs::read_to_string(path) {
111116
Ok(c) => c,
@@ -121,7 +126,7 @@ pub fn scan_project(project_path: &Path, ecosystems: &[String]) -> Vec<CryptoFin
121126
// Match against all applicable patterns
122127
for pattern in &all_patterns {
123128
// Check if this pattern applies to the current ecosystem and file extension
124-
if !pattern_applies(pattern, ecosystems, extension) {
129+
if !pattern_applies(pattern, ecosystems, extension, file_name) {
125130
continue;
126131
}
127132

@@ -181,7 +186,7 @@ fn should_skip_path(path: &Path) -> bool {
181186
skip_dirs.iter().any(|d| path_str.contains(d))
182187
}
183188

184-
fn pattern_applies(pattern: &CryptoPattern, ecosystems: &[String], file_ext: &str) -> bool {
189+
fn pattern_applies(pattern: &CryptoPattern, ecosystems: &[String], file_ext: &str, file_name: &str) -> bool {
185190
// Check ecosystem match
186191
let ecosystem_match = pattern.ecosystem == "any"
187192
|| ecosystems.iter().any(|e| {
@@ -194,7 +199,18 @@ fn pattern_applies(pattern: &CryptoPattern, ecosystems: &[String], file_ext: &st
194199
}
195200

196201
// Check file extension match
197-
pattern.file_extensions.iter().any(|ext| *ext == file_ext)
202+
if pattern.file_extensions.iter().any(|ext| *ext == file_ext) {
203+
return true;
204+
}
205+
206+
// For extensionless files (Gemfile, Podfile, Pipfile, etc.), check manifest names
207+
if file_ext.is_empty() {
208+
return patterns::ecosystem_manifests(pattern.ecosystem)
209+
.iter()
210+
.any(|m| !m.contains('*') && *m == file_name);
211+
}
212+
213+
false
198214
}
199215

200216
fn resolve_algorithm(name: &str, _matched_text: &str) -> CryptoAlgorithm {
@@ -277,20 +293,30 @@ fn resolve_algorithm(name: &str, _matched_text: &str) -> CryptoAlgorithm {
277293
}
278294
}
279295

296+
fn confidence_rank(c: &Confidence) -> u8 {
297+
match c {
298+
Confidence::High => 3,
299+
Confidence::Medium => 2,
300+
Confidence::Low => 1,
301+
}
302+
}
303+
280304
fn deduplicate_findings(findings: Vec<CryptoFinding>) -> Vec<CryptoFinding> {
281-
let mut seen: HashSet<String> = HashSet::new();
282-
let mut result = Vec::new();
305+
let mut best: HashMap<String, CryptoFinding> = HashMap::new();
283306

284307
for finding in findings {
285308
let key = format!(
286309
"{}:{}:{}",
287310
finding.algorithm.name, finding.file_path, finding.line_number
288311
);
289-
if !seen.contains(&key) {
290-
seen.insert(key);
291-
result.push(finding);
292-
}
312+
best.entry(key)
313+
.and_modify(|existing| {
314+
if confidence_rank(&finding.confidence) > confidence_rank(&existing.confidence) {
315+
*existing = finding.clone();
316+
}
317+
})
318+
.or_insert(finding);
293319
}
294320

295-
result
321+
best.into_values().collect()
296322
}

src/App/Fossa/Analyze.hs

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,7 @@ import Data.String.Conversion (decodeUtf8, toText)
132132
import Data.Text.Extra (showT)
133133
import Data.Traversable (for)
134134
import Diag.Diagnostic as DI
135-
import Diag.Result (resultToMaybe)
135+
import Diag.Result (Result (Failure, Success), resultToMaybe)
136136
import Discovery.Archive qualified as Archive
137137
import Discovery.Filters (AllFilters, MavenScopeFilters, applyFilters, filterIsVSIOnly, ignoredPaths, isDefaultNonProductionPath)
138138
import Discovery.Projects (withDiscoveredProjects)
@@ -143,6 +143,7 @@ import Effect.Logger (
143143
logDebug,
144144
logInfo,
145145
logStdout,
146+
logWarn,
146147
renderIt,
147148
)
148149
import Effect.ReadFS (ReadFS)
@@ -420,11 +421,12 @@ analyze cfg = Diag.context "fossa-analyze" $ do
420421
maybeCbomBytes <-
421422
Diag.errorBoundaryIO . diagToDebug $
422423
Diag.context "crypto-cbom-output" . runStickyLogger SevInfo $ analyzeCryptoScanCBOM basedir
423-
case join (resultToMaybe maybeCbomBytes) of
424-
Just bytes -> do
424+
case maybeCbomBytes of
425+
Success _ (Just bytes) -> do
425426
sendIO $ BL.writeFile cbomPath bytes
426427
logInfo $ "CycloneDX 1.7 CBOM written to: " <> pretty cbomPath
427-
Nothing -> logInfo "No crypto findings to write to CBOM file"
428+
Success _ Nothing -> logInfo "No crypto findings to write to CBOM file"
429+
Failure _ _ -> logWarn "Crypto CBOM generation failed; see diagnostics above"
428430
Nothing -> pure ()
429431

430432
let -- This makes nice with additionalSourceUnits below, but throws out additional Result data.
@@ -520,11 +522,12 @@ analyze cfg = Diag.context "fossa-analyze" $ do
520522

521523
-- Render FIPS compliance report if requested
522524
when (Config.cryptoFipsReport cfg) $
523-
case join (resultToMaybe maybeCryptoScanResults) of
524-
Just cryptoResults -> do
525+
case maybeCryptoScanResults of
526+
Success _ (Just cryptoResults) -> do
525527
logInfo ""
526528
logInfo . renderIt $ renderFipsReport cryptoResults
527-
Nothing -> logInfo "No crypto findings for FIPS report"
529+
Success _ Nothing -> logInfo "No crypto findings for FIPS report"
530+
Failure _ _ -> logWarn "Crypto scan failed; skipping FIPS report"
528531

529532
-- Need to check if vendored is empty as well, even if its a boolean that vendoredDeps exist
530533
let licenseSourceUnits =

src/App/Fossa/Analyze/ScanSummary.hs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -332,9 +332,9 @@ summarizeProjectScan (SkippedDueToProvidedFilter dpi) = renderDiscoveredProjectI
332332
summarizeProjectScan (SkippedDueToDefaultFilter dpi) = renderDiscoveredProjectIdentifier dpi <> skippedDueDefaultFilter
333333

334334
summarizeCryptoScan :: Result (Maybe CryptoScanResults) -> [Doc AnsiStyle]
335-
summarizeCryptoScan (Success _ (Just (CryptoScanResults findings)))
335+
summarizeCryptoScan (Success wg (Just (CryptoScanResults findings)))
336336
| not (null findings) =
337-
[successColorCoded [] $ listSymbol <> "Crypto Scan" <> ": succeeded"]
337+
[successColorCoded wg $ listSymbol <> "Crypto Scan" <> renderSucceeded wg]
338338
<> itemize (" " <> listSymbol) renderCryptoFinding findings
339339
| otherwise = []
340340
summarizeCryptoScan (Failure _ _) = [failColorCoded $ annotate bold $ listSymbol <> "Crypto Scan" <> renderFailed]

src/App/Fossa/CryptoScan/FipsReport.hs

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,9 +47,16 @@ compliancePercentage FipsReportStats{..}
4747
| totalAlgorithms == 0 = 100
4848
| otherwise = (approvedCount * 100) `div` totalAlgorithms
4949

50+
-- | Dedupe key for algorithm variants — includes parameter set so that
51+
-- e.g. RSA-2048 and RSA-1024 are counted separately.
52+
dedupeKey :: CryptoFinding -> (Text, Maybe Text)
53+
dedupeKey finding =
54+
let algo = cryptoFindingAlgorithm finding
55+
in (Text.toCaseFold (cryptoAlgorithmName algo), cryptoAlgorithmParameterSet algo)
56+
5057
computeFipsStats :: CryptoScanResults -> FipsReportStats
5158
computeFipsStats (CryptoScanResults findings) =
52-
let uniqueAlgos = nubBy (\a b -> cryptoAlgorithmName (cryptoFindingAlgorithm a) == cryptoAlgorithmName (cryptoFindingAlgorithm b)) findings
59+
let uniqueAlgos = nubBy (\a b -> dedupeKey a == dedupeKey b) findings
5360
statuses = map (cryptoAlgorithmFipsStatus . cryptoFindingAlgorithm) uniqueAlgos
5461
in FipsReportStats
5562
{ totalAlgorithms = length uniqueAlgos
@@ -62,7 +69,7 @@ computeFipsStats (CryptoScanResults findings) =
6269
renderFipsReport :: CryptoScanResults -> Doc AnsiStyle
6370
renderFipsReport results@(CryptoScanResults findings) =
6471
let stats = computeFipsStats results
65-
uniqueFindings = nubBy (\a b -> cryptoAlgorithmName (cryptoFindingAlgorithm a) == cryptoAlgorithmName (cryptoFindingAlgorithm b)) findings
72+
uniqueFindings = nubBy (\a b -> dedupeKey a == dedupeKey b) findings
6673
categorized = categorizeFindings uniqueFindings
6774
in vsep
6875
[ annotate bold "FIPS Compliance Report"

0 commit comments

Comments
 (0)