diff --git a/fcli-core/fcli-app/src/main/java/com/fortify/cli/app/FortifyCLI.java b/fcli-core/fcli-app/src/main/java/com/fortify/cli/app/FortifyCLI.java index f2ab435cb7..34eaf65c90 100644 --- a/fcli-core/fcli-app/src/main/java/com/fortify/cli/app/FortifyCLI.java +++ b/fcli-core/fcli-app/src/main/java/com/fortify/cli/app/FortifyCLI.java @@ -17,9 +17,9 @@ /** *

This class provides the {@link #main(String[])} entrypoint into the application, - * and also registers some GraalVM features, allowing the application to run properly + * and also registers some GraalVM features, allowing the application to run properly * as GraalVM native images.

- * + * * @author Ruud Senden */ public class FortifyCLI { diff --git a/fcli-core/fcli-aviator-common/src/main/java/com/fortify/cli/aviator/fpr/processor/RemediationProcessor.java b/fcli-core/fcli-aviator-common/src/main/java/com/fortify/cli/aviator/fpr/processor/RemediationProcessor.java index 2f717dd66b..8d5babcd8c 100644 --- a/fcli-core/fcli-aviator-common/src/main/java/com/fortify/cli/aviator/fpr/processor/RemediationProcessor.java +++ b/fcli-core/fcli-aviator-common/src/main/java/com/fortify/cli/aviator/fpr/processor/RemediationProcessor.java @@ -30,6 +30,8 @@ import java.util.Map; import java.util.Objects; import java.util.Set; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import java.util.stream.Collectors; import java.util.zip.ZipFile; @@ -53,8 +55,7 @@ import com.fortify.cli.aviator.fpr.utils.SourceDecoders; import com.fortify.cli.aviator.fpr.utils.SourceEncoder; import com.fortify.cli.aviator.fpr.utils.SourceEncoder.SourceEncodeException; -import com.fortify.cli.aviator.util.FprHandle; -import com.fortify.cli.aviator.util.FuzzyContextSearcher; +import com.fortify.cli.aviator.util.*; public class RemediationProcessor { private static final Logger LOG = LoggerFactory.getLogger(RemediationProcessor.class); @@ -64,10 +65,10 @@ public class RemediationProcessor { private final String sourceCodeDirectory; private final ISourceDecoder sourceDecoder; - public record RemediationMetric(int totalRemediations, int appliedRemediations, int skippedRemediations, Set modifiedFiles, + public record RemediationMetric(int totalRemediations, int appliedRemediations, int identicalRemediations,int skippedRemediations, Set modifiedFiles, Map skippedByReason) { - public RemediationMetric(int totalRemediations, int appliedRemediations, int skippedRemediations, Set modifiedFiles) { - this(totalRemediations, appliedRemediations, skippedRemediations, modifiedFiles, Map.of()); + public RemediationMetric(int totalRemediations, int appliedRemediations,int identicalRemediations,int skippedRemediations, Set modifiedFiles) { + this(totalRemediations, appliedRemediations,identicalRemediations, skippedRemediations, modifiedFiles, Map.of()); } } @@ -75,9 +76,15 @@ private record SourceFileContent(String content, Charset charset, String encodin private record PendingFileWrite(String filename, Path filePath, String content, Charset charset, String encodingSource, byte[] updatedBytes) {} + private record AppliedChange(Path filePath, int originalStart, int originalEnd, int resultingStart, int resultingEnd, String remediationId) { + private int lineDelta() { return (resultingEnd - resultingStart + 1) - (originalEnd - originalStart + 1); } + } + private record ChangeApplication(String content, AppliedChange appliedChange) {} private record RollbackFileWrite(String filename, Path filePath, byte[] originalBytes) {} + private record RemediationKey(String fileName, Path filePath,int lineFrom,int lineTo,String comparisonCode){} + private enum SkipReason { SOURCE_FILE_MISSING("Source file missing"), SOURCE_FILE_OUTSIDE_SOURCE_DIR("Source file outside source directory"), @@ -88,11 +95,14 @@ private enum SkipReason { SOURCE_CONTEXT_NOT_FOUND("Source context not found"), SOURCE_CONTEXT_AMBIGUOUS("Source context matched multiple locations"), ORIGINAL_CODE_NOT_FOUND("Original code not found"), + ANCHOR_MISMATCH("Anchor does not match"), + CONFLICT("Conflicts with another fix"), REMEDIATION_ENCODE_FAILED("Remediation encode failed"), SOURCE_WRITE_FAILED("Source file write failed"), NO_CHANGES("No file changes found"), UNEXPECTED_ERROR("Unexpected remediation processing error"); + private final String displayName; SkipReason(String displayName) { @@ -154,9 +164,12 @@ public RemediationMetric processRemediationXML() { Document remediationDoc; int totalRemediations; int appliedRemediations; + int identicalRemediations = 0; Set modifiedFiles = new LinkedHashSet<>(); Map skippedByReason = new LinkedHashMap<>(); - + Map remediationLookup = new LinkedHashMap<>(); + Map> appliedChangesByFile = new LinkedHashMap<>(); + LOG.debug("in the processRemediationXML method"); // Sanitize and normalize the base source directory path once. String trimmedSourceDir = sourceCodeDirectory.trim(); if (trimmedSourceDir.length() > 1 && @@ -183,10 +196,76 @@ public RemediationMetric processRemediationXML() { totalRemediations = remediationNodes.getLength(); LOG.debug("Loaded {} remediation entries from {}", totalRemediations, remediationPath); appliedRemediations = 0; + for (int i = 0; i < remediationNodes.getLength(); i++) { - Element remediation = (Element) remediationNodes.item(i); - if (processRemediation(remediation, sourceBasePath, fvdlMetadata, modifiedFiles, skippedByReason)) { + LOG.debug("........................"); + Element remediation = + (Element) remediationNodes.item(i); + + String instanceId = + remediation.getAttribute("instanceId"); + LOG.debug("remediation{}",instanceId); + + List remediationKeys = + createRemediationKeys( + remediation, + sourceBasePath); + + LOG.debug( + "Remediation {} generated {} lookup key(s): {}", + instanceId, + remediationKeys.size(), + remediationKeys); + + String identicalInstanceId = null; + + /* + * A remediation is identical only when all of its changes + * match an existing remediation. + */ + if (!remediationKeys.isEmpty()) { + for (String existingInstanceId : + new LinkedHashSet<>(remediationLookup.values())) { + + List existingKeys = + remediationLookup.entrySet().stream() + .filter(entry -> + existingInstanceId.equals(entry.getValue())) + .map(Map.Entry::getKey) + .toList(); + + if (existingKeys.size() == remediationKeys.size() + && existingKeys.containsAll(remediationKeys)) { + identicalInstanceId = existingInstanceId; + break; + } + } + } + + if (identicalInstanceId != null) { + identicalRemediations++; + appliedRemediations++; + + LOG.info( + "Identical found: {}", + identicalInstanceId); + + LOG.info( + "Identical Remediation Applied: {} is identical to {}", + instanceId, + identicalInstanceId); + + continue; + } + + if (processRemediation(remediation, sourceBasePath, fvdlMetadata, modifiedFiles, skippedByReason, appliedChangesByFile)) { + appliedRemediations++; + + for (RemediationKey key : remediationKeys) { + LOG.debug("putting {}",instanceId); + remediationLookup.put(key, instanceId); + } } } @@ -195,29 +274,37 @@ public RemediationMetric processRemediationXML() { throw new AviatorTechnicalException("Error processing remediation.xml file.", e); } catch (AviatorTechnicalException e) { throw e; + } catch (Exception e) { LOG.error("Unexpected error processing remediation.xml: {}", remediationPath, e); throw new AviatorTechnicalException("Unexpected error processing remediations.xml.", e); } + int skippedRemediations = totalRemediations - appliedRemediations; - LOG.info("Auto-remediation summary: total={}, applied={}, skipped={}", totalRemediations, appliedRemediations, skippedRemediations); + LOG.info("Auto-remediation summary: total={}, applied={},indentical={},skipped={}", totalRemediations, appliedRemediations, identicalRemediations,skippedRemediations); + if (!skippedByReason.isEmpty()) { - LOG.info("Skipped remediations by reason: {}", formatSkippedReasons(skippedByReason)); + LOG.info("Skipped remediations by reason: {}",formatSkippedReasons(skippedByReason)); } - return new RemediationMetric(totalRemediations, appliedRemediations, skippedRemediations, modifiedFiles, skippedByReason); + return new RemediationMetric(totalRemediations, appliedRemediations, identicalRemediations, skippedRemediations, modifiedFiles, skippedByReason); } - private boolean processRemediation(Element remediation, Path sourceBasePath, FVDLMetadata fvdlMetadata, - Set modifiedFiles, Map skippedByReason) { + + private boolean processRemediation(Element remediation, Path sourceBasePath, FVDLMetadata fvdlMetadata, Set modifiedFiles, + Map skippedByReason, Map> appliedChangesByFile) { String instanceId = remediation.getAttribute("instanceId"); try { - Map pendingWrites = prepareFileChanges(remediation, sourceBasePath, fvdlMetadata); + List stagedChanges = new ArrayList<>(); + Map pendingWrites = prepareFileChanges(remediation, sourceBasePath, fvdlMetadata, appliedChangesByFile, stagedChanges); if (pendingWrites.isEmpty()) { recordSkipped(skippedByReason, SkipReason.NO_CHANGES.displayName); return false; } try { commitRemediationWrites(instanceId, pendingWrites, modifiedFiles); + for (AppliedChange change : stagedChanges) { + appliedChangesByFile.computeIfAbsent(change.filePath(), key -> new ArrayList<>()).add(change); + } return true; } catch (RemediationCommitException e) { rollbackRemediationWrites(instanceId, e.getRollbacks()); @@ -238,8 +325,7 @@ private boolean processRemediation(Element remediation, Path sourceBasePath, FVD } } - private Map prepareFileChanges(Element remediation, Path sourceBasePath, - FVDLMetadata fvdlMetadata) { + private Map prepareFileChanges(Element remediation, Path sourceBasePath, FVDLMetadata fvdlMetadata, Map> appliedChangesByFile, List stagedChanges) { NodeList fileChangesNodes = remediation.getElementsByTagNameNS(NAMESPACE_URI, "FileChanges"); if (fileChangesNodes.getLength() == 0) { throw new SkipRemediationException(SkipReason.NO_CHANGES, "No file changes found"); @@ -247,14 +333,13 @@ private Map prepareFileChanges(Element remediation, Path Map pendingWrites = new LinkedHashMap<>(); for (int j = 0; j < fileChangesNodes.getLength(); j++) { - processFileChanges(remediation, (Element) fileChangesNodes.item(j), sourceBasePath, fvdlMetadata, pendingWrites); + processFileChanges(remediation, (Element) fileChangesNodes.item(j), sourceBasePath, fvdlMetadata, pendingWrites, appliedChangesByFile, stagedChanges); } return pendingWrites; } - private boolean processFileChanges(Element remediation, Element fileChanges, Path sourceBasePath, FVDLMetadata fvdlMetadata, - Map pendingWrites) { - String instanceId = remediation.getAttribute("instanceId"); + private boolean processFileChanges(Element remediation, Element fileChanges, Path sourceBasePath, FVDLMetadata fvdlMetadata, Map pendingWrites, Map> appliedChangesByFile, List stagedChanges) {String instanceId = remediation.getAttribute("instanceId"); String filename = getRequiredElementText(fileChanges, "Filename"); Path filePath = sourceBasePath.resolve(filename).normalize(); LOG.debug("Processing remediation {} file change for '{}' resolved to '{}'", instanceId, filename, filePath); @@ -280,8 +365,9 @@ private boolean processFileChanges(Element remediation, Element fileChanges, Pat String updatedContent = sourceFileContent.content(); for (int k = 0; k < changesNodes.getLength(); k++) { - updatedContent = applyChange(instanceId, filename, fileHash, sourceEncoding, updatedContent, - (Element) changesNodes.item(k), k + 1); + ChangeApplication application = applyChange(instanceId, filename, filePath, fileHash, sourceEncoding, updatedContent, (Element) changesNodes.item(k), k + 1, appliedChangesByFile, stagedChanges); + updatedContent = application.content(); + stagedChanges.add(application.appliedChange()); } byte[] updatedBytes = encodeSourceFile(updatedContent, sourceEncoding, filename); pendingWrites.put(filePath, new PendingFileWrite(filename, filePath, updatedContent, sourceEncoding, @@ -291,62 +377,73 @@ private boolean processFileChanges(Element remediation, Element fileChanges, Pat return true; } - private String applyChange(String instanceId, String filename, String fileHash, Charset sourceEncoding, String originalContent, - Element change, int changeIndex) { + private ChangeApplication applyChange(String instanceId, String filename, Path filePath, String fileHash, Charset sourceEncoding, String originalContent, Element change, int changeIndex, Map> appliedChangesByFile, List stagedChanges) { String lineSeparator = detectLineSeparator(originalContent); String content = normalizeLineEndings(originalContent); - List originalLines = Arrays.asList(content.split("\n", -1)); - LOG.debug("Decoded '{}' using {}; lineSeparator={}, normalizedLines={}", filename, sourceEncoding.name(), - describeLineSeparator(lineSeparator), originalLines.size()); - - int lineFrom = parseRequiredInt(change, "LineFrom"); - int lineTo = parseRequiredInt(change, "LineTo"); - LOG.debug("Remediation {} change {} for '{}' targets lines {}-{}", instanceId, changeIndex, filename, lineFrom, lineTo); - + int declaredLineFrom = parseRequiredInt(change, "LineFrom"); + int declaredLineTo = parseRequiredInt(change, "LineTo"); String calculatedHash = calculateHashBase64(content, "SHA-256"); boolean fileHashMatches = calculatedHash.equals(fileHash); - LOG.debug("Remediation {} hash check for '{}': {}", instanceId, filename, fileHashMatches ? "matched" : "mismatched"); - if (!fileHashMatches) { - LOG.debug("File hash mismatch for remediation {} in {}; searching changed source content", instanceId, filename); + List previousChanges = new ArrayList<>(); + List committedChanges = appliedChangesByFile.get(filePath); + if (committedChanges != null) { previousChanges.addAll(committedChanges); } + for (AppliedChange stagedChange : stagedChanges) { if (filePath.equals(stagedChange.filePath())) { previousChanges.add(stagedChange); } } + int lineFrom = declaredLineFrom; + int lineTo = declaredLineTo; + if (!previousChanges.isEmpty()) { + int[] projectedRange = projectLineRange(declaredLineFrom, declaredLineTo, previousChanges, instanceId, filename); + lineFrom = projectedRange[0]; + lineTo = projectedRange[1]; + validateLineRange(lineFrom, lineTo, originalLines.size(), filename); + verifyOriginalCodeAtRange(instanceId, filename, originalLines, lineFrom, lineTo, change); + } else if (!fileHashMatches) { Element contextElement = getRequiredElement(change, "Context"); - String contextText = contextElement.getTextContent(); - List contextLine = Arrays.asList(contextText.split("\\r?\\n")); + List contextLine = Arrays.asList(contextElement.getTextContent().split("\\r?\\n")); int contextLineFrom = fuzzySearchContext(instanceId, filename, originalLines, contextLine); - if (contextLineFrom == -1) { - LOG.debug("Context search failed for remediation {} in {}; context lines={}, source lines={}", instanceId, filename, - contextLine.size(), originalLines.size()); - throw new SkipRemediationException(SkipReason.SOURCE_CONTEXT_NOT_FOUND, "Source context not found for file '" + filename + - "'; file may have changed or remediation may overlap a previous change"); - } - LOG.debug("Context for remediation {} in {} matched at line {}", instanceId, filename, contextLineFrom + 1); - + if (contextLineFrom == -1) { throw new SkipRemediationException(SkipReason.SOURCE_CONTEXT_NOT_FOUND, "Source context not found for file '" + filename + "'"); } String originalCodeText = getRequiredElementText(change, "OriginalCode"); List originalCodeLine = Arrays.asList(originalCodeText.split("\\r?\\n")); int contextBefore = parseRequiredContextAttribute(contextElement, "before"); int contextAfter = parseRequiredContextAttribute(contextElement, "after"); - int[] lineFromTo = fuzzySearchOriginalCode(instanceId, filename, originalLines, originalCodeLine, - contextLineFrom, contextLine.size(), contextBefore, contextAfter); - if (lineFromTo[0] == -1 || lineFromTo[1] == -1) { - LOG.debug("Original code search failed for remediation {} in {}; context line={}, original code lines={}, source lines={}", - instanceId, filename, contextLineFrom + 1, originalCodeLine.size(), originalLines.size()); - throw new SkipRemediationException(SkipReason.ORIGINAL_CODE_NOT_FOUND, "Original code not found for file '" + filename + - "'; file may have changed or remediation may overlap a previous change"); - } + int[] lineFromTo = fuzzySearchOriginalCode(instanceId, filename, originalLines, originalCodeLine, contextLineFrom, contextLine.size(), contextBefore, contextAfter); + if (lineFromTo[0] == -1 || lineFromTo[1] == -1) { throw new SkipRemediationException(SkipReason.ORIGINAL_CODE_NOT_FOUND, "Original code not found for file '" + filename + "'"); } lineFrom = lineFromTo[0] + 1; lineTo = lineFromTo[1] + 1; - LOG.debug("Original code for remediation {} in {} matched at lines {}-{}", instanceId, filename, lineFrom, lineTo); } - validateLineRange(lineFrom, lineTo, originalLines.size(), filename); List newCodeLines = Arrays.asList(getRequiredElementText(change, "NewCode").split("\n")); List updatedLines = new ArrayList<>(); updatedLines.addAll(originalLines.subList(0, lineFrom - 1)); updatedLines.addAll(newCodeLines); updatedLines.addAll(originalLines.subList(lineTo, originalLines.size())); - LOG.debug("Staged remediation {} change {} for '{}' using FVDL encoding {}; updatedLines={}", instanceId, changeIndex, - filename, sourceEncoding.name(), updatedLines.size()); - return String.join(lineSeparator, updatedLines); + String updatedContent = String.join(lineSeparator, updatedLines); + AppliedChange appliedChange = new AppliedChange(filePath, declaredLineFrom, declaredLineTo, lineFrom, lineFrom + newCodeLines.size() - 1, instanceId); + return new ChangeApplication(updatedContent, appliedChange); + } + private int[] projectLineRange(int originalStart, int originalEnd, List appliedChanges, String instanceId, String filename) { + int projectedStart = originalStart; + int projectedEnd = originalEnd; + for (AppliedChange applied : appliedChanges) { + boolean overlaps = originalStart <= applied.originalEnd() && originalEnd >= applied.originalStart(); + if (overlaps) { + int overlapStart = Math.max(originalStart, applied.originalStart()); + int overlapEnd = Math.min(originalEnd, applied.originalEnd()); + throw new SkipRemediationException(SkipReason.CONFLICT, "Remediation '" + instanceId + "' conflicts with remediation '" + applied.remediationId() + "' in file '" + filename + "'; overlapping original lines " + overlapStart + "-" + overlapEnd); + } + if (originalStart > applied.originalEnd()) { + projectedStart += applied.lineDelta(); + projectedEnd += applied.lineDelta(); + } + } + return new int[] {projectedStart, projectedEnd}; + } + private void verifyOriginalCodeAtRange(String instanceId, String filename, List sourceLines, int lineFrom, int lineTo, Element change) { + List expectedLines = Arrays.asList(normalizeLineEndings(getRequiredElementText(change, "OriginalCode")).split("\n", -1)); + List actualLines = sourceLines.subList(lineFrom - 1, lineTo); + if (!expectedLines.equals(actualLines)) { + throw new SkipRemediationException(SkipReason.ANCHOR_MISMATCH, "Anchor does not match for remediation '" + instanceId + "' in file '" + filename + "' at lines " + lineFrom + "-" + lineTo); + } } private SourceFileContent getPendingOrSourceContent(Path filePath, String filename, FVDLMetadata fvdlMetadata, @@ -577,4 +674,179 @@ private String formatSkippedReasons(Map skippedByReason) { skippedByReason.forEach((reason, count) -> parts.add(reason + "=" + count)); return String.join(", ", parts); } + + private RemediationKey createRemediationKey( + Element fileChanges, + Element change, + Path sourceBasePath, + String comparisonCode) { + + String fileName = getRequiredElementText(fileChanges, "Filename"); + Path filePath = sourceBasePath.resolve(fileName).normalize(); + + int lineFrom = parseRequiredInt(change, "LineFrom"); + int lineTo = parseRequiredInt(change, "LineTo"); + + return new RemediationKey( + fileName, + filePath, + lineFrom, + lineTo, + comparisonCode + ); + } + + + private String trimBlankLines(String content) { + String[] lines = content.split("\\R", -1); + + int start = 0; + int end = lines.length - 1; + + while (start <= end && lines[start].isBlank()) { + start++; + } + + while (end >= start && lines[end].isBlank()) { + end--; + } + + if (start > end) { + return ""; + } + + return String.join( + System.lineSeparator(), + Arrays.copyOfRange(lines, start, end + 1)); + } + + private String normalizeProposedCode(String content, String fileName) { + if (content == null) { + return null; + } + + String language = FileTypeLanguageMapperUtil.getProgrammingLanguage( + FileUtil.getFileExtension(fileName)); + + String commentSymbol = + LanguageCommentMapperUtil.getProgrammingLanguageComment(language); + + if ("Unknown".equals(commentSymbol)) { + return trimBlankLines(content); + } + + String closingToken = commentSymbol.equals("" + : commentSymbol.equals("<%--") ? "--%>" + : null; + + Pattern markerPattern = Pattern.compile( + "[ \\t]*" + Pattern.quote(commentSymbol) + " L\\d+" + + (closingToken != null + ? "[ \\t]*" + Pattern.quote(closingToken) + : "") + + "[ \\t]*$"); + + String[] lines = content.split("\\R", -1); + StringBuilder result = new StringBuilder(); + + for (int i = 0; i < lines.length; i++) { + Matcher matcher = markerPattern.matcher(lines[i]); + + result.append( + matcher.find() + ? lines[i].substring(0, matcher.start()) + : lines[i]); + + if (i < lines.length - 1) { + result.append(System.lineSeparator()); + } + } + + return trimBlankLines(result.toString()); + } + + private String createComparisonCode(String normalizedCode, String fileName) { + if (normalizedCode == null) { + return null; + } + + String language = FileTypeLanguageMapperUtil.getProgrammingLanguage( + FileUtil.getFileExtension(fileName)); + + String commentSymbol = + LanguageCommentMapperUtil.getProgrammingLanguageComment(language); + + if ("Unknown".equals(commentSymbol)) { + return normalizedCode.replaceAll("\\s+", ""); + } + + String comparisonCode = normalizedCode; + + // Remove block comments + String closingToken = commentSymbol.equals("" + : commentSymbol.equals("<%--") ? "--%>" + : null; + + if (closingToken != null) { + comparisonCode = comparisonCode.replaceAll( + "(?s)" + Pattern.quote(commentSymbol) + + ".*?" + Pattern.quote(closingToken), + ""); + } else if ("//".equals(commentSymbol)) { + comparisonCode = comparisonCode.replaceAll( + "(?m)" + Pattern.quote(commentSymbol) + ".*$", + ""); + comparisonCode = comparisonCode.replaceAll( + "(?s)/\\*.*?\\*/", + ""); + } else if ("#".equals(commentSymbol)) { + comparisonCode = comparisonCode.replaceAll( + "(?m)" + Pattern.quote(commentSymbol) + ".*$", + ""); + } + + // Normalize whitespace + return comparisonCode.replaceAll("\\s+", ""); + } + + private List createRemediationKeys( + Element remediation, + Path sourceBasePath) { + + List keys = new ArrayList<>(); + + NodeList fileChangesNodes = + remediation.getElementsByTagNameNS(NAMESPACE_URI, "FileChanges"); + + for (int i = 0; i < fileChangesNodes.getLength(); i++) { + Element fileChanges = (Element) fileChangesNodes.item(i); + + NodeList changeNodes = + fileChanges.getElementsByTagNameNS(NAMESPACE_URI, "Change"); + + for (int j = 0; j < changeNodes.getLength(); j++) { + Element change = (Element) changeNodes.item(j); + + String fileName = + getRequiredElementText(fileChanges, "Filename"); + + String newCode = + getRequiredElementText(change, "NewCode"); + + String normalizedCode = + normalizeProposedCode(newCode, fileName); + + String comparisonCode = + createComparisonCode(normalizedCode, fileName); + + keys.add(createRemediationKey( + fileChanges, + change, + sourceBasePath, + comparisonCode)); + } + } + + return keys; + } }