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
> recorded = new java.util.ArrayList<>();
+ java.util.function.Function