fleetd #705: gate fleet_poll's ticket lookup by the creating caller's terminal #712

2026-10-04T06:50:57+02:00

agent commented

2026-10-04 06:50:57 +02:00

Fixes fleetd #705: a ticket id is a plain sequential counter (task-1, task-2, ...), and poll(ticket) took no caller argument, so any session holding TASK_READ could walk every ticket id and read another session's delegation reply.

MessageService.Task now records the terminal of the caller whose fleet_send{wait:false} created it. poll(ticket, callerTerminal) refuses a caller whose terminal differs from the recorded one, returning a Phase.FAILED view with no reply text. A caller with no terminal (the unnamed primary, resolved by token or loopback trust) is always allowed through, since it never carries a herdr pane to compare against.

Both MessageService.sendAsync/poll and FleetMcp's matching static helpers keep their original no-owner overloads (delegating to the new ones with null), so every existing call site - REST, other tests - compiles and behaves exactly as before. Only the real fleet_send/fleet_poll MCP handlers were updated to pass the live caller terminal through.

Authz, Principal, and the role table are untouched, per the ticket's constraint - the fix lives entirely in the ticket store (MessageService) and the fleet_poll/fleet_send handlers (FleetMcp).

Acceptance criteria

A pollByAnotherTerminalIsRefused: green with the owner comparison present; measured red when the comparison is deleted (mutation testing, see PR description below).
B unnamedPrimaryStillReadsAnyTicket: green with the null-terminal allowance present; measured red when deleted.
C creatorReadsItsOwnTicket: green with the real recorded field; measured red when the recorded owner is replaced with a constant.
D Control on the refusal branch itself: changing it to return the reply text turns test A red, confirming A asserts on the right thing.
E mvn clean install: Tests run: 2003, Failures: 0, Errors: 0, Skipped: 0, BUILD SUCCESS.

All four mutations (A-D) were applied with a line-anchored sed, confirmed mvn compile stayed green (so the mutation was live, not a compile error), confirmed the named test went red, then restored and confirmed green again.

Two questions from the ticket

Does a lead keep the same terminal id across fleet_handover? Yes. LeadRollover.runRolloverUnguarded sends /clear and then the bootstrap text to p.leadTerminal() - the exact same terminal id recorded at open() - never a new pane. The handover clears and reboots a session inside the same herdr pane; it does not create a new one. So a lead's tickets created before a handover stay readable by the lead's own terminal after the handover.
What happens to a ticket whose creating pane is gone? It becomes permanently unreadable by any other terminal-bearing caller (worker, lead, architect, collaborator) - only the unnamed primary (null terminal) can still read it. In practice this rarely matters: a PENDING ticket whose target session is torn down gets abandoned (WORKER_FAILED) by the existing teardown path regardless, and a terminal ticket is pruned after TICKET_TTL_NANOS (10 minutes) anyway. The only real exposure is a named lead (one bound to a specific terminal in config) whose pane is closed and replaced by a different physical pane outside of fleet_handover (which keeps the same terminal, see above) - that lead's own old tickets would become unreadable to it. This is a real but narrow limitation, not fixed here since the ticket's gate is the owner check, not pane lifecycle.

Other handlers with the same shape (reported, not fixed)

GET /tasks/{ticket} (REST, FleetApp.java:893-897) calls messages.poll(ticket) with no caller terminal at all - the identical hole, unaddressed, via the REST door instead of MCP.
fleet_send{turnId} -> MessageService.answer(turnId, ...): turnId is built as session + "#" + askSeq.incrementAndGet() (Rendezvous.java:148) - partly predictable - and answer() never checks that the calling lead/architect is the one whose fleet_ask wait actually owns that turn; Authz.Action.ANSWER is granted to any primary or architect (Authz.java:118), so one architect could in principle answer a turn opened for a different architect's own blocked fleet_send. Reported only, not investigated further, per scope.

Build

mvn clean install in this worktree: Tests run: 2003, Failures: 0, Errors: 0, Skipped: 0, BUILD SUCCESS.

