#205 Address OpenVuln findings

change-proneness-ranker/src/main/java/org/hjug/git/GitLogReader.java

@@ -96,6 +96,12 @@ public String getRepoUrl() throws IOException {
             return "";
         }
 
+        // Only web URLs may be embedded in generated reports as source links;
+        // anything else (javascript:, data:, file:, ...) is rejected outright.
+        if (!originUrl.startsWith("https://") && !originUrl.startsWith("http://")) {
+            return "";
+        }
+
         repoUrl = originUrl.replace(".git", "");
 
         if (repoUrl.contains("gitlab")) {
@@ -105,7 +111,11 @@ public String getRepoUrl() throws IOException {
         } else {
             repoUrl = repoUrl + "/blob/" + getCurrentCommitHash() + "/";
         }
-        return repoUrl;
+
+        // Keep only RFC 3986 URL characters: drops every char that is markup- or
+        // JS-significant outside a URL (space, ", ', <, >, `, {, }, \), so the value
+        // cannot break out of an attribute or a <script> template literal downstream.
+        return repoUrl.replaceAll("[^A-Za-z0-9._~:/?#\\[\\]@!$&'()*+,;=%-]", "");
     }
 
     // log --follow implementation may be worth adopting in the future

change-proneness-ranker/src/test/java/org/hjug/git/GitLogReaderGetRepoUrlTest.java

@@ -1,6 +1,7 @@
 package org.hjug.git;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
 import java.io.File;
@@ -100,4 +101,128 @@ void testGetRepoUrlWithNoOrigin() throws Exception {
             assertEquals("", repoUrl);
         }
     }
