From ccc86d35946126b1a5d5f9e267f08afaa8b44a2f Mon Sep 17 00:00:00 2001 From: jochen Date: Tue, 13 Oct 2026 00:06:29 +0200 Subject: [PATCH 1/2] =?UTF-8?q?Put=20the=20tool=20runner=20in=20the=20own?= =?UTF-8?q?=20group=20of=20each=20module=20whose=20tools=20it=20runs=20(hq?= =?UTF-8?q?=20ADR=200334=20=C2=A72)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A joining module's own directories belong to its own group, and its tools run in the tool runner, which could not read them. An identity may now name the tool runner's account: the engine puts it in mesh- as it puts it in a library's group, says the change as identity: for the runner's unit to restart once, takes it back when no identity names it, and refuses an account that is missing or root's. The engine states contract 7. --- cmd/mesh-host/main.go | 2 +- cmd/mesh-host/units_test.go | 8 +-- internal/apply/apply.go | 17 +++++- internal/apply/library.go | 60 ++++++++++++++----- internal/apply/library_test.go | 89 +++++++++++++++++++++++++++++ internal/declaration/declaration.go | 6 ++ internal/link/messages.go | 6 ++ internal/store/store.go | 4 +- 8 files changed, 170 insertions(+), 22 deletions(-) diff --git a/cmd/mesh-host/main.go b/cmd/mesh-host/main.go index 416814b..83b5473 100644 --- a/cmd/mesh-host/main.go +++ b/cmd/mesh-host/main.go @@ -1736,7 +1736,7 @@ func healthAsReported(st liveness.Statement, ns *network.Statement) *link.Health // LibraryContract: 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 reads a declaration's identities, groups and libraries // (novox/hq ADR 0328), which is what tells the controller it may be sent any of those fields. - h := &link.Health{Contract: link.LibraryContract, At: st.At.UTC(), Resources: []link.ResourceHealth{}} + h := &link.Health{Contract: link.OwnGroupContract, 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, diff --git a/cmd/mesh-host/units_test.go b/cmd/mesh-host/units_test.go index c1b8a88..0084497 100644 --- a/cmd/mesh-host/units_test.go +++ b/cmd/mesh-host/units_test.go @@ -87,10 +87,10 @@ func TestTheStatementSaysAnAccountJudgedForRoot(t *testing.T) { Groups: []string{"openrazer"}}, State: accounts.Healthy, Since: at}, }} h := withAccounts(healthAsReported(liveness.Statement{At: at}, nil), as) - // At RootContract or later: an engine that also reads libraries says LibraryContract (novox/hq ADR 0328), one - // above ADR 0325's IdentityContract. - if h.Contract < link.RootContract || link.RootContract != 3 || h.Contract != link.LibraryContract || - link.LibraryContract != link.IdentityContract+1 { + // At RootContract or later: an engine that also reads an identity's runner says OwnGroupContract (novox/hq ADR + // 0334), one above ADR 0328's LibraryContract. + if h.Contract < link.RootContract || link.RootContract != 3 || h.Contract != link.OwnGroupContract || + link.OwnGroupContract != link.LibraryContract+1 { t.Fatalf("contract %d", h.Contract) } got := map[string]string{} diff --git a/internal/apply/apply.go b/internal/apply/apply.go index 6b83d7c..bb36d36 100644 --- a/internal/apply/apply.go +++ b/internal/apply/apply.go @@ -627,6 +627,21 @@ func ApplyMindingWindows( "was attempted" continue } + // The tool runner joins the module's own group, so its tools read what the module keeps (ADR 0334 + // §2); a change of `identity:` restarts it once, through its unit's restart-on. + if id.Runner != "" { + moved, said, err := joinGroup(ctx, sys, id.Runner, id.Name, store.Membership{Module: id.Module}, &known, run) + if err != nil { + identityFailures = append(identityFailures, &Error{Resource: "identity " + id.Name, Err: err}) + log(fmt.Sprintf(" failed identity %s's runner: %v", id.Name, err)) + } else if moved { + changed[IdentityChange(id.Module)] = true + outcome.Detail = joinDetail(outcome.Detail, said) + if outcome.Action == "unchanged" { + outcome.Action = "updated" + } + } + } report.Outcomes = append(report.Outcomes, outcome) if outcome.Action != "unchanged" { log(fmt.Sprintf(" %s %s: %s", outcome.Action, outcome.ID, outcome.Detail)) @@ -1051,7 +1066,7 @@ func ApplyMindingWindows( // Only the mesh says what is assigned: a carried bundle's silence undeclares no identity and no folder. if origin == store.OriginDeclared { // A tool runner no longer put in a library's group leaves it (ADR 0328 §11, ADR 0252 §3). - for _, o := range leaveRunners(ctx, sys, d.Libraries, &known, run, wanted) { + for _, o := range leaveRunners(ctx, sys, d.Libraries, d.Identities, &known, run, wanted) { report.Outcomes = append(report.Outcomes, o) log(fmt.Sprintf(" %s %s: %s", o.Action, o.ID, o.Detail)) } diff --git a/internal/apply/library.go b/internal/apply/library.go index e44ecea..d1235f9 100644 --- a/internal/apply/library.go +++ b/internal/apply/library.go @@ -395,43 +395,70 @@ func JudgeLibrary(ctx context.Context, l declaration.Library) LibraryJudgement { return j } -// joinRunner puts a library's tool runner's account in its group (ADR 0328 §11), as ADR 0252 puts an account in a -// group: recorded only when the mesh put it there, so one it was in before is never taken back. Whether it moved. +// joinRunner puts a library's tool runner's account in its group (ADR 0328 §11). func joinRunner(ctx context.Context, sys system.System, l declaration.Library, known *store.State, run Runner) (bool, string, error) { if l.Runner == "" { return false, "", nil } - in, err := system.GroupsOf(ctx, system.Runner(run), l.Runner) - if err != nil { - return false, "", fmt.Errorf("the tool runner's account %s could not be read: %w", l.Runner, err) + return joinGroup(ctx, sys, l.Runner, l.Group, store.Membership{Library: l.Library}, known, run) +} + +// IdentityChange is what a tool runner joining a module's own group changes (ADR 0334 §2): `identity:`, +// which the tool runner's unit names under restart-on, as it names `library:`. +func IdentityChange(module string) string { return "identity:" + module } + +// joinGroup puts a tool runner's account in a group (ADR 0328 §11, ADR 0334 §2), as ADR 0252 puts an account in a +// group: recorded only when the mesh put it there, so one it was in before is never taken back. The account must be +// here and never root's: uid 0, or a root group as its primary (§15). Whether it moved, and what was said. +func joinGroup(ctx context.Context, sys system.System, account, group string, of store.Membership, known *store.State, + run Runner) (bool, string, error) { + who, found, err := lookUp(ctx, run, "passwd", account) + switch { + case err != nil: + return false, "", fmt.Errorf("the tool runner's account %s could not be read: %w", account, err) + case !found: + return false, "", fmt.Errorf("the tool runner's account %s is not on this machine, so it joins no group", account) } - if setOf(in)[l.Group] { + if err := rootIdentity(who.id, who.prim); err != nil { + return false, "", fmt.Errorf("the tool runner's account %s joins no group: %w", account, err) + } + in, err := system.GroupsOf(ctx, system.Runner(run), account) + if err != nil { + return false, "", fmt.Errorf("the tool runner's account %s could not be read: %w", account, err) + } + if setOf(in)[group] { return false, "", nil } - if err := sys.AddUserToGroup(ctx, system.Runner(run), l.Runner, l.Group); err != nil { + if err := sys.AddUserToGroup(ctx, system.Runner(run), account, group); err != nil { return false, "", err } for _, m := range known.Members { - if m.Account == l.Runner && m.Group == l.Group { - return true, fmt.Sprintf("put the tool runner's %s back in %s", l.Runner, l.Group), nil + if m.Account == account && m.Group == group { + return true, fmt.Sprintf("put the tool runner's %s back in %s", account, group), nil } } - known.Members = append(known.Members, store.Membership{Account: l.Runner, Group: l.Group, Library: l.Library}) - return true, fmt.Sprintf("put the tool runner's %s in %s; it holds it once restarted", l.Runner, l.Group), nil + of.Account, of.Group = account, group + known.Members = append(known.Members, of) + return true, fmt.Sprintf("put the tool runner's %s in %s; it holds it once restarted", account, group), nil } -// leaveRunners takes every tool runner's account the mesh put in a library's group out of it once no declared -// library puts it there and no declared user resource asks for it (ADR 0252 §3). Never fatal: what could not be +// leaveRunners takes every tool runner's account the mesh put in a library's group, or a module's own group, out of +// it once no declared library or identity puts it there and no declared user resource asks for it (ADR 0252 §3). Never fatal: what could not be // left stays recorded, said, for the next apply. -func leaveRunners(ctx context.Context, sys system.System, libs []declaration.Library, known *store.State, run Runner, - wanted Wanted) []Outcome { +func leaveRunners(ctx context.Context, sys system.System, libs []declaration.Library, ids []declaration.Identity, + known *store.State, run Runner, wanted Wanted) []Outcome { declared := map[string]bool{} for _, l := range libs { if l.Runner != "" { declared[l.Runner+" "+l.Group] = true } } + for _, id := range ids { + if id.Runner != "" { + declared[id.Runner+" "+id.Name] = true + } + } var out []Outcome var keep []store.Membership for _, m := range known.Members { @@ -440,6 +467,9 @@ func leaveRunners(ctx context.Context, sys system.System, libs []declaration.Lib continue } o := Outcome{ID: LibraryAccess(m.Library) + " runner", Type: "library", Target: m.Account} + if m.Module != "" { + o.ID, o.Type = "identity "+declaration.IdentityPrefix+m.Module+" runner", "identity" + } if others := wanted.othersAsk(m.Account, m.Group, ""); len(others) > 0 { o.Action, o.Detail = "forgotten", fmt.Sprintf("left %s in %s, which %s asks for", m.Account, m.Group, strings.Join(others, ", ")) diff --git a/internal/apply/library_test.go b/internal/apply/library_test.go index a1fef80..4bb756b 100644 --- a/internal/apply/library_test.go +++ b/internal/apply/library_test.go @@ -832,3 +832,92 @@ func TestAnApplyWaitsForALockThatIsNoWalk(t *testing.T) { t.Fatalf("a library whose root is not there was read as kept while its lock was held: %+v", o) } } + +// ADR 0334 §2: the tool runner's account joins a module's own group once, its unit restarted for it (a change of +// identity:), and is taken out once no identity names it; an account not here, or root's, is refused. +func TestTheToolRunnerJoinsAModulesOwnGroup(t *testing.T) { + m := &accountsMachine{users: map[string][2]int{"nox": {1003, 1003}}, groups: map[string][2]int{"nox": {1003, 0}}} + d := withGroups(t, ``, `{"module":"radarr","name":"mesh-radarr","uid":61001,"gid":61001,"runner":"nox"}`, ``) + report, known, err := Apply(context.Background(), archHost(t), d, store.State{}, store.OriginDeclared, m.run, nil, nil) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(strings.Join(m.ran, "\n"), "usermod --append --groups mesh-radarr nox") { + t.Fatalf("the tool runner was not put in the module's own group: %v", m.ran) + } + if o := outcomeOf(report, "identity mesh-radarr"); !strings.Contains(o.Detail, "tool runner") || + len(known.Members) != 1 || known.Members[0].Module != "radarr" { + t.Fatalf("its joining is not said, or not recorded: %+v %+v", o, known.Members) + } + m.ran = nil + if _, known, err = Apply(context.Background(), archHost(t), d, known, store.OriginDeclared, m.run, nil, nil); err != nil { + t.Fatal(err) + } + if strings.Contains(strings.Join(m.ran, "\n"), "usermod") { + t.Fatal("the tool runner was put in its group again") + } + none := withGroups(t, ``, `{"module":"radarr","name":"mesh-radarr","uid":61001,"gid":61001}`, ``) + if _, known, err = Apply(context.Background(), archHost(t), none, known, store.OriginDeclared, m.run, nil, nil); err != nil { + t.Fatal(err) + } + if !strings.Contains(strings.Join(m.ran, "\n"), "gpasswd --delete nox mesh-radarr") || len(known.Members) != 0 { + t.Fatalf("the tool runner was not taken out of a group no identity puts it in: %v", m.ran) + } + + was := RootGroupIDs + RootGroupIDs = func() map[int]string { return map[int]string{10: "wheel"} } + defer func() { RootGroupIDs = was }() + for runner, says := range map[string]string{"ghost": "not on this machine", "root": "never root", + "admin": "root group"} { + m := &accountsMachine{users: map[string][2]int{"root": {0, 0}, "admin": {1001, 10}}, groups: map[string][2]int{}} + d := withGroups(t, ``, `{"module":"radarr","name":"mesh-radarr","uid":61001,"gid":61001,"runner":"`+ + runner+`"}`, ``) + _, _, err := Apply(context.Background(), archHost(t), d, store.State{}, store.OriginDeclared, m.run, nil, nil) + if err == nil || !strings.Contains(err.Error(), says) { + t.Errorf("%s: not refused saying %q: %v", runner, says, err) + } + if strings.Contains(strings.Join(m.ran, "\n"), "usermod --append") { + t.Errorf("%s: put in a group", runner) + } + } +} + +// ADR 0334 §2: a service naming identity: under restart-on is restarted when the tool runner joins the +// module's own group, and not on an apply where nothing moved. +func TestTheToolRunnerIsRestartedOnceForTheOwnGroup(t *testing.T) { + m := &accountsMachine{users: map[string][2]int{"nox": {1003, 1003}}, groups: map[string][2]int{"nox": {1003, 0}}} + d := withGroups(t, ``, `{"module":"radarr","name":"mesh-radarr","uid":61001,"gid":61001,"runner":"nox"}`, + `{"id":"node-tools.unit","type":"service","unit":"node-tools.service","state":"running",`+ + `"restart-on":["identity:radarr"]}`) + var verbs []string + systemd := systemctlStub(t, "loaded", "active", "enabled", &verbs) + run := func(ctx context.Context, name string, args ...string) (string, error) { + if name == "systemctl" { + return systemd(ctx, name, args...) + } + return m.run(ctx, name, args...) + } + stops := func() int { + n := 0 + for _, v := range verbs { + if v == "stop" { + n++ + } + } + return n + } + _, known, err := Apply(context.Background(), archHost(t), d, store.State{}, store.OriginDeclared, run, nil, nil) + if err != nil { + t.Fatal(err) + } + if stops() != 1 { + t.Fatalf("the tool runner was restarted %d times for joining the own group, and once was wanted", stops()) + } + verbs = nil + if _, _, err := Apply(context.Background(), archHost(t), d, known, store.OriginDeclared, run, nil, nil); err != nil { + t.Fatal(err) + } + if stops() != 0 { + t.Fatalf("the tool runner was restarted %d times with nothing moved", stops()) + } +} diff --git a/internal/declaration/declaration.go b/internal/declaration/declaration.go index 16ffdc8..5bd7ff6 100644 --- a/internal/declaration/declaration.go +++ b/internal/declaration/declaration.go @@ -1564,6 +1564,9 @@ type Identity struct { UID int `json:"uid"` GID int `json:"gid"` Groups []int `json:"groups,omitempty"` + // Runner is the tool runner's account, put in the module's own group (novox/hq ADR 0334 §2) when its tools run + // on this machine. Absent for a module that declares no tools here, and from a controller older than ADR 0334. + Runner string `json:"runner,omitempty"` } // Group is one library's group, as the controller allocated it (ADR 0328 §3). @@ -1707,6 +1710,9 @@ func checkIdentities(ids []Identity, groups []Group) []string { "declaration makes", where, g)) } } + if id.Runner != "" && !accountName.MatchString(id.Runner) { + problems = append(problems, fmt.Sprintf("%s: its runner %q is not an account's name", where, id.Runner)) + } names[id.Name], uids[id.UID] = true, id.Name } return problems diff --git a/internal/link/messages.go b/internal/link/messages.go index 9609f27..69f2c1c 100644 --- a/internal/link/messages.go +++ b/internal/link/messages.go @@ -274,6 +274,12 @@ const IdentityContract = 5 // it, its presence is what tells the controller this host may be sent those fields. const LibraryContract = 6 +// OwnGroupContract is the statement of a host that also reads an identity's `runner` (novox/hq ADR 0334 §2): it puts +// the tool runner's account in the module's own group, takes it back when no identity names it, and says the change +// as `identity:`, which the tool runner's unit names under restart-on to be restarted once. Like the fields +// before it, its presence is what tells the controller this host may be sent `runner` on an identity. +const OwnGroupContract = 7 + // 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. diff --git a/internal/store/store.go b/internal/store/store.go index c803530..5bdd2a4 100644 --- a/internal/store/store.go +++ b/internal/store/store.go @@ -267,7 +267,9 @@ type MadeGroup struct { type Membership struct { Account string `json:"account"` Group string `json:"group"` - Library string `json:"library"` + // Library is the library whose group it is; Module, for a module's own group (ADR 0334 §2), the module. + Library string `json:"library,omitempty"` + Module string `json:"module,omitempty"` } // MadeIdentity is one module identity as the engine made it. -- 2.54.0 From c7b534a08c98c68f10ee5c1a4aa72c6f527791a0 Mon Sep 17 00:00:00 2001 From: jochen Date: Tue, 13 Oct 2026 00:11:50 +0200 Subject: [PATCH 2/2] Keep a module directory's setgid bit, warn about a refused runner, and restart the runner when it leaves (hq ADR 0334 review) A module's own directories need setgid so what is made beneath keeps its own group, and a mode of 2750 refused the whole declaration while every apply dropped the bit: a directory's mode now carries setgid (never setuid or sticky), set, kept and said. A tool runner's account that is missing or root's failed the whole apply and dropped the library's outcome: it is now a warning, said as runner: or runner:library:. And a runner that leaves a group is taken out before the resources apply, so its unit is restarted in that apply to lose it. --- cmd/mesh-host/libraries.go | 17 +++- internal/apply/apply.go | 102 +++++++++++++++++----- internal/apply/homes_linux.go | 6 +- internal/apply/library.go | 29 ++++++- internal/apply/library_test.go | 107 +++++++++++++++++++++++- internal/apply/mesh_dirs_apply_linux.go | 19 +++-- internal/declaration/declaration.go | 8 +- internal/link/messages.go | 5 ++ 8 files changed, 257 insertions(+), 36 deletions(-) diff --git a/cmd/mesh-host/libraries.go b/cmd/mesh-host/libraries.go index 8588901..f0addb1 100644 --- a/cmd/mesh-host/libraries.go +++ b/cmd/mesh-host/libraries.go @@ -163,10 +163,25 @@ func (j *libraryJudge) look(now time.Time) ([]link.ResourceHealth, bool) { read[i] = v } + // And every tool runner's account the last apply could not put in a group, as a resource of its own (ADR 0334 + // §2): unhealthy, never failing the apply. + warnings := apply.RunnerWarnings() + j.mu.Lock() defer j.mu.Unlock() changed := false - out := make([]link.ResourceHealth, 0, len(ls)) + out := make([]link.ResourceHealth, 0, len(ls)+len(warnings)) + for _, w := range warnings { + v := link.ResourceHealth{Module: w.Module, Resource: w.Resource, Kind: link.KindRunner, State: link.StateUnhealthy, + Reason: "the tool runner's account joins no group: " + w.Why, Since: now.UTC(), Streak: 1} + if was, had := j.was[w.Resource]; had && was.Reason == v.Reason { + v.Since, v.Streak = was.Since, was.Streak+1 + } else { + changed = true + } + j.was[w.Resource] = v + out = append(out, v) + } for i, l := range ls { r := read[i] id := apply.LibraryAccess(l.Library) diff --git a/internal/apply/apply.go b/internal/apply/apply.go index bb36d36..dbad871 100644 --- a/internal/apply/apply.go +++ b/internal/apply/apply.go @@ -41,6 +41,9 @@ type Outcome struct { ID string `json:"id"` Type string `json:"type"` Target string `json:"target"` + // changes is what a tool runner leaving a group changes (`library:`, `identity:`): what the runner's + // unit names under restart-on. Never said. + changes string // Action is created · updated · unchanged · corrected · removed — and held-still, a container // a maintenance window is holding stopped, left for the next apply after it (novox/hq issue 224). // @@ -581,6 +584,8 @@ func ApplyMindingWindows( // cannot be made — its name or numbers another's here — fails, naming whose, and what needs it is not // attempted; the rest of the machine is. var identityFailures []*Error + // warnings is every tool runner's account that joins no group this apply: said, never failing it. + var warnings []RunnerWarning brokenGroups := map[int]bool{} if len(d.Groups) > 0 || len(d.Identities) > 0 { tool, toolErr := toolFor(sys) @@ -632,8 +637,12 @@ func ApplyMindingWindows( if id.Runner != "" { moved, said, err := joinGroup(ctx, sys, id.Runner, id.Name, store.Membership{Module: id.Module}, &known, run) if err != nil { - identityFailures = append(identityFailures, &Error{Resource: "identity " + id.Name, Err: err}) - log(fmt.Sprintf(" failed identity %s's runner: %v", id.Name, err)) + // A warning, never a failure of the apply or of the module: the module runs; its tools may not + // read its directories until the account is right (said as runner:). + warnings = append(warnings, RunnerWarning{Resource: "runner:" + id.Module, Module: id.Module, + Why: err.Error()}) + outcome.Detail = joinDetail(outcome.Detail, "its tool runner joins no group: "+err.Error()) + log(fmt.Sprintf(" warning identity %s's runner: %v", id.Name, err)) } else if moved { changed[IdentityChange(id.Module)] = true outcome.Detail = joinDetail(outcome.Detail, said) @@ -659,7 +668,15 @@ func ApplyMindingWindows( if err == nil { var moved bool var said string - moved, said, err = joinRunner(ctx, sys, l, &known, run) + var refused error + moved, said, refused = joinRunner(ctx, sys, l, &known, run) + if refused != nil { + // A warning, never a failure: the library is kept as it is, and said as runner:library:. + warnings = append(warnings, RunnerWarning{Resource: "runner:" + LibraryAccess(l.Library), + Why: refused.Error()}) + outcome.Detail = joinDetail(outcome.Detail, "its tool runner joins no group: "+refused.Error()) + log(fmt.Sprintf(" warning %s's runner: %v", LibraryAccess(l.Library), refused)) + } if moved { changed[LibraryAccess(l.Library)] = true outcome.Detail = joinDetail(outcome.Detail, said) @@ -679,6 +696,18 @@ func ApplyMindingWindows( } } in.held = heldRoots(verdicts, d.Libraries) + // A tool runner no longer put in a group leaves it before anything is applied, so its unit, which names the + // change under restart-on, is restarted in this same apply to lose it (ADR 0328 §11, ADR 0334 §2, ADR 0252 §3). + if origin == store.OriginDeclared { + for _, o := range leaveRunners(ctx, sys, d.Libraries, d.Identities, &known, run, wanted) { + if o.Action == "restored" { + changed[o.changes] = true + } + report.Outcomes = append(report.Outcomes, o) + log(fmt.Sprintf(" %s %s: %s", o.Action, o.ID, o.Detail)) + } + } + setRunnerWarnings(warnings) for i, resource := range ordered { if !orphansRemoved && i == guardFirst { // **Only a guard that is up may let the filter go.** Removing the derived filter's @@ -1065,11 +1094,6 @@ func ApplyMindingWindows( // Only the mesh says what is assigned: a carried bundle's silence undeclares no identity and no folder. if origin == store.OriginDeclared { - // A tool runner no longer put in a library's group leaves it (ADR 0328 §11, ADR 0252 §3). - for _, o := range leaveRunners(ctx, sys, d.Libraries, d.Identities, &known, run, wanted) { - report.Outcomes = append(report.Outcomes, o) - log(fmt.Sprintf(" %s %s: %s", o.Action, o.ID, o.Detail)) - } // And an identity, then a library group, no longer declared goes, once what ran as it went above (ADR 0328 // §3, §4). if tool, err := toolFor(sys); err == nil { @@ -1268,6 +1292,44 @@ func begin(r declaration.Resource) Outcome { return Outcome{ID: r.Identity(), Type: string(r.Kind()), Target: r.Target()} } +// dirModeOf is a directory's mode as declared: its permission bits, and the setgid bit when it is 2xxx (novox/hq ADR +// 0334 §1), as os.ModeSetgid. +func dirModeOf(spec string, fallback os.FileMode) (os.FileMode, error) { + mode, err := modeOf(spec, fallback) + if err != nil { + return 0, err + } + out := mode & os.ModePerm + if mode&0o2000 != 0 { + out |= os.ModeSetgid + } + return out, nil +} + +// dirBits is what of a directory's mode the mesh keeps: its permission bits and its setgid bit. +func dirBits(m os.FileMode) os.FileMode { return m & (os.ModePerm | os.ModeSetgid) } + +// unixDirMode is dirBits as a chmod takes it. +func unixDirMode(m os.FileMode) uint32 { + out := uint32(m.Perm()) + if m&os.ModeSetgid != 0 { + out |= 0o2000 + } + return out +} + +// statDirMode is a directory's mode as the kernel says it, in dirBits' terms. +func statDirMode(raw uint32) os.FileMode { + out := os.FileMode(raw & 0o777) + if raw&0o2000 != 0 { + out |= os.ModeSetgid + } + return out +} + +// modeText is a directory's mode as a person writes it: 0750, 2750. +func modeText(m os.FileMode) string { return fmt.Sprintf("%04o", unixDirMode(m)) } + func modeOf(spec string, fallback os.FileMode) (os.FileMode, error) { if spec == "" { return fallback, nil @@ -1292,7 +1354,7 @@ func modeOf(spec string, fallback os.FileMode) (os.FileMode, error) { // already has the declared owner and mode, so a person who sets them by hand at the machine hands it over. func applyDirectory(r *declaration.Directory, previous store.Applied, wasBefore bool) (Outcome, error) { out := begin(r) - mode, err := modeOf(r.Mode, 0o755) + mode, err := dirModeOf(r.Mode, 0o755) if err != nil { return out, err } @@ -1336,20 +1398,20 @@ func applyDirectory(r *declaration.Directory, previous store.Applied, wasBefore if err != nil { return out, err } - if !owned || before.Mode().Perm() != mode.Perm() { + if !owned || dirBits(before.Mode()) != dirBits(mode) { out.Action, out.asFound = "unchanged", true owner := r.Owner if owner == "" { owner = "root" } - out.asFoundDeclared = fmt.Sprintf("%s, mode %o", owner, mode.Perm()) + out.asFoundDeclared = fmt.Sprintf("%s, mode %s", owner, modeText(mode)) out.asFoundAt = time.Now().UTC() if previous.AsFound && filepath.Clean(previous.Target) == filepath.Clean(r.Path) && !previous.FoundAt.IsZero() { out.asFoundAt = previous.FoundAt } - out.Detail = fmt.Sprintf("found here before the mesh and used as found: its owner %s and mode %o are "+ + out.Detail = fmt.Sprintf("found here before the mesh and used as found: its owner %s and mode %s are "+ "left, though %s were declared (novox/hq issue 339); `nox node hand-over %s` on the "+ - "control-node hands it to the mesh", ownerText(before), before.Mode().Perm(), out.asFoundDeclared, r.Path) + "control-node hands it to the mesh", ownerText(before), modeText(dirBits(before.Mode())), out.asFoundDeclared, r.Path) return out, nil } } @@ -1380,8 +1442,8 @@ func applyDirectory(r *declaration.Directory, previous store.Applied, wasBefore if !after.IsDir() { return out, fmt.Errorf("%s is not a directory after applying", r.Path) } - if after.Mode().Perm() != mode.Perm() { - return out, fmt.Errorf("%s is mode %o after setting %o", r.Path, after.Mode().Perm(), mode.Perm()) + if dirBits(after.Mode()) != dirBits(mode) { + return out, fmt.Errorf("%s is mode %s after setting %s", r.Path, modeText(dirBits(after.Mode())), modeText(mode)) } ownedAlready, err := ownedBy(r.Path, r.Owner) @@ -1399,9 +1461,9 @@ func applyDirectory(r *declaration.Directory, previous store.Applied, wasBefore out.Action = "created" } else if !ownedAlready { out.Action = "updated" - } else if before.Mode().Perm() != mode.Perm() { + } else if dirBits(before.Mode()) != dirBits(mode) { out.Action = "updated" - out.Detail = fmt.Sprintf("mode %o to %o", before.Mode().Perm(), mode.Perm()) + out.Detail = fmt.Sprintf("mode %s to %s", modeText(dirBits(before.Mode())), modeText(mode)) } return out, nil } @@ -1437,10 +1499,10 @@ func applyHandedOver(r *declaration.Directory, out Outcome, mode os.FileMode) (O return out, err } out.Action = "unchanged" - if before.Perm() != mode.Perm() || (chown && (beforeUID != uid || beforeGID != gid)) { + if dirBits(before) != dirBits(mode) || (chown && (beforeUID != uid || beforeGID != gid)) { out.Action = "updated" - out.Detail = fmt.Sprintf("handed to the mesh: owner %d:%d, mode %o, given what was declared", beforeUID, beforeGID, - before.Perm()) + out.Detail = fmt.Sprintf("handed to the mesh: owner %d:%d, mode %s, given what was declared", beforeUID, beforeGID, + modeText(dirBits(before))) } return out, nil } diff --git a/internal/apply/homes_linux.go b/internal/apply/homes_linux.go index 79d2844..80ea742 100644 --- a/internal/apply/homes_linux.go +++ b/internal/apply/homes_linux.go @@ -137,7 +137,7 @@ func chmodUnderAs(home, path string, mode os.FileMode, want int) error { return err } defer unix.Close(fd) - if err := unix.Fchmod(fd, uint32(mode.Perm())); err != nil { + if err := unix.Fchmod(fd, unixDirMode(mode)); err != nil { return &os.PathError{Op: "chmod", Path: path, Err: err} } return nil @@ -256,8 +256,8 @@ func giveHandedOver(path string, mode os.FileMode, uid, gid int, chown bool) (be if err := unix.Fstat(fd, &st); err != nil { return 0, 0, 0, &os.PathError{Op: "fstat", Path: path, Err: err} } - before, beforeUID, beforeGID = os.FileMode(st.Mode&0o777), int(st.Uid), int(st.Gid) - if err := unix.Fchmod(fd, uint32(mode.Perm())); err != nil { + before, beforeUID, beforeGID = statDirMode(st.Mode), int(st.Uid), int(st.Gid) + if err := unix.Fchmod(fd, unixDirMode(mode)); err != nil { return 0, 0, 0, &os.PathError{Op: "chmod", Path: path, Err: err} } if chown && (beforeUID != uid || beforeGID != gid) { diff --git a/internal/apply/library.go b/internal/apply/library.go index d1235f9..a198813 100644 --- a/internal/apply/library.go +++ b/internal/apply/library.go @@ -466,9 +466,11 @@ func leaveRunners(ctx context.Context, sys system.System, libs []declaration.Lib keep = append(keep, m) continue } - o := Outcome{ID: LibraryAccess(m.Library) + " runner", Type: "library", Target: m.Account} + o := Outcome{ID: LibraryAccess(m.Library) + " runner", Type: "library", Target: m.Account, + changes: LibraryAccess(m.Library)} if m.Module != "" { o.ID, o.Type = "identity "+declaration.IdentityPrefix+m.Module+" runner", "identity" + o.changes = IdentityChange(m.Module) } if others := wanted.othersAsk(m.Account, m.Group, ""); len(others) > 0 { o.Action, o.Detail = "forgotten", fmt.Sprintf("left %s in %s, which %s asks for", m.Account, m.Group, @@ -534,3 +536,28 @@ func unionPaths(a, b []string) []string { sort.Strings(a) return a } + +// RunnerWarning is a tool runner's account that joins no group (missing, or root's): said as a resource of its own, +// `runner:` or `runner:library:`, unhealthy with words with no path — never a failure of the apply. +type RunnerWarning struct { + Resource, Module, Why string +} + +var runnerWarnings struct { + sync.Mutex + list []RunnerWarning +} + +// setRunnerWarnings is the warnings the last apply found, in place of the ones before. +func setRunnerWarnings(ws []RunnerWarning) { + runnerWarnings.Lock() + defer runnerWarnings.Unlock() + runnerWarnings.list = append([]RunnerWarning(nil), ws...) +} + +// RunnerWarnings is every tool runner's account the last apply could not put in a group. +func RunnerWarnings() []RunnerWarning { + runnerWarnings.Lock() + defer runnerWarnings.Unlock() + return append([]RunnerWarning(nil), runnerWarnings.list...) +} diff --git a/internal/apply/library_test.go b/internal/apply/library_test.go index 4bb756b..29ce381 100644 --- a/internal/apply/library_test.go +++ b/internal/apply/library_test.go @@ -872,9 +872,17 @@ func TestTheToolRunnerJoinsAModulesOwnGroup(t *testing.T) { m := &accountsMachine{users: map[string][2]int{"root": {0, 0}, "admin": {1001, 10}}, groups: map[string][2]int{}} d := withGroups(t, ``, `{"module":"radarr","name":"mesh-radarr","uid":61001,"gid":61001,"runner":"`+ runner+`"}`, ``) - _, _, err := Apply(context.Background(), archHost(t), d, store.State{}, store.OriginDeclared, m.run, nil, nil) - if err == nil || !strings.Contains(err.Error(), says) { - t.Errorf("%s: not refused saying %q: %v", runner, says, err) + report, _, err := Apply(context.Background(), archHost(t), d, store.State{}, store.OriginDeclared, m.run, nil, nil) + if err != nil { + t.Errorf("%s: a refused runner failed the apply: %v", runner, err) + } + ws := RunnerWarnings() + if len(ws) != 1 || ws[0].Resource != "runner:radarr" || ws[0].Module != "radarr" || + !strings.Contains(ws[0].Why, says) || absPath.MatchString(ws[0].Why) { + t.Errorf("%s: not said as runner:radarr saying %q, with no path: %+v", runner, says, ws) + } + if o := outcomeOf(report, "identity mesh-radarr"); o.Action != "created" { + t.Errorf("%s: the identity was not made beside the refused runner: %+v", runner, o) } if strings.Contains(strings.Join(m.ran, "\n"), "usermod --append") { t.Errorf("%s: put in a group", runner) @@ -921,3 +929,96 @@ func TestTheToolRunnerIsRestartedOnceForTheOwnGroup(t *testing.T) { t.Fatalf("the tool runner was restarted %d times with nothing moved", stops()) } } + +// Review: a library's runner that is refused keeps the library's outcome, is said as runner:library:, and never +// fails the apply; a runner that leaves a group restarts the tool runner once, in the apply it leaves. +func TestARefusedLibraryRunnerIsAWarningAndLeavingRestarts(t *testing.T) { + gid := testGID(t) + libraries(t, []string{"/srv/media"}, nil) + m := &accountsMachine{users: map[string][2]int{"nox": {1003, 1003}}, groups: map[string][2]int{"nox": {1003, 0}}} + lib := func(runner string) string { + return `{"library":"media","root":"/srv/media","group":"mesh-media","gid":` + strconv.Itoa(gid) + + `,"home":false,"runner":"` + runner + `"}` + } + report, _, err := Apply(context.Background(), archHost(t), libraryDecl(t, lib("ghost")), store.State{}, + store.OriginDeclared, m.run, nil, nil) + if err != nil { + t.Fatalf("a refused library runner failed the apply: %v", err) + } + if o := outcomeOf(report, "library:media"); o.ID == "" { + t.Fatal("the library's outcome was dropped") + } + if ws := RunnerWarnings(); len(ws) != 1 || ws[0].Resource != "runner:library:media" { + t.Fatalf("not said as runner:library:media: %+v", ws) + } + + // Joins, then leaves: the tool runner's unit, naming library:media, is restarted on each. + var verbs []string + systemd := systemctlStub(t, "loaded", "active", "enabled", &verbs) + run := func(ctx context.Context, name string, args ...string) (string, error) { + if name == "systemctl" { + return systemd(ctx, name, args...) + } + return m.run(ctx, name, args...) + } + unit := `{"id":"node-tools.unit","type":"service","unit":"node-tools.service","state":"running",` + + `"restart-on":["library:media"]}` + _, known, err := Apply(context.Background(), archHost(t), libraryDecl(t, lib("nox"), unit), store.State{}, + store.OriginDeclared, run, nil, nil) + if err != nil { + t.Fatal(err) + } + if len(RunnerWarnings()) != 0 { + t.Error("a warning stayed once the runner was right") + } + verbs = nil + gone := `{"library":"media","root":"/srv/media","group":"mesh-media","gid":` + strconv.Itoa(gid) + `,"home":false}` + if _, _, err := Apply(context.Background(), archHost(t), libraryDecl(t, gone, unit), known, store.OriginDeclared, run, + nil, nil); err != nil { + t.Fatal(err) + } + if !strings.Contains(strings.Join(m.ran, "\n"), "gpasswd --delete nox mesh-media") || + strings.Count(strings.Join(verbs, " "), "stop") != 1 { + t.Fatalf("leaving the group did not restart the tool runner once: %v %v", verbs, m.ran) + } +} + +// ADR 0334 §1: a module's own directory may carry the setgid bit, so what is made beneath it keeps its own group; +// it is set, held across applies, put back and said when it is taken off. Setuid and sticky are refused. +func TestADirectorysSetgidBitIsKept(t *testing.T) { + dir := filepath.Join(t.TempDir(), "state") + d := declare(t, `{"id":"radarr.state","type":"directory","path":"`+dir+`","mode":"2750"}`) + if _, _, err := Apply(context.Background(), archHost(t), d, store.State{}, store.OriginDeclared, noServices, nil, + nil); err != nil { + t.Fatalf("a directory with setgid was refused: %v", err) + } + if st := statOf(t, dir); st.Mode&0o7777 != 0o2750 { + t.Fatalf("made at %o, and 2750 was declared", st.Mode&0o7777) + } + _, known, err := Apply(context.Background(), archHost(t), d, store.State{}, store.OriginDeclared, noServices, nil, nil) + if err != nil { + t.Fatal(err) + } + if st := statOf(t, dir); st.Mode&0o7777 != 0o2750 { + t.Fatalf("the setgid bit did not hold across applies: %o", st.Mode&0o7777) + } + if err := unix.Chmod(dir, 0o750); err != nil { + t.Fatal(err) + } + report, _, err := Apply(context.Background(), archHost(t), d, known, store.OriginDeclared, noServices, nil, nil) + if err != nil { + t.Fatal(err) + } + if o := outcomeOf(report, "radarr.state"); o.Action != "updated" || !strings.Contains(o.Detail, "0750 to 2750") { + t.Fatalf("the setgid bit taken off was not put back and said: %+v", o) + } + if st := statOf(t, dir); st.Mode&0o7777 != 0o2750 { + t.Fatalf("the setgid bit was not put back: %o", st.Mode&0o7777) + } + for _, mode := range []string{"4750", "1750", "6750"} { + if _, err := declaration.Parse([]byte(`{"declaration":1,"resources":[{"id":"m.d","type":"directory","path":"` + + dir + `","mode":"` + mode + `"}]}`)); err == nil { + t.Errorf("a directory mode %s was accepted", mode) + } + } +} diff --git a/internal/apply/mesh_dirs_apply_linux.go b/internal/apply/mesh_dirs_apply_linux.go index 754c845..af56f41 100644 --- a/internal/apply/mesh_dirs_apply_linux.go +++ b/internal/apply/mesh_dirs_apply_linux.go @@ -77,9 +77,10 @@ func applyMeshDirectory(r *declaration.Directory, out Outcome, mode os.FileMode, if err := unix.Fstat(fd, &st); err != nil { return out, &os.PathError{Op: "fstat", Path: r.Path, Err: err} } - beforeMode, beforeUID, beforeGID := os.FileMode(st.Mode&0o777), int(st.Uid), int(st.Gid) - // Set explicitly even when it existed: a mode given at creation is not a mode maintained. - if err := unix.Fchmod(fd, uint32(mode.Perm())); err != nil { + beforeMode, beforeUID, beforeGID := statDirMode(st.Mode), int(st.Uid), int(st.Gid) + // Set explicitly even when it existed: a mode given at creation is not a mode maintained — its setgid bit + // with it (novox/hq ADR 0334 §1). + if err := unix.Fchmod(fd, unixDirMode(mode)); err != nil { return out, &os.PathError{Op: "chmod", Path: r.Path, Err: err} } ownedAlready := !chown || (beforeUID == uid && beforeGID == gid) @@ -87,6 +88,10 @@ func applyMeshDirectory(r *declaration.Directory, out Outcome, mode os.FileMode, if err := unix.Fchown(fd, uid, gid); err != nil { return out, fmt.Errorf("cannot give %s to %q: %w", r.Path, r.Owner, &os.PathError{Op: "chown", Path: r.Path, Err: err}) } + // A chown may take the setgid bit: the mode is set again after it. + if err := unix.Fchmod(fd, unixDirMode(mode)); err != nil { + return out, &os.PathError{Op: "chmod", Path: r.Path, Err: err} + } } // Read back, from the descriptor that was changed. @@ -96,8 +101,8 @@ func applyMeshDirectory(r *declaration.Directory, out Outcome, mode os.FileMode, if st.Mode&unix.S_IFMT != unix.S_IFDIR { return out, fmt.Errorf("%s is not a directory after applying", r.Path) } - if got := os.FileMode(st.Mode & 0o777); got != mode.Perm() { - return out, fmt.Errorf("%s is mode %o after setting %o", r.Path, got, mode.Perm()) + if got := statDirMode(st.Mode); got != dirBits(mode) { + return out, fmt.Errorf("%s is mode %s after setting %s", r.Path, modeText(got), modeText(mode)) } out.Action = "unchanged" @@ -106,9 +111,9 @@ func applyMeshDirectory(r *declaration.Directory, out Outcome, mode os.FileMode, out.Action = "created" case !ownedAlready: out.Action = "updated" - case beforeMode != mode.Perm(): + case beforeMode != dirBits(mode): out.Action = "updated" - out.Detail = fmt.Sprintf("mode %o to %o", beforeMode, mode.Perm()) + out.Detail = fmt.Sprintf("mode %s to %s", modeText(beforeMode), modeText(mode)) } return out, nil } diff --git a/internal/declaration/declaration.go b/internal/declaration/declaration.go index 5bd7ff6..4e54fc7 100644 --- a/internal/declaration/declaration.go +++ b/internal/declaration/declaration.go @@ -133,7 +133,13 @@ func (d *Directory) validate(where string, _ bool) []string { if d.Path == "" { problems = append(problems, where+": a directory needs a path") } - return append(problems, checkMode(where, d.Mode)...) + // A directory's mode may carry the setgid bit (2xxx), so what is made beneath it keeps its group (novox/hq ADR + // 0334 §1); never setuid or sticky, which mean nothing the mesh asks of a directory it owns. + mode := d.Mode + if len(mode) == 4 && mode[0] == '2' { + mode = "0" + mode[1:] + } + return append(problems, checkMode(where, mode)...) } // File is a file with literal content. The host renders nothing. diff --git a/internal/link/messages.go b/internal/link/messages.go index 69f2c1c..08b237c 100644 --- a/internal/link/messages.go +++ b/internal/link/messages.go @@ -327,6 +327,11 @@ const KindAccess = "access" // reason keep or disk, unknown while its filesystem could not be named, unhealthy otherwise. const KindLibrary = "library" +// KindRunner is a tool runner's account the engine could not put in a group (novox/hq ADR 0334 §2, ADR 0328 §11): +// missing, or root's. Said unhealthy as `runner:` (with its module) or `runner:library:`, words with no +// path, never failing an apply; gone once the account is right. +const KindRunner = "runner" + // ReasonUsedAsFound starts the reason of a directory used as found. const ReasonUsedAsFound = "used as found:" -- 2.54.0