Fixes fleetd #705: a ticket id is a plain sequential counter (`task-1`, `task-2`, ...), and `poll(ticket)` took no caller argument, so any session holding `TASK_READ` could walk every ticket id and read another session's delegation reply. `MessageService.Task` now records the terminal of the caller whose `fleet_send{wait:false}` created it. `poll(ticket, callerTerminal)` refuses a caller whose terminal differs from the recorded one, returning a `Phase.FAILED` view with no reply text. A caller with no terminal (the unnamed primary, resolved by token or loopback trust) is always allowed through, since it never carries a herdr pane to compare against. Both `MessageService.sendAsync`/`poll` and `FleetMcp`'s matching static helpers keep their original no-owner overloads (delegating to the new ones with `null`), so every existing call site - REST, other tests - compiles and behaves exactly as before. Only the real `fleet_send`/`fleet_poll` MCP handlers were updated to pass the live caller terminal through. `Authz`, `Principal`, and the role table are untouched, per the ticket's constraint - the fix lives entirely in the ticket store (`MessageService`) and the `fleet_poll`/`fleet_send` handlers (`FleetMcp`). ## Acceptance criteria - **A** `pollByAnotherTerminalIsRefused`: green with the owner comparison present; measured red when the comparison is deleted (mutation testing, see PR description below). - **B** `unnamedPrimaryStillReadsAnyTicket`: green with the null-terminal allowance present; measured red when deleted. - **C** `creatorReadsItsOwnTicket`: green with the real recorded field; measured red when the recorded owner is replaced with a constant. - **D** Control on the refusal branch itself: changing it to return the reply text turns test A red, confirming A asserts on the right thing. - **E** `mvn clean install`: `Tests run: 2003, Failures: 0, Errors: 0, Skipped: 0`, `BUILD SUCCESS`. All four mutations (A-D) were applied with a line-anchored `sed`, confirmed `mvn compile` stayed green (so the mutation was live, not a compile error), confirmed the named test went red, then restored and confirmed green again. ## Two questions from the ticket 1. **Does a lead keep the same terminal id across `fleet_handover`?** Yes. `LeadRollover.runRolloverUnguarded` sends `/clear` and then the bootstrap text to `p.leadTerminal()` - the exact same terminal id recorded at `open()` - never a new pane. The handover clears and reboots a session inside the same herdr pane; it does not create a new one. So a lead's tickets created before a handover stay readable by the lead's own terminal after the handover. 2. **What happens to a ticket whose creating pane is gone?** It becomes permanently unreadable by any other terminal-bearing caller (worker, lead, architect, collaborator) - only the unnamed primary (null terminal) can still read it. In practice this rarely matters: a `PENDING` ticket whose target session is torn down gets abandoned (`WORKER_FAILED`) by the existing teardown path regardless, and a terminal ticket is pruned after `TICKET_TTL_NANOS` (10 minutes) anyway. The only real exposure is a named lead (one bound to a specific terminal in config) whose pane is closed and replaced by a different physical pane outside of `fleet_handover` (which keeps the same terminal, see above) - that lead's own old tickets would become unreadable to it. This is a real but narrow limitation, not fixed here since the ticket's gate is the owner check, not pane lifecycle. ## Other handlers with the same shape (reported, not fixed) - `GET /tasks/{ticket}` (REST, `FleetApp.java:893-897`) calls `messages.poll(ticket)` with no caller terminal at all - the identical hole, unaddressed, via the REST door instead of MCP. - `fleet_send{turnId}` -> `MessageService.answer(turnId, ...)`: `turnId` is built as `session + "#" + askSeq.incrementAndGet()` (`Rendezvous.java:148`) - partly predictable - and `answer()` never checks that the calling lead/architect is the one whose `fleet_ask` wait actually owns that turn; `Authz.Action.ANSWER` is granted to any primary or architect (`Authz.java:118`), so one architect could in principle answer a turn opened for a different architect's own blocked `fleet_send`. Reported only, not investigated further, per scope. ## Build `mvn clean install` in this worktree: `Tests run: 2003, Failures: 0, Errors: 0, Skipped: 0`, `BUILD SUCCESS`.

