fleet/fleetd
3
0
Fork 0
Code Issues 73 Pull Requests Actions Packages Projects Releases 1 Wiki Activity

A failed peer-PID lookup promotes a worker to primary: the escalation PaneLocator's own javadoc names, through a trigger CB-161 did not close #317

New Issue
Closed
opened 2026-09-04 08:51:25 +02:00 by ltms · 1 comment
ltms commented 2026-09-04 08:51:25 +02:00
Owner
Copy Link
Copy Source

Found by a delegated hunter. I read every line and confirmed it, corrected one part of its reasoning, and found the fix is already sitting in the data.

This is the third instance today of "a one-way gate is not a gate", and the second privilege escalation in the same resolver after #305.

Live on this daemon

fleetd/fleetd.yaml has no auth: block. FleetConfig documents auth == null → loopback-trust, and loopback-trust is the mode where a loopback non-worker is the primary with no credential. So this is not dormant.

The chain

ConnectionIdentity.resolve:

public Caller resolve(String remoteAddr, int remotePort) {
    if (!isLoopback(remoteAddr)) {
        return new Caller(null, -1); // only same-host callers can be workers
    }
    long pid = pids.pidForLocalPort(remotePort);
    return new Caller(panes.terminalForPid(pid), pid);
}

LsofPeerPidLookup.pidForLocalPort returns -1 on any failure, and terminalForPid(-1) matches no pane, so terminal is null. CallerResolver.resolve then falls to its last line:

// loopback-trust: same-host callers that are not workers are the primary.
return isLoopback(remoteAddr) ? Principal.primary(c.pid()) : Principal.anonymous();

Authz.permits grants that principal SPAWN, STOP, SEND and DRAIN. A real worker's call has become indistinguishable from the lead's.

The codebase already named this exact escalation

PaneLocator's own javadoc:

A pid that is neither of those directly — e.g. a grandchild a worker spawned, such as a python3 or curl helper that opens its own MCP connection — is resolved by walking its ancestry … 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.

CB-161 added the ancestry walk and closed that trigger. The walk only helps once a candidate pid exists. When the lookup itself returns -1 there is nothing to walk, and no retry and no fail-closed path. The gate was built facing the direction the first incident came from.

Correcting the hunter on the trigger

The hunter said the failure happens when lsof "misses the 2s window". That is not right, and the real trigger is worse:

try (BufferedReader r = ...) {
    while ((line = r.readLine()) != null) { ... found = current; ... }
}
if (!p.waitFor(2, TimeUnit.SECONDS)) {
    p.destroyForcibly();
}
return found;

The 2-second waitFor runs after the read loop has already drained stdout to EOF, and its result is discarded — found is returned either way. So a slow lsof blocks in readLine, not at 2 seconds.

The likelier trigger is the one that logs nothing at all: lsof succeeds and simply reports no matching process, so found stays -1 with no exception and no log line. A connection queried before the OS socket table settles, or any output this parser does not match, produces that. The catch (Exception e) path — lsof missing, a fork failure under load, an IO error — is the second way in, and only that one logs, at DEBUG.

The fix is already in the data — this is the part I want you to see

A genuine primary and a failed lookup are not the same Caller:

caller pid terminal
the real primary a real pid, > 0 null — no pane owns it
a worker whose lookup failed -1 null — nothing to match

The primary is a real process with a real connection, so lsof finds its pid; it gets terminal == null because no pane matches, not because the lookup failed. CallerResolver already receives c.pid() — it passes it straight into Principal.primary(c.pid()) and never tests it. The signal that separates the two cases is present and ignored.

What I want

Goal: a caller whose identity could not be resolved must never be treated as the primary. "Not resolvable" and "resolved, and not a worker" must reach different outcomes.

