Skip to content

Commit dd253b5

Browse files
kireetivarcdatla
kireetivar
and
cdatla
authored
chore: correct filtering logic to prevent valid issues from being skipped during audit (#882)
fix: `fcli aviator`: Correct filtering logic to prevent valid issues from being skipped during audit fix: `fcli aviator`: Ensure consistent file hash generation across different builds Co-authored-by: cdatla <[email protected]>
1 parent 8b5baad commit dd253b5

File tree

6 files changed

+508
-78
lines changed

6 files changed

+508
-78
lines changed

‎fcli-core/fcli-aviator-common/src/main/java/com/fortify/cli/aviator/audit/IssueAuditor.java‎

Lines changed: 38 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@
2828
import java.util.concurrent.TimeUnit;
2929
import java.util.concurrent.TimeoutException;
3030
import java.util.stream.Collectors;
31-
import java.util.stream.Stream;
3231

3332
import org.slf4j.Logger;
3433
import org.slf4j.LoggerFactory;
@@ -44,8 +43,8 @@
4443
import com.fortify.cli.aviator.fpr.filter.Filter;
4544
import com.fortify.cli.aviator.fpr.filter.FilterSet;
4645
import com.fortify.cli.aviator.fpr.filter.FolderDefinition;
47-
import com.fortify.cli.aviator.fpr.filter.SearchTree;
4846
import com.fortify.cli.aviator.fpr.filter.TagDefinition;
47+
import com.fortify.cli.aviator.fpr.filter.VulnerabilityFilterer;
4948
import com.fortify.cli.aviator.fpr.model.AuditIssue;
5049
import com.fortify.cli.aviator.fpr.model.FPRInfo;
5150
import com.fortify.cli.aviator.fpr.processor.AuditProcessor;
@@ -279,106 +278,69 @@ private boolean isAudited(UserPrompt userPrompt) {
279278
}
280279
return false;
281280
}
281+
282282
private List<Vulnerability> filterVulnerabilities(List<Vulnerability> allVulnerabilities, FilterSet fs) {
283283
List<String> targetFolderNames = filterSelection.getTargetFolderNames();
284284

285285
List<Filter> folderFilters = fs.getFilters().stream()
286-
.filter(f -> "setFolder".equalsIgnoreCase(f.getAction())).collect(Collectors.toList());
286+
.filter(f -> "setFolder".equalsIgnoreCase(f.getAction()))
287+
.collect(Collectors.toList());
288+
287289
List<Filter> hideFilters = fs.getFilters().stream()
288-
.filter(f -> "hide".equalsIgnoreCase(f.getAction())).collect(Collectors.toList());
289-
290-
Map<Filter, SearchTree> parsedQueries = new HashMap<>();
291-
// Use a stream to populate the map
292-
Stream.concat(folderFilters.stream(), hideFilters.stream()).forEach(f -> {
293-
try {
294-
parsedQueries.put(f, com.fortify.cli.aviator.fpr.filter.engine.FilterParser.parse(f.getQuery()));
295-
} catch (Exception e) {
296-
LOG.error("Failed to parse filter query: '{}'. This filter will be skipped.", f.getQuery(), e);
297-
parsedQueries.put(f, new com.fortify.cli.aviator.fpr.filter.SearchTree(null));
298-
}
299-
});
290+
.filter(f -> "hide".equalsIgnoreCase(f.getAction()))
291+
.collect(Collectors.toList());
300292

301293
Map<String, List<Vulnerability>> folderContents = new HashMap<>();
302-
for (Vulnerability vuln : allVulnerabilities) {
294+
295+
// 1. Process "setFolder" filters
296+
if (folderFilters.isEmpty()) {
297+
folderContents.put("Default", new ArrayList<>(allVulnerabilities));
298+
} else {
303299
for (Filter folderFilter : folderFilters) {
304-
if (com.fortify.cli.aviator.fpr.filter.engine.VulnerabilityEvaluator.evaluate(parsedQueries.get(folderFilter), vuln)) {
305-
folderContents.computeIfAbsent(folderFilter.getActionParam(), k -> new ArrayList<>()).add(vuln);
306-
break;
307-
}
300+
List<Vulnerability> matches = VulnerabilityFilterer.filter(allVulnerabilities, folderFilter.getQuery());
301+
folderContents.computeIfAbsent(folderFilter.getActionParam(), k -> new ArrayList<>()).addAll(matches);
308302
}
309303
}
310304

311-
folderContents.values().forEach(vulnList ->
312-
vulnList.removeIf(vuln -> hideFilters.stream().anyMatch(
313-
hideFilter -> com.fortify.cli.aviator.fpr.filter.engine.VulnerabilityEvaluator.evaluate(parsedQueries.get(hideFilter), vuln)
314-
))
315-
);
305+
// 2. Process "hide" filters
306+
for (Filter hideFilter : hideFilters) {
307+
for (List<Vulnerability> folderList : folderContents.values()) {
308+
if (folderList.isEmpty()) continue;
316309

310+
List<Vulnerability> toHide = VulnerabilityFilterer.filter(folderList, hideFilter.getQuery());
311+
if (!toHide.isEmpty()) {
312+
folderList.removeAll(toHide);
313+
}
314+
}
315+
}
316+
317+
// 3. Selection Logic (Flatten all or select specific folders)
317318
if (!filterSelection.isFilteringByFolder()) {
318-
List<Vulnerability> result = folderContents.values().stream().flatMap(List::stream).collect(Collectors.toList());
319+
List<Vulnerability> result = folderContents.values().stream()
320+
.flatMap(List::stream)
321+
.distinct()
322+
.collect(Collectors.toList());
319323
logger.info("FilterSet '{}' applied. {} of {} total vulnerabilities remain.", fs.getTitle(), result.size(), allVulnerabilities.size());
320324
return result;
321325
} else {
322-
// This logic correctly finds folder IDs based on user-provided names.
323326
Set<String> targetFolderIds = fs.getFolderDefinitions().stream()
324-
.filter(fd -> targetFolderNames.stream().anyMatch(name -> name.equalsIgnoreCase(fd.getName())))
325-
.map(FolderDefinition::getId)
326-
.collect(Collectors.toSet());
327+
.filter(fd -> targetFolderNames.stream().anyMatch(name -> name.equalsIgnoreCase(fd.getName())))
328+
.map(FolderDefinition::getId)
329+
.collect(Collectors.toSet());
327330

328331
if (targetFolderIds.isEmpty()) {
329332
String available = fs.getFolderDefinitions().stream().map(FolderDefinition::getName).collect(Collectors.joining("', '", "'", "'"));
330333
throw new AviatorSimpleException("Folder(s) not found in FilterSet '"+fs.getTitle()+"'. Available folders: "+available);
331334
}
332335

333336
List<Vulnerability> result = folderContents.entrySet().stream()
334-
.filter(entry -> targetFolderIds.contains(entry.getKey()))
335-
.flatMap(entry -> entry.getValue().stream())
336-
.collect(Collectors.toList());
337+
.filter(entry -> targetFolderIds.contains(entry.getKey()))
338+
.flatMap(entry -> entry.getValue().stream())
339+
.distinct()
340+
.collect(Collectors.toList());
337341

338342
logger.info("Filtered by folder(s) '{}'. {} vulnerabilities remain.", targetFolderNames, result.size());
339343
return result;
340344
}
341345
}
342-
343-
private void updateSkippedIssue(UserPrompt userPrompt, String reasonTemplate, int issuesInCategory, int totalIssues) {
344-
if (userPrompt == null || userPrompt.getIssueData() == null) {
345-
LOG.error("Cannot update skipped issue status for null userPrompt or issue data.");
346-
return;
347-
}
348-
String instanceId = userPrompt.getIssueData().getInstanceID();
349-
AuditIssue auditIssue = auditIssueMap.get(instanceId);
350-
351-
String comment = reasonTemplate
352-
.replace("{issues_new_in_category}", String.valueOf(issuesInCategory))
353-
.replace("{MAX_PER_CATEGORY}", String.valueOf(MAX_PER_CATEGORY))
354-
.replace("{issues_new_total}", String.valueOf(totalIssues))
355-
.replace("{MAX_TOTAL}", String.valueOf(MAX_TOTAL));
356-
357-
if (auditIssue != null) {
358-
LOG.debug("Updating existing audit entry for skipped issue: {}", instanceId);
359-
try {
360-
auditProcessor.addCommentToIssueXml(instanceId, comment, Constants.USER_NAME);
361-
} catch (Exception e) {
362-
LOG.error("Failed to add comment to XML for skipped issue {}: {}", instanceId, e.getMessage());
363-
}
364-
365-
auditProcessor.updateIssueTag(auditIssue, Constants.AVIATOR_STATUS_TAG_ID, Constants.PROCESSED_BY_AVIATOR);
366-
367-
Map<String, String> tags = auditIssue.getTags();
368-
String currentAnalysisStatus = tags.get(Constants.ANALYSIS_TAG_ID);
369-
if (currentAnalysisStatus == null || currentAnalysisStatus.isEmpty() || "Not Set".equalsIgnoreCase(currentAnalysisStatus)) {
370-
auditProcessor.updateIssueTag(auditIssue, Constants.ANALYSIS_TAG_ID, Constants.PENDING_REVIEW);
371-
LOG.debug("Set Analysis tag to Pending Review for skipped issue: {}", instanceId);
372-
} else {
373-
LOG.debug("Skipped issue {} already has Analysis status '{}', not changing.", instanceId, currentAnalysisStatus);
374-
}
375-
376-
} else {
377-
try {
378-
auditProcessor.addSkippedIssueElement(instanceId, comment);
379-
} catch (Exception e) {
380-
LOG.error("Failed to add skipped issue element to audit.xml for {}: {}", instanceId, e.getMessage());
381-
}
382-
}
383-
}
384-
}
346+
}

