Skip to content

Exit 1 for an existing output folder that can't be written to [patch] - #165

Open
matt-edmondson wants to merge 3 commits into
mainfrom
fix/162-read-only-output-folder
Open

matt-edmondson wants to merge 3 commits into
mainfrom
fix/162-read-only-output-folder

Conversation

@matt-edmondson

Copy link
Copy Markdown
Contributor

Fixes #162

What changed

  • ProcessDirectory calls a new EnsureWritable right after Directory.CreateDirectory. It opens a dot-named, uniquely named file in the output folder with FileMode.CreateNew and FileOptions.DeleteOnClose. When the folder is read-only, that throws UnauthorizedAccessException or IOException before any icon is decoded. The existing catch in Execute then reports "Argument validation failed" and exits 1, which is the same result as an output folder that can't be created.
  • The probe name starts with a dot, so if a crash ever leaves one behind, a later run that uses the folder as input skips it as metadata.
  • Updated the exit-code docs in the README and CLAUDE.md.

Tests

  • AnExistingReadOnlyOutputFolderExitsWithInvalidArgumentsAndWritesNothing: the output folder is mode 555 and already exists. The test expects exit 1 and nothing written, the probe included. It reports Inconclusive on Windows, and also when the process can still write to the folder (root, or a file system that ignores mode bits).
  • LeavesNoWriteProbeBehindInAWritableOutputFolder: on a normal run, the output folder holds only icon.png.

I proved the read-only test outside root by running the test executable under setpriv --inh-caps=-all --bounding-set=-all, which drops CAP_DAC_OVERRIDE:

  • With the fix: 90 passed, 0 failed, 0 skipped.
  • With the EnsureWritable call removed: the read-only test fails, because Execute returned 2 instead of 1.

Under plain root, the read-only test reports Inconclusive as designed, and the other 89 pass. CI runners are not root, so the test runs there.

I ran the suite locally with -p:NuGetAudit=false. Restore on main currently fails with NU1903 on SixLabors.ImageSharp 3.1.12, which #145 bumps.


Generated by Claude Code

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 #162

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017vfJW4hvLssY2rDFbS3wDM
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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017vfJW4hvLssY2rDFbS3wDM

matt-edmondson commented Oct 31, 2026 •

Copy link
Copy Markdown
Contributor Author

This PR's CI is red because of main, not this change, and the fix is a maintainer decision.

To unblock main and this PR, choose one of these:

  • Add a Six Labors license key as SixLaborsLicenseKey.
  • Suppress this advisory with NuGetAuditSuppress while you decide.
  • Move off ImageSharp.

The tests for this change pass locally on 3.1.12 with the audit turned off. With file-permission overrides dropped (non-root), 90 passed and 0 failed.


Generated by Claude Code

… 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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017vfJW4hvLssY2rDFbS3wDM
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

An existing but read-only output folder exits 2 and fails every file one by one, while an uncreatable one exits 1

2 participants