diff --git a/internal/catalogue/declaration.go b/internal/catalogue/declaration.go index 4b191182..ff5be865 100644 --- a/internal/catalogue/declaration.go +++ b/internal/catalogue/declaration.go @@ -911,6 +911,15 @@ func (r Resolution) compose(with Rendering, owner map[string]string, return nil, err } } + // **And every placeholder no pass fills where it stands is refused** (novox/hq issue 231): + // judged here, over the definition as written and before any pass, by the function the + // catalogue check runs — so a manifest registered by an older binary is judged too, and + // what a setting or a binding later puts into a file is its software's text, never swept. + for _, own := range m.Resources { + if problems := unconsumedPlaceholders(m.Module, own); len(problems) > 0 { + return nil, fmt.Errorf("%s", problems[0]) + } + } // Which of this module's files carry a secret, for the rule that a container may not read // one of them as its environment without saying so (ADR 0086, issue 041). diff --git a/internal/catalogue/environment_into.go b/internal/catalogue/environment_into.go index 41128e4a..ed2bbe28 100644 --- a/internal/catalogue/environment_into.go +++ b/internal/catalogue/environment_into.go @@ -226,6 +226,9 @@ func (m Manifest) contributionPlaceholderProblems() []string { for _, r := range m.Resources { problems = append(problems, placeholderProblems(m, r)...) problems = append(problems, seatPlaceholderProblems(m, r)...) + // And every placeholder no pass fills where it stands: a misspelt namespace, a key its + // namespace cannot take, or a field its pass does not read (novox/hq issue 231). + problems = append(problems, unconsumedPlaceholders(m.Module, r)...) } return problems } diff --git a/internal/catalogue/unconsumed_placeholder.go b/internal/catalogue/unconsumed_placeholder.go new file mode 100644 index 00000000..23f81a93 --- /dev/null +++ b/internal/catalogue/unconsumed_placeholder.go @@ -0,0 +1,172 @@ +package catalogue + +import ( + "fmt" + "regexp" + "strings" +) + +// A placeholder no pass consumes is refused, never written out as text (novox/hq issue 231). +// +// Each namespace is filled by its own pass with its own pattern, in the fields that pass reads. A +// word in that shape that no pattern matched — `${machnie:address}`, `${shel:zsh:first}`, a setting +// key no definition could declare such as `${setting:Undeclared}` — was left in the file as it was +// written and reached a machine as a value: the predecessor's failure the namespaced placeholders +// were meant to end (novox/hq ADR 0164). So the definition is swept, field by field, against the +// same table of what each pass fills where. +// +// **The definition, never the values.** Swept as the manifest wrote it, at the catalogue check and +// again at composition before any pass has run: what an operator's setting, a binding or a merged +// JSON setting puts into a file is its own software's text — `${env:HOME}` for Log4j, `${timeout:30}` +// for Spring — and refusing it there would refuse something its author never wrote, on a machine the +// check had passed (novox/hq issue 231, review of mesh-controller #211). +// +// What is refused: +// - a placeholder of a namespace the mesh knows, in a field that namespace's pass does not read: it +// would reach the machine as the same text; +// - any other `${:`, case-insensitively, unless a shell's parameter operator follows +// its colon — `${Machine:address}`, `${machine:.address}`, `${machine: address}`, an unclosed +// `${machine:address` — because no pass's pattern takes it; +// - a namespace-shaped placeholder of a word the mesh does not know: a lower-case word, a colon, and +// a key that begins with a letter or a digit and holds no space, brace or `$`. +// +// **The shell's own syntax is not that shape, and passes.** `${NAME:-…}` has an upper-case word, +// `${(%):-…}` and `${1:-.}` begin with no letter, and a lower-case variable with an operator after its +// colon — `${count:-}`, `${trial:+…}`, `${state:=…}` — has no key that begins with a letter or a digit. +// And an operator — `:-`, `:=`, `:+`, `:?` — after a word the mesh also uses is the shell's too: +// `${PORT:-8080}`, `${SHELL:-/bin/sh}`, `${SECRET:?unset}` and `${dir:-/tmp}` are among the commonest +// lines of a script or an env file, and pass whatever the word's case. +// What the shape does catch is a zsh modifier (`${path:t}`) or a substring (`${where:0:12}`) in a +// resource's own text: shell code of that kind belongs in the module's contributed shell code, which +// is not a resource and is never swept (novox/hq ADR 0204). + +// filler is one namespace's pass: the patterns it fills with, and the fields it reads them in, by +// resource type ("*" for any type). +type filler struct { + namespace string + patterns []*regexp.Regexp + fields map[string][]string + // why, when set, is the rule that keeps the namespace out of every other field, said with a + // refusal of one written there. + why string +} + +// fillers is the table of what each pass fills where — read from the passes themselves: settingInto, +// dirInto, accessInto, intoFile (whose ${secret:…} the node-engine fills), boundInto, portInto, +// seatInto, machineInto and contributionsInto. A pass that comes to read another field adds it here, +// or the sweep refuses the placeholder it would have filled. +var fillers = []filler{ + {namespace: "setting", patterns: []*regexp.Regexp{settingRef}, fields: map[string][]string{"file": {"content"}, "service": {"unit"}}}, + {namespace: "dir", patterns: []*regexp.Regexp{dirRef}, fields: map[string][]string{"*": {"path", "content", "volumes", "env", "env-file"}}}, + {namespace: "access", patterns: []*regexp.Regexp{accessRef}, fields: map[string][]string{"*": {"path", "content", "volumes", "env", "env-file"}}}, + {namespace: "secret", patterns: []*regexp.Regexp{placeholder}, fields: map[string][]string{"file": {"content"}}, + why: "a secret is never filled into an environment variable or any other field: put it in a file the " + + "module declares and mount that (novox/hq ADR 0086)"}, + {namespace: "bound", patterns: []*regexp.Regexp{bound}, fields: map[string][]string{"file": {"content"}}}, + {namespace: "port", patterns: []*regexp.Regexp{ofPort}, fields: map[string][]string{"file": {"content"}, "container": {"env"}, "process": {"env"}}}, + {namespace: "seat", patterns: []*regexp.Regexp{ofSeat, ofSeatReach}, fields: map[string][]string{"file": {"content"}, "container": {"env"}, "process": {"env"}}}, + {namespace: "machine", patterns: []*regexp.Regexp{ofMachine}, fields: map[string][]string{"*": {"path", "owner", "content", "name", "user", "root", "home"}}}, + // Placed in a file's content by their holders, and judged there by their own rules + // (placeholderProblems, seatPlaceholderProblems). + {namespace: "environment", patterns: []*regexp.Regexp{ofEnvironment}, fields: map[string][]string{"*": {"content"}}}, + {namespace: "shell", patterns: []*regexp.Regexp{ofShell}, fields: map[string][]string{"*": {"content"}}}, + {namespace: "contribution", patterns: []*regexp.Regexp{ofContribution}, fields: map[string][]string{"*": {"content"}}}, + // Filled in what a provider serves (consumer_into_serves.go), never in a resource. + {namespace: "consumer", patterns: []*regexp.Regexp{consumerFact}}, +} + +// reads is whether this pass fills a field of a resource of this type. +func (f filler) reads(kind, field string) bool { + return oneOf(f.fields[kind], field) || oneOf(f.fields["*"], field) +} + +// ofKnownNamespace is any `${:` and what follows it up to its brace or the end of +// its line, whatever its case, unless what follows the colon is a shell's parameter operator (`-`, +// `=`, `+`, `?`): what remains of one once every pass's pattern is set aside is a misspelling. +var ofKnownNamespace = regexp.MustCompile(`(?im)\$\{(?:` + strings.Join(namespacesOf(fillers), "|") + + `):(?:[^-=+?}\n][^}\n]*\}?|\}|$)`) + +// namespaceShaped is a placeholder in the mesh's shape: a lower-case word, a colon, and a key that +// begins with a letter or a digit and holds no space, brace or `$`. +var namespaceShaped = regexp.MustCompile(`\$\{[a-z][a-z0-9_-]*:[A-Za-z0-9][^\s{}$]*\}`) + +func namespacesOf(fs []filler) []string { + out := make([]string, len(fs)) + for i, f := range fs { + out[i] = f.namespace + } + return out +} + +// unconsumedPlaceholders is every placeholder in one resource of a definition that no pass fills +// where it stands, each named with the module, the resource, the field and the token. The same +// function at the catalogue check and at composition, over the resource as the manifest wrote it. +func unconsumedPlaceholders(module string, r map[string]any) []string { + kind := fmt.Sprint(r["type"]) + var problems []string + var sweep func(top, field string, v any) + sweep = func(top, field string, v any) { + switch v := v.(type) { + case string: + rest := v + for _, f := range fillers { + for _, p := range f.patterns { + if !f.reads(kind, top) { + for _, token := range p.FindAllString(rest, -1) { + problems = append(problems, fmt.Sprintf( + "%s's resource %v holds %s in its %s, and ${%s:…} is not filled in this field: "+ + "it would reach the machine as that text (novox/hq issue 231)%s", + module, r["id"], token, field, f.namespace, whereFilled(f))) + } + } + rest = p.ReplaceAllString(rest, "") + } + } + for _, token := range ofKnownNamespace.FindAllString(rest, -1) { + problems = append(problems, fmt.Sprintf( + "%s's resource %v holds %s in its %s, which is no placeholder the mesh fills: a "+ + "misspelt key, or a namespace in the wrong case, would reach the machine as that "+ + "text (novox/hq issue 231)", module, r["id"], token, field)) + } + rest = ofKnownNamespace.ReplaceAllString(rest, "") + for _, token := range namespaceShaped.FindAllString(rest, -1) { + problems = append(problems, fmt.Sprintf( + "%s's resource %v holds %s in its %s, and no pass of the mesh fills it: a misspelt "+ + "placeholder would reach the machine as that text (novox/hq issue 231). The mesh "+ + "fills ${%s:…}; the shell's own syntax belongs in the module's shell code (ADR 0204)", + module, r["id"], token, field, strings.Join(namespacesOf(fillers), ":…}, ${"))) + } + case []any: + for i, e := range v { + sweep(top, fmt.Sprintf("%s[%d]", field, i), e) + } + case map[string]any: + for _, k := range sortedKeys(v) { + sweep(top, field+"."+k, v[k]) + } + } + } + for _, k := range sortedKeys(r) { + sweep(k, k, r[k]) + } + return problems +} + +// whereFilled says where a namespace's pass does fill, for a refusal of one written elsewhere. +func whereFilled(f filler) string { + if f.why != "" { + return ". " + strings.ToUpper(f.why[:1]) + f.why[1:] + } + if len(f.fields) == 0 { + return "; it is filled only in what a provider serves" + } + var where []string + for _, kind := range sortedKeys(f.fields) { + of := "a " + kind + "'s" + if kind == "*" { + of = "any resource's" + } + where = append(where, of+" "+strings.Join(f.fields[kind], ", ")) + } + return "; it is filled in " + strings.Join(where, "; ") +} diff --git a/internal/catalogue/unconsumed_placeholder_test.go b/internal/catalogue/unconsumed_placeholder_test.go new file mode 100644 index 00000000..c82df102 --- /dev/null +++ b/internal/catalogue/unconsumed_placeholder_test.go @@ -0,0 +1,159 @@ +package catalogue + +import ( + "strings" + "testing" +) + +// novox/hq issue 231 — a misspelled placeholder is written out as text. +// +// The four placeholders of the issue, in one file: two misspelled namespaces, a setting key no +// definition could declare, and the shell's own syntax. The first three are refused by name, at the +// catalogue check and at composition; the fourth reaches the file as written. +const ( + misspelledShell = "${shel:zsh:first}" + undeclaredSetting = "${setting:Undeclared}" + misspelledMachine = "${machnie:address}" + shellsOwn = "${XDG_CACHE_HOME:-x}" + // The shell's operators after a word the mesh also uses, in any case: the shell's, and passed. + shellsOperators = "${PORT:-8080} ${SHELL:-/bin/sh} ${SECRET:?unset} ${dir:-/tmp}" +) + +// The catalogue check — the strict parse registration runs too — refuses each by name, with the +// module and the field it stands in, and says nothing about the shell's own syntax. +func TestAMisspelledPlaceholderIsRefusedAtTheCheck(t *testing.T) { + raw := `{"module":"speller","version":"1","resources":[{"id":"rc","type":"file","path":"/etc/speller.rc",` + + `"content":"a=` + misspelledShell + `\nb=` + undeclaredSetting + `\nc=` + misspelledMachine + + `\nd=` + shellsOwn + `\n"}]}` + _, err := ParseManifest([]byte(raw)) + if err == nil { + t.Fatal("a file holding three placeholders no pass consumes was accepted") + } + for _, token := range []string{misspelledShell, undeclaredSetting, misspelledMachine} { + if !strings.Contains(err.Error(), "speller's resource rc holds "+token+" in its content") { + t.Errorf("the refusal does not name %s with its module and field: %v", token, err) + } + } + if strings.Contains(err.Error(), "XDG_CACHE_HOME") { + t.Errorf("the shell's own syntax was refused: %v", err) + } + + // A namespace the mesh knows, misspelt in any way its own pattern does not take, and the same + // namespace in a field its pass does not read: refused at the check as at composition. + for _, c := range []struct{ resource, token, field string }{ + {`{"id":"rc","type":"file","path":"/etc/rc","content":"${Machine:address}"}`, "${Machine:address}", "content"}, + {`{"id":"rc","type":"file","path":"/etc/rc","content":"${machine:.address}"}`, "${machine:.address}", "content"}, + {`{"id":"rc","type":"file","path":"/etc/rc","content":"${machine: address}"}`, "${machine: address}", "content"}, + {`{"id":"rc","type":"file","path":"/etc/rc","content":"${Dir:x}"}`, "${Dir:x}", "content"}, + {`{"id":"rc","type":"file","path":"/etc/rc","content":"a=${machine:address\nb=1"}`, "${machine:address", "content"}, + {`{"id":"rc","type":"process","name":"speller","env":{"NAME":"${machine:name}"}}`, "${machine:name}", "env.NAME"}, + } { + raw := `{"module":"speller","version":"1","resources":[` + c.resource + `]}` + _, err := ParseManifest([]byte(raw)) + if err == nil || !strings.Contains(err.Error(), "speller's resource rc holds "+c.token+" in its "+c.field) { + t.Errorf("%s in its %s was accepted at the check, or not named: %v", c.token, c.field, err) + } + } + + // And the shell's syntax alone, beside placeholders every pass knows, is accepted — as is the + // shape a lower-case shell variable takes with an operator after its colon. + raw = `{"module":"speller","version":"1","resources":[{"id":"rc","type":"file","path":"${machine:account-home}/.rc",` + + `"content":"` + shellsOwn + ` ${(%):-%n} ${1:-.} ${count:-} ${trial:+ on trial} ${machine:address} ` + + shellsOperators + `\n"},` + + `{"id":"server","type":"container","name":"server","env":{"PORT":"${PORT:-80}"}}]}` + if _, err := ParseManifest([]byte(raw)); err != nil { + t.Fatalf("the shell's own syntax was refused: %v", err) + } +} + +// Composition refuses the same placeholders in the same words — a manifest the store already holds +// was never parsed by this binary — and a namespace the mesh knows, written in a field its pass does +// not read, is refused there too, because it would reach the machine as the same literal text. +func TestAMisspelledPlaceholderIsRefusedAtComposition(t *testing.T) { + for _, c := range []struct { + field string + r map[string]any + token string + }{ + {"content", map[string]any{"id": "rc", "type": "file", "path": "/etc/speller.rc", "content": "a=" + misspelledShell + "\n"}, misspelledShell}, + {"content", map[string]any{"id": "rc", "type": "file", "path": "/etc/speller.rc", "content": "b=" + undeclaredSetting + "\n"}, undeclaredSetting}, + {"content", map[string]any{"id": "rc", "type": "file", "path": "/etc/speller.rc", "content": "c=" + misspelledMachine + "\n"}, misspelledMachine}, + {"env.NAME", map[string]any{"id": "rc", "type": "process", "name": "speller", "env": map[string]any{"NAME": "${machine:name}"}}, "${machine:name}"}, + {"content", map[string]any{"id": "rc", "type": "file", "path": "/etc/speller.rc", "content": "${Machine:address}"}, "${Machine:address}"}, + {"content", map[string]any{"id": "rc", "type": "file", "path": "/etc/speller.rc", "content": "${machine:.address}"}, "${machine:.address}"}, + {"content", map[string]any{"id": "rc", "type": "file", "path": "/etc/speller.rc", "content": "${machine: address}"}, "${machine: address}"}, + {"content", map[string]any{"id": "rc", "type": "file", "path": "/etc/speller.rc", "content": "${Dir:x}"}, "${Dir:x}"}, + {"content", map[string]any{"id": "rc", "type": "file", "path": "/etc/speller.rc", "content": "a=${machine:address\nb=1"}, "${machine:address"}, + } { + m := Manifest{Module: "speller", Version: "1", Resources: []map[string]any{c.r}} + r := Resolution{Node: "workstation", Account: "op", Modules: []Manifest{m}} + _, err := r.Declaration(Rendering{}) + if err == nil || !strings.Contains(err.Error(), "speller's resource rc holds "+c.token+" in its "+c.field) { + t.Errorf("%s in its %s was composed rather than refused by name: %v", c.token, c.field, err) + } + } + + m := Manifest{Module: "speller", Version: "1", Resources: []map[string]any{ + {"id": "rc", "type": "file", "path": "/etc/speller.rc", "content": "d=" + shellsOwn + " " + shellsOperators + "\n"}, + {"id": "server", "type": "process", "name": "server", "env": map[string]any{"PORT": "${PORT:-80}"}}, + }} + out, err := Resolution{Node: "workstation", Account: "op", Modules: []Manifest{m}}.Declaration(Rendering{}) + if err != nil { + t.Fatalf("the shell's own syntax was refused: %v", err) + } + if got := contentOf(t, out, "speller.rc"); got != "d="+shellsOwn+" "+shellsOperators+"\n" { + t.Fatalf("the shell's own syntax did not pass through as written: %q", got) + } +} + +// contentOf is the content of the composed resource with this id, failing when it was not composed. +func contentOf(t *testing.T, out []map[string]any, id string) string { + t.Helper() + for _, res := range out { + if res["id"] == id { + return plainly(res["content"]) + } + } + t.Fatalf("%s was not composed: %v", id, out) + return "" +} + +// What a value puts into a file is its software's text, not the definition's, and is never swept +// (review of mesh-controller #211): an operator's setting holding `${labels:instance}` filled through +// ${setting:…}, and a JSON setting `${level:upper}` merged into a mergeable file, reach the machine as +// set — software that templates its own configuration (Log4j's `${env:…}`, Spring's `${timeout:30}`) +// is configured exactly this way. +func TestAValueHoldingAPlaceholderShapeComposesUnchanged(t *testing.T) { + m := Manifest{Module: "templater", Version: "1", Resources: []map[string]any{ + {"id": "conf", "type": "file", "path": "/etc/templater.conf", "content": "template=${setting:template}\n"}, + {"id": "json", "type": "file", "path": "/etc/templater.json", "merge": MergeJSON, "content": `{"fmt":"plain"}`}, + }} + settings := SettingsBy{"templater": {{From: "the operator", Values: map[string]any{ + "template": "${labels:instance}", "fmt": "${level:upper}", + }}}} + out, err := Resolution{Node: "workstation", Account: "op", Modules: []Manifest{m}}.Declaration(Rendering{Settings: settings}) + if err != nil { + t.Fatalf("a value holding a placeholder's shape was refused as the definition's: %v", err) + } + if got := contentOf(t, out, "templater.conf"); got != "template=${labels:instance}\n" { + t.Errorf("the setting did not reach the file as set: %q", got) + } + if got := contentOf(t, out, "templater.json"); !strings.Contains(got, `${level:upper}`) { + t.Errorf("the merged setting did not reach the file as set: %q", got) + } +} + +// Contributed shell code is the shell's, and no pass reads it (novox/hq ADR 0204): zsh's own +// `${path:t}` is namespace-shaped, and reaches the holder's file untouched. +func TestContributedShellCodeIsNotSwept(t *testing.T) { + modules := []Manifest{zshHolder(), {Module: "modifier", Shell: []ShellCode{ + {For: "zsh", Slot: "normal", Code: "echo ${path:t} ${shel:zsh:first}"}, + }}} + out, err := Resolution{Node: "workstation", Account: "op", Modules: modules}.Declaration(Rendering{}) + if err != nil { + t.Fatalf("contributed shell code was swept: %v", err) + } + if got := contentOf(t, out, "zsh.zshrc"); !strings.Contains(got, "echo ${path:t} ${shel:zsh:first}") { + t.Fatalf("the contributed code did not reach the holder's file as written: %q", got) + } +}