diff --git a/cmd/mesh-controller/modules.go b/cmd/mesh-controller/modules.go index 9a8e3c8b..830575a3 100644 --- a/cmd/mesh-controller/modules.go +++ b/cmd/mesh-controller/modules.go @@ -414,6 +414,9 @@ func settingsCommand(ctx context.Context, args []string) error { // word (novox/hq issue 304). Adding and changing keys needs nothing; removing one needs this. replace := set.Bool("replace", false, "for set: remove the keys the new layer does not name") history := set.Bool("history", false, "for show: the layers this one replaced, the latest first") + // Set by the settings verb on every line it composes (novox/hq ADR 0266): a verb may not change where a + // module's directories are placed or which of the machine's paths it reaches. + throughVerb := set.Bool("through-verb", false, "the line came from the settings verb: places and accesses are refused") positionals, err := parseAround(set, args[1:]) if err != nil { return err @@ -445,6 +448,11 @@ func settingsCommand(ctx context.Context, args []string) error { if err != nil { return err } + if *throughVerb { + if key := terminalSettingChanged(before, values); key != "" { + return terminalSettingRefusal(key, positionals[0], where) + } + } added, changed, removed := settingsChange(before, values) if len(removed) > 0 && !*replace { return fmt.Errorf("%s on %s: this layer would no longer set %s. A layer is replaced whole; "+ @@ -596,6 +604,15 @@ func settingsCommand(ctx context.Context, args []string) error { if len(positionals) != 1 { return errors.New("settings clear [--node ]") } + if *throughVerb { + before, _, err := inv.Layer(ctx, *node, positionals[0]) + if err != nil { + return err + } + if key := terminalSettingChanged(before, nil); key != "" { + return terminalSettingRefusal(key, positionals[0], where) + } + } if err := inv.ClearSettings(ctx, *node, positionals[0]); err != nil { return err } @@ -1051,3 +1068,28 @@ func declaresTools(m catalogue.Manifest) bool { } return false } + +// terminalSettings are the keys a verb may not change (novox/hq ADR 0266). `places` says where the node-engine +// creates and, as root, owns a module's directories, with an owner the setting names; `accesses` says which of +// the machine's paths are mounted into a module's container. Set through a verb, either would let any caller — +// an agent among them — have root hand it a directory, or mount one of the machine's into a container it +// reaches. They are the operator's, at the controller's terminal. +var terminalSettings = []string{catalogue.PlacesSetting, catalogue.AccessesSetting} + +// terminalSettingChanged is the first of those keys a layer change would add, change or remove, or "". +func terminalSettingChanged(before, after map[string]any) string { + for _, key := range terminalSettings { + was, _ := json.Marshal(before[key]) + now, _ := json.Marshal(after[key]) + if string(was) != string(now) { + return key + } + } + return "" +} + +func terminalSettingRefusal(key, module, where string) error { + return terminalRefusal("%s of %s on %s is set at the controller's terminal only, never through a verb: it says "+ + "where root creates and owns a module's directories, or which of the machine's paths reach its container, "+ + "and whoever may call a verb includes agents (novox/hq ADR 0266). Nothing was changed", key, module, where) +} diff --git a/cmd/mesh-controller/seatverbs.go b/cmd/mesh-controller/seatverbs.go index 99650fd8..cd2eff34 100644 --- a/cmd/mesh-controller/seatverbs.go +++ b/cmd/mesh-controller/seatverbs.go @@ -805,13 +805,16 @@ func (a *verbArguments) commandLine() ([]string, error) { var argv []string switch { case on("clear"): - argv = []string{"settings", "clear", str("module")} + argv = []string{"settings", "clear", str("module"), "--through-verb"} case str("values") != "": argv = []string{"settings", "set", str("module"), str("values")} // What a set removes is refused unless meant (novox/hq ADR 0217). if on("replace") { argv = append(argv, "--replace") } + // Through a verb, never places or accesses (novox/hq ADR 0266): the command refuses a change to + // either when told the line came from a verb. + argv = append(argv, "--through-verb") default: // Neither values nor clear: the layer as it stands, which is what a caller reads before // replacing it (novox/hq ADR 0217) — and with history, the layers it replaced. @@ -1359,7 +1362,10 @@ var commandReadForms = map[string]func(rest []string) bool{ // `plan ` previews a node's declaration; it sends nothing. "plan": func([]string) bool { return true }, // `plans` lists and `plans ` shows one; `plans stop|close|go` acts. - "plans": func(r []string) bool { return !subIn(r, "stop", "close", "go") }, + // Judged on every word, not the first: a flag before the subcommand (`plans --json go `) still acts. + "plans": func(r []string) bool { + return !slices.ContainsFunc(r, func(w string) bool { return slices.Contains(plansActs, w) }) + }, // `doctor` answers the last run, `probes` and `signals` describe; `doctor run` runs. "doctor": func(r []string) bool { return flagsOnly(r) || subIn(r, "probes", "signals") }, "conditions": func(r []string) bool { return flagsOnly(r) || subIn(r, "list", "show", "history") }, @@ -1380,6 +1386,9 @@ var commandReadForms = map[string]func(rest []string) bool{ }, } +// plansActs are the `plans` subcommands that act on a walk; no other word of a plans line is one of them. +var plansActs = []string{"go", "stop", "close", "retry"} + // heldAtTheTerminal is a refusal of policy (novox/hq ADR 0266): the verb is known and served, and this line is // the operator's at the controller's terminal. Never read as a verb this binary is behind on. type heldAtTheTerminal struct{ msg string } diff --git a/cmd/mesh-controller/seatverbs_schema_test.go b/cmd/mesh-controller/seatverbs_schema_test.go index 176b3455..347b693e 100644 --- a/cmd/mesh-controller/seatverbs_schema_test.go +++ b/cmd/mesh-controller/seatverbs_schema_test.go @@ -268,8 +268,9 @@ var accountedFlags = map[string]map[string]string{ "self": "set by the verb from the repository's form: a path on the forge, or a URL", "dry-run": "withheld: a dry run answers only when the build ends, which a call cannot wait for; `command` reaches it", }, - "builds": {"n": "=limit"}, - "plans": {"n": "=limit", "what-if": "=repository"}, + "builds": {"n": "=limit"}, + "plans": {"n": "=limit", "what-if": "=repository"}, + "settings": {"through-verb": "set by the verb on every set and clear: places and accesses are the terminal's (novox/hq ADR 0266)"}, // The machine's tunnel key, named as the verb's other arguments are (novox/hq ADR 0169). "durations": { "json": "set by the verb: the answer is data", diff --git a/cmd/mesh-controller/seatverbs_test.go b/cmd/mesh-controller/seatverbs_test.go index b00517a3..54a93557 100644 --- a/cmd/mesh-controller/seatverbs_test.go +++ b/cmd/mesh-controller/seatverbs_test.go @@ -123,11 +123,11 @@ func TestTokenIsRefusedThroughAVerb(t *testing.T) { // `settings` is `settings set|clear` at a shell, with the values passed inline (novox/hq issue 198). func TestSettingsSetsOrClearsALayer(t *testing.T) { argv, err := argvFor("settings", map[string]any{"module": "dnsmasq", "values": `{"a":1}`, "node": "ace"}) - if err != nil || strings.Join(argv, " ") != `settings set dnsmasq {"a":1} --node ace` { + if err != nil || strings.Join(argv, " ") != `settings set dnsmasq {"a":1} --through-verb --node ace` { t.Fatalf("set on a machine: %v %v", argv, err) } argv, _ = argvFor("settings", map[string]any{"module": "dnsmasq", "clear": "true"}) - if strings.Join(argv, " ") != "settings clear dnsmasq" { + if strings.Join(argv, " ") != "settings clear dnsmasq --through-verb" { t.Fatalf("clear for the mesh: %v", argv) } // Neither values nor clear reads the layer as it stands (novox/hq ADR 0217): what a caller reads @@ -369,7 +369,8 @@ func TestTheCommandVerbOnlyReads(t *testing.T) { "secret recover a", "secret export a", "token issue --new x", "identity show", "broker users", "api key", "licence show", "push anchor --why w", "assign novox m", "settings set m {}", "settings clear m", "module add f", "module check /etc", "module forget m", "module issue m --node a", - "seat rename a b", "plans close p --why w", "plans go p", "doctor run", "conditions silence c --why w", + "seat rename a b", "plans close p --why w", "plans go p", "plans retry p", "plans stop p", + "plans --json go p", "plans -n 3 close p", "plans --what-if r retry p", "doctor run", "conditions silence c --why w", "retire approve x", "cleanup delete x", "delivery check", "delivery go x", "bus upgrade", "mirrors --record x", "mirrors --confirm", "hand-act record x --why y --cause z", "serve", "migrate", "prepare", "declare x", "overlay x", "facts", "merge-gate", "check-here", "build x", "rebuild x", diff --git a/cmd/mesh-controller/terminal_settings_test.go b/cmd/mesh-controller/terminal_settings_test.go new file mode 100644 index 00000000..76bf5d65 --- /dev/null +++ b/cmd/mesh-controller/terminal_settings_test.go @@ -0,0 +1,93 @@ +package main + +import ( + "strings" + "testing" + + "github.com/novox/mesh-controller/internal/catalogue" +) + +// Where root creates and owns a module's directories, and which of the machine's paths reach its container, +// are the controller's terminal's alone (novox/hq ADR 0266): through the settings verb, a caller who set +// `places` to /etc with an owner of its own would have the next send hand it /etc. + +func TestTheSettingsVerbMarksEverySetAndClearAsAVerbs(t *testing.T) { + for _, args := range []map[string]any{ + {"module": "plex", "values": `{"places":{"data":"/etc"}}`}, + {"module": "plex", "values": `{}`, "replace": "true", "node": "home"}, + {"module": "plex", "clear": "true"}, + } { + argv, err := argvFor("settings", args) + if err != nil || !strings.Contains(strings.Join(argv, " "), "--through-verb") { + t.Fatalf("%v: %v %v", args, argv, err) + } + } + // The generic verb never reaches settings set or clear at all. + for _, line := range []string{"settings set plex {}", "settings clear plex"} { + if _, err := argvFor("command", map[string]any{"command": line}); err == nil { + t.Fatalf("command ran %q", line) + } + } +} + +func TestATerminalSettingIsChangedOnlyAtTheTerminal(t *testing.T) { + cases := []struct { + before, after map[string]any + want string + }{ + {nil, map[string]any{"places": map[string]any{"data": "/etc"}}, "places"}, + {map[string]any{"accesses": map[string]any{"m": "/storage"}}, map[string]any{}, "accesses"}, + {map[string]any{"places": map[string]any{"d": "/srv/d"}}, nil, "places"}, + {map[string]any{"places": map[string]any{"d": "/srv/d"}, "a": 1.0}, + map[string]any{"places": map[string]any{"d": "/srv/d"}, "a": 2.0}, ""}, + } + for _, c := range cases { + if got := terminalSettingChanged(c.before, c.after); got != c.want { + t.Errorf("%v → %v: %q, want %q", c.before, c.after, got, c.want) + } + } +} + +// Over the real stores: the verb's line is refused for places, the terminal's is taken, and a clear through +// the verb of a layer that places a directory is refused too. +func TestPlacesAreRefusedThroughTheVerbAndTakenAtTheTerminal(t *testing.T) { + open := aMesh(t) + ctx := t.Context() + register(t, open, catalogue.Manifest{Module: "notes", Version: "1", + Resources: []map[string]any{{"id": "data", "type": "directory", "mode": "0755"}, + {"id": "rc", "type": "file", "path": "/etc/notes.conf", "mode": "0644", "content": "x = ${setting:x}\n"}}}) + if _, err := assign(ctx, open, "laptop", "notes"); err != nil { + t.Fatal(err) + } + set := func(through bool, values string) error { + args := []string{"set", "notes", values, "--node", "laptop"} + if through { + args = append(args, "--through-verb") + } + return settingsCommand(ctx, args) + } + if err := set(true, `{"places":{"data":{"path":"/srv/notes","owner":"1000:1000"}}}`); err == nil || + !strings.Contains(err.Error(), "controller's terminal only") { + t.Fatalf("places through the verb: %v", err) + } + if err := set(false, `{"places":{"data":{"path":"/srv/notes","owner":"1000:1000"}},"x":0}`); err != nil { + t.Fatalf("places at the terminal: %v", err) + } + if err := set(true, `{"places":{"data":{"path":"/srv/notes","owner":"1000:1000"}},"x":1}`); err != nil { + t.Fatalf("a verb may change another key and keep places as they are: %v", err) + } + if err := settingsCommand(ctx, []string{"clear", "notes", "--node", "laptop", "--through-verb"}); err == nil || + !strings.Contains(err.Error(), "controller's terminal only") { + t.Fatalf("a clear through the verb took places away: %v", err) + } + // And never at /etc, from anywhere. + if err := set(false, `{"places":{"data":{"path":"/etc","owner":"1000:1000"}},"x":1}`); err == nil || + !strings.Contains(err.Error(), "/etc") { + t.Fatalf("a place at /etc: %v", err) + } + // A line break in any setting is refused where it is kept. + if err := set(false, `{"places":{"data":{"path":"/srv/notes","owner":"1000:1000"}},"x":"a\nPATH=/tmp"}`); err == nil || + !strings.Contains(err.Error(), "line break") { + t.Fatalf("a line break: %v", err) + } +} diff --git a/cmd/mesh-controller/unseen_test.go b/cmd/mesh-controller/unseen_test.go index 32f9ddfd..879b826b 100644 --- a/cmd/mesh-controller/unseen_test.go +++ b/cmd/mesh-controller/unseen_test.go @@ -158,7 +158,7 @@ func TestTheVerbsCarryReadReplaceAndMove(t *testing.T) { }{ {"settings", map[string]any{"module": "plex", "node": "home"}, []string{"settings", "show", "plex", "--node", "home"}}, {"settings", map[string]any{"module": "plex", "history": "true"}, []string{"settings", "show", "plex", "--history"}}, - {"settings", map[string]any{"module": "plex", "values": "{}", "replace": "true"}, []string{"settings", "set", "plex", "{}", "--replace"}}, + {"settings", map[string]any{"module": "plex", "values": "{}", "replace": "true"}, []string{"settings", "set", "plex", "{}", "--replace", "--through-verb"}}, {"push", map[string]any{"node": "home", "move": "plex", "why": "w"}, []string{"push", "home", "--wait", "0", "--move", "plex", "--why", "w"}}, {"push", map[string]any{"why": "w"}, []string{"push", "--behind", "--wait", "0", "--why", "w"}}, diff --git a/internal/catalogue/agent_account_test.go b/internal/catalogue/agent_account_test.go index 70b18b26..8aad7abf 100644 --- a/internal/catalogue/agent_account_test.go +++ b/internal/catalogue/agent_account_test.go @@ -1,6 +1,7 @@ package catalogue import ( + "strings" "testing" ) @@ -111,3 +112,46 @@ func TestTheRuntimeIsToldTheAgentAccount(t *testing.T) { t.Error("a bundle may tell the runtime whom agents run as") } } + +// No placement and no access at the machine's own system or the mesh's state, however it is spelled (novox/hq +// ADR 0266); a module's own place elsewhere is taken. +func TestAPlacementOrAnAccessAtTheMachinesOwnIsRefused(t *testing.T) { + m := Manifest{Module: "notes", Resources: []map[string]any{{"id": "data", "type": "directory"}}, + Accesses: []Access{{ID: "media"}}} + for _, path := range []string{"/", "/etc", "/etc/sudoers.d", "/usr/bin", "/root", "/var/lib", "/home", + "/var/lib/mesh/x", "/var/lib/mesh-host", "/srv/../etc", "/proc/1", "/dev"} { + layers := []Layer{{From: "laptop", Values: map[string]any{PlacesSetting: map[string]any{"data": path}}}} + if _, err := Places(m, layers); err == nil { + t.Errorf("a place at %s was taken", path) + } + layers = []Layer{{From: "laptop", Values: map[string]any{AccessesSetting: map[string]any{"media": path}}}} + if _, err := AccessPlaces(m, layers); err == nil { + t.Errorf("an access at %s was taken", path) + } + } + for _, path := range []string{"/srv/notes", "/mnt/plex/data", "/storage/media", "/home/restic", "/var/lib/notes/data"} { + layers := []Layer{{From: "laptop", Values: map[string]any{PlacesSetting: map[string]any{"data": path}}}} + if got, err := Places(m, layers); err != nil || got["data"].Path != path { + t.Errorf("a place at %s: %v %v", path, got, err) + } + } +} + +// A line break or a NUL in any string of any setting is refused, at any depth; PEM blocks alone may hold lines. +func TestASettingHoldsOneLine(t *testing.T) { + m := Manifest{Module: "mailu"} + for _, v := range []any{"a\nDEBUG=1", "a\rb", "a\x00b", map[string]any{"k": []any{"ok", "x\ny"}}, + map[string]any{"k\nx": "v"}} { + if err := JudgeSettings(m, []Layer{{From: "home", Values: map[string]any{"v": v}}}, false); err == nil || + !strings.Contains(err.Error(), "line break") { + t.Errorf("%q: %v", v, err) + } + } + pem := "-----BEGIN CERTIFICATE-----\nMIIBeDCCAR2gAwIBAgIQ\n-----END CERTIFICATE-----\n" + if err := JudgeSettings(m, []Layer{{From: "home", Values: map[string]any{"root": pem}}}, false); err != nil { + t.Errorf("a PEM block: %v", err) + } + if err := JudgeSettings(m, []Layer{{From: "home", Values: map[string]any{"root": pem + "PATH=/tmp evil\n"}}}, false); err == nil { + t.Error("a PEM block with a line of something else after it was taken") + } +} diff --git a/internal/catalogue/placement.go b/internal/catalogue/placement.go index 6a406a26..d54e07a7 100644 --- a/internal/catalogue/placement.go +++ b/internal/catalogue/placement.go @@ -2,6 +2,7 @@ package catalogue import ( "fmt" + "path/filepath" "regexp" "sort" "strings" @@ -44,6 +45,33 @@ type Placement struct { var ownerShape = regexp.MustCompile(`^[0-9]+:[0-9]+$`) +// systemTrees are where no placement and no access may be: the machine's own system, and the node-engine's +// and the tool runner's state (novox/hq ADR 0266). A placed directory is created and chowned by the +// node-engine as root, and an access is mounted into a container: a place at /etc, owned by an account a +// caller names, hands that account the machine. Refused at or below each of these. +var systemTrees = []string{"/etc", "/usr", "/boot", "/root", "/proc", "/sys", "/dev", "/run", "/bin", "/sbin", + "/lib", "/lib64", "/var/lib/mesh", "/var/lib/mesh-host", "/var/lib/mesh-bus-conf"} + +// systemRoots are directories a placement may be below but never be: each holds the whole machine's, or +// every module's or every person's, directories. +var systemRoots = []string{"/", "/var", "/var/lib", "/home", "/mnt", "/srv", "/opt", "/storage", "/data", "/tmp", "/var/tmp"} + +// systemPath says why a path is the machine's own and no placement's, or "". +func systemPath(path string) string { + clean := filepath.Clean(path) + for _, root := range systemRoots { + if clean == root { + return clean + " is a whole tree of the machine's" + } + } + for _, tree := range systemTrees { + if clean == tree || strings.HasPrefix(clean, tree+"/") { + return clean + " is in " + tree + ", the machine's own or the mesh's state" + } + } + return "" +} + // accessRef is how a module names one of its accesses: ${access:}. var accessRef = regexp.MustCompile(`\$\{access:([a-z0-9][a-z0-9-]*)\}`) @@ -103,7 +131,11 @@ func Places(m Manifest, layers []Layer) (map[string]Placement, error) { if !strings.HasPrefix(p.Path, "/") { return nil, fmt.Errorf("%s places %q at %q, which is not an absolute path", m.Module, id, p.Path) } - p.Path = strings.TrimRight(p.Path, "/") + p.Path = filepath.Clean(p.Path) + if why := systemPath(p.Path); why != "" { + return nil, fmt.Errorf("%s places %q at %s: %s, and the node-engine would create and own it as "+ + "root (novox/hq ADR 0266)", m.Module, id, p.Path, why) + } out[id] = p } } @@ -147,7 +179,12 @@ func AccessPlaces(m Manifest, layers []Layer) (map[string]string, error) { return nil, fmt.Errorf("%s places the access %q at %v, which is not an absolute path", m.Module, id, body) } - out[id] = strings.TrimRight(path, "/") + path = filepath.Clean(path) + if why := systemPath(path); why != "" { + return nil, fmt.Errorf("%s places the access %q at %s: %s, and an access is mounted into the "+ + "module's container (novox/hq ADR 0266)", m.Module, id, path, why) + } + out[id] = path } } if len(out) == 0 { diff --git a/internal/catalogue/settings.go b/internal/catalogue/settings.go index d7ff5f8b..87b67bae 100644 --- a/internal/catalogue/settings.go +++ b/internal/catalogue/settings.go @@ -209,6 +209,68 @@ func deepCopy(in map[string]any) map[string]any { return out } +// settingsHoldOneLine refuses a line break or a NUL in any string of any setting, for every module, at any +// depth: a key or a value, in an object or a list (novox/hq ADR 0266). A value is substituted into env and +// configuration files the node-engine writes as root (an app's .env, a logind drop-in), and a line break +// there is a directive of the caller's own; a NUL ends a string early wherever C reads it. Judged where a +// setting is kept and again where it is composed, so a stored value with one costs its module its place +// and says which key. One shape is let through: PEM blocks alone (a certificate authority's root handed to a +// provider), whose lines are base64 between BEGIN and END. Anything else that must hold lines is the +// module's own file, not a setting. +func settingsHoldOneLine(module string, layers []Layer) error { + for _, layer := range layers { + for _, key := range sortedKeysAny(layer.Values) { + if at := lineBreakIn(layer.Values[key], key); at != "" { + return fmt.Errorf("%s: the setting %s in %q holds a line break or a NUL, which a file it is written "+ + "into would read as a directive of its own (novox/hq ADR 0266); a setting is one line", + module, at, layer.From) + } + } + } + return nil +} + +// pemShape is the one value with lines a setting may hold: PEM blocks and nothing else — a certificate +// authority's root the operator hands a provider is one. Its lines are base64 between BEGIN and END: no +// space, quote, dot or underscore, so no path, option or command — at most a padded line an env file would +// read as an empty assignment, which names no program. +var pemShape = regexp.MustCompile(`^(-----BEGIN [A-Z0-9 ]+-----\n([A-Za-z0-9+/]{1,76}={0,2}\n)+-----END [A-Z0-9 ]+-----\n?)+$`) + +// lineBreakIn is the path of the first string under v holding \n, \r or NUL, or "". +func lineBreakIn(v any, at string) string { + switch t := v.(type) { + case string: + if strings.ContainsAny(t, "\n\r\x00") && !pemShape.MatchString(t) { + return at + } + case map[string]any: + for _, k := range sortedKeysAny(t) { + if strings.ContainsAny(k, "\n\r\x00") { + return at + "." + strings.ToValidUTF8(strings.NewReplacer("\n", "\\n", "\r", "\\r", "\x00", "\\0").Replace(k), "?") + } + if found := lineBreakIn(t[k], at+"."+k); found != "" { + return found + } + } + case []any: + for i, e := range t { + if found := lineBreakIn(e, fmt.Sprintf("%s[%d]", at, i)); found != "" { + return found + } + } + } + return "" +} + +func sortedKeysAny(m map[string]any) []string { + out := make([]string, 0, len(m)) + for k := range m { + out = append(out, k) + } + sort.Strings(out) + return out +} + // UnusedSettings names settings that reach nothing. // // Somebody who sets a key on a module with nothing mergeable, or misspells one, has changed @@ -405,6 +467,9 @@ var networkName = regexp.MustCompile(`^[A-Za-z0-9][A-Za-z0-9_.-]*$`) // refused where it is stored (SetSettings, with UnusedSettings) and said where a plan is read, // and never costs a module its place. func JudgeSettings(m Manifest, layers []Layer, adopted bool) error { + if err := settingsHoldOneLine(m.Module, layers); err != nil { + return err + } // With no layers too: a definition may ask for a setting nobody made — an access placed by // nobody, a file's ${setting:…} nothing sets — and that is the same statement, missing. if _, err := GivenPorts(m, layers); err != nil {