Stop flag interpretation at unknown flags
This commit is contained in:
@@ -1,6 +1,6 @@
|
|||||||
# Invocation reporting audit remediation
|
# Invocation reporting audit remediation
|
||||||
|
|
||||||
Status: remediation in progress; 6 of 7 findings complete.
|
Status: remediation complete; all 7 findings addressed.
|
||||||
|
|
||||||
## Review scope
|
## Review scope
|
||||||
|
|
||||||
@@ -31,7 +31,8 @@ native reports, and maintained consumers. Representative successful invocations
|
|||||||
covered command and scoped flag aliases with explicit zero and repeated values,
|
covered command and scoped flag aliases with explicit zero and repeated values,
|
||||||
quiet help/version, aggregate configuration YAML, quiet comparison, concrete
|
quiet help/version, aggregate configuration YAML, quiet comparison, concrete
|
||||||
request validation, and offline aggregate smoke testing. Findings 4 and 5 record
|
request validation, and offline aggregate smoke testing. Findings 4 and 5 record
|
||||||
the additional gaps; findings 1–3 remain open and are not duplicated below.
|
the additional gaps; findings 1–3 were still open at that review and were not
|
||||||
|
duplicated below.
|
||||||
|
|
||||||
The algorithmic audit at `ee76b0f` examined the reporting-specific parser passes,
|
The algorithmic audit at `ee76b0f` examined the reporting-specific parser passes,
|
||||||
delivery bookkeeping, deferred storage, native diagnostic partitioning, and
|
delivery bookkeeping, deferred storage, native diagnostic partitioning, and
|
||||||
@@ -444,7 +445,13 @@ storage analysis is sufficient; no benchmark or allocation threshold is proposed
|
|||||||
|
|
||||||
## 7. Stop normalizing an argument tail that parsing cannot reach
|
## 7. Stop normalizing an argument tail that parsing cannot reach
|
||||||
|
|
||||||
Priority: P3. Status: for consideration.
|
Priority: P3. Status: complete.
|
||||||
|
|
||||||
|
- [x] Completed: interpretation returns the first fatal unknown-flag prefix,
|
||||||
|
retaining earlier canonical/alias occurrences and short-cluster members.
|
||||||
|
Temporary slices grow with that prefix; unreachable values and spellings are
|
||||||
|
no longer normalized or copied. Existing error precedence, values, and
|
||||||
|
terminator coverage passes. Validation: root module suite passes.
|
||||||
|
|
||||||
### Finding and complexity
|
### Finding and complexity
|
||||||
|
|
||||||
@@ -500,3 +507,17 @@ the extra work statically. Existing prefix-order, missing-value precedence,
|
|||||||
short-cluster, and terminator/value tests provide relevant behavior coverage if
|
short-cluster, and terminator/value tests provide relevant behavior coverage if
|
||||||
this is selected. Preserve the backend error and consumed occurrence prefix on
|
this is selected. Preserve the backend error and consumed occurrence prefix on
|
||||||
early failure; no timing threshold or real-world performance data is needed.
|
early failure; no timing threshold or real-world performance data is needed.
|
||||||
|
|
||||||
|
## Final remediation validation
|
||||||
|
|
||||||
|
All seven items have completion markers and focused regression evidence. The
|
||||||
|
following checks pass for the assembled changes:
|
||||||
|
|
||||||
|
- `GOTOOLCHAIN=local go test ./...` at the repository root and in each of
|
||||||
|
`integrations/ckconfig` and `integrations/ckprompt`, including maintained examples.
|
||||||
|
- `GOWORK=off GOTOOLCHAIN=local go test ./...` at the repository root.
|
||||||
|
- `GOTOOLCHAIN=local go test -race -run '^TestReporter|^TestHidden' .` at root.
|
||||||
|
- Go formatting, staged/unstaged whitespace review, and local document-link review.
|
||||||
|
|
||||||
|
The ignored local workspace is preserved. No dependency upgrade, release,
|
||||||
|
publication, or change to upstream domain behavior was part of this work.
|
||||||
|
|||||||
@@ -27,9 +27,8 @@ func interpretFlagTokens(argv []string, flags []commandmeta.Flag, aliases []comm
|
|||||||
}
|
}
|
||||||
|
|
||||||
spellings := make(map[string][]string)
|
spellings := make(map[string][]string)
|
||||||
occurrences := make([]commandmeta.FlagOccurrence, 0, len(argv))
|
var occurrences []commandmeta.FlagOccurrence
|
||||||
interpreted := make([]string, 0, len(argv))
|
var interpreted []string
|
||||||
recordingOccurrences := true
|
|
||||||
for index := 0; index < len(argv); {
|
for index := 0; index < len(argv); {
|
||||||
token := argv[index]
|
token := argv[index]
|
||||||
if token == "--" {
|
if token == "--" {
|
||||||
@@ -50,35 +49,27 @@ func interpretFlagTokens(argv []string, flags []commandmeta.Flag, aliases []comm
|
|||||||
}
|
}
|
||||||
if !ok {
|
if !ok {
|
||||||
interpreted = append(interpreted, token)
|
interpreted = append(interpreted, token)
|
||||||
recordingOccurrences = false
|
return interpreted, spellings, occurrences, nil
|
||||||
index++
|
|
||||||
continue
|
|
||||||
}
|
}
|
||||||
spellings[flag.Name] = append(spellings[flag.Name], original)
|
spellings[flag.Name] = append(spellings[flag.Name], original)
|
||||||
if equal >= 0 {
|
if equal >= 0 {
|
||||||
value := body[equal+1:]
|
value := body[equal+1:]
|
||||||
interpreted = append(interpreted, "--"+flag.Name+"="+value)
|
interpreted = append(interpreted, "--"+flag.Name+"="+value)
|
||||||
if recordingOccurrences {
|
occurrences = append(occurrences, commandmeta.FlagOccurrence{Name: flag.Name, Spelling: original, RawValue: value})
|
||||||
occurrences = append(occurrences, commandmeta.FlagOccurrence{Name: flag.Name, Spelling: original, RawValue: value})
|
|
||||||
}
|
|
||||||
index++
|
index++
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
if !flag.Boolean && index+1 < len(argv) {
|
if !flag.Boolean && index+1 < len(argv) {
|
||||||
value := argv[index+1]
|
value := argv[index+1]
|
||||||
interpreted = append(interpreted, "--"+flag.Name+"="+value)
|
interpreted = append(interpreted, "--"+flag.Name+"="+value)
|
||||||
if recordingOccurrences {
|
occurrences = append(occurrences, commandmeta.FlagOccurrence{Name: flag.Name, Spelling: original, RawValue: value})
|
||||||
occurrences = append(occurrences, commandmeta.FlagOccurrence{Name: flag.Name, Spelling: original, RawValue: value})
|
|
||||||
}
|
|
||||||
index += 2
|
index += 2
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
if !flag.Boolean {
|
if !flag.Boolean {
|
||||||
return interpreted, spellings, occurrences, &commandmeta.Failure{Message: fmt.Sprintf("flag needs an argument: %s", original)}
|
return interpreted, spellings, occurrences, &commandmeta.Failure{Message: fmt.Sprintf("flag needs an argument: %s", original)}
|
||||||
}
|
}
|
||||||
if recordingOccurrences {
|
occurrences = append(occurrences, commandmeta.FlagOccurrence{Name: flag.Name, Spelling: original, RawValue: "true"})
|
||||||
occurrences = append(occurrences, commandmeta.FlagOccurrence{Name: flag.Name, Spelling: original, RawValue: "true"})
|
|
||||||
}
|
|
||||||
interpreted = append(interpreted, "--"+flag.Name)
|
interpreted = append(interpreted, "--"+flag.Name)
|
||||||
index++
|
index++
|
||||||
continue
|
continue
|
||||||
@@ -98,8 +89,7 @@ func interpretFlagTokens(argv []string, flags []commandmeta.Flag, aliases []comm
|
|||||||
}
|
}
|
||||||
if !ok {
|
if !ok {
|
||||||
interpreted = append(interpreted, "-"+token[offset:])
|
interpreted = append(interpreted, "-"+token[offset:])
|
||||||
recordingOccurrences = false
|
return interpreted, spellings, occurrences, nil
|
||||||
break
|
|
||||||
}
|
}
|
||||||
spellings[flag.Name] = append(spellings[flag.Name], original)
|
spellings[flag.Name] = append(spellings[flag.Name], original)
|
||||||
suffix := token[offset+1:]
|
suffix := token[offset+1:]
|
||||||
@@ -107,14 +97,10 @@ func interpretFlagTokens(argv []string, flags []commandmeta.Flag, aliases []comm
|
|||||||
if strings.HasPrefix(suffix, "=") {
|
if strings.HasPrefix(suffix, "=") {
|
||||||
value := suffix[1:]
|
value := suffix[1:]
|
||||||
interpreted = append(interpreted, "--"+flag.Name+"="+value)
|
interpreted = append(interpreted, "--"+flag.Name+"="+value)
|
||||||
if recordingOccurrences {
|
occurrences = append(occurrences, commandmeta.FlagOccurrence{Name: flag.Name, Spelling: original, RawValue: value})
|
||||||
occurrences = append(occurrences, commandmeta.FlagOccurrence{Name: flag.Name, Spelling: original, RawValue: value})
|
|
||||||
}
|
|
||||||
break
|
break
|
||||||
}
|
}
|
||||||
if recordingOccurrences {
|
occurrences = append(occurrences, commandmeta.FlagOccurrence{Name: flag.Name, Spelling: original, RawValue: "true"})
|
||||||
occurrences = append(occurrences, commandmeta.FlagOccurrence{Name: flag.Name, Spelling: original, RawValue: "true"})
|
|
||||||
}
|
|
||||||
interpreted = append(interpreted, "--"+flag.Name)
|
interpreted = append(interpreted, "--"+flag.Name)
|
||||||
offset++
|
offset++
|
||||||
continue
|
continue
|
||||||
@@ -125,17 +111,13 @@ func interpretFlagTokens(argv []string, flags []commandmeta.Flag, aliases []comm
|
|||||||
value = value[1:]
|
value = value[1:]
|
||||||
}
|
}
|
||||||
interpreted = append(interpreted, "--"+flag.Name+"="+value)
|
interpreted = append(interpreted, "--"+flag.Name+"="+value)
|
||||||
if recordingOccurrences {
|
occurrences = append(occurrences, commandmeta.FlagOccurrence{Name: flag.Name, Spelling: original, RawValue: value})
|
||||||
occurrences = append(occurrences, commandmeta.FlagOccurrence{Name: flag.Name, Spelling: original, RawValue: value})
|
|
||||||
}
|
|
||||||
break
|
break
|
||||||
}
|
}
|
||||||
if index+1 < len(argv) {
|
if index+1 < len(argv) {
|
||||||
value := argv[index+1]
|
value := argv[index+1]
|
||||||
interpreted = append(interpreted, "--"+flag.Name+"="+value)
|
interpreted = append(interpreted, "--"+flag.Name+"="+value)
|
||||||
if recordingOccurrences {
|
occurrences = append(occurrences, commandmeta.FlagOccurrence{Name: flag.Name, Spelling: original, RawValue: value})
|
||||||
occurrences = append(occurrences, commandmeta.FlagOccurrence{Name: flag.Name, Spelling: original, RawValue: value})
|
|
||||||
}
|
|
||||||
index += 2
|
index += 2
|
||||||
consumedNext = true
|
consumedNext = true
|
||||||
break
|
break
|
||||||
|
|||||||
@@ -264,3 +264,31 @@ func TestParseFailuresRetainEnteredFlagSpellings(t *testing.T) {
|
|||||||
t.Fatalf("earlier parser failure = %#v", result.Failure)
|
t.Fatalf("earlier parser failure = %#v", result.Failure)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestFlagInterpretationStopsAtTheBackendFailurePrefix(t *testing.T) {
|
||||||
|
for _, test := range []struct {
|
||||||
|
name string
|
||||||
|
args []string
|
||||||
|
want []string
|
||||||
|
}{
|
||||||
|
{"long", []string{"--debug", "--unknown"}, []string{"--debug", "--unknown"}},
|
||||||
|
{"short cluster", []string{"-dqz"}, []string{"--debug", "--quiet", "-z"}},
|
||||||
|
} {
|
||||||
|
t.Run(test.name, func(t *testing.T) {
|
||||||
|
// The later value must not be canonicalized or retained in spellings.
|
||||||
|
args := append(append([]string(nil), test.args...), "--message="+strings.Repeat("value", 1024), "--quiet=false")
|
||||||
|
flags := append(commandmeta.ReportingFlags(), commandmeta.Flag{Name: "message"})
|
||||||
|
interpreted, spellings, occurrences, failure := interpretFlagTokens(args, flags, reportingFlagOptions().FlagAliases)
|
||||||
|
if failure != nil || !reflect.DeepEqual(interpreted, test.want) || len(spellings["message"]) != 0 {
|
||||||
|
t.Fatalf("interpreted=%q spellings=%v failure=%v", interpreted, spellings, failure)
|
||||||
|
}
|
||||||
|
if len(occurrences) != len(test.want)-1 || occurrences[0].Name != "debug" {
|
||||||
|
t.Fatalf("consumed occurrences=%#v", occurrences)
|
||||||
|
}
|
||||||
|
result := Parse(reportingFlagTree(), append([]string{"run"}, args...), reportingFlagOptions())
|
||||||
|
if result.Failure == nil || !strings.Contains(result.Failure.Message, "unknown") || !reflect.DeepEqual(result.FlagOccurrences, occurrences) {
|
||||||
|
t.Fatalf("backend result=%#v", result)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user