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

The production authorization gate is unpinned: flipping FleetdAssembly.java:481 to UNENFORCED leaves all 1928 tests green #672

New Issue
Closed
opened 2026-10-03 20:29:22 +02:00 by ltms · 1 comment
ltms commented 2026-10-03 20:29:22 +02:00
Owner
Copy Link
Copy Source

Found while verifying #670. The #670 worker was asked to report other instances of the same shape without fixing them, and it named this one. I measured it, and it is worse than the gap #670 closed.

The gap

FleetdAssembly.java:481 passes FleetMcp.AuthorizationMode.ENFORCED to the FleetMcp constructor as a literal. It is never varied by config — a grep for AuthorizationMode over fleetd/src/main/java shows this is the only production use, with the rest being the enum declaration and its javadoc in FleetMcp.java.

That literal is the on/off switch for the whole role table. FleetMcp.java:410 reduces it to one boolean: … == AuthorizationMode.ENFORCED. Flip it and every Authz decision stops being enforced — including the primary-only SPAWN, STOP, DRAIN and HANDOVER at Authz.java:68-72.

No test pins it. The only test that exercises the parameter is FleetMcpAuthzTest.java:89, which constructs its own FleetMcp and chooses its own value: enforce ? FleetMcp.AuthorizationMode.ENFORCED : FleetMcp.AuthorizationMode.UNENFORCED. That tests the seam. It says nothing about what production passes.

Measured on 7f9a9c0

Mutation run in a throwaway worktree, never the main clone.

perl -i -pe 's/AuthorizationMode\.ENFORCED/AuthorizationMode.UNENFORCED/ if $. == 481' \
    fleetd/src/main/java/dev/ltms/fleet/FleetdAssembly.java

git diff --numstat   ->  1  1  fleetd/src/main/java/dev/ltms/fleet/FleetdAssembly.java
mvn -o clean install ->  AUTHZ_MUTANT_EXIT=0
                         [INFO] Tests run: 1928, Failures: 0, Errors: 0, Skipped: 0
                         [INFO] BUILD SUCCESS
                         report files: 171

A grep for every <<< FAILURE and <<< ERROR line in the log returned nothing. Not one test went red.

I then reverted and confirmed the worktree was byte-identical to HEAD, with both line 481 and line 265 restored.

Why this is more serious than #670

#670 was a latent risk: Set.of() is correct, and a future edit in the wrong direction would have reintroduced a demotion bug. Here the unpinned value is the security control. Compare the blast radius:

