Compare commits

..

9 Commits

Author SHA1 Message Date
Dai Ha 2c467c2553 fleetd #693, #676: pin lead-tab case-insensitivity, drop stale validator count
CI / shell-tests (pull_request) Failing after 20s
CI / contract (pull_request) Successful in 51s
CI / build (pull_request) Failing after 1m40s
#693: add a FleetConfigTest case for two fleet.leaders tabs differing only
in case — mutating equalsIgnoreCase to equals left this uncaught before.

#676: rename theSweepMechanismIsGenericNotHardcodedToFleetConfigsSixNames
to theSweepRunsEveryValidateMethodOnAnUnrelatedClass, since there are
eight validators now, not six, and the count had drifted into the name
and its class-javadoc {@link}.
2026-10-03 22:51:23 +02:00
Dai Ha bfee23acc3 Merge PR #691: fleetd #677 — refuse two leads sharing one exact tab (supersedes #686)
CI / shell-tests (push) Failing after 10s
CI / contract (push) Successful in 52s
CI / build (push) Failing after 1m37s
2026-10-03 22:45:23 +02:00
Dai Ha 7dec74f1b4 Merge PR #690: fleetd #638 — stop the verdict userinfo mask from crossing / or whitespace (supersedes #685)
CI / shell-tests (push) Failing after 7s
CI / contract (push) Successful in 54s
CI / build (push) Failing after 1m58s
2026-10-03 22:39:42 +02:00
Dai Ha 482598e2a6 fleetd #677: refuse two leads that share one exact tab
CI / shell-tests (pull_request) Failing after 10s
CI / contract (pull_request) Successful in 1m17s
CI / build (pull_request) Failing after 2m17s
validateLeadTabPrefixes() compared each lead's tab against the member
tabLabel template, but never against another lead's tab. Two leads
configured with the same exact tab loaded cleanly, even though exact
tab is the only thing lead identity is matched on, so only one of them
could ever be found.

Add an independent pass, case-insensitive, that refuses when two
fleet.leaders entries share one exact tab, with its own exception so
the message stays accurate for this relation.

Decided not to also refuse a lead's tab starting with a sibling's
tabPrefix: tabPrefix plays no role in identity resolution (only the
exact tab does), and the default tabPrefix is "lead:", the same
string used as the conventional lead tab prefix throughout this
codebase's own fixtures (e.g. "lead: opus" / "lead: sol"). Refusing
that case would reject the standard multi-lead setup with no matching
identity hazard.
2026-10-03 22:36:44 +02:00
Dai Ha 5f5d16fbd4 fleetd #638: stop the verdict userinfo mask from crossing / or whitespace
CI / shell-tests (pull_request) Failing after 10s
CI / contract (pull_request) Successful in 45s
CI / build (pull_request) Failing after 1m57s
mask_verdict_userinfo's character class [^@]* crossed a '/' or a space, so a
verdict line with a URI that has no userinfo plus a later @ (e.g. an email
address in diagnostic prose) had everything between them destroyed. Restrict
the class to [^@/[:space:]]* so the match stops at the end of the URI.

Adds the uncovered-direction test: a URI with no userinfo plus a later @ in
the same line must pass through byte for byte. Also drops the design-rationale
sentence from the helper's comment (now in the PR description).
2026-10-03 22:34:02 +02:00
Dai Ha 7df7985a16 Merge remote-tracking branch 'refs/remotes/pr/686' into worker/677-fix-lead-collision-f69073-12 2026-10-03 22:29:29 +02:00
Dai Ha 724b35b46e Merge remote-tracking branch 'refs/remotes/pr/685' into worker/638-fix-overmask-dbb1bf-11 2026-10-03 22:29:14 +02:00
Dai Ha cb4a6869b9 fleetd #677: guard exact lead tab collisions
CI / shell-tests (pull_request) Failing after 8s
CI / contract (pull_request) Successful in 46s
CI / build (pull_request) Failing after 1m56s
2026-10-03 22:10:29 +02:00
Dai Ha 28b45d97e5 fleetd #638: mask userinfo in the daemon verdict line before it prints
CI / shell-tests (pull_request) Failing after 7s
CI / contract (pull_request) Successful in 1m7s
CI / build (pull_request) Failing after 2m4s
Apply the userinfo-only rewrite (sed -E 's#://[^@]*@#://<redacted>@#g') to every
path that prints config-edit.sh's $VERDICT_LINE: restore_and_confirm's two
prints, report_outcome's shared local copy (covering its clean/needs-restart/
refused branches), and check_mode's "last verdict in log" line via
last_verdict_line. Deliberately not routed through redact() — that function's
key:value masking does not match this line's prose, and the rest of the line
(e.g. the pattern quoted in a parse-failure refusal) is the detail an operator
needs to fix the refusal.

