From 9a807de65960e41cbb2db84b41fd3031ff1cb2ae Mon Sep 17 00:00:00 2001 From: jochen Date: Fri, 9 Oct 2026 19:58:27 +0200 Subject: [PATCH] Give a handed-over directory through no link on every apply, not only the first (hq issue 356, review) After the first apply the record is no longer used as found, so the directory read as the mesh's own and later applies set its mode and owner by path: a parent swapped for a link after the hand-over took carried root's chmod and chown to its target. The hand-over is now checked before that shortcut, so a handed-over directory always goes through applyHandedOver. The test swaps the parent and the grandparent after an apply already gave the directory its mode; both cases fail with the old order. --- internal/apply/apply.go | 13 ++++++++----- internal/apply/found_is_said_test.go | 25 +++++++++++++++++++++++-- 2 files changed, 31 insertions(+), 7 deletions(-) diff --git a/internal/apply/apply.go b/internal/apply/apply.go index e6c9e1e..e5ff726 100644 --- a/internal/apply/apply.go +++ b/internal/apply/apply.go @@ -1045,13 +1045,16 @@ func applyDirectory(r *declaration.Directory, previous store.Applied, wasBefore if existed && !before.IsDir() { return out, fmt.Errorf("%s exists and is not a directory", r.Path) } + // Handed to the mesh by the operator (`nox node hand-over`, or `hand-over` at the machine): given the declared + // owner and mode through a descriptor that followed no link (giveHandedOver) — **on every apply, not only the + // first** (review of issue 356). After the first, the record is no longer used as found and would read as the + // mesh's own, whose mode and owner are set by path; a parent swapped for a link after that would carry root's + // chmod and chown to its target. + if existed && HandedOver()[handOverKey(r.ID, r.Path)] { + return applyHandedOver(r, out, mode) + } if existed && wasBefore { ours := previous.Target != "" && filepath.Clean(previous.Target) == filepath.Clean(r.Path) && !previous.AsFound - // Or handed to the mesh by the operator (`nox node hand-over`, or `hand-over` at the machine): then it is - // given the declared owner and mode — through a descriptor that followed no link (giveHandedOver). - if !ours && HandedOver()[handOverKey(r.ID, r.Path)] { - return applyHandedOver(r, out, mode) - } if !ours { owned, err := ownedBy(r.Path, r.Owner) if err != nil { diff --git a/internal/apply/found_is_said_test.go b/internal/apply/found_is_said_test.go index 5510466..5155cce 100644 --- a/internal/apply/found_is_said_test.go +++ b/internal/apply/found_is_said_test.go @@ -142,8 +142,16 @@ func TestAHandOverIsForTheResourceThatAskedForIt(t *testing.T) { // a victim tree. The apply refuses, and the victim's directory keeps its mode — and so does a directory whose own // name became a link, and one whose grandparent did. func TestAHandedOverDirectoryIsChangedThroughNoLink(t *testing.T) { - for _, swap := range []string{"parent", "grandparent", "itself"} { - t.Run(swap, func(t *testing.T) { + for _, c := range []struct { + swap string + applyOnce bool + }{{"parent", false}, {"grandparent", false}, {"itself", false}, {"parent", true}, {"grandparent", true}} { + swap, applyOnce := c.swap, c.applyOnce + name := swap + if applyOnce { + name += " after an apply gave it its owner" + } + t.Run(name, func(t *testing.T) { dir, err := filepath.EvalSymlinks(t.TempDir()) if err != nil { t.Fatal(err) @@ -164,6 +172,19 @@ func TestAHandedOverDirectoryIsChangedThroughNoLink(t *testing.T) { before := HandedOver HandedOver = func() map[string]bool { return map[string]bool{handOverKey("notes.data", found): true} } t.Cleanup(func() { HandedOver = before }) + if applyOnce { + // The hand-over takes: the directory is given its mode, and the record is no longer as found. + if _, state, err = Apply(context.Background(), archHost(t), declare(t, decl), state, store.OriginDeclared, + noServices, nil, nil); err != nil { + t.Fatal(err) + } + if a, _ := state.Find("notes.data"); a.AsFound { + t.Fatalf("still as found after the hand-over took: %+v", a) + } + if info, _ := os.Stat(found); info.Mode().Perm() != 0o755 { + t.Fatalf("the hand-over did not take: %o", info.Mode().Perm()) + } + } // The victim: a tree of the same shape somewhere else, its data directory mode 0700. victim := filepath.Join(dir, "victim", "top", "parent", "data")