From 8e9cc5830119328ac13e0832f81dfb73931cc5bb Mon Sep 17 00:00:00 2001 From: jochen Date: Sun, 1 Nov 2026 04:49:23 +0100 Subject: [PATCH] Fall back to a captured copy only for mesh-host, which the checker places beside no mesh-delivery check (issue 1585) Review of fb96216: a missing mesh-sdk or catalogue in a merge check is the checker's fault and fails, never judged against a stale copy. The captured placement guard is refreshed to mesh-host's main 233bb23. Claude-Session: https://claude.ai/code/session_01NcbgfJ9Gum47zkFKWRaovL --- .../internal/beside/beside.go | 18 ++- .../testdata/beside/CAPTURED | 14 ++- .../apply/placement_guard.go.captured | 103 +++++++++++++++++- 3 files changed, 120 insertions(+), 15 deletions(-) diff --git a/modules/mesh-delivery-indexer/internal/beside/beside.go b/modules/mesh-delivery-indexer/internal/beside/beside.go index ed3de0e..0bf4a8b 100644 --- a/modules/mesh-delivery-indexer/internal/beside/beside.go +++ b/modules/mesh-delivery-indexer/internal/beside/beside.go @@ -50,6 +50,11 @@ const Env = "MESH_CHECK_BESIDE" // CapturedSuffix is what a captured file's name carries that the repository's own does not. const CapturedSuffix = ".captured" +// notPlaced are the repositories the checker places beside no mesh-delivery check (besideRules in +// mesh-delivery-checker/internal/check/beside.go places mesh-sdk and the catalogue): a test reads them from the captured +// copy even in a merge check, and says so (issue 1585). Every other sibling missing from a merge check fails the test. +var notPlaced = map[string]bool{"mesh-host": true} + // Dir is the named repository a test reads — the clone beside a merge check, or the captured copy — and // says which in the test's log. func Dir(t testing.TB, repository string) string { @@ -57,17 +62,20 @@ func Dir(t testing.TB, repository string) string { root := os.Getenv(Env) if root != "" { dir := filepath.Join(root, repository) - if _, err := os.Stat(dir); err == nil { + _, err := os.Stat(dir) + switch { + case err == nil: t.Logf("judged against %s as cloned beside this check", dir) return dir + case !notPlaced[repository]: + // A sibling the checker places that is missing is the checker's fault, never judged against a stale copy. + t.Fatalf("%s is not beside this check in %s=%s, so its agreement with this repository cannot be "+ + "judged here: %v", repository, Env, root, err) } } - // Not beside this run: the checker places beside mesh-delivery only what its besideRules name for it (mesh-sdk and - // the catalogue, issue 1585), so a sibling it does not place (mesh-host) is read from the captured copy, said, and a - // check is never skipped. Only a sibling with neither fails. from := filepath.Join(Captured(), repository) if _, err := os.Stat(from); err != nil { - t.Fatalf("%s is neither beside this check (%s=%q) nor captured at %s: %v", repository, Env, root, from, err) + t.Fatalf("no captured copy of %s at %s: %v", repository, from, err) } if root != "" { t.Logf("%s is not beside this check in %s=%s (the checker places it beside no mesh-delivery check): read from "+ diff --git a/modules/mesh-delivery-indexer/testdata/beside/CAPTURED b/modules/mesh-delivery-indexer/testdata/beside/CAPTURED index 9f1bb1c..6db0cff 100644 --- a/modules/mesh-delivery-indexer/testdata/beside/CAPTURED +++ b/modules/mesh-delivery-indexer/testdata/beside/CAPTURED @@ -7,20 +7,22 @@ The copy carries the catalogue's private details: domains, first names in module private addresses. That is acceptable while mesh-sdk is private; if it ever goes public, this copy is a second place to clean besides the catalogue itself. -Only what go/catalogue's tests read is captured (R4b, issue 1379). Refreshed 2026-10-21 from the mains of both -repositories; before that it was mesh-controller's copy, which still held two modules that had left the catalogue. +Only what go/catalogue's tests read is captured (R4b, issue 1379), copied here from mesh-sdk with the judge's tests +(issue 1585). mesh-host refreshed 2026-11-01 from its main, the catalogue 2026-10-21 from its main; before that it +was mesh-controller's copy, which still held two modules that had left the catalogue. mesh-catalog b5dbfbb784507e211372a7c9640d91f7d6dadd84 modules/*/module.json, modules/*/files/**, modules/*/templates/**, predefined.json -mesh-host cdcba9ca73755854d21e4e657d7e46c32b9b5265 internal/apply/placement_guard.go +mesh-host 233bb23bbcb1a829ce2a6164a31a5c5cc4f3d950 internal/apply/placement_guard.go -To move them, from this repository's go/ directory, with the two repositories checked out beside mesh-sdk: +To move them, from mesh-delivery's modules/mesh-delivery-indexer directory, with the two repositories checked out +beside mesh-delivery: rm -rf testdata/beside/mesh-catalog testdata/beside/mesh-host mkdir -p testdata/beside/mesh-catalog testdata/beside/mesh-host/internal/apply - git -C ../../mesh-catalog archive -- predefined.json ':(glob)modules/*/module.json' \ + git -C ../../../mesh-catalog archive -- predefined.json ':(glob)modules/*/module.json' \ ':(glob)modules/*/files/**' ':(glob)modules/*/templates/**' | tar -x -C testdata/beside/mesh-catalog find testdata/beside -name module.json -exec mv {} {}.captured \; - git -C ../../mesh-host show :internal/apply/placement_guard.go \ + git -C ../../../mesh-host show :internal/apply/placement_guard.go \ > testdata/beside/mesh-host/internal/apply/placement_guard.go.captured and write the commits here. The placement guard is kept as .captured so no Go tool reads it as this diff --git a/modules/mesh-delivery-indexer/testdata/beside/mesh-host/internal/apply/placement_guard.go.captured b/modules/mesh-delivery-indexer/testdata/beside/mesh-host/internal/apply/placement_guard.go.captured index e23ef45..864c4cb 100644 --- a/modules/mesh-delivery-indexer/testdata/beside/mesh-host/internal/apply/placement_guard.go.captured +++ b/modules/mesh-delivery-indexer/testdata/beside/mesh-host/internal/apply/placement_guard.go.captured @@ -56,7 +56,7 @@ import ( "sync" "time" - "github.com/novox/mesh-host/internal/declaration" + "git.novox.be/novox/mesh-sdk/go/declaration" ) // protectedRoots are paths no directory, access or mount source may be, nor hold. @@ -384,6 +384,9 @@ func ownedByAccount(owner string, a homeAccount) bool { if owner == a.Name { return true } + if a.UID < 0 { + return false // an account the declaration makes has no uid yet: by name alone, never by a number + } user, _, _ := strings.Cut(owner, ":") if uid, err := strconv.Atoi(user); err == nil { return uid == a.UID @@ -413,12 +416,91 @@ const ( // refusePath says why a path is not placed or mounted; nil when it may be. module is the resource's module, // owner a directory's declared owner. func refusePath(path string, kind placing, module, owner string) error { + return refusePathWith(path, kind, module, owner, nil) +} + +// declaredHomes is the home of each account the declaration itself makes (a user resource with a home), by the +// account's name; the account is not in the user database yet when the declaration is judged, and the engine makes +// it where it is declared, before the directory that is owned by it is reached (novox/hq issue 1542). Its uid is +// not known, so a directory is owned by it by name alone. Only for the directory placement guard; no other +// reader of accountsOfHomes sees these. +func declaredHomes(resources []declaration.Resource) map[string]homeAccount { + var accounts []passwdEntry + var out map[string]homeAccount + for _, r := range resources { + u, ok := r.(*declaration.User) + if !ok || u.Name == "" { + continue + } + home := filepath.Clean(u.Home) + if u.Home == "" || !filepath.IsAbs(home) || home == "/" { + continue + } + if accounts == nil { + accounts = passwdEntries() + } + // Only ever adds a home. An account the machine has keeps the home it has (applyUser never moves it), so + // a declared home of one is not its home, unless it is the very home it has: the account an earlier apply + // made, as declared, which must be judged the same at every later apply. And a home at or below another + // account's, or at or above a protected root, is not a new one. + taken := false + for _, e := range accounts { + if e.name == u.Name && e.home != home { + taken = true + } + if e.name != u.Name && e.home != "" && atOrBelow(home, e.home) { + taken = true + } + } + for _, root := range protectedRoots { + taken = taken || home == root || below(root, home) + } + if taken { + continue + } + if out == nil { + out = map[string]homeAccount{} + } + out[home] = homeAccount{Name: u.Name, UID: -1} + } + return out +} + +// passwdEntry is an account of the user database and its home, whatever its uid. +type passwdEntry struct{ name, home string } + +// passwdEntries is every account of the user database with its home (empty and root homes as ""). +func passwdEntries() []passwdEntry { + out := []passwdEntry{} + f, err := os.Open(passwdFile) + if err != nil { + return out + } + defer f.Close() + sc := bufio.NewScanner(f) + for sc.Scan() { + fields := strings.Split(sc.Text(), ":") + if len(fields) < 6 { + continue + } + home := filepath.Clean(fields[5]) + if home == "." || home == "/" || home == "/nonexistent" { + home = "" + } + out = append(out, passwdEntry{fields[0], home}) + } + return out +} + +// refusePathWith is refusePath with the homes of the accounts the declaration makes (declaredHomes) counted +// among the machine's. +func refusePathWith(path string, kind placing, module, owner string, declared map[string]homeAccount) error { clean := filepath.Clean(path) if !filepath.IsAbs(clean) { return nil // the declaration refuses a relative path already; a named volume is not a path } for _, p := range []string{clean, resolved(clean)} { - if err := refuseOne(p, kind, module, owner); err != nil { + if err := refuseOne(p, kind, module, owner, declared); err != nil { if p != clean { err.Why = fmt.Sprintf("through a link, it is %s, and %s", p, err.Why) err.Path = clean @@ -429,7 +511,7 @@ func refusePath(path string, kind placing, module, owner string) error { return nil } -func refuseOne(path string, kind placing, module, owner string) *PlacementRefusedError { +func refuseOne(path string, kind placing, module, owner string, declared map[string]homeAccount) *PlacementRefusedError { for _, root := range protectedRoots { if path == root || below(root, path) { return &PlacementRefusedError{Path: path, Why: root + " is one of the machine's own directories, " + @@ -482,6 +564,18 @@ func refuseOne(path string, kind placing, module, owner string) *PlacementRefuse return nil } homes := accountsOfHomes() + if ssh && len(declared) > 0 { + merged := map[string]homeAccount{} + for home, a := range homes { + merged[home] = a + } + for home, a := range declared { + if _, held := merged[home]; !held { + merged[home] = a + } + } + homes = merged + } var deepest string for home := range homes { if below(path, home) && len(home) > len(deepest) { @@ -518,11 +612,12 @@ func moduleOfID(id string) string { // a file into it, because nothing judged the file's path. func refusedPlaces(resources []declaration.Resource) map[string]error { out := map[string]error{} + declared := declaredHomes(resources) for _, r := range resources { var err error switch res := r.(type) { case *declaration.Directory: - err = refusePath(res.Path, placingDirectory, moduleOfID(res.ID), res.Owner) + err = refusePathWith(res.Path, placingDirectory, moduleOfID(res.ID), res.Owner, declared) // And outside every home, the walk's judgement before anything is applied (novox/hq issue 358): a // link another account could make in its path refuses it, and with it what is below it, whatever // order they are declared in. The apply walks again at the moment of use. A directory handed over