From d057d56156f82030920a10cb3ca3ac61630735a0 Mon Sep 17 00:00:00 2001 From: Dai Ha Date: Fri, 4 Sep 2026 16:07:50 +0700 Subject: [PATCH] #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. --- .../HerdrPeerLauncherAllowListWiringTest.java | 92 +++++++++++++++++++ 1 file changed, 92 insertions(+) diff --git a/fleetd/src/test/java/dev/ltms/fleet/member/HerdrPeerLauncherAllowListWiringTest.java b/fleetd/src/test/java/dev/ltms/fleet/member/HerdrPeerLauncherAllowListWiringTest.java index 2340878..426aed6 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/member/HerdrPeerLauncherAllowListWiringTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/member/HerdrPeerLauncherAllowListWiringTest.java @@ -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. + * + *

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 credsState = new AtomicReference<>( + new FleetConfig.MemberCredentials(null, List.of(), List.of(), null)); // deny-by-default + AtomicReference> 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 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 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 credsState = new AtomicReference<>( + new FleetConfig.MemberCredentials( + FleetConfig.MemberCredentials.POLICY_ALLOW_LIST, List.of(), List.of(), null)); + AtomicReference> 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 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 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