Invariants:

  1. Fail toward the refusal. The charter already says which way to fall: a primary refused is loud and self-correcting, a member acting as the primary fails silently and nobody notices. If you cannot tell who a caller is, refuse.
  2. A genuine non-worker loopback caller must still be the primary in loopback-trust. CallerResolverTest.loopbackTrustTreatsANonWorkerLoopbackCallerAsThePrimary pins this and must stay green. This ticket is about an unresolvable caller, not a non-worker one.
  3. Worker resolution stays unforgeable and never token-gated. Do not touch the branch that returns Principal.worker(...).
  4. Identity resolution runs on every MCP call (contextExtractor, FleetMcp:240). Whatever you add runs on the hot path — it must not add an unbounded wait.

Candidate mechanism, as a candidate only: in the loopback-trust fallback, return the primary only when c.pid() > 0, and otherwise return Principal.anonymous(). Decide it yourself and justify it.

Things to weigh, and I do not know the answers:

  • Is pid > 0 the right predicate, or should ConnectionIdentity say so explicitly? A sentinel that two classes have to agree about is the shape that produced #305 today — one rule, two copies, drifted. A named "unresolved" state on Caller may be the better fix. Argue it.
  • Should the lookup retry before giving up? One lsof miss right after connect() is plausible and transient. A bounded retry might turn most escalations into a small delay instead of a refusal. It also adds latency to every call on the hot path. Say whether it is worth it.
  • What happens to the primary when its own lookup fails? Under the candidate fix its call is refused. That is invariant 1 working as intended, but confirm the refusal is a clean named error and not something that looks like a bug to the operator.
  • token mode is already safe — the same failure lands on presentedTokenMatches, so a bad lookup degrades to ANONYMOUS. Check that your change does not disturb it.

A tested, reported deviation is a good outcome here.

Rules

  • Prove it with a test that fails without the fix: a loopback caller whose PID lookup returns -1 must not resolve to Role.PRIMARY. Build it next to the existing CallerResolverTest cases and use the same construction they use.
  • Add the companion test that invariant 2 still holds — a loopback caller with a real pid that matches no pane is still the primary.
  • Mutation proof required: revert the fix, quote the real failure output, restore it.
  • Describe the class of problem only. Do not write a working exploit, and do not add a test that opens a real socket to escalate. A unit test on the resolver is what is wanted.
  • Do not run git stash — the stash is shared across every worktree here.
  • Do not run git worktree remove or git worktree prune — other workers are live in these worktrees.
  • Stage files explicitly; never git add -A. Never merge.
  • Run cd fleetd && mvn clean install unpiped, and quote the real Tests run: and BUILD lines. Never pipe maven through tail/head, and never read $? after a pipe.
  • Put your full report in the PR body as well as in your fleet_reply.

Shape check

When done, look in auth/ and mcp/ only for the same shape: a failure that is downgraded to a value which is indistinguishable from a legitimate result. -1, null, an empty collection, false. One line each, do not fix any of it.

