#405 Improve path security and error handling

Address GitHub Copilot code review suggestions:

1. Enhanced path traversal security
   - Replace string-based startsWith() check with NIO Path API
   - Use Path.normalize() for more robust path validation
   - Prevent sophisticated path traversal attacks

2. Improved error diagnostics
   - Add detailed context to directory creation failures
   - Include directory existence status and parent permissions
   - Make debugging filesystem issues easier

3. Comprehensive test coverage
   - Add testPathTraversalAttackIsBlocked
   - Add testPathTraversalWithSymlinkStyleAttackIsBlocked
   - Add testEnhancedErrorMessageWhenDirectoryCreationFails
   - Add testEnhancedErrorMessageFormatIsCorrect
   - All 24 tests pass successfully

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

Author: ascheman

htmlSanityCheck-core/src/main/java/org/aim42/htmlsanitycheck/report/JUnitXmlReporter.java

@@ -12,6 +12,7 @@
 import java.io.File;
 import java.io.FileWriter;
 import java.io.IOException;
+import java.nio.file.Path;
 import java.util.UUID;
 
 /************************************************************************
@@ -147,8 +148,12 @@ private File getHierarchicalOutputFile(String name) {
                 File tempPath = new File(outputPath, parentDir.getPath());
                 testOutputDir = tempPath.getCanonicalFile();
 
-                // Verify the canonical path is still under outputPath
-                if (!testOutputDir.getAbsolutePath().startsWith(outputPath.getCanonicalPath())) {
+                // Verify the canonical path is still under outputPath using NIO Path API
+                // This provides better security against path traversal attacks
+                Path normalizedOutputPath = outputPath.getCanonicalFile().toPath().normalize();
+                Path normalizedTestOutputDir = testOutputDir.toPath().normalize();
+
+                if (!normalizedTestOutputDir.startsWith(normalizedOutputPath)) {
                     // Path tries to escape outputPath, so just use outputPath directly
                     testOutputDir = outputPath;
                 }
@@ -162,7 +167,13 @@ private File getHierarchicalOutputFile(String name) {
 
         // Ensure the directory exists
         if (!testOutputDir.exists() && !testOutputDir.mkdirs()) {
-            throw new RuntimeException("Cannot create directory " + testOutputDir); //NOSONAR(S112)
+            StringBuilder errorMsg = new StringBuilder("Cannot create directory: ")
+                .append(testOutputDir.getAbsolutePath());
+            errorMsg.append(" (exists: ").append(testOutputDir.exists())
+                    .append(", parent canWrite: ")
+                    .append(testOutputDir.getParentFile() != null ? testOutputDir.getParentFile().canWrite() : "unknown")
+                    .append(")");
+            throw new RuntimeException(errorMsg.toString()); //NOSONAR(S112)
         }
 
         // Create the test file with a simple, sanitized filename

htmlSanityCheck-core/src/test/groovy/org/aim42/htmlsanitycheck/report/JUnitXmlReporterTest.groovy

@@ -10,6 +10,7 @@ import org.junit.Before
 import org.junit.Test
 
 import static org.junit.Assert.assertEquals
+import static org.junit.Assert.assertNotNull
 import static org.junit.Assert.assertTrue
 
 // see end-of-file for license information
@@ -521,4 +522,144 @@ class JUnitXmlReporterTest {
         File[] allFiles = outputPath.listFiles()
         assertTrue("Should have created at least one file or directory", allFiles != null && allFiles.length > 0)
     }
+
+    @Test
+    void testPathTraversalAttackIsBlocked() {
+        // Given: a malicious path trying to escape the output directory
+        // This simulates a path traversal attack like "../../../etc/passwd"
+        SinglePageResults maliciousPage = new SinglePageResults(
+                "index.html",
+                "../../../malicious/path/index.html",
+                "Malicious Page",
+                1000,
+                new ArrayList<>())
+        PerRunResults runResults = new PerRunResults()
+        runResults.addPageResults(maliciousPage)
+
+        // When: we generate the report in HIERARCHICAL mode
+        new JUnitXmlReporter(runResults, outputPath.absolutePath, Configuration.JunitOutputStyle.HIERARCHICAL)
+                .reportPageSummary(maliciousPage)
+
+        // Then: the file should be created safely within outputPath, not outside it
+        File[] allFiles = outputPath.listFiles()
+        assertTrue("Should have created file or directory", allFiles != null && allFiles.length > 0)
+
+        // Verify no files were created outside outputPath
+        def outputPathCanonical = outputPath.canonicalPath
+        def createdFile = findFirstXmlFile(outputPath)
+        assertNotNull(createdFile)
+
+        // The created file should be within outputPath
+        assertTrue("File should be within output directory",
+                   createdFile.canonicalPath.startsWith(outputPathCanonical))
+    }
+
+    @Test
+    void testPathTraversalWithSymlinkStyleAttackIsBlocked() {
+        // Given: a more sophisticated path traversal attack that tries to bypass simple checks
+        // Example: "validdir/../../escape/test.html" which could bypass startsWith() on strings
+        SinglePageResults sophisticatedAttack = new SinglePageResults(
+                "test.html",
+                "valid/../../../escape/test.html",
+                "Sophisticated Attack",
+                1000,
+                new ArrayList<>())
+        PerRunResults runResults = new PerRunResults()
+        runResults.addPageResults(sophisticatedAttack)
+
+        // When: we generate the report in HIERARCHICAL mode
+        new JUnitXmlReporter(runResults, outputPath.absolutePath, Configuration.JunitOutputStyle.HIERARCHICAL)
+                .reportPageSummary(sophisticatedAttack)
+
+        // Then: verify the file is safely contained
+        def outputPathCanonical = outputPath.canonicalPath
+        def createdFile = findFirstXmlFile(outputPath)
+        assertNotNull(createdFile)
+
+        // Use NIO Path API to verify containment (same method as production code)
+        def normalizedOutputPath = outputPath.canonicalFile.toPath().normalize()
+        def normalizedCreatedPath = createdFile.canonicalFile.toPath().normalize()
+
+        assertTrue("File should be within output directory using NIO Path API",
+                   normalizedCreatedPath.startsWith(normalizedOutputPath))
+    }
+
+    @Test
+    void testEnhancedErrorMessageWhenDirectoryCreationFails() {
+        // Given: a non-existent parent directory that cannot be created
+        // We'll use a path that's invalid on the filesystem
+        File invalidOutputPath = new File("/nonexistent/deeply/nested/path/that/cannot/be/created")
+
+        SinglePageResults page = new SinglePageResults(
+                "test.html",
+                "some/deep/path/test.html",
+                "Test Page",
+                1000,
+                new ArrayList<>())
+        PerRunResults runResults = new PerRunResults()
+        runResults.addPageResults(page)
+
+        // When/Then: directory creation should fail with enhanced error message
+        try {
+            new JUnitXmlReporter(runResults, invalidOutputPath.absolutePath, Configuration.JunitOutputStyle.HIERARCHICAL)
+                    .reportPageSummary(page)
+            fail("Should have thrown RuntimeException for directory creation failure")
+        } catch (RuntimeException e) {
+            // Verify the error message contains diagnostic information
+            String errorMsg = e.message
+            assertTrue("Error message should mention 'Cannot create directory'",
+                       errorMsg.contains("Cannot create directory"))
+            assertTrue("Error message should contain full path",
+                       errorMsg.contains(invalidOutputPath.absolutePath))
+            assertTrue("Error message should include 'exists:' diagnostic",
+                       errorMsg.contains("exists:"))
+            assertTrue("Error message should include 'parent canWrite:' diagnostic",
+                       errorMsg.contains("parent canWrite:"))
+        }
+    }
+
+    @Test
+    void testEnhancedErrorMessageFormatIsCorrect() {
+        // Given: setup that will trigger directory creation failure
+        File readOnlyParent = new File(outputPath, "readonly-parent")
+        readOnlyParent.mkdirs()
+
+        // Try to make it read-only (this may not work on all platforms, especially Windows)
+        boolean madeReadOnly = readOnlyParent.setReadOnly()
+
+        if (!madeReadOnly || readOnlyParent.canWrite()) {
+            // Skip test if we cannot make directory read-only on this platform
+            System.err.println("Skipping testEnhancedErrorMessageFormatIsCorrect - cannot make directory read-only on this platform")
+            return
+        }
+
+        try {
+            SinglePageResults page = new SinglePageResults(
+                    "test.html",
+                    "readonly-parent/subdir/test.html",
+                    "Test Page",
+                    1000,
+                    new ArrayList<>())
+            PerRunResults runResults = new PerRunResults()
+            runResults.addPageResults(page)
+
+            // When: attempting to create subdirectory in read-only parent
+            new JUnitXmlReporter(runResults, outputPath.absolutePath, Configuration.JunitOutputStyle.HIERARCHICAL)
+                    .reportPageSummary(page)
+            fail("Should have thrown RuntimeException")
+        } catch (RuntimeException e) {
+            // Then: error message should have proper format with parentheses
+            String errorMsg = e.message
+            assertTrue("Error message should contain opening parenthesis",
+                       errorMsg.contains("("))
+            assertTrue("Error message should contain closing parenthesis",
+                       errorMsg.contains(")"))
+            // Should have format like: "... (exists: true, parent canWrite: false)"
+            assertTrue("Error message should match expected format pattern",
+                       errorMsg.matches(".*\\(exists: .*, parent canWrite: .*\\).*"))
+        } finally {
+            // Cleanup: restore write permission
+            readOnlyParent.setWritable(true)
+        }
+    }
 }