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

fleet_list hands every worker the coordinator row: peer coord-ids and 80-char previews of lead-to-lead bodies #439

New Issue
Closed
opened 2026-09-10 09:07:48 +02:00 by ltms · 1 comment
ltms commented 2026-09-10 09:07:48 +02:00
Owner
Copy Link
Copy Source

Noticed by the #421 implementer, outside its scope and correctly left alone. I verified it myself before filing. The fleet01 lead independently warned about this direction while #421 was being built.

This exists today, on main. #421/PR #438 does not create it, though it adds two fields to what leaks.

The gate

fleet_list maps to READ:

case "fleet_status", "fleet_list", "fleet_profiles", "fleet_whoami" -> Authz.Action.READ;

and READ is open to every authenticated role:

case READ, METRICS -> caller.isPrimary() || caller.isWorker() || caller.isArchitect();

What the row contains

coordinatorView puts all of this into a READ response:

row.put("selfId", selfId);
row.put("configured", true);
row.put("mailbox", mailboxView(probe(channel, selfId)));
row.put("heldCount", held.size());
row.put("heldDurable", true);
row.put("held", held.stream().map(FleetMcp::heldView).toList());
row.put("peers", coordination.peers().stream().map(p -> peerView(channel, p)).toList());

So a worker gets:

  • this daemon's coord-id, and every peer's coord-id with live reachability — cross-host topology.
  • how many messages the lead is holding, and whether they survive a restart.
  • an 80-char preview of each held lead-to-lead body.

That last one is the part that matters. A truncated body is still a body. Leads use this channel to discuss host shapes, credential states, unmerged work and each other's mistakes; the first 80 characters of such a message are usually its subject line, which is the most informative 80 characters it has. My own current previews include things like which tickets are closed and what a peer withdrew.

Why READ's own justification does not cover this

READ's case carries a stated reason: "the roster carries no secrets." That is a claim about the roster. The coordinator row is not the roster — it is coordination state between orchestrators.

#421 had to carve COORD_READ out of READ for exactly this reason, and did so for full bodies on fleet_poll{coordId}. The same argument applies to the previews on fleet_list, and there it was not applied. The result is a gate that is strict about the whole body and open about its first 80 characters.

This is the shape #272 had, in the disclosure direction rather than the destruction direction: the required permission depends on what the response will contain, and the handler picks the action before considering that.

Why the fix is not "raise fleet_list to COORD_READ"

fleet_list is the roster call. Workers legitimately use it, and it is called constantly. Gating the whole tool on a primary-only action would break normal worker operation to protect one field.

The shape that fits is the one #421 used for fleet_poll: let what the response contains depend on who asked. Candidates, none of them decided here:

  1. Omit the coordinator row entirely for a non-primary caller. Simplest, and a worker has no use for it. A worker's fleet_list would carry leads and members and nothing about coordination.
  2. Keep the row, drop held and peers for a non-primary caller. Leaves configured/selfId visible, which is arguably harmless, and removes both the previews and the topology.
  3. Split the tool. More surface, and it does not obviously beat 1.

I lean to 1, because the value of the row to a worker is zero and a field-by-field allow-list is one more thing to keep true as fields are added — this ticket exists because two fields were added to that row without anyone re-asking who can see it.

The invariant worth pinning, whichever is chosen

A response field that only a primary should see must not be reachable through an action a worker holds. Today nothing forces that: heldCount and heldDurable were added to a READ-gated response in PR #438 without any test objecting, exactly as held[] was before them.

So the fix should come with a test that fails when a non-primary fleet_list response contains a coordination field — a test keyed on the response, not on a list of field names, or it becomes the next hardcoded array that stops being exhaustive.

Out of scope

Whether heldView's 80-char cap is the right number. The cap is not the defect; who can see the output of it is. Do not widen or narrow it here — HELD_PREVIEW_MAX_CHARS exists so a constantly-called roster scan never dumps a full body, and #421's tests now pin that boundary with a sentinel character.

