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

fleetd #518: FleetMcp caller resolution is an explicit choice, and tested for real #524

Merged
ltms merged 1 commits from worker/518-fleetmcp-resolver-wiring-8ef96c-1 into main 2026-09-12 07:10:22 +02:00
Conversation 0 Commits 1 Files Changed 5
+218 -56
agent commented 2026-09-12 06:35:03 +02:00
Member
Copy Link
Copy Source

fleetd #518 — "Nothing tests that FleetMcp uses the CallerResolver".

The defect

FleetMcp's contextExtractor picked its caller-resolution path off callers == null:
enforced -> callers.resolve(...), legacy -> a second, separately-maintained heuristic
(legacyPrincipal). Nothing ever drove a real MCP request through that closure, so a mutant
that replaced the whole decision with an unconditional legacyPrincipal(...) call passed
the full suite (1696 tests) — the closure was an unexercised claim, not a control.

Part 1 — the legacy choice is now explicit at construction

  • callers (CallerResolver) is now a required, non-null constructor parameter, always.
  • A new FleetMcp.AuthorizationMode enum (ENFORCED / UNENFORCED) is a required
    constructor parameter with no default, replacing the callers == null idiom for
    whether denyFor enforces the CB-505 policy table at all.
  • legacyPrincipal is deleted. There is now exactly one resolution path
    (callers.resolve(remoteAddr, remotePort, authorizationHeader)), used unconditionally —
    even under UNENFORCED, so markSpawnedMemberPresent/recordPrimarySingleton still see a
    real identity.

The mutation is impossible to write, not merely caught. I reproduced the ticket's exact
mutation (replace the resolution line with legacyPrincipal(identity, req.getRemoteAddr(), req.getRemotePort())) against the fixed code and it fails to compile:

[ERROR] FleetMcp.java:[360,35] cannot find symbol
  symbol:   method legacyPrincipal(dev.ltms.fleet.mcp.ConnectionIdentity,java.lang.String,int)
  location: class dev.ltms.fleet.mcp.FleetMcp

I verified the mutation actually applied first (grep for the mutant text present, the
original text gone, and a grep -n re-read of the line), watched mvn -o compile fail with
exit 1, then restored the file from a saved copy and confirmed by shasum -a 256 that the
restored file exactly matches the pre-mutation (fixed) content, then ran a green control
build (mvn -o compile exit 0, then the full mvn -o clean install).

Part 2 — a test that actually drives the closure

FleetMcpContextExtractorTest (new) boots the real
HttpServletStreamableServerTransportProvider on a real embedded Jetty server, and drives it
with a real MCP client (io.modelcontextprotocol.sdk streamable-HTTP client) over actual
HTTP. It uses CallerResolver in token mode with a peer-pid lookup that never resolves to a
worker pane, so the only way fleet_whoami can come back "role":"primary" is if the
contextExtractor closure really called callers.resolve(...) and read the Authorization
header — a behaviour the deleted legacyPrincipal heuristic never had. A second call with no
credential at all is asserted refused (ANONYMOUS). This is the one test that would have gone
red under the original mutation; every other existing test calls denyFor(Principal, ...)
with a hand-built Principal and never touches the transport at all.

Must-keep test

FleetMcpAuthzTest.theLegacyConstructorLeavesTheGateOpen still expresses the same thing
(authorization can be turned off), now via mcp(false) passing
AuthorizationMode.UNENFORCED explicitly instead of a null CallerResolver. Adapted, not
deleted.

