Refuse a placeholder no pass consumes, so a misspelling never reaches a machine as text (issue 231)

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 whatever is left once every pass's own placeholders are set aside: a misspelt
+		// namespace, or a key its namespace cannot take (novox/hq issue 231).
+		problems = append(problems, unconsumedPlaceholders(m.Module, r, filledByAPass, nil)...)
 	}
 	return problems
 }
@@ -548,7 +551,13 @@ func shellCode(modules []Manifest, shell, slot string) string {
 // is filled before the shell's code is, and each is replaced in a single pass over what the holder
 // wrote, so a contributed piece is never scanned again.
 func contributionsInto(resource map[string]any, m Manifest, modules []Manifest, facts map[string]string, with Rendering, caps map[string]bool, unplaced *[]string) error {
-	if problems := append(placeholderProblems(m, resource), seatPlaceholderProblems(m, resource)...); len(problems) > 0 {
+	// **The sweep stands here, after every other pass and before any contributed text is placed**
+	// (novox/hq issue 231): what is left that the mesh did not fill would reach the machine as text,
+	// and contributed shell code, placed below, is the shell's and is never swept (ADR 0204). The
+	// same function the catalogue check runs, over what the passes left.
+	problems := append(placeholderProblems(m, resource), seatPlaceholderProblems(m, resource)...)
+	problems = append(problems, unconsumedPlaceholders(m.Module, resource, nil, leftForLater)...)
+	if len(problems) > 0 {
 		return fmt.Errorf("%s", problems[0])
 	}
 	content, ok := resource["content"].(string)

internal/catalogue/unconsumed_placeholder.go

@@ -0,0 +1,92 @@
+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, and a word in that shape that no
+// pattern matches — `${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. That is the predecessor's failure the namespaced placeholders were meant to end
+// (novox/hq ADR 0164). So after every pass, what is left is swept: a `${<word>:<key>}` whose word is
+// a lower-case token and whose key begins with a letter or a digit is the mesh's shape, and nothing
+// the mesh could still fill.
+//
+// **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; nor does an expansion that holds a space, a brace or a `$`. 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 no pass reads (novox/hq ADR 0204)
+// and which this sweep never sees, because it runs before that code is placed.
+
+// 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{}$]*\}`)
+
+// filledByAPass is every placeholder a pass over a resource consumes, each by the pattern that pass
+// fills with. Judged at the catalogue check, before any of them has run.
+var filledByAPass = []*regexp.Regexp{
+	settingRef, dirRef, accessRef, placeholder, bound, ofPort, ofSeat, ofSeatReach, ofMachine,
+	ofEnvironment, ofShell, ofContribution,
+}
+
+// leftForLater is what composition leaves in a file's content when the sweep runs, on purpose: a
+// secret the node-engine fills from what it alone decrypts (ADR 0086), and the environment, the
+// shell's code and the seats' contributions, which are placed after the sweep so that contributed
+// text is never swept (ADR 0203, ADR 0204, ADR 0212). Each is judged by its own rules
+// (placeholderProblems, seatPlaceholderProblems); anything else still standing was consumed by no
+// pass, in whatever field.
+var leftForLater = []*regexp.Regexp{placeholder, ofEnvironment, ofShell, ofContribution}
+
+// unconsumedPlaceholders is every namespace-shaped placeholder in one resource that none of the
+// given patterns accounts for, named with the module, the resource and the field it stands in.
+// `anywhere` are accounted for in any field; `inContent` only in a file's content.
+func unconsumedPlaceholders(module string, r map[string]any, anywhere, inContent []*regexp.Regexp) []string {
+	var problems []string
+	var sweep func(field string, v any)
+	sweep = func(field string, v any) {
+		switch v := v.(type) {
+		case string:
+			rest := v
+			for _, p := range anywhere {
+				rest = p.ReplaceAllString(rest, "")
+			}
+			if field == "content" {
+				for _, p := range inContent {
+					rest = p.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, or a key its namespace cannot take, 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(placeholderNamespaces, ", ")))
+			}
+		case []any:
+			for i, e := range v {
+				sweep(fmt.Sprintf("%s[%d]", field, i), e)
+			}
+		case map[string]any:
+			for _, k := range sortedKeys(v) {
+				sweep(field+"."+k, v[k])
+			}
+		}
+	}
+	for _, k := range sortedKeys(r) {
+		sweep(k, r[k])
+	}
+	return problems
+}
+
+// placeholderNamespaces is what a refusal lists as the namespaces the mesh fills in a resource.
+var placeholderNamespaces = []string{
+	"${access:…}", "${bound:…}", "${contribution:…}", "${dir:…}", "${environment:…}", "${machine:…}",
+	"${port:…}", "${seat:…}", "${secret:…}", "${setting:…}", "${shell:…}",
+}

internal/catalogue/unconsumed_placeholder_test.go

@@ -0,0 +1,99 @@
+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 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)
+	}
+
+	// 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}\n"}]}`
+	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}"},
+	} {
+		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 + "\n",
+	}}}
+	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)
+	}
+	for _, res := range out {
+		if res["id"] == "speller.rc" && res["content"] != "d="+shellsOwn+"\n" {
+			t.Fatalf("the shell's own syntax did not pass through as written: %q", res["content"])
+		}
+	}
+}
+
+// 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)
+	}
+	for _, res := range out {
+		if res["id"] == "zsh.zshrc" && !strings.Contains(res["content"].(string), "echo ${path:t} ${shel:zsh:first}") {
+			t.Fatalf("the contributed code did not reach the holder's file as written: %q", res["content"])
+		}
+	}
+}