#341: pin the noise control, and the reverse policy order

The fix reported every distinct unprotected name, but nothing held it there.

Mutation: replacing .filter(unprotectedGapNamesWarned::add) with a filter that
adds and always returns true - so every name is logged on every spawn - left
all 1358 tests green. The Set behaved; nothing proved this class used it as a
guard rather than as a record.

Two tests added:

- theSameUnprotectedNameIsWarnedAboutOnlyOnceAcrossSpawns pins invariant 1, the
  noise control. It now fails on that mutation, showing both duplicate WARNs.
- anAllowListWarnDoesNotSuppressALaterDenyByDefaultWarnForADifferentName covers
  the reverse policy order. The defect was found going deny-by-default then
  allow-list; a guard fixed in one direction is not fixed in the other.
This commit is contained in:
Dai Ha
2026-09-04 16:07:50 +07:00
parent b32a30fd47
commit d057d56156
@@ -434,6 +434,98 @@ class HerdrPeerLauncherAllowListWiringTest {
+ "Got: " + messages);
}
/**
* fleetd #341 follow-up: the OTHER half of the guard's contract. The set exists to report every
* distinct name, but it must still report each one only ONCE — the noise control is the reason
* a guard is here at all, and the ticket named it as invariant 1. Two spawns, same policy, same
* gap name: exactly one WARN mentioning it.
*
* <p>Measured before this test existed: replacing {@code .filter(unprotectedGapNamesWarned::add)}
* with a filter that adds and always returns {@code true} — so every name is logged on every
* spawn — left all 1358 tests green. The fix was correct and nothing held it there. That is the
* "a test on the seam does not prove the caller" shape: the {@code Set} behaves, and nothing
* proved this class used it as a guard rather than as a record.
*/
@Test
void theSameUnprotectedNameIsWarnedAboutOnlyOnceAcrossSpawns() {
FakeHerdr herdr = new FakeHerdr();
AtomicReference<FleetConfig.MemberCredentials> credsState = new AtomicReference<>(
new FleetConfig.MemberCredentials(null, List.of(), List.of(), null)); // deny-by-default
AtomicReference<Set<String>> hostEnvState =
new AtomicReference<>(Set.of("REPEATED_UNCOVERED_TOKEN"));
WiringLauncher launcher = new WiringLauncher(herdr, credsState::get, "/bin/zsh", hostEnvState::get);
Logger logger = (Logger) LoggerFactory.getLogger(HerdrPeerLauncher.class);
Level original = logger.getLevel();
logger.setLevel(Level.WARN);
ListAppender<ILoggingEvent> appender = new ListAppender<>();
appender.start();
logger.addAppender(appender);
try {
launcher.spawn(new SpawnRequest("test", null, null, null, null, MemberRole.DEV));
// Same policy, same gap, second spawn. Nothing new to tell the operator.
launcher.spawn(new SpawnRequest("test", null, null, null, null, MemberRole.DEV));
} finally {
logger.detachAppender(appender);
logger.setLevel(original);
}
List<String> messages = appender.list.stream().map(ILoggingEvent::getFormattedMessage).toList();
long mentioning = messages.stream()
.filter(m -> m.contains("REPEATED_UNCOVERED_TOKEN"))
.count();
assertEquals(1, mentioning,
"one unchanged unprotected name across two spawns must produce exactly one WARN — "
+ "the set is a guard, not just a record. Got: " + messages);
}
/**
* fleetd #341 follow-up: the reverse policy order. The original defect was found going
* deny-by-default then allow-list, and a guard that is fixed in one direction is not
* necessarily fixed in the other — "ask which states still OPEN the gate". Here spawn 1 runs
* under {@code allow-list} (the {@code keptByDerivedList} branch) and spawn 2 under
* {@code deny-by-default} ({@link HerdrPeerLauncher#warnGapUnprotected}), with a different name
* each time. Both must be reported.
*/
@Test
void anAllowListWarnDoesNotSuppressALaterDenyByDefaultWarnForADifferentName() {
FakeHerdr herdr = new FakeHerdr();
AtomicReference<FleetConfig.MemberCredentials> credsState = new AtomicReference<>(
new FleetConfig.MemberCredentials(
FleetConfig.MemberCredentials.POLICY_ALLOW_LIST, List.of(), List.of(), null));
AtomicReference<Set<String>> hostEnvState =
new AtomicReference<>(Set.of("FLEETD_WORKER_TOKEN"));
WiringLauncher launcher = new WiringLauncher(herdr, credsState::get, "/bin/zsh", hostEnvState::get);
Logger logger = (Logger) LoggerFactory.getLogger(HerdrPeerLauncher.class);
Level original = logger.getLevel();
logger.setLevel(Level.WARN);
ListAppender<ILoggingEvent> appender = new ListAppender<>();
appender.start();
logger.addAppender(appender);
try {
// Spawn 1: allow-list, gap kept by the derived list — the keptByDerivedList WARN.
launcher.spawn(new SpawnRequest("test", null, null, null, null, MemberRole.DEV));
// Live reload the OTHER way: back to deny-by-default, with a different name.
credsState.set(new FleetConfig.MemberCredentials(null, List.of(), List.of(), null));
hostEnvState.set(Set.of("LATER_UNCOVERED_TOKEN"));
launcher.spawn(new SpawnRequest("test", null, null, null, null, MemberRole.DEV));
} finally {
logger.detachAppender(appender);
logger.setLevel(original);
}
List<String> messages = appender.list.stream().map(ILoggingEvent::getFormattedMessage).toList();
assertTrue(messages.stream().anyMatch(
m -> m.contains("UNBLOCKED") && m.contains("FLEETD_WORKER_TOKEN")),
"spawn 1's allow-list gap must warn — got: " + messages);
assertTrue(messages.stream().anyMatch(
m -> m.contains("UNBLOCKED") && m.contains("LATER_UNCOVERED_TOKEN")),
"spawn 2's deny-by-default gap names a different variable and must still be warned "
+ "about, even though an allow-list WARN already fired. Got: " + messages);
}
/**
* fleetd #185 stage 2: with {@code memberHerdrSocket:} configured, member panes run under a
* different OS user — {@link HerdrPeerLauncher#hostEnvNames} describes fleetd's own process, not