Noticed by the #421 implementer, outside its scope and correctly left alone. I verified it myself before filing. The fleet01 lead independently warned about this direction while #421 was being built. This exists **today**, on main. #421/PR #438 does not create it, though it adds two fields to what leaks. ## The gate `fleet_list` maps to `READ`: ```java case "fleet_status", "fleet_list", "fleet_profiles", "fleet_whoami" -> Authz.Action.READ; ``` and `READ` is open to every authenticated role: ```java case READ, METRICS -> caller.isPrimary() || caller.isWorker() || caller.isArchitect(); ``` ## What the row contains `coordinatorView` puts all of this into a `READ` response: ```java row.put("selfId", selfId); row.put("configured", true); row.put("mailbox", mailboxView(probe(channel, selfId))); row.put("heldCount", held.size()); row.put("heldDurable", true); row.put("held", held.stream().map(FleetMcp::heldView).toList()); row.put("peers", coordination.peers().stream().map(p -> peerView(channel, p)).toList()); ``` So a worker gets: - this daemon's coord-id, and **every peer's coord-id** with live reachability — cross-host topology. - how many messages the lead is holding, and whether they survive a restart. - **an 80-char preview of each held lead-to-lead body.** That last one is the part that matters. A truncated body is still a body. Leads use this channel to discuss host shapes, credential states, unmerged work and each other's mistakes; the first 80 characters of such a message are usually its subject line, which is the most informative 80 characters it has. My own current previews include things like which tickets are closed and what a peer withdrew. ## Why `READ`'s own justification does not cover this `READ`'s case carries a stated reason: *"the roster carries no secrets."* That is a claim about the roster. The coordinator row is not the roster — it is coordination state between orchestrators. #421 had to carve `COORD_READ` out of `READ` for exactly this reason, and did so for **full** bodies on `fleet_poll{coordId}`. The same argument applies to the previews on `fleet_list`, and there it was not applied. The result is a gate that is strict about the whole body and open about its first 80 characters. This is the shape #272 had, in the disclosure direction rather than the destruction direction: the required permission depends on what the response will contain, and the handler picks the action before considering that. ## Why the fix is not "raise fleet_list to COORD_READ" `fleet_list` is the roster call. Workers legitimately use it, and it is called constantly. Gating the whole tool on a primary-only action would break normal worker operation to protect one field. The shape that fits is the one `#421` used for `fleet_poll`: let what the response contains depend on who asked. Candidates, none of them decided here: 1. **Omit the `coordinator` row entirely for a non-primary caller.** Simplest, and a worker has no use for it. A worker's `fleet_list` would carry `leads` and `members` and nothing about coordination. 2. **Keep the row, drop `held` and `peers` for a non-primary caller.** Leaves `configured`/`selfId` visible, which is arguably harmless, and removes both the previews and the topology. 3. **Split the tool.** More surface, and it does not obviously beat 1. I lean to **1**, because the value of the row to a worker is zero and a field-by-field allow-list is one more thing to keep true as fields are added — this ticket exists because two fields were added to that row without anyone re-asking who can see it. ## The invariant worth pinning, whichever is chosen A response field that only a primary should see must not be reachable through an action a worker holds. Today nothing forces that: `heldCount` and `heldDurable` were added to a `READ`-gated response in PR #438 without any test objecting, exactly as `held[]` was before them. So the fix should come with a test that fails when a non-primary `fleet_list` response contains a coordination field — a test keyed on the response, not on a list of field names, or it becomes the next hardcoded array that stops being exhaustive. ## Out of scope Whether `heldView`'s 80-char cap is the right number. The cap is not the defect; who can see the output of it is. Do not widen or narrow it here — `HELD_PREVIEW_MAX_CHARS` exists so a constantly-called roster scan never dumps a full body, and #421's tests now pin that boundary with a sentinel character.
ltms commented 2026-09-10 13:44:28 +02:00
Author
Owner
Copy Link
Copy Source

