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

The subscription guard's call site is unpinned: deleting assertPrimaryClean from Fleetd.main leaves the whole suite green #625

New Issue
Open
opened 2026-09-22 06:45:51 +02:00 by ltms · 0 comments
ltms commented 2026-09-22 06:45:51 +02:00
Owner
Copy Link
Copy Source

The defect

Fleetd.java:168 is the one place the subscription guard actually runs at startup:

guard.assertPrimaryClean(System.getenv());

Nothing pins that call site. I deleted the line and ran the full suite:

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

It compiled clean and every test passed. Then I restored it and confirmed the tree was clean and the anchor count was back to 1.

This is a surviving mutant, and it is not an equivalent mutant — removing that line genuinely changes behaviour. The daemon would stop refusing to boot with ANTHROPIC_BASE_URL set. It simply has no test.

Why it matters more than an ordinary coverage gap

This is the check behind invariant 1 of the bridge charter: never set, export, or forward ANTHROPIC_BASE_URL or ANTHROPIC_AUTH_TOKEN. The guard is what stops the primary being moved off the operator's subscription. A guard whose call site nothing pins can be deleted by an unrelated refactor, and the build will say everything is fine.

fleetd #612 has just spent a whole unit on exactly this shape — a call that runs before the boundary a test can reach, so deleting it is invisible. This is the same shape, on the most security-relevant call in main.

Why it is not covered today, and what the real fix is

SubscriptionGuardTest does test the method, but only by handing it a map directly:

SubscriptionGuardTest.java:43   () -> guard.assertPrimaryClean(Map.of("ANTHROPIC_BASE_URL", "http://gx00.gw:8000"))
SubscriptionGuardTest.java:49   assertDoesNotThrow(() -> guard.assertPrimaryClean(Map.of("PATH", "/usr/bin")))
SubscriptionGuardTest.java:50   assertDoesNotThrow(() -> guard.assertPrimaryClean(Map.of("ANTHROPIC_BASE_URL", "")))

That proves the method. It says nothing about the call site, which is the thing that can be deleted.

The structural reason no test drives it is that the call site passes System.getenv() — the real process environment, which a test cannot taint from inside the JVM. So this cannot be fixed by writing a test against the code as it stands; the call site needs an injectable environment seam first.

FleetdAssembly already has exactly that seam: it reads the environment through ports.environment(). assertPrimaryClean runs at line 168, before cfg.validateAll(), so it is currently outside the assembly boundary that fleetd #612 just moved.

Do not simply move it inside the assembly to make it testable. It runs before validation on purpose — the guard should refuse a tainted environment before anything else happens, including config validation. Any fix has to keep that ordering. Routing the environment read through an injectable supplier, while leaving the call where it is, is the shape that preserves both properties.

A useful negative result from the same sweep

The five report calls immediately before it — reportRequiredSecrets, reportGitHostShape, reportMemberTrustModel, reportMemberCredentialsGap, reportExhaustedPatternGap — are not in this category. FleetdStartupReportTest already drives Fleetd.main end to end and asserts each one's log line, so their call sites are pinned. The gap is specific to assertPrimaryClean.

Note also that FleetdStartupValidationTest does drive the real Fleetd.main, and its file does contain the string ANTHROPIC_BASE_URL — but at lines 82-83 that string is inside a YAML config fixture for the member env allowlist, not the process environment. It is easy to read that file and wrongly conclude this call site is already covered.

Acceptance

  • The call site is pinned: deleting guard.assertPrimaryClean(...) from Fleetd.main must make a named test fail.
  • Prove it the way #612 requires — delete the call, show the named test red, restore, show green. Paste both.
  • The guard still runs before cfg.validateAll() and before any socket, broker or HTTP work. A test must pin the ordering too, not just the presence, or the fix trades one invisible deletion for an invisible reordering.
  • SubscriptionGuardTest's existing direct-call tests stay. They are not redundant — they pin the method's behaviour, which is a different claim from the call site running.

Provenance

Found by a dev member briefed on fleetd #612 A-gaps to look for the same shape and report it without fixing it. It correctly reported and did not touch it. I verified the claim myself with the mutation above before filing.

