From 4f64fe23da6d54ba6791327f458e32f1cedc5534 Mon Sep 17 00:00:00 2001 From: "pozdn.r.a" Date: Wed, 6 May 2026 19:40:36 +0700 Subject: [PATCH 01/23] move claim runtime services out of events --- .../talhanation/bannermod/events/ClaimEvents.java | 4 +++- .../runtime}/ClaimRuntimeService.java | 13 +++++++------ .../runtime}/SettlementHeartbeatService.java | 10 +++++----- 3 files changed, 15 insertions(+), 12 deletions(-) rename src/main/java/com/talhanation/bannermod/{events => settlement/runtime}/ClaimRuntimeService.java (72%) rename src/main/java/com/talhanation/bannermod/{events => settlement/runtime}/SettlementHeartbeatService.java (96%) diff --git a/src/main/java/com/talhanation/bannermod/events/ClaimEvents.java b/src/main/java/com/talhanation/bannermod/events/ClaimEvents.java index be6f62d4..a7ee1908 100644 --- a/src/main/java/com/talhanation/bannermod/events/ClaimEvents.java +++ b/src/main/java/com/talhanation/bannermod/events/ClaimEvents.java @@ -2,6 +2,8 @@ import com.talhanation.bannermod.entity.military.AbstractRecruitEntity; import com.talhanation.bannermod.entity.military.RecruitIndex; +import com.talhanation.bannermod.settlement.runtime.ClaimRuntimeService; +import com.talhanation.bannermod.settlement.runtime.SettlementHeartbeatService; import com.talhanation.bannermod.util.RuntimeProfilingCounters; import com.talhanation.bannermod.persistence.military.*; import net.minecraft.server.MinecraftServer; @@ -33,7 +35,7 @@ public static RecruitsClaimManager claimManager() { return recruitsClaimManager; } - static void installRuntime(MinecraftServer currentServer, RecruitsClaimManager currentClaimManager) { + public static void installRuntime(MinecraftServer currentServer, RecruitsClaimManager currentClaimManager) { server = currentServer; recruitsClaimManager = currentClaimManager; } diff --git a/src/main/java/com/talhanation/bannermod/events/ClaimRuntimeService.java b/src/main/java/com/talhanation/bannermod/settlement/runtime/ClaimRuntimeService.java similarity index 72% rename from src/main/java/com/talhanation/bannermod/events/ClaimRuntimeService.java rename to src/main/java/com/talhanation/bannermod/settlement/runtime/ClaimRuntimeService.java index 413d4984..249d1790 100644 --- a/src/main/java/com/talhanation/bannermod/events/ClaimRuntimeService.java +++ b/src/main/java/com/talhanation/bannermod/settlement/runtime/ClaimRuntimeService.java @@ -1,5 +1,6 @@ -package com.talhanation.bannermod.events; +package com.talhanation.bannermod.settlement.runtime; +import com.talhanation.bannermod.events.ClaimEvents; import com.talhanation.bannermod.persistence.military.RecruitsClaimManager; import net.minecraft.server.level.ServerLevel; import net.minecraft.server.level.ServerPlayer; @@ -8,8 +9,8 @@ import net.neoforged.neoforge.event.server.ServerStartingEvent; import net.neoforged.neoforge.event.server.ServerStoppingEvent; -final class ClaimRuntimeService { - void onServerStarting(ServerStartingEvent event) { +public final class ClaimRuntimeService { + public void onServerStarting(ServerStartingEvent event) { ServerLevel level = event.getServer().overworld(); RecruitsClaimManager claimManager = new RecruitsClaimManager(); @@ -17,15 +18,15 @@ void onServerStarting(ServerStartingEvent event) { ClaimEvents.installRuntime(event.getServer(), claimManager); } - void onServerStopping(ServerStoppingEvent event) { + public void onServerStopping(ServerStoppingEvent event) { ClaimEvents.claimManager().save(ClaimEvents.server().overworld()); } - void onWorldSave(LevelEvent.Save event) { + public void onWorldSave(LevelEvent.Save event) { ClaimEvents.claimManager().save(ClaimEvents.server().overworld()); } - void onPlayerJoin(EntityJoinLevelEvent event) { + public void onPlayerJoin(EntityJoinLevelEvent event) { if(event.getLevel().isClientSide()) return; if(event.getEntity() instanceof ServerPlayer player){ diff --git a/src/main/java/com/talhanation/bannermod/events/SettlementHeartbeatService.java b/src/main/java/com/talhanation/bannermod/settlement/runtime/SettlementHeartbeatService.java similarity index 96% rename from src/main/java/com/talhanation/bannermod/events/SettlementHeartbeatService.java rename to src/main/java/com/talhanation/bannermod/settlement/runtime/SettlementHeartbeatService.java index 108995ee..198b3e14 100644 --- a/src/main/java/com/talhanation/bannermod/events/SettlementHeartbeatService.java +++ b/src/main/java/com/talhanation/bannermod/settlement/runtime/SettlementHeartbeatService.java @@ -1,17 +1,17 @@ -package com.talhanation.bannermod.events; +package com.talhanation.bannermod.settlement.runtime; +import com.talhanation.bannermod.events.ClaimEvents; import com.talhanation.bannermod.governance.BannerModGovernorHeartbeat; import com.talhanation.bannermod.governance.BannerModGovernorManager; import com.talhanation.bannermod.governance.BannerModTreasuryManager; import com.talhanation.bannermod.settlement.BannerModSettlementManager; import com.talhanation.bannermod.settlement.BannerModSettlementOrchestrator; import com.talhanation.bannermod.settlement.BannerModSettlementService; -import com.talhanation.bannermod.settlement.runtime.SettlementClaimBindingService; import com.talhanation.bannermod.util.AdaptiveRuntimeBudgets; import com.talhanation.bannermod.util.RuntimeProfilingCounters; import net.minecraft.server.level.ServerLevel; -final class SettlementHeartbeatService { +public final class SettlementHeartbeatService { private static final int GOVERNOR_TICK_INTERVAL = 200; private static final int GOVERNOR_HEARTBEAT_BATCH_SIZE = 16; private static final int SETTLEMENT_REFRESH_BATCH_SIZE = 16; @@ -25,13 +25,13 @@ final class SettlementHeartbeatService { private int governorMaintenanceStage; private int governorMaintenanceCursor; - void reset() { + public void reset() { governorCounter = 0; governorMaintenanceStage = GOVERNOR_STAGE_IDLE; governorMaintenanceCursor = 0; } - void tick(ServerLevel level) { + public void tick(ServerLevel level) { governorCounter++; if(governorMaintenanceStage == GOVERNOR_STAGE_IDLE && governorCounter >= GOVERNOR_TICK_INTERVAL){ From 8562ef0a78118f3c4bcc48805170c79572abe638 Mon Sep 17 00:00:00 2001 From: "pozdn.r.a" Date: Wed, 6 May 2026 19:40:44 +0700 Subject: [PATCH 02/23] move citizen birth service out of events --- .../runtime}/CitizenBirthService.java | 10 ++++++---- .../bannermod/events/WorkerSettlementClaimPolicy.java | 11 ++++++----- .../events/WorkerSettlementEventService.java | 1 + 3 files changed, 13 insertions(+), 9 deletions(-) rename src/main/java/com/talhanation/bannermod/{events => citizen/runtime}/CitizenBirthService.java (94%) diff --git a/src/main/java/com/talhanation/bannermod/events/CitizenBirthService.java b/src/main/java/com/talhanation/bannermod/citizen/runtime/CitizenBirthService.java similarity index 94% rename from src/main/java/com/talhanation/bannermod/events/CitizenBirthService.java rename to src/main/java/com/talhanation/bannermod/citizen/runtime/CitizenBirthService.java index 9ce8d5bd..9f49dca2 100644 --- a/src/main/java/com/talhanation/bannermod/events/CitizenBirthService.java +++ b/src/main/java/com/talhanation/bannermod/citizen/runtime/CitizenBirthService.java @@ -1,8 +1,10 @@ -package com.talhanation.bannermod.events; +package com.talhanation.bannermod.citizen.runtime; import com.talhanation.bannermod.config.WorkersServerConfig; import com.talhanation.bannermod.entity.citizen.CitizenEntity; import com.talhanation.bannermod.entity.citizen.CitizenIndex; +import com.talhanation.bannermod.events.ClaimEvents; +import com.talhanation.bannermod.events.WorkerSettlementClaimPolicy; import com.talhanation.bannermod.persistence.military.RecruitsClaim; import com.talhanation.bannermod.registry.citizen.ModCitizenEntityTypes; import com.talhanation.bannermod.settlement.civilian.CitizenBirthRules; @@ -30,7 +32,7 @@ * out of scope for this slice — a server restart resets the cooldown, which * is acceptable for a 24000-tick (1 day) default. */ -final class CitizenBirthService { +public final class CitizenBirthService { private static final Logger LOGGER = LogUtils.getLogger(); private static final Map LAST_BIRTH_GAME_TIME = new HashMap<>(); private static final Map LAST_DENIAL_REASON = new HashMap<>(); @@ -38,12 +40,12 @@ final class CitizenBirthService { private CitizenBirthService() { } - static void resetRuntimeState() { + public static void resetRuntimeState() { LAST_BIRTH_GAME_TIME.clear(); LAST_DENIAL_REASON.clear(); } - static void runCitizenBirthPass(ServerLevel level) { + public static void runCitizenBirthPass(ServerLevel level) { if (level == null || ClaimEvents.claimManager() == null) { return; } diff --git a/src/main/java/com/talhanation/bannermod/events/WorkerSettlementClaimPolicy.java b/src/main/java/com/talhanation/bannermod/events/WorkerSettlementClaimPolicy.java index cfe99a76..3b437934 100644 --- a/src/main/java/com/talhanation/bannermod/events/WorkerSettlementClaimPolicy.java +++ b/src/main/java/com/talhanation/bannermod/events/WorkerSettlementClaimPolicy.java @@ -1,5 +1,6 @@ package com.talhanation.bannermod.events; +import com.talhanation.bannermod.citizen.runtime.CitizenBirthService; import com.talhanation.bannermod.entity.civilian.AbstractWorkerEntity; import com.talhanation.bannermod.entity.civilian.AnimalFarmerEntity; import com.talhanation.bannermod.entity.civilian.BuilderEntity; @@ -44,7 +45,7 @@ import java.util.Map; import java.util.UUID; -final class WorkerSettlementClaimPolicy { +public final class WorkerSettlementClaimPolicy { private WorkerSettlementClaimPolicy() { } @@ -107,7 +108,7 @@ static BannerModSettlementBinding.Binding resolveSettlementBinding(Villager vill return BannerModSettlementBinding.resolveSettlementStatus(ClaimEvents.claimManager(), villager.blockPosition(), factionId); } - static BannerModSettlementBinding.Binding resolveClaimGrowthBinding(RecruitsClaim claim, String settlementFactionId) { + public static BannerModSettlementBinding.Binding resolveClaimGrowthBinding(RecruitsClaim claim, String settlementFactionId) { ChunkPos anchorChunk = resolveClaimAnchorChunk(claim); return BannerModSettlementBinding.resolveSettlementStatus(claim, anchorChunk, settlementFactionId); } @@ -199,7 +200,7 @@ static Map countWorkersByP * Returns 0 when no snapshot is available — the spawn rules treat that as * "no slack" and deny when {@code requireHousing} is on. */ - static int housingSlackForClaim(ServerLevel level, RecruitsClaim claim) { + public static int housingSlackForClaim(ServerLevel level, RecruitsClaim claim) { if (level == null || claim == null) { return 0; } @@ -230,7 +231,7 @@ static int housingSlackForClaim(ServerLevel level, RecruitsClaim claim) { * Used by {@link CitizenBirthService} to gate births on a settlement * actually having food. */ - static int claimFoodCount(ServerLevel level, RecruitsClaim claim) { + public static int claimFoodCount(ServerLevel level, RecruitsClaim claim) { if (level == null || claim == null || claim.getClaimedChunks().isEmpty()) { return 0; } @@ -266,7 +267,7 @@ static int claimFoodCount(ServerLevel level, RecruitsClaim claim) { * already gate on {@link #housingSlackForClaim} so this is a best-effort * assignment for the path where housing is not strictly required. */ - static void assignHomeIfAvailable(ServerLevel level, RecruitsClaim claim, UUID residentUuid, long gameTime) { + public static void assignHomeIfAvailable(ServerLevel level, RecruitsClaim claim, UUID residentUuid, long gameTime) { if (level == null || claim == null || residentUuid == null) { return; } diff --git a/src/main/java/com/talhanation/bannermod/events/WorkerSettlementEventService.java b/src/main/java/com/talhanation/bannermod/events/WorkerSettlementEventService.java index 49ba2fa5..aca50d97 100644 --- a/src/main/java/com/talhanation/bannermod/events/WorkerSettlementEventService.java +++ b/src/main/java/com/talhanation/bannermod/events/WorkerSettlementEventService.java @@ -1,5 +1,6 @@ package com.talhanation.bannermod.events; +import com.talhanation.bannermod.citizen.runtime.CitizenBirthService; import com.talhanation.bannermod.citizen.CitizenProfession; import com.talhanation.bannermod.config.WorkersServerConfig; import com.talhanation.bannermod.entity.civilian.AbstractWorkerEntity; From 5cb6ac23ffe1e1f1e0d2e7b0e988e12b7f509974 Mon Sep 17 00:00:00 2001 From: "pozdn.r.a" Date: Wed, 6 May 2026 19:40:52 +0700 Subject: [PATCH 03/23] add building type validator dispatcher --- .../validation/DefaultBuildingValidator.java | 37 +++++++--- .../types/BuildingTypeValidator.java | 7 ++ .../BuildingTypeValidatorDispatcher.java | 38 ++++++++++ .../types/BuildingValidationContext.java | 21 ++++++ .../BuildingTypeValidatorDispatcherTest.java | 70 +++++++++++++++++++ 5 files changed, 164 insertions(+), 9 deletions(-) create mode 100644 src/main/java/com/talhanation/bannermod/settlement/validation/types/BuildingTypeValidator.java create mode 100644 src/main/java/com/talhanation/bannermod/settlement/validation/types/BuildingTypeValidatorDispatcher.java create mode 100644 src/main/java/com/talhanation/bannermod/settlement/validation/types/BuildingValidationContext.java create mode 100644 src/test/java/com/talhanation/bannermod/settlement/validation/types/BuildingTypeValidatorDispatcherTest.java diff --git a/src/main/java/com/talhanation/bannermod/settlement/validation/DefaultBuildingValidator.java b/src/main/java/com/talhanation/bannermod/settlement/validation/DefaultBuildingValidator.java index d356b064..1e42276e 100644 --- a/src/main/java/com/talhanation/bannermod/settlement/validation/DefaultBuildingValidator.java +++ b/src/main/java/com/talhanation/bannermod/settlement/validation/DefaultBuildingValidator.java @@ -8,6 +8,8 @@ import com.talhanation.bannermod.settlement.building.ValidatedBuildingRegistryData; import com.talhanation.bannermod.settlement.building.ZoneRole; import com.talhanation.bannermod.settlement.building.ZoneSelection; +import com.talhanation.bannermod.settlement.validation.types.BuildingTypeValidatorDispatcher; +import com.talhanation.bannermod.settlement.validation.types.BuildingValidationContext; import net.minecraft.core.BlockPos; import net.minecraft.server.level.ServerLevel; import net.minecraft.world.entity.player.Player; @@ -40,9 +42,16 @@ public class DefaultBuildingValidator implements BuildingValidator { private static final Set PROHIBITED_OVERLAP_ROLE_PAIRS = prohibitedRolePairs(); private final BuildingDefinitionRegistry definitionRegistry; + private final BuildingTypeValidatorDispatcher typeValidatorDispatcher; public DefaultBuildingValidator(BuildingDefinitionRegistry definitionRegistry) { + this(definitionRegistry, new BuildingTypeValidatorDispatcher()); + } + + public DefaultBuildingValidator(BuildingDefinitionRegistry definitionRegistry, + BuildingTypeValidatorDispatcher typeValidatorDispatcher) { this.definitionRegistry = definitionRegistry; + this.typeValidatorDispatcher = typeValidatorDispatcher; } @Override @@ -68,16 +77,26 @@ public BuildingValidationResult validate(ServerLevel level, Player player, Build } EnumMap zonesByRole = toRoleMap(request.zones()); + BuildingValidationContext context = new BuildingValidationContext(level, player, request, zonesByRole, warnings, blocking); + return this.typeValidatorDispatcher.validate(context, this::validateByTypeFallback); + } + + private BuildingValidationResult validateByTypeFallback(BuildingValidationContext context) { + BuildingValidationRequest request = context.request(); + Map zonesByRole = context.zonesByRole(); + List warnings = context.warnings(); + List blocking = context.blocking(); + return switch (request.type()) { - case STARTER_FORT -> validateStarterFort(level, request, zonesByRole, warnings, blocking); - case HOUSE -> validateHouse(level, request, zonesByRole, warnings, blocking); - case FARM -> validateFarm(level, request, zonesByRole, warnings, blocking); - case MINE -> validateMine(level, request, zonesByRole, warnings, blocking); - case LUMBER_CAMP -> validateLumberCamp(level, request, zonesByRole, warnings, blocking); - case SMITHY -> validateSmithy(level, request, zonesByRole, warnings, blocking); - case STORAGE -> validateStorage(level, request, zonesByRole, warnings, blocking); - case ARCHITECT_WORKSHOP -> validateArchitectWorkshop(level, request, zonesByRole, warnings, blocking); - case BARRACKS -> validateBarracks(level, request, zonesByRole, warnings, blocking); + case STARTER_FORT -> validateStarterFort(context.level(), request, zonesByRole, warnings, blocking); + case HOUSE -> validateHouse(context.level(), request, zonesByRole, warnings, blocking); + case FARM -> validateFarm(context.level(), request, zonesByRole, warnings, blocking); + case MINE -> validateMine(context.level(), request, zonesByRole, warnings, blocking); + case LUMBER_CAMP -> validateLumberCamp(context.level(), request, zonesByRole, warnings, blocking); + case SMITHY -> validateSmithy(context.level(), request, zonesByRole, warnings, blocking); + case STORAGE -> validateStorage(context.level(), request, zonesByRole, warnings, blocking); + case ARCHITECT_WORKSHOP -> validateArchitectWorkshop(context.level(), request, zonesByRole, warnings, blocking); + case BARRACKS -> validateBarracks(context.level(), request, zonesByRole, warnings, blocking); default -> BuildingValidationResult.blockingFailure(request.type(), "validator_not_implemented", "Validator pipeline for this building type is not implemented yet."); }; } diff --git a/src/main/java/com/talhanation/bannermod/settlement/validation/types/BuildingTypeValidator.java b/src/main/java/com/talhanation/bannermod/settlement/validation/types/BuildingTypeValidator.java new file mode 100644 index 00000000..3fc69cd3 --- /dev/null +++ b/src/main/java/com/talhanation/bannermod/settlement/validation/types/BuildingTypeValidator.java @@ -0,0 +1,7 @@ +package com.talhanation.bannermod.settlement.validation.types; + +import com.talhanation.bannermod.settlement.validation.BuildingValidationResult; + +public interface BuildingTypeValidator { + BuildingValidationResult validate(BuildingValidationContext context); +} diff --git a/src/main/java/com/talhanation/bannermod/settlement/validation/types/BuildingTypeValidatorDispatcher.java b/src/main/java/com/talhanation/bannermod/settlement/validation/types/BuildingTypeValidatorDispatcher.java new file mode 100644 index 00000000..380756e5 --- /dev/null +++ b/src/main/java/com/talhanation/bannermod/settlement/validation/types/BuildingTypeValidatorDispatcher.java @@ -0,0 +1,38 @@ +package com.talhanation.bannermod.settlement.validation.types; + +import com.talhanation.bannermod.settlement.building.BuildingType; +import com.talhanation.bannermod.settlement.validation.BuildingValidationResult; + +import java.util.EnumMap; +import java.util.Map; +import java.util.Objects; +import java.util.Optional; +import java.util.function.Function; + +public final class BuildingTypeValidatorDispatcher { + private final EnumMap validators = new EnumMap<>(BuildingType.class); + + public BuildingTypeValidatorDispatcher() { + } + + public BuildingTypeValidatorDispatcher(Map validators) { + validators.forEach(this::register); + } + + public void register(BuildingType type, BuildingTypeValidator validator) { + this.validators.put(Objects.requireNonNull(type), Objects.requireNonNull(validator)); + } + + public Optional validatorFor(BuildingType type) { + return Optional.ofNullable(this.validators.get(type)); + } + + public BuildingValidationResult validate(BuildingValidationContext context, + Function fallback) { + Objects.requireNonNull(context); + Objects.requireNonNull(fallback); + + BuildingTypeValidator validator = this.validators.get(context.request().type()); + return validator == null ? fallback.apply(context) : validator.validate(context); + } +} diff --git a/src/main/java/com/talhanation/bannermod/settlement/validation/types/BuildingValidationContext.java b/src/main/java/com/talhanation/bannermod/settlement/validation/types/BuildingValidationContext.java new file mode 100644 index 00000000..6880aa7b --- /dev/null +++ b/src/main/java/com/talhanation/bannermod/settlement/validation/types/BuildingValidationContext.java @@ -0,0 +1,21 @@ +package com.talhanation.bannermod.settlement.validation.types; + +import com.talhanation.bannermod.settlement.building.ZoneRole; +import com.talhanation.bannermod.settlement.building.ZoneSelection; +import com.talhanation.bannermod.settlement.validation.BuildingValidationRequest; +import com.talhanation.bannermod.settlement.validation.ValidationIssue; +import net.minecraft.server.level.ServerLevel; +import net.minecraft.world.entity.player.Player; + +import java.util.List; +import java.util.Map; + +public record BuildingValidationContext( + ServerLevel level, + Player player, + BuildingValidationRequest request, + Map zonesByRole, + List warnings, + List blocking +) { +} diff --git a/src/test/java/com/talhanation/bannermod/settlement/validation/types/BuildingTypeValidatorDispatcherTest.java b/src/test/java/com/talhanation/bannermod/settlement/validation/types/BuildingTypeValidatorDispatcherTest.java new file mode 100644 index 00000000..94f7c3ec --- /dev/null +++ b/src/test/java/com/talhanation/bannermod/settlement/validation/types/BuildingTypeValidatorDispatcherTest.java @@ -0,0 +1,70 @@ +package com.talhanation.bannermod.settlement.validation.types; + +import com.talhanation.bannermod.settlement.building.BuildingType; +import com.talhanation.bannermod.settlement.validation.BuildingValidationRequest; +import com.talhanation.bannermod.settlement.validation.BuildingValidationResult; +import net.minecraft.core.BlockPos; +import org.junit.jupiter.api.Test; + +import java.util.EnumMap; +import java.util.List; +import java.util.Map; +import java.util.UUID; +import java.util.concurrent.atomic.AtomicBoolean; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertTrue; + +class BuildingTypeValidatorDispatcherTest { + @Test + void returnsRegisteredValidatorForEveryBuildingType() { + BuildingTypeValidatorDispatcher dispatcher = new BuildingTypeValidatorDispatcher(); + Map validators = new EnumMap<>(BuildingType.class); + + for (BuildingType type : BuildingType.values()) { + BuildingTypeValidator validator = context -> BuildingValidationResult.blockingFailure( + context.request().type(), + "registered", + "Registered validator used." + ); + validators.put(type, validator); + dispatcher.register(type, validator); + } + + for (BuildingType type : BuildingType.values()) { + assertTrue(dispatcher.validatorFor(type).isPresent()); + assertSame(validators.get(type), dispatcher.validatorFor(type).orElseThrow()); + } + } + + @Test + void fallsBackForUnregisteredBuildingType() { + BuildingTypeValidatorDispatcher dispatcher = new BuildingTypeValidatorDispatcher(); + AtomicBoolean fallbackUsed = new AtomicBoolean(false); + BuildingValidationResult fallbackResult = BuildingValidationResult.blockingFailure( + BuildingType.FARM, + "fallback", + "Fallback validator used." + ); + + BuildingValidationResult result = dispatcher.validate(context(BuildingType.FARM), context -> { + fallbackUsed.set(true); + return fallbackResult; + }); + + assertFalse(dispatcher.validatorFor(BuildingType.FARM).isPresent()); + assertTrue(fallbackUsed.get()); + assertSame(fallbackResult, result); + } + + private static BuildingValidationContext context(BuildingType type) { + BuildingValidationRequest request = new BuildingValidationRequest( + new UUID(0L, 0L), + type, + BlockPos.ZERO, + List.of() + ); + return new BuildingValidationContext(null, null, request, Map.of(), List.of(), List.of()); + } +} From a23506897c9d628262d41b8500b577c44476df51 Mon Sep 17 00:00:00 2001 From: "pozdn.r.a" Date: Wed, 6 May 2026 19:44:27 +0700 Subject: [PATCH 04/23] backlog: close eventspkg and validator tasks --- docs/BANNERMOD_BACKLOG.json | 42 ++++++++++++++++++++++++++----------- 1 file changed, 30 insertions(+), 12 deletions(-) diff --git a/docs/BANNERMOD_BACKLOG.json b/docs/BANNERMOD_BACKLOG.json index d2ebf4bc..f44164e2 100644 --- a/docs/BANNERMOD_BACKLOG.json +++ b/docs/BANNERMOD_BACKLOG.json @@ -8143,8 +8143,8 @@ { "id": "BLDGVALIDATOR-002", "title": "Extract BuildingTypeValidator strategy interface + dispatcher", - "status": "open", - "updated": "2026-05-05", + "status": "done", + "updated": "2026-05-06", "why": "Phase 1 of the BLDGVALIDATOR-001 split: introduce the strategy seam before per-type validators land. Without the dispatcher, per-type validators have no entry point.", "scope": [ "Define interface BuildingTypeValidator under settlement/validation/types/ with a single validate(BuildingValidationContext) method.", @@ -8160,8 +8160,14 @@ ], "dependencies": [], "progress": [], - "verification": [], - "evidence": [] + "verification": [ + { + "date": "2026-05-06", + "result": "1) BuildingTypeValidator and BuildingTypeValidatorDispatcher exist under settlement/validation/types; dispatcher is 38 LOC. 2) DefaultBuildingValidator now constructs a BuildingValidationContext and dispatches through BuildingTypeValidatorDispatcher before the fallback switch, preserving behavior with zero registered validators. 3) BuildingTypeValidatorDispatcherTest covers every BuildingType registration plus unregistered fallback; ./gradlew compileJava, ./gradlew test, and ./gradlew compileGametestJava passed on the task branch and the integrated branch. Full runGameTestServer was retried and failed only unrelated required tests fiverecruitformationholdsacrossdimensionteleport, authoredroutecouriermovesitemsbetweenstorageendpoints, and starterbootstrapseedsrealworkerassignmentsandwaitingreasons, with no validation gametest failures reported. 4) tools/backlog validate passed." + } + ], + "evidence": [], + "doneDate": "2026-05-06" }, { "id": "BLDGVALIDATOR-003", @@ -8376,8 +8382,8 @@ { "id": "EVENTSPKG-002", "title": "Move SettlementHeartbeatService + ClaimRuntimeService out of events/ into settlement/runtime/", - "status": "open", - "updated": "2026-05-05", + "status": "done", + "updated": "2026-05-06", "why": "EVENTSPKG-001 phase: these are services, not @SubscribeEvent classes — they belong in their subsystem runtime/ folder. Migrating per-service keeps each PR small and reviewable.", "scope": [ "Move events/SettlementHeartbeatService.java to settlement/runtime/SettlementHeartbeatService.java.", @@ -8392,14 +8398,20 @@ ], "dependencies": [], "progress": [], - "verification": [], - "evidence": [] + "verification": [ + { + "date": "2026-05-06", + "result": "1) events/ no longer contains ClaimRuntimeService or SettlementHeartbeatService; both are under settlement/runtime and ClaimEvents imports the new package. 2) Reference check found only new runtime packages; ./gradlew compileJava and ./gradlew test passed on the task branch and the integrated branch. 3) tools/backlog validate passed." + } + ], + "evidence": [], + "doneDate": "2026-05-06" }, { "id": "EVENTSPKG-003", "title": "Move CitizenBirthService out of events/ into citizen/runtime/", - "status": "open", - "updated": "2026-05-05", + "status": "done", + "updated": "2026-05-06", "why": "EVENTSPKG-001 phase: per-service migration for the citizen subsystem.", "scope": [ "Move events/CitizenBirthService.java to citizen/runtime/CitizenBirthService.java.", @@ -8413,8 +8425,14 @@ ], "dependencies": [], "progress": [], - "verification": [], - "evidence": [] + "verification": [ + { + "date": "2026-05-06", + "result": "1) events/ no longer contains CitizenBirthService; it is under citizen/runtime. 2) Reference check found imports updated to com.talhanation.bannermod.citizen.runtime; ./gradlew compileJava and ./gradlew test passed on the task branch and the integrated branch. 3) tools/backlog validate passed." + } + ], + "evidence": [], + "doneDate": "2026-05-06" }, { "id": "EVENTSPKG-004", From d1657b705a1840a7beed16bca097e4034ec86ba8 Mon Sep 17 00:00:00 2001 From: "pozdn.r.a" Date: Thu, 7 May 2026 06:09:56 +0700 Subject: [PATCH 05/23] move movement formation service out of events --- .../MovementFormationCommandService.java | 47 ++++++++++--------- .../bannermod/events/CommandEvents.java | 1 + .../MovementFormationCommandServiceTest.java | 1 + 3 files changed, 26 insertions(+), 23 deletions(-) rename src/main/java/com/talhanation/bannermod/{events => army/command/runtime}/MovementFormationCommandService.java (88%) diff --git a/src/main/java/com/talhanation/bannermod/events/MovementFormationCommandService.java b/src/main/java/com/talhanation/bannermod/army/command/runtime/MovementFormationCommandService.java similarity index 88% rename from src/main/java/com/talhanation/bannermod/events/MovementFormationCommandService.java rename to src/main/java/com/talhanation/bannermod/army/command/runtime/MovementFormationCommandService.java index 9eb7a61b..aa1eb373 100644 --- a/src/main/java/com/talhanation/bannermod/events/MovementFormationCommandService.java +++ b/src/main/java/com/talhanation/bannermod/army/command/runtime/MovementFormationCommandService.java @@ -1,4 +1,4 @@ -package com.talhanation.bannermod.events; +package com.talhanation.bannermod.army.command.runtime; import com.talhanation.bannermod.ai.military.controller.RecruitCommandStateTransitions; import com.talhanation.bannermod.army.command.MovementCommandState; @@ -7,6 +7,7 @@ import com.talhanation.bannermod.entity.military.AbstractRecruitEntity; import com.talhanation.bannermod.entity.military.CaptainEntity; import com.talhanation.bannermod.entity.military.RecruitIndex; +import com.talhanation.bannermod.events.RecruitEvents; import com.talhanation.bannermod.persistence.military.RecruitsGroup; import com.talhanation.bannermod.util.RuntimeProfilingCounters; import com.talhanation.bannermod.util.FormationUtils; @@ -29,7 +30,7 @@ import java.util.Objects; import java.util.UUID; -final class MovementFormationCommandService { +public final class MovementFormationCommandService { private static final String ACTIVE_GROUPS_KEY = "ActiveGroups"; private static final String FORMATION_KEY = "Formation"; @@ -37,15 +38,15 @@ final class MovementFormationCommandService { private MovementFormationCommandService() { } - static void onMovementCommand(Player player, List recruits, int movementState, int formation) { + public static void onMovementCommand(Player player, List recruits, int movementState, int formation) { onMovementCommand(player, recruits, movementState, formation, false); } - static void onMovementCommand(Player player, List recruits, int movementState, int formation, boolean tight) { + public static void onMovementCommand(Player player, List recruits, int movementState, int formation, boolean tight) { onMovementCommand(player, recruits, movementState, formation, tight, null); } - static void onMovementCommand(Player player, List recruits, int movementState, int formation, boolean tight, @Nullable Vec3 explicitTargetPos) { + public static void onMovementCommand(Player player, List recruits, int movementState, int formation, boolean tight, @Nullable Vec3 explicitTargetPos) { if (formation != 0 && MovementCommandState.usesFormationTarget(movementState)) { Vec3 targetPos = null; @@ -137,11 +138,11 @@ static void onMovementCommand(Player player, List recruit } } - static void applyFormation(int formation, List recruits, Player player, Vec3 targetPos) { + public static void applyFormation(int formation, List recruits, Player player, Vec3 targetPos) { applyFormation(formation, recruits, player, targetPos, false); } - static void applyFormation(int formation, List recruits, Player player, Vec3 targetPos, boolean tight) { + public static void applyFormation(int formation, List recruits, Player player, Vec3 targetPos, boolean tight) { saveFormationCenter(player, targetPos); double spacingMultiplier = tight ? 0.5 : 1.0; @@ -158,7 +159,7 @@ static void applyFormation(int formation, List recruits, } } - static void onFaceCommand(Player player, List recruits, int formation, boolean tight) { + public static void onFaceCommand(Player player, List recruits, int formation, boolean tight) { if (recruits.isEmpty()) { return; } @@ -186,7 +187,7 @@ static void onFaceCommand(Player player, List recruits, i } } - static void onMovementCommandGUI(AbstractRecruitEntity recruit, int movementState) { + public static void onMovementCommandGUI(AbstractRecruitEntity recruit, int movementState) { int state = recruit.getFollowState(); switch (movementState) { @@ -230,7 +231,7 @@ static void onMovementCommandGUI(AbstractRecruitEntity recruit, int movementStat recruit.forcedUpkeep = false; } - static void checkPatrolLeaderState(AbstractRecruitEntity recruit) { + public static void checkPatrolLeaderState(AbstractRecruitEntity recruit) { if (recruit instanceof AbstractLeaderEntity leader) { AbstractLeaderEntity.State patrolState = AbstractLeaderEntity.State.fromIndex(leader.getPatrollingState()); AbstractLeaderEntity.State nextState = RecruitCommandStateTransitions.afterManualMovement(patrolState); @@ -243,7 +244,7 @@ static void checkPatrolLeaderState(AbstractRecruitEntity recruit) { } } - static void onServerPlayerTick(ServerPlayer serverPlayer) { + public static void onServerPlayerTick(ServerPlayer serverPlayer) { int formation = getSavedFormation(serverPlayer); if (formation <= 0) { return; @@ -281,12 +282,12 @@ static void onServerPlayerTick(ServerPlayer serverPlayer) { saveFormationPos(serverPlayer, new int[]{(int) targetPosition.x, (int) targetPosition.z}); } - static void initializePlayerCommandState(Player player) { + public static void initializePlayerCommandState(Player player) { CompoundTag playerData = player.getPersistentData(); initializePlayerCommandState(playerData, (int) player.getX(), (int) player.getZ(), RecruitsServerConfig.MaxRecruitsForPlayer.get()); } - static void initializePlayerCommandState(CompoundTag playerData, int playerX, int playerZ, int maxRecruits) { + public static void initializePlayerCommandState(CompoundTag playerData, int playerX, int playerZ, int maxRecruits) { CompoundTag data = playerData.getCompound(Player.PERSISTED_NBT_TAG); if (!data.contains("MaxRecruits")) { @@ -311,12 +312,12 @@ static void initializePlayerCommandState(CompoundTag playerData, int playerX, in playerData.put(Player.PERSISTED_NBT_TAG, data); } - static void copyPersistentCommandPreferences(Player original, Player clone) { + public static void copyPersistentCommandPreferences(Player original, Player clone) { copyPersistentCommandPreferences(original.getPersistentData(), clone.getPersistentData()); initializePlayerCommandState(clone); } - static void copyPersistentCommandPreferences(CompoundTag originalPlayerData, CompoundTag clonePlayerData) { + public static void copyPersistentCommandPreferences(CompoundTag originalPlayerData, CompoundTag clonePlayerData) { CompoundTag originalData = originalPlayerData.getCompound(Player.PERSISTED_NBT_TAG); CompoundTag cloneData = clonePlayerData.getCompound(Player.PERSISTED_NBT_TAG); @@ -330,17 +331,17 @@ static void copyPersistentCommandPreferences(CompoundTag originalPlayerData, Com clonePlayerData.put(Player.PERSISTED_NBT_TAG, cloneData); } - static int getSavedFormation(Player player) { + public static int getSavedFormation(Player player) { return getPersistedData(player).getInt(FORMATION_KEY); } - static void saveFormation(Player player, int formation) { + public static void saveFormation(Player player, int formation) { CompoundTag persisted = getPersistedData(player); persisted.putInt(FORMATION_KEY, formation); savePersistedData(player, persisted); } - static void saveUUIDList(Player player, String key, Collection uuids) { + public static void saveUUIDList(Player player, String key, Collection uuids) { CompoundTag persisted = getPersistedData(player); ListTag list = new ListTag(); @@ -354,7 +355,7 @@ static void saveUUIDList(Player player, String key, Collection uuids) { savePersistedData(player, persisted); } - static List getSavedUUIDList(Player player, String key) { + public static List getSavedUUIDList(Player player, String key) { CompoundTag persisted = getPersistedData(player); List result = new ArrayList<>(); if (!persisted.contains(key, Tag.TAG_LIST)) { @@ -372,17 +373,17 @@ static List getSavedUUIDList(Player player, String key) { return result; } - static int[] getSavedFormationPos(Player player) { + public static int[] getSavedFormationPos(Player player) { return getPersistedData(player).getIntArray("FormationPos"); } - static void saveFormationPos(Player player, int[] pos) { + public static void saveFormationPos(Player player, int[] pos) { CompoundTag persisted = getPersistedData(player); persisted.putIntArray("FormationPos", pos); savePersistedData(player, persisted); } - static void saveFormationCenter(Player player, Vec3 center) { + public static void saveFormationCenter(Player player, Vec3 center) { CompoundTag persisted = getPersistedData(player); persisted.putDouble("FormationCenterX", center.x); persisted.putDouble("FormationCenterY", center.y); @@ -391,7 +392,7 @@ static void saveFormationCenter(Player player, Vec3 center) { } @Nullable - static Vec3 getSavedFormationCenter(Player player) { + public static Vec3 getSavedFormationCenter(Player player) { CompoundTag persisted = getPersistedData(player); if (!persisted.contains("FormationCenterX")) { return null; diff --git a/src/main/java/com/talhanation/bannermod/events/CommandEvents.java b/src/main/java/com/talhanation/bannermod/events/CommandEvents.java index aa36d916..a87e056d 100644 --- a/src/main/java/com/talhanation/bannermod/events/CommandEvents.java +++ b/src/main/java/com/talhanation/bannermod/events/CommandEvents.java @@ -2,6 +2,7 @@ import com.talhanation.bannermod.bootstrap.BannerModMain; import com.talhanation.bannermod.ai.military.CombatStance; +import com.talhanation.bannermod.army.command.runtime.MovementFormationCommandService; import com.talhanation.bannermod.army.command.RecruitSelectionRegistry; import com.talhanation.bannermod.client.military.ClientManager; import com.talhanation.bannermod.entity.military.*; diff --git a/src/test/java/com/talhanation/bannermod/events/MovementFormationCommandServiceTest.java b/src/test/java/com/talhanation/bannermod/events/MovementFormationCommandServiceTest.java index 07d4fd99..b1859e3a 100644 --- a/src/test/java/com/talhanation/bannermod/events/MovementFormationCommandServiceTest.java +++ b/src/test/java/com/talhanation/bannermod/events/MovementFormationCommandServiceTest.java @@ -1,5 +1,6 @@ package com.talhanation.bannermod.events; +import com.talhanation.bannermod.army.command.runtime.MovementFormationCommandService; import net.minecraft.nbt.CompoundTag; import net.minecraft.nbt.ListTag; import net.minecraft.nbt.Tag; From afd2701ad9be21688fe3aa99bc8a0feed928057c Mon Sep 17 00:00:00 2001 From: "pozdn.r.a" Date: Thu, 7 May 2026 06:10:00 +0700 Subject: [PATCH 06/23] fix back-to-mount packet owner authority --- .../military/MessageBackToMountEntity.java | 16 +++++++++++++++- .../MilitaryPacketActorIdentityTest.java | 15 +++++++++++++++ 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/talhanation/bannermod/network/messages/military/MessageBackToMountEntity.java b/src/main/java/com/talhanation/bannermod/network/messages/military/MessageBackToMountEntity.java index dac7cea4..1dab8b6f 100644 --- a/src/main/java/com/talhanation/bannermod/network/messages/military/MessageBackToMountEntity.java +++ b/src/main/java/com/talhanation/bannermod/network/messages/military/MessageBackToMountEntity.java @@ -37,8 +37,9 @@ public PacketFlow getExecutingSide() { public void executeServerSide(BannerModNetworkContext context) { context.enqueueWork(() -> { ServerPlayer player = Objects.requireNonNull(context.getSender()); + UUID actorUuid = authorizedPlayerUuid(player.getUUID(), this.uuid); List recruits = this.group == null - ? RecruitIndex.instance().ownerInRange(player.getCommandSenderWorld(), this.uuid, player.position(), 100.0D) + ? RecruitIndex.instance().ownerInRange(player.getCommandSenderWorld(), actorUuid, player.position(), 100.0D) : RecruitIndex.instance().groupInRange(player.getCommandSenderWorld(), this.group, player.position(), 100.0D); if (recruits == null) { RuntimeProfilingCounters.increment("recruit.index.fallback_scans"); @@ -47,11 +48,24 @@ public void executeServerSide(BannerModNetworkContext context) { player.getBoundingBox().inflate(100) ); } + if (this.group == null) { + recruits = recruits.stream() + .filter(recruit -> recruit != null && isAuthorizedOwner(recruit.getOwnerUUID(), actorUuid)) + .toList(); + } CommandIntentDispatcher.dispatch(player, new CommandIntent.SiegeMachine( player.level().getGameTime(), CommandIntentPriority.HIGH, false, null, group, true), recruits); }); } + static UUID authorizedPlayerUuid(UUID senderUuid, UUID ignoredWireUuid) { + return senderUuid; + } + + static boolean isAuthorizedOwner(UUID recruitOwnerUuid, UUID actorUuid) { + return actorUuid != null && actorUuid.equals(recruitOwnerUuid); + } + public MessageBackToMountEntity fromBytes(FriendlyByteBuf buf) { this.uuid = buf.readUUID(); this.group = buf.readUUID(); diff --git a/src/test/java/com/talhanation/bannermod/network/messages/military/MilitaryPacketActorIdentityTest.java b/src/test/java/com/talhanation/bannermod/network/messages/military/MilitaryPacketActorIdentityTest.java index 35b59813..fc43ca5d 100644 --- a/src/test/java/com/talhanation/bannermod/network/messages/military/MilitaryPacketActorIdentityTest.java +++ b/src/test/java/com/talhanation/bannermod/network/messages/military/MilitaryPacketActorIdentityTest.java @@ -5,6 +5,9 @@ import java.util.UUID; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; class MilitaryPacketActorIdentityTest { @Test @@ -30,4 +33,16 @@ void upkeepIgnoresSpoofedWireUuid() { assertEquals(sender, MessageUpkeepEntity.authorizedPlayerUuid(sender, spoofed)); } + + @Test + void backToMountUsesSenderUuidSoForgedUuidCannotDispatchSiegeIntentToVictimRecruits() { + UUID sender = UUID.randomUUID(); + UUID forgedVictim = UUID.randomUUID(); + + UUID authorizedOwner = MessageBackToMountEntity.authorizedPlayerUuid(sender, forgedVictim); + assertEquals(sender, authorizedOwner); + assertNotEquals(forgedVictim, authorizedOwner); + assertTrue(MessageBackToMountEntity.isAuthorizedOwner(sender, authorizedOwner)); + assertFalse(MessageBackToMountEntity.isAuthorizedOwner(forgedVictim, authorizedOwner)); + } } From 165245e8a53bcf1e1306f95188edaf747294dd55 Mon Sep 17 00:00:00 2001 From: "pozdn.r.a" Date: Thu, 7 May 2026 06:10:04 +0700 Subject: [PATCH 07/23] add movement packet thread-safety gametest --- .../NetworkThreadEnqueueGameTests.java | 96 ++++++++++++++++++- 1 file changed, 95 insertions(+), 1 deletion(-) diff --git a/src/gametest/java/com/talhanation/bannermod/network/NetworkThreadEnqueueGameTests.java b/src/gametest/java/com/talhanation/bannermod/network/NetworkThreadEnqueueGameTests.java index eebe0117..3686e408 100644 --- a/src/gametest/java/com/talhanation/bannermod/network/NetworkThreadEnqueueGameTests.java +++ b/src/gametest/java/com/talhanation/bannermod/network/NetworkThreadEnqueueGameTests.java @@ -3,7 +3,10 @@ import com.talhanation.bannermod.BannerModDedicatedServerGameTestSupport; import com.talhanation.bannermod.bootstrap.BannerModMain; import com.talhanation.bannermod.network.compat.BannerModNetworkContext; +import com.talhanation.bannermod.network.messages.military.MessageMovement; import com.talhanation.bannermod.network.messages.military.MessageUpkeepPos; +import com.talhanation.bannermod.network.throttle.PacketRateLimitConfig; +import com.talhanation.bannermod.network.throttle.PacketRateLimiter; import net.minecraft.core.BlockPos; import net.minecraft.gametest.framework.GameTest; import net.minecraft.gametest.framework.GameTestHelper; @@ -22,10 +25,10 @@ import net.neoforged.neoforge.gametest.PrefixGameTestTemplate; import net.neoforged.neoforge.network.handling.IPayloadContext; +import java.util.ArrayList; import java.util.Collections; import java.util.List; import java.util.UUID; -import java.util.ArrayList; import java.util.concurrent.CompletableFuture; import java.util.concurrent.atomic.AtomicInteger; import java.util.function.Supplier; @@ -67,6 +70,8 @@ public class NetworkThreadEnqueueGameTests { private static final int DISPATCHES_PER_WORKER = 250; private static final int TOTAL_DISPATCHES = WORKER_COUNT * DISPATCHES_PER_WORKER; private static final UUID SENDER_UUID = UUID.fromString("00000000-0000-0000-0000-0000feed0001"); + private static final UUID MOVEMENT_SENDER_UUID = UUID.fromString("00000000-0000-0000-0000-0000feed0002"); + private static final UUID MOVEMENT_GROUP_UUID = UUID.fromString("00000000-0000-0000-0000-0000feed1002"); @PrefixGameTestTemplate(false) @GameTest(template = "harness_empty", timeoutTicks = 1200) @@ -148,6 +153,95 @@ public static void thousandConcurrentDispatchesAllRunOnMainThreadWithNoException }); } + @PrefixGameTestTemplate(false) + @GameTest(template = "harness_empty", timeoutTicks = 1200) + public static void thousandConcurrentMovementPacketsDoNotTouchEntityCollectionsOffThread(GameTestHelper helper) { + ServerLevel level = helper.getLevel(); + MinecraftServer server = level.getServer(); + helper.assertTrue(server != null, "Gametest must run inside a real MinecraftServer context"); + + ServerPlayer sender = (ServerPlayer) BannerModDedicatedServerGameTestSupport + .createFakeServerPlayer(level, MOVEMENT_SENDER_UUID, "movement-enqueue-test-sender"); + + AtomicInteger completed = new AtomicInteger(); + List errors = Collections.synchronizedList(new ArrayList<>()); + List runnerThreadNames = Collections.synchronizedList(new ArrayList<>()); + + IPayloadContext deferringContext = new MainThreadDeferringContext( + sender, + server, + completed, + errors, + runnerThreadNames + ); + BannerModNetworkContext bannerCtx = new BannerModNetworkContext(deferringContext); + + PacketRateLimiter limiter = PacketRateLimiter.shared(); + limiter.setCooldownSource(packetClass -> packetClass == MessageMovement.class ? 0L : -1L); + limiter.clearState(); + + try { + Thread[] workers = new Thread[WORKER_COUNT]; + for (int t = 0; t < WORKER_COUNT; t++) { + workers[t] = new Thread(() -> { + for (int i = 0; i < DISPATCHES_PER_WORKER; i++) { + try { + MessageMovement msg = new MessageMovement( + MOVEMENT_SENDER_UUID, + 6, + MOVEMENT_GROUP_UUID, + 0, + false + ); + msg.executeServerSide(bannerCtx); + } catch (Throwable t1) { + errors.add(t1); + } + } + }, "movement-enqueue-test-worker-" + t); + workers[t].setDaemon(true); + workers[t].start(); + } + for (Thread w : workers) { + try { + w.join(10_000L); + } catch (InterruptedException ie) { + Thread.currentThread().interrupt(); + helper.fail("Worker thread join interrupted"); + } + helper.assertTrue(!w.isAlive(), "Worker thread did not finish dispatching within 10s"); + } + } finally { + PacketRateLimitConfig.install(); + limiter.clearState(); + } + + String mainThreadName = server.getRunningThread().getName(); + helper.succeedWhen(() -> { + helper.assertTrue( + errors.isEmpty(), + "Expected zero movement packet exceptions; first was: " + + (errors.isEmpty() ? "" : errors.get(0).toString()) + ); + int done = completed.get(); + helper.assertTrue( + done == TOTAL_DISPATCHES, + "Expected " + TOTAL_DISPATCHES + " movement packet bodies to complete; got " + done + ); + List snapshot; + synchronized (runnerThreadNames) { + snapshot = new ArrayList<>(runnerThreadNames); + } + boolean allOnMain = snapshot.stream().allMatch(name -> name.equals(mainThreadName)); + helper.assertTrue( + allOnMain, + "Every movement body must run on the main server thread (" + mainThreadName + + "); observed distinct threads: " + + snapshot.stream().distinct().sorted().toList() + ); + }); + } + /** * Test-only {@link IPayloadContext} whose {@code enqueueWork} defers to * the server main-thread executor. Records each runnable's runner thread From 4c57ff3b275c3f2ff76785c9ed160eaf27db8fd4 Mon Sep 17 00:00:00 2001 From: "pozdn.r.a" Date: Thu, 7 May 2026 06:12:33 +0700 Subject: [PATCH 08/23] backlog: close eventspkg packetauth testnetthread --- docs/BANNERMOD_BACKLOG.json | 42 ++++++++++++++++++++++++++----------- 1 file changed, 30 insertions(+), 12 deletions(-) diff --git a/docs/BANNERMOD_BACKLOG.json b/docs/BANNERMOD_BACKLOG.json index f44164e2..c9dd424a 100644 --- a/docs/BANNERMOD_BACKLOG.json +++ b/docs/BANNERMOD_BACKLOG.json @@ -7460,8 +7460,8 @@ { "id": "TESTNETTHREAD-001", "title": "Network-thread safety regression test", - "status": "open", - "updated": "2026-05-04", + "status": "done", + "updated": "2026-05-07", "why": "Once ENQUEUE-001 lands, this guards against future regressions of the wrap. Source: review Part A 11.2 #9.", "scope": [ "Test that runs 1000 movement packets in parallel from a fake netty pool and asserts zero CME on entity collections" @@ -7473,8 +7473,14 @@ "ENQUEUE-001" ], "progress": [], - "verification": [], - "evidence": [] + "verification": [ + { + "date": "2026-05-07", + "result": "1) NetworkThreadEnqueueGameTests now dispatches 1000 real MessageMovement packets from worker threads through a deferring IPayloadContext and asserts zero exceptions, 1000 completed bodies, and all bodies executed on the server main thread, so bypassing enqueueWork leaves completion below 1000 or records worker-thread execution. 2) ./gradlew compileGametestJava and focused NetworkThreadGuardTest passed; integrated ./gradlew compileJava, ./gradlew test, and ./gradlew compileGametestJava passed. 3) Integrated runGameTestServer executed 161 tests; the new movement-thread test was not listed as failing, while unrelated authoredroutecouriermovesitemsbetweenstorageendpoints, starterbootstrapseedsrealworkerassignmentsandwaitingreasons, and unrelatedclaimstateispreservedwhensiblingclaimisdeleted failed. 4) tools/backlog validate passed." + } + ], + "evidence": [], + "doneDate": "2026-05-07" }, { "id": "TESTMIGRATE-001", @@ -8437,8 +8443,8 @@ { "id": "EVENTSPKG-004", "title": "Move MovementFormationCommandService out of events/ into army/command/runtime/", - "status": "open", - "updated": "2026-05-05", + "status": "done", + "updated": "2026-05-07", "why": "EVENTSPKG-001 phase: per-service migration for the army-command subsystem.", "scope": [ "Move events/MovementFormationCommandService.java to army/command/runtime/MovementFormationCommandService.java.", @@ -8452,8 +8458,14 @@ ], "dependencies": [], "progress": [], - "verification": [], - "evidence": [] + "verification": [ + { + "date": "2026-05-07", + "result": "1) events/ no longer contains MovementFormationCommandService; the class is under army/command/runtime. 2) CommandEvents and MovementFormationCommandServiceTest import the new runtime package; reference search found no old events package reference. 3) ./gradlew compileJava and ./gradlew test passed on the task branch and integrated branch. 4) tools/backlog validate passed." + } + ], + "evidence": [], + "doneDate": "2026-05-07" }, { "id": "EVENTSPKG-005", @@ -8485,8 +8497,8 @@ { "id": "PACKETAUTH-001", "title": "Verify sender owns recruits in MessageBackToMountEntity", - "status": "open", - "updated": "2026-05-06", + "status": "done", + "updated": "2026-05-07", "why": "MessageBackToMountEntity reads this.uuid (intended-owner UUID) from the wire and passes it to RecruitIndex.ownerInRange(...) to gather recruits, then dispatches a SiegeMachine intent. No equality check vs context.getSender().getUUID(); a malicious client can target another player's recruits in range.", "scope": [ "src/main/java/com/talhanation/bannermod/network/messages/military/MessageBackToMountEntity.java executeServerSide; either drop this.uuid in favour of player.getUUID() or short-circuit when sender.getUUID()!=this.uuid (mirror MessageRest/Shields/UpkeepEntity authorizedPlayerUuid pattern)." @@ -8496,8 +8508,14 @@ ], "dependencies": [], "progress": [], - "verification": [], - "evidence": [] + "verification": [ + { + "date": "2026-05-07", + "result": "1) MessageBackToMountEntity now derives actorUuid from context sender UUID and uses it for ownerInRange, ignoring the wire UUID. 2) Owner-scope fallback scans are filtered through isAuthorizedOwner so forged victim-owned recruits are excluded before SiegeMachine dispatch. 3) MilitaryPacketActorIdentityTest forged-UUID regression asserts sender is authorized and forged victim owner is rejected; focused test and full ./gradlew test passed. 4) tools/backlog validate passed." + } + ], + "evidence": [], + "doneDate": "2026-05-07" }, { "id": "PACKETAUTH-002", From a776c39b51dcaadfc50ed3cf61a58d29ce61a418 Mon Sep 17 00:00:00 2001 From: "pozdn.r.a" Date: Thu, 7 May 2026 06:27:09 +0700 Subject: [PATCH 09/23] fix clear upkeep packet owner authority --- .../messages/military/MessageClearUpkeep.java | 18 ++++++++++++++++-- .../MilitaryPacketActorIdentityTest.java | 12 ++++++++++++ 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/talhanation/bannermod/network/messages/military/MessageClearUpkeep.java b/src/main/java/com/talhanation/bannermod/network/messages/military/MessageClearUpkeep.java index f4a49213..cef77ec3 100644 --- a/src/main/java/com/talhanation/bannermod/network/messages/military/MessageClearUpkeep.java +++ b/src/main/java/com/talhanation/bannermod/network/messages/military/MessageClearUpkeep.java @@ -33,8 +33,9 @@ public PacketFlow getExecutingSide() { public void executeServerSide(BannerModNetworkContext context) { context.enqueueWork(() -> { ServerPlayer player = Objects.requireNonNull(context.getSender()); + UUID actorUuid = authorizedPlayerUuid(player.getUUID(), this.uuid); List recruits = this.group == null - ? RecruitIndex.instance().ownerInRange(player.getCommandSenderWorld(), this.uuid, player.position(), 100.0D) + ? RecruitIndex.instance().ownerInRange(player.getCommandSenderWorld(), actorUuid, player.position(), 100.0D) : RecruitIndex.instance().groupInRange(player.getCommandSenderWorld(), this.group, player.position(), 100.0D); if (recruits == null) { RuntimeProfilingCounters.increment("recruit.index.fallback_scans"); @@ -43,12 +44,25 @@ public void executeServerSide(BannerModNetworkContext context) { player.getBoundingBox().inflate(100) ); } + if (this.group == null) { + recruits = recruits.stream() + .filter(recruit -> recruit != null && isAuthorizedOwner(recruit.getOwnerUUID(), actorUuid)) + .toList(); + } recruits.forEach( - (recruit) -> CommandEvents.onClearUpkeepButton(uuid, recruit, group) + (recruit) -> CommandEvents.onClearUpkeepButton(actorUuid, recruit, group) ); }); } + static UUID authorizedPlayerUuid(UUID senderUuid, UUID ignoredWireUuid) { + return senderUuid; + } + + static boolean isAuthorizedOwner(UUID recruitOwnerUuid, UUID actorUuid) { + return actorUuid != null && actorUuid.equals(recruitOwnerUuid); + } + public MessageClearUpkeep fromBytes(FriendlyByteBuf buf) { this.uuid = buf.readUUID(); this.group = buf.readUUID(); diff --git a/src/test/java/com/talhanation/bannermod/network/messages/military/MilitaryPacketActorIdentityTest.java b/src/test/java/com/talhanation/bannermod/network/messages/military/MilitaryPacketActorIdentityTest.java index fc43ca5d..91c51c10 100644 --- a/src/test/java/com/talhanation/bannermod/network/messages/military/MilitaryPacketActorIdentityTest.java +++ b/src/test/java/com/talhanation/bannermod/network/messages/military/MilitaryPacketActorIdentityTest.java @@ -34,6 +34,18 @@ void upkeepIgnoresSpoofedWireUuid() { assertEquals(sender, MessageUpkeepEntity.authorizedPlayerUuid(sender, spoofed)); } + @Test + void clearUpkeepUsesSenderUuidSoForgedUuidCannotClearVictimRecruits() { + UUID sender = UUID.randomUUID(); + UUID forgedVictim = UUID.randomUUID(); + + UUID authorizedOwner = MessageClearUpkeep.authorizedPlayerUuid(sender, forgedVictim); + assertEquals(sender, authorizedOwner); + assertNotEquals(forgedVictim, authorizedOwner); + assertTrue(MessageClearUpkeep.isAuthorizedOwner(sender, authorizedOwner)); + assertFalse(MessageClearUpkeep.isAuthorizedOwner(forgedVictim, authorizedOwner)); + } + @Test void backToMountUsesSenderUuidSoForgedUuidCannotDispatchSiegeIntentToVictimRecruits() { UUID sender = UUID.randomUUID(); From cadfb2525a23ded7f8b0ea54b0d11471375e1a6c Mon Sep 17 00:00:00 2001 From: "pozdn.r.a" Date: Thu, 7 May 2026 06:27:13 +0700 Subject: [PATCH 10/23] fix mount packet owner authority --- .../messages/military/MessageMountEntity.java | 18 ++++++++++++++++-- .../MilitaryPacketActorIdentityTest.java | 12 ++++++++++++ 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/talhanation/bannermod/network/messages/military/MessageMountEntity.java b/src/main/java/com/talhanation/bannermod/network/messages/military/MessageMountEntity.java index 2112bdd3..abcc2a63 100644 --- a/src/main/java/com/talhanation/bannermod/network/messages/military/MessageMountEntity.java +++ b/src/main/java/com/talhanation/bannermod/network/messages/military/MessageMountEntity.java @@ -47,22 +47,36 @@ public void executeServerSide(BannerModNetworkContext context) { return; } + UUID actorUuid = authorizedPlayerUuid(player.getUUID(), this.uuid); List recruits = this.group == null - ? RecruitIndex.instance().ownerInRange(player.getCommandSenderWorld(), this.uuid, player.position(), 100.0D) + ? RecruitIndex.instance().ownerInRange(player.getCommandSenderWorld(), actorUuid, player.position(), 100.0D) : RecruitIndex.instance().groupInRange(player.getCommandSenderWorld(), this.group, player.position(), 100.0D); if (recruits == null) { RuntimeProfilingCounters.increment("recruit.index.fallback_scans"); recruits = player.getCommandSenderWorld().getEntitiesOfClass( AbstractRecruitEntity.class, player.getBoundingBox().inflate(100), - (recruit) -> recruit.isEffectedByCommand(uuid, group) + (recruit) -> recruit.isEffectedByCommand(actorUuid, group) ); } + recruits = recruits.stream() + .filter(recruit -> recruit != null + && isAuthorizedOwner(recruit.getOwnerUUID(), actorUuid) + && recruit.isEffectedByCommand(actorUuid, group)) + .toList(); CommandIntentDispatcher.dispatch(player, new CommandIntent.SiegeMachine( player.level().getGameTime(), CommandIntentPriority.HIGH, false, target, group, false), recruits); }); } + static UUID authorizedPlayerUuid(UUID senderUuid, UUID ignoredWireUuid) { + return senderUuid; + } + + static boolean isAuthorizedOwner(UUID recruitOwnerUuid, UUID actorUuid) { + return actorUuid != null && actorUuid.equals(recruitOwnerUuid); + } + public MessageMountEntity fromBytes(FriendlyByteBuf buf) { this.uuid = buf.readUUID(); this.target = buf.readUUID(); diff --git a/src/test/java/com/talhanation/bannermod/network/messages/military/MilitaryPacketActorIdentityTest.java b/src/test/java/com/talhanation/bannermod/network/messages/military/MilitaryPacketActorIdentityTest.java index fc43ca5d..ab66054b 100644 --- a/src/test/java/com/talhanation/bannermod/network/messages/military/MilitaryPacketActorIdentityTest.java +++ b/src/test/java/com/talhanation/bannermod/network/messages/military/MilitaryPacketActorIdentityTest.java @@ -45,4 +45,16 @@ void backToMountUsesSenderUuidSoForgedUuidCannotDispatchSiegeIntentToVictimRecru assertTrue(MessageBackToMountEntity.isAuthorizedOwner(sender, authorizedOwner)); assertFalse(MessageBackToMountEntity.isAuthorizedOwner(forgedVictim, authorizedOwner)); } + + @Test + void mountUsesSenderUuidSoForgedUuidCannotDispatchSiegeIntentToVictimRecruits() { + UUID sender = UUID.randomUUID(); + UUID forgedVictim = UUID.randomUUID(); + + UUID authorizedOwner = MessageMountEntity.authorizedPlayerUuid(sender, forgedVictim); + assertEquals(sender, authorizedOwner); + assertNotEquals(forgedVictim, authorizedOwner); + assertTrue(MessageMountEntity.isAuthorizedOwner(sender, authorizedOwner)); + assertFalse(MessageMountEntity.isAuthorizedOwner(forgedVictim, authorizedOwner)); + } } From e93abc3845e03d32f3e51403c51eacf04a28ce66 Mon Sep 17 00:00:00 2001 From: "pozdn.r.a" Date: Thu, 7 May 2026 06:27:23 +0700 Subject: [PATCH 11/23] backlog: split stale settlement treasury refactor --- docs/BANNERMOD_BACKLOG.json | 36 ++++++++++++++++++++++++++++++++---- 1 file changed, 32 insertions(+), 4 deletions(-) diff --git a/docs/BANNERMOD_BACKLOG.json b/docs/BANNERMOD_BACKLOG.json index c9dd424a..2c7560a0 100644 --- a/docs/BANNERMOD_BACKLOG.json +++ b/docs/BANNERMOD_BACKLOG.json @@ -8076,8 +8076,8 @@ { "id": "SETTREFACTOR-003", "title": "Extract SettlementTreasuryDerivationService from BannerModSettlementService", - "status": "open", - "updated": "2026-05-05", + "status": "in_progress", + "updated": "2026-05-07", "why": "Phase 2 of the SETTREFACTOR-001 split: isolate the treasury-derivation hook integration so it can be reasoned about without the rest of the god class.", "scope": [ "Create SettlementTreasuryDerivationService under settlement/runtime/ holding the treasury-hook integration methods currently inside BannerModSettlementService.", @@ -8091,9 +8091,14 @@ "./gradlew compileJava + ./gradlew test green; tools/backlog validate passes." ], "dependencies": [ - "SETTREFACTOR-002" + "SETTREFACTOR-003A" + ], + "progress": [ + { + "date": "2026-05-07", + "text": "Attempted SETTREFACTOR-003 on feature/settrefactor-003; live code inspection found no treasury/fiscal/ledger/tax-hook methods in BannerModSettlementService or BannerModSettlementServiceTest, so the original extraction target is stale. Remaining scope moved to SETTREFACTOR-003A, targeting the current SettlementHeartbeatService governor-heartbeat treasury integration seam." + } ], - "progress": [], "verification": [], "evidence": [] }, @@ -8864,6 +8869,29 @@ "progress": [], "verification": [], "evidence": [] + }, + { + "id": "SETTREFACTOR-003A", + "title": "Extract settlement treasury heartbeat derivation service", + "status": "open", + "updated": "2026-05-07", + "why": "SETTREFACTOR-003 targeted treasury-hook methods in BannerModSettlementService, but current code no longer has treasury references there. The live treasury integration now sits in SettlementHeartbeatService's governor heartbeat path, so the extraction must target the actual runtime seam.", + "scope": [ + "Create SettlementTreasuryDerivationService under settlement/runtime/ for the live governor-heartbeat treasury integration currently embedded in SettlementHeartbeatService.", + "Route SettlementHeartbeatService through the new service without changing heartbeat timing, batch counters, or settlement refresh/orchestrator stages.", + "Add focused SettlementTreasuryDerivationServiceTest coverage, or move existing heartbeat treasury assertions if present, so the extracted seam is covered." + ], + "acceptance": [ + "settlement/runtime/SettlementTreasuryDerivationService.java exists and owns the BannerModTreasuryManager-backed governor heartbeat treasury derivation call.", + "SettlementHeartbeatService no longer imports or directly calls BannerModTreasuryManager for the treasury derivation hook, while its batch counter keys and stage transitions remain unchanged.", + "SettlementTreasuryDerivationServiceTest covers the extracted seam; ./gradlew compileJava and ./gradlew test pass; tools/backlog validate passes." + ], + "dependencies": [ + "SETTREFACTOR-002" + ], + "progress": [], + "verification": [], + "evidence": [] } ] } From bc8235e11221cabef517cd990b9a843ebe636db0 Mon Sep 17 00:00:00 2001 From: "pozdn.r.a" Date: Thu, 7 May 2026 06:28:46 +0700 Subject: [PATCH 12/23] backlog: close mount and upkeep auth tasks --- docs/BANNERMOD_BACKLOG.json | 28 ++++++++++++++++++++-------- 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/docs/BANNERMOD_BACKLOG.json b/docs/BANNERMOD_BACKLOG.json index 2c7560a0..1c7665e9 100644 --- a/docs/BANNERMOD_BACKLOG.json +++ b/docs/BANNERMOD_BACKLOG.json @@ -8525,8 +8525,8 @@ { "id": "PACKETAUTH-002", "title": "Verify sender in MessageClearUpkeep", - "status": "open", - "updated": "2026-05-06", + "status": "done", + "updated": "2026-05-07", "why": "MessageClearUpkeep reads this.uuid as the recruit-owner UUID, passes it both to RecruitIndex.ownerInRange(...) and to CommandEvents.onClearUpkeepButton(uuid, recruit, group). No equality check vs sender.getUUID(); any client can clear upkeep on another player's recruits in range.", "scope": [ "src/main/java/com/talhanation/bannermod/network/messages/military/MessageClearUpkeep.java executeServerSide; substitute sender.getUUID() (mirror MessageUpkeepEntity.authorizedPlayerUuid) or reject when this.uuid != sender.getUUID()." @@ -8536,14 +8536,20 @@ ], "dependencies": [], "progress": [], - "verification": [], - "evidence": [] + "verification": [ + { + "date": "2026-05-07", + "result": "1) MessageClearUpkeep now derives actorUuid from context sender UUID and uses it for ownerInRange and CommandEvents.onClearUpkeepButton instead of the wire UUID. 2) Owner-scope fallback scans are filtered through isAuthorizedOwner so forged victim-owned recruits are excluded. 3) MilitaryPacketActorIdentityTest forged-UUID regression asserts sender authorization and forged victim rejection; ./gradlew compileJava and ./gradlew test passed on the task branch and integrated branch. 4) tools/backlog validate passed." + } + ], + "evidence": [], + "doneDate": "2026-05-07" }, { "id": "PACKETAUTH-003", "title": "Verify sender in MessageMountEntity", - "status": "open", - "updated": "2026-05-06", + "status": "done", + "updated": "2026-05-07", "why": "MessageMountEntity reads this.uuid as the recruit-owner UUID and passes it to RecruitIndex.ownerInRange(...) and to recruit.isEffectedByCommand in the fallback predicate. Sender authority is never validated; a client can mount-command another player's recruits onto a target entity in range.", "scope": [ "src/main/java/com/talhanation/bannermod/network/messages/military/MessageMountEntity.java executeServerSide; replace this.uuid with sender.getUUID() in both lookup branches and predicate, or short-circuit on mismatch." @@ -8553,8 +8559,14 @@ ], "dependencies": [], "progress": [], - "verification": [], - "evidence": [] + "verification": [ + { + "date": "2026-05-07", + "result": "1) MessageMountEntity now derives actorUuid from context sender UUID and uses it for ownerInRange plus fallback isEffectedByCommand checks instead of the wire UUID. 2) Candidate recruits are filtered by sender-owned isAuthorizedOwner and isEffectedByCommand before SiegeMachine dispatch, excluding forged victim-owned recruits. 3) MilitaryPacketActorIdentityTest forged-UUID regression asserts sender authorization and forged victim rejection; ./gradlew compileJava and ./gradlew test passed on the task branch and integrated branch. 4) tools/backlog validate passed." + } + ], + "evidence": [], + "doneDate": "2026-05-07" }, { "id": "PACKETAUTH-004", From b7e60e7f6c113ad8aeb99ce9e36f63f2d6be6f30 Mon Sep 17 00:00:00 2001 From: "pozdn.r.a" Date: Thu, 7 May 2026 06:37:41 +0700 Subject: [PATCH 13/23] fix protect packet owner authority --- .../military/MessageProtectEntity.java | 19 +++++++++++++++++-- .../MilitaryPacketActorIdentityTest.java | 12 ++++++++++++ 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/talhanation/bannermod/network/messages/military/MessageProtectEntity.java b/src/main/java/com/talhanation/bannermod/network/messages/military/MessageProtectEntity.java index 78c4d7cc..a1acd259 100644 --- a/src/main/java/com/talhanation/bannermod/network/messages/military/MessageProtectEntity.java +++ b/src/main/java/com/talhanation/bannermod/network/messages/military/MessageProtectEntity.java @@ -38,8 +38,9 @@ public PacketFlow getExecutingSide() { public void executeServerSide(BannerModNetworkContext context){ context.enqueueWork(() -> { ServerPlayer player = Objects.requireNonNull(context.getSender()); + UUID actorUuid = authorizedPlayerUuid(player.getUUID(), this.uuid); List recruits = this.group == null - ? RecruitIndex.instance().ownerInRange(player.getCommandSenderWorld(), this.uuid, player.position(), 100.0D) + ? RecruitIndex.instance().ownerInRange(player.getCommandSenderWorld(), actorUuid, player.position(), 100.0D) : RecruitIndex.instance().groupInRange(player.getCommandSenderWorld(), this.group, player.position(), 100.0D); if (recruits == null) { RuntimeProfilingCounters.increment("recruit.index.fallback_scans"); @@ -48,9 +49,23 @@ public void executeServerSide(BannerModNetworkContext context){ player.getBoundingBox().inflate(100) ); } - recruits.forEach((recruit) -> CommandEvents.onProtectButton(uuid, recruit, target, group)); + if (this.group == null) { + recruits = recruits.stream() + .filter(recruit -> recruit != null && isAuthorizedOwner(recruit.getOwnerUUID(), actorUuid)) + .toList(); + } + recruits.forEach((recruit) -> CommandEvents.onProtectButton(actorUuid, recruit, target, group)); }); } + + static UUID authorizedPlayerUuid(UUID senderUuid, UUID ignoredWireUuid) { + return senderUuid; + } + + static boolean isAuthorizedOwner(UUID recruitOwnerUuid, UUID actorUuid) { + return actorUuid != null && actorUuid.equals(recruitOwnerUuid); + } + public MessageProtectEntity fromBytes(FriendlyByteBuf buf) { this.uuid = buf.readUUID(); this.target = buf.readUUID(); diff --git a/src/test/java/com/talhanation/bannermod/network/messages/military/MilitaryPacketActorIdentityTest.java b/src/test/java/com/talhanation/bannermod/network/messages/military/MilitaryPacketActorIdentityTest.java index 04eab255..c911fbca 100644 --- a/src/test/java/com/talhanation/bannermod/network/messages/military/MilitaryPacketActorIdentityTest.java +++ b/src/test/java/com/talhanation/bannermod/network/messages/military/MilitaryPacketActorIdentityTest.java @@ -69,4 +69,16 @@ void mountUsesSenderUuidSoForgedUuidCannotDispatchSiegeIntentToVictimRecruits() assertTrue(MessageMountEntity.isAuthorizedOwner(sender, authorizedOwner)); assertFalse(MessageMountEntity.isAuthorizedOwner(forgedVictim, authorizedOwner)); } + + @Test + void protectUsesSenderUuidSoForgedUuidCannotOrderVictimRecruits() { + UUID sender = UUID.randomUUID(); + UUID forgedVictim = UUID.randomUUID(); + + UUID authorizedOwner = MessageProtectEntity.authorizedPlayerUuid(sender, forgedVictim); + assertEquals(sender, authorizedOwner); + assertNotEquals(forgedVictim, authorizedOwner); + assertTrue(MessageProtectEntity.isAuthorizedOwner(sender, authorizedOwner)); + assertFalse(MessageProtectEntity.isAuthorizedOwner(forgedVictim, authorizedOwner)); + } } From 502722310cc26fbc3a83b2ca9fe04d500416b891 Mon Sep 17 00:00:00 2001 From: "pozdn.r.a" Date: Thu, 7 May 2026 06:37:46 +0700 Subject: [PATCH 14/23] fix group update ownership authority --- .../GroupAssignmentAuthorityGameTests.java | 28 +++++++++++++++++++ .../military/RecruitsGroupsManager.java | 11 ++++++-- 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/src/gametest/java/com/talhanation/bannermod/network/messages/military/GroupAssignmentAuthorityGameTests.java b/src/gametest/java/com/talhanation/bannermod/network/messages/military/GroupAssignmentAuthorityGameTests.java index 787d73c6..59601d38 100644 --- a/src/gametest/java/com/talhanation/bannermod/network/messages/military/GroupAssignmentAuthorityGameTests.java +++ b/src/gametest/java/com/talhanation/bannermod/network/messages/military/GroupAssignmentAuthorityGameTests.java @@ -30,6 +30,9 @@ public class GroupAssignmentAuthorityGameTests { private static final UUID TRANSFER_GROUP_UUID = UUID.fromString("00000000-0000-0000-0000-000000000824"); private static final UUID SPOOFED_TRANSFER_GROUP_UUID = UUID.fromString("00000000-0000-0000-0000-000000000825"); private static final UUID TRUSTED_NEW_OWNER_UUID = UUID.fromString("00000000-0000-0000-0000-000000000826"); + private static final UUID UPDATE_GROUP_UUID = UUID.fromString("00000000-0000-0000-0000-000000000827"); + private static final UUID UPDATE_OWNER_UUID = UUID.fromString("00000000-0000-0000-0000-000000000828"); + private static final UUID UPDATE_OUTSIDER_UUID = UUID.fromString("00000000-0000-0000-0000-000000000829"); @PrefixGameTestTemplate(false) @GameTest(template = "harness_empty") @@ -118,6 +121,31 @@ public static void groupTransferUpdatesGroupAndMembersFromTrustedPlayer(GameTest helper.succeed(); } + @PrefixGameTestTemplate(false) + @GameTest(template = "harness_empty") + public static void groupUpdateRejectsNonOwnerAndKeepsExistingGroup(GameTestHelper helper) { + ServerLevel level = helper.getLevel(); + ServerPlayer owner = createPlayer(helper, level, UPDATE_OWNER_UUID, "update-owner"); + ServerPlayer outsider = createPlayer(helper, level, UPDATE_OUTSIDER_UUID, "update-outsider"); + + RecruitsGroup group = new RecruitsGroup("Owner Group", owner, 1); + group.setUUID(UPDATE_GROUP_UUID); + RecruitEvents.groupsManager().addOrUpdateGroup(level, owner, group); + + RecruitsGroup spoofedUpdate = new RecruitsGroup("Spoofed Group", outsider, 9); + spoofedUpdate.setUUID(UPDATE_GROUP_UUID); + spoofedUpdate.removed = true; + RecruitEvents.groupsManager().addOrUpdateGroup(level, outsider, spoofedUpdate); + + RecruitsGroup saved = RecruitEvents.groupsManager().getGroup(UPDATE_GROUP_UUID); + helper.assertTrue(saved != null, "Expected denied update to keep existing group"); + helper.assertTrue(UPDATE_OWNER_UUID.equals(saved.getPlayerUUID()), "Expected denied update to keep owner UUID"); + helper.assertTrue("update-owner".equals(saved.getPlayerName()), "Expected denied update to keep owner name"); + helper.assertTrue("Owner Group".equals(saved.getName()), "Expected denied update to keep group name"); + helper.assertFalse(saved.removed, "Expected denied update not to remove existing group"); + helper.succeed(); + } + private static ServerPlayer createPlayer(GameTestHelper helper, ServerLevel level, UUID playerId, String name) { Player player = BannerModDedicatedServerGameTestSupport.createFakeServerPlayer(level, playerId, name); BlockPos pos = helper.absolutePos(RecruitsBattleGameTestSupport.SquadAnchor.WEST.anchor()); diff --git a/src/main/java/com/talhanation/bannermod/persistence/military/RecruitsGroupsManager.java b/src/main/java/com/talhanation/bannermod/persistence/military/RecruitsGroupsManager.java index 940783ee..d490fc2b 100644 --- a/src/main/java/com/talhanation/bannermod/persistence/military/RecruitsGroupsManager.java +++ b/src/main/java/com/talhanation/bannermod/persistence/military/RecruitsGroupsManager.java @@ -38,14 +38,22 @@ public void save(ServerLevel level) { data.setDirty(); } public void addOrUpdateGroup(ServerLevel level, ServerPlayer player, RecruitsGroup incoming) { - if (incoming == null || level == null) return; + if (incoming == null || level == null || player == null) return; UUID id = resolveGroup(incoming.getUUID()); RecruitsGroup existing = groupMap.get(id); if (existing != null) { + if (!player.hasPermissions(2) && !player.getUUID().equals(existing.getPlayerUUID())) { + return; + } + incoming.members = existing.members; incoming.setUUID(existing.getUUID()); + incoming.setPlayer(new RecruitsPlayerInfo(existing.getPlayerUUID(), existing.getPlayerName())); + } + else { + incoming.setPlayer(player); } removeGroup(id); @@ -316,4 +324,3 @@ private String getNextGroupName(String baseName) { } } - From 11f3ba8d0f904861c894d9ffda33fc9bcd9d78d0 Mon Sep 17 00:00:00 2001 From: "pozdn.r.a" Date: Thu, 7 May 2026 06:37:53 +0700 Subject: [PATCH 15/23] gate debug GUI recruit mutations --- .../messages/military/MessageDebugGui.java | 24 ++++++++++++++ .../MessageDebugGuiAuthorityTest.java | 31 +++++++++++++++++++ 2 files changed, 55 insertions(+) create mode 100644 src/test/java/com/talhanation/bannermod/network/messages/military/MessageDebugGuiAuthorityTest.java diff --git a/src/main/java/com/talhanation/bannermod/network/messages/military/MessageDebugGui.java b/src/main/java/com/talhanation/bannermod/network/messages/military/MessageDebugGui.java index d3363623..c4f82ffd 100644 --- a/src/main/java/com/talhanation/bannermod/network/messages/military/MessageDebugGui.java +++ b/src/main/java/com/talhanation/bannermod/network/messages/military/MessageDebugGui.java @@ -1,5 +1,8 @@ package com.talhanation.bannermod.network.messages.military; +import com.talhanation.bannermod.army.command.CommandHierarchy; +import com.talhanation.bannermod.army.command.CommandRole; +import com.talhanation.bannermod.army.command.RecruitCommandAuthority; import com.talhanation.bannermod.events.DebugEvents; import com.talhanation.bannermod.entity.military.AbstractRecruitEntity; import com.talhanation.bannermod.network.payload.BannerModMessage; @@ -36,12 +39,33 @@ public void executeServerSide(BannerModNetworkContext context) { ServerPlayer player = Objects.requireNonNull(context.getSender()); AbstractRecruitEntity recruit = RecruitMessageEntityResolver.resolveRecruitInInflatedBox(player, this.uuid, 16.0D); if (recruit != null) { + if (!shouldHandleDebugMessage(id, player, recruit)) { + return; + } + DebugEvents.handleMessage(id, recruit, context.getSender()); recruit.setCustomName(Component.literal(name)); } }); } + static boolean shouldHandleDebugMessage(int id, ServerPlayer player, AbstractRecruitEntity recruit) { + return hasDebugAuthority(player, recruit); + } + + static boolean shouldHandleDebugMessage(int id, UUID senderUuid, String senderTeamName, boolean senderOp, UUID recruitOwnerUuid, String recruitTeamName, boolean recruitOwned) { + return hasDebugAuthority(senderUuid, senderTeamName, senderOp, recruitOwnerUuid, recruitTeamName, recruitOwned); + } + + static boolean hasDebugAuthority(ServerPlayer player, AbstractRecruitEntity recruit) { + return player.hasPermissions(2) || RecruitCommandAuthority.canDirectlyControl(player, recruit); + } + + static boolean hasDebugAuthority(UUID senderUuid, String senderTeamName, boolean senderOp, UUID recruitOwnerUuid, String recruitTeamName, boolean recruitOwned) { + return senderOp + || CommandHierarchy.roleFor(senderUuid, senderTeamName, false, recruitOwnerUuid, recruitTeamName, recruitOwned) != CommandRole.NONE; + } + public MessageDebugGui fromBytes(FriendlyByteBuf buf) { this.id = buf.readInt(); this.uuid = buf.readUUID(); diff --git a/src/test/java/com/talhanation/bannermod/network/messages/military/MessageDebugGuiAuthorityTest.java b/src/test/java/com/talhanation/bannermod/network/messages/military/MessageDebugGuiAuthorityTest.java new file mode 100644 index 00000000..36d225f5 --- /dev/null +++ b/src/test/java/com/talhanation/bannermod/network/messages/military/MessageDebugGuiAuthorityTest.java @@ -0,0 +1,31 @@ +package com.talhanation.bannermod.network.messages.military; + +import org.junit.jupiter.api.Test; + +import java.util.UUID; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +class MessageDebugGuiAuthorityTest { + private static final UUID OWNER = UUID.fromString("00000000-0000-0000-0000-000000000a01"); + private static final UUID OUTSIDER = UUID.fromString("00000000-0000-0000-0000-000000000a02"); + + @Test + void nonOwnerNonOpCannotUseDebugScreenToHealKillOrDisbandForeignRecruit() { + int[] mutatingDebugActions = {14, 15, 26}; + + for (int debugAction : mutatingDebugActions) { + assertFalse( + MessageDebugGui.shouldHandleDebugMessage(debugAction, OUTSIDER, null, false, OWNER, null, true), + "debug action " + debugAction + " must require owner/direct-control or op authority" + ); + } + } + + @Test + void ownerAndOpCanUseDebugScreen() { + assertTrue(MessageDebugGui.shouldHandleDebugMessage(14, OWNER, null, false, OWNER, null, true)); + assertTrue(MessageDebugGui.shouldHandleDebugMessage(15, OUTSIDER, null, true, OWNER, null, true)); + } +} From 01494fd22749ed1129404adf8acd35334dc0b236 Mon Sep 17 00:00:00 2001 From: "pozdn.r.a" Date: Thu, 7 May 2026 06:40:20 +0700 Subject: [PATCH 16/23] backlog: close protect group debug auth tasks --- docs/BANNERMOD_BACKLOG.json | 42 ++++++++++++++++++++++++++----------- 1 file changed, 30 insertions(+), 12 deletions(-) diff --git a/docs/BANNERMOD_BACKLOG.json b/docs/BANNERMOD_BACKLOG.json index 1c7665e9..c434dbf1 100644 --- a/docs/BANNERMOD_BACKLOG.json +++ b/docs/BANNERMOD_BACKLOG.json @@ -8571,8 +8571,8 @@ { "id": "PACKETAUTH-004", "title": "Verify sender in MessageProtectEntity", - "status": "open", - "updated": "2026-05-06", + "status": "done", + "updated": "2026-05-07", "why": "MessageProtectEntity reads this.uuid as the recruit-owner UUID, passes it to RecruitIndex.ownerInRange(...), and forwards it to CommandEvents.onProtectButton(uuid, recruit, target, group). Sender authority is never checked; any client can issue protect orders to another player's recruits.", "scope": [ "src/main/java/com/talhanation/bannermod/network/messages/military/MessageProtectEntity.java executeServerSide; replace this.uuid with sender.getUUID() (or reject mismatch) in ownerInRange call and onProtectButton actor argument." @@ -8582,14 +8582,20 @@ ], "dependencies": [], "progress": [], - "verification": [], - "evidence": [] + "verification": [ + { + "date": "2026-05-07", + "result": "1) MessageProtectEntity now derives actorUuid from context sender UUID and uses it for ownerInRange plus CommandEvents.onProtectButton instead of the wire UUID. 2) Owner-scope fallback scans are filtered through isAuthorizedOwner so forged victim-owned recruits are excluded. 3) MilitaryPacketActorIdentityTest forged-UUID regression asserts sender authorization and forged victim rejection; ./gradlew compileJava and ./gradlew test passed on the task branch and integrated branch. 4) tools/backlog validate passed." + } + ], + "evidence": [], + "doneDate": "2026-05-07" }, { "id": "PACKETAUTH-005", "title": "Reject foreign-owner edits in MessageUpdateGroup", - "status": "open", - "updated": "2026-05-06", + "status": "done", + "updated": "2026-05-07", "why": "MessageUpdateGroup deserializes a RecruitsGroup from client-supplied NBT (including embedded playerUUID/playerName) and calls RecruitsGroupsManager.addOrUpdateGroup with no sender-vs-existing-owner check. A malicious client can rewrite arbitrary groups (rename, reassign owner, change image, etc.).", "scope": [ "src/main/java/com/talhanation/bannermod/network/messages/military/MessageUpdateGroup.java executeServerSide and/or RecruitsGroupsManager.addOrUpdateGroup; require that an existing group's playerUUID matches the sender (or sender is op) before applying mutations, and clamp/strip the incoming playerUUID/playerName so they cannot be transferred via this packet (transfer should go through MessageAssignGroupToPlayer's audited path)." @@ -8599,14 +8605,20 @@ ], "dependencies": [], "progress": [], - "verification": [], - "evidence": [] + "verification": [ + { + "date": "2026-05-07", + "result": "1) RecruitsGroupsManager.addOrUpdateGroup now rejects existing-group mutations unless the sender owns the existing group or has op permission. 2) Incoming NBT owner fields are clamped: existing group owner/name are preserved on updates, and new groups are assigned to the sender. MessageAssignGroupToPlayer transfer path was left unchanged. 3) GroupAssignmentAuthorityGameTests adds groupUpdateRejectsNonOwnerAndKeepsExistingGroup; compileJava, test, and compileGametestJava passed on the task branch and integrated branch. Integrated runGameTestServer executed 162 tests; the new group update regression was not listed as failing, while unrelated fiverecruitformationholdsacrossdimensionteleport, starterbootstrapseedsrealworkerassignmentsandwaitingreasons, and friendlyclaimbindingallowsplacementandsettlementoperation failed. 4) tools/backlog validate passed." + } + ], + "evidence": [], + "doneDate": "2026-05-07" }, { "id": "PACKETAUTH-006", "title": "Gate MessageDebugGui behind ownership / op", - "status": "open", - "updated": "2026-05-06", + "status": "done", + "updated": "2026-05-07", "why": "MessageDebugGui resolves any recruit within a 16-block AABB by UUID and forwards the action to DebugEvents.handleMessage, which allows kill, disband, heal, XP grants, color/variant edits and more. There is no ownership or permission gate; any client can mutate or destroy any nearby recruit.", "scope": [ "src/main/java/com/talhanation/bannermod/network/messages/military/MessageDebugGui.java executeServerSide and com/talhanation/bannermod/events/DebugEvents.java; require sender.hasPermissions(2) (debug tooling) and/or RecruitCommandAuthority.canDirectlyControl(sender, recruit) before invoking DebugEvents.handleMessage and recruit.setCustomName." @@ -8616,8 +8628,14 @@ ], "dependencies": [], "progress": [], - "verification": [], - "evidence": [] + "verification": [ + { + "date": "2026-05-07", + "result": "1) MessageDebugGui now returns before DebugEvents.handleMessage and recruit.setCustomName unless the sender is op or RecruitCommandAuthority.canDirectlyControl(sender, recruit). 2) MessageDebugGuiAuthorityTest covers heal, kill, and disband actions for non-owner non-op denial plus owner/op allow paths. 3) ./gradlew compileJava and ./gradlew test passed on the task branch and integrated branch. 4) tools/backlog validate passed." + } + ], + "evidence": [], + "doneDate": "2026-05-07" }, { "id": "PACKETAUTH-007", From abf71381afb1210f5daaa653222acda059b49b39 Mon Sep 17 00:00:00 2001 From: "pozdn.r.a" Date: Fri, 8 May 2026 00:28:24 +0700 Subject: [PATCH 17/23] gate mount GUI recruit commands --- .../military/MessageMountEntityGui.java | 5 ++++ .../MessageMountEntityGuiAuthorityTest.java | 26 +++++++++++++++++++ 2 files changed, 31 insertions(+) create mode 100644 src/test/java/com/talhanation/bannermod/network/messages/military/MessageMountEntityGuiAuthorityTest.java diff --git a/src/main/java/com/talhanation/bannermod/network/messages/military/MessageMountEntityGui.java b/src/main/java/com/talhanation/bannermod/network/messages/military/MessageMountEntityGui.java index 83484583..74722791 100644 --- a/src/main/java/com/talhanation/bannermod/network/messages/military/MessageMountEntityGui.java +++ b/src/main/java/com/talhanation/bannermod/network/messages/military/MessageMountEntityGui.java @@ -4,6 +4,7 @@ import com.talhanation.bannermod.army.command.CommandIntent; import com.talhanation.bannermod.army.command.CommandIntentDispatcher; import com.talhanation.bannermod.army.command.CommandIntentPriority; +import com.talhanation.bannermod.army.command.RecruitCommandAuthority; import com.talhanation.bannermod.entity.military.AbstractRecruitEntity; import com.talhanation.bannermod.network.payload.BannerModMessage; import net.minecraft.network.protocol.PacketFlow; @@ -50,6 +51,10 @@ public void executeServerSide(BannerModNetworkContext context) { @SuppressWarnings({"all"}) private void mount(ServerPlayer player, AbstractRecruitEntity recruit) { + if (!RecruitCommandAuthority.canDirectlyControl(player, recruit)) { + return; + } + if (this.back && recruit.getMountUUID() != null) { CommandIntentDispatcher.dispatch(player, new CommandIntent.SiegeMachine( player.level().getGameTime(), CommandIntentPriority.HIGH, false, null, null, true), List.of(recruit)); diff --git a/src/test/java/com/talhanation/bannermod/network/messages/military/MessageMountEntityGuiAuthorityTest.java b/src/test/java/com/talhanation/bannermod/network/messages/military/MessageMountEntityGuiAuthorityTest.java new file mode 100644 index 00000000..125f48c5 --- /dev/null +++ b/src/test/java/com/talhanation/bannermod/network/messages/military/MessageMountEntityGuiAuthorityTest.java @@ -0,0 +1,26 @@ +package com.talhanation.bannermod.network.messages.military; + +import org.junit.jupiter.api.Test; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; + +import static org.junit.jupiter.api.Assertions.assertTrue; + +class MessageMountEntityGuiAuthorityTest { + @Test + void forgedRecruitUuidCannotDispatchSiegeMachineIntentForForeignRecruit() throws IOException { + Path handler = Paths.get("src/main/java/com/talhanation/bannermod/network/messages/military/MessageMountEntityGui.java"); + String source = Files.readString(handler); + + int authorityCheck = source.indexOf("RecruitCommandAuthority.canDirectlyControl(player, recruit)"); + int siegeIntent = source.indexOf("new CommandIntent.SiegeMachine"); + + assertTrue(authorityCheck >= 0, "mount GUI must check direct recruit command authority"); + assertTrue(siegeIntent >= 0, "mount GUI must still dispatch SiegeMachine intents for authorized recruits"); + assertTrue(authorityCheck < siegeIntent, + "a forged recruit UUID must hit the authority check before any SiegeMachine intent can dispatch"); + } +} From 7a20cb3c049eaa8fb7c770789eb080fa86bfdf5a Mon Sep 17 00:00:00 2001 From: "pozdn.r.a" Date: Fri, 8 May 2026 00:28:29 +0700 Subject: [PATCH 18/23] gate patrol waypoint packet authority --- .../MessagePatrolLeaderAddWayPoint.java | 2 + .../PatrolLeaderWaypointAuthorityTest.java | 43 +++++++++++++++++++ 2 files changed, 45 insertions(+) create mode 100644 src/test/java/com/talhanation/bannermod/network/messages/military/PatrolLeaderWaypointAuthorityTest.java diff --git a/src/main/java/com/talhanation/bannermod/network/messages/military/MessagePatrolLeaderAddWayPoint.java b/src/main/java/com/talhanation/bannermod/network/messages/military/MessagePatrolLeaderAddWayPoint.java index bdf8b61d..9a771525 100644 --- a/src/main/java/com/talhanation/bannermod/network/messages/military/MessagePatrolLeaderAddWayPoint.java +++ b/src/main/java/com/talhanation/bannermod/network/messages/military/MessagePatrolLeaderAddWayPoint.java @@ -1,5 +1,6 @@ package com.talhanation.bannermod.network.messages.military; +import com.talhanation.bannermod.army.command.RecruitCommandAuthority; import com.talhanation.bannermod.bootstrap.BannerModMain; import com.talhanation.bannermod.entity.military.AbstractLeaderEntity; import com.talhanation.bannermod.entity.military.CaptainEntity; @@ -48,6 +49,7 @@ public void executeServerSide(BannerModNetworkContext context) { Entity entity = player.serverLevel().getEntity(this.worker); if (entity instanceof AbstractLeaderEntity leader && leader.isAlive() + && RecruitCommandAuthority.canDirectlyControl(player, leader) && player.getBoundingBox().inflate(100.0D).intersects(leader.getBoundingBox())) { this.addWayPoint(new BlockPos(x, y, z), player, leader); } diff --git a/src/test/java/com/talhanation/bannermod/network/messages/military/PatrolLeaderWaypointAuthorityTest.java b/src/test/java/com/talhanation/bannermod/network/messages/military/PatrolLeaderWaypointAuthorityTest.java new file mode 100644 index 00000000..125ab930 --- /dev/null +++ b/src/test/java/com/talhanation/bannermod/network/messages/military/PatrolLeaderWaypointAuthorityTest.java @@ -0,0 +1,43 @@ +package com.talhanation.bannermod.network.messages.military; + +import com.talhanation.bannermod.army.command.CommandHierarchy; +import com.talhanation.bannermod.army.command.CommandRole; +import org.junit.jupiter.api.Test; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.UUID; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +class PatrolLeaderWaypointAuthorityTest { + private static final Path ROOT = Path.of(""); + private static final Path MESSAGE = ROOT.resolve( + "src/main/java/com/talhanation/bannermod/network/messages/military/MessagePatrolLeaderAddWayPoint.java"); + + private static final UUID OWNER = UUID.fromString("00000000-0000-0000-0000-000000000911"); + private static final UUID FOREIGN_SENDER = UUID.fromString("00000000-0000-0000-0000-000000000912"); + + @Test + void forgedForeignLeaderWaypointPacketCannotReachMutation() throws IOException { + assertEquals(CommandRole.NONE, + CommandHierarchy.roleFor(FOREIGN_SENDER, null, false, OWNER, null, true), + "Foreign non-op sender must not directly control another player's leader"); + + String src = Files.readString(MESSAGE); + String authorityGate = "RecruitCommandAuthority.canDirectlyControl(player, leader)"; + String handlerMutation = "this.addWayPoint(new BlockPos(x, y, z), player, leader)"; + + int gateIndex = src.indexOf(authorityGate); + int mutationIndex = src.indexOf(handlerMutation); + + assertTrue(gateIndex >= 0, "Waypoint add handler must use the canonical recruit authority gate"); + assertTrue(mutationIndex >= 0, "Waypoint add handler must still route mutations through addWayPoint"); + assertTrue(gateIndex < mutationIndex, + "Forged packet for a foreign leader must fail authority before waypoint state can change"); + assertEquals(mutationIndex, src.lastIndexOf(handlerMutation), + "The guarded handler path must be the only waypoint-add mutation entry point"); + } +} From a03bf3e2938b580edf07943aa13629f42d0ea486 Mon Sep 17 00:00:00 2001 From: "pozdn.r.a" Date: Fri, 8 May 2026 00:28:35 +0700 Subject: [PATCH 19/23] backlog: split assassin count authority task --- docs/BANNERMOD_BACKLOG.json | 60 ++++++++++++++++++++++++++++++++++--- 1 file changed, 56 insertions(+), 4 deletions(-) diff --git a/docs/BANNERMOD_BACKLOG.json b/docs/BANNERMOD_BACKLOG.json index c434dbf1..11351efc 100644 --- a/docs/BANNERMOD_BACKLOG.json +++ b/docs/BANNERMOD_BACKLOG.json @@ -8657,8 +8657,8 @@ { "id": "PACKETAUTH-008", "title": "Verify leader ownership in MessageAssassinCount", - "status": "open", - "updated": "2026-05-06", + "status": "in_progress", + "updated": "2026-05-08", "why": "MessageAssassinCount targets an AssassinLeaderEntity by UUID and only checks a 16-block AABB before calling leader.setCount(this.count). No ownership check; any client in range can rewrite a foreign assassin leader's count.", "scope": [ "src/main/java/com/talhanation/bannermod/network/messages/military/MessageAssassinCount.java executeServerSide; add ownership / canDirectlyControl gate using leader.getOwnerUUID() vs sender.getUUID() (or sender.hasPermissions(2))." @@ -8666,8 +8666,16 @@ "acceptance": [ "Handler bails when sender does not own the leader; regression test with a foreign assassin leader confirms count is unchanged." ], - "dependencies": [], - "progress": [], + "dependencies": [ + "PACKETAUTH-008A", + "PACKETAUTH-008B" + ], + "progress": [ + { + "date": "2026-05-08", + "text": "Attempted PACKETAUTH-008 on feature/packetauth-008; live code inspection found AssassinLeaderEntity has no existing server-authoritative owner/control source. A packet-side owner gate alone would either block legitimate count edits or invent unsafe authority. Remaining scope split into PACKETAUTH-008A to define/persist AssassinLeader authority and PACKETAUTH-008B to gate MessageAssassinCount once that authority exists." + } + ], "verification": [], "evidence": [] }, @@ -8922,6 +8930,50 @@ "progress": [], "verification": [], "evidence": [] + }, + { + "id": "PACKETAUTH-008A", + "title": "Define server-authoritative AssassinLeader ownership", + "status": "open", + "updated": "2026-05-08", + "why": "PACKETAUTH-008 needs an ownership check, but AssassinLeaderEntity currently has no persisted server-authoritative owner source. Adding a packet-side owner gate without defining how leaders become owned would either block legitimate count edits or let clients claim authority indirectly.", + "scope": [ + "Audit AssassinLeaderEntity spawn, interaction, menu-open, and count-update flows to choose the server-owned authority source for assassin leaders.", + "Implement and persist the chosen owner/control field without trusting client-supplied packet data.", + "Add focused tests proving the owner/control field survives save/load or the relevant server lifecycle path." + ], + "acceptance": [ + "AssassinLeaderEntity has a documented server-authoritative owner/control source that is assigned outside MessageAssassinCount and persisted if the entity persists.", + "Tests prove the owner/control field is assigned through the server-side flow and cannot be set by client count packets.", + "./gradlew compileJava and ./gradlew test pass; tools/backlog validate passes." + ], + "dependencies": [], + "progress": [], + "verification": [], + "evidence": [] + }, + { + "id": "PACKETAUTH-008B", + "title": "Gate MessageAssassinCount by AssassinLeader authority", + "status": "open", + "updated": "2026-05-08", + "why": "After PACKETAUTH-008A defines trustworthy AssassinLeader authority, MessageAssassinCount can safely reject foreign sender mutations without breaking legitimate users.", + "scope": [ + "Update MessageAssassinCount executeServerSide to reject senders that do not own/control the target AssassinLeaderEntity and are not op.", + "Keep the existing range/AABB validation intact.", + "Add a forged-sender regression test proving a foreign non-op leaves count unchanged, plus an owner or op allow-path test." + ], + "acceptance": [ + "MessageAssassinCount bails when sender does not own/control the leader and lacks op permission.", + "Regression test with a foreign assassin leader confirms count is unchanged; owner/op allow path still updates count.", + "./gradlew compileJava and ./gradlew test pass; tools/backlog validate passes." + ], + "dependencies": [ + "PACKETAUTH-008A" + ], + "progress": [], + "verification": [], + "evidence": [] } ] } From 8dced34da9786fe63ca72de4e9e82687c9627b07 Mon Sep 17 00:00:00 2001 From: "pozdn.r.a" Date: Fri, 8 May 2026 00:29:55 +0700 Subject: [PATCH 20/23] backlog: close mount GUI and patrol auth tasks --- docs/BANNERMOD_BACKLOG.json | 28 ++++++++++++++++++++-------- 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/docs/BANNERMOD_BACKLOG.json b/docs/BANNERMOD_BACKLOG.json index 11351efc..7f69e30c 100644 --- a/docs/BANNERMOD_BACKLOG.json +++ b/docs/BANNERMOD_BACKLOG.json @@ -8640,8 +8640,8 @@ { "id": "PACKETAUTH-007", "title": "Verify recruit ownership in MessageMountEntityGui", - "status": "open", - "updated": "2026-05-06", + "status": "done", + "updated": "2026-05-08", "why": "MessageMountEntityGui resolves any recruit within 32 blocks by UUID and dispatches a SiegeMachine mount intent without any ownership / canDirectlyControl check. A client can mount-control any visible recruit (own or foreign).", "scope": [ "src/main/java/com/talhanation/bannermod/network/messages/military/MessageMountEntityGui.java executeServerSide / mount(); add RecruitCommandAuthority.canDirectlyControl(sender, recruit) before the dispatch (mirror MessageAggroGui/MessageDismountGui)." @@ -8651,8 +8651,14 @@ ], "dependencies": [], "progress": [], - "verification": [], - "evidence": [] + "verification": [ + { + "date": "2026-05-08", + "result": "1) MessageMountEntityGui.mount now checks RecruitCommandAuthority.canDirectlyControl(player, recruit) before any SiegeMachine intent dispatch. 2) MessageMountEntityGuiAuthorityTest confirms the authority check appears before SiegeMachine dispatch, covering forged foreign recruit UUID denial. 3) ./gradlew compileJava and ./gradlew test passed on the task branch and integrated branch. 4) tools/backlog validate passed." + } + ], + "evidence": [], + "doneDate": "2026-05-08" }, { "id": "PACKETAUTH-008", @@ -8682,8 +8688,8 @@ { "id": "PACKETAUTH-009", "title": "Verify leader ownership in MessagePatrolLeaderAddWayPoint", - "status": "open", - "updated": "2026-05-06", + "status": "done", + "updated": "2026-05-08", "why": "MessagePatrolLeaderAddWayPoint resolves an AbstractLeaderEntity by UUID and only verifies a range AABB / distance before calling leader.addWayPoint(...). No ownership check; any client in range can mutate a foreign player's leader entity (waypoints, cycle, speed, info mode, etc.).", "scope": [ "src/main/java/com/talhanation/bannermod/network/messages/military/MessagePatrolLeaderAddWayPoint.java executeServerSide; add canDirectlyControl(sender, leader) (or equivalent owner-equality check via leader.getOwnerUUID()) before the mutation." @@ -8693,8 +8699,14 @@ ], "dependencies": [], "progress": [], - "verification": [], - "evidence": [] + "verification": [ + { + "date": "2026-05-08", + "result": "1) MessagePatrolLeaderAddWayPoint now requires RecruitCommandAuthority.canDirectlyControl(player, leader) before addWayPoint mutation. 2) PatrolLeaderWaypointAuthorityTest confirms foreign non-op has no command role and the authority gate occurs before the only waypoint mutation path. 3) ./gradlew compileJava and ./gradlew test passed on the task branch and integrated branch. 4) tools/backlog validate passed." + } + ], + "evidence": [], + "doneDate": "2026-05-08" }, { "id": "PACKETAUTH-010", From daa67be072ba1412ad6e016bc81ffab7fc70d3f2 Mon Sep 17 00:00:00 2001 From: "pozdn.r.a" Date: Fri, 8 May 2026 00:46:52 +0700 Subject: [PATCH 21/23] gate patrol waypoint removal authority --- .../MessagePatrolLeaderRemoveWayPoint.java | 2 ++ .../PatrolLeaderWaypointAuthorityTest.java | 23 +++++++++++++++++++ 2 files changed, 25 insertions(+) diff --git a/src/main/java/com/talhanation/bannermod/network/messages/military/MessagePatrolLeaderRemoveWayPoint.java b/src/main/java/com/talhanation/bannermod/network/messages/military/MessagePatrolLeaderRemoveWayPoint.java index 130ccdbc..1b1bd578 100644 --- a/src/main/java/com/talhanation/bannermod/network/messages/military/MessagePatrolLeaderRemoveWayPoint.java +++ b/src/main/java/com/talhanation/bannermod/network/messages/military/MessagePatrolLeaderRemoveWayPoint.java @@ -1,5 +1,6 @@ package com.talhanation.bannermod.network.messages.military; +import com.talhanation.bannermod.army.command.RecruitCommandAuthority; import com.talhanation.bannermod.bootstrap.BannerModMain; import com.talhanation.bannermod.entity.military.AbstractLeaderEntity; import com.talhanation.bannermod.network.payload.BannerModMessage; @@ -35,6 +36,7 @@ public void executeServerSide(BannerModNetworkContext context) { Entity entity = player.serverLevel().getEntity(this.worker); if (entity instanceof AbstractLeaderEntity leader && leader.isAlive() + && RecruitCommandAuthority.canDirectlyControl(player, leader) && player.getBoundingBox().inflate(100.0D).intersects(leader.getBoundingBox())) { this.removeLastWayPoint(player, leader); } diff --git a/src/test/java/com/talhanation/bannermod/network/messages/military/PatrolLeaderWaypointAuthorityTest.java b/src/test/java/com/talhanation/bannermod/network/messages/military/PatrolLeaderWaypointAuthorityTest.java index 125ab930..b11fe55b 100644 --- a/src/test/java/com/talhanation/bannermod/network/messages/military/PatrolLeaderWaypointAuthorityTest.java +++ b/src/test/java/com/talhanation/bannermod/network/messages/military/PatrolLeaderWaypointAuthorityTest.java @@ -16,6 +16,8 @@ class PatrolLeaderWaypointAuthorityTest { private static final Path ROOT = Path.of(""); private static final Path MESSAGE = ROOT.resolve( "src/main/java/com/talhanation/bannermod/network/messages/military/MessagePatrolLeaderAddWayPoint.java"); + private static final Path REMOVE_MESSAGE = ROOT.resolve( + "src/main/java/com/talhanation/bannermod/network/messages/military/MessagePatrolLeaderRemoveWayPoint.java"); private static final UUID OWNER = UUID.fromString("00000000-0000-0000-0000-000000000911"); private static final UUID FOREIGN_SENDER = UUID.fromString("00000000-0000-0000-0000-000000000912"); @@ -40,4 +42,25 @@ void forgedForeignLeaderWaypointPacketCannotReachMutation() throws IOException { assertEquals(mutationIndex, src.lastIndexOf(handlerMutation), "The guarded handler path must be the only waypoint-add mutation entry point"); } + + @Test + void forgedForeignLeaderRemoveWaypointPacketCannotReachMutation() throws IOException { + assertEquals(CommandRole.NONE, + CommandHierarchy.roleFor(FOREIGN_SENDER, null, false, OWNER, null, true), + "Foreign non-op sender must not directly control another player's leader"); + + String src = Files.readString(REMOVE_MESSAGE); + String authorityGate = "RecruitCommandAuthority.canDirectlyControl(player, leader)"; + String handlerMutation = "this.removeLastWayPoint(player, leader)"; + + int gateIndex = src.indexOf(authorityGate); + int mutationIndex = src.indexOf(handlerMutation); + + assertTrue(gateIndex >= 0, "Waypoint remove handler must use the canonical recruit authority gate"); + assertTrue(mutationIndex >= 0, "Waypoint remove handler must still route mutations through removeLastWayPoint"); + assertTrue(gateIndex < mutationIndex, + "Forged packet for a foreign leader must fail authority before waypoint state can change"); + assertEquals(mutationIndex, src.lastIndexOf(handlerMutation), + "The guarded handler path must be the only waypoint-remove mutation entry point"); + } } From 256696b9fbded043d6fd1541255bf586634734da Mon Sep 17 00:00:00 2001 From: "pozdn.r.a" Date: Fri, 8 May 2026 00:46:57 +0700 Subject: [PATCH 22/23] gate patrol cycle packet authority --- .../military/MessagePatrolLeaderSetCycle.java | 5 +++- .../PatrolLeaderWaypointAuthorityTest.java | 27 +++++++++++++++++-- 2 files changed, 29 insertions(+), 3 deletions(-) diff --git a/src/main/java/com/talhanation/bannermod/network/messages/military/MessagePatrolLeaderSetCycle.java b/src/main/java/com/talhanation/bannermod/network/messages/military/MessagePatrolLeaderSetCycle.java index f8477e49..c2902fbd 100644 --- a/src/main/java/com/talhanation/bannermod/network/messages/military/MessagePatrolLeaderSetCycle.java +++ b/src/main/java/com/talhanation/bannermod/network/messages/military/MessagePatrolLeaderSetCycle.java @@ -1,5 +1,6 @@ package com.talhanation.bannermod.network.messages.military; +import com.talhanation.bannermod.army.command.RecruitCommandAuthority; import com.talhanation.bannermod.entity.military.AbstractLeaderEntity; import com.talhanation.bannermod.network.payload.BannerModMessage; import net.minecraft.network.protocol.PacketFlow; @@ -33,7 +34,9 @@ public void executeServerSide(BannerModNetworkContext context) { context.enqueueWork(() -> { ServerPlayer player = Objects.requireNonNull(context.getSender()); Entity entity = player.serverLevel().getEntity(this.recruit); - if (entity instanceof AbstractLeaderEntity leader && leader.distanceToSqr(player) <= 100.0D * 100.0D) { + if (entity instanceof AbstractLeaderEntity leader + && RecruitCommandAuthority.canDirectlyControl(player, leader) + && leader.distanceToSqr(player) <= 100.0D * 100.0D) { leader.setCycle(this.cycle); } }); diff --git a/src/test/java/com/talhanation/bannermod/network/messages/military/PatrolLeaderWaypointAuthorityTest.java b/src/test/java/com/talhanation/bannermod/network/messages/military/PatrolLeaderWaypointAuthorityTest.java index 125ab930..eb7cae53 100644 --- a/src/test/java/com/talhanation/bannermod/network/messages/military/PatrolLeaderWaypointAuthorityTest.java +++ b/src/test/java/com/talhanation/bannermod/network/messages/military/PatrolLeaderWaypointAuthorityTest.java @@ -14,8 +14,10 @@ class PatrolLeaderWaypointAuthorityTest { private static final Path ROOT = Path.of(""); - private static final Path MESSAGE = ROOT.resolve( + private static final Path ADD_WAYPOINT_MESSAGE = ROOT.resolve( "src/main/java/com/talhanation/bannermod/network/messages/military/MessagePatrolLeaderAddWayPoint.java"); + private static final Path SET_CYCLE_MESSAGE = ROOT.resolve( + "src/main/java/com/talhanation/bannermod/network/messages/military/MessagePatrolLeaderSetCycle.java"); private static final UUID OWNER = UUID.fromString("00000000-0000-0000-0000-000000000911"); private static final UUID FOREIGN_SENDER = UUID.fromString("00000000-0000-0000-0000-000000000912"); @@ -26,7 +28,7 @@ void forgedForeignLeaderWaypointPacketCannotReachMutation() throws IOException { CommandHierarchy.roleFor(FOREIGN_SENDER, null, false, OWNER, null, true), "Foreign non-op sender must not directly control another player's leader"); - String src = Files.readString(MESSAGE); + String src = Files.readString(ADD_WAYPOINT_MESSAGE); String authorityGate = "RecruitCommandAuthority.canDirectlyControl(player, leader)"; String handlerMutation = "this.addWayPoint(new BlockPos(x, y, z), player, leader)"; @@ -40,4 +42,25 @@ void forgedForeignLeaderWaypointPacketCannotReachMutation() throws IOException { assertEquals(mutationIndex, src.lastIndexOf(handlerMutation), "The guarded handler path must be the only waypoint-add mutation entry point"); } + + @Test + void forgedForeignLeaderCyclePacketCannotReachMutation() throws IOException { + assertEquals(CommandRole.NONE, + CommandHierarchy.roleFor(FOREIGN_SENDER, null, false, OWNER, null, true), + "Foreign non-op sender must not directly control another player's leader"); + + String src = Files.readString(SET_CYCLE_MESSAGE); + String authorityGate = "RecruitCommandAuthority.canDirectlyControl(player, leader)"; + String handlerMutation = "leader.setCycle(this.cycle)"; + + int gateIndex = src.indexOf(authorityGate); + int mutationIndex = src.indexOf(handlerMutation); + + assertTrue(gateIndex >= 0, "Cycle handler must use the canonical recruit authority gate"); + assertTrue(mutationIndex >= 0, "Cycle handler must still mutate cycle for authorized leaders"); + assertTrue(gateIndex < mutationIndex, + "Forged packet for a foreign leader must fail authority before cycle state can change"); + assertEquals(mutationIndex, src.lastIndexOf(handlerMutation), + "The guarded handler path must be the only cycle mutation entry point"); + } } From b1539d8058d2de240bfa4a7e9da7c80af4ff9e43 Mon Sep 17 00:00:00 2001 From: "pozdn.r.a" Date: Fri, 8 May 2026 00:50:45 +0700 Subject: [PATCH 23/23] backlog: close patrol remove and cycle auth tasks --- docs/BANNERMOD_BACKLOG.json | 28 ++++++++++++++++++++-------- 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/docs/BANNERMOD_BACKLOG.json b/docs/BANNERMOD_BACKLOG.json index 7f69e30c..1de2c31b 100644 --- a/docs/BANNERMOD_BACKLOG.json +++ b/docs/BANNERMOD_BACKLOG.json @@ -8711,8 +8711,8 @@ { "id": "PACKETAUTH-010", "title": "Verify leader ownership in MessagePatrolLeaderRemoveWayPoint", - "status": "open", - "updated": "2026-05-06", + "status": "done", + "updated": "2026-05-08", "why": "MessagePatrolLeaderRemoveWayPoint resolves an AbstractLeaderEntity by UUID and only verifies a range AABB / distance before calling leader.removeLastWayPoint(...). No ownership check; any client in range can mutate a foreign player's leader entity (waypoints, cycle, speed, info mode, etc.).", "scope": [ "src/main/java/com/talhanation/bannermod/network/messages/military/MessagePatrolLeaderRemoveWayPoint.java executeServerSide; add canDirectlyControl(sender, leader) (or equivalent owner-equality check via leader.getOwnerUUID()) before the mutation." @@ -8722,14 +8722,20 @@ ], "dependencies": [], "progress": [], - "verification": [], - "evidence": [] + "verification": [ + { + "date": "2026-05-08", + "result": "1) MessagePatrolLeaderRemoveWayPoint now requires RecruitCommandAuthority.canDirectlyControl(player, leader) before removeLastWayPoint mutation. 2) PatrolLeaderWaypointAuthorityTest confirms foreign non-op has no command role and the authority gate occurs before the only remove-waypoint mutation path. 3) Focused PatrolLeaderWaypointAuthorityTest, ./gradlew compileJava, ./gradlew test, and ./gradlew compileGametestJava passed on the integrated branch. 4) tools/backlog validate passed." + } + ], + "evidence": [], + "doneDate": "2026-05-08" }, { "id": "PACKETAUTH-011", "title": "Verify leader ownership in MessagePatrolLeaderSetCycle", - "status": "open", - "updated": "2026-05-06", + "status": "done", + "updated": "2026-05-08", "why": "MessagePatrolLeaderSetCycle resolves an AbstractLeaderEntity by UUID and only verifies a range AABB / distance before calling leader.setCycle(...). No ownership check; any client in range can mutate a foreign player's leader entity (waypoints, cycle, speed, info mode, etc.).", "scope": [ "src/main/java/com/talhanation/bannermod/network/messages/military/MessagePatrolLeaderSetCycle.java executeServerSide; add canDirectlyControl(sender, leader) (or equivalent owner-equality check via leader.getOwnerUUID()) before the mutation." @@ -8739,8 +8745,14 @@ ], "dependencies": [], "progress": [], - "verification": [], - "evidence": [] + "verification": [ + { + "date": "2026-05-08", + "result": "1) MessagePatrolLeaderSetCycle now requires RecruitCommandAuthority.canDirectlyControl(player, leader) before setCycle mutation. 2) PatrolLeaderWaypointAuthorityTest confirms foreign non-op has no command role and the authority gate occurs before the only cycle mutation path. 3) Focused PatrolLeaderWaypointAuthorityTest, ./gradlew compileJava, ./gradlew test, and ./gradlew compileGametestJava passed on the integrated branch. 4) tools/backlog validate passed." + } + ], + "evidence": [], + "doneDate": "2026-05-08" }, { "id": "PACKETAUTH-012",