Merged as 92a96fc (PR #462). 3 files, 211 insertions, 5 deletions.

What I verified myself

I rebuilt the merge in my own worktree rather than trusting the branch. Base 1348287, branch tip c1ca627.

  • mvn -B clean test in fleetd/: Tests run: 1583, Failures: 0, Errors: 0, Skipped: 0, BUILD SUCCESS, rc=0. Output redirected to a file; exit code captured on its own line.
  • A 4-cell mutation battery, each cell with a proof gate on the occurrence count before and after, tree verified clean before the run and restored after each cell. Run on merge 8402923 — same two parents, one commit earlier on main.
cell mutation result
CONTROL none 1583, 0 failures, rc=0
M2r coordinatorVisibleTo(principal(exchange)) -> literal true at the one production call site killed by FleetMcpAuthzTest.theFleetListHandlerActuallyConsultsCoordinatorVisibleTo
M3 anchor renamed, listHandler -> listHandlerX, 2 sites, behaviour identical the detector failed, rc=1 — it cannot pass on an empty scrape
M4 return caller.isPrimary(); -> return true; killed by FleetMcpAuthzTest.onlyThePrimaryMaySeeTheCoordinatorRow

M2r is the point of the round. In round 1 that exact mutation survived: the gate existed and worked, every new test called listFleet directly, and nothing asked whether the handler consulted it. It is now killed.

M3 is the cell I care about most, because a source-reading test is code that can quietly stop looking. Renaming its anchor makes it fail loudly instead of reporting "no violation found". That is the difference between a control and a real check.

Two more things I checked outside the battery:

  • ANON is pinned as well as worker and architect: FleetMcpAuthzTest asserts coordinatorVisibleTo(ANON) is false.
  • listFleet( appears in exactly one main file, mcp/FleetMcp.java, and no class under rest/ builds the coordinator row. So the detector reading one file covers every live call site today. Control for that sweep: 109 main .java files matched a string they all contain, so the loop was reading the tree.

Left open, filed separately

#463 — the compat overloads inherit callerIsPrimary = true from one line, FleetMcp.java:1373, which fails open. Nothing is exposed today: the single production call site passes the computed value. Measured precisely there, because my earlier wording ("six overloads default it") was loose — six overloads reach the default, one writes it.

Wiki Features entry added in wiki 840aa28.

Merged as `92a96fc` (PR #462). 3 files, 211 insertions, 5 deletions. ## What I verified myself I rebuilt the merge in my own worktree rather than trusting the branch. Base `1348287`, branch tip `c1ca627`. - `mvn -B clean test` in `fleetd/`: `Tests run: 1583, Failures: 0, Errors: 0, Skipped: 0`, `BUILD SUCCESS`, rc=0. Output redirected to a file; exit code captured on its own line. - A 4-cell mutation battery, each cell with a proof gate on the occurrence count before and after, tree verified clean before the run and restored after each cell. Run on merge `8402923` — same two parents, one commit earlier on main. | cell | mutation | result | |---|---|---| | CONTROL | none | 1583, 0 failures, rc=0 | | M2r | `coordinatorVisibleTo(principal(exchange))` -> literal `true` at the one production call site | **killed** by `FleetMcpAuthzTest.theFleetListHandlerActuallyConsultsCoordinatorVisibleTo` | | M3 | anchor renamed, `listHandler` -> `listHandlerX`, 2 sites, behaviour identical | the detector **failed**, rc=1 — it cannot pass on an empty scrape | | M4 | `return caller.isPrimary();` -> `return true;` | **killed** by `FleetMcpAuthzTest.onlyThePrimaryMaySeeTheCoordinatorRow` | M2r is the point of the round. In round 1 that exact mutation survived: the gate existed and worked, every new test called `listFleet` directly, and nothing asked whether the handler consulted it. It is now killed. M3 is the cell I care about most, because a source-reading test is code that can quietly stop looking. Renaming its anchor makes it fail loudly instead of reporting "no violation found". That is the difference between a control and a real check. Two more things I checked outside the battery: - `ANON` is pinned as well as worker and architect: `FleetMcpAuthzTest` asserts `coordinatorVisibleTo(ANON)` is false. - `listFleet(` appears in exactly one main file, `mcp/FleetMcp.java`, and no class under `rest/` builds the coordinator row. So the detector reading one file covers every live call site today. Control for that sweep: 109 main `.java` files matched a string they all contain, so the loop was reading the tree. ## Left open, filed separately #463 — the compat overloads inherit `callerIsPrimary = true` from one line, `FleetMcp.java:1373`, which fails open. Nothing is exposed today: the single production call site passes the computed value. Measured precisely there, because my earlier wording ("six overloads default it") was loose — six overloads *reach* the default, one writes it. Wiki Features entry added in wiki `840aa28`.
ltms closed this issue 2026-09-10 13:44:30 +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#439
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?