CB-525: isolate a worker's tool surface to what its launcher mounts
A worker in a provisioned worktree was inheriting the primary's MCP servers by two independent routes: the repo commits a .mcp.json declaring the IDE servers, so a fresh checkout mounts them, and the default parity overlay then copied the primary's own copy over the top. Those servers are bound to the primary's IntelliJ project, so every path they hand back points into the primary's checkout. A CB-523 worker made all 59 of its edits there while running `mvn -f bridged/pom.xml` against its worktree — every build it ran was of code that did not contain its changes, and it passed. The worker's own `ls` of the file it had "edited" returned "No such file". GitWorktrees now neutralizes .mcp.json at provisioning: an explicitly empty server map, --skip-worktree'd when tracked so it never reads as pending work a worker might commit. Unconditional, because the overlay was only half the leak. The bridge itself is unaffected — it reaches a worker through the launcher's --mcp-config flag, not the project file, so bridge_reply still works. - BridgedConfig: .mcp.json out of the default parity overlay - GitWorktrees: isolateToolSurface() on add(), with the rationale in javadoc - GitWorktreesTest: 4 real-git acceptance tests (2 fail if the call is removed) - implementer skill: work from $PWD, and quote a green unpiped `mvn clean install` from the worktree as the acceptance criterion mvn clean install: Tests run: 392, Failures: 0, Errors: 0 — BUILD SUCCESS
This commit is contained in:
@@ -64,7 +64,16 @@ herdrSocket: ~/.config/herdr/herdr.sock
|
||||
# skills/MCP/hooks. Omit to leave the worker on the host default.
|
||||
# parityOverlay → repo-relative paths copied primary→worktree so a worker in a provisioned
|
||||
# worktree sees the same local config (CB-301-ext). Omit for the default set:
|
||||
# [.mcp.json, .claude/settings.local.json, .env, .envrc].
|
||||
# [.claude/settings.local.json, .env, .envrc].
|
||||
#
|
||||
# Do NOT add .mcp.json (CB-525). A worker's tools are whatever its launcher
|
||||
# mounts — the bridge, and nothing else. Replicating the primary's MCP config
|
||||
# handed a worker the primary's IDE servers, which are bound to the primary's
|
||||
# checkout, so its navigation returned paths OUTSIDE its own worktree: one
|
||||
# worker made all 59 of its edits in the primary tree while compiling its
|
||||
# worktree, and every build it ran was of code that did not contain them.
|
||||
# bridged neutralizes a provisioned worktree's .mcp.json for this reason;
|
||||
# listing it here would copy the primary's back over that.
|
||||
# gitTokenEnv → host env var holding the git-forge API token. When set, its value is injected
|
||||
# as GITEA_TOKEN so the worker can open its OWN PR at checkpoint (CB-302).
|
||||
# Opt-in by design — omit and the worker gets no PR-create grant (push over
|
||||
@@ -106,7 +115,7 @@ workers:
|
||||
# gitHostEnv: GITEA_HOST # defaults to GITEA_HOST; injected only with gitTokenEnv
|
||||
# configDir: /Users/me/.ccs/instances/gx10 # CLAUDE_CONFIG_DIR — inherit that profile's skills/MCP
|
||||
# cwd: /Users/me/src/myrepo # pin the working dir; omit to inherit the primary's
|
||||
# parityOverlay: [".mcp.json", ".claude/settings.local.json", ".env", ".envrc"]
|
||||
# parityOverlay: [".claude/settings.local.json", ".env", ".envrc"] # never add .mcp.json — see above
|
||||
ollama:
|
||||
baseUrl: http://ollama.ltms.dev # local/self-hosted; usually no token
|
||||
placement: tab
|
||||
|
||||
@@ -140,8 +140,12 @@ public record BridgedConfig(
|
||||
placement = (placement == null || placement.isBlank()) ? "tab" : placement.toLowerCase();
|
||||
workspace = (workspace == null || workspace.isBlank()) ? "bridged-workers" : workspace;
|
||||
tabLabel = (tabLabel == null || tabLabel.isBlank()) ? "worker: {profile} #{n}" : tabLabel;
|
||||
// CB-525: .mcp.json is deliberately NOT here. Replicating the primary's MCP config gave a
|
||||
// worker the primary's IDE servers, which are bound to the primary's checkout — so its
|
||||
// navigation returned paths outside its own worktree. GitWorktrees now neutralizes that
|
||||
// file instead; a worker's tools are whatever its launcher mounts.
|
||||
parityOverlay = (parityOverlay == null || parityOverlay.isEmpty())
|
||||
? List.of(".mcp.json", ".claude/settings.local.json", ".env", ".envrc")
|
||||
? List.of(".claude/settings.local.json", ".env", ".envrc")
|
||||
: List.copyOf(parityOverlay);
|
||||
// gitTokenEnv stays null when unset (opt-in). gitHostEnv defaults so operators enabling
|
||||
// checkpoints need only set gitTokenEnv; it is injected only alongside a resolved token.
|
||||
|
||||
@@ -27,6 +27,12 @@ public final class GitWorktrees implements Worktrees {
|
||||
|
||||
private static final Logger log = LoggerFactory.getLogger(GitWorktrees.class);
|
||||
|
||||
/** Project-level MCP config. Present in the repo, so every worktree checks the primary's out. */
|
||||
private static final String MCP_CONFIG = ".mcp.json";
|
||||
|
||||
/** What {@link #isolateToolSurface} writes: a valid, explicitly empty server map. */
|
||||
private static final String NEUTRAL_MCP_CONFIG = "{\n \"mcpServers\": {}\n}\n";
|
||||
|
||||
private final String configuredRoot;
|
||||
private final SecureRandom random = new SecureRandom();
|
||||
private final AtomicLong seq = new AtomicLong();
|
||||
@@ -55,9 +61,41 @@ public final class GitWorktrees implements Worktrees {
|
||||
String wt = path.toAbsolutePath().toString();
|
||||
log.info("adding worktree branch={} path={} base={}", branch, wt, base);
|
||||
exec("git", "-C", repoRoot, "worktree", "add", wt, "-b", branch, base);
|
||||
isolateToolSurface(wt);
|
||||
return wt;
|
||||
}
|
||||
|
||||
/**
|
||||
* Neutralize the worktree's project MCP config so a worker inherits only the tools its launcher
|
||||
* mounts (the bridge, via {@code --mcp-config}) — never the primary's.
|
||||
*
|
||||
* <p>This is unconditional, and it is not the same job as the parity overlay. The repo's own
|
||||
* committed {@code .mcp.json} declares the primary's IDE servers, so a fresh checkout mounts them
|
||||
* whether or not the overlay copies anything; a worker that inherits them navigates and edits
|
||||
* through tools bound to the <em>primary's</em> IntelliJ project, which silently hands it absolute
|
||||
* paths outside its own worktree. That is not hypothetical: a CB-523 worker made all 59 of its
|
||||
* edits in the primary checkout while compiling its worktree, so every build it ran was of code
|
||||
* that did not contain its changes.
|
||||
*
|
||||
* <p>Writing an empty server map (rather than deleting the file) keeps a project-level
|
||||
* {@code .mcp.json} present and explicit, and the {@code --skip-worktree} bit keeps the
|
||||
* neutralized copy from ever showing up as a local modification the worker might commit.
|
||||
*/
|
||||
private void isolateToolSurface(String worktreePath) {
|
||||
Path root = Path.of(worktreePath).toAbsolutePath().normalize();
|
||||
Path mcp = root.resolve(MCP_CONFIG);
|
||||
try {
|
||||
Files.writeString(mcp, NEUTRAL_MCP_CONFIG);
|
||||
} catch (IOException e) {
|
||||
throw new WorktreeException("cannot neutralize " + MCP_CONFIG + " in the worktree: "
|
||||
+ e.getMessage(), e);
|
||||
}
|
||||
if (isTracked(root, MCP_CONFIG)) {
|
||||
exec("git", "-C", worktreePath, "update-index", "--skip-worktree", MCP_CONFIG);
|
||||
}
|
||||
log.debug("neutralized {} — worker tool surface is launcher-mounted only", MCP_CONFIG);
|
||||
}
|
||||
|
||||
@Override
|
||||
public void remove(String repoRoot, String worktreePath) {
|
||||
Path p = Path.of(worktreePath);
|
||||
|
||||
@@ -0,0 +1,119 @@
|
||||
package dev.ltms.bridged.session;
|
||||
|
||||
import org.junit.jupiter.api.Test;
|
||||
import org.junit.jupiter.api.io.TempDir;
|
||||
|
||||
import java.nio.file.Files;
|
||||
import java.nio.file.Path;
|
||||
import java.util.List;
|
||||
import java.util.concurrent.TimeUnit;
|
||||
|
||||
import static org.junit.jupiter.api.Assertions.*;
|
||||
|
||||
/**
|
||||
* CB-525 acceptance test for tool-surface isolation. This is one of the few tests that drives real
|
||||
* {@code git} — the behaviour under test is precisely what {@link GitWorktrees} does to a checkout,
|
||||
* so a fake would assert nothing. Everything happens inside a {@link TempDir} throwaway repo.
|
||||
*/
|
||||
class GitWorktreesTest {
|
||||
|
||||
/** A project MCP config with servers in it — what this repo actually commits. */
|
||||
private static final String WITH_SERVERS = """
|
||||
{
|
||||
"mcpServers": {
|
||||
"jetbrains": { "type": "sse", "url": "http://localhost:64342/sse" }
|
||||
}
|
||||
}
|
||||
""";
|
||||
|
||||
private static Path initRepo(Path dir) throws Exception {
|
||||
Files.createDirectories(dir);
|
||||
git(dir, "init", "-q", "-b", "main");
|
||||
git(dir, "config", "user.email", "test@example.invalid");
|
||||
git(dir, "config", "user.name", "Test");
|
||||
Files.writeString(dir.resolve(".mcp.json"), WITH_SERVERS);
|
||||
Files.writeString(dir.resolve("README.md"), "seed\n");
|
||||
git(dir, "add", ".mcp.json", "README.md");
|
||||
git(dir, "commit", "-q", "-m", "seed");
|
||||
return dir;
|
||||
}
|
||||
|
||||
private static void git(Path cwd, String... args) throws Exception {
|
||||
List<String> cmd = new java.util.ArrayList<>(List.of("git"));
|
||||
cmd.addAll(List.of(args));
|
||||
Process p = new ProcessBuilder(cmd).directory(cwd.toFile()).redirectErrorStream(true).start();
|
||||
String out = new String(p.getInputStream().readAllBytes());
|
||||
assertTrue(p.waitFor(30, TimeUnit.SECONDS), "git timed out: " + String.join(" ", cmd));
|
||||
assertEquals(0, p.exitValue(), "git " + String.join(" ", args) + " failed:\n" + out);
|
||||
}
|
||||
|
||||
/** Pending changes to {@code .mcp.json} in {@code cwd}, empty when git considers it unmodified. */
|
||||
private static String mcpStatus(Path cwd) throws Exception {
|
||||
Process p = new ProcessBuilder("git", "status", "--porcelain", "--", ".mcp.json")
|
||||
.directory(cwd.toFile()).redirectErrorStream(true).start();
|
||||
String out = new String(p.getInputStream().readAllBytes());
|
||||
assertTrue(p.waitFor(30, TimeUnit.SECONDS), "git status timed out");
|
||||
return out;
|
||||
}
|
||||
|
||||
/**
|
||||
* The heart of CB-525: a provisioned worktree must not inherit the primary's MCP servers. Without
|
||||
* the isolation step the checked-out {@code .mcp.json} carries them in, and a worker navigating
|
||||
* through the primary's IDE servers edits the primary's tree while building its own.
|
||||
*/
|
||||
@Test
|
||||
void aProvisionedWorktreeInheritsNoMcpServers(@TempDir Path tmp) throws Exception {
|
||||
Path repo = initRepo(tmp.resolve("repo"));
|
||||
String wt = new GitWorktrees(tmp.resolve("wts").toString())
|
||||
.add(repo.toString(), "cb-525-a", "HEAD");
|
||||
|
||||
Path mcp = Path.of(wt).resolve(".mcp.json");
|
||||
assertTrue(Files.exists(mcp), ".mcp.json must still exist — present and explicitly empty");
|
||||
String body = Files.readString(mcp);
|
||||
assertFalse(body.contains("jetbrains"), "worktree inherited the primary's MCP servers:\n" + body);
|
||||
assertTrue(body.replaceAll("\\s+", "").contains("\"mcpServers\":{}"),
|
||||
"expected an explicitly empty server map, got:\n" + body);
|
||||
}
|
||||
|
||||
/** Neutralizing must not look like work in progress, or a worker would commit it into its PR. */
|
||||
@Test
|
||||
void theNeutralizedConfigIsNotAPendingLocalModification(@TempDir Path tmp) throws Exception {
|
||||
Path repo = initRepo(tmp.resolve("repo"));
|
||||
String wt = new GitWorktrees(tmp.resolve("wts").toString())
|
||||
.add(repo.toString(), "cb-525-b", "HEAD");
|
||||
|
||||
assertEquals("", mcpStatus(Path.of(wt)),
|
||||
"the neutralized .mcp.json shows as modified — --skip-worktree did not take");
|
||||
}
|
||||
|
||||
/** Isolation is the worktree's business only; the primary's own checkout must be untouched. */
|
||||
@Test
|
||||
void thePrimaryCheckoutIsLeftAlone(@TempDir Path tmp) throws Exception {
|
||||
Path repo = initRepo(tmp.resolve("repo"));
|
||||
new GitWorktrees(tmp.resolve("wts").toString()).add(repo.toString(), "cb-525-c", "HEAD");
|
||||
|
||||
assertEquals(WITH_SERVERS, Files.readString(repo.resolve(".mcp.json")),
|
||||
"the primary's .mcp.json was rewritten — isolation reached out of the worktree");
|
||||
}
|
||||
|
||||
/** A repo that commits no {@code .mcp.json} still gets one, so nothing can be inherited later. */
|
||||
@Test
|
||||
void aRepoWithoutAnMcpConfigStillGetsANeutralOne(@TempDir Path tmp) throws Exception {
|
||||
Path repo = tmp.resolve("repo");
|
||||
Files.createDirectories(repo);
|
||||
git(repo, "init", "-q", "-b", "main");
|
||||
git(repo, "config", "user.email", "test@example.invalid");
|
||||
git(repo, "config", "user.name", "Test");
|
||||
Files.writeString(repo.resolve("README.md"), "seed\n");
|
||||
git(repo, "add", "README.md");
|
||||
git(repo, "commit", "-q", "-m", "seed");
|
||||
|
||||
String wt = new GitWorktrees(tmp.resolve("wts").toString())
|
||||
.add(repo.toString(), "cb-525-d", "HEAD");
|
||||
|
||||
// Untracked is the normal case here, so the --skip-worktree branch must be skipped rather
|
||||
// than run and fail: `update-index --skip-worktree` on an unknown path exits non-zero.
|
||||
String body = Files.readString(Path.of(wt).resolve(".mcp.json"));
|
||||
assertTrue(body.replaceAll("\\s+", "").contains("\"mcpServers\":{}"), body);
|
||||
}
|
||||
}
|
||||
@@ -82,7 +82,7 @@ class WorktreeSessionManagerTest {
|
||||
void worktreeAcquireRunsParityOverlayWithProfileDefaults() {
|
||||
FakeHerdr herdr = new FakeHerdr();
|
||||
FakeWorktrees worktrees = new FakeWorktrees().withRepoRoot("/repo").withPrefix("/wt")
|
||||
.track(".mcp.json")
|
||||
.track(".envrc")
|
||||
.exists(".claude/settings.local.json");
|
||||
SessionManager sessions = new SessionManager(workerService(herdr), worktrees);
|
||||
|
||||
@@ -93,11 +93,14 @@ class WorktreeSessionManagerTest {
|
||||
FakeWorktrees.OverlayCall overlay = worktrees.lastOverlay();
|
||||
assertNotNull(overlay);
|
||||
assertEquals("/repo", overlay.repoRoot());
|
||||
assertEquals(List.of(".mcp.json", ".claude/settings.local.json", ".env", ".envrc"),
|
||||
assertEquals(List.of(".claude/settings.local.json", ".env", ".envrc"),
|
||||
overlay.requested(), "default parity overlay is used when unset");
|
||||
assertEquals(List.of(".mcp.json", ".claude/settings.local.json"), overlay.copied(),
|
||||
assertFalse(overlay.requested().contains(".mcp.json"),
|
||||
"CB-525: replicating the primary's MCP config gives a worker the primary's IDE "
|
||||
+ "servers, which navigate its edits out of its own worktree");
|
||||
assertEquals(List.of(".claude/settings.local.json", ".envrc"), overlay.copied(),
|
||||
"existing paths are copied; missing paths are skipped");
|
||||
assertEquals(List.of(".mcp.json"), overlay.skipWorktree(),
|
||||
assertEquals(List.of(".envrc"), overlay.skipWorktree(),
|
||||
"tracked copied paths are --skip-worktree'd");
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user