#670 this
What the literal controls one scanner's workspace filter whether the role table is enforced at all
Effect if flipped a lead could be demoted to worker every caller passes every capability check
Caught by a test now yes (#671) no

The failure mode is also quiet in the worst way. An unenforced gate does not throw, does not log a refusal, and does not change any response shape that a test asserts on. The fleet keeps working — it just stops checking. fleet_spawn from a worker would succeed.

This is the same shape as #670, and that is the point

Two instances in one file, found one day apart, both with the identical signature: a production call site supplies a constant that encodes a real decision, and the only test covering that parameter supplies its own value instead. FleetMcp's own javadoc at :354-361 even explains that UNENFORCED exists for the pre-CB-513 test suite — so the dangerous value is a legitimate, reachable constant, not something a typo would have to invent.

Worth sweeping FleetdAssembly for the rest of the family rather than fixing these two and stopping.

Acceptance criteria

  1. A test asserts that FleetdAssembly's production boot path builds a FleetMcp with AuthorizationMode.ENFORCED. Mutating line 481 to UNENFORCED must turn that test red. Report the mutant output, not only the green run.
  2. The test must observe the production producer. A test that constructs FleetMcp itself repeats the blind spot at FleetMcpAuthzTest.java:89.
  3. Name the failing test class and method from the mutant run, and confirm no unrelated test reddened. FleetdAssemblyLeadTabScannerExclusionTest (merged in #671) is a working model for reaching into the assembled object graph: it drives FleetdAssembly.assembleAndStart with a fake ResourcePorts that never binds a port, and it carries a loud control assertion so it cannot pass for the wrong reason.
  4. Prefer a behavioural assertion over a field read if one is reachable. A test that proves an unauthorized caller is actually refused through the assembled FleetMcp is stronger evidence than reading back the enum. Only fall back to reflection if the behavioural route needs a seam that does not exist.
  5. Do not change FleetdAssembly.java:481. ENFORCED is correct. This ticket adds a test.

Also worth doing, separately

Sweep the remaining constructor arguments in FleetdAssembly for the same shape and report them with file:line, without fixing. Two confirmed instances in one file suggest the file is the pattern, not the exception.

Found while verifying #670. The #670 worker was asked to report other instances of the same shape without fixing them, and it named this one. I measured it, and it is worse than the gap #670 closed. ## The gap `FleetdAssembly.java:481` passes `FleetMcp.AuthorizationMode.ENFORCED` to the `FleetMcp` constructor as a literal. It is never varied by config — a grep for `AuthorizationMode` over `fleetd/src/main/java` shows this is the only production use, with the rest being the enum declaration and its javadoc in `FleetMcp.java`. **That literal is the on/off switch for the whole role table.** `FleetMcp.java:410` reduces it to one boolean: `… == AuthorizationMode.ENFORCED`. Flip it and every `Authz` decision stops being enforced — including the primary-only `SPAWN`, `STOP`, `DRAIN` and `HANDOVER` at `Authz.java:68-72`. **No test pins it.** The only test that exercises the parameter is `FleetMcpAuthzTest.java:89`, which constructs its own `FleetMcp` and chooses its own value: `enforce ? FleetMcp.AuthorizationMode.ENFORCED : FleetMcp.AuthorizationMode.UNENFORCED`. That tests the seam. It says nothing about what production passes. ## Measured on 7f9a9c0 Mutation run in a throwaway worktree, never the main clone. ``` perl -i -pe 's/AuthorizationMode\.ENFORCED/AuthorizationMode.UNENFORCED/ if $. == 481' \ fleetd/src/main/java/dev/ltms/fleet/FleetdAssembly.java git diff --numstat -> 1 1 fleetd/src/main/java/dev/ltms/fleet/FleetdAssembly.java mvn -o clean install -> AUTHZ_MUTANT_EXIT=0 [INFO] Tests run: 1928, Failures: 0, Errors: 0, Skipped: 0 [INFO] BUILD SUCCESS report files: 171 ``` A grep for every `<<< FAILURE` and `<<< ERROR` line in the log returned **nothing**. Not one test went red. I then reverted and confirmed the worktree was byte-identical to `HEAD`, with both line 481 and line 265 restored. ## Why this is more serious than #670 #670 was a latent risk: `Set.of()` is correct, and a future edit in the wrong direction would have reintroduced a demotion bug. Here the unpinned value **is** the security control. Compare the blast radius: | | #670 | this | |---|---|---| | What the literal controls | one scanner's workspace filter | whether the role table is enforced at all | | Effect if flipped | a lead could be demoted to worker | every caller passes every capability check | | Caught by a test | now yes (#671) | **no** | The failure mode is also quiet in the worst way. An unenforced gate does not throw, does not log a refusal, and does not change any response shape that a test asserts on. The fleet keeps working — it just stops checking. `fleet_spawn` from a worker would succeed. ## This is the same shape as #670, and that is the point Two instances in one file, found one day apart, both with the identical signature: a production call site supplies a constant that encodes a real decision, and the only test covering that parameter supplies its own value instead. `FleetMcp`'s own javadoc at `:354-361` even explains that `UNENFORCED` exists for the pre-CB-513 test suite — so the dangerous value is a legitimate, reachable constant, not something a typo would have to invent. Worth sweeping `FleetdAssembly` for the rest of the family rather than fixing these two and stopping. ## Acceptance criteria 1. A test asserts that `FleetdAssembly`'s production boot path builds a `FleetMcp` with `AuthorizationMode.ENFORCED`. Mutating line 481 to `UNENFORCED` must turn that test **red**. Report the mutant output, not only the green run. 2. The test must observe the production producer. A test that constructs `FleetMcp` itself repeats the blind spot at `FleetMcpAuthzTest.java:89`. 3. Name the failing test class and method from the mutant run, and confirm no unrelated test reddened. `FleetdAssemblyLeadTabScannerExclusionTest` (merged in #671) is a working model for reaching into the assembled object graph: it drives `FleetdAssembly.assembleAndStart` with a fake `ResourcePorts` that never binds a port, and it carries a loud control assertion so it cannot pass for the wrong reason. 4. **Prefer a behavioural assertion over a field read if one is reachable.** A test that proves an unauthorized caller is actually refused through the assembled `FleetMcp` is stronger evidence than reading back the enum. Only fall back to reflection if the behavioural route needs a seam that does not exist. 5. Do not change `FleetdAssembly.java:481`. `ENFORCED` is correct. This ticket adds a test. ## Also worth doing, separately Sweep the remaining constructor arguments in `FleetdAssembly` for the same shape and report them with `file:line`, without fixing. Two confirmed instances in one file suggest the file is the pattern, not the exception.
ltms commented 2026-10-03 20:54:18 +02:00
Author
Owner
Copy Link
Copy Source

Merged as 6d97d21 (PR 673) plus b96fba4, pushed. Closing.

I re-ran the mutation myself rather than taking the worker's word for it. Everything below is a command I ran in a throwaway worktree at the PR head — never the main clone, because a build there breaks the running daemon's shutdown drain (#664).

The mutation is killed, and by exactly one test

Phase Result
Baseline Tests run: 1929, Failures: 0, Errors: 0, Skipped: 0, BUILD SUCCESS, 172 report files
Mutant — line 481 ENFORCED → UNENFORCED BUILD FAILURE, Tests run: 1929, Failures: 1
Revert line 481 back to ENFORCED

The mutation was one line in one file (git diff --numstat → 1 1). The only red class was:

[ERROR] dev.ltms.fleet.FleetdAssemblyAuthorizationModeTest.productionBootPathRefusesAnUnauthorizedCallerThroughTheAssembledFleetMcp

No other test went red, and that is the load-bearing part. An extra red would have meant the site was already pinned somewhere else and this test was only coupled to it. Zero extra reds says the call site was genuinely unobserved before, so the gap was real and this test is what closes it.

Controls I ran, because a clean zero reads exactly like good news: the baseline reported 172 surefire XML files, and the new test class appears in the report directory (2 files). Without that check, a build that never reached the test phase would have produced the same "no failures" line.

One thing I got wrong first

My first verification run reported 0 failing tests under the mutant — which looks like "the test does not work". It was my own mistake: I ran mvn at the repo root, where there is no POM, so the build died before any test ran. The POM is in fleetd/. A broken build and a surviving mutant produce the same empty failure list, which is exactly why the report-file control matters.

A follow-up commit on the comments

b96fba4. The test's javadoc carried a ticket key, a FleetdAssembly.java:481 reference, a comparison with FleetMcpAuthzTest, and pointers at two other assembly tests explaining why the test exists. Under this project's comment rule that is history and review justification: it belongs in the commit message and the PR, not in the code. The javadoc now names the behaviour the test protects. The reflection note keeps what a maintainer needs — the package boundary, and that no catch can hide a renamed denyFor — and the assertion message no longer names a line number that will drift.

The merged tree builds green with both commits: Tests run: 1929, Failures: 0, BUILD SUCCESS.

The sweep is real work and is not lost

The worker found three more unpinned constructor arguments in FleetdAssembly and fixed none, as instructed. Filed separately rather than left in this ticket's comments.

## Merged as `6d97d21` (PR 673) plus `b96fba4`, pushed. Closing. I re-ran the mutation myself rather than taking the worker's word for it. Everything below is a command I ran in a throwaway worktree at the PR head — never the main clone, because a build there breaks the running daemon's shutdown drain (#664). ### The mutation is killed, and by exactly one test | Phase | Result | |---|---| | Baseline | `Tests run: 1929, Failures: 0, Errors: 0, Skipped: 0`, `BUILD SUCCESS`, 172 report files | | Mutant — line 481 `ENFORCED` → `UNENFORCED` | `BUILD FAILURE`, `Tests run: 1929, Failures: 1` | | Revert | line 481 back to `ENFORCED` | The mutation was one line in one file (`git diff --numstat` → `1 1`). The only red class was: ``` [ERROR] dev.ltms.fleet.FleetdAssemblyAuthorizationModeTest.productionBootPathRefusesAnUnauthorizedCallerThroughTheAssembledFleetMcp ``` **No other test went red, and that is the load-bearing part.** An extra red would have meant the site was already pinned somewhere else and this test was only coupled to it. Zero extra reds says the call site was genuinely unobserved before, so the gap was real and this test is what closes it. Controls I ran, because a clean zero reads exactly like good news: the baseline reported 172 surefire XML files, and the new test class appears in the report directory (2 files). Without that check, a build that never reached the test phase would have produced the same "no failures" line. ### One thing I got wrong first My first verification run reported `0` failing tests under the mutant — which looks like "the test does not work". It was my own mistake: I ran `mvn` at the repo root, where there is no POM, so the build died before any test ran. The POM is in `fleetd/`. A broken build and a surviving mutant produce the same empty failure list, which is exactly why the report-file control matters. ### A follow-up commit on the comments `b96fba4`. The test's javadoc carried a ticket key, a `FleetdAssembly.java:481` reference, a comparison with `FleetMcpAuthzTest`, and pointers at two other assembly tests explaining why the test exists. Under this project's comment rule that is history and review justification: it belongs in the commit message and the PR, not in the code. The javadoc now names the behaviour the test protects. The reflection note keeps what a maintainer needs — the package boundary, and that no `catch` can hide a renamed `denyFor` — and the assertion message no longer names a line number that will drift. The merged tree builds green with both commits: `Tests run: 1929, Failures: 0`, `BUILD SUCCESS`. ### The sweep is real work and is not lost The worker found three more unpinned constructor arguments in `FleetdAssembly` and fixed none, as instructed. Filed separately rather than left in this ticket's comments.
ltms closed this issue 2026-10-03 20:54:40 +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#672
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?