Found by a delegated hunter. I read every line and confirmed it, corrected one part of its reasoning, and found the fix is already sitting in the data. This is the **third** instance today of "a one-way gate is not a gate", and the second privilege escalation in the same resolver after #305. ## Live on this daemon `fleetd/fleetd.yaml` has no `auth:` block. `FleetConfig` documents `auth == null → loopback-trust`, and `loopback-trust` is the mode where a loopback non-worker is the primary with no credential. **So this is not dormant.** ## The chain `ConnectionIdentity.resolve`: ```java public Caller resolve(String remoteAddr, int remotePort) { if (!isLoopback(remoteAddr)) { return new Caller(null, -1); // only same-host callers can be workers } long pid = pids.pidForLocalPort(remotePort); return new Caller(panes.terminalForPid(pid), pid); } ``` `LsofPeerPidLookup.pidForLocalPort` returns `-1` on any failure, and `terminalForPid(-1)` matches no pane, so `terminal` is `null`. `CallerResolver.resolve` then falls to its last line: ```java // loopback-trust: same-host callers that are not workers are the primary. return isLoopback(remoteAddr) ? Principal.primary(c.pid()) : Principal.anonymous(); ``` `Authz.permits` grants that principal SPAWN, STOP, SEND and DRAIN. A real worker's call has become indistinguishable from the lead's. ## The codebase already named this exact escalation `PaneLocator`'s own javadoc: > A pid that is neither of those directly — e.g. a grandchild a worker spawned, such as a `python3` or `curl` helper that opens its own MCP connection — is resolved by walking its ancestry … **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.** CB-161 added the ancestry walk and closed that trigger. **The walk only helps once a candidate pid exists.** When the lookup itself returns `-1` there is nothing to walk, and no retry and no fail-closed path. The gate was built facing the direction the first incident came from. ## Correcting the hunter on the trigger The hunter said the failure happens when `lsof` "misses the 2s window". That is not right, and the real trigger is worse: ```java try (BufferedReader r = ...) { while ((line = r.readLine()) != null) { ... found = current; ... } } if (!p.waitFor(2, TimeUnit.SECONDS)) { p.destroyForcibly(); } return found; ``` The 2-second `waitFor` runs **after** the read loop has already drained stdout to EOF, and its result is discarded — `found` is returned either way. So a slow `lsof` blocks in `readLine`, not at 2 seconds. The likelier trigger is the one that logs nothing at all: **`lsof` succeeds and simply reports no matching process**, so `found` stays `-1` with no exception and no log line. A connection queried before the OS socket table settles, or any output this parser does not match, produces that. The `catch (Exception e)` path — `lsof` missing, a fork failure under load, an IO error — is the second way in, and only that one logs, at DEBUG. ## The fix is already in the data — this is the part I want you to see A genuine primary and a failed lookup are **not** the same `Caller`: | caller | `pid` | `terminal` | |---|---|---| | the real primary | a real pid, `> 0` | `null` — no pane owns it | | a worker whose lookup failed | `-1` | `null` — nothing to match | The primary is a real process with a real connection, so `lsof` finds its pid; it gets `terminal == null` because no *pane* matches, not because the lookup failed. `CallerResolver` already receives `c.pid()` — it passes it straight into `Principal.primary(c.pid())` and never tests it. The signal that separates the two cases is present and ignored. ## What I want **Goal:** a caller whose identity could not be resolved must never be treated as the primary. "Not resolvable" and "resolved, and not a worker" must reach different outcomes. **Invariants:** 1. **Fail toward the refusal.** The charter already says which way to fall: a primary refused is loud and self-correcting, a member acting as the primary fails silently and nobody notices. If you cannot tell who a caller is, refuse. 2. **A genuine non-worker loopback caller must still be the primary in `loopback-trust`.** `CallerResolverTest.loopbackTrustTreatsANonWorkerLoopbackCallerAsThePrimary` pins this and must stay green. This ticket is about an *unresolvable* caller, not a *non-worker* one. 3. **Worker resolution stays unforgeable and never token-gated.** Do not touch the branch that returns `Principal.worker(...)`. 4. **Identity resolution runs on every MCP call** (`contextExtractor`, `FleetMcp:240`). Whatever you add runs on the hot path — it must not add an unbounded wait. **Candidate mechanism, as a candidate only:** in the `loopback-trust` fallback, return the primary only when `c.pid() > 0`, and otherwise return `Principal.anonymous()`. **Decide it yourself and justify it.** Things to weigh, and I do not know the answers: - **Is `pid > 0` the right predicate, or should `ConnectionIdentity` say so explicitly?** A sentinel that two classes have to agree about is the shape that produced #305 today — one rule, two copies, drifted. A named "unresolved" state on `Caller` may be the better fix. Argue it. - **Should the lookup retry before giving up?** One `lsof` miss right after `connect()` is plausible and transient. A bounded retry might turn most escalations into a small delay instead of a refusal. It also adds latency to every call on the hot path. Say whether it is worth it. - **What happens to the primary when its own lookup fails?** Under the candidate fix its call is refused. That is invariant 1 working as intended, but confirm the refusal is a clean named error and not something that looks like a bug to the operator. - **`token` mode is already safe** — the same failure lands on `presentedTokenMatches`, so a bad lookup degrades to ANONYMOUS. Check that your change does not disturb it. A tested, reported deviation is a good outcome here. ## Rules - Prove it with a test that fails without the fix: a loopback caller whose PID lookup returns `-1` must not resolve to `Role.PRIMARY`. Build it next to the existing `CallerResolverTest` cases and use the same construction they use. - Add the companion test that invariant 2 still holds — a loopback caller with a real pid that matches no pane is still the primary. - Mutation proof required: revert the fix, quote the real failure output, restore it. - **Describe the class of problem only. Do not write a working exploit, and do not add a test that opens a real socket to escalate.** A unit test on the resolver is what is wanted. - Do not run `git stash` — the stash is shared across every worktree here. - Do not run `git worktree remove` or `git worktree prune` — other workers are live in these worktrees. - Stage files explicitly; never `git add -A`. Never merge. - Run `cd fleetd && mvn clean install` **unpiped**, and quote the real `Tests run:` and `BUILD` lines. Never pipe maven through `tail`/`head`, and never read `$?` after a pipe. - Put your full report in the PR body as well as in your `fleet_reply`. ## Shape check When done, look in `auth/` and `mcp/` only for the same shape: **a failure that is downgraded to a value which is indistinguishable from a legitimate result.** `-1`, `null`, an empty collection, `false`. One line each, do **not** fix any of it.
ltms commented 2026-09-04 09:05:41 +02:00
Author
Owner
Copy Link
Copy Source