No current refusal message echoes a URI, token or password, so this is a guard
against a future validator doing so, not a fix for an observed leak.
2026-10-03 22:06:54 +02:00
8 changed files with 288 additions and 310 deletions
@@ -1129,11 +1129,8 @@ public record FleetConfig(
* {@code tab} can never be discovered, launched or not
* @param instances how many of this lead should be live (default 1). The daemon
* launches only the shortfall, so a restart adopts rather than doubles
* @param tabPrefix no longer used to find a lead's tab — {@code tab} is matched
* exactly. Its only remaining job is the startup collision guard
* ({@link #validateLeadTabPrefixes()}), which still uses it to refuse
* a worker {@code tabLabel} template that could be misread as a lead.
* Default {@code "lead:"}
* @param tabPrefix lead-tab naming convention checked against member labels. Lead
* identity uses {@code tab}. Default {@code "lead:"}
* @param scanIntervalSeconds how long a tab scan is cached before herdr is asked again; also the
* worst case before a newly-labelled tab is recognised. Default 10
* @param kind which agent runs there ({@code claude}, {@code opencode}, …)
@@ -1236,10 +1233,7 @@ public record FleetConfig(
String tabLabel) {
/**
* Role first, so the tab bar reads as the fleet and so the label shares a namespace with a
* lead's {@code tabPrefix}. Because {@code {role}} comes from a closed enum, a generated
* member label can never begin with {@code "lead:"} — the clash that
* {@link #validateLeadTabPrefixes()} used to have to check for is unrepresentable here.
* Role first, so the tab bar identifies the member's fleet role.
*/
public static final String DEFAULT_TAB_LABEL = "{role}: {profile} #{n}";
@@ -2685,28 +2679,14 @@ public record FleetConfig(
}
/**
* Reject a lead-scan convention that a worker tab would also satisfy (CB-531).
* Reject a member tab-label template that could render as a configured lead tab or match a
* lead-tab naming convention, and reject two {@code fleet.leaders} entries that share one exact
* tab.
*
* <p>The scan reads a tab label and concludes "a lead lives here". fleetd also <em>writes</em>
* tab labels — every member gets one rendered into its tab. Choose a lead {@code tabPrefix} that
* a member template matches and the daemon starts labelling its own members as leads, promoting
* the entire fleet to {@link dev.ltms.fleet.auth.Role#PRIMARY} with no message and no diff.
* {@link #validatePanePlacementAgainstLeadTabs()} is the check that stops a pane-placed member
* from landing inside a lead's tab in the first place; this check is a second, independent
* guard that catches the hazard even when every profile places members correctly, by refusing
* a label that a scan would still misread as a lead.
*
* <p>CB-557 shrank this check rather than removing it. The default template is
* {@code "{role}: {profile} #{n}"} and {@code {role}} comes from a closed enum, so a
* <em>generated</em> label can no longer collide by construction. What remains checkable is what
* an operator still writes by hand: the {@code fleet.tabLabel} template and any per-profile
* {@code tabLabel} override.
*
* <p>Fatal rather than a warning, unlike {@link #warnUnknownTopLevelKeys}: an unknown key means
* a feature does nothing, while this means a feature does the opposite of what it says.
*
* @throws IllegalStateException when the fleet template or any profile's {@code tabLabel}
* override starts with a configured lead prefix
* @throws IllegalStateException when the fleet template or a profile {@code tabLabel} override
* can render as a configured lead tab or match a lead-tab prefix,
* or when two {@code fleet.leaders} entries carry the same exact
* {@code tab} (case-insensitively)
*/
public void validateLeadTabPrefixes() {
if (fleet == null || fleet.leaders().isEmpty()) {
@@ -2717,28 +2697,90 @@ public record FleetConfig(
if (leader == null) {
return;
}
String tab = leader.tab();
String prefix = leader.tabPrefix();
// The fleet-wide template is checked once per prefix: it labels every member that has no
// override, so one bad template promotes the entire fleet, not one profile.
if (startsWithIgnoreCase(fleet.tabLabel(), prefix)) {
if (templateCanRenderAs(fleet.tabLabel(), tab)) {
bad.add("fleet.tabLabel=\"" + fleet.tabLabel() + "\" can render as the tab of "
+ "lead '" + leadName + "' (\"" + tab + "\")");
} else if (startsWithIgnoreCase(fleet.tabLabel(), prefix)) {
bad.add("fleet.tabLabel=\"" + fleet.tabLabel() + "\" starts with the tabPrefix of "
+ "lead '" + leadName + "' (\"" + prefix + "\")");
}
profiles().entrySet().stream()
.filter(e -> startsWithIgnoreCase(e.getValue().tabLabel(), prefix))
.map(Map.Entry::getKey)
.sorted()
.forEach(p -> bad.add("profile '" + p + "' overrides tabLabel with \""
+ profiles().get(p).tabLabel() + "\", which starts with the tabPrefix of "
+ "lead '" + leadName + "' (\"" + prefix + "\")"));
.forEach(p -> {
String label = profiles().get(p).tabLabel();
if (templateCanRenderAs(label, tab)) {
bad.add("profile '" + p + "' overrides tabLabel with \"" + label
+ "\", which can render as the tab of lead '" + leadName
+ "' (\"" + tab + "\")");
} else if (startsWithIgnoreCase(label, prefix)) {
bad.add("profile '" + p + "' overrides tabLabel with \"" + label
+ "\", which starts with the tabPrefix of lead '" + leadName
+ "' (\"" + prefix + "\")");
}
});
});
if (bad.isEmpty()) {
if (!bad.isEmpty()) {
throw new IllegalStateException("refusing to start: " + String.join("; ", bad)
+ ". Every member labelled that way would be read back as a lead and granted "
+ "spawn/stop/send on the whole fleet. Change one of the two so member tabs "
+ "and lead tabs cannot be confused.");
}
List<String> collisions = new ArrayList<>();
List<String> leadNames = fleet.leaders().keySet().stream().sorted().toList();
for (int i = 0; i < leadNames.size(); i++) {
String nameA = leadNames.get(i);
Leader a = fleet.leaders().get(nameA);
if (a == null || a.tab() == null || a.tab().isBlank()) {
continue;
}
for (int j = i + 1; j < leadNames.size(); j++) {
String nameB = leadNames.get(j);
Leader b = fleet.leaders().get(nameB);
if (b == null || b.tab() == null || b.tab().isBlank()) {
continue;
}
if (a.tab().equalsIgnoreCase(b.tab())) {
collisions.add("lead '" + nameA + "' and lead '" + nameB + "' both use tab \""
+ a.tab() + "\"");
}
}
}
if (collisions.isEmpty()) {
return;
}
throw new IllegalStateException("refusing to start: " + String.join("; ", bad)
+ ". Every member labelled that way would be read back as a lead and granted "
+ "spawn/stop/send on the whole fleet. Change one of the two so member tabs and "
+ "lead tabs cannot be confused.");
throw new IllegalStateException("refusing to start: " + String.join("; ", collisions)
+ ". Tab identity is matched exactly, so only one of two leads sharing a tab can "
+ "ever be found — the other is silently unreachable. Give each lead its own "
+ "exact tab.");
}
private static boolean templateCanRenderAs(String template, String tab) {
if (template == null || template.isBlank() || tab == null || tab.isBlank()) {
return false;
}
var placeholders = Pattern.compile("\\{(?:role|profile|model|n)}").matcher(template);
StringBuilder expression = new StringBuilder("^");
int literalStart = 0;
while (placeholders.find()) {
expression.append(Pattern.quote(template.substring(literalStart, placeholders.start())));
expression.append(".*");
literalStart = placeholders.end();
}
expression.append(Pattern.quote(template.substring(literalStart))).append("$");
return Pattern.compile(expression.toString(), Pattern.CASE_INSENSITIVE).matcher(tab).matches();
}
/** Case-insensitive prefix test that tolerates a null or blank label. */
private static boolean startsWithIgnoreCase(String label, String prefix) {
if (label == null || prefix == null || prefix.isBlank()) {
return false;
}
String stripped = label.strip();
return stripped.regionMatches(true, 0, prefix, 0, prefix.length());
}
/**
@@ -2800,15 +2842,6 @@ public record FleetConfig(
}
}
/** Case-insensitive prefix test that tolerates a null/blank label. */
private static boolean startsWithIgnoreCase(String label, String prefix) {
if (label == null || prefix == null || prefix.isBlank()) {
return false;
}
String stripped = label.strip();
return stripped.regionMatches(true, 0, prefix, 0, prefix.length());
}
/**
* Reject a subscription profile whose {@code env:} block tries to reseat the Anthropic binding
* (CB-542).
@@ -83,18 +83,6 @@ public final class FleetApp {
};
}
/**
* The second gate for {@code POST /sessions/{id}/message}: checked only when {@code turnId}
* is present and non-blank, against {@link Authz.Action#ANSWER}. A request with no {@code
* turnId} passes this gate unconditionally, without consulting {@code permit} at all, having
* already cleared the coarse {@link Authz.Action#SEND} grant checked ahead of it.
*
* @param permit reports whether the caller holds the named grant
*/
static boolean answerGatePasses(String turnId, Predicate<Authz.Action> permit) {
return turnId == null || turnId.isBlank() || permit.test(Authz.Action.ANSWER);
}
/** Default blocking window for a message; kept under typical HTTP idle timeouts. */
private static final long DEFAULT_MESSAGE_TIMEOUT_MS = 25_000;
private static final long MAX_MESSAGE_TIMEOUT_MS = 120_000;
@@ -637,29 +625,25 @@ public final class FleetApp {
*
* <p>Two call shapes share this route, exactly as {@code fleet_send} does over MCP (see
* {@code FleetMcp#sendAction}): a plain delivery to {@code id}, and -- when the body carries
* {@code turnId} -- resolving a worker's blocked question. The coarse {@link
* Authz.Action#SEND} grant is checked first, before the body is read at all; only once that
* passes is the body parsed, and a present {@code turnId} is then checked again against
* {@link Authz.Action#ANSWER}. A body that fails to parse is rejected with 400 and reaches
* neither {@code messages.answer} nor {@code messages.send}.
* {@code turnId} -- resolving a worker's blocked question. The body is parsed before the
* authorization check so the right one of {@link Authz.Action#SEND}/{@link Authz.Action#ANSWER}
* reaches the gate; a body that fails to parse is treated as the plain shape for that check
* alone, and is rejected afterward exactly as before.
*/
private void sendMessage(Context ctx) {
String id = ctx.pathParam("id");
if (!allow(ctx, routeAction("POST /sessions/{id}/message"), id)) {
return;
}
JsonNode body;
try {
body = mapper.readTree(ctx.body());
} catch (Exception e) {
body = null;
}
if (body == null) {
ctx.status(400).json(Map.of("error", "bad_request", "detail", "body must be JSON"));
String turnId = body == null ? null : body.path("turnId").asText(null);
if (!allow(ctx, routeAction("POST /sessions/{id}/message", turnId), id)) {
return;
}
String turnId = body.path("turnId").asText(null);
if (!answerGatePasses(turnId, action -> allow(ctx, action, id))) {
if (body == null) {
ctx.status(400).json(Map.of("error", "bad_request", "detail", "body must be JSON"));
return;
}
String content = body.path("content").asText("");
@@ -676,12 +676,8 @@ class FleetConfigTest {
assertEquals(5, hb.quietNudgeCap());
}
/**
* The hazard the guard exists for: fleetd writes worker tab labels and reads lead tab labels.
* Overlap the two and every worker it spawns is read back as a lead.
*/
@Test
void aLeadPrefixThatAProfileTabLabelOverrideAlsoMatchesRefusesToStart(@TempDir Path dir)
void aProfileTabLabelOverrideMatchingALeadTabRefusesToStart(@TempDir Path dir)
throws Exception {
Path f = dir.resolve("collide.yaml");
Files.writeString(f, """
@@ -689,32 +685,33 @@ class FleetConfigTest {
port: 8080
profiles:
gx10:
tabLabel: "lead: {profile} #{n}"
tabLabel: "alpha"
fleet:
leaders:
opus:
tab: "lead: opus"
tabPrefix: "lead:"
tab: "alpha"
""");
FleetConfig cfg = FleetConfig.load(f);
IllegalStateException e =
assertThrows(IllegalStateException.class, cfg::validateLeadTabPrefixes);
assertTrue(e.getMessage().contains("gx10"), "the message must name the offending profile");
assertTrue(e.getMessage().contains("alpha"), "the message must name the offending label");
}
/** A bad fleet-wide template promotes every member, not one profile — so it is checked too. */
@Test
void aFleetTabLabelThatMatchesALeadPrefixRefusesToStart(@TempDir Path dir) throws Exception {
void aFleetTabLabelTemplateThatCanRenderAsALeadTabRefusesToStart(@TempDir Path dir) throws Exception {
Path f = dir.resolve("collide-template.yaml");
Files.writeString(f, """
bind:
port: 8080
profiles:
pha: {}
fleet:
tabLabel: "lead: {role} {profile}"
tabLabel: "al{profile}"
leaders:
opus:
tab: "lead: opus"
tab: "alpha"
""");
FleetConfig cfg = FleetConfig.load(f);
@@ -723,12 +720,28 @@ class FleetConfigTest {
assertTrue(e.getMessage().contains("fleet.tabLabel"));
}
/**
* The point of making role the label's first field: {@code {role}} comes from a closed enum, so
* a generated label cannot begin with {@code "lead:"} however the fleet is configured.
*/
@Test
void theDefaultTabLabelCannotCollideWithTheDefaultLeadPrefix(@TempDir Path dir) throws Exception {
void anExactFleetTabLabelCollisionRefusesToStart(@TempDir Path dir) throws Exception {
Path f = dir.resolve("exact-tab-collision.yaml");
Files.writeString(f, """
bind:
port: 8080
fleet:
tabLabel: "alpha"
leaders:
alpha:
tab: "alpha"
""");
IllegalStateException e = assertThrows(IllegalStateException.class,
() -> FleetConfig.load(f).validateAll());
assertTrue(e.getMessage().contains("fleet.tabLabel"),
"the message must name the offending label");
assertTrue(e.getMessage().contains("alpha"), "the message must name the colliding lead tab");
}
@Test
void aFleetTabLabelTemplateThatCannotRenderAsALeadTabIsAllowed(@TempDir Path dir) throws Exception {
Path f = dir.resolve("ok.yaml");
Files.writeString(f, """
bind:
@@ -737,17 +750,13 @@ class FleetConfigTest {
gx10:
baseUrl: http://gx00.gw:8000
fleet:
tabLabel: "worker-{profile}"
leaders:
opus:
tab: "lead: opus"
tab: "alpha"
""");
assertDoesNotThrow(() -> FleetConfig.load(f).validateLeadTabPrefixes());
for (MemberRole role : MemberRole.values()) {
assertFalse(FleetConfig.Fleet.DEFAULT_TAB_LABEL
.replace("{role}", role.wireName()).startsWith("lead:"),
"no role renders a label that reads as a lead");
}
assertDoesNotThrow(() -> FleetConfig.load(f).validateAll());
}
@Test
@@ -765,6 +774,78 @@ class FleetConfigTest {
"a label that collides with a convention nobody reads is not a problem");
}
/**
* fleetd #677: identity is matched on a lead's exact {@code tab} alone, so two leads sharing
* one tab means only one of them is ever found — the guard must catch this independently of
* the member-template checks above.
*/
@Test
void twoLeadsSharingTheSameExactTabRefusesToStart(@TempDir Path dir) throws Exception {
Path f = dir.resolve("shared-tab.yaml");
Files.writeString(f, """
bind:
port: 8080
fleet:
leaders:
opus:
tab: "shared tab"
sonnet:
tab: "shared tab"
""");
FleetConfig cfg = FleetConfig.load(f);
IllegalStateException e =
assertThrows(IllegalStateException.class, cfg::validateLeadTabPrefixes);
assertTrue(e.getMessage().contains("opus"), "the message must name one offending lead");
assertTrue(e.getMessage().contains("sonnet"), "the message must name the other offending lead");
}
/**
* fleetd #693: the guard matches tabs case-insensitively, because
* {@code LeadTabScanner} keys its tab map on a lowercased label — two tabs differing only in
* case collide there too, and the guard must catch that independently of the exact-match case
* above.
*/
@Test
void twoLeadsSharingTheSameTabInDifferentCaseRefusesToStart(@TempDir Path dir) throws Exception {
Path f = dir.resolve("shared-tab-case.yaml");
Files.writeString(f, """
bind:
port: 8080
fleet:
leaders:
opus:
tab: "Shared Tab"
sonnet:
tab: "shared tab"
""");
FleetConfig cfg = FleetConfig.load(f);
IllegalStateException e =
assertThrows(IllegalStateException.class, cfg::validateLeadTabPrefixes);
assertTrue(e.getMessage().contains("opus"), "the message must name one offending lead");
assertTrue(e.getMessage().contains("sonnet"), "the message must name the other offending lead");
}
/** Control for {@link #twoLeadsSharingTheSameExactTabRefusesToStart}: distinct tabs load cleanly. */
@Test
void twoLeadsWithDistinctExactTabsAreAllowed(@TempDir Path dir) throws Exception {
Path f = dir.resolve("distinct-tabs.yaml");
Files.writeString(f, """
bind:
port: 8080
fleet:
leaders:
opus:
tab: "opus tab"
sonnet:
tab: "sonnet tab"
""");
FleetConfig cfg = FleetConfig.load(f);
assertDoesNotThrow(cfg::validateLeadTabPrefixes);
}
// ── validatePanePlacementAgainstLeadTabs ────────────────────────────────────────────────────
/**
@@ -17,63 +17,21 @@ 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.
* Tests the reflective validator sweep and {@link FleetConfig#validateAll()} reachability.
*
* <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 #validateAllReachesEveryOneOfTodaysRealValidators()} proves {@link
* FleetConfig#validateAll()} itself is wired to that same generic mechanism and genuinely
* reaches every one of today's real validators — reusing the exact minimal failing
* configurations {@code FleetConfigTest} already established for each one directly, plus a
* dedicated fixture for {@link FleetConfig#validateLeadRollover()}, which no other test
* drives through {@code validateAll()} — so a single call to {@code validateAll()} is shown
* to reproduce every one of those 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 method calls leaves the whole
* suite green. 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 validators, and a hardcoded list satisfies both. So the
* reflective sweep is a convenience, not the guarantee. The guarantee is {@link
* #fleetConfigDeclaresExactlyTheseValidatorsToday()}: it fails the moment any validator is added
* or removed, which forces whoever changes the set to look at this file.
* <p>{@link #theSweepRunsEveryValidateMethodOnAnUnrelatedClass()} and its neighbours
* prove that {@link FleetConfig#invokeAllValidators} runs each public, no-arg, void
* {@code validateXxx()} method on its target. {@link #fleetConfigDeclaresExactlyTheseValidatorsToday()}
* is the canary for the validator set. {@link #validateAllReachesEveryOneOfTodaysRealValidators()}
* is the reachability check for that set.
*/
class FleetConfigValidateAllTest {
// ── Claim 1: the reflective sweep is a general mechanism, not six names in disguise ──────────
// ── Claim 1: the reflective sweep is a general mechanism ─────────────────────────────────────
/**
* 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}.
* Fixture with public, no-arg, void methods named {@code validateXxx}. It proves the sweep uses
* the target's method shape rather than special handling for {@link FleetConfig}.
*/
static class ThreeValidators {
final List<String> ran = new ArrayList<>();
@@ -92,7 +50,7 @@ class FleetConfigValidateAllTest {
}
@Test
void theSweepMechanismIsGenericNotHardcodedToFleetConfigsSixNames() {
void theSweepRunsEveryValidateMethodOnAnUnrelatedClass() {
ThreeValidators target = new ThreeValidators();
FleetConfig.invokeAllValidators(target);
assertEquals(List.of("validateAlpha", "validateBeta", "validateGamma"), target.ran,
@@ -102,12 +60,8 @@ class FleetConfigValidateAllTest {
}
/**
* 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.
* Fixture with an added valid method. It proves the sweep reaches a method because it matches
* the validator shape.
*/
static class FourValidators {
final List<String> ran = new ArrayList<>();
@@ -135,7 +89,7 @@ class FleetConfigValidateAllTest {
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 "
"the added method must be reached automatically — proving a class can grow the "
+ "set of things it validates with no change to the sweep itself");
}
@@ -297,16 +251,16 @@ class FleetConfigValidateAllTest {
port: 8765
""", "auth.mode: token");
// validateLeadTabPrefixes: a fleet-wide tabLabel that starts with a lead's own tabPrefix.
// validateLeadTabPrefixes: a fleet-wide tabLabel that equals a lead tab.
assertValidateAllRefuses(dir, "lead-tab-prefixes.yaml", """
bind:
host: 127.0.0.1
port: 8765
fleet:
tabLabel: "lead: {role} {profile}"
tabLabel: "alpha"
leaders:
opus:
tab: "lead: opus"
tab: "alpha"
""", "fleet.tabLabel");
// validateSubscriptionProfiles: subscription: true with env: reseating ANTHROPIC_BASE_URL.
@@ -1,8 +1,5 @@
package dev.ltms.fleet.rest;
import ch.qos.logback.classic.Level;
import ch.qos.logback.classic.spi.ILoggingEvent;
import com.fasterxml.jackson.databind.ObjectMapper;
import dev.ltms.fleet.auth.CallerResolver;
import dev.ltms.fleet.auth.Authz;
import dev.ltms.fleet.auth.MemberRegistry;
@@ -21,7 +18,6 @@ import dev.ltms.fleet.msg.Rendezvous;
import dev.ltms.fleet.session.FakeWorktrees;
import dev.ltms.fleet.session.SessionManager;
import dev.ltms.fleet.member.ClaudeCodeLauncher;
import dev.ltms.fleet.testing.CapturedLog;
import io.javalin.Javalin;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Test;
@@ -32,13 +28,10 @@ import java.net.http.HttpRequest;
import java.net.http.HttpResponse;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Set;
import java.util.function.Predicate;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import java.util.stream.Collectors;
@@ -151,38 +144,6 @@ class FleetAppAuthTest {
assertEquals(Authz.Action.ANSWER, FleetApp.routeAction("POST /sessions/{id}/message", "turn-1"));
}
/**
* fleetd #689: {@code answerGatePasses} is the second, conditional gate behind {@code
* sendMessage}'s coarse {@link Authz.Action#SEND} check. With the {@code ANSWER} grant denied,
* a {@code turnId}-bearing request is refused while a plain one still passes — and the denied
* permit is queried only for the {@code turnId} case, never for the plain one, which is what
* proves this is a genuinely separate, conditional check rather than the {@code SEND} check
* renamed or an unconditional call whose result is ignored. Flipping only the {@code ANSWER}
* grant to allowed then flips only the {@code turnId} shape's outcome.
*/
@Test
void answerGatePassesOnlyWhenTurnIdAbsentOrAnswerGranted() {
List<Authz.Action> queried = new ArrayList<>();
Predicate<Authz.Action> denyAnswer = action -> {
queried.add(action);
return false;
};
assertFalse(FleetApp.answerGatePasses("turn-1", denyAnswer),
"ANSWER denied ⇒ the turnId shape is refused");
assertEquals(List.of(Authz.Action.ANSWER), queried,
"the ANSWER grant, specifically, must be the one consulted");
queried.clear();
assertTrue(FleetApp.answerGatePasses(null, denyAnswer),
"no turnId ⇒ the plain shape passes even though ANSWER is denied");
assertTrue(FleetApp.answerGatePasses(" ", denyAnswer), "a blank turnId is treated as absent");
assertEquals(List.of(), queried, "the plain shape must never consult the permit at all");
assertTrue(FleetApp.answerGatePasses("turn-1", action -> true),
"flipping only the ANSWER grant to allowed flips only the turnId shape's outcome");
}
private static Set<String> routesTheServerRegisters() {
try {
String source = Files.readString(REST_SOURCE).lines()
@@ -262,92 +223,6 @@ class FleetAppAuthTest {
"resolving another session's blocked question would be a worker escalating too");
}
/**
* fleetd #689: a caller refused the coarse {@link Authz.Action#SEND} grant is refused on
* {@code SEND} specifically, even on the {@code turnId}-bearing shape that otherwise raises
* the check to {@link Authz.Action#ANSWER} — proving {@code turnId} was never read from the
* body before the refusal (reading it would have changed which action is named in the 403).
* The same caller refused with no body at all gets the identical detail, which could not hold
* if the decision depended on anything read from the body. Control: a caller who IS granted
* reaches past the gate and the body is used normally.
*/
@Test
void aDeniedCallerIsRefusedOnSendEvenWithATurnIdBodyAndNeverReadsTheBody() throws Exception {
int workerPort = start(FakeHerdr.WORKER_PID, false, null); // denied: not primary/architect
HttpResponse<String> withTurnId = send(workerPort, "POST", "/sessions/term_b/message",
"{\"turnId\":\"turn-1\",\"content\":\"hi\"}", null);
assertEquals(403, withTurnId.statusCode());
assertTrue(withTurnId.body().contains("may not SEND"),
"the SEND check must be the one that fired, not ANSWER — ANSWER would only be "
+ "reachable by having already read turnId out of the body");
HttpResponse<String> noBody = send(workerPort, "POST", "/sessions/term_b/message", null, null);
assertEquals(403, noBody.statusCode());
assertTrue(noBody.body().contains("may not SEND"),
"refused identically with no body at all — the refusal cannot depend on body content");
// Control: a primary IS granted SEND, so the same turnId body is read and acted on —
// reaching messages.answer, which reports this unknown turnId as a stale one.
int primaryPort = start(999_999, false, null);
HttpResponse<String> granted = send(primaryPort, "POST", "/sessions/term_b/message",
"{\"turnId\":\"turn-1\",\"content\":\"hi\"}", null);
assertEquals(409, granted.statusCode());
assertTrue(granted.body().contains("stale_turn"), "a granted caller's body IS read and acted on");
}
/**
* fleetd #689 (ticket comment 18353): the only place {@code sendMessage}'s call to {@code
* answerGatePasses} is observable is the audit trail — {@code allow()} logs an {@code
* "allowed"} entry for every granted action except {@code READ}/{@code METRICS}/{@code
* TASK_READ}, and {@code ANSWER} is none of those. A granted {@code turnId} request must
* therefore log both a {@code SEND} and an {@code ANSWER} entry; a granted plain request must
* log {@code SEND} alone. A unit test of the extracted helper pins the helper; this pins the
* call site — deleting the {@code answerGatePasses} call from {@code sendMessage} leaves the
* helper's own test green but turns this one red.
*/
@Test
void aGrantedTurnIdRequestAuditsBothSendAndAnswerButAPlainRequestAuditsSendAlone() throws Exception {
int port = start(999_999, false, null); // primary: granted both SEND and ANSWER
ObjectMapper mapper = new ObjectMapper();
try (CapturedLog audit = CapturedLog.at("audit", Level.INFO)) {
send(port, "POST", "/sessions/term_b/message",
"{\"turnId\":\"turn-1\",\"content\":\"hi\"}", null);
List<String> allowed = allowedActions(audit, mapper);
assertTrue(allowed.contains("SEND"),
"a turnId request must still clear the coarse SEND grant first");
assertTrue(allowed.contains("ANSWER"),
"a turnId request must ALSO clear the ANSWER grant — this is the call site itself");
}
try (CapturedLog audit = CapturedLog.at("audit", Level.INFO)) {
send(port, "POST", "/sessions/term_b/message",
"{\"content\":\"hi\",\"timeoutMs\":50}", null);
List<String> allowed = allowedActions(audit, mapper);
assertEquals(List.of("SEND"), allowed,
"a plain request must log SEND and nothing else — ANSWER is conditional on "
+ "turnId, not something every request happens to log");
}
}
private static List<String> allowedActions(CapturedLog audit, ObjectMapper mapper) {
return audit.events().stream()
.map(ILoggingEvent::getFormattedMessage)
.map(line -> {
try {
return mapper.readTree(line);
} catch (Exception e) {
throw new AssertionError("audit line is not valid JSON: " + line, e);
}
})
.filter(n -> "allowed".equals(n.path("outcome").asText()))
.map(n -> n.path("action").asText())
.toList();
}
// --- token mode ---------------------------------------------------------------------------
@Test
@@ -675,26 +675,6 @@ class FleetAppTest {
assertEquals(400, postMessage(port, "{}").statusCode());
}
/**
* fleetd #689: a body that fails to parse is rejected with 400 before {@code turnId} is ever
* read from it, so it reaches neither {@code messages.answer} (which needs a {@code turnId})
* nor {@code messages.send} — confirmed here for {@code send} by the fake agent's idle status,
* which would otherwise make an immediate {@code agent.prompt} delivery observable.
*/
@Test
void malformedBodyReturns400AndNeverReachesSendOrAnswer() throws Exception {
FakeHerdr herdr = new FakeHerdr().agentStatus("idle"); // idle ⇒ send would deliver right away if reached
int port = start(herdr, "http://gx00.gw:8000", Set.of("gx00.gw"));
HttpResponse<String> res = postMessage(port, "not json at all");
assertEquals(400, res.statusCode());
JsonNode err = mapper.readTree(res.body());
assertEquals("bad_request", err.get("error").asText());
assertEquals("body must be JSON", err.get("detail").asText());
assertFalse(herdr.called("agent.prompt"),
"a malformed body must never reach messages.send's delivery");
}
@Test
void sessionStatusReportsLiveAgentStatus() throws Exception {
FakeHerdr herdr = new FakeHerdr().agentStatus("blocked");
+10 -4
View File
@@ -580,6 +580,12 @@ install_candidate() {
# -------------------------------------------------------------------------------- the report path
#
# Masks basic-auth userinfo (scheme://user:pass@host) in a daemon verdict line before it reaches
# the terminal.
mask_verdict_userinfo() {
printf '%s\n' "$1" | sed -E 's#://[^@/[:space:]]*@#://<redacted>@#g'
}
# Prints the literal command the operator (or a test) can run to restore the backup by hand — the
# absolute path to THIS script plus the overrides actually in force, so it works from any cwd.
restore_command_line() {
@@ -599,8 +605,8 @@ restore_and_confirm() {
ok "restored from $backup"
if wait_for_verdict "$LOG" "$mark2" "$WAIT_SECONDS"; then
case "$VERDICT_KIND" in
refused) warn "the RESTORE was also refused by the daemon: $VERDICT_LINE" ;;
*) ok "restore confirmed: $VERDICT_LINE" ;;
refused) warn "the RESTORE was also refused by the daemon: $(mask_verdict_userinfo "$VERDICT_LINE")" ;;
*) ok "restore confirmed: $(mask_verdict_userinfo "$VERDICT_LINE")" ;;
esac
else
warn "the restore is on disk, but no confirming verdict line appeared within ${WAIT_SECONDS}s"
@@ -616,7 +622,7 @@ report_outcome() {
say "waiting for the daemon's verdict (up to ${WAIT_SECONDS}s)"
if wait_for_verdict "$LOG" "$mark" "$WAIT_SECONDS"; then
kind="$VERDICT_KIND"; line="$VERDICT_LINE"
kind="$VERDICT_KIND"; line="$(mask_verdict_userinfo "$VERDICT_LINE")"
else
kind="none"
fi
@@ -673,7 +679,7 @@ check_mode() {
local verdict
verdict="$(last_verdict_line "$LOG")"
if [ -n "$verdict" ]; then
ok "last verdict in log: $verdict"
ok "last verdict in log: $(mask_verdict_userinfo "$verdict")"
else
warn "no reload verdict line found in $LOG"
fi
+65
View File
@@ -629,6 +629,65 @@ test_refusal_shape_from_parse_failure_wording_is_recognised() {
assert_equals 4 "$RUN_RC" "the parse-failure refusal shape must also exit 4, not be read as silence"
}
# A verdict line carrying a credentialed URI has its userinfo masked, with a positive control
# proving the rest of the line still reaches the output unchanged.
test_verdict_userinfo_is_masked_with_positive_control() {
local dir
dir="$(new_fixture)"
start_run "$dir" 5 --set '.profiles.sonnet.weight=4'
sleep 1
printf 'config reload from %s refused, keeping the running config: refusing to start: malformed pattern — profiles.local.errorPattern ("amqp://user:hunter2@host/vhost"): Unclosed character class near index 8\n' \
"$dir/fleetd.yaml" >> "$dir/fleetd.out"
collect_run "$dir"
assert_equals 4 "$RUN_RC" "refusal-with-userinfo exit code"
assert_not_contains "user:hunter2" "$RUN_OUTPUT" "the userinfo must never reach the output"
assert_contains "amqp://<redacted>@host/vhost" "$RUN_OUTPUT" \
"the userinfo must be MASKED, not deleted — the rest of the quoted value must survive"
# Positive control: the diagnostic prose on both sides of the userinfo must still reach the
# output. Without this, a mutant that drops the whole verdict line would pass identically.
assert_contains "malformed pattern" "$RUN_OUTPUT" "prose BEFORE the userinfo must still reach the output"
assert_contains "Unclosed character class near index 8" "$RUN_OUTPUT" \
"prose AFTER the userinfo must still reach the output"
}
# An ordinary refusal line quotes the offending pattern, not a credential, and must survive byte
# for byte: the rewrite is scoped to userinfo only, and the quoted pattern is the detail an
# operator needs to fix the refusal.
test_ordinary_refusal_line_passes_through_unchanged() {
local dir real_line
dir="$(new_fixture)"
real_line="config reload from $dir/fleetd.yaml refused, keeping the running config: refusing to start: malformed pattern — profiles.local.errorPattern (\"[unclosed\"): Unclosed character class near index 8"
start_run "$dir" 5 --set '.profiles.sonnet.weight=4'
sleep 1
printf '%s\n' "$real_line" >> "$dir/fleetd.out"
collect_run "$dir"
assert_equals 4 "$RUN_RC" "ordinary refusal exit code"
assert_contains "$real_line" "$RUN_OUTPUT" \
"an ordinary refusal with no userinfo must pass through byte for byte, unchanged"
}
# A verdict line can hold a URI with NO userinfo and a later, unrelated @ further on in the same
# line (an email address in diagnostic prose, for example). The rewrite must stop at the end of
# the URI and must not treat the later @ as a second userinfo delimiter.
test_uri_without_userinfo_survives_a_later_at_sign() {
local dir real_line
dir="$(new_fixture)"
real_line="config reload from $dir/fleetd.yaml refused, keeping the running config: broker.uri amqp://broker.local/vhost unreachable, contact ops@example.com"
start_run "$dir" 5 --set '.profiles.sonnet.weight=4'
sleep 1
printf '%s\n' "$real_line" >> "$dir/fleetd.out"
collect_run "$dir"
assert_equals 4 "$RUN_RC" "no-userinfo-with-later-at-sign exit code"
assert_contains "$real_line" "$RUN_OUTPUT" \
"a URI with no userinfo plus a later @ in the same line must pass through byte for byte"
}
# --set runs yq over the whole candidate. It warns when that changes more lines than the requested
# pairs, but a simple file with only the intended changed line must stay quiet.
new_fixture_reformat_sensitive() {
@@ -720,6 +779,12 @@ echo "== extra: --check is read-only and always exits 0 =="
test_check_is_read_only_and_exits_zero
echo "== extra: the parse-failure refusal shape is also recognised =="
test_refusal_shape_from_parse_failure_wording_is_recognised
echo "== verdict-redaction criteria 2+3: verdict userinfo is masked, rest of line survives =="
test_verdict_userinfo_is_masked_with_positive_control
echo "== verdict-redaction criterion 4: an ordinary refusal passes through unchanged =="
test_ordinary_refusal_line_passes_through_unchanged
echo "== fleetd #638: a URI with no userinfo survives a later @ in the same line =="
test_uri_without_userinfo_survives_a_later_at_sign
echo "== acceptance criterion 17: --set warns about yq formatting churn =="
test_set_warns_when_yq_reformats_extra_lines
echo "== acceptance criterion 18: --set stays quiet without formatting churn =="