From f4efc3cad0a690c8b7f72053a03ac506c2088752 Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 12 Oct 2026 12:23:38 +0200 Subject: [PATCH 1/2] Grant a read-write access's path group to what runs the module's code, so a module can write shared data without the mesh changing it (hq ADR 0312, issue 537) The engine reads the group owning the accessed path at apply and gives it to the containers and the tools' account an access's new `grant` names: --group-add for a container (recreated with it), an account group for the account (recorded, taken back as ADR 0252 takes back). The path is never changed; an access whose group cannot write is stated unhealthy for the operator to decide; group 0 and the root-granting groups are refused by name. The tool runner restarts through the restart-on the controller gives it, because the access reports updated exactly when the account's groups moved. Health says GrantContract (4), which tells the controller it may send `grant`. --- cmd/mesh-host/grants.go | 123 ++++++++ cmd/mesh-host/grants_test.go | 78 +++++ cmd/mesh-host/main.go | 35 ++- cmd/mesh-host/units_test.go | 3 +- internal/apply/apply.go | 56 +++- internal/apply/grants.go | 407 +++++++++++++++++++++++++ internal/apply/grants_test.go | 457 ++++++++++++++++++++++++++++ internal/apply/mesh_names_test.go | 6 +- internal/apply/plan.go | 5 + internal/apply/reads_test.go | 2 +- internal/apply/schedule.go | 17 +- internal/apply/vocabulary_test.go | 6 +- internal/declaration/declaration.go | 72 +++++ internal/link/messages.go | 12 + 14 files changed, 1249 insertions(+), 30 deletions(-) create mode 100644 cmd/mesh-host/grants.go create mode 100644 cmd/mesh-host/grants_test.go create mode 100644 internal/apply/grants.go create mode 100644 internal/apply/grants_test.go diff --git a/cmd/mesh-host/grants.go b/cmd/mesh-host/grants.go new file mode 100644 index 0000000..0a275bc --- /dev/null +++ b/cmd/mesh-host/grants.go @@ -0,0 +1,123 @@ +package main + +import ( + "sort" + "strings" + "sync" + "time" + + "github.com/novox/mesh-host/internal/apply" + "github.com/novox/mesh-host/internal/declaration" + "github.com/novox/mesh-host/internal/link" +) + +// A read-write access whose group cannot write is never quiet (novox/hq ADR 0312 §2). +// +// The engine grants a read-write access's path group to the identities the module's code runs as, and changes +// nothing on the path: whether its group may write is the operator's, as the data is. So an access whose path's +// group has no write permission is stated, on every look, as its module's resource of kind `access`, unhealthy, +// saying so and that the operator decides — and one owned by a group that is never granted (§4) is stated +// unhealthy saying that. The controller raises it as that module's condition on two statements in a row and +// clears it on the first healthy one, as it does every resource's health (ADR 0240). The path is the target, +// never in the reason: the controller says a reason on the operator's channel, which withholds a path (ADR 0234 +// §6). + +// grantedAccess is one read-write access that grants, as the judge reads it. +type grantedAccess struct { + module, id, path string +} + +// accessJudge reads, on every look, whether each read-write access that grants may write its path through its +// group. Safe for the apply and the looking loop at once. +type accessJudge struct { + mu sync.Mutex + accesses []grantedAccess + // was is each access's last verdict, with since when and how many looks in a row found it unhealthy. + was map[string]link.ResourceHealth +} + +// accesses is the one access judge on this machine. +var accesses = &accessJudge{was: map[string]link.ResourceHealth{}} + +// set is what the declaration just applied asks: every read-write access of a module that names a grant, not +// held as found. An access no longer declared is forgotten. +func (j *accessJudge) set(d *declaration.Declaration, held map[string]bool) { + var as []grantedAccess + if d != nil { + for _, r := range d.Resources { + a, ok := r.(*declaration.Access) + if !ok || a.Grant == nil || a.Mode != declaration.AccessReadWrite || held[a.ID] { + continue + } + at := strings.LastIndex(a.ID, ".") + if at <= 0 || strings.HasPrefix(a.ID, declaration.AdoptionPrefix) { + continue + } + as = append(as, grantedAccess{module: a.ID[:at], id: a.ID, path: a.Path}) + } + } + sort.Slice(as, func(a, b int) bool { return as[a].id < as[b].id }) + j.mu.Lock() + defer j.mu.Unlock() + j.accesses = as + declared := map[string]bool{} + for _, a := range as { + declared[a.id] = true + } + for id := range j.was { + if !declared[id] { + delete(j.was, id) + } + } +} + +// look reads every access now: healthy when its group may write and may be granted, unhealthy saying why +// otherwise. Whether any verdict changed since the last look, for the loop to say it. +func (j *accessJudge) look(now time.Time) ([]link.ResourceHealth, bool) { + j.mu.Lock() + defer j.mu.Unlock() + changed := false + out := make([]link.ResourceHealth, 0, len(j.accesses)) + for _, a := range j.accesses { + state, reason := link.StateHealthy, "" + g, err := apply.GroupOfPath(a.path) + switch { + case err != nil: + state, reason = link.StateUnhealthy, "its group cannot be read: "+strings.ReplaceAll(err.Error(), a.path, "the path") + case apply.GrantRefusal(g) != nil: + state, reason = link.StateUnhealthy, "its group is never granted: "+apply.GrantRefusal(g).Error() + case !g.Writable: + state, reason = link.StateUnhealthy, apply.CannotWriteWords(g) + } + v := link.ResourceHealth{Module: a.module, Resource: a.id, Kind: link.KindAccess, Target: a.path, + State: state, Reason: reason, Since: now.UTC()} + if was, had := j.was[a.id]; had && was.State == state && was.Reason == reason { + v.Since = was.Since + v.Streak = was.Streak + } else { + changed = true + } + if state != link.StateHealthy { + v.Streak++ + } + j.was[a.id] = v + out = append(out, v) + } + return out, changed +} + +// withAccesses adds to a statement every read-write access that grants, as its module's resource of kind access. +func withAccesses(h *link.Health, vs []link.ResourceHealth) *link.Health { + h.Resources = append(h.Resources, vs...) + return h +} + +// accessesHealthy says no access in vs is anything but healthy. +func accessesHealthy(vs []link.ResourceHealth) bool { + for _, v := range vs { + if v.State != link.StateHealthy { + return false + } + } + return true +} diff --git a/cmd/mesh-host/grants_test.go b/cmd/mesh-host/grants_test.go new file mode 100644 index 0000000..51510de --- /dev/null +++ b/cmd/mesh-host/grants_test.go @@ -0,0 +1,78 @@ +package main + +import ( + "strings" + "testing" + "time" + + "github.com/novox/mesh-host/internal/apply" + "github.com/novox/mesh-host/internal/declaration" + "github.com/novox/mesh-host/internal/link" +) + +// A read-write access whose path's group cannot write is stated unhealthy on every look, saying so and that the +// operator decides, and one whose group is never granted says that (novox/hq ADR 0312 §2, §4). The path is the +// target and never in the reason (ADR 0234 §6); a read access, granting nothing, is not stated at all. +func TestAnAccessWhoseGroupCannotWriteIsStatedUnhealthyWithTheOperatorsRemedy(t *testing.T) { + spool, library, shared, read := t.TempDir(), t.TempDir(), t.TempDir(), t.TempDir() + groups := map[string]apply.PathGroup{ + spool: {GID: 1002, Name: "downloads", Writable: false}, + library: {GID: 1001, Name: "media", Writable: true}, + shared: {GID: 0, Name: "root", Writable: true}, + } + was := apply.GroupOfPath + apply.GroupOfPath = func(path string) (apply.PathGroup, error) { return groups[path], nil } + t.Cleanup(func() { apply.GroupOfPath = was }) + + d, err := declaration.Parse([]byte(`{"declaration":1,"resources":[ + {"id":"nzbget.downloads","type":"access","path":"` + spool + `","mode":"read-write","grant":{"account":"operator"}}, + {"id":"importer.music","type":"access","path":"` + library + `","mode":"read-write","grant":{"account":"operator"}}, + {"id":"importer.shared","type":"access","path":"` + shared + `","mode":"read-write","grant":{"account":"operator"}}, + {"id":"player.music","type":"access","path":"` + read + `","mode":"read"}]}`)) + if err != nil { + t.Fatal(err) + } + j := &accessJudge{was: map[string]link.ResourceHealth{}} + j.set(d, nil) + at := time.Date(2026, 10, 12, 12, 0, 0, 0, time.UTC) + vs, changed := j.look(at) + if !changed || len(vs) != 3 { + t.Fatalf("stated %d accesses (changed %v), want the three read-write ones: %+v", len(vs), changed, vs) + } + got := map[string]link.ResourceHealth{} + for _, v := range vs { + got[v.Resource] = v + if v.Kind != link.KindAccess || strings.Contains(v.Reason, v.Target) { + t.Errorf("%s is stated as %q with its path in the reason %q", v.Resource, v.Kind, v.Reason) + } + } + if v := got["nzbget.downloads"]; v.State != link.StateUnhealthy || v.Module != "nzbget" || v.Target != spool || + !strings.Contains(v.Reason, "cannot write") || !strings.Contains(v.Reason, "operator's to decide") || v.Streak != 1 { + t.Errorf("an access whose group cannot write is stated %+v", v) + } + if v := got["importer.shared"]; v.State != link.StateUnhealthy || !strings.Contains(v.Reason, "never granted") { + t.Errorf("an access of the root group is stated %+v", v) + } + if v := got["importer.music"]; v.State != link.StateHealthy { + t.Errorf("an access whose group may write is stated %+v", v) + } + if h := withAccesses(&link.Health{}, vs); accessesHealthy(vs) || len(h.Resources) != 3 { + t.Errorf("the statement does not carry the accesses as unhealthy: %+v", h.Resources) + } + + // Looked at again: the same verdict, its since kept and its streak counted. + vs, changed = j.look(at.Add(time.Minute)) + for _, v := range vs { + if v.Resource == "nzbget.downloads" && (changed || v.Streak != 2 || !v.Since.Equal(at)) { + t.Errorf("the second look is %+v (changed %v)", v, changed) + } + } + // The operator gives the group write: healthy at the next look, and nothing the engine did made it so. + groups[spool] = apply.PathGroup{GID: 1002, Name: "downloads", Writable: true} + vs, _ = j.look(at.Add(2 * time.Minute)) + for _, v := range vs { + if v.Resource == "nzbget.downloads" && v.State != link.StateHealthy { + t.Errorf("an access whose group was given write is stated %+v", v) + } + } +} diff --git a/cmd/mesh-host/main.go b/cmd/mesh-host/main.go index 86be16a..c5e2fb0 100644 --- a/cmd/mesh-host/main.go +++ b/cmd/mesh-host/main.go @@ -1572,6 +1572,16 @@ func judgeWhatRuns(ctx context.Context, j *liveness.Judge, queue *link.Queue, sa } } } + // Every read-write access that grants, read on every look (novox/hq ADR 0312 §2). + accessSt, accessesChanged := accesses.look(time.Now()) + owed = owed || accessesChanged + if accessesChanged { + for _, v := range accessSt { + if v.State != link.StateHealthy { + say(fmt.Sprintf("%s (access %s) is %s: %s", v.Resource, v.Target, v.State, v.Reason)) + } + } + } var netSt *network.Statement if n := netJudge.get(); n != nil { ns, netChanged := n.Look(ctx) @@ -1597,7 +1607,8 @@ func judgeWhatRuns(ctx context.Context, j *liveness.Judge, queue *link.Queue, sa found := foundNow() since := time.Since(lastSaid) healthy := st.Healthy() && (netSt == nil || netSt.State != network.Unhealthy) && - (unitSt == nil || len(unitSt.Failed) == 0) && (accountSt == nil || accountSt.Healthy()) && len(found) == 0 + (unitSt == nil || len(unitSt.Failed) == 0) && (accountSt == nil || accountSt.Healthy()) && len(found) == 0 && + accessesHealthy(accessSt) if !owed && !(!healthy && since >= sayUnhealthyAgain) && since < sayAnyway { continue } @@ -1608,8 +1619,8 @@ func judgeWhatRuns(ctx context.Context, j *liveness.Judge, queue *link.Queue, sa } } } - if queue.SayHealth(ctx, *withFoundDirectories(withAccounts(withUnits(healthAsReported(st, netSt), unitSt), - accountSt), found)) { + if queue.SayHealth(ctx, *withAccesses(withFoundDirectories(withAccounts(withUnits(healthAsReported(st, netSt), + unitSt), accountSt), found), accessSt)) { lastSaid, owed = time.Now(), false } } @@ -1617,10 +1628,10 @@ func judgeWhatRuns(ctx context.Context, j *liveness.Judge, queue *link.Queue, sa // healthAsReported is a statement as the report and the event carry it. func healthAsReported(st liveness.Statement, ns *network.Statement) *link.Health { - // RootContract: this engine reads a resource's declared `health` and judges it (ADR 0240 Phase B), and - // judges a user's declared `root` (novox/hq ADR 0266), which is what tells the controller it may be sent - // either field. - h := &link.Health{Contract: link.RootContract, At: st.At.UTC(), Resources: []link.ResourceHealth{}} + // GrantContract: this engine reads a resource's declared `health` and judges it (ADR 0240 Phase B), judges a + // user's declared `root` (novox/hq ADR 0266), and grants a read-write access's path group (novox/hq ADR 0312), + // which is what tells the controller it may be sent any of those fields. + h := &link.Health{Contract: link.GrantContract, At: st.At.UTC(), Resources: []link.ResourceHealth{}} for _, r := range st.Resources { h.Resources = append(h.Resources, link.ResourceHealth{Module: r.Module, Resource: r.ID, Kind: r.Kind, Target: r.Target, State: r.State, Reason: r.Reason, Since: r.Since.UTC(), Streak: r.Streak, @@ -2007,12 +2018,16 @@ func applyAndKeepHeld(ctx context.Context, opts options, raw []byte, signed *sto // said with it, relogin needed included, in the report that says the apply. var accountSt *accounts.Statement if a := accountJudge; a != nil { - a.Set(accounts.Of(declared, held)) + // A read-write access's account too (novox/hq ADR 0312 §6): relogin needed is said for a session of + // the account's own manager that began before the grant, as for a user resource's group. + a.Set(append(accounts.Of(declared, held), apply.GrantedAccounts(declared, held)...)) as, _ := a.Look(ctx) accountSt = &as } - report.Health = withFoundDirectories(withAccounts(withUnits(healthAsReported(st, netSt), unitSt), accountSt), - foundDirectories(updated)) + accesses.set(declared, held) + accessSt, _ := accesses.look(time.Now()) + report.Health = withAccesses(withFoundDirectories(withAccounts(withUnits(healthAsReported(st, netSt), unitSt), + accountSt), foundDirectories(updated)), accessSt) } // Which of this machine's links face outside, for the filter the mesh writes around them // (novox/hq ADR 0140). Reported whatever the node's mode: a converged node's filter needs it, diff --git a/cmd/mesh-host/units_test.go b/cmd/mesh-host/units_test.go index b7c5e37..f8cddc2 100644 --- a/cmd/mesh-host/units_test.go +++ b/cmd/mesh-host/units_test.go @@ -87,7 +87,8 @@ func TestTheStatementSaysAnAccountJudgedForRoot(t *testing.T) { Groups: []string{"openrazer"}}, State: accounts.Healthy, Since: at}, }} h := withAccounts(healthAsReported(liveness.Statement{At: at}, nil), as) - if h.Contract != link.RootContract || link.RootContract != 3 { + // At RootContract or later: an engine that also grants an access's group says GrantContract (novox/hq ADR 0312). + if h.Contract < link.RootContract || link.RootContract != 3 || h.Contract != link.GrantContract { t.Fatalf("contract %d", h.Contract) } got := map[string]string{} diff --git a/internal/apply/apply.go b/internal/apply/apply.go index 2e64757..14121ec 100644 --- a/internal/apply/apply.go +++ b/internal/apply/apply.go @@ -340,6 +340,9 @@ func ApplyMindingWindows( action, detail, err = removeOpening(ctx, orphan, run, known.Firewall) case declaration.TypeUser: action, detail, err = removeUser(ctx, sys, orphan, run, wanted) + case declaration.TypeAccess: + // The path is left as it is; what it granted an account is taken back (novox/hq ADR 0312 §5). + action, detail = removeAccess(ctx, sys, orphan, run, wanted) default: action, detail, err = remove(ctx, sys, orphan, run, made) } @@ -511,6 +514,12 @@ func ApplyMindingWindows( // which declared directories were already here: one this apply makes is the mesh's, one found may not be. refreshRuntimeRoots(ctx) in.refused = refusedPlaces(d.Resources) + // The group of every read-write access that grants, read once (novox/hq ADR 0312): what each container is + // created with, and what each account is asked to be in beside its user resources' groups — read before any + // orphan goes, so one access going leaves an account in a group another still grants. + in.grants = grantsOf(d.Resources, in.refused) + in.containerGroups = containerGroups(d.Resources, in.grants) + wanted.withGrants(d.Resources, in.grants) in.agentHomes = rootNeverHomes(d) in.dirsBefore = map[string]bool{} for _, resource := range d.Resources { @@ -1081,7 +1090,7 @@ func applyOne(ctx context.Context, sys system.System, r declaration.Resource, ru case *declaration.Network: return applyNetwork(ctx, res, run) case *declaration.Access: - return applyAccess(res) + return applyAccess(ctx, sys, res, run, previous, in) default: // Unreachable: the declaration refused this already. Present because "unreachable" // stops being true the moment someone adds a kind and forgets this switch. @@ -1268,7 +1277,10 @@ func applyHandedOver(r *declaration.Directory, out Outcome, mode os.FileMode) (O // operator must provide the path, rather than conjuring a directory it does not own and cannot // give the right owner. Nothing is written, so this never reports a change: the machine did not // move, the host merely confirmed a fact about it. -func applyAccess(r *declaration.Access) (Outcome, error) { +// +// **A read-write access grants its path's group** (novox/hq ADR 0312): read, never changed — grantAccess. +func applyAccess(ctx context.Context, sys system.System, r *declaration.Access, run Runner, previous store.Applied, + in inputs) (Outcome, error) { out := begin(r) info, err := os.Stat(r.Path) if errors.Is(err, os.ErrNotExist) { @@ -1292,6 +1304,9 @@ func applyAccess(r *declaration.Access) (Outcome, error) { } out.Action = "unchanged" out.Detail = "operator-owned; present, " + mode + ", nothing managed" + if err := grantAccess(ctx, sys, r, run, previous, in, &out); err != nil { + return out, err + } return out, nil } @@ -2112,6 +2127,10 @@ type inputs struct { // dirsBefore is every declared directory that was on the machine when this apply began (novox/hq issue // 339): one the apply itself made — a file's parent — is the mesh's, one that was there may not be. dirsBefore map[string]bool + // grants is what every read-write access that grants grants, by access id, read once (novox/hq ADR 0312); + // containerGroups the number of every group granted to a container, by container id. + grants map[string]grant + containerGroups map[string][]int } // fileDigest is what a file the container reads holds, by digest. @@ -2186,13 +2205,15 @@ func (in inputs) reads(r *declaration.Container) map[string]string { // containerSpec is the identity of a declared container: everything that, if changed, means // the running container is no longer what was asked for. func containerSpec(r *declaration.Container, in inputs) string { - return containerSpecReading(r, in.declares, in.reads(r)) + return containerSpecReading(r, in.declares, in.reads(r), in.containerGroups[r.ID]) } // containerSpecReading is containerSpec with what the container reads already read — so an // applier that also records those digests reads each file once, and the label and the record // cannot disagree about a file that moved between two reads. -func containerSpecReading(r *declaration.Container, declares, reads map[string]string) string { +// +// groups is the number of every group its read-write accesses grant it (novox/hq ADR 0312). +func containerSpecReading(r *declaration.Container, declares, reads map[string]string, groups []int) string { keys := make([]string, 0, len(r.Env)) for k := range r.Env { keys = append(keys, k) @@ -2271,6 +2292,12 @@ func containerSpecReading(r *declaration.Container, declares, reads map[string]s if h := r.Health; h.RunByRuntime() { b.WriteString("health " + strings.Join(runtimeCheckArgs(h), " ") + "\n") } + // And the groups its read-write accesses grant it (novox/hq ADR 0312 §6): a process's supplementary groups + // are fixed when the container is created, so one gained, dropped or renumbered is a container recreated + // with it. Only when there is one, so no container that is granted nothing has its digest moved. + for _, gid := range groups { + b.WriteString("group " + strconv.Itoa(gid) + "\n") + } // **What this container reads is part of what it is.** // // A container takes its environment and its mounted files once, at start, and never looks @@ -2371,7 +2398,8 @@ func applyContainer(ctx context.Context, r *declaration.Container, run Runner, out := begin(r) // Read once, so the spec and the record agree on what was read even if a file moves under them. reads := in.reads(r) - want := containerSpecReading(r, in.declares, reads) + groups := in.containerGroups[r.ID] + want := containerSpecReading(r, in.declares, reads, groups) out.reads = reads cri, err := containerRuntime(ctx, run) @@ -2399,7 +2427,7 @@ func applyContainer(ctx context.Context, r *declaration.Container, run Runner, } if r.RunOnce { - return applyRunOnce(ctx, r, run, cri, want, previous) + return applyRunOnce(ctx, r, run, cri, want, groups, previous) } before, err := containerState(ctx, r.Name, run) @@ -2444,7 +2472,7 @@ func applyContainer(ctx context.Context, r *declaration.Container, run Runner, // genuine recreate. The trade-off, stated: a container that was ALREADY stale when the host // upgraded — created against a file that has since changed — is not caught by this, and could // not be by the alternative either, which recreates it without knowing whether it needed to. - legacy := len(reads) > 0 && before.Spec == containerSpecReading(r, in.declares, nil) && + legacy := len(reads) > 0 && before.Spec == containerSpecReading(r, in.declares, nil, groups) && (wasReading == nil || sameReads(wasReading, reads)) // **A container a maintenance window holds still is left as it is** (novox/hq issue 224, ADR @@ -2514,6 +2542,7 @@ func applyContainer(ctx context.Context, r *declaration.Container, run Runner, for _, v := range r.Volumes { args = append(args, "--volume", v) } + args = append(args, groupAddArgs(groups)...) // **The one check the container carries is the one the module declared** (ADR 0240 rule 3): an // exec command, or the image's own adopted by name, with the declared timing. Nothing else on the // machine sets a container's check. @@ -2570,6 +2599,9 @@ func applyContainer(ctx context.Context, r *declaration.Container, run Runner, } case len(reasons) > 0: out.Detail = "recreated to pick up " + strings.Join(reasons, ", ") + case len(groups) > 0 && before.Spec == containerSpecReading(r, in.declares, reads, nil): + // It was created before its read-write accesses granted it anything (novox/hq ADR 0312 §6). + out.Detail = "recreated to hold the groups its read-write accesses grant (gid " + gidWords(groups) + ")" default: out.Detail = "replaced; a container's configuration is fixed when it is created" } @@ -2609,7 +2641,8 @@ func runtimeCheckArgs(h *declaration.Health) []string { // - It writes a seed only if it is absent and never reconciles it, so what a running program // grows in that seed afterward is never wiped (04-ISSUES/035). The host's marker keeps the // step from re-running; the step's own code keeps it from clobbering on the pass it does run. -func applyRunOnce(ctx context.Context, r *declaration.Container, run Runner, cri, want string, previous store.Applied) (Outcome, error) { +func applyRunOnce(ctx context.Context, r *declaration.Container, run Runner, cri, want string, groups []int, + previous store.Applied) (Outcome, error) { out := begin(r) // Already completed for this exact declaration. The marker is the store, because a run-once @@ -2628,7 +2661,7 @@ func applyRunOnce(ctx context.Context, r *declaration.Container, run Runner, cri // Run in the foreground so the runtime waits for the container and hands back its exit code. // No --detach and no --restart: a step that is restarted is not a step. - args := foregroundRunArgs(r, want) + args := foregroundRunArgs(r, want, groups) if _, err := run(ctx, cri, args...); err != nil { // A non-zero exit or a runtime that could not start it. Either way the step did not make @@ -2650,7 +2683,9 @@ func applyRunOnce(ctx context.Context, r *declaration.Container, run Runner, cri // exit code — no --detach, no --restart, because a step that is restarted is not a step. Shared by // a run-once step (novox/hq ADR 0052) and by one fire of a scheduled step (ADR 0053), which are the // same "run the container and let it exit" up to how often it happens. -func foregroundRunArgs(r *declaration.Container, want string) []string { +// +// groups is the number of every group its read-write accesses grant it (novox/hq ADR 0312). +func foregroundRunArgs(r *declaration.Container, want string, groups []int) []string { args := []string{"run", "--name", r.Name} for _, file := range r.EnvFile { args = append(args, "--env-file", file) @@ -2665,6 +2700,7 @@ func foregroundRunArgs(r *declaration.Container, want string) []string { for _, v := range r.Volumes { args = append(args, "--volume", v) } + args = append(args, groupAddArgs(groups)...) for _, h := range r.Hosts { args = append(args, "--add-host", h) } diff --git a/internal/apply/grants.go b/internal/apply/grants.go new file mode 100644 index 0000000..af3e04f --- /dev/null +++ b/internal/apply/grants.go @@ -0,0 +1,407 @@ +package apply + +import ( + "context" + "fmt" + "os" + "os/user" + "path/filepath" + "slices" + "sort" + "strconv" + "strings" + + "github.com/novox/mesh-host/internal/accounts" + "github.com/novox/mesh-host/internal/declaration" + "github.com/novox/mesh-host/internal/store" + "github.com/novox/mesh-host/internal/system" +) + +// A read-write access grants the path's own group (novox/hq ADR 0312, issue 537). +// +// **An access is not owning.** ADR 0051 left the operator's shared paths exactly as they are: the engine +// confirms one is there and mounts it, and sets no owner, group or mode on it or below it. Nothing decided how +// a `read-write` access then becomes writable, and a module whose identity was not the path's owner could not +// write it at all — a download client could not remove a folder it had filed (issue 535). +// +// **The path's group is the operator's statement of who may share it.** So the engine reads, at apply, which +// group owns the accessed path — only the machine knows — and makes the identities the controller names for +// the access members of it: each container that mounts it gains the group as a supplementary group of its +// process (`--group-add`, by number, because a container's own group database is not the machine's), and the +// account that runs the module's tools is put in it as an account group (ADR 0252 §1). The module then writes +// the path as its group may, and as nothing else. +// +// **The path is never changed** (§2). A group that cannot write the path is the operator's choice, as the data +// is: the access is stated unhealthy on every look, saying so and that the operator decides, and nothing is +// done to make it writable. **A read access grants nothing** (§3). **The root group is never granted** (§4): a +// path owned by group 0, or by a group whose members act as root (accounts.RootGroups, the account judge's own +// closed list), is refused by name. +// +// **What was granted is taken back as ADR 0252 takes back** (§5): the access records each group it put the +// account in that the account was not in before; when no declared resource asks for it any more, the account +// leaves it, and a group the account held before is never recorded and never taken back. A container needs no +// record: the group is part of what it is created as, so one no longer granted is recreated without it. +// +// **And what runs the code is restarted to hold it** (§6). A container is recreated with the group, because +// its spec carries it. The tool runner — a process of the machine's own service manager with `User=` — is +// restarted because the controller names the access under its `restart-on`, and the access is reported +// updated exactly when the account's groups moved: that manager reads the account's groups from the database +// each time it starts the unit, so the restarted process holds the group with no new login. A unit of the +// account's own manager, or a login session, keeps the groups its login began with: the account judge +// (internal/accounts) says *relogin needed* there, for the access's account as for a user resource's. + +// PathGroup is the group that owns an accessed path, as the engine reads it. +type PathGroup struct { + GID int + // Name is the group's name in the machine's group database; empty when the database names nothing for + // the number. + Name string + // Writable is whether the group's write bit is set on the path itself. + Writable bool +} + +// Words names the group for a person: its name and number, or its number alone. +func (g PathGroup) Words() string { + if g.Name == "" { + return fmt.Sprintf("gid %d", g.GID) + } + return fmt.Sprintf("%s (gid %d)", g.Name, g.GID) +} + +// GroupOfPath reads which group owns a path, by its number, its name and whether it may write. Reads only. A +// variable so a test can stand in for a machine whose groups it cannot make. +var GroupOfPath = func(path string) (PathGroup, error) { + info, err := os.Stat(path) + if err != nil { + return PathGroup{}, err + } + _, gid, ok := statOwner(info) + if !ok { + return PathGroup{}, fmt.Errorf("the group that owns %s cannot be read on this machine", path) + } + g := PathGroup{GID: gid, Writable: info.Mode().Perm()&0o020 != 0} + if found, err := user.LookupGroupId(strconv.Itoa(gid)); err == nil { + g.Name = found.Name + } + return g, nil +} + +// GrantRefusal is why a path's group is never granted (novox/hq ADR 0312 §4), or nil when it may be: group 0, +// or a group whose members act as root by membership alone. +func GrantRefusal(g PathGroup) error { + switch { + case g.GID == 0: + return fmt.Errorf("it is owned by group 0 (%s), and a read-write access never grants the root group: a "+ + "shared path is never a way to root (novox/hq ADR 0312 §4); nothing was granted, and nothing was "+ + "changed on it", orRoot(g.Name)) + case slices.Contains(accounts.RootGroups, g.Name): + return fmt.Errorf("it is owned by the group %s, whose members act as root, and a read-write access never "+ + "grants such a group: a shared path is never a way to root (novox/hq ADR 0312 §4); nothing was "+ + "granted, and nothing was changed on it", g.Words()) + } + return nil +} + +// CannotWriteWords is the reason an access whose group may not write its path is stated unhealthy with (novox/hq +// ADR 0312 §2). Never the path: the controller says a reason on the operator's channel, which withholds a path +// (ADR 0234 §6), and keeps the target as evidence. +func CannotWriteWords(g PathGroup) string { + return fmt.Sprintf("its group cannot write it: %s owns it without group write, and the mesh changes nothing on "+ + "an accessed path (novox/hq ADR 0312 §2) — whether its group may write is the operator's to decide", + g.Words()) +} + +func orRoot(name string) string { + if name == "" { + return "root" + } + return name +} + +// grant is what one read-write access grants, read once per apply. +type grant struct { + path string + group PathGroup + // refused is why nothing is granted: the path could not be read, or its group is never granted. + refused error +} + +// grantsOf reads the group of every read-write access that names a grant, by access id. One that is refused +// grants nothing — the access itself fails, saying why — and one whose path the engine refuses to place +// anything at (issue 339) is not read at all. +func grantsOf(resources []declaration.Resource, refused map[string]error) map[string]grant { + out := map[string]grant{} + for _, r := range resources { + a, ok := r.(*declaration.Access) + if !ok || a.Grant == nil || a.Mode != declaration.AccessReadWrite { + continue + } + if why := refused[filepath.Clean(a.Path)]; why != nil { + out[a.ID] = grant{path: a.Path, refused: why} + continue + } + g, err := GroupOfPath(a.Path) + if err == nil { + err = GrantRefusal(g) + } + out[a.ID] = grant{path: a.Path, group: g, refused: err} + } + return out +} + +// containerGroups is, for each container, the number of every group a read-write access grants it, sorted and +// once each: what it is created with as supplementary groups. +func containerGroups(resources []declaration.Resource, grants map[string]grant) map[string][]int { + out := map[string][]int{} + for _, r := range resources { + a, ok := r.(*declaration.Access) + if !ok || a.Grant == nil { + continue + } + g, granted := grants[a.ID] + if !granted || g.refused != nil { + continue + } + for _, id := range a.Grant.Containers { + if !slices.Contains(out[id], g.group.GID) { + out[id] = append(out[id], g.group.GID) + } + } + } + for id := range out { + sort.Ints(out[id]) + } + return out +} + +// ContainerGroupsOf is containerGroups for a whole declaration, read now: what the scheduler arms a scheduled +// step with, so a fire runs with the groups an apply would have created it with. +func ContainerGroupsOf(d *declaration.Declaration) map[string][]int { + if d == nil { + return nil + } + return containerGroups(d.Resources, grantsOf(d.Resources, nil)) +} + +// withGrants adds to what the declaration asks of each account the groups its read-write accesses grant (novox/hq +// ADR 0312 §5): one user resource or access going leaves an account in a group another still asks for. +func (w Wanted) withGrants(resources []declaration.Resource, grants map[string]grant) Wanted { + for _, r := range resources { + a, ok := r.(*declaration.Access) + if !ok || a.Grant == nil || a.Grant.Account == "" { + continue + } + g, granted := grants[a.ID] + if !granted || g.refused != nil || g.group.Name == "" { + continue + } + if w[a.Grant.Account] == nil { + w[a.Grant.Account] = map[string][]string{} + } + w[a.Grant.Account][g.group.Name] = append(w[a.Grant.Account][g.group.Name], a.ID) + } + return w +} + +// GrantedAccounts is every account a read-write access puts in its path's group, as the account judge reads +// them (novox/hq ADR 0312 §6): judged like a user resource's account groups, so a session of the account's own +// manager that began before is said *relogin needed*, and only there. Read now; a path whose group is refused +// or cannot be read asks nothing of the judge — the access says that itself. +func GrantedAccounts(d *declaration.Declaration, held map[string]bool) []accounts.Account { + if d == nil { + return nil + } + grants := grantsOf(d.Resources, nil) + var out []accounts.Account + for _, r := range d.Resources { + a, ok := r.(*declaration.Access) + if !ok || a.Grant == nil || a.Grant.Account == "" || held[a.ID] { + continue + } + g, granted := grants[a.ID] + if !granted || g.refused != nil || g.group.Name == "" { + continue + } + at := strings.LastIndex(a.ID, ".") + if at <= 0 || strings.HasPrefix(a.ID, declaration.AdoptionPrefix) { + continue + } + out = append(out, accounts.Account{Module: a.ID[:at], ID: a.ID, Name: a.Grant.Account, + Groups: []string{g.group.Name}}) + } + sort.Slice(out, func(i, j int) bool { return out[i].ID < out[j].ID }) + return out +} + +// grantAccess is applyAccess's grant (novox/hq ADR 0312): the account put in the path's group, what an earlier +// apply of this access put an account in and nobody asks for any more taken back, and the outcome said. The +// path itself is only read. +func grantAccess(ctx context.Context, sys system.System, r *declaration.Access, run Runner, previous store.Applied, + in inputs, out *Outcome) error { + var g grant + granting := r.Grant != nil && r.Mode == declaration.AccessReadWrite + if granting { + var read bool + g, read = in.grants[r.ID] + if !read { + // An apply that read nothing ahead — a caller with no inputs — reads it here. + g = grantsOf([]declaration.Resource{r}, in.refused)[r.ID] + } + if g.refused != nil { + return fmt.Errorf("%s grants nothing: %w", r.Path, g.refused) + } + } + account := "" + if granting { + account = r.Grant.Account + } + if account != "" && g.group.Name == "" { + return fmt.Errorf("%s is owned by %s, which this machine's group database names nothing, and an account "+ + "is put in a group by name: %s was not (novox/hq ADR 0312 §1)", r.Path, g.group.Words(), account) + } + + var said []string + moved := false + // What this access put an account in before. An access whose account changed gives the old one back + // everything it put it in; the new one starts with nothing recorded. + recorded := previous.Groups + if previous.User != "" && previous.User != account { + gave, words := takeBack(ctx, sys, previous.User, r.ID, previous.Groups, nil, run, in.groups) + moved = moved || gave + said = append(said, words...) + recorded = nil + } + var kept []string + if account != "" { + groups, err := system.GroupsOf(ctx, system.Runner(run), account) + if err != nil { + return err + } + inGroup := setOf(groups)[g.group.Name] + if !inGroup { + if err := sys.AddUserToGroup(ctx, system.Runner(run), account, g.group.Name); err != nil { + return err + } + moved = true + said = append(said, fmt.Sprintf("put %s in %s, the group that owns it; the tool runner holds it once "+ + "restarted, a session that began before only after a new login", account, g.group.Name)) + } + // Recorded only when the mesh put it there, now or before: a group held before is never taken back. + if !inGroup || slices.Contains(recorded, g.group.Name) { + kept = append(kept, g.group.Name) + } + var others []string + for _, x := range recorded { + if x != g.group.Name { + others = append(others, x) + } + } + gave, words := takeBack(ctx, sys, account, r.ID, others, &kept, run, in.groups) + moved = moved || gave + said = append(said, words...) + } + if len(kept) > 0 { + out.groups, out.user = kept, account + } + + if granting { + var to []string + if len(r.Grant.Containers) > 0 { + to = append(to, "its containers "+strings.Join(r.Grant.Containers, ", ")) + } + if account != "" { + to = append(to, account) + } + if len(to) > 0 { + out.Detail = joinDetail(out.Detail, fmt.Sprintf("grants %s to %s", g.group.Words(), strings.Join(to, " and "))) + } + if !g.group.Writable { + out.Detail = joinDetail(out.Detail, "its group cannot write it: stated unhealthy, and nothing was changed "+ + "on it — the operator decides") + } + } + if len(said) > 0 { + out.Detail = joinDetail(out.Detail, strings.Join(said, "; ")) + } + if moved { + // Updated exactly when the account's groups moved: what names this access under restart-on — the + // tool runner — is restarted to hold them (novox/hq ADR 0312 §6). + out.Action = "updated" + } + return nil +} + +// takeBack takes account out of every group of groups the mesh put it in for the resource id, unless another +// declared resource still asks for it — then it stays, and stays recorded on keep when keep is given, so it is +// taken back once nobody asks. A group it is no longer in is nothing to give back. Never fatal: a failure is +// said, and the group stays recorded where it can be. Whether any group was taken back, and what was said. +func takeBack(ctx context.Context, sys system.System, account, id string, groups []string, keep *[]string, + run Runner, wanted Wanted) (bool, []string) { + if len(groups) == 0 { + return false, nil + } + in, err := system.GroupsOf(ctx, system.Runner(run), account) + if err != nil { + if keep != nil { + *keep = append(*keep, groups...) + } + return false, []string{fmt.Sprintf("the groups the mesh put %s in (%s) were not given back: %v", account, + strings.Join(groups, ", "), err)} + } + already := setOf(in) + gave := false + var said []string + for _, g := range groups { + if !already[g] { + continue + } + if others := wanted.othersAsk(account, g, id); len(others) > 0 { + said = append(said, fmt.Sprintf("left %s in %s, which %s still asks for", account, g, strings.Join(others, ", "))) + if keep != nil { + *keep = append(*keep, g) + } + continue + } + gone, why := leaveGroup(ctx, sys, account, g, run) + if !gone && keep != nil { + *keep = append(*keep, g) + } + gave = gave || gone + said = append(said, account+" "+why) + } + return gave, said +} + +// removeAccess is an access no longer declared: the path left exactly as it is (novox/hq ADR 0051), and every +// group it put an account in taken back unless another declared resource still asks for it (ADR 0312 §5). +func removeAccess(ctx context.Context, sys system.System, a store.Applied, run Runner, wanted Wanted) (string, string) { + const left = "an operator-owned path is never the host's to remove" + if a.User == "" || len(a.Groups) == 0 { + return "forgotten", left + } + gave, said := takeBack(ctx, sys, a.User, a.ID, a.Groups, nil, run, wanted) + action := "forgotten" + if gave { + action = "restored" + } + return action, joinDetail(left, strings.Join(said, "; ")) +} + +// groupAddArgs is the runtime's arguments that give a container's process each granted group as a supplementary +// group, by number: the container's own group database is not the machine's, and a name could mean another +// group there, or none. +func groupAddArgs(groups []int) []string { + var args []string + for _, gid := range groups { + args = append(args, "--group-add", strconv.Itoa(gid)) + } + return args +} + +func gidWords(groups []int) string { + words := make([]string, 0, len(groups)) + for _, gid := range groups { + words = append(words, strconv.Itoa(gid)) + } + return strings.Join(words, ", ") +} diff --git a/internal/apply/grants_test.go b/internal/apply/grants_test.go new file mode 100644 index 0000000..8f3d176 --- /dev/null +++ b/internal/apply/grants_test.go @@ -0,0 +1,457 @@ +package apply + +import ( + "context" + "errors" + "fmt" + "os" + "os/user" + "path/filepath" + "strings" + "testing" + + "github.com/novox/mesh-host/internal/accounts" + "github.com/novox/mesh-host/internal/declaration" + "github.com/novox/mesh-host/internal/store" +) + +// Defends novox/hq ADR 0312 and issue 537: a read-write access grants its path's own group to the containers and +// the account the controller names, and the path is never changed; a read access grants nothing; the root group, +// and a group whose members act as root, are refused by name; what was granted is taken back as ADR 0252 takes +// back; a container is recreated with the group, and a unit of the machine's own manager running as the account +// is restarted once the account holds it. + +// grantMachine is the group database of groupDB, a container runtime that keeps each container's spec label and +// the arguments it was run with, and a service manager that notes, at each restart, the groups the account holds +// in the database then — which is what the machine's own manager gives the unit's new process. +type grantMachine struct { + *groupDB + containers map[string]string + runs map[string][]string + restarts []string +} + +func newGrantMachine(account string, in []string, exists ...string) *grantMachine { + g := newGroupDB(in, exists...) + g.account = account + return &grantMachine{groupDB: g, containers: map[string]string{}, runs: map[string][]string{}} +} + +func (m *grantMachine) run(ctx context.Context, name string, args ...string) (string, error) { + a := oldShape(name, args) + switch name { + case "docker": + m.asked = append(m.asked, name+" "+strings.Join(a, " ")) + switch a[0] { + case "info": + return "27.0\n", nil + case "container": + spec, there := m.containers[a[len(a)-1]] + if !there { + return "", errors.New("no such container") + } + return "true\t" + spec, nil + case "rm": + delete(m.containers, a[len(a)-1]) + case "run": + var named, spec string + for i := 0; i+1 < len(a); i++ { + switch { + case a[i] == "--name": + named = a[i+1] + case a[i] == "--label" && strings.HasPrefix(a[i+1], specLabel+"="): + spec = strings.TrimPrefix(a[i+1], specLabel+"=") + } + } + m.containers[named] = spec + m.runs[named] = a + return "deadbeef\n", nil + } + return "", nil + case "systemctl": + m.asked = append(m.asked, name+" "+strings.Join(a, " ")) + if a[0] == "restart" { + m.restarts = append(m.restarts, a[1]+": "+m.groups()) + } + return "", nil + } + return m.groupDB.run(ctx, name, args...) +} + +// groupOfPaths stands in for the machine's group database and the paths' groups for the length of a test. +func groupOfPaths(t *testing.T, groups map[string]PathGroup) { + t.Helper() + was := GroupOfPath + GroupOfPath = func(path string) (PathGroup, error) { + if _, err := os.Stat(path); err != nil { + return PathGroup{}, err + } + g, ok := groups[path] + if !ok { + return PathGroup{}, errors.New("no group in this test for " + path) + } + return g, nil + } + t.Cleanup(func() { GroupOfPath = was }) +} + +func accessGranting(id, path, account string, containers ...string) string { + grant := `"grant":{"account":"` + account + `"` + if len(containers) > 0 { + grant += `,"containers":["` + strings.Join(containers, `","`) + `"]` + } + return `{"id":"` + id + `","type":"access","path":"` + path + `","mode":"read-write",` + grant + `}}` +} + +func mounting(id, path string) string { + return `{"id":"` + id + `","type":"container","name":"` + strings.ReplaceAll(id, ".", "-") + `","image":"` + pinned + + `","volumes":["` + path + `:/library"]}` +} + +func applyGrants(t *testing.T, m *grantMachine, known store.State, resources ...string) (Report, store.State, error) { + t.Helper() + return Apply(context.Background(), archHost(t), parse(t, `{"declaration":1,"resources":[`+ + strings.Join(resources, ",")+`]}`), known, store.OriginDeclared, m.run, nil, nil) +} + +func statOf(t *testing.T, path string) string { + t.Helper() + info, err := os.Stat(path) + if err != nil { + t.Fatal(err) + } + uid, gid, _ := OwnerOf(info) + return fmt.Sprintf("%s %d:%d %s", info.Mode(), uid, gid, info.ModTime()) +} + +func TestAReadWriteAccessGrantsItsPathsGroupToTheContainerAndTheAccountAndLeavesThePath(t *testing.T) { + library := t.TempDir() + if err := os.WriteFile(filepath.Join(library, "album"), []byte("x"), 0o640); err != nil { + t.Fatal(err) + } + groupOfPaths(t, map[string]PathGroup{library: {GID: 1001, Name: "media", Writable: true}}) + before, beforeInside := statOf(t, library), statOf(t, filepath.Join(library, "album")) + m := newGrantMachine("operator", []string{"operator"}, "media") + + report, state, err := applyGrants(t, m, store.State{}, + accessGranting("importer.music", library, "operator", "importer.app"), mounting("importer.app", library)) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(" "+m.groups()+" ", " media ") { + t.Fatalf("the account was not put in the path's group: %q", m.groups()) + } + if run := strings.Join(m.runs["importer-app"], " "); !strings.Contains(run, "--group-add 1001") { + t.Fatalf("the container was not created with the path's group: %s", run) + } + o := outcomeOf(report, "importer.music") + if o.Action != "updated" || !strings.Contains(o.Detail, "grants media (gid 1001)") || + !strings.Contains(o.Detail, "put operator in media") { + t.Errorf("the access did not say what it granted: %+v", o) + } + if rec, _ := state.Find("importer.music"); rec.User != "operator" || strings.Join(rec.Groups, " ") != "media" { + t.Errorf("the access did not record the group it put the account in: %+v", rec) + } + // The path, and what is under it, exactly as they were (ADR 0312 §2). + if after := statOf(t, library); after != before { + t.Errorf("the accessed path changed: %s, was %s", after, before) + } + if after := statOf(t, filepath.Join(library, "album")); after != beforeInside { + t.Errorf("a file under the accessed path changed: %s, was %s", after, beforeInside) + } + for _, asked := range m.asked { + for _, verb := range []string{"chown", "chmod", "chgrp", "setfacl"} { + if strings.HasPrefix(asked, verb) || strings.Contains(asked, " "+verb+" ") { + t.Errorf("the engine asked the machine to change something: %q", asked) + } + } + } + + // Applied again: nothing moves, and nothing is recreated. + m.runs = map[string][]string{} + report, _, err = applyGrants(t, m, state, + accessGranting("importer.music", library, "operator", "importer.app"), mounting("importer.app", library)) + if err != nil { + t.Fatal(err) + } + if o := outcomeOf(report, "importer.music"); o.Action != "unchanged" { + t.Errorf("a second apply moved the access: %+v", o) + } + if len(m.runs) != 0 { + t.Errorf("a second apply recreated the container: %v", m.runs) + } +} + +func TestAReadAccessGrantsNothing(t *testing.T) { + library := t.TempDir() + groupOfPaths(t, map[string]PathGroup{library: {GID: 1001, Name: "media", Writable: true}}) + + // A grant on a read access is refused whole: the controller asked for something the decision forbids. + _, err := declaration.Parse([]byte(`{"declaration":1,"resources":[{"id":"player.music","type":"access","path":"` + + library + `","mode":"read","grant":{"account":"operator"}}]}`)) + if err == nil || !strings.Contains(err.Error(), "a read access grants nothing") { + t.Fatalf("a grant on a read access was not refused by name: %v", err) + } + + m := newGrantMachine("operator", []string{"operator"}, "media") + report, state, err := applyGrants(t, m, store.State{}, + `{"id":"player.music","type":"access","path":"`+library+`","mode":"read"}`, mounting("player.app", library)) + if err != nil { + t.Fatal(err) + } + if strings.Contains(m.groups(), "media") { + t.Errorf("a read access put the account in a group: %q", m.groups()) + } + if run := strings.Join(m.runs["player-app"], " "); strings.Contains(run, "--group-add") { + t.Errorf("a read access gave its container a group: %s", run) + } + if o := outcomeOf(report, "player.music"); o.Action != "unchanged" { + t.Errorf("a read access moved something: %+v", o) + } + if rec, _ := state.Find("player.music"); len(rec.Groups) != 0 || rec.User != "" { + t.Errorf("a read access recorded a grant: %+v", rec) + } +} + +func TestAnAccessWhoseGroupCannotWriteIsGrantedAndSaysTheOperatorDecides(t *testing.T) { + spool := t.TempDir() + groupOfPaths(t, map[string]PathGroup{spool: {GID: 1002, Name: "downloads", Writable: false}}) + before := statOf(t, spool) + m := newGrantMachine("operator", nil, "downloads") + + report, _, err := applyGrants(t, m, store.State{}, + accessGranting("nzbget.downloads", spool, "operator", "nzbget.app"), mounting("nzbget.app", spool)) + if err != nil { + t.Fatal(err) + } + o := outcomeOf(report, "nzbget.downloads") + if !strings.Contains(o.Detail, "its group cannot write it") || !strings.Contains(o.Detail, "the operator decides") || + !strings.Contains(o.Detail, "nothing was changed") { + t.Errorf("the access did not say its group cannot write and that the operator decides: %+v", o) + } + if after := statOf(t, spool); after != before { + t.Errorf("the engine changed a path whose group cannot write: %s, was %s", after, before) + } + // The words the look states it unhealthy with: the remedy is the operator's, and the path is never in them. + words := CannotWriteWords(PathGroup{GID: 1002, Name: "downloads"}) + if !strings.Contains(words, "the operator's to decide") || strings.Contains(words, spool) { + t.Errorf("the unhealthy reason is %q", words) + } +} + +func TestTheRootGroupIsNeverGranted(t *testing.T) { + for _, g := range []PathGroup{{GID: 0, Name: "root", Writable: true}, {GID: 968, Name: "docker", Writable: true}, + {GID: 10, Name: "wheel", Writable: true}} { + t.Run(g.Name, func(t *testing.T) { + shared := t.TempDir() + groupOfPaths(t, map[string]PathGroup{shared: g}) + m := newGrantMachine("operator", nil, g.Name) + + report, _, err := applyGrants(t, m, store.State{}, + accessGranting("importer.shared", shared, "operator", "importer.app"), mounting("importer.app", shared)) + if err == nil { + t.Fatal("a read-write access to a path of a root-granting group applied") + } + var failed *Error + if !errors.As(err, &failed) || failed.Resource != "importer.shared" || + !strings.Contains(err.Error(), g.Name) || !strings.Contains(err.Error(), "never") { + t.Errorf("the refusal does not name the access and the group: %v", err) + } + if strings.Contains(m.groups(), g.Name) { + t.Errorf("the account was put in %s", g.Name) + } + if run := strings.Join(m.runs["importer-app"], " "); strings.Contains(run, "--group-add") { + t.Errorf("the container was given a root-granting group: %s", run) + } + _ = report + }) + } +} + +func TestAGrantedGroupIsTakenBackUnlessStillAskedForOrHeldBefore(t *testing.T) { + music, movies := t.TempDir(), t.TempDir() + groupOfPaths(t, map[string]PathGroup{music: {GID: 1001, Name: "media", Writable: true}, + movies: {GID: 1001, Name: "media", Writable: true}}) + + t.Run("no longer asked", func(t *testing.T) { + m := newGrantMachine("operator", []string{"operator"}, "media") + _, state, err := applyGrants(t, m, store.State{}, accessGranting("importer.music", music, "operator")) + if err != nil { + t.Fatal(err) + } + report, state, err := applyGrants(t, m, state, mounting("other.app", t.TempDir())) + if err != nil { + t.Fatal(err) + } + if strings.Contains(m.groups(), "media") { + t.Errorf("the group the mesh granted was not taken back: %q", m.groups()) + } + if o := outcomeOf(report, "importer.music"); o.Action != "restored" || !strings.Contains(o.Detail, "taken out of media") { + t.Errorf("the removal did not say it took the group back: %+v", o) + } + if _, still := state.Find("importer.music"); still { + t.Error("the access is still recorded") + } + }) + + t.Run("still asked by another access", func(t *testing.T) { + m := newGrantMachine("operator", []string{"operator"}, "media") + _, state, err := applyGrants(t, m, store.State{}, accessGranting("importer.music", music, "operator"), + accessGranting("sonarr.movies", movies, "operator")) + if err != nil { + t.Fatal(err) + } + report, _, err := applyGrants(t, m, state, accessGranting("sonarr.movies", movies, "operator")) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(m.groups(), "media") { + t.Errorf("a group another access still grants was taken back: %q", m.groups()) + } + if o := outcomeOf(report, "importer.music"); !strings.Contains(o.Detail, "still asks for") { + t.Errorf("the removal did not say why the group stayed: %+v", o) + } + }) + + t.Run("still asked by a user resource", func(t *testing.T) { + m := newGrantMachine("operator", []string{"operator"}, "media") + _, state, err := applyGrants(t, m, store.State{}, accessGranting("importer.music", music, "operator")) + if err != nil { + t.Fatal(err) + } + if _, _, err := applyGrants(t, m, state, + `{"id":"player.account","type":"user","name":"operator","groups":["media"]}`); err != nil { + t.Fatal(err) + } + if !strings.Contains(m.groups(), "media") { + t.Errorf("a group a user resource asks for was taken back: %q", m.groups()) + } + }) + + t.Run("held before", func(t *testing.T) { + m := newGrantMachine("operator", []string{"operator", "media"}) + _, state, err := applyGrants(t, m, store.State{}, accessGranting("importer.music", music, "operator")) + if err != nil { + t.Fatal(err) + } + if rec, _ := state.Find("importer.music"); len(rec.Groups) != 0 { + t.Fatalf("a group the account held before was recorded as the mesh's: %+v", rec) + } + if _, _, err := applyGrants(t, m, state, mounting("other.app", t.TempDir())); err != nil { + t.Fatal(err) + } + if !strings.Contains(m.groups(), "media") { + t.Errorf("a group the account held before the mesh was taken back: %q", m.groups()) + } + }) +} + +func TestAContainerIsRecreatedWithTheGroupItIsGranted(t *testing.T) { + library := t.TempDir() + groupOfPaths(t, map[string]PathGroup{library: {GID: 1001, Name: "media", Writable: true}}) + m := newGrantMachine("operator", nil, "media") + + // Running before its access granted anything: a controller older than the grant. + _, state, err := applyGrants(t, m, store.State{}, + `{"id":"importer.music","type":"access","path":"`+library+`","mode":"read-write"}`, mounting("importer.app", library)) + if err != nil { + t.Fatal(err) + } + if run := strings.Join(m.runs["importer-app"], " "); strings.Contains(run, "--group-add") { + t.Fatalf("a container granted nothing was given a group: %s", run) + } + report, _, err := applyGrants(t, m, state, + accessGranting("importer.music", library, "", "importer.app"), mounting("importer.app", library)) + if err != nil { + t.Fatal(err) + } + o := outcomeOf(report, "importer.app") + if o.Action != "updated" || !strings.Contains(o.Detail, "recreated to hold the groups") { + t.Errorf("the container was not recreated for its group: %+v", o) + } + if run := strings.Join(m.runs["importer-app"], " "); !strings.Contains(run, "--group-add 1001") { + t.Errorf("the recreated container does not hold the group: %s", run) + } +} + +func TestTheToolRunnerIsRestartedOnceItsAccountHoldsTheGroup(t *testing.T) { + me, err := user.Current() + if err != nil { + t.Skip("no current account to run the tool runner as") + } + units, bundles := t.TempDir(), t.TempDir() + wasUnits, wasBundles := unitDir, daemonRoot + unitDir, daemonRoot = units, bundles + t.Cleanup(func() { unitDir, daemonRoot = wasUnits, wasBundles }) + library := t.TempDir() + groupOfPaths(t, map[string]PathGroup{library: {GID: 1001, Name: "media", Writable: true}}) + m := newGrantMachine(me.Username, nil, "media") + + body, digest := anArchive(t, map[string]string{"main.js": "console.log(1)\n"}) + runner := `{"id":"node-tools.runtime","type":"process","name":"node-tools","source":"` + serving(t, body) + + `","digest":"` + digest + `","run":["node","main.js"],"user":"` + me.Username + + `","restart-on":["importer.music"]}` + // Running before the grant. + _, state, err := applyGrants(t, m, store.State{}, runner) + if err != nil { + t.Fatal(err) + } + m.restarts = nil + report, state, err := applyGrants(t, m, state, accessGranting("importer.music", library, me.Username), runner) + if err != nil { + t.Fatal(err) + } + if len(m.restarts) != 1 || !strings.HasPrefix(m.restarts[0], "node-tools.service: ") || + !strings.Contains(m.restarts[0], "media") { + t.Fatalf("the tool runner was not restarted once its account held the group: %v", m.restarts) + } + if o := outcomeOf(report, "node-tools.runtime"); !strings.Contains(o.Detail, "importer.music") { + t.Errorf("the restart does not say what it was for: %+v", o) + } + // And not again while nothing moves. + m.restarts = nil + if _, _, err := applyGrants(t, m, state, accessGranting("importer.music", library, me.Username), runner); err != nil { + t.Fatal(err) + } + if len(m.restarts) != 0 { + t.Errorf("the tool runner was restarted with nothing moved: %v", m.restarts) + } +} + +// sessionReader is an account whose own service manager runs, holding the groups it began with. +type sessionReader struct{ database, session []string } + +func (s sessionReader) InDatabase(context.Context, string) ([]string, error) { return s.database, nil } +func (s sessionReader) Session(_ context.Context, _ string, groups []string) (accounts.Session, error) { + has := map[string]bool{} + for _, g := range s.session { + has[g] = true + } + return accounts.Session{Running: true, Has: has}, nil +} +func (s sessionReader) Escalation(context.Context, string, []string) ([]string, error) { + return nil, nil +} + +func TestAnAccountsOwnSessionIsSaidReloginNeededForAGrantedGroup(t *testing.T) { + library := t.TempDir() + groupOfPaths(t, map[string]PathGroup{library: {GID: 1001, Name: "media", Writable: true}}) + d := parse(t, `{"declaration":1,"resources":[`+accessGranting("importer.music", library, "operator")+`]}`) + + as := GrantedAccounts(d, nil) + if len(as) != 1 || as[0].Module != "importer" || as[0].Name != "operator" || strings.Join(as[0].Groups, " ") != "media" { + t.Fatalf("the account the access grants is not judged: %+v", as) + } + j := accounts.New(sessionReader{database: []string{"operator", "media"}, session: []string{"operator"}}) + j.Set(as) + st, _ := j.Look(context.Background()) + if len(st.Accounts) != 1 || !strings.HasPrefix(st.Accounts[0].Reason, accounts.ReasonRelogin) { + t.Errorf("a session of the account's own manager that began before the grant is not said relogin needed: %+v", + st.Accounts) + } + // A read access, or one refused, asks nothing of the judge. + groupOfPaths(t, map[string]PathGroup{library: {GID: 0, Name: "root", Writable: true}}) + if as := GrantedAccounts(d, nil); len(as) != 0 { + t.Errorf("a refused grant is judged: %+v", as) + } +} diff --git a/internal/apply/mesh_names_test.go b/internal/apply/mesh_names_test.go index 794911f..54ff8cd 100644 --- a/internal/apply/mesh_names_test.go +++ b/internal/apply/mesh_names_test.go @@ -23,7 +23,7 @@ func TestAContainersMeshNamesAreComparedLikeTheRestOfIt(t *testing.T) { Name: was.Name, Image: was.Image, Hosts: []string{"novox.internal:10.10.0.1", "umami.novox.be:10.10.0.1"}, } - if containerSpecReading(was, nil, nil) == containerSpecReading(moved, nil, nil) { + if containerSpecReading(was, nil, nil, nil) == containerSpecReading(moved, nil, nil, nil) { t.Fatal("a container whose mesh names moved compares equal, so it is never recreated") } @@ -33,14 +33,14 @@ func TestAContainersMeshNamesAreComparedLikeTheRestOfIt(t *testing.T) { Name: moved.Name, Image: moved.Image, Hosts: []string{moved.Hosts[1], moved.Hosts[0]}, } - if containerSpecReading(moved, nil, nil) != containerSpecReading(reordered, nil, nil) { + if containerSpecReading(moved, nil, nil, nil) != containerSpecReading(reordered, nil, nil, nil) { t.Fatal("the same names in another order read as a different container") } // A container the mesh gives no names is unaffected, so nothing is recreated for a field it // does not set. plain := &declaration.Container{Name: "plex", Image: was.Image} - if containerSpecReading(plain, nil, nil) == containerSpecReading(was, nil, nil) { + if containerSpecReading(plain, nil, nil, nil) == containerSpecReading(was, nil, nil, nil) { return // different for other reasons, which is fine } } diff --git a/internal/apply/plan.go b/internal/apply/plan.go index c632b88..9967a2f 100644 --- a/internal/apply/plan.go +++ b/internal/apply/plan.go @@ -111,6 +111,11 @@ func Plan(d *declaration.Declaration, known store.State, origin string) []Step { // In removeWhole's words (novox/hq ADR 0118): written over, so given back, not deleted. step.Verb, step.Why = "restore", "no longer declared; the original the mesh wrote over goes back "+ "from "+orphan.Kept+", unless the file was changed since the mesh last wrote it" + case orphan.Type == string(declaration.TypeAccess) && orphan.User != "" && len(orphan.Groups) > 0: + // In removeAccess's words (novox/hq ADR 0312 §5). + step.Verb, step.Why = "restore", "no longer declared; the path is left as it is, and "+orphan.User+ + " leaves "+strings.Join(orphan.Groups, ", ")+", which the mesh put it in, unless another declared "+ + "resource still asks for it" case orphan.Type == string(declaration.TypeService): // What removal will do, said before it does it (novox/hq ADR 0118), in removeService's // words. "restore" only where it may stop or disable something — the record cannot say diff --git a/internal/apply/reads_test.go b/internal/apply/reads_test.go index be2d8fe..deaf6f3 100644 --- a/internal/apply/reads_test.go +++ b/internal/apply/reads_test.go @@ -212,7 +212,7 @@ func TestAContainerLabelledBeforeTheHostReadItsFilesIsAcceptedNotRecreated(t *te t.Fatal(err) } d := declare("5432") - legacy := containerSpecReading(d.Resources[1].(*declaration.Container), nil, nil) + legacy := containerSpecReading(d.Resources[1].(*declaration.Container), nil, nil, nil) known := store.State{} known.Record(store.Applied{ID: "forge.env", Type: "file", Origin: store.OriginCarried, Target: env, Wrote: digestOf("DATABASE_PORT=5432\n")}) diff --git a/internal/apply/schedule.go b/internal/apply/schedule.go index 2d8e969..690e032 100644 --- a/internal/apply/schedule.go +++ b/internal/apply/schedule.go @@ -73,6 +73,9 @@ type scheduledJob struct { // hold is the runtime names of the containers held still for the duration of a run, in the // order the step named them (novox/hq ADR 0189). hold []string + // groups is the number of every group its read-write accesses grant it, read when it was armed (novox/hq + // ADR 0312): a fire runs with them as an apply's container would. + groups []int } // heldStillFor is the runtime names of the containers a step holds still, resolved from ids. @@ -131,6 +134,14 @@ func (s *Scheduler) Sync(d *declaration.Declaration, held map[string]bool) { defer s.mu.Unlock() seen := map[string]bool{} + // The groups each step's read-write accesses grant (novox/hq ADR 0312), read once, only when a step is here. + var granted map[string][]int + for _, r := range d.Resources { + if c, ok := r.(*declaration.Container); ok && c.Schedule != "" { + granted = ContainerGroupsOf(d) + break + } + } for _, r := range d.Resources { c, ok := r.(*declaration.Container) if !ok || c.Schedule == "" { @@ -158,7 +169,7 @@ func (s *Scheduler) Sync(d *declaration.Declaration, held map[string]bool) { // Nothing to read: a scheduled container may not declare restart-on — it runs to completion // on its cadence rather than staying running to be restarted — so its identity cannot // depend on another resource's content and there is nothing to pass. - spec := containerSpec(c, inputs{}) + spec := containerSpec(c, inputs{containerGroups: granted}) // The runtime stops containers by name; the declaration names them by id. Resolved here, // against the declaration this job was armed from, so a fire never has to look anything up // (novox/hq ADR 0189). The parser has already refused an id that is not a container here. @@ -167,12 +178,14 @@ func (s *Scheduler) Sync(d *declaration.Declaration, held map[string]bool) { // Unchanged: keep where it is in its cadence, refresh the declaration pointer only. existing.container = c existing.hold = hold + existing.groups = granted[c.Identity()] continue } // New or changed: arm it for the next due minute after now. next, _ := cron.Next(s.clock.Now()) s.jobs[c.Identity()] = &scheduledJob{ id: c.Identity(), spec: spec, cron: cron, container: c, next: next, hold: hold, + groups: granted[c.Identity()], } } @@ -282,7 +295,7 @@ func (s *Scheduler) fire(ctx context.Context, j *scheduledJob) { // one that is not there is the state we want, so its error is ignored — the same as run-once. _, _ = s.run(ctx, cri, "rm", "-f", j.container.Name) - args := foregroundRunArgs(j.container, j.spec) + args := foregroundRunArgs(j.container, j.spec, j.groups) if _, err := s.run(ctx, cri, args...); err != nil { s.log(fmt.Sprintf( "scheduled step %s: run exited non-zero — recorded, and left for the next run "+ diff --git a/internal/apply/vocabulary_test.go b/internal/apply/vocabulary_test.go index 5fc8257..882d056 100644 --- a/internal/apply/vocabulary_test.go +++ b/internal/apply/vocabulary_test.go @@ -447,11 +447,11 @@ func TestAChangedResolverOrAddressIsAChangedContainer(t *testing.T) { base := &declaration.Container{ID: "c", Name: "x", Image: "a@sha256:00"} withDns := &declaration.Container{ID: "c", Name: "x", Image: "a@sha256:00", Dns: []string{"192.168.203.254"}} withIP := &declaration.Container{ID: "c", Name: "x", Image: "a@sha256:00", IP: "192.168.203.254"} - plain := containerSpecReading(base, nil, nil) - if containerSpecReading(withDns, nil, nil) == plain { + plain := containerSpecReading(base, nil, nil, nil) + if containerSpecReading(withDns, nil, nil, nil) == plain { t.Error("adding a resolver did not change the spec, so it can never reach a running container") } - if containerSpecReading(withIP, nil, nil) == plain { + if containerSpecReading(withIP, nil, nil, nil) == plain { t.Error("adding an address did not change the spec, so it can never reach a running container") } } diff --git a/internal/declaration/declaration.go b/internal/declaration/declaration.go index 50db892..e4318f9 100644 --- a/internal/declaration/declaration.go +++ b/internal/declaration/declaration.go @@ -464,6 +464,35 @@ type Access struct { Path string `json:"path"` // Mode is how this module reaches the path: read or read-write. Absent narrows to read. Mode string `json:"mode,omitempty"` + + // Grant is who the path's own group is granted to, for a read-write access (novox/hq ADR 0312 §1): the + // identities the module's code runs as. The controller says who; the node-engine reads which group owns + // the path when it applies, because only the machine knows, and never changes the path (§2). Absent is a + // controller older than the grant, or an access that grants nothing — and a read access always is one (§3). + Grant *AccessGrant `json:"grant,omitempty"` +} + +// AccessGrant is the identities a read-write access grants its path's group to (novox/hq ADR 0312 §1). +type AccessGrant struct { + // Containers are the module's containers that mount the path, by id: each is created with the group as a + // supplementary group of its process, and recreated when the group moves (§6). + Containers []string `json:"containers,omitempty"` + // Account is the account that runs the module's tools: put in the group as an account group, as ADR 0252 + // §1 puts one, and taken out again as §3 takes back (§5). Empty for a module whose code runs nowhere as + // an account. + Account string `json:"account,omitempty"` +} + +// UnmarshalJSON reads a grant as strictly as a resource is read: a field the engine does not know is a +// thing the controller believes it asked for, and is refused rather than ignored. +func (g *AccessGrant) UnmarshalJSON(raw []byte) error { + type plain AccessGrant + var p plain + if err := strictDecode(raw, &p); err != nil { + return fmt.Errorf("a grant: %w", err) + } + *g = AccessGrant(p) + return nil } func (a *Access) Identity() string { return a.ID } @@ -482,6 +511,22 @@ func (a *Access) validate(where string, _ bool) []string { problems = append(problems, fmt.Sprintf( "%s: an access is %q or %q, not %q", where, AccessRead, AccessReadWrite, a.Mode)) } + if a.Grant != nil && a.Mode != AccessReadWrite { + // A read access reads as any account may, and gains no group (novox/hq ADR 0312 §3). + problems = append(problems, where+": a read access grants nothing, and this one names a grant; "+ + "only a read-write access grants its path's group (novox/hq ADR 0312 §3)") + } + if a.Grant != nil { + for _, id := range a.Grant.Containers { + if id == "" { + problems = append(problems, where+": a grant names a container with no id") + } + } + if a.Grant.Account != "" && strings.ContainsAny(a.Grant.Account, " :/\t\n") { + problems = append(problems, where+": a grant's account is an account's name, and "+ + a.Grant.Account+" is not one") + } + } return problems } @@ -1668,6 +1713,7 @@ func parse(raw []byte, allowActions bool) (*Declaration, error) { d.Resources = append(d.Resources, resource) } problems = append(problems, checkWhileStopped(d.Resources)...) + problems = append(problems, checkGrants(d.Resources)...) problems = append(problems, checkAdoption(env.Adoption, d.Resources, allowActions)...) problems = append(problems, checkReplaces(d.Resources)...) if env.Adoption == nil { @@ -1732,6 +1778,32 @@ func checkWhileStopped(resources []Resource) []string { return problems } +// checkGrants judges each access's grant against the whole declaration (novox/hq ADR 0312): a container it +// names is a container of this declaration, as a maintenance window's are. A grant to a container the machine +// is not told about would be a group given to nothing, said as granted. +func checkGrants(resources []Resource) []string { + containers := map[string]bool{} + for _, r := range resources { + if r.Kind() == TypeContainer { + containers[r.Identity()] = true + } + } + var problems []string + for _, r := range resources { + a, ok := r.(*Access) + if !ok || a.Grant == nil { + continue + } + for _, id := range a.Grant.Containers { + if id != "" && !containers[id] { + problems = append(problems, fmt.Sprintf("resource %q: its grant names %q, and this declaration "+ + "has no container by that id", a.ID, id)) + } + } + } + return problems +} + func strictDecode(raw []byte, into any) error { // DisallowUnknownFields is the whole point rather than strictness for its own sake: a // field the host does not know is a thing the control plane believes it asked for. diff --git a/internal/link/messages.go b/internal/link/messages.go index 0183930..d34c744 100644 --- a/internal/link/messages.go +++ b/internal/link/messages.go @@ -246,6 +246,13 @@ const ReadinessContract = 2 // refuses the whole declaration for a field it does not know. const RootContract = 3 +// GrantContract is the statement of a host that also reads an access's `grant` (novox/hq ADR 0312): it grants a +// read-write access's path group to the containers and the account the grant names, takes it back, and states the +// access unhealthy while its group cannot write. Like the fields before it, its presence is what tells the +// controller this host may be sent `grant`; an older host refuses the whole declaration for a field it does not +// know. +const GrantContract = 4 + // Health is one statement of every long-running resource's state on this machine (to-be 48 §4). Said in // every report, as an event on each change, and again every minute while anything is not healthy — so a // lost statement is not a lost fault. @@ -284,6 +291,11 @@ const KindAccount = "account" // starting ReasonUsedAsFound, until the operator hands it to the mesh (`nox node hand-over ` on the control-node, novox/hq issue 356). const KindDirectory = "directory" +// KindAccess is a read-write access that grants its path's group (novox/hq ADR 0312): healthy while the group may +// write the path, unhealthy while it cannot — the operator's to decide, since the mesh changes nothing on the path +// (§2) — or while the path is owned by a group that is never granted (§4). +const KindAccess = "access" + // ReasonUsedAsFound starts the reason of a directory used as found. const ReasonUsedAsFound = "used as found:" From c433e76de2295d52863289228e030f1829c0a991 Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 12 Oct 2026 12:53:42 +0200 Subject: [PATCH 2/2] Judge a granted group by number, grant nothing a group cannot write, and keep the look bounded (hq ADR 0312, issue 537 review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The engine is built without cgo, so a check by name missed a root group a name service holds or one number carrying another name: the root groups are resolved to numbers (now with kmem and shadow) and a number named nothing is refused for containers too. A group without write is granted nothing (§2). The look reads each path off the lock within five seconds, so a dead network mount cannot stop every health statement. A grant's account may not start with a dash. A group a record leaves for another asker is handed to it, so it is still taken back when the last asker goes (§5). The scheduler skips paths issue 339's guard refuses. --- cmd/mesh-host/grants.go | 72 ++++++++++-- cmd/mesh-host/grants_test.go | 62 ++++++++++- internal/accounts/accounts.go | 8 +- internal/apply/apply.go | 14 ++- internal/apply/grants.go | 167 +++++++++++++++++++++++----- internal/apply/grants_test.go | 114 ++++++++++++++++++- internal/apply/groups.go | 13 ++- internal/apply/user.go | 8 +- internal/declaration/declaration.go | 12 +- 9 files changed, 409 insertions(+), 61 deletions(-) diff --git a/cmd/mesh-host/grants.go b/cmd/mesh-host/grants.go index 0a275bc..892ca57 100644 --- a/cmd/mesh-host/grants.go +++ b/cmd/mesh-host/grants.go @@ -34,10 +34,18 @@ type accessJudge struct { accesses []grantedAccess // was is each access's last verdict, with since when and how many looks in a row found it unhealthy. was map[string]link.ResourceHealth + + // reading guards inFlight: every path whose read has not returned yet. + reading sync.Mutex + inFlight map[string]bool } // accesses is the one access judge on this machine. -var accesses = &accessJudge{was: map[string]link.ResourceHealth{}} +var accesses = newAccessJudge() + +func newAccessJudge() *accessJudge { + return &accessJudge{was: map[string]link.ResourceHealth{}, inFlight: map[string]bool{}} +} // set is what the declaration just applied asks: every read-write access of a module that names a grant, not // held as found. An access no longer declared is forgotten. @@ -71,17 +79,58 @@ func (j *accessJudge) set(d *declaration.Declaration, held map[string]bool) { } } +// lookBound is how long a look waits for one accessed path to answer. A hard network mount whose server is down +// blocks a stat for ever, and one access must never stop every health statement this machine makes. +var lookBound = 5 * time.Second + +// pathGroupBounded reads a path's group within lookBound, outside any lock. A read that has not answered is said +// so; while it still hangs, later looks do not start another on the same path, so a dead mount costs one blocked +// read, not one per look. +func (j *accessJudge) pathGroupBounded(path string) (apply.PathGroup, bool, error) { + j.reading.Lock() + if j.inFlight[path] { + j.reading.Unlock() + return apply.PathGroup{}, false, nil + } + j.inFlight[path] = true + j.reading.Unlock() + type read struct { + g apply.PathGroup + err error + } + done := make(chan read, 1) + go func() { + g, err := apply.GroupOfPath(path) + j.reading.Lock() + delete(j.inFlight, path) + j.reading.Unlock() + done <- read{g, err} + }() + select { + case r := <-done: + return r.g, true, r.err + case <-time.After(lookBound): + return apply.PathGroup{}, false, nil + } +} + // look reads every access now: healthy when its group may write and may be granted, unhealthy saying why -// otherwise. Whether any verdict changed since the last look, for the loop to say it. +// otherwise. Whether any verdict changed since the last look, for the loop to say it. The paths are read with no +// lock held, each within lookBound. func (j *accessJudge) look(now time.Time) ([]link.ResourceHealth, bool) { j.mu.Lock() - defer j.mu.Unlock() - changed := false - out := make([]link.ResourceHealth, 0, len(j.accesses)) - for _, a := range j.accesses { + as := append([]grantedAccess(nil), j.accesses...) + j.mu.Unlock() + + type verdict struct{ state, reason string } + read := make([]verdict, len(as)) + for i, a := range as { state, reason := link.StateHealthy, "" - g, err := apply.GroupOfPath(a.path) + g, answered, err := j.pathGroupBounded(a.path) switch { + case !answered: + state, reason = link.StateUnhealthy, "its path does not answer: reading which group owns it did not "+ + "return within "+lookBound.String() case err != nil: state, reason = link.StateUnhealthy, "its group cannot be read: "+strings.ReplaceAll(err.Error(), a.path, "the path") case apply.GrantRefusal(g) != nil: @@ -89,6 +138,15 @@ func (j *accessJudge) look(now time.Time) ([]link.ResourceHealth, bool) { case !g.Writable: state, reason = link.StateUnhealthy, apply.CannotWriteWords(g) } + read[i] = verdict{state, reason} + } + + j.mu.Lock() + defer j.mu.Unlock() + changed := false + out := make([]link.ResourceHealth, 0, len(as)) + for i, a := range as { + state, reason := read[i].state, read[i].reason v := link.ResourceHealth{Module: a.module, Resource: a.id, Kind: link.KindAccess, Target: a.path, State: state, Reason: reason, Since: now.UTC()} if was, had := j.was[a.id]; had && was.State == state && was.Reason == reason { diff --git a/cmd/mesh-host/grants_test.go b/cmd/mesh-host/grants_test.go index 51510de..27f1227 100644 --- a/cmd/mesh-host/grants_test.go +++ b/cmd/mesh-host/grants_test.go @@ -2,6 +2,7 @@ package main import ( "strings" + "sync/atomic" "testing" "time" @@ -23,6 +24,9 @@ func TestAnAccessWhoseGroupCannotWriteIsStatedUnhealthyWithTheOperatorsRemedy(t was := apply.GroupOfPath apply.GroupOfPath = func(path string) (apply.PathGroup, error) { return groups[path], nil } t.Cleanup(func() { apply.GroupOfPath = was }) + wasRoot := apply.RootGroupIDs + apply.RootGroupIDs = func() map[int]string { return map[int]string{0: "root"} } + t.Cleanup(func() { apply.RootGroupIDs = wasRoot }) d, err := declaration.Parse([]byte(`{"declaration":1,"resources":[ {"id":"nzbget.downloads","type":"access","path":"` + spool + `","mode":"read-write","grant":{"account":"operator"}}, @@ -32,7 +36,7 @@ func TestAnAccessWhoseGroupCannotWriteIsStatedUnhealthyWithTheOperatorsRemedy(t if err != nil { t.Fatal(err) } - j := &accessJudge{was: map[string]link.ResourceHealth{}} + j := newAccessJudge() j.set(d, nil) at := time.Date(2026, 10, 12, 12, 0, 0, 0, time.UTC) vs, changed := j.look(at) @@ -76,3 +80,59 @@ func TestAnAccessWhoseGroupCannotWriteIsStatedUnhealthyWithTheOperatorsRemedy(t } } } + +// A path that does not answer — a hard network mount whose server is down — is stated unhealthy within the bound, +// and stops neither this look nor the next: one blocked read per path, never one per look. +func TestAnAccessWhosePathDoesNotAnswerIsStatedWithinTheBound(t *testing.T) { + hung, library := t.TempDir(), t.TempDir() + release, finished := make(chan struct{}), make(chan struct{}) + var started atomic.Int32 + was, wasBound := apply.GroupOfPath, lookBound + apply.GroupOfPath = func(path string) (apply.PathGroup, error) { + if path == hung { + started.Add(1) + <-release + defer close(finished) + } + return apply.PathGroup{GID: 1001, Name: "media", Writable: true}, nil + } + lookBound = 50 * time.Millisecond + wasRoot := apply.RootGroupIDs + apply.RootGroupIDs = func() map[int]string { return map[int]string{0: "root"} } + t.Cleanup(func() { apply.GroupOfPath, lookBound, apply.RootGroupIDs = was, wasBound, wasRoot }) + // Run first: the read that hangs is let go and has returned before anything it read is put back. + t.Cleanup(func() { close(release); <-finished }) + + d, err := declaration.Parse([]byte(`{"declaration":1,"resources":[ + {"id":"nzbget.downloads","type":"access","path":"` + hung + `","mode":"read-write","grant":{}}, + {"id":"importer.music","type":"access","path":"` + library + `","mode":"read-write","grant":{}}]}`)) + if err != nil { + t.Fatal(err) + } + j := newAccessJudge() + j.set(d, nil) + for look := 0; look < 3; look++ { + done := make(chan []link.ResourceHealth, 1) + go func() { vs, _ := j.look(time.Now()); done <- vs }() + select { + case vs := <-done: + for _, v := range vs { + switch v.Resource { + case "nzbget.downloads": + if v.State != link.StateUnhealthy || !strings.Contains(v.Reason, "does not answer") { + t.Errorf("look %d: a path that does not answer is stated %+v", look, v) + } + case "importer.music": + if v.State != link.StateHealthy { + t.Errorf("look %d: the path that answers is stated %+v", look, v) + } + } + } + case <-time.After(5 * time.Second): + t.Fatalf("look %d did not return while one path hangs", look) + } + } + if n := started.Load(); n != 1 { + t.Errorf("%d reads were started on the path that hangs; want one", n) + } +} diff --git a/internal/accounts/accounts.go b/internal/accounts/accounts.go index f72169e..1316527 100644 --- a/internal/accounts/accounts.go +++ b/internal/accounts/accounts.go @@ -65,9 +65,11 @@ const ReasonRoot = "can become root without a person" // RootGroups are the groups whose members become root by membership alone, without a sudo rule naming // them: the administrators' groups a distribution's own rules or polkit treat as root, the container // runtime's socket (a container with the host's root mounted), the raw disks, and the virtualisation -// daemons that start a guest with the host's devices. A closed list, so what the judge calls "grants root" -// is written down and reviewable rather than guessed per machine. -var RootGroups = []string{"root", "wheel", "sudo", "admin", "docker", "disk", "lxd", "incus-admin", "libvirt"} +// daemons that start a guest with the host's devices — and the kernel's memory (kmem) and the password hashes +// (shadow), either of which is root a step away (novox/hq ADR 0312 §4 review). A closed list, so what the judge +// calls "grants root" is written down and reviewable rather than guessed per machine. +var RootGroups = []string{"root", "wheel", "sudo", "admin", "docker", "disk", "lxd", "incus-admin", "libvirt", + "kmem", "shadow"} // Account is one user resource of a module that declares groups. type Account struct { diff --git a/internal/apply/apply.go b/internal/apply/apply.go index 14121ec..867293b 100644 --- a/internal/apply/apply.go +++ b/internal/apply/apply.go @@ -315,6 +315,8 @@ func ApplyMindingWindows( // (novox/hq ADR 0252), on the unit's rule above: two modules may each want the account in one group, // and the one going takes nothing the other still needs. wanted := groupsWanted(d.Resources) + // And what a record going hands to a resource that still asks for its group (novox/hq ADR 0312 §5). + handed := Handed{} removeOrphan := func(orphan store.Applied) error { var action, detail string @@ -339,10 +341,10 @@ func ApplyMindingWindows( case declaration.TypeOpening: action, detail, err = removeOpening(ctx, orphan, run, known.Firewall) case declaration.TypeUser: - action, detail, err = removeUser(ctx, sys, orphan, run, wanted) + action, detail, err = removeUser(ctx, sys, orphan, run, wanted, handed) case declaration.TypeAccess: // The path is left as it is; what it granted an account is taken back (novox/hq ADR 0312 §5). - action, detail = removeAccess(ctx, sys, orphan, run, wanted) + action, detail = removeAccess(ctx, sys, orphan, run, wanted, handed) default: action, detail, err = remove(ctx, sys, orphan, run, made) } @@ -509,7 +511,7 @@ func ApplyMindingWindows( // And one patience with the artifact store for the whole apply (novox/hq issue 291): a fetch it // does not answer during a maintenance window waits for the window instead of failing. in := inputs{declares: map[string]string{}, known: &known, windows: windows, away: newStoreAway(windows, log), - groups: wanted} + groups: wanted, handed: handed} // Where nothing is placed or mounted from, judged before anything is applied (novox/hq issue 339), and // which declared directories were already here: one this apply makes is the mesh's, one found may not be. refreshRuntimeRoots(ctx) @@ -1080,7 +1082,7 @@ func applyOne(ctx context.Context, sys system.System, r declaration.Resource, ru case *declaration.Container: return applyContainer(ctx, res, run, changed, in, previous) case *declaration.User: - return applyUser(ctx, sys, res, run, previous, in.groups) + return applyUser(ctx, sys, res, run, previous, in.groups, in.handed) case *declaration.Archive: return applyArchive(ctx, res, previous, in.away) case *declaration.Process: @@ -1968,7 +1970,7 @@ func remove(ctx context.Context, sys system.System, a store.Applied, run Runner, case declaration.TypeUser: // Kept, with its shell given back when that is safe (novox/hq ADR 0176 §2, issue 228). - return removeUser(ctx, sys, a, run, nil) + return removeUser(ctx, sys, a, run, nil, nil) case declaration.TypeNetwork: // **The reason this is a shape at all** (novox/hq ADR 0029). Orphans are removed in @@ -2131,6 +2133,8 @@ type inputs struct { // containerGroups the number of every group granted to a container, by container id. grants map[string]grant containerGroups map[string][]int + // handed is every group a record that went this apply handed to a resource still asking for it. + handed Handed } // fileDigest is what a file the container reads holds, by digest. diff --git a/internal/apply/grants.go b/internal/apply/grants.go index af3e04f..da702cf 100644 --- a/internal/apply/grants.go +++ b/internal/apply/grants.go @@ -4,12 +4,15 @@ import ( "context" "fmt" "os" + "os/exec" "os/user" "path/filepath" "slices" "sort" "strconv" "strings" + "sync" + "time" "github.com/novox/mesh-host/internal/accounts" "github.com/novox/mesh-host/internal/declaration" @@ -32,10 +35,11 @@ import ( // the path as its group may, and as nothing else. // // **The path is never changed** (§2). A group that cannot write the path is the operator's choice, as the data -// is: the access is stated unhealthy on every look, saying so and that the operator decides, and nothing is -// done to make it writable. **A read access grants nothing** (§3). **The root group is never granted** (§4): a +// is: the access grants nothing, is stated unhealthy on every look, saying so and that the operator decides, and +// nothing is done to make it writable. **A read access grants nothing** (§3). **The root group is never granted** (§4): a // path owned by group 0, or by a group whose members act as root (accounts.RootGroups, the account judge's own -// closed list), is refused by name. +// closed list, judged by the numbers those groups have here), or by a number the machine names nothing for, is +// refused by name. // // **What was granted is taken back as ADR 0252 takes back** (§5): the access records each group it put the // account in that the account was not in before; when no declared resource asks for it any more, the account @@ -86,18 +90,88 @@ var GroupOfPath = func(path string) (PathGroup, error) { return g, nil } -// GrantRefusal is why a path's group is never granted (novox/hq ADR 0312 §4), or nil when it may be: group 0, -// or a group whose members act as root by membership alone. +// RootGroupIDs is the number of every group whose members act as root (accounts.RootGroups), as this machine +// numbers them: read from /etc/group itself and from the machine's name service (`getent group`), since the engine +// is built without cgo and Go's own lookup reads /etc/group alone — a group a name service holds, or one number +// carrying several names, would otherwise pass a check made by name. Read at most once a minute, every look and +// apply judging against the same answer, and the name service given five seconds in all. A variable so a test can +// stand in. +var RootGroupIDs = func() map[int]string { + rootIDs.Lock() + defer rootIDs.Unlock() + if rootIDs.ids == nil || time.Since(rootIDs.at) > time.Minute { + rootIDs.ids, rootIDs.at = readRootGroupIDs(), time.Now() + } + return rootIDs.ids +} + +var rootIDs struct { + sync.Mutex + ids map[int]string + at time.Time +} + +func readRootGroupIDs() map[int]string { + out := map[int]string{} + if raw, err := os.ReadFile("/etc/group"); err == nil { + for name, gid := range groupEntries(string(raw)) { + if slices.Contains(accounts.RootGroups, name) { + out[gid] = name + } + } + } + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + for _, name := range accounts.RootGroups { + raw, err := exec.CommandContext(ctx, "getent", "group", name).Output() + if err != nil { + continue + } + for found, gid := range groupEntries(string(raw)) { + if found == name { + out[gid] = name + } + } + } + return out +} + +// groupEntries reads group database lines, name:password:gid:members, as name to number. +func groupEntries(raw string) map[string]int { + out := map[string]int{} + for _, line := range strings.Split(raw, "\n") { + fields := strings.Split(strings.TrimSpace(line), ":") + if len(fields) < 3 || strings.HasPrefix(fields[0], "#") { + continue + } + if gid, err := strconv.Atoi(fields[2]); err == nil { + out[fields[0]] = gid + } + } + return out +} + +// GrantRefusal is why a path's group is never granted (novox/hq ADR 0312 §4), or nil when it may be: group 0, a +// group whose members act as root by membership alone — judged by its NUMBER against every number those groups +// have here, and by its name — or a group this machine names nothing for, which cannot be judged at all. func GrantRefusal(g PathGroup) error { - switch { - case g.GID == 0: - return fmt.Errorf("it is owned by group 0 (%s), and a read-write access never grants the root group: a "+ - "shared path is never a way to root (novox/hq ADR 0312 §4); nothing was granted, and nothing was "+ - "changed on it", orRoot(g.Name)) - case slices.Contains(accounts.RootGroups, g.Name): + const never = "a shared path is never a way to root (novox/hq ADR 0312 §4); nothing was granted, and nothing " + + "was changed on it" + if g.GID == 0 { + return fmt.Errorf("it is owned by group 0 (%s), and a read-write access never grants the root group: %s", + orRoot(g.Name), never) + } + if root, is := RootGroupIDs()[g.GID]; is { + return fmt.Errorf("it is owned by gid %d, which is %s here, whose members act as root, and a read-write "+ + "access never grants such a group: %s", g.GID, root, never) + } + if slices.Contains(accounts.RootGroups, g.Name) { return fmt.Errorf("it is owned by the group %s, whose members act as root, and a read-write access never "+ - "grants such a group: a shared path is never a way to root (novox/hq ADR 0312 §4); nothing was "+ - "granted, and nothing was changed on it", g.Words()) + "grants such a group: %s", g.Words(), never) + } + if g.Name == "" { + return fmt.Errorf("it is owned by gid %d, which this machine's group database names nothing, so whether "+ + "its members act as root cannot be judged, and an unjudged group is never granted: %s", g.GID, never) } return nil } @@ -126,6 +200,10 @@ type grant struct { refused error } +// grants says the group is given: it may be granted, and it may write the path. A group that cannot write is +// granted nothing (novox/hq ADR 0312 §2) — the access is stated unhealthy for the operator to decide. +func (g grant) grants() bool { return g.refused == nil && g.group.Writable } + // grantsOf reads the group of every read-write access that names a grant, by access id. One that is refused // grants nothing — the access itself fails, saying why — and one whose path the engine refuses to place // anything at (issue 339) is not read at all. @@ -159,7 +237,7 @@ func containerGroups(resources []declaration.Resource, grants map[string]grant) continue } g, granted := grants[a.ID] - if !granted || g.refused != nil { + if !granted || !g.grants() { continue } for _, id := range a.Grant.Containers { @@ -175,12 +253,13 @@ func containerGroups(resources []declaration.Resource, grants map[string]grant) } // ContainerGroupsOf is containerGroups for a whole declaration, read now: what the scheduler arms a scheduled -// step with, so a fire runs with the groups an apply would have created it with. +// step with, so a fire runs with the groups an apply would have created it with. A path issue 339's guard refuses +// is not read, as the apply reads none. func ContainerGroupsOf(d *declaration.Declaration) map[string][]int { if d == nil { return nil } - return containerGroups(d.Resources, grantsOf(d.Resources, nil)) + return containerGroups(d.Resources, grantsOf(d.Resources, refusedPlaces(d.Resources))) } // withGrants adds to what the declaration asks of each account the groups its read-write accesses grant (novox/hq @@ -192,7 +271,7 @@ func (w Wanted) withGrants(resources []declaration.Resource, grants map[string]g continue } g, granted := grants[a.ID] - if !granted || g.refused != nil || g.group.Name == "" { + if !granted || !g.grants() { continue } if w[a.Grant.Account] == nil { @@ -211,7 +290,7 @@ func GrantedAccounts(d *declaration.Declaration, held map[string]bool) []account if d == nil { return nil } - grants := grantsOf(d.Resources, nil) + grants := grantsOf(d.Resources, refusedPlaces(d.Resources)) var out []accounts.Account for _, r := range d.Resources { a, ok := r.(*declaration.Access) @@ -219,7 +298,7 @@ func GrantedAccounts(d *declaration.Declaration, held map[string]bool) []account continue } g, granted := grants[a.ID] - if !granted || g.refused != nil || g.group.Name == "" { + if !granted || !g.grants() { continue } at := strings.LastIndex(a.ID, ".") @@ -251,8 +330,9 @@ func grantAccess(ctx context.Context, sys system.System, r *declaration.Access, return fmt.Errorf("%s grants nothing: %w", r.Path, g.refused) } } + // A group that cannot write is granted nothing (§2): what an earlier apply granted is taken back below. account := "" - if granting { + if granting && g.group.Writable { account = r.Grant.Account } if account != "" && g.group.Name == "" { @@ -286,8 +366,9 @@ func grantAccess(ctx context.Context, sys system.System, r *declaration.Access, said = append(said, fmt.Sprintf("put %s in %s, the group that owns it; the tool runner holds it once "+ "restarted, a session that began before only after a new login", account, g.group.Name)) } - // Recorded only when the mesh put it there, now or before: a group held before is never taken back. - if !inGroup || slices.Contains(recorded, g.group.Name) { + // Recorded only when the mesh put it there, now or before, or a record that went handed it over: a group + // held before is never taken back. + if !inGroup || slices.Contains(recorded, g.group.Name) || in.handed.has(account, g.group.Name) { kept = append(kept, g.group.Name) } var others []string @@ -312,13 +393,13 @@ func grantAccess(ctx context.Context, sys system.System, r *declaration.Access, if account != "" { to = append(to, account) } - if len(to) > 0 { + switch { + case !g.group.Writable: + out.Detail = joinDetail(out.Detail, fmt.Sprintf("its group %s cannot write it, so nothing is granted: "+ + "stated unhealthy, and nothing was changed on it — the operator decides", g.group.Words())) + case len(to) > 0: out.Detail = joinDetail(out.Detail, fmt.Sprintf("grants %s to %s", g.group.Words(), strings.Join(to, " and "))) } - if !g.group.Writable { - out.Detail = joinDetail(out.Detail, "its group cannot write it: stated unhealthy, and nothing was changed "+ - "on it — the operator decides") - } } if len(said) > 0 { out.Detail = joinDetail(out.Detail, strings.Join(said, "; ")) @@ -373,13 +454,21 @@ func takeBack(ctx context.Context, sys system.System, account, id string, groups } // removeAccess is an access no longer declared: the path left exactly as it is (novox/hq ADR 0051), and every -// group it put an account in taken back unless another declared resource still asks for it (ADR 0312 §5). -func removeAccess(ctx context.Context, sys system.System, a store.Applied, run Runner, wanted Wanted) (string, string) { +// group it put an account in taken back unless another declared resource still asks for it (ADR 0312 §5) — then +// handed to that resource, which records it as the mesh's and takes it back when it stops asking. +func removeAccess(ctx context.Context, sys system.System, a store.Applied, run Runner, wanted Wanted, + handed Handed) (string, string) { const left = "an operator-owned path is never the host's to remove" if a.User == "" || len(a.Groups) == 0 { return "forgotten", left } - gave, said := takeBack(ctx, sys, a.User, a.ID, a.Groups, nil, run, wanted) + var stays []string + gave, said := takeBack(ctx, sys, a.User, a.ID, a.Groups, &stays, run, wanted) + for _, g := range stays { + if len(wanted.othersAsk(a.User, g, a.ID)) > 0 { + handed.hand(a.User, g) + } + } action := "forgotten" if gave { action = "restored" @@ -405,3 +494,21 @@ func gidWords(groups []int) string { } return strings.Join(words, ", ") } + +// Handed is every group a record that went in this apply put an account in, and another declared resource still +// asks for: account → group. The resource that still asks records it as the mesh's when it is applied, so the group +// is taken back once nobody asks for it any more, rather than staying for ever because the record that put it +// there went first (novox/hq ADR 0252 §3, ADR 0312 §5). +type Handed map[string]map[string]bool + +func (h Handed) hand(account, group string) { + if h == nil { + return + } + if h[account] == nil { + h[account] = map[string]bool{} + } + h[account][group] = true +} + +func (h Handed) has(account, group string) bool { return h[account][group] } diff --git a/internal/apply/grants_test.go b/internal/apply/grants_test.go index 8f3d176..e540a4d 100644 --- a/internal/apply/grants_test.go +++ b/internal/apply/grants_test.go @@ -93,6 +93,15 @@ func groupOfPaths(t *testing.T, groups map[string]PathGroup) { return g, nil } t.Cleanup(func() { GroupOfPath = was }) + rootGroupsAre(t, map[int]string{0: "root", 10: "wheel", 968: "docker"}) +} + +// rootGroupsAre stands in for the numbers this machine gives the groups whose members act as root. +func rootGroupsAre(t *testing.T, ids map[int]string) { + t.Helper() + was := RootGroupIDs + RootGroupIDs = func() map[int]string { return ids } + t.Cleanup(func() { RootGroupIDs = was }) } func accessGranting(id, path, account string, containers ...string) string { @@ -213,7 +222,7 @@ func TestAReadAccessGrantsNothing(t *testing.T) { } } -func TestAnAccessWhoseGroupCannotWriteIsGrantedAndSaysTheOperatorDecides(t *testing.T) { +func TestAnAccessWhoseGroupCannotWriteGrantsNothingAndSaysTheOperatorDecides(t *testing.T) { spool := t.TempDir() groupOfPaths(t, map[string]PathGroup{spool: {GID: 1002, Name: "downloads", Writable: false}}) before := statOf(t, spool) @@ -225,10 +234,17 @@ func TestAnAccessWhoseGroupCannotWriteIsGrantedAndSaysTheOperatorDecides(t *test t.Fatal(err) } o := outcomeOf(report, "nzbget.downloads") - if !strings.Contains(o.Detail, "its group cannot write it") || !strings.Contains(o.Detail, "the operator decides") || + if !strings.Contains(o.Detail, "cannot write it, so nothing is granted") || !strings.Contains(o.Detail, "the operator decides") || !strings.Contains(o.Detail, "nothing was changed") { t.Errorf("the access did not say its group cannot write and that the operator decides: %+v", o) } + // Nothing is given (ADR 0312 §2): no account group, no container group. + if strings.Contains(m.groups(), "downloads") { + t.Errorf("a group that cannot write was granted to the account: %q", m.groups()) + } + if run := strings.Join(m.runs["nzbget-app"], " "); strings.Contains(run, "--group-add") { + t.Errorf("a group that cannot write was granted to the container: %s", run) + } if after := statOf(t, spool); after != before { t.Errorf("the engine changed a path whose group cannot write: %s, was %s", after, before) } @@ -241,8 +257,16 @@ func TestAnAccessWhoseGroupCannotWriteIsGrantedAndSaysTheOperatorDecides(t *test func TestTheRootGroupIsNeverGranted(t *testing.T) { for _, g := range []PathGroup{{GID: 0, Name: "root", Writable: true}, {GID: 968, Name: "docker", Writable: true}, - {GID: 10, Name: "wheel", Writable: true}} { - t.Run(g.Name, func(t *testing.T) { + {GID: 10, Name: "wheel", Writable: true}, {GID: 42, Name: "shadow", Writable: true}, + // A number the root groups have here under another name — judged by the number, never the name. + {GID: 968, Name: "containers", Writable: true}, + // A number the machine names nothing for: unjudged, so never granted, to a container either. + {GID: 4242, Name: "", Writable: true}} { + want := g.Name + if want == "" || want == "containers" { + want = fmt.Sprintf("gid %d", g.GID) + } + t.Run(fmt.Sprintf("%d %s", g.GID, g.Name), func(t *testing.T) { shared := t.TempDir() groupOfPaths(t, map[string]PathGroup{shared: g}) m := newGrantMachine("operator", nil, g.Name) @@ -254,10 +278,10 @@ func TestTheRootGroupIsNeverGranted(t *testing.T) { } var failed *Error if !errors.As(err, &failed) || failed.Resource != "importer.shared" || - !strings.Contains(err.Error(), g.Name) || !strings.Contains(err.Error(), "never") { + !strings.Contains(err.Error(), want) || !strings.Contains(err.Error(), "never") { t.Errorf("the refusal does not name the access and the group: %v", err) } - if strings.Contains(m.groups(), g.Name) { + if g.Name != "" && strings.Contains(m.groups(), g.Name) { t.Errorf("the account was put in %s", g.Name) } if run := strings.Join(m.runs["importer-app"], " "); strings.Contains(run, "--group-add") { @@ -455,3 +479,81 @@ func TestAnAccountsOwnSessionIsSaidReloginNeededForAGrantedGroup(t *testing.T) { t.Errorf("a refused grant is judged: %+v", as) } } + +// An access going while a user resource still asks for its group hands the group over: the user resource records it +// as the mesh's, and takes it back when it stops asking too (novox/hq ADR 0312 §5, ADR 0252 §3). +func TestAGroupStillAskedForIsHandedOverAndTakenBackWhenTheLastAskerGoes(t *testing.T) { + music := t.TempDir() + groupOfPaths(t, map[string]PathGroup{music: {GID: 1001, Name: "media", Writable: true}}) + player := `{"id":"player.account","type":"user","name":"operator","groups":["media"]}` + m := newGrantMachine("operator", []string{"operator"}, "media") + + _, state, err := applyGrants(t, m, store.State{}, accessGranting("importer.music", music, "operator"), player) + if err != nil { + t.Fatal(err) + } + if rec, _ := state.Find("player.account"); len(rec.Groups) != 0 { + t.Fatalf("the user resource recorded a group the access put the account in: %+v", rec) + } + _, state, err = applyGrants(t, m, state, player) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(m.groups(), "media") { + t.Fatalf("a group the user resource still asks for was taken back: %q", m.groups()) + } + if rec, _ := state.Find("player.account"); strings.Join(rec.Groups, " ") != "media" { + t.Fatalf("the group was not handed to the user resource that still asks for it: %+v", rec) + } + if _, _, err := applyGrants(t, m, state, mounting("other.app", t.TempDir())); err != nil { + t.Fatal(err) + } + if strings.Contains(m.groups(), "media") { + t.Errorf("the group stayed after the last resource asking for it went: %q", m.groups()) + } +} + +// A grant's account is an argument to id, usermod and gpasswd, and a name that starts with a dash would be read as +// an option by each. +func TestAGrantsAccountMayNotReadAsAnOption(t *testing.T) { + for _, name := range []string{"-ogroups", "--help", "a b", "a:b"} { + _, err := declaration.Parse([]byte(`{"declaration":1,"resources":[{"id":"m.lib","type":"access","path":"/srv/lib",` + + `"mode":"read-write","grant":{"account":"` + name + `"}}]}`)) + if err == nil || !strings.Contains(err.Error(), "not starting with a dash") { + t.Errorf("the account %q was accepted: %v", name, err) + } + } + if _, err := declaration.Parse([]byte(`{"declaration":1,"resources":[{"id":"m.lib","type":"access","path":"/srv/lib",` + + `"mode":"read-write","grant":{"account":"jochen"}}]}`)); err != nil { + t.Errorf("an ordinary account was refused: %v", err) + } +} + +// The scheduler reads no path issue 339's guard refuses, as the apply reads none. +func TestAScheduledStepIsArmedWithoutReadingARefusedPath(t *testing.T) { + var read []string + was := GroupOfPath + GroupOfPath = func(path string) (PathGroup, error) { + read = append(read, path) + return PathGroup{GID: 1001, Name: "media", Writable: true}, nil + } + t.Cleanup(func() { GroupOfPath = was }) + rootGroupsAre(t, map[int]string{0: "root"}) + d := parse(t, `{"declaration":1,"resources":[`+accessGranting("m.etc", "/etc", "", "m.step")+`,`+ + `{"id":"m.step","type":"container","name":"step","image":"`+pinned+`","schedule":"0 3 * * *","volumes":["/etc:/x"]}]}`) + if got := ContainerGroupsOf(d); len(got["m.step"]) != 0 { + t.Errorf("a step was armed with the group of a refused path: %v", got) + } + if len(read) != 0 { + t.Errorf("the scheduler read a path the guard refuses: %v", read) + } +} + +// The group database is read as name to number, so a number carrying a root group's name under any other name +// is still that number. +func TestGroupEntriesAreReadByNumber(t *testing.T) { + got := groupEntries("root:x:0:\n# a comment\ndocker:x:968:operator\ncontainers:x:968:\nbroken\n") + if got["root"] != 0 || got["docker"] != 968 || got["containers"] != 968 || len(got) != 3 { + t.Errorf("read %v", got) + } +} diff --git a/internal/apply/groups.go b/internal/apply/groups.go index e19672e..308fa31 100644 --- a/internal/apply/groups.go +++ b/internal/apply/groups.go @@ -63,7 +63,7 @@ func (w Wanted) othersAsk(account, group, except string) []string { // applyGroups makes the account be in every group the resource declares, and takes it out of every group // this resource put it in and no longer asks for. What the mesh put the account in is recorded on out. func applyGroups(ctx context.Context, sys system.System, r *declaration.User, run Runner, - previous store.Applied, wanted Wanted, out *Outcome) error { + previous store.Applied, wanted Wanted, handed Handed, out *Outcome) error { // What this resource put the account in before, for this account only: a declaration that renamed its // user says nothing about the new one's groups. var added []string @@ -83,6 +83,10 @@ func applyGroups(ctx context.Context, sys system.System, r *declaration.User, ru var put []string for _, want := range r.Groups { if already[want] { + // A record that went handed it here: the mesh's, and this resource's to take back (novox/hq ADR 0312 §5). + if handed.has(r.Name, want) && !contains(added, want) { + added = append(added, want) + } continue } exists, err := system.GroupExists(ctx, system.Runner(run), want) @@ -139,7 +143,11 @@ func applyGroups(ctx context.Context, sys system.System, r *declaration.User, ru // giveGroupsBack is removeUser's groups: every group the mesh put the account in, taken back unless // another declared resource still asks for it. Never fatal, for the shell's reason: a removal that failed // would stay recorded and fail the same way on every apply after. Empty when there was nothing to say. -func giveGroupsBack(ctx context.Context, sys system.System, a store.Applied, run Runner, wanted Wanted) (bool, string) { +// +// A group left because another declared resource still asks for it is handed to that resource (handed), which +// records it as the mesh's, so it is taken back once nobody asks (novox/hq ADR 0312 §5). +func giveGroupsBack(ctx context.Context, sys system.System, a store.Applied, run Runner, wanted Wanted, + handed Handed) (bool, string) { if len(a.Groups) == 0 { return false, "" } @@ -157,6 +165,7 @@ func giveGroupsBack(ctx context.Context, sys system.System, a store.Applied, run } if others := wanted.othersAsk(a.Target, g, a.ID); len(others) > 0 { said = append(said, fmt.Sprintf("left in %s, which %s still asks for", g, strings.Join(others, ", "))) + handed.hand(a.Target, g) continue } gone, why := leaveGroup(ctx, sys, a.Target, g, run) diff --git a/internal/apply/user.go b/internal/apply/user.go index 44e227b..4891e74 100644 --- a/internal/apply/user.go +++ b/internal/apply/user.go @@ -35,7 +35,7 @@ const FirstLoginUID = 1000 // wanted is every group the whole declaration asks an account to be in (novox/hq ADR 0252), so a group // this resource stops asking for is kept while another resource asks for it. func applyUser(ctx context.Context, sys system.System, r *declaration.User, run Runner, - previous store.Applied, wanted Wanted) (Outcome, error) { + previous store.Applied, wanted Wanted, handed Handed) (Outcome, error) { out := begin(r) out.Action = "unchanged" // What was found is carried from the record for as long as the resource is recorded — for this @@ -100,7 +100,7 @@ func applyUser(ctx context.Context, sys system.System, r *declaration.User, run // Groups before the shell, so that a failure here comes before the shell is changed: a record // is written only for an apply that worked, and a shell changed by a failed one would be read // next time as the account's own, and the one it replaced lost. - if err := applyGroups(ctx, sys, r, run, previous, wanted, &out); err != nil { + if err := applyGroups(ctx, sys, r, run, previous, wanted, handed, &out); err != nil { return out, err } @@ -207,7 +207,7 @@ func onOff(on bool) string { // And every group the mesh put the account in, while no other declared resource asks for it (novox/hq // ADR 0252), on the same rule: never fatal, and never a group the account was in before. func removeUser(ctx context.Context, sys system.System, a store.Applied, run Runner, - wanted Wanted) (string, string, error) { + wanted Wanted, handed Handed) (string, string, error) { const kept = "the account is kept; the host never deletes a login" login, exists, err := system.LookUpUser(ctx, system.Runner(run), a.Target) if err != nil { @@ -226,7 +226,7 @@ func removeUser(ctx context.Context, sys system.System, a store.Applied, run Run action = "restored" } } - if gave, said := giveGroupsBack(ctx, sys, a, run, wanted); said != "" { + if gave, said := giveGroupsBack(ctx, sys, a, run, wanted, handed); said != "" { detail += "; " + said if gave { action = "restored" diff --git a/internal/declaration/declaration.go b/internal/declaration/declaration.go index e4318f9..e2f6d29 100644 --- a/internal/declaration/declaration.go +++ b/internal/declaration/declaration.go @@ -522,9 +522,11 @@ func (a *Access) validate(where string, _ bool) []string { problems = append(problems, where+": a grant names a container with no id") } } - if a.Grant.Account != "" && strings.ContainsAny(a.Grant.Account, " :/\t\n") { - problems = append(problems, where+": a grant's account is an account's name, and "+ - a.Grant.Account+" is not one") + if a.Grant.Account != "" && !accountName.MatchString(a.Grant.Account) { + // Handed to id, usermod and gpasswd as an argument: a name starting with a dash would be read as an + // option by each of them. + problems = append(problems, fmt.Sprintf("%s: a grant's account is an account's name — letters, digits, "+ + "dots, underscores and dashes, not starting with a dash — and %q is not one", where, a.Grant.Account)) } } return problems @@ -1778,6 +1780,10 @@ func checkWhileStopped(resources []Resource) []string { return problems } +// accountName is what a grant's account may be: the portable account name the user tools take, optionally ending +// in `$`, never starting with a dash. +var accountName = regexp.MustCompile(`^[A-Za-z0-9_.][A-Za-z0-9_.-]*\$?$`) + // checkGrants judges each access's grant against the whole declaration (novox/hq ADR 0312): a container it // names is a container of this declaration, as a maintenance window's are. A grant to a container the machine // is not told about would be a group given to nothing, said as granted.