agent added 1 commit 2026-10-04 06:50:58 +02:00

fleetd #705 : gate fleet_poll's ticket lookup by the creating caller's terminal

CI / shell-tests (pull_request) Failing after 10s

Details

CI / contract (pull_request) Successful in 51s

Details

CI / build (pull_request) Failing after 1m45s

Details

11eccc3a1b

A ticket id is a plain sequential counter, so any session holding
TASK_READ could walk task-1, task-2, ... and read another session's
delegation reply. sendAsync now records the creating caller's terminal
on the Task, and poll refuses a caller whose terminal differs from it.
A caller with no terminal (the unnamed primary) is still allowed
through regardless, since it never carries a herdr pane to compare.

ltms referenced this issue from a commit

2026-10-04 07:00:44 +02:00

Merge PR #712: fleetd #705 — gate fleet_poll's ticket lookup by the creating caller

ltms commented

2026-10-04 07:01:23 +02:00

Merged locally in 25d53e6. Closing by hand, because a local merge never closes a PR here.

What I verified myself

I did not promote the worker's "clean" to a fact. Merged onto origin/main in a throwaway
worktree and ran mvn clean install myself, output written to a file and not piped, because a
pipe hides a failure behind a zero exit:

exit 0, BUILD SUCCESS, Tests run: 2008, Failures: 0, Errors: 0, Skipped: 0
MessageServiceTest: 93

2008 is 2005 plus the three new tests. The worker reported 2003 from its own base, which was
before PR #709 landed: 2000 + 3 = 2003 and 2005 + 3 = 2008 both agree, so there was no merge
surprise. The tree I pushed is byte-identical to the tree I built (6939bf0).

The hinge of the fix, which I checked rather than assumed

ownsTicket allows any caller whose terminal is null. That is safe only if null cannot also
mean "I could not tell who this is" — one symbol standing for two states is how this kind of gate
usually fails. It does not here, and this is why:

CALLER_TERMINAL has exactly one writer, in the contextExtractor, which runs on every MCP
call and always sets the key (orEmpty(p.terminal())). So null out of callerTerminal() means
precisely "the resolved Principal carries no terminal".
Every non-primary Principal in CallerResolver is built with c.terminal(). The only two
terminal-less constructions are Principal.primary(...) and Principal.anonymous().
ANONYMOUS is denied TASK_READ at Authz.java:145
(caller.isPrimary() || caller.isWorker() || caller.isArchitect()), so it never reaches poll.

So null really is just the primary. Note the cross-gate dependency: ownsTicket's
null-allowance is safe because Authz denies ANONYMOUS TASK_READ. Nothing pins that
coupling — granting ANONYMOUS a read action would silently open the ticket gate.

A confirmed gap: the handler wiring is not pinned

The three new tests prove MessageService.poll(ticket, callerTerminal) refuses correctly. That is
the seam. Nothing proves the MCP handler passes the caller's real terminal into it.

I measured this rather than asserting it. A line-anchored sed replaced
callerTerminal(exchange) with null at FleetMcp.java:527, mvn compile stayed green so the
mutation was live and not a compile error, and then:

Tests run: 2008, Failures: 0, Errors: 0, Skipped: 0
BUILD SUCCESS

The mutation survives the whole suite. The entire fix can go inert and nothing notices. I
restored the file and confirmed it byte-identical before merging.

This repo already has the idiom for it — FleetMcpAuthzTest scrapes a handler block and asserts
the argument it passes, with a control assertion so it cannot pass by failing to find the block.
theFleetListHandlerActuallyConsultsCollaboratorsVisibleTo is the model.

Scope note

This closes the MCP door and deliberately leaves the REST door open — FleetApp.java:898 still
calls the no-check poll(ticket) overload, and TASK_READ is granted to a worker. The worker
reported that honestly and I confirmed it. #705 therefore stays open. Merging anyway: it is a
strict improvement and blocking one because it is not yet total would be the wrong trade.

Both remaining items are now one follow-up unit. Thanks for the mutation work on A–D and for
reporting the REST hole and the turnId observation unprompted.

