https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43546 Bug ID: 43546 Summary: Standardize command-line scripts on Getopt::Long::Descriptive for safer, more consistent option handling Initiative type: --- Sponsorship --- status: Product: Koha Version: Main Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Command-line Utilities Assignee: koha-bugs@lists.koha-community.org Reporter: martin.renvoize@openfifth.co.uk QA Contact: testopia@bugs.koha-community.org CC: jake.deery@openfifth.co.uk, robin@catalyst.net.nz Target Milestone: --- Koha's command-line scripts (misc/cronjobs/*.pl and more broadly the roughly 150 scripts under misc/) use plain Getopt::Long, which has a structural gap: its = and : spec modifiers only mean "this flag needs a value if it's used" -- there is no way to express "this flag itself is mandatory." Scripts that need a genuinely required option have to bolt on their own ad hoc check after parsing, for example: GetOptions( 'h|hours=s' => \$hours ); unless ($hours) { print $usage; die "ERROR: No --hours (-h) option defined"; } A survey of misc/cronjobs/ (58 scripts, 43 using GetOptions) found 14 scripts doing this kind of manual required-option check, each with its own bespoke error text, none of it declared anywhere Getopt::Long itself understands. This is a reliability and maintainability gap: Reliability -- a required option silently omitted is only caught by whatever ad hoc check (if any) the script's author remembered to write, which is inconsistent across scripts, easy to miss when writing a new one, and the failure mode for missing checks is scripts running with undef or empty values instead of failing fast. That is particularly risky for unattended cron execution where nobody is watching stdout. Security/robustness -- consistent, declarative validation of required input reduces the chance of an unset or malformed option flowing further into a script (file paths, SQL parameters, shell arguments) before anything checks it. This is defense-in-depth rather than a specific known vulnerability. Maintainability/readability -- option name, description, type, and required-ness end up scattered across the GetOptions call, a separate POD block, and a separate manual check, all of which can drift out of sync. A declarative spec keeps them in one place. Modernization -- this is a small, low-risk step toward more structured, self-documenting CLI code, consistent with Koha's general direction of reducing boilerplate and inconsistency across similar scripts. Proposal: Adopt Getopt::Long::Descriptive (pure Perl, no XS, actively maintained, built directly on top of Getopt::Long -- see https://metacpan.org/pod/Getopt::Long::Descriptive) as the standard for new/converted scripts, using its native "required" constraint, for example: my ($opt, $usage) = describe_options( '%c %o', [ 'hours|h=i', "hours since item was returned", { required => 1 } ], ... ); This makes "mandatory option" an explicit, declarative property of the script rather than something reconstructed from Getopt::Long spec syntax or scattered ad hoc checks, and it auto-generates consistent --help/usage text from the same declaration, removing the separately-maintained pod2usage boilerplate found in roughly 32 of the surveyed scripts. Suggested scope (staged, not a single patch): 1. Add Getopt::Long::Descriptive to cpanfile. 2. Document the convention for how new/converted scripts should declare and parse options -- possibly formalized as a small helper alongside Koha::Script, which currently only handles locking/logging/userenv and has no opinion on option parsing. 3. Convert a small number of scripts as a proof of concept, chosen for having a genuinely mandatory option today (e.g. cart_to_shelf.pl's -h/--hours). 4. Convert the remainder opportunistically over time, not as a single mass patch -- this changes each script's help/error output, which is a real (if minor) behavior change worth reviewing script by script. One real-world motivating example: the third-party koha-plugin-crontab plugin (still a work in progress, not recommended for production use yet -- https://github.com/openfifth/koha-plugin-crontab) lets staff schedule these scripts from a UI and tries to surface which options are required in its form. Lacking any core-level signal, it has to either mis-infer this from Getopt::Long spec syntax or fall back to an administrator manually curating the list per install -- a workaround, not a fix. That is a symptom of the underlying gap this bug is about, not the primary motivation for it. Test plan: - For each converted script, confirm --help output is sensible and existing documented options still work. - Confirm omitting a required option now fails with a clear, consistent error before any side effects occur. - Confirm koha-qa.pl passes for converted scripts. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.