Record shell completion audit findings
This commit is contained in:
@@ -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<TAB>`, 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<TAB>` 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.
|
||||
|
||||
|
||||
Reference in New Issue
Block a user