cli icon indicating copy to clipboard operation
cli copied to clipboard

Improve `AllowExtFlags` robustness

Open bbadour opened this issue 1 month ago • 0 comments

Checklist

  • [X] Are you running the latest v3 release?
  • [X] Did you check the manual for your release?
  • [X] Did you perform a search about this feature?

What problem does this solve?

  • cryptic error messages like "syntax error: expected flag.go:234"
  • errors requiring external boolean flags have a value when used
  • avoidable astonishment when AllowExtFlags flags behave differently from identical flags defined using urfave/cli

Recently, while addressing CVE-2025-52881, I found urfave/cli/v3 became a transitive dependency of numerous programs with a large number of dependencies, many of which define flags using the standard golang "flags" package.

The support for external flags in urfave/cli/v3, AllowExtFlags added code to set the value of each external flag to its default value. While it seems reasonable that every flag would accept its default value, that is not always the case.

It's not clear to me that urfave/cli should ever fail in this case. That said, it is possible to identify a tiny subset of flags that are most likely to reject their default value, namely variable flags that use a pointer to a structure as the variable and lazily use the string representation of the zero-value of the structure instead of an empty string as the default. In any case, where urfave/cli does fail, the user would benefit from additional context.

All of the urfave/cli flag types follow a Value+Flag paradigm except extFlag. As a result, an identical flag brought in via AllowExtFlags behaves differently in perhaps subtle ways.

Solution description

Reimplement extFlag using delegation to implement the Value+Flag paradigm, add context to any errors reported when setting the value to the default value, and suppress those errors some or all of the time.

Describe alternatives you've considered

One could insist that all external flags accept the default value, but: 1) the "flags" package explicitly describes the only purpose for the default value as the usage message, and 2) a small change can make a big difference in the transitive dependency footprint introducing many new dependencies some of which might be externally sourced.

bbadour avatar Nov 21 '25 18:11 bbadour