## The defect `Fleetd.java:168` is the one place the subscription guard actually runs at startup: ```java guard.assertPrimaryClean(System.getenv()); ``` **Nothing pins that call site.** I deleted the line and ran the full suite: ``` Tests run: 1879, Failures: 0, Errors: 0, Skipped: 0 BUILD SUCCESS ``` It compiled clean and every test passed. Then I restored it and confirmed the tree was clean and the anchor count was back to 1. This is a surviving mutant, and it is not an equivalent mutant — removing that line genuinely changes behaviour. The daemon would stop refusing to boot with `ANTHROPIC_BASE_URL` set. It simply has no test. ## Why it matters more than an ordinary coverage gap This is the check behind invariant 1 of the bridge charter: *never set, export, or forward `ANTHROPIC_BASE_URL` or `ANTHROPIC_AUTH_TOKEN`*. The guard is what stops the primary being moved off the operator's subscription. A guard whose call site nothing pins can be deleted by an unrelated refactor, and the build will say everything is fine. fleetd #612 has just spent a whole unit on exactly this shape — a call that runs before the boundary a test can reach, so deleting it is invisible. This is the same shape, on the most security-relevant call in `main`. ## Why it is not covered today, and what the real fix is `SubscriptionGuardTest` does test the method, but only by handing it a map directly: ```java SubscriptionGuardTest.java:43 () -> guard.assertPrimaryClean(Map.of("ANTHROPIC_BASE_URL", "http://gx00.gw:8000")) SubscriptionGuardTest.java:49 assertDoesNotThrow(() -> guard.assertPrimaryClean(Map.of("PATH", "/usr/bin"))) SubscriptionGuardTest.java:50 assertDoesNotThrow(() -> guard.assertPrimaryClean(Map.of("ANTHROPIC_BASE_URL", ""))) ``` That proves the **method**. It says nothing about the **call site**, which is the thing that can be deleted. The structural reason no test drives it is that the call site passes `System.getenv()` — the real process environment, which a test cannot taint from inside the JVM. So this cannot be fixed by writing a test against the code as it stands; the call site needs an injectable environment seam first. `FleetdAssembly` already has exactly that seam: it reads the environment through `ports.environment()`. `assertPrimaryClean` runs at line 168, before `cfg.validateAll()`, so it is currently outside the assembly boundary that fleetd #612 just moved. **Do not simply move it inside the assembly to make it testable.** It runs before validation on purpose — the guard should refuse a tainted environment before anything else happens, including config validation. Any fix has to keep that ordering. Routing the environment read through an injectable supplier, while leaving the call where it is, is the shape that preserves both properties. ## A useful negative result from the same sweep The five report calls immediately before it — `reportRequiredSecrets`, `reportGitHostShape`, `reportMemberTrustModel`, `reportMemberCredentialsGap`, `reportExhaustedPatternGap` — are **not** in this category. `FleetdStartupReportTest` already drives `Fleetd.main` end to end and asserts each one's log line, so their call sites are pinned. The gap is specific to `assertPrimaryClean`. Note also that `FleetdStartupValidationTest` does drive the real `Fleetd.main`, and its file does contain the string `ANTHROPIC_BASE_URL` — but at lines 82-83 that string is inside a YAML config fixture for the member env allowlist, not the process environment. It is easy to read that file and wrongly conclude this call site is already covered. ## Acceptance - The call site is pinned: deleting `guard.assertPrimaryClean(...)` from `Fleetd.main` must make a named test fail. - Prove it the way #612 requires — delete the call, show the named test red, restore, show green. Paste both. - The guard still runs **before** `cfg.validateAll()` and before any socket, broker or HTTP work. A test must pin the ordering too, not just the presence, or the fix trades one invisible deletion for an invisible reordering. - `SubscriptionGuardTest`'s existing direct-call tests stay. They are not redundant — they pin the method's behaviour, which is a different claim from the call site running. ## Provenance Found by a `dev` member briefed on fleetd #612 A-gaps to look for the same shape and report it without fixing it. It correctly reported and did not touch it. I verified the claim myself with the mutation above before filing.
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#625
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?