5 changed files with 370 additions and 9 deletions
@@ -0,0 +1,131 @@
+# CB-161: fix the pane-ownership ancestry gap in PaneLocator
+
+## The bug
+
+`PaneLocator.paneOwnsPid` matched a pid against only two things: a pane's `shell_pid`, and
+the pids in `foreground_processes`. A grandchild process a member spawns (for example a
+`python3` or `curl` helper that opens its own connection to `127.0.0.1:8765`) matched no
+pane. `ConnectionIdentity.resolve` then fell through to loopback-trust and resolved that
+connection as the **primary** — a worker→primary privilege escalation.
+
+## The fix
+
+`fleetd/src/main/java/dev/ltms/fleet/herdr/PaneLocator.java`:
+
+- `terminalForPid` now builds the caller's **ancestor pid set once** (the pid itself, then its
+  parents up the tree) before scanning any pane, and reuses that same set across every `HerdrClient`
+  searched (the CB-185 two-daemon path). It does not walk the tree once per pane.
+- The walk is bounded at 32 generations (`MAX_ANCESTRY_DEPTH`) and also stops the moment it revisits
+  a pid it has already recorded (cycle guard), so a cycle or a pathologically deep tree cannot hang
+  identity resolution, which runs on every MCP call.
+- `paneOwnsPid` (renamed `paneOwnsAnyOf`) now checks whether a pane's `shell_pid` or any
+  `foreground_processes` pid is a **member of that ancestor set**, instead of comparing the raw pid
+  directly. A pid that IS the shell pid or a foreground pid still matches (ancestry sets always
+  contain the pid itself) — no regression there.
+- A vanished ancestor (`ParentResolver.parentOf` returning empty) ends the walk quietly, not as an
+  error — matches the brief.
+
+### The seam: `ParentResolver`
+
+New file `fleetd/src/main/java/dev/ltms/fleet/herdr/ParentResolver.java` — a one-method interface
+(`OptionalLong parentOf(long pid)`) so the ancestry walk is testable without spawning real OS
+processes. `ParentResolver.PROCESS_HANDLE` is the production implementation, backed by
+`ProcessHandle.of(pid)` / `.parent()`. `PaneLocator` gained two new constructor overloads
+(`PaneLocator(HerdrClient, ParentResolver)` and `PaneLocator(HerdrClient, HerdrClient,
+ParentResolver)`) that accept an injected resolver; the existing constructors delegate to these with
+`ParentResolver.PROCESS_HANDLE`, so every existing call site (`Fleetd.java`, `ConnectionIdentity`
+tests, the contract test) is unchanged and compiles as-is.
+
+## Tests
+
+New file `fleetd/src/test/java/dev/ltms/fleet/herdr/FakeParentResolver.java` — a fake `ParentResolver`
+backed by an explicit pid→parent `Map`.
+
+New tests added to `fleetd/src/test/java/dev/ltms/fleet/herdr/PaneLocatorTest.java` (using a new
+small `OnePaneHerdr` fake nested in that test class, purpose-built so shell_pid and a foreground pid
+can be given distinct values — the shared `FakeHerdr` fixture happens to use the same pid for both,
+which would not isolate the two cases):
+
+1. `stillResolvesAPidThatIsExactlyThePaneShellPid` — regression, no ancestry walk needed.
+2. `stillResolvesAPidThatIsExactlyAForegroundPid` — regression, via the foreground-processes list.
+3. `resolvesAGrandchildPidTwoLevelsBelowTheShellPid` — **the bug.** grandchild → child → shell_pid.
+4. `nullForAPidWhoseAncestryMatchesNoPane` — a pid whose ancestry never reaches any pane still
+   resolves to `null`, so the real primary is not demoted to a worker.
+5. `ancestryWalkTerminatesOnACycleInsteadOfHanging` — a 2-cycle fake parent map (100→101→100)
+   terminates and returns `null` instead of hanging.
+6. `ancestrySetIsComputedOnceAcrossBothClientsInTheTwoDaemonConstructor` — wraps the fake resolver
+   with a call counter, uses the two-arg (CB-185) constructor with one client reporting no panes,
+   and asserts exactly 3 `parentOf` calls (not 6) — the ancestor set is walked once and reused
+   across both clients, not recomputed per client.
+
+All existing `PaneLocatorTest` tests (the CB-185 two-daemon fallback tests) still pass unchanged.
+
+### Proof that tests 3 and 5 fail without the fix
+
+I temporarily reverted `ancestorsOf` twice, ran the specific test, then restored the real fix
+(`git diff` is clean of these — they were never committed):
+
+**Test 3, reverted to "no ancestry walk at all" (`return Set.of(pid);`):**
+
+```
+mvn -Dtest=PaneLocatorTest#resolvesAGrandchildPidTwoLevelsBelowTheShellPid test
+...
+org.opentest4j.AssertionFailedError: expected: <term_x> but was: <null>
+[ERROR]   PaneLocatorTest.resolvesAGrandchildPidTwoLevelsBelowTheShellPid:100 expected: <term_x> but was: <null>
+[ERROR] Tests run: 1, Failures: 1, Errors: 0, Skipped: 0
+[INFO] BUILD FAILURE
+```
+
+**Test 5, reverted to a naive walk (no `MAX_ANCESTRY_DEPTH` bound, no "already seen" cycle guard —
+just `while (current > 0) { ancestry.add(current); ... }`):**
+
+Ran `mvn -Dtest=PaneLocatorTest#ancestryWalkTerminatesOnACycleInsteadOfHanging test` in the
+background and polled it. It was still running (stuck inside `PaneLocatorTest`, no test-run summary
+line ever printed) after 20 seconds and had to be force-killed — confirming the naive walk hangs on
+the 100↔101 cycle fixture. The real fix's depth bound and cycle guard are what make this test pass
+in ~0.25s instead.
+
+Both reverts were made only in-memory in my worktree (I kept a copy of the correct file at
+`/tmp/PaneLocator.java.fixed` beforehand) and restored via `cp` before running the full build below.
+`git status` / `git diff` at commit time show only the intended fix, not either revert.
+
+## Full build
+
+```
+cd fleetd && mvn clean install
+```
+
+Ran to completion, unpiped. Final lines:
+
+```
+[INFO] Tests run: 1043, Failures: 0, Errors: 0, Skipped: 0
+[INFO] BUILD SUCCESS
+[INFO] Total time:  35.061 s
+```
+
+(main was 1037 tests; this branch adds 6 new tests in `PaneLocatorTest` = 1043.)
+
+## Scope discipline — what I did NOT touch
+
+Per the brief: only `PaneLocator`, its tests, and the minimal `ParentResolver` seam + its fake.
+`Authz`, `ConnectionIdentity`'s fallthrough, and the loopback-trust policy are unchanged — those
+stay the deliberate primary-only decision they were.
+
+One thing spotted but out of scope, noted only: `terminalForPid` still issues one
+`pane.process_info` herdr call **per pane** on every resolution (no caching) — the class javadoc
+already flags this as "the obvious optimization once wired into `ClaudeCodeLauncher`." Not touched.
+
+## What I could NOT check
+
+- No IDE tooling (no `ide_diagnostics` / IntelliJ inspections) — `mvn clean install` is my only
+  verification, as the worker charter expects.
+- Cannot restart or query the live `fleetd` daemon — this fix is unverified against a real herdr
+  process tree. The pre-existing `PaneLocatorContractTest` (tagged `contract`, run with
+  `-Pcontract`) exercises `PaneLocator` against a real herdr daemon and a real shell pid (not a
+  grandchild), but I did not run it — it requires a live herdr socket, which I don't have in this
+  worktree, and it wasn't part of the standard `mvn clean install` run above (it's a separate
+  Maven profile). I did not add a grandchild-pid variant of it, since the brief scoped tests to
+  unit tests against a fake resolver.
+- I did not attempt to reproduce the original privilege-escalation scenario end-to-end (spawning a
+  real grandchild process against a live daemon) — the ticket says that half is already measured
+  and confirmed, and I have no way to drive a live daemon from this worktree.
@@ -2,8 +2,11 @@ package dev.ltms.fleet.herdr;

 import com.fasterxml.jackson.databind.JsonNode;

