Keep a directory owner's settings at the controller's terminal (issue 279)

Root chowns a directory to the number its owner's setting names, so a key
an owner asks for must not change through the settings verb: any caller
could have set puid to 0 or to its own uid. TerminalKeys now takes every
${setting:…} in a directory's owner, whatever else asks for it, and the
owner is written as the parsed number so "0001" never reaches the
node-engine as text (review 60).

cmd/mesh-controller/terminal_settings_test.go

@@ -344,3 +344,58 @@ func TestAnyKeyOfAModuleWithATrustedMergeableFileIsRefusedThroughAVerb(t *testin
 		t.Fatalf("a file marked untrusted through the verb: %v", err)
 	}
 }
+
+// Who owns a module's directory is said at the terminal alone (novox/hq issue 279): a directory's owner may name
+// the settings the module runs as, root chowns the folder to them at the next push, and through the settings verb
+// any caller could have set puid to 0 or to its own uid. The key is refused through every verb route even where a
+// file nothing trusts also asks for it, and taken at the terminal; the layer a refused call meets is unchanged.
+func TestADirectoryOwnersKeyIsRefusedThroughAVerb(t *testing.T) {
+	open := aMesh(t)
+	ctx := t.Context()
+	register(t, open, catalogue.Manifest{Module: "sonarr", Version: "1",
+		Resources: []map[string]any{
+			{"id": "recycle-bin", "type": "directory", "mode": "0755", "owner": "${setting:puid}:${setting:pgid}"},
+			{"id": "identity", "type": "file", "path": "/etc/sonarr.env", "mode": "0644", "trusted": false,
+				"content": "PUID=${setting:puid}\nPGID=${setting:pgid}\nX=${setting:x}\n"}}})
+	if err := atTheTerminal(t, "settings", "set", "sonarr", `{"puid":1001,"pgid":2000,"x":0}`,
+		"--node", "anchor"); err != nil {
+		t.Fatalf("the owner at the terminal: %v", err)
+	}
+	if _, err := assign(ctx, open, "anchor", "sonarr"); err != nil {
+		t.Fatal(err)
+	}
+	layer := func() string {
+		t.Helper()
+		values, _, err := open.inventory.Layer(ctx, "anchor", "sonarr")
+		if err != nil {
+			t.Fatal(err)
+		}
+		raw, _ := json.Marshal(values)
+		return string(raw)
+	}
+	kept := layer()
+	refused := func(what string, err error) {
+		t.Helper()
+		if err == nil || !strings.Contains(err.Error(), "controller's terminal") {
+			t.Fatalf("%s: %v", what, err)
+		}
+	}
+	refused("puid 0 through the verb", throughVerb(t, "settings", map[string]any{"module": "sonarr",
+		"node": "anchor", "values": `{"puid":0,"pgid":2000,"x":0}`}))
+	refused("pgid through the verb, mesh-wide", throughVerb(t, "settings", map[string]any{"module": "sonarr",
+		"values": `{"pgid":0}`}))
+	refused("cleared through the verb", throughVerb(t, "settings", map[string]any{"module": "sonarr",
+		"node": "anchor", "clear": "true"}))
+	if err := throughVerb(t, "command", map[string]any{
+		"command": `settings set sonarr '{"puid":0,"pgid":2000,"x":0}' --node anchor`}); err == nil {
+		t.Fatal("the command verb set the owner")
+	}
+	if got := layer(); got != kept {
+		t.Fatalf("a refused call changed the layer: %s, was %s", got, kept)
+	}
+	// Another key, the owner kept, is still the verb's.
+	if err := throughVerb(t, "settings", map[string]any{"module": "sonarr", "node": "anchor",
+		"values": `{"puid":1001,"pgid":2000,"x":1}`}); err != nil {
+		t.Fatalf("another key through the verb, the owner kept: %v", err)
+	}
+}

internal/catalogue/setting_in_owner_test.go

@@ -150,3 +150,32 @@ func TestASettingAnOwnerAsksForIsNotStray(t *testing.T) {
 		t.Errorf("called stray: %s", stray)
 	}
 }
+
+// An owner is written as the number parsed: "0001" and "00" reach the node-engine as 1 and 0, never as text.
+// Uid 0 is allowed, because a key an owner asks for is the terminal's alone.
+func TestAnOwnerSettingIsWrittenAsTheNumber(t *testing.T) {
+	m := recycler()
+	r := copied(m.Resources[0])
+	if err := settingInto(r, []Layer{{From: "ace", Values: map[string]any{"puid": "0001", "pgid": "00"}}}, m.Module); err != nil {
+		t.Fatal(err)
+	}
+	if r["owner"] != "1:0" {
+		t.Fatalf("owned by %q, not 1:0", r["owner"])
+	}
+}
+
+// Every key a directory's owner asks for is the controller's terminal's alone (novox/hq issue 279, issue 339):
+// root chowns the folder to it, so it is terminal-only whatever else asks for it — a file nothing trusts, a
+// default, or nothing at all.
+func TestAnOwnersKeyIsTheTerminals(t *testing.T) {
+	m := recycler()
+	m.Resources[1] = copied(m.Resources[1])
+	m.Resources[1]["trusted"] = false
+	m.Settings = map[string]SettingDeclaration{"pgid": {Kind: KindPreference, Default: "1000", Why: "a test"}}
+	keys := strings.Join(TerminalKeys(m), ",")
+	for _, k := range []string{"puid", "pgid"} {
+		if !strings.Contains(","+keys+",", ","+k+",") {
+			t.Errorf("%s is not the terminal's: %s", k, keys)
+		}
+	}
+}

