Compare commits

...

12 Commits

Author SHA1 Message Date
Dai Ha af4c88d54b t398: correct two false statements in FleetConfigValidateAllTest
CI / contract (pull_request) Successful in 46s
CI / build (pull_request) Successful in 1m58s
Measured at review: reverting validateAll() to a hardcoded list of
today's six calls leaves the suite green (1491 tests, 0 failures). The
class javadoc claimed that mutation fails a test. It does not — claim 1
pins the generic helper on an unrelated class, claim 2 pins today's six,
and a hardcoded list satisfies both.

The interaction was the real hazard. The denominator assertion IS a
tripwire (declaring a seventh validator fails it), but its failure
message said the sweep reaches new validators 'by construction' and told
the author to just update the expected set. If the sweep were ever
replaced by a name list, the one assertion that fires would hand back a
false all-clear at the moment it fired.

Javadoc now states the measurement, and names the denominator test as
the actual guarantee. The assertion message now says to confirm
validateAll() still delegates to invokeAllValidators(this) BEFORE
updating the expected set.
2026-09-10 09:01:49 +07:00
Dai Ha b540a1744b fleetd #398 follow-up: pin the startup validators with a reflective validateAll()
CI / contract (pull_request) Successful in 1m9s
CI / build (pull_request) Successful in 1m28s
Mutation testing found that deleting a cfg.validateXxx() call from
Fleetd.main left the full suite green: every test called a validator
directly and none exercised main as the caller.

FleetConfig.validateAll() sweeps this class's own public no-arg void
validateXxx() methods by reflection and invokes each in alphabetical
order, so a newly written validator is wired into both callers
(Fleetd.main and ConfigRef.reload) with no second step to forget.
FleetdStartupValidationTest calls the real Fleetd.main with six configs,
each failing exactly one validator.

Recovered by the lead: the worker's agent died mid-turn with this work
uncommitted, and had left the startup call commented out as
'// MUTATION-TEST-TEMP: cfg.validateAll();' from its own mutation run.
I restored the call before committing. Build after restoring:
Tests run: 1491, Failures: 0, BUILD SUCCESS.

NOT covered, and not claimed to be: the five log-only reporters in
main (reportRequiredSecrets, reportGitHostShape, reportMemberTrustModel,
reportMemberCredentialsGap, and reportExhaustedPatternGap on current
main) are not validateXxx() methods, so the sweep does not reach them
and their call sites stay unpinned.
2026-09-10 08:56:20 +07:00
Dai Ha e7b33fe3a0 fleetd: central allow-list of usable models (models: + validateModels())
CI / contract (pull_request) Successful in 48s
CI / build (pull_request) Successful in 1m34s
Add an optional top-level `models:` block (Models{allow: List<ModelEntry>})
naming the models any profiles: entry may use. Absent/empty allow: keeps
today's behaviour exactly (no check, no warning). When configured,
FleetConfig.validateModels() fails config load (and reload, via ConfigRef)
naming both the model and the profile, if any profile's model: is outside
the list. The check is one-way: editing profiles: alone can never widen
what is permitted, only models.allow: can.