+import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.OptionalLong;
+import java.util.Set;

 /**
 * Resolves which herdr pane a process belongs to — the herdr half of connection-based MCP
@@ -12,23 +15,46 @@ import java.util.Map;
 * is calling without the worker sending anything spoofable.
 *
 * <p>herdr owns the PID→pane truth: {@code pane.process_info} reports each pane's {@code shell_pid}
- * and foreground process PIDs. This scans agent panes; a spawn-time {@code pid→terminal} cache is
- * the obvious optimization once wired into {@code ClaudeCodeLauncher}.
+ * and foreground process PIDs. A pid that is neither of those directly — e.g. a grandchild a
+ * worker spawned, such as a {@code python3} or {@code curl} helper that opens its own MCP
+ * connection — is resolved by walking its ancestry (via {@link ParentResolver}) up to the root and
+ * matching any ancestor against a pane's {@code shell_pid} or foreground pids (CB-161). Without
+ * this walk such a pid matches no pane, and the caller falls through to loopback-trust and is
+ * resolved as the primary — a worker→primary privilege escalation.
+ *
+ * <p>This scans agent panes; a spawn-time {@code pid→terminal} cache is the obvious optimization
+ * once wired into {@code ClaudeCodeLauncher}.
 *
 * <p>CB-185 split the fleet across two herdr daemons — lead operations on one, members on the
 * other ({@code memberHerdrSocket}). A caller's pane can live on <em>either</em> daemon (a lead's
 * MCP connection resolves against the lead daemon; a member's against the member daemon), so this
 * must be able to search more than one client. {@link #PaneLocator(HerdrClient, HerdrClient)}
 * searches the lead client first, then the member client, and collapses to a single scan when the
- * two are the same object (the historical single-daemon deployment).
+ * two are the same object (the historical single-daemon deployment). The caller's ancestor set is
+ * computed once per {@link #terminalForPid} call and reused across every client searched — it
+ * does not depend on which daemon a pane happens to live on.
 */
 public final class PaneLocator {

+    /**
+     * Bound on how many ancestor generations {@link #ancestorsOf} walks. This runs on every MCP
+     * call, so a cycle or a pathologically deep process tree must not hang identity resolution;
+     * 32 generations is far more than any real worker→helper process tree needs.
+     */
+    private static final int MAX_ANCESTRY_DEPTH = 32;
+
    private final List<HerdrClient> herdrs;
+    private final ParentResolver parentResolver;

    /** Search only this client — the single-daemon deployment. */
    public PaneLocator(HerdrClient herdr) {
+        this(herdr, ParentResolver.PROCESS_HANDLE);
+    }
+
+    /** Search only this client, resolving ancestry through {@code parentResolver} — for tests. */
+    public PaneLocator(HerdrClient herdr, ParentResolver parentResolver) {
        this.herdrs = List.of(herdr);
+        this.parentResolver = parentResolver;
    }

    /**
@@ -37,7 +63,13 @@ public final class PaneLocator {
     * collapses to one client and one scan, exactly {@link #PaneLocator(HerdrClient)}'s behaviour.
     */
    public PaneLocator(HerdrClient lead, HerdrClient member) {
+        this(lead, member, ParentResolver.PROCESS_HANDLE);
+    }
+
+    /** Two-daemon deployment, resolving ancestry through {@code parentResolver} — for tests. */
+    public PaneLocator(HerdrClient lead, HerdrClient member, ParentResolver parentResolver) {
        this.herdrs = lead == member ? List.of(lead) : List.of(lead, member);
+        this.parentResolver = parentResolver;
    }

    /**
@@ -49,8 +81,9 @@ public final class PaneLocator {
        if (pid <= 0) {
            return null;
        }
+        Set<Long> ancestry = ancestorsOf(pid);
        for (HerdrClient herdr : herdrs) {
-            String terminal = terminalForPid(herdr, pid);
+            String terminal = terminalForPid(herdr, ancestry);
            if (terminal != null) {
                return terminal;
            }
@@ -58,28 +91,54 @@ public final class PaneLocator {
        return null;
    }

-    private static String terminalForPid(HerdrClient herdr, long pid) {
+    /**
+     * {@code pid} itself plus its ancestor chain, walked through {@link #parentResolver} up to
+     * {@link #MAX_ANCESTRY_DEPTH} generations or pid 1, whichever comes first. A vanished ancestor
+     * ({@link ParentResolver#parentOf} returning empty) ends the walk without error — it just means
+     * the chain is shorter than the bound. A cycle in a fake resolver is caught by the "already
+     * seen" check and also ends the walk, so this can never loop.
+     */
+    private Set<Long> ancestorsOf(long pid) {
+        Set<Long> ancestry = new LinkedHashSet<>();
+        long current = pid;
+        for (int depth = 0; depth < MAX_ANCESTRY_DEPTH; depth++) {
+            if (current <= 0 || !ancestry.add(current)) {
+                break; // vanished/invalid pid, or a cycle back to a pid already recorded
+            }
+            if (current == 1) {
+                break; // reached the root of the process tree
+            }
+            OptionalLong parent = parentResolver.parentOf(current);
+            if (parent.isEmpty()) {
+                break; // vanished ancestor — not an error, just the end of the chain
+            }
+            current = parent.getAsLong();
+        }
+        return ancestry;
+    }
+
+    private static String terminalForPid(HerdrClient herdr, Set<Long> ancestry) {
        for (JsonNode pane : herdr.call("pane.list", Map.of()).path("panes")) {
            String paneId = pane.path("pane_id").asText(null);
-            if (paneId != null && paneOwnsPid(herdr, paneId, pid)) {
+            if (paneId != null && paneOwnsAnyOf(herdr, paneId, ancestry)) {
                return pane.path("terminal_id").asText(null);
            }
        }
        return null;
    }

-    private static boolean paneOwnsPid(HerdrClient herdr, String paneId, long pid) {
+    private static boolean paneOwnsAnyOf(HerdrClient herdr, String paneId, Set<Long> ancestry) {
        JsonNode info;
        try {
            info = herdr.call("pane.process_info", Map.of("pane_id", paneId)).path("process_info");
        } catch (HerdrException e) {
            return false; // pane vanished mid-scan — just skip it
        }
-        if (info.path("shell_pid").asLong(-1) == pid) {
+        if (ancestry.contains(info.path("shell_pid").asLong(-1))) {
            return true;
        }
        for (JsonNode p : info.path("foreground_processes")) {
-            if (p.path("pid").asLong(-1) == pid) {
+            if (ancestry.contains(p.path("pid").asLong(-1))) {
                return true;
            }
        }
@@ -0,0 +1,21 @@
+package dev.ltms.fleet.herdr;
+
+import java.util.OptionalLong;
+
+/**
+ * Resolves a pid's parent pid — the seam {@link PaneLocator} walks a process's ancestry through,
+ * so its tests can drive the walk from a fake pid→parent map instead of spawning real processes.
+ *
+ * <p>{@link #PROCESS_HANDLE} is the production implementation, backed by {@link ProcessHandle}.
+ */
+public interface ParentResolver {
+
+    /** The parent pid of {@code pid}, or empty if {@code pid} is gone or has no known parent. */
+    OptionalLong parentOf(long pid);
+
+    /** Production resolver: asks the JVM's {@link ProcessHandle} view of the OS process tree. */
+    ParentResolver PROCESS_HANDLE = pid -> ProcessHandle.of(pid)
+            .flatMap(ProcessHandle::parent)
+            .map(parent -> OptionalLong.of(parent.pid()))
+            .orElse(OptionalLong.empty());
+}
@@ -0,0 +1,27 @@
+package dev.ltms.fleet.herdr;
+
+import java.util.HashMap;
+import java.util.Map;
+import java.util.OptionalLong;
+
+/**
+ * Fake {@link ParentResolver} backed by an explicit pid→parent map — lets {@link PaneLocatorTest}
+ * drive {@link PaneLocator}'s ancestry walk (grandchild pids, cycles) without spawning real OS
+ * processes.
+ */
+final class FakeParentResolver implements ParentResolver {
+
+    private final Map<Long, Long> parents = new HashMap<>();
+
+    /** {@code pid}'s parent is {@code parentPid}. A pid with no entry here has no known parent. */
+    FakeParentResolver parent(long pid, long parentPid) {
+        parents.put(pid, parentPid);
+        return this;
+    }
+
+    @Override
+    public OptionalLong parentOf(long pid) {
+        Long parent = parents.get(pid);
+        return parent == null ? OptionalLong.empty() : OptionalLong.of(parent);
+    }
+}
@@ -1,7 +1,11 @@
 package dev.ltms.fleet.herdr;

+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.ObjectMapper;
 import org.junit.jupiter.api.Test;

+import java.util.concurrent.atomic.AtomicInteger;
+
 import static org.junit.jupiter.api.Assertions.*;

 /** Unit tests for PID → pane resolution (the herdr half of connection-based MCP identity). */
@@ -63,4 +67,123 @@ class PaneLocatorTest {
        long paneListCalls = shared.calls.stream().filter(c -> c.method().equals("pane.list")).count();
        assertEquals(1, paneListCalls, "same-object lead/member must scan exactly once, not twice");
    }
+
+    // --- ancestry walk (CB-161: grandchild pids matched no pane, resolving as primary) --------
+
+    @Test
+    void stillResolvesAPidThatIsExactlyThePaneShellPid() {
+        // Regression: a pid with no parent chain at all — no ancestry walk is needed to match it.
+        OnePaneHerdr pane = new OnePaneHerdr("term_x", "pX", 5000, 6000);
+        PaneLocator loc = new PaneLocator(pane, new FakeParentResolver());
+        assertEquals("term_x", loc.terminalForPid(5000));
+    }
+
+    @Test
+    void stillResolvesAPidThatIsExactlyAForegroundPid() {
+        // Regression: same as above, but matching via the foreground-processes list.
+        OnePaneHerdr pane = new OnePaneHerdr("term_x", "pX", 5000, 6000);
+        PaneLocator loc = new PaneLocator(pane, new FakeParentResolver());
+        assertEquals("term_x", loc.terminalForPid(6000));
+    }
+
+    @Test
+    void resolvesAGrandchildPidTwoLevelsBelowTheShellPid() {
+        // The bug: a helper process a worker spawns (python3, curl, ...) is a grandchild of the
+        // pane's shell — not the shell_pid and not a foreground pid directly. Before the fix,
+        // paneOwnsPid only checked direct pid equality, so this pid matched no pane and the
+        // caller fell through to loopback-trust as the primary.
+        OnePaneHerdr pane = new OnePaneHerdr("term_x", "pX", 5000, 6000);
+        FakeParentResolver parents = new FakeParentResolver()
+                .parent(7002, 7001)   // grandchild -> child
+                .parent(7001, 5000);  // child -> shell (the pane's shell_pid)
+        PaneLocator loc = new PaneLocator(pane, parents);
+        assertEquals("term_x", loc.terminalForPid(7002));
+    }
+
+    @Test
+    void nullForAPidWhoseAncestryMatchesNoPane() {
+        // Must not break the other direction: a pid that truly belongs to nothing here (e.g. the
+        // real primary) must still resolve to null. Resolving everything to a worker would demote
+        // the actual lead and refuse every orchestration call.
+        OnePaneHerdr pane = new OnePaneHerdr("term_x", "pX", 5000, 6000);
+        FakeParentResolver parents = new FakeParentResolver()
+                .parent(9002, 9001)
+                .parent(9001, 9000); // chain never reaches 5000 or 6000
+        PaneLocator loc = new PaneLocator(pane, parents);
+        assertNull(loc.terminalForPid(9002));
+    }
+
+    @Test
+    void ancestryWalkTerminatesOnACycleInsteadOfHanging() {
+        // A fake (or corrupted) parent map that cycles must not hang identity resolution, which
+        // runs on every MCP call. The walk must still terminate and correctly resolve to null.
+        OnePaneHerdr pane = new OnePaneHerdr("term_x", "pX", 5000, 6000);
+        FakeParentResolver parents = new FakeParentResolver()
+                .parent(100, 101)
+                .parent(101, 100); // cycle, never reaches the pane's pids
+        PaneLocator loc = new PaneLocator(pane, parents);
+        assertNull(loc.terminalForPid(100));
+    }
+
+    @Test
+    void ancestrySetIsComputedOnceAcrossBothClientsInTheTwoDaemonConstructor() {
+        // CB-185: the two-daemon constructor searches lead then member. The ancestor set is
+        // per-caller, not per-client — it must be walked once and reused, not recomputed for
+        // each client searched.
+        OnePaneHerdr pane = new OnePaneHerdr("term_x", "pX", 5000, 6000);
+        FakeParentResolver parents = new FakeParentResolver()
+                .parent(7002, 7001)
+                .parent(7001, 5000);
+        AtomicInteger calls = new AtomicInteger();
+        ParentResolver counting = pid -> {
+            calls.incrementAndGet();
+            return parents.parentOf(pid);
+        };
+        HerdrClient noPanes = new FakeHerdr().withNoPanes();
+        PaneLocator two = new PaneLocator(noPanes, pane, counting);
+        assertEquals("term_x", two.terminalForPid(7002));
+        assertEquals(3, calls.get(), "ancestry must be walked once (3 lookups: 7002, 7001, 5000), "
+                + "not re-walked per herdr client");
+    }
+
+    /** Minimal single-pane {@link HerdrClient} fake, purpose-built for the ancestry tests above. */
+    private static final class OnePaneHerdr implements HerdrClient {
+        private final ObjectMapper mapper = new ObjectMapper();
+        private final String terminalId;
+        private final String paneId;
+        private final long shellPid;
+        private final long foregroundPid;
+
+        OnePaneHerdr(String terminalId, String paneId, long shellPid, long foregroundPid) {
+            this.terminalId = terminalId;
+            this.paneId = paneId;
+            this.shellPid = shellPid;
+            this.foregroundPid = foregroundPid;
+        }
+
+        @Override
+        public JsonNode call(String method, Object params) {
+            try {
+                return switch (method) {
+                    case "pane.list" -> mapper.readTree(("""
+                            {"type":"pane_list","panes":[
+                              {"pane_id":"%s","terminal_id":"%s","workspace_id":"w1","tab_id":"w1:t1","agent":"claude"}]}""")
+                            .formatted(paneId, terminalId));
+                    case "pane.process_info" -> mapper.readTree(("""
+                            {"type":"pane_process_info","process_info":{"pane_id":"%s","shell_pid":%d,
+                              "foreground_processes":[{"pid":%d,"name":"node","argv0":"claude"}]}}""")
+                            .formatted(paneId, shellPid, foregroundPid));
+                    default -> throw new HerdrException("OnePaneHerdr has no canned response for " + method);
+                };
+            } catch (HerdrException e) {
+                throw e;
+            } catch (Exception e) {
+                throw new HerdrException("OnePaneHerdr decode failed for " + method, e);
+            }
+        }
+
+        @Override
+        public void close() {
+        }
+    }
 }