+
+    @Test
+    void testGetRepoUrl_returnsEmpty_whenOriginUrlIsJavascriptScheme() throws Exception {
+        git.remoteAdd()
+                .setName("origin")
+                .setUri(new URIish("javascript:alert(1)"))
+                .call();
+
+        try (GitLogReader gitLogReader = new GitLogReader(projectBaseDir)) {
+            String repoUrl = gitLogReader.getRepoUrl();
+            assertEquals("", repoUrl, "javascript: scheme should be rejected");
+        }
+    }
+
+    @Test
+    void testGetRepoUrl_returnsEmpty_whenOriginUrlIsDataScheme() throws Exception {
+        git.remoteAdd()
+                .setName("origin")
+                .setUri(new URIish("data:text/html,<script>alert(1)</script>"))
+                .call();
+
+        try (GitLogReader gitLogReader = new GitLogReader(projectBaseDir)) {
+            String repoUrl = gitLogReader.getRepoUrl();
+            assertEquals("", repoUrl, "data: scheme should be rejected");
+        }
+    }
+
+    @Test
+    void testGetRepoUrl_returnsEmpty_whenOriginUrlIsFileScheme() throws Exception {
+        git.remoteAdd()
+                .setName("origin")
+                .setUri(new URIish("file:///etc/passwd"))
+                .call();
+
+        try (GitLogReader gitLogReader = new GitLogReader(projectBaseDir)) {
+            String repoUrl = gitLogReader.getRepoUrl();
+            assertEquals("", repoUrl, "file: scheme should be rejected");
+        }
+    }
+
+    @Test
+    void testGetRepoUrl_returnsEmpty_whenOriginUrlIsNotHttpOrHttps() throws Exception {
+        git.remoteAdd()
+                .setName("origin")
+                .setUri(new URIish("ssh://git@example.com/repo.git"))
+                .call();
+
+        try (GitLogReader gitLogReader = new GitLogReader(projectBaseDir)) {
+            String repoUrl = gitLogReader.getRepoUrl();
+            assertEquals("", repoUrl, "ssh: scheme should be rejected");
+        }
+    }
+
+    @Test
+    void testGetRepoUrl_sanitizesUrl_removingMarkupCharacters() throws Exception {
+        git.getRepository().getConfig().setString("remote", "origin", "url", "https://example.com/repo\"<unsafe>.git");
+        git.getRepository().getConfig().save();
+
+        try (GitLogReader gitLogReader = new GitLogReader(projectBaseDir)) {
+            String repoUrl = gitLogReader.getRepoUrl();
+            assertTrue(repoUrl.startsWith("https://example.com/repounsafe"));
+            assertFalse(repoUrl.contains("\""), "URL must not retain quotes: " + repoUrl);
+            assertFalse(repoUrl.contains("<"), "URL must not retain angle brackets: " + repoUrl);
+        }
+    }
+
+    @Test
+    void testGetRepoUrl_allowsValidHttpsUrl() throws Exception {
+        git.remoteAdd()
+                .setName("origin")
+                .setUri(new URIish("https://github.com/user/repo.git"))
+                .call();
+
+        try (GitLogReader gitLogReader = new GitLogReader(projectBaseDir)) {
+            String repoUrl = gitLogReader.getRepoUrl();
+            assertTrue(
+                    repoUrl.startsWith("https://github.com/user/repo/blob/"),
+                    "Valid HTTPS URL should be allowed: " + repoUrl);
+        }
+    }
+
+    @Test
+    void testGetRepoUrl_allowsValidHttpUrl() throws Exception {
+        git.remoteAdd()
+                .setName("origin")
+                .setUri(new URIish("http://example.com/repo.git"))
+                .call();
+
+        try (GitLogReader gitLogReader = new GitLogReader(projectBaseDir)) {
+            String repoUrl = gitLogReader.getRepoUrl();
+            assertTrue(
+                    repoUrl.startsWith("http://example.com/repo/blob/"),
+                    "Valid HTTP URL should be allowed: " + repoUrl);
+        }
+    }
+
+    @Test
+    void testGetRepoUrl_stripsGitSuffix() throws Exception {
+        git.remoteAdd()
+                .setName("origin")
+                .setUri(new URIish("https://github.com/user/repo.git"))
+                .call();
+
+        try (GitLogReader gitLogReader = new GitLogReader(projectBaseDir)) {
+            String repoUrl = gitLogReader.getRepoUrl();
+            assertTrue(!repoUrl.contains(".git"), "URL should not contain .git suffix");
+        }
+    }
+
+    @Test
+    void testGetRepoUrl_appendsBlobPath_forNonGithubHosts() throws Exception {
+        git.remoteAdd()
+                .setName("origin")
+                .setUri(new URIish("https://example.com/user/repo.git"))
+                .call();
+
+        try (GitLogReader gitLogReader = new GitLogReader(projectBaseDir)) {
+            String repoUrl = gitLogReader.getRepoUrl();
+            String commitHash = git.log().call().iterator().next().getName();
+            assertTrue(
+                    repoUrl.endsWith("/blob/" + commitHash + "/"),
+                    "Non-GitHub hosts should get /blob/ path: " + repoUrl);
+        }
+    }
 }

cli/src/main/java/org/hjug/refactorfirst/ReportCommand.java

@@ -11,6 +11,7 @@
 import org.apache.maven.project.MavenProject;
 import org.hjug.refactorfirst.report.CsvReport;
 import org.hjug.refactorfirst.report.HtmlReport;
+import org.hjug.refactorfirst.report.ReportWriter;
 import org.hjug.refactorfirst.report.SimpleHtmlReport;
 import org.hjug.refactorfirst.report.json.JsonReportExecutor;
 import picocli.CommandLine.Command;
