fleetd #505: refuse (not promote) a caller whose pane scan errored #508

Merged

ltms merged 1 commits from worker/505-03f8b2-1 into main 2026-09-12 05:35:42 +02:00

Conversation 0 Commits 1 Files Changed 8

+209 -39

agent commented 2026-09-12 05:28:26 +02:00

Member

Copy Link

Copy Source

fleetd #505 — a herdr error during the pane scan must not resolve a worker as the primary

The defect

PaneLocator.paneOwnsAnyOf caught HerdrException from pane.process_info and returned
false — "this pane does not own the pid" — for both a pane that genuinely vanished mid-scan
and a pane that failed for a transient reason while it actually did own the caller's pid.
If the failing pane was the caller's own, the scan finished with a clean-looking null
terminal on a resolved (real) pid — exactly the shape CallerResolver's loopback-trust
fallback reads as the primary. That is a worker→primary privilege escalation through a door
fleetd #317 did not close: #317 guards a failed lsof lookup (Caller.resolved()), not a
failed herdr pane scan.

The fix — the third-state shape, and why

Per the ticket, Caller.resolved() is not widened — it stays exactly pid > 0, centralised
next to the lsof -1 sentinel it tests (ConnectionIdentity.java:48-59).

Instead, PaneLocator.terminalForPid now returns a record, Lookup(String terminal, boolean complete), rather than a bare String. I chose the record shape (the ticket's second option)
over a three-valued paneOwnsAnyOf propagated all the way up, because the record cleanly
separates "the answer" from "how much of the scan is behind that answer" at every level
(paneOwnsAnyOf → Ownership enum internally, scan → Lookup per herdr client,
terminalForPid → Lookup across every client) without needing a second return channel.

• paneOwnsAnyOf now returns a private Ownership enum: OWNS / DOES_NOT_OWN / UNKNOWN.
  UNKNOWN is the new case — a herdr error, not a confirmed non-match.
• scan (one herdr client) returns Lookup(terminal, complete): a match short-circuits
  immediately as complete = true regardless of any other pane's earlier failure — a
  pane that genuinely vanished mid-scan but was never the caller's own must not turn into a
  refusal (the ticket's second acceptance test).
• terminalForPid (across every searched herdr client, CB-185's lead/member split) returns the
  first definite match found on any client, else Lookup(null, complete) where complete is
  the AND of every client's completeness — one daemon's error must not read as a clean negative
  for the whole scan.
• ConnectionIdentity.Caller gains a third component, scanComplete, carried straight from
  Lookup.complete(). resolved() is untouched.
• CallerResolver's loopback-trust fallback now requires both c.resolved() and
  c.scanComplete() before granting Principal.primary(...); an incomplete scan resolves
  Principal.anonymous() — failing toward the recoverable error, as the ticket asks: a refused
  primary retries loudly, a promoted worker would not.
• Logs a warn naming the pane id and which herdr client (of how many) failed, so the
  incomplete-scan path is diagnosable rather than silent (#317's own lesson, per the ticket).

LsofPeerPidLookup.java and LsofProcessCwdLookup.java were not touched, per the ticket.

Tests

• PaneLocatorTest: the discriminating case — pane.process_info fails for exactly the pane
  that owns the caller's pid (w2:p7, term_a) — asserts terminal() == null and
  complete() == false. Companion test: a different, non-owning pane fails
  (w2:p9) — asserts the real match is still found (term_a) and complete() == true.
• ConnectionIdentityTest: propagates the same scenario through Caller — resolved() true,
  terminal() null, scanComplete() false; distinguished from the existing "real primary, no
  herdr error" test which now also asserts scanComplete() true.
• CallerResolverTest: the acceptance-level pair — an error on the owning pane resolves
  Role.ANONYMOUS (not PRIMARY); an error on a non-owning pane still resolves Role.WORKER
  with the right terminal. All pre-existing #317 tests (failed lsof lookup, real primary, token
  mode, leads/architects) stay green untouched.
• FakeHerdr gained processInfoFailsForPane(paneId, code), mirroring the existing
  paneCloseFailsForPane/tabCloseFailsForTab per-target-failure pattern.
• PaneLocatorContractTest updated for the Lookup return type only (.terminalForPid(pid) →
  .terminalForPid(pid).terminal()); not run here (needs a live herdr socket — assumeTrue
  skips without one, consistent with the addendum's herdr-socket-test carve-out).

Mutation proof (self-verified; the lead will re-run)

1. Baseline (fixed code): PaneLocatorTest 15/15, ConnectionIdentityTest 8/8,
   CallerResolverTest 40/40 green (full-suite run: Tests run: 1694, Failures: 0, Errors: 0).
2. Reverted the actual fix line only (kept the Lookup/Ownership API, so tests still compile):
   src/main/java/dev/ltms/fleet/herdr/PaneLocator.java:183, return Ownership.UNKNOWN; →
   return Ownership.DOES_NOT_OWN; (pre-fix behaviour: a herdr error reads as a clean
   non-match again).
3. Re-ran mvn test -Dtest=PaneLocatorTest,ConnectionIdentityTest,CallerResolverTest:
   BUILD FAILURE, exactly Tests run: 63, Failures: 3, Errors: 0:
   • CallerResolverTest.aHerdrErrorOnTheOwningPaneDuringTheScanIsRefusedNotPromotedToPrimary —
     expected: <ANONYMOUS> but was: <PRIMARY>
   • ConnectionIdentityTest.scanIsIncompleteWhenHerdrErrorsOnThePaneThatOwnsThePid —
     expected: <false> but was: <true>
   • PaneLocatorTest.anErrorOnThePaneThatOwnsThePidMakesTheScanIncompleteNotAClearNegative —
     expected: <false> but was: <true>
     All 60 other tests in those 3 classes (including the "vanished pane, still resolves" and
     "real primary" companions) stayed green — the mutant is caught by exactly the 3 tests meant
     to pin it, nothing else.
4. Proved the mutant text with two different greps: one matching the mutant's own marker
   comment (present), one matching the original return Ownership.UNKNOWN; line (absent,
   grep exit 1).
5. Restored the line, then shasum -a 256 on PaneLocator.java matched the pre-mutation hash
   byte-for-byte (81b797a6cfb823e489e938b215956271d5741cce49dc9732f779c7ed64a3b811).
6. Control run after restoring: mvn clean install, unpiped —
   Tests run: 1694, Failures: 0, Errors: 0, Skipped: 0, BUILD SUCCESS.

Build

cd fleetd && mvn clean install (unpiped): Tests run: 1694, Failures: 0, Errors: 0, Skipped: 0,
BUILD SUCCESS.

Out of scope, noted only

LsofPeerPidLookup.java/LsofProcessCwdLookup.java — left untouched per the ticket's explicit
instruction; not the same defect.

## fleetd #505 — a herdr error during the pane scan must not resolve a worker as the primary ### The defect `PaneLocator.paneOwnsAnyOf` caught `HerdrException` from `pane.process_info` and returned `false` — "this pane does not own the pid" — for both a pane that genuinely vanished mid-scan **and** a pane that failed for a transient reason while it actually did own the caller's pid. If the failing pane was the caller's own, the scan finished with a clean-looking `null` terminal on a **resolved** (real) pid — exactly the shape `CallerResolver`'s loopback-trust fallback reads as the primary. That is a worker→primary privilege escalation through a door fleetd #317 did not close: #317 guards a failed **lsof** lookup (`Caller.resolved()`), not a failed **herdr pane scan**. ### The fix — the third-state shape, and why Per the ticket, `Caller.resolved()` is **not** widened — it stays exactly `pid > 0`, centralised next to the lsof `-1` sentinel it tests (`ConnectionIdentity.java:48-59`). Instead, `PaneLocator.terminalForPid` now returns a **record**, `Lookup(String terminal, boolean complete)`, rather than a bare `String`. I chose the record shape (the ticket's second option) over a three-valued `paneOwnsAnyOf` propagated all the way up, because the record cleanly separates "the answer" from "how much of the scan is behind that answer" at every level (`paneOwnsAnyOf` → `Ownership` enum internally, `scan` → `Lookup` per herdr client, `terminalForPid` → `Lookup` across every client) without needing a second return channel. - `paneOwnsAnyOf` now returns a private `Ownership` enum: `OWNS` / `DOES_NOT_OWN` / `UNKNOWN`. `UNKNOWN` is the new case — a herdr error, not a confirmed non-match. - `scan` (one herdr client) returns `Lookup(terminal, complete)`: a match short-circuits immediately as `complete = true` regardless of any *other* pane's earlier failure — a pane that genuinely vanished mid-scan but was never the caller's own must not turn into a refusal (the ticket's second acceptance test). - `terminalForPid` (across every searched herdr client, CB-185's lead/member split) returns the first definite match found on any client, else `Lookup(null, complete)` where `complete` is the AND of every client's completeness — one daemon's error must not read as a clean negative for the whole scan. - `ConnectionIdentity.Caller` gains a third component, `scanComplete`, carried straight from `Lookup.complete()`. `resolved()` is untouched. - `CallerResolver`'s loopback-trust fallback now requires **both** `c.resolved()` **and** `c.scanComplete()` before granting `Principal.primary(...)`; an incomplete scan resolves `Principal.anonymous()` — failing toward the recoverable error, as the ticket asks: a refused primary retries loudly, a promoted worker would not. - Logs a `warn` naming the pane id and which herdr client (of how many) failed, so the incomplete-scan path is diagnosable rather than silent (#317's own lesson, per the ticket). `LsofPeerPidLookup.java` and `LsofProcessCwdLookup.java` were **not** touched, per the ticket. ### Tests - `PaneLocatorTest`: the discriminating case — `pane.process_info` fails for exactly the pane that owns the caller's pid (`w2:p7`, `term_a`) — asserts `terminal() == null` **and** `complete() == false`. Companion test: a *different*, non-owning pane fails (`w2:p9`) — asserts the real match is still found (`term_a`) and `complete() == true`. - `ConnectionIdentityTest`: propagates the same scenario through `Caller` — `resolved()` true, `terminal()` null, `scanComplete()` false; distinguished from the existing "real primary, no herdr error" test which now also asserts `scanComplete()` true. - `CallerResolverTest`: the acceptance-level pair — an error on the owning pane resolves `Role.ANONYMOUS` (not `PRIMARY`); an error on a non-owning pane still resolves `Role.WORKER` with the right terminal. All pre-existing #317 tests (failed lsof lookup, real primary, token mode, leads/architects) stay green untouched. - `FakeHerdr` gained `processInfoFailsForPane(paneId, code)`, mirroring the existing `paneCloseFailsForPane`/`tabCloseFailsForTab` per-target-failure pattern. - `PaneLocatorContractTest` updated for the `Lookup` return type only (`.terminalForPid(pid)` → `.terminalForPid(pid).terminal()`); not run here (needs a live herdr socket — `assumeTrue` skips without one, consistent with the addendum's herdr-socket-test carve-out). ### Mutation proof (self-verified; the lead will re-run) 1. Baseline (fixed code): `PaneLocatorTest` 15/15, `ConnectionIdentityTest` 8/8, `CallerResolverTest` 40/40 green (full-suite run: `Tests run: 1694, Failures: 0, Errors: 0`). 2. Reverted the actual fix line only (kept the `Lookup`/`Ownership` API, so tests still compile): `src/main/java/dev/ltms/fleet/herdr/PaneLocator.java:183`, `return Ownership.UNKNOWN;` → `return Ownership.DOES_NOT_OWN;` (pre-fix behaviour: a herdr error reads as a clean non-match again). 3. Re-ran `mvn test -Dtest=PaneLocatorTest,ConnectionIdentityTest,CallerResolverTest`: `BUILD FAILURE`, exactly `Tests run: 63, Failures: 3, Errors: 0`: - `CallerResolverTest.aHerdrErrorOnTheOwningPaneDuringTheScanIsRefusedNotPromotedToPrimary` — `expected: <ANONYMOUS> but was: <PRIMARY>` - `ConnectionIdentityTest.scanIsIncompleteWhenHerdrErrorsOnThePaneThatOwnsThePid` — `expected: <false> but was: <true>` - `PaneLocatorTest.anErrorOnThePaneThatOwnsThePidMakesTheScanIncompleteNotAClearNegative` — `expected: <false> but was: <true>` All 60 other tests in those 3 classes (including the "vanished pane, still resolves" and "real primary" companions) stayed green — the mutant is caught by exactly the 3 tests meant to pin it, nothing else. 4. Proved the mutant text with two different greps: one matching the mutant's own marker comment (present), one matching the original `return Ownership.UNKNOWN;` line (absent, grep exit 1). 5. Restored the line, then `shasum -a 256` on `PaneLocator.java` matched the pre-mutation hash byte-for-byte (`81b797a6cfb823e489e938b215956271d5741cce49dc9732f779c7ed64a3b811`). 6. Control run after restoring: `mvn clean install`, unpiped — `Tests run: 1694, Failures: 0, Errors: 0, Skipped: 0`, `BUILD SUCCESS`. ### Build `cd fleetd && mvn clean install` (unpiped): `Tests run: 1694, Failures: 0, Errors: 0, Skipped: 0`, `BUILD SUCCESS`. ### Out of scope, noted only `LsofPeerPidLookup.java`/`LsofProcessCwdLookup.java` — left untouched per the ticket's explicit instruction; not the same defect.

agent added 1 commit 2026-09-12 05:28:26 +02:00

fleetd #505: a herdr error during the pane scan must not read as a clean negative ... 36870836aa

A transient herdr error on pane.process_info during PaneLocator's pid→pane scan used
to be swallowed into a plain "does not own it", so a real worker whose owning pane
errored mid-scan resolved with a null terminal but a resolved (real) pid — exactly
what CallerResolver's loopback-trust fallback reads as the primary. That is a
worker→primary privilege escalation through the door fleetd #317 did not close: #317
guards a failed lsof lookup (c.resolved()), not a failed herdr pane scan.

Fix: add a third state to the scan instead of widening Caller.resolved() (which stays
centralised next to the lsof sentinel it tests, per #505's explicit instruction not
to reopen that decision). PaneLocator.terminalForPid now returns a Lookup(terminal,
complete) record: a HerdrException on one pane marks that pane's ownership UNKNOWN,
not DOES_NOT_OWN, and the scan is complete only if every pane was either matched or
confirmed not to own the pid. A definite match found elsewhere in the same scan
still short-circuits as complete — a pane that genuinely vanished mid-scan without
being the caller's own does not turn into a refusal.

ConnectionIdentity.Caller carries the new scanComplete flag alongside the unchanged
resolved(). CallerResolver's loopback-trust fallback now requires both resolved()
and scanComplete() before promoting to Principal.primary(); an incomplete scan
resolves anonymous, which fails toward the recoverable error (a refused primary
retries loudly; a promoted worker would not).

Logs a warning naming the pane and which herdr client (of how many) failed, so the
incomplete-scan path is diagnosable rather than silent (fleetd #317's own lesson).

ltms merged commit 40c593cd09 into main 2026-09-12 05:35:42 +02:00

ltms referenced this pull request 2026-09-12 05:36:12 +02:00

#505 follow-up: the two-client completeness fold is unpinned, and legacyPrincipal can still re-open the escalation #509

Sign in to join this conversation.

Reviewers

No Reviewers

Labels

Clear labels

blocked

Cannot start until something else lands. The body says what.

needs-live-proof

Merged and green, but never shown working on the running daemon. Not the same as done.

ready-to-delegate

Scope, files and acceptance criteria are written. A worker can be briefed from the body alone.

silent-default

A feature that compiles, passes tests, and ships turned off. Nine recurrences and counting.

No Label

Milestone

No items

No Milestone

Projects

Clear projects

No project

Assignees

Clear assignees

agent kevin ltms

No Assignees

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: fleet/fleetd#508