Protect canonical operand prefixes from aliases
This commit is contained in:
@@ -1,6 +1,6 @@
|
|||||||
# Alias implementation review remediation
|
# Alias implementation review remediation
|
||||||
|
|
||||||
Status: open; R-1 through R-8 require follow-up.
|
Status: in progress; completed issues are marked individually below.
|
||||||
|
|
||||||
## Review scope and evidence
|
## Review scope and evidence
|
||||||
|
|
||||||
@@ -27,7 +27,7 @@ or private representations unnecessarily.
|
|||||||
|
|
||||||
## R-1: Protect prefixes of canonical operand space during installation
|
## R-1: Protect prefixes of canonical operand space during installation
|
||||||
|
|
||||||
Status: open. Priority: P1. Category: canonical command reachability.
|
Status: complete. Priority: P1. Category: canonical command reachability.
|
||||||
|
|
||||||
**Evidence:** [traceCanonicalPath](../../registry.go#L318) uses
|
**Evidence:** [traceCanonicalPath](../../registry.go#L318) uses
|
||||||
[acceptsOperandCount](../../registry.go#L337), which requires the source's entire
|
[acceptsOperandCount](../../registry.go#L337), which requires the source's entire
|
||||||
@@ -58,6 +58,12 @@ side is added later; preserve the preexisting registry on rejection. Include
|
|||||||
multi-required and repeated operands and a valid distinct child/shortcut case
|
multi-required and repeated operands and a valid distinct child/shortcut case
|
||||||
so the correction does not reserve every route beneath any runnable ancestor.
|
so the correction does not reserve every route beneath any runnable ancestor.
|
||||||
|
|
||||||
|
**Completed:** Installation now protects operand prefixes that can be completed
|
||||||
|
by later arguments without requiring the minimum count immediately. New
|
||||||
|
regressions failed before the correction and pass for fixed/repeated operands,
|
||||||
|
construction, both later registration orders, and rollback. Existing child and
|
||||||
|
longer-shortcut cases and the complete offline root suite passed on Go 1.26.8.
|
||||||
|
|
||||||
## R-2: Select canonical routing context before validating operand counts
|
## R-2: Select canonical routing context before validating operand counts
|
||||||
|
|
||||||
Status: open. Priority: P2. Category: unintended parsing restriction and identity.
|
Status: open. Priority: P2. Category: unintended parsing restriction and identity.
|
||||||
|
|||||||
11
registry.go
11
registry.go
@@ -326,7 +326,7 @@ func traceCanonicalPath(roots map[string]*canonicalNode, path commandpath.Path)
|
|||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
operandCount := len(path) - index
|
operandCount := len(path) - index
|
||||||
if current != nil && acceptsOperandCount(current.command, operandCount) {
|
if current != nil && acceptsOperandPrefix(current.command, operandCount) {
|
||||||
return commandAliasTrace{operandOwner: current}
|
return commandAliasTrace{operandOwner: current}
|
||||||
}
|
}
|
||||||
return commandAliasTrace{}
|
return commandAliasTrace{}
|
||||||
@@ -334,14 +334,15 @@ func traceCanonicalPath(roots map[string]*canonicalNode, path commandpath.Path)
|
|||||||
return commandAliasTrace{exact: current}
|
return commandAliasTrace{exact: current}
|
||||||
}
|
}
|
||||||
|
|
||||||
func acceptsOperandCount(command Command, count int) bool {
|
// acceptsOperandPrefix reports whether more operands could complete this prefix.
|
||||||
|
// A source need not already satisfy the command's minimum to intercept it.
|
||||||
|
func acceptsOperandPrefix(command Command, count int) bool {
|
||||||
if command.record == nil || command.record.payload == nil || !command.record.payload.hasRun() {
|
if command.record == nil || command.record.payload == nil || !command.record.payload.hasRun() {
|
||||||
return false
|
return false
|
||||||
}
|
}
|
||||||
arguments := command.record.payload.bridgeArguments()
|
arguments := command.record.payload.bridgeArguments()
|
||||||
minimum, maximum := 0, 0
|
maximum := 0
|
||||||
for _, argument := range arguments {
|
for _, argument := range arguments {
|
||||||
minimum += argument.Min
|
|
||||||
if maximum >= 0 {
|
if maximum >= 0 {
|
||||||
if argument.Max < 0 {
|
if argument.Max < 0 {
|
||||||
maximum = -1
|
maximum = -1
|
||||||
@@ -350,5 +351,5 @@ func acceptsOperandCount(command Command, count int) bool {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
return count >= minimum && (maximum < 0 || count <= maximum)
|
return maximum < 0 || count <= maximum
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -203,6 +203,40 @@ func TestCommandAliasesRejectCanonicalRoutesAndOperands(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestCommandAliasesProtectIncompleteCanonicalOperandPrefixes(t *testing.T) {
|
||||||
|
for _, bounds := range []struct{ min, max int }{{2, 2}, {3, -1}} {
|
||||||
|
t.Run(fmt.Sprintf("%d_%d", bounds.min, bounds.max), func(t *testing.T) {
|
||||||
|
run := runnableWithOperands("run", bounds.min, bounds.max)
|
||||||
|
mapping := Alias("run first", "inspect")
|
||||||
|
if _, err := New(Identity{Name: "Example"}, run, simpleRunnable("inspect"), mapping); err == nil {
|
||||||
|
t.Fatal("construction accepted an alias intercepting an operand prefix")
|
||||||
|
}
|
||||||
|
for _, aliasFirst := range []bool{false, true} {
|
||||||
|
app, err := New(Identity{Name: "Example"}, simpleRunnable("inspect"))
|
||||||
|
if err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
var first, second Declaration = run, mapping
|
||||||
|
if aliasFirst {
|
||||||
|
first, second = second, first
|
||||||
|
}
|
||||||
|
if err := app.Register(first); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
if err := app.Register(second, simpleRunnable("unrelated")); err == nil {
|
||||||
|
t.Fatal("later registration accepted an operand interception")
|
||||||
|
}
|
||||||
|
if app.commands["unrelated"] != nil || (app.commands["run"] != nil) == aliasFirst {
|
||||||
|
t.Fatal("rejected batch changed canonical registration")
|
||||||
|
}
|
||||||
|
if (len(app.routingRules) == 1) != aliasFirst {
|
||||||
|
t.Fatal("rejected batch changed alias registration")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func TestLongerAliasBeneathAlternateSpellingCanUseOperandWords(t *testing.T) {
|
func TestLongerAliasBeneathAlternateSpellingCanUseOperandWords(t *testing.T) {
|
||||||
app, err := New(Identity{Name: "Example"},
|
app, err := New(Identity{Name: "Example"},
|
||||||
runnableWithOperands("run", 1, 1),
|
runnableWithOperands("run", 1, 1),
|
||||||
|
|||||||
Reference in New Issue
Block a user