Evaluate urfave/cli ArgValidator PR for cli-guard #236

New issue

Open

opened 2026-07-22 05:36:18 +00:00 by coilyco-ops · 0 comments

coilyco-ops commented

2026-07-22 05:36:18 +00:00

Member

Context

Upstream urfave/cli PR 2377 implements the tree-wide argument-validation hook requested in urfave/cli issue 2327:

https://github.com/urfave/cli/pull/2377
https://github.com/urfave/cli/issues/2327

Do not comment on or review the upstream GitHub PR until the GitHub repositories are cleaned up.

Questions to resolve

Does ArgValidator materially strengthen cli-guard, or merely duplicate the existing verb.Wrap validation stage?
Can cli-guard register validation once at the root while retaining leaf-specific ArgsFunc selection, SkipPolicy exceptions, coded errors, rejection auditing, profile evaluation, and action-result auditing?
Should ancestor validators compose instead of the nearest child validator replacing the root? The current replacement behavior can silently remove a tree-wide security policy.
Does the hook expose every value cli-guard must validate? The PR example checks cmd.Args only, which omits normally parsed flag values.
Is the timing correct around typed argument parsing, flag actions, Before, Action, and After? After still runs when validation rejects the invocation.
Is a true inherited around-action middleware still required for the validate, action, audit pipeline?

Current assessment

The PR appears useful as a narrow fail-closed backstop. It does not currently replace verb.Wrap or solve the full middleware need. Existing urfave/cli Before hooks already run across the resolved ancestor chain and already short-circuit Action on error, so the new value is the dedicated validator receiving the resolved leaf command.

Done

Re-read the upstream diff, discussion, and checks after GitHub cleanup.
Decide whether cli-guard should use the hook as its primary validation boundary, an additional backstop, or not at all.
If cli-guard should adopt it, specify the integration shape and the security-claim tests needed before changing the boundary.
Post the resulting review upstream only after the GitHub cleanup gate is cleared.

## Context Upstream urfave/cli PR 2377 implements the tree-wide argument-validation hook requested in urfave/cli issue 2327: https://github.com/urfave/cli/pull/2377 https://github.com/urfave/cli/issues/2327 Do not comment on or review the upstream GitHub PR until the GitHub repositories are cleaned up. ## Questions to resolve * Does ArgValidator materially strengthen cli-guard, or merely duplicate the existing verb.Wrap validation stage? * Can cli-guard register validation once at the root while retaining leaf-specific ArgsFunc selection, SkipPolicy exceptions, coded errors, rejection auditing, profile evaluation, and action-result auditing? * Should ancestor validators compose instead of the nearest child validator replacing the root? The current replacement behavior can silently remove a tree-wide security policy. * Does the hook expose every value cli-guard must validate? The PR example checks cmd.Args only, which omits normally parsed flag values. * Is the timing correct around typed argument parsing, flag actions, Before, Action, and After? After still runs when validation rejects the invocation. * Is a true inherited around-action middleware still required for the validate, action, audit pipeline? ## Current assessment The PR appears useful as a narrow fail-closed backstop. It does not currently replace verb.Wrap or solve the full middleware need. Existing urfave/cli Before hooks already run across the resolved ancestor chain and already short-circuit Action on error, so the new value is the dedicated validator receiving the resolved leaf command. ## Done * Re-read the upstream diff, discussion, and checks after GitHub cleanup. * Decide whether cli-guard should use the hook as its primary validation boundary, an additional backstop, or not at all. * If cli-guard should adopt it, specify the integration shape and the security-claim tests needed before changing the boundary. * Post the resulting review upstream only after the GitHub cleanup gate is cleared.

coilyco-ops added the

autonomy

live-collab

priority

labels

2026-07-22 05:36:18 +00:00

coilyco-ops added

priority

and removed

priority

labels

2026-07-29 06:19:39 +00:00