Skip to content

Exit 1 for a stray argument instead of silently dropping it [patch] - #168

Open
matt-edmondson wants to merge 1 commit into
mainfrom
fix/167-reject-unbound-arguments
Open

matt-edmondson wants to merge 1 commit into
mainfrom
fix/167-reject-unbound-arguments

Conversation

@matt-edmondson

Copy link
Copy Markdown
Contributor

Fixes #167

What changed

  • Arguments now binds every token that no option claims to a hidden [Value(0)] Unbound property. CommandLineParser 2.9.1 used to drop these tokens without reporting an error.
  • Validate rejects a non-empty Unbound with Unexpected argument(s): <tokens>. Quote paths that contain spaces. Execute then exits 1 before it creates any output folder.
Command Before After
-i w -o my icons -c red writes to ./my, exit 0 exit 1, names icons
-i w -o x -s 64 32 size 64, exit 0 exit 1, names 32
-i w -o x extra exit 0 exit 1, names extra

Tests

ArgumentsTests gains five tests that parse a real command line the way Main does:

  • the unquoted-path case

  • a second size

  • several stray tokens

  • Execute exits 1 and leaves the output folder uncreated

  • a clean command line still validates

  • I removed the Validate check and kept the property: four of the new tests failed. With the check back in, all 25 ArgumentsTests pass.

  • The full suite gives 85 passed and 8 failed. The 8 failures are OutputMatchesTheGoldMaster, which fails the same way on main in this environment because the gold-master PNGs are Git LFS pointers that weren't fetched.

  • Restore on main currently fails with NU1903, a new high-severity advisory against SixLabors.ImageSharp 3.1.12. I ran the suite with -p:NuGetAudit=false. Dependabot PR Bump SixLabors.ImageSharp from 3.1.12 to 3.2.0 #145, which bumps it to 3.2.0, is the fix for that.


Generated by Claude Code

CommandLineParser discards any token that no option binds, so an unquoted
path with a space (`-o my icons`) wrote to ./my and exited 0. Bind the
leftover tokens with a hidden [Value] and reject them in Validate, naming
them and pointing at quoting, before any output folder is created.

Fixes #167

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017vfJW6aQDv4uGsdgeCABoe

Copy link
Copy Markdown
Contributor Author

CI is red, and the cause isn't in this PR. All three ci / .NET / Test on … jobs fail at restore with:

error NU1903: Warning As Error: Package 'SixLabors.ImageSharp' 3.1.12 has a known high severity vulnerability, https://github.com/advisories/GHSA-v76p-62qx-wwq2

This is a newly published advisory against ImageSharp 3.1.12. main (f640ca4) has the same pin, so every IconHelper PR fails restore the same way. This diff doesn't touch packages.

No ready fix exists to port:

  • Dependabot Bump SixLabors.ImageSharp from 3.1.12 to 3.2.0 #145, which bumps to 3.2.0, fails with No Six Labors license found because of the split licence that Directory.Build.props describes.
  • The repo's policy is to suppress each 3.1.x advisory by name, so that a new advisory fails restore. Whether to accept GHSA-v76p-62qx-wwq2 by adding it to that list is a security decision for the maintainer, so I haven't added it here.

Once restore passes on main, this PR needs main merged in and CI re-run. Locally, with the audit off (-p:NuGetAudit=false), the ArgumentsTests pass. The rest is as the PR description says.


Generated by Claude Code

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 unquoted path with a space, or any stray argument, is silently dropped: -o my icons writes to ./my and exits 0

2 participants