@@ -92,46 +93,52 @@ public Integer call() {
         // TODO: add support for inferring arguments from gradle properties
         inferArgumentsFromMavenProject();
         populateDefaultArguments();
-        switch (reportType) {
-            case SIMPLE_HTML:
-                SimpleHtmlReport simpleHtmlReport = new SimpleHtmlReport();
-                simpleHtmlReport.execute(
-                        backEdgeAnalysisCount,
-                        analyzeCycles,
-                        showDetails,
-                        minifiyHtml,
-                        excludeTests,
-                        testSourceDirectory,
-                        projectName,
-                        projectVersion,
-                        baseDir,
-                        outputDirectory);
-                return 0;
-            case HTML:
-                HtmlReport htmlReport = new HtmlReport();
-                htmlReport.execute(
-                        backEdgeAnalysisCount,
-                        analyzeCycles,
-                        showDetails,
-                        minifiyHtml,
-                        excludeTests,
-                        testSourceDirectory,
-                        projectName,
-                        projectVersion,
-                        baseDir,
-                        outputDirectory);
-                return 0;
-            case JSON:
-                JsonReportExecutor jsonReportExecutor = new JsonReportExecutor();
-                jsonReportExecutor.execute(baseDir, outputDirectory);
-                return 0;
-            case CSV:
-                CsvReport csvReport = new CsvReport();
-                csvReport.execute(showDetails, projectName, projectVersion, outputDirectory, baseDir);
-                return 0;
+        try {
+            outputDirectory = ReportWriter.containReportDirectory(baseDir, outputDirectory);
+            switch (reportType) {
+                case SIMPLE_HTML:
+                    SimpleHtmlReport simpleHtmlReport = new SimpleHtmlReport();
+                    simpleHtmlReport.execute(
+                            backEdgeAnalysisCount,
+                            analyzeCycles,
+                            showDetails,
+                            minifiyHtml,
+                            excludeTests,
+                            testSourceDirectory,
+                            projectName,
+                            projectVersion,
+                            baseDir,
+                            outputDirectory);
+                    return 0;
+                case HTML:
+                    HtmlReport htmlReport = new HtmlReport();
+                    htmlReport.execute(
+                            backEdgeAnalysisCount,
+                            analyzeCycles,
+                            showDetails,
+                            minifiyHtml,
+                            excludeTests,
+                            testSourceDirectory,
+                            projectName,
+                            projectVersion,
+                            baseDir,
+                            outputDirectory);
+                    return 0;
+                case JSON:
+                    JsonReportExecutor jsonReportExecutor = new JsonReportExecutor();
+                    jsonReportExecutor.execute(baseDir, outputDirectory);
+                    return 0;
+                case CSV:
+                    CsvReport csvReport = new CsvReport();
+                    csvReport.execute(showDetails, projectName, projectVersion, outputDirectory, baseDir);
+                    return 0;
+            }
+        } catch (IllegalArgumentException | ReportWriter.ReportWriteException e) {
+            log.error("Report generation failed: {}", e.getMessage());
+            return 1;
         }
 
-        return 0;
+        return 1;
     }
 
     private void populateDefaultArguments() {

cli/src/test/java/org/hjug/refactorfirst/ReportCommandTest.java

@@ -0,0 +1,50 @@
+package org.hjug.refactorfirst;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assumptions.assumeTrue;
+
+import java.lang.reflect.Field;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
+
+class ReportCommandTest {
+
+    @Test
+    void call_returnsFailureWhenOutputPathContainsSymlink(@TempDir Path tempDir) throws Exception {
+        Path target = Files.createDirectory(tempDir.resolve("target"));
+        Path link = tempDir.resolve("link");
+        try {
+            Files.createSymbolicLink(link, target);
+        } catch (UnsupportedOperationException e) {
+            assumeTrue(false, "Symbolic links not supported on this platform");
+        }
+
+        ReportCommand command = new ReportCommand();
+        setField(command, "baseDir", tempDir.toFile());
+        setField(command, "outputDirectory", "link/reports");
+        setField(command, "reportType", ReportType.CSV);
+
+        assertEquals(1, command.call());
+    }
+
+    @Test
+    void call_returnsFailureWhenOutputPathTraversesOutsideBase(@TempDir Path tempDir) throws Exception {
+        Path baseDirectory = Files.createDirectory(tempDir.resolve("project"));
+        ReportCommand command = new ReportCommand();
+        setField(command, "baseDir", baseDirectory.toFile());
+        setField(command, "outputDirectory", "../escape");
+        setField(command, "reportType", ReportType.CSV);
+
+        assertEquals(1, command.call());
+        assertFalse(Files.exists(tempDir.resolve("escape")));
+    }
+
+    private static void setField(ReportCommand command, String name, Object value) throws ReflectiveOperationException {
+        Field field = ReportCommand.class.getDeclaredField(name);
+        field.setAccessible(true);
+        field.set(command, value);
+    }
+}

cost-benefit-calculator/src/main/java/org/hjug/cbc/CostBenefitCalculator.java

@@ -5,6 +5,8 @@
 
 import java.io.File;
 import java.io.IOException;
+import java.net.URI;
+import java.net.URISyntaxException;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.*;
@@ -139,11 +141,18 @@ public List<DisharmonyInstance> getClassDisharmonies(CodebaseGraphDTO codebaseGr
 
         List<DisharmonyInstance> instances = raw.stream()
                 .map(d -> {
+                    String filePath = classToSourceFilePathMapping.get(d.getClassName());
+                    if (filePath == null && d.getClassName().contains("$")) {
+                        filePath = classToSourceFilePathMapping.get(
+                                d.getClassName().substring(0, d.getClassName().indexOf("$")));
+                    }
+                    if (filePath == null) {
+                        log.warn("No source file mapping found for class disharmony in class: {}", d.getClassName());
+                    }
                     DisharmonyInstance instance = new DisharmonyInstance(
                             disharmonyType,
                             d.getClassName(),
-                            canonicaliseURIStringForRepoLookup(
-                                    d.getMetrics().getSourceFilePath().replace("\\", "/")),
+                            filePath,
                             d.getMetrics().getPackageName(),
                             null,
                             new ArrayList<>(d.getMetricValues()));
@@ -460,9 +469,27 @@ private String getFileName(RuleViolation violation) {
     }
 
     String canonicaliseURIStringForRepoLookup(String uriString) {
-        if (repositoryPath.startsWith("/") || repositoryPath.startsWith("\\")) {
-            return uriString.replace("file://" + repositoryPath.replace("\\", "/") + "/", "");
+        return canonicaliseURIStringForRepoLookup(repositoryPath, uriString);
+    }
+
+    static String canonicaliseURIStringForRepoLookup(String repositoryPath, String uriString) {
+        try {
+            URI fileUri = new URI(uriString);
+            if (!"file".equalsIgnoreCase(fileUri.getScheme())) {
+                return uriString;
+            }
+            if (fileUri.isOpaque()) {
+                return fileUri.getSchemeSpecificPart().replace("\\", "/");
+            }
+
+            Path repository = Path.of(repositoryPath).toAbsolutePath().normalize();
+            Path file = Path.of(fileUri).toAbsolutePath().normalize();
+            if (file.startsWith(repository)) {
+                return repository.relativize(file).toString().replace("\\", "/");
+            }
+        } catch (IllegalArgumentException | URISyntaxException e) {
+            log.debug("Unable to canonicalize file URI {}", uriString, e);
         }
-        return uriString.replace("file:///" + repositoryPath.replace("\\", "/") + "/", "");
+        return uriString;
     }
 }

cost-benefit-calculator/src/test/java/org/hjug/cbc/CostBenefitCalculatorTest.java

@@ -43,6 +43,13 @@ public void tearDown() {
         repository.close();
     }
 
+    @Test
+    void canonicaliseURIStringForRepoLookup_relativizesAbsoluteUnixPath() {
+        Assertions.assertEquals(
+                "src/Foo.java",
+                CostBenefitCalculator.canonicaliseURIStringForRepoLookup("/tmp/repo", "file:///tmp/repo/src/Foo.java"));
+    }
+
     @Test
     void testCBOViolation() throws IOException, GitAPIException, InterruptedException {
         // Has CBO violation