From 5e63d7513f851964f8b1723fe5861c9d5f600a73 Mon Sep 17 00:00:00 2001 From: Jim Bethancourt Date: Fri, 4 Sep 2026 07:36:49 -0500 Subject: [PATCH 1/3] #207 Suppress CLI creation - Suppress CLI creation - Update ReportWriter so it will work on Windows --- pom.xml | 2 +- .../refactorfirst/report/ReportWriter.java | 203 +++++++++++++----- 2 files changed, 145 insertions(+), 60 deletions(-) diff --git a/pom.xml b/pom.xml index 066a5e2c..708c798f 100644 --- a/pom.xml +++ b/pom.xml @@ -77,7 +77,7 @@ coverage report - cli + diff --git a/report/src/main/java/org/hjug/refactorfirst/report/ReportWriter.java b/report/src/main/java/org/hjug/refactorfirst/report/ReportWriter.java index 881b4f5b..1c851c40 100644 --- a/report/src/main/java/org/hjug/refactorfirst/report/ReportWriter.java +++ b/report/src/main/java/org/hjug/refactorfirst/report/ReportWriter.java @@ -52,17 +52,14 @@ public static void writeReportToDisk( final String reportOutputDirectory, final String filename, final String string) { Path outputDirectory = Path.of(reportOutputDirectory).toAbsolutePath().normalize(); Path reportName = validateFilename(filename); - List> openedDirectories = new ArrayList<>(); try { - SecureDirectoryStream outputStream = openSecureDirectoryPath(outputDirectory, openedDirectories); - writeAtomically(outputStream, reportName, string); + SecureDirectoryOps ops = SecureDirectoryOps.create(outputDirectory); + ops.writeAtomically(reportName, string); log.info("Done! View the report at {}", outputDirectory.resolve(reportName)); } catch (IOException | UnsupportedOperationException e) { log.error("Unable to write report {}", outputDirectory.resolve(reportName), e); throw new ReportWriteException("Unable to write report " + outputDirectory.resolve(reportName), e); - } finally { - closeDirectories(openedDirectories); } } @@ -80,73 +77,161 @@ private static Path validateFilename(String filename) { return reportName; } - private static SecureDirectoryStream openSecureDirectoryPath( - Path outputDirectory, List> openedDirectories) throws IOException { - Path root = outputDirectory.getRoot(); - if (root == null) { - throw new IOException("Report output directory has no filesystem root: " + outputDirectory); + private interface SecureDirectoryOps { + static SecureDirectoryOps create(Path outputDirectory) throws IOException { + // Try to use secure directory streams (Unix-like) + Path current = outputDirectory; + while (current != null) { + if (Files.exists(current)) { + try (DirectoryStream stream = Files.newDirectoryStream(current)) { + if (stream instanceof SecureDirectoryStream) { + return new SecureDirectoryOpsImpl(current, outputDirectory); + } + } catch (IOException | UnsupportedOperationException ignored) { + } + } + current = current.getParent(); + } + // Fallback for Windows + return new FallbackDirectoryOps(outputDirectory); } - DirectoryStream rootStream = Files.newDirectoryStream(root); - openedDirectories.add(rootStream); - SecureDirectoryStream current = asSecureDirectoryStream(rootStream, root); - Path currentPath = root; + void writeAtomically(Path reportName, String content) throws IOException; + } - for (Path component : root.relativize(outputDirectory)) { - SecureDirectoryStream child; + private static final class SecureDirectoryOpsImpl implements SecureDirectoryOps { + private final Path startPath; + private final Path outputDirectory; + private final List> openedDirectories = new ArrayList<>(); + + SecureDirectoryOpsImpl(Path startPath, Path outputDirectory) { + this.startPath = startPath; + this.outputDirectory = outputDirectory; + } + + @Override + public void writeAtomically(Path reportName, String content) throws IOException { + SecureDirectoryStream current = openSecurePath(); try { - child = current.newDirectoryStream(component, NOFOLLOW_LINKS); - } catch (NoSuchFileException e) { - Path directoryToCreate = currentPath.resolve(component); - Files.createDirectory(directoryToCreate); - child = current.newDirectoryStream(component, NOFOLLOW_LINKS); + writeAtomicallySecure(current, reportName, content); + } finally { + closeDirectories(openedDirectories); } - openedDirectories.add(child); - current = child; - currentPath = currentPath.resolve(component); } - return current; - } - @SuppressWarnings("unchecked") - private static SecureDirectoryStream asSecureDirectoryStream(DirectoryStream stream, Path directory) { - if (!(stream instanceof SecureDirectoryStream)) { - throw new UnsupportedOperationException( - "Secure directory operations are unavailable for report output: " + directory); + private SecureDirectoryStream openSecurePath() throws IOException { + DirectoryStream startStream = Files.newDirectoryStream(startPath); + openedDirectories.add(startStream); + SecureDirectoryStream current = asSecureDirectoryStream(startStream, startPath); + Path currentPath = startPath; + + for (Path component : startPath.relativize(outputDirectory)) { + SecureDirectoryStream child; + try { + child = current.newDirectoryStream(component, NOFOLLOW_LINKS); + } catch (NoSuchFileException e) { + Path directoryToCreate = currentPath.resolve(component); + Files.createDirectory(directoryToCreate); + child = current.newDirectoryStream(component, NOFOLLOW_LINKS); + } + openedDirectories.add(child); + current = child; + currentPath = currentPath.resolve(component); + } + return current; } - return (SecureDirectoryStream) stream; - } - private static void writeAtomically(SecureDirectoryStream directory, Path reportName, String content) - throws IOException { - BasicFileAttributeView targetView = - directory.getFileAttributeView(reportName, BasicFileAttributeView.class, NOFOLLOW_LINKS); - try { - BasicFileAttributes attributes = targetView.readAttributes(); - if (attributes.isSymbolicLink() || attributes.isDirectory()) { - throw new IOException("Refusing to replace non-regular report path: " + reportName); + private void writeAtomicallySecure(SecureDirectoryStream directory, Path reportName, String content) + throws IOException { + BasicFileAttributeView targetView = + directory.getFileAttributeView(reportName, BasicFileAttributeView.class, NOFOLLOW_LINKS); + try { + BasicFileAttributes attributes = targetView.readAttributes(); + if (attributes.isSymbolicLink() || attributes.isDirectory()) { + throw new IOException("Refusing to replace non-regular report path: " + reportName); + } + } catch (NoSuchFileException ignored) { + } + + Path temporaryName = Path.of("." + reportName + "." + UUID.randomUUID() + ".tmp"); + Set options = Set.of(CREATE_NEW, WRITE, NOFOLLOW_LINKS); + boolean moved = false; + try { + try (SeekableByteChannel channel = directory.newByteChannel(temporaryName, options); + BufferedWriter writer = new BufferedWriter( + new OutputStreamWriter(Channels.newOutputStream(channel), Charset.defaultCharset()))) { + writer.write(content); + } + directory.move(temporaryName, directory, reportName); + moved = true; + } finally { + if (!moved) { + try { + directory.deleteFile(temporaryName); + } catch (NoSuchFileException ignored) { + } + } } - } catch (NoSuchFileException ignored) { - // The normal first-write case. } - Path temporaryName = Path.of("." + reportName + "." + UUID.randomUUID() + ".tmp"); - Set options = Set.of(CREATE_NEW, WRITE, NOFOLLOW_LINKS); - boolean moved = false; - try { - try (SeekableByteChannel channel = directory.newByteChannel(temporaryName, options); - BufferedWriter writer = new BufferedWriter( - new OutputStreamWriter(Channels.newOutputStream(channel), Charset.defaultCharset()))) { - writer.write(content); + @SuppressWarnings("unchecked") + private static SecureDirectoryStream asSecureDirectoryStream( + DirectoryStream stream, Path directory) { + if (!(stream instanceof SecureDirectoryStream)) { + throw new UnsupportedOperationException( + "Secure directory operations are unavailable for report output: " + directory); } - directory.move(temporaryName, directory, reportName); - moved = true; - } finally { - if (!moved) { - try { - directory.deleteFile(temporaryName); - } catch (NoSuchFileException ignored) { - // Nothing to clean up. + return (SecureDirectoryStream) stream; + } + } + + private static final class FallbackDirectoryOps implements SecureDirectoryOps { + private final Path outputDirectory; + + FallbackDirectoryOps(Path outputDirectory) { + this.outputDirectory = outputDirectory; + } + + @Override + public void writeAtomically(Path reportName, String content) throws IOException { + // Ensure parent directories exist + Files.createDirectories(outputDirectory); + + Path targetFile = outputDirectory.resolve(reportName); + + // Check if target exists and is a symlink or directory + if (Files.exists(targetFile, NOFOLLOW_LINKS)) { + BasicFileAttributes attrs = Files.readAttributes(targetFile, BasicFileAttributes.class, NOFOLLOW_LINKS); + if (attrs.isSymbolicLink() || attrs.isDirectory()) { + throw new IOException("Refusing to replace non-regular report path: " + reportName); + } + } else { + // Verify no symlink in path components + rejectExistingSymbolicLinkComponents(targetFile); + } + + // Write to temporary file then atomically move + Path temporaryName = outputDirectory.resolve("." + reportName + "." + UUID.randomUUID() + ".tmp"); + Set options = Set.of(CREATE_NEW, WRITE); + boolean moved = false; + try { + try (SeekableByteChannel channel = Files.newByteChannel(temporaryName, options); + BufferedWriter writer = new BufferedWriter( + new OutputStreamWriter(Channels.newOutputStream(channel), Charset.defaultCharset()))) { + writer.write(content); + } + // Verify temp file is not a symlink before move + if (Files.isSymbolicLink(temporaryName)) { + throw new IOException("Temporary file is a symbolic link"); + } + Files.move(temporaryName, targetFile, java.nio.file.StandardCopyOption.ATOMIC_MOVE); + moved = true; + } finally { + if (!moved) { + try { + Files.deleteIfExists(temporaryName); + } catch (IOException ignored) { + } } } } From fd706e118f5a53f992edf28b463145fcf993a084 Mon Sep 17 00:00:00 2001 From: "coderabbitai[bot]" <136622811+coderabbitai[bot]@users.noreply.github.com> Date: Fri, 4 Sep 2026 12:51:48 +0000 Subject: [PATCH 2/3] Fix CodeRabbit issues in PR #208 --- .../org/hjug/refactorfirst/report/ReportWriter.java | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/report/src/main/java/org/hjug/refactorfirst/report/ReportWriter.java b/report/src/main/java/org/hjug/refactorfirst/report/ReportWriter.java index 1c851c40..c9d9ece8 100644 --- a/report/src/main/java/org/hjug/refactorfirst/report/ReportWriter.java +++ b/report/src/main/java/org/hjug/refactorfirst/report/ReportWriter.java @@ -111,8 +111,8 @@ private static final class SecureDirectoryOpsImpl implements SecureDirectoryOps @Override public void writeAtomically(Path reportName, String content) throws IOException { - SecureDirectoryStream current = openSecurePath(); try { + SecureDirectoryStream current = openSecurePath(); writeAtomicallySecure(current, reportName, content); } finally { closeDirectories(openedDirectories); @@ -126,6 +126,9 @@ private SecureDirectoryStream openSecurePath() throws IOException { Path currentPath = startPath; for (Path component : startPath.relativize(outputDirectory)) { + if (component.toString().isEmpty()) { + continue; + } SecureDirectoryStream child; try { child = current.newDirectoryStream(component, NOFOLLOW_LINKS); @@ -194,20 +197,18 @@ private static final class FallbackDirectoryOps implements SecureDirectoryOps { @Override public void writeAtomically(Path reportName, String content) throws IOException { + Path targetFile = outputDirectory.resolve(reportName); + rejectExistingSymbolicLinkComponents(targetFile); + // Ensure parent directories exist Files.createDirectories(outputDirectory); - Path targetFile = outputDirectory.resolve(reportName); - // Check if target exists and is a symlink or directory if (Files.exists(targetFile, NOFOLLOW_LINKS)) { BasicFileAttributes attrs = Files.readAttributes(targetFile, BasicFileAttributes.class, NOFOLLOW_LINKS); if (attrs.isSymbolicLink() || attrs.isDirectory()) { throw new IOException("Refusing to replace non-regular report path: " + reportName); } - } else { - // Verify no symlink in path components - rejectExistingSymbolicLinkComponents(targetFile); } // Write to temporary file then atomically move From 62a8d3984f1bf46813bcd9c3d1054ce2d29f8e6b Mon Sep 17 00:00:00 2001 From: "coderabbitai[bot]" <136622811+coderabbitai[bot]@users.noreply.github.com> Date: Fri, 4 Sep 2026 12:52:22 +0000 Subject: [PATCH 3/3] Fix CodeRabbit issues in PR #208 --- .../main/java/org/hjug/refactorfirst/report/ReportWriter.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/report/src/main/java/org/hjug/refactorfirst/report/ReportWriter.java b/report/src/main/java/org/hjug/refactorfirst/report/ReportWriter.java index c9d9ece8..fe0095da 100644 --- a/report/src/main/java/org/hjug/refactorfirst/report/ReportWriter.java +++ b/report/src/main/java/org/hjug/refactorfirst/report/ReportWriter.java @@ -82,7 +82,7 @@ static SecureDirectoryOps create(Path outputDirectory) throws IOException { // Try to use secure directory streams (Unix-like) Path current = outputDirectory; while (current != null) { - if (Files.exists(current)) { + if (Files.exists(current, NOFOLLOW_LINKS) && !Files.isSymbolicLink(current)) { try (DirectoryStream stream = Files.newDirectoryStream(current)) { if (stream instanceof SecureDirectoryStream) { return new SecureDirectoryOpsImpl(current, outputDirectory);