diff --git a/fleetd/src/main/java/dev/ltms/fleet/member/ClaudeCodeLauncher.java b/fleetd/src/main/java/dev/ltms/fleet/member/ClaudeCodeLauncher.java index 6dddea8..fa1b391 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/member/ClaudeCodeLauncher.java +++ b/fleetd/src/main/java/dev/ltms/fleet/member/ClaudeCodeLauncher.java @@ -459,12 +459,83 @@ public final class ClaudeCodeLauncher extends HerdrPeerLauncher { * {@link OpenCodeLauncher#writeConfig} already uses for its charter file, since the process that * reads this file (the spawned peer) outlives this JVM call and there is no spawn-scoped teardown * hook to delete it synchronously. + * + *

fleetd #222. {@code Files.createTempFile(prefix, suffix)} with no directory argument + * resolves against {@code java.io.tmpdir} — on macOS the per-user {@code $TMPDIR} under + * {@code /var/folders/...}, mode {@code 0700}, both resolved against FLEETD's own OS user. Under + * {@code memberHerdrSocket:} the member pane runs as a DIFFERENT OS user, so that user cannot even + * traverse the directory, let alone read the file — and since fleetd #220 the charter file is the + * ONLY delivery path for {@code --append-system-prompt-file}, always, not merely the fallback it + * used to be. A member handed a path it cannot read is not degraded, it is broken: see this + * method's refusal branch below. + * + *

Measured severity (fleetd #222 real-binary check, claude 2.1.258): an unreadable + * {@code --append-system-prompt-file} is the LOUD failure, not the silent one. {@code claude} + * checks the file before touching auth or the network — invoked with a bogus API key against a + * {@code chmod 000} file, it printed {@code Error reading append system prompt file: EACCES: + * permission denied, open ''} and exited 1 immediately (a nonexistent path gets {@code + * Error: Append system prompt file not found: }, same exit code). So the pre-fix bug did + * NOT leave a charter-less member silently occupying a pane and never calling {@code + * fleet_reply} — it made the herdr pane exit immediately, which the CB-306 spawn-readiness gate + * (this launcher's {@code spawnReadyTimeoutMs} poll) would have surfaced as "did not reach + * injectable state", the same unexplained-timeout shape fleetd #220 already describes. Still a + * real defect (every claude-code member under {@code memberHerdrSocket} would have failed to + * spawn), but not the worse, undetectable failure mode. + * + *

+ * + *

Follows fleetd #219's REFUSAL decision, not #213's degrade decision. The ZDOTDIR + * scrub is a credential CONTROL — a degraded control (the CB-596 sentinel overlay) still has + * value, so #213 falls back rather than refusing. A charter is NOT a control, it is the member's + * TURN CONTRACT (the rule that ends every turn with {@code fleet_reply}). A member spawned with no + * charter is not degraded, it is broken: either claude-code exits on the unreadable + * {@code --append-system-prompt-file} path and the spawn dies at the readiness gate (loud), or it + * starts anyway with no charter and never calls {@code fleet_reply} — the sender silently gets + * nothing (silent). Neither outcome is worth trading for "spawn something." So a missing {@code + * worktreeRoot}/{@code worktreeGroup} under {@code memberHerdrSocket} refuses the spawn here, + * naming the missing key, exactly like {@link OpenCodeLauncher#configParentDir()}. + * + * @throws IllegalStateException when {@code memberHerdrSocket} is configured but {@code + * worktreeRoot} and/or {@code worktreeGroup} is not */ - private static Path writeCharterFile(String charterText) { + private Path writeCharterFile(String charterText) { try { - Path file = Files.createTempFile("fleetd-role-charter-", ".md"); + if (!memberHerdrSocketConfigured()) { + Path file = Files.createTempFile("fleetd-role-charter-", ".md"); + Files.writeString(file, charterText); + file.toFile().deleteOnExit(); + return file; + } + Path parentDir = memberScrubParentDir(); + String group = memberGroup(); + if (parentDir == null || group == null) { + throw new IllegalStateException("memberHerdrSocket is configured, so the role/reply " + + "charter file (mounted via --append-system-prompt-file) must be placed where " + + "the member's OS user can read it — worktreeRoot, shared via worktreeGroup — " + + "but " + (parentDir == null ? "worktreeRoot" : "worktreeGroup") + " is not " + + "configured. Refusing to spawn rather than hand the member a charter path it " + + "cannot read: that member's turn contract (the fleet_reply rule) would never " + + "reach it. Configure both worktreeRoot and worktreeGroup to enable claude-code " + + "member spawns under memberHerdrSocket."); + } + Path dir = Files.createTempDirectory(parentDir, "fleetd-role-charter-"); + dir.toFile().deleteOnExit(); + Path file = dir.resolve("charter.md"); Files.writeString(file, charterText); file.toFile().deleteOnExit(); + EnvAllowListScrub.shareWithGroup(dir, group); return file; } catch (IOException e) { throw new UncheckedIOException("cannot write role charter temp file", e); diff --git a/fleetd/src/test/java/dev/ltms/fleet/member/ClaudeCodeLauncherTest.java b/fleetd/src/test/java/dev/ltms/fleet/member/ClaudeCodeLauncherTest.java index 3d7a4ec..255e947 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/member/ClaudeCodeLauncherTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/member/ClaudeCodeLauncherTest.java @@ -20,8 +20,10 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; import org.slf4j.LoggerFactory; +import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; +import java.nio.file.attribute.PosixFilePermissions; import java.util.List; import java.util.Map; import java.util.Set; @@ -31,6 +33,7 @@ import java.util.function.Function; import java.util.function.Supplier; import static org.junit.jupiter.api.Assertions.*; +import static org.junit.jupiter.api.Assumptions.assumeTrue; /** The step-4 launch-flag injection: the bridge MCP + reply charter are appended to the argv. */ class ClaudeCodeLauncherTest { @@ -1751,4 +1754,184 @@ class ClaudeCodeLauncherTest { assertEquals(List.of("dev: sonnet #1", "[sonnet] dev 2"), tabLabels(herdr)); } + + // ── fleetd #222: the charter file must not land under fleetd's own java.io.tmpdir when the ── + // ── member pane runs as a different OS user ───────────────────────────────────────────────── + + /** A profile that mounts the bridge MCP (so a reply charter is always generated). */ + private static FleetConfig.Profile charterCfg() { + return new FleetConfig.Profile( + "ltms-local", "http://gx00.gw:8000", "coder", null, "FLEETD_WORKER_TOKEN", + List.of("claude"), "tab", "fleetd-workers", "worker: {profile} #{n}", + "http://127.0.0.1:8765/mcp", null, null); + } + + /** A config with {@code memberHerdrSocket:} set, and optionally {@code worktreeRoot:}/{@code worktreeGroup:}. */ + private static FleetConfig configWithMemberHerdrSocket(String worktreeRoot, String worktreeGroup) { + return new FleetConfig( + null, // bind + null, // herdrSocket + "/tmp/other-user.sock", // memberHerdrSocket + Map.of(), // profiles + null, // guard + worktreeRoot, // worktreeRoot + null, // lifecycle + null, // spawnReadyTimeoutMs + null, // spawnReadyPollMs + null, // broker + null, // primary + null, // fleet + null, // leadHeartbeat + null, // health + null, // placement + null, // auth + null, // configReload + null, // quarantineCooldownSeconds + null, // memberCredentials + null, // coordinator + worktreeGroup, // worktreeGroup + null // memberLoginShell + ).withDefaults(); + } + + private static ClaudeCodeLauncher serviceWithConfig(FakeHerdr herdr, FleetConfig.Profile cfg, + Supplier config) { + return new ClaudeCodeLauncher(new AgentControl(herdr), new WorkspaceControl(herdr), + new SubscriptionGuard(Set.of("gx00.gw")), Map.of(cfg.profile(), cfg), cfg.profile(), _ -> null, + 0, System::currentTimeMillis, () -> { }, null, null, null, config); + } + + /** + * 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; + } + + /** + * fleetd #222 acceptance criterion 1: with {@code memberHerdrSocket} configured and both + * {@code worktreeRoot}/{@code worktreeGroup} set, the charter file lives in a fresh per-spawn + * directory under {@code worktreeRoot} — NEVER under {@code java.io.tmpdir} (fleetd's own 0700 + * temp dir, unreadable by the member's different OS user) — and that directory is shared + * read-only with the group via the SAME mechanism (fleetd #213/#219's {@link + * EnvAllowListScrub#shareWithGroup}) the ZDOTDIR scrub and the opencode config directory use. + */ + @Test + void memberHerdrSocketWithWorktreeRootAndGroupPutsCharterUnderWorktreeRootAndSharesIt( + @TempDir Path worktreeRoot) throws Exception { + String group = currentUserGroup(); + FakeHerdr herdr = new FakeHerdr(); + serviceWithConfig(herdr, charterCfg(), + () -> configWithMemberHerdrSocket(worktreeRoot.toString(), group)).spawn(); + + List args = spawnedArgs(herdr); + int fileFlag = args.indexOf("--append-system-prompt-file"); + assertTrue(fileFlag >= 0, "the charter is still mounted via file: " + args); + Path charterFile = Path.of(args.get(fileFlag + 1)); + Path dir = charterFile.getParent(); + + // NOTE: JUnit's own @TempDir provider places worktreeRoot itself under java.io.tmpdir on this + // host, so "not under java.io.tmpdir" is not a meaningful assertion here (it would hold by + // accident of the fixture, not by anything this method does). What this fix actually promises + // is that the directory is created UNDER worktreeRoot specifically — never resolved from the + // no-argument Files.createTempFile default (java.io.tmpdir) the pre-fix code always used — so + // that is the assertion: the parent is exactly worktreeRoot, whatever directory JUnit gave it. + assertEquals(worktreeRoot.toAbsolutePath().normalize(), dir.getParent(), + "the generated directory's parent must be worktreeRoot, not java.io.tmpdir — got " + + "parent " + dir.getParent()); + + assertEquals("rwxr-x---", PosixFilePermissions.toString(Files.getPosixFilePermissions(dir)), + "the directory must be group-traversable+readable, owner-only writable"); + assertEquals("rw-r-----", PosixFilePermissions.toString(Files.getPosixFilePermissions(charterFile)), + "the charter file must be group-readable, never group-writable"); + assertTrue(Files.readString(charterFile).contains("fleet_reply"), + "the charter content itself is unaffected by where it is written"); + } + + /** + * fleetd #222 acceptance criterion 2: with {@code memberHerdrSocket} configured but NEITHER + * {@code worktreeRoot} nor {@code worktreeGroup} set, the launcher must refuse the spawn rather + * than hand the member a {@code --append-system-prompt-file} path under {@code java.io.tmpdir} + * it cannot read — the member's whole turn contract would never reach it. + */ + @Test + void memberHerdrSocketWithoutWorktreeRootOrGroupRefusesTheSpawn() { + FakeHerdr herdr = new FakeHerdr(); + ClaudeCodeLauncher launcher = serviceWithConfig(herdr, charterCfg(), + () -> configWithMemberHerdrSocket(null, null)); + + IllegalStateException ex = assertThrows(IllegalStateException.class, launcher::spawn, + "a missing worktreeRoot/worktreeGroup must refuse the spawn, not write an unreadable charter"); + assertTrue(ex.getMessage().contains("worktreeRoot"), + "the refusal must name the missing config key — got: " + ex.getMessage()); + assertFalse(herdr.called("agent.start"), + "the spawn must be refused BEFORE the member is ever started — got calls: " + herdr.calls); + } + + /** + * fleetd #222 acceptance criterion 2 (the other missing half): {@code worktreeRoot} set but + * {@code worktreeGroup} missing must ALSO refuse — either one alone is not enough to guarantee + * the member's OS user can read the charter file. + */ + @Test + void memberHerdrSocketWithWorktreeRootButNoGroupRefusesTheSpawn(@TempDir Path worktreeRoot) { + FakeHerdr herdr = new FakeHerdr(); + ClaudeCodeLauncher launcher = serviceWithConfig(herdr, charterCfg(), + () -> configWithMemberHerdrSocket(worktreeRoot.toString(), null)); + + IllegalStateException ex = assertThrows(IllegalStateException.class, launcher::spawn); + assertTrue(ex.getMessage().contains("worktreeGroup"), + "worktreeRoot alone is not enough — got: " + ex.getMessage()); + } + + /** + * fleetd #222 acceptance criterion 3: with {@code memberHerdrSocket} ABSENT — even when a live, + * non-null {@code config} supplier is threaded through (not merely {@code config == null}, which + * every other test in this file already exercises) — the charter file must still be created + * directly under {@code java.io.tmpdir} via the same no-directory-argument + * {@code Files.createTempFile} call as before this fix, byte-identical to today. + */ + @Test + void memberHerdrSocketAbsentStaysUnderJavaIoTmpdirEvenWithALiveConfigSupplier() throws Exception { + FakeHerdr herdr = new FakeHerdr(); + FleetConfig config = new FleetConfig(null, null, null, Map.of(), null, null, null, null, null, + null, null, null, null, null, null, null, null, null, null, null, null, null).withDefaults(); + serviceWithConfig(herdr, charterCfg(), () -> config).spawn(); + + List args = spawnedArgs(herdr); + int fileFlag = args.indexOf("--append-system-prompt-file"); + assertTrue(fileFlag >= 0); + Path charterFile = Path.of(args.get(fileFlag + 1)); + assertTrue(charterFile.startsWith(Path.of(System.getProperty("java.io.tmpdir"))), + "with memberHerdrSocket absent, the charter file must still land directly under " + + "java.io.tmpdir, unchanged from before this fix"); + assertTrue(charterFile.getFileName().toString().startsWith("fleetd-role-charter-"), + "same file-naming scheme as before this fix (no wrapping directory): " + charterFile); + } }