fleetd #224 + #225: make worktreeRoot group-traversable, and stop two tests depending on the checkout location
CI / contract (push) Successful in 53s
CI / build (push) Successful in 1m41s

Verified by the lead before merge: read the full production diff, confirmed `group` is normalised to null at GitWorktrees:148 so the `group == null` guard is complete, and confirmed the refusal runs before `git worktree add` so a failure leaves no half-made worktree. Independent build in the worker's worktree: BUILD SUCCESS, 1093 tests, 0 failures, 0 skipped.
This commit was merged in pull request #230.
This commit is contained in:
2026-09-02 02:52:12 +02:00
5 changed files with 252 additions and 12 deletions
@@ -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.
*
* <p>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.
*
* <p>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 <em>common</em> git directory as an absolute path — where {@code objects},
* {@code refs} and {@code worktrees} actually live. {@code git rev-parse --git-common-dir}
@@ -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.
*
* <p>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.
@@ -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. */
@@ -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;
}
/**
@@ -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<List<String>> recorded = new java.util.ArrayList<>();
java.util.function.Function<String[], String> 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<List<String>> recorded = new java.util.ArrayList<>();
java.util.function.Function<String[], String> 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<Path> 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");
}
}
}