diff --git a/fleetd/src/main/java/dev/ltms/fleet/session/GitWorktrees.java b/fleetd/src/main/java/dev/ltms/fleet/session/GitWorktrees.java index 23d7740..2314d5a 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/session/GitWorktrees.java +++ b/fleetd/src/main/java/dev/ltms/fleet/session/GitWorktrees.java @@ -656,13 +656,31 @@ public final class GitWorktrees implements Worktrees { *
    *
  1. {@code git -C repoRoot config core.sharedRepository group} so every future write by * either uid stays group-writable;
    2. - *
  2. a one-time {@code chgrp}/{@code chmod g+rwX} fix-up over the worktree directory, the - * repo's {@code .git/objects}, {@code refs}, {@code logs}, {@code worktrees}, and (when - * present) {@code packed-refs}, with setgid ({@code chmod g+s}) applied only to the - * directories among them so files created later inherit the group;
    3. + *
  3. a one-time {@code chgrp}/{@code chmod g+rwX} fix-up over the worktree directory and, + * under the repo's common git directory, {@code objects}, {@code refs}, + * {@code logs}, {@code worktrees} and {@code packed-refs} — with setgid + * ({@code chmod g+s}) applied only to the directories among them, so files created later + * inherit the group;
    4. *
  4. one INFO line naming the group and the paths touched.
    5. *
* + *

Every path is skipped when it does not exist. {@code .git/logs} is absent in a repo + * with {@code core.logAllRefUpdates=false} or one that has had no ref update yet, and + * {@code packed-refs} is absent until refs are packed. Passing a missing path to {@code chgrp} + * exits non-zero, which would fail every provisioning spawn with a message blaming a + * group that is in fact fine. + * + *

The git directory is resolved, not assumed. {@code /.git} is a + * file, not a directory, when the checkout is itself a linked worktree — the very + * thing this class creates for every member. {@code git rev-parse --git-common-dir} gives the + * real shared store, and it may answer relatively, so it is resolved against {@code repoRoot}. + * + *

The fix-up re-runs on every spawn, by design. {@code core.sharedRepository=group} + * governs only what git writes after it is set; the walk is what covers everything + * already on disk. It is not redundant work to optimise away — dropping it silently leaves + * pre-existing objects unreadable to the member. It costs three walks of the object store per + * spawn (about 3000 files in this repo, well under a second, but it grows with the repo). + * *

This only fixes up file ownership/permissions on the operator's shared repo so a * different-uid member can write to it — it isolates credentials, not the repository. A member * in the group can still write the operator's git objects and refs. @@ -679,16 +697,12 @@ public final class GitWorktrees implements Worktrees { List touched = new ArrayList<>(); try { shareGroupRunner.apply(new String[]{"git", "-C", repoRoot, "config", "core.sharedRepository", "group"}); - for (String dir : List.of(worktreePath, repoRoot + "/.git/objects", repoRoot + "/.git/refs", - repoRoot + "/.git/logs", repoRoot + "/.git/worktrees")) { - shareGroupPath(dir, true); - touched.add(dir); - } - String packedRefs = repoRoot + "/.git/packed-refs"; - if (Files.exists(Path.of(packedRefs))) { - shareGroupPath(packedRefs, false); - touched.add(packedRefs); + String commonDir = gitCommonDir(repoRoot); + shareGroupPathIfPresent(worktreePath, true, touched); + for (String name : List.of("objects", "refs", "logs", "worktrees")) { + shareGroupPathIfPresent(commonDir + "/" + name, true, touched); } + shareGroupPathIfPresent(commonDir + "/packed-refs", false, touched); } catch (WorktreeException e) { throw new WorktreeException("cannot share worktree with group '" + group + "': " + e.getMessage() + " — the group must exist, and the fleetd operator (" @@ -698,6 +712,38 @@ public final class GitWorktrees implements Worktrees { group, repoRoot, worktreePath, touched); } + /** + * The repo's common git directory as an absolute path — where {@code objects}, + * {@code refs} and {@code worktrees} actually live. {@code git rev-parse --git-common-dir} + * answers relative to {@code repoRoot} in the ordinary case ({@code .git}) and absolutely for a + * linked worktree, so the answer is resolved against {@code repoRoot} either way. Never + * hardcode {@code repoRoot + "/.git"}: that is a FILE when the checkout is itself a linked + * worktree. + */ + private String gitCommonDir(String repoRoot) { + String answer = shareGroupRunner.apply( + new String[]{"git", "-C", repoRoot, "rev-parse", "--git-common-dir"}); + String trimmed = answer == null ? "" : answer.trim(); + if (trimmed.isEmpty()) { + trimmed = ".git"; + } + return Path.of(repoRoot).resolve(trimmed).normalize().toString(); + } + + /** + * {@link #shareGroupPath} when {@code path} exists, recording it in {@code touched}; otherwise + * nothing at all. A missing path is normal, not an error — see {@link #shareWithGroup}'s + * javadoc for which ones are routinely absent and why passing them to {@code chgrp} would fail + * every spawn. + */ + private void shareGroupPathIfPresent(String path, boolean recursive, List touched) { + if (!Files.exists(Path.of(path))) { + return; + } + shareGroupPath(path, recursive); + touched.add(path); + } + /** * {@code chgrp}/{@code chmod g+rwX} {@code path} to {@link #group}. When {@code recursive}, * also walks the directories under {@code path} (including {@code path} itself, when it is a diff --git a/fleetd/src/test/java/dev/ltms/fleet/session/GitWorktreesTest.java b/fleetd/src/test/java/dev/ltms/fleet/session/GitWorktreesTest.java index c104a46..21f5402 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/session/GitWorktreesTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/session/GitWorktreesTest.java @@ -935,6 +935,21 @@ class GitWorktreesTest { return List.of(command); } + /** Remove {@code path} and anything under it. Tolerates an already-absent path. */ + private static void deleteRecursively(Path path) throws Exception { + if (!Files.exists(path)) { + return; + } + if (Files.isDirectory(path)) { + try (java.util.stream.Stream children = Files.list(path)) { + for (Path child : children.toList()) { + deleteRecursively(child); + } + } + } + Files.delete(path); + } + /** {@code worktreeGroup} absent ⇒ zero processes spawned and no git config written. */ @Test void shareWithGroupIsNoopWhenNoGroupConfigured(@TempDir Path tmp) throws Exception { @@ -957,11 +972,14 @@ class GitWorktreesTest { void shareWithGroupRunsConfigThenChgrpChmodSetgidPerPath(@TempDir Path tmp) throws Exception { Path repo = initRepo(tmp.resolve("repo")); String repoRoot = repo.toString(); - String worktreePath = repo.resolve("some-worktree").toString(); + Path worktree = Files.createDirectories(repo.resolve("some-worktree")); + String worktreePath = worktree.toString(); + Files.createDirectories(repo.resolve(".git/worktrees")); List> recorded = new java.util.ArrayList<>(); java.util.function.Function recordingRunner = cmd -> { recorded.add(joined(cmd)); - return ""; + // What real git answers for an ordinary (non-linked) checkout: relative to repoRoot. + return List.of(cmd).contains("--git-common-dir") ? ".git\n" : ""; }; GitWorktrees gitWorktrees = new GitWorktrees(tmp.resolve("wts").toString(), "devteam", _ -> {}, recordingRunner); @@ -970,6 +988,9 @@ class GitWorktreesTest { assertEquals(List.of("git", "-C", repoRoot, "config", "core.sharedRepository", "group"), recorded.get(0), "core.sharedRepository must be set first, so it keeps working after the one-time fix-up"); + assertTrue(recorded.contains(List.of("git", "-C", repoRoot, "rev-parse", "--git-common-dir")), + "the git dir must be asked for, never hardcoded as /.git — that is a FILE " + + "when the checkout is itself a linked worktree: " + recorded); for (String dir : List.of(worktreePath, repoRoot + "/.git/objects", repoRoot + "/.git/refs", repoRoot + "/.git/logs", repoRoot + "/.git/worktrees")) { @@ -985,6 +1006,61 @@ class GitWorktreesTest { "packed-refs is absent here and must be skipped, not chgrp'd: " + recorded); } + /** + * A path that does not exist is skipped, never handed to {@code chgrp}. {@code .git/logs} is + * absent whenever {@code core.logAllRefUpdates} is false or no ref has been updated yet, and + * {@code chgrp} on a missing path exits non-zero — which would fail EVERY provisioning spawn + * with a message blaming a group that is in fact fine. + */ + @Test + void shareWithGroupSkipsPathsThatDoNotExist(@TempDir Path tmp) throws Exception { + Path repo = initRepo(tmp.resolve("repo")); + String repoRoot = repo.toString(); + deleteRecursively(repo.resolve(".git/logs")); + assertFalse(Files.exists(repo.resolve(".git/logs")), "fixture: .git/logs must be gone"); + List> recorded = new java.util.ArrayList<>(); + java.util.function.Function recordingRunner = cmd -> { + recorded.add(joined(cmd)); + return List.of(cmd).contains("--git-common-dir") ? ".git\n" : ""; + }; + GitWorktrees gitWorktrees = + new GitWorktrees(tmp.resolve("wts").toString(), "devteam", _ -> {}, recordingRunner); + + gitWorktrees.shareWithGroup(repoRoot, repo.resolve("no-such-worktree").toString()); + + String logs = repoRoot + "/.git/logs"; + assertTrue(recorded.stream().noneMatch(c -> c.contains(logs)), + "a missing .git/logs must be skipped, not chgrp'd: " + recorded); + assertTrue(recorded.stream().noneMatch(c -> c.contains(repo.resolve("no-such-worktree").toString())), + "a missing worktree path must be skipped too: " + recorded); + assertTrue(recorded.contains(List.of("chgrp", "-R", "devteam", repoRoot + "/.git/objects")), + "paths that DO exist are still shared: " + recorded); + } + + /** + * The git store is located by {@code rev-parse --git-common-dir}, not by appending + * {@code /.git}. When git answers with an absolute path — what it does for a linked worktree, + * where {@code /.git} is a file — every shared path must follow that answer. + */ + @Test + void shareWithGroupFollowsAnAbsoluteGitCommonDir(@TempDir Path tmp) throws Exception { + Path repo = initRepo(tmp.resolve("repo")); + Path realGitDir = repo.resolve(".git"); + List> recorded = new java.util.ArrayList<>(); + java.util.function.Function recordingRunner = cmd -> { + recorded.add(joined(cmd)); + return List.of(cmd).contains("--git-common-dir") ? realGitDir + "\n" : ""; + }; + GitWorktrees gitWorktrees = + new GitWorktrees(tmp.resolve("wts").toString(), "devteam", _ -> {}, recordingRunner); + + gitWorktrees.shareWithGroup(tmp.resolve("some/linked/worktree").toString(), + repo.resolve("wt").toString()); + + assertTrue(recorded.contains(List.of("chgrp", "-R", "devteam", realGitDir + "/objects")), + "objects must be taken from the reported common dir, not /.git: " + recorded); + } + /** {@code packed-refs}, when present, is chgrp/chmod'd but never setgid'd (it is a file, not a dir). */ @Test void shareWithGroupIncludesPackedRefsWhenPresent(@TempDir Path tmp) throws Exception {