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 2314d5a..d598889 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/session/GitWorktrees.java +++ b/fleetd/src/main/java/dev/ltms/fleet/session/GitWorktrees.java @@ -13,6 +13,7 @@ import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.StandardCopyOption; +import java.nio.file.attribute.PosixFilePermissions; import java.security.SecureRandom; import java.util.ArrayList; import java.util.HashSet; @@ -161,6 +162,7 @@ public final class GitWorktrees implements Worktrees { } catch (IOException e) { throw new WorktreeException("cannot create worktree root " + root + ": " + e.getMessage(), e); } + shareRootWithGroup(root); String wt = path.toAbsolutePath().toString(); log.info("adding worktree branch={} path={} base={}", branch, wt, base); removeUserInfoFromHttpsOrigin(repoRoot); @@ -712,6 +714,54 @@ public final class GitWorktrees implements Worktrees { group, repoRoot, worktreePath, touched); } + /** + * fleetd #224: make {@code worktreeRoot} ITSELF group-traversable — established once, here, + * where the root is created, never at a use site. {@link #shareWithGroup} shares each worktree + * (and the repo's common git dir) with {@link #group}, but never the PARENT directory that + * contains every worktree — and under {@code memberHerdrSocket:} the member pane runs as a + * different OS user, which needs the execute bit on every ancestor directory to reach anything + * underneath, no matter how carefully each child is shared. Without this, a member cannot read + * the ephemeral {@code opencode.json} #219 places under this root, cannot reach its own + * worktree, and cannot read #213's ZDOTDIR scrub when placed here either. + * + *

No-op — no process spawned — when {@link #group} is null/blank, so behaviour with + * {@code worktreeGroup:} unset (today's only live mode) is unchanged. Only {@code root} itself + * is touched (non-recursive): each child underneath is shared individually, either by + * {@link #shareWithGroup} for a worktree or by the launcher that generates it (fleetd #213/#219) + * for a scrub/config directory — sharing this level again would just duplicate that policy in + * the wrong layer. + * + *

Fails loudly, naming {@code root}, its mode at the time of the attempt, and {@link #group}: + * a member that starts and then cannot see its own checkout is worse than a refused spawn, since + * nothing about that failure mode points at a directory's permission bits. + */ + private void shareRootWithGroup(Path root) { + if (group == null) { + return; + } + String mode = currentPosixMode(root); + try { + shareGroupRunner.apply(new String[]{"chgrp", group, root.toString()}); + shareGroupRunner.apply(new String[]{"chmod", "g+x", root.toString()}); + } catch (WorktreeException e) { + throw new WorktreeException("cannot make worktree root " + root + " (mode " + mode + + ") group-traversable for group '" + group + "': " + e.getMessage() + + " — the group must exist, and the fleetd operator (" + System.getProperty("user.name") + + ") must be a member of it", e); + } + log.info("worktreeGroup={} made worktree root {} group-traversable (was mode {})", group, root, mode); + } + + /** {@code root}'s current POSIX permission string, or {@code "unknown"} on a filesystem that does + * not support POSIX permissions — used only to name the mode in a refusal message. */ + private static String currentPosixMode(Path root) { + try { + return PosixFilePermissions.toString(Files.getPosixFilePermissions(root)); + } catch (IOException | UnsupportedOperationException e) { + return "unknown"; + } + } + /** * 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} diff --git a/fleetd/src/test/java/dev/ltms/fleet/member/EnvAllowListScrubTest.java b/fleetd/src/test/java/dev/ltms/fleet/member/EnvAllowListScrubTest.java index 5d072be..d1534e0 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/member/EnvAllowListScrubTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/member/EnvAllowListScrubTest.java @@ -114,6 +114,68 @@ class EnvAllowListScrubTest { assertNull(EnvAllowListScrub.readReport(dir)); } + /** + * fleetd #224 criterion 5 (a gap the #221 reviewer flagged): {@link EnvAllowListScrub#shareWithGroup} + * lists one FLAT level of {@code dir} and shares every file it finds there — which does cover the + * OPTIONAL files a caller may or may not have written before calling it ({@code member-charter.md}, + * {@code ide-rules.md} — both written by {@code OpenCodeLauncher#writeConfig}), but nothing + * pinned that down. + * Without this test, a future change that writes a file AFTER the sharing call, or into a + * subdirectory, would pass every existing test while quietly leaving that file unreadable to a + * different-uid member. + * + *

This drives {@code shareWithGroup} directly against a directory holding several flat files — + * not only {@code opencode.json}, but also the two optional ones named above plus a third, + * unrelated file, so the assertion is "every flat file", not "the two files someone thought of". + */ + @Test + void shareWithGroupCoversEveryFlatFileIncludingTheOptionalOnes(@TempDir Path dir) throws Exception { + String group = currentUserGroup(); + Files.writeString(dir.resolve("opencode.json"), "{}\n"); + Files.writeString(dir.resolve("member-charter.md"), "# charter\n"); + Files.writeString(dir.resolve("ide-rules.md"), "# ide rules\n"); + Files.writeString(dir.resolve("another-flat-file.txt"), "unrelated\n"); + + EnvAllowListScrub.shareWithGroup(dir, group); + + assertEquals("rwxr-x---", java.nio.file.attribute.PosixFilePermissions.toString( + Files.getPosixFilePermissions(dir)), + "the directory itself must be group-traversable+readable, owner-only writable"); + for (String name : List.of("opencode.json", "member-charter.md", "ide-rules.md", "another-flat-file.txt")) { + Path file = dir.resolve(name); + assertEquals("rw-r-----", java.nio.file.attribute.PosixFilePermissions.toString( + Files.getPosixFilePermissions(file)), + name + " must be group-readable, never group-writable"); + } + } + + /** + * The current process's REAL primary group — resolved via {@code id -gn}, never by reading a + * directory's owning group (fleetd #225: that reads wherever Maven happened to be started from, + * not the process's own group, and the two diverge outside a home checkout). Skips (never fails) + * when {@code id} is unavailable or its primary group cannot be resolved on this host. + */ + private static String currentUserGroup() { + String out; + boolean ok; + try { + Process p = new ProcessBuilder("id", "-gn").redirectErrorStream(true).start(); + String raw = new String(p.getInputStream().readAllBytes(), StandardCharsets.UTF_8).trim(); + out = raw; + ok = p.waitFor(5, java.util.concurrent.TimeUnit.SECONDS) && p.exitValue() == 0 && !raw.isBlank(); + } catch (IOException e) { + out = null; + ok = false; + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + out = null; + ok = false; + } + assumeTrue(ok, "cannot resolve this process's real primary group via `id -gn` on this host " + + "— skipping a POSIX-group-dependent test rather than failing it"); + return out; + } + /** * The same equality, for a shell that is INTERACTIVE but NOT a login shell — the shape herdr * opens on Linux. diff --git a/fleetd/src/test/java/dev/ltms/fleet/member/HerdrPeerLauncherAllowListWiringTest.java b/fleetd/src/test/java/dev/ltms/fleet/member/HerdrPeerLauncherAllowListWiringTest.java index cb0ea16..2d0d69b 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/member/HerdrPeerLauncherAllowListWiringTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/member/HerdrPeerLauncherAllowListWiringTest.java @@ -18,7 +18,6 @@ import org.slf4j.LoggerFactory; import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; -import java.nio.file.attribute.PosixFileAttributeView; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -513,11 +512,37 @@ class HerdrPeerLauncherAllowListWiringTest { + "java.io.tmpdir, unchanged from before this fix: " + dir); } - /** The current process's own primary group — resolvable on whatever host runs this test. */ - private static String currentUserGroup() throws IOException { - PosixFileAttributeView view = Files.getFileAttributeView(Path.of("."), PosixFileAttributeView.class); - assumeTrue(view != null, "this host's filesystem does not support POSIX group ownership"); - return view.readAttributes().group().getName(); + /** + * The current process's REAL primary group — resolved via {@code id -gn}, never by reading a + * directory's owning group (fleetd #225). Those two coincide only by accident: a directory's + * group is whichever group happened to own the path Maven was started from — {@code staff} in + * a home checkout, {@code wheel} under {@code /private/tmp} on macOS — and the fix-up this test + * exercises then fails for real when the operator is not a member of that borrowed group, + * exactly the case {@code assumeTrue(view != null, ...)} never covered (it only detects a + * filesystem with no POSIX groups at all, not a resolvable-but-wrong one). Skips (never fails) + * when {@code id} is unavailable or its primary group cannot be resolved on this host. + */ + private static String currentUserGroup() { + String out; + boolean ok; + try { + Process p = new ProcessBuilder("id", "-gn").redirectErrorStream(true).start(); + try (java.io.BufferedReader r = new java.io.BufferedReader( + new java.io.InputStreamReader(p.getInputStream(), java.nio.charset.StandardCharsets.UTF_8))) { + out = r.lines().collect(java.util.stream.Collectors.joining("\n")).trim(); + } + ok = p.waitFor(5, java.util.concurrent.TimeUnit.SECONDS) && p.exitValue() == 0 && !out.isBlank(); + } catch (IOException e) { + out = null; + ok = false; + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + out = null; + ok = false; + } + assumeTrue(ok, "cannot resolve this process's real primary group via `id -gn` on this host " + + "— skipping a POSIX-group-dependent test rather than failing it"); + return out; } /** Spawn once through the real launcher path, capturing every INFO+ line this class logs. */ diff --git a/fleetd/src/test/java/dev/ltms/fleet/member/OpenCodeLauncherTest.java b/fleetd/src/test/java/dev/ltms/fleet/member/OpenCodeLauncherTest.java index d1028fe..fe789ec 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/member/OpenCodeLauncherTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/member/OpenCodeLauncherTest.java @@ -23,7 +23,6 @@ import org.slf4j.LoggerFactory; import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; -import java.nio.file.attribute.PosixFileAttributeView; import java.nio.file.attribute.PosixFilePermissions; import java.util.List; import java.util.Map; @@ -657,11 +656,37 @@ class OpenCodeLauncherTest { 0, System::currentTimeMillis, () -> { }, configRoot, discoveryRoot, null, null, config); } - /** The current process's own primary group — resolvable on whatever host runs this test. */ - private static String currentUserGroup() throws IOException { - PosixFileAttributeView view = Files.getFileAttributeView(Path.of("."), PosixFileAttributeView.class); - assumeTrue(view != null, "this host's filesystem does not support POSIX group ownership"); - return view.readAttributes().group().getName(); + /** + * The current process's REAL primary group — resolved via {@code id -gn}, never by reading a + * directory's owning group (fleetd #225). Those two coincide only by accident: a directory's + * group is whichever group happened to own the path Maven was started from — {@code staff} in + * a home checkout, {@code wheel} under {@code /private/tmp} on macOS — and the fix-up this test + * exercises then fails for real when the operator is not a member of that borrowed group, + * exactly the case {@code assumeTrue(view != null, ...)} never covered (it only detects a + * filesystem with no POSIX groups at all, not a resolvable-but-wrong one). Skips (never fails) + * when {@code id} is unavailable or its primary group cannot be resolved on this host. + */ + private static String currentUserGroup() { + String out; + boolean ok; + try { + Process p = new ProcessBuilder("id", "-gn").redirectErrorStream(true).start(); + try (java.io.BufferedReader r = new java.io.BufferedReader( + new java.io.InputStreamReader(p.getInputStream(), java.nio.charset.StandardCharsets.UTF_8))) { + out = r.lines().collect(java.util.stream.Collectors.joining("\n")).trim(); + } + ok = p.waitFor(5, java.util.concurrent.TimeUnit.SECONDS) && p.exitValue() == 0 && !out.isBlank(); + } catch (IOException e) { + out = null; + ok = false; + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + out = null; + ok = false; + } + assumeTrue(ok, "cannot resolve this process's real primary group via `id -gn` on this host " + + "— skipping a POSIX-group-dependent test rather than failing it"); + return out; } /** 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 21f5402..75396fd 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/session/GitWorktreesTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/session/GitWorktreesTest.java @@ -1099,4 +1099,82 @@ class GitWorktreesTest { assertTrue(e.getMessage().contains("cb185-nonexistent-group-zz"), "exception must name the missing/refused group: " + e.getMessage()); } + + // --- fleetd #224: worktreeRoot itself must be group-traversable, established in add() --------- + + /** + * {@code add} must make the worktree ROOT itself group-traversable when a group is configured — + * established once here, where the root is created, never at a use site (never inside a + * launcher). A recording runner stands in for chgrp/chmod, the same seam + * {@link #shareWithGroupRunsConfigThenChgrpChmodSetgidPerPath} uses for the per-worktree share. + */ + @Test + void addSharesWorktreeRootWithGroupWhenConfigured(@TempDir Path tmp) throws Exception { + Path repo = initRepo(tmp.resolve("repo")); + Path root = tmp.resolve("wts"); + List> recorded = new java.util.ArrayList<>(); + java.util.function.Function recordingRunner = cmd -> { + recorded.add(joined(cmd)); + return ""; + }; + GitWorktrees gitWorktrees = new GitWorktrees(root.toString(), "devteam", _ -> {}, recordingRunner); + + gitWorktrees.add(repo.toString(), "cb-224-branch", "HEAD"); + + assertTrue(recorded.contains(List.of("chgrp", "devteam", root.toString())), + "the worktree root itself must be chgrp'd to the configured group: " + recorded); + assertTrue(recorded.contains(List.of("chmod", "g+x", root.toString())), + "the worktree root itself must gain group-execute so a different-uid member can " + + "traverse into it: " + recorded); + } + + /** {@code worktreeGroup} unset (today's only live mode) ⇒ {@code add} spawns no share process + * for the root at all — behaviour must be byte-identical to before fleetd #224. */ + @Test + void addSharesNothingForTheRootWhenNoGroupConfigured(@TempDir Path tmp) throws Exception { + Path repo = initRepo(tmp.resolve("repo")); + Path root = tmp.resolve("wts"); + List> recorded = new java.util.ArrayList<>(); + java.util.function.Function recordingRunner = cmd -> { + recorded.add(joined(cmd)); + return ""; + }; + GitWorktrees gitWorktrees = new GitWorktrees(root.toString(), null, _ -> {}, recordingRunner); + + gitWorktrees.add(repo.toString(), "cb-224-nogroup", "HEAD"); + + assertTrue(recorded.isEmpty(), "no group configured must spawn no share process for the " + + "root at all: " + recorded); + } + + /** + * fleetd #224, acceptance criterion 2/3: when the root cannot be made group-traversable — here + * because the configured group does not exist, the same real-failure shape + * {@link #shareWithGroupThrowsNamingTheGroupWhenChgrpFails} drives for the per-worktree share — + * the spawn is refused with a message naming the root, its current mode, and the group. This + * drives the REAL {@code chgrp} (no recording runner), and the refusal happens before {@code git + * worktree add} ever runs, so no partial worktree is left behind either. + */ + @Test + void addRefusesWhenWorktreeRootCannotBeMadeGroupTraversable(@TempDir Path tmp) throws Exception { + Path repo = initRepo(tmp.resolve("repo")); + Path root = tmp.resolve("wts"); + GitWorktrees gitWorktrees = new GitWorktrees(root.toString(), "cb224-nonexistent-group-zz"); + + WorktreeException e = assertThrows(WorktreeException.class, + () -> gitWorktrees.add(repo.toString(), "cb-224-refuse", "HEAD")); + + assertTrue(e.getMessage().contains(root.toString()), + "refusal must name the worktree root: " + e.getMessage()); + assertTrue(e.getMessage().contains("cb224-nonexistent-group-zz"), + "refusal must name the missing/refused group: " + e.getMessage()); + assertTrue(Files.isDirectory(root), "the root is created before the group check runs"); + String mode = java.nio.file.attribute.PosixFilePermissions.toString(Files.getPosixFilePermissions(root)); + assertTrue(e.getMessage().contains(mode), + "refusal must name the root's current mode (" + mode + "): " + e.getMessage()); + try (java.util.stream.Stream children = Files.list(root)) { + assertTrue(children.findAny().isEmpty(), "no worktree must be left behind under the root: " + + "the refusal must happen before `git worktree add` ever runs"); + } + } }