P/umadevis1/remediation part2

fcli-core/fcli-aviator-common/src/main/java/com/fortify/cli/aviator/fpr/remediation/classifier/AppliedChangeLedger.java

@@ -84,6 +84,8 @@ private static AppliedChange fromPending(PendingAppliedChange pac) {
         return AppliedChange.builder()
             .originalLineFrom(pac.lineFrom())
             .originalLineTo(pac.lineTo())
+            .declaredLineFrom(pac.declaredLineFrom())
+            .declaredLineTo(pac.declaredLineTo())
             .deltaLines(pac.deltaLines())
             .comparisonCode(pac.comparisonCode())
             .lineNormalizedCode(pac.lineNormalizedCode())

fcli-core/fcli-aviator-common/src/main/java/com/fortify/cli/aviator/fpr/remediation/classifier/HunkClassifier.java

@@ -80,13 +80,13 @@ public List<HunkOutcome> classifyRemediationHunks(Remediation remediation, Path
      */
     private HunkOutcome classifyRange(int lineFrom, int lineTo, List<AppliedChange> applied, String candidateComparisonCode) {
         for (AppliedChange ac : applied) {
-            if (ac.coversFully(lineFrom, lineTo)) {
+            if (ac.coversDeclaredRange(lineFrom, lineTo)) {
                 if (ac.contentCovers(candidateComparisonCode, lineFrom, lineTo)) {
                     return HunkOutcome.SUPERSEDED;
                 }
                 return HunkOutcome.POSSIBLY_REMEDIATED;
             }
-            if (ac.overlapsPartially(lineFrom, lineTo)) {
+            if (ac.overlapsDeclaredRangePartially(lineFrom, lineTo)) {
                 return HunkOutcome.CONFLICTS;
             }
         }

fcli-core/fcli-aviator-common/src/main/java/com/fortify/cli/aviator/fpr/remediation/classifier/PendingAppliedChange.java

@@ -14,6 +14,6 @@
 
 import java.nio.file.Path;
 
-public record PendingAppliedChange(Path filePath, int lineFrom, int lineTo, int deltaLines,
-                                    String comparisonCode, String lineNormalizedCode) {
+public record PendingAppliedChange(Path filePath, int lineFrom, int lineTo, int declaredLineFrom, int declaredLineTo,
+                                    int deltaLines, String comparisonCode, String lineNormalizedCode) {
 }

fcli-core/fcli-aviator-common/src/main/java/com/fortify/cli/aviator/fpr/remediation/model/AppliedChange.java

@@ -26,18 +26,22 @@
 public final class AppliedChange {
     private final int originalLineFrom;
     private final int originalLineTo;
+    private final int declaredLineFrom;
+    private final int declaredLineTo;
     private final int deltaLines;
     private final String comparisonCode;
     private final String[] lineNormalizedContent;
 
     public AppliedChange(int originalLineFrom, int originalLineTo, int deltaLines, String comparisonCode) {
-        this(originalLineFrom, originalLineTo, deltaLines, comparisonCode, null);
+        this(originalLineFrom, originalLineTo, originalLineFrom, originalLineTo, deltaLines, comparisonCode, null);
     }
 
     @Builder
-    public AppliedChange(int originalLineFrom, int originalLineTo, int deltaLines, String comparisonCode, String lineNormalizedCode) {
+    public AppliedChange(int originalLineFrom, int originalLineTo, int declaredLineFrom, int declaredLineTo, int deltaLines, String comparisonCode, String lineNormalizedCode) {
         this.originalLineFrom = originalLineFrom;
         this.originalLineTo = originalLineTo;
+        this.declaredLineFrom = declaredLineFrom;
+        this.declaredLineTo = declaredLineTo;
         this.deltaLines = deltaLines;
         this.comparisonCode = comparisonCode;
         // Store line-by-line normalized content for offset-anchored comparison (newlines preserved, each line normalized)
@@ -63,8 +67,21 @@ public boolean coversFully(int lineFrom, int lineTo) {
 
     /** True if [lineFrom, lineTo] overlaps this change's original range without either side fully containing the other. */
     public boolean overlapsPartially(int lineFrom, int lineTo) {
-        boolean disjoint = lineTo < originalLineFrom || lineFrom > originalLineTo;
-        return !disjoint;
+        boolean overlaps = lineFrom <= originalLineTo && originalLineFrom <= lineTo;
+        boolean candidateContainsThis = lineFrom <= originalLineFrom && originalLineTo <= lineTo;
+        return overlaps && !coversFully(lineFrom, lineTo) && !candidateContainsThis;
+    }
+
+    /** True if this change's declared range fully contains [lineFrom, lineTo]. */
+    public boolean coversDeclaredRange(int lineFrom, int lineTo) {
+        return declaredLineFrom <= lineFrom && lineTo <= declaredLineTo;
+    }
+
+    /** True if [lineFrom, lineTo] overlaps this change's declared range without either side fully containing the other. */
+    public boolean overlapsDeclaredRangePartially(int lineFrom, int lineTo) {
+        boolean overlaps = lineFrom <= declaredLineTo && declaredLineFrom <= lineTo;
+        boolean candidateContainsThis = lineFrom <= declaredLineFrom && declaredLineTo <= lineTo;
+        return overlaps && !coversDeclaredRange(lineFrom, lineTo) && !candidateContainsThis;
     }
 
     /**

fcli-core/fcli-aviator-common/src/main/java/com/fortify/cli/aviator/fpr/remediation/writer/FileWriteCoordinator.java

@@ -130,7 +130,8 @@ private void processFileChanges(Remediation remediation, FileChange fileChange,
             int linesAfterChange = updatedContent.split("\n", -1).length;
             int delta = linesAfterChange - linesBeforeChange;
             String lineNormalizedCode = hunk.lineNormalizedCode(filename);
-            ledger.stage(new PendingAppliedChange(filePath, actualLineFrom, actualLineTo, delta, comparisonCode, lineNormalizedCode));
+            ledger.stage(new PendingAppliedChange(filePath, actualLineFrom, actualLineTo, declaredLineFrom, declaredLineTo,
+                    delta, comparisonCode, lineNormalizedCode));
             appliedKeysOut.add(key);
             appliedInThisFile++;
         }
@@ -151,12 +152,25 @@ public void commitRemediationWrites(String instanceId, Map<Path, PendingFileWrit
             throws RemediationCommitException {
         List<RollbackFileWrite> rollbacks = new ArrayList<>();
         for (PendingFileWrite pendingWrite : pendingWrites.values()) {
+            Path filePath = pendingWrite.filePath();
+            // A permission failure is rejected atomically before any bytes are written, so a file
+            // that's already known unwritable needs no rollback entry at all: attempting one would
+            // just retry the same failing write. Checking this upfront (rather than relying on
+            // Files.write's exception to prove the file was untouched) matters once a write CAN
+            // start: e.g. running out of disk space mid-write can truncate/partially overwrite the
+            // file before failing, so a write that's confirmed writable must be recorded for
+            // rollback before attempting it, not after.
+            if (!Files.isWritable(filePath)) {
+                throw new RemediationCommitException(
+                        "Source code file is not writable: '" + pendingWrite.filename() + "'",
+                        new IOException("File not writable: " + filePath), rollbacks);
+            }
             try {
-                byte[] originalBytes = Files.readAllBytes(pendingWrite.filePath());
-                rollbacks.add(new RollbackFileWrite(pendingWrite.filename(), pendingWrite.filePath(), originalBytes));
+                byte[] originalBytes = Files.readAllBytes(filePath);
+                rollbacks.add(new RollbackFileWrite(pendingWrite.filename(), filePath, originalBytes));
                 LOG.debug("Writing remediation {} to '{}' using staged bytes; encodedBytes={}", instanceId, pendingWrite.filename(),
                         pendingWrite.updatedBytes().length);
-                Files.write(pendingWrite.filePath(), pendingWrite.updatedBytes());
+                Files.write(filePath, pendingWrite.updatedBytes());
             } catch (Exception e) {
                 throw new RemediationCommitException("Error writing source code file '" + pendingWrite.filename() + "'", e, rollbacks);
             }

fcli-core/fcli-aviator-common/src/test/java/com/fortify/cli/aviator/fpr/processor/RemediationProcessorTest.java

@@ -678,6 +678,39 @@ void illegalFilenameIsSkippedAndValidRemediationsStillApply() throws Exception {
         assertEquals("before\nREPLACED\nafter\n", Files.readString(sourceFile));
     }
 
+    /**
+     * A source file whose write fails (e.g., made read-only) must be skipped via
+     * {@code SOURCE_WRITE_FAILED} on its own, not abort the whole batch: the rollback attempt for
+     * that failed write must not itself retry writing to the same permission-denied file, which
+     * previously escalated into a batch-aborting {@code RollbackRemediationException} and caused
+     * every other remediation in the run - even ones touching unrelated, writable files - to fail.
+     */
+    @Test
+    @EnabledOnOs(OS.WINDOWS)
+    void readOnlyFileWriteFailureIsSkippedAndOtherRemediationsStillApply() throws Exception {
+        Path readOnlyFile = writeSourceFile("ReadOnly.java", "before\nTARGET\nafter\n");
+        Path goodFile = writeSourceFile("Good.java", "before\nTARGET\nafter\n");
+        readOnlyFile.toFile().setReadOnly();
+        try {
+            Path fprPath = buildFpr(List.of(
+                new MultiHunkRemediationSpec("blocked", List.of(new FileSpec("ReadOnly.java", List.of(
+                    new HunkSpec(2, 2, 1, 1, "before\ntarget\nafter", "TARGET", "REPLACED"))))),
+                new MultiHunkRemediationSpec("valid", List.of(new FileSpec("Good.java", List.of(
+                    new HunkSpec(2, 2, 1, 1, "before\ntarget\nafter", "TARGET", "REPLACED")))))));
+
+            RemediationMetric metric = apply(fprPath);
+
+            assertEquals(2, metric.totalRemediations());
+            assertEquals(1, metric.appliedRemediations(), "the remediation for the writable file must still be applied");
+            assertEquals(1, metric.skippedRemediations());
+            assertEquals(Map.of("Source file write failed", 1), metric.skippedByReason());
+            assertEquals("before\nTARGET\nafter\n", Files.readString(readOnlyFile));
+            assertEquals("before\nREPLACED\nafter\n", Files.readString(goodFile));
+        } finally {
+            readOnlyFile.toFile().setWritable(true);
+        }
+    }
+
     /**
      * The offset ledger must record where a hunk actually landed via the fuzzy anchor, not where
      * it was declared: {@code relocated} declares line 12 but its context/OriginalCode only exist
@@ -777,6 +810,41 @@ void secondFprDoesNotApplyOverAFixTheFirstFprAlreadyRewrote() throws Exception {
         assertEquals(afterFirst, Files.readString(sourceFile), "the second FPR must leave the file untouched");
     }
 
+    /**
+     * Regression test for coordinate-space mismatch bug: HunkClassifier must compare declared
+     * (pristine-file) line numbers against declared ranges, not actual (post-shift) ranges, when
+     * pre-classifying narrower hunks nested inside a broader fix that shifted them. The broader
+     * fix (wide-deletes) declares lines 1-5 and deletes them, replacing with 2 lines (delta -3).
+     * The narrower fix (narrow-inside) declares line 3, which sits inside 1-5's declared range.
+     * Before the fix, classifyRange would compare declared-3 against the broader hunk's ACTUAL
+     * range (which is now at a shifted position), missing the nesting; after the fix, it compares
+     * declared-against-declared and correctly classifies as POSSIBLY_REMEDIATED.
+     */
+    @Test
+    void narrowerRemediationInsideBroaderDeleteIsClassifiedAsPossiblyRemediated() throws Exception {
+        String originalSource = "line1\nline2\nline3\nline4\nline5\nline6\n";
+        writeSourceFile("Example.java", originalSource);
+        Path fprPath = buildFpr(List.of(
+            new MultiHunkRemediationSpec("wide-deletes", List.of(new FileSpec("Example.java", List.of(
+                new HunkSpec(1, 5, 0, 0,
+                    "line1\nline2\nline3\nline4\nline5",
+                    "line1\nline2\nline3\nline4\nline5",
+                    "replacement1\nreplacement2"))))),
+            new MultiHunkRemediationSpec("narrow-inside", List.of(new FileSpec("Example.java", List.of(
+                new HunkSpec(3, 3, 1, 1, "line2\nline3\nline4",
+                    "line3", "line3-modified")))))));
+
+        RemediationMetric metric = apply(fprPath);
+
+        assertEquals(2, metric.totalRemediations());
+        assertEquals(1, metric.appliedRemediations(), "the broader fix must be applied");
+        assertEquals(1, metric.possiblyRemediatedRemediations(),
+            "the narrower fix's declared line 3 sits inside the broader fix's declared 1-5, so it must be "
+                + "pre-classified as POSSIBLY_REMEDIATED even though its actual position shifted");
+        assertEquals(0, metric.skippedRemediations(),
+            "no remediation should be skipped; the narrower one must not fail with ANCHOR_DOES_NOT_MATCH");
+    }
+
     private record HunkSpec(int lineFrom, int lineTo, int contextBefore, int contextAfter,
             String context, String originalCode, String newCode) {}