Wired into Fleetd.main() alongside the other validateXxx() calls, and into
ConfigRef.reload()/DEFERRED_KEYS so a bad edit can't slip in through a
reload either. Each ModelEntry is its own record (not a bare string) so a
later unit can add per-model on/off or load-limit state without changing
the YAML shape. One flat string namespace covers both a bare Claude id and
an opencode provider-prefixed id.
2026-09-10 08:09:27 +07:00
Dai Ha 799014e99d Merge #388: scrub a pane shell that is neither login nor interactive
CI / contract (push) Successful in 1m8s
CI / build (push) Successful in 1m38s
2026-09-10 07:21:48 +07:00
Dai Ha 69e09b10fa #388: scrub a pane shell that is neither login nor interactive
CI / contract (pull_request) Successful in 49s
CI / build (pull_request) Successful in 1m32s
EnvAllowListScrub generated four zsh startup files but only .zshrc and
.zlogin sourced the scrub — .zshenv (the one file zsh always reads) did
not. A pane shell that is neither login nor interactive reads only
.zshenv and stops, so it was never scrubbed at all (measured on fleet01,
issue #388).

Adding an unguarded scrub to .zshenv (the ticket's own suggested fix) is
wrong: .zshenv is read by every zsh, including a short-lived `zsh -c`
a member's own tooling forks for a single command. Those children are
also neither login nor interactive, so they would scrub the environment
their parent deliberately set for them (GIT_DIR, VIRTUAL_ENV, ...), and
the rewritten scrub-report.txt would describe the last child to exit
instead of the pane.

Fix (per comment 15387, measured): keep .zshrc/.zlogin unconditional,
and add to .zshenv a pass guarded on the exact condition that defines
the gap (neither login nor interactive), plus a per-pane sentinel
(_CB633_SCRUBBED) so it runs once per pane, not once per process. The
sentinel is exported only after the scrub runs, and is folded into the
scrub's own allow-list so a later pass in the same pane cannot blank it
back to empty.

Also corrects the class javadoc's wrong premise (a bare argv[0] proves
NOT login, not "therefore interactive") and its now-stale two-file
walkthrough.

Tests: two new real-zsh tests in EnvAllowListScrubTest run actual
non-login/non-interactive zsh processes (never string-match the
generated files) to prove: a neither-shell pane is scrubbed; a child
that pane forks keeps variables the pane deliberately set for it; the
child does not re-scrub; and scrub-report.txt still describes the pane
after the child exits. Both fail without the production fix (verified
by reverting it and re-running: AssertionFailedError on the sentinel
and on the decoy secret surviving).
2026-09-10 07:21:08 +07:00
Dai Ha b9d09e044e t386: pin the per-member drift baseline the fix's own tests left open
CI / contract (push) Successful in 37s
CI / build (push) Successful in 1m57s
The two tests merged with #386 both start with the member already BUSY, so a
single global drift baseline passes them. This one sleeps the host while nothing
is busy and only then starts a turn, which fails without the per-member map.
2026-09-10 06:59:42 +07:00
Dai Ha fd8650cda4 Merge #386: correct the stall check for a monotonic clock frozen by host sleep 2026-09-10 06:56:37 +07:00
Dai Ha 11050e24ed t384: fix javadoc indentation on the merged shareWithGroup lines
CI / contract (push) Successful in 1m7s
CI / build (push) Successful in 1m29s
2026-09-10 06:54:51 +07:00
Dai Ha 6f71f40047 Merge #384: pre-create the scrub receipt and give it group write 2026-09-10 06:49:46 +07:00
Dai Ha fb36c5238f Merge #382: SpawnRequest.withProfile() replaces the six-accessor rebuild 2026-09-10 06:49:46 +07:00
Dai Ha cc9cdc938b #384: write shared scrub receipts 2026-09-10 06:45:30 +07:00
Dai Ha 5fede82468 #382: give SpawnRequest a withProfile wither, guard it against the arity trap
CI / contract (pull_request) Successful in 44s
CI / build (pull_request) Successful in 1m49s
CompositePeerLauncher:372 rebuilt a routed SpawnRequest from a literal
new SpawnRequest(...) call listing six of the original request's own
accessors. That call is only correct because it happens to match the
canonical 6-arg constructor today; add a 7th component plus the
established back-compat constructor at the old (now-shorter) arity and
this call would silently rebind to it, dropping the new field on every
profile-routed spawn with no compile error — the same defect shape
already guarded on FleetConfig.withDefaults() (#357) and MemberSession
(#358).

Add SpawnRequest.withProfile(String), modeled on
MemberSession.withState/withActivity, and use it at the call site
instead. Add a guard test that resolves the true canonical constructor
by exact component types (never by argument count), gives every
component a distinctive value, and asserts every component but
profileName survives withProfile() unchanged.

Proved the guard against the real mechanism: temporarily dropped the
last (role) argument from withProfile()'s constructor call so it bound
to the 5-arg back-compat constructor — it still compiled, and the new
test failed, catching the silently-defaulted role. Restored the fix
and reconfirmed green.
2026-09-10 06:44:15 +07:00
15 changed files with 1457 additions and 59 deletions
+29
View File
@@ -877,3 +877,32 @@ guard:
# terminal: term_65619bd6174568
# pushReminders: 5
# pushBackoffMs: 15000
# Central allow-list of models any profiles: entry may name. Nothing checked a profile's model:
# value before this block existed — it was a free-form string handed straight to the backend
# adapter, and a withdrawn or misspelled name failed silently instead of at config load (opencode
# falls back to a default model rather than erroring on an unknown -m).
#
# Absent, or present with an empty allow:, is OFF: no profile's model: is checked, exactly like
# before this block existed. fleetd.yaml is gitignored on every host, so an upgrade must not force
# every operator to enumerate their models before the daemon will start.
#
# The list is the authority; profiles: is checked against it, never the reverse — adding or
# editing a profiles: entry cannot, by itself, widen what is permitted here.
#
# Enforcement is at CONFIG LOAD only (a bad model: fails the daemon at startup, naming both the
# model and the profile). There is no spawn-time enforcement, no runtime on/off switch, and no
# interaction with BackendQuarantine — those are separate, later units.
#
# allow → the permitted models. Each entry is its own block (not a bare string) so a later unit
# can add an on/off state or a load limit per model without changing this shape.
# model → the model id exactly as a profiles: entry's model: field would write it. One flat,
# opaque-string namespace: a bare Claude id (claude-sonnet-5) and an opencode
# provider-prefixed id (openai/gpt-5.6-terra) both fit here unchanged — the check is a
# plain string match, never a parse of the provider prefix or a branch on kind:.
# models:
# allow:
# - model: claude-sonnet-5
# - model: claude-opus-5
# - model: openai/gpt-5.6-terra
# - model: amazon.nova-pro-v1:0
+10 -13
View File
@@ -144,19 +144,16 @@ public final class Fleetd {
SubscriptionGuard guard = new SubscriptionGuard(cfg.guard().hostSet());
guard.assertPrimaryClean(System.getenv());
// CB-501: refuse to start if the bind is wider than the auth mode can defend. Under
// loopback-trust, "not a known worker" means "the primary" — sound only because the OS
// refuses remote connections to a loopback socket. This throws rather than warns so the
// dangerous configuration cannot be reached by ignoring a log line.
cfg.validateAuthExposure();
cfg.validateLeadTabPrefixes();
// CB-542: a subscription:true profile whose env: reseats ANTHROPIC_BASE_URL/AUTH_TOKEN would
// reach an unguarded endpoint (the launcher skips SubscriptionGuard for it). Refuse at load.
cfg.validateSubscriptionProfiles();
cfg.validateCharters();
// CB-548: every architect slot must name a configured workers: profile — the strong-model
// backend the future spawn lifecycle would read. A stale reference dies here, not later.
cfg.validateMembers();
// Every FleetConfig.validateXxx() the operator's config can fail — CB-501's auth-exposure
// check, CB-531's lead-tab-prefix check, CB-542's subscription-profile check, the charter
// and member-slot checks, and the "central allow-list of usable models" check — must run
// here, at load, before anything below opens a socket or spawns a member. fleetd ticket
// "central allow-list of usable models" follow-up: mutation testing found six individual
// calls here with nothing proving any of them still ran (deleting one left the full suite
// green). validateAll() replaces them with the one call that FleetConfigValidateAllTest
// and the Fleetd-startup tests actually pin — see FleetConfig#validateAll's javadoc for
// why a name-by-name list here would have the same defect it replaces.
cfg.validateAll();
Path socket = cfg.herdrSocket() != null && !cfg.herdrSocket().isBlank()
? Path.of(cfg.herdrSocket())
@@ -43,6 +43,12 @@ import java.util.function.Supplier;
* running daemon keeps whatever this was at startup regardless of a later edit),
* {@code spawnReadyTimeoutMs} / {@code spawnReadyPollMs}, {@code quarantineCooldownSeconds}
* (CB-578 stage B — baked once into the {@code BackendQuarantine} built at startup),
* {@code models:} (fleetd ticket "central allow-list of usable models" — {@link
* FleetConfig#validateModels()} re-runs against the fresh config in {@link #reload()}
* (via {@link FleetConfig#validateAll()}), so a
* models.allow: edit that would refuse to boot still refuses the reload; a change that
* passes has nothing built at startup to rebuild, so it is reported deferred rather than
* silently accepted with no report at all),
* {@code guard:}, {@code worktreeRoot:}, {@code worktreeGroup:} and {@code memberSkills:}
* (all three of the latter baked once into the {@code GitWorktrees} built at
* {@code Fleetd.java:251} and never rebuilt — fleetd #323 instance 2 found
@@ -135,8 +141,9 @@ import java.util.function.Supplier;
* </ul>
*
* <p><strong>The denominator, measured on 2026-09-04 (fleetd #330; recounted for fleetd #333);
* recounted again for fleetd #362, and again after {@code idleSleepGuard:} was added.</strong>
* {@code FleetConfig} has 24 top-level record components: 5 cold, 13 deferred, 3 split, 3
* recounted again for fleetd #362, again after {@code idleSleepGuard:} was added, and again after
* {@code models:} was added.</strong>
* {@code FleetConfig} has 25 top-level record components: 5 cold, 14 deferred, 3 split, 3
* hot-excluded. Three of them are named nowhere in this file, and the reason is the same for all
* three: {@code placement}, {@code memberCredentials} and {@code memberLoginShell} are
* <strong>hot</strong> and correctly absent — all three are read live off {@code config.get()}
@@ -219,7 +226,7 @@ public final class ConfigRef implements Supplier<FleetConfig> {
static final Set<String> DEFERRED_KEYS = Set.of(
"guard", "worktreeRoot", "worktreeGroup", "memberSkills", "primary", "configReload",
"leadHeartbeat", "lifecycle", "spawnReadyTimeoutMs", "spawnReadyPollMs",
"quarantineCooldownSeconds", "profiles", "idleSleepGuard");
"quarantineCooldownSeconds", "profiles", "idleSleepGuard", "models");
private final Path path;
private final AtomicReference<FleetConfig> current;
@@ -322,12 +329,12 @@ public final class ConfigRef implements Supplier<FleetConfig> {
fresh = FleetConfig.load(path);
// The same gate startup runs. A config that would have refused to boot must not be able
// to slip in through a reload — that is how a daemon ends up in a state it could never
// have started in, which is the hardest kind to debug.
fresh.validateAuthExposure();
fresh.validateLeadTabPrefixes();
fresh.validateSubscriptionProfiles();
fresh.validateCharters();
fresh.validateMembers();
// have started in, which is the hardest kind to debug. fleetd ticket "central allow-list
// of usable models" follow-up: this used to be six individual validateXxx() calls, and
// mutation testing found two of the six unpinned here even though startup pinned nothing
// at all — see FleetConfig#validateAll's javadoc for why the fix is one reflective call,
// not a longer hand-maintained list.
fresh.validateAll();
} catch (RuntimeException e) {
String msg = e.getMessage() == null ? e.toString() : e.getMessage();
log.warn("config reload from {} refused, keeping the running config: {}", path, msg);
@@ -439,6 +446,15 @@ public final class ConfigRef implements Supplier<FleetConfig> {
if (!Objects.equals(old.idleSleepGuard(), fresh.idleSleepGuard())) {
changed.add("idleSleepGuard");
}
// fleetd ticket "central allow-list of usable models": validateModels() runs again in
// reload() above (via validateAll()), so a bad edit is already refused as cold-adjacent
// (the whole reload is refused via the catch block, never partially applied). A GOOD edit
// to the allow-list
// itself has nothing built at startup to rebuild — it only ever mattered to the validation
// call that already ran — so report it deferred rather than silently swallowing the change.
if (!Objects.equals(old.models(), fresh.models())) {
changed.add("models");
}
if (!Objects.equals(old.spawnReadyTimeoutMs(), fresh.spawnReadyTimeoutMs())
|| !Objects.equals(old.spawnReadyPollMs(), fresh.spawnReadyPollMs())) {
changed.add("spawnReady*");
@@ -16,11 +16,15 @@ import org.slf4j.LoggerFactory;
import java.io.IOException;
import java.io.UncheckedIOException;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.lang.reflect.Modifier;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.Comparator;
import java.util.HashSet;
import java.util.LinkedHashMap;
import java.util.List;
@@ -124,6 +128,13 @@ import java.util.regex.PatternSyntaxException;
* under a member's long turn. {@code null} (the block omitted) behaves the
* same as an explicit {@code enabled: true}; set {@code enabled: false} to
* turn it off. See {@link dev.ltms.fleet.power.IdleSleepGuard}.
* @param models central allow-list of models any {@code profiles:} entry may name. {@code
* null} or an empty {@code allow:} ⇒ off: {@link #validateModels()} checks
* nothing and every existing config keeps working exactly as it does today.
* When non-empty, a profile whose {@code model:} is not one of {@link
* Models#ids()} fails config load, naming both the model and the profile —
* see {@link #validateModels()}. This block only decides what may be
* CONFIGURED; nothing here enforces it at spawn time. See {@link Models}.
*/
@JsonIgnoreProperties(ignoreUnknown = true)
public record FleetConfig(
@@ -150,7 +161,22 @@ public record FleetConfig(
String worktreeGroup,
String memberLoginShell,
String memberSkills,
IdleSleepGuard idleSleepGuard) {
IdleSleepGuard idleSleepGuard,
Models models) {
/** Back-compat form before the {@code models:} block was added. */
public FleetConfig(Bind bind, String herdrSocket, String memberHerdrSocket, Map<String, Profile> profiles,
Guard guard, String worktreeRoot, Lifecycle lifecycle, Integer spawnReadyTimeoutMs,
Integer spawnReadyPollMs, Broker broker, Primary primary, Fleet fleet,
LeadHeartbeat leadHeartbeat, Health health, String placement, Auth auth,
ConfigReload configReload, Integer quarantineCooldownSeconds,
MemberCredentials memberCredentials, Coordinator coordinator, String worktreeGroup,
String memberLoginShell, String memberSkills, IdleSleepGuard idleSleepGuard) {
this(bind, herdrSocket, memberHerdrSocket, profiles, guard, worktreeRoot, lifecycle, spawnReadyTimeoutMs,
spawnReadyPollMs, broker, primary, fleet, leadHeartbeat, health, placement, auth,
configReload, quarantineCooldownSeconds, memberCredentials, coordinator, worktreeGroup,
memberLoginShell, memberSkills, idleSleepGuard, null);
}
/** Back-compat form before the {@code idleSleepGuard:} block was added. */
public FleetConfig(Bind bind, String herdrSocket, String memberHerdrSocket, Map<String, Profile> profiles,
@@ -1318,6 +1344,72 @@ public record FleetConfig(
}
}
/**
* Central allow-list of models any {@code profiles:} entry may name (fleetd ticket: "a central
* allow-list of usable models"). Nothing before this block checked a profile's {@code model:}
* against anything — it was a free-form string handed straight to the backend adapter, and a
* withdrawn or misspelled name failed silently (opencode falls back to a default model rather
* than erroring on an unknown {@code -m}) rather than at config load, where a mistake is cheap.
*
* <p><b>Absent or empty {@code allow:} is "off"</b>, on purpose: this is a large deployment
* with gitignored {@code fleetd.yaml} on more than one host, and a change that forced every
* operator to enumerate their models before the daemon would start would break every one of
* them on upgrade. See {@link FleetConfig#validateModels()}, which is where the allow-list is
* actually enforced, at config load.
*
* <p><b>The list is the authority; a {@code profiles:} entry is checked against it, never the
* other way around.</b> Adding or editing a {@code profiles:} entry cannot, by itself, widen
* the set of permitted models — only editing {@code models.allow:} itself can. This is the
* invariant the ticket asked for: the two blocks are validated in one direction only.
*
* <p><b>Out of scope here, deliberately:</b> nothing in this block is read at spawn time —
* enforcing it against a live spawn, an on/off runtime switch, and any interaction with {@code
* BackendQuarantine} are separate units. This block is config-load validation only.
*
* @param allow the permitted models, each its own {@link ModelEntry} rather than a bare
* string — see that record's javadoc for why. {@code null}/empty ⇒ the block is
* treated as absent: {@link FleetConfig#validateModels()} checks nothing.
*/
@JsonIgnoreProperties(ignoreUnknown = true)
public record Models(List<ModelEntry> allow) {
public Models {
allow = (allow == null) ? List.of() : List.copyOf(allow);
}
/**
* One permitted model, named as a record rather than a bare string on purpose: a later unit
* needs to hang an on/off state and a load-limit state off each entry, and a bare {@code
* List<String>} cannot grow those fields without changing the YAML shape underneath every
* operator who already wrote one. {@link #model()} is intentionally a single flat,
* opaque-string namespace — a bare Claude id ({@code claude-sonnet-5}) and an opencode
* provider-prefixed id ({@code openai/gpt-5.6-terra}) both fit it unchanged, because
* {@link FleetConfig#validateModels()} only ever compares a profile's {@code model:} value
* against this string for exact equality; it never parses a provider prefix or branches on
* a profile's {@code kind:}.
*
* @param model the model id exactly as a {@code profiles:} entry's {@code model:} field
* would name it
*/
@JsonIgnoreProperties(ignoreUnknown = true)
public record ModelEntry(String model) {
public ModelEntry {
model = (model == null || model.isBlank()) ? null : model.trim();
}
}
/** {@link #allow}'s model ids, as a set for membership checks. Blank/null entries are dropped. */
public Set<String> ids() {
Set<String> ids = new java.util.LinkedHashSet<>();
for (ModelEntry e : allow) {
if (e != null && e.model() != null) {
ids.add(e.model());
}
}
return Collections.unmodifiableSet(ids);
}
}
/**
* The terminal → lead-name map seeded from the legacy singular {@code primary:} pin (CB-530).
*
@@ -1569,7 +1661,7 @@ public record FleetConfig(
"lifecycle", "spawnReadyTimeoutMs", "spawnReadyPollMs", "broker", "primary", "fleet",
"leadHeartbeat", "health", "placement", "auth", "configReload", "quarantineCooldownSeconds",
"memberCredentials", "coordinator", "worktreeGroup", "memberLoginShell", "memberSkills",
"idleSleepGuard");
"idleSleepGuard", "models");
/** Load and validate config from {@code path}. */
public static FleetConfig load(Path path) {
@@ -2254,10 +2346,15 @@ public record FleetConfig(
// enabled: true — see its javadoc), so defaulting the block here would change nothing a
// reader observes and would only obscure that "block omitted" and "block present and
// enabled" are deliberately the same outcome.
// models is left as-is, like broker/primary/coordinator above: null/empty is "off", and an
// absent block must validate nothing (see Models's javadoc) — defaulting it here to an
// empty Models would be a no-op for validateModels() either way, since an empty allow-list
// already means "check nothing", so there is nothing to gain and one more null check to
// avoid by leaving it exactly as configured.
return new FleetConfig(b, herdrSocket, memberHerdrSocket, profiles, g, worktreeRoot, l, timeout, pollMs,
broker, primary, f, leadHeartbeat, health, placementOrDefault, a, configReload,
quarantineCooldown, mc, coordinator, worktreeGroup, memberLoginShell, memberSkills,
idleSleepGuard);
idleSleepGuard, models);
}
/**
@@ -2477,6 +2574,118 @@ public record FleetConfig(
}
}
/**
* Reject a {@code profiles:} entry whose {@code model:} is not on the configured {@link
* #models} allow-list.
*
* <p>Absent or empty {@code models.allow:} validates nothing — see {@link Models}'s javadoc:
* an existing config with no such block must keep working exactly as it does today. Once the
* operator declares at least one entry, every profile's {@code model:} (when set — a profile
* may legitimately leave it {@code null}, e.g. a {@code subscription: true} profile relying on
* the account's own default) must equal one of {@link Models#ids()} exactly. The comparison is
* a flat string match: a bare Claude id and an opencode {@code provider/model} id are both
* just opaque strings here, so nothing here needs to know which {@code kind:} a profile runs.
*
* <p>The check runs one direction only, by construction: it reads {@link #profiles} and
* {@link #models}, and only ever adds to {@code bad} when a profile's model is missing from
* the allow-list. Nothing here can be satisfied by widening a {@code profiles:} entry — only
* editing {@code models.allow:} itself changes what passes. That is the invariant the ticket
* asked for: the list is the authority, profiles are checked against it.
*
* @throws IllegalStateException when any profile names a model outside the configured
* allow-list, naming both the model and the profile that wanted it
*/
public void validateModels() {
if (models == null || models.allow().isEmpty()) {
return;
}
Set<String> allowed = models.ids();
List<String> bad = new ArrayList<>();
profiles.forEach((name, p) -> {
String model = p.model();
if (model != null && !model.isBlank() && !allowed.contains(model)) {
bad.add("profile '" + name + "' names model '" + model + "', which is not in "
+ "models.allow: (have: " + allowed + ").");
}
});
if (!bad.isEmpty()) {
throw new IllegalStateException("refusing to start: " + String.join(" ", bad));
}
}
/**
* Runs every validator this class declares — found by reflection, not by name.
*
* <p>fleetd ticket "central allow-list of usable models", follow-up: mutation testing found
* that although each of the six validators above was well pinned on its own, nothing proved
* either real caller ({@code Fleetd.main} and {@link ConfigRef#reload()}) still
* invoked it — deleting a call site left the full suite green. The fix is not a seventh test
* per caller; a hand-maintained list of six names here would have the exact same defect its
* own javadoc would warn against: the seventh validator someone adds next month has no reason
* to be added to it. So this method does not name any validator. It sweeps {@link
* #getClass()}'s own public, no-argument, {@code void} methods whose name starts with {@code
* "validate"} (excluding itself) and invokes every one it finds, via {@link
* #invokeAllValidators}. A new {@code validateXxx()} method is therefore wired into both
* callers the moment it is written — there is no second step to forget, and so no state in
* which it silently never runs.
*
* <p>{@code Fleetd.main} and {@link ConfigRef#reload()} each call this one method instead of
* the six individually — see the comments at those two call sites for why
* each must run it.
*
* <p>Methods run in a fixed (alphabetical) order, so a config with more than one violation
* always names the same one first, on every run.
*
* @throws IllegalStateException (or whatever unchecked exception a validator itself throws),
* propagated unchanged from the first validator, in that order,
* that finds a problem
*/
public void validateAll() {
invokeAllValidators(this);
}
/**
* The reflective sweep behind {@link #validateAll()}, kept as its own method — taking any
* {@code target}, not just {@code this} — so a test can prove the MECHANISM is generic (it
* would sweep a seventh {@code validateXxx()} method added to any class, not just something
* special-cased to today's six on {@link FleetConfig}) without needing to add a real, unwanted
* seventh validator to this class just to exercise that claim. See {@code
* FleetConfigValidateAllTest} for that proof.
*
* @param target an object whose public, no-argument, {@code void} methods named {@code
* validateXxx} (any name starting with {@code "validate"}, excluding {@code
* validateAll} itself) should all run, in alphabetical-by-name order
*/
static void invokeAllValidators(Object target) {
List<Method> methods = new ArrayList<>();
for (Method m : target.getClass().getMethods()) {
if (Modifier.isPublic(m.getModifiers())
&& m.getParameterCount() == 0
&& m.getReturnType() == void.class
&& m.getName().startsWith("validate")
&& !m.getName().equals("validateAll")) {
methods.add(m);
}
}
methods.sort(Comparator.comparing(Method::getName));
for (Method m : methods) {
try {
m.invoke(target);
} catch (InvocationTargetException e) {
Throwable cause = e.getCause();
if (cause instanceof RuntimeException re) {
throw re;
}
if (cause instanceof Error err) {
throw err;
}
throw new IllegalStateException("validator " + m.getName() + " failed", cause);
} catch (IllegalAccessException e) {
throw new IllegalStateException("cannot invoke validator " + m.getName(), e);
}
}
}
/** True for the loopback addresses and the unspecified-but-local forms we treat as same-host. */
private static boolean isLoopbackBind(String host) {
if (host == null || host.isBlank()) {
@@ -369,8 +369,7 @@ public final class CompositePeerLauncher implements PeerLauncher {
// CB-547a: route the chosen profile but keep the caller's session identity — dropping it
// here would silently sever the resume handle on every policy-routed spawn. CB-557: the
// role rides along for the same reason, or a routed spawn would be labelled as a dev.
SpawnRequest routedReq = new SpawnRequest(chosen.profile(), req.requestedCwd(), req.callerCwd(),
req.sessionName(), req.resumeSessionId(), req.role());
SpawnRequest routedReq = req.withProfile(chosen.profile());
try {
PeerHandle handle = d.spawn(routedReq);
spawnedBy.put(handle.id(), d);
@@ -13,6 +13,7 @@ import java.nio.file.attribute.PosixFilePermissions;
import java.time.Duration;
import java.time.Instant;
import java.util.ArrayList;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import java.util.stream.Stream;
@@ -28,24 +29,46 @@ import java.util.stream.Stream;
* control lacked: herdr applies that overlay BEFORE the shell starts, so any sourced file can undo
* it — and did.
*
* <p><b>Which file is last depends on the platform, so the scrub runs from two of them.</b> zsh
* <p><b>zsh reads its four startup files under three different conditions, so no single file is
* guaranteed to run — the scrub has to cover the gap between them, not just the platforms.</b> zsh
* reads {@code .zshenv} always, {@code .zprofile} and {@code .zlogin} only for a LOGIN shell, and
* {@code .zshrc} only for an INTERACTIVE one. herdr does not open the same kind of shell
* everywhere — measured on herdr 0.8.0: macOS panes run {@code -zsh} (login, so {@code .zlogin}
* runs), Linux panes run a plain {@code /usr/bin/zsh} (interactive but NOT login, so
* {@code .zlogin} never runs at all). A scrub in {@code .zlogin} alone is therefore a control that
* silently does nothing on Linux — the exact failure this class exists to remove, one platform
* over.
* {@code .zshrc} only for an INTERACTIVE one. A pane shell that is at least one of login or
* interactive is covered by sourcing the scrub from {@code .zshrc} and {@code .zlogin} (below), but
* a pane shell that is NEITHER reads only {@code .zshenv} and stops — fleetd #388, measured: a herdr
* pane can be neither login nor interactive, and such a pane read {@code .zshenv}, never reached
* {@code scrub.zsh}, and left no report at all. A bare {@code argv[0]} of {@code /usr/bin/zsh}
* proves the shell is NOT a login shell; it says nothing about whether it is interactive, so it
* must never be read as "therefore interactive" — that wrong inference is what let #388 ship.
*
* <p>So both {@code .zshrc} and {@code .zlogin} source the same generated {@code scrub.zsh} after
* sourcing their {@code $HOME} counterpart. On Linux only the first fires; on macOS both do, and
* the second pass is deliberate rather than merely harmless — it re-scrubs anything the operator's
* own {@code ~/.zlogin} exported after {@code .zshrc} had finished. Re-running is idempotent: a
* name already blank is blanked again, and the report is rewritten with the same counts.
* <p>So {@code .zshenv} carries a THIRD pass, guarded by the exact condition that defines the gap:
* {@code [[ ! -o login && ! -o interactive ]]}. That guard is why this pass cannot double-scrub a
* pane that {@code .zshrc} or {@code .zlogin} will also cover — one of {@code -o login}/
* {@code -o interactive} is always true there, so the {@code .zshenv} pass never fires for them, and
* their own unconditional sourcing is untouched. The guard also carries a sentinel
* ({@value #SCRUB_SENTINEL}) so it fires once per PANE and not once per PROCESS: {@code .zshenv} is
* read by every zsh a member's own tooling forks (a plain {@code zsh -c '...'} for a single
* command is itself neither login nor interactive), and those children inherit variables their
* parent deliberately set for them (git hooks get {@code GIT_DIR}, a venv gets
* {@code VIRTUAL_ENV}, a build tool gets {@code NODE_OPTIONS} or {@code JAVA_TOOL_OPTIONS}).
* Re-scrubbing every such child would blank all of that, and would also make the pane's own
* {@code scrub-report.txt} — rewritten on every pass — describe whichever child exited last
* instead of the pane. The sentinel is exported only AFTER {@code scrub.zsh} runs, so the pass
* that sets it never sees it and cannot blank it; it must also be on the scrub's own allow-list
* (see {@link #generate(Path, Set)}) so a later pass, in the same pane, cannot blank it back to
* empty — an exported-but-empty sentinel reads as unset to the {@code -z} guard and would silently
* re-enable scrubbing for every subsequent child of that pane.
*
* <p>So all three of {@code .zshenv} (gap only, guarded), {@code .zshrc}, and {@code .zlogin}
* source the same generated {@code scrub.zsh} after sourcing their {@code $HOME} counterpart. A
* login-and-interactive pane runs the {@code .zshrc} and {@code .zlogin} passes, and the second is
* deliberate rather than merely harmless — it re-scrubs anything the operator's own
* {@code ~/.zlogin} exported after {@code .zshrc} had finished. A pane that is neither runs only the
* {@code .zshenv} pass. Re-running is idempotent: a name already blank is blanked again, and the
* report is rewritten with the same counts.
*
* <p>Each generated file sources its {@code $HOME} counterpart FIRST, so {@code PATH} and every
* toolchain binary still resolve exactly as the operator configured them; only afterwards does
* {@code .zlogin} run the scrub: every EXPORTED variable not on the derived allow-list is re-exported
* toolchain binary still resolve exactly as the operator configured them; only afterwards does the
* scrub run: every EXPORTED variable not on the derived allow-list is re-exported
* blank. Blank, not credential-shaped-pattern-filtered: a pattern list ({@code *TOKEN*}, …) is an
* enumeration and misses what it did not think of — a username is the other half of a credential and
* is shaped like none. Credential-SHAPED names among the blanked set go to the WARN log only,
@@ -63,7 +86,11 @@ public final class EnvAllowListScrub {
/** Name of the report file written into the generated directory by the scrub itself. */
static final String REPORT_FILE = "scrub-report.txt";
/** The scrub body, generated once and sourced from both {@code .zshrc} and {@code .zlogin}. */
/**
* The scrub body, generated once and sourced from {@code .zshrc} and {@code .zlogin}
* unconditionally, and from {@code .zshenv} when the pane shell is neither login nor
* interactive (fleetd #388) — see the class javadoc.
*/
static final String SCRUB_FILE = "scrub.zsh";
/** Prefix of every generated directory — also what {@link #reapOrphans} matches on. */
@@ -79,6 +106,28 @@ public final class EnvAllowListScrub {
private static final String SOURCE_SCRUB =
"source \"$ZDOTDIR/" + SCRUB_FILE + "\"\n";
/**
* fleetd #388: marks a pane, not a process, as already scrubbed. Set only by the guarded
* {@code .zshenv} pass (see {@link #NEITHER_LOGIN_NOR_INTERACTIVE_SCRUB}) after
* {@code scrub.zsh} has run, so it must also be folded into that pass's own allow-list — see
* the class javadoc's "must also be on the scrub's own allow-list" paragraph.
*/
static final String SCRUB_SENTINEL = "_CB633_SCRUBBED";
/**
* Appended to {@code .zshenv}, after its {@code $HOME} source: the third pass, guarded on the
* exact condition that defines the gap {@code .zshrc}/{@code .zlogin} do not cover — a shell
* that is neither login nor interactive. The sentinel export happens only once the scrub has
* already run, and only for as long as the current pane's environment has not been rebuilt from
* scratch (a fresh {@code env -i} child would not inherit it — that is out of scope here, since
* such a child is no longer running under the pane's own environment at all).
*/
private static final String NEITHER_LOGIN_NOR_INTERACTIVE_SCRUB =
"if [[ ! -o login && ! -o interactive && -z \"${" + SCRUB_SENTINEL + ":-}\" ]]; then\n"
+ " " + SOURCE_SCRUB
+ " export " + SCRUB_SENTINEL + "=1\n"
+ "fi\n";
private EnvAllowListScrub() {
}
@@ -105,11 +154,16 @@ public final class EnvAllowListScrub {
reapOrphans(parentDir);
Path dir = Files.createTempDirectory(parentDir, DIR_PREFIX);
dir.toFile().deleteOnExit();
// The report is written by zsh, after these hooks are registered, so register its path
// too — otherwise the directory is non-empty at JVM exit and cannot be removed at all.
dir.resolve(REPORT_FILE).toFile().deleteOnExit();
write(dir, SCRUB_FILE, scrubScript(allowedNames));
write(dir, ".zshenv", homeSourcingFile(".zshenv"));
// zsh truncates this pre-created receipt after these hooks are registered. Register its
// path too — otherwise the directory is non-empty at JVM exit and cannot be removed.
Files.createFile(dir.resolve(REPORT_FILE)).toFile().deleteOnExit();
// fleetd #388: scrub.zsh's OWN allow-list must also keep SCRUB_SENTINEL, or a later
// pass in the same pane blanks it back to empty and the .zshenv guard below thinks it
// was never scrubbed — see the class javadoc.
Set<String> namesForScrubScript = new HashSet<>(allowedNames);
namesForScrubScript.add(SCRUB_SENTINEL);
write(dir, SCRUB_FILE, scrubScript(namesForScrubScript));
write(dir, ".zshenv", homeSourcingFile(".zshenv") + NEITHER_LOGIN_NOR_INTERACTIVE_SCRUB);
write(dir, ".zprofile", homeSourcingFile(".zprofile"));
write(dir, ".zshrc", homeSourcingFile(".zshrc") + SOURCE_SCRUB);
write(dir, ".zlogin", homeSourcingFile(".zlogin") + SOURCE_SCRUB);
@@ -147,12 +201,11 @@ public final class EnvAllowListScrub {
* into it: owner keeps full access, {@code group} gets traverse+read on the directory ({@code
* rwxr-x---}, so a member process — a login shell reading it via {@code ZDOTDIR}, or another
* process simply opening a file under it — running under that group can find and read the
* files) and read-only on each file ({@code rw-r-----}) — deliberately no group WRITE anywhere,
* since a member never needs to add or change what fleetd generated. (For the ZDOTDIR scrub
* specifically, this also means the scrub script's own report write inside the pane fails
* closed rather than open — see {@code scrub.zsh}'s trailing {@code 2>/dev/null} — which {@link
* dev.ltms.fleet.member.HerdrPeerLauncher#releaseZdotdir} already treats as "cannot be
* confirmed to have run" rather than success.)
* files) and read-only on each file ({@code rw-r-----}), except the pre-created ZDOTDIR
* {@code scrub-report.txt}. That receipt gets group write ({@code rw-rw----}), so
* {@code scrub.zsh} can truncate and write it without granting group write on the directory.
* If its optional permission change fails, the member cannot write a receipt and the launcher
* keeps its existing WARN rather than failing the spawn.
*
* <p>Package-private and named generically on purpose: fleetd #213 built this for the ZDOTDIR
* scrub directory, and fleetd #219 reuses it verbatim for {@link
@@ -167,7 +220,15 @@ public final class EnvAllowListScrub {
setGroupAndPermissions(dir, principal, "rwxr-x---");
try (Stream<Path> entries = Files.list(dir)) {
for (Path file : entries.toList()) {
setGroupAndPermissions(file, principal, "rw-r-----");
if (REPORT_FILE.equals(file.getFileName().toString())) {
try {
setGroupAndPermissions(file, principal, "rw-rw----");
} catch (IOException | UnsupportedOperationException ignored) {
// The receipt is optional. Its absence keeps the existing WARN path.
}
} else {
setGroupAndPermissions(file, principal, "rw-r-----");
}
}
}
} catch (IOException e) {
@@ -245,11 +306,13 @@ public final class EnvAllowListScrub {
}
return """
# generated by fleetd (CB-633 memberCredentials policy=allow-list) — do not edit.
# Sourced from .zshrc and again from .zlogin, each time AFTER that file has sourced
# its $HOME counterpart — so this runs after everything the operator sourced, on a
# login shell (macOS panes) and on a plain interactive one (Linux panes) alike.
# Running twice is idempotent and deliberate: the second pass catches anything
# ~/.zlogin exported after ~/.zshrc had finished.
# Sourced unconditionally from .zshrc and again from .zlogin, each time AFTER that
# file has sourced its $HOME counterpart — so this runs after everything the
# operator sourced, on any pane that is login and/or interactive. Running twice is
# idempotent and deliberate: the second pass catches anything ~/.zlogin exported
# after ~/.zshrc had finished. Also sourced, once, from a guarded pass in .zshenv
# (fleetd #388) when the pane shell is NEITHER login nor interactive — the one gap
# those two files do not cover.
typeset -A _cb633_allowed
for _cb633_n in %s; do _cb633_allowed[$_cb633_n]=1; done
@@ -29,4 +29,9 @@ public record SpawnRequest(String profileName, String requestedCwd, String calle
String sessionName, String resumeSessionId) {
this(profileName, requestedCwd, callerCwd, sessionName, resumeSessionId, null);
}
/** Return a copy of this request with {@code profileName} replaced by {@code profile}. */
public SpawnRequest withProfile(String profile) {
return new SpawnRequest(profile, requestedCwd, callerCwd, sessionName, resumeSessionId, role);
}
}
@@ -0,0 +1,132 @@
package dev.ltms.fleet;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.io.TempDir;
import java.nio.file.Files;
import java.nio.file.Path;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
/**
* Proves the one thing no test proved before this ticket's follow-up: that {@code Fleetd.main}
* ITSELF — not a copy of its logic, not the validator called directly — still refuses to start on
* a bad config. Mutation testing found that deleting {@code cfg.validateAll();} (née six
* individual {@code cfg.validateXxx();} calls) from {@code Fleetd.main} left the full 1478-test
* suite green; every existing test called a validator directly and none exercised {@code
* Fleetd.main} as the caller. See {@code FleetConfigValidateAllTest} for why the fix collapses
* those six calls into one reflective {@link FleetConfig#validateAll()}, and {@code
* ConfigRefTest} for the equivalent proof on the {@link
* dev.ltms.fleet.config.ConfigRef#reload()} path.
*
* <p>This is deliberately the real {@code static void main(String[] args)} — package-private, so
* only a test in this package can call it, which is exactly what makes this proof strong: it is
* not a helper extracted for testability, it is the literal method {@code java -jar fleetd.jar}
* invokes. Every fixture below is otherwise-valid and fails exactly one validator, and — because
* {@link FleetConfig#validateAll()} runs immediately after {@code SubscriptionGuard.
* assertPrimaryClean}, before {@code main} opens the herdr socket, binds Javalin, or touches
* anything else with a real side effect — calling {@code Fleetd.main} with one of these configs is
* safe: it is guaranteed to throw before reaching any of that, precisely because the config is
* deliberately invalid.
*/
class FleetdStartupValidationTest {
private static void assertMainRefuses(Path dir, String fileName, String yaml,
String mustContain) throws Exception {
Path f = dir.resolve(fileName);
Files.writeString(f, yaml);
IllegalStateException e = assertThrows(IllegalStateException.class,
() -> Fleetd.main(new String[]{f.toString()}),
fileName + ": Fleetd.main must refuse this config before doing anything else");
assertTrue(e.getMessage().contains(mustContain),
fileName + ": expected message to contain \"" + mustContain + "\" but was: "
+ e.getMessage());
}
@Test
void mainRefusesANonLoopbackBindWithoutTokenMode(@TempDir Path dir) throws Exception {
assertMainRefuses(dir, "auth-exposure.yaml", """
bind:
host: 0.0.0.0
port: 8765
""", "auth.mode: token");
}
@Test
void mainRefusesALeadTabPrefixCollision(@TempDir Path dir) throws Exception {
assertMainRefuses(dir, "lead-tab-prefixes.yaml", """
bind:
host: 127.0.0.1
port: 8765
fleet:
tabLabel: "lead: {role} {profile}"
leaders:
opus:
tab: "lead: opus"
""", "fleet.tabLabel");
}
@Test
void mainRefusesASubscriptionProfileThatReseatsAnthropicBaseUrl(@TempDir Path dir)
throws Exception {
assertMainRefuses(dir, "subscription-profiles.yaml", """
bind:
host: 127.0.0.1
port: 8765
profiles:
sonnet:
subscription: true
argv: ["ccs", "sonnet"]
env:
ANTHROPIC_BASE_URL: http://anything-not-on-the-allowlist
""", "ANTHROPIC_BASE_URL");
}
@Test
void mainRefusesAnUnknownCharterKey(@TempDir Path dir) throws Exception {
assertMainRefuses(dir, "charters.yaml", """
bind:
host: 127.0.0.1
port: 8765
fleet:
charters:
architetc: text
""", "architetc");
}
@Test
void mainRefusesAnArchitectSlotNamingAnUnconfiguredProfile(@TempDir Path dir) throws Exception {
assertMainRefuses(dir, "members.yaml", """
bind:
host: 127.0.0.1
port: 8765
profiles:
gx10:
baseUrl: http://gx10.gw:8000
fleet:
architects:
lead-designer:
profile: sonnet
""", "lead-designer");
}
@Test
void mainRefusesAProfileNamingAModelOutsideTheAllowList(@TempDir Path dir) throws Exception {
assertMainRefuses(dir, "models.yaml", """
bind:
host: 127.0.0.1
port: 8765
profiles:
sonnet:
baseUrl: http://gx10.gw:8000
model: claude-sonnet-5
rogue:
baseUrl: http://gx11.gw:8000
model: claude-opus-9000
models:
allow:
- model: claude-sonnet-5
""", "rogue");
}
}
@@ -109,6 +109,8 @@ class ConfigRefTopLevelReportingCoverageTest {
v.put("memberLoginShell", null);
v.put("memberSkills", "/skills/a");
v.put("idleSleepGuard", new FleetConfig.IdleSleepGuard(true));
v.put("models", new FleetConfig.Models(
List.of(new FleetConfig.Models.ModelEntry("model-a"))));
assertNamesMatchComponents(v);
return v;
}
@@ -151,6 +153,8 @@ class ConfigRefTopLevelReportingCoverageTest {
v.put("memberLoginShell", null);
v.put("memberSkills", "/skills/b");
v.put("idleSleepGuard", new FleetConfig.IdleSleepGuard(false));
v.put("models", new FleetConfig.Models(
List.of(new FleetConfig.Models.ModelEntry("model-b"))));
assertNamesMatchComponents(v);
return v;
}
@@ -2683,4 +2683,222 @@ class FleetConfigTest {
assertTrue(cfg.idleSleepGuard().isEnabled(),
"unlike ConfigReload/Health, this block defaults to ON even when present but empty");
}
// --- models: central allow-list of usable models -----------------------------------------
/**
* An absent {@code models:} block is today's behaviour exactly: no profile's {@code model:} is
* checked against anything, whatever it says. This is the "existing config keeps working"
* invariant — an operator on a gitignored {@code fleetd.yaml} that predates this feature must
* not be broken by upgrading the daemon.
*/
@Test
void absentModelsBlockValidatesNothing(@TempDir Path dir) throws Exception {
Path f = dir.resolve("fleetd.yaml");
Files.writeString(f, """
bind:
port: 8080
profiles:
gx10:
baseUrl: http://gx10.gw:8000
model: totally-unheard-of-model-xyz
""");
FleetConfig cfg = FleetConfig.load(f);
assertDoesNotThrow(cfg::validateModels);
}
/**
* {@code models:} present but with an empty (or absent) {@code allow:} must behave exactly like
* the block being absent — an operator adding the block for the first time with nothing in it
* yet must not be surprised by every profile suddenly refusing to start.
*/
@Test
void modelsBlockPresentButEmptyValidatesNothing(@TempDir Path dir) throws Exception {
Path f = dir.resolve("fleetd.yaml");
Files.writeString(f, """
bind:
port: 8080
profiles:
gx10:
baseUrl: http://gx10.gw:8000
model: whatever-the-operator-typed
models:
allow: []
""");
FleetConfig cfg = FleetConfig.load(f);
assertDoesNotThrow(cfg::validateModels);
}
/**
* The core of the ticket: a profile naming a model outside the configured allow-list fails
* config load, naming both the model and the profile that wanted it.
*/
@Test
void aProfileNamingAModelOutsideTheAllowListRefusesToStart(@TempDir Path dir) throws Exception {
Path f = dir.resolve("fleetd.yaml");
Files.writeString(f, """
bind:
port: 8080
profiles:
sonnet:
baseUrl: http://gx10.gw:8000
model: claude-sonnet-5
rogue:
baseUrl: http://gx11.gw:8000
model: claude-opus-9000
models:
allow:
- model: claude-sonnet-5
- model: claude-haiku-5
""");
FleetConfig cfg = FleetConfig.load(f);
IllegalStateException e = assertThrows(IllegalStateException.class, cfg::validateModels);
assertTrue(e.getMessage().contains("rogue"), "the refusal names the offending profile");
assertTrue(e.getMessage().contains("claude-opus-9000"), "the refusal names the offending model");
assertFalse(e.getMessage().contains("'sonnet'"),
"the profile whose model IS allowed must not be reported");
}
/** A profile whose {@code model:} is on the allow-list loads fine. */
@Test
void aProfileNamingAModelOnTheAllowListLoadsFine(@TempDir Path dir) throws Exception {
Path f = dir.resolve("fleetd.yaml");
Files.writeString(f, """
bind:
port: 8080
profiles:
sonnet:
baseUrl: http://gx10.gw:8000
model: claude-sonnet-5
models:
allow:
- model: claude-sonnet-5
""");
FleetConfig cfg = FleetConfig.load(f);
assertDoesNotThrow(cfg::validateModels);
}
/**
* A profile that never sets {@code model:} (a {@code subscription: true} profile relying on the
* account's own default is the live example) must not be refused just because an allow-list is
* active — there is nothing to check it against.
*/
@Test
void aProfileWithNoModelPassesEvenWithAnActiveAllowList(@TempDir Path dir) throws Exception {
Path f = dir.resolve("fleetd.yaml");
Files.writeString(f, """
bind:
port: 8080
profiles:
opus:
subscription: true
models:
allow:
- model: claude-sonnet-5
""");
FleetConfig cfg = FleetConfig.load(f);
assertDoesNotThrow(cfg::validateModels);
}
/**
* Reproduces the live config shape this ticket measured: a mix of {@code amazon-bedrock},
* {@code opencode} and {@code openai}-backed profiles, some naming a bare Claude id and some a
* provider-prefixed opencode id, in ONE allow-list. Both forms are just opaque strings compared
* for exact equality — proves the shape decision holds against the shape actually seen live,
* not just against a synthetic single-provider example.
*/
@Test
void aBareClaudeIdAndAnOpencodeProviderPrefixedIdBothFitOneAllowList(@TempDir Path dir) throws Exception {
Path f = dir.resolve("fleetd.yaml");
Files.writeString(f, """
bind:
port: 8080
profiles:
sonnet:
baseUrl: http://gx10.gw:8000
model: claude-sonnet-5
terra:
kind: opencode
model: openai/gpt-5.6-terra
nova:
kind: opencode
model: amazon-bedrock/amazon.nova-pro-v1:0
models:
allow:
- model: claude-sonnet-5
- model: openai/gpt-5.6-terra
- model: amazon-bedrock/amazon.nova-pro-v1:0
""");
FleetConfig cfg = FleetConfig.load(f);
assertDoesNotThrow(cfg::validateModels);
}
/** The same live shape, but one opencode profile's model is missing from the allow-list. */
@Test
void anUnlistedOpencodeProviderPrefixedModelRefusesToStart(@TempDir Path dir) throws Exception {
Path f = dir.resolve("fleetd.yaml");
Files.writeString(f, """
bind:
port: 8080
profiles:
sonnet:
baseUrl: http://gx10.gw:8000
model: claude-sonnet-5
terra:
kind: opencode
model: openai/gpt-5.6-terra-withdrawn
models:
allow:
- model: claude-sonnet-5
- model: openai/gpt-5.6-terra
""");
FleetConfig cfg = FleetConfig.load(f);
IllegalStateException e = assertThrows(IllegalStateException.class, cfg::validateModels);
assertTrue(e.getMessage().contains("terra"), "the refusal names the offending profile");
assertTrue(e.getMessage().contains("openai/gpt-5.6-terra-withdrawn"),
"the refusal names the offending model, with its provider prefix intact");
}
/**
* Editing a {@code profiles:} entry alone must never be able to widen what is permitted — only
* editing {@code models.allow:} itself can. This is the invariant the ticket states explicitly;
* this test pins it by giving a profile a plausible-looking model that was never added to the
* allow-list and confirming it is still refused.
*/
@Test
void addingAProfileCannotWidenTheAllowListByItself(@TempDir Path dir) throws Exception {
Path f = dir.resolve("fleetd.yaml");
Files.writeString(f, """
bind:
port: 8080
profiles:
sonnet:
baseUrl: http://gx10.gw:8000
model: claude-sonnet-5
brand-new:
baseUrl: http://gx12.gw:8000
model: claude-sonnet-6-preview
models:
allow:
- model: claude-sonnet-5
""");
FleetConfig cfg = FleetConfig.load(f);
IllegalStateException e = assertThrows(IllegalStateException.class, cfg::validateModels);
assertTrue(e.getMessage().contains("brand-new"));
assertTrue(e.getMessage().contains("claude-sonnet-6-preview"));
}
/** {@code models} is a brand-new top-level key and must be recognized, not WARN-ed as unknown. */
@Test
void modelsIsAKnownTopLevelKey() {
assertTrue(FleetConfig.KNOWN_TOP_LEVEL_KEYS.contains("models"));
}
}
@@ -0,0 +1,358 @@
package dev.ltms.fleet.config;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.io.TempDir;
import java.lang.reflect.Method;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.List;
import java.util.Set;
import java.util.TreeSet;
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
/**
* The gap this class exists to close: mutation testing on the fleetd ticket "central allow-list
* of usable models" found that although {@link FleetConfig#validateModels()}'s own logic was well
* pinned, nothing proved either real caller ({@code Fleetd.main} and {@link ConfigRef#reload()})
* still invoked it — deleting the call site left the full suite green (1478/0/0/0). A follow-up
* measurement (same technique — remove one call site, run the suite, not read the code) found the
* SAME gap for all five of {@link FleetConfig}'s other validators at startup, and for four of the
* six inside {@link ConfigRef#reload()}. This is a class of gap, not one line's mistake: every one
* of those thirteen tests called the validator itself directly, never the real caller that was
* supposed to.
*
* <p>The fix replaces the six individual {@code cfg.validateXxx()} calls at each of the two real
* call sites with one {@link FleetConfig#validateAll()}, which reaches every validator by
* reflection rather than by a hand-maintained list of names. A hand-maintained list of six names
* would have exactly the defect it replaces: the seventh validator someone adds next month has no
* reason to be added to it, and nothing would say so. This class proves TWO separate claims, and
* keeps them separate on purpose:
*
* <ol>
* <li>{@link #theSweepMechanismIsGenericNotHardcodedToFleetConfigsSixNames()} and its neighbours
* prove the reflective sweep itself ({@link FleetConfig#invokeAllValidators}) is a general
* mechanism — it runs whatever public, no-arg, void {@code validateXxx()} methods a class
* happens to declare today, including a class with more of them than {@link FleetConfig}
* has right now. This is the proof that a future, real seventh validator on {@link
* FleetConfig} would be swept automatically, without needing to add a real (unwanted)
* seventh validator just to exercise the claim.</li>
* <li>{@link #validateAllReachesEveryOneOfTodaysSixValidators()} proves {@link
* FleetConfig#validateAll()} itself is wired to that same generic mechanism and genuinely
* reaches each of today's six real validators — reusing the exact minimal failing
* configurations {@code FleetConfigTest} already established for each one directly, so a
* single call to {@code validateAll()} is shown to reproduce every one of those six
* failures.</li>
* </ol>
*
* <p>Together with the direct-{@code Fleetd.main}-invocation tests in {@code
* FleetdStartupValidationTest} (which prove the real startup call site still calls {@code
* validateAll()}) and the {@code ConfigRefTest} reload tests (which prove the same for {@link
* ConfigRef#reload()}), removing {@code cfg.validateAll();} from either real call site now fails
* a test in this module.
*
* <p><b>What is NOT pinned, measured rather than assumed.</b> Reverting {@link
* FleetConfig#validateAll()} to a hardcoded list of today's six method calls leaves the whole
* suite green (measured at review: 1491 tests, 0 failures). Nothing ties {@code validateAll()} to
* the generic sweep — claim 1 proves {@link FleetConfig#invokeAllValidators} is generic, and claim
* 2 proves {@code validateAll()} reaches today's six, and a hardcoded list satisfies both. So the
* reflective sweep is a convenience, not the guarantee. The guarantee is {@link
* #fleetConfigDeclaresExactlyTheseSixValidatorsToday()}: it fails the moment a seventh validator
* is declared, which forces whoever adds it to look at this file.
*/
class FleetConfigValidateAllTest {
// ── Claim 1: the reflective sweep is a general mechanism, not six names in disguise ──────────
/**
* A throwaway fixture class, unrelated to {@link FleetConfig} in every way except shape: three
* public, no-arg, void methods named {@code validateXxx}. Proves the sweep works on ANY class
* with this shape, not on something special-cased to {@link FleetConfig}.
*/
static class ThreeValidators {
final List<String> ran = new ArrayList<>();
public void validateAlpha() {
ran.add("validateAlpha");
}
public void validateBeta() {
ran.add("validateBeta");
}
public void validateGamma() {
ran.add("validateGamma");
}
}
@Test
void theSweepMechanismIsGenericNotHardcodedToFleetConfigsSixNames() {
ThreeValidators target = new ThreeValidators();
FleetConfig.invokeAllValidators(target);
assertEquals(List.of("validateAlpha", "validateBeta", "validateGamma"), target.ran,
"every validateXxx() method on this unrelated class must run, in alphabetical "
+ "order — the sweep reads the class's own shape, not a name FleetConfig "
+ "happens to know about");
}
/**
* The core of the "self-maintaining" requirement: the exact same class shape as {@link
* ThreeValidators}, plus one more method — standing in for "a developer adds a validator next
* month". Nothing about the sweep changes to pick it up; the new method is invoked purely
* because it exists and matches the shape. This is what makes adding a seventh real validator
* to {@link FleetConfig} safe without touching {@link FleetConfig#validateAll()} or either
* call site — there is no "wire it in" step left to forget.
*/
static class FourValidators {
final List<String> ran = new ArrayList<>();
public void validateAlpha() {
ran.add("validateAlpha");
}
public void validateBeta() {
ran.add("validateBeta");
}
public void validateGamma() {
ran.add("validateGamma");
}
public void validateDelta() {
ran.add("validateDelta");
}
}
@Test
void addingAFourthValidatorMethodGetsSweptWithNoOtherChange() {
FourValidators target = new FourValidators();
FleetConfig.invokeAllValidators(target);
assertEquals(List.of("validateAlpha", "validateBeta", "validateDelta", "validateGamma"),
sorted(target.ran),
"the fourth method must be reached automatically — proving a class can grow the "
+ "set of things it validates with no change to the sweep itself");
}
private static List<String> sorted(List<String> in) {
List<String> copy = new ArrayList<>(in);
copy.sort(String::compareTo);
return copy;
}
@Test
void aFailingValidatorStopsTheSweepAndPropagatesUnchanged() {
class OneFails {
@SuppressWarnings("unused")
public void validateOk() {
// passes
}
public void validateBoom() {
throw new IllegalStateException("refusing to start: boom");
}
}
IllegalStateException e = assertThrows(IllegalStateException.class,
() -> FleetConfig.invokeAllValidators(new OneFails()));
assertEquals("refusing to start: boom", e.getMessage(),
"the real exception must propagate unchanged, not be wrapped or swallowed");
}
/**
* Every rule the sweep's filter applies, proven independently: only public, no-arg, void
* methods whose name starts with {@code "validate"} run, {@code validateAll} itself is
* excluded (so a class that declares one of its own — as {@link FleetConfig} does — cannot
* recurse into itself), and a same-shaped-but-wrongly-named or wrongly-shaped method never
* runs. A reader who "simplifies" the filter in {@link FleetConfig#invokeAllValidators} in a
* way that widens or narrows it breaks one of these.
*/
static class FilterEdgeCases {
final List<String> ran = new ArrayList<>();
public void validateReal() {
ran.add("validateReal");
}
/** Wrong name — must not run. */
public void checkSomething() {
ran.add("checkSomething");
}
/** Wrong shape — takes an argument. */
public void validateWithArg(String ignored) {
ran.add("validateWithArg");
}
/** Wrong shape — returns something. */
public boolean validateReturnsBoolean() {
ran.add("validateReturnsBoolean");
return true;
}
/** Excluded by name on purpose, so the sweep cannot call itself. */
public void validateAll() {
ran.add("validateAll");
}
}
@Test
void onlyPublicNoArgVoidValidateNamedMethodsRun() {
FilterEdgeCases target = new FilterEdgeCases();
FleetConfig.invokeAllValidators(target);
assertEquals(List.of("validateReal"), target.ran,
"checkSomething (wrong name), validateWithArg (wrong shape), "
+ "validateReturnsBoolean (wrong shape), and validateAll (excluded by "
+ "name) must all be skipped");
}
// ── Claim 2: FleetConfig.validateAll() is wired to that mechanism and reaches all six today ──
/**
* Reflectively enumerates {@link FleetConfig}'s own public, no-arg, void {@code validateXxx()}
* methods (excluding {@code validateAll} itself) — the exact same filter {@link
* FleetConfig#invokeAllValidators} applies. This is not the mechanism proof (that is claim 1,
* above, on an unrelated class) — it is a visible denominator: today there are six, named
* here, so a reader adding a seventh sees this assertion name the new count rather than a
* silent pass at the old one.
*/
@Test
void fleetConfigDeclaresExactlyTheseSixValidatorsToday() {
Set<String> names = new TreeSet<>();
for (Method m : FleetConfig.class.getMethods()) {
if (java.lang.reflect.Modifier.isPublic(m.getModifiers())
&& m.getParameterCount() == 0
&& m.getReturnType() == void.class
&& m.getName().startsWith("validate")
&& !m.getName().equals("validateAll")) {
names.add(m.getName());
}
}
assertEquals(new TreeSet<>(Set.of("validateAuthExposure", "validateLeadTabPrefixes",
"validateSubscriptionProfiles", "validateCharters", "validateMembers",
"validateModels")), names,
"FleetConfig's public validate*() methods changed. Do TWO things, in this "
+ "order. First confirm validateAll() still delegates to "
+ "invokeAllValidators(this) — a hardcoded list there passes every other "
+ "test in this class, so this assertion is the only place that will ever "
+ "make you check. Only then update the expected set to match.");
}
/** A minimal, otherwise-valid file — same shape FleetConfigTest and ConfigRefTest use. */
private static String minimalValidYaml() {
return """
bind:
host: 127.0.0.1
port: 8765
profiles:
sonnet:
baseUrl: http://gx00.gw:8000
model: sonnet
""";
}
@Test
void aFullyValidConfigPassesValidateAll(@TempDir Path dir) throws Exception {
Path f = dir.resolve("fleetd.yaml");
Files.writeString(f, minimalValidYaml());
assertDoesNotThrow(() -> FleetConfig.load(f).validateAll());
}
/**
* The heart of claim 2: for each of today's six real validators, a minimal file that fails
* ONLY that one — the exact fixtures {@code FleetConfigTest} uses to test each validator
* directly — must also fail through {@link FleetConfig#validateAll()}. If a future edit to
* {@code validateAll()} silently dropped one validator from the sweep (e.g. a typo'd name
* filter), exactly one of these six would start passing when it must not.
*/
@Test
void validateAllReachesEveryOneOfTodaysSixValidators(@TempDir Path dir) throws Exception {
// validateAuthExposure: a non-loopback bind without token mode.
assertValidateAllRefuses(dir, "auth-exposure.yaml", """
bind:
host: 0.0.0.0
port: 8765
""", "auth.mode: token");
// validateLeadTabPrefixes: a fleet-wide tabLabel that starts with a lead's own tabPrefix.
assertValidateAllRefuses(dir, "lead-tab-prefixes.yaml", """
bind:
host: 127.0.0.1
port: 8765
fleet:
tabLabel: "lead: {role} {profile}"
leaders:
opus:
tab: "lead: opus"
""", "fleet.tabLabel");
// validateSubscriptionProfiles: subscription: true with env: reseating ANTHROPIC_BASE_URL.
assertValidateAllRefuses(dir, "subscription-profiles.yaml", """
bind:
host: 127.0.0.1
port: 8765
profiles:
sonnet:
subscription: true
argv: ["ccs", "sonnet"]
env:
ANTHROPIC_BASE_URL: http://anything-not-on-the-allowlist
""", "ANTHROPIC_BASE_URL");
// validateCharters: a charter key that is not a role wire name.
assertValidateAllRefuses(dir, "charters.yaml", """
bind:
host: 127.0.0.1
port: 8765
fleet:
charters:
architetc: text
""", "architetc");
// validateMembers: an architect slot naming an unconfigured profile.
assertValidateAllRefuses(dir, "members.yaml", """
bind:
host: 127.0.0.1
port: 8765
profiles:
gx10:
baseUrl: http://gx10.gw:8000
fleet:
architects:
lead-designer:
profile: sonnet
""", "lead-designer");
// validateModels: a profile naming a model outside the configured allow-list.
assertValidateAllRefuses(dir, "models.yaml", """
bind:
host: 127.0.0.1
port: 8765
profiles:
sonnet:
baseUrl: http://gx10.gw:8000
model: claude-sonnet-5
rogue:
baseUrl: http://gx11.gw:8000
model: claude-opus-9000
models:
allow:
- model: claude-sonnet-5
""", "rogue");
}
private static void assertValidateAllRefuses(Path dir, String fileName, String yaml,
String mustContain) throws Exception {
Path f = dir.resolve(fileName);
Files.writeString(f, yaml);
FleetConfig cfg = FleetConfig.load(f);
IllegalStateException e = assertThrows(IllegalStateException.class, cfg::validateAll,
fileName + ": validateAll() must refuse this config");
assertTrue(e.getMessage().contains(mustContain),
fileName + ": expected message to contain \"" + mustContain + "\" but was: "
+ e.getMessage());
}
}
@@ -97,6 +97,8 @@ class FleetConfigWithDefaultsPreservesEveryComponentTest {
v.put("memberLoginShell", "/bin/zsh");
v.put("memberSkills", "/skills/guard");
v.put("idleSleepGuard", new FleetConfig.IdleSleepGuard(true));
v.put("models", new FleetConfig.Models(
List.of(new FleetConfig.Models.ModelEntry("model-guard"))));
assertNamesMatchComponents(v);
return v;
}
@@ -30,6 +30,7 @@ import java.util.concurrent.ScheduledFuture;
import java.util.concurrent.ScheduledThreadPoolExecutor;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicLong;
import java.util.concurrent.atomic.AtomicReference;
import java.util.function.BiConsumer;
import java.util.function.LongSupplier;
@@ -277,6 +278,58 @@ class FleetHealthMonitorTest {
}
}
/**
* fleetd #386 follow-up, added on merge. The fix carries a PER-MEMBER drift baseline, so drift
* from a sleep that happened BEFORE a member went busy is never charged to that member. The
* two tests shipped with the fix both start with the member already BUSY, so a single global
* baseline passes them — this one fails without the per-member map.
*
* <p>Order matters: the host sleeps while nothing is busy, and only then does a member take a
* turn. Its stall clock must start at zero.
*/
@Test void driftFromASleepBeforeAMemberWentBusyIsNotChargedToThatMember() {
Logger logger = (Logger) LoggerFactory.getLogger(FleetHealthMonitor.class);
ListAppender<ILoggingEvent> appender = new ListAppender<>();
appender.start();
logger.addAppender(appender);
try {
AtomicLong mono = new AtomicLong(0);
AtomicLong real = new AtomicLong(0);
AtomicReference<List<MemberSession>> roster = new AtomicReference<>(List.of());
FakeHerdr herdr = new FakeHerdr().withAgent("busy", "term_busy", "pane_busy", "tab_busy");
var scheduler = Executors.newSingleThreadScheduledExecutor();
AgentControl agents = new AgentControl(herdr);
FleetHealthMonitor monitor = new FleetHealthMonitor(agents, roster::get,
new MessageService(agents, new Injector(agents), new Rendezvous(), new InMemoryReplyInbox()),
scheduler, mono::get, real::get, 60, 600, (_, _) -> { });
monitor.tick(); // baseline, no members yet
// The host sleeps for 700s with nobody busy: the monotonic clock stands still.
real.set(TimeUnit.SECONDS.toNanos(700));
monitor.tick();
// Awake again. Only NOW does a member start a turn, with a fresh activity stamp taken
// from the monotonic clock. Both clocks advance together from here.
mono.set(TimeUnit.SECONDS.toNanos(10));
real.set(TimeUnit.SECONDS.toNanos(710));
roster.set(List.of(member("term_busy", MemberSession.State.BUSY,
0, TimeUnit.SECONDS.toNanos(10))));
monitor.tick();
mono.set(TimeUnit.SECONDS.toNanos(20));
real.set(TimeUnit.SECONDS.toNanos(720));
monitor.tick();
monitor.stop();
assertEquals(0, appender.list.stream().filter(event -> event.getFormattedMessage()
.contains("member=term_busy state=STALL_SUSPECTED")).count(),
"the member has been busy for 10s, not 710s — the earlier sleep is not its stall");
} finally {
logger.detachAppender(appender);
}
}
private static FleetHealthMonitor monitorWithClocks(FakeHerdr herdr, List<MemberSession> roster,
java.util.concurrent.ScheduledExecutorService scheduler, LongSupplier clock,
LongSupplier realtimeClock, long intervalSeconds, long workingSuspectAfterSeconds,
@@ -8,6 +8,7 @@ import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
@@ -45,6 +46,15 @@ class EnvAllowListScrubTest {
/** Env var names appearing in command output; anything else (prompts, wrapped lines) is noise. */
private static final Pattern ENV_NAME = Pattern.compile("^([A-Za-z_][A-Za-z0-9_]*)$");
/**
* fleetd #388: the sentinel name the generated {@code .zshenv} guard uses, kept here as a
* literal rather than referencing {@link EnvAllowListScrub#SCRUB_SENTINEL} — the two tests that
* use it must still compile and run against the pre-fix production class (which has no such
* constant), so the revert-and-prove-it-fails step exercises a real assertion instead of a
* compilation error.
*/
private static final String SCRUB_SENTINEL_NAME = "_CB633_SCRUBBED";
/**
* The equality test. Expected survivors = baseline exports ∩ allowed — i.e. every survivor is
* allowed AND every allowed name that existed survives. The operator's own secret-store exports
@@ -108,6 +118,31 @@ class EnvAllowListScrubTest {
"allowed N of M with N <= M — the denominator is always reported");
}
/** A group-shared ZDOTDIR still lets the member truncate and write its pre-created receipt. */
@Test
void groupSharedScrubWritesAndReadsItsReport(@TempDir Path tmp) throws Exception {
assumeTrue(Files.isExecutable(ZSH), "/bin/zsh not present — nothing to prove here");
Set<String> allowed = MemberEnvAllowList.derive(List.of());
Path zdotdir = EnvAllowListScrub.generate(tmp, allowed, currentUserGroup());
Map<String, String> cleanParent = Map.of(
"HOME", System.getProperty("user.home"),
"PATH", "/usr/bin:/bin",
"SHELL", "/bin/zsh");
exportedNamesFromCleanParent(cleanParent, zdotdir);
EnvAllowListScrub.ScrubReport report = EnvAllowListScrub.readReport(zdotdir);
assertNotNull(report, "a group-shared completed login shell must leave a report behind");
assertTrue(report.allowed() >= 0 && report.total() >= report.allowed(),
"allowed N of M with N <= M — the denominator is always reported");
assertEquals("rw-rw----", java.nio.file.attribute.PosixFilePermissions.toString(
Files.getPosixFilePermissions(zdotdir.resolve(EnvAllowListScrub.REPORT_FILE))),
"the pre-created receipt must be group-writable");
assertEquals("rwxr-x---", java.nio.file.attribute.PosixFilePermissions.toString(
Files.getPosixFilePermissions(zdotdir)),
"group sharing must not make the ZDOTDIR directory group-writable");
}
/** Report parsing is lenient: absent file → null (no measurement), not an exception. */
@Test
void readReportReturnsNullForADirectoryWithoutOne(@TempDir Path dir) {
@@ -224,6 +259,140 @@ class EnvAllowListScrubTest {
+ "shell. A difference here means the scrub is dead on Linux.");
}
/**
* fleetd #388: the actual gap. zsh reads {@code .zshenv} always, {@code .zprofile}/
* {@code .zlogin} only for a LOGIN shell, and {@code .zshrc} only for an INTERACTIVE one — so a
* shell that is NEITHER (a bare {@code /bin/zsh} reading a script off a non-tty stdin, no
* {@code -l}, no {@code -i}) reads only {@code .zshenv} and stops. Before this fix, that shell
* never reached {@code scrub.zsh} at all: the decoy secret below would survive untouched. This
* test injects that decoy directly into the process environment (not via a sourced dotfile,
* since the whole point of the gap is that {@code .zshenv} is normally close to empty) so the
* test does not depend on any real {@code ~/.zshrc} content existing on the host.
*/
@Test
void scrubRunsInAShellThatIsNeitherLoginNorInteractive(@TempDir Path tmp) throws Exception {
assumeTrue(Files.isExecutable(ZSH), "/bin/zsh not present — nothing to prove here");
Set<String> allowed = MemberEnvAllowList.derive(List.of());
Path zdotdir = EnvAllowListScrub.generate(tmp, allowed);
Map<String, String> cleanParent = new HashMap<>(Map.of(
"HOME", System.getProperty("user.home"),
"PATH", "/usr/bin:/bin",
"SHELL", "/bin/zsh",
"USER", System.getProperty("user.name", "nobody"),
"TMPDIR", tmp.toString()));
cleanParent.put("FLEETD_TEST_DECOY_SECRET", "x"); // not on any allow-list; must be blanked
List<String> neither = List.of(); // no -l, no -i; stdin is a pipe (never a tty) either way
Set<String> baseline = exportedNamesFromCleanParent(cleanParent, null, neither);
Set<String> scrubbed = exportedNamesFromCleanParent(cleanParent, zdotdir, neither);
Set<String> expected = new TreeSet<>();
for (String name : baseline) {
if (MemberEnvAllowList.keeps(allowed, name)) {
expected.add(name);
}
}
assertTrue(baseline.contains("FLEETD_TEST_DECOY_SECRET"),
"sanity: the decoy must actually reach the un-scrubbed baseline, or this test proves "
+ "nothing");
expected.add("ZDOTDIR"); // the harness set it and it is infrastructure, so it must survive
expected.add(SCRUB_SENTINEL_NAME); // set by the new .zshenv guard once scrubbed
assertEquals(expected, scrubbed,
"a pane shell that is NEITHER login nor interactive must still be scrubbed — its "
+ "surviving exported names must EQUAL baseline ∩ allow-list, plus the "
+ "sentinel the guard sets once it has run. FLEETD_TEST_DECOY_SECRET surviving "
+ "here means the gap is still open.");
}
/**
* fleetd #388 invariants 3 and 4, which a name-set equality cannot show: a member's own tooling
* forks plain, non-login, non-interactive zsh processes for a single command (the same shape as
* the pane shell itself), and such a child must (a) keep whatever its parent deliberately set
* for it, never (b) re-run the scrub and blank it, and never (c) overwrite the pane's own
* {@code scrub-report.txt} with a description of itself instead of the pane. All three can only
* be shown by actually running a child process from within the scrubbed pane shell.
*
* <p>The pane and the child both report presence via {@code ${NAME:+present}} — empty when a
* name is unset OR blanked (exported empty), {@code present} when it is set and non-empty. No
* value is ever printed, only these two shapes and the literal word {@code set}/{@code unset}
* for the sentinel.
*/
@Test
void neitherShellChildKeepsParentVariablesAndReceiptStillDescribesThePane(@TempDir Path tmp) throws Exception {
assumeTrue(Files.isExecutable(ZSH), "/bin/zsh not present — nothing to prove here");
Set<String> allowed = MemberEnvAllowList.derive(List.of());
Path zdotdir = EnvAllowListScrub.generate(tmp, allowed);
Map<String, String> paneEnv = new HashMap<>(Map.of(
"HOME", System.getProperty("user.home"),
"PATH", "/usr/bin:/bin",
"SHELL", "/bin/zsh",
"USER", System.getProperty("user.name", "nobody"),
"TMPDIR", tmp.toString()));
paneEnv.put("FLEETD_TEST_DECOY_SECRET", "x"); // not allow-listed; the pane must blank it
paneEnv.put("ZDOTDIR", zdotdir.toAbsolutePath().toString());
// The pane's own script reports what IT sees, then forks a plain non-login, non-interactive
// child — the shape a member's own tooling uses — carrying a variable the "parent" (this
// pane) deliberately set for it, the way git sets GIT_DIR for a hook.
String outerScript = """
print -r -- "PANE_SENTINEL=${%1$s:+set}"
print -r -- "PANE_DECOY=${FLEETD_TEST_DECOY_SECRET:+present}"
FLEETD_TEST_TOOL_VAR=keep /bin/zsh <<'CHILD'
print -r -- "CHILD_LOGIN=$([[ -o login ]] && echo yes || echo no)"
print -r -- "CHILD_INTERACTIVE=$([[ -o interactive ]] && echo yes || echo no)"
print -r -- "CHILD_TOOL_VAR=${FLEETD_TEST_TOOL_VAR:+present}"
print -r -- "CHILD_DECOY=${FLEETD_TEST_DECOY_SECRET:+present}"
print -r -- "CHILD_SENTINEL=${%1$s:+set}"
CHILD
exit
""".formatted(SCRUB_SENTINEL_NAME);
ProcessBuilder pb = new ProcessBuilder("/bin/zsh"); // no -l, no -i: the pane's own shape
pb.environment().clear();
pb.environment().putAll(paneEnv);
pb.redirectError(ProcessBuilder.Redirect.DISCARD);
Process zsh = pb.start();
zsh.getOutputStream().write(outerScript.getBytes(StandardCharsets.UTF_8));
zsh.getOutputStream().flush();
zsh.getOutputStream().close();
String stdout = new String(zsh.getInputStream().readAllBytes(), StandardCharsets.UTF_8);
assertTrue(zsh.waitFor(60, java.util.concurrent.TimeUnit.SECONDS),
"the pane+child probe did not exit within 60s");
assertTrue(zsh.exitValue() == 0, "probe zsh exited non-zero: " + stdout);
Map<String, String> reported = new HashMap<>();
for (String line : stdout.split("\n")) {
int eq = line.indexOf('=');
if (eq > 0) {
reported.put(line.substring(0, eq).trim(), line.substring(eq + 1).trim());
}
}
assertEquals("set", reported.get("PANE_SENTINEL"),
"the pane itself is neither login nor interactive, so the .zshenv guard must have "
+ "run the scrub and exported the sentinel");
assertEquals("", reported.get("PANE_DECOY"),
"the pane must blank a non-allow-listed name — invariant 1");
assertEquals("no", reported.get("CHILD_LOGIN"), "sanity: the child must also be non-login");
assertEquals("no", reported.get("CHILD_INTERACTIVE"), "sanity: the child must also be non-interactive");
assertEquals("present", reported.get("CHILD_TOOL_VAR"),
"invariant 3: a variable the pane deliberately set for its child must survive — a "
+ "child that re-ran the scrub would have blanked it");
assertEquals("", reported.get("CHILD_DECOY"),
"a name already blanked by the pane must stay blanked in the child, never resurrected");
assertEquals("set", reported.get("CHILD_SENTINEL"),
"the child must inherit the sentinel from the pane's environment, or it would re-scrub");
EnvAllowListScrub.ScrubReport report = EnvAllowListScrub.readReport(zdotdir);
assertNotNull(report, "the pane's own scrub pass must leave a report behind");
assertTrue(report.blanked().stream().noneMatch(n -> n.startsWith("FLEETD_TEST_TOOL_VAR")),
"invariant 4: the receipt must still describe the PANE, not the child — a child that "
+ "re-ran the scrub would have rewritten this file to list its own "
+ "FLEETD_TEST_TOOL_VAR as blanked");
}
/**
* Run {@code /bin/zsh -l -i} from a clean parent and return the NAMES it has exported by prompt
* time. With {@code zdotdir} non-null, {@code ZDOTDIR} points at a generated scrub directory, so
@@ -0,0 +1,144 @@
package dev.ltms.fleet.peer;
import org.junit.jupiter.api.Test;
import java.lang.reflect.Constructor;
import java.lang.reflect.RecordComponent;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import java.util.TreeSet;
import static org.junit.jupiter.api.Assertions.assertEquals;
/**
* Fleetd #382, the same "defect factory" #357 and #358 guarded on {@code FleetConfig.withDefaults()}
* and {@link dev.ltms.fleet.session.MemberSession}'s rebuild sites — reproduced here on
* {@link SpawnRequest#withProfile(String)}.
*
* <p>{@code SpawnRequest} has back-compat constructors at arity 3 and 5 alongside its canonical
* arity-6 constructor. Before this ticket, {@code CompositePeerLauncher} routed a profile by
* building a fresh {@code SpawnRequest} from a literal {@code new SpawnRequest(...)} call listing
* six of the original request's own accessors. That call is only correct because it happens to
* name exactly six arguments today — add a 7th component and the established back-compat pattern
* (a new constructor at the old, now-shorter arity) and a call one argument short of the new
* canonical arity would silently rebind to that back-compat constructor, dropping the new
* component on every profile-routed spawn without any compile error. {@link #withProfile} replaces
* that literal call, so this test guards the ONE rebuild site instead of a call site scattered
* through a launcher.
*
* <p>The check below builds a {@link SpawnRequest} through the TRUE canonical constructor —
* resolved by the record's own component types via {@code getDeclaredConstructor}, never by
* argument count, so it can never itself land on a back-compat overload — with a real, distinctive,
* non-null value in every component, calls {@link SpawnRequest#withProfile(String)}, and asserts
* every component the method is not documented to change survives unchanged, while {@code
* profileName} comes back as the new value it was given. A component that comes back anything else
* was silently dropped — the shape of the defect this test exists to catch.
*
* <p>{@link #EXCLUDED_FROM_SURVIVAL_CHECK} is kept deliberately empty and size-pinned by
* {@link #exclusionListSizeIsPinned()}, for the same reason the other two guards pin theirs at
* zero: a checker whose escape hatch can grow to silence a failure is not a checker. Every one of
* {@link SpawnRequest}'s 6 current components has a real, non-null value here and none is excluded.
*/
class SpawnRequestWithProfilePreservesEveryComponentTest {
private static final RecordComponent[] COMPONENTS = SpawnRequest.class.getRecordComponents();
/** Deliberately empty today; grow it only with a matching justification, and re-pin the size. */
private static final Set<String> EXCLUDED_FROM_SURVIVAL_CHECK = Set.of();
/** One real, distinctive, non-null value per component — none of the 6 is excluded. */
private static Map<String, Object> baseValues() {
Map<String, Object> v = new LinkedHashMap<>();
v.put("profileName", "profile-guard");
v.put("requestedCwd", "/wt/requested-guard");
v.put("callerCwd", "/wt/caller-guard");
v.put("sessionName", "session-guard");
v.put("resumeSessionId", "resume-guard");
v.put("role", MemberRole.REVIEWER);
assertNamesMatchComponents(v);
return v;
}
/**
* Guards {@link #baseValues()} itself against drifting from the record's real shape — forgetting
* to add a new component here fails this assertion by name, rather than silently checking one
* component fewer than the record has.
*/
private static void assertNamesMatchComponents(Map<String, Object> values) {
Set<String> names = new TreeSet<>();
for (RecordComponent rc : COMPONENTS) {
names.add(rc.getName());
}
assertEquals(names, new TreeSet<>(values.keySet()),
"this test's value map has drifted from SpawnRequest's actual components — "
+ "update baseValues() alongside the record");
}
/**
* Builds a {@link SpawnRequest} through the TRUE canonical constructor — resolved by the
* record's own component types, not by argument count — so this never accidentally exercises a
* back-compat overload the way a literal {@code new SpawnRequest(...)} call risks doing.
*/
private static SpawnRequest requestOf(Map<String, Object> values) throws ReflectiveOperationException {
Class<?>[] types = Arrays.stream(COMPONENTS).map(RecordComponent::getType).toArray(Class<?>[]::new);
Object[] args = Arrays.stream(COMPONENTS).map(rc -> values.get(rc.getName())).toArray();
Constructor<SpawnRequest> ctor = SpawnRequest.class.getDeclaredConstructor(types);
return ctor.newInstance(args);
}
@Test
void exclusionListSizeIsPinned() {
assertEquals(0, EXCLUDED_FROM_SURVIVAL_CHECK.size(),
"EXCLUDED_FROM_SURVIVAL_CHECK grew from 0 — every entry needs a justification in "
+ "this test class's javadoc AND this assertion re-pinned to the new size; a "
+ "growing exclusion list that silences failures on its own is not a guard");
}
@Test
void withProfilePreservesEveryOtherComponent() throws ReflectiveOperationException {
Map<String, Object> base = baseValues();
SpawnRequest request = requestOf(base);
SpawnRequest result = request.withProfile("profile-updated");
Map<String, Object> expectedOverrides = Map.of("profileName", "profile-updated");
List<String> dropped = new ArrayList<>();
int checked = 0;
for (RecordComponent rc : COMPONENTS) {
String name = rc.getName();
if (EXCLUDED_FROM_SURVIVAL_CHECK.contains(name)) {
continue;
}
checked++;
Object expected = expectedOverrides.containsKey(name) ? expectedOverrides.get(name) : base.get(name);
Object actual;
try {
actual = rc.getAccessor().invoke(result);
} catch (ReflectiveOperationException e) {
throw new RuntimeException("failed to read SpawnRequest." + name + "()", e);
}
if (!Objects.equals(expected, actual)) {
dropped.add(String.format(Locale.ROOT,
"%s: withProfile() was expected to carry (%s) for '%s' but returned %s — a "
+ "component silently dropped by withProfile(), the shape of the defect "
+ "this test exists to catch (its final \"return new SpawnRequest(...)\" "
+ "call binding to a back-compat constructor instead of the true "
+ "canonical one)",
name, expected, name, actual));
}
}
System.out.printf(Locale.ROOT,
"SpawnRequest.withProfile() component-survival coverage — %d components, %d checked, "
+ "%d excluded, %d survived%n",
COMPONENTS.length, checked, EXCLUDED_FROM_SURVIVAL_CHECK.size(), checked - dropped.size());
assertEquals(List.of(), dropped,
"withProfile() silently dropped these components: " + dropped);
}
}