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/manifest.go b/internal/catalogue/manifest.go index be5d13dd..024d850a 100644 --- a/internal/catalogue/manifest.go +++ b/internal/catalogue/manifest.go @@ -1859,6 +1859,7 @@ func ParseManifest(raw []byte) (Manifest, error) { problems = append(problems, invokeProblems(m)...) problems = append(problems, replacesProblems(m)...) problems = append(problems, UnitSettingProblems(m)...) + problems = append(problems, OwnerSettingProblems(m)...) problems = append(problems, endpointNameProblems(m)...) problems = append(problems, RouteProblems(m)...) for _, port := range m.Guards { diff --git a/internal/catalogue/setting_in_owner_test.go b/internal/catalogue/setting_in_owner_test.go new file mode 100644 index 00000000..00affaa6 --- /dev/null +++ b/internal/catalogue/setting_in_owner_test.go @@ -0,0 +1,181 @@ +package catalogue + +import ( + "strings" + "testing" +) + +// A declared directory may be owned by the account its module runs as (novox/hq issue 279): a media +// manager runs as the puid and pgid its assignment sets per node, and a folder it must write was owned +// by the manifest's literal, so the app refused it on a machine whose identity was not the image's. + +func recycler() Manifest { + return Manifest{Module: "sonarr", + Resources: []map[string]any{ + {"id": "recycle-bin", "type": "directory", "path": "/srv/sonarr/recycle", "mode": "0755", + "owner": "${setting:puid}:${setting:pgid}"}, + {"id": "identity", "type": "file", "path": "/srv/sonarr/identity.env", "mode": "0644", + "content": "PUID=${setting:puid}\nPGID=${setting:pgid}\n"}, + }, + } +} + +func copied(r map[string]any) map[string]any { + out := map[string]any{} + for k, v := range r { + out[k] = v + } + return out +} + +// The node's identity reaches the directory's owner, the node's layer over the mesh's, a number given +// as a string or as a JSON number alike. +func TestASettingOwnsADirectory(t *testing.T) { + m := recycler() + layers := []Layer{ + {From: "the mesh", Values: map[string]any{"puid": "1000", "pgid": "1000"}}, + {From: "this node", Values: map[string]any{"puid": float64(1001), "pgid": "2000"}}, + } + r := copied(m.Resources[0]) + if err := settingInto(r, layers, m.Module); err != nil { + t.Fatal(err) + } + if r["owner"] != "1001:2000" { + t.Fatalf("owned by %q, not 1001:2000", r["owner"]) + } + user := copied(m.Resources[0]) + user["owner"] = "${setting:puid}" + if err := settingInto(user, layers, m.Module); err != nil || user["owner"] != "1001" { + t.Fatalf("user alone: %v, %v", user["owner"], err) + } +} + +// Composed for a machine, the way a push writes it: the directory reaches the node-engine owned by the +// number the node's settings name, never by the placeholder. +func TestAComposedMachineGetsTheOwnerTheSettingNames(t *testing.T) { + r := anAdoptedAnchor() + r.Modules = append(r.Modules, recycler()) + with := anchorRendering(false) + with.Settings["sonarr"] = []Layer{{From: "anchor", Values: map[string]any{"puid": "1001", "pgid": "2000"}}} + composed, err := r.Compose(with) + if err != nil { + t.Fatal(err) + } + if why, left := composed.LeftOut["sonarr"]; left { + t.Fatalf("left out: %s", why) + } + got := byID(composed.Resources)["sonarr.recycle-bin"] + if got["owner"] != "1001:2000" { + t.Fatalf("composed owned by %v, not 1001:2000", got["owner"]) + } +} + +// Only a whole number owns a directory: an account's name, text, a sign, a fraction or a value past the +// kernel's ids is refused by name, and the directory is left as it was. So is a key nothing sets. +func TestAnOwnerSettingThatIsNoNumberIsRefused(t *testing.T) { + m := recycler() + for _, bad := range []any{"abc", "root", "", "-1", "+5", "1000:1000", "10 00", "1000\nx", float64(1.5), + float64(-3), "4294967295", "99999999999", true} { + r := copied(m.Resources[0]) + err := settingInto(r, []Layer{{From: "ace", Values: map[string]any{"puid": bad, "pgid": "1000"}}}, m.Module) + if err == nil || !strings.Contains(err.Error(), `"puid"`) || !strings.Contains(err.Error(), "whole number") { + t.Errorf("%v: %v", bad, err) + } + if r["owner"] != m.Resources[0]["owner"] { + t.Errorf("%v: the owner was changed on refusal: %v", bad, r["owner"]) + } + } + r := copied(m.Resources[0]) + if err := settingInto(r, nil, m.Module); err == nil || !strings.Contains(err.Error(), "${setting:puid}") { + t.Errorf("nothing set: %v", err) + } + if err := JudgeSettings(m, []Layer{{From: "ace", Values: map[string]any{"puid": "media", "pgid": "1000"}}}, false); err == nil || + !strings.Contains(err.Error(), "whole number") { + t.Errorf("a stored name is not refused when judged: %v", err) + } +} + +// Where the manifest is read: an owner holding a setting is a whole placeholder or a whole number on +// each side of the colon, and every key is one the module asks for elsewhere or declares a default for. +func TestAnOwnerSettingIsJudgedWhereTheManifestIsRead(t *testing.T) { + ok := []string{"${setting:puid}:${setting:pgid}", "${setting:puid}", "${setting:puid}:1000", "1000:${setting:pgid}", + "1000:1000", "media:media"} + bad := []string{ + "${setting:puid}x", + "x${setting:puid}", + "${setting:puid}:${setting:pgid}:1", + "${setting:puid}${setting:pgid}", + "media:${setting:pgid}", + "${setting:puid}:", + "${setting:other}:${setting:pgid}", + } + for _, owner := range append(ok, bad...) { + m := recycler() + m.Resources[0] = copied(m.Resources[0]) + m.Resources[0]["owner"] = owner + err := parsed(t, m) + refused := err != nil && strings.Contains(err.Error(), "owner") + want := false + for _, b := range bad { + want = want || b == owner + } + if refused != want { + t.Errorf("%s: refused %v, want %v (%v)", owner, refused, want, err) + } + } + // A key with a default is declared, though nothing else asks for it. + m := recycler() + m.Resources = m.Resources[:1] + m.Resources[0] = copied(m.Resources[0]) + m.Resources[0]["owner"] = "${setting:workers}" + m.Settings = map[string]SettingDeclaration{"workers": {Kind: KindPreference, Default: float64(4), Why: "a test"}} + if err := parsed(t, m); err != nil && strings.Contains(err.Error(), "owner") { + t.Errorf("a declared key was refused: %v", err) + } + // A file's owner takes no setting: the sweep says so. + f := recycler() + f.Resources[1] = copied(f.Resources[1]) + f.Resources[1]["owner"] = "${setting:puid}" + if err := parsed(t, f); err == nil || !strings.Contains(err.Error(), "not filled in this field") { + t.Errorf("a file's owner setting passed: %v", err) + } +} + +// A key a directory's owner asks for is a destination, so setting it is not called stray. +func TestASettingAnOwnerAsksForIsNotStray(t *testing.T) { + m := recycler() + m.Resources = m.Resources[:1] + stray := strings.Join(UnusedSettings(m, []Layer{{From: "ace", Values: map[string]any{"puid": "1001", "pgid": "2000"}}}), "; ") + if strings.Contains(stray, "puid") || strings.Contains(stray, "pgid") { + 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 30967997..ee04ee18 100644 --- a/internal/catalogue/setting_into.go +++ b/internal/catalogue/setting_into.go @@ -4,6 +4,7 @@ import ( "fmt" "regexp" "sort" + "strconv" "strings" ) @@ -15,7 +16,8 @@ import ( // (novox/hq issues 122, 134). ADR 0112 names the operator as one of the four providers; this is the // operator answering. // -// `${setting:}` in a file's content, and in a service's unit name, is filled from the module's settings layers — the mesh's, +// `${setting:}` in a file's content, in a service's unit name and in a directory's owner (a uid or +// gid only, issue 279) is filled from the module's settings layers — the mesh's, // then this node's — the same layers a mergeable JSON file and a contribution already take, so // `settings set ` is the one place a person's values go. **Refused when no layer sets it**, // naming the key and the remedy: a definition that carried a default for a mail domain would be @@ -59,6 +61,9 @@ func settingInto(resource map[string]any, layers []Layer, module string) error { if fmt.Sprint(resource["type"]) == "service" { return settingIntoUnit(resource, layers, module) } + if fmt.Sprint(resource["type"]) == "directory" { + return settingIntoOwner(resource, layers, module) + } if fmt.Sprint(resource["type"]) != "file" { return nil } @@ -143,6 +148,107 @@ func settingIntoUnit(resource map[string]any, layers []Layer, module string) err return nil } +// A declared directory may be owned by the account its module runs as (novox/hq issue 279). +// +// A containerised module runs as the identity its assignment sets per node — its puid and pgid +// settings, written into the environment file its image reads (ADR 0112, issue 153) — and a directory +// its app must write, beyond the one its image re-owns at start, was owned by whatever the manifest +// spelt: right on a machine whose identity matched the image's default, refused by the app on one whose +// did not. So a directory's `owner` may name the settings that identity is: `${setting:}` for the +// user alone, `${setting:}:${setting:}` for both, filled from the same layers at +// composition. +// +// **Whole numbers only.** An owner a setting fills is a uid or a gid — never an account's name, never +// text — so no value can put anything into the ownership the node-engine applies but a number; one that +// is not is refused at composition, naming the key, and the directory is left as it was. The shape is +// judged where the manifest is read (OwnerSettingProblems): each side of the colon a whole placeholder +// or a whole number, and every key one the module itself asks for elsewhere — in a file, a unit, a +// contribution or a served fact — or declares a default for, so the owner follows the identity its +// process is told it runs as, never a key that reaches nothing else. + +// ownerSide is one side of an owner a setting may fill: a whole placeholder or a whole number. +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 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 0, false + } + n, err := strconv.ParseUint(v, 10, 32) + return n, err == nil && n < 1<<32-1 +} + +// OwnerSettingProblems refuses, where a manifest is read, a directory whose owner carries a setting +// in any shape but a whole placeholder or number on each side of the colon, or names a key the module +// asks for nowhere else and declares no default for. +func OwnerSettingProblems(m Manifest) []string { + var problems []string + asked := settingKeysAskedOutsideOwners(m) + for k := range m.Settings { + asked[k] = true + } + for _, r := range m.Resources { + owner, _ := r["owner"].(string) + if !strings.Contains(owner, "${setting:") { + continue + } + if fmt.Sprint(r["type"]) != "directory" { + // Said by the placeholder sweep: only a directory's owner takes a setting. + continue + } + if !ownerWithSetting.MatchString(owner) { + problems = append(problems, fmt.Sprintf("%s: the directory %v's owner %q carries a setting in a shape "+ + "the mesh does not fill; an owner names the account by number only, each side of the colon a whole "+ + "${setting:key} or a whole number (${setting:puid}:${setting:pgid}) (novox/hq issue 279)", + m.Module, r["id"], owner)) + continue + } + for _, key := range settingsUsed(owner) { + if !asked[key] { + problems = append(problems, fmt.Sprintf("%s: the directory %v's owner says ${setting:%s}, and the "+ + "module asks for %q nowhere else and declares no default for it: an owner follows the identity "+ + "the module's process is told it runs as (its identity file's ${setting:%s}), never a key of its "+ + "own (novox/hq issue 279)", m.Module, r["id"], key, key, key)) + } + } + } + return problems +} + +// settingIntoOwner fills ${setting:…} in a directory's owner, refusing — with the key and why, the +// resource left as it was — a key nothing sets and a value that is not a whole number. +func settingIntoOwner(resource map[string]any, layers []Layer, module string) error { + owner, ok := resource["owner"].(string) + if !ok { + return nil + } + for _, key := range settingsUsed(owner) { + value, set := settingValue(layers, key) + if !set { + return &UnsetSettingError{Module: module, Setting: key, said: fmt.Sprintf( + "%s has a directory %v whose owner says ${setting:%s}, and nothing sets %q for it — an operator's "+ + "value is the assignment's, never the definition's (novox/hq ADR 0112): "+ + "`settings set %s ` with {%q: …}%s", + module, resource["id"], key, key, module, key, orNoSettings(layers))} + } + v := plainly(value) + 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"]) + } + // 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 +} + func settingValue(layers []Layer, key string) (any, bool) { var value any set := false @@ -171,9 +277,25 @@ func orNoSettings(layers []Layer) string { return "; set today: " + strings.Join(keys, ", ") } -// settingKeysUsedBy is every key a module's files, contributions and served facts ask for, so a -// setting that lands in one is not called stray. +// settingKeysUsedBy is every key a module's files, units, directory owners, contributions and served +// facts ask for, so a setting that lands in one is not called stray. func settingKeysUsedBy(m Manifest) map[string]bool { + used := settingKeysAskedOutsideOwners(m) + for _, r := range m.Resources { + if fmt.Sprint(r["type"]) == "directory" { + if owner, ok := r["owner"].(string); ok { + for _, k := range settingsUsed(owner) { + used[k] = true + } + } + } + } + return used +} + +// settingKeysAskedOutsideOwners is every key a module's files, units, contributions and served facts +// ask for: what a directory's owner may follow (OwnerSettingProblems). +func settingKeysAskedOutsideOwners(m Manifest) map[string]bool { used := map[string]bool{} note := func(s string) { for _, k := range settingsUsed(s) { 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/internal/catalogue/unconsumed_placeholder.go b/internal/catalogue/unconsumed_placeholder.go index 23f81a93..5c681f4f 100644 --- a/internal/catalogue/unconsumed_placeholder.go +++ b/internal/catalogue/unconsumed_placeholder.go @@ -56,7 +56,7 @@ type filler struct { // 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: "setting", patterns: []*regexp.Regexp{settingRef}, fields: map[string][]string{"file": {"content"}, "service": {"unit"}, "directory": {"owner"}}}, {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"}}, 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