From a1a6f6493d50c4afe40e09819e661305df8eed32 Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 12 Oct 2026 16:46:29 +0200 Subject: [PATCH] Refuse a plans argument that is not a plan's id (issue 561) Every argument of plans that names a plan (id, wait, stop, close, retry, go) is checked once where the call becomes a command line, against the ids the mesh gives plans (plan-, release-), so a quote from a caller never reaches a command or the store (review 75 on #266). --- cmd/mesh-controller/plan_ids.go | 26 +++++++++++++++++++ cmd/mesh-controller/plan_ids_test.go | 37 ++++++++++++++++++++++++++++ cmd/mesh-controller/seatverbs.go | 3 +++ 3 files changed, 66 insertions(+) create mode 100644 cmd/mesh-controller/plan_ids.go create mode 100644 cmd/mesh-controller/plan_ids_test.go diff --git a/cmd/mesh-controller/plan_ids.go b/cmd/mesh-controller/plan_ids.go new file mode 100644 index 00000000..d9dca230 --- /dev/null +++ b/cmd/mesh-controller/plan_ids.go @@ -0,0 +1,26 @@ +package main + +import ( + "fmt" + "regexp" +) + +// planID is every id the mesh gives a plan: a merge's walk or batch (plan-) or a release +// (release-). +var planID = regexp.MustCompile(`^(plan|release)-[0-9]+$`) + +// planIDArguments are the arguments of plans that name a plan. wait is among them for when it is declared +// (novox/hq issue 229); an argument not declared is refused before this is asked. +var planIDArguments = []string{"id", "wait", "stop", "close", "retry", "go"} + +// plainPlanIDs refuses a plans call naming a plan by anything but a plan's id (novox/hq issue 561): checked +// once, where the call becomes a command line, so nothing else a caller sent reaches a command or the store. +func plainPlanIDs(given map[string]string) error { + for _, arg := range planIDArguments { + if id, named := given[arg]; named && !planID.MatchString(id) { + return fmt.Errorf("plans %s: %q is not a plan's id (plan- or release-, as `plans` "+ + "lists them); nothing was done", arg, id) + } + } + return nil +} diff --git a/cmd/mesh-controller/plan_ids_test.go b/cmd/mesh-controller/plan_ids_test.go new file mode 100644 index 00000000..f49039f4 --- /dev/null +++ b/cmd/mesh-controller/plan_ids_test.go @@ -0,0 +1,37 @@ +package main + +import ( + "strings" + "testing" +) + +// **A plan's id is checked once, where a call becomes a command line** (novox/hq issue 561): every argument of +// plans that names a plan is refused unless it is an id the mesh gives a plan, so a quote never reaches a +// command, nor the store behind it. +func TestAPlanIdThatIsNoPlansIdIsRefusedBeforeACommandStarts(t *testing.T) { + for _, arg := range []string{"id", "stop", "close", "retry", "go"} { + for _, bad := range []string{"x' or '1'='1", "plan-1'", "plan-", "plan-1 2", "batch-1", "PLAN-1"} { + args := map[string]any{arg: bad} + if arg == "stop" || arg == "close" || arg == "go" { + args["why"] = "a test" + } + argv, err := argvFor("plans", args) + if err == nil { + t.Errorf("plans %s %q was composed: %v", arg, bad, argv) + continue + } + if !strings.Contains(err.Error(), "is not a plan's id") { + t.Errorf("plans %s %q was refused for another reason: %v", arg, bad, err) + } + } + for _, good := range []string{"plan-1791813626034291775", "release-1791811938267404408"} { + args := map[string]any{arg: good} + if arg == "stop" || arg == "close" || arg == "go" { + args["why"] = "a test" + } + if _, err := argvFor("plans", args); err != nil { + t.Errorf("plans %s %q was refused: %v", arg, good, err) + } + } + } +} diff --git a/cmd/mesh-controller/seatverbs.go b/cmd/mesh-controller/seatverbs.go index 3e4d1d18..017d9fea 100644 --- a/cmd/mesh-controller/seatverbs.go +++ b/cmd/mesh-controller/seatverbs.go @@ -336,6 +336,9 @@ func (a *verbArguments) commandLine() ([]string, error) { } return argv, nil case "plans": + if err := plainPlanIDs(a.given); err != nil { + return nil, err + } if r := str("repository"); r != "" { argv := []string{"plans", "--what-if", r} if p := str("paths"); p != "" {