FleetMcpAuthzTest.legacyPrincipalIsAnonymousNotPrimaryForAnUnresolvedCaller tested the now-
deleted legacyPrincipal method directly (fleetd #509's fix). Since there is no longer a
second heuristic, I renamed/adapted it to
anUnresolvedNonLoopbackCallerIsAnonymousUnderTheOneRealResolver, which proves the same
property (an unresolved, non-loopback caller earns no authority) against the one real
CallerResolver now in use — the same property CallerResolverTest .aNonLoopbackCallerIsNeverThePrimaryUnderLoopbackTrust already independently proves on that
class.

Other call sites updated

  • Fleetd.java (production wiring): passes AuthorizationMode.ENFORCED — production never
    passed a null CallerResolver, so this is a no-behaviour-change wiring update only.
  • FleetMcpHandoverTest.java: same, AuthorizationMode.ENFORCED (it always passed a real
    resolver already).

Also — same shape found elsewhere (NOT fixed, per ticket scope)

  • FleetMcp.principalFrom(Object role, String terminal, long pid, String name) (around line
    577 pre-change): if (role == null) { return terminal != null ? worker : Principal.primary(pid); }
    — a missing/omitted role silently promotes an unresolved caller to primary, the same
    "unsafe branch reached by omission" shape as the defect this ticket fixes, and arguably
    worse (defaults to full authority, not anonymous).
  • dev.ltms.fleet.rest.FleetApp.java:85: private final CallerResolver auth; // CB-501: null -> authz not enforced (legacy behaviour) — the REST-side sibling of the exact field this
    ticket removed from FleetMcp.

Acceptance

  1. mvn -f fleetd/pom.xml clean install — exit 0. Tests run: 1697, Failures: 0, Errors: 0, Skipped: 0 / BUILD SUCCESS (1696 baseline + 1 new test).
  2. Mutation is impossible: compiler error quoted above.
  3. Mutation-applied proof: two greps (mutant text present / original text absent) plus a
    grep -n re-read of the line — all run and shown in the session transcript.
  4. Restored file's shasum -a 256 matches the saved pre-mutation (fixed) copy exactly, and a
    green control build followed. Note: FleetMcp.java's hash necessarily differs from the
    ticket's original pristine hash (517279e6...), because Part 1 is an intentional,
    permanent production-code change to that file — the byte-identical check applies to
    undoing the temporary mutation test, not to reverting the fix itself.

Caveats for review

  • No IDE MCP tooling available to me (worker); verification is mvn output only, shown
    above and in the PR history.
  • FleetMcpContextExtractorTest starts a real Jetty server on an ephemeral port (new Server(0)) and a real MCP HTTP client — it is the first test in this codebase to do so for
    FleetMcp; it adds ~0.3-0.9s to the suite.
fleetd #518 — "Nothing tests that FleetMcp uses the CallerResolver". ## The defect `FleetMcp`'s `contextExtractor` picked its caller-resolution path off `callers == null`: enforced -> `callers.resolve(...)`, legacy -> a second, separately-maintained heuristic (`legacyPrincipal`). Nothing ever drove a real MCP request through that closure, so a mutant that replaced the whole decision with an unconditional `legacyPrincipal(...)` call passed the full suite (1696 tests) — the closure was an unexercised claim, not a control. ## Part 1 — the legacy choice is now explicit at construction - `callers` (`CallerResolver`) is now a **required, non-null** constructor parameter, always. - A new `FleetMcp.AuthorizationMode` enum (`ENFORCED` / `UNENFORCED`) is a required constructor parameter with **no default**, replacing the `callers == null` idiom for whether `denyFor` enforces the CB-505 policy table at all. - `legacyPrincipal` is **deleted**. There is now exactly one resolution path (`callers.resolve(remoteAddr, remotePort, authorizationHeader)`), used unconditionally — even under `UNENFORCED`, so `markSpawnedMemberPresent`/`recordPrimarySingleton` still see a real identity. **The mutation is impossible to write, not merely caught.** I reproduced the ticket's exact mutation (replace the resolution line with `legacyPrincipal(identity, req.getRemoteAddr(), req.getRemotePort())`) against the fixed code and it fails to compile: ``` [ERROR] FleetMcp.java:[360,35] cannot find symbol symbol: method legacyPrincipal(dev.ltms.fleet.mcp.ConnectionIdentity,java.lang.String,int) location: class dev.ltms.fleet.mcp.FleetMcp ``` I verified the mutation actually applied first (grep for the mutant text present, the original text gone, and a `grep -n` re-read of the line), watched `mvn -o compile` fail with exit 1, then restored the file from a saved copy and confirmed by `shasum -a 256` that the restored file exactly matches the pre-mutation (fixed) content, then ran a green control build (`mvn -o compile` exit 0, then the full `mvn -o clean install`). ## Part 2 — a test that actually drives the closure `FleetMcpContextExtractorTest` (new) boots the real `HttpServletStreamableServerTransportProvider` on a real embedded Jetty server, and drives it with a real MCP client (`io.modelcontextprotocol.sdk` streamable-HTTP client) over actual HTTP. It uses `CallerResolver` in token mode with a peer-pid lookup that never resolves to a worker pane, so the *only* way `fleet_whoami` can come back `"role":"primary"` is if the `contextExtractor` closure really called `callers.resolve(...)` and read the `Authorization` header — a behaviour the deleted `legacyPrincipal` heuristic never had. A second call with no credential at all is asserted refused (ANONYMOUS). This is the one test that would have gone red under the original mutation; every other existing test calls `denyFor(Principal, ...)` with a hand-built `Principal` and never touches the transport at all. ## Must-keep test `FleetMcpAuthzTest.theLegacyConstructorLeavesTheGateOpen` still expresses the same thing (authorization can be turned off), now via `mcp(false)` passing `AuthorizationMode.UNENFORCED` explicitly instead of a null `CallerResolver`. Adapted, not deleted. `FleetMcpAuthzTest.legacyPrincipalIsAnonymousNotPrimaryForAnUnresolvedCaller` tested the now- deleted `legacyPrincipal` method directly (fleetd #509's fix). Since there is no longer a second heuristic, I renamed/adapted it to `anUnresolvedNonLoopbackCallerIsAnonymousUnderTheOneRealResolver`, which proves the same property (an unresolved, non-loopback caller earns no authority) against the one real `CallerResolver` now in use — the same property `CallerResolverTest .aNonLoopbackCallerIsNeverThePrimaryUnderLoopbackTrust` already independently proves on that class. ## Other call sites updated - `Fleetd.java` (production wiring): passes `AuthorizationMode.ENFORCED` — production never passed a null `CallerResolver`, so this is a no-behaviour-change wiring update only. - `FleetMcpHandoverTest.java`: same, `AuthorizationMode.ENFORCED` (it always passed a real resolver already). ## Also — same shape found elsewhere (NOT fixed, per ticket scope) - `FleetMcp.principalFrom(Object role, String terminal, long pid, String name)` (around line 577 pre-change): `if (role == null) { return terminal != null ? worker : Principal.primary(pid); }` — a missing/omitted `role` silently promotes an unresolved caller to **primary**, the same "unsafe branch reached by omission" shape as the defect this ticket fixes, and arguably worse (defaults to full authority, not anonymous). - `dev.ltms.fleet.rest.FleetApp.java:85`: `private final CallerResolver auth; // CB-501: null -> authz not enforced (legacy behaviour)` — the REST-side sibling of the exact field this ticket removed from `FleetMcp`. ## Acceptance 1. `mvn -f fleetd/pom.xml clean install` — exit 0. `Tests run: 1697, Failures: 0, Errors: 0, Skipped: 0` / `BUILD SUCCESS` (1696 baseline + 1 new test). 2. Mutation is impossible: compiler error quoted above. 3. Mutation-applied proof: two greps (mutant text present / original text absent) plus a `grep -n` re-read of the line — all run and shown in the session transcript. 4. Restored file's `shasum -a 256` matches the saved pre-mutation (fixed) copy exactly, and a green control build followed. Note: `FleetMcp.java`'s hash necessarily differs from the ticket's *original pristine* hash (`517279e6...`), because Part 1 is an intentional, permanent production-code change to that file — the byte-identical check applies to undoing the *temporary mutation test*, not to reverting the fix itself. ## Caveats for review - No IDE MCP tooling available to me (worker); verification is `mvn` output only, shown above and in the PR history. - `FleetMcpContextExtractorTest` starts a real Jetty server on an ephemeral port (`new Server(0)`) and a real MCP HTTP client — it is the first test in this codebase to do so for `FleetMcp`; it adds ~0.3-0.9s to the suite.
agent added 1 commit 2026-09-12 06:35:04 +02:00
fleetd #518: make the FleetMcp caller-resolution wiring an explicit choice, and test it once for real
CI / contract (pull_request) Successful in 1m13s
Details
CI / build (pull_request) Successful in 2m12s
Details
6a7342b1f0 
FleetMcp's contextExtractor picked its principal-resolution path off `callers == null`, so
"authorization off" also silently swapped in a second, untested identity heuristic
(legacyPrincipal). Nothing drove that closure through a real MCP request, so the whole wiring
was an unexercised claim.

- callers (CallerResolver) is now required, never null.
- A new AuthorizationMode enum (ENFORCED/UNENFORCED) is a required constructor parameter with
  no default, replacing the null-means-legacy idiom for whether denyFor enforces at all.
- legacyPrincipal is deleted: there is exactly one resolution path now
  (callers.resolve(...)), so the mutation that swapped it for an unconditional legacy call no
  longer compiles ("cannot find symbol: method legacyPrincipal").
- FleetMcpContextExtractorTest boots the real transport on a real Jetty server and drives it
  with a real MCP client, proving fleet_whoami's resolved role comes from CallerResolver's
  token check.
- Adapted FleetMcpAuthzTest/FleetMcpHandoverTest call sites; theLegacyConstructorLeavesTheGateOpen
  keeps its meaning under the new AuthorizationMode.UNENFORCED value.
ltms merged commit bb6fc9e0d7 into main 2026-09-12 07:10:22 +02:00
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
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: fleet/fleetd#524
Delete Branch "worker/518-fleetmcp-resolver-wiring-8ef96c-1"

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?