From d3d40acea0a6baffc4ac7fadb59506be689fa7fa Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Tue, 3 Nov 2026 12:48:52 +0000 Subject: [PATCH] Record shell completion audit findings --- docs/roadmap/remediation.md | 119 +++++++++++++++++++++++++++++++++++- 1 file changed, 118 insertions(+), 1 deletion(-) diff --git a/docs/roadmap/remediation.md b/docs/roadmap/remediation.md index 11f192d..7b439f0 100644 --- a/docs/roadmap/remediation.md +++ b/docs/roadmap/remediation.md @@ -1,4 +1,121 @@ -# Invocation reporting audit remediation +# Shell completion audit remediation + +Status: five findings open. The completed invocation reporting audit is retained +below as historical evidence. + +## Completion review scope + +Reviewed `10a799a..0450ecf` against the [completion roadmap](completion.md) +and [implementation plan](implementation.md). Core captures static hints, +resolves entered routes against the installed snapshot, and chooses candidates +without dispatching the target. The internal Cobra adapter generates the scripts +and translates replies; optional modules contribute hints through core metadata. +The opt-in installation, hidden dispatch, and reporting boundary are in place. +The principal gaps are in the transport and its public compatibility effects. + +Evidence includes source review, a temporary public consumer using the maintained +fixture's shape, interactive shell reproductions, and a compile probe. No +implementation changes were made in this review. + +### 1. Keep shell completion data out of Zsh evaluation + +Priority: P1. Status: open. + +[Zsh script adjustment](../../internal/cobra/completion.go#L146) leaves the +generator's `eval _describe ... $flagPrefix` in place. The generator builds +`flagPrefix` from a regex match on the current word. Thus a quoted literal value +can become executable shell source when the operator presses Tab. In an +interactive Zsh session with the generated script, typing +`probe run --format '-$(touch AUDIT_TAB_ONLY)=x'` and pressing Tab, **without +pressing Enter**, created the sentinel in a disposable directory. The value was +being entered for a separate flag, so it did not even have an attached prefix. + +Replace evaluation of word-derived text with argument-safe calls. Pass prefix +options as separate arguments and use the actual parsed value context. Review +other retained script evaluation paths for the same data/code boundary, including +query argv construction; preserve ordinary shell completion semantics without +executing literal data. Add an interactive regression that presses Tab without +Enter and verifies no side effect, with quoted and escaped input. + +### 2. Use parser-derived replacement context in every shell + +Priority: P2. Status: open. + +[Core interpretation](../../completion_flags.go) correctly prioritizes a +pending value and honors `--`. But [reply encoding and script +adjustments](../../internal/cobra/completion.go) omit explicit prefix metadata +for equals forms and let the scripts infer an attached flag from the word's +shape. That inference cannot distinguish an option from a separate value or +operand that happens to contain a dash and equals sign. + +Reproduced with a static choice `--tag=chosen` on the value flag `--format`. +For `probe run --format --tag=ch`, the hidden query returned the correct +literal choice. Bash and Zsh left `--tag=ch` unchanged; Fish inserted +`--tag=--tag=chosen`. These are edited invocation results, not merely candidate +list differences. + +Carry the parser's replacement context to the shell, including an explicit +distinction between whole-word replacement and replacement after an attached +prefix. Account for Bash's native word-break handling without treating it as +CommandKit grammar. Exercise actual edited argv for separate option-looking +values, operands after `--`, and real attached flags, including aliases and path +hints. Coordinate with finding 1 so the same context drives safe shell calls. + +### 3. Separate transport control records from literal candidates + +Priority: P2. Status: open. + +[CompletionReply](../../internal/cobra/completion.go#L19) emits literal choices +in a stream where generated scripts also recognize Cobra's `_activeHelp_ ` +prefix and CommandKit's private short-attachment marker. Control and candidate +records are not distinguished unambiguously. For example, a positional choice +`_activeHelp_ choice` appears in the query response, but interactive Bash +completion of `probe run _act` leaves `_act` unchanged because the script +interprets the choice as a control record. + +Disable control features that CommandKit does not use and use unambiguous +framing or escaping for necessary metadata. Prefer coordinating this work with +finding 2 to avoid another content-dependent prefix heuristic. Preserve +representable literal choices rather than reserving additional public value +prefixes. Test an edited invocation with marker-like values and ensure a +control record cannot appear as a candidate. + +### 4. Preserve comparability of public metadata + +Priority: P2. Status: open. + +[CompletionHint](../../completion.go#L62) embeds a slice by value. Adding it +to [FlagMeta and ArgumentMeta](../../command.go#L93) makes both existing exported +types noncomparable, even when an application never opts into completion. +A public compile probe now rejects both `map[commandkit.FlagMeta]int{}` and +`commandkit.ArgumentMeta{} == commandkit.ArgumentMeta{}`. Before the sprint +these structs contained only comparable fields. The chosen metadata field +addition does not require this extra compatibility break. + +Keep the hint representation comparable, for example with a private pointer to +immutable copied choice data. Preserve copying and the zero hint. Verify that +ordinary metadata can again be compared and used as map keys. Do not promise +semantic equality between separately constructed nonzero choice hints. + +### 5. Document the Bash initialization prerequisite + +Priority: P3. Status: open. + +The [CLI instructions](../cli.md#shell-completion) tell operators to source the +generated Bash script without mentioning Bash completion helpers. The +[interactive fixture](../../internal/cobra/testdata/completion_terminal.py) +sources those helpers first. In an otherwise clean interactive Bash, sourcing +only the generated script and completing `probe ru` produced +`_get_comp_words_by_ref: command not found` and left the word uncompleted. +The path branches also use `_filedir`. + +Document how to initialize the Bash completion package before sourcing the +script, and align the maintained examples and validation instructions. There +is no need to bundle a replacement Bash completion library. + +--- + +## Historical invocation reporting audit remediation Status: remediation complete; all 7 findings addressed.