Merged as de70aa3. Pushed to main.

What I checked myself

Build after the merge, unpiped, on main with #315 already in it:
Tests run: 1330, Failures: 0, Errors: 0, Skipped: 0 — BUILD SUCCESS.
(The worker reported 1328 on its own branch. The two extra tests are #315's, which merged after
the branch forked. The numbers agree.)

The branch was behind main. git merge-base --is-ancestor said so. The files are disjoint
(placement/ vs auth/ + mcp/), the merge was clean, and the build above is what proves it
still compiles — a clean auto-merge is not a compiling merge.

My own mutations, one per direction of the gate

The worker reverted the CallerResolver guard line. I ran two different ones, because the shape
that keeps getting missed here is not "does the gate close" but "which states still open it".

H — the sentinel passes resolved() again. return pid > 0; → return pid != 0;:

CallerResolverTest.aFailedPeerPidLookupIsRefusedNotPromotedToPrimary:124
  an unresolvable caller must never be silently promoted to the primary
  ==> expected: <ANONYMOUS> but was: <PRIMARY>
ConnectionIdentityTest.callerIsUnresolvedWhenThePeerPidLookupFails:51
  a -1 pid means the lookup failed, not that this pid owns no pane
  ==> expected: <false> but was: <true>

I — the gate closes on the real primary too. Added && c.terminal() != null to the grant:

CallerResolverTest.aNonWorkerOnAnyLoopbackSourceAddressIsStillThePrimary:485 source 127.0.0.1
  ==> expected: <PRIMARY> but was: <ANONYMOUS>
CallerResolverTest.aRealPidThatOwnsNoPaneIsStillThePrimaryNotRefused:139
  ==> expected: <PRIMARY> but was: <ANONYMOUS>
CallerResolverTest.loopbackTrustTreatsANonWorkerLoopbackCallerAsThePrimary:103
  the historical behaviour, now an explicit choice ==> expected: <PRIMARY> but was: <ANONYMOUS>

So both halves are pinned: the refusal cannot be loosened, and the primary's own grant cannot be
tightened away by accident. Restored both files and confirmed the tree was clean before pushing.

On the worker's judgment calls

Both deviations are right and I am keeping them.

  • Putting resolved() on the record rather than a bare pid > 0 at the call site is the better
    answer. The precedent it cites is real: #305 was one rule with two copies that drifted.
  • Refusing the retry is right. pidForLocalPort already forks lsof on every MCP call, and there
    is no evidence the failure is transient. Adding unproven latency to that path buys nothing.
  • The new DEBUG line in LsofPeerPidLookup is outside what I asked for and I am glad it is there.
    The silent no-match path is the reason this went unnoticed; a refusal nobody can see is its own
    problem.

