diff --git a/cmd/mesh-controller/terminal_settings_test.go b/cmd/mesh-controller/terminal_settings_test.go index 6eb7e6a9..a70d4ead 100644 --- a/cmd/mesh-controller/terminal_settings_test.go +++ b/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) + } +} diff --git a/internal/catalogue/setting_in_owner_test.go b/internal/catalogue/setting_in_owner_test.go index e7704639..00affaa6 100644 --- a/internal/catalogue/setting_in_owner_test.go +++ b/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) + } + } +} diff --git a/internal/catalogue/setting_into.go b/internal/catalogue/setting_into.go index 2c91ec63..ee04ee18 100644 --- a/internal/catalogue/setting_into.go +++ b/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 diff --git a/internal/catalogue/terminal_keys.go b/internal/catalogue/terminal_keys.go index cd5bb8e1..d3cf47aa 100644 --- a/internal/catalogue/terminal_keys.go +++ b/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 } diff --git a/testdata/store-tests b/testdata/store-tests index 67d064c8..591ba44c 100644 --- a/testdata/store-tests +++ b/testdata/store-tests @@ -26,3 +26,4 @@ ./cmd/mesh-controller TestAnUnassignThatLeavesItsNodeUnresolvedSaysSoOnceInItsAnswer ./cmd/mesh-controller TestAnUnassignThatLeavesItsNodeResolvedSaysNothingOfIt ./cmd/mesh-controller TestAnAssignThatLeavesItsNodeUnresolvedSaysOnlyThePlansRefusal +./cmd/mesh-controller TestADirectoryOwnersKeyIsRefusedThroughAVerb