‎fcli-core/fcli-aviator-common/src/main/java/com/fortify/cli/aviator/fpr/utils/FileUtils.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414

1515

1616
import java.io.IOException;
17+
import java.nio.charset.StandardCharsets;
1718
import java.nio.file.Files;
1819
import java.nio.file.Path;
1920
import java.util.Arrays;
@@ -44,7 +45,7 @@ public List<String> readFileWithFallback(Path filePath) {
4445
return fileContentCache.computeIfAbsent(filePath, path -> {
4546
try {
4647
byte[] fileBytes = Files.readAllBytes(path);
47-
String content = new String(fileBytes);
48+
String content = new String(fileBytes, StandardCharsets.UTF_8);
4849
return Arrays.asList(content.split("\\r?\\n"));
4950
} catch (IOException e) {
5051
logger.error("Failed to read file: {}", path, e);
@@ -137,4 +138,4 @@ public Optional<String> getSourceFileContent(FprHandle fprHandle, String relativ
137138
return Optional.empty();
138139
}
139140
}
140-
}
141+
}

‎fcli-core/fcli-aviator-common/src/test/java/com/fortify/cli/aviator/audit/IssueAuditorTest.java‎

Lines changed: 149 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,149 @@
1+
/*
2+
* Copyright 2021-2025 Open Text.
3+
*
4+
* The only warranties for products and services of Open Text
5+
* and its affiliates and licensors ("Open Text") are as may
6+
* be set forth in the express warranty statements accompanying
7+
* such products and services. Nothing herein should be construed
8+
* as constituting an additional warranty. Open Text shall not be
9+
* liable for technical or editorial errors or omissions contained
10+
* herein. The information contained herein is subject to change
11+
* without notice.
12+
*/
13+
package com.fortify.cli.aviator.audit;
14+
15+
import static org.junit.jupiter.api.Assertions.assertEquals;
16+
import static org.junit.jupiter.api.Assertions.assertTrue;
17+
18+
import java.io.IOException;
19+
import java.lang.reflect.Method;
20+
import java.nio.charset.StandardCharsets;
21+
import java.nio.file.Files;
22+
import java.nio.file.Path;
23+
import java.util.ArrayList;
24+
import java.util.Arrays;
25+
import java.util.Collections;
26+
import java.util.HashMap;
27+
import java.util.List;
28+
import java.util.stream.Collectors;
29+
import java.util.zip.ZipEntry;
30+
import java.util.zip.ZipOutputStream;
31+
32+
import org.junit.jupiter.api.AfterEach;
33+
import org.junit.jupiter.api.BeforeEach;
34+
import org.junit.jupiter.api.Test;
35+
36+
import com.fortify.cli.aviator.audit.model.FilterSelection;
37+
import com.fortify.cli.aviator.config.IAviatorLogger;
38+
import com.fortify.cli.aviator.fpr.Vulnerability;
39+
import com.fortify.cli.aviator.fpr.filter.Filter;
40+
import com.fortify.cli.aviator.fpr.filter.FilterSet;
41+
import com.fortify.cli.aviator.fpr.filter.FilterTemplate;
42+
import com.fortify.cli.aviator.fpr.model.FPRInfo;
43+
import com.fortify.cli.aviator.util.FprHandle;
44+
45+
46+
class IssueAuditorTest {
47+
48+
private Path tempFprFile;
49+
private FprHandle fprHandle;
50+
51+
@BeforeEach
52+
void setup() throws IOException {
53+
// 1. Create a temporary dummy FPR file (ZIP format)
54+
tempFprFile = Files.createTempFile("test_aviator", ".fpr");
55+
try (ZipOutputStream zos = new ZipOutputStream(Files.newOutputStream(tempFprFile))) {
56+
57+
// A. Add a minimal audit.fvdl
58+
ZipEntry entry = new ZipEntry("audit.fvdl");
59+
zos.putNextEntry(entry);
60+
String minimalXml = "<?xml version=\"1.0\" encoding=\"UTF-8\"?><FVDL><UUID>test-uuid</UUID><Build><BuildID>test-build</BuildID></Build></FVDL>";
61+
zos.write(minimalXml.getBytes(StandardCharsets.UTF_8));
62+
zos.closeEntry();
63+
64+
// B. Add src-archive/index.xml to suppress the warning
65+
ZipEntry indexEntry = new ZipEntry("src-archive/index.xml");
66+
zos.putNextEntry(indexEntry);
67+
String indexXml = "<?xml version=\"1.0\" encoding=\"UTF-8\"?><index></index>";
68+
zos.write(indexXml.getBytes(StandardCharsets.UTF_8));
69+
zos.closeEntry();
70+
}
71+
fprHandle = new FprHandle(tempFprFile);
72+
}
73+
74+
@AfterEach
75+
void tearDown() throws IOException {
76+
if (fprHandle != null) {
77+
fprHandle.close();
78+
}
79+
if (tempFprFile != null) {
80+
Files.deleteIfExists(tempFprFile);
81+
}
82+
}
83+
84+
@Test
85+
void testFilterVulnerabilities_LegacySyntaxWithSpaces() throws Exception {
86+
87+
// Manual Logger Stub (No-op)
88+
IAviatorLogger dummyLogger = new IAviatorLogger() {
89+
@Override public void progress(String format, Object... args) {}
90+
@Override public void info(String format, Object... args) {}
91+
@Override public void warn(String format, Object... args) {}
92+
@Override public void error(String format, Object... args) {}
93+
};
94+
95+
FPRInfo fprInfo = new FPRInfo(fprHandle);
96+
97+
FilterTemplate dummyTemplate = new FilterTemplate();
98+
dummyTemplate.setTagDefinitions(new ArrayList<>());
99+
// Note: FPRInfo.setFilterTemplate() exists in the provided code
100+
fprInfo.setFilterTemplate(dummyTemplate);
101+
102+
// --- 2. SETUP FILTER SET WITH LEGACY SYNTAX ---
103+
// The "Buggy" Filter String (Legacy syntax: Space in range AND between terms)
104+
String legacyQuery = "impact:![2.5, 5.0] [Analysis Type]:!SONATYPE";
105+
106+
Filter filter = new Filter();
107+
filter.setAction("hide");
108+
filter.setQuery(legacyQuery);
109+
110+
FilterSet filterSet = new FilterSet();
111+
filterSet.setTitle("Regression Test Set");
112+
filterSet.setFilters(Collections.singletonList(filter));
113+
114+
FilterSelection selection = new FilterSelection(filterSet, null);
115+
116+
// Target Issue: High Impact (4.0), Null Analysis Type.
117+
// Result: Should NOT be hidden.
118+
Vulnerability targetVuln = new Vulnerability();
119+
targetVuln.setInstanceID("TARGET_ISSUE");
120+
targetVuln.setImpact(4.0);
121+
122+
// Noise Issue: Low Impact (1.0).
123+
// Result: Should be hidden.
124+
Vulnerability hiddenVuln = new Vulnerability();
125+
hiddenVuln.setInstanceID("HIDDEN_ISSUE");
126+
hiddenVuln.setImpact(1.0);
127+
128+
List<Vulnerability> inputList = Arrays.asList(targetVuln, hiddenVuln);
129+
130+
IssueAuditor auditor = new IssueAuditor(
131+
inputList, null, new HashMap<>(), fprInfo,
132+
"TestApp", "1.0", selection, dummyLogger
133+
);
134+
135+
Method filterMethod = IssueAuditor.class.getDeclaredMethod("filterVulnerabilities", List.class, FilterSet.class);
136+
filterMethod.setAccessible(true);
137+
138+
@SuppressWarnings("unchecked")
139+
List<Vulnerability> results = (List<Vulnerability>) filterMethod.invoke(auditor, inputList, filterSet);
140+
141+
List<String> remainingIds = results.stream()
142+
.map(Vulnerability::getInstanceID)
143+
.collect(Collectors.toList());
144+
145+
assertEquals(1, remainingIds.size(), "Should verify that exactly one issue remains");
146+
assertTrue(remainingIds.contains("TARGET_ISSUE"),
147+
"Regression Failed: IssueAuditor hid the target issue. It likely used the Modern parser on a Legacy query string.");
148+
}
149+
}

0 commit comments

Comments
 (0)