The worker's read of the waitFor(2, SECONDS) point agrees with my correction, and it says so
rather than repeating the hunter's original claim. That is the right way to report a disagreement.

The trade this makes, stated plainly

This is now fail-closed. If lsof fails for the primary's own connection, the primary is
refused as ANONYMOUS and its orchestration calls stop working until the next call resolves. That
is a real availability cost, and it is the correct trade: the old behaviour handed workers
primary rights in the same situation. A loud, recoverable refusal beats a silent escalation. If
that refusal ever shows up in practice, the new DEBUG line is what will name it.

Closing.

Merged as `de70aa3`. Pushed to `main`. ## What I checked myself **Build after the merge**, unpiped, on `main` with #315 already in it: `Tests run: 1330, Failures: 0, Errors: 0, Skipped: 0` — `BUILD SUCCESS`. (The worker reported 1328 on its own branch. The two extra tests are #315's, which merged after the branch forked. The numbers agree.) **The branch was behind `main`.** `git merge-base --is-ancestor` said so. The files are disjoint (`placement/` vs `auth/` + `mcp/`), the merge was clean, and the build above is what proves it still compiles — a clean auto-merge is not a compiling merge. ## My own mutations, one per direction of the gate The worker reverted the `CallerResolver` guard line. I ran two different ones, because the shape that keeps getting missed here is not "does the gate close" but "which states still open it". **H — the sentinel passes `resolved()` again.** `return pid > 0;` → `return pid != 0;`: ``` CallerResolverTest.aFailedPeerPidLookupIsRefusedNotPromotedToPrimary:124 an unresolvable caller must never be silently promoted to the primary ==> expected: <ANONYMOUS> but was: <PRIMARY> ConnectionIdentityTest.callerIsUnresolvedWhenThePeerPidLookupFails:51 a -1 pid means the lookup failed, not that this pid owns no pane ==> expected: <false> but was: <true> ``` **I — the gate closes on the real primary too.** Added `&& c.terminal() != null` to the grant: ``` CallerResolverTest.aNonWorkerOnAnyLoopbackSourceAddressIsStillThePrimary:485 source 127.0.0.1 ==> expected: <PRIMARY> but was: <ANONYMOUS> CallerResolverTest.aRealPidThatOwnsNoPaneIsStillThePrimaryNotRefused:139 ==> expected: <PRIMARY> but was: <ANONYMOUS> CallerResolverTest.loopbackTrustTreatsANonWorkerLoopbackCallerAsThePrimary:103 the historical behaviour, now an explicit choice ==> expected: <PRIMARY> but was: <ANONYMOUS> ``` So both halves are pinned: the refusal cannot be loosened, and the primary's own grant cannot be tightened away by accident. Restored both files and confirmed the tree was clean before pushing. ## On the worker's judgment calls Both deviations are right and I am keeping them. - Putting `resolved()` on the record rather than a bare `pid > 0` at the call site is the better answer. The precedent it cites is real: #305 was one rule with two copies that drifted. - Refusing the retry is right. `pidForLocalPort` already forks `lsof` on every MCP call, and there is no evidence the failure is transient. Adding unproven latency to that path buys nothing. - The new DEBUG line in `LsofPeerPidLookup` is outside what I asked for and I am glad it is there. The silent no-match path is the reason this went unnoticed; a refusal nobody can see is its own problem. The worker's read of the `waitFor(2, SECONDS)` point agrees with my correction, and it says so rather than repeating the hunter's original claim. That is the right way to report a disagreement. ## The trade this makes, stated plainly This is now fail-closed. If `lsof` fails for the **primary's own** connection, the primary is refused as `ANONYMOUS` and its orchestration calls stop working until the next call resolves. That is a real availability cost, and it is the correct trade: the old behaviour handed *workers* primary rights in the same situation. A loud, recoverable refusal beats a silent escalation. If that refusal ever shows up in practice, the new DEBUG line is what will name it. Closing.
ltms closed this issue 2026-09-04 09:05:48 +02:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
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
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: fleet/fleetd#317
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?