From 1e05ab1003b3e959d5d7da7a1a28bb7b79475ab7 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 31 Oct 2026 23:27:40 +0000 Subject: [PATCH 1/3] Exit 1 for an existing output folder that can't be written to [patch] Directory.CreateDirectory is a no-op on a folder that already exists, so a read-only output folder got past the #125 check. Every icon was then decoded and processed only to fail at the save, and the run exited 2 as if the inputs were bad. ProcessDirectory now probes the folder with a dot-named DeleteOnClose file before processing anything, so the existing catch in Execute reports it as "Argument validation failed" and exits 1. Fixes ktsu-dev/IconHelper#162 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017vfJW4hvLssY2rDFbS3wDM --- CLAUDE.md | 8 +-- IconHelper.Test/ProcessDirectoryTests.cs | 69 ++++++++++++++++++++++++ IconHelper/IconHelper.cs | 17 +++++- README.md | 2 +- 4 files changed, 91 insertions(+), 5 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index fd1f059..8534107 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -149,9 +149,11 @@ help text and exited 0. `Run` is a one-line `Environment.Exit` wrapper around `Execute(Arguments)`, which returns the exit code so a whole run is testable. `Execute` also turns an `IOException` or `UnauthorizedAccessException` from -`ProcessDirectory` into exit 1. Only the up-front `Directory.CreateDirectory` and `Directory.GetFiles` -can raise one, since each file's own failures are caught inside the batch, and a folder that cannot be -created or listed used to end the run with a stack trace and exit 134. +`ProcessDirectory` into exit 1. Only the up-front `Directory.CreateDirectory`, the `EnsureWritable` probe +and `Directory.GetFiles` can raise one, since each file's own failures are caught inside the batch, and +a folder that cannot be created or listed used to end the run with a stack trace and exit 134. The probe +writes and deletes a dot-named temp file, because `CreateDirectory` is a no-op on an existing folder: +without it a read-only output folder failed every icon one by one and exited 2. ### Known Rough Edges diff --git a/IconHelper.Test/ProcessDirectoryTests.cs b/IconHelper.Test/ProcessDirectoryTests.cs index 9a7ddf3..ec16a57 100644 --- a/IconHelper.Test/ProcessDirectoryTests.cs +++ b/IconHelper.Test/ProcessDirectoryTests.cs @@ -228,6 +228,75 @@ public void AnOutputFolderUnderAFileExitsWithInvalidArgumentsInsteadOfThrowing() Assert.AreEqual(IconHelper.ExitInvalidArguments, IconHelper.Execute(ArgumentsFor(input, output))); } + [TestMethod] + public void AnExistingReadOnlyOutputFolderExitsWithInvalidArgumentsAndWritesNothing() + { + // Mode bits are POSIX; Windows read-only folders are an attribute Explorer itself ignores. + if (OperatingSystem.IsWindows()) + { + Assert.Inconclusive("Read-only folder modes are POSIX only."); + return; + } + + using TempDirectory temp = new(); + string input = temp.Combine("in"); + string output = temp.Combine("out"); + Directory.CreateDirectory(input); + Directory.CreateDirectory(output); + WritePng(Path.Join(input, "a.png"), 64); + WritePng(Path.Join(input, "b.png"), 64); + + UnixFileMode original = File.GetUnixFileMode(output); + File.SetUnixFileMode(output, UnixFileMode.UserRead | UnixFileMode.UserExecute | UnixFileMode.GroupRead | UnixFileMode.GroupExecute | UnixFileMode.OtherRead | UnixFileMode.OtherExecute); + try + { + if (CanCreateAFileIn(output)) + { + // Root, and some container file systems, ignore mode bits, so there is nothing to test. + Assert.Inconclusive("This process can write to a mode 555 folder, so the folder is not read-only to it."); + } + + Assert.AreEqual(IconHelper.ExitInvalidArguments, IconHelper.Execute(ArgumentsFor(input, output))); + Assert.IsEmpty(Directory.GetFileSystemEntries(output), "Nothing should be written, the write probe included."); + } + finally + { + File.SetUnixFileMode(output, original); + } + } + + [TestMethod] + public void LeavesNoWriteProbeBehindInAWritableOutputFolder() + { + using TempDirectory temp = new(); + string input = temp.Combine("in"); + string output = temp.Combine("out"); + Directory.CreateDirectory(input); + WritePng(Path.Join(input, "icon.png"), 64); + + Assert.AreEqual(IconHelper.ExitSuccess, IconHelper.Execute(ArgumentsFor(input, output))); + CollectionAssert.AreEqual(new[] { Path.Join(output, "icon.png") }, Directory.GetFileSystemEntries(output)); + } + + private static bool CanCreateAFileIn(string directory) + { + string probe = Path.Join(directory, $"probe-{Guid.NewGuid():N}"); + try + { + File.WriteAllBytes(probe, []); + File.Delete(probe); + return true; + } + catch (UnauthorizedAccessException) + { + return false; + } + catch (IOException) + { + return false; + } + } + [TestMethod] public void ExecuteReportsInvalidArgumentsForAMissingInput() { diff --git a/IconHelper/IconHelper.cs b/IconHelper/IconHelper.cs index eceabea..c8896a0 100644 --- a/IconHelper/IconHelper.cs +++ b/IconHelper/IconHelper.cs @@ -89,7 +89,7 @@ internal static int Execute(Arguments args) catch (Exception exception) when (exception is IOException or UnauthorizedAccessException) { // Validate can only check that the paths are well formed. Whether the output folder can be - // created, or the input folder listed, is known only on trying, and both happen before the + // created and written to, or the input folder listed, is known only on trying, and both happen before the // per-file catch, so without this an unusable folder ended the run with a stack trace and // exit 134. Each file's own failures never reach here: ProcessDirectory catches those. Console.WriteLine($"Argument validation failed:\n\t{exception.Message}"); @@ -128,6 +128,7 @@ internal static BatchResult ProcessDirectory(Arguments args, Color color) } Directory.CreateDirectory(outputDirectory); + EnsureWritable(outputDirectory); int processed = 0; int failed = 0; @@ -188,6 +189,20 @@ internal static BatchResult ProcessDirectory(Arguments args, Color color) return new BatchResult(processed, failed); } + /// + /// Throws the or that writing + /// an icon would, if the output folder cannot take a new file. CreateDirectory is a no-op on a + /// folder that already exists, so on its own it let a read-only one through, and every icon was + /// then decoded and processed only to fail at the save, exiting 2 as if the inputs were bad. The + /// probe is dot-named, so even a copy left behind by a crash is skipped as metadata. + /// + private static void EnsureWritable(AbsoluteDirectoryPath outputDirectory) + { + FileName probeName = FileName.Create($".iconhelper-write-probe-{Guid.NewGuid():N}"); + AbsoluteFilePath probePath = outputDirectory / probeName; + using FileStream probe = new(probePath, FileMode.CreateNew, FileAccess.Write, FileShare.None, bufferSize: 1, FileOptions.DeleteOnClose); + } + /// /// Names that Finder and Explorer drop into any folder they open: dot-files such as .DS_Store and /// ._ AppleDouble files, plus Thumbs.db, ehthumbs.db and desktop.ini. They are not icons, so they diff --git a/README.md b/README.md index c5ed48e..78bd0a7 100644 --- a/README.md +++ b/README.md @@ -284,7 +284,7 @@ already contains its own output will not reprocess those files. | Code | Meaning | |------|---------| | `0` | Every file was processed successfully. Also returned for `--help` and `--version` | -| `1` | The arguments were unusable, for example an `--input` directory that does not exist, an unrecognised `--color`, or `padding * 2 >= size` | +| `1` | The arguments were unusable, for example an `--input` directory that does not exist, an `--output` directory that cannot be created or written to, an unrecognised `--color`, or `padding * 2 >= size` | | `2` | The batch ran to completion but at least one file could not be processed | Code `2` means the run finished and the remaining icons were still written. Check the summary line From 666ad9915a70e4c0b6b55e3ec3a523b9e08eca49 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 31 Oct 2026 23:31:36 +0000 Subject: [PATCH 2/3] Bump SixLabors.ImageSharp to 3.2.0 so restore passes NU1903 3.1.12 carries GHSA-v76p-62qx-wwq2, and NuGet audit warnings are errors, so restore fails on main and on every PR. Ported from #145. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017vfJW4hvLssY2rDFbS3wDM --- Directory.Packages.props | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Directory.Packages.props b/Directory.Packages.props index 3e701c9..f94954b 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -5,7 +5,7 @@ - + From 0991902d5acedd529c544fc80f255b9deec2e0e3 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 31 Oct 2026 23:33:13 +0000 Subject: [PATCH 3/3] Revert the ImageSharp 3.2.0 bump: 3.2.0 fails CI without a Six Labors license 3.2.0 is the only release that fixes GHSA-v76p-62qx-wwq2, but its build targets fail any CI build that has no SixLaborsLicenseKey. Picking a license, an audit suppression or another library is a maintainer decision, so this PR goes back to main's dependency set. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017vfJW4hvLssY2rDFbS3wDM --- Directory.Packages.props | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Directory.Packages.props b/Directory.Packages.props index f94954b..3e701c9 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -5,7 +5,7 @@ - +