internal/catalogue/setting_into.go

@@ -172,13 +172,14 @@ var ownerSide = `(?:\$\{setting:[a-z0-9][a-z0-9_.-]*\}|[0-9]+)`
 // ownerWithSetting is the only shape an owner holding a setting may have: user, or user:group.
 var ownerWithSetting = regexp.MustCompile(`^` + ownerSide + `(?::` + ownerSide + `)?$`)
 
-// wholeID is whether a value is a uid or a gid: a whole number below the kernel's "no id" (2^32-1).
-func wholeID(v string) bool {
+// wholeID is a value as a uid or a gid, and whether it is one: a whole number below the kernel's "no id"
+// (2^32-1). Uid 0 is one: a key an owner asks for is the controller's terminal's alone (TerminalKeys).
+func wholeID(v string) (uint64, bool) {
 	if v == "" || strings.TrimLeft(v, "0123456789") != "" {
-		return false
+		return 0, false
 	}
 	n, err := strconv.ParseUint(v, 10, 32)
-	return err == nil && n < 1<<32-1
+	return n, err == nil && n < 1<<32-1
 }
 
 // OwnerSettingProblems refuses, where a manifest is read, a directory whose owner carries a setting
@@ -235,12 +236,14 @@ func settingIntoOwner(resource map[string]any, layers []Layer, module string) er
 				module, resource["id"], key, key, module, key, orNoSettings(layers))}
 		}
 		v := plainly(value)
-		if !wholeID(v) {
+		id, whole := wholeID(v)
+		if !whole {
 			return fmt.Errorf("%s: the setting %q is %q, which cannot own the directory %v: an owner a setting "+
 				"names is a uid or a gid, a whole number, never an account's name or other text (novox/hq issue 279)",
 				module, key, v, resource["id"])
 		}
-		owner = strings.ReplaceAll(owner, "${setting:"+key+"}", v)
+		// Written as the number parsed, so "0001" reaches the node-engine as 1, never as text.
+		owner = strings.ReplaceAll(owner, "${setting:"+key+"}", strconv.FormatUint(id, 10))
 	}
 	resource["owner"] = owner
 	return nil

internal/catalogue/terminal_keys.go

@@ -33,6 +33,9 @@ import (
 //  4. **A module's whole layer, when it has a mergeable file not marked `"trusted": false`** (novox/hq issue 340,
 //     TrustedMergeable). A mergeable file takes any key a layer sets, not only those its content names, so no list
 //     of keys covers it: an empty runtime configuration a provider reads would take a `url` of the caller's.
+//  5. **Every setting a directory's owner asks for** (novox/hq issue 279): root chowns the directory to the number
+//     at the next push, so through a verb any caller could hand itself — or uid 0 — a module's folder. Whatever
+//     else asks for the key, a file marked `"trusted": false` included, the owner makes it the terminal's.
 //
 // Derived from the manifest, never listed by hand, so a provider or a trusted file added tomorrow is covered.
 
@@ -42,8 +45,8 @@ import (
 const TrustedField = "trusted"
 
 // TerminalKeys are the settings keys of a module that are set at the controller's terminal alone: places and
-// accesses, every key its provisions serve and every setting a served value asks for, and every setting a file
-// asks for unless it says `"trusted": false`. Places and accesses first, then the rest sorted.
+// accesses, every key its provisions serve and every setting a served value asks for, every setting a file
+// asks for unless it says `"trusted": false`, and every setting a directory's owner asks for. Places and accesses first, then the rest sorted.
 func TerminalKeys(m Manifest) []string {
 	keys := map[string]bool{}
 	for _, served := range m.Serves {
@@ -57,6 +60,13 @@ func TerminalKeys(m Manifest) []string {
 		}
 	}
 	for _, r := range m.Resources {
+		if fmt.Sprint(r["type"]) == "directory" {
+			owner, _ := r["owner"].(string)
+			for _, asked := range settingsUsed(owner) {
+				keys[asked] = true
+			}
+			continue
+		}
 		if fmt.Sprint(r["type"]) != "file" {
 			continue
 		}

testdata/store-tests

@@ -22,3 +22,4 @@
 ./cmd/mesh-controller TestAWalkEndedByCancelOrClearIsSaidEnded
 ./cmd/mesh-controller TestAWalkRestartedByARebuildIsSaidStarted
 ./cmd/mesh-controller TestAWalkStoppedByHandAndRetriedEndsFailedWhenItFailsAgain
+./cmd/mesh-controller TestADirectoryOwnersKeyIsRefusedThroughAVerb