Merged locally in `25d53e6`. Closing by hand, because a local merge never closes a PR here. ## What I verified myself I did not promote the worker's "clean" to a fact. Merged onto `origin/main` in a throwaway worktree and ran `mvn clean install` myself, output written to a file and not piped, because a pipe hides a failure behind a zero exit: - exit 0, `BUILD SUCCESS`, `Tests run: 2008, Failures: 0, Errors: 0, Skipped: 0` - `MessageServiceTest: 93` 2008 is 2005 plus the three new tests. The worker reported 2003 from its own base, which was before PR #709 landed: 2000 + 3 = 2003 and 2005 + 3 = 2008 both agree, so there was no merge surprise. The tree I pushed is byte-identical to the tree I built (`6939bf0`). ## The hinge of the fix, which I checked rather than assumed `ownsTicket` allows any caller whose terminal is `null`. That is safe only if `null` cannot also mean "I could not tell who this is" — one symbol standing for two states is how this kind of gate usually fails. It does not here, and this is why: - `CALLER_TERMINAL` has exactly **one** writer, in the `contextExtractor`, which runs on every MCP call and always sets the key (`orEmpty(p.terminal())`). So `null` out of `callerTerminal()` means precisely "the resolved Principal carries no terminal". - Every non-primary Principal in `CallerResolver` is built with `c.terminal()`. The only two terminal-less constructions are `Principal.primary(...)` and `Principal.anonymous()`. - `ANONYMOUS` is **denied `TASK_READ`** at `Authz.java:145` (`caller.isPrimary() || caller.isWorker() || caller.isArchitect()`), so it never reaches `poll`. So `null` really is just the primary. **Note the cross-gate dependency:** `ownsTicket`'s null-allowance is safe *because* `Authz` denies `ANONYMOUS` `TASK_READ`. Nothing pins that coupling — granting `ANONYMOUS` a read action would silently open the ticket gate. ## A confirmed gap: the handler wiring is not pinned The three new tests prove `MessageService.poll(ticket, callerTerminal)` refuses correctly. That is the **seam**. Nothing proves the MCP handler passes the caller's *real* terminal into it. I measured this rather than asserting it. A line-anchored `sed` replaced `callerTerminal(exchange)` with `null` at `FleetMcp.java:527`, `mvn compile` stayed green so the mutation was live and not a compile error, and then: ``` Tests run: 2008, Failures: 0, Errors: 0, Skipped: 0 BUILD SUCCESS ``` **The mutation survives the whole suite.** The entire fix can go inert and nothing notices. I restored the file and confirmed it byte-identical before merging. This repo already has the idiom for it — `FleetMcpAuthzTest` scrapes a handler block and asserts the argument it passes, with a control assertion so it cannot pass by failing to find the block. `theFleetListHandlerActuallyConsultsCollaboratorsVisibleTo` is the model. ## Scope note This closes the MCP door and deliberately leaves the REST door open — `FleetApp.java:898` still calls the no-check `poll(ticket)` overload, and `TASK_READ` is granted to a worker. The worker reported that honestly and I confirmed it. **#705 therefore stays open.** Merging anyway: it is a strict improvement and blocking one because it is not yet total would be the wrong trade. Both remaining items are now one follow-up unit. Thanks for the mutation work on A–D and for reporting the REST hole and the `turnId` observation unprompted.

ltms closed this pull request

2026-10-04 07:01:26 +02:00

ltms referenced this pull request

2026-10-04 07:01:42 +02:00

fleetd #710: Remove CallerResolver members accessor #713

ltms referenced this pull request

2026-10-04 07:03:14 +02:00

A turnId is a guessable counter and answer() takes no caller identity, so any architect can answer a turn opened for someone else — the same shape as #705, on a write path #715

ltms referenced this pull request

2026-10-04 07:03:37 +02:00

Any unconfigured herdr pane resolves to WORKER, and WORKER holds TASK_READ — so an unlisted tab can walk every ticket and read other sessions' delegation replies #705

ltms referenced this pull request